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 condition between module initialization and concurrent device addition. --
--- Patch [2]: [PATCH v6 2/7] drm/bridge: Implement generic USB Type-C DP HPD 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/gpu/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 = (struct device *)data; > + struct typec_altmode *alt = to_typec_altmode(dev); > + struct device_node *np; > + > + if (action != 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 == USB_TYPEC_DP_SID) { > + np = 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 == 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 = { > + .notifier_call = 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 > added. > + * 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; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
