Re: [PATCH v6 2/7] drm/bridge: Implement generic USB Type-C DP HPD bridge
[email protected] Tue, 04 Aug 2026 07:25:34 +0000
| Newsgroups | org.freedesktop.lists.dri-devel,org.infradead.lists.linux-phy |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [High] Type-C DP HPD bridges are erroneously shared across multiple ports= on non-DeviceTree (e.g., ACPI) systems due to NULL `of_node` matching. - [Medium] Double registration of the DP HPD bridge due to a TOCTOU race co= ndition between module initialization and concurrent device addition. -- --- Patch [2]: [PATCH v6 2/7] drm/bridge: Implement generic USB Type-C DP H= PD bridge --- commit da360ed1d856c20444642867c8a203f9c48362b1 Author: Chaoyi Chen <[email protected]> drm/bridge: Implement generic USB Type-C DP HPD bridge This patch implements a generic DP HPD bridge based on aux-hpd-bridge.c. It monitors Type-C bus events, and when a Type-C port device containing the DP svid is registered, it will create an HPD bridge for it without the need for the USB-C controller driver to implement it. > diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/g= pu/drm/bridge/aux-hpd-typec-dp-bridge.c > new file mode 100644 > index 0000000000000..682bc192e0934 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c > @@ -0,0 +1,67 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2026 Rockchip Electronics Co., Ltd. > + * > + * Author: Chaoyi Chen <[email protected]> > + */ > +#include <linux/of.h> > +#include <linux/usb/typec_altmode.h> > +#include <linux/usb/typec_dp.h> > + > +#include <drm/bridge/aux-bridge.h> > + > +static int drm_typec_bus_event(struct notifier_block *nb, unsigned long = action, > + void *data) > +{ > + struct device *dev =3D (struct device *)data; > + struct typec_altmode *alt =3D to_typec_altmode(dev); > + struct device_node *np; > + > + if (action !=3D BUS_NOTIFY_ADD_DEVICE) > + return NOTIFY_OK; > + > + /* > + * alt->dev.parent->parent : USB-C controller device > + * alt->dev.parent : USB-C connector device > + */ > + if (is_typec_port_altmode(&alt->dev) && alt->svid =3D=3D USB_TYPEC_DP_S= ID) { > + np =3D to_of_node(alt->dev.parent->fwnode); > + if (!drm_dev_has_dp_hpd_bridge(alt->dev.parent->parent, np)) > + drm_dp_hpd_bridge_register(alt->dev.parent->parent, np); [Severity: High] On non-DeviceTree systems (like ACPI platforms), will this fail to register subsequent DP ports on a single USB-C controller? Since to_of_node() evaluates to NULL for non-OF systems, np will be NULL. When the first port registers the bridge, it uses NULL as the device node. When the second port is processed, drm_dev_has_dp_hpd_bridge() will search for an existing bridge. The underlying hpd_bridge_match() function checks: return adev->dev.platform_data =3D=3D np; Since adev->dev.platform_data is NULL and np is NULL, this returns true, incorrectly concluding that the second port's bridge is already registered. Could this silently break hotplug detection for those additional ports? > + } > + > + return NOTIFY_OK; > +} > + > +static struct notifier_block drm_typec_event_nb =3D { > + .notifier_call =3D drm_typec_bus_event, > +}; > + > +static int check_device_already_added(struct device *dev, void *data) > +{ > + drm_typec_bus_event(NULL, BUS_NOTIFY_ADD_DEVICE, dev); > + return 0; > +} [ ... ] > +static int __init drm_aux_hpd_typec_dp_bridge_module_init(void) > +{ > + bus_register_notifier(&typec_bus, &drm_typec_event_nb); > + /* > + * Before module initialization, some devices may have already been add= ed. > + * Register the HPD bridge for these devices. > + */ > + bus_for_each_dev(&typec_bus, NULL, NULL, check_device_already_added); [Severity: Medium] Could this setup a race condition resulting in a double registration of the DP HPD bridge? If a device is hotplugged precisely during module loading, the notifier runs on the hotplug thread, and bus_for_each_dev() might visit the same device on the init thread. Both threads would evaluate !drm_dev_has_dp_hpd_bridge() as true in drm_typec_bus_event() since it is a lockless check. Would this cause drm_dp_hpd_bridge_register() to be called twice for the same USB-C controller port? > + return 0; > +} --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804070730.68-1= [email protected]?part=3D2