[v3] USB Type-C alternate mode priorities

[PATCH v3 1/5] usb: typec: Add mode_control field to port property

Posted by Andrei Kuchynski 5 months, 1 week 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 | 9 ++++++---
 drivers/usb/typec/class.h | 2 ++
 include/linux/usb/typec.h | 2 ++
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 67a533e35150..9f86605ce125 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -457,11 +457,13 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
 					     struct attribute *attr, int n)
 {
 	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) &&
-		    (!adev->ops || !adev->ops->activate))
-			return 0444;
+		if (!is_typec_port(adev->dev.parent)) {
+			if (!port->mode_control || !adev->ops || !adev->ops->activate)
+				return 0444;
+		}
 
 	return attr->mode;
 }
@@ -2681,6 +2683,7 @@ struct typec_port *typec_register_port(struct device *parent,
 	}
 
 	port->pd = cap->pd;
+	port->mode_control = !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..c53a04b9dc75 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				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..8f51665d44ec 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -287,6 +287,7 @@ enum usb_pd_svdm_ver {
  * @prefer_role: Initial role preference (DRP ports).
  * @accessory: Supported Accessory Modes
  * @usb_capability: Supported USB Modes
+ * @no_mode_control: Ability to manage Alternate Modes
  * @fwnode: Optional fwnode of the port
  * @driver_data: Private pointer for driver specific info
  * @pd: Optional USB Power Delivery Support
@@ -304,6 +305,7 @@ struct typec_capability {
 	enum typec_accessory	accessory[TYPEC_MAX_ACCESSORY];
 	unsigned int		orientation_aware:1;
 	u8			usb_capability;
+	bool			no_mode_control;
 
 	struct fwnode_handle	*fwnode;
 	void			*driver_data;
-- 
2.51.0.355.g5224444f11-goog

Re: [PATCH v3 1/5] usb: typec: Add mode_control field to port property

Posted by Greg Kroah-Hartman 5 months ago

On Fri, Sep 05, 2025 at 02:22:02PM +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 | 9 ++++++---
>  drivers/usb/typec/class.h | 2 ++
>  include/linux/usb/typec.h | 2 ++
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 67a533e35150..9f86605ce125 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -457,11 +457,13 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
>  					     struct attribute *attr, int n)
>  {
>  	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) &&
> -		    (!adev->ops || !adev->ops->activate))
> -			return 0444;
> +		if (!is_typec_port(adev->dev.parent)) {
> +			if (!port->mode_control || !adev->ops || !adev->ops->activate)
> +				return 0444;
> +		}
>  
>  	return attr->mode;
>  }
> @@ -2681,6 +2683,7 @@ struct typec_port *typec_register_port(struct device *parent,
>  	}
>  
>  	port->pd = cap->pd;
> +	port->mode_control = !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..c53a04b9dc75 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				mode_control;

Shouldn't this go up higher in this structure, to avoid the hole you
created, and to take advantage of the existing hole?  Perhaps after
orientation?

thanks,

greg k-h

Re: [PATCH v3 1/5] usb: typec: Add mode_control field to port property

Posted by Andrei Kuchynski 5 months ago

On Wed, Sep 10, 2025 at 1:30 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Sep 05, 2025 at 02:22:02PM +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 | 9 ++++++---
> >  drivers/usb/typec/class.h | 2 ++
> >  include/linux/usb/typec.h | 2 ++
> >  3 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 67a533e35150..9f86605ce125 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -457,11 +457,13 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
> >                                            struct attribute *attr, int n)
> >  {
> >       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) &&
> > -                 (!adev->ops || !adev->ops->activate))
> > -                     return 0444;
> > +             if (!is_typec_port(adev->dev.parent)) {
> > +                     if (!port->mode_control || !adev->ops || !adev->ops->activate)
> > +                             return 0444;
> > +             }
> >
> >       return attr->mode;
> >  }
> > @@ -2681,6 +2683,7 @@ struct typec_port *typec_register_port(struct device *parent,
> >       }
> >
> >       port->pd = cap->pd;
> > +     port->mode_control = !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..c53a04b9dc75 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                            mode_control;
>
> Shouldn't this go up higher in this structure, to avoid the hole you
> created, and to take advantage of the existing hole?  Perhaps after
> orientation?
>

I will replace the variable.

Thanks,
Andrei

Re: [PATCH v3 1/5] usb: typec: Add mode_control field to port property

Posted by Heikki Krogerus 5 months ago

On Fri, Sep 05, 2025 at 02:22:02PM +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>

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

> ---
>  drivers/usb/typec/class.c | 9 ++++++---
>  drivers/usb/typec/class.h | 2 ++
>  include/linux/usb/typec.h | 2 ++
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 67a533e35150..9f86605ce125 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -457,11 +457,13 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
>  					     struct attribute *attr, int n)
>  {
>  	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) &&
> -		    (!adev->ops || !adev->ops->activate))
> -			return 0444;
> +		if (!is_typec_port(adev->dev.parent)) {
> +			if (!port->mode_control || !adev->ops || !adev->ops->activate)
> +				return 0444;
> +		}
>  
>  	return attr->mode;
>  }
> @@ -2681,6 +2683,7 @@ struct typec_port *typec_register_port(struct device *parent,
>  	}
>  
>  	port->pd = cap->pd;
> +	port->mode_control = !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..c53a04b9dc75 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				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..8f51665d44ec 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -287,6 +287,7 @@ enum usb_pd_svdm_ver {
>   * @prefer_role: Initial role preference (DRP ports).
>   * @accessory: Supported Accessory Modes
>   * @usb_capability: Supported USB Modes
> + * @no_mode_control: Ability to manage Alternate Modes
>   * @fwnode: Optional fwnode of the port
>   * @driver_data: Private pointer for driver specific info
>   * @pd: Optional USB Power Delivery Support
> @@ -304,6 +305,7 @@ struct typec_capability {
>  	enum typec_accessory	accessory[TYPEC_MAX_ACCESSORY];
>  	unsigned int		orientation_aware:1;
>  	u8			usb_capability;
> +	bool			no_mode_control;
>  
>  	struct fwnode_handle	*fwnode;
>  	void			*driver_data;
> -- 
> 2.51.0.355.g5224444f11-goog

-- 
heikki