pinctrl: Provide NOIRQ PM helper and use it

_DEFINE_DEV_PM_OPS() helps to define PM operations for the system sleep
and/or runtime PM cases. Some of the existing users want to have _noirq()
variants to be set. For that purpose introduce a new helper which sets
up _noirq() callbacks to be set and struct dev_pm_ops be provided.

Signed-off-by Andy Shevchenko <[email protected]>

---
 include/linux/pm.h | 9 +++++++++

 1 file changed, 9 insertions(+)

include/linux/pm.h +0 -0

index badad7d11f4f..0f19af8d5493 100644
@@ -448,6 +448,15 @@ const struct dev_pm_ops __maybe_unused name = {
 \
         SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
  }

+/*
+ * Use this if you want to have the suspend and resume callbacks be called
+ * with disabled IRQs.

+ */

include/linux/pm.h +5 -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) \
+}
+

 #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr)) #define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

I'm not really sure that we need this macro, but I don't really object either. As long as it has callers I guess it's fine, I just don't want <linux/pm.h> to become too bloated and confusing.

Anyway:

Reviewed-by Paul Cercueil <[email protected]>

_DEFINE_DEV_PM_OPS() helps to define PM operations for the system sleep and/or runtime PM cases. Some of the existing users want to have _noirq() variants to be set. For that purpose introduce a new helper which sets up _noirq() callbacks to be set and struct dev_pm_ops be provided.

Signed-off-by Andy Shevchenko <[email protected]>

---
 include/linux/pm.h | 9 +++++++++
 1 file changed, 9 insertions(+)

include/linux/pm.h +9 -0

index badad7d11f4f..0f19af8d5493 100644
@@ -448,6 +448,15 @@ const struct dev_pm_ops __maybe_unused name = { \
 	SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
 }
 
+/*
+ * Use this if you want to have the suspend and resume callbacks be called
+ * with disabled IRQs.
+ */
+#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))
 

_DEFINE_DEV_PM_OPS() helps to define PM operations for the system sleep and/or runtime PM cases. Some of the existing users want to have _noirq() variants to be set. For that purpose introduce a new helper which sets up _noirq() callbacks to be set and struct dev_pm_ops be provided.

Signed-off-by Andy Shevchenko <[email protected]>

_DEFINE_DEV_PM_OPS() helps to define PM operations for the system sleep and/or runtime PM cases. Some of the existing users want to have _noirq() variants to be set. For that purpose introduce a new helper which sets up _noirq() callbacks to be set and struct dev_pm_ops be provided.

Signed-off-by Andy Shevchenko <[email protected]>

---
 include/linux/pm.h | 9 +++++++++
 1 file changed, 9 insertions(+)

include/linux/pm.h +9 -1

index badad7d11f4f..0f19af8d5493 100644
@@ -448,6 +448,15 @@ const struct dev_pm_ops __maybe_unused name = { \
        SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
 }
 
+/*
+ * Use this if you want to have the suspend and resume callbacks be called
+ * with disabled IRQs.
+ */
+#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))
 
--

2.40.0.1.gaa8946217a0b

Cleaning up the driver to use pm_ptr() and *_PM_OPS() macros that make it simpler and allows the compiler to remove those functions if built without CONFIG_PM and CONFIG_PM_SLEEP support.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/pinctrl/intel/pinctrl-baytrail.c | 11 +++--------

 1 file changed, 3 insertions(+), 8 deletions(-)

drivers/pinctrl/intel/pinctrl-baytrail.c +0 -0

index 27aef62fc7c0..66aabac6be9c 100644
@@ -1733,7 +1733,6 @@ static int byt_pinctrl_probe(struct
 platform_device *pdev)
         return 0;
  }

-#ifdef CONFIG_PM_SLEEP
 static int byt_gpio_suspend(struct device *dev) {
        struct intel_pinctrl *vg = dev_get_drvdata(dev);

drivers/pinctrl/intel/pinctrl-baytrail.c +0 -1

@@ -1817,9 +1816,7 @@ static int byt_gpio_resume(struct device *dev)
         raw_spin_unlock_irqrestore(&byt_lock, flags);
         return 0;
  }
-#endif

