Re: [PATCH v6 1/8] usb: typec: Add default HPD device when register DisplayPort altmode

[Chaoyi Chen <chaoyi.chen@rock-chips.com>]

Hi Heikki,

On 10/20/2025 6:02 PM, Heikki Krogerus wrote:
> On Thu, Oct 16, 2025 at 10:27:34AM +0800, Chaoyi Chen wrote:
>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>
>> Add default DRM AUX HPD bridge device when register DisplayPort
>> altmode. That makes it redundant for each Type-C driver to implement
>> a similar registration process in embedded scenarios.
>>
>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>> ---
>>
>> Changes in v6:
>> - Fix depend in Kconfig.
>>
>> Changes in v5:
>> - Remove the calls related to `drm_aux_hpd_bridge_notify()`.
>> - Place the helper functions in the same compilation unit.
>> - Add more comments about parent device.
>>
>>   drivers/usb/typec/Kconfig         |  2 ++
>>   drivers/usb/typec/class.c         | 26 ++++++++++++++++++++++++++
>>   include/linux/usb/typec_altmode.h |  2 ++
>>   3 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
>> index 2f80c2792dbd..a6730fbb576b 100644
>> --- a/drivers/usb/typec/Kconfig
>> +++ b/drivers/usb/typec/Kconfig
>> @@ -2,6 +2,8 @@
>>   
>>   menuconfig TYPEC
>>   	tristate "USB Type-C Support"
>> +	depends on DRM || DRM=n
>> +	select DRM_AUX_HPD_BRIDGE if DRM_BRIDGE && OF
> This is wrong. DRM should not dictate how this entire subsystem core
> is configured. The dependency needs to be on the DRM bridge side.
>
> You can for example use the bus notification there to see when a new
> alternate mode is being registered, or use some other notification
> mechanism.

Is it a good idea to implement notification functions like drivers/usb/core/notify.c in TCPM, and then let other subsystems (such as DRM) listen to these notifications?


>
> thanks,
>
>>   	help
>>   	  USB Type-C Specification defines a cable and connector for USB where
>>   	  only one type of plug is supported on both ends, i.e. there will not
>> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
>> index 67a533e35150..e9d7772d1a8f 100644
>> --- a/drivers/usb/typec/class.c
>> +++ b/drivers/usb/typec/class.c
>> @@ -8,14 +8,18 @@
>>   
>>   #include <linux/module.h>
>>   #include <linux/mutex.h>
>> +#include <linux/of.h>
>>   #include <linux/property.h>
>>   #include <linux/slab.h>
>>   #include <linux/string_choices.h>
>>   #include <linux/usb/pd_vdo.h>
>> +#include <linux/usb/typec_dp.h>
>>   #include <linux/usb/typec_mux.h>
>>   #include <linux/usb/typec_retimer.h>
>>   #include <linux/usb.h>
>>   
>> +#include <drm/bridge/aux-bridge.h>
>> +
>>   #include "bus.h"
>>   #include "class.h"
>>   #include "pd.h"
>> @@ -538,6 +542,21 @@ const struct device_type typec_altmode_dev_type = {
>>   	.release = typec_altmode_release,
>>   };
>>   
>> +static void dp_altmode_hpd_device_register(struct typec_altmode *alt)
>> +{
>> +	if (alt->svid != USB_TYPEC_DP_SID)
>> +		return;
>> +
>> +	/*
>> +	 * alt->dev.parent->parent : USB-C controller device
>> +	 * alt->dev.parent         : USB-C connector device
>> +	 */
>> +	alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
>> +						  to_of_node(alt->dev.parent->fwnode));
>> +	if (IS_ERR(alt->hpd_dev))
>> +		alt->hpd_dev = NULL;
>> +}
>> +
>>   static struct typec_altmode *
>>   typec_register_altmode(struct device *parent,
>>   		       const struct typec_altmode_desc *desc)
>> @@ -600,6 +619,13 @@ typec_register_altmode(struct device *parent,
>>   		return ERR_PTR(ret);
>>   	}
>>   
>> +	/*
>> +	 * It is too late to register the HPD device when the DisplayPort
>> +	 * altmode device becomes ready. If the current altmode is DP,
>> +	 * register a static HPD device.
>> +	 */
>> +	dp_altmode_hpd_device_register(&alt->adev);
>> +
>>   	return &alt->adev;
>>   }
>>   
>> diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
>> index b3c0866ea70f..acb0af1b9d5d 100644
>> --- a/include/linux/usb/typec_altmode.h
>> +++ b/include/linux/usb/typec_altmode.h
>> @@ -21,6 +21,7 @@ struct typec_altmode_ops;
>>    * @desc: Optional human readable description of the mode
>>    * @ops: Operations vector from the driver
>>    * @cable_ops: Cable operations vector from the driver.
>> + * @hpd_dev: HPD device for DisplayPort
>>    */
>>   struct typec_altmode {
>>   	struct device			dev;
>> @@ -32,6 +33,7 @@ struct typec_altmode {
>>   	char				*desc;
>>   	const struct typec_altmode_ops	*ops;
>>   	const struct typec_cable_ops	*cable_ops;
>> +	struct device			*hpd_dev;
>>   };
>>   
>>   #define to_typec_altmode(d) container_of(d, struct typec_altmode, dev)
>> -- 
>> 2.49.0

-- 
Best,
Chaoyi