Skip to contentSkip to conversation

pinctrl: intel: Provide NOIRQ PM helper and use it

PATCH Superseded Andy Shevchenko 3 participants 7 messages Dec 29, 2022 – Dec 30, 2022

Subsystems (6)
GPIO Pin control Suspend to RAM Hibernation +2 more subsystems

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

cover pinctrl: intel: Provide NOIRQ PM helper and use it

Intel pin control drivers use NOIRQ variant of PM callbacks. Besides that several other drivers do similar. Provide a helper to make them smaller.

Andy Shevchenko (3):
pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper
pinctrl: intel: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper
pinctrl: cherryview: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

 drivers/pinctrl/intel/pinctrl-cherryview.c | 4 +---
 drivers/pinctrl/intel/pinctrl-intel.h      | 6 ++----
 include/linux/pm.h                         | 5 +++++
 3 files changed, 8 insertions(+), 7 deletions(-)
Signature · 1 line
2.35.1
author
#1 lore raw

PATCH 1/3 pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper

There are a few drivers and might be more in the future that open code the functionality of proposed DEFINE_NOIRQ_DEV_PM_OPS() helper. From now on they may switch to the new helper and save a few lines of code.

Signed-off-by Andy Shevchenko <[email protected]>
---
 include/linux/pm.h | 5 +++++
 1 file changed, 5 insertions(+)
include/linux/pm.h +5 -0
index 93cd34f00822..eba96822b1d9 100644
@@ -444,6 +444,11 @@ const struct dev_pm_ops __maybe_unused name = { \
 	SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
 }
 
+#define DEFINE_NOIRQ_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+const struct dev_pm_ops name = { \
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+}
+
 #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
 #define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))
Signature · 1 line
2.35.1
maintainer
#2 lore raw

Re: [PATCH v1 1/3] pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper

Quoted 16 lines · Andy Shevchenko

There are a few drivers and might be more in the future that open code the functionality of proposed DEFINE_NOIRQ_DEV_PM_OPS() helper. From now on they may switch to the new helper and save a few lines of code.

Signed-off-by Andy Shevchenko <[email protected]>
---
 include/linux/pm.h | 5 +++++
 1 file changed, 5 insertions(+)
include/linux/pm.h +0 -0
index 93cd34f00822..eba96822b1d9 100644

include/linux/pm.h const struct dev_pm_ops __maybe_unused name = { \ · line 450

Quoted 7 lines
include/linux/pm.h +4 -0
@@ -444,6 +444,11 @@ const struct dev_pm_ops __maybe_unused name = { \
        SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
 }
 
+#define DEFINE_NOIRQ_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+const struct dev_pm_ops name = { \
+       NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+}

There is NOIRQ_SYSTEM_SLEEP_PM_OPS(), so why is the above needed in addition?

Quoted 5 lines
include/linux/pm.h +1 -1
+
 #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
 #define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))
 
--
#3 lore raw

Re: [PATCH v1 1/3] pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper

Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Andy Shevchenko

There are a few drivers and might be more in the future that open code the functionality of proposed DEFINE_NOIRQ_DEV_PM_OPS() helper. From now on they may switch to the new helper and save a few lines of code.

...

Quoted 6 lines

include/linux/pm.h const struct dev_pm_ops __maybe_unused name = { \ · line 450

Quoted 4 lines
include/linux/pm.h +4 -0
+#define DEFINE_NOIRQ_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+const struct dev_pm_ops name = { \
+       NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+}

There is NOIRQ_SYSTEM_SLEEP_PM_OPS(), so why is the above needed in addition?

It defines the constant object of struct dev_pm_ops type with this included and as the commit message says, allows to save a few lines of code in each of the drivers that uses NOIRQ_SYSTEM_SLEEP_PM_OPS() currently. The examples on how to convert are provided in the patches 2 and 3.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#4 lore raw

Re: [PATCH v1 1/3] pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper

Quoted 19 lines · Andy Shevchenko
Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Andy Shevchenko

There are a few drivers and might be more in the future that open code the functionality of proposed DEFINE_NOIRQ_DEV_PM_OPS() helper. From now on they may switch to the new helper and save a few lines of code.

...

Quoted 6 lines

include/linux/pm.h const struct dev_pm_ops __maybe_unused name = { \ · line 450

Quoted 4 lines
include/linux/pm.h +4 -0
+#define DEFINE_NOIRQ_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+const struct dev_pm_ops name = { \
+       NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+}

There is NOIRQ_SYSTEM_SLEEP_PM_OPS(), so why is the above needed in addition?

Quoted 5 lines

It defines the constant object of struct dev_pm_ops type with this included and as the commit message says, allows to save a few lines of code in each of the drivers that uses NOIRQ_SYSTEM_SLEEP_PM_OPS() currently. The examples on how to convert are provided in the patches 2 and 3.

So this is in analogy with _DEFINE_DEV_PM_OPS(), isn't it? It would be good to mention this in the changelog.

IMO the changelog is rather hard to follow in general and it should not refer to the changes made in order to understand what's going on.

author
#5 lore raw

PATCH 2/3 pinctrl: intel: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

In reply to Andy Shevchenko

Since pm.h provides a helper for system no-IRQ PM callbacks, switch the driver to use it instead of open coded variant.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/pinctrl/intel/pinctrl-intel.h | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/pinctrl/intel/pinctrl-intel.h +2 -4
index 350328988571..207ef71f4a7d 100644
@@ -261,9 +261,7 @@ int intel_pinctrl_probe_by_uid(struct platform_device *pdev);
 int intel_pinctrl_suspend_noirq(struct device *dev);
 int intel_pinctrl_resume_noirq(struct device *dev);
 
-#define INTEL_PINCTRL_PM_OPS(_name)								\
-const struct dev_pm_ops _name = {								\
-	NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pinctrl_suspend_noirq, intel_pinctrl_resume_noirq)	\
-}
+#define INTEL_PINCTRL_PM_OPS(name)								\
+	DEFINE_NOIRQ_DEV_PM_OPS((name), intel_pinctrl_suspend_noirq, intel_pinctrl_resume_noirq)
 
 #endif /* PINCTRL_INTEL_H */
Signature · 1 line
2.35.1
author
#6 lore raw

PATCH 3/3 pinctrl: cherryview: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

In reply to Andy Shevchenko

Since pm.h provides a helper for system no-IRQ PM callbacks, switch the driver to use it instead of open coded variant.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pinctrl/intel/pinctrl-cherryview.c +1 -3
index 917563cef965..ddb83a40cce5 100644
@@ -1875,9 +1875,7 @@ static int chv_pinctrl_resume_noirq(struct device *dev)
 	return 0;
 }
 
-static const struct dev_pm_ops chv_pinctrl_pm_ops = {
-	NOIRQ_SYSTEM_SLEEP_PM_OPS(chv_pinctrl_suspend_noirq, chv_pinctrl_resume_noirq)
-};
+static DEFINE_NOIRQ_DEV_PM_OPS(chv_pinctrl_pm_ops, chv_pinctrl_suspend_noirq, chv_pinctrl_resume_noirq);
 
 static const struct acpi_device_id chv_pinctrl_acpi_match[] = {
 	{ "INT33FF", (kernel_ulong_t)chv_soc_data },
Signature · 1 line
2.35.1