* [PATCH] hwmon: tmp103: use SIMPLE_DEV_PM_OPS helper macro @ 2017-04-23 14:10 Rahul Bedarkar 2017-04-23 15:43 ` Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: Rahul Bedarkar @ 2017-04-23 14:10 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck; +Cc: linux-hwmon, linux-kernel, Rahul Bedarkar Replace ifdefs with SIMPLE_DEV_PM_OPS helper macro. Signed-off-by: Rahul Bedarkar <rahulbedarkar89@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] hwmon: tmp103: use SIMPLE_DEV_PM_OPS helper macro 2017-04-23 14:10 [PATCH] hwmon: tmp103: use SIMPLE_DEV_PM_OPS helper macro Rahul Bedarkar @ 2017-04-23 15:43 ` Guenter Roeck 2017-04-24 3:58 ` Heiko Schocher 0 siblings, 1 reply; 6+ messages in thread From: Guenter Roeck @ 2017-04-23 15:43 UTC (permalink / raw) To: Rahul Bedarkar, Jean Delvare; +Cc: linux-hwmon, linux-kernel, Heiko Schocher 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 <rahulbedarkar89@gmail.com> 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, > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hwmon: tmp103: use SIMPLE_DEV_PM_OPS helper macro 2017-04-23 15:43 ` Guenter Roeck @ 2017-04-24 3:58 ` Heiko Schocher 2017-04-24 6:49 ` Rahul Bedarkar 2017-04-24 15:45 ` Guenter Roeck 0 siblings, 2 replies; 6+ messages in thread From: Heiko Schocher @ 2017-04-24 3:58 UTC (permalink / raw) To: Guenter Roeck; +Cc: Rahul Bedarkar, Jean Delvare, linux-hwmon, linux-kernel 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 <rahulbedarkar89@gmail.com> > > 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 <hs@denx.de> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hwmon: tmp103: use SIMPLE_DEV_PM_OPS helper macro 2017-04-24 3:58 ` Heiko Schocher @ 2017-04-24 6:49 ` Rahul Bedarkar 2017-04-24 15:44 ` Guenter Roeck 2017-04-24 15:45 ` Guenter Roeck 1 sibling, 1 reply; 6+ messages in thread From: Rahul Bedarkar @ 2017-04-24 6:49 UTC (permalink / raw) To: hs, Guenter Roeck; +Cc: Jean Delvare, linux-hwmon, open list Hello Guenter, Heiko, On Mon, Apr 24, 2017 at 9:28 AM, Heiko Schocher <hs@denx.de> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hwmon: tmp103: use SIMPLE_DEV_PM_OPS helper macro 2017-04-24 6:49 ` Rahul Bedarkar @ 2017-04-24 15:44 ` Guenter Roeck 0 siblings, 0 replies; 6+ messages in thread From: Guenter Roeck @ 2017-04-24 15:44 UTC (permalink / raw) To: Rahul Bedarkar; +Cc: hs, Jean Delvare, linux-hwmon, open list 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 <hs@denx.de> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hwmon: tmp103: use SIMPLE_DEV_PM_OPS helper macro 2017-04-24 3:58 ` Heiko Schocher 2017-04-24 6:49 ` Rahul Bedarkar @ 2017-04-24 15:45 ` Guenter Roeck 1 sibling, 0 replies; 6+ messages in thread From: Guenter Roeck @ 2017-04-24 15:45 UTC (permalink / raw) To: Heiko Schocher; +Cc: Rahul Bedarkar, Jean Delvare, linux-hwmon, linux-kernel 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 <rahulbedarkar89@gmail.com> > > > >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 <hs@denx.de> > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-24 15:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-23 14:10 [PATCH] hwmon: tmp103: use SIMPLE_DEV_PM_OPS helper macro Rahul Bedarkar 2017-04-23 15:43 ` Guenter Roeck 2017-04-24 3:58 ` Heiko Schocher 2017-04-24 6:49 ` Rahul Bedarkar 2017-04-24 15:44 ` Guenter Roeck 2017-04-24 15:45 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox