Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Devres allocations in component bind() cause permanent initialization failure (-EBUSY) on probe deferral. - [High] Active devm-managed IRQ and I2C handlers access hardware with the clock disabled after unbind(), causing a system crash. -- --- Patch [1]: [PATCH v1] drm/rockchip: rk3066_hdmi: convert to devm_drm_bridge_alloc() API --- commit 2ee09d3b212b24956cbceb44c6bf494785f5586d Author: Johan Jonker <[email protected]> drm/rockchip: rk3066_hdmi: convert to devm_drm_bridge_alloc() API The function devm_drm_bridge_alloc() is the new API for allocating DRM bridges. This conversion was missed during the initial conversion of all bridges to the new API. > diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c > b/drivers/gpu/drm/rockchip/rk3066_hdmi.c > index f28d6a883e099..39c65ee07caac 100644 > --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c > +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c [ ... ] > @@ -747,9 +746,10 @@ static int rk3066_hdmi_bind(struct device *dev, struct > device *master, > int irq; > int ret; > > - hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL); > - if (!hdmi) > - return -ENOMEM; > + hdmi = devm_drm_bridge_alloc(dev, struct rk3066_hdmi, bridge, > + &rk3066_hdmi_bridge_funcs); > + if (IS_ERR(hdmi)) > + return PTR_ERR(hdmi); [Severity: High] This is a pre-existing issue, but does using devm_ allocations in the component bind() function cause a permanent failure on probe deferral? rk3066_hdmi_bind() allocates devres-managed resources using the component device, including devm_platform_ioremap_resource() right below this hunk. Devres actions are tied to the device's probe/remove lifecycle, not the component bind/unbind lifecycle. If bind() fails, for example when rk3066_hdmi_register() returns -EPROBE_DEFER due to missing CRTCs, the devres resources are not freed. When the master DRM device retries probing and calls bind() again, devm_platform_ioremap_resource() will attempt to request the same physical memory region via request_mem_region(). Because the first request was never released by devres, this fails with -EBUSY, preventing the display from ever initializing. [Severity: High] This is a pre-existing issue, but can the active devm-managed IRQ and I2C handlers access hardware with the clock disabled after unbind(), causing a system crash? In rk3066_hdmi_bind(), the HDMI IRQ handler and I2C adapter are registered using devm_request_threaded_irq() and devm_i2c_add_adapter() on the component device. Because they are devres-managed on the component device's probe/remove lifecycle, they are not removed when the master DRM device unbinds and calls rk3066_hdmi_unbind(). In rk3066_hdmi_unbind(), the hardware bus clock is explicitly disabled: clk_disable_unprepare(hdmi->hclk); If an I2C transfer is initiated from userspace (via /dev/i2c-*), or if a shared IRQ fires after the master is unbound, the still-active handlers (rk3066_hdmi_i2c_xfer or rk3066_hdmi_hardirq) will execute and access HDMI hardware registers. Accessing hardware registers while the clock is disabled causes a synchronous external abort (bus fault), resulting in a kernel panic. > > hdmi->dev = dev; > hdmi->drm_dev = drm; -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
