From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

Some other part of kernel may want to know the event of typec bus.

This patch add common notifier function to notify these event.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---

Changes in v10:
- Notify TYPEC_ALTMODE_UNREGISTERED when altmode removed. 

Changes in v9:
- Remove redundant header files.

Changes in v8:
- Fix coding style.

 drivers/usb/typec/Makefile        |  2 +-
 drivers/usb/typec/bus.h           |  2 ++
 drivers/usb/typec/class.c         |  3 +++
 drivers/usb/typec/notify.c        | 23 +++++++++++++++++++++++
 include/linux/usb/typec_altmode.h |  9 +++++++++
 5 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/typec/notify.c

diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 7a368fea61bc..20d09c5314d7 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_TYPEC)		+= typec.o
-typec-y				:= class.o mux.o bus.o pd.o retimer.o
+typec-y				:= class.o mux.o notify.o bus.o pd.o retimer.o
 typec-$(CONFIG_ACPI)		+= port-mapper.o
 obj-$(CONFIG_TYPEC)		+= altmodes/
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
diff --git a/drivers/usb/typec/bus.h b/drivers/usb/typec/bus.h
index 643b8c81786d..820b59b6d434 100644
--- a/drivers/usb/typec/bus.h
+++ b/drivers/usb/typec/bus.h
@@ -26,6 +26,8 @@ struct altmode {
 	struct altmode			*plug[2];
 };
 
+void typec_notify_event(unsigned long event, void *data);
+
 #define to_altmode(d) container_of(d, struct altmode, adev)
 
 extern const struct bus_type typec_bus;
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9b2647cb199b..1ccf5385d559 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -600,6 +600,8 @@ typec_register_altmode(struct device *parent,
 		return ERR_PTR(ret);
 	}
 
+	typec_notify_event(TYPEC_ALTMODE_REGISTERED, &alt->adev);
+
 	return &alt->adev;
 }
 
