2022-01-07 18:17:34

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 0/6] DEV_PM_OPS macros rework v3

Hi,

A V2 of my patchset that tweaks a bit the *_DEV_PM_OPS() macros that
were introduced recently.

Changes since V2:
* [1/6]: - Keep UNIVERSAL_DEV_PM_OPS() macro deprecated
- Rework commit message
* [3/6]: - Reorder the code to have non-private macros together in the
file
- Add comment about the necesity to use the new export macro
when the dev_pm_ops has to be exported
* [5/6]: Add comment about the necesity to use the new export macro
when the dev_pm_ops has to be exported

Cheers,
-Paul

Paul Cercueil (6):
PM: core: Remove DEFINE_UNIVERSAL_DEV_PM_OPS() macro
PM: core: Remove static qualifier in DEFINE_SIMPLE_DEV_PM_OPS macro
PM: core: Add EXPORT[_GPL]_SIMPLE_DEV_PM_OPS macros
PM: runtime: Add DEFINE_RUNTIME_DEV_PM_OPS() macro
PM: runtime: Add EXPORT[_GPL]_RUNTIME_DEV_PM_OPS macros
iio: pressure: bmp280: Use new PM macros

drivers/iio/pressure/bmp280-core.c | 11 ++----
drivers/iio/pressure/bmp280-i2c.c | 2 +-
drivers/iio/pressure/bmp280-spi.c | 2 +-
drivers/mmc/host/jz4740_mmc.c | 4 +--
drivers/mmc/host/mxcmmc.c | 2 +-
include/linux/pm.h | 55 ++++++++++++++++++++++--------
include/linux/pm_runtime.h | 24 +++++++++++++
7 files changed, 71 insertions(+), 29 deletions(-)

--
2.34.1



2022-01-07 18:17:49

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 1/6] PM: core: Remove DEFINE_UNIVERSAL_DEV_PM_OPS() macro

The deprecated UNIVERSAL_DEV_PM_OPS() macro uses the provided callbacks
for both runtime PM and system sleep, which is very likely to be a
mistake, as a system sleep can be triggered while a given device is
already PM-suspended, which would cause the suspend callback to be
called twice.

The amount of users of UNIVERSAL_DEV_PM_OPS() is also tiny (16
occurences) compared to the number of places where
SET_SYSTEM_SLEEP_PM_OPS() is used with pm_runtime_force_suspend() and
pm_runtime_force_resume(), which makes me think that none of these cases
are actually valid.

As the new macro DEFINE_UNIVERSAL_DEV_PM_OPS() which was introduced to
replace UNIVERSAL_DEV_PM_OPS() is currently unused, remove it before
someone starts to use it in yet another invalid case.

Signed-off-by: Paul Cercueil <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---

Notes:
v2: No change
v3: - Keep UNIVERSAL_DEV_PM_OPS deprecated
- Rework commit message

include/linux/pm.h | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index e1e9402180b9..02f059d814bb 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -366,6 +366,12 @@ static const struct dev_pm_ops name = { \
SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
}

+/* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+const struct dev_pm_ops __maybe_unused name = { \
+ SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+}
+
/*
* Use this for defining a set of PM operations to be used in all situations
* (system suspend, hibernation or runtime PM).
@@ -378,20 +384,9 @@ static const struct dev_pm_ops name = { \
* suspend and "early" resume callback pointers, .suspend_late() and
* .resume_early(), to the same routines as .runtime_suspend() and
* .runtime_resume(), respectively (and analogously for hibernation).
+ *
+ * Deprecated. You most likely don't want this macro.
*/
-#define DEFINE_UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
-static const struct dev_pm_ops name = { \
- SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
- RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
-}
-
-/* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
-#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-const struct dev_pm_ops __maybe_unused name = { \
- SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
-}
-
-/* Deprecated. Use DEFINE_UNIVERSAL_DEV_PM_OPS() instead. */
#define UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
const struct dev_pm_ops __maybe_unused name = { \
SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
--
2.34.1


2022-01-07 18:17:58

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 2/6] PM: core: Remove static qualifier in DEFINE_SIMPLE_DEV_PM_OPS macro

Keep this macro in line with the other ones. This makes it possible to
use them in the cases where the underlying dev_pm_ops structure is
exported.

Restore the "static" qualifier in the two drivers where the
DEFINE_SIMPLE_DEV_PM_OPS macro was used.

Signed-off-by: Paul Cercueil <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
Reviewed-by: Ulf Hansson <[email protected]>
---

Notes:
v2: Merge the driver changes to make the commit atomic
v3: No change

drivers/mmc/host/jz4740_mmc.c | 4 ++--
drivers/mmc/host/mxcmmc.c | 2 +-
include/linux/pm.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 7693236c946f..7ab1b38a7be5 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -1128,8 +1128,8 @@ static int jz4740_mmc_resume(struct device *dev)
return pinctrl_select_default_state(dev);
}