-#ifdef CONFIG_PM
 static int byt_gpio_runtime_suspend(struct device *dev) {
        return 0;

drivers/pinctrl/intel/pinctrl-baytrail.c +0 -1

@@ -1829,19 +1826,17 @@ static int byt_gpio_runtime_resume(struct
 device *dev)
  {
         return 0;
  }
-#endif

 static const struct dev_pm_ops byt_gpio_pm_ops = {
-       SET_LATE_SYSTEM_SLEEP_PM_OPS(byt_gpio_suspend, byt_gpio_resume)
-       SET_RUNTIME_PM_OPS(byt_gpio_runtime_suspend, byt_gpio_runtime_resume,
-                          NULL)
+       LATE_SYSTEM_SLEEP_PM_OPS(byt_gpio_suspend, byt_gpio_resume) +       RUNTIME_PM_OPS(byt_gpio_runtime_suspend, byt_gpio_runtime_resume, NULL)
 };

 static struct platform_driver byt_gpio_driver = { .probe          = byt_pinctrl_probe,
        .driver         = {
                .name                   = "byt_gpio",
-               .pm                     = &byt_gpio_pm_ops, +               .pm                     = pm_ptr(&byt_gpio_pm_ops), .acpi_match_table       = byt_gpio_acpi_match, .suppress_bind_attrs    = true,
        },

Cleaning up the driver to use pm_ptr() and *_PM_OPS() macros that make it simpler and allows the compiler to remove those functions if built without CONFIG_PM and CONFIG_PM_SLEEP support.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/pinctrl/intel/pinctrl-baytrail.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

drivers/pinctrl/intel/pinctrl-baytrail.c +3 -8

index 27aef62fc7c0..66aabac6be9c 100644
@@ -1733,7 +1733,6 @@ static int byt_pinctrl_probe(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
 static int byt_gpio_suspend(struct device *dev)
 {
 	struct intel_pinctrl *vg = dev_get_drvdata(dev);

@@ -1817,9 +1816,7 @@ static int byt_gpio_resume(struct device *dev)
 	raw_spin_unlock_irqrestore(&byt_lock, flags);
 	return 0;
 }
-#endif
 
-#ifdef CONFIG_PM
 static int byt_gpio_runtime_suspend(struct device *dev)
 {
 	return 0;

@@ -1829,19 +1826,17 @@ static int byt_gpio_runtime_resume(struct device *dev)
 {
 	return 0;
 }
-#endif
 
 static const struct dev_pm_ops byt_gpio_pm_ops = {
-	SET_LATE_SYSTEM_SLEEP_PM_OPS(byt_gpio_suspend, byt_gpio_resume)
-	SET_RUNTIME_PM_OPS(byt_gpio_runtime_suspend, byt_gpio_runtime_resume,
-			   NULL)
+	LATE_SYSTEM_SLEEP_PM_OPS(byt_gpio_suspend, byt_gpio_resume)
+	RUNTIME_PM_OPS(byt_gpio_runtime_suspend, byt_gpio_runtime_resume, NULL)
 };
 
 static struct platform_driver byt_gpio_driver = {
 	.probe          = byt_pinctrl_probe,
 	.driver         = {
 		.name			= "byt_gpio",
-		.pm			= &byt_gpio_pm_ops,
+		.pm			= pm_ptr(&byt_gpio_pm_ops),
 		.acpi_match_table	= byt_gpio_acpi_match,
 		.suppress_bind_attrs	= true,
 	},

Quoted 5 lines · On Mon, 17 Jul 2023 20:28:13 +0300 Andy Shevchenko…

Cleaning up the driver to use pm_ptr() and *_PM_OPS() macros that make it simpler and allows the compiler to remove those functions if built without CONFIG_PM and CONFIG_PM_SLEEP support.

Signed-off-by Andy Shevchenko <[email protected]>

LGTM but why is it in a set that claims to be about NOIRQ PM helper?

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 | 9 ++-------

 1 file changed, 2 insertions(+), 7 deletions(-)

drivers/pinctrl/intel/pinctrl-cherryview.c +0 -0

index eee0f9bc3d32..7a2fc9fe175d 100644
@@ -1793,7 +1793,6 @@ static int chv_pinctrl_remove(struct
 platform_device *pdev)
         return 0;
  }

-#ifdef CONFIG_PM_SLEEP
 static int chv_pinctrl_suspend_noirq(struct device *dev) {
        struct intel_pinctrl *pctrl = dev_get_drvdata(dev);

drivers/pinctrl/intel/pinctrl-cherryview.c +0 -0

@@ -1877,12 +1876,8 @@ static int chv_pinctrl_resume_noirq(struct
 device *dev)

        return 0;
 }
-#endif

drivers/pinctrl/intel/pinctrl-cherryview.c +1 -4

-static const struct dev_pm_ops chv_pinctrl_pm_ops = {
-       SET_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 },

drivers/pinctrl/intel/pinctrl-cherryview.c +1 -1

@@ -1895,7 +1890,7 @@ static struct platform_driver
 chv_pinctrl_driver = {
         .remove = chv_pinctrl_remove,
         .driver = {
                 .name = "cherryview-pinctrl",
-               .pm = &chv_pinctrl_pm_ops,
+               .pm = pm_sleep_ptr(&chv_pinctrl_pm_ops),
                 .acpi_match_table = chv_pinctrl_acpi_match,
         },

 };

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 2 lines

+static DEFINE_NOIRQ_DEV_PM_OPS(chv_pinctrl_pm_ops, chv_pinctrl_suspend_noirq, chv_pinctrl_resume_noirq);

That's over 100 characters, please break this line.

Quoted 1 line · Paul Cercueil

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

...

Quoted 4 lines

Quoted 2 lines

+static DEFINE_NOIRQ_DEV_PM_OPS(chv_pinctrl_pm_ops, chv_pinctrl_suspend_noirq, chv_pinctrl_resume_noirq);

That's over 100 characters, please break this line.

If it's a problem, I prefer to shorten the names of the callbacks. Would it work for you?

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 | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

drivers/pinctrl/intel/pinctrl-cherryview.c +0 -1

index eee0f9bc3d32..7a2fc9fe175d 100644
@@ -1793,7 +1793,6 @@ static int chv_pinctrl_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
 static int chv_pinctrl_suspend_noirq(struct device *dev)
 {
 	struct intel_pinctrl *pctrl = dev_get_drvdata(dev);

drivers/pinctrl/intel/pinctrl-cherryview.c +1 -1

 static const struct acpi_device_id chv_pinctrl_acpi_match[] = {
 	{ "INT33FF", (kernel_ulong_t)chv_soc_data },

@@ -1895,7 +1890,7 @@ static struct platform_driver chv_pinctrl_driver = {
 	.remove = chv_pinctrl_remove,
 	.driver = {
 		.name = "cherryview-pinctrl",
-		.pm = &chv_pinctrl_pm_ops,
+		.pm = pm_sleep_ptr(&chv_pinctrl_pm_ops),
 		.acpi_match_table = chv_pinctrl_acpi_match,
 	},
 };

On Mon, 17 Jul 2023 20:28:14 +0300
Andy Shevchenko <[email protected]> wrote:

Reviewed-by Jonathan Cameron <[email protected]>

Quoted 1 line

+ static DEFINE_NOIRQ_DEV_PM_OPS(chv_pinctrl_pm_ops, chv_pinctrl_suspend_noirq, chv_pinctrl_resume_noirq);

Very long line and readability not hurt by breaking it.

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.c | 5 +---- drivers/pinctrl/intel/pinctrl-intel.h | 9 ++-------

 2 files changed, 3 insertions(+), 11 deletions(-)

drivers/pinctrl/intel/pinctrl-intel.c +0 -0

index 64c3e62b4348..40e45c4889ee 100644
@@ -929,7 +929,7 @@ static int intel_gpio_to_pin(struct intel_pinctrl
 *pctrl, unsigned int offset,
   *
   * Return: a GPIO offset, or negative error code if translation

can't be done.
  */
-static __maybe_unused int intel_pin_to_gpio(struct intel_pinctrl *pctrl, int pin)
+static int intel_pin_to_gpio(struct intel_pinctrl *pctrl, int pin) {
        const struct intel_community *community;
        const struct intel_padgroup *padgrp;

drivers/pinctrl/intel/pinctrl-intel.c +0 -0

@@ -1488,7 +1488,6 @@ static int intel_pinctrl_pm_init(struct
 intel_pinctrl *pctrl)
         if (!communities)
                 return -ENOMEM;

-

        for (i = 0; i < pctrl->ncommunities; i++) { struct intel_community *community = &pctrl-

Quoted 1 line

communities[i];

                u32 *intmask, *hostown;

drivers/pinctrl/intel/pinctrl-intel.c +0 -0

@@ -1712,7 +1711,6 @@ const struct intel_pinctrl_soc_data
 *intel_pinctrl_get_soc_data(struct platform_
  }
  EXPORT_SYMBOL_GPL(intel_pinctrl_get_soc_data);

-#ifdef CONFIG_PM_SLEEP
 static bool __intel_gpio_is_direct_irq(u32 value) {
        return (value & PADCFG0_GPIROUTIOXAPIC) && (value & PADCFG0_GPIOTXDIS) &&

 MODULE_AUTHOR("Mathias Nyman <[email protected]>"); MODULE_AUTHOR("Mika Westerberg <[email protected]>");

drivers/pinctrl/intel/pinctrl-intel.h +0 -0

index 1faf2ada480a..65b1699a2ed5 100644
@@ -255,15 +255,10 @@ struct intel_pinctrl {
  int intel_pinctrl_probe_by_hid(struct platform_device *pdev);
  int intel_pinctrl_probe_by_uid(struct platform_device *pdev);

-#ifdef CONFIG_PM_SLEEP
 int intel_pinctrl_suspend_noirq(struct device *dev); int intel_pinctrl_resume_noirq(struct device *dev); -#endif

-#define
INTEL_PINCTRL_PM_OPS(_name)                                    \
-const struct dev_pm_ops _name =
{                                      \
- SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pinctrl_suspend_noirq, \
-
intel_pinctrl_resume_noirq)       \

drivers/pinctrl/intel/pinctrl-intel.h +1 -1

-}
+#define

INTEL_PINCTRL_PM_OPS(_name)
                  \
+       DEFINE_NOIRQ_DEV_PM_OPS(_name, intel_pinctrl_suspend_noirq, intel_pinctrl_resume_noirq)

 #endif /* PINCTRL_INTEL_H */

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Unrelated change.

So the correct way to update this driver would be to have a conditionally-exported dev_pm_ops structure:

EXPORT_GPL_DEV_PM_OPS(intel_pinctrl_pm_ops) = {
NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pinctrl_suspend_noirq,
intel_pinctrl_resume_noirq),
};

Then your two callbacks can be "static" and without #ifdef guards.

The resulting "intel_pinctrl_pm_ops" can be marked as "extern" in the pinctrl-intel.h without any guards, as long as it is only referenced with the pm_ptr() macro.

Quoted 1 line · Paul Cercueil

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

...

Quoted 1 line

Unrelated change.

OK.

...

Quoted 3 lines

So the correct way to update this driver would be to have a conditionally-exported dev_pm_ops structure:

Quoted 4 lines

EXPORT_GPL_DEV_PM_OPS(intel_pinctrl_pm_ops) = { NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pinctrl_suspend_noirq, intel_pinctrl_resume_noirq),
};

This looks ugly. I didn't know that EXPORT*PM_OPS designed that way, but it seems pm.h in such case needs EXPORT for NOIRQ case as well.

Quoted 7 lines

Then your two callbacks can be "static" and without #ifdef guards.

The resulting "intel_pinctrl_pm_ops" can be marked as "extern" in the
pinctrl-intel.h without any guards, as long as it is only referenced
with the pm_ptr() macro.

I'm not sure I got this. Currently drivers do not have any guards. Moreover, the correct one for noirq is pm_sleep_ptr(), isn't it?

Quoted 3 lines · Andy Shevchenko

Quoted 1 line · Paul Cercueil

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 10 lines

Quoted 7 lines

So the correct way to update this driver would be to have a conditionally-exported dev_pm_ops structure:

EXPORT_GPL_DEV_PM_OPS(intel_pinctrl_pm_ops) = {
NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pinctrl_suspend_noirq,
intel_pinctrl_resume_noirq),
};

This looks ugly. I didn't know that EXPORT*PM_OPS designed that way, but it seems pm.h in such case needs EXPORT for NOIRQ case as well.

It's designed so that when CONFIG_PM is disabled, the dev_pm_ops is garbage-collected along with all its callbacks.

I know it looks ugly, but we already have 4 variants (regular, namespace, GPL, namespace + GPL), if we start to add macros for specific use-cases then it will become bloated really quick.

And the "bloat" I'm trying to avoid here is the extreme expansion of the API which makes it hard for people not familiar to the code to understand what should be used and how.

Quoted 8 lines

Quoted 7 lines

Then your two callbacks can be "static" and without #ifdef guards.

The resulting "intel_pinctrl_pm_ops" can be marked as "extern" in the
pinctrl-intel.h without any guards, as long as it is only referenced
with the pm_ptr() macro.

Quoted 2 lines

I'm not sure I got this. Currently drivers do not have any guards. Moreover, the correct one for noirq is pm_sleep_ptr(), isn't it?

The EXPORT_*_DEV_PM_OPS() macros do export the "dev_pm_ops" conditionally depending on CONFIG_PM. We could add variants that export it conditionally depending on CONFIG_PM_SLEEP, but we're back at the problem of adding bloat.

You could use pm_sleep_ptr() indeed, with the existing macros, with the drawback that in the case where CONFIG_PM && !CONFIG_PM_SLEEP, the dev_pm_ops + callbacks are compiled in but never referenced.

Quoted 6 lines · Paul Cercueil

Quoted 5 lines · Andy Shevchenko

Quoted 2 lines · Paul Cercueil

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

...

Quoted 13 lines

Quoted 12 lines

Quoted 7 lines

So the correct way to update this driver would be to have a conditionally-exported dev_pm_ops structure:

EXPORT_GPL_DEV_PM_OPS(intel_pinctrl_pm_ops) = { NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pinctrl_suspend_noirq, intel_pinctrl_resume_noirq),
};

This looks ugly. I didn't know that EXPORT*PM_OPS designed that way,
but it seems pm.h in such case needs EXPORT for NOIRQ case as well.

Quoted 6 lines

It's designed so that when CONFIG_PM is disabled, the dev_pm_ops is garbage-collected along with all its callbacks.

I know it looks ugly, but we already have 4 variants (regular, namespace, GPL, namespace + GPL), if we start to add macros for specific use-cases then it will become bloated really quick.

Maybe macros can be replaced / changed to make it scale?

Quoted 5 lines

And the "bloat" I'm trying to avoid here is the extreme expansion of
the API which makes it hard for people not familiar to the code to understand what should be used and how.

So far, based on the rest of the messages in the thread the EXPORT*PM_OPS() have the following issues:
1) do not scale (for variants with different scope we need new set of macros);
2) do not cover cases with pm_sleep_ptr();
3) export symbols in case when it's not needed.

Am I right?

Quoted 15 lines

Quoted 11 lines

Quoted 10 lines

Then your two callbacks can be "static" and without #ifdef guards.

The resulting "intel_pinctrl_pm_ops" can be marked as "extern" in
the
pinctrl-intel.h without any guards, as long as it is only referenced
with the pm_ptr() macro.

Quoted 3 lines

I'm not sure I got this. Currently drivers do not have any guards.
Moreover, the correct one for noirq is pm_sleep_ptr(), isn't it?

Quoted 6 lines

The EXPORT_*_DEV_PM_OPS() macros do export the "dev_pm_ops" conditionally depending on CONFIG_PM. We could add variants that export
it conditionally depending on CONFIG_PM_SLEEP, but we're back at the
problem of adding bloat.

Exactly.

Quoted 4 lines

You could use pm_sleep_ptr() indeed, with the existing macros, with the
drawback that in the case where CONFIG_PM && !CONFIG_PM_SLEEP, the dev_pm_ops + callbacks are compiled in but never referenced.

And exactly.

I don't think they are ready to use (in the current form). But let's see what we may do better here...

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.c | 5 +----
 drivers/pinctrl/intel/pinctrl-intel.h | 9 ++-------
 2 files changed, 3 insertions(+), 11 deletions(-)

drivers/pinctrl/intel/pinctrl-intel.c +1 -3

index 64c3e62b4348..40e45c4889ee 100644
@@ -929,7 +929,7 @@ static int intel_gpio_to_pin(struct intel_pinctrl *pctrl, unsigned int offset,
  *
  * Return: a GPIO offset, or negative error code if translation can't be done.
  */
-static __maybe_unused int intel_pin_to_gpio(struct intel_pinctrl *pctrl, int pin)
+static int intel_pin_to_gpio(struct intel_pinctrl *pctrl, int pin)
 {
 	const struct intel_community *community;
 	const struct intel_padgroup *padgrp;

@@ -1488,7 +1488,6 @@ static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
 	if (!communities)
 		return -ENOMEM;
 
-
 	for (i = 0; i < pctrl->ncommunities; i++) {
 		struct intel_community *community = &pctrl->communities[i];
 		u32 *intmask, *hostown;

@@ -1712,7 +1711,6 @@ const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_
 }
 EXPORT_SYMBOL_GPL(intel_pinctrl_get_soc_data);
 
-#ifdef CONFIG_PM_SLEEP
 static bool __intel_gpio_is_direct_irq(u32 value)
 {
 	return (value & PADCFG0_GPIROUTIOXAPIC) && (value & PADCFG0_GPIOTXDIS) &&

drivers/pinctrl/intel/pinctrl-intel.c +0 -0

 MODULE_AUTHOR("Mathias Nyman <[email protected]>");
 MODULE_AUTHOR("Mika Westerberg <[email protected]>");

drivers/pinctrl/intel/pinctrl-intel.h +2 -7

index 1faf2ada480a..65b1699a2ed5 100644
@@ -255,15 +255,10 @@ struct intel_pinctrl {
 int intel_pinctrl_probe_by_hid(struct platform_device *pdev);
 int intel_pinctrl_probe_by_uid(struct platform_device *pdev);
 
-#ifdef CONFIG_PM_SLEEP
 int intel_pinctrl_suspend_noirq(struct device *dev);
 int intel_pinctrl_resume_noirq(struct device *dev);
-#endif
 
-#define INTEL_PINCTRL_PM_OPS(_name)					\
-const struct dev_pm_ops _name = {					\
-	SET_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 */

On Mon, 17 Jul 2023 20:28:15 +0300
Andy Shevchenko <[email protected]> wrote:

Quoted 1 line

EXPORT_SYMBOL_GPL(intel_pinctrl_resume_noirq);

Can you check if this is successfully removed? I think it won't be. Not immediately obvious how to tidy that up given these are used in a macro called from lots of drivers.

Maybe just leaving the ifdef is best we can do here.

Quoted 2 lines · Jonathan Cameron

On Mon, 17 Jul 2023 20:28:15 +0300
Andy Shevchenko <[email protected]> wrote:

...

Quoted 5 lines

Quoted 1 line

EXPORT_SYMBOL_GPL(intel_pinctrl_resume_noirq);

Can you check if this is successfully removed? I think it won't be. Not immediately obvious how to tidy that up given these are used in a macro called from lots of drivers.

That's what Paul noticed I think with his proposal to export only the ops variable and make these to be static.

Quoted 1 line

Maybe just leaving the ifdef is best we can do here.

See above.

Cleaning up the driver to use pm_ptr() and *_PM_OPS() macros that make it simpler and allows the compiler to remove those functions if built without CONFIG_PM and CONFIG_PM_SLEEP support.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/pinctrl/intel/pinctrl-lynxpoint.c | 7 +++----

 1 file changed, 3 insertions(+), 4 deletions(-)

drivers/pinctrl/intel/pinctrl-lynxpoint.c +0 -0

index cdace55aaeac..50d92bf80e20 100644
@@ -948,9 +948,8 @@ static int lp_gpio_resume(struct device *dev)
  }

 static const struct dev_pm_ops lp_gpio_pm_ops = {
-       .runtime_suspend = lp_gpio_runtime_suspend,
-       .runtime_resume = lp_gpio_runtime_resume,
-       .resume = lp_gpio_resume,
+       SYSTEM_SLEEP_PM_OPS(NULL, lp_gpio_resume)
+       RUNTIME_PM_OPS(lp_gpio_runtime_suspend, lp_gpio_runtime_resume, NULL)
 };

 static const struct acpi_device_id lynxpoint_gpio_acpi_match[] = {

drivers/pinctrl/intel/pinctrl-lynxpoint.c +1 -1

@@ -965,7 +964,7 @@ static struct platform_driver lp_gpio_driver = {
         .remove         = lp_gpio_remove,
         .driver         = {
                 .name   = "lp_gpio",
-               .pm     = &lp_gpio_pm_ops,
+               .pm     = pm_ptr(&lp_gpio_pm_ops),
                 .acpi_match_table = lynxpoint_gpio_acpi_match,
         },
  };

Cleaning up the driver to use pm_ptr() and *_PM_OPS() macros that make it simpler and allows the compiler to remove those functions if built without CONFIG_PM and CONFIG_PM_SLEEP support.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/pinctrl/intel/pinctrl-lynxpoint.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

drivers/pinctrl/intel/pinctrl-lynxpoint.c +3 -4

index cdace55aaeac..50d92bf80e20 100644
@@ -948,9 +948,8 @@ static int lp_gpio_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops lp_gpio_pm_ops = {
-	.runtime_suspend = lp_gpio_runtime_suspend,
-	.runtime_resume = lp_gpio_runtime_resume,
-	.resume = lp_gpio_resume,
+	SYSTEM_SLEEP_PM_OPS(NULL, lp_gpio_resume)
+	RUNTIME_PM_OPS(lp_gpio_runtime_suspend, lp_gpio_runtime_resume, NULL)
 };
 
 static const struct acpi_device_id lynxpoint_gpio_acpi_match[] = {

@@ -965,7 +964,7 @@ static struct platform_driver lp_gpio_driver = {
 	.remove         = lp_gpio_remove,
 	.driver         = {
 		.name   = "lp_gpio",
-		.pm	= &lp_gpio_pm_ops,
+		.pm	= pm_ptr(&lp_gpio_pm_ops),
 		.acpi_match_table = lynxpoint_gpio_acpi_match,
 	},
 };

Quoted 3 lines · On Mon, 17 Jul 2023 20:28:16 +0300 Andy Shevchenko…

Cleaning up the driver to use pm_ptr() and *_PM_OPS() macros that make it simpler and allows the compiler to remove those functions if built without CONFIG_PM and CONFIG_PM_SLEEP support.

Those macros

add a load more callbacks... Whilst that may well be fine, you should definitely mention that in this patch description.

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/pinctrl-at91.c | 10 ++++------

 1 file changed, 4 insertions(+), 6 deletions(-)

drivers/pinctrl/pinctrl-at91.c +0 -0

index 39956d821ad7..608f55c5ba5f 100644
@@ -1657,7 +1657,7 @@ static int gpio_irq_set_wake(struct irq_data
 *d, unsigned state)
         return 0;
  }

drivers/pinctrl/pinctrl-at91.c +1 -1

-static int __maybe_unused at91_gpio_suspend(struct device *dev)
+static int at91_gpio_suspend(struct device *dev)

 {
        struct at91_gpio_chip *at91_chip = dev_get_drvdata(dev); void __iomem *pio = at91_chip->regbase;

drivers/pinctrl/pinctrl-at91.c +0 -0

@@ -1675,7 +1675,7 @@ static int __maybe_unused
 at91_gpio_suspend(struct device *dev)
         return 0;
  }

drivers/pinctrl/pinctrl-at91.c +1 -1

-static int __maybe_unused at91_gpio_resume(struct device *dev)
+static int at91_gpio_resume(struct device *dev)

 {
        struct at91_gpio_chip *at91_chip = dev_get_drvdata(dev); void __iomem *pio = at91_chip->regbase;

drivers/pinctrl/pinctrl-at91.c +0 -0

@@ -1903,15 +1903,13 @@ static int at91_gpio_probe(struct
 platform_device *pdev)
         return 0;
  }

-static const struct dev_pm_ops at91_gpio_pm_ops = { -       NOIRQ_SYSTEM_SLEEP_PM_OPS(at91_gpio_suspend, at91_gpio_resume)

drivers/pinctrl/pinctrl-at91.c +1 -1

-};
+static DEFINE_NOIRQ_DEV_PM_OPS(at91_gpio_pm_ops, at91_gpio_suspend,

at91_gpio_resume);

 static struct platform_driver at91_gpio_driver = { .driver = {
                .name = "gpio-at91", .of_match_table = at91_gpio_of_match, -               .pm = pm_ptr(&at91_gpio_pm_ops), +               .pm = pm_sleep_ptr(&at91_gpio_pm_ops), },
        .probe = at91_gpio_probe,
 };

Quoted 4 lines · 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]>

Reviewed-by Paul Cercueil <[email protected]>

Although you could add a bit more info in the message of this patch, to explain why it's OK to remove the __maybe_unused tags (the code is always visible) and why switch from pm_ptr() to pm_sleep_ptr() (it's only used for system-PM callbacks).

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]>

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/pinctrl-at91.c | 10 ++++------
  1 file changed, 4 insertions(+), 6 deletions(-)

drivers/pinctrl/pinctrl-at91.c +4 -6

index 39956d821ad7..608f55c5ba5f 100644
@@ -1657,7 +1657,7 @@ static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
  	return 0;
  }
 
-static int __maybe_unused at91_gpio_suspend(struct device *dev)
+static int at91_gpio_suspend(struct device *dev)
  {
  	struct at91_gpio_chip *at91_chip = dev_get_drvdata(dev);
  	void __iomem *pio = at91_chip->regbase;

@@ -1675,7 +1675,7 @@ static int __maybe_unused at91_gpio_suspend(struct device *dev)
  	return 0;
  }
 
-static int __maybe_unused at91_gpio_resume(struct device *dev)
+static int at91_gpio_resume(struct device *dev)
  {
  	struct at91_gpio_chip *at91_chip = dev_get_drvdata(dev);
  	void __iomem *pio = at91_chip->regbase;

@@ -1903,15 +1903,13 @@ static int at91_gpio_probe(struct platform_device *pdev)
  	return 0;
  }
 
-static const struct dev_pm_ops at91_gpio_pm_ops = {
-	NOIRQ_SYSTEM_SLEEP_PM_OPS(at91_gpio_suspend, at91_gpio_resume)
-};
+static DEFINE_NOIRQ_DEV_PM_OPS(at91_gpio_pm_ops, at91_gpio_suspend, at91_gpio_resume);
 
  static struct platform_driver at91_gpio_driver = {
  	.driver = {
  		.name = "gpio-at91",
  		.of_match_table = at91_gpio_of_match,
-		.pm = pm_ptr(&at91_gpio_pm_ops),
+		.pm = pm_sleep_ptr(&at91_gpio_pm_ops),
  	},
  	.probe = at91_gpio_probe,
  };

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/mediatek/pinctrl-mtk-common.c | 5 +---- drivers/pinctrl/mediatek/pinctrl-paris.c      | 9 +++------

 2 files changed, 4 insertions(+), 10 deletions(-)

drivers/pinctrl/mediatek/pinctrl-mtk-common.c +0 -0

index 665dec419e7c..2bf5082d3aa9 100644
@@ -922,10 +922,7 @@ static int mtk_eint_resume(struct device
 *device)
         return mtk_eint_do_resume(pctl->eint);
  }

drivers/pinctrl/mediatek/pinctrl-mtk-common.c +1 -4

-const struct dev_pm_ops mtk_eint_pm_ops = {
-       .suspend_noirq = mtk_eint_suspend,
-       .resume_noirq = mtk_eint_resume,
-};
+DEFINE_NOIRQ_DEV_PM_OPS(mtk_eint_pm_ops, mtk_eint_suspend,

mtk_eint_resume);

 static int mtk_pctrl_build_state(struct platform_device *pdev) {

drivers/pinctrl/mediatek/pinctrl-paris.c +0 -0

index 33d6c3fb7908..b1cbd5bafa2e 100644
@@ -1119,24 +1119,21 @@ int mtk_paris_pinctrl_probe(struct
 platform_device *pdev)
  }
  EXPORT_SYMBOL_GPL(mtk_paris_pinctrl_probe);

drivers/pinctrl/mediatek/pinctrl-paris.c +1 -1

-static int mtk_paris_pinctrl_suspend(struct device *device)
+static int mtk_paris_suspend(struct device *device)

 {
        struct mtk_pinctrl *pctl = dev_get_drvdata(device);
        return mtk_eint_do_suspend(pctl->eint);
 }

drivers/pinctrl/mediatek/pinctrl-paris.c +1 -1

-static int mtk_paris_pinctrl_resume(struct device *device)
+static int mtk_paris_resume(struct device *device)

 {
        struct mtk_pinctrl *pctl = dev_get_drvdata(device);
        return mtk_eint_do_resume(pctl->eint);
 }

drivers/pinctrl/mediatek/pinctrl-paris.c +1 -4

-const struct dev_pm_ops mtk_paris_pinctrl_pm_ops = {
-       .suspend_noirq = mtk_paris_pinctrl_suspend,
-       .resume_noirq = mtk_paris_pinctrl_resume,
-};
+DEFINE_NOIRQ_DEV_PM_OPS(mtk_paris_pinctrl_pm_ops, mtk_paris_suspend,

mtk_paris_resume);

 MODULE_LICENSE("GPL v2"); MODULE_DESCRIPTION("MediaTek Pinctrl Common Driver V2 Paris");

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 2 lines

+DEFINE_NOIRQ_DEV_PM_OPS(mtk_paris_pinctrl_pm_ops, mtk_paris_suspend, mtk_paris_resume);

It's a bit more work, but I think you should use EXPORT_GPL_DEV_PM_OPS (or even better, EXPORT_NS_GPL_DEV_PM_OPS) so that the dev_pm_ops is conditionally exported. All callers would have to be updated to use pm_ptr().

Quoted 1 line · Paul Cercueil

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

...

Quoted 10 lines

Quoted 3 lines

+DEFINE_NOIRQ_DEV_PM_OPS(mtk_paris_pinctrl_pm_ops, mtk_paris_suspend,
mtk_paris_resume);

It's a bit more work, but I think you should use EXPORT_GPL_DEV_PM_OPS
(or even better, EXPORT_NS_GPL_DEV_PM_OPS) so that the dev_pm_ops is
conditionally exported. All callers would have to be updated to use pm_ptr().

Why pm_ptr()? What did I miss?
The rest is OK.

Quoted 3 lines · Andy Shevchenko

Quoted 1 line · Paul Cercueil

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 11 lines

Quoted 10 lines

Quoted 3 lines

+DEFINE_NOIRQ_DEV_PM_OPS(mtk_paris_pinctrl_pm_ops, mtk_paris_suspend,
mtk_paris_resume);

It's a bit more work, but I think you should use EXPORT_GPL_DEV_PM_OPS
(or even better, EXPORT_NS_GPL_DEV_PM_OPS) so that the dev_pm_ops is
conditionally exported. All callers would have to be updated to use pm_ptr().

Quoted 2 lines

Why pm_ptr()? What did I miss?
The rest is OK.

Or pm_sleep_ptr(). As I said in my answer to the other patch, EXPORT_*_DEV_PM_OPS() currently only exports on CONFIG_PM, so it doesn't really matter which one you use.

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/mediatek/pinctrl-mtk-common.c | 5 +----
  drivers/pinctrl/mediatek/pinctrl-paris.c      | 9 +++------
  2 files changed, 4 insertions(+), 10 deletions(-)

drivers/pinctrl/mediatek/pinctrl-mtk-common.c +1 -4

index 665dec419e7c..2bf5082d3aa9 100644
@@ -922,10 +922,7 @@ static int mtk_eint_resume(struct device *device)
  	return mtk_eint_do_resume(pctl->eint);
  }
 
-const struct dev_pm_ops mtk_eint_pm_ops = {
-	.suspend_noirq = mtk_eint_suspend,
-	.resume_noirq = mtk_eint_resume,
-};
+DEFINE_NOIRQ_DEV_PM_OPS(mtk_eint_pm_ops, mtk_eint_suspend, mtk_eint_resume);
 
  static int mtk_pctrl_build_state(struct platform_device *pdev)
  {

drivers/pinctrl/mediatek/pinctrl-paris.c +0 -0

index 33d6c3fb7908..b1cbd5bafa2e 100644
@@ -1119,24 +1119,21 @@ int mtk_paris_pinctrl_probe(struct platform_device *pdev)
  }
  EXPORT_SYMBOL_GPL(mtk_paris_pinctrl_probe);
 

drivers/pinctrl/mediatek/pinctrl-paris.c +1 -4

  {
  	struct mtk_pinctrl *pctl = dev_get_drvdata(device);
 
  	return mtk_eint_do_resume(pctl->eint);
  }
 
-const struct dev_pm_ops mtk_paris_pinctrl_pm_ops = {
-	.suspend_noirq = mtk_paris_pinctrl_suspend,
-	.resume_noirq = mtk_paris_pinctrl_resume,
-};
+DEFINE_NOIRQ_DEV_PM_OPS(mtk_paris_pinctrl_pm_ops, mtk_paris_suspend, mtk_paris_resume);
 
  MODULE_LICENSE("GPL v2");
  MODULE_DESCRIPTION("MediaTek Pinctrl Common Driver V2 Paris");

Il 17/07/23 19:28, Andy Shevchenko ha scritto:

drivers/pinctrl/mediatek/pinctrl-paris.c int mtk_paris_pinctrl_probe(struct platform_device *pdev) · line 1129

Quoted 5 lines

drivers/pinctrl/mediatek/pinctrl-paris.c +2 -2

-static int mtk_paris_pinctrl_suspend(struct device *device)
+static int mtk_paris_suspend(struct device *device)
 
-static int mtk_paris_pinctrl_resume(struct device *device)
+static int mtk_paris_resume(struct device *device)

What's the reason why you changed the suspend/resume function names? I don't really mind, but please at least mention that in the commit description.

Quoted 1 line

+DEFINE_NOIRQ_DEV_PM_OPS(mtk_paris_pinctrl_pm_ops, mtk_paris_suspend, mtk_paris_resume);

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/mediatek/pinctrl-mtk-common.c | 5 +----
 drivers/pinctrl/mediatek/pinctrl-paris.c      | 9 +++------
 2 files changed, 4 insertions(+), 10 deletions(-)

drivers/pinctrl/mediatek/pinctrl-mtk-common.c +1 -4

index 665dec419e7c..2bf5082d3aa9 100644
@@ -922,10 +922,7 @@ static int mtk_eint_resume(struct device *device)
 	return mtk_eint_do_resume(pctl->eint);
 }
 
-const struct dev_pm_ops mtk_eint_pm_ops = {
-	.suspend_noirq = mtk_eint_suspend,
-	.resume_noirq = mtk_eint_resume,
-};
+DEFINE_NOIRQ_DEV_PM_OPS(mtk_eint_pm_ops, mtk_eint_suspend, mtk_eint_resume);
 
 static int mtk_pctrl_build_state(struct platform_device *pdev)
 {

drivers/pinctrl/mediatek/pinctrl-paris.c +3 -6

index 33d6c3fb7908..b1cbd5bafa2e 100644
@@ -1119,24 +1119,21 @@ int mtk_paris_pinctrl_probe(struct platform_device *pdev)
 }
 EXPORT_SYMBOL_GPL(mtk_paris_pinctrl_probe);
 
-static int mtk_paris_pinctrl_suspend(struct device *device)
+static int mtk_paris_suspend(struct device *device)
 {
 	struct mtk_pinctrl *pctl = dev_get_drvdata(device);
 
 	return mtk_eint_do_suspend(pctl->eint);
 }
 
-static int mtk_paris_pinctrl_resume(struct device *device)
+static int mtk_paris_resume(struct device *device)
 {
 	struct mtk_pinctrl *pctl = dev_get_drvdata(device);
 
 	return mtk_eint_do_resume(pctl->eint);
 }
 
-const struct dev_pm_ops mtk_paris_pinctrl_pm_ops = {
-	.suspend_noirq = mtk_paris_pinctrl_suspend,
-	.resume_noirq = mtk_paris_pinctrl_resume,
-};
+DEFINE_NOIRQ_DEV_PM_OPS(mtk_paris_pinctrl_pm_ops, mtk_paris_suspend, mtk_paris_resume);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("MediaTek Pinctrl Common Driver V2 Paris");

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/mvebu/pinctrl-armada-37xx.c | 14 +++-----------

 1 file changed, 3 insertions(+), 11 deletions(-)

drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +0 -0

index 67c6751a6f06..46351c00ee73 100644
@@ -1013,7 +1013,6 @@ static int armada_37xx_pinctrl_register(struct
 platform_device *pdev,
         return 0;
  }

-#if defined(CONFIG_PM)
 static int armada_3700_pinctrl_suspend(struct device *dev) {
        struct armada_37xx_pinctrl *info = dev_get_drvdata(dev);

drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +0 -0

@@ -1107,15 +1106,8 @@ static int armada_3700_pinctrl_resume(struct
 device *dev)
   * Since pinctrl is an infrastructure module, its resume should be

issued prior
  * to other IO drivers.
  */

drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +2 -9

-static const struct dev_pm_ops armada_3700_pinctrl_pm_ops = {
-       .suspend_noirq = armada_3700_pinctrl_suspend,
-       .resume_noirq = armada_3700_pinctrl_resume,
-};
-
-#define PINCTRL_ARMADA_37XX_DEV_PM_OPS (&armada_3700_pinctrl_pm_ops)
-#else
-#define PINCTRL_ARMADA_37XX_DEV_PM_OPS NULL
-#endif /* CONFIG_PM */
+static DEFINE_NOIRQ_DEV_PM_OPS(armada_3700_pinctrl_pm_ops,
+                              armada_3700_pinctrl_suspend,

armada_3700_pinctrl_resume);

 static const struct of_device_id armada_37xx_pinctrl_of_match[] = { {

drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +1 -1

@@ -1182,7 +1174,7 @@ static struct platform_driver
 armada_37xx_pinctrl_driver = {
         .driver = {
                 .name = "armada-37xx-pinctrl",
                 .of_match_table = armada_37xx_pinctrl_of_match,
-               .pm = PINCTRL_ARMADA_37XX_DEV_PM_OPS,
+               .pm = pm_sleep_ptr(&armada_3700_pinctrl_pm_ops),
         },
  };

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]>

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/renesas/core.c | 16 +++++++---------

 1 file changed, 7 insertions(+), 9 deletions(-)

drivers/pinctrl/renesas/core.c +0 -0

index 0c8d081da6a8..34232b016960 100644
@@ -649,7 +649,7 @@ static const struct of_device_id
 sh_pfc_of_table[] = {
  };
  #endif

drivers/pinctrl/renesas/core.c +1 -1

-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARM_PSCI_FW)
+#if defined(CONFIG_ARM_PSCI_FW)

 static void sh_pfc_nop_reg(struct sh_pfc *pfc, u32 reg, unsigned int idx)
 {
 }

drivers/pinctrl/renesas/core.c +5 -7

@@ -732,15 +732,13 @@ static int sh_pfc_resume_noirq(struct device
 *dev)
                 sh_pfc_walk_regs(pfc, sh_pfc_restore_reg);
         return 0;
  }
-
-static const struct dev_pm_ops sh_pfc_pm  = {
-       SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(sh_pfc_suspend_noirq,
 sh_pfc_resume_noirq)
-};
-#define DEV_PM_OPS     &sh_pfc_pm
  #else
  static int sh_pfc_suspend_init(struct sh_pfc *pfc) { return 0; }
-#define DEV_PM_OPS     NULL
-#endif /* CONFIG_PM_SLEEP && CONFIG_ARM_PSCI_FW */
+static int sh_pfc_suspend_noirq(struct device *dev) { return 0; }
+static int sh_pfc_resume_noirq(struct device *dev) { return 0; }
+#endif /* CONFIG_ARM_PSCI_FW */
+
+static DEFINE_NOIRQ_DEV_PM_OPS(sh_pfc_pm, sh_pfc_suspend_noirq,
 sh_pfc_resume_noirq);

 #ifdef DEBUG
 #define SH_PFC_MAX_REGS                300

drivers/pinctrl/renesas/core.c +0 -0

         },
  };

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

