Hi Greg,
> -----Original Message-----
> From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> Sent: Sunday, February 12, 2023 9:05 PM
> To: Gopal, Saranya <saranya.gopal@xxxxxxxxx>
> Cc: linux-usb@xxxxxxxxxxxxxxx; Heikki Krogerus
> <heikki.krogerus@xxxxxxxxxxxxxxx>; Regupathy, Rajaram
> <rajaram.regupathy@xxxxxxxxx>
> 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@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
> > Signed-off-by: Saranya Gopal <saranya.gopal@xxxxxxxxx>
> > ---
> > .../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@xxxxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxx>
> > +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
