Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing `dphy_get_timing` callback initialization in `rk3506_chip_data` causes a NULL pointer dereference on the RK3506 platform. --
--- Patch [2]: [PATCH v3 2/3] drm/rockchip: dsi: Add dphy_get_timing support for multiple PHY types --- commit 266dcada14f6495a40e461eefa6793e6e1dcb8ca Author: Chaoyi Chen <[email protected]> drm/rockchip: dsi: Add dphy_get_timing support for multiple PHY types This commit introduces platform-specific D-PHY timing callbacks to handle timing differences between older and newer Rockchip platforms. > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > index fd91343d0f6d5..557708bc4faa4 100644 > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c [ ... ] > @@ -737,6 +742,32 @@ dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int > lane_mbps, > return 0; > } > > +static const struct dw_mipi_dsi_dphy_timing dphy_timing_px30 = { > + .clk_lp2hs = 0x40, > + .clk_hs2lp = 0x40, > + .data_lp2hs = 0x10, > + .data_hs2lp = 0x14, > +}; > + > +static int > +dw_mipi_dsi_phy_px30_get_timing(struct dw_mipi_dsi_rockchip *dsi, > + unsigned int lane_mbps, > + struct dw_mipi_dsi_dphy_timing *timing) > +{ > + *timing = dphy_timing_px30; > + > + return 0; > +} > + > +static int > +dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps, > + struct dw_mipi_dsi_dphy_timing *timing) > +{ > + struct dw_mipi_dsi_rockchip *dsi = priv_data; > + > + return dsi->cdata->dphy_get_timing(dsi, lane_mbps, timing); [Severity: High] Will this cause a NULL pointer dereference on the RK3506 platform? The patch updates multiple platform definitions to set the .dphy_get_timing callback, but it appears to have missed rk3506_chip_data, leaving its callback as NULL. When dw_mipi_dsi_phy_get_timing() is called during modeset on RK3506, it unconditionally dereferences dsi->cdata->dphy_get_timing, which will lead to an immediate crash. Should a NULL check be added here, or should rk3506_chip_data be updated to include a timing callback? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
