[PATCH v2 00/10] pinctrl: Provide NOIRQ PM helper and use it

[PATCH v2 00/10] pinctrl: Provide NOIRQ PM helper and use it

[Andy Shevchenko]

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.

Changelog v2:
- rewritten commit message in patch 1 (Rafael)
- converted non-Intel pin control drivers as well
- added couple of kinda related patches to use pm_ptr()

Andy Shevchenko (10):
  pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper
  pinctrl: baytrail: Make use of pm_ptr()
  pinctrl: cherryview: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper
  pinctrl: intel: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper
  pinctrl: lynxpoint: Make use of pm_ptr()
  pinctrl: at91: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper
  pinctrl: mediatek: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper
  pinctrl: mvebu: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper
  pinctrl: renesas: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper
  pinctrl: tegra: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

 drivers/pinctrl/intel/pinctrl-baytrail.c      | 11 +++--------
 drivers/pinctrl/intel/pinctrl-cherryview.c    |  9 ++-------
 drivers/pinctrl/intel/pinctrl-intel.c         |  5 +----
 drivers/pinctrl/intel/pinctrl-intel.h         |  9 ++-------
 drivers/pinctrl/intel/pinctrl-lynxpoint.c     |  7 +++----
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c |  5 +----
 drivers/pinctrl/mediatek/pinctrl-paris.c      |  9 +++------
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c   | 14 +++-----------
 drivers/pinctrl/pinctrl-at91.c                | 10 ++++------
 drivers/pinctrl/renesas/core.c                | 16 +++++++---------
 drivers/pinctrl/tegra/pinctrl-tegra.c         |  5 +----
 include/linux/pm.h                            |  9 +++++++++
 12 files changed, 39 insertions(+), 70 deletions(-)

-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 01/10] pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper

[Andy Shevchenko]

_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 <andriy.shevchenko@linux.intel.com>
---
 include/linux/pm.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index badad7d11f4f..0f19af8d5493 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 01/10] pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper

[Paul Cercueil]

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> _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 <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/pm.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index badad7d11f4f..0f19af8d5493 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -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.

with disabled IRQs -> with IRQs disabled?

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 <paul@crapouillou.net>

Cheers,
-Paul

> + */
> +#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))
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 01/10] pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:12 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> _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 <andriy.shevchenko@linux.intel.com>

Seems reasonable to me given it is fairly common

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  include/linux/pm.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index badad7d11f4f..0f19af8d5493 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -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))
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 02/10] pinctrl: baytrail: Make use of pm_ptr()

[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 <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-baytrail.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c
index 27aef62fc7c0..66aabac6be9c 100644
--- a/drivers/pinctrl/intel/pinctrl-baytrail.c
+++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
@@ -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,
 	},
-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 02/10] pinctrl: baytrail: Make use of pm_ptr()

[Paul Cercueil]

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> 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 <andriy.shevchenko@linux.intel.com>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/pinctrl/intel/pinctrl-baytrail.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c
> b/drivers/pinctrl/intel/pinctrl-baytrail.c
> index 27aef62fc7c0..66aabac6be9c 100644
> --- a/drivers/pinctrl/intel/pinctrl-baytrail.c
> +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
> @@ -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,
>         },

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 02/10] pinctrl: baytrail: Make use of pm_ptr()

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:13 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> 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 <andriy.shevchenko@linux.intel.com>

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

FWIW
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/pinctrl/intel/pinctrl-baytrail.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c
> index 27aef62fc7c0..66aabac6be9c 100644
> --- a/drivers/pinctrl/intel/pinctrl-baytrail.c
> +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
> @@ -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,
>  	},


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 02/10] pinctrl: baytrail: Make use of pm_ptr()

[Andy Shevchenko]

On Tue, Jul 18, 2023 at 10:53:22AM +0100, Jonathan Cameron wrote:
> On Mon, 17 Jul 2023 20:28:13 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > 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 <andriy.shevchenko@linux.intel.com>
> 
> LGTM but why is it in a set that claims to be about NOIRQ PM helper?

Have you chance to read a cover letter? I explained that I added them as
they are "kinda" to the topic (PM related).

I can apply them and drop for next version.

Thank you for the review!

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 03/10] pinctrl: cherryview: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[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 <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index eee0f9bc3d32..7a2fc9fe175d 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -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);
@@ -1877,12 +1876,8 @@ static int chv_pinctrl_resume_noirq(struct device *dev)
 
 	return 0;
 }
-#endif
 
-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 },
@@ -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,
 	},
 };
-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 03/10] pinctrl: cherryview: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Paul Cercueil]

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> 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 <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c
> b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index eee0f9bc3d32..7a2fc9fe175d 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -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);
> @@ -1877,12 +1876,8 @@ static int chv_pinctrl_resume_noirq(struct
> device *dev)
>  
>         return 0;
>  }
> -#endif
>  
> -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);

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

