| Message ID | 23f24d6d-ae79-4014-b4df-dc19ddb88e3f@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: jc42: Simplify pm ops handling | expand |
On Mon, Nov 13, 2023 at 08:20:49PM +0100, Heiner Kallweit wrote: > Use pm_sleep_ptr/DEFINE_SIMPLE_DEV_PM_OPS to simplify the code. > > Note: Apparently conditional compiling based on CONFIG_PM > wasn't optimal, it should have been CONFIG_PM_SLEEP. Maybe that is apparent to you, but not to me, and I assume it won't be apparent to others either. That warrants a real explanation, not just "apparently". Guenter
On 14.11.2023 14:53, Guenter Roeck wrote: > On Mon, Nov 13, 2023 at 08:20:49PM +0100, Heiner Kallweit wrote: >> Use pm_sleep_ptr/DEFINE_SIMPLE_DEV_PM_OPS to simplify the code. >> >> Note: Apparently conditional compiling based on CONFIG_PM >> wasn't optimal, it should have been CONFIG_PM_SLEEP. > > Maybe that is apparent to you, but not to me, and I assume it won't > be apparent to others either. That warrants a real explanation, > not just "apparently". > See e.g. pm_generic_suspend(), it's NULL if CONFIG_PM_SLEEP isn't defined. Another hint is the following from pm.h: #ifdef CONFIG_PM_SLEEP #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) #else #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) #endif CONFIG_PM_SLEEP implies CONFIG_PM, but we can have a setup where CONFIG_PM is defined but CONFIG_PM_SLEEP is not. In this case jc42_suspend/resume would be defined but not used by the PM core. > Guenter Heiner
diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c index f958e830b..5cd0b8cc6 100644 --- a/drivers/hwmon/jc42.c +++ b/drivers/hwmon/jc42.c @@ -570,8 +570,6 @@ static void jc42_remove(struct i2c_client *client) } } -#ifdef CONFIG_PM - static int jc42_suspend(struct device *dev) { struct jc42_data *data = dev_get_drvdata(dev); @@ -598,15 +596,7 @@ static int jc42_resume(struct device *dev) return regcache_sync(data->regmap); } -static const struct dev_pm_ops jc42_dev_pm_ops = { - .suspend = jc42_suspend, - .resume = jc42_resume, -}; - -#define JC42_DEV_PM_OPS (&jc42_dev_pm_ops) -#else -#define JC42_DEV_PM_OPS NULL -#endif /* CONFIG_PM */ +static DEFINE_SIMPLE_DEV_PM_OPS(jc42_pm_ops, jc42_suspend, jc42_resume); static const struct i2c_device_id jc42_id[] = { { "jc42", 0 }, @@ -626,7 +616,7 @@ static struct i2c_driver jc42_driver = { .class = I2C_CLASS_SPD | I2C_CLASS_HWMON, .driver = { .name = "jc42", - .pm = JC42_DEV_PM_OPS, + .pm = pm_sleep_ptr(&jc42_pm_ops), .of_match_table = of_match_ptr(jc42_of_ids), }, .probe = jc42_probe,
Use pm_sleep_ptr/DEFINE_SIMPLE_DEV_PM_OPS to simplify the code. Note: Apparently conditional compiling based on CONFIG_PM wasn't optimal, it should have been CONFIG_PM_SLEEP. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/hwmon/jc42.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)