Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Low] Empty remove callback left in the driver. Pre-existing issues: - [High] Missing module reference pinning for I2C adapter leads to invalid text execution on module unload. -- commit 7124bfd2ca52b03e9596ef39d9090b280144457a Author: Guangshuo Li <[email protected]> drm/mediatek: mtk_hdmi: Fix DDC adapter double put in v2 This commit removes an extra i2c_put_adapter() call from the HDMI v2 remove callback. It relies on the common devm action to release the adapter device reference, fixing a double put issue. > mtk_hdmi_common_probe() gets the DDC adapter with > of_find_i2c_adapter_by_node() and registers a devm action to release the > adapter device reference with put_device(). [Severity: High] This isn't a bug introduced by this patch, but does this missing module reference pinning for the I2C adapter lead to an invalid text execution on module unload? If the I2C adapter driver module is unloaded, the module's .text section is freed because the mtk_hdmi driver only holds a device reference (via of_find_i2c_adapter_by_node()) and not a module reference. If an EDID read is triggered after the I2C adapter module is unloaded, drm_edid_read_ddc() calls i2c_transfer(), which dereferences the stale hdmi->ddc_adpt->algo->master_xfer pointer, causing a kernel panic. Should the driver use of_get_i2c_adapter_by_node() and pair it with i2c_put_adapter() in the devm action instead? > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > b/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > index d9316d30fd5f7..ce6cb952dc683 100644 > --- a/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c [ ... ] > 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: Low] Does this leave an unnecessary empty remove callback in the driver? Since the only statement from mtk_hdmi_v2_remove() is being removed, could this empty function and its assignment in the platform driver struct be removed entirely? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
