Re: [RFC PATCH 2/3] usb: typec: Add product_type sysfs attribute file for partners and cables
From: Benson Leung
Date: Wed Nov 18 2020 - 12:48:52 EST
Hi Heikki,
On Wed, Nov 18, 2020 at 06:00:58PM +0300, Heikki Krogerus wrote:
> USB Power Delivery Specification defines a set of product
> types for partners and cables. The product type is defined
> in the ID Header VDO, which is the first object in the
> response to the Discover Identity command.
>
> This sysfs attribute file is only created for the partners
> and cables if the product type is really known in the
> driver. Some interfaces do not give access to the Discover
> Identity response from the partner or cable, but they may
> still supply the product type separately in some cases.
>
> When the product type of the partner or cable is detected,
> uevent is also raised with PRODUCT_TYPE set to show the
> actual product type (for example PRODUCT_TYPE=host).
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> Documentation/ABI/testing/sysfs-class-typec | 55 ++++++++
> drivers/usb/typec/class.c | 132 ++++++++++++++++++--
> 2 files changed, 180 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index b7794e02ad205..4c09e327c62be 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -139,6 +139,42 @@ Description:
> Shows if the partner supports USB Power Delivery communication:
> Valid values: yes, no
>
> +What: /sys/class/typec/<port>-partner/product_type
> +Date: December 2020
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description: USB Power Delivery Specification defines a set of product types
> + for the partner devices. This file will show the product type of
> + the partner if it is known. Dual-role capable partners will have
> + both UFP and DFP product types defined, but only one that
> + matches the current role will be active at the time. If the
> + product type of the partner is not visible to the device driver,
> + this file will not exist.
> +
> + When the partner product type is detected, or changed with role
> + swap, uvevent is also raised that contains PRODUCT_TYPE=<product
> + type> (for example PRODUCT_TYPE=hub).
> +
> + Valid values:
> +
> + UFP / device role
> + ======================== ==========================
> + undefined -
> + hub PDUSB Hub
> + peripheral PDUSB Peripheral
> + psd Power Bank
> + ama Alternate Mode Adapter
> + vpd VCONN Powered USB Device
I have it on good authority that "vpd" is incorrectly categorized here,
and for future proofing, we'd better not introduce vpd as a product
type for UFP...
A vpd is actually more closely related to a "cable" than it is a "UFP."
A closer reading of the USB Type-C and USB PD specs will reveal that
VPDs can only ever appear as SOP' and not as SOP, so having its type
appear under UFP is a mistake.
In other words, the USB PD V3.0 R2.0 spec is wrong. A change has been
working its way through the spec committee to fix this, but it is not yet
published.
In order to reduce the amount of churn, I would recommend not
including vpd as a possible type until a new version of the spec (or the ECN)
is published.
Thanks,
Benson
> + ======================== ==========================
> +
> + DFP / host role
> + ======================== ==========================
> + undefined -
> + hub PDUSB Hub
> + host PDUSB Host
> + power_brick Power Brick
> + amc Alternate Mode Controller
> + ======================== ==========================
> +
> What: /sys/class/typec/<port>-partner>/identity/
> Date: April 2017
> Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> @@ -202,6 +238,25 @@ Description:
> - type-c
> - captive
>
> +What: /sys/class/typec/<port>-cable/product_type
> +Date: December 2020
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description: USB Power Delivery Specification defines a set of product types
> + for the cables. This file will show the product type of the
> + cable if it is known. If the product type of the cable is not
> + visible to the device driver, this file will not exist.
> +
> + When the cable product type is detected, uvevent is also raised
> + with PRODUCT_TYPE showing the product type of the cable.
> +
> + Valid values:
> +
> + ======================== ==========================
> + undefined -
> + active Active Cable
> + passive Passive Cable
> + ======================== ==========================
> +
> What: /sys/class/typec/<port>-cable/identity/
> Date: April 2017
> Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 35eec707cb512..303f054181ff7 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -11,6 +11,7 @@
> #include <linux/mutex.h>
> #include <linux/property.h>
> #include <linux/slab.h>
> +#include <linux/usb/pd_vdo.h>
>
> #include "bus.h"
>
> @@ -81,6 +82,30 @@ static const char * const typec_accessory_modes[] = {
> [TYPEC_ACCESSORY_DEBUG] = "debug",
> };
>
> +/* Product types defined in USB PD Specification R3.0 V2.0 */
> +static const char * const product_type_ufp[8] = {
> + [IDH_PTYPE_UNDEF] = "undefined",
> + [IDH_PTYPE_HUB] = "hub",
> + [IDH_PTYPE_PERIPH] = "peripheral",
> + [IDH_PTYPE_PSD] = "psd",
> + [IDH_PTYPE_AMA] = "ama",
> + [IDH_PTYPE_VPD] = "vpd",
> +};
> +
> +static const char * const product_type_dfp[8] = {
> + [IDH_PTYPE_DFP_UNDEF] = "undefined",
> + [IDH_PTYPE_DFP_HUB] = "hub",
> + [IDH_PTYPE_DFP_HOST] = "host",
> + [IDH_PTYPE_DFP_PB] = "power_brick",
> + [IDH_PTYPE_DFP_AMC] = "amc",
> +};
> +
> +static const char * const product_type_cable[8] = {
> + [IDH_PTYPE_UNDEF] = "undefined",
> + [IDH_PTYPE_PCABLE] = "passive",
> + [IDH_PTYPE_ACABLE] = "active",
> +};
> +
> static struct usb_pd_identity *get_pd_identity(struct device *dev)
> {
> if (is_typec_partner(dev)) {
> @@ -95,6 +120,24 @@ static struct usb_pd_identity *get_pd_identity(struct device *dev)
> return NULL;
> }
>
> +static const char *get_pd_product_type(struct device *dev)
> +{
> + struct typec_port *port = to_typec_port(dev->parent);
> + struct usb_pd_identity *id = get_pd_identity(dev);
> + const char *ptype = NULL;
> +
> + if (is_typec_partner(dev)) {
> + if (port->data_role == TYPEC_HOST)
> + ptype = product_type_ufp[PD_IDH_PTYPE(id->id_header)];
> + else
> + ptype = product_type_dfp[PD_IDH_DFP_PTYPE(id->id_header)];
> + } else if (is_typec_cable(dev)) {
> + ptype = product_type_cable[PD_IDH_PTYPE(id->id_header)];
> + }
> +
> + return ptype;
> +}
> +
> static ssize_t id_header_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> @@ -139,11 +182,55 @@ static const struct attribute_group *usb_pd_id_groups[] = {
> NULL,
> };
>
> +static void typec_product_type_notify(struct device *dev)
> +{
> + const char *ptype;
> + char *envp[2];
> +
> + ptype = get_pd_product_type(dev);
> + if (!ptype)
> + return;
> +
> + sysfs_notify(&dev->kobj, NULL, "product_type");
> +
> + envp[0] = kasprintf(GFP_KERNEL, "PRODUCT_TYPE=%s", ptype);
> + if (!envp[0])
> + return;
> +
> + envp[1] = NULL;
> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> + kfree(envp[0]);
> +}
> +
> 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");
> + typec_product_type_notify(dev);
> +}
> +
> +static ssize_t
> +product_type_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + const char *ptype;
> +
> + ptype = get_pd_product_type(dev);
> + if (!ptype)
> + return 0;
> +
> + return sysfs_emit(buf, "%s\n", ptype);
> +}
> +static DEVICE_ATTR_RO(product_type);
> +
> +static umode_t typec_product_type_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + if (attr == &dev_attr_product_type.attr)
> + if (!get_pd_identity(kobj_to_dev(kobj)))
> + return 0;
> +
> + return attr->mode;
> }
>
> /* ------------------------------------------------------------------------- */
> @@ -534,10 +621,20 @@ static DEVICE_ATTR_RO(supports_usb_power_delivery);
>
> static struct attribute *typec_partner_attrs[] = {
> &dev_attr_accessory_mode.attr,
> + &dev_attr_product_type.attr,
> &dev_attr_supports_usb_power_delivery.attr,
> NULL
> };
> -ATTRIBUTE_GROUPS(typec_partner);
> +
> +static struct attribute_group typec_partner_group = {
> + .is_visible = typec_product_type_attr_is_visible,
> + .attrs = typec_partner_attrs,
> +};
> +
> +static const struct attribute_group *typec_partner_groups[] = {
> + &typec_partner_group,
> + NULL
> +};
>
> static void typec_partner_release(struct device *dev)
> {
> @@ -773,9 +870,19 @@ static DEVICE_ATTR_RO(plug_type);
> static struct attribute *typec_cable_attrs[] = {
> &dev_attr_type.attr,
> &dev_attr_plug_type.attr,
> + &dev_attr_product_type.attr,
> + NULL
> +};
> +
> +static struct attribute_group typec_cable_group = {
> + .is_visible = typec_product_type_attr_is_visible,
> + .attrs = typec_cable_attrs,
> +};
> +
> +static const struct attribute_group *typec_cable_groups[] = {
> + &typec_cable_group,
> NULL
> };
> -ATTRIBUTE_GROUPS(typec_cable);
>
> static void typec_cable_release(struct device *dev)
> {
> @@ -1352,6 +1459,11 @@ const struct device_type typec_port_dev_type = {
> /* --------------------------------------- */
> /* Driver callbacks to report role updates */
>
> +static int partner_match(struct device *dev, void *data)
> +{
> + return is_typec_partner(dev);
> +}
> +
> /**
> * typec_set_data_role - Report data role change
> * @port: The USB Type-C Port where the role was changed
> @@ -1361,12 +1473,23 @@ const struct device_type typec_port_dev_type = {
> */
> void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
> {
> + struct device *partner_dev;
> +
> if (port->data_role == role)
> return;
>
> port->data_role = role;
> sysfs_notify(&port->dev.kobj, NULL, "data_role");
> kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
> +
> + partner_dev = device_find_child(&port->dev, NULL, partner_match);
> + if (!partner_dev)
> + return;
> +
> + if (to_typec_partner(partner_dev)->identity)
> + typec_product_type_notify(partner_dev);
> +
> + put_device(partner_dev);
> }
> EXPORT_SYMBOL_GPL(typec_set_data_role);
>
> @@ -1407,11 +1530,6 @@ void typec_set_vconn_role(struct typec_port *port, enum typec_role role)
> }
> EXPORT_SYMBOL_GPL(typec_set_vconn_role);
>
> -static int partner_match(struct device *dev, void *data)
> -{
> - return is_typec_partner(dev);
> -}
> -
> /**
> * typec_set_pwr_opmode - Report changed power operation mode
> * @port: The USB Type-C Port where the mode was changed
> --
> 2.29.2
>
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@xxxxxxxxxx
Chromium OS Project
bleung@xxxxxxxxxxxx
Attachment:
signature.asc
Description: PGP signature