Re: [PATCH v6 2/7] drm/bridge: Implement generic USB Type-C DP HPD bridge

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

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy