[PATCH] media: rc: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[PATCH] media: rc: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Triet Hoang]

Convert the deprecated SIMPLE_DEV_PM_OPS
to DEFINE_SIMPLE_DEV_PM_OPS
and pm_sleep_ptr().

This lets us drop the __maybe_unused annotations
from its suspend and resume callbacks,
also reduces kernel size in case CONFIG_PM or
CONFIG_PM_SLEEP is disabled.

Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
---
 drivers/media/rc/meson-ir.c  | 6 +++---
 drivers/media/rc/sunxi-cir.c | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index 272ebb0d97c8..795b38ca864a 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -567,7 +567,7 @@ static void meson_ir_shutdown(struct platform_device *pdev)
 	spin_unlock_irqrestore(&ir->lock, flags);
 }
 
-static __maybe_unused int meson_ir_resume(struct device *dev)
+static int meson_ir_resume(struct device *dev)
 {
 	struct meson_ir *ir = dev_get_drvdata(dev);
 
@@ -579,7 +579,7 @@ static __maybe_unused int meson_ir_resume(struct device *dev)
 	return 0;
 }
 
-static __maybe_unused int meson_ir_suspend(struct device *dev)
+static int meson_ir_suspend(struct device *dev)
 {
 	struct meson_ir *ir = dev_get_drvdata(dev);
 	unsigned long flags;
@@ -591,7 +591,7 @@ static __maybe_unused int meson_ir_suspend(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(meson_ir_pm_ops, meson_ir_suspend, meson_ir_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(meson_ir_pm_ops, meson_ir_suspend, meson_ir_resume);
 
 static const struct meson_ir_param meson6_ir_param = {
 	.support_hw_decoder = false,
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 28e840a7e5b8..7fd48c510143 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -236,19 +236,19 @@ static void sunxi_ir_hw_exit(struct device *dev)
 	reset_control_assert(ir->rst);
 }
 
-static int __maybe_unused sunxi_ir_suspend(struct device *dev)
+static int sunxi_ir_suspend(struct device *dev)
 {
 	sunxi_ir_hw_exit(dev);
 
 	return 0;
 }
 
-static int __maybe_unused sunxi_ir_resume(struct device *dev)
+static int sunxi_ir_resume(struct device *dev)
 {
 	return sunxi_ir_hw_init(dev);
 }
 
-static SIMPLE_DEV_PM_OPS(sunxi_ir_pm_ops, sunxi_ir_suspend, sunxi_ir_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(sunxi_ir_pm_ops, sunxi_ir_suspend, sunxi_ir_resume);
 
 static int sunxi_ir_probe(struct platform_device *pdev)
 {
@@ -422,7 +422,7 @@ static struct platform_driver sunxi_ir_driver = {
 	.driver = {
 		.name = SUNXI_IR_DEV,
 		.of_match_table = sunxi_ir_match,
-		.pm = &sunxi_ir_pm_ops,
+		.pm = pm_sleep_ptr(&sunxi_ir_pm_ops),
 	},
 };
 
-- 
2.53.0


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

[PATCH] media: rc: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Triet Hoang]

Convert the deprecated SIMPLE_DEV_PM_OPS
to DEFINE_SIMPLE_DEV_PM_OPS
and pm_sleep_ptr().

This lets us drop the __maybe_unused annotations
from its suspend and resume callbacks,
also reduces kernel size in case CONFIG_PM or
CONFIG_PM_SLEEP is disabled.

Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
---
 drivers/media/rc/meson-ir.c  | 6 +++---
 drivers/media/rc/sunxi-cir.c | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index 272ebb0d97c8..795b38ca864a 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -567,7 +567,7 @@ static void meson_ir_shutdown(struct platform_device *pdev)
 	spin_unlock_irqrestore(&ir->lock, flags);
 }
 
-static __maybe_unused int meson_ir_resume(struct device *dev)
+static int meson_ir_resume(struct device *dev)
 {
 	struct meson_ir *ir = dev_get_drvdata(dev);
 
@@ -579,7 +579,7 @@ static __maybe_unused int meson_ir_resume(struct device *dev)
 	return 0;
 }
 
-static __maybe_unused int meson_ir_suspend(struct device *dev)
+static int meson_ir_suspend(struct device *dev)
 {
 	struct meson_ir *ir = dev_get_drvdata(dev);
 	unsigned long flags;
@@ -591,7 +591,7 @@ static __maybe_unused int meson_ir_suspend(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(meson_ir_pm_ops, meson_ir_suspend, meson_ir_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(meson_ir_pm_ops, meson_ir_suspend, meson_ir_resume);
 
 static const struct meson_ir_param meson6_ir_param = {
 	.support_hw_decoder = false,
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 28e840a7e5b8..7fd48c510143 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -236,19 +236,19 @@ static void sunxi_ir_hw_exit(struct device *dev)
 	reset_control_assert(ir->rst);
 }
 
-static int __maybe_unused sunxi_ir_suspend(struct device *dev)
+static int sunxi_ir_suspend(struct device *dev)
 {
 	sunxi_ir_hw_exit(dev);
 
 	return 0;
 }
 
-static int __maybe_unused sunxi_ir_resume(struct device *dev)
+static int sunxi_ir_resume(struct device *dev)
 {
 	return sunxi_ir_hw_init(dev);
 }
 
-static SIMPLE_DEV_PM_OPS(sunxi_ir_pm_ops, sunxi_ir_suspend, sunxi_ir_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(sunxi_ir_pm_ops, sunxi_ir_suspend, sunxi_ir_resume);
 
 static int sunxi_ir_probe(struct platform_device *pdev)
 {
@@ -422,7 +422,7 @@ static struct platform_driver sunxi_ir_driver = {
 	.driver = {
 		.name = SUNXI_IR_DEV,
 		.of_match_table = sunxi_ir_match,
-		.pm = &sunxi_ir_pm_ops,
+		.pm = pm_sleep_ptr(&sunxi_ir_pm_ops),
 	},
 };
 
-- 
2.53.0

Re: [PATCH] media: rc: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Incorrect PM wrapper macro used for the dev_pm_ops structure pointer.

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but there is a use-after-free of the RC device on driver removal and probe error paths due to an active devres-managed IRQ handler.
--

--- Patch [1]: [PATCH] media: rc: Convert to DEFINE_SIMPLE_DEV_PM_OPS() ---
commit 5ad013ce47b901f4bd8a53a1c02b222a8df6858a
Author: Triet Hoang <triet.hoang.dev@gmail.com>

media: rc: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

Convert the deprecated SIMPLE_DEV_PM_OPS to DEFINE_SIMPLE_DEV_PM_OPS
and pm_sleep_ptr(). This lets us drop the __maybe_unused annotations
from its suspend and resume callbacks, also reduces kernel size in
case CONFIG_PM or CONFIG_PM_SLEEP is disabled.

> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 28e840a7e5b8c..7fd48c510143d 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c

[ ... ]

> @@ -422,7 +422,7 @@ static struct platform_driver sunxi_ir_driver = {
>  	.driver = {
>  		.name = SUNXI_IR_DEV,
>  		.of_match_table = sunxi_ir_match,
> -		.pm = &sunxi_ir_pm_ops,
> +		.pm = pm_sleep_ptr(&sunxi_ir_pm_ops),

[Severity: Low]
Does this code use the correct wrapper macro for a dev_pm_ops structure
pointer? Since sunxi_ir_pm_ops is a structure pointer rather than a sleep
callback, should it use pm_ptr() instead of pm_sleep_ptr()?

>  	},
>  };

[Severity: Critical]
This is a pre-existing issue, but could there be a use-after-free of the RC
device on driver removal and probe error paths?

The driver initializes a devres-managed IRQ in sunxi_ir_probe():

	ret = devm_request_irq(dev, ir->irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);

On probe failure or during sunxi_ir_remove(), the driver manually calls
rc_unregister_device() and rc_free_device(), freeing the RC device memory:

exit_unregister_dev:
	rc_unregister_device(ir->rc);
exit_free_dev:
	rc_free_device(ir->rc);

Because devres cleanup has not yet run, the IRQ remains active. If an
interrupt fires or is already in-flight on another CPU during this window,
would sunxi_ir_irq() dereference the freed ir->rc?

	ir_raw_event_overflow(ir->rc);

Also, could this read from disabled hardware if an interrupt fires after
sunxi_ir_hw_exit() is called?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821071533.215821-1-triet.hoang.dev@gmail.com?part=1

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

Re: [PATCH] media: rc: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Incorrect PM wrapper macro used for the dev_pm_ops structure pointer.

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but there is a use-after-free of the RC device on driver removal and probe error paths due to an active devres-managed IRQ handler.
--

--- Patch [1]: [PATCH] media: rc: Convert to DEFINE_SIMPLE_DEV_PM_OPS() ---
commit 5ad013ce47b901f4bd8a53a1c02b222a8df6858a
Author: Triet Hoang <triet.hoang.dev@gmail.com>

media: rc: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

Convert the deprecated SIMPLE_DEV_PM_OPS to DEFINE_SIMPLE_DEV_PM_OPS
and pm_sleep_ptr(). This lets us drop the __maybe_unused annotations
from its suspend and resume callbacks, also reduces kernel size in
case CONFIG_PM or CONFIG_PM_SLEEP is disabled.

> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 28e840a7e5b8c..7fd48c510143d 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c

[ ... ]

> @@ -422,7 +422,7 @@ static struct platform_driver sunxi_ir_driver = {
>  	.driver = {
>  		.name = SUNXI_IR_DEV,
>  		.of_match_table = sunxi_ir_match,
> -		.pm = &sunxi_ir_pm_ops,
> +		.pm = pm_sleep_ptr(&sunxi_ir_pm_ops),

[Severity: Low]
Does this code use the correct wrapper macro for a dev_pm_ops structure
pointer? Since sunxi_ir_pm_ops is a structure pointer rather than a sleep
callback, should it use pm_ptr() instead of pm_sleep_ptr()?

>  	},
>  };

[Severity: Critical]
This is a pre-existing issue, but could there be a use-after-free of the RC
device on driver removal and probe error paths?

The driver initializes a devres-managed IRQ in sunxi_ir_probe():

	ret = devm_request_irq(dev, ir->irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);

On probe failure or during sunxi_ir_remove(), the driver manually calls
rc_unregister_device() and rc_free_device(), freeing the RC device memory:

exit_unregister_dev:
	rc_unregister_device(ir->rc);
exit_free_dev:
	rc_free_device(ir->rc);

Because devres cleanup has not yet run, the IRQ remains active. If an
interrupt fires or is already in-flight on another CPU during this window,
would sunxi_ir_irq() dereference the freed ir->rc?

	ir_raw_event_overflow(ir->rc);

Also, could this read from disabled hardware if an interrupt fires after
sunxi_ir_hw_exit() is called?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821071533.215821-1-triet.hoang.dev@gmail.com?part=1