* [PATCH] usb: typec: pd: Add higher_capability sysfs for sink PDO
@ 2023-02-12 14:48 Saranya Gopal
2023-02-12 15:34 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Saranya Gopal @ 2023-02-12 14:48 UTC (permalink / raw)
To: linux-usb; +Cc: gregkh, Saranya Gopal, Heikki Krogerus, Rajaram Regupathy
As per USB PD specification, 28th bit of sink fixed power supply
PDO represents higher capability. If this bit is set, it indicates
that the sink needs more than vsafe5V to provide full functionality.
This patch replaces usb_suspend_supported sysfs with higher_capability
sysfs for sink PDO.
Fixes: 662a60102c12 ("usb: typec: Separate USB Power Delivery from USB Type-C")
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reported-by: Rajaram Regupathy <rajaram.regupathy@intel.com>
Signed-off-by: Saranya Gopal <saranya.gopal@intel.com>
---
.../ABI/testing/sysfs-class-usb_power_delivery | 10 +++++++++-
drivers/usb/typec/pd.c | 9 ++++++++-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-usb_power_delivery b/Documentation/ABI/testing/sysfs-class-usb_power_delivery
index ce2b1b563cb3..59757118b6a3 100644
--- a/Documentation/ABI/testing/sysfs-class-usb_power_delivery
+++ b/Documentation/ABI/testing/sysfs-class-usb_power_delivery
@@ -69,7 +69,7 @@ Description:
This file contains boolean value that tells does the device
support both source and sink power roles.
-What: /sys/class/usb_power_delivery/.../<capability>/1:fixed_supply/usb_suspend_supported
+What: /sys/class/usb_power_delivery/.../source-capabilities/1:fixed_supply/usb_suspend_supported
Date: May 2022
Contact: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Description:
@@ -78,6 +78,14 @@ Description:
will follow the USB 2.0 and USB 3.2 rules for suspend and
resume.
+What: /sys/class/usb_power_delivery/.../sink-capabilities/1:fixed_supply/higher_capability
+Date: February 2023
+Contact: Saranya Gopal <saranya.gopal@linux.intel.com>
+Description:
+ This file shows the value of the Higher capability bit in vsafe5V Fixed Supply Object.
+ If the bit is set, then the sink needs more than vsafe5V(eg. 12 V) to provide
+ full functionality.
+
What: /sys/class/usb_power_delivery/.../<capability>/1:fixed_supply/unconstrained_power
Date: May 2022
Contact: Heikki Krogerus <heikki.krogerus@linux.intel.com>
diff --git a/drivers/usb/typec/pd.c b/drivers/usb/typec/pd.c
index dc72005d68db..59c537a5e600 100644
--- a/drivers/usb/typec/pd.c
+++ b/drivers/usb/typec/pd.c
@@ -48,6 +48,13 @@ usb_suspend_supported_show(struct device *dev, struct device_attribute *attr, ch
}
static DEVICE_ATTR_RO(usb_suspend_supported);
+static ssize_t
+higher_capability_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%u\n", !!(to_pdo(dev)->pdo & PDO_FIXED_HIGHER_CAP));
+}
+static DEVICE_ATTR_RO(higher_capability);
+
static ssize_t
unconstrained_power_show(struct device *dev, struct device_attribute *attr, char *buf)
{
@@ -161,7 +168,7 @@ static struct device_type source_fixed_supply_type = {
static struct attribute *sink_fixed_supply_attrs[] = {
&dev_attr_dual_role_power.attr,
- &dev_attr_usb_suspend_supported.attr,
+ &dev_attr_higher_capability.attr,
&dev_attr_unconstrained_power.attr,
&dev_attr_usb_communication_capable.attr,
&dev_attr_dual_role_data.attr,
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] usb: typec: pd: Add higher_capability sysfs for sink PDO
2023-02-12 14:48 [PATCH] usb: typec: pd: Add higher_capability sysfs for sink PDO Saranya Gopal
@ 2023-02-12 15:34 ` Greg KH
2023-02-12 16:13 ` Gopal, Saranya
0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2023-02-12 15:34 UTC (permalink / raw)
To: Saranya Gopal; +Cc: linux-usb, Heikki Krogerus, Rajaram Regupathy
On Sun, Feb 12, 2023 at 08:18:38PM +0530, Saranya Gopal wrote:
> As per USB PD specification, 28th bit of sink fixed power supply
> PDO represents higher capability. If this bit is set, it indicates
> that the sink needs more than vsafe5V to provide full functionality.
> This patch replaces usb_suspend_supported sysfs with higher_capability
> sysfs for sink PDO.
>
> Fixes: 662a60102c12 ("usb: typec: Separate USB Power Delivery from USB Type-C")
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Where was this reviewed? I don't see that on the list, am I missing it?
> Reported-by: Rajaram Regupathy <rajaram.regupathy@intel.com>
> Signed-off-by: Saranya Gopal <saranya.gopal@intel.com>
> ---
> .../ABI/testing/sysfs-class-usb_power_delivery | 10 +++++++++-
> drivers/usb/typec/pd.c | 9 ++++++++-
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-usb_power_delivery b/Documentation/ABI/testing/sysfs-class-usb_power_delivery
> index ce2b1b563cb3..59757118b6a3 100644
> --- a/Documentation/ABI/testing/sysfs-class-usb_power_delivery
> +++ b/Documentation/ABI/testing/sysfs-class-usb_power_delivery
> @@ -69,7 +69,7 @@ Description:
> This file contains boolean value that tells does the device
> support both source and sink power roles.
>
> -What: /sys/class/usb_power_delivery/.../<capability>/1:fixed_supply/usb_suspend_supported
> +What: /sys/class/usb_power_delivery/.../source-capabilities/1:fixed_supply/usb_suspend_supported
> Date: May 2022
> Contact: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Description:
> @@ -78,6 +78,14 @@ Description:
> will follow the USB 2.0 and USB 3.2 rules for suspend and
> resume.
>
> +What: /sys/class/usb_power_delivery/.../sink-capabilities/1:fixed_supply/higher_capability
> +Date: February 2023
> +Contact: Saranya Gopal <saranya.gopal@linux.intel.com>
> +Description:
> + This file shows the value of the Higher capability bit in vsafe5V Fixed Supply Object.
> + If the bit is set, then the sink needs more than vsafe5V(eg. 12 V) to provide
> + full functionality.
You don't explain what this file will show. 0/1? Y/N? J/N?
Also, properly wrap your lines at 80 columns for documentation please.
And adding a new sysfs entry does not "Fix" anything, why is this tagged
as such?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH] usb: typec: pd: Add higher_capability sysfs for sink PDO
2023-02-12 15:34 ` Greg KH
@ 2023-02-12 16:13 ` Gopal, Saranya
2023-02-13 13:09 ` Heikki Krogerus
0 siblings, 1 reply; 4+ messages in thread
From: Gopal, Saranya @ 2023-02-12 16:13 UTC (permalink / raw)
To: Greg KH; +Cc: linux-usb@vger.kernel.org, Heikki Krogerus, Regupathy, Rajaram
Hi Greg,
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Sunday, February 12, 2023 9:05 PM
> To: Gopal, Saranya <saranya.gopal@intel.com>
> Cc: linux-usb@vger.kernel.org; Heikki Krogerus
> <heikki.krogerus@linux.intel.com>; Regupathy, Rajaram
> <rajaram.regupathy@intel.com>
> Subject: Re: [PATCH] usb: typec: pd: Add higher_capability sysfs for
> sink PDO
>
> On Sun, Feb 12, 2023 at 08:18:38PM +0530, Saranya Gopal wrote:
> > As per USB PD specification, 28th bit of sink fixed power supply
> > PDO represents higher capability. If this bit is set, it indicates
> > that the sink needs more than vsafe5V to provide full functionality.
> > This patch replaces usb_suspend_supported sysfs with
> higher_capability
> > sysfs for sink PDO.
> >
> > Fixes: 662a60102c12 ("usb: typec: Separate USB Power Delivery
> from USB Type-C")
> > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
> Where was this reviewed? I don't see that on the list, am I missing
> it?
It was reviewed internally in Intel's internal mailing list.
>
> > Reported-by: Rajaram Regupathy <rajaram.regupathy@intel.com>
> > Signed-off-by: Saranya Gopal <saranya.gopal@intel.com>
> > ---
> > .../ABI/testing/sysfs-class-usb_power_delivery | 10
> +++++++++-
> > drivers/usb/typec/pd.c | 9 ++++++++-
> > 2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-
> usb_power_delivery b/Documentation/ABI/testing/sysfs-class-
> usb_power_delivery
> > index ce2b1b563cb3..59757118b6a3 100644
> > --- a/Documentation/ABI/testing/sysfs-class-usb_power_delivery
> > +++ b/Documentation/ABI/testing/sysfs-class-usb_power_delivery
> > @@ -69,7 +69,7 @@ Description:
> > This file contains boolean value that tells does the
> device
> > support both source and sink power roles.
> >
> > -What:
> /sys/class/usb_power_delivery/.../<capability>/1:fixed_sup
> ply/usb_suspend_supported
> > +What: /sys/class/usb_power_delivery/.../source-
> capabilities/1:fixed_supply/usb_suspend_supported
> > Date: May 2022
> > Contact: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Description:
> > @@ -78,6 +78,14 @@ Description:
> > will follow the USB 2.0 and USB 3.2 rules for
> suspend and
> > resume.
> >
> > +What: /sys/class/usb_power_delivery/.../sink-
> capabilities/1:fixed_supply/higher_capability
> > +Date: February 2023
> > +Contact: Saranya Gopal <saranya.gopal@linux.intel.com>
> > +Description:
> > + This file shows the value of the Higher capability bit
> in vsafe5V Fixed Supply Object.
> > + If the bit is set, then the sink needs more than
> vsafe5V(eg. 12 V) to provide
> > + full functionality.
>
> You don't explain what this file will show. 0/1? Y/N? J/N?
>
> Also, properly wrap your lines at 80 columns for documentation
> please.
This sysfs will show 0/1 value.
I have tried to maintain consistency with the rest of the file.
(https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-usb_power_delivery)
That is why I did not add the 0/1 value and also did not wrap the lines to 80.
I can fix these in v2 if you are not convinced.
>
> And adding a new sysfs entry does not "Fix" anything, why is this
> tagged
> as such?
Sink fixed supply PDO wrongly shows usb_suspend_supported sysfs instead of higher_capability sysfs.
Sink PDO does not have this 'usb_suspend_supported' bit.
Only source fixed supply PDO has this bit. So, this patch adds higher_capability bit support for sink PDO.
That is why 'fixes' tag was added.
Thanks,
Saranya
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] usb: typec: pd: Add higher_capability sysfs for sink PDO
2023-02-12 16:13 ` Gopal, Saranya
@ 2023-02-13 13:09 ` Heikki Krogerus
0 siblings, 0 replies; 4+ messages in thread
From: Heikki Krogerus @ 2023-02-13 13:09 UTC (permalink / raw)
To: Gopal, Saranya; +Cc: Greg KH, linux-usb@vger.kernel.org, Regupathy, Rajaram
Hi,
On Sun, Feb 12, 2023 at 04:13:22PM +0000, Gopal, Saranya wrote:
> Hi Greg,
>
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Sunday, February 12, 2023 9:05 PM
> > To: Gopal, Saranya <saranya.gopal@intel.com>
> > Cc: linux-usb@vger.kernel.org; Heikki Krogerus
> > <heikki.krogerus@linux.intel.com>; Regupathy, Rajaram
> > <rajaram.regupathy@intel.com>
> > Subject: Re: [PATCH] usb: typec: pd: Add higher_capability sysfs for
> > sink PDO
> >
> > On Sun, Feb 12, 2023 at 08:18:38PM +0530, Saranya Gopal wrote:
> > > As per USB PD specification, 28th bit of sink fixed power supply
> > > PDO represents higher capability. If this bit is set, it indicates
> > > that the sink needs more than vsafe5V to provide full functionality.
> > > This patch replaces usb_suspend_supported sysfs with
> > higher_capability
> > > sysfs for sink PDO.
> > >
> > > Fixes: 662a60102c12 ("usb: typec: Separate USB Power Delivery
> > from USB Type-C")
> > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >
> > Where was this reviewed? I don't see that on the list, am I missing
> > it?
> It was reviewed internally in Intel's internal mailing list.
>
> >
> > > Reported-by: Rajaram Regupathy <rajaram.regupathy@intel.com>
> > > Signed-off-by: Saranya Gopal <saranya.gopal@intel.com>
> > > ---
> > > .../ABI/testing/sysfs-class-usb_power_delivery | 10
> > +++++++++-
> > > drivers/usb/typec/pd.c | 9 ++++++++-
> > > 2 files changed, 17 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-class-
> > usb_power_delivery b/Documentation/ABI/testing/sysfs-class-
> > usb_power_delivery
> > > index ce2b1b563cb3..59757118b6a3 100644
> > > --- a/Documentation/ABI/testing/sysfs-class-usb_power_delivery
> > > +++ b/Documentation/ABI/testing/sysfs-class-usb_power_delivery
> > > @@ -69,7 +69,7 @@ Description:
> > > This file contains boolean value that tells does the
> > device
> > > support both source and sink power roles.
> > >
> > > -What:
> > /sys/class/usb_power_delivery/.../<capability>/1:fixed_sup
> > ply/usb_suspend_supported
> > > +What: /sys/class/usb_power_delivery/.../source-
> > capabilities/1:fixed_supply/usb_suspend_supported
> > > Date: May 2022
> > > Contact: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > Description:
> > > @@ -78,6 +78,14 @@ Description:
> > > will follow the USB 2.0 and USB 3.2 rules for
> > suspend and
> > > resume.
> > >
> > > +What: /sys/class/usb_power_delivery/.../sink-
> > capabilities/1:fixed_supply/higher_capability
> > > +Date: February 2023
> > > +Contact: Saranya Gopal <saranya.gopal@linux.intel.com>
> > > +Description:
> > > + This file shows the value of the Higher capability bit
> > in vsafe5V Fixed Supply Object.
> > > + If the bit is set, then the sink needs more than
> > vsafe5V(eg. 12 V) to provide
> > > + full functionality.
> >
> > You don't explain what this file will show. 0/1? Y/N? J/N?
> >
> > Also, properly wrap your lines at 80 columns for documentation
> > please.
> This sysfs will show 0/1 value.
I think we need to fix this one. Although, the description does
clearly say that this file shows the value of a bit, it's still better
to separately show the possible values - so 0 or 1.
> I have tried to maintain consistency with the rest of the file.
> (https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-usb_power_delivery)
> That is why I did not add the 0/1 value and also did not wrap the lines to 80.
> I can fix these in v2 if you are not convinced.
But the descriptions are wrapped at 80 characters in that document?
> > And adding a new sysfs entry does not "Fix" anything, why is this
> > tagged
> > as such?
> Sink fixed supply PDO wrongly shows usb_suspend_supported sysfs instead of higher_capability sysfs.
> Sink PDO does not have this 'usb_suspend_supported' bit.
> Only source fixed supply PDO has this bit. So, this patch adds higher_capability bit support for sink PDO.
> That is why 'fixes' tag was added.
Yep. So the value was assigned to the wrong sysfs file. I do think
this is a fix.
Br,
--
heikki
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-13 13:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-12 14:48 [PATCH] usb: typec: pd: Add higher_capability sysfs for sink PDO Saranya Gopal
2023-02-12 15:34 ` Greg KH
2023-02-12 16:13 ` Gopal, Saranya
2023-02-13 13:09 ` Heikki Krogerus
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.