diff mbox series

[v2,1/6] media: synopsys: hdmirx: Use devm_of_reserved_mem_device_init()

Message ID 20260902200703.2016410-2-mukesh.ojha@oss.qualcomm.com (mailing list archive)
State New
Headers
Series media: Use devm_of_reserved_mem_device_init() |

Commit Message

Mukesh Ojha Sept. 2, 2026, 8:06 p.m. UTC 
Replace the hand-rolled devm_hdmirx_of_reserved_mem_device_release()
devres action with devm_of_reserved_mem_device_init(), which handles the
cleanup automatically. This removes both the wrapper function and the
devm_add_action_or_reset() call.

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 .../media/platform/synopsys/hdmirx/snps_hdmirx.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

Comments

sashiko-bot@kernel.org Sept. 2, 2026, 8:15 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Probe deferral breakage due to overriding devm_clk_bulk_get_all() error code.
--

commit 9b061d6dbcd6ec9063d57f4982dfa6c2f6ec7b98
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
Subject: media: synopsys: hdmirx: Use devm_of_reserved_mem_device_init()

This commit replaces the hand-rolled devres action for releasing reserved
memory with devm_of_reserved_mem_device_init(), which handles the cleanup
automatically.

> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> index 25f8ca0d6d946..7c6acc6d7e882 100644
> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> @@ -2265,11 +2265,6 @@ static const struct hdmirx_cec_ops hdmirx_cec_ops = {
>  	.read = hdmirx_readl,
>  };
>  
> -static void devm_hdmirx_of_reserved_mem_device_release(void *dev)
> -{
> -	of_reserved_mem_device_release(dev);
> -}
> -
>  static int hdmirx_parse_dt(struct snps_hdmirx_dev *hdmirx_dev)
>  {
>  	struct device *dev = hdmirx_dev->dev;

[Severity: High]
This isn't a bug introduced by this patch, but does this code break probe
deferral?

Looking at hdmirx_parse_dt() in snps_hdmirx.c, when devm_clk_bulk_get_all()
fails, the negative error code is discarded:

	hdmirx_dev->num_clks = devm_clk_bulk_get_all(dev, &hdmirx_dev->clks);
	if (hdmirx_dev->num_clks < 1)
		return -ENODEV;

If devm_clk_bulk_get_all() returns -EPROBE_DEFER because the clock providers
are not yet ready, returning -ENODEV masks the deferral request. This
converts a transient dependency delay into a permanent initialization failure.

Should this code be updated to propagate the negative error code instead?

[ ... ]
diff mbox series

Patch

diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index 25f8ca0d6d94..7c6acc6d7e88 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -2265,11 +2265,6 @@  static const struct hdmirx_cec_ops hdmirx_cec_ops = {
 	.read = hdmirx_readl,
 };
 
-static void devm_hdmirx_of_reserved_mem_device_release(void *dev)
-{
-	of_reserved_mem_device_release(dev);
-}
-
 static int hdmirx_parse_dt(struct snps_hdmirx_dev *hdmirx_dev)
 {
 	struct device *dev = hdmirx_dev->dev;
@@ -2316,16 +2311,9 @@  static int hdmirx_parse_dt(struct snps_hdmirx_dev *hdmirx_dev)
 	if (!device_property_read_bool(dev, "hpd-is-active-low"))
 		hdmirx_dev->hpd_trigger_level_high = true;
 
-	ret = of_reserved_mem_device_init(dev);
-	if (ret) {
+	ret = devm_of_reserved_mem_device_init(dev);
+	if (ret)
 		dev_warn(dev, "no reserved memory for HDMIRX, use default CMA\n");
-	} else {
-		ret = devm_add_action_or_reset(dev,
-					       devm_hdmirx_of_reserved_mem_device_release,
-					       dev);
-		if (ret)
-			return ret;
-	}
 
 	return 0;
 }