On Thu, Nov 17, 2016 at 12:50:35PM +0200, 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 the port supports
> USB Power Delivery, also control over power role swapping
> and Alternate Modes.
> 
> Signed-off-by: Heikki Krogerus <heikki.kroge...@linux.intel.com>
> ---
>  Documentation/ABI/testing/sysfs-class-typec |  222 ++++++
>  Documentation/usb/typec.txt                 |  103 +++
>  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                   | 1012 
> +++++++++++++++++++++++++++
>  include/linux/usb/typec.h                   |  252 +++++++
>  9 files changed, 1610 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..4fac77c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -0,0 +1,222 @@
> +USB Type-C port devices (eg. /sys/class/typec/usbc0/)
> +
> +What:                /sys/class/typec/<port>/current_data_role
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             The current USB data role the port is operating in. This
> +             attribute can be used for requesting data role swapping on the
> +             port. Swapping is supported as synchronous operation, so
> +             write(2) to the attribute will not return until the operation
> +             has finished. The attribute is notified about role changes so
> +             that poll(2) on the attribute wakes up. Change on the role will
> +             also generate uevent KOBJ_CHANGE on the port.
> +
> +             Valid values:
> +             - host
> +             - device
> +
> +What:                /sys/class/typec/<port>/current_power_role
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             The current power role of the port. This attribute can be used
> +             to request power role swap on the port when the port supports
> +             USB Power Delivery. Swapping is supported as synchronous
> +             operation, so write(2) to the attribute will not return until
> +             the operation has finished. The attribute is notified about role
> +             changes so that poll(2) on the attribute wakes up. Change on the
> +             role will also generate uevent KOBJ_CHANGE.
> +
> +             Valid values:
> +             - source
> +             - sink
> +
> +What:                /sys/class/typec/<port>/vconn_source
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Shows is the port VCONN Source. This attribute can be used to
> +             request VCONN swap to change the VCONN Source during connection
> +             when both the port and the partner support USB Power Delivery.
> +             Swapping is supported as synchronous operation, so write(2) to
> +             the attribute will not return until the operation has finished.
> +             The attribute is notified about VCONN source changes so that
> +             poll(2) on the attribute wakes up. Change on VCONN source also
> +             generates uevent KOBJ_CHANGE.
> +
> +             Valid values are:
> +             - 0 when the port is not the VCONN Source
> +             - 1 when the port is the VCONN Source
> +
> +What:                /sys/class/typec/<port>/power_operation_mode
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Shows the current power operational mode the port is in.
> +
> +             Valid values:
> +             - 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:                /sys/class/typec/<port>/preferred_role
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +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. By
> +             default there is no preferred role.
> +
> +             Valid values:
> +             - source
> +             - sink
> +             - none - to remove preference
> +
> +What:                /sys/class/typec/<port>/supported_accessory_modes
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Lists the Accessory Modes, defined in the USB Type-C
> +             specification, the port supports.
> +
> +What:                /sys/class/typec/<port>/supported_data_roles
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Lists the USB data roles the port is capable of supporting.
> +
> +             Valid values:
> +             - device
> +             - host
> +             - device, host (DRD as defined in USB Type-C specification v1.2)
> +
> +What:                /sys/class/typec/<port>/supported_power_roles
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Lists the power roles the port is capable of supporting.
> +
> +             Valid values:
> +             - source
> +             - sink
> +             - source, sink (DRP as defined in USB Type-C specification v1.2)
> +
> +What:                /sys/class/typec/<port>/supports_usb_power_delivery
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Shows if the port supports USB Power Delivery.
> +             - 1 if USB Power Delivery is supported
> +             - 0 when it's not
> +
> +
> +USB Type-C partner devices (eg. /sys/class/typec/usbc0-partner/)
> +
> +What:                /sys/class/typec/<port>-partner/accessory_mode
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Shows the Accessory Mode name, or "no" when the partner does not
> +             support Accesory Modes. The Accessory Modes are defined in USB
> +             Type-C Specification.
> +
> +What:                
> /sys/class/typec/<port>-partner/supports_usb_power_delivery
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Shows if the partner supports USB Power Delivery:
> +             - 0 when USB Power Delivery is not supported
> +             - 1 when USB Power Delivery is supported
> +
> +
> +USB Type-C cable devices (eg. /sys/class/typec/usbc0-cable/)
> +
> +Note: Electronically Marked Cables will have a device also for one cable plug
> +(eg. /sys/class/typec/usbc0-plug0). If the cable is active and has also SOP
> +Double Prime controller (USB Power Deliver specification ch. 2.4) it will 
> have
> +second device also for the other plug. Both plugs may have their alternate 
> modes
> +as described in USB Type-C and USB Power Delivery specifications.
> +
> +What:                /sys/class/typec/<port>-cable/active
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Shows if the cable is active or passive.
> +
> +             Valid values:
> +             - 0 when the cable is passive
> +             - 1 when the cable is active
> +
> +What:                /sys/class/typec/<port>-cable/plug_type
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +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
> +
> +What:                
> /sys/class/typec/<port>-cable/supports_usb_power_delivery
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Shows if the cable supports USB Power Delivery:
> +             - 0 when USB Power Delivery is not supported
> +             - 1 when USB Power Delivery is supported
> +
> +
> +Alternate Mode devices (For example,
> +/sys/class/typec/usbc0-partner/usbc0-partner.svid:xxxx/). The ports, partners
> +and cable plugs can have alternate modes.
> +
> +What:                /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/active
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +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. Entering/exiting modes is
> +             supported as synchronous operation so write(2) to the attribute
> +             does not return until the enter/exit mode operation has
> +             finished. The attribute is notified when the mode is
> +             entered/exited so poll(2) on the attribute wakes up.
> +             Entering/exiting a mode will also generate uevent KOBJ_CHANGE.
> +
> +             Valid values:
> +             - 0 when the mode is deactive
> +             - 1 when the mode is active
> +
> +What:                
> /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/description
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +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
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +Description:
> +             Shows the VDO in hexadecimal returned from the Discover Modes
> +             command.
> +
> +What:                
> /sys/class/typec/<port>/<port>.svid:<svid>/<mode>/supported_roles
> +Date:                December 2016
> +Contact:     Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +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..7095a2a
> --- /dev/null
> +++ b/Documentation/usb/typec.txt
> @@ -0,0 +1,103 @@
> +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
> +except the user space interface implementation in hope that it can be 
> utilized
> +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 offer 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.
> +
> +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". Full path to 
> the
> +device would be /sys/class/typec/usb0/usb0-partner/.
> +
> +The cable and the two plugs on it may also be optionally 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". The parent of a cable will always be the port, and 
> the
> +parent of the cable plugs will always be the cable.
> +
> +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 5e25ba1..baa2322 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12761,6 +12761,15 @@ F:   drivers/usb/
>  F:   include/linux/usb.h
>  F:   include/linux/usb/
>  
> +USB TYPEC SUBSYSTEM
> +M:   Heikki Krogerus <heikki.kroge...@linux.intel.com>
> +L:   linux-usb@vger.kernel.org
> +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 <st...@rowland.harvard.edu>
>  L:   linux-usb@vger.kernel.org
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index fbe493d..89c322e 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -152,6 +152,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 && LEDS_TRIGGERS
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7791af6..c7f4098 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -62,3 +62,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"
> +
> +config TYPEC
> +     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
> diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
> new file mode 100644
> index 0000000..e2909a6
> --- /dev/null
> +++ b/drivers/usb/typec/typec.c
> @@ -0,0 +1,1012 @@
> +/*
> + * USB Type-C Connector Class
> + *
> + * Copyright (C) 2016, Intel Corporation
> + * Author: Heikki Krogerus <heikki.kroge...@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/usb/typec.h>
> +
> +struct typec_port {
> +     unsigned int            id;
> +     struct device           dev;
> +
> +     int                     prefer_role;
> +
> +     enum typec_data_role    data_role;
> +     enum typec_role         pwr_role;
> +     enum typec_role         vconn_role;
> +     enum typec_pwr_opmode   pwr_opmode;
> +
> +     struct typec_partner    *partner;
> +     struct typec_cable      *cable;
> +
> +     unsigned int            connected:1;
> +
> +     const struct typec_capability *cap;
> +};
> +
> +#define to_typec_port(p) container_of(p, struct typec_port, dev)
> +#define to_typec_plug(p) container_of(p, struct typec_plug, dev)
> +#define to_typec_cable(p) container_of(p, struct typec_cable, dev)
> +#define to_typec_partner(p) container_of(p, struct typec_partner, dev)
> +
> +static DEFINE_IDA(typec_index_ida);
> +
> +static struct class typec_class = {
> +     .name = "typec",
> +};
> +
> +static const char * const typec_accessory_modes[] = {
> +     [TYPEC_ACCESSORY_NONE]  = "no",
> +     [TYPEC_ACCESSORY_AUDIO] = "Audio Adapter Accessory Mode",
> +     [TYPEC_ACCESSORY_DEBUG] = "Debug Accessory Mode",
> +};
> +
> +static struct device_type typec_partner_dev_type;
> +static struct device_type typec_cable_dev_type;
> +static struct device_type typec_port_dev_type;
> +
> +static ssize_t supports_usb_power_delivery_show(struct device *dev,
> +                                             struct device_attribute *attr,
> +                                             char *buf)
> +{
> +     if (dev->type == &typec_partner_dev_type) {
> +             struct typec_partner *p = to_typec_partner(dev);
> +
> +             return sprintf(buf, "%d\n", p->usb_pd);
> +     } else if (dev->type == &typec_partner_dev_type) {
> +             struct typec_cable *p = to_typec_cable(dev);
> +
> +             return sprintf(buf, "%d\n", p->usb_pd);
> +     } else {
> +             struct typec_port *p = to_typec_port(dev);
> +
> +             return sprintf(buf, "%d\n", p->cap->usb_pd);
> +     }
> +}
> +static DEVICE_ATTR_RO(supports_usb_power_delivery);
> +
> +/* ------------------------------------------------------------------------- 
> */
> +/* Type-C Partners */
> +
> +static ssize_t accessory_mode_show(struct device *dev,
> +                                struct device_attribute *attr,
> +                                char *buf)
> +{
> +     struct typec_partner *p = to_typec_partner(dev);
> +
> +     return sprintf(buf, "%s\n", typec_accessory_modes[p->accessory]);
> +}
> +static DEVICE_ATTR_RO(accessory_mode);
> +
> +static struct attribute *typec_partner_attrs[] = {
> +     &dev_attr_accessory_mode.attr,
> +     &dev_attr_supports_usb_power_delivery.attr,
> +     NULL
> +};
> +ATTRIBUTE_GROUPS(typec_partner);
> +
> +static void typec_partner_release(struct device *dev)
> +{
> +     struct typec_port *port = to_typec_port(dev->parent);
> +
> +     typec_unregister_altmodes(dev);
> +     port->partner = NULL;
> +}

This doesn't feel right, why are you both exporting
typec_unregister_altmodes() and also calling it from release callbacks?
That implies there is two way to clean stuff up, so what is a driver
writer to use?  Please simplify and force it to be one way or the other.

Also typec_unregister_altmodes() doesn't free memory, which release is
supposed to be doing, which also implies that the reference counting
logic is all wrong here.  Please fix, like I asked you to previously.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to