Skip to contentSkip to conversation

IIO: Where dev_pm_ops rework and namespaces meet.

PATCH Superseded Jonathan Cameron 5 participants 19 messages Feb 20, 2022 – Apr 1, 2022

Subsystems (7)
lore
Share & tools
lore

Jonathan Cameron

author maintainer
#0 lore raw

cover IIO: Where dev_pm_ops rework and namespaces meet.

From: Jonathan Cameron <[email protected]>

Hi All,

As these two activities have been ongoing across IIO at the same time it was inevitable that there would be drivers were they both interact. In particularly when we have EXPORT* of struct dev_pm_ops.

This series covers those cases by introducing (in patch 2) EXPORT_NS[_GPL]_SIMPLE_DEV_PM_OPS() and EXPORT_NS[_GPL]_RUNTIME_DEV_PM_OPS()
that add a namespace as the final parameter.

In each driver (with the exception of the kxsd9 which was already using a IIO_KXSD9 namespace) I first move to the new *_PM_OPS() and pm_[sleep_]_ptr() then in a second patch move to the new namespaces. Initially I had these two steps done as one patch per driver but that was harder to follow and broke the rule of one thing per patch.

All comments welcome.

Thanks,

Jonathan

Jonathan Cameron (8):
iio: chemical: scd30: Export dev_pm_ops instead of suspend() and
resume()
PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and
runtime pm equiv
iio: chemical: scd30: Move symbol exports into IIO_SCD30 namespace iio:accel:kxsd9: Switch from CONFIG_PM guards to pm_ptr() etc
iio: humidity: hts221: Use EXPORT_SIMPLE_DEV_PM_OPS() to allow
compiler to remove dead code.
iio: humidity: hts221: Move symbol exports into IIO_HTS221 namespace
iio: imu: lsm6dsx: Use new pm_sleep_ptr() and
EXPORT_SIMPLE_DEV_PM_OPS()
iio: imu: lsm6dsx: Move exported symbols to the IIO_LSM6DSX namespace

 drivers/iio/accel/kxsd9-i2c.c                |  2 +-
 drivers/iio/accel/kxsd9-spi.c                |  2 +-
 drivers/iio/accel/kxsd9.c                    | 11 ++---------
 drivers/iio/chemical/scd30.h                 |  5 +----
 drivers/iio/chemical/scd30_core.c            | 10 +++++-----
 drivers/iio/chemical/scd30_i2c.c             |  3 ++-
 drivers/iio/chemical/scd30_serial.c          |  3 ++-
 drivers/iio/humidity/hts221_core.c           | 12 +++++-------
 drivers/iio/humidity/hts221_i2c.c            |  3 ++-
 drivers/iio/humidity/hts221_spi.c            |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 12 +++++-------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c  |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c  |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c  |  3 ++-
 include/linux/pm.h                           | 14 +++++++++-----
 include/linux/pm_runtime.h                   | 10 ++++++++--
 16 files changed, 51 insertions(+), 48 deletions(-)
Signature · 1 line
2.35.1
author maintainer
#1 lore raw

PATCH 1/8 iio: chemical: scd30: Export dev_pm_ops instead of suspend() and resume()

From: Jonathan Cameron <[email protected]>

Whilst here move to the new infrastructure using pm_sleep_ptr() and EXPORT_DEV_PM_OPS() so as to let the compiler remove the unused code if CONFIG_SLEEP is not defined.

---
 drivers/iio/chemical/scd30.h        | 5 +----
 drivers/iio/chemical/scd30_core.c   | 8 ++++----
 drivers/iio/chemical/scd30_i2c.c    | 2 +-
 drivers/iio/chemical/scd30_serial.c | 2 +-
 4 files changed, 7 insertions(+), 10 deletions(-)
drivers/iio/chemical/scd30.h +1 -4
index f60127bfe0f4..1ac9f3f79271 100644
@@ -68,10 +68,7 @@ struct scd30_state {
 	scd30_command_t command;
 };
 
-int scd30_suspend(struct device *dev);
-int scd30_resume(struct device *dev);
-
-static __maybe_unused SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend, scd30_resume);
+extern const struct dev_pm_ops scd30_pm_ops;
 
 int scd30_probe(struct device *dev, int irq, const char *name, void *priv, scd30_command_t command);
 
drivers/iio/chemical/scd30_core.c +4 -4
index 9fe6bbe9ee04..6c6c11c2772a 100644
@@ -517,7 +517,7 @@ static const struct iio_chan_spec scd30_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
-int __maybe_unused scd30_suspend(struct device *dev)
+static int scd30_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct scd30_state *state  = iio_priv(indio_dev);
@@ -529,9 +529,8 @@ int __maybe_unused scd30_suspend(struct device *dev)
 
 	return regulator_disable(state->vdd);
 }
-EXPORT_SYMBOL(scd30_suspend);
 
-int __maybe_unused scd30_resume(struct device *dev)
+static int scd30_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct scd30_state *state = iio_priv(indio_dev);
@@ -543,7 +542,8 @@ int __maybe_unused scd30_resume(struct device *dev)
 
 	return scd30_command_write(state, CMD_START_MEAS, state->pressure_comp);
 }
-EXPORT_SYMBOL(scd30_resume);
+
+EXPORT_SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend, scd30_resume);
 
 static void scd30_stop_meas(void *data)
 {
drivers/iio/chemical/scd30_i2c.c +1 -1
index 875892a070ee..7c332e4e8e46 100644
@@ -128,7 +128,7 @@ static struct i2c_driver scd30_i2c_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.of_match_table = scd30_i2c_of_match,
-		.pm = &scd30_pm_ops,
+		.pm = pm_sleep_ptr(&scd30_pm_ops),
 	},
 	.probe_new = scd30_i2c_probe,
 };
drivers/iio/chemical/scd30_serial.c +1 -1
index 568b34486c44..8664f3ce6b33 100644
@@ -252,7 +252,7 @@ static struct serdev_device_driver scd30_serdev_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.of_match_table = scd30_serdev_of_match,
-		.pm = &scd30_pm_ops,
+		.pm = pm_sleep_ptr(&scd30_pm_ops),
 	},
 	.probe = scd30_serdev_probe,
 };
Signature · 1 line
2.35.1
author maintainer
#2 lore raw

PATCH 2/8 PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and runtime pm equiv

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

As more drivers start to use namespaces, we need to have varients of these useful macros that allow the export to be in a particular namespace.

Cc Paul Cercueil <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
---
 include/linux/pm.h         | 14 +++++++++-----
 include/linux/pm_runtime.h | 10 ++++++++--
 2 files changed, 17 insertions(+), 7 deletions(-)
