Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] Failing probe when deprecated DT properties are missing breaks DT ABI and prevents their deprecation. --
--- Patch [3]: [PATCH v6 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties --- commit b573dba50acc6087122ffa4a65680322f4d004f8 Author: Damon Ding <[email protected]> drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties This commit reverts a change that made the samsung,link-rate and samsung,lane-count properties optional for Exynos DP. It adds error checking to fail the probe early if these properties are missing, aiming to prevent link training failures caused by zero values. > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > index 8cf6b73bceac0..293d8adfe9856 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c [ ... ] > @@ -1258,10 +1259,14 @@ static int analogix_dp_dt_parse_pdata(struct > analogix_dp_device *dp) > * NOTE: those property parseing code is used for > * providing backward compatibility for samsung platform. > */ > - of_property_read_u32(dp_node, "samsung,link-rate", > - &video_info->max_link_rate); > - of_property_read_u32(dp_node, "samsung,lane-count", > - &video_info->max_lane_count); > + if (of_property_read_u32(dp_node, "samsung,link-rate", &val)) > + return dev_err_probe(dp->dev, -EINVAL, > + "Failed to get > samsung,link-rate\n"); [Severity: Critical] Does failing the probe when this property is missing break the DT ABI? Since samsung,link-rate is a deprecated property, a modern schema-compliant Device Tree might intentionally omit it. If a system boots with such a DT, this check will fail the probe with -EINVAL and disable the display. Could the driver instead fall back to hardware maximums or sensible defaults when the deprecated property is absent, rather than returning an error? > + video_info->max_link_rate = val; > + if (of_property_read_u32(dp_node, "samsung,lane-count", &val)) > + return dev_err_probe(dp->dev, -EINVAL, > + "Failed to get > samsung,lane-count\n"); [Severity: Critical] Does making samsung,lane-count mandatory similarly break backward compatibility for valid Device Trees that have intentionally omitted this deprecated property? > + video_info->max_lane_count = val; > break; > } > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