I think you could do:

.pm = IF_PTR(IS_ENABLED(CONFIG_ARM_PSCI_FW), pm_sleep_ptr(&sh_pfc_pm)),

Then you wouldn't need the #if defined(CONFIG_ARM_PSCI_FW) guard either (as long as the code still compiles fine when that config option is disabled), and you wouldn't need those dummy callbacks.

Quoted 1 line · Paul Cercueil

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

...

Quoted 5 lines

I think you could do:

.pm = IF_PTR(IS_ENABLED(CONFIG_ARM_PSCI_FW),
pm_sleep_ptr(&sh_pfc_pm)),

Quoted 4 lines

Then you wouldn't need the #if defined(CONFIG_ARM_PSCI_FW) guard either
(as long as the code still compiles fine when that config option is disabled), and you wouldn't need those dummy callbacks.

Thanks for the hint, but it's ugly looking code.
If we go this direction, we would need local arm_psci_fw_ptr() macro or so.

Quoted 49 lines · 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/renesas/core.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

drivers/pinctrl/renesas/core.c +1 -1

index 0c8d081da6a8..34232b016960 100644
@@ -649,7 +649,7 @@ static const struct of_device_id
 sh_pfc_of_table[] = {
 };
 #endif
 
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARM_PSCI_FW)
+#if defined(CONFIG_ARM_PSCI_FW)
 static void sh_pfc_nop_reg(struct sh_pfc *pfc, u32 reg, unsigned int
 idx)

