Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Using IS_ERR() to validate the return value of platform_get_mem_or_io() fails to catch NULL, leading to a kernel crash when the resource is missing. - [Medium] The `dp_dp_rockchip_get_vo_grf` function fails to implement the documented fallback to read the `vo-grf` phandle from the DisplayPort device node. --
commit 2739e6354d59ce0c07c5ba0c2a12b2779e399b4a Author: Sebastian Reichel <[email protected]> drm/rockchip: dw_dp: Implement out-of-band HPD handling Implement out-of-band hotplug handling to receive external hotplug information from the USB-C state machine. This properly detects if a cable is plugged in for adapters. > diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c > b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c > index 35598ab9fe84a..9c53f1d2c29ab 100644 > --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c [ ... ] > @@ -72,14 +117,49 @@ 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] Does this function actually implement the forward-compatibility behavior described in the comment? If a new platform follows the advice and adds a VO GRF phandle directly to the DisplayPort DT node (dropping the "phys" property), it looks like the unconditional of_parse_phandle_with_args() check for "phys" will fail and return ERR_PTR(-ENODEV) early. Should there be a primary lookup path for "rockchip,vo-grf" on the DP node itself before falling back to this PHY workaround? > + > +out_put_node: > + of_node_put(args.np); > + return regmap; > +} > + > static int dw_dp_rockchip_bind(struct device *dev, struct device *master, > void *data) > { > - const struct dw_dp_plat_data *plat_data; > + const struct rockchip_dw_dp_plat_data *plat_data_const; > + struct platform_device *pdev = to_platform_device(dev); > + struct dw_dp_plat_data *plat_data; > struct drm_device *drm_dev = data; > struct rockchip_dw_dp *dp; > struct drm_encoder *encoder; > struct drm_connector *connector; > - int ret; > + struct resource *res; > + int ret, id; > + > + plat_data = drmm_kzalloc(drm_dev, sizeof(*plat_data), GFP_KERNEL); > + if (!plat_data) > + return -ENOMEM; [ ... ] > + dp->pdata = plat_data_const; > + > + res = platform_get_mem_or_io(pdev, 0); > + if (IS_ERR(res)) > + return PTR_ERR(res); > + > + /* find the DisplayPort ID from the io address */ > + dp->id = -ENODEV; > + for (id = 0; id < plat_data_const->num_ctrls; id++) { > + if (res->start == plat_data_const->ctrl_ids[id]) { > + dp->id = id; > + break; > + } > + } [Severity: High] Can this code cause a NULL pointer dereference if the resource is missing from the device tree? Looking at platform_get_mem_or_io(), it returns NULL when the resource is not found, rather than an ERR_PTR. Because IS_ERR(NULL) evaluates to false, the check "if (IS_ERR(res))" would let a NULL pointer slip through. This NULL pointer would then be dereferenced when accessing res->start in the loop. Should this be updated to check "if (!res)" instead? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260612-synopsys-dw-dp-improvements-v3-0-dc61e6352...@collabora.com?part=6