-DEFINE_SIMPLE_DEV_PM_OPS(jz4740_mmc_pm_ops, jz4740_mmc_suspend,
- jz4740_mmc_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(jz4740_mmc_pm_ops, jz4740_mmc_suspend,
+ jz4740_mmc_resume);

static struct platform_driver jz4740_mmc_driver = {
.probe = jz4740_mmc_probe,
diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index 98c218bd6669..40b6878bea6c 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -1210,7 +1210,7 @@ static int mxcmci_resume(struct device *dev)
return ret;
}

-DEFINE_SIMPLE_DEV_PM_OPS(mxcmci_pm_ops, mxcmci_suspend, mxcmci_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(mxcmci_pm_ops, mxcmci_suspend, mxcmci_resume);

static struct platform_driver mxcmci_driver = {
.probe = mxcmci_probe,
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 02f059d814bb..8e13387e70ec 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -362,7 +362,7 @@ struct dev_pm_ops {
* to RAM and hibernation.
*/
#define DEFINE_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-static const struct dev_pm_ops name = { \
+const struct dev_pm_ops name = { \
SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
}

--
2.34.1


2022-01-07 18:18:04

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 3/6] PM: core: Add EXPORT[_GPL]_SIMPLE_DEV_PM_OPS macros

These macros are defined conditionally, according to CONFIG_PM:
- if CONFIG_PM is enabled, these macros resolve to
DEFINE_SIMPLE_DEV_PM_OPS(), and the dev_pm_ops symbol will be
exported.

- if CONFIG_PM is disabled, these macros will result in a dummy static
dev_pm_ops to be created with the __maybe_unused flag. The dev_pm_ops
will then be discarded by the compiler, along with the provided
callback functions if they are not used anywhere else.

In the second case, the symbol is not exported, which should be
perfectly fine - users of the symbol should all use the pm_ptr() or
pm_sleep_ptr() macro, so the dev_pm_ops marked as "extern" in the
client's code will never be accessed.

Signed-off-by: Paul Cercueil <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---

Notes:
v2: Remove useless empty line
v3: - Reorder the code to have non-private macros together in the file
- Add comment about the necesity to use the new export macro when
the dev_pm_ops has to be exported

include/linux/pm.h | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 8e13387e70ec..8279af2c538a 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -8,6 +8,7 @@
#ifndef _LINUX_PM_H
#define _LINUX_PM_H

+#include <linux/export.h>
#include <linux/list.h>
#include <linux/workqueue.h>
#include <linux/spinlock.h>
@@ -357,14 +358,42 @@ struct dev_pm_ops {
#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
#endif

+#define _DEFINE_DEV_PM_OPS(name, \
+ suspend_fn, resume_fn, \
+ runtime_suspend_fn, runtime_resume_fn, idle_fn) \
+const struct dev_pm_ops name = { \
+ SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+ RUNTIME_PM_OPS(runtime_suspend_fn, runtime_resume_fn, idle_fn) \
+}
+
+#ifdef CONFIG_PM
+#define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
+ runtime_resume_fn, idle_fn, sec) \
+ _DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
+ runtime_resume_fn, idle_fn); \
+ _EXPORT_SYMBOL(name, sec)
+#else
+#define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
+ runtime_resume_fn, idle_fn, sec) \
+static __maybe_unused _DEFINE_DEV_PM_OPS(__static_##name, suspend_fn, \
+ resume_fn, runtime_suspend_fn, \
+ runtime_resume_fn, idle_fn)
+#endif
+
/*
* Use this if you want to use the same suspend and resume callbacks for suspend
* to RAM and hibernation.
+ *
+ * If the underlying dev_pm_ops struct symbol has to be exported, use
+ * EXPORT_SIMPLE_DEV_PM_OPS() or EXPORT_GPL_SIMPLE_DEV_PM_OPS() instead.
*/
#define DEFINE_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-const struct dev_pm_ops name = { \
- SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
-}
+ _DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL)
+
+#define EXPORT_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+ _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "")
+#define EXPORT_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+ _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl")

/* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
--
2.34.1


2022-01-07 18:18:13

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 4/6] PM: runtime: Add DEFINE_RUNTIME_DEV_PM_OPS() macro

A lot of drivers create a dev_pm_ops struct with the system sleep
suspend/resume callbacks set to pm_runtime_force_suspend() and
pm_runtime_force_resume().

These drivers can now use the DEFINE_RUNTIME_DEV_PM_OPS() macro, which
will use pm_runtime_force_{suspend,resume}() as the system sleep
callbacks, while having the same dead code removal characteristic that
is already provided by DEFINE_SIMPLE_DEV_PM_OPS().

Signed-off-by: Paul Cercueil <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---

Notes:
v2-v3: No change

include/linux/pm.h | 3 ++-
include/linux/pm_runtime.h | 14 ++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 8279af2c538a..f7d2be686359 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -414,7 +414,8 @@ const struct dev_pm_ops __maybe_unused name = { \
* .resume_early(), to the same routines as .runtime_suspend() and
* .runtime_resume(), respectively (and analogously for hibernation).
*
- * Deprecated. You most likely don't want this macro.
+ * Deprecated. You most likely don't want this macro. Use
+ * DEFINE_RUNTIME_DEV_PM_OPS() instead.
*/
#define UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
const struct dev_pm_ops __maybe_unused name = { \
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 016de5776b6d..4af454d29281 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -22,6 +22,20 @@
usage_count */
#define RPM_AUTO 0x08 /* Use autosuspend_delay */

+/*
+ * Use this for defining a set of PM operations to be used in all situations
+ * (system suspend, hibernation or runtime PM).
+ *
+ * Note that the behaviour differs from the deprecated UNIVERSAL_DEV_PM_OPS()
+ * macro, which uses the provided callbacks for both runtime PM and system
+ * sleep, while DEFINE_RUNTIME_DEV_PM_OPS() uses pm_runtime_force_suspend()
+ * and pm_runtime_force_resume() for its system sleep callbacks.
+ */
+#define DEFINE_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
+ _DEFINE_DEV_PM_OPS(name, pm_runtime_force_suspend, \
+ pm_runtime_force_resume, suspend_fn, \
+ resume_fn, idle_fn)
+
#ifdef CONFIG_PM
extern struct workqueue_struct *pm_wq;

--
2.34.1


2022-01-07 18:18:24

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 5/6] PM: runtime: Add EXPORT[_GPL]_RUNTIME_DEV_PM_OPS macros

Similar to EXPORT[_GPL]_SIMPLE_DEV_PM_OPS, but for users with runtime-PM
suspend/resume callbacks.

Signed-off-by: Paul Cercueil <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---

Notes:
v2: No change
v3: Add comment about the necesity to use the new export macro when
the dev_pm_ops has to be exported

include/linux/pm_runtime.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 4af454d29281..9f09601c465a 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -30,12 +30,22 @@
* macro, which uses the provided callbacks for both runtime PM and system
* sleep, while DEFINE_RUNTIME_DEV_PM_OPS() uses pm_runtime_force_suspend()
* and pm_runtime_force_resume() for its system sleep callbacks.
+ *
+ * If the underlying dev_pm_ops struct symbol has to be exported, use
+ * EXPORT_RUNTIME_DEV_PM_OPS() or EXPORT_GPL_RUNTIME_DEV_PM_OPS() instead.
*/
#define DEFINE_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
_DEFINE_DEV_PM_OPS(name, pm_runtime_force_suspend, \
pm_runtime_force_resume, suspend_fn, \
resume_fn, idle_fn)

+#define EXPORT_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
+ _EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
+ suspend_fn, resume_fn, idle_fn, "")
+#define EXPORT_GPL_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
+ _EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
+ suspend_fn, resume_fn, idle_fn, "_gpl")
+
#ifdef CONFIG_PM
extern struct workqueue_struct *pm_wq;

--
2.34.1


2022-01-07 18:18:44

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 6/6] iio: pressure: bmp280: Use new PM macros

Use the new EXPORT_RUNTIME_DEV_PM_OPS() macro. It allows the underlying
dev_pm_ops struct as well as the suspend/resume callbacks to be detected
as dead code in the case where CONFIG_PM is disabled, without having to
wrap everything inside #ifdef CONFIG_PM guards.

Signed-off-by: Paul Cercueil <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---

Notes:
v2: New patch
v3: No change

drivers/iio/pressure/bmp280-core.c | 11 ++---------
drivers/iio/pressure/bmp280-i2c.c | 2 +-
drivers/iio/pressure/bmp280-spi.c | 2 +-
3 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 6b7da40f99c8..bf8167f43c56 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -1138,7 +1138,6 @@ int bmp280_common_probe(struct device *dev,
}
EXPORT_SYMBOL(bmp280_common_probe);

-#ifdef CONFIG_PM
static int bmp280_runtime_suspend(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
@@ -1159,15 +1158,9 @@ static int bmp280_runtime_resume(struct device *dev)
usleep_range(data->start_up_time, data->start_up_time + 100);
return data->chip_info->chip_config(data);
}
-#endif /* CONFIG_PM */

-const struct dev_pm_ops bmp280_dev_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
- pm_runtime_force_resume)
- SET_RUNTIME_PM_OPS(bmp280_runtime_suspend,
- bmp280_runtime_resume, NULL)
-};
-EXPORT_SYMBOL(bmp280_dev_pm_ops);
+EXPORT_RUNTIME_DEV_PM_OPS(bmp280_dev_pm_ops, bmp280_runtime_suspend,
+ bmp280_runtime_resume, NULL);

MODULE_AUTHOR("Vlad Dogaru <[email protected]>");
MODULE_DESCRIPTION("Driver for Bosch Sensortec BMP180/BMP280 pressure and temperature sensor");
diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
index 8b03ea15c0d0..35045bd92846 100644
--- a/drivers/iio/pressure/bmp280-i2c.c
+++ b/drivers/iio/pressure/bmp280-i2c.c
@@ -58,7 +58,7 @@ static struct i2c_driver bmp280_i2c_driver = {
.driver = {
.name = "bmp280",
.of_match_table = bmp280_of_i2c_match,
- .pm = &bmp280_dev_pm_ops,
+ .pm = pm_ptr(&bmp280_dev_pm_ops),
},
.probe = bmp280_i2c_probe,
.id_table = bmp280_i2c_id,
diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index 625b86878ad8..41f6cc56d229 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -109,7 +109,7 @@ static struct spi_driver bmp280_spi_driver = {
.driver = {
.name = "bmp280",
.of_match_table = bmp280_of_spi_match,
- .pm = &bmp280_dev_pm_ops,
+ .pm = pm_ptr(&bmp280_dev_pm_ops),
},
.id_table = bmp280_spi_id,
.probe = bmp280_spi_probe,
--
2.34.1


2022-01-07 18:21:04

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] DEV_PM_OPS macros rework v3



Le ven., janv. 7 2022 at 18:17:17 +0000, Paul Cercueil
<[email protected]> a ?crit :
> Hi,
>
> A V2 of my patchset that tweaks a bit the *_DEV_PM_OPS() macros that
> were introduced recently.

I meant V3 here. Bad copy-paste.

-Paul



2022-01-08 17:33:57

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] PM: core: Add EXPORT[_GPL]_SIMPLE_DEV_PM_OPS macros

On Fri, 7 Jan 2022 18:17:20 +0000
Paul Cercueil <[email protected]> wrote:

> These macros are defined conditionally, according to CONFIG_PM:
> - if CONFIG_PM is enabled, these macros resolve to
> DEFINE_SIMPLE_DEV_PM_OPS(), and the dev_pm_ops symbol will be
> exported.
>
> - if CONFIG_PM is disabled, these macros will result in a dummy static
> dev_pm_ops to be created with the __maybe_unused flag. The dev_pm_ops
> will then be discarded by the compiler, along with the provided
> callback functions if they are not used anywhere else.
>
> In the second case, the symbol is not exported, which should be
> perfectly fine - users of the symbol should all use the pm_ptr() or
> pm_sleep_ptr() macro, so the dev_pm_ops marked as "extern" in the
> client's code will never be accessed.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>

Hi Paul,

Can definitely be a follow up rather than needing to be in this series
but an EXPORT_NS_[_GPL]_SIMPLE_DEV_PM_OPS() will be needed as I suspect
a lot of the places that export pm_ops structures will have their exports
moved to a namespace at somepoint.

That can easily go in with the first user though rather than needing
to be rushed in now.

Jonathan

> ---
>
> Notes:
> v2: Remove useless empty line
> v3: - Reorder the code to have non-private macros together in the file
> - Add comment about the necesity to use the new export macro when
> the dev_pm_ops has to be exported
>
> include/linux/pm.h | 35 ++++++++++++++++++++++++++++++++---
> 1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 8e13387e70ec..8279af2c538a 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -8,6 +8,7 @@
> #ifndef _LINUX_PM_H
> #define _LINUX_PM_H
>
> +#include <linux/export.h>
> #include <linux/list.h>
> #include <linux/workqueue.h>
> #include <linux/spinlock.h>
> @@ -357,14 +358,42 @@ struct dev_pm_ops {
> #define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
> #endif
>
> +#define _DEFINE_DEV_PM_OPS(name, \
> + suspend_fn, resume_fn, \
> + runtime_suspend_fn, runtime_resume_fn, idle_fn) \
> +const struct dev_pm_ops name = { \
> + SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> + RUNTIME_PM_OPS(runtime_suspend_fn, runtime_resume_fn, idle_fn) \
> +}
> +
> +#ifdef CONFIG_PM
> +#define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
> + runtime_resume_fn, idle_fn, sec) \
> + _DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
> + runtime_resume_fn, idle_fn); \
> + _EXPORT_SYMBOL(name, sec)
> +#else
> +#define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
> + runtime_resume_fn, idle_fn, sec) \
> +static __maybe_unused _DEFINE_DEV_PM_OPS(__static_##name, suspend_fn, \
> + resume_fn, runtime_suspend_fn, \
> + runtime_resume_fn, idle_fn)
> +#endif
> +
> /*
> * Use this if you want to use the same suspend and resume callbacks for suspend
> * to RAM and hibernation.
> + *
> + * If the underlying dev_pm_ops struct symbol has to be exported, use
> + * EXPORT_SIMPLE_DEV_PM_OPS() or EXPORT_GPL_SIMPLE_DEV_PM_OPS() instead.
> */
> #define DEFINE_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> -const struct dev_pm_ops name = { \
> - SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -}
> + _DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL)
> +
> +#define EXPORT_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> + _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "")
> +#define EXPORT_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> + _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl")
>
> /* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
> #define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \


2022-01-10 11:58:54

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] PM: core: Remove DEFINE_UNIVERSAL_DEV_PM_OPS() macro

On Fri, 7 Jan 2022 at 19:17, Paul Cercueil <[email protected]> wrote:
>
> The deprecated UNIVERSAL_DEV_PM_OPS() macro uses the provided callbacks
> for both runtime PM and system sleep, which is very likely to be a
> mistake, as a system sleep can be triggered while a given device is
> already PM-suspended, which would cause the suspend callback to be
> called twice.
>
> The amount of users of UNIVERSAL_DEV_PM_OPS() is also tiny (16
> occurences) compared to the number of places where
> SET_SYSTEM_SLEEP_PM_OPS() is used with pm_runtime_force_suspend() and
> pm_runtime_force_resume(), which makes me think that none of these cases
> are actually valid.
>
> As the new macro DEFINE_UNIVERSAL_DEV_PM_OPS() which was introduced to
> replace UNIVERSAL_DEV_PM_OPS() is currently unused, remove it before
> someone starts to use it in yet another invalid case.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe


> ---
>
> Notes:
> v2: No change
> v3: - Keep UNIVERSAL_DEV_PM_OPS deprecated
> - Rework commit message
>
> include/linux/pm.h | 21 ++++++++-------------
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index e1e9402180b9..02f059d814bb 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -366,6 +366,12 @@ static const struct dev_pm_ops name = { \
> SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> }
>
> +/* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
> +#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> +const struct dev_pm_ops __maybe_unused name = { \
> + SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +}
> +
> /*
> * Use this for defining a set of PM operations to be used in all situations
> * (system suspend, hibernation or runtime PM).
> @@ -378,20 +384,9 @@ static const struct dev_pm_ops name = { \
> * suspend and "early" resume callback pointers, .suspend_late() and
> * .resume_early(), to the same routines as .runtime_suspend() and
> * .runtime_resume(), respectively (and analogously for hibernation).
> + *
> + * Deprecated. You most likely don't want this macro.
> */
> -#define DEFINE_UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
> -static const struct dev_pm_ops name = { \
> - SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> - RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> -}
> -
> -/* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
> -#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> -const struct dev_pm_ops __maybe_unused name = { \
> - SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -}
> -
> -/* Deprecated. Use DEFINE_UNIVERSAL_DEV_PM_OPS() instead. */
> #define UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
> const struct dev_pm_ops __maybe_unused name = { \
> SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> --
> 2.34.1
>

2022-01-10 12:19:18

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] PM: core: Add EXPORT[_GPL]_SIMPLE_DEV_PM_OPS macros

