Re: [PATCHv3 1/2] usb: USB Type-C connector class

From: Roger Quadros
Date: Thu Jun 23 2016 - 07:54:57 EST

Next message: Arnaldo Carvalho de Melo: "Re: [PATCH v10 2/3] perf config: Reimplement perf_config() introducing new perf_config_init() and perf_config_finish()"
Previous message: Cyril Hrubis: "Re: [LTP] [patch V2 00/20] timer: Refactor the timer wheel"
In reply to: Heikki Krogerus: "Re: [PATCHv3 1/2] usb: USB Type-C connector class"
Next in thread: Heikki Krogerus: "Re: [PATCHv3 1/2] usb: USB Type-C connector class"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Heikki,

On 21/06/16 17:51, Heikki Krogerus wrote:
> The purpose of USB Type-C connector class is to provide
> unified interface for the user space to get the status and
> basic information about USB Type-C connectors on a system,
> control over data role swapping, and when USB PD is

s/PD/PD (Power Delivery)

> available, also control over power role swapping and
> Alternate Modes.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> Documentation/ABI/testing/sysfs-class-typec | 163 ++++
> Documentation/usb/typec.txt | 101 +++
> MAINTAINERS | 9 +
> drivers/usb/Kconfig | 2 +
> drivers/usb/Makefile | 2 +
> drivers/usb/typec/Kconfig | 7 +
> drivers/usb/typec/Makefile | 1 +
> drivers/usb/typec/typec.c | 1173 +++++++++++++++++++++++++++
> include/linux/usb/typec.h | 255 ++++++
> 9 files changed, 1713 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-typec
> create mode 100644 Documentation/usb/typec.txt
> create mode 100644 drivers/usb/typec/Kconfig
> create mode 100644 drivers/usb/typec/Makefile
> create mode 100644 drivers/usb/typec/typec.c
> create mode 100644 include/linux/usb/typec.h
>
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> new file mode 100644
> index 0000000..ce577f3
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -0,0 +1,163 @@
> +USB Type-C port devices (eg. /sys/class/typec/usbc0/)
> +
> +What: /sys/class/typec/<port>/current_data_role
> +Data: June 2016

s/Data/Date everywhere in this patch.

> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + The current USB data role the port is operating in. This
> + attribute can be used for requestion data role swapping on th

s/requestion/requesting

s/th/the

> + port.

Is it better to list all the values that can be here with a short note for each?
Same for all properties below?

Otherwise as a user I have no clue as to what all values I can set this to.

> +
> +What: /sys/class/typec/<port>/current_power_role
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + The current power role, source or sink, of the port. This
> + attribute can be used to request power role swap on the port
> + when USB Power Delivery is available.

So if it is not yet available, why add this now?

> +
> +What: /sys/class/typec/<port>/current_vconn_role
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows the current VCONN role, source or sink, of the port. This
> + attribute can be used to request vconn role swap on the port

s/vconn/VCONN

> + when USB Power Delivery is available.

Why add if not yet available?

> +
> +What: /sys/class/typec/<port>/power_operation_mode
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows the current power power mode the port is in. Reads as on

s/on/one

> + of the following:
> + - USB - Normal power levels defined in USB specifications
> + - BC1.2 - Power levels defined in Battery Charging Specification
> + v1.2
> + - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C
> + specification.
> + - USB Type-C 3.0A - Higher 3A current defined in USB Type-C
> + specification.
> + - USB Power Delivery - The voltages and currents defined in USB
> + Power Delivery specification

What would it show when the port is disconnected?

Instead, is it somehow possible to list the actual negotiated current (in mA)
here or in some other property, if you want to know the mode as well?

> +
> +What: /sys/class/typec/<port>/preferred_role
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + The user space can notify the driver about the preferred role.
> + It should be handled as enabling of Try.SRC or Try.SNK, as
> + defined in USB Type-C specification, in the port drivers.

What would be the default setting?

> +
> +What: /sys/class/typec/<port>/supported_accessory_modes
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Lists the Accessory Modes, defined in the USB Type-C
> + specification, the port supports.
> +
> +What: /sys/class/typec/<port>/supported_data_roles
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Lists the USB data roles, host or device, the port is capable
> + of supporting.

Can't it support both e.g in DRP case?

> +
> +What: /sys/class/typec/<port>/supported_power_roles
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Lists the power roles, source and/or sink, the port is capable
> + of supporting.
> +
> +What: /sys/class/typec/<port>/supports_usb_power_delivery
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows if the port support USB Power Delivery.

s/support/supports

> +
> +
> +USB Type-C partner devices (eg. /sys/class/typec/usbc0-partner/)
> +
> +What: /sys/class/typec/<port>-partner/accessory

<port>-partner/ or <port>/partner ?

> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows the name of the Accessory Mode if the partner attached to
> + the port is an accessory. Otherwise shows nothing.
> +
> +What: /sys/class/typec/<port>-partner/type
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows the type of the partner. Can be one of the following:
> + - USB - When the partner is normal USB host/peripheral.
> + - Charger - When the partner has been identified as dedicated
> + charger.
> + - Alternate Mode - When the partner supports Alternate Modes.
> + - Accessory - When the partner is one of the accessories with
> + specific Accessory Mode defined in USB Type-C
> + specification.
> +
> +
> +USB Type-C cable devices (eg. /sys/class/typec/usbc0-cable/)
> +
> +Note: The two cable plugs will have their own devices presenting when available

I didn't get this part. How come two cable plugs can have 2 devices?
presenting what?

> +(eg. /sys/class/typec/usbc0-plug0). Both plugs may have their own active
> +alternate modes as described in USB Type-C and USB Power Delivery
> +specifications.
> +
> +What: /sys/class/typec/<port>-cable/active

<port>-cable/ or <port>/cable ?

> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows if the cable is active or passive. Only active cables can
> + have configurable alternate modes, therefore only active cables
> + will have plug devices.
> +
> +What: /sys/class/typec/<port>-cable/plug_type
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows type of the plug on the cable:
> + Type-A - Standard A
> + Type-B - Standard B
> + Type-C - USB Type-C
> + Captive - Non-standard
> +
> +
> +Alternate Mode devices (For example,
> +/sys/class/typec/usbc0-partner/usbc0-partner.svid:xxxx/)

Why should it show usbc0-partner twice?

> +
> +What: /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/active
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows if the mode is active or not. The attribute can be used
> + for entering/exiting the mode with partners and cable plugs, and
> + with the port alternate modes it can be used for disabling
> + support for specific alternate modes.
> +
> +What: /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/description
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows description of the mode. The description is optional for
> + the drivers, just like with the Billboard Devices.
> +
> +What: /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/vdo
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows the VDO in hexadecimal returned from the Discover Modes
> + command.
> +
> +What: /sys/class/typec/<port>/<port>.svid:<svid>/<mode>/supported_roles
> +Data: June 2016
> +Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +Description:
> + Shows the roles, source or sink, the mode is supported with.
> +
> + This attribute is available for the devices describing the
> + alternate modes a port supports, and it will not be exposed with
> + the devices presenting the alternate modes the partners or cable
> + plugs support.
> diff --git a/Documentation/usb/typec.txt b/Documentation/usb/typec.txt
> new file mode 100644
> index 0000000..ddeda97
> --- /dev/null
> +++ b/Documentation/usb/typec.txt
> @@ -0,0 +1,101 @@
> +USB Type-C connector class
> +==========================
> +
> +Introduction
> +------------
> +The typec class is meant for describing the USB Type-C ports in a system to the
> +user space in unified fashion. The class is designed to provide nothing else
> +expect the user space interface implementation in hope that it can be utilized

s/expect/except/

> +on as many platforms as possible.
> +
> +The platforms are expected to register every USB Type-C port they have with the
> +class. In a normal case the registration will be done by a USB Type-C or PD PHY
> +driver, but it may be a driver for firmware interface such as UCSI, driver for
> +USB PD controller or even driver for Thunderbolt3 controller. This document
> +considers the component registering the USB Type-C ports with the class as "port
> +driver".
> +
> +On top of showing the capabilities, the class also offers the user space control
> +over the roles and alternate modes they support when the port driver is capable
> +of supporting those features.
> +
> +The class provides an API for the port drivers described in this document. The
> +attributes are described in Documentation/ABI/testing/sysfs-class-typec.
> +
> +
> +Interface
> +---------
> +Every port will be presented as its own device under /sys/class/typec/. The
> +first port will be named "usbc0", the second "usbc1" and so on.

We already know it is typec since it sits in /sys/class/typec

Why not just call the instances "0", "1" and so on?

> +
> +When connected, the partner will be presented also as its own device under
> +/sys/class/typec/. The parent of the partner device will always be the port. The
> +partner attached to port "usbc0" will be named "usbc0-partner".

Can the partner exist without the port? If not the partner should be located within
the port directory.

> +
> +The cable and the two plugs on it may also be optionally be presented as their
> +own devices under /sys/class/typec/. The cable attached to the port "usbc0" port
> +will be named usbc0-cable and the plug on the SOP Prime end (see USB Power
> +Delivery Specification ch. 2.4) will be named "usbc-plug0" and on the SOP Double
> +Prime end "usbc0-plug1".

If cables and plugs can't exist without the port they must come up within the port
directory.

