Re: [PATCH v5 1/6] drm/bridge: Implement generic USB Type-C DP HPD bridge

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Resource leak and duplicate bridge registration on module reload due to lifecycle mismatch.
- [High] Duplicate registration conflict with USB-C controller drivers that natively implement HPD bridges.
- [Medium] Race condition between bus notifier registration and device iteration leading to duplicate bridge registration.
--

--- Patch [1]: [PATCH v5 1/6] drm/bridge: Implement generic USB Type-C DP HPD bridge ---
commit 9877135b6e3e85a72f21170c14a083ad3e0308af
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

drm/bridge: Implement generic USB Type-C DP HPD bridge

This patch implements a generic USB Type-C DP HPD bridge that monitors the
Type-C bus for devices containing the DP SVID. It automatically creates an
HPD bridge for these devices without requiring the USB-C controller driver
to implement it manually.

> 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..43af3ea20f201
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> @@ -0,0 +1,64 @@

[ ... ]

> +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);
> +
> +	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)
> +		drm_dp_hpd_bridge_register(alt->dev.parent->parent,
> +					   to_of_node(alt->dev.parent->fwnode));

[Severity: High]
Does this unconditional registration create duplicate bridges for controller
drivers (like fusb302 or qcom_pmic_typec) that already manually allocate
their own HPD bridge for the same device node?

If the DRM subsystem binds to the generic bridge instead of the native one,
could HPD events sent by the controller via drm_aux_hpd_bridge_notify() to
its private bridge be lost?

[ ... ]

> +static void drm_aux_hpd_typec_dp_bridge_module_exit(void)
> +{
> +	bus_unregister_notifier(&typec_bus, &drm_typec_event_nb);
> +}

[Severity: High]
Does this leak resources and create duplicate bridges if the module is unloaded
and reloaded?

Since drm_dp_hpd_bridge_register() uses devm semantics tied to the USB-C
controller device (alt->dev.parent->parent) rather than this observer module,
the bridges will outlive the module. When the module is reloaded, it will
iterate the bus and unconditionally allocate a second set of duplicate bridges.

> +
> +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]
Is there a race condition here between registering the bus notifier and iterating
over the existing devices?

If a Type-C altmode device is added concurrently during module initialization,
could it trigger the notifier callback and also be processed by the bus iteration
loop? Since drm_dp_hpd_bridge_register() lacks deduplication logic, this TOCTOU
race might result in duplicate bridge registrations.

> +	return 0;
> +}
> +
> +module_init(drm_aux_hpd_typec_dp_bridge_module_init);
> +module_exit(drm_aux_hpd_typec_dp_bridge_module_exit);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730013349.51-1-kernel@airkyi.com?part=1