Re: [PATCH v4 6/7] typec: tcpm: Represent source supply through power_supply class

From: Heikki Krogerus
Date: Tue Jan 30 2018 - 08:11:42 EST


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.

> 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.


Thanks,

--
heikki