@@ -614,6 +616,7 @@ void typec_unregister_altmode(struct typec_altmode *adev)
 {
 	if (IS_ERR_OR_NULL(adev))
 		return;
+	typec_notify_event(TYPEC_ALTMODE_UNREGISTERED, adev);
 	typec_retimer_put(to_altmode(adev)->retimer);
 	typec_mux_put(to_altmode(adev)->mux);
 	device_unregister(&adev->dev);
diff --git a/drivers/usb/typec/notify.c b/drivers/usb/typec/notify.c
new file mode 100644
index 000000000000..9e50b54da359
--- /dev/null
+++ b/drivers/usb/typec/notify.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/notifier.h>
+
+#include "bus.h"
+
+static BLOCKING_NOTIFIER_HEAD(typec_notifier_list);
+
+int typec_altmode_register_notify(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&typec_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(typec_altmode_register_notify);
+
+int typec_altmode_unregister_notify(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&typec_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(typec_altmode_unregister_notify);
+
+void typec_notify_event(unsigned long event, void *data)
+{
+	blocking_notifier_call_chain(&typec_notifier_list, event, data);
+}
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index f7db3bd4c90e..59e20702504b 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -10,6 +10,7 @@
 #define MODE_DISCOVERY_MAX	6
 
 struct typec_altmode_ops;
+struct notifier_block;
 
 /**
  * struct typec_altmode - USB Type-C alternate mode device
@@ -77,6 +78,14 @@ int typec_altmode_notify(struct typec_altmode *altmode, unsigned long conf,
 const struct typec_altmode *
 typec_altmode_get_partner(struct typec_altmode *altmode);
 
+enum usb_typec_event {
+	TYPEC_ALTMODE_REGISTERED,
+	TYPEC_ALTMODE_UNREGISTERED,
+};
+
+int typec_altmode_register_notify(struct notifier_block *nb);
+int typec_altmode_unregister_notify(struct notifier_block *nb);
+
 /**
  * struct typec_cable_ops - Cable alternate mode operations vector
  * @enter: Operations to be executed with Enter Mode Command
-- 
2.51.1

On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> Some other part of kernel may want to know the event of typec bus.

Be specific, WHAT part of the kernel will need to know this?

And why a new notifier, why not just use the existing notifiers that you
already have?  And what is this going to be used for?

Notifiers are a pain, and should almost never be added.  Use real
function calls instead.

thanks,

greg k-h

Hi Greg,

On 11/21/2025 10:07 PM, Greg Kroah-Hartman wrote:
> On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote:
>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>
>> Some other part of kernel may want to know the event of typec bus.
> Be specific, WHAT part of the kernel will need to know this?

For now, it is DRM.


>
> And why a new notifier, why not just use the existing notifiers that you
> already have?  And what is this going to be used for?

We have discussed this before, but the current bus notifier cannot achieve the expected notification [0].

[0] https://lore.kernel.org/all/aPsuLREPS_FEV3DS@kuha.fi.intel.com/


>
> Notifiers are a pain, and should almost never be added.  Use real
> function calls instead.

In v6, I used direct function calls, but had to switch to notifiers because couldn't resolve the dependencies between DRM and Type-C [1]. Do you have any good ideas? Thank you.

[1] https://lore.kernel.org/all/aPYImGmesrZWwyqh@kuha.fi.intel.com/


>
> thanks,
>
> greg k-h
>
>
-- 
Best,
Chaoyi

On Mon, Nov 24, 2025 at 09:40:03AM +0800, Chaoyi Chen wrote:
> Hi Greg,
> 
> On 11/21/2025 10:07 PM, Greg Kroah-Hartman wrote:
> > On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote:
> > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > 
> > > Some other part of kernel may want to know the event of typec bus.
> > Be specific, WHAT part of the kernel will need to know this?
> 
> For now, it is DRM.

Then say this.

> > And why a new notifier, why not just use the existing notifiers that you
> > already have?  And what is this going to be used for?
> 
> We have discussed this before, but the current bus notifier cannot achieve the expected notification [0].
> 
> [0] https://lore.kernel.org/all/aPsuLREPS_FEV3DS@kuha.fi.intel.com/

Then you need to document the heck out of this in the changelog text.
But I'm still not quite understanding why the bus notifier does not work
here, as you only want this information if the usb device is bound to
the bus there, you do not want to know this if it did not complete.

That thread says you want this not "too late", but why?  What is the
problem there, and how will you handle your code getting loaded after
the typec code is loaded?  Notifier callbacks don't work for that
situation, right?

> > Notifiers are a pain, and should almost never be added.  Use real
> > function calls instead.
> 
> In v6, I used direct function calls, but had to switch to notifiers because couldn't resolve the dependencies between DRM and Type-C [1]. Do you have any good ideas? Thank you.

Only allow this DRM code to be built if typec code is enabled, do NOT
use a select, use a depends in the drm code.

thanks,

greg k-h

Hi Greg,

On 11/24/2025 3:10 PM, Greg Kroah-Hartman wrote:

> On Mon, Nov 24, 2025 at 09:40:03AM +0800, Chaoyi Chen wrote:
>> Hi Greg,
>>
>> On 11/21/2025 10:07 PM, Greg Kroah-Hartman wrote:
>>> On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote:
>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>
>>>> Some other part of kernel may want to know the event of typec bus.
>>> Be specific, WHAT part of the kernel will need to know this?
>> For now, it is DRM.
> Then say this.

Okay, please refer to the discussion below.

>
>>> And why a new notifier, why not just use the existing notifiers that you
>>> already have?  And what is this going to be used for?
>> We have discussed this before, but the current bus notifier cannot achieve the expected notification [0].
>>
>> [0] https://lore.kernel.org/all/aPsuLREPS_FEV3DS@kuha.fi.intel.com/
> Then you need to document the heck out of this in the changelog text.
> But I'm still not quite understanding why the bus notifier does not work
> here, as you only want this information if the usb device is bound to
> the bus there, you do not want to know this if it did not complete.
>
> That thread says you want this not "too late", but why?  What is the
> problem there, and how will you handle your code getting loaded after
> the typec code is loaded?  Notifier callbacks don't work for that
> situation, right?

In fact, the typec_register_altmode() function generates two registered events. The first one is the registered event of the port device,

and the second one is the registered event of the partner device. The second one event only occurs after a Type-C device is inserted.

The bus notifier event does not actually take effect for the port device, because it only sets the bus for the partner device:

     /* The partners are bind to drivers */
     if (is_typec_partner(parent))
         alt->adev.dev.bus = &typec_bus;


I hope it's not too late. In fact, the notifier here will notify DRM to establish a bridge chain.

The downstream DP controller driver hopes to obtain the fwnode of the last-level Type-C device

through this bridge chain to create a DRM connector. And when a device is inserted,

drivers/usb/typec/altmodes/displayport.c can notify the HPD (Hot Plug Detect) event.

If relying on the second event, the bridge chain may never be established, and the operations of the DP driver will be

always deferred. Furthermore, other parts of the display controller driver will also be deferred accordingly.

>
>>> Notifiers are a pain, and should almost never be added.  Use real
>>> function calls instead.
>> In v6, I used direct function calls, but had to switch to notifiers because couldn't resolve the dependencies between DRM and Type-C [1]. Do you have any good ideas? Thank you.
> Only allow this DRM code to be built if typec code is enabled, do NOT
> use a select, use a depends in the drm code.

Sorry, I didn't get your point. Does this mean that the current notifiers approach still needs to be changed to direct function calls?

If so, then based on the previous discussion, typec should not depend on any DRM components. Does this mean that we should add the if (IS_REACHABLE(CONFIG_DRM_AUX_BRIDGE)) before the direct function call?

Additionally, the current version of CONFIG_DRM_AUX_BRIDGE is selected by the DP driver in patch9.

-- 
Best,
Chaoyi

On Mon, Nov 24, 2025 at 04:05:53PM +0800, Chaoyi Chen wrote:
> Hi Greg,
> 
> On 11/24/2025 3:10 PM, Greg Kroah-Hartman wrote:
> 
> > On Mon, Nov 24, 2025 at 09:40:03AM +0800, Chaoyi Chen wrote:
> > > Hi Greg,
> > > 
> > > On 11/21/2025 10:07 PM, Greg Kroah-Hartman wrote:
> > > > On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote:
> > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > 
> > > > > Some other part of kernel may want to know the event of typec bus.
> > > > Be specific, WHAT part of the kernel will need to know this?
> > > For now, it is DRM.
> > Then say this.
> 
> Okay, please refer to the discussion below.
> 
> > 
> > > > And why a new notifier, why not just use the existing notifiers that you
> > > > already have?  And what is this going to be used for?
> > > We have discussed this before, but the current bus notifier cannot achieve the expected notification [0].
> > > 
> > > [0] https://lore.kernel.org/all/aPsuLREPS_FEV3DS@kuha.fi.intel.com/
> > Then you need to document the heck out of this in the changelog text.
> > But I'm still not quite understanding why the bus notifier does not work
> > here, as you only want this information if the usb device is bound to
> > the bus there, you do not want to know this if it did not complete.
> > 
> > That thread says you want this not "too late", but why?  What is the
> > problem there, and how will you handle your code getting loaded after
> > the typec code is loaded?  Notifier callbacks don't work for that
> > situation, right?
> 
> In fact, the typec_register_altmode() function generates two
> registered events. The first one is the registered event of the port
> device, and the second one is the registered event of the partner
> device. The second one event only occurs after a Type-C device is
> inserted.
> The bus notifier event does not actually take effect for the port
> device, because it only sets the bus for the partner device:
> 
>     /* The partners are bind to drivers */
>     if (is_typec_partner(parent))
>         alt->adev.dev.bus = &typec_bus;

Setting the bus is correct, then it needs to be registered with the
driver core so the bus link shows up (and a driver is bound to it.)
That is when the bus notifier can happen, right?

> I hope it's not too late. In fact, the notifier here will notify DRM to establish a bridge chain.

What is a "bridge chain"?

> The downstream DP controller driver hopes to obtain the fwnode of the last-level Type-C device
> through this bridge chain to create a DRM connector. And when a device is inserted,
> drivers/usb/typec/altmodes/displayport.c can notify the HPD (Hot Plug Detect) event.

But aren't you just the driver for the "partner device"?

If not, why isn't a real device being created that you then bind to,
what "fake" type of thing are you attempting to do here that would
require you to do this out-of-band?

> If relying on the second event, the bridge chain may never be established, and the operations of the DP driver will be
> always deferred. Furthermore, other parts of the display controller driver will also be deferred accordingly.

What operations?  What exactly is delayed?  You should not be touching a
device before you have it on your bus, right?

> > > > Notifiers are a pain, and should almost never be added.  Use real
> > > > function calls instead.
> > > In v6, I used direct function calls, but had to switch to notifiers because couldn't resolve the dependencies between DRM and Type-C [1]. Do you have any good ideas? Thank you.
> > Only allow this DRM code to be built if typec code is enabled, do NOT
> > use a select, use a depends in the drm code.
> 
> Sorry, I didn't get your point. Does this mean that the current notifiers approach still needs to be changed to direct function calls?

If you somehow convince me that the existing bus notifiers will not
work, yes :)

> If so, then based on the previous discussion, typec should not depend
> on any DRM components. Does this mean that we should add the if
> (IS_REACHABLE(CONFIG_DRM_AUX_BRIDGE)) before the direct function call?

No, do it properly like any other function call to another subsystem.

> Additionally, the current version of CONFIG_DRM_AUX_BRIDGE is selected
> by the DP driver in patch9.

Don't do "select" if at all possible, always try to do "depends on".

thanks,

greg k-h

On 11/25/2025 12:33 AM, Greg Kroah-Hartman wrote:
> On Mon, Nov 24, 2025 at 04:05:53PM +0800, Chaoyi Chen wrote:
>> Hi Greg,
>>
>> On 11/24/2025 3:10 PM, Greg Kroah-Hartman wrote:
>>
>>> On Mon, Nov 24, 2025 at 09:40:03AM +0800, Chaoyi Chen wrote:
>>>> Hi Greg,
>>>>
>>>> On 11/21/2025 10:07 PM, Greg Kroah-Hartman wrote:
>>>>> On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote:
>>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>
>>>>>> Some other part of kernel may want to know the event of typec bus.
>>>>> Be specific, WHAT part of the kernel will need to know this?
>>>> For now, it is DRM.
>>> Then say this.
>> Okay, please refer to the discussion below.
>>
>>>>> And why a new notifier, why not just use the existing notifiers that you
>>>>> already have?  And what is this going to be used for?
>>>> We have discussed this before, but the current bus notifier cannot achieve the expected notification [0].
>>>>
>>>> [0] https://lore.kernel.org/all/aPsuLREPS_FEV3DS@kuha.fi.intel.com/
>>> Then you need to document the heck out of this in the changelog text.
>>> But I'm still not quite understanding why the bus notifier does not work
>>> here, as you only want this information if the usb device is bound to
>>> the bus there, you do not want to know this if it did not complete.
>>>
>>> That thread says you want this not "too late", but why?  What is the
>>> problem there, and how will you handle your code getting loaded after
>>> the typec code is loaded?  Notifier callbacks don't work for that
>>> situation, right?
>> In fact, the typec_register_altmode() function generates two
>> registered events. The first one is the registered event of the port
>> device, and the second one is the registered event of the partner
>> device. The second one event only occurs after a Type-C device is
>> inserted.
>> The bus notifier event does not actually take effect for the port
>> device, because it only sets the bus for the partner device:
>>
>>      /* The partners are bind to drivers */
>>      if (is_typec_partner(parent))
>>          alt->adev.dev.bus = &typec_bus;
> Setting the bus is correct, then it needs to be registered with the
> driver core so the bus link shows up (and a driver is bound to it.)
> That is when the bus notifier can happen, right?
Yes, this is valid for the partner device. But for the port device, since the bus is not specified here, the corresponding bus notifier will not take effect.

>
>> I hope it's not too late. In fact, the notifier here will notify DRM to establish a bridge chain.
> What is a "bridge chain"?
In DRM, the bridge chain is often used to describe the chain connection relationship
of the output of multi level display conversion chips. The bridge chain we are referring to here
is actually a chain  structure formed by connecting various devices using a simple transparent bridge [0].

For example, the schematic diagram of a bridge chain is as follows:

DP controller bridge -> DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge

Here, apart from the DP controller bridge, the rest are transparent DRM bridges, which are used solely to
describe the link relationships between various devices.


[0] https://patchwork.freedesktop.org/patch/msgid/20231203114333.1305826-2-dmitry.baryshkov@linaro.org
>
>> The downstream DP controller driver hopes to obtain the fwnode of the last-level Type-C device
>> through this bridge chain to create a DRM connector. And when a device is inserted,
>> drivers/usb/typec/altmodes/displayport.c can notify the HPD (Hot Plug Detect) event.
> But aren't you just the driver for the "partner device"?
>
> If not, why isn't a real device being created that you then bind to,
> what "fake" type of thing are you attempting to do here that would
> require you to do this out-of-band?
The HPD event is pass by drm_connector_oob_hotplug_event(), which does not use the device in Type-C.
This function will find the corresponding DRM connector device, and the lookup of the DRM connector is
done through the fwnode.

And the partner device and the port device have the same fwnode.

>
>> If relying on the second event, the bridge chain may never be established, and the operations of the DP driver will be
>> always deferred. Furthermore, other parts of the display controller driver will also be deferred accordingly.
> What operations?  What exactly is delayed?  You should not be touching a
> device before you have it on your bus, right?
To complete the HPD operation, it is necessary to create a drm connector device that
has the appropriate fwnode. This operation will be carried out by the DP controller driver.

As you can see, since it cross multiple devices, we need to set the fwnode to the last device fusb302.
This requires relying on the bridge chain. We can register bridges for multiple devices and then connect
them to form a chain. The connection process is completed through drm_bridge_attach().

A brief example of the process of establishing a bridge chain is as follows, starting from the last bridge:

step1: fusb302 HPD bridge
step2: fsa4480 analog audio switch bridge -> fusb302 HPD bridge
step3: onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
step4: DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
step5: DP controller bridge -> DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge

Step 1 is the most crucial, because essentially, regardless of whether we use notifiers or not, what we ultimately want to achieve is to create an HPD bridge.
The DP controller needs to wait for the subsequent bridge chain to be established because it needs to know the connection relationships of the devices.

The question now is when to create the HPD bridge, during the registration of the port device or during the registration of the partner device.
If it's the latter, then the delay occurs here.

And I don't think I'm touching the Type-C device here. I'm just using the bridge chain to get a suitable fwnode and create a suitable DRM connector device.
The subsequent Type-C HPD events will be on the DRM connector device.

This solution is somewhat complex, and I apologize once again for any confusion caused earlier.

>
>>>>> Notifiers are a pain, and should almost never be added.  Use real
>>>>> function calls instead.
>>>> In v6, I used direct function calls, but had to switch to notifiers because couldn't resolve the dependencies between DRM and Type-C [1]. Do you have any good ideas? Thank you.
>>> Only allow this DRM code to be built if typec code is enabled, do NOT
>>> use a select, use a depends in the drm code.
>> Sorry, I didn't get your point. Does this mean that the current notifiers approach still needs to be changed to direct function calls?
> If you somehow convince me that the existing bus notifiers will not
> work, yes :)
>
>> If so, then based on the previous discussion, typec should not depend
>> on any DRM components. Does this mean that we should add the if
>> (IS_REACHABLE(CONFIG_DRM_AUX_BRIDGE)) before the direct function call?
> No, do it properly like any other function call to another subsystem.
>
>> Additionally, the current version of CONFIG_DRM_AUX_BRIDGE is selected
>> by the DP driver in patch9.
> Don't do "select" if at all possible, always try to do "depends on".
Thank you for clarifying this. However, CONFIG_DRM_AUX_BRIDGE is not exposed in the menu, and it is not intended for the end user to select it by design. Therefore, I think there still needs to be some place to select it?

-- 
Best,
Chaoyi

Tue, Nov 25, 2025 at 10:23:02AM +0800, Chaoyi Chen kirjoitti:
> On 11/25/2025 12:33 AM, Greg Kroah-Hartman wrote:
> > On Mon, Nov 24, 2025 at 04:05:53PM +0800, Chaoyi Chen wrote:
> > > Hi Greg,
> > > 
> > > On 11/24/2025 3:10 PM, Greg Kroah-Hartman wrote:
> > > 
> > > > On Mon, Nov 24, 2025 at 09:40:03AM +0800, Chaoyi Chen wrote:
> > > > > Hi Greg,
> > > > > 
> > > > > On 11/21/2025 10:07 PM, Greg Kroah-Hartman wrote:
> > > > > > On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote:
> > > > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > > 
> > > > > > > Some other part of kernel may want to know the event of typec bus.
> > > > > > Be specific, WHAT part of the kernel will need to know this?
> > > > > For now, it is DRM.
> > > > Then say this.
> > > Okay, please refer to the discussion below.
> > > 
> > > > > > And why a new notifier, why not just use the existing notifiers that you
> > > > > > already have?  And what is this going to be used for?
> > > > > We have discussed this before, but the current bus notifier cannot achieve the expected notification [0].
> > > > > 
> > > > > [0] https://lore.kernel.org/all/aPsuLREPS_FEV3DS@kuha.fi.intel.com/
> > > > Then you need to document the heck out of this in the changelog text.
> > > > But I'm still not quite understanding why the bus notifier does not work
> > > > here, as you only want this information if the usb device is bound to
> > > > the bus there, you do not want to know this if it did not complete.
> > > > 
> > > > That thread says you want this not "too late", but why?  What is the
> > > > problem there, and how will you handle your code getting loaded after
> > > > the typec code is loaded?  Notifier callbacks don't work for that
> > > > situation, right?
> > > In fact, the typec_register_altmode() function generates two
> > > registered events. The first one is the registered event of the port
> > > device, and the second one is the registered event of the partner
> > > device. The second one event only occurs after a Type-C device is
> > > inserted.
> > > The bus notifier event does not actually take effect for the port
> > > device, because it only sets the bus for the partner device:
> > > 
> > >      /* The partners are bind to drivers */
> > >      if (is_typec_partner(parent))
> > >          alt->adev.dev.bus = &typec_bus;
> > Setting the bus is correct, then it needs to be registered with the
> > driver core so the bus link shows up (and a driver is bound to it.)
> > That is when the bus notifier can happen, right?
> Yes, this is valid for the partner device. But for the port device, since the bus is not specified here, the corresponding bus notifier will not take effect.

Perhaps we should just fix this part and make also the port altmodes
part of the bus.

Some background, in case this is not clear; the port alternate mode
devices represent the capability of the ports to support specific
alternate modes. The partner alternate mode devices do the same
for the partner devices attached to the ports, but on top of the
showing the capability, they are also used for the alternate mode
specific communication using the USB Power Delivery VDM (vendor
defined messages). That's why only the partner altmodes are bound
to the generic alternate mode drivers.

And that's why the hack where the port altmodes are not added to the
bus. But maybe it's not needed.

Chaoyi, can you try the attached patch?

> > > I hope it's not too late. In fact, the notifier here will notify DRM to establish a bridge chain.
> > What is a "bridge chain"?
> In DRM, the bridge chain is often used to describe the chain connection relationship
> of the output of multi level display conversion chips. The bridge chain we are referring to here
> is actually a chain  structure formed by connecting various devices using a simple transparent bridge [0].
> 
> For example, the schematic diagram of a bridge chain is as follows:
> 
> DP controller bridge -> DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
> 
> Here, apart from the DP controller bridge, the rest are transparent DRM bridges, which are used solely to
> describe the link relationships between various devices.
> 
> 
> [0] https://patchwork.freedesktop.org/patch/msgid/20231203114333.1305826-2-dmitry.baryshkov@linaro.org
> > 
> > > The downstream DP controller driver hopes to obtain the fwnode of the last-level Type-C device
> > > through this bridge chain to create a DRM connector. And when a device is inserted,
> > > drivers/usb/typec/altmodes/displayport.c can notify the HPD (Hot Plug Detect) event.
> > But aren't you just the driver for the "partner device"?
> > 
> > If not, why isn't a real device being created that you then bind to,
> > what "fake" type of thing are you attempting to do here that would
> > require you to do this out-of-band?
> The HPD event is pass by drm_connector_oob_hotplug_event(), which does not use the device in Type-C.
> This function will find the corresponding DRM connector device, and the lookup of the DRM connector is
> done through the fwnode.
> 
> And the partner device and the port device have the same fwnode.
> 
> > 
> > > If relying on the second event, the bridge chain may never be established, and the operations of the DP driver will be
> > > always deferred. Furthermore, other parts of the display controller driver will also be deferred accordingly.
> > What operations?  What exactly is delayed?  You should not be touching a
> > device before you have it on your bus, right?
> To complete the HPD operation, it is necessary to create a drm connector device that
> has the appropriate fwnode. This operation will be carried out by the DP controller driver.
> 
> As you can see, since it cross multiple devices, we need to set the fwnode to the last device fusb302.
> This requires relying on the bridge chain. We can register bridges for multiple devices and then connect
> them to form a chain. The connection process is completed through drm_bridge_attach().
> 
> A brief example of the process of establishing a bridge chain is as follows, starting from the last bridge:
> 
> step1: fusb302 HPD bridge
> step2: fsa4480 analog audio switch bridge -> fusb302 HPD bridge
> step3: onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
> step4: DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
> step5: DP controller bridge -> DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
> 
> Step 1 is the most crucial, because essentially, regardless of whether we use notifiers or not, what we ultimately want to achieve is to create an HPD bridge.
> The DP controller needs to wait for the subsequent bridge chain to be established because it needs to know the connection relationships of the devices.
> 
> The question now is when to create the HPD bridge, during the registration of the port device or during the registration of the partner device.
> If it's the latter, then the delay occurs here.
> 
> And I don't think I'm touching the Type-C device here. I'm just using the bridge chain to get a suitable fwnode and create a suitable DRM connector device.
> The subsequent Type-C HPD events will be on the DRM connector device.
> 
> This solution is somewhat complex, and I apologize once again for any confusion caused earlier.
> 
> > 
> > > > > > Notifiers are a pain, and should almost never be added.  Use real
> > > > > > function calls instead.
> > > > > In v6, I used direct function calls, but had to switch to notifiers because couldn't resolve the dependencies between DRM and Type-C [1]. Do you have any good ideas? Thank you.
> > > > Only allow this DRM code to be built if typec code is enabled, do NOT
> > > > use a select, use a depends in the drm code.
> > > Sorry, I didn't get your point. Does this mean that the current notifiers approach still needs to be changed to direct function calls?
> > If you somehow convince me that the existing bus notifiers will not
> > work, yes :)
> > 
> > > If so, then based on the previous discussion, typec should not depend
> > > on any DRM components. Does this mean that we should add the if
> > > (IS_REACHABLE(CONFIG_DRM_AUX_BRIDGE)) before the direct function call?
> > No, do it properly like any other function call to another subsystem.
> > 
> > > Additionally, the current version of CONFIG_DRM_AUX_BRIDGE is selected
> > > by the DP driver in patch9.
> > Don't do "select" if at all possible, always try to do "depends on".
> Thank you for clarifying this. However, CONFIG_DRM_AUX_BRIDGE is not exposed in the menu, and it is not intended for the end user to select it by design. Therefore, I think there still needs to be some place to select it?

You don't "select TYPEC", you already "depend on TYPEC", so you are
all set with this one.

thanks,

-- 
heikki
From 8cca23b3064bf8b33e316245732a8baf834142d2 Mon Sep 17 00:00:00 2001
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Date: Tue, 25 Nov 2025 10:38:04 +0100
Subject: [PATCH] usb: typec: Set the bus also for the port altmodes

The port altmodes can't be bound to the altmode drivers
because the altmode drivers are meant for partner
commuication using the VDM (vendor defined messages), but
they can still be part of the bus. The bus will make sure
that the normal bus notifications are available also with
the port altmodes.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/bus.c   | 24 +++++++++++++++++++++++-
 drivers/usb/typec/class.c |  4 +---
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index a884cec9ab7e..1daf7a353c01 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -445,7 +445,23 @@ static struct attribute *typec_attrs[] = {
 	&dev_attr_description.attr,
 	NULL
 };
-ATTRIBUTE_GROUPS(typec);
+
+static umode_t typec_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+	if (is_typec_port(kobj_to_dev(kobj)->parent))
+		return 0;
+	return attr->mode;
+}
+
+static const struct attribute_group typec_group = {
+	.is_visible = typec_is_visible,
+	.attrs = typec_attrs,
+};
+
+static const struct attribute_group *typec_groups[] = {
+	&typec_group,
+	NULL
+};
 
 static int typec_match(struct device *dev, const struct device_driver *driver)
 {
@@ -453,6 +469,9 @@ static int typec_match(struct device *dev, const struct device_driver *driver)
 	struct typec_altmode *altmode = to_typec_altmode(dev);
 	const struct typec_device_id *id;
 
+	if (is_typec_port(dev->parent))
+		return 0;
+
 	for (id = drv->id_table; id->svid; id++)
 		if (id->svid == altmode->svid)
 			return 1;
@@ -469,6 +488,9 @@ static int typec_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	if (add_uevent_var(env, "MODE=%u", altmode->mode))
 		return -ENOMEM;
 
+	if (is_typec_port(dev->parent))
+		return 0;
+
 	return add_uevent_var(env, "MODALIAS=typec:id%04X", altmode->svid);
 }
 
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9b2647cb199b..26bf76328487 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -584,9 +584,7 @@ typec_register_altmode(struct device *parent,
 	if (!is_port)
 		typec_altmode_set_partner(alt);
 
-	/* The partners are bind to drivers */
-	if (is_typec_partner(parent))
-		alt->adev.dev.bus = &typec_bus;
+	alt->adev.dev.bus = &typec_bus;
 
 	/* Plug alt modes need a class to generate udev events. */
 	if (is_typec_plug(parent))
-- 
2.50.1

> +static umode_t typec_is_visible(struct kobject *kobj, struct attribute *attr, int n)
> +{
> +	if (is_typec_port(kobj_to_dev(kobj)->parent))

Why look at the parent?  Doesn't the device have a type that should show
this?

Otherwise, looks good to me.

thanks,

greg k-h

On 11/25/2025 7:49 PM, Greg Kroah-Hartman wrote:
>> +static umode_t typec_is_visible(struct kobject *kobj, struct attribute *attr, int n)
>> +{
>> +	if (is_typec_port(kobj_to_dev(kobj)->parent))
> 
> Why look at the parent?  Doesn't the device have a type that should show
> this?
> 
> Otherwise, looks good to me.

They have same deivce type "typec_altmode_dev_type".
The parent device has a different device type to distinguish between
port device and partner device.

> 
> thanks,
> 
> greg k-h
> 
> 

-- 
Best, 
Chaoyi

Wed, Nov 26, 2025 at 09:46:19AM +0800, Chaoyi Chen kirjoitti:
> On 11/25/2025 7:49 PM, Greg Kroah-Hartman wrote:
> >> +static umode_t typec_is_visible(struct kobject *kobj, struct attribute *attr, int n)
> >> +{
> >> +	if (is_typec_port(kobj_to_dev(kobj)->parent))
> > 
> > Why look at the parent?  Doesn't the device have a type that should show
> > this?
> > 
> > Otherwise, looks good to me.
> 
> They have same deivce type "typec_altmode_dev_type".
> The parent device has a different device type to distinguish between
> port device and partner device.

I was already wondering would it make sense to provide separate device
types for the port, and also plug, alternate modes, but I'm not sure
if that's the right thing to do.

There is a plan to register an "altmode" also for the USB4 mode,
which of course is not an alternate mode. So USB4 will definitely need a
separate device type.

So if we supply separate device types for the port, plug and partner
alternate modes, we need to supply separate device types for port, plug
and partner USB4 mode as well.

We certainly can still do that, but I'm just not sure if it makes
sense?

I'll prepare a new version for this and include a separate patch where
instead of defining separate device types for the port and plug
alternate modes I'll just supply helpers is_port_alternate_mode() and
is_plug_alternate_mode().

thanks,

-- 
heikki

On Wed, Nov 26, 2025 at 11:42:43AM +0200, Heikki Krogerus wrote:
> Wed, Nov 26, 2025 at 09:46:19AM +0800, Chaoyi Chen kirjoitti:
> > On 11/25/2025 7:49 PM, Greg Kroah-Hartman wrote:
> > >> +static umode_t typec_is_visible(struct kobject *kobj, struct attribute *attr, int n)
> > >> +{
> > >> +	if (is_typec_port(kobj_to_dev(kobj)->parent))
> > > 
> > > Why look at the parent?  Doesn't the device have a type that should show
> > > this?
> > > 
> > > Otherwise, looks good to me.
> > 
> > They have same deivce type "typec_altmode_dev_type".
> > The parent device has a different device type to distinguish between
> > port device and partner device.
> 
> I was already wondering would it make sense to provide separate device
> types for the port, and also plug, alternate modes, but I'm not sure
> if that's the right thing to do.
> 
> There is a plan to register an "altmode" also for the USB4 mode,
> which of course is not an alternate mode. So USB4 will definitely need a
> separate device type.
> 
> So if we supply separate device types for the port, plug and partner
> alternate modes, we need to supply separate device types for port, plug
> and partner USB4 mode as well.
> 
> We certainly can still do that, but I'm just not sure if it makes
> sense?
> 
> I'll prepare a new version for this and include a separate patch where
> instead of defining separate device types for the port and plug
> alternate modes I'll just supply helpers is_port_alternate_mode() and
> is_plug_alternate_mode().

That feels like it would be better in the long run as it would be
easier to "match" on the device type.

thanks,

greg k-h

On 11/26/2025 7:44 PM, Greg Kroah-Hartman wrote:
> On Wed, Nov 26, 2025 at 11:42:43AM +0200, Heikki Krogerus wrote:
>> Wed, Nov 26, 2025 at 09:46:19AM +0800, Chaoyi Chen kirjoitti:
>>> On 11/25/2025 7:49 PM, Greg Kroah-Hartman wrote:
>>>>> +static umode_t typec_is_visible(struct kobject *kobj, struct attribute *attr, int n)
>>>>> +{
>>>>> +	if (is_typec_port(kobj_to_dev(kobj)->parent))
>>>>
>>>> Why look at the parent?  Doesn't the device have a type that should show
>>>> this?
>>>>
>>>> Otherwise, looks good to me.
>>>
>>> They have same deivce type "typec_altmode_dev_type".
>>> The parent device has a different device type to distinguish between
>>> port device and partner device.
>>
>> I was already wondering would it make sense to provide separate device
>> types for the port, and also plug, alternate modes, but I'm not sure
>> if that's the right thing to do.
>>
>> There is a plan to register an "altmode" also for the USB4 mode,
>> which of course is not an alternate mode. So USB4 will definitely need a
>> separate device type.
>>
>> So if we supply separate device types for the port, plug and partner
>> alternate modes, we need to supply separate device types for port, plug
>> and partner USB4 mode as well.
>>
>> We certainly can still do that, but I'm just not sure if it makes
>> sense?
>>
>> I'll prepare a new version for this and include a separate patch where
>> instead of defining separate device types for the port and plug
>> alternate modes I'll just supply helpers is_port_alternate_mode() and
>> is_plug_alternate_mode().
> 
> That feels like it would be better in the long run as it would be
> easier to "match" on the device type.
>

It make sense. But now can we first use the current "match" device type
operation and then modify them later?

> thanks,
> 
> greg k-h
> 
> 

-- 
Best, 
Chaoyi

Wed, Nov 26, 2025 at 07:51:33PM +0800, Chaoyi Chen kirjoitti:
> On 11/26/2025 7:44 PM, Greg Kroah-Hartman wrote:
> > On Wed, Nov 26, 2025 at 11:42:43AM +0200, Heikki Krogerus wrote:
> >> Wed, Nov 26, 2025 at 09:46:19AM +0800, Chaoyi Chen kirjoitti:
> >>> On 11/25/2025 7:49 PM, Greg Kroah-Hartman wrote:
> >>>>> +static umode_t typec_is_visible(struct kobject *kobj, struct attribute *attr, int n)
> >>>>> +{
> >>>>> +	if (is_typec_port(kobj_to_dev(kobj)->parent))
> >>>>
> >>>> Why look at the parent?  Doesn't the device have a type that should show
> >>>> this?
> >>>>
> >>>> Otherwise, looks good to me.
> >>>
> >>> They have same deivce type "typec_altmode_dev_type".
> >>> The parent device has a different device type to distinguish between
> >>> port device and partner device.
> >>
> >> I was already wondering would it make sense to provide separate device
> >> types for the port, and also plug, alternate modes, but I'm not sure
> >> if that's the right thing to do.
> >>
> >> There is a plan to register an "altmode" also for the USB4 mode,
> >> which of course is not an alternate mode. So USB4 will definitely need a
> >> separate device type.
> >>
> >> So if we supply separate device types for the port, plug and partner
> >> alternate modes, we need to supply separate device types for port, plug
> >> and partner USB4 mode as well.
> >>
> >> We certainly can still do that, but I'm just not sure if it makes
> >> sense?
> >>
> >> I'll prepare a new version for this and include a separate patch where
> >> instead of defining separate device types for the port and plug
> >> alternate modes I'll just supply helpers is_port_alternate_mode() and
> >> is_plug_alternate_mode().
> > 
> > That feels like it would be better in the long run as it would be
> > easier to "match" on the device type.
> >
> 
> It make sense. But now can we first use the current "match" device type
> operation and then modify them later?

Let's do this right from the beginning. Here's a version with the
dedicated device types.

-- 
heikki
From c0b2afa035cff0788c68869bf454c43eab2b201f Mon Sep 17 00:00:00 2001
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Date: Tue, 25 Nov 2025 10:38:04 +0100
Subject: [PATCH v2] usb: typec: Set the bus also for the port and plug altmodes

The port and plug altmodes can't be bound to the altmode
drivers because the altmode drivers are meant for partner
communication using the VDM (vendor defined messages), but
they can still be part of the bus. The bus will make sure
that the normal bus notifications are available also with
the port altmodes.

The previously used common device type for all alternate
modes is replaced with separate dedicated device types for
port, plug, and partner alternate modes.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---

v2: Added the dedicated device types.

---
 drivers/usb/typec/bus.c   | 24 +++++++++++++++++++++++-
 drivers/usb/typec/bus.h   |  8 ++++++--
 drivers/usb/typec/class.c | 33 ++++++++++++++++++++++-----------
 3 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index a884cec9ab7e..048c0edf6ca4 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -445,7 +445,23 @@ static struct attribute *typec_attrs[] = {
 	&dev_attr_description.attr,
 	NULL
 };
-ATTRIBUTE_GROUPS(typec);
+
+static umode_t typec_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+	if (is_typec_partner_altmode(kobj_to_dev(kobj)))
+		return attr->mode;
+	return 0;
+}
+
+static const struct attribute_group typec_group = {
+	.is_visible = typec_is_visible,
+	.attrs = typec_attrs,
+};
+
+static const struct attribute_group *typec_groups[] = {
+	&typec_group,
+	NULL
+};
 
 static int typec_match(struct device *dev, const struct device_driver *driver)
 {
@@ -453,6 +469,9 @@ static int typec_match(struct device *dev, const struct device_driver *driver)
 	struct typec_altmode *altmode = to_typec_altmode(dev);
 	const struct typec_device_id *id;
 
+	if (!is_typec_partner_altmode(dev))
+		return 0;
+
 	for (id = drv->id_table; id->svid; id++)
 		if (id->svid == altmode->svid)
 			return 1;
@@ -469,6 +488,9 @@ static int typec_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	if (add_uevent_var(env, "MODE=%u", altmode->mode))
 		return -ENOMEM;
 
+	if (!is_typec_partner_altmode(dev))
+		return 0;
+
 	return add_uevent_var(env, "MODALIAS=typec:id%04X", altmode->svid);
 }
 
diff --git a/drivers/usb/typec/bus.h b/drivers/usb/typec/bus.h
index 643b8c81786d..b58e131450d1 100644
--- a/drivers/usb/typec/bus.h
+++ b/drivers/usb/typec/bus.h
@@ -29,8 +29,12 @@ struct altmode {
 #define to_altmode(d) container_of(d, struct altmode, adev)
 
 extern const struct bus_type typec_bus;
-extern const struct device_type typec_altmode_dev_type;
+extern const struct device_type typec_port_altmode_dev_type;
+extern const struct device_type typec_plug_altmode_dev_type;
+extern const struct device_type typec_partner_altmode_dev_type;
 
-#define is_typec_altmode(_dev_) (_dev_->type == &typec_altmode_dev_type)
+#define is_typec_port_altmode(dev) ((dev)->type == &typec_port_altmode_dev_type)
+#define is_typec_plug_altmode(dev) ((dev)->type == &typec_plug_altmode_dev_type)
+#define is_typec_partner_altmode(dev) ((dev)->type == &typec_partner_altmode_dev_type)
 
 #endif /* __USB_TYPEC_ALTMODE_H__ */
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9b2647cb199b..d6b88317f8a4 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -235,7 +235,7 @@ static int altmode_match(struct device *dev, const void *data)
 	struct typec_altmode *adev = to_typec_altmode(dev);
 	const struct typec_device_id *id = data;
 
-	if (!is_typec_altmode(dev))
+	if (!is_typec_port_altmode(dev))
 		return 0;
 
 	return (adev->svid == id->svid);
@@ -532,15 +532,28 @@ static void typec_altmode_release(struct device *dev)
 	kfree(alt);
 }
 
-const struct device_type typec_altmode_dev_type = {
-	.name = "typec_alternate_mode",
+const struct device_type typec_port_altmode_dev_type = {
+	.name = "typec_port_alternate_mode",
+	.groups = typec_altmode_groups,
+	.release = typec_altmode_release,
+};
+
+const struct device_type typec_plug_altmode_dev_type = {
+	.name = "typec_plug_alternate_mode",
+	.groups = typec_altmode_groups,
+	.release = typec_altmode_release,
+};
+
+const struct device_type typec_partner_altmode_dev_type = {
+	.name = "typec_partner_alternate_mode",
 	.groups = typec_altmode_groups,
 	.release = typec_altmode_release,
 };
 
 static struct typec_altmode *
 typec_register_altmode(struct device *parent,
-		       const struct typec_altmode_desc *desc)
+		       const struct typec_altmode_desc *desc,
+		       const struct device_type *type)
 {
 	unsigned int id = altmode_id_get(parent);
 	bool is_port = is_typec_port(parent);
@@ -575,7 +588,7 @@ typec_register_altmode(struct device *parent,
 
 	alt->adev.dev.parent = parent;
 	alt->adev.dev.groups = alt->groups;
-	alt->adev.dev.type = &typec_altmode_dev_type;
+	alt->adev.dev.type = type;
 	dev_set_name(&alt->adev.dev, "%s.%u", dev_name(parent), id);
 
 	get_device(alt->adev.dev.parent);
@@ -584,9 +597,7 @@ typec_register_altmode(struct device *parent,
 	if (!is_port)
 		typec_altmode_set_partner(alt);
 
-	/* The partners are bind to drivers */
-	if (is_typec_partner(parent))
-		alt->adev.dev.bus = &typec_bus;
+	alt->adev.dev.bus = &typec_bus;
 
 	/* Plug alt modes need a class to generate udev events. */
 	if (is_typec_plug(parent))
@@ -963,7 +974,7 @@ struct typec_altmode *
 typec_partner_register_altmode(struct typec_partner *partner,
 			       const struct typec_altmode_desc *desc)
 {
-	return typec_register_altmode(&partner->dev, desc);
+	return typec_register_altmode(&partner->dev, desc, &typec_partner_altmode_dev_type);
 }
 EXPORT_SYMBOL_GPL(typec_partner_register_altmode);
 
@@ -1193,7 +1204,7 @@ struct typec_altmode *
 typec_plug_register_altmode(struct typec_plug *plug,
 			    const struct typec_altmode_desc *desc)
 {
-	return typec_register_altmode(&plug->dev, desc);
+	return typec_register_altmode(&plug->dev, desc, &typec_plug_altmode_dev_type);
 }
 EXPORT_SYMBOL_GPL(typec_plug_register_altmode);
 
@@ -2493,7 +2504,7 @@ typec_port_register_altmode(struct typec_port *port,
 		return ERR_CAST(retimer);
 	}
 
-	adev = typec_register_altmode(&port->dev, desc);
+	adev = typec_register_altmode(&port->dev, desc, &typec_port_altmode_dev_type);
 	if (IS_ERR(adev)) {
 		typec_retimer_put(retimer);
 		typec_mux_put(mux);
-- 
2.50.1

On 11/25/2025 6:06 PM, Heikki Krogerus wrote:
> Tue, Nov 25, 2025 at 10:23:02AM +0800, Chaoyi Chen kirjoitti:
>> On 11/25/2025 12:33 AM, Greg Kroah-Hartman wrote:
>>> On Mon, Nov 24, 2025 at 04:05:53PM +0800, Chaoyi Chen wrote:
>>>> Hi Greg,
>>>>
>>>> On 11/24/2025 3:10 PM, Greg Kroah-Hartman wrote:
>>>>
>>>>> On Mon, Nov 24, 2025 at 09:40:03AM +0800, Chaoyi Chen wrote:
>>>>>> Hi Greg,
>>>>>>
>>>>>> On 11/21/2025 10:07 PM, Greg Kroah-Hartman wrote:
>>>>>>> On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote:
>>>>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>>>
>>>>>>>> Some other part of kernel may want to know the event of typec bus.
>>>>>>> Be specific, WHAT part of the kernel will need to know this?
>>>>>> For now, it is DRM.
>>>>> Then say this.
>>>> Okay, please refer to the discussion below.
>>>>
>>>>>>> And why a new notifier, why not just use the existing notifiers that you
>>>>>>> already have?  And what is this going to be used for?
>>>>>> We have discussed this before, but the current bus notifier cannot achieve the expected notification [0].
>>>>>>
>>>>>> [0] https://lore.kernel.org/all/aPsuLREPS_FEV3DS@kuha.fi.intel.com/
>>>>> Then you need to document the heck out of this in the changelog text.
>>>>> But I'm still not quite understanding why the bus notifier does not work
>>>>> here, as you only want this information if the usb device is bound to
>>>>> the bus there, you do not want to know this if it did not complete.
>>>>>
>>>>> That thread says you want this not "too late", but why?  What is the
>>>>> problem there, and how will you handle your code getting loaded after
>>>>> the typec code is loaded?  Notifier callbacks don't work for that
>>>>> situation, right?
>>>> In fact, the typec_register_altmode() function generates two
>>>> registered events. The first one is the registered event of the port
>>>> device, and the second one is the registered event of the partner
>>>> device. The second one event only occurs after a Type-C device is
>>>> inserted.
>>>> The bus notifier event does not actually take effect for the port
>>>> device, because it only sets the bus for the partner device:
>>>>
>>>>      /* The partners are bind to drivers */
>>>>      if (is_typec_partner(parent))
>>>>          alt->adev.dev.bus = &typec_bus;
>>> Setting the bus is correct, then it needs to be registered with the
>>> driver core so the bus link shows up (and a driver is bound to it.)
>>> That is when the bus notifier can happen, right?
>> Yes, this is valid for the partner device. But for the port device, since the bus is not specified here, the corresponding bus notifier will not take effect.
> 
> Perhaps we should just fix this part and make also the port altmodes
> part of the bus.
> 
> Some background, in case this is not clear; the port alternate mode
> devices represent the capability of the ports to support specific
> alternate modes. The partner alternate mode devices do the same
> for the partner devices attached to the ports, but on top of the
> showing the capability, they are also used for the alternate mode
> specific communication using the USB Power Delivery VDM (vendor
> defined messages). That's why only the partner altmodes are bound
> to the generic alternate mode drivers.
> 
> And that's why the hack where the port altmodes are not added to the
> bus. But maybe it's not needed.
> 
> Chaoyi, can you try the attached patch?
> 

Thank you for providing the background information. Maybe this is what
we really want. I will try this in v11 :)

>>>> I hope it's not too late. In fact, the notifier here will notify DRM to establish a bridge chain.
>>> What is a "bridge chain"?
>> In DRM, the bridge chain is often used to describe the chain connection relationship
>> of the output of multi level display conversion chips. The bridge chain we are referring to here
>> is actually a chain  structure formed by connecting various devices using a simple transparent bridge [0].
>>
>> For example, the schematic diagram of a bridge chain is as follows:
>>
>> DP controller bridge -> DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
>>
>> Here, apart from the DP controller bridge, the rest are transparent DRM bridges, which are used solely to
>> describe the link relationships between various devices.
>>
>>
>> [0] https://patchwork.freedesktop.org/patch/msgid/20231203114333.1305826-2-dmitry.baryshkov@linaro.org
>>>
>>>> The downstream DP controller driver hopes to obtain the fwnode of the last-level Type-C device
>>>> through this bridge chain to create a DRM connector. And when a device is inserted,
>>>> drivers/usb/typec/altmodes/displayport.c can notify the HPD (Hot Plug Detect) event.
>>> But aren't you just the driver for the "partner device"?
>>>
>>> If not, why isn't a real device being created that you then bind to,
>>> what "fake" type of thing are you attempting to do here that would
>>> require you to do this out-of-band?
>> The HPD event is pass by drm_connector_oob_hotplug_event(), which does not use the device in Type-C.
>> This function will find the corresponding DRM connector device, and the lookup of the DRM connector is
>> done through the fwnode.
>>
>> And the partner device and the port device have the same fwnode.
>>
>>>
>>>> If relying on the second event, the bridge chain may never be established, and the operations of the DP driver will be
>>>> always deferred. Furthermore, other parts of the display controller driver will also be deferred accordingly.
>>> What operations?  What exactly is delayed?  You should not be touching a
>>> device before you have it on your bus, right?
>> To complete the HPD operation, it is necessary to create a drm connector device that
>> has the appropriate fwnode. This operation will be carried out by the DP controller driver.
>>
>> As you can see, since it cross multiple devices, we need to set the fwnode to the last device fusb302.
>> This requires relying on the bridge chain. We can register bridges for multiple devices and then connect
>> them to form a chain. The connection process is completed through drm_bridge_attach().
>>
>> A brief example of the process of establishing a bridge chain is as follows, starting from the last bridge:
>>
>> step1: fusb302 HPD bridge
>> step2: fsa4480 analog audio switch bridge -> fusb302 HPD bridge
>> step3: onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
>> step4: DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
>> step5: DP controller bridge -> DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge
>>
>> Step 1 is the most crucial, because essentially, regardless of whether we use notifiers or not, what we ultimately want to achieve is to create an HPD bridge.
>> The DP controller needs to wait for the subsequent bridge chain to be established because it needs to know the connection relationships of the devices.
>>
>> The question now is when to create the HPD bridge, during the registration of the port device or during the registration of the partner device.
>> If it's the latter, then the delay occurs here.
>>
>> And I don't think I'm touching the Type-C device here. I'm just using the bridge chain to get a suitable fwnode and create a suitable DRM connector device.
>> The subsequent Type-C HPD events will be on the DRM connector device.
>>
>> This solution is somewhat complex, and I apologize once again for any confusion caused earlier.
>>
>>>
>>>>>>> Notifiers are a pain, and should almost never be added.  Use real
>>>>>>> function calls instead.
>>>>>> In v6, I used direct function calls, but had to switch to notifiers because couldn't resolve the dependencies between DRM and Type-C [1]. Do you have any good ideas? Thank you.
>>>>> Only allow this DRM code to be built if typec code is enabled, do NOT
>>>>> use a select, use a depends in the drm code.
>>>> Sorry, I didn't get your point. Does this mean that the current notifiers approach still needs to be changed to direct function calls?
>>> If you somehow convince me that the existing bus notifiers will not
>>> work, yes :)
>>>
>>>> If so, then based on the previous discussion, typec should not depend
>>>> on any DRM components. Does this mean that we should add the if
>>>> (IS_REACHABLE(CONFIG_DRM_AUX_BRIDGE)) before the direct function call?
>>> No, do it properly like any other function call to another subsystem.
>>>
>>>> Additionally, the current version of CONFIG_DRM_AUX_BRIDGE is selected
>>>> by the DP driver in patch9.
>>> Don't do "select" if at all possible, always try to do "depends on".
>> Thank you for clarifying this. However, CONFIG_DRM_AUX_BRIDGE is not exposed in the menu, and it is not intended for the end user to select it by design. Therefore, I think there still needs to be some place to select it?
> 
> You don't "select TYPEC", you already "depend on TYPEC", so you are
> all set with this one.
> 
> thanks,
> 

Ah, it is.

-- 
Best, 
Chaoyi