Re: [PATCH v5 3/6] usb: mux: add common code for Intel dual role port mux

From: Chanwoo Choi
Date: Fri Mar 18 2016 - 03:52:46 EST


Hi Baolu,

On 2016ë 03ì 18ì 15:32, Lu Baolu wrote:
> Several Intel PCHs and SOCs have an internal mux that is used to
> share one USB port between device controller and host controller.
>
> A usb port mux could be abstracted as the following elements:
> 1) mux state: HOST or PERIPHERAL;
> 2) an extcon cable which triggers the change of mux state between
> HOST and PERIPHERAL;
> 3) The required action to do the real port switch.
>
> This patch adds the common code to handle usb port mux. With this
> common code, the individual mux driver, which always is platform
> dependent, could focus on the real operation of mux switch.
>
> Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Reviewed-by: Felipe Balbi <balbi@xxxxxxxxxx>

Looks good to me about extcon usage.

For extcon usage,
Reviewed-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>

Best Regards,
Chanwoo Choi


> ---
> Documentation/ABI/testing/sysfs-bus-platform | 15 +++
> MAINTAINERS | 7 ++
> drivers/usb/Kconfig | 2 +
> drivers/usb/Makefile | 1 +
> drivers/usb/mux/Kconfig | 12 ++
> drivers/usb/mux/Makefile | 4 +
> drivers/usb/mux/intel-mux.c | 175 +++++++++++++++++++++++++++
> include/linux/usb/intel-mux.h | 40 ++++++
> 8 files changed, 256 insertions(+)
> create mode 100644 drivers/usb/mux/Kconfig
> create mode 100644 drivers/usb/mux/Makefile
> create mode 100644 drivers/usb/mux/intel-mux.c
> create mode 100644 include/linux/usb/intel-mux.h
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform
> index 5172a61..23bf76e 100644
> --- a/Documentation/ABI/testing/sysfs-bus-platform
> +++ b/Documentation/ABI/testing/sysfs-bus-platform
> @@ -18,3 +18,18 @@ 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/.../port_mux
> +Date: Febuary 2016
> +Contact: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> +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. port_mux attribute shows and stores the mux
> + state.
> + For read:
> + 'peripheral' - mux switched to PERIPHERAL controller;
> + 'host' - mux 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/MAINTAINERS b/MAINTAINERS
> index da3e4d8..0dbee11 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11399,6 +11399,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
> S: Maintained
> F: drivers/usb/phy/
>
> +USB PORT MUX DRIVER
> +M: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> +L: linux-usb@xxxxxxxxxxxxxxx
> +S: Supported
> +F: include/linux/usb/intel-mux.h
> +F: drivers/usb/mux/intel-mux.c
> +
> USB PRINTER DRIVER (usblp)
> M: Pete Zaitcev <zaitcev@xxxxxxxxxx>
> L: linux-usb@xxxxxxxxxxxxxxx
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8ed451d..dbd6620 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -149,6 +149,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 d5c57f1..6433f0c 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..62e2cc3
> --- /dev/null
> +++ b/drivers/usb/mux/Kconfig
> @@ -0,0 +1,12 @@
> +#
> +# USB port mux driver configuration
> +#
> +menu "USB Port MUX drivers"
> +config INTEL_USB_MUX
> + select EXTCON
> + def_bool n
> + help
> + Common code for all Intel dual role port mux drivers. All Intel
> + usb port mux drivers should select it.
> +
> +endmenu
> diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
> new file mode 100644
> index 0000000..84f0ae8
> --- /dev/null
> +++ b/drivers/usb/mux/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# Makefile for USB port mux drivers
> +#
> +obj-$(CONFIG_INTEL_USB_MUX) += intel-mux.o
> diff --git a/drivers/usb/mux/intel-mux.c b/drivers/usb/mux/intel-mux.c
> new file mode 100644
> index 0000000..d18b96f
> --- /dev/null
> +++ b/drivers/usb/mux/intel-mux.c
> @@ -0,0 +1,175 @@
> +/**
> + * intel_mux.c - USB Port Mux support
> + *
> + * Copyright (C) 2016 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> + *
> + * 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/notifier.h>
> +#include <linux/extcon.h>
> +#include <linux/err.h>
> +
> +struct intel_usb_mux {
> + struct device *dev;
> + struct extcon_dev *edev;
> + int (*cable_set_cb)(struct device *dev);
> + int (*cable_unset_cb)(struct device *dev);
> +
> + struct notifier_block nb;
> +
> + /*
> + * The state of the mux.
> + * 0, 1 - mux switch state
> + * -1 - uninitialized state
> + */
> + int mux_state;
> +
> + /* lock for mux_state */
> + struct mutex mux_mutex;
> +};
> +
> +static int usb_mux_change_state(struct intel_usb_mux *mux, int state)
> +{
> + int ret;
> + struct device *dev = mux->dev;
> +
> + dev_WARN_ONCE(dev,
> + !mutex_is_locked(&mux->mux_mutex),
> + "mutex is unlocked\n");
> +
> + mux->mux_state = state;
> +
> + if (mux->mux_state)
> + ret = mux->cable_set_cb(dev);
> + else
> + ret = mux->cable_unset_cb(dev);
> +
> + return ret;
> +}
> +
> +static int usb_mux_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct intel_usb_mux *mux;
> + int state;
> + int ret = NOTIFY_DONE;
> +
> + mux = container_of(nb, struct intel_usb_mux, nb);
> +
> + state = extcon_get_cable_state_(mux->edev, EXTCON_USB_HOST);
> +
> + if (mux->mux_state == -1 || mux->mux_state != state) {
> + mutex_lock(&mux->mux_mutex);
> + ret = usb_mux_change_state(mux, state);
> + mutex_unlock(&mux->mux_mutex);
> + }
> +
> + return ret;
> +}
> +
> +static ssize_t port_mux_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct intel_usb_mux *mux = dev_get_drvdata(dev);
> +
> + if (dev_WARN_ONCE(dev, !mux, "mux without data structure\n"))
> + return 0;
> +
> + return sprintf(buf, "%s\n", mux->mux_state ? "host" : "peripheral");
> +}
> +
> +static ssize_t port_mux_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct intel_usb_mux *mux = dev_get_drvdata(dev);
> + int state;
> +
> + if (dev_WARN_ONCE(dev, !mux, "mux without data structure\n"))
> + return -EINVAL;
> +
> + if (sysfs_streq(buf, "peripheral"))
> + state = 0;
> + else if (sysfs_streq(buf, "host"))
> + state = 1;
> + else
> + return -EINVAL;
> +
> + mutex_lock(&mux->mux_mutex);
> + usb_mux_change_state(mux, state);
> + mutex_unlock(&mux->mux_mutex);
> +
> + return count;
> +}
> +static DEVICE_ATTR_RW(port_mux);
> +
> +int intel_usb_mux_bind_cable(struct device *dev, const char *extcon_name,
> + int (*cable_set_cb)(struct device *dev),
> + int (*cable_unset_cb)(struct device *dev))
> +{
> + int ret;
> + struct intel_usb_mux *mux;
> + struct extcon_dev *edev;
> +
> + edev = extcon_get_extcon_dev(extcon_name);
> + if (IS_ERR_OR_NULL(edev))
> + return -ENODEV;
> +
> + mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
> + if (!mux)
> + return -ENOMEM;
> +
> + mux->dev = dev;
> + mux->edev = edev;
> + mux->cable_set_cb = cable_set_cb;
> + mux->cable_unset_cb = cable_unset_cb;
> + mux->nb.notifier_call = usb_mux_notifier;
> + mutex_init(&mux->mux_mutex);
> + mux->mux_state = -1;
> +
> + dev_set_drvdata(dev, mux);
> + ret = extcon_register_notifier(edev, EXTCON_USB_HOST, &mux->nb);
> + if (ret < 0) {
> + dev_err(dev, "failed to register extcon notifier\n");
> + return -ENODEV;
> + }
> +
> + usb_mux_notifier(&mux->nb, 0, NULL);
> +
> + /* register the sysfs interface */
> + ret = device_create_file(dev, &dev_attr_port_mux);
> + if (ret) {
> + extcon_unregister_notifier(edev, EXTCON_USB_HOST, &mux->nb);
> + dev_err(dev, "failed to create sysfs attribute\n");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_usb_mux_bind_cable);
> +
> +int intel_usb_mux_unbind_cable(struct device *dev)
> +{
> + struct intel_usb_mux *mux = dev_get_drvdata(dev);
> +
> + device_remove_file(dev, &dev_attr_port_mux);
> + extcon_unregister_notifier(mux->edev, EXTCON_USB_HOST, &mux->nb);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_usb_mux_unbind_cable);
> +
> +#ifdef CONFIG_PM_SLEEP
> +void intel_usb_mux_complete(struct device *dev)
> +{
> + struct intel_usb_mux *mux = dev_get_drvdata(dev);
> +
> + usb_mux_notifier(&mux->nb, 0, NULL);
> +}
> +EXPORT_SYMBOL_GPL(intel_usb_mux_complete);
> +#endif
> diff --git a/include/linux/usb/intel-mux.h b/include/linux/usb/intel-mux.h
> new file mode 100644
> index 0000000..b00975a
> --- /dev/null
> +++ b/include/linux/usb/intel-mux.h
> @@ -0,0 +1,40 @@
> +/**
> + * intel_mux.h - USB Port Mux definitions
> + *
> + * Copyright (C) 2016 Intel Corporation
> + *
> + * Author: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> + *
> + * 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_INTEL_MUX_H
> +#define __LINUX_USB_INTEL_MUX_H
> +
> +#if IS_ENABLED(CONFIG_INTEL_USB_MUX)
> +int intel_usb_mux_bind_cable(struct device *dev, const char *extcon_name,
> + int (*cable_set_cb)(struct device *dev),
> + int (*cable_unset_cb)(struct device *dev));
> +int intel_usb_mux_unbind_cable(struct device *dev);
> +#ifdef CONFIG_PM_SLEEP
> +void intel_usb_mux_complete(struct device *dev);
> +#endif
> +
> +#else
> +static inline int
> +int intel_usb_mux_bind_cable(struct device *dev, const char *extcon_name,
> + int (*cable_set_cb)(struct device *dev),
> + int (*cable_unset_cb)(struct device *dev))
> +{
> + return -ENODEV;
> +}
> +
> +static inline int intel_usb_mux_unbind_cable(struct device *dev)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_INTEL_USB_MUX */
> +
> +#endif /* __LINUX_USB_INTEL_MUX_H */
>