On Fri, 7 Jan 2022 at 19:17, Paul Cercueil <[email protected]> wrote:
>
> These macros are defined conditionally, according to CONFIG_PM:
> - if CONFIG_PM is enabled, these macros resolve to
> DEFINE_SIMPLE_DEV_PM_OPS(), and the dev_pm_ops symbol will be
> exported.
>
> - if CONFIG_PM is disabled, these macros will result in a dummy static
> dev_pm_ops to be created with the __maybe_unused flag. The dev_pm_ops
> will then be discarded by the compiler, along with the provided
> callback functions if they are not used anywhere else.
>
> In the second case, the symbol is not exported, which should be
> perfectly fine - users of the symbol should all use the pm_ptr() or
> pm_sleep_ptr() macro, so the dev_pm_ops marked as "extern" in the
> client's code will never be accessed.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>

Clearly this can be useful!

My main concern is rather that the macros become a bit complicated
(for understandable reasons) and that kind of continues while looking
at patch4 and patch5 too. Hopefully that doesn't prevent users from
adopting them, and the current situation deserves improvements, so I
think this is worth a try!

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe


> ---
>
> Notes:
> v2: Remove useless empty line
> v3: - Reorder the code to have non-private macros together in the file
> - Add comment about the necesity to use the new export macro when
> the dev_pm_ops has to be exported
>
> include/linux/pm.h | 35 ++++++++++++++++++++++++++++++++---
> 1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 8e13387e70ec..8279af2c538a 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -8,6 +8,7 @@
> #ifndef _LINUX_PM_H
> #define _LINUX_PM_H
>
> +#include <linux/export.h>
> #include <linux/list.h>
> #include <linux/workqueue.h>
> #include <linux/spinlock.h>
> @@ -357,14 +358,42 @@ struct dev_pm_ops {
> #define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
> #endif
>
> +#define _DEFINE_DEV_PM_OPS(name, \
> + suspend_fn, resume_fn, \
> + runtime_suspend_fn, runtime_resume_fn, idle_fn) \
> +const struct dev_pm_ops name = { \
> + SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> + RUNTIME_PM_OPS(runtime_suspend_fn, runtime_resume_fn, idle_fn) \
> +}
> +
> +#ifdef CONFIG_PM
> +#define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
> + runtime_resume_fn, idle_fn, sec) \
> + _DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
> + runtime_resume_fn, idle_fn); \
> + _EXPORT_SYMBOL(name, sec)
> +#else
> +#define _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
> + runtime_resume_fn, idle_fn, sec) \
> +static __maybe_unused _DEFINE_DEV_PM_OPS(__static_##name, suspend_fn, \
> + resume_fn, runtime_suspend_fn, \
> + runtime_resume_fn, idle_fn)
> +#endif
> +
> /*
> * Use this if you want to use the same suspend and resume callbacks for suspend
> * to RAM and hibernation.
> + *
> + * If the underlying dev_pm_ops struct symbol has to be exported, use
> + * EXPORT_SIMPLE_DEV_PM_OPS() or EXPORT_GPL_SIMPLE_DEV_PM_OPS() instead.
> */
> #define DEFINE_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> -const struct dev_pm_ops name = { \
> - SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -}
> + _DEFINE_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL)
> +
> +#define EXPORT_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> + _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "")
> +#define EXPORT_GPL_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> + _EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, NULL, NULL, NULL, "_gpl")
>
> /* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
> #define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> --
> 2.34.1
>

2022-01-10 12:19:52

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v3 4/6] PM: runtime: Add DEFINE_RUNTIME_DEV_PM_OPS() macro

On Fri, 7 Jan 2022 at 19:17, Paul Cercueil <[email protected]> wrote:
>
> A lot of drivers create a dev_pm_ops struct with the system sleep
> suspend/resume callbacks set to pm_runtime_force_suspend() and
> pm_runtime_force_resume().
>
> These drivers can now use the DEFINE_RUNTIME_DEV_PM_OPS() macro, which
> will use pm_runtime_force_{suspend,resume}() as the system sleep
> callbacks, while having the same dead code removal characteristic that
> is already provided by DEFINE_SIMPLE_DEV_PM_OPS().
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe

> ---
>
> Notes:
> v2-v3: No change
>
> include/linux/pm.h | 3 ++-
> include/linux/pm_runtime.h | 14 ++++++++++++++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 8279af2c538a..f7d2be686359 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -414,7 +414,8 @@ const struct dev_pm_ops __maybe_unused name = { \
> * .resume_early(), to the same routines as .runtime_suspend() and
> * .runtime_resume(), respectively (and analogously for hibernation).
> *
> - * Deprecated. You most likely don't want this macro.
> + * Deprecated. You most likely don't want this macro. Use
> + * DEFINE_RUNTIME_DEV_PM_OPS() instead.
> */
> #define UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
> const struct dev_pm_ops __maybe_unused name = { \
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index 016de5776b6d..4af454d29281 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -22,6 +22,20 @@
> usage_count */
> #define RPM_AUTO 0x08 /* Use autosuspend_delay */
>
> +/*
> + * Use this for defining a set of PM operations to be used in all situations
> + * (system suspend, hibernation or runtime PM).
> + *
> + * Note that the behaviour differs from the deprecated UNIVERSAL_DEV_PM_OPS()
> + * macro, which uses the provided callbacks for both runtime PM and system
> + * sleep, while DEFINE_RUNTIME_DEV_PM_OPS() uses pm_runtime_force_suspend()
> + * and pm_runtime_force_resume() for its system sleep callbacks.
> + */
> +#define DEFINE_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
> + _DEFINE_DEV_PM_OPS(name, pm_runtime_force_suspend, \
> + pm_runtime_force_resume, suspend_fn, \
> + resume_fn, idle_fn)
> +
> #ifdef CONFIG_PM
> extern struct workqueue_struct *pm_wq;
>
> --
> 2.34.1
>

2022-01-10 12:20:21

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v3 5/6] PM: runtime: Add EXPORT[_GPL]_RUNTIME_DEV_PM_OPS macros