Cheers,
-Paul

>  
>  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,
>         },
>  };


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 03/10] pinctrl: cherryview: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:14 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> 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 <andriy.shevchenko@linux.intel.com>
Formatting comment inline. Otherwise LGTM

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index eee0f9bc3d32..7a2fc9fe175d 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -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);
> @@ -1877,12 +1876,8 @@ static int chv_pinctrl_resume_noirq(struct device *dev)
>  
>  	return 0;
>  }
> -#endif
>  
> -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);

Very long line and readability not hurt by breaking it.

>  
>  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,
>  	},
>  };


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 03/10] pinctrl: cherryview: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Andy Shevchenko]

On Tue, Jul 18, 2023 at 10:54:53AM +0100, Jonathan Cameron wrote:
> On Mon, 17 Jul 2023 20:28:14 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

...

> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks!

...

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

Sure.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 04/10] pinctrl: intel: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

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

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 64c3e62b4348..40e45c4889ee 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -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) &&
@@ -1913,7 +1911,6 @@ int intel_pinctrl_resume_noirq(struct device *dev)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(intel_pinctrl_resume_noirq);
-#endif
 
 MODULE_AUTHOR("Mathias Nyman <mathias.nyman@linux.intel.com>");
 MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index 1faf2ada480a..65b1699a2ed5 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -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 */
-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 04/10] pinctrl: intel: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Paul Cercueil]

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> 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 <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 5 +----
>  drivers/pinctrl/intel/pinctrl-intel.h | 9 ++-------
>  2 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> b/drivers/pinctrl/intel/pinctrl-intel.c
> index 64c3e62b4348..40e45c4889ee 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -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;
>  
> -

Unrelated change.

>         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) &&
> @@ -1913,7 +1911,6 @@ int intel_pinctrl_resume_noirq(struct device
> *dev)
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(intel_pinctrl_resume_noirq);
> -#endif

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.

Cheers,
-Paul

>  
>  MODULE_AUTHOR("Mathias Nyman <mathias.nyman@linux.intel.com>");
>  MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.h
> b/drivers/pinctrl/intel/pinctrl-intel.h
> index 1faf2ada480a..65b1699a2ed5 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.h
> +++ b/drivers/pinctrl/intel/pinctrl-intel.h
> @@ -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 */


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 04/10] pinctrl: intel: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:15 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> 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 <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 5 +----
>  drivers/pinctrl/intel/pinctrl-intel.h | 9 ++-------
>  2 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index 64c3e62b4348..40e45c4889ee 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -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) &&
> @@ -1913,7 +1911,6 @@ int intel_pinctrl_resume_noirq(struct device *dev)
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(intel_pinctrl_resume_noirq);
> -#endif

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.



>  
>  MODULE_AUTHOR("Mathias Nyman <mathias.nyman@linux.intel.com>");
>  MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
> index 1faf2ada480a..65b1699a2ed5 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.h
> +++ b/drivers/pinctrl/intel/pinctrl-intel.h
> @@ -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 */


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 04/10] pinctrl: intel: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Andy Shevchenko]

On Tue, Jul 18, 2023 at 11:04:51AM +0100, Jonathan Cameron wrote:
> On Mon, 17 Jul 2023 20:28:15 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

...

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

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

See above.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 04/10] pinctrl: intel: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Jonathan Cameron]

On Tue, 18 Jul 2023 16:53:29 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Tue, Jul 18, 2023 at 11:04:51AM +0100, Jonathan Cameron wrote:
> > On Mon, 17 Jul 2023 20:28:15 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:  
> 
> ...
> 
> > >  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.
> 
> > Maybe just leaving the ifdef is best we can do here.  
> 
> See above.
> 
Ah. I noticed it was a macro, but not that all it did was
set the name of the resulting structure (so thought you couldn't
use the export approach).

Indeed that's the best option here

Jonathan



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 05/10] pinctrl: lynxpoint: Make use of pm_ptr()

[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 <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-lynxpoint.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-lynxpoint.c b/drivers/pinctrl/intel/pinctrl-lynxpoint.c
index cdace55aaeac..50d92bf80e20 100644
--- a/drivers/pinctrl/intel/pinctrl-lynxpoint.c
+++ b/drivers/pinctrl/intel/pinctrl-lynxpoint.c
@@ -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,
 	},
 };
-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/10] pinctrl: lynxpoint: Make use of pm_ptr()