{
}

drivers/pinctrl/renesas/core.c +5 -7

@@ -732,15 +732,13 @@ static int sh_pfc_resume_noirq(struct device
 *dev)
                sh_pfc_walk_regs(pfc, sh_pfc_restore_reg);
        return 0;
 }
-
-static const struct dev_pm_ops sh_pfc_pm  = {
-       SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(sh_pfc_suspend_noirq,
 sh_pfc_resume_noirq)
-};
-#define DEV_PM_OPS     &sh_pfc_pm
 #else
 static int sh_pfc_suspend_init(struct sh_pfc *pfc) { return 0; }
-#define DEV_PM_OPS     NULL
-#endif /* CONFIG_PM_SLEEP && CONFIG_ARM_PSCI_FW */
+static int sh_pfc_suspend_noirq(struct device *dev) { return 0; }
+static int sh_pfc_resume_noirq(struct device *dev) { return 0; }
+#endif /* CONFIG_ARM_PSCI_FW */
+
+static DEFINE_NOIRQ_DEV_PM_OPS(sh_pfc_pm, sh_pfc_suspend_noirq,
 sh_pfc_resume_noirq);

#ifdef DEBUG
#define SH_PFC_MAX_REGS 300

drivers/pinctrl/renesas/core.c static struct platform_driver sh_pfc_driver = { · line 1419

Quoted 5 lines

drivers/pinctrl/renesas/core.c +1 -1

@@ -1418,7 +1416,7 @@ static struct platform_driver sh_pfc_driver = {
        .driver         = {
                .name   = DRV_NAME,
                .of_match_table = of_match_ptr(sh_pfc_of_table),
-               .pm     = DEV_PM_OPS,
+               .pm     = pm_sleep_ptr(&sh_pfc_pm),

I think you could do:

.pm = IF_PTR(IS_ENABLED(CONFIG_ARM_PSCI_FW), pm_sleep_ptr(&sh_pfc_pm)),

Then you wouldn't need the #if defined(CONFIG_ARM_PSCI_FW) guard either (as long as the code still compiles fine when that config option is disabled), and you wouldn't need those dummy callbacks.

Quoted 1 line · Paul Cercueil

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 7 lines

I think you could do:

.pm = IF_PTR(IS_ENABLED(CONFIG_ARM_PSCI_FW), pm_sleep_ptr(&sh_pfc_pm)),

Then you wouldn't need the #if defined(CONFIG_ARM_PSCI_FW) guard either (as long as the code still compiles fine when that config option is disabled), and you wouldn't need those dummy callbacks.

Unfortunately not, as the code refers to psci_ops.cpu_suspend.

You could create a small wrapper for that, though.

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]>

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]>

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/tegra/pinctrl-tegra.c | 5 +----

 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/pinctrl/tegra/pinctrl-tegra.c +0 -0

index 4547cf66d03b..734c71ef005b 100644
@@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct device
 *dev)
         return 0;
  }