On Fri, 7 Jan 2022 at 19:17, Paul Cercueil <[email protected]> wrote:
>
> Similar to EXPORT[_GPL]_SIMPLE_DEV_PM_OPS, but for users with runtime-PM
> suspend/resume callbacks.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe

> ---
>
> Notes:
> v2: No change
> v3: Add comment about the necesity to use the new export macro when
> the dev_pm_ops has to be exported
>
> include/linux/pm_runtime.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index 4af454d29281..9f09601c465a 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -30,12 +30,22 @@
> * macro, which uses the provided callbacks for both runtime PM and system
> * sleep, while DEFINE_RUNTIME_DEV_PM_OPS() uses pm_runtime_force_suspend()
> * and pm_runtime_force_resume() for its system sleep callbacks.
> + *
> + * If the underlying dev_pm_ops struct symbol has to be exported, use
> + * EXPORT_RUNTIME_DEV_PM_OPS() or EXPORT_GPL_RUNTIME_DEV_PM_OPS() instead.
> */
> #define DEFINE_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
> _DEFINE_DEV_PM_OPS(name, pm_runtime_force_suspend, \
> pm_runtime_force_resume, suspend_fn, \
> resume_fn, idle_fn)
>
> +#define EXPORT_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
> + _EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
> + suspend_fn, resume_fn, idle_fn, "")
> +#define EXPORT_GPL_RUNTIME_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
> + _EXPORT_DEV_PM_OPS(name, pm_runtime_force_suspend, pm_runtime_force_resume, \
> + suspend_fn, resume_fn, idle_fn, "_gpl")
> +
> #ifdef CONFIG_PM
> extern struct workqueue_struct *pm_wq;
>
> --
> 2.34.1
>

2022-01-10 12:21:28

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v3 6/6] iio: pressure: bmp280: Use new PM macros

On Fri, 7 Jan 2022 at 19:18, Paul Cercueil <[email protected]> wrote:
>
> Use the new EXPORT_RUNTIME_DEV_PM_OPS() macro. It allows the underlying
> dev_pm_ops struct as well as the suspend/resume callbacks to be detected
> as dead code in the case where CONFIG_PM is disabled, without having to
> wrap everything inside #ifdef CONFIG_PM guards.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe

> ---
>
> Notes:
> v2: New patch
> v3: No change
>
> drivers/iio/pressure/bmp280-core.c | 11 ++---------
> drivers/iio/pressure/bmp280-i2c.c | 2 +-
> drivers/iio/pressure/bmp280-spi.c | 2 +-
> 3 files changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 6b7da40f99c8..bf8167f43c56 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -1138,7 +1138,6 @@ int bmp280_common_probe(struct device *dev,
> }
> EXPORT_SYMBOL(bmp280_common_probe);
>
> -#ifdef CONFIG_PM
> static int bmp280_runtime_suspend(struct device *dev)
> {
> struct iio_dev *indio_dev = dev_get_drvdata(dev);
> @@ -1159,15 +1158,9 @@ static int bmp280_runtime_resume(struct device *dev)
> usleep_range(data->start_up_time, data->start_up_time + 100);
> return data->chip_info->chip_config(data);
> }
> -#endif /* CONFIG_PM */
>
> -const struct dev_pm_ops bmp280_dev_pm_ops = {
> - SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> - pm_runtime_force_resume)
> - SET_RUNTIME_PM_OPS(bmp280_runtime_suspend,
> - bmp280_runtime_resume, NULL)
> -};
> -EXPORT_SYMBOL(bmp280_dev_pm_ops);
> +EXPORT_RUNTIME_DEV_PM_OPS(bmp280_dev_pm_ops, bmp280_runtime_suspend,
> + bmp280_runtime_resume, NULL);
>
> MODULE_AUTHOR("Vlad Dogaru <[email protected]>");
> MODULE_DESCRIPTION("Driver for Bosch Sensortec BMP180/BMP280 pressure and temperature sensor");
> diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
> index 8b03ea15c0d0..35045bd92846 100644
> --- a/drivers/iio/pressure/bmp280-i2c.c
> +++ b/drivers/iio/pressure/bmp280-i2c.c
> @@ -58,7 +58,7 @@ static struct i2c_driver bmp280_i2c_driver = {
> .driver = {
> .name = "bmp280",
> .of_match_table = bmp280_of_i2c_match,
> - .pm = &bmp280_dev_pm_ops,
> + .pm = pm_ptr(&bmp280_dev_pm_ops),
> },
> .probe = bmp280_i2c_probe,
> .id_table = bmp280_i2c_id,
> diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> index 625b86878ad8..41f6cc56d229 100644
> --- a/drivers/iio/pressure/bmp280-spi.c
> +++ b/drivers/iio/pressure/bmp280-spi.c
> @@ -109,7 +109,7 @@ static struct spi_driver bmp280_spi_driver = {
> .driver = {
> .name = "bmp280",
> .of_match_table = bmp280_of_spi_match,
> - .pm = &bmp280_dev_pm_ops,
> + .pm = pm_ptr(&bmp280_dev_pm_ops),
> },
> .id_table = bmp280_spi_id,
> .probe = bmp280_spi_probe,
> --
> 2.34.1
>

2022-01-17 06:56:00

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] DEV_PM_OPS macros rework v3

Hi Rafael,

Could patches [1/6] and [2/6] make it to 5.17-rc1, or at least -rc2?

I'm afraid that if these two have to wait for the 5.18 cycle, then I'll
have more drivers to fix later.

Should I add a Fixes tag maybe?

Cheers,
-Paul


Le ven., janv. 7 2022 at 18:17:17 +0000, Paul Cercueil
<[email protected]> a ?crit :
> Hi,
>
> A V2 of my patchset that tweaks a bit the *_DEV_PM_OPS() macros that
> were introduced recently.
>
> Changes since V2:
> * [1/6]: - Keep UNIVERSAL_DEV_PM_OPS() macro deprecated
> - Rework commit message
> * [3/6]: - Reorder the code to have non-private macros together in the
> file
> - Add comment about the necesity to use the new export macro
> when the dev_pm_ops has to be exported
> * [5/6]: Add comment about the necesity to use the new export macro
> when the dev_pm_ops has to be exported
>
> Cheers,
> -Paul
>
> Paul Cercueil (6):
> PM: core: Remove DEFINE_UNIVERSAL_DEV_PM_OPS() macro
> PM: core: Remove static qualifier in DEFINE_SIMPLE_DEV_PM_OPS macro
> PM: core: Add EXPORT[_GPL]_SIMPLE_DEV_PM_OPS macros
> PM: runtime: Add DEFINE_RUNTIME_DEV_PM_OPS() macro
> PM: runtime: Add EXPORT[_GPL]_RUNTIME_DEV_PM_OPS macros
> iio: pressure: bmp280: Use new PM macros
>
> drivers/iio/pressure/bmp280-core.c | 11 ++----
> drivers/iio/pressure/bmp280-i2c.c | 2 +-
> drivers/iio/pressure/bmp280-spi.c | 2 +-
> drivers/mmc/host/jz4740_mmc.c | 4 +--
> drivers/mmc/host/mxcmmc.c | 2 +-
> include/linux/pm.h | 55
> ++++++++++++++++++++++--------
> include/linux/pm_runtime.h | 24 +++++++++++++
> 7 files changed, 71 insertions(+), 29 deletions(-)
>
> --
> 2.34.1
>


2022-01-18 02:28:06

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] DEV_PM_OPS macros rework v3

Hi,

On Sun, Jan 16, 2022 at 1:05 PM Paul Cercueil <[email protected]> wrote:
>
> Hi Rafael,
>
> Could patches [1/6] and [2/6] make it to 5.17-rc1, or at least -rc2?

Yes. I'm going to send a PR with the whole series later today.

