[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20250116094249.1.I29b0b621abb613ddc70ab4996426a3909e1aa75f@changeid>
Date: Thu, 16 Jan 2025 09:42:50 -0800
From: Douglas Anderson <dianders@...omium.org>
To: Chun-Kuang Hu <chunkuang.hu@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>
Cc: Douglas Anderson <dianders@...omium.org>,
Alexandre Mergnat <amergnat@...libre.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
CK Hu <ck.hu@...iatek.com>,
David Airlie <airlied@...il.com>,
Matthias Brugger <matthias.bgg@...il.com>,
Simona Vetter <simona@...ll.ch>,
dri-devel@...ts.freedesktop.org,
linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org
Subject: [PATCH] drm/mediatek: dp: drm_err => dev_err in HPD path to avoid NULL ptr
The function mtk_dp_wait_hpd_asserted() may be called before the
`mtk_dp->drm_dev` pointer is assigned in mtk_dp_bridge_attach().
Specifically it can be called via this callpath:
- mtk_edp_wait_hpd_asserted
- [panel probe]
- dp_aux_ep_probe
Using "drm" level prints anywhere in this callpath causes a NULL
pointer dereference. Change the error message directly in
mtk_dp_wait_hpd_asserted() to dev_err() to avoid this. Also change the
error messages in mtk_dp_parse_capabilities(), which is called by
mtk_dp_wait_hpd_asserted().
While touching these prints, also add the error code to them to make
future debugging easier.
Fixes: 7eacba9a083b ("drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus")
Signed-off-by: Douglas Anderson <dianders@...omium.org>
---
Unfortunately, I have only been able to compile-time test this code. I
hit the NULL pointer dereference on a device that's nowhere near
upstream and it was running (sigh) a heavily modified copy of this
code where the eDP stuff has been forked out of DP. Specifically, you
can see <https://crrev.com/c/6073744>. It's pretty easy to understand
that the same problem affects both codebases though, so I'm posting
this "blind" in the hopes to at least fix upstream.
I'll also note that the fact that mtk_edp_wait_hpd_asserted() calls
mtk_dp_parse_capabilities() feels weird/wrong to me based on other eDP
code I've worked on, but I've only barely looked at the Mediatek
driver and perhaps others have already debated this. In any case,
that's not directly related to this patch.
drivers/gpu/drm/mediatek/mtk_dp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 0687672f0e52..ccd104d8851f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1763,7 +1763,7 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp)
ret = drm_dp_dpcd_readb(&mtk_dp->aux, DP_MSTM_CAP, &val);
if (ret < 1) {
- drm_err(mtk_dp->drm_dev, "Read mstm cap failed\n");
+ dev_err(mtk_dp->dev, "Read mstm cap failed: %zd\n", ret);
return ret == 0 ? -EIO : ret;
}
@@ -1773,7 +1773,7 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp)
DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
&val);
if (ret < 1) {
- drm_err(mtk_dp->drm_dev, "Read irq vector failed\n");
+ dev_err(mtk_dp->dev, "Read irq vector failed: %zd\n", ret);
return ret == 0 ? -EIO : ret;
}
@@ -2056,7 +2056,7 @@ static int mtk_dp_wait_hpd_asserted(struct drm_dp_aux *mtk_aux, unsigned long wa
ret = mtk_dp_parse_capabilities(mtk_dp);
if (ret) {
- drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
+ dev_err(mtk_dp->dev, "Can't parse capabilities: %d\n", ret);
return ret;
}
--
2.48.0.rc2.279.g1de40edade-goog
Powered by blists - more mailing lists