drivers/pinctrl/tegra/pinctrl-tegra.c +1 -4

-const struct dev_pm_ops tegra_pinctrl_pm = {
-       .suspend_noirq = &tegra_pinctrl_suspend,
-       .resume_noirq = &tegra_pinctrl_resume
-};
+DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend,

tegra_pinctrl_resume);

 static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx *pmx) {

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more sense.

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 22 lines

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/tegra/pinctrl-tegra.c | 5 +----

 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/pinctrl/tegra/pinctrl-tegra.c +0 -0

index 4547cf66d03b..734c71ef005b 100644
@@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct device
 *dev)
         return 0;
  }

drivers/pinctrl/tegra/pinctrl-tegra.c +0 -4

-const struct dev_pm_ops tegra_pinctrl_pm = {
-       .suspend_noirq = &tegra_pinctrl_suspend,
-       .resume_noirq = &tegra_pinctrl_resume
-};

Quoted 5 lines

+DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend, tegra_pinctrl_resume);

 static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx *pmx) {

Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more sense.

Quoted 28 lines

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 25 lines

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/tegra/pinctrl-tegra.c | 5 +----

 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/pinctrl/tegra/pinctrl-tegra.c +0 -0

index 4547cf66d03b..734c71ef005b 100644
@@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct

device
*dev)
        return 0;
 }