[Paul Cercueil]

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> 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 <andriy.shevchenko@linux.intel.com>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/pinctrl/intel/pinctrl-lynxpoint.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-lynxpoint.c
> b/drivers/pinctrl/intel/pinctrl-lynxpoint.c
> index cdace55aaeac..50d92bf80e20 100644
> --- a/drivers/pinctrl/intel/pinctrl-lynxpoint.c
> +++ b/drivers/pinctrl/intel/pinctrl-lynxpoint.c
> @@ -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,
>         },
>  };

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/10] pinctrl: lynxpoint: Make use of pm_ptr()

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:16 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

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

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-lynxpoint.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-lynxpoint.c b/drivers/pinctrl/intel/pinctrl-lynxpoint.c
> index cdace55aaeac..50d92bf80e20 100644
> --- a/drivers/pinctrl/intel/pinctrl-lynxpoint.c
> +++ b/drivers/pinctrl/intel/pinctrl-lynxpoint.c
> @@ -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,
>  	},
>  };


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/10] pinctrl: lynxpoint: Make use of pm_ptr()

[Andy Shevchenko]

On Tue, Jul 18, 2023 at 11:06:20AM +0100, Jonathan Cameron wrote:
> On Mon, 17 Jul 2023 20:28:16 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > 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

I believe you meant here "The SYSTEM_SLEEP... macro..."

Or is runtime PM also altered? Hmm...

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

Sure.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 06/10] pinctrl: at91: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[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 <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-at91.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 39956d821ad7..608f55c5ba5f 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -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,
 };
-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/10] pinctrl: at91: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Paul Cercueil]

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> 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 <andriy.shevchenko@linux.intel.com>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

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).

Cheers,
-Paul

> ---
>  drivers/pinctrl/pinctrl-at91.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-at91.c
> b/drivers/pinctrl/pinctrl-at91.c
> index 39956d821ad7..608f55c5ba5f 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -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,
>  };


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/10] pinctrl: at91: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:17 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> 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 <andriy.shevchenko@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/10] pinctrl: at91: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[claudiu beznea]

On 17.07.2023 20:28, Andy Shevchenko wrote:
> 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 <andriy.shevchenko@linux.intel.com>

Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>

> ---
>   drivers/pinctrl/pinctrl-at91.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 39956d821ad7..608f55c5ba5f 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -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,
>   };

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 07/10] pinctrl: mediatek: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

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

diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
index 665dec419e7c..2bf5082d3aa9 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
@@ -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)
 {
diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c
index 33d6c3fb7908..b1cbd5bafa2e 100644
--- a/drivers/pinctrl/mediatek/pinctrl-paris.c
+++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
@@ -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");
-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 07/10] pinctrl: mediatek: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Paul Cercueil]

Hi Andy,


Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> 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 <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 5 +----
>  drivers/pinctrl/mediatek/pinctrl-paris.c      | 9 +++------
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> index 665dec419e7c..2bf5082d3aa9 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> @@ -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)
>  {
> diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c
> b/drivers/pinctrl/mediatek/pinctrl-paris.c
> index 33d6c3fb7908..b1cbd5bafa2e 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-paris.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
> @@ -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);

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().

Cheers,
-Paul

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 07/10] pinctrl: mediatek: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[AngeloGioacchino Del Regno]

Il 17/07/23 19:28, Andy Shevchenko ha scritto:
> 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 <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 5 +----
>   drivers/pinctrl/mediatek/pinctrl-paris.c      | 9 +++------
>   2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> index 665dec419e7c..2bf5082d3aa9 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> @@ -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)
>   {
> diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c
> index 33d6c3fb7908..b1cbd5bafa2e 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-paris.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
> @@ -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)

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.

Thanks,
Angelo

>   {
>   	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");


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 07/10] pinctrl: mediatek: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:18 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

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

Good to mention the renames as well.

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 5 +----
>  drivers/pinctrl/mediatek/pinctrl-paris.c      | 9 +++------
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> index 665dec419e7c..2bf5082d3aa9 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> @@ -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)
>  {
> diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c
> index 33d6c3fb7908..b1cbd5bafa2e 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-paris.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
> @@ -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");


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 08/10] pinctrl: mvebu: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[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 <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 67c6751a6f06..46351c00ee73 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -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);
@@ -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.
  */
-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[] = {
 	{
@@ -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),
 	},
 };
 
-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 08/10] pinctrl: mvebu: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Paul Cercueil]

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> 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 <andriy.shevchenko@linux.intel.com>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index 67c6751a6f06..46351c00ee73 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -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);
> @@ -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.
>   */
> -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[] = {
>         {
> @@ -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),
>         },
>  };
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 08/10] pinctrl: mvebu: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:19 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> 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 <andriy.shevchenko@linux.intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 09/10] pinctrl: renesas: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[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 <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/renesas/core.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/pinctrl/renesas/core.c b/drivers/pinctrl/renesas/core.c
index 0c8d081da6a8..34232b016960 100644
--- a/drivers/pinctrl/renesas/core.c
+++ b/drivers/pinctrl/renesas/core.c
@@ -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)
 {
 }