include/linux/pm.h +9 -5
index f7d2be686359..112b8125d4be 100644
@@ -368,13 +368,13 @@ const struct dev_pm_ops name = { \
 
 #ifdef CONFIG_PM
 #define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
-			   runtime_resume_fn, idle_fn, sec) \
+			   runtime_resume_fn, idle_fn, sec, ns)		\
 	_DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
 			   runtime_resume_fn, idle_fn); \
-	_EXPORT_SYMBOL(name, sec)
+	__EXPORT_SYMBOL(name, sec, ns)
 #else
 #define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
-			   runtime_resume_fn, idle_fn, sec) \
+			   runtime_resume_fn, idle_fn, sec, ns) \
 static __maybe_unused _DEFINE_DEV_PM_OPS(__static_##name, suspend_fn, \
 					 resume_fn, runtime_suspend_fn, \
 					 runtime_resume_fn, idle_fn)
@@ -391,9 +391,13 @@ static __maybe_unused _DEFINE_DEV_PM_OPS(__static_##name, suspend_fn, \
 	_DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL)
 
 #define EXPORT_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "")
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "", "")
 #define EXPORT_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl")
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl", "")
+#define EXPORT_NS_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn, ns)	\
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "", #ns)
+#define EXPORT_NS_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn, ns)	\
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl", #ns)
 
 /* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
 #define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
include/linux/pm_runtime.h +8 -2
index 9f09601c465a..6a8b9551ecad 100644
@@ -41,10 +41,16 @@
 
 #define EXPORT_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
 	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
-			   suspend_fn, resume_fn, idle_fn, "")
+			   suspend_fn, resume_fn, idle_fn, "", "")
 #define EXPORT_GPL_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
 	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
-			   suspend_fn, resume_fn, idle_fn, "_gpl")
+			   suspend_fn, resume_fn, idle_fn, "_gpl", "")
+#define EXPORT_NS_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn, ns) \
+	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
+			   suspend_fn, resume_fn, idle_fn, "", #ns)
+#define EXPORT_NS_GPL_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn, ns) \
+	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
+			   suspend_fn, resume_fn, idle_fn, "_gpl", #ns)
 
 #ifdef CONFIG_PM
 extern struct workqueue_struct *pm_wq;
Signature · 1 line
2.35.1
#3 lore raw

Re: [PATCH 2/8] PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and runtime pm equiv

CC linux-pm
Quoted 8 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

As more drivers start to use namespaces, we need to have varients of these useful macros that allow the export to be in a particular namespace.

Cc Paul Cercueil <[email protected]>
Cc Rafael J. Wysocki <[email protected]>

I'd rather route this through linux-pm unless you have dependent changes.

Quoted 65 lines
---
  include/linux/pm.h         | 14 +++++++++-----
  include/linux/pm_runtime.h | 10 ++++++++--
  2 files changed, 17 insertions(+), 7 deletions(-)
include/linux/pm.h +9 -5
index f7d2be686359..112b8125d4be 100644
@@ -368,13 +368,13 @@ const struct dev_pm_ops name = { \
 
  #ifdef CONFIG_PM
  #define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
-			   runtime_resume_fn, idle_fn, sec) \
+			   runtime_resume_fn, idle_fn, sec, ns)		\
  	_DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
  			   runtime_resume_fn, idle_fn); \
-	_EXPORT_SYMBOL(name, sec)
+	__EXPORT_SYMBOL(name, sec, ns)
  #else
  #define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
-			   runtime_resume_fn, idle_fn, sec) \
+			   runtime_resume_fn, idle_fn, sec, ns) \
  static __maybe_unused _DEFINE_DEV_PM_OPS(__static_##name, suspend_fn, \
  					 resume_fn, runtime_suspend_fn, \
  					 runtime_resume_fn, idle_fn)
@@ -391,9 +391,13 @@ static __maybe_unused _DEFINE_DEV_PM_OPS(__static_##name, suspend_fn, \
  	_DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL)
 
  #define EXPORT_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "")
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "", "")
  #define EXPORT_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl")
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl", "")
+#define EXPORT_NS_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn, ns)	\
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "", #ns)
+#define EXPORT_NS_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn, ns)	\
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl", #ns)
 
  /* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
  #define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
include/linux/pm_runtime.h +8 -2
index 9f09601c465a..6a8b9551ecad 100644
@@ -41,10 +41,16 @@
 
  #define EXPORT_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
  	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
-			   suspend_fn, resume_fn, idle_fn, "")
+			   suspend_fn, resume_fn, idle_fn, "", "")
  #define EXPORT_GPL_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
  	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
-			   suspend_fn, resume_fn, idle_fn, "_gpl")
+			   suspend_fn, resume_fn, idle_fn, "_gpl", "")
+#define EXPORT_NS_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn, ns) \
+	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
+			   suspend_fn, resume_fn, idle_fn, "", #ns)
+#define EXPORT_NS_GPL_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn, ns) \
+	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
+			   suspend_fn, resume_fn, idle_fn, "_gpl", #ns)
 
  #ifdef CONFIG_PM
  extern struct workqueue_struct *pm_wq;
author maintainer
#4 lore raw

Re: [PATCH 2/8] PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and runtime pm equiv

On Mon, 21 Feb 2022 20:37:57 +0100
"Rafael J. Wysocki" <[email protected]> wrote:

Hi Rafael,

Quoted 1 line
CC linux-pm

Oops. Stupid omission on my part, sorry about that!

Quoted 12 lines · Jonathan Cameron
Quoted 8 lines

From: Jonathan Cameron <[email protected]>

As more drivers start to use namespaces, we need to have varients of these useful macros that allow the export to be in a particular namespace.

Cc Paul Cercueil <[email protected]>
Cc Rafael J. Wysocki <[email protected]>

I'd rather route this through linux-pm unless you have dependent changes.

Ok.

The kxsd9 patch (4) is dependent on other changes queued for
the merge window in IIO. If we want to do it through linux-pm I'd love it if we can manage to get the ground work in for the coming merge window.

So options are:

1) This patch alone via linux-pm and I queue the users up for next cycle Fine by me but always awkward to have infrastructure with no users.
2) First 3 patches via linux-pm so we have a user (scd30) in a low churn driver and I'll queue the rest for 5.19. Fine by me as well. That goes on cleanly on 5.17-rc1 and there is nothing else in my review queue touching that driver.

I'm also interested to hear your view on the discussion going on in reply to the cover letter. Specifically Paul suggested we 'only' have the namespaced versions of these macros.

Thanks,

Jonathan

Quoted 69 lines
Quoted 67 lines

---
  include/linux/pm.h         | 14 +++++++++-----
  include/linux/pm_runtime.h | 10 ++++++++--
  2 files changed, 17 insertions(+), 7 deletions(-)
include/linux/pm.h +9 -5
index f7d2be686359..112b8125d4be 100644
@@ -368,13 +368,13 @@ const struct dev_pm_ops name = { \
 
  #ifdef CONFIG_PM
  #define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
-			   runtime_resume_fn, idle_fn, sec) \
+			   runtime_resume_fn, idle_fn, sec, ns)		\
  	_DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
  			   runtime_resume_fn, idle_fn); \
-	_EXPORT_SYMBOL(name, sec)
+	__EXPORT_SYMBOL(name, sec, ns)
  #else
  #define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
-			   runtime_resume_fn, idle_fn, sec) \
+			   runtime_resume_fn, idle_fn, sec, ns) \
  static __maybe_unused _DEFINE_DEV_PM_OPS(__static_##name, suspend_fn, \
  					 resume_fn, runtime_suspend_fn, \
  					 runtime_resume_fn, idle_fn)
@@ -391,9 +391,13 @@ static __maybe_unused _DEFINE_DEV_PM_OPS(__static_##name, suspend_fn, \
  	_DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL)
 
  #define EXPORT_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "")
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "", "")
  #define EXPORT_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl")
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl", "")
+#define EXPORT_NS_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn, ns)	\
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "", #ns)
+#define EXPORT_NS_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn, ns)	\
+	_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl", #ns)
 
  /* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
  #define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
include/linux/pm_runtime.h +8 -2
index 9f09601c465a..6a8b9551ecad 100644
@@ -41,10 +41,16 @@
 
  #define EXPORT_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
  	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
-			   suspend_fn, resume_fn, idle_fn, "")
+			   suspend_fn, resume_fn, idle_fn, "", "")
  #define EXPORT_GPL_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
  	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
-			   suspend_fn, resume_fn, idle_fn, "_gpl")
+			   suspend_fn, resume_fn, idle_fn, "_gpl", "")
+#define EXPORT_NS_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn, ns) \
+	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
+			   suspend_fn, resume_fn, idle_fn, "", #ns)
+#define EXPORT_NS_GPL_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn, ns) \
+	_EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
+			   suspend_fn, resume_fn, idle_fn, "_gpl", #ns)
 
  #ifdef CONFIG_PM
  extern struct workqueue_struct *pm_wq;  
#5 lore raw

Re: [PATCH 2/8] PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and runtime pm equiv

Quoted 28 lines · Jonathan Cameron

On Mon, 21 Feb 2022 20:37:57 +0100
"Rafael J. Wysocki" <[email protected]> wrote:

Hi Rafael,

Quoted 1 line
CC linux-pm

Oops. Stupid omission on my part, sorry about that!

Quoted 12 lines · Jonathan Cameron
Quoted 8 lines

From: Jonathan Cameron <[email protected]>

As more drivers start to use namespaces, we need to have varients of these useful macros that allow the export to be in a particular namespace.

Cc Paul Cercueil <[email protected]>
Cc Rafael J. Wysocki <[email protected]>

I'd rather route this through linux-pm unless you have dependent changes.

Ok.

The kxsd9 patch (4) is dependent on other changes queued for
the merge window in IIO. If we want to do it through linux-pm I'd love it if we can manage to get the ground work in for the coming merge window.

Quoted 8 lines

So options are:

1) This patch alone via linux-pm and I queue the users up for next cycle Fine by me but always awkward to have infrastructure with no users.
2) First 3 patches via linux-pm so we have a user (scd30) in a low churn driver and I'll queue the rest for 5.19. Fine by me as well. That goes on cleanly on 5.17-rc1 and there is nothing else in my review queue touching that driver.

That would work for me.

Quoted 3 lines

I'm also interested to hear your view on the discussion going on in reply to the cover letter. Specifically Paul suggested we 'only' have the namespaced versions of these macros.

Well, I'm a bit afraid that providing the namespaced versions only would slow down the adoption.

#6 lore raw

Re: [PATCH 2/8] PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and runtime pm equiv

Quoted 31 lines · On Mon, 28 Feb 2022 21:13:25 +0100 "Rafael J. Wysocki" <[email protected]>…
Quoted 30 lines · Jonathan Cameron

On Mon, 21 Feb 2022 20:37:57 +0100
"Rafael J. Wysocki" <[email protected]> wrote:

Hi Rafael,

Quoted 1 line
CC linux-pm

Oops. Stupid omission on my part, sorry about that!

Quoted 12 lines · Jonathan Cameron
Quoted 8 lines

From: Jonathan Cameron <[email protected]>

As more drivers start to use namespaces, we need to have varients of these useful macros that allow the export to be in a particular namespace.

Cc Paul Cercueil <[email protected]>
Cc Rafael J. Wysocki <[email protected]>

I'd rather route this through linux-pm unless you have dependent changes.

Ok.

The kxsd9 patch (4) is dependent on other changes queued for
the merge window in IIO. If we want to do it through linux-pm I'd love it if we can manage to get the ground work in for the coming merge window.

So options are:

Quoted 8 lines
Quoted 6 lines

1) This patch alone via linux-pm and I queue the users up for next cycle Fine by me but always awkward to have infrastructure with no users.
2) First 3 patches via linux-pm so we have a user (scd30) in a low churn driver and I'll queue the rest for 5.19. Fine by me as well. That goes on cleanly on 5.17-rc1 and there is nothing else in my review queue touching that driver.

That would work for me.

Great. Let's do that then. Are you fine picking them from this thread, or would you like me to resend with just those 3 patches as a fresh series?

Quoted 7 lines
Quoted 4 lines

I'm also interested to hear your view on the discussion going on in reply to the cover letter. Specifically Paul suggested we 'only' have the namespaced versions of these macros.

Well, I'm a bit afraid that providing the namespaced versions only would slow down the adoption.

Agreed, that's a concern and as Paul was happy with the route of adding NS and perhaps looking eventually at dropping the non NS variant I think we can move forward with this patch.

Thanks,

Jonathan

#7 lore raw

Re: [PATCH 2/8] PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and runtime pm equiv

Quoted 41 lines · On Tue, 1 Mar 2022 11:31:45 +0000 Jonathan Cameron…
Quoted 38 lines · On Mon, 28 Feb 2022 21:13:25 +0100 "Rafael J. Wysocki" <[email protected]>…
Quoted 36 lines · Jonathan Cameron

On Mon, 21 Feb 2022 20:37:57 +0100
"Rafael J. Wysocki" <[email protected]> wrote:

Hi Rafael,

Quoted 1 line
CC linux-pm

Oops. Stupid omission on my part, sorry about that!

Quoted 12 lines · Jonathan Cameron
Quoted 8 lines

From: Jonathan Cameron <[email protected]>

As more drivers start to use namespaces, we need to have varients of these useful macros that allow the export to be in a particular namespace.

Cc Paul Cercueil <[email protected]>
Cc Rafael J. Wysocki <[email protected]>

I'd rather route this through linux-pm unless you have dependent changes.

Ok.

The kxsd9 patch (4) is dependent on other changes queued for
the merge window in IIO. If we want to do it through linux-pm I'd love it if we can manage to get the ground work in for the coming merge window.

So options are:

1) This patch alone via linux-pm and I queue the users up for next cycle Fine by me but always awkward to have infrastructure with no users.
2) First 3 patches via linux-pm so we have a user (scd30) in a low churn driver and I'll queue the rest for 5.19. Fine by me as well. That goes on cleanly on 5.17-rc1 and there is nothing else in my review queue touching that driver.

Quoted 4 lines
Quoted 1 line

That would work for me.

Great. Let's do that then. Are you fine picking them from this thread, or would you like me to resend with just those 3 patches as a fresh series?

Hi Rafael,

I've not heard back from you, so have been assuming you'd pick those first 3 patches up from this series. Is that a correct assumption?

Thanks,

Jonathan

Quoted 18 lines
Quoted 8 lines
Quoted 5 lines

I'm also interested to hear your view on the discussion going on in reply to the cover letter. Specifically Paul suggested we 'only' have the namespaced versions of these macros.

Well, I'm a bit afraid that providing the namespaced versions only would slow down the adoption.

Agreed, that's a concern and as Paul was happy with the route of adding NS and perhaps looking eventually at dropping the non NS variant I think we can move forward with this patch.

Thanks,

Jonathan

#8 lore raw

Re: [PATCH 2/8] PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and runtime pm equiv

Hi Jonathan,

Quoted 45 lines · Jonathan Cameron
Quoted 41 lines · On Tue, 1 Mar 2022 11:31:45 +0000 Jonathan Cameron…
Quoted 38 lines · On Mon, 28 Feb 2022 21:13:25 +0100 "Rafael J. Wysocki" <[email protected]>…
Quoted 36 lines · Jonathan Cameron

On Mon, 21 Feb 2022 20:37:57 +0100
"Rafael J. Wysocki" <[email protected]> wrote:

Hi Rafael,

Quoted 1 line
CC linux-pm

Oops. Stupid omission on my part, sorry about that!

Quoted 12 lines · Jonathan Cameron
Quoted 8 lines

From: Jonathan Cameron <[email protected]>

As more drivers start to use namespaces, we need to have varients of these useful macros that allow the export to be in a particular namespace.

Cc Paul Cercueil <[email protected]>
Cc Rafael J. Wysocki <[email protected]>

I'd rather route this through linux-pm unless you have dependent changes.

Ok.

The kxsd9 patch (4) is dependent on other changes queued for
the merge window in IIO. If we want to do it through linux-pm I'd love it if we can manage to get the ground work in for the coming merge window.

So options are:

1) This patch alone via linux-pm and I queue the users up for next cycle Fine by me but always awkward to have infrastructure with no users.
2) First 3 patches via linux-pm so we have a user (scd30) in a low churn driver and I'll queue the rest for 5.19. Fine by me as well. That goes on cleanly on 5.17-rc1 and there is nothing else in my review queue touching that driver.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

That would work for me.

Great. Let's do that then. Are you fine picking them from this thread, or would you like me to resend with just those 3 patches as a fresh series?

Hi Rafael,

I've not heard back from you, so have been assuming you'd pick those first 3 patches up from this series. Is that a correct assumption?

This was my intention, but then I lost track of them and now I can't find them in the linux-pm Patchwork. Sorry about this.

Can you please resend just the 3 patches?

Thanks!

#9 lore raw

Re: [PATCH 2/8] PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and runtime pm equiv

Quoted 53 lines · On Thu, 31 Mar 2022 19:21:14 +0200 "Rafael J. Wysocki" <[email protected]>…

Hi Jonathan,

Quoted 49 lines · Jonathan Cameron
Quoted 45 lines · On Tue, 1 Mar 2022 11:31:45 +0000 Jonathan Cameron…
Quoted 31 lines · On Mon, 28 Feb 2022 21:13:25 +0100 "Rafael J. Wysocki" <[email protected]>…
Quoted 30 lines · Jonathan Cameron

On Mon, 21 Feb 2022 20:37:57 +0100
"Rafael J. Wysocki" <[email protected]> wrote:

Hi Rafael,

Quoted 1 line
CC linux-pm

Oops. Stupid omission on my part, sorry about that!

Quoted 12 lines · Jonathan Cameron
Quoted 8 lines

From: Jonathan Cameron <[email protected]>

As more drivers start to use namespaces, we need to have varients of these useful macros that allow the export to be in a particular namespace.

Cc Paul Cercueil <[email protected]>
Cc Rafael J. Wysocki <[email protected]>

I'd rather route this through linux-pm unless you have dependent changes.

Ok.

The kxsd9 patch (4) is dependent on other changes queued for
the merge window in IIO. If we want to do it through linux-pm I'd love it if we can manage to get the ground work in for the coming merge window.

So options are:

Quoted 8 lines
Quoted 6 lines

1) This patch alone via linux-pm and I queue the users up for next cycle Fine by me but always awkward to have infrastructure with no users.
2) First 3 patches via linux-pm so we have a user (scd30) in a low churn driver and I'll queue the rest for 5.19. Fine by me as well. That goes on cleanly on 5.17-rc1 and there is nothing else in my review queue touching that driver.

That would work for me.

Great. Let's do that then. Are you fine picking them from this thread, or would you like me to resend with just those 3 patches as a fresh series?

Quoted 7 lines
Quoted 4 lines

Hi Rafael,

I've not heard back from you, so have been assuming you'd pick those first 3 patches up from this series. Is that a correct assumption?

This was my intention, but then I lost track of them and now I can't find them in the linux-pm Patchwork. Sorry about this.

My fault, I think I forgot to cc linux-pm on the initial send and you added it in your reply.

Quoted 2 lines

Can you please resend just the 3 patches?

On their way.

Quoted 2 lines

Thanks!

author maintainer
#10 lore raw

PATCH 3/8 iio: chemical: scd30: Move symbol exports into IIO_SCD30 namespace

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Avoid unnecessary pollution of the global symbol namespace by moving library functions in to a specific namespace and import that into the drivers that make use of the functions.

For more info: https://lwn.net/Articles/760045/

---
 drivers/iio/chemical/scd30_core.c   | 4 ++--
 drivers/iio/chemical/scd30_i2c.c    | 1 +
 drivers/iio/chemical/scd30_serial.c | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)
drivers/iio/chemical/scd30_core.c +2 -2
index 6c6c11c2772a..682fca39d14d 100644
@@ -543,7 +543,7 @@ static int scd30_resume(struct device *dev)
 	return scd30_command_write(state, CMD_START_MEAS, state->pressure_comp);
 }
 
-EXPORT_SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend, scd30_resume);
+EXPORT_NS_SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend, scd30_resume, IIO_SCD30);
 
 static void scd30_stop_meas(void *data)
 {
@@ -759,7 +759,7 @@ int scd30_probe(struct device *dev, int irq, const char *name, void *priv,
 
 	return devm_iio_device_register(dev, indio_dev);
 }
-EXPORT_SYMBOL(scd30_probe);
+EXPORT_SYMBOL_NS(scd30_probe, IIO_SCD30);
 
 MODULE_AUTHOR("Tomasz Duszynski <[email protected]>");
 MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor core driver");
drivers/iio/chemical/scd30_i2c.c +1 -0
index 7c332e4e8e46..bae479a4721f 100644
@@ -137,3 +137,4 @@ module_i2c_driver(scd30_i2c_driver);
 MODULE_AUTHOR("Tomasz Duszynski <[email protected]>");
 MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor i2c driver");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_SCD30);
drivers/iio/chemical/scd30_serial.c +1 -0
index 8664f3ce6b33..3c519103d30b 100644
@@ -261,3 +261,4 @@ module_serdev_device_driver(scd30_serdev_driver);
 MODULE_AUTHOR("Tomasz Duszynski <[email protected]>");
 MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor serial driver");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_SCD30);
Signature · 1 line
2.35.1
author maintainer
#11 lore raw

PATCH 4/8 iio:accel:kxsd9: Switch from CONFIG_PM guards to pm_ptr() etc

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Letting the compiler remove these functions when the kernel is built without CONFIG_PM support is simpler and less error prone than the use of #ifdef based config guards.

Removing instances of this approach from IIO also stops them being copied into new drivers.

---
 drivers/iio/accel/kxsd9-i2c.c |  2 +-
 drivers/iio/accel/kxsd9-spi.c |  2 +-
 drivers/iio/accel/kxsd9.c     | 11 ++---------
 3 files changed, 4 insertions(+), 11 deletions(-)
drivers/iio/accel/kxsd9-i2c.c +1 -1
index c8dc52f11037..d57f264bd6c8 100644
@@ -55,7 +55,7 @@ static struct i2c_driver kxsd9_i2c_driver = {
 	.driver = {
 		.name	= "kxsd9",
 		.of_match_table = kxsd9_of_match,
-		.pm = &kxsd9_dev_pm_ops,
+		.pm = pm_ptr(&kxsd9_dev_pm_ops),
 	},
 	.probe		= kxsd9_i2c_probe,
 	.remove		= kxsd9_i2c_remove,
drivers/iio/accel/kxsd9-spi.c +1 -1
index a06a3a273de7..3870334488a6 100644
@@ -54,7 +54,7 @@ MODULE_DEVICE_TABLE(of, kxsd9_of_match);
 static struct spi_driver kxsd9_spi_driver = {
 	.driver = {
 		.name = "kxsd9",
-		.pm = &kxsd9_dev_pm_ops,
+		.pm = pm_ptr(&kxsd9_dev_pm_ops),
 		.of_match_table = kxsd9_of_match,
 	},
 	.probe = kxsd9_spi_probe,
drivers/iio/accel/kxsd9.c +2 -9
index 3975860331a6..ba99649fe195 100644
@@ -492,7 +492,6 @@ void kxsd9_common_remove(struct device *dev)
 }
 EXPORT_SYMBOL_NS(kxsd9_common_remove, IIO_KXSD9);
 
-#ifdef CONFIG_PM
 static int kxsd9_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
@@ -508,15 +507,9 @@ static int kxsd9_runtime_resume(struct device *dev)
 
 	return kxsd9_power_up(st);
 }
-#endif /* CONFIG_PM */
 
