[PATCH v1 1/5] usb: typec: Add alt_mode_override field to port property

Andrei Kuchynski posted 5 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v1 1/5] usb: typec: Add alt_mode_override field to port property
Posted by Andrei Kuchynski 1 month, 2 weeks ago
This new field in the port properties dictates whether the Platform Policy
Manager (PPM) allows the OS Policy Manager (OPM) to change the currently
active, negotiated alternate mode.

Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
---
 drivers/usb/typec/class.c | 14 +++++++++++---
 drivers/usb/typec/class.h |  2 ++
 include/linux/usb/typec.h |  1 +
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 67a533e35150..a72325ff099a 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -459,9 +459,16 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
 	struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
 
 	if (attr == &dev_attr_active.attr)
-		if (!is_typec_port(adev->dev.parent) &&
-		    (!adev->ops || !adev->ops->activate))
-			return 0444;
+		if (!is_typec_port(adev->dev.parent)) {
+			struct typec_partner *partner =
+				to_typec_partner(adev->dev.parent);
+			struct typec_port *port =
+				to_typec_port(partner->dev.parent);
+
+			if (!port->alt_mode_override || !adev->ops ||
+				!adev->ops->activate)
+				return 0444;
+		}
 
 	return attr->mode;
 }
@@ -2681,6 +2688,7 @@ struct typec_port *typec_register_port(struct device *parent,
 	}
 
 	port->pd = cap->pd;
+	port->alt_mode_override = cap->alt_mode_override;
 
 	ret = device_add(&port->dev);
 	if (ret) {
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index db2fe96c48ff..f05d9201c233 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -80,6 +80,8 @@ struct typec_port {
 	 */
 	struct device			*usb2_dev;
 	struct device			*usb3_dev;
+
+	bool				alt_mode_override;
 };
 
 #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 252af3f77039..6e09e68788dd 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -304,6 +304,7 @@ struct typec_capability {
 	enum typec_accessory	accessory[TYPEC_MAX_ACCESSORY];
 	unsigned int		orientation_aware:1;
 	u8			usb_capability;
+	bool			alt_mode_override;
 
 	struct fwnode_handle	*fwnode;
 	void			*driver_data;
-- 
2.51.0.rc0.215.g125493bb4a-goog
Re: [PATCH v1 1/5] usb: typec: Add alt_mode_override field to port property
Posted by Heikki Krogerus 1 month, 2 weeks ago
On Thu, Aug 14, 2025 at 06:44:51PM +0000, Andrei Kuchynski wrote:
> This new field in the port properties dictates whether the Platform Policy
> Manager (PPM) allows the OS Policy Manager (OPM) to change the currently
> active, negotiated alternate mode.
> 
> Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> ---
>  drivers/usb/typec/class.c | 14 +++++++++++---
>  drivers/usb/typec/class.h |  2 ++
>  include/linux/usb/typec.h |  1 +
>  3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 67a533e35150..a72325ff099a 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -459,9 +459,16 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
>  	struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
>  
>  	if (attr == &dev_attr_active.attr)
> -		if (!is_typec_port(adev->dev.parent) &&
> -		    (!adev->ops || !adev->ops->activate))
> -			return 0444;
> +		if (!is_typec_port(adev->dev.parent)) {
> +			struct typec_partner *partner =
> +				to_typec_partner(adev->dev.parent);

That looks a bit unnecessary. Also, can't the altmode be a plug alt mode?

> +			struct typec_port *port =
> +				to_typec_port(partner->dev.parent);
> +
> +			if (!port->alt_mode_override || !adev->ops ||
> +				!adev->ops->activate)
> +				return 0444;
> +		}

How about:

	struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
        struct typec_port *port = typec_altmode2port(adev);

        if (attr == &dev_attr_active.attr) {
               if (!is_typec_port(adev->dev.parent)) {
                        if (!port->alt_mode_override || !adev->ops || !adev->ops->activate)
                                return 0444;
                }
        }

>  	return attr->mode;
>  }
> @@ -2681,6 +2688,7 @@ struct typec_port *typec_register_port(struct device *parent,
>  	}
>  
>  	port->pd = cap->pd;
> +	port->alt_mode_override = cap->alt_mode_override;