drivers/pinctrl/tegra/pinctrl-tegra.c +1 -4

-const struct dev_pm_ops tegra_pinctrl_pm = {
-       .suspend_noirq = &tegra_pinctrl_suspend,
-       .resume_noirq = &tegra_pinctrl_resume
-};
+DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend,

Quoted 8 lines

Quoted 5 lines

tegra_pinctrl_resume);

 static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx *pmx)
 {

Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more sense.

We don't currently export these PM ops because none of the Tegra pinctrl
drivers can be built as a module.

Hi Thierry,

Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit :

Quoted 38 lines · Paul Cercueil

Quoted 36 lines

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 25 lines

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/tegra/pinctrl-tegra.c | 5 +----

 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/pinctrl/tegra/pinctrl-tegra.c +0 -0

index 4547cf66d03b..734c71ef005b 100644
@@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct

device
*dev)
        return 0;
 }

drivers/pinctrl/tegra/pinctrl-tegra.c +1 -4

-const struct dev_pm_ops tegra_pinctrl_pm = {
-       .suspend_noirq = &tegra_pinctrl_suspend,
-       .resume_noirq = &tegra_pinctrl_resume
-};
+DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend,

Quoted 5 lines

tegra_pinctrl_resume);

 static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx *pmx)
 {

Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more sense.

Quoted 3 lines

We don't currently export these PM ops because none of the Tegra pinctrl
drivers can be built as a module.

This doesn't change anything. You'd want to use EXPORT_GPL_DEV_PM_OPS (or better, the namespaced version) so that the PM ops can be defined in one file and referenced in another, while still having them garbage- collected when CONFIG_PM is disabled.

Quoted 49 lines

Hi Thierry,

Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit :

Quoted 42 lines · Paul Cercueil

Quoted 32 lines

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 28 lines

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/tegra/pinctrl-tegra.c | 5 +----

 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/pinctrl/tegra/pinctrl-tegra.c +0 -0

index 4547cf66d03b..734c71ef005b 100644
@@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct

device
*dev)
        return 0;
 }