-const struct dev_pm_ops kxsd9_dev_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
-				pm_runtime_force_resume)
-	SET_RUNTIME_PM_OPS(kxsd9_runtime_suspend,
-			   kxsd9_runtime_resume, NULL)
-};
-EXPORT_SYMBOL_NS(kxsd9_dev_pm_ops, IIO_KXSD9);
+EXPORT_NS_RUNTIME_DEV_PM_OPS(kxsd9_dev_pm_ops, kxsd9_runtime_suspend,
+			     kxsd9_runtime_resume, NULL, IIO_KXSD9);
 
 MODULE_AUTHOR("Jonathan Cameron <[email protected]>");
 MODULE_DESCRIPTION("Kionix KXSD9 driver");
Signature · 1 line
2.35.1
author maintainer
#12 lore raw

PATCH 5/8 iio: humidity: hts221: Use EXPORT_SIMPLE_DEV_PM_OPS() to allow compiler to remove dead code.

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

If CONFIG_PM_SLEEP is not defined using EXPORT_SIMPLE_DEV_PM_OPS() in conjunction with pm_sleep_ptr() allows the compiler to remove the unused code and data. This removes the need for __maybe_unused markings etc.

---
 drivers/iio/humidity/hts221_core.c | 9 +++------
 drivers/iio/humidity/hts221_i2c.c  | 2 +-
 drivers/iio/humidity/hts221_spi.c  | 2 +-
 3 files changed, 5 insertions(+), 8 deletions(-)