This needs to be enabled by default:

	port->alt_mode_override = !cap->no_mode_control;

>  	ret = device_add(&port->dev);
>  	if (ret) {
> diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> index db2fe96c48ff..f05d9201c233 100644
> --- a/drivers/usb/typec/class.h
> +++ b/drivers/usb/typec/class.h
> @@ -80,6 +80,8 @@ struct typec_port {
>  	 */
>  	struct device			*usb2_dev;
>  	struct device			*usb3_dev;
> +
> +	bool				alt_mode_override;

s/alt_mode_override/mode_control/ ?

>  };
>  
>  #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 252af3f77039..6e09e68788dd 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -304,6 +304,7 @@ struct typec_capability {
>  	enum typec_accessory	accessory[TYPEC_MAX_ACCESSORY];
>  	unsigned int		orientation_aware:1;
>  	u8			usb_capability;
> +	bool			alt_mode_override;
>  
>  	struct fwnode_handle	*fwnode;
>  	void			*driver_data;
> -- 
> 2.51.0.rc0.215.g125493bb4a-goog

-- 
heikki
Re: [PATCH v1 1/5] usb: typec: Add alt_mode_override field to port property
Posted by Andrei Kuchynski 1 month, 2 weeks ago
On Wed, Aug 20, 2025 at 12:53 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Thu, Aug 14, 2025 at 06:44:51PM +0000, Andrei Kuchynski wrote:
> > This new field in the port properties dictates whether the Platform Policy
> > Manager (PPM) allows the OS Policy Manager (OPM) to change the currently
> > active, negotiated alternate mode.
> >
> > Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
> > ---
> >  drivers/usb/typec/class.c | 14 +++++++++++---
> >  drivers/usb/typec/class.h |  2 ++
> >  include/linux/usb/typec.h |  1 +
> >  3 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 67a533e35150..a72325ff099a 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -459,9 +459,16 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
> >       struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
> >
> >       if (attr == &dev_attr_active.attr)
> > -             if (!is_typec_port(adev->dev.parent) &&
> > -                 (!adev->ops || !adev->ops->activate))
> > -                     return 0444;
> > +             if (!is_typec_port(adev->dev.parent)) {
> > +                     struct typec_partner *partner =
> > +                             to_typec_partner(adev->dev.parent);
>
> That looks a bit unnecessary. Also, can't the altmode be a plug alt mode?
>
> > +                     struct typec_port *port =
> > +                             to_typec_port(partner->dev.parent);
> > +
> > +                     if (!port->alt_mode_override || !adev->ops ||
> > +                             !adev->ops->activate)
> > +                             return 0444;
> > +             }
>
> How about:
>
>         struct typec_altmode *adev = to_typec_altmode(kobj_to_dev(kobj));
>         struct typec_port *port = typec_altmode2port(adev);
>
>         if (attr == &dev_attr_active.attr) {
>                if (!is_typec_port(adev->dev.parent)) {
>                         if (!port->alt_mode_override || !adev->ops || !adev->ops->activate)
>                                 return 0444;
>                 }
>         }
>

I completely missed typec_altmode2port function.
Thank you!

> >       return attr->mode;
> >  }
> > @@ -2681,6 +2688,7 @@ struct typec_port *typec_register_port(struct device *parent,
> >       }
> >
> >       port->pd = cap->pd;
> > +     port->alt_mode_override = cap->alt_mode_override;
>
> This needs to be enabled by default:
>
>         port->alt_mode_override = !cap->no_mode_control;
>

Agreed.  I'll make it enabled by default.

> >       ret = device_add(&port->dev);
> >       if (ret) {
> > diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> > index db2fe96c48ff..f05d9201c233 100644
> > --- a/drivers/usb/typec/class.h
> > +++ b/drivers/usb/typec/class.h
> > @@ -80,6 +80,8 @@ struct typec_port {
> >        */
> >       struct device                   *usb2_dev;
> >       struct device                   *usb3_dev;
> > +
> > +     bool                            alt_mode_override;
>
> s/alt_mode_override/mode_control/ ?
>

Agreed. mode_control is a clearer name.

Thank you for your review!


Andrei