[PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Prashant Malani]

A PD-capable device can return up to 3 Product Type VDOs as part of its
DiscoverIdentity Response (USB PD Spec, Rev 3.0, Version 2.0, Section
6.4.4.3.1). Add a sysfs attribute to expose these to userspace.

Cc: Benson Leung <bleung@chromium.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

NOTE: I didn't include Benson's Reviewed-by from v1, since this version
introduced the sysfs_notify() call.

Changes in v2:
- Added sysfs_notify() call for the attribute.
- Added description for the attribute in
  Documentation/ABI/testing/sysfs-class-typec.

 Documentation/ABI/testing/sysfs-class-typec | 17 +++++++++++++++++
 drivers/usb/typec/class.c                   | 11 +++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index b834671522d6..16440a236b66 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -170,6 +170,14 @@ Description:
 		will show 0 until Discover Identity command result becomes
 		available. The value can be polled.
 
+What:		/sys/class/typec/<port>-partner/identity/product_type_vdo
+Date:		October 2020
+Contact:	Prashant Malani <pmalani@chromium.org>
+Description:
+		Product Type VDOs part of Discover Identity command result. 3 values
+		are displayed (for the 3 possible Product Type VDOs), one per line.
+		The values will show 0s until Discover Identity command result becomes
+		available. The values can be polled.
 
 USB Type-C cable devices (eg. /sys/class/typec/port0-cable/)
 
@@ -230,6 +238,15 @@ Description:
 		will show 0 until Discover Identity command result becomes
 		available. The value can be polled.
 
+What:		/sys/class/typec/<port>-cable/identity/product_type_vdo
+Date:		October 2020
+Contact:	Prashant Malani <pmalani@chromium.org>
+Description:
+		Product Type VDOs part of Discover Identity command result. 3 values
+		are displayed (for the 3 possible Product Type VDOs), one per line.
+		The values will show 0s until Discover Identity command result becomes
+		available. The values can be polled.
+
 
 USB Type-C port alternate mode devices.
 
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 35eec707cb51..37fa4501e75f 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -122,10 +122,20 @@ static ssize_t product_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(product);
 
+static ssize_t product_type_vdo_show(struct device *dev, struct device_attribute *attr,
+				     char *buf)
+{
+	struct usb_pd_identity *id = get_pd_identity(dev);
+
+	return sprintf(buf, "0x%08x\n0x%08x\n0x%08x\n", id->vdo[0], id->vdo[1], id->vdo[2]);
+}
+static DEVICE_ATTR_RO(product_type_vdo);
+
 static struct attribute *usb_pd_id_attrs[] = {
 	&dev_attr_id_header.attr,
 	&dev_attr_cert_stat.attr,
 	&dev_attr_product.attr,
+	&dev_attr_product_type_vdo.attr,
 	NULL
 };
 
@@ -144,6 +154,7 @@ static void typec_report_identity(struct device *dev)
 	sysfs_notify(&dev->kobj, "identity", "id_header");
 	sysfs_notify(&dev->kobj, "identity", "cert_stat");
 	sysfs_notify(&dev->kobj, "identity", "product");
+	sysfs_notify(&dev->kobj, "identity", "product_type_vdo");
 }
 
 /* ------------------------------------------------------------------------- */
-- 
2.29.0.rc1.297.gfa9743e501-goog

Re: [PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Greg KH]

On Wed, Oct 21, 2020 at 11:15:54PM -0700, Prashant Malani wrote:
> A PD-capable device can return up to 3 Product Type VDOs as part of its
> DiscoverIdentity Response (USB PD Spec, Rev 3.0, Version 2.0, Section
> 6.4.4.3.1). Add a sysfs attribute to expose these to userspace.
> 
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> ---
> 
> NOTE: I didn't include Benson's Reviewed-by from v1, since this version
> introduced the sysfs_notify() call.
> 
> Changes in v2:
> - Added sysfs_notify() call for the attribute.
> - Added description for the attribute in
>   Documentation/ABI/testing/sysfs-class-typec.
> 
>  Documentation/ABI/testing/sysfs-class-typec | 17 +++++++++++++++++
>  drivers/usb/typec/class.c                   | 11 +++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index b834671522d6..16440a236b66 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -170,6 +170,14 @@ Description:
>  		will show 0 until Discover Identity command result becomes
>  		available. The value can be polled.
>  
> +What:		/sys/class/typec/<port>-partner/identity/product_type_vdo
> +Date:		October 2020
> +Contact:	Prashant Malani <pmalani@chromium.org>
> +Description:
> +		Product Type VDOs part of Discover Identity command result. 3 values
> +		are displayed (for the 3 possible Product Type VDOs), one per line.