drivers/iio/humidity/hts221_core.c +3 -6
index 6a39615b6961..da9c08432ef2 100644
@@ -670,7 +670,7 @@ int hts221_probe(struct device *dev, int irq, const char *name,
 }
 EXPORT_SYMBOL(hts221_probe);
 
-static int __maybe_unused hts221_suspend(struct device *dev)
+static int hts221_suspend(struct device *dev)
 {
 	struct iio_dev *iio_dev = dev_get_drvdata(dev);
 	struct hts221_hw *hw = iio_priv(iio_dev);
@@ -680,7 +680,7 @@ static int __maybe_unused hts221_suspend(struct device *dev)
 				  FIELD_PREP(HTS221_ENABLE_MASK, false));
 }
 
-static int __maybe_unused hts221_resume(struct device *dev)
+static int hts221_resume(struct device *dev)
 {
 	struct iio_dev *iio_dev = dev_get_drvdata(dev);
 	struct hts221_hw *hw = iio_priv(iio_dev);
@@ -694,10 +694,7 @@ static int __maybe_unused hts221_resume(struct device *dev)
 	return err;
 }
 
-const struct dev_pm_ops hts221_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(hts221_suspend, hts221_resume)
-};
-EXPORT_SYMBOL(hts221_pm_ops);
+EXPORT_SIMPLE_DEV_PM_OPS(hts221_pm_ops, hts221_suspend, hts221_resume);
 
 MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
 MODULE_DESCRIPTION("STMicroelectronics hts221 sensor driver");