drivers/pinctrl/tegra/pinctrl-tegra.c +1 -4

-const struct dev_pm_ops tegra_pinctrl_pm = {
-       .suspend_noirq = &tegra_pinctrl_suspend,
-       .resume_noirq = &tegra_pinctrl_resume
-};
+DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm,

tegra_pinctrl_suspend,
tegra_pinctrl_resume);

Quoted 8 lines

Quoted 4 lines

 static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx
*pmx)
 {

Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more
sense.

Quoted 3 lines

We don't currently export these PM ops because none of the Tegra pinctrl
drivers can be built as a module.

Quoted 7 lines

This doesn't change anything. You'd want to use EXPORT_GPL_DEV_PM_OPS
(or better, the namespaced version) so that the PM ops can be defined
in one file and referenced in another, while still having them garbage-
collected when CONFIG_PM is disabled.

Looking at the definition of EXPORT_GPL_DEV_PM_OPS(), it will cause an
EXPORT_SYMBOL_GPL() to be added. So there very well is a change. And it's a completely bogus change because no module is ever going to use that symbol. If we were to ever support building the pinctrl driver as
a module, then this would perhaps make sense, but we don't.

Quoted 1 line · Thierry Reding

On Tue, Jul 18, 2023 at 10:42:47AM +0200, Paul Cercueil wrote:

Quoted 7 lines

Looking at the definition of EXPORT_GPL_DEV_PM_OPS(), it will cause an
EXPORT_SYMBOL_GPL() to be added. So there very well is a change. And it's a completely bogus change because no module is ever going to use that symbol. If we were to ever support building the pinctrl driver as
a module, then this would perhaps make sense, but we don't.

In this particular case the EXPORT_SYMBOL_GPL() isn't really important, the rest of EXPORT_GPL_DEV_PM_OPS() is.

I don't think having a symbol exported it is a big deal, TBH, if you use the namespaced version. If you really don't want that, we need a version of EXPORT_GPL_DEV_PM_OPS() that doesn't export the symbol.

Le mardi 18 juillet 2023 à 13:41 +0200, Thierry Reding a écrit :

Quoted 58 lines · Paul Cercueil

Quoted 49 lines

Hi Thierry,

Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit :

Quoted 42 lines · Paul Cercueil

Quoted 32 lines

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 28 lines

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/tegra/pinctrl-tegra.c | 5 +----

 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/pinctrl/tegra/pinctrl-tegra.c +0 -0

index 4547cf66d03b..734c71ef005b 100644
@@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct

device
*dev)
        return 0;
 }

