Re: [PATCH v3 2/4] usb: typec: Add attribute file showing the USB Modes of the partner

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Fri, Oct 11, 2024 at 03:09:57PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Oct 11, 2024 at 03:44:00PM +0300, Heikki Krogerus wrote:
> > This attribute file shows the supported USB modes (USB 2.0,
> > USB 3.0 and USB4) of the partner, and the currently active
> > mode.
> > 
> > The active mode is determined primarily by checking the
> > speed of the enumerated USB device. When USB Power Delivery
> > is supported, the active USB mode should be always the mode
> > that was used with the Enter_USB Message, regardless of the
> > result of the USB enumeration. The port drivers can
> > separately assign the mode with a dedicated API.
> > 
> > If USB Power Delivery Identity is supplied for the partner
> > device, the supported modes are extracted from it.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  Documentation/ABI/testing/sysfs-class-typec |  14 +++
> >  drivers/usb/typec/class.c                   | 123 +++++++++++++++++++-
> >  drivers/usb/typec/class.h                   |   2 +
> >  include/linux/usb/typec.h                   |   5 +
> >  4 files changed, 140 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index 7c307f02d99e..a3afe04b2688 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -233,6 +233,20 @@ Description:
> >  		directory exists, it will have an attribute file for every VDO
> >  		in Discover Identity command result.
> >  
> > +What:		/sys/class/typec/<port>-partner/usb_mode
> > +Date:		February 2024
> 
> It's later than this :)

Indeed. I'm sorry.

> 
> > +Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:	The USB Modes that the partner device supports. The active mode
> > +		is displayed in brackets. The active USB mode can be changed by
> > +		writing to this file when the port driver is able to send Data
> > +		Reset Message to the partner. That requires USB Power Delivery
> > +		contract between the partner and the port.
> > +
> > +		Valid values:
> > +		- usb2 (USB 2.0)
> > +		- usb3 (USB 3.2)
> > +		- usb4 (USB4)
> 
> We should probably add all of this info to 'lsusb' one of these days.
> I'll add it to my todo list...
> 
> > +
> >  USB Type-C cable devices (eg. /sys/class/typec/port0-cable/)
> >  
> >  Note: Electronically Marked Cables will have a device also for one cable plug
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index ea9ee47bb246..a6fedafc9c86 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -618,6 +618,74 @@ EXPORT_SYMBOL_GPL(typec_unregister_altmode);
> >  /* ------------------------------------------------------------------------- */
> >  /* Type-C Partners */
> >  
> > +/**
> > + * typec_partner_set_usb_mode - Assign active USB Mode for the partner
> > + * @partner: USB Type-C partner
> > + * @mode: USB Mode (USB2, USB3 or USB4)
> > + *
> > + * The port drivers can use this function to assign the active USB Mode to
> > + * @partner. The USB Mode can change for example due to Data Reset.
> > + */
> > +void typec_partner_set_usb_mode(struct typec_partner *partner, enum usb_mode mode)
> > +{
> > +	if (!partner || partner->usb_mode == mode)
> > +		return;
> > +
> > +	partner->usb_capability |= BIT(mode - 1);
> > +	partner->usb_mode = mode;
> > +	sysfs_notify(&partner->dev.kobj, NULL, "usb_mode");
> 
> Who is listening for this and what are they going to do with the
> information?

I'll drop it, unless Abhishek, you guys would have use for it. Let me
know. I'll send v4 next week.

> > +}
> > +EXPORT_SYMBOL_GPL(typec_partner_set_usb_mode);
> > +
> > +static ssize_t
> > +usb_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +	struct typec_partner *partner = to_typec_partner(dev);
> > +	int len = 0;
> > +	int i;
> > +
> > +	for (i = USB_MODE_USB2; i < USB_MODE_USB4 + 1; i++) {
> > +		if (!(BIT(i - 1) & partner->usb_capability))
> > +			continue;
> > +
> > +		if (i == partner->usb_mode)
> > +			len += sysfs_emit_at(buf, len, "[%s] ", usb_modes[i]);
> > +		else
> > +			len += sysfs_emit_at(buf, len, "%s ", usb_modes[i]);
> > +	}
> > +
> > +	buf[len - 1] = '\n';
> 
> Again, sysfs_emit_at()?

Yes. These are going back to the internal review. Too many mistakes.
Sorry.

thanks,

-- 
heikki