Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] The VO GRF regmap lookup fails to parse the DisplayPort DT node for new platforms, contradicting its documented design. Pre-existing issues: - [High] This problem wasn't introduced by this patch, but missing cancel_work_sync() for hpd_work in dw_dp_unbind() causes a Use-After-Free. -- --- Patch [6]: [PATCH v4 06/10] drm/rockchip: dw_dp: Implement out-of-band HPD handling --- commit 78e1ce0dbd7ef8a004080f25b4449ba54b71de3a Author: Sebastian Reichel <[email protected]> drm/rockchip: dw_dp: Implement out-of-band HPD handling This patch implements out-of-band hotplug handling to receive external hotplug information from the USB-C state machine. It uses the vo-grf registers to control HPD signal routing and configuration. > diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c > b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c > index 005938dc66c9f..0ecbad24dd4e8 100644 > --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c [ ... ] > @@ -71,6 +116,34 @@ static const struct drm_encoder_helper_funcs > dw_dp_encoder_helper_funcs = { > .atomic_check = dw_dp_encoder_atomic_check, > }; > > +static struct regmap *dp_dp_rockchip_get_vo_grf(struct rockchip_dw_dp *dp) > +{ > + struct device_node *np = dev_of_node(dp->dev); > + struct of_phandle_args args; > + struct regmap *regmap; > + int ret; > + > + ret = of_parse_phandle_with_args(np, "phys", "#phy-cells", 0, &args); > + if (ret) > + return ERR_PTR(-ENODEV); > + > + /* > + * Limit this workaround to RK3576 and RK3588, new platforms should > + * add a VO GRF phandle in the DisplayPort DT node. > + */ > + if (!of_device_is_compatible(args.np, "rockchip,rk3576-usbdp-phy") && > + !of_device_is_compatible(args.np, "rockchip,rk3588-usbdp-phy")) { > + regmap = ERR_PTR(-ENODEV); > + goto out_put_node; > + } > + > + regmap = syscon_regmap_lookup_by_phandle(args.np, "rockchip,vo-grf"); [Severity: Medium] Will this fail to probe on new platforms? The comment states new platforms should add a VO GRF phandle in the DisplayPort DT node, but the code only looks up "rockchip,vo-grf" on the phy node (args.np) after explicitly rejecting any phy other than rk3576 and rk3588. Should it try to parse the property from the DisplayPort node (np) instead? > + > +out_put_node: > + of_node_put(args.np); > + return regmap; > +} [ ... ] While reviewing this patch, I also noticed a pre-existing issue in the related dw_dp_unbind() teardown path: drivers/gpu/drm/bridge/synopsys/dw-dp.c:dw_dp_unbind() { ... disable_irq(dp->irq); phy_exit(dp->phy); drm_bridge_put(dp->next_bridge); drm_dp_aux_unregister(&dp->aux); drm_bridge_remove(&dp->bridge); } [Severity: High] This isn't a bug introduced by this patch, but can this cause a use-after-free if an HPD interrupt fires right before disable_irq() is called? If dp->hpd_work is scheduled and the devm-allocated struct dw_dp is freed during device removal, the background worker might execute dw_dp_hpd_work() on freed memory. Does dw_dp_unbind() need to call cancel_work_sync(&dp->hpd_work)? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260721-synopsys-dw-dp-improvements-v4-0-f0f4a4ede...@collabora.com?part=6
