Replace ifdefs with SIMPLE_DEV_PM_OPS helper macro.
Signed-off-by: Rahul Bedarkar <[email protected]>
---
drivers/hwmon/tmp103.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/hwmon/tmp103.c b/drivers/hwmon/tmp103.c
index d0bb28b..7f85b14 100644
--- a/drivers/hwmon/tmp103.c
+++ b/drivers/hwmon/tmp103.c
@@ -150,8 +150,7 @@ static int tmp103_probe(struct i2c_client *client,
return PTR_ERR_OR_ZERO(hwmon_dev);
}
-#ifdef CONFIG_PM
-static int tmp103_suspend(struct device *dev)
+static int __maybe_unused tmp103_suspend(struct device *dev)
{
struct regmap *regmap = dev_get_drvdata(dev);
@@ -159,7 +158,7 @@ static int tmp103_suspend(struct device *dev)
TMP103_CONF_SD_MASK, 0);
}
-static int tmp103_resume(struct device *dev)
+static int __maybe_unused tmp103_resume(struct device *dev)
{
struct regmap *regmap = dev_get_drvdata(dev);
@@ -167,15 +166,7 @@ static int tmp103_resume(struct device *dev)
TMP103_CONF_SD_MASK, TMP103_CONF_SD);
}
-static const struct dev_pm_ops tmp103_dev_pm_ops = {
- .suspend = tmp103_suspend,
- .resume = tmp103_resume,
-};
-
-#define TMP103_DEV_PM_OPS (&tmp103_dev_pm_ops)
-#else
-#define TMP103_DEV_PM_OPS NULL
-#endif /* CONFIG_PM */
+static SIMPLE_DEV_PM_OPS(tmp103_dev_pm_ops, tmp103_suspend, tmp103_resume);
static const struct i2c_device_id tmp103_id[] = {
{ "tmp103", 0 },
@@ -193,7 +184,7 @@ static struct i2c_driver tmp103_driver = {
.driver = {
.name = "tmp103",
.of_match_table = of_match_ptr(tmp103_of_match),
- .pm = TMP103_DEV_PM_OPS,
+ .pm = &tmp103_dev_pm_ops,
},
.probe = tmp103_probe,
.id_table = tmp103_id,
--
2.7.4
Hi Rahul,
On 04/23/2017 07:10 AM, Rahul Bedarkar wrote:
> Replace ifdefs with SIMPLE_DEV_PM_OPS helper macro.
>
> Signed-off-by: Rahul Bedarkar <[email protected]>
Thanks a lot for your patch.
While I in general prefer code that avoids #ifdef, I have seen patches
which do the opposite when handling PM code. It appears that it is
unsettled if __maybe_unused or #ifdef should be used in such situations.
Until that is the case, I won't accept patches changing one into another
unless they are from the driver author or Acked by the driver author.
Thanks,
Guenter
> ---
> drivers/hwmon/tmp103.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hwmon/tmp103.c b/drivers/hwmon/tmp103.c
> index d0bb28b..7f85b14 100644
> --- a/drivers/hwmon/tmp103.c
> +++ b/drivers/hwmon/tmp103.c
> @@ -150,8 +150,7 @@ static int tmp103_probe(struct i2c_client *client,
> return PTR_ERR_OR_ZERO(hwmon_dev);
> }
>
> -#ifdef CONFIG_PM
> -static int tmp103_suspend(struct device *dev)
> +static int __maybe_unused tmp103_suspend(struct device *dev)
> {
> struct regmap *regmap = dev_get_drvdata(dev);
>
> @@ -159,7 +158,7 @@ static int tmp103_suspend(struct device *dev)
> TMP103_CONF_SD_MASK, 0);
> }
>
> -static int tmp103_resume(struct device *dev)
> +static int __maybe_unused tmp103_resume(struct device *dev)
> {
> struct regmap *regmap = dev_get_drvdata(dev);
>
> @@ -167,15 +166,7 @@ static int tmp103_resume(struct device *dev)
> TMP103_CONF_SD_MASK, TMP103_CONF_SD);
> }
>
> -static const struct dev_pm_ops tmp103_dev_pm_ops = {
> - .suspend = tmp103_suspend,
> - .resume = tmp103_resume,
> -};
> -
> -#define TMP103_DEV_PM_OPS (&tmp103_dev_pm_ops)
> -#else
> -#define TMP103_DEV_PM_OPS NULL
> -#endif /* CONFIG_PM */
> +static SIMPLE_DEV_PM_OPS(tmp103_dev_pm_ops, tmp103_suspend, tmp103_resume);
>
> static const struct i2c_device_id tmp103_id[] = {
> { "tmp103", 0 },
> @@ -193,7 +184,7 @@ static struct i2c_driver tmp103_driver = {
> .driver = {
> .name = "tmp103",
> .of_match_table = of_match_ptr(tmp103_of_match),
> - .pm = TMP103_DEV_PM_OPS,
> + .pm = &tmp103_dev_pm_ops,
> },
> .probe = tmp103_probe,
> .id_table = tmp103_id,
>
Hello Guenter, Rahul,
Am 23.04.2017 um 17:43 schrieb Guenter Roeck:
> Hi Rahul,
>
> On 04/23/2017 07:10 AM, Rahul Bedarkar wrote:
>> Replace ifdefs with SIMPLE_DEV_PM_OPS helper macro.
>>
>> Signed-off-by: Rahul Bedarkar <[email protected]>
>
> Thanks a lot for your patch.
>
> While I in general prefer code that avoids #ifdef, I have seen patches
> which do the opposite when handling PM code. It appears that it is
> unsettled if __maybe_unused or #ifdef should be used in such situations.
> Until that is the case, I won't accept patches changing one into another
> unless they are from the driver author or Acked by the driver author.
Hmm.. I like this patch too, but have also no idea, what is preffered.
Looking into drivers/hwmon
pollux:linux hs [master] $ grep -lr __maybe_unused drivers/hwmon/
drivers/hwmon/tmp108.c
drivers/hwmon/nct6775.c
drivers/hwmon/hwmon-vid.c
drivers/hwmon/max31722.c
Ok, there are hwmon drivers, which use this version already ...
pollux:linux hs [master] $ grep -lr CONFIG_PM drivers/hwmon/
drivers/hwmon/max6639.c
drivers/hwmon/jc42.c
drivers/hwmon/fam15h_power.c
drivers/hwmon/tmp102.c
drivers/hwmon/gpio-fan.c
drivers/hwmon/pwm-fan.c
drivers/hwmon/tmp103.c
drivers/hwmon/pmbus/Makefile
drivers/hwmon/lm75.c
drivers/hwmon/nct6683.c
drivers/hwmon/adt7x10.h
drivers/hwmon/w83627hf.c
drivers/hwmon/abituguru3.c
drivers/hwmon/Makefile
drivers/hwmon/acpi_power_meter.c
drivers/hwmon/adt7x10.c
drivers/hwmon/abituguru.c
drivers/hwmon/w83627ehf.c
May this conversion should be done in a patch, which touches more
devices?
Nevertheless, I like this approach, so:
Acked-by: Heiko Schocher <[email protected]>
bye,
Heiko
>
> Thanks,
> Guenter
>
>> ---
>> drivers/hwmon/tmp103.c | 17 ++++-------------
>> 1 file changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/hwmon/tmp103.c b/drivers/hwmon/tmp103.c
>> index d0bb28b..7f85b14 100644
>> --- a/drivers/hwmon/tmp103.c
>> +++ b/drivers/hwmon/tmp103.c
>> @@ -150,8 +150,7 @@ static int tmp103_probe(struct i2c_client *client,
>> return PTR_ERR_OR_ZERO(hwmon_dev);
>> }
>>
>> -#ifdef CONFIG_PM
>> -static int tmp103_suspend(struct device *dev)
>> +static int __maybe_unused tmp103_suspend(struct device *dev)
>> {
>> struct regmap *regmap = dev_get_drvdata(dev);
>>
>> @@ -159,7 +158,7 @@ static int tmp103_suspend(struct device *dev)
>> TMP103_CONF_SD_MASK, 0);
>> }
>>
>> -static int tmp103_resume(struct device *dev)
>> +static int __maybe_unused tmp103_resume(struct device *dev)
>> {
>> struct regmap *regmap = dev_get_drvdata(dev);
>>
>> @@ -167,15 +166,7 @@ static int tmp103_resume(struct device *dev)
>> TMP103_CONF_SD_MASK, TMP103_CONF_SD);
>> }
>>
>> -static const struct dev_pm_ops tmp103_dev_pm_ops = {
>> - .suspend = tmp103_suspend,
>> - .resume = tmp103_resume,
>> -};
>> -
>> -#define TMP103_DEV_PM_OPS (&tmp103_dev_pm_ops)
>> -#else
>> -#define TMP103_DEV_PM_OPS NULL
>> -#endif /* CONFIG_PM */
>> +static SIMPLE_DEV_PM_OPS(tmp103_dev_pm_ops, tmp103_suspend, tmp103_resume);
>>
>> static const struct i2c_device_id tmp103_id[] = {
>> { "tmp103", 0 },
>> @@ -193,7 +184,7 @@ static struct i2c_driver tmp103_driver = {
>> .driver = {
>> .name = "tmp103",
>> .of_match_table = of_match_ptr(tmp103_of_match),
>> - .pm = TMP103_DEV_PM_OPS,
>> + .pm = &tmp103_dev_pm_ops,
>> },
>> .probe = tmp103_probe,
>> .id_table = tmp103_id,
>>
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Hello Guenter, Heiko,
On Mon, Apr 24, 2017 at 9:28 AM, Heiko Schocher <[email protected]> wrote:
> Hello Guenter, Rahul,
>
> Hmm.. I like this patch too, but have also no idea, what is preffered.
>
> Looking into drivers/hwmon
>
> pollux:linux hs [master] $ grep -lr __maybe_unused drivers/hwmon/
> drivers/hwmon/tmp108.c
> drivers/hwmon/nct6775.c
> drivers/hwmon/hwmon-vid.c
> drivers/hwmon/max31722.c
>
> Ok, there are hwmon drivers, which use this version already ...
Yes, some hwmon drivers already use this approach. Some drivers in
other sub systems also using it from start or moving towards this
approach.
>
> pollux:linux hs [master] $ grep -lr CONFIG_PM drivers/hwmon/
> drivers/hwmon/max6639.c
> drivers/hwmon/jc42.c
> drivers/hwmon/fam15h_power.c
> drivers/hwmon/tmp102.c
> drivers/hwmon/gpio-fan.c
> drivers/hwmon/pwm-fan.c
> drivers/hwmon/tmp103.c
> drivers/hwmon/pmbus/Makefile
> drivers/hwmon/lm75.c
> drivers/hwmon/nct6683.c
> drivers/hwmon/adt7x10.h
> drivers/hwmon/w83627hf.c
> drivers/hwmon/abituguru3.c
> drivers/hwmon/Makefile
> drivers/hwmon/acpi_power_meter.c
> drivers/hwmon/adt7x10.c
> drivers/hwmon/abituguru.c
> drivers/hwmon/w83627ehf.c
>
> May this conversion should be done in a patch, which touches more
> devices?
I'm happy send patches converting remaining drivers once this is
settled or accepted.
Thanks for your reviews.
Regards,
Rahul
On Mon, Apr 24, 2017 at 12:19:18PM +0530, Rahul Bedarkar wrote:
> Hello Guenter, Heiko,
>
> On Mon, Apr 24, 2017 at 9:28 AM, Heiko Schocher <[email protected]> wrote:
> > Hello Guenter, Rahul,
> >
> > Hmm.. I like this patch too, but have also no idea, what is preffered.
> >
> > Looking into drivers/hwmon
> >
> > pollux:linux hs [master] $ grep -lr __maybe_unused drivers/hwmon/
> > drivers/hwmon/tmp108.c
> > drivers/hwmon/nct6775.c
> > drivers/hwmon/hwmon-vid.c
> > drivers/hwmon/max31722.c
> >
> > Ok, there are hwmon drivers, which use this version already ...
>
> Yes, some hwmon drivers already use this approach. Some drivers in
> other sub systems also using it from start or moving towards this
> approach.
>
Yes, but as I mentioned it is unsettled if one or the other approach
is preferred, which makes me a bit wary. I'll be open to accepting
patches for jc42 and nct6883 since I am the author of those drivers.
Thanks,
Guenter
> >
> > pollux:linux hs [master] $ grep -lr CONFIG_PM drivers/hwmon/
> > drivers/hwmon/max6639.c
> > drivers/hwmon/jc42.c
> > drivers/hwmon/fam15h_power.c
> > drivers/hwmon/tmp102.c
> > drivers/hwmon/gpio-fan.c
> > drivers/hwmon/pwm-fan.c
> > drivers/hwmon/tmp103.c
> > drivers/hwmon/pmbus/Makefile
> > drivers/hwmon/lm75.c
> > drivers/hwmon/nct6683.c
> > drivers/hwmon/adt7x10.h
> > drivers/hwmon/w83627hf.c
> > drivers/hwmon/abituguru3.c
> > drivers/hwmon/Makefile
> > drivers/hwmon/acpi_power_meter.c
> > drivers/hwmon/adt7x10.c
> > drivers/hwmon/abituguru.c
> > drivers/hwmon/w83627ehf.c
> >
> > May this conversion should be done in a patch, which touches more
> > devices?
>
> I'm happy send patches converting remaining drivers once this is
> settled or accepted.
>
> Thanks for your reviews.
>
> Regards,
> Rahul
On Mon, Apr 24, 2017 at 05:58:18AM +0200, Heiko Schocher wrote:
> Hello Guenter, Rahul,
>
> Am 23.04.2017 um 17:43 schrieb Guenter Roeck:
> >Hi Rahul,
> >
> >On 04/23/2017 07:10 AM, Rahul Bedarkar wrote:
> >>Replace ifdefs with SIMPLE_DEV_PM_OPS helper macro.
> >>
> >>Signed-off-by: Rahul Bedarkar <[email protected]>
> >
> >Thanks a lot for your patch.
> >
> >While I in general prefer code that avoids #ifdef, I have seen patches
> >which do the opposite when handling PM code. It appears that it is
> >unsettled if __maybe_unused or #ifdef should be used in such situations.
> >Until that is the case, I won't accept patches changing one into another
> >unless they are from the driver author or Acked by the driver author.
>
> Hmm.. I like this patch too, but have also no idea, what is preffered.
>
> Looking into drivers/hwmon
>
> pollux:linux hs [master] $ grep -lr __maybe_unused drivers/hwmon/
> drivers/hwmon/tmp108.c
> drivers/hwmon/nct6775.c
> drivers/hwmon/hwmon-vid.c
> drivers/hwmon/max31722.c
>
> Ok, there are hwmon drivers, which use this version already ...
>
> pollux:linux hs [master] $ grep -lr CONFIG_PM drivers/hwmon/
> drivers/hwmon/max6639.c
> drivers/hwmon/jc42.c
> drivers/hwmon/fam15h_power.c
> drivers/hwmon/tmp102.c
> drivers/hwmon/gpio-fan.c
> drivers/hwmon/pwm-fan.c
> drivers/hwmon/tmp103.c
> drivers/hwmon/pmbus/Makefile
> drivers/hwmon/lm75.c
> drivers/hwmon/nct6683.c
> drivers/hwmon/adt7x10.h
> drivers/hwmon/w83627hf.c
> drivers/hwmon/abituguru3.c
> drivers/hwmon/Makefile
> drivers/hwmon/acpi_power_meter.c
> drivers/hwmon/adt7x10.c
> drivers/hwmon/abituguru.c
> drivers/hwmon/w83627ehf.c
>
> May this conversion should be done in a patch, which touches more
> devices?
>
> Nevertheless, I like this approach, so:
>
> Acked-by: Heiko Schocher <[email protected]>
>
Thanks, applied to hwmon-next.
Guenter
> bye,
> Heiko
>
> >
> >Thanks,
> >Guenter
> >
> >>---
> >> drivers/hwmon/tmp103.c | 17 ++++-------------
> >> 1 file changed, 4 insertions(+), 13 deletions(-)
> >>
> >>diff --git a/drivers/hwmon/tmp103.c b/drivers/hwmon/tmp103.c
> >>index d0bb28b..7f85b14 100644
> >>--- a/drivers/hwmon/tmp103.c
> >>+++ b/drivers/hwmon/tmp103.c
> >>@@ -150,8 +150,7 @@ static int tmp103_probe(struct i2c_client *client,
> >> return PTR_ERR_OR_ZERO(hwmon_dev);
> >> }
> >>
> >>-#ifdef CONFIG_PM
> >>-static int tmp103_suspend(struct device *dev)
> >>+static int __maybe_unused tmp103_suspend(struct device *dev)
> >> {
> >> struct regmap *regmap = dev_get_drvdata(dev);
> >>
> >>@@ -159,7 +158,7 @@ static int tmp103_suspend(struct device *dev)
> >> TMP103_CONF_SD_MASK, 0);
> >> }
> >>
> >>-static int tmp103_resume(struct device *dev)
> >>+static int __maybe_unused tmp103_resume(struct device *dev)
> >> {
> >> struct regmap *regmap = dev_get_drvdata(dev);
> >>
> >>@@ -167,15 +166,7 @@ static int tmp103_resume(struct device *dev)
> >> TMP103_CONF_SD_MASK, TMP103_CONF_SD);
> >> }
> >>
> >>-static const struct dev_pm_ops tmp103_dev_pm_ops = {
> >>- .suspend = tmp103_suspend,
> >>- .resume = tmp103_resume,
> >>-};
> >>-
> >>-#define TMP103_DEV_PM_OPS (&tmp103_dev_pm_ops)
> >>-#else
> >>-#define TMP103_DEV_PM_OPS NULL
> >>-#endif /* CONFIG_PM */
> >>+static SIMPLE_DEV_PM_OPS(tmp103_dev_pm_ops, tmp103_suspend, tmp103_resume);
> >>
> >> static const struct i2c_device_id tmp103_id[] = {
> >> { "tmp103", 0 },
> >>@@ -193,7 +184,7 @@ static struct i2c_driver tmp103_driver = {
> >> .driver = {
> >> .name = "tmp103",
> >> .of_match_table = of_match_ptr(tmp103_of_match),
> >>- .pm = TMP103_DEV_PM_OPS,
> >>+ .pm = &tmp103_dev_pm_ops,
> >> },
> >> .probe = tmp103_probe,
> >> .id_table = tmp103_id,
> >>
> >
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html