drivers/iio/humidity/hts221_i2c.c +1 -1
index cab39c4756f8..933b05e4d972 100644
@@ -62,7 +62,7 @@ MODULE_DEVICE_TABLE(i2c, hts221_i2c_id_table);
 static struct i2c_driver hts221_driver = {
 	.driver = {
 		.name = "hts221_i2c",
-		.pm = &hts221_pm_ops,
+		.pm = pm_sleep_ptr(&hts221_pm_ops),
 		.of_match_table = hts221_i2c_of_match,
 		.acpi_match_table = ACPI_PTR(hts221_acpi_match),
 	},
drivers/iio/humidity/hts221_spi.c +1 -1
index 729e86e433b1..888c5eab944c 100644
@@ -55,7 +55,7 @@ MODULE_DEVICE_TABLE(spi, hts221_spi_id_table);
 static struct spi_driver hts221_driver = {
 	.driver = {
 		.name = "hts221_spi",
-		.pm = &hts221_pm_ops,
+		.pm = pm_sleep_ptr(&hts221_pm_ops),
 		.of_match_table = hts221_spi_of_match,
 	},
 	.probe = hts221_spi_probe,
Signature · 1 line
2.35.1
author maintainer
#13 lore raw

PATCH 6/8 iio: humidity: hts221: Move symbol exports into IIO_HTS221 namespace

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Avoid unnecessary pollution of the global symbol namespace by moving library functions in to a specific namespace and import that into the drivers that make use of the functions.

For more info: https://lwn.net/Articles/760045/

---
 drivers/iio/humidity/hts221_core.c | 5 +++--
 drivers/iio/humidity/hts221_i2c.c  | 1 +
 drivers/iio/humidity/hts221_spi.c  | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)
drivers/iio/humidity/hts221_core.c +3 -2
index da9c08432ef2..517158307d8c 100644
@@ -668,7 +668,7 @@ int hts221_probe(struct device *dev, int irq, const char *name,
 
 	return devm_iio_device_register(hw->dev, iio_dev);
 }
-EXPORT_SYMBOL(hts221_probe);
+EXPORT_SYMBOL_NS(hts221_probe, IIO_HTS221);
 
 static int hts221_suspend(struct device *dev)
 {
@@ -694,7 +694,8 @@ static int hts221_resume(struct device *dev)
 	return err;
 }
 
-EXPORT_SIMPLE_DEV_PM_OPS(hts221_pm_ops, hts221_suspend, hts221_resume);
+EXPORT_NS_SIMPLE_DEV_PM_OPS(hts221_pm_ops, hts221_suspend, hts221_resume,
+			    IIO_HTS221);
 
 MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
 MODULE_DESCRIPTION("STMicroelectronics hts221 sensor driver");
drivers/iio/humidity/hts221_i2c.c +1 -0
index 933b05e4d972..afbc611f7712 100644
@@ -74,3 +74,4 @@ module_i2c_driver(hts221_driver);
 MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
 MODULE_DESCRIPTION("STMicroelectronics hts221 i2c driver");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_HTS221);
drivers/iio/humidity/hts221_spi.c +1 -0
index 888c5eab944c..fc4adb68faf6 100644
@@ -66,3 +66,4 @@ module_spi_driver(hts221_driver);
 MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
 MODULE_DESCRIPTION("STMicroelectronics hts221 spi driver");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_HTS221);
Signature · 1 line
2.35.1
author maintainer
#14 lore raw

PATCH 7/8 iio: imu: lsm6dsx: Use new pm_sleep_ptr() and EXPORT_SIMPLE_DEV_PM_OPS()

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

These new functions move the burden of removing unused code when CONFIG_PM_SLEEP is not defined onto the compiler rather than requiring the use of CONFIG_PM guards and similar.

---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 10 ++++------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c  |  2 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c  |  2 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c  |  2 +-
 4 files changed, 7 insertions(+), 9 deletions(-)
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +4 -6
index 727b4b6ac696..0a86534bd1dd 100644
@@ -2283,7 +2283,7 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 }
 EXPORT_SYMBOL(st_lsm6dsx_probe);
 
-static int __maybe_unused st_lsm6dsx_suspend(struct device *dev)
+static int st_lsm6dsx_suspend(struct device *dev)
 {
 	struct st_lsm6dsx_hw *hw = dev_get_drvdata(dev);
 	struct st_lsm6dsx_sensor *sensor;
@@ -2322,7 +2322,7 @@ static int __maybe_unused st_lsm6dsx_suspend(struct device *dev)
 	return err;
 }
 
-static int __maybe_unused st_lsm6dsx_resume(struct device *dev)
+static int st_lsm6dsx_resume(struct device *dev)
 {
 	struct st_lsm6dsx_hw *hw = dev_get_drvdata(dev);
 	struct st_lsm6dsx_sensor *sensor;
@@ -2358,10 +2358,8 @@ static int __maybe_unused st_lsm6dsx_resume(struct device *dev)
 	return err;
 }
 
