Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [Medium] Logic error: HPD interrupt is unconditionally unmasked during `setup_hpd`, defeating the patch's intent to defer arming to the bridge framework. Pre-existing issues: - [High] High-severity Use-After-Free and resource leaks due to improper use of `devm_*` APIs in the component `bind` callback. - [High] RK3588 HPD IRQ handler clears the interrupt status a second time when unmasking, causing potential loss of hotplug events. -- --- Patch [34]: [PATCH v10 34/69] drm/rockchip: dw_hdmi_qp: Mask RK3576 HPD IRQ in io_init --- commit 1e56140d6c0038883d14ac31b8ac46f871e3f59e Author: Cristian Ciocaltea <[email protected]> drm/rockchip: dw_hdmi_qp: Mask RK3576 HPD IRQ in io_init This commit masks the HPD interrupt during I/O initialization for RK3576, intending to defer interrupt arming to the bridge framework. > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > @@ -432,7 +432,7 @@ static void dw_hdmi_qp_rk3576_io_init(struct > rockchip_hdmi_qp *hdmi) > > regmap_write(hdmi->vo_regmap, RK3576_VO0_GRF_SOC_CON14, val); > > - val = FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 0); > + val = FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 1); > regmap_write(hdmi->regmap, RK3576_IOC_MISC_CON0, val); > } [Severity: Medium] Does this change actually defer the interrupt arming? Looking at dw_hdmi_qp_rk3576_setup_hpd() in the same file, the interrupt appears to be unconditionally unmasked later during hardware initialization: drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rk3576_setup_hpd() { ... val = (FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_CLR, 1) | FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 0)); ... } Additionally, it appears that rk3576_hdmi_phy_ops lacks the .enable_hpd and .disable_hpd callbacks. Can the bridge framework actually control the interrupt without these callbacks implemented? [Severity: High] This is a pre-existing issue, but I noticed a potential use-after-free in the component bind/unbind lifecycle. In dw_hdmi_qp_rockchip_bind(), devm_request_threaded_irq() is used to allocate the IRQ handler on the platform device: drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rockchip_bind() { ... ret = devm_request_threaded_irq(dev, irq, ...); ... } However, the dev_id passed is hdmi, which is allocated via drmm_kzalloc(). Since devm_ resources are tied to the device probe/remove lifecycle rather than the component bind/unbind lifecycle, the IRQ is not freed in dw_hdmi_qp_rockchip_unbind(). If the master unbinds, the drmm memory is freed, but the IRQ handler remains active. Could an interrupt firing after unbind cause a use-after-free when it accesses the freed hdmi pointer? [Severity: High] This is a pre-existing issue, but does dw_hdmi_qp_rk3588_irq() accidentally clear the interrupt a second time when unmasking? drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rk3588_irq() { ... val |= FIELD_PREP_WM16(RK3588_HDMI0_HPD_INT_MSK, 0); regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val); ... } Because this uses a bitwise OR, val retains the RK3588_HDMI0_HPD_INT_CLR write-enable mask and data bit from earlier in the function. Does this second regmap_write() clear the interrupt again, potentially losing hotplug events that occurred between the two writes? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=34
