Hi Heikki, On Thu, Oct 22, 2020 at 5:43 AM Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > > > +What: /sys/class/typec/<port>-partner/identity/product_type_vdo > > > > > > +Date: October 2020 > > > > > > +Contact: Prashant Malani <pmalani@xxxxxxxxxxxx> > > > > > > +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@xxxxxxxxxx Chromium OS Project bleung@xxxxxxxxxxxx