> +
> +If the port, partner or cable plug support Alternate Modes, every Alternate Mode
> +SVID will have their own device describing them. The Alternate Modes will not be
> +attached to the typec class. For the port's "usbc0" partner, the Alternate Modes
> +would have devices presented under /sys/class/typec/usbc0-partner/. Every mode
> +that is supported will have its own group under the Alternate Mode device named
> +"mode<id>". For example /sys/class/typec/usbc0/usbc0.svid:xxxx/mode0/. The
> +requests for entering/exiting the modes happens with the "active" attribute in
> +that group.
> +
> +
> +API
> +---
> +
> +* Registering the ports
> +
> +The port drivers will describe every Type-C port they control with struct
> +typec_capability data structure, and register them with the following API:
> +
> +struct typec_port *typec_register_port(struct device *dev,
> + const struct typec_capability *cap);
> +
> +The class will provide handle to struct typec_port on success and ERR_PTR on
> +failure. The un-registration of the port happens with the following API:
> +
> +void typec_unregister_port(struct typec_port *port);
> +
> +
> +* Notifications
> +
> +When connection happens on a port, the port driver fills struct typec_connection
> +which is passed to the class. The class provides the following API for reporting
> +connection/disconnection:
> +
> +int typec_connect(struct typec_port *port, struct typec_connection *);
> +void typec_disconnect(struct typec_port *);
> +
> +When the partner end has executed a role change, the port driver uses the
> +following APIs to report it to the class:
> +
> +void typec_set_data_role(struct typec_port *, enum typec_data_role);
> +void typec_set_pwr_role(struct typec_port *, enum typec_role);
> +void typec_set_vconn_role(struct typec_port *, enum typec_role);
> +void typec_set_pwr_opmode(struct typec_port *, enum typec_pwr_opmode);
> +
> +
> +* Alternate Modes
> +
> +After connection, the port drivers register the alternate modes the partner
> +and/or cable plugs support. And before reporting disconnection, the port driver
> +_must_ unregister all the alternate modes registered for the partner and cable
> +plugs. The API takes the struct device of the partner or the cable plug as
> +parameter:
> +
> +int typec_register_altmodes(struct device *, struct typec_altmode *);
> +void typec_unregister_altmodes(struct device *);
> +
> +When the partner end enters or exits the modes, the port driver needs to notify
> +the class with the following API:
> +
> +void typec_altmode_update_active(struct typec_altmode *alt, int mode,
> + bool active);
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e1b090f..0db33fb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11983,6 +11983,15 @@ F: drivers/usb/
> F: include/linux/usb.h
> F: include/linux/usb/
>
> +USB TYPEC SUBSYSTEM
> +M: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> +L: linux-usb@xxxxxxxxxxxxxxx
> +S: Maintained
> +F: Documentation/ABI/testing/sysfs-class-typec
> +F: Documentation/usb/typec.txt
> +F: drivers/usb/typec/
> +F: include/linux/usb/typec.h
> +
> USB UHCI DRIVER
> M: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> L: linux-usb@xxxxxxxxxxxxxxx
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8689dcb..f42a3d3 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -150,6 +150,8 @@ source "drivers/usb/phy/Kconfig"
>
> source "drivers/usb/gadget/Kconfig"
>
> +source "drivers/usb/typec/Kconfig"
> +
> config USB_LED_TRIG
> bool "USB LED Triggers"
> depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index dca7856..51e381e 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -61,3 +61,5 @@ obj-$(CONFIG_USB_GADGET) += gadget/
> obj-$(CONFIG_USB_COMMON) += common/
>
> obj-$(CONFIG_USBIP_CORE) += usbip/
> +
> +obj-$(CONFIG_TYPEC) += typec/
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> new file mode 100644
> index 0000000..b229fb9
> --- /dev/null
> +++ b/drivers/usb/typec/Kconfig
> @@ -0,0 +1,7 @@
> +
> +menu "USB PD and Type-C drivers"

USB PD is independent of Type C right? Why club them together here?

> +
> +config TYPEC

No help description?

> + tristate
> +
> +endmenu
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> new file mode 100644
> index 0000000..1012a8b
> --- /dev/null
> +++ b/drivers/usb/typec/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_TYPEC) += typec.o

<snip>

I'll review the code a bit later.

cheers,
-roger

Next message: Arnaldo Carvalho de Melo: "Re: [PATCH v10 2/3] perf config: Reimplement perf_config() introducing new perf_config_init() and perf_config_finish()"
Previous message: Cyril Hrubis: "Re: [LTP] [patch V2 00/20] timer: Refactor the timer wheel"
In reply to: Heikki Krogerus: "Re: [PATCHv3 1/2] usb: USB Type-C connector class"
Next in thread: Heikki Krogerus: "Re: [PATCHv3 1/2] usb: USB Type-C connector class"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]