| Message ID | 20181217155644.29278-6-marek.vasut@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Eduardo Valentin |
| Headers | show |
| Series | thermal: Align devm_thermal_zone_{device,of_sensor}_register | expand |
On Mon, Dec 17, 2018 at 04:56:43PM +0100, marek.vasut@gmail.com wrote: > From: Marek Vasut <marek.vasut@gmail.com> > > Convert the rcar code to devm_thermal_zone_of_sensor_register_params(), > no functional change. > > From: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org> > Cc: Eduardo Valentin <edubezval@gmail.com> > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> > Cc: Zhang Rui <rui.zhang@intel.com> > Cc: linux-renesas-soc@vger.kernel.org > To: linux-pm@vger.kernel.org > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > --- > V2: No change > V3: - Work around the From line and SoB line checkpatch warning > - Reorder the SoB line at the end As per v2: This patch looks good to me, though I'm not sure why { } need to be introduced into the 4th hunk. That notwithstanding: Reviewed-by: Simon Horman <horms+renesas@verge.net.au> > --- > drivers/thermal/rcar_thermal.c | 33 +++++++++++++-------------------- > 1 file changed, 13 insertions(+), 20 deletions(-) > > diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c > index 8014a207d8d9..ec70c2119c77 100644 > --- a/drivers/thermal/rcar_thermal.c > +++ b/drivers/thermal/rcar_thermal.c > @@ -19,8 +19,6 @@ > #include <linux/spinlock.h> > #include <linux/thermal.h> > > -#include "thermal_hwmon.h" > - > #define IDLE_INTERVAL 5000 > > #define COMMON_STR 0x00 > @@ -54,6 +52,10 @@ struct rcar_thermal_chip { > unsigned int nirqs; > }; > > +static struct thermal_zone_params rcar_thermal_params = { > + .no_hwmon = false, > +}; > + > static const struct rcar_thermal_chip rcar_thermal = { > .use_of_thermal = 0, > .has_filonoff = 1, > @@ -458,9 +460,7 @@ static int rcar_thermal_remove(struct platform_device *pdev) > rcar_thermal_for_each_priv(priv, common) { > rcar_thermal_irq_disable(priv); > cancel_delayed_work_sync(&priv->work); > - if (priv->chip->use_of_thermal) > - thermal_remove_hwmon_sysfs(priv->zone); > - else > + if (!priv->chip->use_of_thermal) > thermal_zone_device_unregister(priv->zone); > } > > @@ -554,16 +554,20 @@ static int rcar_thermal_probe(struct platform_device *pdev) > if (ret < 0) > goto error_unregister; > > - if (chip->use_of_thermal) > - priv->zone = devm_thermal_zone_of_sensor_register( > + if (chip->use_of_thermal) { > + priv->zone = > + devm_thermal_zone_of_sensor_register_params( > dev, i, priv, > - &rcar_thermal_zone_of_ops); > - else > + &rcar_thermal_zone_of_ops, > + &rcar_thermal_params); > + } else { > priv->zone = thermal_zone_device_register( > "rcar_thermal", > 1, 0, priv, > &rcar_thermal_zone_ops, NULL, 0, > idle); > + } > + > if (IS_ERR(priv->zone)) { > dev_err(dev, "can't register thermal zone\n"); > ret = PTR_ERR(priv->zone); > @@ -571,17 +575,6 @@ static int rcar_thermal_probe(struct platform_device *pdev) > goto error_unregister; > } > > - if (chip->use_of_thermal) { > - /* > - * thermal_zone doesn't enable hwmon as default, > - * but, enable it here to keep compatible > - */ > - priv->zone->tzp->no_hwmon = false; > - ret = thermal_add_hwmon_sysfs(priv->zone); > - if (ret) > - goto error_unregister; > - } > - > rcar_thermal_irq_enable(priv); > > list_move_tail(&priv->list, &common->head); > -- > 2.18.0 >
On 12/18/2018 12:05 PM, Simon Horman wrote: > On Mon, Dec 17, 2018 at 04:56:43PM +0100, marek.vasut@gmail.com wrote: >> From: Marek Vasut <marek.vasut@gmail.com> >> >> Convert the rcar code to devm_thermal_zone_of_sensor_register_params(), >> no functional change. >> >> From: Marek Vasut <marek.vasut+renesas@gmail.com> >> Cc: Daniel Lezcano <daniel.lezcano@linaro.org> >> Cc: Eduardo Valentin <edubezval@gmail.com> >> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> >> Cc: Zhang Rui <rui.zhang@intel.com> >> Cc: linux-renesas-soc@vger.kernel.org >> To: linux-pm@vger.kernel.org >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >> --- >> V2: No change >> V3: - Work around the From line and SoB line checkpatch warning >> - Reorder the SoB line at the end > > As per v2: > > This patch looks good to me, though I'm not sure why { } need > to be introduced into the 4th hunk. Because it's a multi-line code , even though it's just a single line-wrapped function call. I can drop that part, but I think it makes it visually far more obvious where the conditional block starts/ends and I recall seeing something about this in kernel coding style too. [...] >> @@ -554,16 +554,20 @@ static int rcar_thermal_probe(struct platform_device *pdev) >> if (ret < 0) >> goto error_unregister; >> >> - if (chip->use_of_thermal) >> - priv->zone = devm_thermal_zone_of_sensor_register( >> + if (chip->use_of_thermal) { >> + priv->zone = >> + devm_thermal_zone_of_sensor_register_params( >> dev, i, priv, >> - &rcar_thermal_zone_of_ops); >> - else >> + &rcar_thermal_zone_of_ops, >> + &rcar_thermal_params); >> + } else { >> priv->zone = thermal_zone_device_register( >> "rcar_thermal", >> 1, 0, priv, >> &rcar_thermal_zone_ops, NULL, 0, >> idle); >> + } >> + >> if (IS_ERR(priv->zone)) { >> dev_err(dev, "can't register thermal zone\n"); >> ret = PTR_ERR(priv->zone); [...]
On Thu, Dec 20, 2018 at 12:25:17AM +0100, Marek Vasut wrote: > On 12/18/2018 12:05 PM, Simon Horman wrote: > > On Mon, Dec 17, 2018 at 04:56:43PM +0100, marek.vasut@gmail.com wrote: > >> From: Marek Vasut <marek.vasut@gmail.com> > >> > >> Convert the rcar code to devm_thermal_zone_of_sensor_register_params(), > >> no functional change. > >> > >> From: Marek Vasut <marek.vasut+renesas@gmail.com> > >> Cc: Daniel Lezcano <daniel.lezcano@linaro.org> > >> Cc: Eduardo Valentin <edubezval@gmail.com> > >> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> > >> Cc: Zhang Rui <rui.zhang@intel.com> > >> Cc: linux-renesas-soc@vger.kernel.org > >> To: linux-pm@vger.kernel.org > >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > >> --- > >> V2: No change > >> V3: - Work around the From line and SoB line checkpatch warning > >> - Reorder the SoB line at the end > > > > As per v2: > > > > This patch looks good to me, though I'm not sure why { } need > > to be introduced into the 4th hunk. > > Because it's a multi-line code , even though it's just a single > line-wrapped function call. I can drop that part, but I think it makes > it visually far more obvious where the conditional block starts/ends and > I recall seeing something about this in kernel coding style too. I lean towards removing {} but I do not feel at all strongly about this. > > [...] > > >> @@ -554,16 +554,20 @@ static int rcar_thermal_probe(struct platform_device *pdev) > >> if (ret < 0) > >> goto error_unregister; > >> > >> - if (chip->use_of_thermal) > >> - priv->zone = devm_thermal_zone_of_sensor_register( > >> + if (chip->use_of_thermal) { > >> + priv->zone = > >> + devm_thermal_zone_of_sensor_register_params( > >> dev, i, priv, > >> - &rcar_thermal_zone_of_ops); > >> - else > >> + &rcar_thermal_zone_of_ops, > >> + &rcar_thermal_params); > >> + } else { > >> priv->zone = thermal_zone_device_register( > >> "rcar_thermal", > >> 1, 0, priv, > >> &rcar_thermal_zone_ops, NULL, 0, > >> idle); > >> + } > >> + > >> if (IS_ERR(priv->zone)) { > >> dev_err(dev, "can't register thermal zone\n"); > >> ret = PTR_ERR(priv->zone); > [...] > > -- > Best regards, > Marek Vasut >
On 12/20/2018 08:46 AM, Simon Horman wrote: > On Thu, Dec 20, 2018 at 12:25:17AM +0100, Marek Vasut wrote: >> On 12/18/2018 12:05 PM, Simon Horman wrote: >>> On Mon, Dec 17, 2018 at 04:56:43PM +0100, marek.vasut@gmail.com wrote: >>>> From: Marek Vasut <marek.vasut@gmail.com> >>>> >>>> Convert the rcar code to devm_thermal_zone_of_sensor_register_params(), >>>> no functional change. >>>> >>>> From: Marek Vasut <marek.vasut+renesas@gmail.com> >>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org> >>>> Cc: Eduardo Valentin <edubezval@gmail.com> >>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> >>>> Cc: Zhang Rui <rui.zhang@intel.com> >>>> Cc: linux-renesas-soc@vger.kernel.org >>>> To: linux-pm@vger.kernel.org >>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >>>> --- >>>> V2: No change >>>> V3: - Work around the From line and SoB line checkpatch warning >>>> - Reorder the SoB line at the end >>> >>> As per v2: >>> >>> This patch looks good to me, though I'm not sure why { } need >>> to be introduced into the 4th hunk. >> >> Because it's a multi-line code , even though it's just a single >> line-wrapped function call. I can drop that part, but I think it makes >> it visually far more obvious where the conditional block starts/ends and >> I recall seeing something about this in kernel coding style too. > > I lean towards removing {} but I do not feel at all strongly about this. Well does it improve the readability if they are removed ? >> [...] >> >>>> @@ -554,16 +554,20 @@ static int rcar_thermal_probe(struct platform_device *pdev) >>>> if (ret < 0) >>>> goto error_unregister; >>>> >>>> - if (chip->use_of_thermal) >>>> - priv->zone = devm_thermal_zone_of_sensor_register( >>>> + if (chip->use_of_thermal) { >>>> + priv->zone = >>>> + devm_thermal_zone_of_sensor_register_params( >>>> dev, i, priv, >>>> - &rcar_thermal_zone_of_ops); >>>> - else >>>> + &rcar_thermal_zone_of_ops, >>>> + &rcar_thermal_params); >>>> + } else { >>>> priv->zone = thermal_zone_device_register( >>>> "rcar_thermal", >>>> 1, 0, priv, >>>> &rcar_thermal_zone_ops, NULL, 0, >>>> idle); >>>> + } >>>> + >>>> if (IS_ERR(priv->zone)) { >>>> dev_err(dev, "can't register thermal zone\n"); >>>> ret = PTR_ERR(priv->zone); >> [...] >> >> -- >> Best regards, >> Marek Vasut >>
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c index 8014a207d8d9..ec70c2119c77 100644 --- a/drivers/thermal/rcar_thermal.c +++ b/drivers/thermal/rcar_thermal.c @@ -19,8 +19,6 @@ #include <linux/spinlock.h> #include <linux/thermal.h> -#include "thermal_hwmon.h" - #define IDLE_INTERVAL 5000 #define COMMON_STR 0x00 @@ -54,6 +52,10 @@ struct rcar_thermal_chip { unsigned int nirqs; }; +static struct thermal_zone_params rcar_thermal_params = { + .no_hwmon = false, +}; + static const struct rcar_thermal_chip rcar_thermal = { .use_of_thermal = 0, .has_filonoff = 1, @@ -458,9 +460,7 @@ static int rcar_thermal_remove(struct platform_device *pdev) rcar_thermal_for_each_priv(priv, common) { rcar_thermal_irq_disable(priv); cancel_delayed_work_sync(&priv->work); - if (priv->chip->use_of_thermal) - thermal_remove_hwmon_sysfs(priv->zone); - else + if (!priv->chip->use_of_thermal) thermal_zone_device_unregister(priv->zone); } @@ -554,16 +554,20 @@ static int rcar_thermal_probe(struct platform_device *pdev) if (ret < 0) goto error_unregister; - if (chip->use_of_thermal) - priv->zone = devm_thermal_zone_of_sensor_register( + if (chip->use_of_thermal) { + priv->zone = + devm_thermal_zone_of_sensor_register_params( dev, i, priv, - &rcar_thermal_zone_of_ops); - else + &rcar_thermal_zone_of_ops, + &rcar_thermal_params); + } else { priv->zone = thermal_zone_device_register( "rcar_thermal", 1, 0, priv, &rcar_thermal_zone_ops, NULL, 0, idle); + } + if (IS_ERR(priv->zone)) { dev_err(dev, "can't register thermal zone\n"); ret = PTR_ERR(priv->zone); @@ -571,17 +575,6 @@ static int rcar_thermal_probe(struct platform_device *pdev) goto error_unregister; } - if (chip->use_of_thermal) { - /* - * thermal_zone doesn't enable hwmon as default, - * but, enable it here to keep compatible - */ - priv->zone->tzp->no_hwmon = false; - ret = thermal_add_hwmon_sysfs(priv->zone); - if (ret) - goto error_unregister; - } - rcar_thermal_irq_enable(priv); list_move_tail(&priv->list, &common->head);