-const struct dev_pm_ops st_lsm6dsx_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(st_lsm6dsx_suspend, st_lsm6dsx_resume)
-};
-EXPORT_SYMBOL(st_lsm6dsx_pm_ops);
+EXPORT_SIMPLE_DEV_PM_OPS(st_lsm6dsx_pm_ops, st_lsm6dsx_suspend,
+			 st_lsm6dsx_resume);
 
 MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
 MODULE_AUTHOR("Denis Ciocca <[email protected]>");
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c +1 -1
index 8b4fc2c15622..faf4e67f0f4c 100644
@@ -129,7 +129,7 @@ MODULE_DEVICE_TABLE(i2c, st_lsm6dsx_i2c_id_table);
 static struct i2c_driver st_lsm6dsx_driver = {
 	.driver = {
 		.name = "st_lsm6dsx_i2c",
-		.pm = &st_lsm6dsx_pm_ops,
+		.pm = pm_sleep_ptr(&st_lsm6dsx_pm_ops),
 		.of_match_table = st_lsm6dsx_i2c_of_match,
 	},
 	.probe = st_lsm6dsx_i2c_probe,
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c +1 -1
index 8d4201b86e87..aaa6b29be3be 100644
@@ -44,7 +44,7 @@ static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
 static struct i3c_driver st_lsm6dsx_driver = {
 	.driver = {
 		.name = "st_lsm6dsx_i3c",
-		.pm = &st_lsm6dsx_pm_ops,
+		.pm = pm_sleep_ptr(&st_lsm6dsx_pm_ops),
 	},
 	.probe = st_lsm6dsx_i3c_probe,
 	.id_table = st_lsm6dsx_i3c_ids,
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c +1 -1
index e80110b6b280..6180110a1379 100644
@@ -129,7 +129,7 @@ MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table);
 static struct spi_driver st_lsm6dsx_driver = {
 	.driver = {
 		.name = "st_lsm6dsx_spi",
-		.pm = &st_lsm6dsx_pm_ops,
+		.pm = pm_sleep_ptr(&st_lsm6dsx_pm_ops),
 		.of_match_table = st_lsm6dsx_spi_of_match,
 	},
 	.probe = st_lsm6dsx_spi_probe,
Signature · 1 line
2.35.1
author maintainer
#15 lore raw

PATCH 8/8 iio: imu: lsm6dsx: Move exported symbols to the IIO_LSM6DSX namespace

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Avoid unnecessary pollution of the global symbol namespace by moving library functions in to a specific namespace and import that into the drivers that make use of the functions.

For more info: https://lwn.net/Articles/760045/

---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 6 +++---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c  | 1 +
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c  | 1 +
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c  | 1 +
 4 files changed, 6 insertions(+), 3 deletions(-)
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +3 -3
index 0a86534bd1dd..03f3bc62e27b 100644
@@ -2281,7 +2281,7 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 
 	return 0;
 }
-EXPORT_SYMBOL(st_lsm6dsx_probe);
+EXPORT_SYMBOL_NS(st_lsm6dsx_probe, IIO_LSM6DSX);
 
 static int st_lsm6dsx_suspend(struct device *dev)
 {
@@ -2358,8 +2358,8 @@ static int st_lsm6dsx_resume(struct device *dev)
 	return err;
 }
 
-EXPORT_SIMPLE_DEV_PM_OPS(st_lsm6dsx_pm_ops, st_lsm6dsx_suspend,
-			 st_lsm6dsx_resume);
+EXPORT_NS_SIMPLE_DEV_PM_OPS(st_lsm6dsx_pm_ops, st_lsm6dsx_suspend,
+			    st_lsm6dsx_resume, IIO_LSM6DSX);
 
 MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
 MODULE_AUTHOR("Denis Ciocca <[email protected]>");
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c +1 -0
index faf4e67f0f4c..7ea12ae66afe 100644
@@ -141,3 +141,4 @@ MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
 MODULE_AUTHOR("Denis Ciocca <[email protected]>");
 MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i2c driver");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_LSM6DSX);
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c +1 -0
index aaa6b29be3be..ecf592e4150b 100644
@@ -54,3 +54,4 @@ module_i3c_driver(st_lsm6dsx_driver);
 MODULE_AUTHOR("Vitor Soares <[email protected]>");
 MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_LSM6DSX);
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c +1 -0
index 6180110a1379..55e0f9c204ef 100644
@@ -141,3 +141,4 @@ MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
 MODULE_AUTHOR("Denis Ciocca <[email protected]>");
 MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx spi driver");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_LSM6DSX);
Signature · 1 line
2.35.1
#16 lore raw
In reply to Jonathan Cameron

Hi Jonathan,

Quoted 4 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Hi All,

Quoted 8 lines

As these two activities have been ongoing across IIO at the same time it was inevitable that there would be drivers were they both interact. In particularly when we have EXPORT* of struct dev_pm_ops.

This series covers those cases by introducing (in patch 2) EXPORT_NS[_GPL]_SIMPLE_DEV_PM_OPS() and EXPORT_NS[_GPL]_RUNTIME_DEV_PM_OPS()
that add a namespace as the final parameter.

If we now have namespace versions of the macros, I'm starting to wonder if the original macros are still useful.

In which case would we need to export a dev_pm_ops outside the scope of a namespace?

Cheers,
-Paul

Quoted 49 lines

In each driver (with the exception of the kxsd9 which was already using a IIO_KXSD9 namespace) I first move to the new *_PM_OPS() and pm_[sleep_]_ptr() then in a second patch move to the new namespaces. Initially I had these two steps done as one patch per driver but that was harder to follow and broke the rule of one thing per patch.

All comments welcome.

Thanks,

Jonathan

Jonathan Cameron (8):
iio: chemical: scd30: Export dev_pm_ops instead of suspend() and
resume()
PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and
runtime pm equiv
iio: chemical: scd30: Move symbol exports into IIO_SCD30 namespace iio:accel:kxsd9: Switch from CONFIG_PM guards to pm_ptr() etc
iio: humidity: hts221: Use EXPORT_SIMPLE_DEV_PM_OPS() to allow
compiler to remove dead code.
iio: humidity: hts221: Move symbol exports into IIO_HTS221 namespace
iio: imu: lsm6dsx: Use new pm_sleep_ptr() and
EXPORT_SIMPLE_DEV_PM_OPS()
iio: imu: lsm6dsx: Move exported symbols to the IIO_LSM6DSX
namespace

 drivers/iio/accel/kxsd9-i2c.c                |  2 +-
 drivers/iio/accel/kxsd9-spi.c                |  2 +-
 drivers/iio/accel/kxsd9.c                    | 11 ++---------
 drivers/iio/chemical/scd30.h                 |  5 +----
 drivers/iio/chemical/scd30_core.c            | 10 +++++-----
 drivers/iio/chemical/scd30_i2c.c             |  3 ++-
 drivers/iio/chemical/scd30_serial.c          |  3 ++-
 drivers/iio/humidity/hts221_core.c           | 12 +++++-------
 drivers/iio/humidity/hts221_i2c.c            |  3 ++-
 drivers/iio/humidity/hts221_spi.c            |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 12 +++++-------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c  |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c  |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c  |  3 ++-
 include/linux/pm.h                           | 14 +++++++++-----
 include/linux/pm_runtime.h                   | 10 ++++++++--
 16 files changed, 51 insertions(+), 48 deletions(-)

--
2.35.1

#17 lore raw
Quoted 17 lines · On Wed, 23 Feb 2022 11:50:48 +0000 Paul Cercueil <[email protected]> wrote:

Hi Jonathan,

Quoted 4 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Hi All,

Quoted 8 lines

As these two activities have been ongoing across IIO at the same time it was inevitable that there would be drivers were they both interact. In particularly when we have EXPORT* of struct dev_pm_ops.

This series covers those cases by introducing (in patch 2) EXPORT_NS[_GPL]_SIMPLE_DEV_PM_OPS() and EXPORT_NS[_GPL]_RUNTIME_DEV_PM_OPS()
that add a namespace as the final parameter.

Quoted 5 lines

If we now have namespace versions of the macros, I'm starting to wonder if the original macros are still useful.

In which case would we need to export a dev_pm_ops outside the scope of a namespace?

Excellent point. Long term my assumption is that any place where a dev_pm_ops is being exported probably should be in a namespace because the scope is tightly defined.