@@ -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
@@ -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),
 	},
 };
 
-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 09/10] pinctrl: renesas: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Paul Cercueil]

Hi Andy,


Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> 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 <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/renesas/core.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pinctrl/renesas/core.c
> b/drivers/pinctrl/renesas/core.c
> index 0c8d081da6a8..34232b016960 100644
> --- a/drivers/pinctrl/renesas/core.c
> +++ b/drivers/pinctrl/renesas/core.c
> @@ -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)
>  {
>  }
> @@ -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
> @@ -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.

Cheers,
-Paul

>         },
>  };
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 09/10] pinctrl: renesas: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:20 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> 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 <andriy.shevchenko@linux.intel.com>
Slightly more complex case, but looks fine to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 10/10] pinctrl: tegra: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[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 <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index 4547cf66d03b..734c71ef005b 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -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)
 {
-- 
2.40.0.1.gaa8946217a0b


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 10/10] pinctrl: tegra: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Paul Cercueil]

Hi Andy,

Le lundi 17 juillet 2023 à 20:28 +0300, Andy Shevchenko a écrit :
> 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 <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
> b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index 4547cf66d03b..734c71ef005b 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> @@ -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)
>  {

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

Cheers,
-Paul

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 10/10] pinctrl: tegra: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Jonathan Cameron]

On Mon, 17 Jul 2023 20:28:21 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> 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 <andriy.shevchenko@linux.intel.com>

No pm_sleep_ptr()?

> ---
>  drivers/pinctrl/tegra/pinctrl-tegra.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index 4547cf66d03b..734c71ef005b 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> @@ -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)
>  {


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 10/10] pinctrl: tegra: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 541 bytes --]

On Tue, Jul 18, 2023 at 11:11:43AM +0100, Jonathan Cameron wrote:
> On Mon, 17 Jul 2023 20:28:21 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > 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 <andriy.shevchenko@linux.intel.com>
> 
> 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.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 10/10] pinctrl: tegra: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Paul Cercueil]

Hi Thierry,

Le mardi 18 juillet 2023 à 13:38 +0200, Thierry Reding a écrit :
> On Tue, Jul 18, 2023 at 11:11:43AM +0100, Jonathan Cameron wrote:
> > On Mon, 17 Jul 2023 20:28:21 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > 
> > > 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
> > > <andriy.shevchenko@linux.intel.com>
> > 
> > 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.

Cheers,
-Paul

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 10/10] pinctrl: tegra: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() helper

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 900 bytes --]

On Tue, Jul 18, 2023 at 02:01:27PM +0200, Paul Cercueil wrote:
> Hi Thierry,
> 
> Le mardi 18 juillet 2023 à 13:38 +0200, Thierry Reding a écrit :
> > On Tue, Jul 18, 2023 at 11:11:43AM +0100, Jonathan Cameron wrote:
> > > On Mon, 17 Jul 2023 20:28:21 +0300
> > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > 
> > > > 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
> > > > <andriy.shevchenko@linux.intel.com>
> > > 
> > > 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.

Indeed. I suppose pm_sleep_ptr() would make sense, then.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 00/10] pinctrl: Provide NOIRQ PM helper and use it

[Andy Shevchenko]

On Mon, Jul 17, 2023 at 08:28:11PM +0300, Andy Shevchenko wrote:
> 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.

I have partially applied the series to my review and testing queue with
the following changes (besides the tags added):
- split pm_ptr() patches to be first with lynxpoint commit message updated
- fixed wording in the pm.h comment
- amended cherryview to wrap long line
- explained __maybe_unused and pm_ptr() changes in at91 commit message
- added pm_sleep_ptr() and explained that in the tegra commit message
- renesas and mvebu went as is
- intel and mediatek left aside for better rework

 drivers/pinctrl/intel/pinctrl-baytrail.c    | 11 +++--------
 drivers/pinctrl/intel/pinctrl-cherryview.c  | 10 +++-------
 drivers/pinctrl/intel/pinctrl-lynxpoint.c   |  7 +++----
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 14 +++-----------
 drivers/pinctrl/pinctrl-at91.c              | 10 ++++------
 drivers/pinctrl/renesas/core.c              | 16 +++++++---------
 drivers/pinctrl/tegra/pinctrl-tegra.c       |  5 +----
 drivers/pinctrl/tegra/pinctrl-tegra210.c    |  2 +-
 include/linux/pm.h                          |  9 +++++++++
 9 files changed, 34 insertions(+), 50 deletions(-)


-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel