2022-12-29 13:04:31

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 0/3] pinctrl: intel: Provide NOIRQ PM helper and use it

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

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

drivers/pinctrl/intel/pinctrl-cherryview.c | 4 +---
drivers/pinctrl/intel/pinctrl-intel.h | 6 ++----
include/linux/pm.h | 5 +++++
3 files changed, 8 insertions(+), 7 deletions(-)

--
2.35.1


2022-12-29 13:08:35

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 3/3] pinctrl: cherryview: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() 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 | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 917563cef965..ddb83a40cce5 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1875,9 +1875,7 @@ static int chv_pinctrl_resume_noirq(struct device *dev)
return 0;
}

-static const struct dev_pm_ops chv_pinctrl_pm_ops = {
- NOIRQ_SYSTEM_SLEEP_PM_OPS(chv_pinctrl_suspend_noirq, chv_pinctrl_resume_noirq)
-};
+static DEFINE_NOIRQ_DEV_PM_OPS(chv_pinctrl_pm_ops, chv_pinctrl_suspend_noirq, chv_pinctrl_resume_noirq);

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

2022-12-29 13:10:02

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/3] pm: Introduce DEFINE_NOIRQ_DEV_PM_OPS() helper

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

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

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 93cd34f00822..eba96822b1d9 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -444,6 +444,11 @@ const struct dev_pm_ops __maybe_unused name = { \
SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
}

+#define DEFINE_NOIRQ_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+const struct dev_pm_ops name = { \
+ NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+}
+
#define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
#define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))

--
2.35.1

2022-12-29 13:14:11

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 2/3] pinctrl: intel: Switch to use DEFINE_NOIRQ_DEV_PM_OPS() 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-intel.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index 350328988571..207ef71f4a7d 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -261,9 +261,7 @@ int intel_pinctrl_probe_by_uid(struct platform_device *pdev);
int intel_pinctrl_suspend_noirq(struct device *dev);
int intel_pinctrl_resume_noirq(struct device *dev);

-#define INTEL_PINCTRL_PM_OPS(_name) \
-const struct dev_pm_ops _name = { \
- NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pinctrl_suspend_noirq, intel_pinctrl_resume_noirq) \
-}
+#define INTEL_PINCTRL_PM_OPS(name) \
+ DEFINE_NOIRQ_DEV_PM_OPS((name), intel_pinctrl_suspend_noirq, intel_pinctrl_resume_noirq)

#endif /* PINCTRL_INTEL_H */
--
2.35.1

2022-12-30 19:24:40

by Rafael J. Wysocki

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

On Thu, Dec 29, 2022 at 1:59 PM Andy Shevchenko
<[email protected]> wrote:
>
> There are a few drivers and might be more in the future that
> open code the functionality of proposed DEFINE_NOIRQ_DEV_PM_OPS()
> helper. From now on they may switch to the new helper and save
> a few lines of code.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> include/linux/pm.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 93cd34f00822..eba96822b1d9 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -444,6 +444,11 @@ const struct dev_pm_ops __maybe_unused name = { \
> SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> }
>
> +#define DEFINE_NOIRQ_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> +const struct dev_pm_ops name = { \
> + NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +}

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

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

2022-12-30 19:46:26

by Andy Shevchenko

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

On Fri, Dec 30, 2022 at 8:43 PM Rafael J. Wysocki <[email protected]> wrote:
> On Thu, Dec 29, 2022 at 1:59 PM Andy Shevchenko
> <[email protected]> wrote:
> >
> > There are a few drivers and might be more in the future that
> > open code the functionality of proposed DEFINE_NOIRQ_DEV_PM_OPS()
> > helper. From now on they may switch to the new helper and save
> > a few lines of code.

...

> > +#define DEFINE_NOIRQ_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> > +const struct dev_pm_ops name = { \
> > + NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> > +}
>
> There is NOIRQ_SYSTEM_SLEEP_PM_OPS(), so why is the above needed in addition?

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

--
With Best Regards,
Andy Shevchenko

2022-12-30 19:49:26

by Rafael J. Wysocki

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

On Fri, Dec 30, 2022 at 8:23 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Fri, Dec 30, 2022 at 8:43 PM Rafael J. Wysocki <[email protected]> wrote:
> > On Thu, Dec 29, 2022 at 1:59 PM Andy Shevchenko
> > <[email protected]> wrote:
> > >
> > > There are a few drivers and might be more in the future that
> > > open code the functionality of proposed DEFINE_NOIRQ_DEV_PM_OPS()
> > > helper. From now on they may switch to the new helper and save
> > > a few lines of code.
>
> ...
>
> > > +#define DEFINE_NOIRQ_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> > > +const struct dev_pm_ops name = { \
> > > + NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> > > +}
> >
> > There is NOIRQ_SYSTEM_SLEEP_PM_OPS(), so why is the above needed in addition?
>
> It defines the constant object of struct dev_pm_ops type with this
> included and as the commit message says, allows to save a few lines of
> code in each of the drivers that uses NOIRQ_SYSTEM_SLEEP_PM_OPS()
> currently. The examples on how to convert are provided in the patches
> 2 and 3.

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

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