Re: [PATCH v6 6/7] drm/rockchip: cdn-dp: Support handle lane info without extcon

Thank you for your contribution! Sashiko AI review found 2 potential issue(=
s) to consider:
- [Critical] Logic error in pointer error checking breaks probe for all exi=
sting boards using extcon.
- [High] Missing hotplug detection (HPD) mechanism when extcon is absent.
--

--- Patch [6]: [PATCH v6 6/7] drm/rockchip: cdn-dp: Support handle lane inf=
o without extcon ---
commit 2ec5f110f1557ebaec82cbe6b39d4e2ab7f4a88f
Author: Chaoyi Chen <[email protected]>

drm/rockchip: cdn-dp: Support handle lane info without extcon

This patch add support for get PHY lane info without help of extcon.

There is no extcon needed if the Type-C controller is present. In this
case, the lane info can be get from PHY instead of extcon.

The extcon device should still be supported if Type-C controller is
not present.

> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/roc=
kchip/cdn-dp-core.c
> index 190cce9530c82..74e1f5422a6e2 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c

[ ... ]

> @@ -1027,6 +1033,9 @@ static int cdn_dp_bind(struct device *dev, struct d=
evice *master, void *data)
>  	for (i =3D 0; i < dp->ports; i++) {
>  		port =3D dp->port[i];
> =20
> +		if (!port->extcon)
> +			continue;
> +

[Severity: High]
Does skipping the notifier registration when extcon is absent break hotplug=
=20
detection?=20

The driver relies on cdn_dp_pd_event_work() to transition dp->connected to=
=20
true, load firmware, and train the link. Without the extcon notifier,=20
event_work is never scheduled on hotplug.=20

Since cdn_dp_bridge_detect() simply returns the static dp->connected state:

drivers/gpu/drm/rockchip/cdn-dp-core.c:cdn_dp_bridge_detect() {
    mutex_lock(&dp->lock);
    if (dp->connected)
        status =3D connector_status_connected;
    mutex_unlock(&dp->lock);
    ...
}

Could KMS polling always see the port as disconnected when hardware is=20
configured without an extcon device?

>  		port->event_nb.notifier_call =3D cdn_dp_pd_event;
>  		ret =3D devm_extcon_register_notifier(dp->dev, port->extcon,
>  						    EXTCON_DISP_DP,

[ ... ]

> @@ -1119,14 +1128,14 @@ static int cdn_dp_probe(struct platform_device *p=
dev)
>  		    PTR_ERR(phy) =3D=3D -EPROBE_DEFER)
>  			return -EPROBE_DEFER;
> =20
> -		if (IS_ERR(extcon) || IS_ERR(phy))
> +		if (IS_ERR(phy) || PTR_ERR(extcon) !=3D -ENODEV)
>  			continue;
> =20

[Severity: Critical]
Will this condition accidentally skip port initialization for all valid ext=
con
pointers?

If extcon is a valid pointer, PTR_ERR(extcon) evaluates to its memory addre=
ss,
which will never equal -ENODEV. This causes the condition to evaluate to tr=
ue,
triggering the continue statement and bypassing port setup completely for a=
ny
hardware that has an extcon device.

>  		port =3D devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
>  		if (!port)
>  			return -ENOMEM;
> =20
> -		port->extcon =3D extcon;
> +		port->extcon =3D IS_ERR(extcon) ? NULL : extcon;
>  		port->phy =3D phy;

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804070730.68-1=
[email protected]?part=3D6