| Message ID | 20201118150059.3419-4-heikki.krogerus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: typec: Product Type time | expand |
On Wed, Nov 18, 2020 at 06:00:59PM +0300, Heikki Krogerus wrote: > From: Prashant Malani <pmalani@chromium.org> > > Interim. ABI doc missing. > > 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 attribute to expose these to userspace. > > Signed-off-by: Prashant Malani <pmalani@chromium.org> > [ heikki: Only one instead of three attribute files ] > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>' > --- > drivers/usb/typec/class.c | 41 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > index 303f054181ff7..5e135678f5952 100644 > --- a/drivers/usb/typec/class.c > +++ b/drivers/usb/typec/class.c > @@ -165,15 +165,55 @@ 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); > + size_t len = 0; > + int i; > + > + for (i = 0; i < 3; i++) { > + if (!id->vdo[i]) > + break; > + len += sysfs_emit(buf, "%08x ", id->vdo[i]); > + } > + > + buf[len - 1] = '\n'; > + > + return len; > +} I don't understand what you are trying to print out here, documentation would be helpful :) > + > +static struct device_attribute dev_attr_product_type_vdo = { > + .attr = { > + .name = "product_type", > + .mode = 0444, > + }, > + .show = product_type_vdo_show, > +}; DEVICE_ATTR_RO(product_type_vdo)? Why are you calling it "product_type" and not with the "vdo"? And you have to name it this, there's always __ATTR_RO(), never put a mode in "raw" numbers for a sysfs file if at all possible. thanks, greg k-h
On Wed, Nov 18, 2020 at 04:56:57PM +0100, Greg Kroah-Hartman wrote: > On Wed, Nov 18, 2020 at 06:00:59PM +0300, Heikki Krogerus wrote: > > From: Prashant Malani <pmalani@chromium.org> > > > > Interim. ABI doc missing. > > > > 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 attribute to expose these to userspace. > > > > Signed-off-by: Prashant Malani <pmalani@chromium.org> > > [ heikki: Only one instead of three attribute files ] > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>' > > --- > > drivers/usb/typec/class.c | 41 +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > > index 303f054181ff7..5e135678f5952 100644 > > --- a/drivers/usb/typec/class.c > > +++ b/drivers/usb/typec/class.c > > @@ -165,15 +165,55 @@ 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); > > + size_t len = 0; > > + int i; > > + > > + for (i = 0; i < 3; i++) { > > + if (!id->vdo[i]) > > + break; > > + len += sysfs_emit(buf, "%08x ", id->vdo[i]); > > + } > > + > > + buf[len - 1] = '\n'; > > + > > + return len; > > +} > > I don't understand what you are trying to print out here, documentation > would be helpful :) > > > +static struct device_attribute dev_attr_product_type_vdo = { > > + .attr = { > > + .name = "product_type", > > + .mode = 0444, > > + }, > > + .show = product_type_vdo_show, > > +}; > > DEVICE_ATTR_RO(product_type_vdo)? > > Why are you calling it "product_type" and not with the "vdo"? > > And you have to name it this, there's always __ATTR_RO(), never put a > mode in "raw" numbers for a sysfs file if at all possible. Sorry Greg. This is still work-in-progress. I didn't use the _vdo ending in the file name because the other files exposing the other parts (VDOs) of the response to the discover identity don't have it either. thanks,
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index 303f054181ff7..5e135678f5952 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -165,15 +165,55 @@ 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); + size_t len = 0; + int i; + + for (i = 0; i < 3; i++) { + if (!id->vdo[i]) + break; + len += sysfs_emit(buf, "%08x ", id->vdo[i]); + } + + buf[len - 1] = '\n'; + + return len; +} + +static struct device_attribute dev_attr_product_type_vdo = { + .attr = { + .name = "product_type", + .mode = 0444, + }, + .show = product_type_vdo_show, +}; + +static umode_t +typec_identity_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n) +{ + struct usb_pd_identity *id = get_pd_identity(kobj_to_dev(kobj)); + + if (attr == &dev_attr_product_type_vdo.attr && + !id->vdo[0]) + return 0; + + return attr->mode; +} + 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 }; static const struct attribute_group usb_pd_id_group = { .name = "identity", + .is_visible = typec_identity_attr_is_visible, .attrs = usb_pd_id_attrs, }; @@ -191,6 +231,7 @@ static void typec_product_type_notify(struct device *dev) if (!ptype) return; + sysfs_notify(&dev->kobj, "identity", "product_type"); sysfs_notify(&dev->kobj, NULL, "product_type"); envp[0] = kasprintf(GFP_KERNEL, "PRODUCT_TYPE=%s", ptype);