sysfs is "one value per file", not "one value per line".  This is not
ok.

> +		The values will show 0s until Discover Identity command result becomes
> +		available. The values can be polled.

It can be polled?  Did you try that?  I don't see the logic for that in
your patch.


>  
>  USB Type-C cable devices (eg. /sys/class/typec/port0-cable/)
>  
> @@ -230,6 +238,15 @@ Description:
>  		will show 0 until Discover Identity command result becomes
>  		available. The value can be polled.
>  
> +What:		/sys/class/typec/<port>-cable/identity/product_type_vdo
> +Date:		October 2020
> +Contact:	Prashant Malani <pmalani@chromium.org>
> +Description:
> +		Product Type VDOs part of Discover Identity command result. 3 values
> +		are displayed (for the 3 possible Product Type VDOs), one per line.
> +		The values will show 0s until Discover Identity command result becomes
> +		available. The values can be polled.

Why are you describing the same value in two different locations?

> +
>  
>  USB Type-C port alternate mode devices.
>  
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 35eec707cb51..37fa4501e75f 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -122,10 +122,20 @@ static ssize_t product_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(product);
>  
> +static ssize_t product_type_vdo_show(struct device *dev, struct device_attribute *attr,
> +				     char *buf)
> +{
> +	struct usb_pd_identity *id = get_pd_identity(dev);
> +
> +	return sprintf(buf, "0x%08x\n0x%08x\n0x%08x\n", id->vdo[0], id->vdo[1], id->vdo[2]);

Note, for future sysfs stuff, always use sysfs_emit().

But again, this is not allowed as you have multiple values per a single
file.

thanks,

greg k-h

Re: [PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Prashant Malani]

Thanks for reviewing the patch, Greg.

On Wed, Oct 21, 2020 at 11:56 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Oct 21, 2020 at 11:15:54PM -0700, Prashant Malani wrote:
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index b834671522d6..16440a236b66 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -170,6 +170,14 @@ Description:
> >               will show 0 until Discover Identity command result becomes
> >               available. The value can be polled.
> >
> > +What:                /sys/class/typec/<port>-partner/identity/product_type_vdo
> > +Date:                October 2020
> > +Contact:     Prashant Malani <pmalani@chromium.org>
> > +Description:
> > +             Product Type VDOs part of Discover Identity command result. 3 values
> > +             are displayed (for the 3 possible Product Type VDOs), one per line.
>
> sysfs is "one value per file", not "one value per line".  This is not
> ok.

I see. Would listing these out as three separate vdos (i.e vdo0, vdo1,
vdo2) be better?
>
> > +             The values will show 0s until Discover Identity command result becomes
> > +             available. The values can be polled.
>
> It can be polled?  Did you try that?  I don't see the logic for that in
> your patch.

To be honest, no. I followed the pattern used by the other identity
VDOs (cert_stat, id_header and product),
and re-used their description assuming it to be accurate.

>
>
> >
> >  USB Type-C cable devices (eg. /sys/class/typec/port0-cable/)
> >
> > @@ -230,6 +238,15 @@ Description:
> >               will show 0 until Discover Identity command result becomes
> >               available. The value can be polled.
> >
> > +What:                /sys/class/typec/<port>-cable/identity/product_type_vdo
> > +Date:                October 2020
> > +Contact:     Prashant Malani <pmalani@chromium.org>
> > +Description:
> > +             Product Type VDOs part of Discover Identity command result. 3 values
> > +             are displayed (for the 3 possible Product Type VDOs), one per line.
> > +             The values will show 0s until Discover Identity command result becomes
> > +             available. The values can be polled.
>
> Why are you describing the same value in two different locations?

Both cable and partner can have Discover Identity VDOs and they are
listed separately in sysfs.
The other VDOs (id_header, cert_stat and product) have separate
descriptions for cable and partner too.
Perhaps there is a case for consolidating the listings here (factor
out the ones which are common to cable and partner).

>
> > +
> >
> >  USB Type-C port alternate mode devices.
> >
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 35eec707cb51..37fa4501e75f 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -122,10 +122,20 @@ static ssize_t product_show(struct device *dev, struct device_attribute *attr,
> >  }
> >  static DEVICE_ATTR_RO(product);
> >
> > +static ssize_t product_type_vdo_show(struct device *dev, struct device_attribute *attr,
> > +                                  char *buf)
> > +{
> > +     struct usb_pd_identity *id = get_pd_identity(dev);
> > +
> > +     return sprintf(buf, "0x%08x\n0x%08x\n0x%08x\n", id->vdo[0], id->vdo[1], id->vdo[2]);
>
> Note, for future sysfs stuff, always use sysfs_emit().
>
> But again, this is not allowed as you have multiple values per a single
> file.

Noted. I will keep that in mind for future versions.

Best regards,

>
> thanks,
>
> greg k-h

Re: [PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Greg KH]

On Thu, Oct 22, 2020 at 12:13:54AM -0700, Prashant Malani wrote:
> Thanks for reviewing the patch, Greg.
> 
> On Wed, Oct 21, 2020 at 11:56 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Oct 21, 2020 at 11:15:54PM -0700, Prashant Malani wrote:
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > > index b834671522d6..16440a236b66 100644
> > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > @@ -170,6 +170,14 @@ Description:
> > >               will show 0 until Discover Identity command result becomes
> > >               available. The value can be polled.
> > >
> > > +What:                /sys/class/typec/<port>-partner/identity/product_type_vdo
> > > +Date:                October 2020
> > > +Contact:     Prashant Malani <pmalani@chromium.org>
> > > +Description:
> > > +             Product Type VDOs part of Discover Identity command result. 3 values
> > > +             are displayed (for the 3 possible Product Type VDOs), one per line.
> >
> > sysfs is "one value per file", not "one value per line".  This is not
> > ok.
> 
> I see. Would listing these out as three separate vdos (i.e vdo0, vdo1,
> vdo2) be better?

Given that your current implementation is not acceptable, something has
to change :)

thanks,

greg k-h

Re: [PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Prashant Malani]

Hi Greg,

On Thu, Oct 22, 2020 at 12:17 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > > > +What:                /sys/class/typec/<port>-partner/identity/product_type_vdo
> > > > +Date:                October 2020
> > > > +Contact:     Prashant Malani <pmalani@chromium.org>
> > > > +Description:
> > > > +             Product Type VDOs part of Discover Identity command result. 3 values
> > > > +             are displayed (for the 3 possible Product Type VDOs), one per line.
> > >
> > > sysfs is "one value per file", not "one value per line".  This is not
> > > ok.
> >
> > I see. Would listing these out as three separate vdos (i.e vdo0, vdo1,
> > vdo2) be better?
>
> Given that your current implementation is not acceptable, something has
> to change :)