drivers/pinctrl/tegra/pinctrl-tegra.c +1 -4

-const struct dev_pm_ops tegra_pinctrl_pm = {
-       .suspend_noirq = &tegra_pinctrl_suspend,
-       .resume_noirq = &tegra_pinctrl_resume
-};
+DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm,

tegra_pinctrl_suspend,
tegra_pinctrl_resume);

Quoted 8 lines

Quoted 4 lines

 static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx
*pmx)
 {

Another driver where using EXPORT_GPL_DEV_PM_OPS() would make more
sense.

Quoted 3 lines

We don't currently export these PM ops because none of the Tegra pinctrl
drivers can be built as a module.

Quoted 7 lines

This doesn't change anything. You'd want to use EXPORT_GPL_DEV_PM_OPS
(or better, the namespaced version) so that the PM ops can be defined
in one file and referenced in another, while still having them garbage-
collected when CONFIG_PM is disabled.

Quoted 7 lines

Looking at the definition of EXPORT_GPL_DEV_PM_OPS(), it will cause an
EXPORT_SYMBOL_GPL() to be added. So there very well is a change. And it's a completely bogus change because no module is ever going to use that symbol. If we were to ever support building the pinctrl driver as
a module, then this would perhaps make sense, but we don't.

In this particular case the EXPORT_SYMBOL_GPL() isn't really important, the rest of EXPORT_GPL_DEV_PM_OPS() is.

I don't think having a symbol exported it is a big deal, TBH, if you use the namespaced version. If you really don't want that, we need a version of EXPORT_GPL_DEV_PM_OPS() that doesn't export the symbol.

Quoted 82 lines

Le mardi 18 juillet 2023 à 13:41 +0200, Thierry Reding a écrit :

Quoted 65 lines · Paul Cercueil

Quoted 56 lines

Hi Thierry,

Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit :

Quoted 47 lines

Quoted 36 lines · Paul Cercueil

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :

Quoted 32 lines

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/tegra/pinctrl-tegra.c | 5 +----

 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/pinctrl/tegra/pinctrl-tegra.c +0 -0

index 4547cf66d03b..734c71ef005b 100644
@@ -747,10 +747,7 @@ static int

tegra_pinctrl_resume(struct
device
*dev)
        return 0;
 }

drivers/pinctrl/tegra/pinctrl-tegra.c +1 -4

-const struct dev_pm_ops tegra_pinctrl_pm = {
-       .suspend_noirq = &tegra_pinctrl_suspend,
-       .resume_noirq = &tegra_pinctrl_resume
-};
+DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm,

tegra_pinctrl_suspend,
tegra_pinctrl_resume);

 static bool tegra_pinctrl_gpio_node_has_range(struct

Quoted 8 lines

Quoted 3 lines

tegra_pmx
*pmx)
 {

Another driver where using EXPORT_GPL_DEV_PM_OPS() would make
more
sense.

Quoted 4 lines

We don't currently export these PM ops because none of the Tegra
pinctrl
drivers can be built as a module.

Quoted 7 lines

This doesn't change anything. You'd want to use EXPORT_GPL_DEV_PM_OPS
(or better, the namespaced version) so that the PM ops can be defined
in one file and referenced in another, while still having them garbage-
collected when CONFIG_PM is disabled.

Quoted 11 lines

Looking at the definition of EXPORT_GPL_DEV_PM_OPS(), it will cause
an
EXPORT_SYMBOL_GPL() to be added. So there very well is a change. And
it's a completely bogus change because no module is ever going to use
that symbol. If we were to ever support building the pinctrl driver
as
a module, then this would perhaps make sense, but we don't.

In this particular case the EXPORT_SYMBOL_GPL() isn't really important,
the rest of EXPORT_GPL_DEV_PM_OPS() is.

Quoted 5 lines

I don't think having a symbol exported it is a big deal, TBH, if you
use the namespaced version. If you really don't want that, we need a
version of EXPORT_GPL_DEV_PM_OPS() that doesn't export the symbol.

I do think it's a big deal to export a symbol if there's no reason to do
so.

And please, can we stop adding these macros for every possible scenario?

Maybe I'm just getting old, but I find it increasingly difficult to understand what all of these are supposed to be. I get that people want
to get rid of boilerplate, but I think we need to more carefully balance
boilerplate vs. simplicity.

I'm seeing the same thing with stuff like those mass conversions to atrocities like devm_platform_ioremap_resource() and devm_platform_get_and_ioremap_resource(). There's so much churn involved
in getting those merged for usually saving a single line of code. And it's not even mass conversions, but people tend to send these as one patch per driver, which doesn't exactly help (except perhaps for patch
statistics).

Thierry

Le mardi 18 juillet 2023 à 09:45 +0200, Thierry Reding a écrit :

(or better, the namespaced version)

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/tegra/pinctrl-tegra.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

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/tegra/pinctrl-tegra.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/pinctrl/tegra/pinctrl-tegra.c +1 -4

index 4547cf66d03b..734c71ef005b 100644
@@ -747,10 +747,7 @@ static int tegra_pinctrl_resume(struct device *dev)
 	return 0;
 }
 
-const struct dev_pm_ops tegra_pinctrl_pm = {
-	.suspend_noirq = &tegra_pinctrl_suspend,
-	.resume_noirq = &tegra_pinctrl_resume
-};
+DEFINE_NOIRQ_DEV_PM_OPS(tegra_pinctrl_pm, tegra_pinctrl_suspend, tegra_pinctrl_resume);
 
 static bool tegra_pinctrl_gpio_node_has_range(struct tegra_pmx *pmx)
 {

Quoted 4 lines · On Mon, 17 Jul 2023 20:28:21 +0300 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]>

No pm_sleep_ptr()?

Quoted 6 lines

Quoted 3 lines · On Mon, 17 Jul 2023 20:28:21 +0300 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.

Quoted 4 lines

Quoted 2 lines

Signed-off-by: Andy Shevchenko
<[email protected]>

No pm_sleep_ptr()?

pm_sleep_ptr() is pointless on this driver. This driver is selected by
ARCH_TEGRA and ARCH_TEGRA also always selects PM.

Hi Thierry,

Le mardi 18 juillet 2023 à 13:38 +0200, Thierry Reding a écrit :

Quoted 10 lines · Jonathan Cameron

Quoted 9 lines

Quoted 5 lines · On Mon, 17 Jul 2023 20:28:21 +0300 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]>

Quoted 5 lines

Quoted 1 line

No pm_sleep_ptr()?

pm_sleep_ptr() is pointless on this driver. This driver is selected by
ARCH_TEGRA and ARCH_TEGRA also always selects PM.

If I'm not mistaken, ARCH_TEGRA selects CONFIG_PM, not CONFIG_PM_SLEEP.

Intel pin control drivers use NOIRQ variant of the PM callbacks. Besides that several other drivers do similar. Provide a helper to make them smaller and less error prone against different kernel configurations (with possible defined but not used variables).

The idea is to have an immutable branch that PM tree can pull as well as main pin control one. We also can do other way around, if Rafael prefers that.