On 30 January 2018 13:12, Heikki Krogerus wrote: > Hi Adam, > > On Tue, Jan 02, 2018 at 03:50:54PM +0000, Adam Thomson wrote: > > This commit adds a power_supply class instance to represent a > > PD source's voltage and current properties. This provides an > > interface for reading these properties from user-space or other > > drivers. > > > > For PPS enabled Sources, this also provides write access to set > > the current and voltage and allows for swapping between standard > > PDO and PPS APDO. > > > > As this represents a superset of the information provided in the > > fusb302 driver, the power_supply instance in that code is removed > > as part of this change, so reverting the commit titled > > 'typec: tcpm: Represent source supply through power_supply class' > > > > > Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> > > --- > > .../ABI/testing/sysfs-class-power-tcpm-source-psy | 92 ++++++++ > > drivers/usb/typec/Kconfig | 1 + > > drivers/usb/typec/fusb302/Kconfig | 2 +- > > drivers/usb/typec/fusb302/fusb302.c | 63 +----- > > drivers/usb/typec/tcpm.c | 233 ++++++++++++++++++++- > > 5 files changed, 328 insertions(+), 63 deletions(-) > > create mode 100644 Documentation/ABI/testing/sysfs-class-power-tcpm-source- > psy > > > > diff --git a/Documentation/ABI/testing/sysfs-class-power-tcpm-source-psy > b/Documentation/ABI/testing/sysfs-class-power-tcpm-source-psy > > new file mode 100644 > > index 0000000..4986cba > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-class-power-tcpm-source-psy > > @@ -0,0 +1,92 @@ > > +What: /sys/class/power_supply/tcpm-source-psy/type > > +Date: December 2017 > > +Contact: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> > > +Description: > > + This read-only property describes the main type of source supply. > > + Type-C is a USB standard so this property always returns "USB". > > + > > +What: /sys/class/power_supply/tcpm-source-psy/connected_type > > +Date: December 2017 > > +Contact: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> > > +Description: > > + This read-only property describes the type of source supply that is > > + connected, if the supply is online. The value is always Type C > > + unless a source has been attached which is identified as USB-PD capable. > > + > > + Valid values: > > + - "USB_TYPE_C" : Type C connected supply, not UBS-PD capable > > + (default value) > > + - "USB_PD" : USB-PD capable source supply connected > > + - "USB_PD_PPS" : USB-PD PPS capable source supply connected > > + > > +What: /sys/class/power_supply/tcpm-source-psy/online > > +Date: December 2017 > > +Contact: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> > > +Description: > > + This read-write property describes the online state of the source > > + supply. When the value of this property is not 0, and the supply allows > > + it, then it's possible to switch between online states (i.e. 1 -> 2, > > + 2 -> 1) > > + > > + Valid values: > > + - 0 : Offline, no source supply attached > > + - 1 : Fixed Online, Type-C or USB-PD capable supply > > + attached, non-configurable current and voltage > > + properties in this state. > > + - 2 : PPS Online, USB-PD PPS feature enabled, 'current_now' > > + and 'voltage_now' properties can be modified in this > > + state. Re-writing of this value again, once already > > + set, will re-request the same configured voltage and > > + current values. This can be used as a keep-alive for > > + the PPS connection. > > + [NOTE: This is value only selectable if > > + 'connected_type' reports a value of "USB_PD_PPS"] > > + > > +What: /sys/class/power_supply/tcpm-source-psy/voltage_min > > +Date: December 2017 > > +Contact: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> > > +Description: > > + This read-only property describes the minimum voltage the source supply > > + can provide. > > + > > + Value in microvolts. > > + > > +What: /sys/class/power_supply/tcpm-source-psy/voltage_max > > +Date: December 2017 > > +Contact: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> > > +Description: > > + This read-only property describes the maximum voltage the source supply > > + can provide. > > + > > + Value in microvolts. > > + > > +What: /sys/class/power_supply/tcpm-source-psy/voltage_now > > +Date: December 2017 > > +Contact: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> > > +Description: > > + This read-write property describes the voltage the source supply is > > + providing now. This property can only be written to if the source supply > > + is in online state '2' (PPS enabled), otherwise it's read-only > > + information. > > + > > + Value in microvolts. > > + > > +What: /sys/class/power_supply/tcpm-source-psy/current_max > > +Date: December 2017 > > +Contact: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> > > +Description: > > + This read-only property describes the maximum current the source supply > > + can provide. > > + > > + Value in microamps. > > + > > +What: /sys/class/power_supply/tcpm-source-psy/current_now > > +Date: December 2017 > > +Contact: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx> > > +Description: > > + This read-write property describes the current the source supply can > > + provide now. This property can only be written to if the source supply > > + is in online state '2' (PPS enabled), otherwise it's read-only > > + information. > > + > > + Value in microamps. > > I think those should be documented for the entire psy class, not just > for this driver. Right now there is no documentation for the generic psy class. The stuff in sysfs-class-power is device specific property information, and the same goes for sysfs-class-power-twl4030. The property usage can vary depending on driver implementation, an example being the 'online' property which can differ between drivers, so the usage I have here is very much tcpm related. Also, the ability to write to certain properties varies depending on the driver and HW, so here where we configure 'voltage_now' and 'current_now', the likelihood is that most other psy driver implementations won't allow for this. Sebastian, do you have any thoughts on this discussion? Would be useful to get your input here. > > > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig > > index bcb2744..1ef606d 100644 > > --- a/drivers/usb/typec/Kconfig > > +++ b/drivers/usb/typec/Kconfig > > @@ -48,6 +48,7 @@ if TYPEC > > config TYPEC_TCPM > > tristate "USB Type-C Port Controller Manager" > > depends on USB > > + select POWER_SUPPLY > > I'm a little bit uncomfortable with such a strong dependency on an > other subsystem that we may not always need, but let's see what > Guenter says. Right now I've used the psy class to represent power information from the external PD source supply, so for the scenarios where we're the source or a non-PD device has been attached then this is not going to provide any information to user-space. I guess in the future we could update to represent information when we're the PD source as well, although that'll likely be all just read-only info. Right now, not sure if this is something that would be useful. Sebastian, do you seen any problems on the dependency front? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html