Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [High] Clocks acquired via `of_clk_get_by_name()` are never released, causing a permanent memory and module reference leak. - [High] The bridge reference acquired via `of_drm_find_and_get_bridge()` is leaked on error paths and during driver unbind. - [High] The driver uses `of_find_i2c_adapter_by_node()` instead of `of_get_i2c_adapter_by_node()`, failing to pin the I2C adapter module. -- --- Patch [1]: [PATCH v3] drm/mediatek: mtk_hdmi: Fix DDC adapter double put in v2 --- commit 0f15e03f2f19d66ee64acb93c6d5a19fae97377f Author: Guangshuo Li <[email protected]> Subject: drm/mediatek: mtk_hdmi: Fix DDC adapter double put in v2 This commit fixes a double-put issue with the DDC I2C adapter in the HDMI v2 driver. The probe function registers a devm action to release the adapter, but the remove callback also manually dropped the reference. This removes the manual drop and the empty remove callback. > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > b/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > index d9316d30fd5f7..f0277fcf3345b 100644 > --- a/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > @@ -1499,13 +1499,6 @@ static int mtk_hdmi_v2_probe(struct platform_device > *pdev) > return 0; > } > > -static void mtk_hdmi_v2_remove(struct platform_device *pdev) > -{ > - struct mtk_hdmi *hdmi = platform_get_drvdata(pdev); > - > - i2c_put_adapter(hdmi->ddc_adpt); > -} > - [Severity: High] This isn't a bug introduced by this patch, but while you are cleaning up the I2C adapter references here, the underlying use of of_find_i2c_adapter_by_node() in mtk_hdmi_dt_parse_pdata() might be incorrect: drivers/gpu/drm/mediatek/mtk_hdmi_common.c:mtk_hdmi_dt_parse_pdata() { ... hdmi->ddc_adpt = of_find_i2c_adapter_by_node(i2c_np); ... } It appears of_find_i2c_adapter_by_node() only increments the device reference count, but does not pin the I2C adapter driver module. Could this allow the adapter module to be unexpectedly unloaded while the HDMI driver is still bound? If drm_edid_read_ddc() later attempts to read the EDID via i2c_transfer(), it might cause a crash by accessing the unloaded module. Would it be safer to use of_get_i2c_adapter_by_node() to properly pin the module? [Severity: High] This is a pre-existing issue, but I noticed there might be a memory and module reference leak during clock initialization. drivers/gpu/drm/mediatek/mtk_hdmi_common.c:mtk_hdmi_get_all_clk() { ... hdmi->clk[i] = of_clk_get_by_name(np, clock_names[i]); ... } The of_clk_get_by_name() function allocates a new struct clk and increments the provider's module reference count, but these clocks don't seem to ever be released with clk_put(). If the probe is later deferred (e.g., returning -EPROBE_DEFER when waiting for the remote bridge or DDC I2C adapter), will all successfully acquired clocks be permanently leaked? [Severity: High] This is also a pre-existing issue, but there appears to be a leaked bridge module reference on error paths: drivers/gpu/drm/mediatek/mtk_hdmi_common.c:mtk_hdmi_dt_parse_pdata() { ... hdmi->bridge.next_bridge = of_drm_find_and_get_bridge(remote); ... } This successfully bumps the module reference count of the bridge driver. However, if the subsequent call to of_find_i2c_adapter_by_node() fails and returns -EPROBE_DEFER, the driver bails out of probe without dropping the bridge reference via drm_bridge_put(). Could this permanently leak the bridge module reference count, preventing the external bridge module from being unloaded? > static const struct of_device_id mtk_drm_hdmi_v2_of_ids[] = { > { .compatible = "mediatek,mt8188-hdmi-tx", .data = > &mtk_hdmi_conf_mt8188 }, > { .compatible = "mediatek,mt8195-hdmi-tx", .data = > &mtk_hdmi_conf_mt8195 }, > @@ -1515,7 +1508,6 @@ MODULE_DEVICE_TABLE(of, mtk_drm_hdmi_v2_of_ids); > > static struct platform_driver mtk_hdmi_v2_driver = { > .probe = mtk_hdmi_v2_probe, > - .remove = mtk_hdmi_v2_remove, > .driver = { > .name = "mediatek-drm-hdmi-v2", -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
