[PATCH 0/6] iio: PM macro rework continued.

[PATCH 0/6] iio: PM macro rework continued.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

These are straight forward cases so I've grouped them together.
Aim here is to move to the macros that don't need __maybe_unused markings
and generally simplify the handling of different CONFIG_PM* options.

Jonathan Cameron (6):
  iio: proximity: sx9310: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and
    pm_sleep_ptr()
  iio: proximity: sx9324: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and
    pm_sleep_ptr()
  iio: proximity: sx9360: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and
    pm_sleep_ptr()
  iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops
  iio: accel: bmi088: Use EXPORT_NS_GPL_RUNTIME_DEV_PM_OPS() and
    pm_ptr()
  iio: light: st_uvis25: Use EXPORT_NS_SIMPLE_DEV_PM_OPS()

 drivers/iio/accel/bmi088-accel-core.c | 15 ++++++---------
 drivers/iio/accel/bmi088-accel-spi.c  |  2 +-
 drivers/iio/light/st_uvis25_core.c    |  9 +++------
 drivers/iio/light/st_uvis25_i2c.c     |  2 +-
 drivers/iio/light/st_uvis25_spi.c     |  2 +-
 drivers/iio/proximity/srf04.c         | 10 +++++-----
 drivers/iio/proximity/sx9310.c        |  8 ++++----
 drivers/iio/proximity/sx9324.c        |  8 ++++----
 drivers/iio/proximity/sx9360.c        |  8 ++++----
 9 files changed, 29 insertions(+), 35 deletions(-)

-- 
2.37.1

[PATCH 1/6] iio: proximity: sx9310: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

These new macros avoid the need for marking the callbacks __maybe_unused
whilst ensuring both callbacks and structure may be dropped by the compiler
if CONFIG_PM_SLEEP is not enabled.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/iio/proximity/sx9310.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index ea7318b508ea..0e4747ccd3cf 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -965,7 +965,7 @@ static int sx9310_probe(struct i2c_client *client)
 	return sx_common_probe(client, &sx9310_chip_info, &sx9310_regmap_config);
 }
 
-static int __maybe_unused sx9310_suspend(struct device *dev)
+static int sx9310_suspend(struct device *dev)
 {
 	struct sx_common_data *data = iio_priv(dev_get_drvdata(dev));
 	u8 ctrl0;
@@ -991,7 +991,7 @@ static int __maybe_unused sx9310_suspend(struct device *dev)
 	return ret;
 }
 