Got it. I'd like to see if Heikki has any suggestions on naming these
entries better.

Thanks again and best regards,

-Prashant

>
> thanks,
>
> greg k-h

Re: [PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Heikki Krogerus]

On Thu, Oct 22, 2020 at 12:25:07AM -0700, Prashant Malani wrote:
> Hi Greg,
> 
> On Thu, Oct 22, 2020 at 12:17 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > > > > +What:                /sys/class/typec/<port>-partner/identity/product_type_vdo
> > > > > +Date:                October 2020
> > > > > +Contact:     Prashant Malani <pmalani@chromium.org>
> > > > > +Description:
> > > > > +             Product Type VDOs part of Discover Identity command result. 3 values
> > > > > +             are displayed (for the 3 possible Product Type VDOs), one per line.
> > > >
> > > > sysfs is "one value per file", not "one value per line".  This is not
> > > > ok.
> > >
> > > I see. Would listing these out as three separate vdos (i.e vdo0, vdo1,
> > > vdo2) be better?
> >
> > Given that your current implementation is not acceptable, something has
> > to change :)
> 
> Got it. I'd like to see if Heikki has any suggestions on naming these
> entries better.

Why not have product type specific attribute files?

So if the partner is UFP, then we expose ufp1 and ufp2 files that
return the UFP1 and UFP2 VDO values and hide the other files:

        % ls /sys/class/typec/port0-partner/identity/
        id_header cert_stat product ufp1 ufp2

If the partner is DFP, then you expose the dfp file and hide
everything else:

        % ls /sys/class/typec/port0-partner/identity/
        id_header cert_stat product dfp

And so on.

thanks,

-- 
heikki

Re: [PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Prashant Malani]

Hi Heikki,

Thanks for your feedback.

On Thu, Oct 22, 2020 at 5:43 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Thu, Oct 22, 2020 at 12:25:07AM -0700, Prashant Malani wrote:
> > Hi Greg,
> >
> > On Thu, Oct 22, 2020 at 12:17 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > Given that your current implementation is not acceptable, something has
> > > to change :)
> >
> > Got it. I'd like to see if Heikki has any suggestions on naming these
> > entries better.
>
> Why not have product type specific attribute files?
>
> So if the partner is UFP, then we expose ufp1 and ufp2 files that
> return the UFP1 and UFP2 VDO values and hide the other files:
>
>         % ls /sys/class/typec/port0-partner/identity/
>         id_header cert_stat product ufp1 ufp2
>
> If the partner is DFP, then you expose the dfp file and hide
> everything else:
>
>         % ls /sys/class/typec/port0-partner/identity/
>         id_header cert_stat product dfp
>
> And so on.
>