> I'm afraid that if these two have to wait for the 5.18 cycle, then I'll
> have more drivers to fix later.
>
> Should I add a Fixes tag maybe?

No need, thanks!

> Le ven., janv. 7 2022 at 18:17:17 +0000, Paul Cercueil
> <[email protected]> a écrit :
> > Hi,
> >
> > A V2 of my patchset that tweaks a bit the *_DEV_PM_OPS() macros that
> > were introduced recently.
> >
> > Changes since V2:
> > * [1/6]: - Keep UNIVERSAL_DEV_PM_OPS() macro deprecated
> > - Rework commit message
> > * [3/6]: - Reorder the code to have non-private macros together in the
> > file
> > - Add comment about the necesity to use the new export macro
> > when the dev_pm_ops has to be exported
> > * [5/6]: Add comment about the necesity to use the new export macro
> > when the dev_pm_ops has to be exported
> >
> > Cheers,
> > -Paul
> >
> > Paul Cercueil (6):
> > PM: core: Remove DEFINE_UNIVERSAL_DEV_PM_OPS() macro
> > PM: core: Remove static qualifier in DEFINE_SIMPLE_DEV_PM_OPS macro
> > PM: core: Add EXPORT[_GPL]_SIMPLE_DEV_PM_OPS macros
> > PM: runtime: Add DEFINE_RUNTIME_DEV_PM_OPS() macro
> > PM: runtime: Add EXPORT[_GPL]_RUNTIME_DEV_PM_OPS macros
> > iio: pressure: bmp280: Use new PM macros
> >
> > drivers/iio/pressure/bmp280-core.c | 11 ++----
> > drivers/iio/pressure/bmp280-i2c.c | 2 +-
> > drivers/iio/pressure/bmp280-spi.c | 2 +-
> > drivers/mmc/host/jz4740_mmc.c | 4 +--
> > drivers/mmc/host/mxcmmc.c | 2 +-
> > include/linux/pm.h | 55
> > ++++++++++++++++++++++--------
> > include/linux/pm_runtime.h | 24 +++++++++++++
> > 7 files changed, 71 insertions(+), 29 deletions(-)
> >
> > --
> > 2.34.1
> >
>
>

2022-01-18 02:41:51

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] DEV_PM_OPS macros rework v3



Le lun., janv. 17 2022 at 14:37:45 +0100, Rafael J. Wysocki
<[email protected]> a ?crit :
> Hi,
>
> On Sun, Jan 16, 2022 at 1:05 PM Paul Cercueil <[email protected]>
> wrote:
>>
>> Hi Rafael,
>>
>> Could patches [1/6] and [2/6] make it to 5.17-rc1, or at least -rc2?
>
> Yes. I'm going to send a PR with the whole series later today.

Ok, perfect then. I saw my previous PM patches in upstream/master and
assumed that you already sent your PR.

Cheers,
-Paul

>
>> I'm afraid that if these two have to wait for the 5.18 cycle, then
>> I'll
>> have more drivers to fix later.
>>
>> Should I add a Fixes tag maybe?
>
> No need, thanks!
>
>> Le ven., janv. 7 2022 at 18:17:17 +0000, Paul Cercueil
>> <[email protected]> a ?crit :
>> > Hi,
>> >
>> > A V2 of my patchset that tweaks a bit the *_DEV_PM_OPS() macros
>> that
>> > were introduced recently.
>> >
>> > Changes since V2:
>> > * [1/6]: - Keep UNIVERSAL_DEV_PM_OPS() macro deprecated
>> > - Rework commit message
>> > * [3/6]: - Reorder the code to have non-private macros together
>> in the
>> > file
>> > - Add comment about the necesity to use the new export
>> macro
>> > when the dev_pm_ops has to be exported
>> > * [5/6]: Add comment about the necesity to use the new export
>> macro
>> > when the dev_pm_ops has to be exported
>> >
>> > Cheers,
>> > -Paul
>> >
>> > Paul Cercueil (6):
>> > PM: core: Remove DEFINE_UNIVERSAL_DEV_PM_OPS() macro
>> > PM: core: Remove static qualifier in DEFINE_SIMPLE_DEV_PM_OPS
>> macro
>> > PM: core: Add EXPORT[_GPL]_SIMPLE_DEV_PM_OPS macros
>> > PM: runtime: Add DEFINE_RUNTIME_DEV_PM_OPS() macro
>> > PM: runtime: Add EXPORT[_GPL]_RUNTIME_DEV_PM_OPS macros
>> > iio: pressure: bmp280: Use new PM macros
>> >
>> > drivers/iio/pressure/bmp280-core.c | 11 ++----
>> > drivers/iio/pressure/bmp280-i2c.c | 2 +-
>> > drivers/iio/pressure/bmp280-spi.c | 2 +-
>> > drivers/mmc/host/jz4740_mmc.c | 4 +--
>> > drivers/mmc/host/mxcmmc.c | 2 +-
>> > include/linux/pm.h | 55
>> > ++++++++++++++++++++++--------
>> > include/linux/pm_runtime.h | 24 +++++++++++++
>> > 7 files changed, 71 insertions(+), 29 deletions(-)
>> >
>> > --
>> > 2.34.1
>> >
>>
>>