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

[sashiko-bot at kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Module initialization misses Type-C ports that are already registered.
- [High] Missing `BUS_NOTIFY_DEL_DEVICE` handling and incorrect `devm` usage leads to memory leaks and use-after-free on module unload.
- [High] Generic notifier blindly creates duplicate bridges for controllers that already register them natively.
- [High] Blind use of `alt->dev.parent->parent` causes NULL pointer dereference in devm_ functions if the port lacks a parent.
- [Medium] Kconfig entry lacks a prompt string and has an incorrect dependency structure.
--

commit 6278338190ad225c12a2042f8bb6669eb188e362
Author: Chaoyi Chen <chaoyi.chen at rock-chips.com>

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 automatically creates an HPD bridge.

> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index c3209b0f46786..d92e938757931 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -30,6 +30,16 @@ config DRM_AUX_HPD_BRIDGE
>  	  Simple bridge that terminates the bridge chain and provides HPD
>  	  support.
>  
> +if DRM_AUX_HPD_BRIDGE
> +config DRM_AUX_HPD_TYPEC_BRIDGE
> +	tristate
> +	depends on TYPEC || !TYPEC

[Severity: Medium]
Is there a missing prompt string for this tristate? Without it, the option
is invisible in menuconfig.

Also, since this module directly uses symbols like typec_bus, won't
depends on TYPEC || !TYPEC cause a build failure when CONFIG_TYPEC=n since
the dependency evaluates to true?

> +	default TYPEC
> +	help
> +	  Simple bridge that terminates the bridge chain and provides HPD
> +	  support. It build bridge on each USB-C connector device node.
> +endif

[ ... ]

> 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..d915e0fb0668c
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +#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);
> +
> +	if (action != BUS_NOTIFY_ADD_DEVICE)
> +		goto done;

[Severity: High]
What happens when a port is unregistered? Since drm_dp_hpd_bridge_register()
allocates resources using devm tied to the controller device, ignoring
BUS_NOTIFY_DEL_DEVICE means the bridge won't be freed if the port altmode
is dynamically unregistered, leading to duplicate bridges and memory leaks on
re-registration.

If this module is unloaded, the devres callbacks remain attached to the
controller. Will that cause a use-after-free kernel panic when the controller
is unbound later?

> +
> +	/*
> +	 * 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]
Can alt->dev.parent be NULL here? The Type-C subsystem allows ports to be
registered without a parent. If so, dereferencing alt->dev.parent->parent
will cause a NULL pointer dereference.

[Severity: High]
Several Type-C controller drivers already register their own DP HPD bridges.
Does this unconditionally create a second bridge for those ports?

If two bridges are registered for the same device node, the controller driver
will route HPD events to its native bridge, while the DRM driver might bind
to this generic one, breaking hotplug detection.

> +
> +done:
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block drm_typec_event_nb = {
> +	.notifier_call = drm_typec_bus_event,
> +};
> +
> +static void drm_aux_hpd_typec_dp_bridge_module_exit(void)
> +{
> +	bus_unregister_notifier(&typec_bus, &drm_typec_event_nb);
> +}
> +
> +static int __init drm_aux_hpd_typec_dp_bridge_module_init(void)
> +{
> +	bus_register_notifier(&typec_bus, &drm_typec_event_nb);
> +
> +	return 0;
> +}

[Severity: High]
If this module is loaded after Type-C port controllers have already probed,
will it miss the existing ports?

Calling bus_register_notifier() only catches new BUS_NOTIFY_ADD_DEVICE events,
so bridges wouldn't be created for devices already on the bus.

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