[PATCH v7 1/9] usb: typec: Add notifier functions

Chaoyi Chen posted 9 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v7 1/9] usb: typec: Add notifier functions
Posted by Chaoyi Chen 3 months, 2 weeks ago
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>
---
 drivers/usb/typec/Makefile       |  2 +-
 drivers/usb/typec/class.c        |  3 +++
 drivers/usb/typec/notify.c       | 22 ++++++++++++++++++++++
 include/linux/usb/typec_notify.h | 17 +++++++++++++++++
 4 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/typec/notify.c
 create mode 100644 include/linux/usb/typec_notify.h

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/class.c b/drivers/usb/typec/class.c
index 67a533e35150..11c80bc59cda 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -13,6 +13,7 @@
 #include <linux/string_choices.h>
 #include <linux/usb/pd_vdo.h>
 #include <linux/usb/typec_mux.h>
+#include <linux/usb/typec_notify.h>
 #include <linux/usb/typec_retimer.h>
 #include <linux/usb.h>
 
@@ -600,6 +601,8 @@ typec_register_altmode(struct device *parent,
 		return ERR_PTR(ret);
 	}
 
+	typec_notify_event(TYPEC_ALTMODE_REGISTERED, &alt->adev);
+
 	return &alt->adev;
 }
 
diff --git a/drivers/usb/typec/notify.c b/drivers/usb/typec/notify.c
new file mode 100644
index 000000000000..4ae14dd1f461
--- /dev/null
+++ b/drivers/usb/typec/notify.c
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/notifier.h>
+#include <linux/usb/typec_notify.h>
+
+static BLOCKING_NOTIFIER_HEAD(typec_notifier_list);
+
+int typec_register_notify(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&typec_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(typec_register_notify);
+
+int typec_unregister_notify(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&typec_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(typec_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_notify.h b/include/linux/usb/typec_notify.h
new file mode 100644
index 000000000000..a3f1f3b3ae47
--- /dev/null
+++ b/include/linux/usb/typec_notify.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __USB_TYPEC_NOTIFY
+#define __USB_TYPEC_NOTIFY
+
+#include <linux/notifier.h>
+
+enum usb_typec_event {
+	TYPEC_ALTMODE_REGISTERED
+};
+
+int typec_register_notify(struct notifier_block *nb);
+int typec_unregister_notify(struct notifier_block *nb);
+
+void typec_notify_event(unsigned long event, void *data);
+
+#endif /* __USB_TYPEC_NOTIFY */
-- 
2.49.0
Re: [PATCH v7 1/9] usb: typec: Add notifier functions
Posted by Heikki Krogerus 3 months, 2 weeks ago
> +#include <linux/notifier.h>
> +#include <linux/usb/typec_notify.h>
> +
> +static BLOCKING_NOTIFIER_HEAD(typec_notifier_list);
> +
> +int typec_register_notify(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&typec_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(typec_register_notify);
> +
> +int typec_unregister_notify(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&typec_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(typec_unregister_notify);

Better name these more clearly:

typec_altmode_register_notify()
typec_altmode_unregister_notify()

thanks,

-- 
heikki
Re: [PATCH v7 1/9] usb: typec: Add notifier functions
Posted by Heikki Krogerus 3 months, 2 weeks ago
Hi,

> diff --git a/include/linux/usb/typec_notify.h b/include/linux/usb/typec_notify.h
> new file mode 100644
> index 000000000000..a3f1f3b3ae47
> --- /dev/null
> +++ b/include/linux/usb/typec_notify.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __USB_TYPEC_NOTIFY
> +#define __USB_TYPEC_NOTIFY
> +
> +#include <linux/notifier.h>
> +
> +enum usb_typec_event {
> +	TYPEC_ALTMODE_REGISTERED
> +};

Don't you need to know when the altmode is removed?

> +
> +int typec_register_notify(struct notifier_block *nb);
> +int typec_unregister_notify(struct notifier_block *nb);
> +
> +void typec_notify_event(unsigned long event, void *data);

Declare typec_notify_event() in drivers/usb/typec/bus.h

thanks,

-- 
heikki
Re: [PATCH v7 1/9] usb: typec: Add notifier functions
Posted by Heikki Krogerus 3 months, 2 weeks ago
On Thu, Oct 23, 2025 at 11:10:20AM +0300, Heikki Krogerus wrote:
> Hi,
> 
> > diff --git a/include/linux/usb/typec_notify.h b/include/linux/usb/typec_notify.h
> > new file mode 100644
> > index 000000000000..a3f1f3b3ae47
> > --- /dev/null
> > +++ b/include/linux/usb/typec_notify.h
> > @@ -0,0 +1,17 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef __USB_TYPEC_NOTIFY
> > +#define __USB_TYPEC_NOTIFY
> > +
> > +#include <linux/notifier.h>
> > +
> > +enum usb_typec_event {
> > +	TYPEC_ALTMODE_REGISTERED
> > +};
> 
> Don't you need to know when the altmode is removed?

I noticed that you don't because drm_dp_hpd_bridge_register() is
always resource managed. But I think you could still send an event
also when the altmode is removed already now. That way it does not
need to be separately added if and when it is needed.

thanks,

-- 
heikki
Re: [PATCH v7 1/9] usb: typec: Add notifier functions
Posted by Heikki Krogerus 3 months, 2 weeks ago
On Thu, Oct 23, 2025 at 12:04:44PM +0300, Heikki Krogerus wrote:
> On Thu, Oct 23, 2025 at 11:10:20AM +0300, Heikki Krogerus wrote:
> > Hi,
> > 
> > > diff --git a/include/linux/usb/typec_notify.h b/include/linux/usb/typec_notify.h
> > > new file mode 100644
> > > index 000000000000..a3f1f3b3ae47
> > > --- /dev/null
> > > +++ b/include/linux/usb/typec_notify.h
> > > @@ -0,0 +1,17 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > > +#ifndef __USB_TYPEC_NOTIFY
> > > +#define __USB_TYPEC_NOTIFY
> > > +
> > > +#include <linux/notifier.h>
> > > +
> > > +enum usb_typec_event {
> > > +	TYPEC_ALTMODE_REGISTERED
> > > +};
> > 
> > Don't you need to know when the altmode is removed?
> 
> I noticed that you don't because drm_dp_hpd_bridge_register() is
> always resource managed. But I think you could still send an event
> also when the altmode is removed already now. That way it does not
> need to be separately added if and when it is needed.

Hold on! Every bus has already a notifier chain. That's the one that
we should also use. Sorry for not noticing that earlier.

So let's just export the bus type in this patch - you can then use
bus_register_notifier() in your driver:

diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index a884cec9ab7e..65ded9e3cdaa 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -547,3 +547,4 @@ const struct bus_type typec_bus = {
        .probe = typec_probe,
        .remove = typec_remove,
 };
+EXPORT_SYMBOL_GPL(typec_bus);
diff --git a/drivers/usb/typec/bus.h b/drivers/usb/typec/bus.h
index 643b8c81786d..af9edb3db9d0 100644
--- a/drivers/usb/typec/bus.h
+++ b/drivers/usb/typec/bus.h
@@ -5,7 +5,6 @@
 
 #include <linux/usb/typec_altmode.h>
 
-struct bus_type;
 struct typec_mux;
 struct typec_retimer;
 
@@ -28,7 +27,6 @@ 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;
 
 #define is_typec_altmode(_dev_) (_dev_->type == &typec_altmode_dev_type)
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 309251572e2e..c6fd46902fce 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -20,12 +20,15 @@ struct typec_port;
 struct typec_altmode_ops;
 struct typec_cable_ops;
 
+struct bus_type;
 struct fwnode_handle;
 struct device;
 
 struct usb_power_delivery;
 struct usb_power_delivery_desc;
 
+extern const struct bus_type typec_bus;
+
 enum typec_port_type {
        TYPEC_PORT_SRC,
        TYPEC_PORT_SNK,

thanks,

-- 
heikki
Re: [PATCH v7 1/9] usb: typec: Add notifier functions
Posted by Chaoyi Chen 3 months, 2 weeks ago
Hi Heikki,

On 10/23/2025 5:44 PM, Heikki Krogerus wrote:
> On Thu, Oct 23, 2025 at 12:04:44PM +0300, Heikki Krogerus wrote:
>> On Thu, Oct 23, 2025 at 11:10:20AM +0300, Heikki Krogerus wrote:
>>> Hi,
>>>
>>>> diff --git a/include/linux/usb/typec_notify.h b/include/linux/usb/typec_notify.h
>>>> new file mode 100644
>>>> index 000000000000..a3f1f3b3ae47
>>>> --- /dev/null
>>>> +++ b/include/linux/usb/typec_notify.h
>>>> @@ -0,0 +1,17 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +
>>>> +#ifndef __USB_TYPEC_NOTIFY
>>>> +#define __USB_TYPEC_NOTIFY
>>>> +
>>>> +#include <linux/notifier.h>
>>>> +
>>>> +enum usb_typec_event {
>>>> +	TYPEC_ALTMODE_REGISTERED
>>>> +};
>>> Don't you need to know when the altmode is removed?
>> I noticed that you don't because drm_dp_hpd_bridge_register() is
>> always resource managed. But I think you could still send an event
>> also when the altmode is removed already now. That way it does not
>> need to be separately added if and when it is needed.
> Hold on! Every bus has already a notifier chain. That's the one that
> we should also use. Sorry for not noticing that earlier.
>
> So let's just export the bus type in this patch - you can then use
> bus_register_notifier() in your driver:
>
> diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
> index a884cec9ab7e..65ded9e3cdaa 100644
> --- a/drivers/usb/typec/bus.c
> +++ b/drivers/usb/typec/bus.c
> @@ -547,3 +547,4 @@ const struct bus_type typec_bus = {
>          .probe = typec_probe,
>          .remove = typec_remove,
>   };
> +EXPORT_SYMBOL_GPL(typec_bus);
> diff --git a/drivers/usb/typec/bus.h b/drivers/usb/typec/bus.h
> index 643b8c81786d..af9edb3db9d0 100644
> --- a/drivers/usb/typec/bus.h
> +++ b/drivers/usb/typec/bus.h
> @@ -5,7 +5,6 @@
>   
>   #include <linux/usb/typec_altmode.h>
>   
> -struct bus_type;
>   struct typec_mux;
>   struct typec_retimer;
>   
> @@ -28,7 +27,6 @@ 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;
>   
>   #define is_typec_altmode(_dev_) (_dev_->type == &typec_altmode_dev_type)
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 309251572e2e..c6fd46902fce 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -20,12 +20,15 @@ struct typec_port;
>   struct typec_altmode_ops;
>   struct typec_cable_ops;
>   
> +struct bus_type;
>   struct fwnode_handle;
>   struct device;
>   
>   struct usb_power_delivery;
>   struct usb_power_delivery_desc;
>   
> +extern const struct bus_type typec_bus;
> +
>   enum typec_port_type {
>          TYPEC_PORT_SRC,
>          TYPEC_PORT_SNK,

Thank you for your detailed explanation. I noticed that there is a device_register() action in typec_register_altmode(), so we can just take advantage of this.


Another thing is that we need to distinguish between different devices in the notifier callback, as typec_register_altmode()/typec_register_partner()/typec_register_plug()/typec_register_cable() may all register devices. Since the data passed in bus_notify() is struct device *dev, I think we can distinguish them through `dev->type.name`? We may already have such names, "typec_alternate_mode", "typec_partner", "typec_plug" in class.c . And then extract these names as macros and put them in the typec header file.


Or do you have any better ideas? Thank you.


> thanks,
>
-- 
Best,
Chaoyi
Re: [PATCH v7 1/9] usb: typec: Add notifier functions
Posted by Heikki Krogerus 3 months, 2 weeks ago
> Thank you for your detailed explanation. I noticed that there is a
> device_register() action in typec_register_altmode(), so we can just take
> advantage of this.
> 
> Another thing is that we need to distinguish between different devices in the
> notifier callback, as
> typec_register_altmode()/typec_register_partner()/typec_register_plug()/typec_register_cable()
> may all register devices. Since the data passed in bus_notify() is struct
> device *dev, I think we can distinguish them through `dev->type.name`? We may
> already have such names, "typec_alternate_mode", "typec_partner", "typec_plug"
> in class.c . And then extract these names as macros and put them in the typec
> header file.

You don't need to worry about that. Only partner altmodes are bind to
the bus. The device you see in the notifier will always be an altmode.

But in general, if you need to identify the device type, then
you use the device type itself, not the name of the type. It would
require that the device types are exported, but as said, you don't
need to worry about that in this case.

thanks,

-- 
heikki
Re: [PATCH v7 1/9] usb: typec: Add notifier functions
Posted by Chaoyi Chen 3 months, 2 weeks ago
Hi Heikki,

On 10/23/2025 7:36 PM, Heikki Krogerus wrote:
>> Thank you for your detailed explanation. I noticed that there is a
>> device_register() action in typec_register_altmode(), so we can just take
>> advantage of this.
>>
>> Another thing is that we need to distinguish between different devices in the
>> notifier callback, as
>> typec_register_altmode()/typec_register_partner()/typec_register_plug()/typec_register_cable()
>> may all register devices. Since the data passed in bus_notify() is struct
>> device *dev, I think we can distinguish them through `dev->type.name`? We may
>> already have such names, "typec_alternate_mode", "typec_partner", "typec_plug"
>> in class.c . And then extract these names as macros and put them in the typec
>> header file.
> You don't need to worry about that. Only partner altmodes are bind to
> the bus. The device you see in the notifier will always be an altmode.
>
> But in general, if you need to identify the device type, then
> you use the device type itself, not the name of the type. It would
> require that the device types are exported, but as said, you don't
> need to worry about that in this case.

Very insightful! I will try to do this in v8 :)


>
> thanks,
>
-- 
Best,
Chaoyi
Re: [PATCH v7 1/9] usb: typec: Add notifier functions
Posted by Greg Kroah-Hartman 3 months, 2 weeks ago
On Thu, Oct 23, 2025 at 07:21:31PM +0800, Chaoyi Chen wrote:
> Hi Heikki,
> 
> On 10/23/2025 5:44 PM, Heikki Krogerus wrote:
> > On Thu, Oct 23, 2025 at 12:04:44PM +0300, Heikki Krogerus wrote:
> > > On Thu, Oct 23, 2025 at 11:10:20AM +0300, Heikki Krogerus wrote:
> > > > Hi,
> > > > 
> > > > > diff --git a/include/linux/usb/typec_notify.h b/include/linux/usb/typec_notify.h
> > > > > new file mode 100644
> > > > > index 000000000000..a3f1f3b3ae47
> > > > > --- /dev/null
> > > > > +++ b/include/linux/usb/typec_notify.h
> > > > > @@ -0,0 +1,17 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > +
> > > > > +#ifndef __USB_TYPEC_NOTIFY
> > > > > +#define __USB_TYPEC_NOTIFY
> > > > > +
> > > > > +#include <linux/notifier.h>
> > > > > +
> > > > > +enum usb_typec_event {
> > > > > +	TYPEC_ALTMODE_REGISTERED
> > > > > +};
> > > > Don't you need to know when the altmode is removed?
> > > I noticed that you don't because drm_dp_hpd_bridge_register() is
> > > always resource managed. But I think you could still send an event
> > > also when the altmode is removed already now. That way it does not
> > > need to be separately added if and when it is needed.
> > Hold on! Every bus has already a notifier chain. That's the one that
> > we should also use. Sorry for not noticing that earlier.
> > 
> > So let's just export the bus type in this patch - you can then use
> > bus_register_notifier() in your driver:
> > 
> > diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
> > index a884cec9ab7e..65ded9e3cdaa 100644
> > --- a/drivers/usb/typec/bus.c
> > +++ b/drivers/usb/typec/bus.c
> > @@ -547,3 +547,4 @@ const struct bus_type typec_bus = {
> >          .probe = typec_probe,
> >          .remove = typec_remove,
> >   };
> > +EXPORT_SYMBOL_GPL(typec_bus);
> > diff --git a/drivers/usb/typec/bus.h b/drivers/usb/typec/bus.h
> > index 643b8c81786d..af9edb3db9d0 100644
> > --- a/drivers/usb/typec/bus.h
> > +++ b/drivers/usb/typec/bus.h
> > @@ -5,7 +5,6 @@
> >   #include <linux/usb/typec_altmode.h>
> > -struct bus_type;
> >   struct typec_mux;
> >   struct typec_retimer;
> > @@ -28,7 +27,6 @@ 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;
> >   #define is_typec_altmode(_dev_) (_dev_->type == &typec_altmode_dev_type)
> > diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> > index 309251572e2e..c6fd46902fce 100644
> > --- a/include/linux/usb/typec.h
> > +++ b/include/linux/usb/typec.h
> > @@ -20,12 +20,15 @@ struct typec_port;
> >   struct typec_altmode_ops;
> >   struct typec_cable_ops;
> > +struct bus_type;
> >   struct fwnode_handle;
> >   struct device;
> >   struct usb_power_delivery;
> >   struct usb_power_delivery_desc;
> > +extern const struct bus_type typec_bus;
> > +
> >   enum typec_port_type {
> >          TYPEC_PORT_SRC,
> >          TYPEC_PORT_SNK,
> 
> Thank you for your detailed explanation. I noticed that there is a device_register() action in typec_register_altmode(), so we can just take advantage of this.
> 
> 
> Another thing is that we need to distinguish between different devices in the notifier callback, as typec_register_altmode()/typec_register_partner()/typec_register_plug()/typec_register_cable() may all register devices. Since the data passed in bus_notify() is struct device *dev, I think we can distinguish them through `dev->type.name`? We may already have such names, "typec_alternate_mode", "typec_partner", "typec_plug" in class.c . And then extract these names as macros and put them in the typec header file.
> 
> 
> Or do you have any better ideas? Thank you.

Check based on the type itself, NOT on the type name, as all device
types on a bus should be unique.  If the structure for that is not
properly exported for you to use, just export it.

thanks,

greg k-h