Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

Peter Chen
2016-06-03T07:46:50Z

On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
> Several Intel platforms implement USB dual role by having completely
> separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share
> a single USB port. There is another external port mux which controls
> where the data lines should go. While the USB controllers are part of
> the silicon, the port mux design are platform specific.
>
> This patch adds the generic code to handle such multiple roles of a
> usb port. It exports the necessary interfaces for other components to
> register or unregister a usb mux device, and to control its role.
> It registers the mux device with sysfs as well, so that users are able
> to control the port role from user space.
>
> Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to
> swap usb roles as well. This code could also be leveraged for those archs.
>

Sorry to review this so late, from my point, it is a dual-role switch
driver too, we are reviewing USB OTG/dual-role framework [1], it is
not necessary to create another framework to do it. And USB OTG framework
has already tested at Renesas's platform [2].

[1] http://www.spinics.net/lists/linux-usb/msg140835.html
[2] http://www.spinics.net/lists/linux-usb/msg140827.html

Peter

> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Reviewed-by: Felipe Balbi <balbi@kernel.org>
> ---
> Documentation/ABI/testing/sysfs-bus-platform | 18 +++
> drivers/usb/Kconfig | 2 +
> drivers/usb/Makefile | 1 +
> drivers/usb/mux/Kconfig | 8 ++
> drivers/usb/mux/Makefile | 4 +
> drivers/usb/mux/portmux-core.c | 202 +++++++++++++++++++++++++++
> include/linux/usb/portmux.h | 90 ++++++++++++
> 7 files changed, 325 insertions(+)
> create mode 100644 drivers/usb/mux/Kconfig
> create mode 100644 drivers/usb/mux/Makefile
> create mode 100644 drivers/usb/mux/portmux-core.c
> create mode 100644 include/linux/usb/portmux.h
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform
> index 5172a61..b994e4e 100644
> --- a/Documentation/ABI/testing/sysfs-bus-platform
> +++ b/Documentation/ABI/testing/sysfs-bus-platform
> @@ -18,3 +18,21 @@ Description:
> devices to opt-out of driver binding using a driver_override
> name such as "none". Only a single driver may be specified in
> the override, there is no support for parsing delimiters.
> +
> +What: /sys/bus/platform/devices/.../portmux.N/name
> + /sys/bus/platform/devices/.../portmux.N/state
> +Date: April 2016
> +Contact: Lu Baolu <baolu.lu@linux.intel.com>
> +Description:
> + In some platforms, a single USB port is shared between a USB host
> + controller and a device controller. A USB mux driver is needed to
> + handle the port mux. Read-only attribute "name" shows the name of
> + the port mux device. "state" attribute shows and stores the mux
> + state.
> + For read:
> + 'unknown' - the mux hasn't been set yet;
> + 'peripheral' - mux has been switched to PERIPHERAL controller;
> + 'host' - mux has been switched to HOST controller.
> + For write:
> + 'peripheral' - mux will be switched to PERIPHERAL controller;
> + 'host' - mux will be switched to HOST controller.
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8689dcb..328916e 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -148,6 +148,8 @@ endif # USB
>
> source "drivers/usb/phy/Kconfig"
>
> +source "drivers/usb/mux/Kconfig"
> +
> source "drivers/usb/gadget/Kconfig"
>
> config USB_LED_TRIG
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index dca7856..9a92338 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -6,6 +6,7 @@
>
> obj-$(CONFIG_USB) += core/
> obj-$(CONFIG_USB_SUPPORT) += phy/
> +obj-$(CONFIG_USB_SUPPORT) += mux/
>
> obj-$(CONFIG_USB_DWC3) += dwc3/
> obj-$(CONFIG_USB_DWC2) += dwc2/
> diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
> new file mode 100644
> index 0000000..ba778f2
> --- /dev/null
> +++ b/drivers/usb/mux/Kconfig
> @@ -0,0 +1,8 @@
> +#
> +# USB port mux driver configuration
> +#
> +
> +menuconfig USB_PORTMUX
> + bool "USB dual role port MUX support"
> + help
> + Generic USB dual role port mux support.
> diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
> new file mode 100644
> index 0000000..f85df92
> --- /dev/null
> +++ b/drivers/usb/mux/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# Makefile for USB port mux drivers
> +#
> +obj-$(CONFIG_USB_PORTMUX) += portmux-core.o
> diff --git a/drivers/usb/mux/portmux-core.c b/drivers/usb/mux/portmux-core.c
> new file mode 100644
> index 0000000..75fbb45
> --- /dev/null
> +++ b/drivers/usb/mux/portmux-core.c
> @@ -0,0 +1,202 @@
> +/**
> + * portmux-core.c - USB Port Mux support
> + *
> + * Copyright (C) 2016 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@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/slab.h>
> +#include <linux/err.h>
> +#include <linux/usb/portmux.h>
> +
> +static int usb_mux_change_state(struct portmux_dev *pdev,
> + enum portmux_role role)
> +{
> + struct device *dev = &pdev->dev;
> + int ret = -EINVAL;
> +
> + dev_WARN_ONCE(dev,
> + !mutex_is_locked(&pdev->mux_mutex),
> + "mutex is unlocked\n");
> +
> + switch (role) {
> + case PORTMUX_HOST:
> + if (pdev->desc->ops->set_host_cb)
> + ret = pdev->desc->ops->set_host_cb(pdev->dev.parent);
> + break;
> + case PORTMUX_DEVICE:
> + if (pdev->desc->ops->set_device_cb)
> + ret = pdev->desc->ops->set_device_cb(pdev->dev.parent);
> + break;
> + default:
> + break;
> + }
> +
> + if (!ret)
> + pdev->mux_state = role;
> +
> + return ret;
> +}
> +
> +static const char * const role_name[] = {
> + "unknown", /* PORTMUX_UNKNOWN */
> + "host", /* PORTMUX_HOST */
> + "peripheral" /* PORTMUX_DEVICE */
> +};
> +
> +static ssize_t state_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct portmux_dev *pdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%s\n", role_name[pdev->mux_state]);
> +}
> +
> +static ssize_t state_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct portmux_dev *pdev = dev_get_drvdata(dev);
> + enum portmux_role role;
> +
> + if (sysfs_streq(buf, "peripheral"))
> + role = PORTMUX_DEVICE;
> + else if (sysfs_streq(buf, "host"))
> + role = PORTMUX_HOST;
> + else
> + return -EINVAL;
> +
> + mutex_lock(&pdev->mux_mutex);
> + usb_mux_change_state(pdev, role);
> + mutex_unlock(&pdev->mux_mutex);
> +
> + return count;
> +}
> +static DEVICE_ATTR_RW(state);
> +
> +static ssize_t name_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct portmux_dev *pdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%s\n", pdev->desc->name);
> +}
> +static DEVICE_ATTR_RO(name);
> +
> +static struct attribute *portmux_attrs[] = {
> + &dev_attr_state.attr,
> + &dev_attr_name.attr,
> + NULL,
> +};
> +
> +static struct attribute_group portmux_attr_grp = {
> + .attrs = portmux_attrs,
> +};
> +
> +static const struct attribute_group *portmux_group[] = {
> + &portmux_attr_grp,
> + NULL,
> +};
> +
> +static void portmux_dev_release(struct device *dev)
> +{
> + dev_vdbg(dev, "%s\n", __func__);
> +}
> +
> +/**
> + * portmux_register - register a port mux
> + * @dev: device the mux belongs to
> + * @desc: the descriptor of this port mux
> + *
> + * Called by port mux drivers to register a mux. Returns a valid
> + * pointer to struct portmux_dev on success or an ERR_PTR() on
> + * error.
> + */
> +struct portmux_dev *portmux_register(struct portmux_desc *desc)
> +{
> + static atomic_t portmux_no = ATOMIC_INIT(-1);
> + struct portmux_dev *pdev;
> + int ret;
> +
> + /* parameter sanity check */
> + if (!desc || !desc->name || !desc->ops || !desc->dev)
> + return ERR_PTR(-EINVAL);
> +
> + pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
> + if (!pdev)
> + return ERR_PTR(-ENOMEM);
> +
> + mutex_init(&pdev->mux_mutex);
> + pdev->desc = desc;
> + pdev->dev.parent = desc->dev;
> + pdev->dev.release = portmux_dev_release;
> + dev_set_name(&pdev->dev, "portmux.%lu",
> + (unsigned long)atomic_inc_return(&portmux_no));
> + pdev->dev.groups = portmux_group;
> + ret = device_register(&pdev->dev);
> + if (ret) {
> + kfree(pdev);
> + return ERR_PTR(ret);
> + }
> +
> + dev_set_drvdata(&pdev->dev, pdev);
> +
> + return pdev;
> +}
> +EXPORT_SYMBOL_GPL(portmux_register);
> +
> +/**
> + * portmux_unregister - unregister a port mux
> + * @pdev: the port mux device
> + *
> + * Called by port mux drivers to release a mux.
> + */
> +void portmux_unregister(struct portmux_dev *pdev)
> +{
> + device_unregister(&pdev->dev);
> + kfree(pdev);
> +}
> +EXPORT_SYMBOL_GPL(portmux_unregister);
> +
> +/**
> + * portmux_switch - switch the port role
> + * @pdev: the port mux device
> + * @role: the target role
> + *
> + * Called by other components to switch the port role.
> + */
> +int portmux_switch(struct portmux_dev *pdev, enum portmux_role role)
> +{
> + int ret;
> +
> + mutex_lock(&pdev->mux_mutex);
> + ret = usb_mux_change_state(pdev, role);
> + mutex_unlock(&pdev->mux_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(portmux_switch);
> +
> +#ifdef CONFIG_PM_SLEEP
> +/**
> + * portmux_complete - refresh port state during system resumes back
> + * @pdev: the port mux device
> + *
> + * Called by port mux drivers to refresh port state during system
> + * resumes back.
> + */
> +void portmux_complete(struct portmux_dev *pdev)
> +{
> + mutex_lock(&pdev->mux_mutex);
> + usb_mux_change_state(pdev, pdev->mux_state);
> + mutex_unlock(&pdev->mux_mutex);
> +}
> +EXPORT_SYMBOL_GPL(portmux_complete);
> +#endif
> diff --git a/include/linux/usb/portmux.h b/include/linux/usb/portmux.h
> new file mode 100644
> index 0000000..093620a
> --- /dev/null
> +++ b/include/linux/usb/portmux.h
> @@ -0,0 +1,90 @@
> +/**
> + * portmux.h - USB Port Mux definitions
> + *
> + * Copyright (C) 2016 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@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.
> + */
> +
> +#ifndef __LINUX_USB_PORTMUX_H
> +#define __LINUX_USB_PORTMUX_H
> +
> +#include <linux/device.h>
> +
> +/**
> + * struct portmux_ops - ops two switch the port
> + *
> + * @set_host_cb: callback for switching port to host
> + * @set_device_cb: callback for switching port to device
> + */
> +struct portmux_ops {
> + int (*set_host_cb)(struct device *dev);
> + int (*set_device_cb)(struct device *dev);
> +};
> +
> +/**
> + * struct portmux_desc - port mux device descriptor
> + *
> + * @name: the name of the mux device
> + * @dev: the parent of the mux device
> + * @ops: ops to switch the port role
> + */
> +struct portmux_desc {
> + const char *name;
> + struct device *dev;
> + const struct portmux_ops *ops;
> +};
> +
> +/**
> + * enum portmux_role - role of the port
> + */
> +enum portmux_role {
> + PORTMUX_UNKNOWN,
> + PORTMUX_HOST,
> + PORTMUX_DEVICE,
> +};
> +
> +/**
> + * struct portmux_dev - A mux device
> + *
> + * @desc: the descriptor of the mux
> + * @dev: device of this mux
> + * @mux_mutex: lock to serialize port switch operation
> + * @mux_state: state of the mux
> + */
> +struct portmux_dev {
> + const struct portmux_desc *desc;
> + struct device dev;
> +
> + /* lock for mux_state */
> + struct mutex mux_mutex;
> + enum portmux_role mux_state;
> +};
> +
> +/*
> + * Functions for mux driver
> + */
> +struct portmux_dev *portmux_register(struct portmux_desc *desc);
> +void portmux_unregister(struct portmux_dev *pdev);
> +#ifdef CONFIG_PM_SLEEP
> +void portmux_complete(struct portmux_dev *pdev);
> +#endif
> +
> +/*
> + * Functions for mux consumer
> + */
> +#if defined(CONFIG_USB_PORTMUX)
> +int portmux_switch(struct portmux_dev *pdev, enum portmux_role role);
> +#else
> +static inline int portmux_switch(struct portmux_dev *pdev,
> + enum portmux_role role)
> +{
> + return 0;
> +}
> +#endif
> +
> +#endif /* __LINUX_USB_PORTMUX_H */
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--

Best Regards,
Peter Chen