Re: [PATCH v3 2/2] usb: typec: Expose Product Type VDOs via sysfs

[Heikki Krogerus]

On Fri, Oct 23, 2020 at 02:43:28PM -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 sysfs attributes to expose these to userspace.
> 
> Cc: Benson Leung <bleung@xxxxxxxxxxxx>
> Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Signed-off-by: Prashant Malani <pmalani@xxxxxxxxxxxx>

I'm now fine with this as it is, no more objections. I'll send the
"type2 attribute patch separately.

I think you need to rebase this too, but FWIW:

Reviewed-by Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>

> ---
> 
> Changes in v3:
> - Split each product type VDO into a separate attribute.
> - Changed sprintf() to sysfs_emit().
> - Changed ABI documentation based on consolidation of identity VDO
>   descriptions in the previous patch (1/2).
> 
> 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 | 24 +++++++++++++++
>  drivers/usb/typec/class.c                   | 33 +++++++++++++++++++++
>  2 files changed, 57 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index 0f839fd022f1..0ac144bc5927 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -205,6 +205,30 @@ Description:
>  		will show 0 until Discover Identity command result becomes
>  		available. The value can be polled.
>  
> +What:		/sys/class/typec/<port>-{partner|cable}/identity/product_type_vdo1
> +Date:		October 2020
> +Contact:	Prashant Malani <pmalani@xxxxxxxxxxxx>
> +Description:
> +		1st Product Type VDO of Discover Identity command result.
> +		The value will show 0 until Discover Identity command result becomes
> +		available and a valid Product Type VDO is returned.
> +
> +What:		/sys/class/typec/<port>-{partner|cable}/identity/product_type_vdo2
> +Date:		October 2020
> +Contact:	Prashant Malani <pmalani@xxxxxxxxxxxx>
> +Description:
> +		2nd Product Type VDO of Discover Identity command result.
> +		The value will show 0 until Discover Identity command result becomes
> +		available and a valid Product Type VDO is returned.
> +
> +What:		/sys/class/typec/<port>-{partner|cable}/identity/product_type_vdo3
> +Date:		October 2020
> +Contact:	Prashant Malani <pmalani@xxxxxxxxxxxx>
> +Description:
> +		3rd Product Type VDO of Discover Identity command result.
> +		The value will show 0 until Discover Identity command result becomes
> +		available and a valid Product Type VDO is returned.
> +
>  
>  USB Type-C port alternate mode devices.
>  
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 35eec707cb51..a2c88594b044 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -122,10 +122,40 @@ static ssize_t product_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(product);
>  
> +static ssize_t product_type_vdo1_show(struct device *dev, struct device_attribute *attr,
> +				      char *buf)
> +{
> +	struct usb_pd_identity *id = get_pd_identity(dev);
> +
> +	return sysfs_emit(buf, "0x%08x\n", id->vdo[0]);
> +}
> +static DEVICE_ATTR_RO(product_type_vdo1);
> +
> +static ssize_t product_type_vdo2_show(struct device *dev, struct device_attribute *attr,
> +				      char *buf)
> +{
> +	struct usb_pd_identity *id = get_pd_identity(dev);
> +
> +	return sysfs_emit(buf, "0x%08x\n", id->vdo[1]);
> +}
> +static DEVICE_ATTR_RO(product_type_vdo2);
> +
> +static ssize_t product_type_vdo3_show(struct device *dev, struct device_attribute *attr,
> +				      char *buf)
> +{
> +	struct usb_pd_identity *id = get_pd_identity(dev);
> +
> +	return sysfs_emit(buf, "0x%08x\n", id->vdo[2]);
> +}
> +static DEVICE_ATTR_RO(product_type_vdo3);
> +
>  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_vdo1.attr,
> +	&dev_attr_product_type_vdo2.attr,
> +	&dev_attr_product_type_vdo3.attr,
>  	NULL
>  };
>  
> @@ -144,6 +174,9 @@ 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_vdo1");
> +	sysfs_notify(&dev->kobj, "identity", "product_type_vdo2");
> +	sysfs_notify(&dev->kobj, "identity", "product_type_vdo3");
>  }
>  
>  /* ------------------------------------------------------------------------- */
> -- 
> 2.29.0.rc1.297.gfa9743e501-goog

thanks,

-- 
heikki