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@xxxxxxxxxxxx>
> Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Signed-off-by: Prashant Malani <pmalani@xxxxxxxxxxxx>
> ---
>
> 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@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.
> + 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@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.
> + 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
