Message-ID: <20160603074113.GA30006@shlinux2>
Date:	Fri, 3 Jun 2016 15:41:13 +0800
From:	Peter Chen <hzpeterchen@...il.com>
To:	Lu Baolu <baolu.lu@...ux.intel.com>
Cc:	felipe.balbi@...ux.intel.com,
	Mathias Nyman <mathias.nyman@...el.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Lee Jones <lee.jones@...aro.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, Roger Quadros <rogerq@...com>
Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

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@...ux.intel.com>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> Reviewed-by: Felipe Balbi <balbi@...nel.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@...ux.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@...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/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@...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.
> + */
> +
> +#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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Best Regards,
Peter Chen

Powered by blists - more mailing lists