-static int __maybe_unused sx9310_resume(struct device *dev)
+static int sx9310_resume(struct device *dev)
 {
 	struct sx_common_data *data = iio_priv(dev_get_drvdata(dev));
 	int ret;
@@ -1013,7 +1013,7 @@ static int __maybe_unused sx9310_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(sx9310_pm_ops, sx9310_suspend, sx9310_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(sx9310_pm_ops, sx9310_suspend, sx9310_resume);
 
 static const struct acpi_device_id sx9310_acpi_match[] = {
 	{ "STH9310", SX9310_WHOAMI_VALUE },
@@ -1041,7 +1041,7 @@ static struct i2c_driver sx9310_driver = {
 		.name	= "sx9310",
 		.acpi_match_table = sx9310_acpi_match,
 		.of_match_table = sx9310_of_match,
-		.pm = &sx9310_pm_ops,
+		.pm = pm_sleep_ptr(&sx9310_pm_ops),
 
 		/*
 		 * Lots of i2c transfers in probe + over 200 ms waiting in
-- 
2.37.1

[PATCH 2/6] iio: proximity: sx9324: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

These new macros avoid the need for marking the callbacks __maybe_unused
whilst ensuring both callbacks and structure may be dropped by the compiler
if CONFIG_PM_SLEEP is not enabled.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/iio/proximity/sx9324.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
index edb5a2ce4e27..977cf17cec52 100644
--- a/drivers/iio/proximity/sx9324.c
+++ b/drivers/iio/proximity/sx9324.c
@@ -1073,7 +1073,7 @@ static int sx9324_probe(struct i2c_client *client)
 	return sx_common_probe(client, &sx9324_chip_info, &sx9324_regmap_config);
 }
 
-static int __maybe_unused sx9324_suspend(struct device *dev)
+static int sx9324_suspend(struct device *dev)
 {
 	struct sx_common_data *data = iio_priv(dev_get_drvdata(dev));
 	unsigned int regval;
@@ -1098,7 +1098,7 @@ static int __maybe_unused sx9324_suspend(struct device *dev)
 	return ret;
 }
 
-static int __maybe_unused sx9324_resume(struct device *dev)
+static int sx9324_resume(struct device *dev)
 {
 	struct sx_common_data *data = iio_priv(dev_get_drvdata(dev));
 	int ret;
@@ -1114,7 +1114,7 @@ static int __maybe_unused sx9324_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(sx9324_pm_ops, sx9324_suspend, sx9324_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(sx9324_pm_ops, sx9324_suspend, sx9324_resume);
 
 static const struct acpi_device_id sx9324_acpi_match[] = {
 	{ "STH9324", SX9324_WHOAMI_VALUE },
@@ -1139,7 +1139,7 @@ static struct i2c_driver sx9324_driver = {
 		.name	= "sx9324",
 		.acpi_match_table = sx9324_acpi_match,
 		.of_match_table = sx9324_of_match,
-		.pm = &sx9324_pm_ops,
+		.pm = pm_sleep_ptr(&sx9324_pm_ops),
 
 		/*
 		 * Lots of i2c transfers in probe + over 200 ms waiting in
-- 
2.37.1

[PATCH 3/6] iio: proximity: sx9360: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

These new macros avoid the need for marking the callbacks __maybe_unused
whilst ensuring both callbacks and structure may be dropped by the compiler
if CONFIG_PM_SLEEP is not enabled.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/iio/proximity/sx9360.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
index d9a12e6be6ca..7fa2213d23ba 100644
--- a/drivers/iio/proximity/sx9360.c
+++ b/drivers/iio/proximity/sx9360.c
@@ -819,7 +819,7 @@ static int sx9360_probe(struct i2c_client *client)
 	return sx_common_probe(client, &sx9360_chip_info, &sx9360_regmap_config);
 }
 
-static int __maybe_unused sx9360_suspend(struct device *dev)
+static int sx9360_suspend(struct device *dev)
 {
 	struct sx_common_data *data = iio_priv(dev_get_drvdata(dev));
 	unsigned int regval;
@@ -844,7 +844,7 @@ static int __maybe_unused sx9360_suspend(struct device *dev)
 	return ret;
 }
 
-static int __maybe_unused sx9360_resume(struct device *dev)
+static int sx9360_resume(struct device *dev)
 {
 	struct sx_common_data *data = iio_priv(dev_get_drvdata(dev));
 	int ret;
@@ -861,7 +861,7 @@ static int __maybe_unused sx9360_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(sx9360_pm_ops, sx9360_suspend, sx9360_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(sx9360_pm_ops, sx9360_suspend, sx9360_resume);
 
 static const struct acpi_device_id sx9360_acpi_match[] = {
 	{ "STH9360", SX9360_WHOAMI_VALUE },
@@ -886,7 +886,7 @@ static struct i2c_driver sx9360_driver = {
 		.name	= "sx9360",
 		.acpi_match_table = sx9360_acpi_match,
 		.of_match_table = sx9360_of_match,
-		.pm = &sx9360_pm_ops,
+		.pm = pm_sleep_ptr(&sx9360_pm_ops),
 
 		/*
 		 * Lots of i2c transfers in probe + over 200 ms waiting in
-- 
2.37.1

[PATCH 4/6] iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

If CONFIG_PM is not set, the pm_ptr() will ensure that the struct
dev_pm_ops and callbacks are removed without the need for __maybe_unused
markings.

In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS() because
that would provide suspend and resume functions without the
checks the driver is doing before calling runtime_pm functions
(whether the necessary GPIO is provided).  It may be possible to
clean that up in future by moving the checks into the callbacks.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Andreas Klinger <ak@it-klinger.de>
---
 drivers/iio/proximity/srf04.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
index 05015351a34a..faf2f806ce80 100644
--- a/drivers/iio/proximity/srf04.c
+++ b/drivers/iio/proximity/srf04.c
@@ -359,7 +359,7 @@ static int srf04_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static int __maybe_unused srf04_pm_runtime_suspend(struct device *dev)
+static int  srf04_pm_runtime_suspend(struct device *dev)
 {
 	struct platform_device *pdev = container_of(dev,
 						struct platform_device, dev);
@@ -371,7 +371,7 @@ static int __maybe_unused srf04_pm_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused srf04_pm_runtime_resume(struct device *dev)
+static int srf04_pm_runtime_resume(struct device *dev)
 {
 	struct platform_device *pdev = container_of(dev,
 						struct platform_device, dev);
@@ -385,8 +385,8 @@ static int __maybe_unused srf04_pm_runtime_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops srf04_pm_ops = {
-	SET_RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
-				srf04_pm_runtime_resume, NULL)
+	RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
+		       srf04_pm_runtime_resume, NULL)
 };
 
 static struct platform_driver srf04_driver = {
@@ -395,7 +395,7 @@ static struct platform_driver srf04_driver = {
 	.driver		= {
 		.name		= "srf04-gpio",
 		.of_match_table	= of_srf04_match,
-		.pm		= &srf04_pm_ops,
+		.pm		= pm_ptr(&srf04_pm_ops),
 	},
 };
 
-- 
2.37.1

[PATCH 5/6] iio: accel: bmi088: Use EXPORT_NS_GPL_RUNTIME_DEV_PM_OPS() and pm_ptr()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

These macros allow the compiler to remove unused pm ops functions without
needing to mark them maybe unused.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
Cc: Mike Looijmans <mike.looijmans@topic.nl>
---
 drivers/iio/accel/bmi088-accel-core.c | 15 ++++++---------
 drivers/iio/accel/bmi088-accel-spi.c  |  2 +-
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/accel/bmi088-accel-core.c b/drivers/iio/accel/bmi088-accel-core.c
index bca4cf98bf4d..84edcc78d796 100644
--- a/drivers/iio/accel/bmi088-accel-core.c
+++ b/drivers/iio/accel/bmi088-accel-core.c
@@ -606,7 +606,7 @@ void bmi088_accel_core_remove(struct device *dev)
 }
 EXPORT_SYMBOL_NS_GPL(bmi088_accel_core_remove, IIO_BMI088);
 
-static int __maybe_unused bmi088_accel_runtime_suspend(struct device *dev)
+static int bmi088_accel_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct bmi088_accel_data *data = iio_priv(indio_dev);
@@ -614,7 +614,7 @@ static int __maybe_unused bmi088_accel_runtime_suspend(struct device *dev)
 	return bmi088_accel_power_down(data);
 }
 
-static int __maybe_unused bmi088_accel_runtime_resume(struct device *dev)
+static int bmi088_accel_runtime_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct bmi088_accel_data *data = iio_priv(indio_dev);
@@ -622,13 +622,10 @@ static int __maybe_unused bmi088_accel_runtime_resume(struct device *dev)
 	return bmi088_accel_power_up(data);
 }
 
-const struct dev_pm_ops bmi088_accel_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
-				pm_runtime_force_resume)
-	SET_RUNTIME_PM_OPS(bmi088_accel_runtime_suspend,
-			   bmi088_accel_runtime_resume, NULL)
-};
-EXPORT_SYMBOL_NS_GPL(bmi088_accel_pm_ops, IIO_BMI088);
+EXPORT_NS_GPL_RUNTIME_DEV_PM_OPS(bmi088_accel_pm_ops,
+				 bmi088_accel_runtime_suspend,
+				 bmi088_accel_runtime_resume, NULL,
+				 IIO_BMI088);
 
 MODULE_AUTHOR("Niek van Agt <niek.van.agt@topicproducts.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/bmi088-accel-spi.c b/drivers/iio/accel/bmi088-accel-spi.c
index 9e2ed3bd5661..ee540edd8412 100644
--- a/drivers/iio/accel/bmi088-accel-spi.c
+++ b/drivers/iio/accel/bmi088-accel-spi.c
@@ -80,7 +80,7 @@ MODULE_DEVICE_TABLE(spi, bmi088_accel_id);
 static struct spi_driver bmi088_accel_driver = {
 	.driver = {
 		.name	= "bmi088_accel_spi",
-		.pm	= &bmi088_accel_pm_ops,
+		.pm	= pm_ptr(&bmi088_accel_pm_ops),
 		.of_match_table = bmi088_of_match,
 	},
 	.probe		= bmi088_accel_probe,
-- 
2.37.1

[PATCH 6/6] iio: light: st_uvis25: Use EXPORT_NS_SIMPLE_DEV_PM_OPS()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Using this new macro removes the need to mark the callbacks
__maybe_unused.  One slightly complexity in this case is that
the export will exist if CONFIG_PM is set, but only be used
if CONFIG_PM_SLEEP is also set. This is harmless.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
---
 drivers/iio/light/st_uvis25_core.c | 9 +++------
 drivers/iio/light/st_uvis25_i2c.c  | 2 +-
 drivers/iio/light/st_uvis25_spi.c  | 2 +-
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/light/st_uvis25_core.c b/drivers/iio/light/st_uvis25_core.c
index 3d4cc1180b6a..c737d3e193ae 100644
--- a/drivers/iio/light/st_uvis25_core.c
+++ b/drivers/iio/light/st_uvis25_core.c
@@ -325,7 +325,7 @@ int st_uvis25_probe(struct device *dev, int irq, struct regmap *regmap)
 }
 EXPORT_SYMBOL_NS(st_uvis25_probe, IIO_UVIS25);
 
-static int __maybe_unused st_uvis25_suspend(struct device *dev)
+static int st_uvis25_suspend(struct device *dev)
 {
 	struct iio_dev *iio_dev = dev_get_drvdata(dev);
 	struct st_uvis25_hw *hw = iio_priv(iio_dev);
@@ -334,7 +334,7 @@ static int __maybe_unused st_uvis25_suspend(struct device *dev)
 				  ST_UVIS25_REG_ODR_MASK, 0);
 }
 
-static int __maybe_unused st_uvis25_resume(struct device *dev)
+static int st_uvis25_resume(struct device *dev)
 {
 	struct iio_dev *iio_dev = dev_get_drvdata(dev);
 	struct st_uvis25_hw *hw = iio_priv(iio_dev);
@@ -346,10 +346,7 @@ static int __maybe_unused st_uvis25_resume(struct device *dev)
 	return 0;
 }
 
-const struct dev_pm_ops st_uvis25_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(st_uvis25_suspend, st_uvis25_resume)
-};
-EXPORT_SYMBOL_NS(st_uvis25_pm_ops, IIO_UVIS25);
+EXPORT_NS_SIMPLE_DEV_PM_OPS(st_uvis25_pm_ops, st_uvis25_suspend, st_uvis25_resume, IIO_UVIS25);
 
 MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>");
 MODULE_DESCRIPTION("STMicroelectronics uvis25 sensor driver");
diff --git a/drivers/iio/light/st_uvis25_i2c.c b/drivers/iio/light/st_uvis25_i2c.c
index b06d09af28a3..c982b0b255cf 100644
--- a/drivers/iio/light/st_uvis25_i2c.c
+++ b/drivers/iio/light/st_uvis25_i2c.c
@@ -55,7 +55,7 @@ MODULE_DEVICE_TABLE(i2c, st_uvis25_i2c_id_table);
 static struct i2c_driver st_uvis25_driver = {
 	.driver = {
 		.name = "st_uvis25_i2c",
-		.pm = &st_uvis25_pm_ops,
+		.pm = pm_sleep_ptr(&st_uvis25_pm_ops),
 		.of_match_table = st_uvis25_i2c_of_match,
 	},
 	.probe = st_uvis25_i2c_probe,
diff --git a/drivers/iio/light/st_uvis25_spi.c b/drivers/iio/light/st_uvis25_spi.c
index 3a4dc6d7180c..86a232320d7d 100644
--- a/drivers/iio/light/st_uvis25_spi.c
+++ b/drivers/iio/light/st_uvis25_spi.c
@@ -55,7 +55,7 @@ MODULE_DEVICE_TABLE(spi, st_uvis25_spi_id_table);
 static struct spi_driver st_uvis25_driver = {
 	.driver = {
 		.name = "st_uvis25_spi",
-		.pm = &st_uvis25_pm_ops,
+		.pm = pm_sleep_ptr(&st_uvis25_pm_ops),
 		.of_match_table = st_uvis25_spi_of_match,
 	},
 	.probe = st_uvis25_spi_probe,
-- 
2.37.1

Re: [PATCH 4/6] iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops

[Andy Shevchenko]

On Sun, Aug 7, 2022 at 8:46 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> If CONFIG_PM is not set, the pm_ptr() will ensure that the struct
> dev_pm_ops and callbacks are removed without the need for __maybe_unused
> markings.
>
> In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS() because
> that would provide suspend and resume functions without the
> checks the driver is doing before calling runtime_pm functions
> (whether the necessary GPIO is provided).  It may be possible to
> clean that up in future by moving the checks into the callbacks.

...

>  static const struct dev_pm_ops srf04_pm_ops = {
> -       SET_RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> -                               srf04_pm_runtime_resume, NULL)
> +       RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> +                      srf04_pm_runtime_resume, NULL)
>  };

static DEFINE_RUNTIME_DEV_PM_OPS(...);

?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/6] iio: PM macro rework continued.

[Andy Shevchenko]

On Sun, Aug 7, 2022 at 8:46 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> These are straight forward cases so I've grouped them together.
> Aim here is to move to the macros that don't need __maybe_unused markings
> and generally simplify the handling of different CONFIG_PM* options.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
for non-commented and, once addressed as suggested, for commented.

> Jonathan Cameron (6):
>   iio: proximity: sx9310: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and
>     pm_sleep_ptr()
>   iio: proximity: sx9324: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and
>     pm_sleep_ptr()
>   iio: proximity: sx9360: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and
>     pm_sleep_ptr()
>   iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops
>   iio: accel: bmi088: Use EXPORT_NS_GPL_RUNTIME_DEV_PM_OPS() and
>     pm_ptr()
>   iio: light: st_uvis25: Use EXPORT_NS_SIMPLE_DEV_PM_OPS()
>
>  drivers/iio/accel/bmi088-accel-core.c | 15 ++++++---------
>  drivers/iio/accel/bmi088-accel-spi.c  |  2 +-
>  drivers/iio/light/st_uvis25_core.c    |  9 +++------
>  drivers/iio/light/st_uvis25_i2c.c     |  2 +-
>  drivers/iio/light/st_uvis25_spi.c     |  2 +-
>  drivers/iio/proximity/srf04.c         | 10 +++++-----
>  drivers/iio/proximity/sx9310.c        |  8 ++++----
>  drivers/iio/proximity/sx9324.c        |  8 ++++----
>  drivers/iio/proximity/sx9360.c        |  8 ++++----
>  9 files changed, 29 insertions(+), 35 deletions(-)
>
> --
> 2.37.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/6] iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops

[Paul Cercueil]

Hi Andy,

Le lun., août 8 2022 at 11:28:12 +0200, Andy Shevchenko 
<andy.shevchenko@gmail.com> a écrit :
> On Sun, Aug 7, 2022 at 8:46 PM Jonathan Cameron <jic23@kernel.org> 
> wrote:
>> 
>>  From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> 
>>  If CONFIG_PM is not set, the pm_ptr() will ensure that the struct
>>  dev_pm_ops and callbacks are removed without the need for 
>> __maybe_unused
>>  markings.
>> 
>>  In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS() because
>>  that would provide suspend and resume functions without the
>>  checks the driver is doing before calling runtime_pm functions
>>  (whether the necessary GPIO is provided).  It may be possible to
>>  clean that up in future by moving the checks into the callbacks.
> 
> ...
> 
>>   static const struct dev_pm_ops srf04_pm_ops = {
>>  -       SET_RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
>>  -                               srf04_pm_runtime_resume, NULL)
>>  +       RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
>>  +                      srf04_pm_runtime_resume, NULL)
>>   };
> 
> static DEFINE_RUNTIME_DEV_PM_OPS(...);
> 
> ?

Read the commit message?

Cheers,
-Paul

Re: [PATCH 4/6] iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops

[Andy Shevchenko]

On Mon, Aug 8, 2022 at 11:35 AM Paul Cercueil <paul@crapouillou.net> wrote:
> Le lun., août 8 2022 at 11:28:12 +0200, Andy Shevchenko
> <andy.shevchenko@gmail.com> a écrit :
> > On Sun, Aug 7, 2022 at 8:46 PM Jonathan Cameron <jic23@kernel.org>
> > wrote:

...

> >>  In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS() because
> >>  that would provide suspend and resume functions without the
> >>  checks the driver is doing before calling runtime_pm functions
> >>  (whether the necessary GPIO is provided).  It may be possible to
> >>  clean that up in future by moving the checks into the callbacks.
> >
> > ...
> >
> >>   static const struct dev_pm_ops srf04_pm_ops = {
> >>  -       SET_RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> >>  -                               srf04_pm_runtime_resume, NULL)
> >>  +       RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> >>  +                      srf04_pm_runtime_resume, NULL)
> >>   };
> >
> > static DEFINE_RUNTIME_DEV_PM_OPS(...);
> >
> > ?
>
> Read the commit message?

Yes, and I'm not sure how that part is relevant. The callbacks won't
be called if pm_ptr() equals no-op, no?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/6] iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops

[Paul Cercueil]

Le lun., août 8 2022 at 11:39:56 +0200, Andy Shevchenko 
<andy.shevchenko@gmail.com> a écrit :
> On Mon, Aug 8, 2022 at 11:35 AM Paul Cercueil <paul@crapouillou.net> 
> wrote:
>>  Le lun., août 8 2022 at 11:28:12 +0200, Andy Shevchenko
>>  <andy.shevchenko@gmail.com> a écrit :
>>  > On Sun, Aug 7, 2022 at 8:46 PM Jonathan Cameron <jic23@kernel.org>
>>  > wrote:
> 
> ...
> 
>>  >>  In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS() 
>> because
>>  >>  that would provide suspend and resume functions without the
>>  >>  checks the driver is doing before calling runtime_pm functions
>>  >>  (whether the necessary GPIO is provided).  It may be possible to
>>  >>  clean that up in future by moving the checks into the callbacks.
>>  >
>>  > ...
>>  >
>>  >>   static const struct dev_pm_ops srf04_pm_ops = {
>>  >>  -       SET_RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
>>  >>  -                               srf04_pm_runtime_resume, NULL)
>>  >>  +       RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
>>  >>  +                      srf04_pm_runtime_resume, NULL)
>>  >>   };
>>  >
>>  > static DEFINE_RUNTIME_DEV_PM_OPS(...);
>>  >
>>  > ?
>> 
>>  Read the commit message?
> 
> Yes, and I'm not sure how that part is relevant. The callbacks won't
> be called if pm_ptr() equals no-op, no?

Have a look at the definition of DEFINE_RUNTIME_DEV_PM_OPS(). I believe 
it does not do what you think it does.

What the commit message says is that using DEFINE_RUNTIME_DEV_PM_OPS() 
would add .suspend/.resume callbacks, which aren't provided with the 
current code.

Cheers,
-Paul

Re: [PATCH 4/6] iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops

[Andy Shevchenko]

On Mon, Aug 8, 2022 at 11:49 AM Paul Cercueil <paul@crapouillou.net> wrote:
>
>
>
> Le lun., août 8 2022 at 11:39:56 +0200, Andy Shevchenko
> <andy.shevchenko@gmail.com> a écrit :
> > On Mon, Aug 8, 2022 at 11:35 AM Paul Cercueil <paul@crapouillou.net>
> > wrote:
> >>  Le lun., août 8 2022 at 11:28:12 +0200, Andy Shevchenko
> >>  <andy.shevchenko@gmail.com> a écrit :
> >>  > On Sun, Aug 7, 2022 at 8:46 PM Jonathan Cameron <jic23@kernel.org>
> >>  > wrote:
> >
> > ...
> >
> >>  >>  In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS()
> >> because
> >>  >>  that would provide suspend and resume functions without the
> >>  >>  checks the driver is doing before calling runtime_pm functions
> >>  >>  (whether the necessary GPIO is provided).  It may be possible to
> >>  >>  clean that up in future by moving the checks into the callbacks.
> >>  >
> >>  > ...
> >>  >
> >>  >>   static const struct dev_pm_ops srf04_pm_ops = {
> >>  >>  -       SET_RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> >>  >>  -                               srf04_pm_runtime_resume, NULL)
> >>  >>  +       RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> >>  >>  +                      srf04_pm_runtime_resume, NULL)
> >>  >>   };
> >>  >
> >>  > static DEFINE_RUNTIME_DEV_PM_OPS(...);
> >>  >
> >>  > ?
> >>
> >>  Read the commit message?
> >
> > Yes, and I'm not sure how that part is relevant. The callbacks won't
> > be called if pm_ptr() equals no-op, no?
>
> Have a look at the definition of DEFINE_RUNTIME_DEV_PM_OPS(). I believe
> it does not do what you think it does.
>
> What the commit message says is that using DEFINE_RUNTIME_DEV_PM_OPS()
> would add .suspend/.resume callbacks, which aren't provided with the
> current code.

Effectively the use of DEFINE_RUNTIME_DEV_PM_OPS() enables system
sleep with the same callbacks as runtime PM. I don't see any
disadvantages of using it instead of keeping runtime PM only. That
said, I don't see the commit message that describes that nuance. It
rather says, as I said, something irrelevant.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/6] iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops

[Paul Cercueil]

Le lun., août 8 2022 at 12:09:35 +0200, Andy Shevchenko 
<andy.shevchenko@gmail.com> a écrit :
> On Mon, Aug 8, 2022 at 11:49 AM Paul Cercueil <paul@crapouillou.net> 
> wrote:
>> 
>> 
>> 
>>  Le lun., août 8 2022 at 11:39:56 +0200, Andy Shevchenko
>>  <andy.shevchenko@gmail.com> a écrit :
>>  > On Mon, Aug 8, 2022 at 11:35 AM Paul Cercueil 
>> <paul@crapouillou.net>
>>  > wrote:
>>  >>  Le lun., août 8 2022 at 11:28:12 +0200, Andy Shevchenko
>>  >>  <andy.shevchenko@gmail.com> a écrit :
>>  >>  > On Sun, Aug 7, 2022 at 8:46 PM Jonathan Cameron 
>> <jic23@kernel.org>
>>  >>  > wrote:
>>  >
>>  > ...
>>  >
>>  >>  >>  In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS()
>>  >> because
>>  >>  >>  that would provide suspend and resume functions without the
>>  >>  >>  checks the driver is doing before calling runtime_pm 
>> functions
>>  >>  >>  (whether the necessary GPIO is provided).  It may be 
>> possible to
>>  >>  >>  clean that up in future by moving the checks into the 
>> callbacks.
>>  >>  >
>>  >>  > ...
>>  >>  >
>>  >>  >>   static const struct dev_pm_ops srf04_pm_ops = {
>>  >>  >>  -       SET_RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
>>  >>  >>  -                               srf04_pm_runtime_resume, 
>> NULL)
>>  >>  >>  +       RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
>>  >>  >>  +                      srf04_pm_runtime_resume, NULL)
>>  >>  >>   };
>>  >>  >
>>  >>  > static DEFINE_RUNTIME_DEV_PM_OPS(...);
>>  >>  >
>>  >>  > ?
>>  >>
>>  >>  Read the commit message?
>>  >
>>  > Yes, and I'm not sure how that part is relevant. The callbacks 
>> won't
>>  > be called if pm_ptr() equals no-op, no?
>> 
>>  Have a look at the definition of DEFINE_RUNTIME_DEV_PM_OPS(). I 
>> believe
>>  it does not do what you think it does.
>> 
>>  What the commit message says is that using 
>> DEFINE_RUNTIME_DEV_PM_OPS()
>>  would add .suspend/.resume callbacks, which aren't provided with the
>>  current code.
> 
> Effectively the use of DEFINE_RUNTIME_DEV_PM_OPS() enables system
> sleep with the same callbacks as runtime PM. I don't see any
> disadvantages of using it instead of keeping runtime PM only. That
> said, I don't see the commit message that describes that nuance. It
> rather says, as I said, something irrelevant.

Probably no disavantages, yes, but that would be a functional change, 
so I can understand why it's not done in this patchset. That doesn't 
mean it cannot be done later, though, but somebody would need to test 
it on hardware.

Cheers,
-Paul

Re: [PATCH 4/6] iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops

[Andy Shevchenko]

On Mon, Aug 8, 2022 at 12:17 PM Paul Cercueil <paul@crapouillou.net> wrote:
> Le lun., août 8 2022 at 12:09:35 +0200, Andy Shevchenko
> <andy.shevchenko@gmail.com> a écrit :
> > On Mon, Aug 8, 2022 at 11:49 AM Paul Cercueil <paul@crapouillou.net>
> > wrote:
> >>  Le lun., août 8 2022 at 11:39:56 +0200, Andy Shevchenko
> >>  <andy.shevchenko@gmail.com> a écrit :
> >>  > On Mon, Aug 8, 2022 at 11:35 AM Paul Cercueil
> >> <paul@crapouillou.net>
> >>  > wrote:
> >>  >>  Le lun., août 8 2022 at 11:28:12 +0200, Andy Shevchenko
> >>  >>  <andy.shevchenko@gmail.com> a écrit :
> >>  >>  > On Sun, Aug 7, 2022 at 8:46 PM Jonathan Cameron
> >> <jic23@kernel.org>
> >>  >>  > wrote:
> >>  >
> >>  > ...
> >>  >
> >>  >>  >>  In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS()
> >>  >> because
> >>  >>  >>  that would provide suspend and resume functions without the
> >>  >>  >>  checks the driver is doing before calling runtime_pm
> >> functions
> >>  >>  >>  (whether the necessary GPIO is provided).  It may be
> >> possible to
> >>  >>  >>  clean that up in future by moving the checks into the
> >> callbacks.
> >>  >>  >
> >>  >>  > ...
> >>  >>  >
> >>  >>  >>   static const struct dev_pm_ops srf04_pm_ops = {
> >>  >>  >>  -       SET_RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> >>  >>  >>  -                               srf04_pm_runtime_resume,
> >> NULL)
> >>  >>  >>  +       RUNTIME_PM_OPS(srf04_pm_runtime_suspend,
> >>  >>  >>  +                      srf04_pm_runtime_resume, NULL)
> >>  >>  >>   };
> >>  >>  >
> >>  >>  > static DEFINE_RUNTIME_DEV_PM_OPS(...);
> >>  >>  >
> >>  >>  > ?
> >>  >>
> >>  >>  Read the commit message?
> >>  >
> >>  > Yes, and I'm not sure how that part is relevant. The callbacks
> >> won't
> >>  > be called if pm_ptr() equals no-op, no?
> >>
> >>  Have a look at the definition of DEFINE_RUNTIME_DEV_PM_OPS(). I
> >> believe
> >>  it does not do what you think it does.
> >>
> >>  What the commit message says is that using
> >> DEFINE_RUNTIME_DEV_PM_OPS()
> >>  would add .suspend/.resume callbacks, which aren't provided with the
> >>  current code.
> >
> > Effectively the use of DEFINE_RUNTIME_DEV_PM_OPS() enables system
> > sleep with the same callbacks as runtime PM. I don't see any
> > disadvantages of using it instead of keeping runtime PM only. That
> > said, I don't see the commit message that describes that nuance. It
> > rather says, as I said, something irrelevant.
>
> Probably no disavantages, yes, but that would be a functional change,
> so I can understand why it's not done in this patchset. That doesn't
> mean it cannot be done later, though, but somebody would need to test
> it on hardware.

This is a fair point.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/6] iio: proximity: sx9310: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()

[Jonathan Cameron]

On Sun,  7 Aug 2022 19:56:13 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> These new macros avoid the need for marking the callbacks __maybe_unused
> whilst ensuring both callbacks and structure may be dropped by the compiler
> if CONFIG_PM_SLEEP is not enabled.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Gwendal Grignou <gwendal@chromium.org>
Applied

> ---
>  drivers/iio/proximity/sx9310.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> index ea7318b508ea..0e4747ccd3cf 100644
> --- a/drivers/iio/proximity/sx9310.c
> +++ b/drivers/iio/proximity/sx9310.c
> @@ -965,7 +965,7 @@ static int sx9310_probe(struct i2c_client *client)
>  	return sx_common_probe(client, &sx9310_chip_info, &sx9310_regmap_config);
>  }
>  
> -static int __maybe_unused sx9310_suspend(struct device *dev)
> +static int sx9310_suspend(struct device *dev)
>  {
>  	struct sx_common_data *data = iio_priv(dev_get_drvdata(dev));
>  	u8 ctrl0;
> @@ -991,7 +991,7 @@ static int __maybe_unused sx9310_suspend(struct device *dev)
>  	return ret;
>  }
>  
> -static int __maybe_unused sx9310_resume(struct device *dev)
> +static int sx9310_resume(struct device *dev)
>  {
>  	struct sx_common_data *data = iio_priv(dev_get_drvdata(dev));
>  	int ret;
> @@ -1013,7 +1013,7 @@ static int __maybe_unused sx9310_resume(struct device *dev)
>  	return 0;
>  }
>  
> -static SIMPLE_DEV_PM_OPS(sx9310_pm_ops, sx9310_suspend, sx9310_resume);
> +static DEFINE_SIMPLE_DEV_PM_OPS(sx9310_pm_ops, sx9310_suspend, sx9310_resume);
>  
>  static const struct acpi_device_id sx9310_acpi_match[] = {
>  	{ "STH9310", SX9310_WHOAMI_VALUE },
> @@ -1041,7 +1041,7 @@ static struct i2c_driver sx9310_driver = {
>  		.name	= "sx9310",
>  		.acpi_match_table = sx9310_acpi_match,
>  		.of_match_table = sx9310_of_match,
> -		.pm = &sx9310_pm_ops,
> +		.pm = pm_sleep_ptr(&sx9310_pm_ops),
>  
>  		/*
>  		 * Lots of i2c transfers in probe + over 200 ms waiting in

Re: [PATCH 2/6] iio: proximity: sx9324: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()

[Jonathan Cameron]

On Sun,  7 Aug 2022 19:56:14 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> These new macros avoid the need for marking the callbacks __maybe_unused
> whilst ensuring both callbacks and structure may be dropped by the compiler
> if CONFIG_PM_SLEEP is not enabled.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Gwendal Grignou <gwendal@chromium.org>
Applied

Re: [PATCH 3/6] iio: proximity: sx9360: Switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()

[Jonathan Cameron]

On Sun,  7 Aug 2022 19:56:15 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> These new macros avoid the need for marking the callbacks __maybe_unused
> whilst ensuring both callbacks and structure may be dropped by the compiler
> if CONFIG_PM_SLEEP is not enabled.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Gwendal Grignou <gwendal@chromium.org>

Applied.

Re: [PATCH 4/6] iio: proximity: srf04: Use pm_ptr() to remove unused struct dev_pm_ops

[Jonathan Cameron]

On Sun,  7 Aug 2022 19:56:16 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> If CONFIG_PM is not set, the pm_ptr() will ensure that the struct
> dev_pm_ops and callbacks are removed without the need for __maybe_unused
> markings.
> 
> In this case we can't simply use DEFINE_RUNTIME_DEV_PM_OPS() because
> that would provide suspend and resume functions without the
> checks the driver is doing before calling runtime_pm functions
> (whether the necessary GPIO is provided).  It may be possible to
> clean that up in future by moving the checks into the callbacks.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Andreas Klinger <ak@it-klinger.de>
Some consensus reached in the discussion so even though no one
gave a tag I feel comfortable taking this one.
The suggested follow up change needs hardware to boost confidence
that there are no side effects.

Applied.

Jonathan

Re: [PATCH 5/6] iio: accel: bmi088: Use EXPORT_NS_GPL_RUNTIME_DEV_PM_OPS() and pm_ptr()

[Jonathan Cameron]

On Sun,  7 Aug 2022 19:56:17 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> These macros allow the compiler to remove unused pm ops functions without
> needing to mark them maybe unused.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> Cc: Mike Looijmans <mike.looijmans@topic.nl>
>
Applied.

Re: [PATCH 6/6] iio: light: st_uvis25: Use EXPORT_NS_SIMPLE_DEV_PM_OPS()

[Jonathan Cameron]

On Sun,  7 Aug 2022 19:56:18 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Using this new macro removes the need to mark the callbacks
> __maybe_unused.  One slightly complexity in this case is that
> the export will exist if CONFIG_PM is set, but only be used
> if CONFIG_PM_SLEEP is also set. This is harmless.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
Applied