[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1455101342.8878.5.camel@suse.com>
Date: Wed, 10 Feb 2016 11:49:02 +0100
From: Oliver Neukum <oneukum@...e.com>
To: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc: Greg KH <gregkh@...uxfoundation.org>,
Felipe Balbi <balbi@...nel.org>,
Mathias Nyman <mathias.nyman@...ux.intel.com>,
linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH 1/3] usb: USB Type-C Connector Class
On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote:
> The purpose of this class is to provide unified interface
> for user space to get the status and basic information about
> USB Type-C Connectors in the system, control data role
> swapping, and when USB PD is available, also power role
> swapping and Altenate Modes.
>
> The class will export the following interfaces for every
> USB Type-C Connector in the system to sysfs:
>
> 1. connected - Connection status of the connector
> 2. alternate_mode - The current Alternate Mode
> 3. alternate_modes - Lists all Alternate Modes the connector supports
These names are a bit problematic, as they are too similar.
How about
current_alternate_mode
potential_alternate_modes
> 4. partner_alt_modes - Lists partner's Alternate Modes when connected
> 5. partner_type - Can be USB, Charger, Alt Mode or Accessory
> 6. data_role - The current data role, host or device
> 7. data_roles - Data roles supported by the connector
> 8. power_role - Connector's current power role, source or sink
> 9. power_roles - Power roles supported by the connector
> 10. power_operation_mode - The current power level in use
> 11. usb_pd - yes if the connector supports USB PD.
> 12. audio_accessory - yes if the connector supports Audio Accessory
> 13. debug_accessory - yes if the connector supports Debug Accessory
>
> The data_role, power_role and alternate_mode are also
> writable and can be used for executing role swapping and
> entering modes. When USB PD is not supported by the
> connector or partner, power_role will reflect the value of
> the data_role, and is not swappable independently.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> ---
> drivers/usb/Kconfig | 2 +
> drivers/usb/Makefile | 2 +
> drivers/usb/type-c/Kconfig | 7 +
> drivers/usb/type-c/Makefile | 1 +
> drivers/usb/type-c/typec.c | 446 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/usb/typec.h | 114 +++++++++++
> 6 files changed, 572 insertions(+)
> create mode 100644 drivers/usb/type-c/Kconfig
> create mode 100644 drivers/usb/type-c/Makefile
> create mode 100644 drivers/usb/type-c/typec.c
> create mode 100644 include/linux/usb/typec.h
>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8ed451d..0c45547 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -151,6 +151,8 @@ source "drivers/usb/phy/Kconfig"
>
> source "drivers/usb/gadget/Kconfig"
>
> +source "drivers/usb/type-c/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 d5c57f1..4d712ee 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) += type-c/
> diff --git a/drivers/usb/type-c/Kconfig b/drivers/usb/type-c/Kconfig
> new file mode 100644
> index 0000000..b229fb9
> --- /dev/null
> +++ b/drivers/usb/type-c/Kconfig
> @@ -0,0 +1,7 @@
> +
> +menu "USB PD and Type-C drivers"
> +
> +config TYPEC
> + tristate
> +
> +endmenu
> diff --git a/drivers/usb/type-c/Makefile b/drivers/usb/type-c/Makefile
> new file mode 100644
> index 0000000..1012a8b
> --- /dev/null
> +++ b/drivers/usb/type-c/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_TYPEC) += typec.o
> diff --git a/drivers/usb/type-c/typec.c b/drivers/usb/type-c/typec.c
> new file mode 100644
> index 0000000..e425955
> --- /dev/null
> +++ b/drivers/usb/type-c/typec.c
> @@ -0,0 +1,446 @@
> +/*
> + * USB Type-C class
> + *
> + * Copyright (C) 2016, Intel Corporation
> + * Author: Heikki Krogerus <heikki.krogerus@...ux.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>
> +
> +#define to_typec_port(p) container_of(p, struct typec_port, dev)
> +
> +static DEFINE_IDA(typec_index_ida);
> +
> +/* -------------------------------- */
> +
> +int typec_connect(struct typec_port *port)
> +{
> + kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(typec_connect);
> +
> +void typec_disconnect(struct typec_port *port)
> +{
> + kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
> +}
> +EXPORT_SYMBOL_GPL(typec_disconnect);
> +
> +/* -------------------------------- */
> +
> +static ssize_t alternate_mode_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct typec_port *port = to_typec_port(dev);
> + struct typec_alt_mode alt_mode;
> + int ret;
> +
> + if (!port->cap->set_alt_mode) {
> + dev_warn(dev, "entering Alternate Modes not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (!port->connected)
> + return -ENXIO;
Doesn't this need locking?
And why wouldn't user space want to preselect a mode?
Regards
Oliver
Powered by blists - more mailing lists