My only thought is it adds a burden to moving over to your new infrastructure as it would be odd to move only the dev_pm_ops into a NS without looking at the rest of the exports in a driver.

It was useful to be able to do things in two steps as it allowed separation of using the new dev_pm_ops handling from a namespace move, but clearly could do that the other way around and not need the non namespaced version of this macro.
So do namespace move first then use the new macros and hence only ever need the namespaced one.

Jonathan

Quoted 55 lines

Cheers,
-Paul

Quoted 49 lines

In each driver (with the exception of the kxsd9 which was already using a IIO_KXSD9 namespace) I first move to the new *_PM_OPS() and pm_[sleep_]_ptr() then in a second patch move to the new namespaces. Initially I had these two steps done as one patch per driver but that was harder to follow and broke the rule of one thing per patch.

All comments welcome.

Thanks,

Jonathan

Jonathan Cameron (8):
iio: chemical: scd30: Export dev_pm_ops instead of suspend() and
resume()
PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and
runtime pm equiv
iio: chemical: scd30: Move symbol exports into IIO_SCD30 namespace iio:accel:kxsd9: Switch from CONFIG_PM guards to pm_ptr() etc
iio: humidity: hts221: Use EXPORT_SIMPLE_DEV_PM_OPS() to allow
compiler to remove dead code.
iio: humidity: hts221: Move symbol exports into IIO_HTS221 namespace
iio: imu: lsm6dsx: Use new pm_sleep_ptr() and
EXPORT_SIMPLE_DEV_PM_OPS()
iio: imu: lsm6dsx: Move exported symbols to the IIO_LSM6DSX
namespace

 drivers/iio/accel/kxsd9-i2c.c                |  2 +-
 drivers/iio/accel/kxsd9-spi.c                |  2 +-
 drivers/iio/accel/kxsd9.c                    | 11 ++---------
 drivers/iio/chemical/scd30.h                 |  5 +----
 drivers/iio/chemical/scd30_core.c            | 10 +++++-----
 drivers/iio/chemical/scd30_i2c.c             |  3 ++-
 drivers/iio/chemical/scd30_serial.c          |  3 ++-
 drivers/iio/humidity/hts221_core.c           | 12 +++++-------
 drivers/iio/humidity/hts221_i2c.c            |  3 ++-
 drivers/iio/humidity/hts221_spi.c            |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 12 +++++-------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c  |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c  |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c  |  3 ++-
 include/linux/pm.h                           | 14 +++++++++-----
 include/linux/pm_runtime.h                   | 10 ++++++++--
 16 files changed, 51 insertions(+), 48 deletions(-)

--
2.35.1

#18 lore raw
Quoted 38 lines · Jonathan Cameron
Quoted 19 lines · On Wed, 23 Feb 2022 11:50:48 +0000 Paul Cercueil <[email protected]> wrote:

Hi Jonathan,

Quoted 5 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Hi All,

As these two activities have been ongoing across IIO at the same

time

Quoted 1 line

it was inevitable that there would be drivers were they both

interact.

Quoted 6 lines

In particularly when we have EXPORT* of struct dev_pm_ops.

This series covers those cases by introducing (in patch 2) EXPORT_NS[_GPL]_SIMPLE_DEV_PM_OPS() and EXPORT_NS[_GPL]_RUNTIME_DEV_PM_OPS()
that add a namespace as the final parameter.

Quoted 7 lines

If we now have namespace versions of the macros, I'm starting to
wonder
if the original macros are still useful.

In which case would we need to export a dev_pm_ops outside the
scope of
a namespace?

Excellent point. Long term my assumption is that any place where a dev_pm_ops is being exported probably should be in a namespace because the scope is tightly defined.

My only thought is it adds a burden to moving over to your new infrastructure as it would be odd to move only the dev_pm_ops into a NS without looking at the rest of the exports in a driver.

Quoted 6 lines

It was useful to be able to do things in two steps as it allowed separation of using the new dev_pm_ops handling from a namespace move, but clearly could do that the other way around and not need the non namespaced version of this macro.
So do namespace move first then use the new macros and hence only ever need the namespaced one.

I'd be slightly in favor of enforcing namespaces in the PM macros, if only to simplify the macro names (as you could just extend the existing ones to require a namespace, I don't think anything use them yet).

But I'm fine with keeping the non-namespaced macros as well if it's easier. We can always drop them later.

Cheers,
-Paul

Quoted 61 lines

Jonathan

Quoted 57 lines

Cheers,
-Paul

Quoted 11 lines

In each driver (with the exception of the kxsd9 which was already using a IIO_KXSD9 namespace) I first move to the new *_PM_OPS() and pm_[sleep_]_ptr() then in a second patch move to the new namespaces. Initially I had these two steps done as one patch per driver but that was harder to follow and broke the rule of one thing per patch.

All comments welcome.

Thanks,

Quoted 8 lines

Jonathan

Jonathan Cameron (8):
iio: chemical: scd30: Export dev_pm_ops instead of suspend() and
resume()
PM: core: Add NS varients of EXPORT[_GPL]_SIMPLE_DEV_PM_OPS and
runtime pm equiv
iio: chemical: scd30: Move symbol exports into IIO_SCD30

namespace

Quoted 4 lines

iio:accel:kxsd9: Switch from CONFIG_PM guards to pm_ptr() etc
iio: humidity: hts221: Use EXPORT_SIMPLE_DEV_PM_OPS() to allow
compiler to remove dead code.
iio: humidity: hts221: Move symbol exports into IIO_HTS221

namespace

Quoted 26 lines

iio: imu: lsm6dsx: Use new pm_sleep_ptr() and
EXPORT_SIMPLE_DEV_PM_OPS()
iio: imu: lsm6dsx: Move exported symbols to the IIO_LSM6DSX
namespace

 drivers/iio/accel/kxsd9-i2c.c                |  2 +-
 drivers/iio/accel/kxsd9-spi.c                |  2 +-
 drivers/iio/accel/kxsd9.c                    | 11 ++---------
 drivers/iio/chemical/scd30.h                 |  5 +----
 drivers/iio/chemical/scd30_core.c            | 10 +++++-----
 drivers/iio/chemical/scd30_i2c.c             |  3 ++-
 drivers/iio/chemical/scd30_serial.c          |  3 ++-
 drivers/iio/humidity/hts221_core.c           | 12 +++++-------
 drivers/iio/humidity/hts221_i2c.c            |  3 ++-
 drivers/iio/humidity/hts221_spi.c            |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 12 +++++-------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c  |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c  |  3 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c  |  3 ++-
 include/linux/pm.h                           | 14 +++++++++-----
 include/linux/pm_runtime.h                   | 10 ++++++++--
 16 files changed, 51 insertions(+), 48 deletions(-)

--
2.35.1