Makes sense, thanks! The only query I have here is , does the kernel
*need* to implement this logic? Userspace can read id_header VDO and
figure this on its own (parse the Product Type specific VDOs
accordingly).

Apart from that, I can work on implementing this if there are no concerns.

Best regards,

> thanks,
>
> --
> heikki

Re: [PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Benson Leung]

Hi Heikki,

On Thu, Oct 22, 2020 at 5:43 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Thu, Oct 22, 2020 at 12:25:07AM -0700, Prashant Malani wrote:
> > Hi Greg,
> >
> > On Thu, Oct 22, 2020 at 12:17 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > > > > +What:                /sys/class/typec/<port>-partner/identity/product_type_vdo
> > > > > > +Date:                October 2020
> > > > > > +Contact:     Prashant Malani <pmalani@chromium.org>
> > > > > > +Description:
> > > > > > +             Product Type VDOs part of Discover Identity command result. 3 values
> > > > > > +             are displayed (for the 3 possible Product Type VDOs), one per line.
> > > > >
> > > > > sysfs is "one value per file", not "one value per line".  This is not
> > > > > ok.
> > > >
> > > > I see. Would listing these out as three separate vdos (i.e vdo0, vdo1,
> > > > vdo2) be better?
> > >
> > > Given that your current implementation is not acceptable, something has
> > > to change :)
> >
> > Got it. I'd like to see if Heikki has any suggestions on naming these
> > entries better.
>
> Why not have product type specific attribute files?
>
> So if the partner is UFP, then we expose ufp1 and ufp2 files that
> return the UFP1 and UFP2 VDO values and hide the other files:
>
>         % ls /sys/class/typec/port0-partner/identity/
>         id_header cert_stat product ufp1 ufp2
>
> If the partner is DFP, then you expose the dfp file and hide
> everything else:
>
>         % ls /sys/class/typec/port0-partner/identity/
>         id_header cert_stat product dfp
>
> And so on.

I would caution against any decoding of the VDO contents in the kernel
and making assumptions about the # or the names of these three
individual objects.

Since PD 2.0 through PD 3.0, and PD 3.0's different subrevisions (1.0,
1.3, 2.0), the # of VDOs that have been supported has changed in the
various spec versions.

PD R3.0 V2.0 actually added extra objects here (UFP VDO1 UFP VDO2, DFP
VDO), but thanks to some troublemaker (me, actually...), the PD spec's
next version deprecates and deletes two of them (the AMA VDO and the
UFP VDO2 are gone, thanks to an ECR I put into USB PD).

