[RFC PATCH] drm/meson: dw-hdmi: Propagate reset failures during initialization

[RFC PATCH] drm/meson: dw-hdmi: Propagate reset failures during initialization

[Pengpeng Hou]

meson_dw_hdmi_init() resets the APB, controller and PHY domains before
accessing the HDMI block.  All three reset results are ignored, so bind
or system resume can continue after a reset controller rejected part of
the sequence.

Make the helper return an error and propagate it through bind and
resume.  This RFC intentionally leaves earlier HHI programming and
successful resets in place on failure; feedback is requested on whether
these SoCs require an additional rollback sequence.

The issue was identified via static analysis and manually reviewed.

Assisted-by: LLM
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index fef1702acb14..1b55d1c89f62 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -595,9 +595,10 @@ static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = {
 	.cntl1_init = PHY_CNTL1_INIT,
 };
 
-static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi)
+static int meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi)
 {
 	struct meson_drm *priv = meson_dw_hdmi->priv;
+	int ret;
 
 	/* Enable clocks */
 	regmap_update_bits(priv->hhi, HHI_HDMI_CLK_CNTL, 0xffff, 0x100);
@@ -606,9 +607,17 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi)
 	regmap_update_bits(priv->hhi, HHI_MEM_PD_REG0, 0xff << 8, 0);
 
 	/* Reset HDMITX APB & TX & PHY */
-	reset_control_reset(meson_dw_hdmi->hdmitx_apb);
-	reset_control_reset(meson_dw_hdmi->hdmitx_ctrl);
-	reset_control_reset(meson_dw_hdmi->hdmitx_phy);
+	ret = reset_control_reset(meson_dw_hdmi->hdmitx_apb);
+	if (ret)
+		return ret;
+
+	ret = reset_control_reset(meson_dw_hdmi->hdmitx_ctrl);
+	if (ret)
+		return ret;
+
+	ret = reset_control_reset(meson_dw_hdmi->hdmitx_phy);
+	if (ret)
+		return ret;
 
 	/* Enable APB3 fail on error */
 	if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
@@ -641,6 +650,7 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi)
 	meson_dw_hdmi->data->top_write(meson_dw_hdmi, HDMITX_TOP_INTR_MASKN,
 				       HDMITX_TOP_INTR_CORE);
 
+	return 0;
 }
 
 static void meson_disable_clk(void *data)
@@ -754,7 +764,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
-	meson_dw_hdmi_init(meson_dw_hdmi);
+	ret = meson_dw_hdmi_init(meson_dw_hdmi);
+	if (ret)
+		return ret;
 
 	/* Bridge / Connector */
 
@@ -819,11 +831,14 @@ static int __maybe_unused meson_dw_hdmi_pm_suspend(struct device *dev)
 static int __maybe_unused meson_dw_hdmi_pm_resume(struct device *dev)
 {
 	struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev);
+	int ret;
 
 	if (!meson_dw_hdmi)
 		return 0;
 
-	meson_dw_hdmi_init(meson_dw_hdmi);
+	ret = meson_dw_hdmi_init(meson_dw_hdmi);
+	if (ret)
+		return ret;
 
 	dw_hdmi_resume(meson_dw_hdmi->hdmi);
 
-- 
2.43.0


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

Re: [RFC PATCH] drm/meson: dw-hdmi: Propagate reset failures during initialization

[sashiko-bot]

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

Pre-existing issues:
- [High] The shared IRQ handler is registered before the hardware is fully initialized, risking a NULL pointer dereference if initialization fails.
- [High] Component bind error paths permanently leak `devm_` allocated resources.
--

commit f74bf01d3ded8b1822f2278b49d5f000e0d72a6b
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

drm/meson: dw-hdmi: Propagate reset failures during initialization

This commit updates meson_dw_hdmi_init() to propagate reset errors instead
of ignoring them. It also updates the bind and resume paths to abort if
initialization fails.

> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c

[ ... ]

> @@ -754,7 +764,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>  		return ret;
>  	}
>  
> -	meson_dw_hdmi_init(meson_dw_hdmi);
> +	ret = meson_dw_hdmi_init(meson_dw_hdmi);
> +	if (ret)
> +		return ret;

[Severity: High]
This is a pre-existing issue, but does this new error path expose us to a
NULL pointer dereference from the shared IRQ?

Earlier in meson_dw_hdmi_bind(), a shared IRQ is requested using a devm_
function:

	ret = devm_request_threaded_irq(dev, irq, dw_hdmi_top_irq,
					dw_hdmi_top_thread_irq, IRQF_SHARED,
					"dw_hdmi_top_irq", meson_dw_hdmi);

If meson_dw_hdmi_init() fails and returns early here, the IRQ remains active
because its lifetime is tied to the platform device, not the bind lifecycle.
If the shared IRQ fires, dw_hdmi_top_thread_irq() unconditionally 
dereferences dw_hdmi->hdmi and dw_hdmi->bridge, which will be NULL since
dw_hdmi_probe() was skipped.

[Severity: High]
This isn't a bug introduced by this patch, but does returning early here
also cause unbounded resource leaks?

Because meson_dw_hdmi_bind() is a component bind function, it runs
repeatedly on deferred probe or unbind/bind cycles. It allocates resources
using devm_ allocation functions, such as:

	meson_dw_hdmi = devm_kzalloc(dev, sizeof(*meson_dw_hdmi),
				     GFP_KERNEL);

If this bind operation fails at meson_dw_hdmi_init(), these devm_ resources
are not automatically freed since the underlying platform device is not
destroyed. Can this lead to memory and IRQ exhaustion over time?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828091424.6375-1-pengpeng@iscas.ac.cn?part=1

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