[PATCH] usb: pd: Exposing the Peak Current value of Fixed Supplies to user space

[PATCH] usb: pd: Exposing the Peak Current value of Fixed Supplies to user space

[Heikki Krogerus]

Exposing the value of the field as is.

The Peak Current value has to be interpreted as described
in Table 6-10 (Fixed Power Source Peak Current Capability)
of the USB Power Delivery Specification, but that
interpretation will be done in user space, not in kernel.

Suggested-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/pd.c | 10 ++++------
 include/linux/usb/pd.h |  1 +
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/pd.c b/drivers/usb/typec/pd.c
index 8cc66e4467c4..85d015cdbe1f 100644
--- a/drivers/usb/typec/pd.c
+++ b/drivers/usb/typec/pd.c
@@ -83,14 +83,12 @@ unchunked_extended_messages_supported_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(unchunked_extended_messages_supported);
 
-/*
- * REVISIT: Peak Current requires access also to the RDO.
 static ssize_t
 peak_current_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
-	...
+	return sysfs_emit(buf, "%u\n", (to_pdo(dev)->pdo >> PDO_FIXED_PEAK_CURR_SHIFT) & 3);
 }
-*/
+static DEVICE_ATTR_RO(peak_current);
 
 static ssize_t
 fast_role_swap_current_show(struct device *dev, struct device_attribute *attr, char *buf)
@@ -135,7 +133,7 @@ static struct attribute *source_fixed_supply_attrs[] = {
 	&dev_attr_usb_communication_capable.attr,
 	&dev_attr_dual_role_data.attr,
 	&dev_attr_unchunked_extended_messages_supported.attr,
-	/*&dev_attr_peak_current.attr,*/
+	&dev_attr_peak_current.attr,
 	&dev_attr_voltage.attr,
 	&maximum_current_attr.attr,
 	NULL
@@ -144,7 +142,7 @@ static struct attribute *source_fixed_supply_attrs[] = {
 static umode_t fixed_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
 {
 	if (to_pdo(kobj_to_dev(kobj))->object_position &&
-	    /*attr != &dev_attr_peak_current.attr &&*/
+	    attr != &dev_attr_peak_current.attr &&
 	    attr != &dev_attr_voltage.attr &&
 	    attr != &maximum_current_attr.attr &&
 	    attr != &operational_current_attr.attr)
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index c59fb79a42e8..eb626af0e4e7 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -228,6 +228,7 @@ enum pd_pdo_type {
 #define PDO_FIXED_UNCHUNK_EXT		BIT(24) /* Unchunked Extended Message supported (Source) */
 #define PDO_FIXED_FRS_CURR_MASK		(BIT(24) | BIT(23)) /* FR_Swap Current (Sink) */
 #define PDO_FIXED_FRS_CURR_SHIFT	23
+#define PDO_FIXED_PEAK_CURR_SHIFT	20
 #define PDO_FIXED_VOLT_SHIFT		10	/* 50mV units */
 #define PDO_FIXED_CURR_SHIFT		0	/* 10mA units */
 
-- 
2.40.1

Re: [PATCH] usb: pd: Exposing the Peak Current value of Fixed Supplies to user space

[Greg Kroah-Hartman]

On Wed, Sep 27, 2023 at 02:10:31PM +0300, Heikki Krogerus wrote:
> Exposing the value of the field as is.
> 
> The Peak Current value has to be interpreted as described
> in Table 6-10 (Fixed Power Source Peak Current Capability)
> of the USB Power Delivery Specification, but that
> interpretation will be done in user space, not in kernel.
> 
> Suggested-by: Douglas Gilbert <dgilbert@interlog.com>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/usb/typec/pd.c | 10 ++++------
>  include/linux/usb/pd.h |  1 +
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/typec/pd.c b/drivers/usb/typec/pd.c
> index 8cc66e4467c4..85d015cdbe1f 100644
> --- a/drivers/usb/typec/pd.c
> +++ b/drivers/usb/typec/pd.c
> @@ -83,14 +83,12 @@ unchunked_extended_messages_supported_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(unchunked_extended_messages_supported);
>  
> -/*
> - * REVISIT: Peak Current requires access also to the RDO.
>  static ssize_t
>  peak_current_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
> -	...
> +	return sysfs_emit(buf, "%u\n", (to_pdo(dev)->pdo >> PDO_FIXED_PEAK_CURR_SHIFT) & 3);
>  }
> -*/
> +static DEVICE_ATTR_RO(peak_current);

Don't you need to add a Documentation/ABI/ entry for this new field?

thanks,

greg k-h

Re: [PATCH] usb: pd: Exposing the Peak Current value of Fixed Supplies to user space

[Heikki Krogerus]

On Wed, Sep 27, 2023 at 02:10:38PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 27, 2023 at 02:10:31PM +0300, Heikki Krogerus wrote:
> > Exposing the value of the field as is.
> > 
> > The Peak Current value has to be interpreted as described
> > in Table 6-10 (Fixed Power Source Peak Current Capability)
> > of the USB Power Delivery Specification, but that
> > interpretation will be done in user space, not in kernel.
> > 
> > Suggested-by: Douglas Gilbert <dgilbert@interlog.com>
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  drivers/usb/typec/pd.c | 10 ++++------
> >  include/linux/usb/pd.h |  1 +
> >  2 files changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/usb/typec/pd.c b/drivers/usb/typec/pd.c
> > index 8cc66e4467c4..85d015cdbe1f 100644
> > --- a/drivers/usb/typec/pd.c
> > +++ b/drivers/usb/typec/pd.c
> > @@ -83,14 +83,12 @@ unchunked_extended_messages_supported_show(struct device *dev,
> >  }
> >  static DEVICE_ATTR_RO(unchunked_extended_messages_supported);
> >  
> > -/*
> > - * REVISIT: Peak Current requires access also to the RDO.
> >  static ssize_t
> >  peak_current_show(struct device *dev, struct device_attribute *attr, char *buf)
> >  {
> > -	...
> > +	return sysfs_emit(buf, "%u\n", (to_pdo(dev)->pdo >> PDO_FIXED_PEAK_CURR_SHIFT) & 3);
> >  }
> > -*/
> > +static DEVICE_ATTR_RO(peak_current);
> 
> Don't you need to add a Documentation/ABI/ entry for this new field?

Yes. Sorry about that. I'll send v2.

thanks,

-- 
heikki