(If you've got USB PD working group access, the two ECRs in question
are: https://groups.usb.org/wg/powerdelivery/document/11007 and
https://groups.usb.org/wg/powerdelivery/document/10967).

Since the different spec versions need to all be supported (since the
firmware of PD devices are baked for a particular version of the PD
spec at the time they are released and don't change in practice), the
software on USB PD hosts should provide these objects up to the next
layer without adding any extra decoding, and the upper layer
(userspace) can figure out the specifics based on comparing different
revision and version fields to figure out what vdo1, vdo2, and vdo3
are.

Anyway, hope this helps, and sorry in advance for making this section
of the PD spec more complicated to handle over time...

Benson
>
> thanks,
>
> --
> heikki



-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

Re: [PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Heikki Krogerus]

On Thu, Oct 22, 2020 at 04:25:23PM -0700, Benson Leung wrote:
> Hi Heikki,
> 
> On Thu, Oct 22, 2020 at 5:43 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Thu, Oct 22, 2020 at 12:25:07AM -0700, Prashant Malani wrote:
> > > Hi Greg,
> > >
> > > On Thu, Oct 22, 2020 at 12:17 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > > > > +What:                /sys/class/typec/<port>-partner/identity/product_type_vdo
> > > > > > > +Date:                October 2020
> > > > > > > +Contact:     Prashant Malani <pmalani@chromium.org>
> > > > > > > +Description:
> > > > > > > +             Product Type VDOs part of Discover Identity command result. 3 values
> > > > > > > +             are displayed (for the 3 possible Product Type VDOs), one per line.
> > > > > >
> > > > > > sysfs is "one value per file", not "one value per line".  This is not
> > > > > > ok.
> > > > >
> > > > > I see. Would listing these out as three separate vdos (i.e vdo0, vdo1,
> > > > > vdo2) be better?
> > > >
> > > > Given that your current implementation is not acceptable, something has
> > > > to change :)
> > >
> > > Got it. I'd like to see if Heikki has any suggestions on naming these
> > > entries better.
> >
> > Why not have product type specific attribute files?
> >
> > So if the partner is UFP, then we expose ufp1 and ufp2 files that
> > return the UFP1 and UFP2 VDO values and hide the other files:
> >
> >         % ls /sys/class/typec/port0-partner/identity/
> >         id_header cert_stat product ufp1 ufp2
> >
> > If the partner is DFP, then you expose the dfp file and hide
> > everything else:
> >
> >         % ls /sys/class/typec/port0-partner/identity/
> >         id_header cert_stat product dfp
> >
> > And so on.
> 
> I would caution against any decoding of the VDO contents in the kernel
> and making assumptions about the # or the names of these three
> individual objects.
> 
> Since PD 2.0 through PD 3.0, and PD 3.0's different subrevisions (1.0,
> 1.3, 2.0), the # of VDOs that have been supported has changed in the
> various spec versions.
> 
> PD R3.0 V2.0 actually added extra objects here (UFP VDO1 UFP VDO2, DFP
> VDO), but thanks to some troublemaker (me, actually...), the PD spec's
> next version deprecates and deletes two of them (the AMA VDO and the
> UFP VDO2 are gone, thanks to an ECR I put into USB PD).
> 
> (If you've got USB PD working group access, the two ECRs in question
> are: https://groups.usb.org/wg/powerdelivery/document/11007 and
> https://groups.usb.org/wg/powerdelivery/document/10967).
> 
> Since the different spec versions need to all be supported (since the
> firmware of PD devices are baked for a particular version of the PD
> spec at the time they are released and don't change in practice), the
> software on USB PD hosts should provide these objects up to the next
> layer without adding any extra decoding, and the upper layer
> (userspace) can figure out the specifics based on comparing different
> revision and version fields to figure out what vdo1, vdo2, and vdo3
> are.

Agreed. This is a good point. This reminds me why I never exposed the
product type VDOs in the first place.

> Anyway, hope this helps, and sorry in advance for making this section
> of the PD spec more complicated to handle over time...

Thanks for the heads up. For the record, I don't think you are the
only troublemaker :-). Some of the parts in the USB PD specification
and many parts in the USB Type-C specification have been a bit of a
moving target (though, I was hoping that things would have settled
down by now).

This is btw. the reason why I wanted to use interpreted kernel
specific attribute files for example with the roles instead of trying
to expose things like the specs says. I never accepted terms like UFP,
DFP, DRP or DRD, and I'm really clad I didn't - the meaning of those
has changed over the time in the USB Type-C specifications.

Because of the constantly changing specifications, the goal remains
the same. We pick only the details that we need, and that we are
pretty certain will not change, and expose those in our own format
(which ideally is human readable as well as machine readable) instead
of exposing all the data that those details are part of in the raw
format that follows the specification of today.

But I guess we can't pick any more specific details out of the
response to the Discover Identity. We are going to have to dump the
whole thing to the user as it is.


thanks,

-- 
heikki

Re: [PATCH v2] usb: typec: Expose Product Type VDOs via sysfs

[Prashant Malani]

On Thu, Oct 22, 2020 at 12:13 AM Prashant Malani <pmalani@chromium.org> wrote:
>
> Thanks for reviewing the patch, Greg.
>
> On Wed, Oct 21, 2020 at 11:56 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > >
> > > +What:                /sys/class/typec/<port>-cable/identity/product_type_vdo
> > > +Date:                October 2020
> > > +Contact:     Prashant Malani <pmalani@chromium.org>
> > > +Description:
> > > +             Product Type VDOs part of Discover Identity command result. 3 values
> > > +             are displayed (for the 3 possible Product Type VDOs), one per line.
> > > +             The values will show 0s until Discover Identity command result becomes
> > > +             available. The values can be polled.
> >
> > Why are you describing the same value in two different locations?
>
> Both cable and partner can have Discover Identity VDOs and they are
> listed separately in sysfs.
> The other VDOs (id_header, cert_stat and product) have separate
> descriptions for cable and partner too.
> Perhaps there is a case for consolidating the listings here (factor
> out the ones which are common to cable and partner).
>

Just an update: I added a patch to the series which consolidates these
repeat identity entries into 1 location:
https://lore.kernel.org/linux-usb/20201023214328.1262883-1-pmalani@chromium.org/

Thanks and best regards,