Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1348813 > unrolled thread

[PATCH 0/7] usb: add support for Intel dual role port mux

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] usb: add support for Intel dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-03-03 07:40 +0100
    [PATCH 2/7] extcon: usb-gpio: add support for ACPI gpio interface Lu Baolu <baolu.lu@linux.intel.com> - 2016-03-03 07:40 +0100
      Re: [PATCH 2/7] extcon: usb-gpio: add support for ACPI gpio interface Chanwoo Choi <cw00.choi@samsung.com> - 2016-03-03 08:30 +0100
        Re: [PATCH 2/7] extcon: usb-gpio: add support for ACPI gpio interface Lu Baolu <baolu.lu@linux.intel.com> - 2016-03-03 08:40 +0100
    [PATCH 3/7] usb: misc: add common code for Intel dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-03-03 07:40 +0100
      Re: [PATCH 3/7] usb: misc: add common code for Intel dual role port  mux Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-03-03 17:20 +0100
        Re: [PATCH 3/7] usb: misc: add common code for Intel dual role port  mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-03-04 02:20 +0100
      Re: [PATCH 3/7] usb: misc: add common code for Intel dual role port  mux Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-03-03 17:20 +0100
        Re: [PATCH 3/7] usb: misc: add common code for Intel dual role port  mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-03-04 02:30 +0100
    [PATCH 7/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver Lu Baolu <baolu.lu@linux.intel.com> - 2016-03-03 07:40 +0100
      [PATCH] mfd: intel_vuport: fix platform_no_drv_owner.cocci warnings kbuild test robot <lkp@intel.com> - 2016-03-03 09:20 +0100
      Re: [PATCH 7/7] mfd: intel_vuport: Add Intel virtual USB port MFD  Driver kbuild test robot <lkp@intel.com> - 2016-03-03 09:20 +0100
    [PATCH 1/7] extcon: usb-gpio: add device binding for platform device Lu Baolu <baolu.lu@linux.intel.com> - 2016-03-03 07:40 +0100
      Re: [PATCH 1/7] extcon: usb-gpio: add device binding for platform  device Chanwoo Choi <cw00.choi@samsung.com> - 2016-03-03 08:30 +0100
        Re: [PATCH 1/7] extcon: usb-gpio: add device binding for platform  device Lu Baolu <baolu.lu@linux.intel.com> - 2016-03-03 08:40 +0100

#1348813 — [PATCH 0/7] usb: add support for Intel dual role port mux

Intel SOC chips are featured with USB dual role. The host role is
provided by Intel xHCI IP, and the gadget role is provided by IP
from designware. Tablet platform designs always share a single
port for both host and gadget controllers. There is a mux to
switch the port to the right controller according to the cable
type. OS needs to provide the callback to control the mux when
a plug-in event raises. The method to control the mux is platform
dependent. At least three types of implementation can be found
across current devices. 1) GPIO pins; 2) a unit which can be
controlled by memory mapped registers; 3) ACPI ASL code.

This patch series adds supports for Intel dual role port mux.
It includes:
(1) A helper layer on top of extcon for individual mux driver.
    It listens to the USB-HOST extcon cable and call the switch
    call-back when the cable state changes.
(2) Drivers for GPIO controlled port mux which could be found on
    Baytrail devices. A mfd driver is used to split the GPIOs into
    USB gpio extcon device and a USB mux device. Driver for USB
    gpio extcon device is already in upstream Linux. This patch
    series includes a driver for GPIO USB mux.
(3) Drivers for USB port mux controlled through memory mapped
    registers and the logic to create the mux device. This type
    of dual role port mux could be found in Cherry Trail and
    Broxton devices.

Lu Baolu (7):
  extcon: usb-gpio: add device binding for platform device
  extcon: usb-gpio: add support for ACPI gpio interface
  usb: misc: add common code for Intel dual role port mux
  usb: misc: add driver for Intel gpio controlled port mux
  usb: misc: add driver for Intel drcfg controlled port mux
  usb: pci-quirks: add Intel USB drcfg mux device
  mfd: intel_vuport: Add Intel virtual USB port MFD Driver

 MAINTAINERS                        |  10 +++
 drivers/extcon/extcon-usb-gpio.c   |  10 ++-
 drivers/mfd/Kconfig                |   7 ++
 drivers/mfd/Makefile               |   1 +
 drivers/mfd/intel-vuport.c         |  79 +++++++++++++++++
 drivers/usb/host/pci-quirks.c      |  47 +++++++++-
 drivers/usb/host/xhci-ext-caps.h   |   2 +
 drivers/usb/misc/Kconfig           |  20 +++++
 drivers/usb/misc/Makefile          |   4 +
 drivers/usb/misc/intel-mux-drcfg.c | 174 +++++++++++++++++++++++++++++++++++++
 drivers/usb/misc/intel-mux-gpio.c  | 126 +++++++++++++++++++++++++++
 drivers/usb/misc/mux.c             | 172 ++++++++++++++++++++++++++++++++++++
 include/linux/usb/mux.h            |  71 +++++++++++++++
 13 files changed, 720 insertions(+), 3 deletions(-)
 create mode 100644 drivers/mfd/intel-vuport.c
 create mode 100644 drivers/usb/misc/intel-mux-drcfg.c
 create mode 100644 drivers/usb/misc/intel-mux-gpio.c
 create mode 100644 drivers/usb/misc/mux.c
 create mode 100644 include/linux/usb/mux.h

-- 
2.1.4

[toc] | [next] | [standalone]

#1348814 — [PATCH 2/7] extcon: usb-gpio: add support for ACPI gpio interface

GPIO resource could be retrieved through APCI as well.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
---
 drivers/extcon/extcon-usb-gpio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index af9c8b0..472c431 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -26,6 +26,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
+#include <linux/acpi.h>
 
 #define USB_GPIO_DEBOUNCE_MS	20	/* ms */
 
@@ -91,7 +92,7 @@ static int usb_extcon_probe(struct platform_device *pdev)
 	struct usb_extcon_info *info;
 	int ret;
 
-	if (!np)
+	if (!np && !ACPI_HANDLE(dev))
 		return -EINVAL;
 
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1348827 — Re: [PATCH 2/7] extcon: usb-gpio: add support for ACPI gpio interface

Hi Lu,

On 2016년 03월 03일 15:37, Lu Baolu wrote:
> GPIO resource could be retrieved through APCI as well.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Felipe Balbi <balbi@kernel.org>
> ---
>  drivers/extcon/extcon-usb-gpio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
> index af9c8b0..472c431 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -26,6 +26,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/workqueue.h>
> +#include <linux/acpi.h>
>  
>  #define USB_GPIO_DEBOUNCE_MS	20	/* ms */
>  
> @@ -91,7 +92,7 @@ static int usb_extcon_probe(struct platform_device *pdev)
>  	struct usb_extcon_info *info;
>  	int ret;
>  
> -	if (!np)
> +	if (!np && !ACPI_HANDLE(dev))
>  		return -EINVAL;
>  
>  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> 

Looks good to me.

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Best Regards,
Chanwoo Choi

[toc] | [prev] | [next] | [standalone]

#1348834 — Re: [PATCH 2/7] extcon: usb-gpio: add support for ACPI gpio interface

On 03/03/2016 03:24 PM, Chanwoo Choi wrote:
> Hi Lu,
>
> On 2016년 03월 03일 15:37, Lu Baolu wrote:
>> GPIO resource could be retrieved through APCI as well.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> Reviewed-by: Felipe Balbi <balbi@kernel.org>
>> ---
>>  drivers/extcon/extcon-usb-gpio.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
>> index af9c8b0..472c431 100644
>> --- a/drivers/extcon/extcon-usb-gpio.c
>> +++ b/drivers/extcon/extcon-usb-gpio.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/slab.h>
>>  #include <linux/workqueue.h>
>> +#include <linux/acpi.h>
>>  
>>  #define USB_GPIO_DEBOUNCE_MS	20	/* ms */
>>  
>> @@ -91,7 +92,7 @@ static int usb_extcon_probe(struct platform_device *pdev)
>>  	struct usb_extcon_info *info;
>>  	int ret;
>>  
>> -	if (!np)
>> +	if (!np && !ACPI_HANDLE(dev))
>>  		return -EINVAL;
>>  
>>  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>>
> Looks good to me.
>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Thank you!

>
> Best Regards,
> Chanwoo Choi
>

[toc] | [prev] | [next] | [standalone]

#1348815 — [PATCH 3/7] usb: misc: add common code for Intel dual role port mux

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@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
---
 MAINTAINERS               |   7 ++
 drivers/usb/misc/Kconfig  |   4 ++
 drivers/usb/misc/Makefile |   2 +
 drivers/usb/misc/mux.c    | 172 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/mux.h   |  71 +++++++++++++++++++
 5 files changed, 256 insertions(+)
 create mode 100644 drivers/usb/misc/mux.c
 create mode 100644 include/linux/usb/mux.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d894ee2..45f1e1e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11389,6 +11389,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@linux.intel.com>
+L:	linux-usb@vger.kernel.org
+S:	Supported
+F:	drivers/usb/misc/mux.c
+F:	include/linux/usb/mux.h
+
 USB PRINTER DRIVER (usblp)
 M:	Pete Zaitcev <zaitcev@redhat.com>
 L:	linux-usb@vger.kernel.org
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index f7a7fc2..6496d17 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -3,6 +3,10 @@
 #
 comment "USB Miscellaneous drivers"
 
+config USB_MUX
+	select EXTCON
+	def_bool n
+
 config USB_EMI62
 	tristate "EMI 6|2m USB Audio interface support"
 	---help---
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 45fd4ac..fd79dd5 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -29,3 +29,5 @@ obj-$(CONFIG_USB_CHAOSKEY)		+= chaoskey.o
 
 obj-$(CONFIG_USB_SISUSBVGA)		+= sisusbvga/
 obj-$(CONFIG_USB_LINK_LAYER_TEST)	+= lvstest.o
+
+obj-$(CONFIG_USB_MUX)			+= mux.o
diff --git a/drivers/usb/misc/mux.c b/drivers/usb/misc/mux.c
new file mode 100644
index 0000000..e353fff
--- /dev/null
+++ b/drivers/usb/misc/mux.c
@@ -0,0 +1,172 @@
+/**
+ * mux.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/notifier.h>
+#include <linux/usb/mux.h>
+#include <linux/debugfs.h>
+#include <linux/err.h>
+
+static int usb_mux_change_state(struct usb_mux *mux, int state)
+{
+	int ret;
+	struct usb_mux_dev *umdev = mux->umdev;
+
+	dev_WARN_ONCE(umdev->dev, !mutex_is_locked(&mux->mux_mutex),
+			"mutex is unlocked\n");
+
+	mux->mux_state = state;
+
+	if (mux->mux_state)
+		ret = umdev->cable_set_cb(umdev);
+	else
+		ret = umdev->cable_unset_cb(umdev);
+
+	return ret;
+}
+
+static int usb_mux_notifier(struct notifier_block *nb,
+		unsigned long event, void *ptr)
+{
+	struct usb_mux *mux;
+	int state;
+	int ret = NOTIFY_DONE;
+
+	mux = container_of(nb, struct usb_mux, nb);
+
+	state = extcon_get_cable_state(mux->obj.edev,
+			mux->umdev->cable_name);
+
+	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 mux_debug_read(struct file *file, char __user *user_buf,
+		size_t len, loff_t *offset)
+{
+	struct usb_mux *mux = file->private_data;
+	char output_buf[16];
+
+	memset(output_buf, 0, sizeof(output_buf));
+	if (mux->mux_state)
+		strcpy(output_buf, "host\n");
+	else
+		strcpy(output_buf, "peripheral\n");
+
+	return simple_read_from_buffer(user_buf, len, offset,
+			output_buf, strlen(output_buf));
+}
+
+static ssize_t mux_debug_write(struct file *file, const char __user *user_buf,
+		size_t count, loff_t *offset)
+{
+	struct usb_mux *mux = file->private_data;
+	char input_buf[16];
+	int size, state;
+
+	size = min(count, sizeof(input_buf) - 1);
+	memset(input_buf, 0, sizeof(input_buf));
+	if (strncpy_from_user(input_buf, user_buf, size) < 0)
+		return -EFAULT;
+
+	if (!strncmp(input_buf, "host", 4))
+		state = 1;
+	else if (!strncmp(input_buf, "peripheral", 10))
+		state = 0;
+	else
+		state = -1;
+
+	if (state != -1) {
+		mutex_lock(&mux->mux_mutex);
+		usb_mux_change_state(mux, state);
+		mutex_unlock(&mux->mux_mutex);
+	}
+
+	return count;
+}
+
+static const struct file_operations mux_debug_fops = {
+	.read =		mux_debug_read,
+	.write =	mux_debug_write,
+	.open =		simple_open,
+	.llseek =	default_llseek,
+};
+
+int usb_mux_register(struct usb_mux_dev *umdev)
+{
+	int ret;
+	struct device *dev = umdev->dev;
+	struct usb_mux *mux;
+
+	if (!umdev->cable_name)
+		return -ENODEV;
+
+	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return -ENOMEM;
+
+	mux->umdev = umdev;
+	mux->nb.notifier_call = usb_mux_notifier;
+	mutex_init(&mux->mux_mutex);
+	mux->mux_state = -1;
+	dev_set_drvdata(dev, mux);
+
+	ret = extcon_register_interest(&mux->obj, umdev->extcon_name,
+			umdev->cable_name, &mux->nb);
+	if (ret) {
+		dev_err(dev, "failed to register extcon notifier\n");
+		return -ENODEV;
+	}
+
+	usb_mux_notifier(&mux->nb, 0, NULL);
+
+	mux->debug_file = debugfs_create_file("usb_mux", 0600,
+			usb_debug_root, mux, &mux_debug_fops);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_mux_register);
+
+int usb_mux_unregister(struct device *dev)
+{
+	struct usb_mux *mux = dev_get_drvdata(dev);
+
+	debugfs_remove(mux->debug_file);
+	extcon_unregister_interest(&mux->obj);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_mux_unregister);
+
+struct usb_mux_dev *usb_mux_get_dev(struct device *dev)
+{
+	struct usb_mux *mux = dev_get_drvdata(dev);
+
+	if (mux)
+		return mux->umdev;
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(usb_mux_get_dev);
+
+#ifdef CONFIG_PM_SLEEP
+void usb_mux_complete(struct device *dev)
+{
+	struct usb_mux *mux = dev_get_drvdata(dev);
+
+	usb_mux_notifier(&mux->nb, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(usb_mux_complete);
+#endif
diff --git a/include/linux/usb/mux.h b/include/linux/usb/mux.h
new file mode 100644
index 0000000..5dada48
--- /dev/null
+++ b/include/linux/usb/mux.h
@@ -0,0 +1,71 @@
+/**
+ * mux.h - USB Port Mux defines
+ *
+ * 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_MUX_H
+#define __LINUX_USB_MUX_H
+
+#include <linux/extcon.h>
+#include <linux/usb.h>
+
+struct usb_mux_dev {
+	struct device	*dev;
+	char		*extcon_name;
+	char		*cable_name;
+	int		(*cable_set_cb)(struct usb_mux_dev *mux);
+	int		(*cable_unset_cb)(struct usb_mux_dev *mux);
+};
+
+struct usb_mux {
+	struct usb_mux_dev		*umdev;
+	struct notifier_block		nb;
+	struct extcon_specific_cable_nb	obj;
+
+	/*
+	 * The state of the mux.
+	 * 0, 1 - mux switch state
+	 * -1   - uninitialized state
+	 *
+	 * mux_mutex is lock to protect mux_state
+	 */
+	int				mux_state;
+	struct mutex			mux_mutex;
+
+	struct dentry			*debug_file;
+};
+
+#if IS_ENABLED(CONFIG_USB_MUX)
+extern int usb_mux_register(struct usb_mux_dev *mux);
+extern int usb_mux_unregister(struct device *dev);
+extern struct usb_mux_dev *usb_mux_get_dev(struct device *dev);
+
+#ifdef CONFIG_PM_SLEEP
+extern void usb_mux_complete(struct device *dev);
+#endif
+
+#else /* CONFIG_USB_MUX */
+static inline int usb_mux_register(struct usb_mux_dev *mux)
+{
+	return -ENODEV;
+}
+
+static inline int usb_mux_unregister(struct device *dev)
+{
+	return 0;
+}
+
+static inline struct usb_mux_dev *usb_mux_get_dev(struct device *dev)
+{
+	return NULL;
+}
+#endif /* CONFIG_USB_MUX */
+
+#endif /* __LINUX_USB_MUX_H */
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1349330 — Re: [PATCH 3/7] usb: misc: add common code for Intel dual role port mux

On Thu, Mar 03, 2016 at 02:37:40PM +0800, 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@linux.intel.com>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Reviewed-by: Felipe Balbi <balbi@kernel.org>
> ---
>  MAINTAINERS               |   7 ++
>  drivers/usb/misc/Kconfig  |   4 ++
>  drivers/usb/misc/Makefile |   2 +
>  drivers/usb/misc/mux.c    | 172 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/usb/mux.h   |  71 +++++++++++++++++++
>  5 files changed, 256 insertions(+)
>  create mode 100644 drivers/usb/misc/mux.c
>  create mode 100644 include/linux/usb/mux.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d894ee2..45f1e1e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11389,6 +11389,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@linux.intel.com>
> +L:	linux-usb@vger.kernel.org
> +S:	Supported
> +F:	drivers/usb/misc/mux.c
> +F:	include/linux/usb/mux.h
> +
>  USB PRINTER DRIVER (usblp)
>  M:	Pete Zaitcev <zaitcev@redhat.com>
>  L:	linux-usb@vger.kernel.org
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index f7a7fc2..6496d17 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -3,6 +3,10 @@
>  #
>  comment "USB Miscellaneous drivers"
>  
> +config USB_MUX
> +	select EXTCON
> +	def_bool n
> +
>  config USB_EMI62
>  	tristate "EMI 6|2m USB Audio interface support"
>  	---help---
> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> index 45fd4ac..fd79dd5 100644
> --- a/drivers/usb/misc/Makefile
> +++ b/drivers/usb/misc/Makefile
> @@ -29,3 +29,5 @@ obj-$(CONFIG_USB_CHAOSKEY)		+= chaoskey.o
>  
>  obj-$(CONFIG_USB_SISUSBVGA)		+= sisusbvga/
>  obj-$(CONFIG_USB_LINK_LAYER_TEST)	+= lvstest.o
> +
> +obj-$(CONFIG_USB_MUX)			+= mux.o

"mux.ko" is a _VERY_ generic name of a kernel module.  Please make this
much more specific to your chip as this will not work for any other
platform that has this type of functionality.

Same goes for all of your exported symbols.



> diff --git a/drivers/usb/misc/mux.c b/drivers/usb/misc/mux.c
> new file mode 100644
> index 0000000..e353fff
> --- /dev/null
> +++ b/drivers/usb/misc/mux.c
> @@ -0,0 +1,172 @@
> +/**
> + * mux.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/notifier.h>
> +#include <linux/usb/mux.h>
> +#include <linux/debugfs.h>
> +#include <linux/err.h>
> +
> +static int usb_mux_change_state(struct usb_mux *mux, int state)
> +{
> +	int ret;
> +	struct usb_mux_dev *umdev = mux->umdev;
> +
> +	dev_WARN_ONCE(umdev->dev, !mutex_is_locked(&mux->mux_mutex),
> +			"mutex is unlocked\n");
> +
> +	mux->mux_state = state;
> +
> +	if (mux->mux_state)
> +		ret = umdev->cable_set_cb(umdev);
> +	else
> +		ret = umdev->cable_unset_cb(umdev);
> +
> +	return ret;
> +}
> +
> +static int usb_mux_notifier(struct notifier_block *nb,
> +		unsigned long event, void *ptr)
> +{
> +	struct usb_mux *mux;
> +	int state;
> +	int ret = NOTIFY_DONE;
> +
> +	mux = container_of(nb, struct usb_mux, nb);
> +
> +	state = extcon_get_cable_state(mux->obj.edev,
> +			mux->umdev->cable_name);
> +
> +	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 mux_debug_read(struct file *file, char __user *user_buf,
> +		size_t len, loff_t *offset)
> +{
> +	struct usb_mux *mux = file->private_data;
> +	char output_buf[16];
> +
> +	memset(output_buf, 0, sizeof(output_buf));
> +	if (mux->mux_state)
> +		strcpy(output_buf, "host\n");
> +	else
> +		strcpy(output_buf, "peripheral\n");
> +
> +	return simple_read_from_buffer(user_buf, len, offset,
> +			output_buf, strlen(output_buf));
> +}
> +
> +static ssize_t mux_debug_write(struct file *file, const char __user *user_buf,
> +		size_t count, loff_t *offset)
> +{
> +	struct usb_mux *mux = file->private_data;
> +	char input_buf[16];
> +	int size, state;
> +
> +	size = min(count, sizeof(input_buf) - 1);
> +	memset(input_buf, 0, sizeof(input_buf));
> +	if (strncpy_from_user(input_buf, user_buf, size) < 0)
> +		return -EFAULT;
> +
> +	if (!strncmp(input_buf, "host", 4))
> +		state = 1;
> +	else if (!strncmp(input_buf, "peripheral", 10))
> +		state = 0;
> +	else
> +		state = -1;
> +
> +	if (state != -1) {
> +		mutex_lock(&mux->mux_mutex);
> +		usb_mux_change_state(mux, state);
> +		mutex_unlock(&mux->mux_mutex);
> +	}
> +
> +	return count;
> +}
> +
> +static const struct file_operations mux_debug_fops = {
> +	.read =		mux_debug_read,
> +	.write =	mux_debug_write,
> +	.open =		simple_open,
> +	.llseek =	default_llseek,
> +};
> +
> +int usb_mux_register(struct usb_mux_dev *umdev)
> +{
> +	int ret;
> +	struct device *dev = umdev->dev;
> +	struct usb_mux *mux;
> +
> +	if (!umdev->cable_name)
> +		return -ENODEV;
> +
> +	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
> +	if (!mux)
> +		return -ENOMEM;
> +
> +	mux->umdev = umdev;
> +	mux->nb.notifier_call = usb_mux_notifier;
> +	mutex_init(&mux->mux_mutex);
> +	mux->mux_state = -1;
> +	dev_set_drvdata(dev, mux);
> +
> +	ret = extcon_register_interest(&mux->obj, umdev->extcon_name,
> +			umdev->cable_name, &mux->nb);
> +	if (ret) {
> +		dev_err(dev, "failed to register extcon notifier\n");
> +		return -ENODEV;
> +	}
> +
> +	usb_mux_notifier(&mux->nb, 0, NULL);
> +
> +	mux->debug_file = debugfs_create_file("usb_mux", 0600,
> +			usb_debug_root, mux, &mux_debug_fops);

So you control this through debugfs?  That's not a good idea, what if
you have multiple ones of these?  What if debugfs is not enabled?  Who
is in charge of controlling this from userspace?

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_mux_register);
> +
> +int usb_mux_unregister(struct device *dev)
> +{
> +	struct usb_mux *mux = dev_get_drvdata(dev);
> +
> +	debugfs_remove(mux->debug_file);
> +	extcon_unregister_interest(&mux->obj);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_mux_unregister);
> +
> +struct usb_mux_dev *usb_mux_get_dev(struct device *dev)
> +{
> +	struct usb_mux *mux = dev_get_drvdata(dev);
> +
> +	if (mux)
> +		return mux->umdev;
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(usb_mux_get_dev);
> +
> +#ifdef CONFIG_PM_SLEEP
> +void usb_mux_complete(struct device *dev)
> +{
> +	struct usb_mux *mux = dev_get_drvdata(dev);
> +
> +	usb_mux_notifier(&mux->nb, 0, NULL);
> +}
> +EXPORT_SYMBOL_GPL(usb_mux_complete);
> +#endif
> diff --git a/include/linux/usb/mux.h b/include/linux/usb/mux.h
> new file mode 100644
> index 0000000..5dada48
> --- /dev/null
> +++ b/include/linux/usb/mux.h
> @@ -0,0 +1,71 @@
> +/**
> + * mux.h - USB Port Mux defines
> + *
> + * 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_MUX_H
> +#define __LINUX_USB_MUX_H
> +
> +#include <linux/extcon.h>
> +#include <linux/usb.h>
> +
> +struct usb_mux_dev {
> +	struct device	*dev;
> +	char		*extcon_name;
> +	char		*cable_name;
> +	int		(*cable_set_cb)(struct usb_mux_dev *mux);
> +	int		(*cable_unset_cb)(struct usb_mux_dev *mux);
> +};
> +
> +struct usb_mux {
> +	struct usb_mux_dev		*umdev;
> +	struct notifier_block		nb;
> +	struct extcon_specific_cable_nb	obj;
> +
> +	/*
> +	 * The state of the mux.
> +	 * 0, 1 - mux switch state
> +	 * -1   - uninitialized state
> +	 *
> +	 * mux_mutex is lock to protect mux_state
> +	 */
> +	int				mux_state;
> +	struct mutex			mux_mutex;
> +
> +	struct dentry			*debug_file;
> +};


Why is this a public structure?

And why isn't it properly reference counted if it is a public structure?


> +
> +#if IS_ENABLED(CONFIG_USB_MUX)
> +extern int usb_mux_register(struct usb_mux_dev *mux);
> +extern int usb_mux_unregister(struct device *dev);
> +extern struct usb_mux_dev *usb_mux_get_dev(struct device *dev);
> +
> +#ifdef CONFIG_PM_SLEEP
> +extern void usb_mux_complete(struct device *dev);
> +#endif
> +
> +#else /* CONFIG_USB_MUX */
> +static inline int usb_mux_register(struct usb_mux_dev *mux)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int usb_mux_unregister(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static inline struct usb_mux_dev *usb_mux_get_dev(struct device *dev)
> +{
> +	return NULL;
> +}
> +#endif /* CONFIG_USB_MUX */
> +
> +#endif /* __LINUX_USB_MUX_H */


Why do you need this .h file at all?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1349752 — Re: [PATCH 3/7] usb: misc: add common code for Intel dual role port mux

On 03/04/2016 12:10 AM, Greg Kroah-Hartman wrote:
> On Thu, Mar 03, 2016 at 02:37:40PM +0800, 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@linux.intel.com>
>> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Reviewed-by: Felipe Balbi <balbi@kernel.org>
>> ---
>>  MAINTAINERS               |   7 ++
>>  drivers/usb/misc/Kconfig  |   4 ++
>>  drivers/usb/misc/Makefile |   2 +
>>  drivers/usb/misc/mux.c    | 172 ++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/usb/mux.h   |  71 +++++++++++++++++++
>>  5 files changed, 256 insertions(+)
>>  create mode 100644 drivers/usb/misc/mux.c
>>  create mode 100644 include/linux/usb/mux.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d894ee2..45f1e1e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11389,6 +11389,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@linux.intel.com>
>> +L:	linux-usb@vger.kernel.org
>> +S:	Supported
>> +F:	drivers/usb/misc/mux.c
>> +F:	include/linux/usb/mux.h
>> +
>>  USB PRINTER DRIVER (usblp)
>>  M:	Pete Zaitcev <zaitcev@redhat.com>
>>  L:	linux-usb@vger.kernel.org
>> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
>> index f7a7fc2..6496d17 100644
>> --- a/drivers/usb/misc/Kconfig
>> +++ b/drivers/usb/misc/Kconfig
>> @@ -3,6 +3,10 @@
>>  #
>>  comment "USB Miscellaneous drivers"
>>  
>> +config USB_MUX
>> +	select EXTCON
>> +	def_bool n
>> +
>>  config USB_EMI62
>>  	tristate "EMI 6|2m USB Audio interface support"
>>  	---help---
>> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
>> index 45fd4ac..fd79dd5 100644
>> --- a/drivers/usb/misc/Makefile
>> +++ b/drivers/usb/misc/Makefile
>> @@ -29,3 +29,5 @@ obj-$(CONFIG_USB_CHAOSKEY)		+= chaoskey.o
>>  
>>  obj-$(CONFIG_USB_SISUSBVGA)		+= sisusbvga/
>>  obj-$(CONFIG_USB_LINK_LAYER_TEST)	+= lvstest.o
>> +
>> +obj-$(CONFIG_USB_MUX)			+= mux.o
> "mux.ko" is a _VERY_ generic name of a kernel module.  Please make this
> much more specific to your chip as this will not work for any other
> platform that has this type of functionality.
>
> Same goes for all of your exported symbols.

Sure. I will change the module name to "intel-mux", and change
the exported symbols to "intel_usb_mux_*".

>
>
>> diff --git a/drivers/usb/misc/mux.c b/drivers/usb/misc/mux.c
>> new file mode 100644
>> index 0000000..e353fff
>> --- /dev/null
>> +++ b/drivers/usb/misc/mux.c
>> @@ -0,0 +1,172 @@
>> +/**
>> + * mux.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/notifier.h>
>> +#include <linux/usb/mux.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/err.h>
>> +
>> +static int usb_mux_change_state(struct usb_mux *mux, int state)
>> +{
>> +	int ret;
>> +	struct usb_mux_dev *umdev = mux->umdev;
>> +
>> +	dev_WARN_ONCE(umdev->dev, !mutex_is_locked(&mux->mux_mutex),
>> +			"mutex is unlocked\n");
>> +
>> +	mux->mux_state = state;
>> +
>> +	if (mux->mux_state)
>> +		ret = umdev->cable_set_cb(umdev);
>> +	else
>> +		ret = umdev->cable_unset_cb(umdev);
>> +
>> +	return ret;
>> +}
>> +
>> +static int usb_mux_notifier(struct notifier_block *nb,
>> +		unsigned long event, void *ptr)
>> +{
>> +	struct usb_mux *mux;
>> +	int state;
>> +	int ret = NOTIFY_DONE;
>> +
>> +	mux = container_of(nb, struct usb_mux, nb);
>> +
>> +	state = extcon_get_cable_state(mux->obj.edev,
>> +			mux->umdev->cable_name);
>> +
>> +	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 mux_debug_read(struct file *file, char __user *user_buf,
>> +		size_t len, loff_t *offset)
>> +{
>> +	struct usb_mux *mux = file->private_data;
>> +	char output_buf[16];
>> +
>> +	memset(output_buf, 0, sizeof(output_buf));
>> +	if (mux->mux_state)
>> +		strcpy(output_buf, "host\n");
>> +	else
>> +		strcpy(output_buf, "peripheral\n");
>> +
>> +	return simple_read_from_buffer(user_buf, len, offset,
>> +			output_buf, strlen(output_buf));
>> +}
>> +
>> +static ssize_t mux_debug_write(struct file *file, const char __user *user_buf,
>> +		size_t count, loff_t *offset)
>> +{
>> +	struct usb_mux *mux = file->private_data;
>> +	char input_buf[16];
>> +	int size, state;
>> +
>> +	size = min(count, sizeof(input_buf) - 1);
>> +	memset(input_buf, 0, sizeof(input_buf));
>> +	if (strncpy_from_user(input_buf, user_buf, size) < 0)
>> +		return -EFAULT;
>> +
>> +	if (!strncmp(input_buf, "host", 4))
>> +		state = 1;
>> +	else if (!strncmp(input_buf, "peripheral", 10))
>> +		state = 0;
>> +	else
>> +		state = -1;
>> +
>> +	if (state != -1) {
>> +		mutex_lock(&mux->mux_mutex);
>> +		usb_mux_change_state(mux, state);
>> +		mutex_unlock(&mux->mux_mutex);
>> +	}
>> +
>> +	return count;
>> +}
>> +
>> +static const struct file_operations mux_debug_fops = {
>> +	.read =		mux_debug_read,
>> +	.write =	mux_debug_write,
>> +	.open =		simple_open,
>> +	.llseek =	default_llseek,
>> +};
>> +
>> +int usb_mux_register(struct usb_mux_dev *umdev)
>> +{
>> +	int ret;
>> +	struct device *dev = umdev->dev;
>> +	struct usb_mux *mux;
>> +
>> +	if (!umdev->cable_name)
>> +		return -ENODEV;
>> +
>> +	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
>> +	if (!mux)
>> +		return -ENOMEM;
>> +
>> +	mux->umdev = umdev;
>> +	mux->nb.notifier_call = usb_mux_notifier;
>> +	mutex_init(&mux->mux_mutex);
>> +	mux->mux_state = -1;
>> +	dev_set_drvdata(dev, mux);
>> +
>> +	ret = extcon_register_interest(&mux->obj, umdev->extcon_name,
>> +			umdev->cable_name, &mux->nb);
>> +	if (ret) {
>> +		dev_err(dev, "failed to register extcon notifier\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	usb_mux_notifier(&mux->nb, 0, NULL);
>> +
>> +	mux->debug_file = debugfs_create_file("usb_mux", 0600,
>> +			usb_debug_root, mux, &mux_debug_fops);
> So you control this through debugfs?  That's not a good idea, what if
> you have multiple ones of these?  What if debugfs is not enabled?

Agree. It's really a problem if we have multiple ones (although
currently we have only single one).

I will make it with sysfs instead.

> Who
> is in charge of controlling this from userspace?

Port mux is only handled in kernel. There is no requirement to
control it from user space. This is added for debugging and
information only.

>
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_mux_register);
>> +
>> +int usb_mux_unregister(struct device *dev)
>> +{
>> +	struct usb_mux *mux = dev_get_drvdata(dev);
>> +
>> +	debugfs_remove(mux->debug_file);
>> +	extcon_unregister_interest(&mux->obj);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_mux_unregister);
>> +
>> +struct usb_mux_dev *usb_mux_get_dev(struct device *dev)
>> +{
>> +	struct usb_mux *mux = dev_get_drvdata(dev);
>> +
>> +	if (mux)
>> +		return mux->umdev;
>> +
>> +	return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_mux_get_dev);
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +void usb_mux_complete(struct device *dev)
>> +{
>> +	struct usb_mux *mux = dev_get_drvdata(dev);
>> +
>> +	usb_mux_notifier(&mux->nb, 0, NULL);
>> +}
>> +EXPORT_SYMBOL_GPL(usb_mux_complete);
>> +#endif
>> diff --git a/include/linux/usb/mux.h b/include/linux/usb/mux.h
>> new file mode 100644
>> index 0000000..5dada48
>> --- /dev/null
>> +++ b/include/linux/usb/mux.h
>> @@ -0,0 +1,71 @@
>> +/**
>> + * mux.h - USB Port Mux defines
>> + *
>> + * 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_MUX_H
>> +#define __LINUX_USB_MUX_H
>> +
>> +#include <linux/extcon.h>
>> +#include <linux/usb.h>
>> +
>> +struct usb_mux_dev {
>> +	struct device	*dev;
>> +	char		*extcon_name;
>> +	char		*cable_name;
>> +	int		(*cable_set_cb)(struct usb_mux_dev *mux);
>> +	int		(*cable_unset_cb)(struct usb_mux_dev *mux);
>> +};
>> +
>> +struct usb_mux {
>> +	struct usb_mux_dev		*umdev;
>> +	struct notifier_block		nb;
>> +	struct extcon_specific_cable_nb	obj;
>> +
>> +	/*
>> +	 * The state of the mux.
>> +	 * 0, 1 - mux switch state
>> +	 * -1   - uninitialized state
>> +	 *
>> +	 * mux_mutex is lock to protect mux_state
>> +	 */
>> +	int				mux_state;
>> +	struct mutex			mux_mutex;
>> +
>> +	struct dentry			*debug_file;
>> +};
>
> Why is this a public structure?
>
> And why isn't it properly reference counted if it is a public structure?

Agree. I will move it to .c file and make it private one.

>
>
>> +
>> +#if IS_ENABLED(CONFIG_USB_MUX)
>> +extern int usb_mux_register(struct usb_mux_dev *mux);
>> +extern int usb_mux_unregister(struct device *dev);
>> +extern struct usb_mux_dev *usb_mux_get_dev(struct device *dev);
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +extern void usb_mux_complete(struct device *dev);
>> +#endif
>> +
>> +#else /* CONFIG_USB_MUX */
>> +static inline int usb_mux_register(struct usb_mux_dev *mux)
>> +{
>> +	return -ENODEV;
>> +}
>> +
>> +static inline int usb_mux_unregister(struct device *dev)
>> +{
>> +	return 0;
>> +}
>> +
>> +static inline struct usb_mux_dev *usb_mux_get_dev(struct device *dev)
>> +{
>> +	return NULL;
>> +}
>> +#endif /* CONFIG_USB_MUX */
>> +
>> +#endif /* __LINUX_USB_MUX_H */
>
> Why do you need this .h file at all?

File mux.c is a common file for all port mux drivers. The individual port
mux driver needs this .h file for structure and function interface definition.

>
> thanks,
>
> greg k-h
>

Thank you for your time.

Best Regards,
-Baolu

[toc] | [prev] | [next] | [standalone]

#1349333 — Re: [PATCH 3/7] usb: misc: add common code for Intel dual role port mux

On Thu, Mar 03, 2016 at 02:37:40PM +0800, Lu Baolu wrote:
> +#if IS_ENABLED(CONFIG_USB_MUX)
> +extern int usb_mux_register(struct usb_mux_dev *mux);
> +extern int usb_mux_unregister(struct device *dev);
> +extern struct usb_mux_dev *usb_mux_get_dev(struct device *dev);

This api needs a lot of work, for such a "tiny" api, it's obviously
pretty incorrect.  Please fix it up to be sane...

[toc] | [prev] | [next] | [standalone]

#1349755 — Re: [PATCH 3/7] usb: misc: add common code for Intel dual role port mux

On 03/04/2016 12:15 AM, Greg Kroah-Hartman wrote:
> On Thu, Mar 03, 2016 at 02:37:40PM +0800, Lu Baolu wrote:
>> +#if IS_ENABLED(CONFIG_USB_MUX)
>> +extern int usb_mux_register(struct usb_mux_dev *mux);
>> +extern int usb_mux_unregister(struct device *dev);
>> +extern struct usb_mux_dev *usb_mux_get_dev(struct device *dev);
> This api needs a lot of work, for such a "tiny" api, it's obviously
> pretty incorrect.  Please fix it up to be sane...

Sure.

>
>
>

[toc] | [prev] | [next] | [standalone]

#1348816 — [PATCH 7/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver

Some Intel platforms have an USB port mux controlled by GPIOs.
There's a single ACPI platform device that provides both USB ID
extcon device and a USB port mux device. This MFD driver will
split the 2 devices for their respective drivers.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Suggested-by: David Cohen <david.a.cohen@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
---
 MAINTAINERS                |  1 +
 drivers/mfd/Kconfig        |  7 ++++
 drivers/mfd/Makefile       |  1 +
 drivers/mfd/intel-vuport.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+)
 create mode 100644 drivers/mfd/intel-vuport.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 20eb873..6e0bb12 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11397,6 +11397,7 @@ F:	drivers/usb/misc/mux.c
 F:	include/linux/usb/mux.h
 F:	drivers/usb/misc/intel-mux-gpio.c
 F:	drivers/usb/misc/intel-mux-drcfg.c
+F:	drivers/mfd/intel-vuport.c
 
 USB PRINTER DRIVER (usblp)
 M:	Pete Zaitcev <zaitcev@redhat.com>
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 9ca66de..0913494 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1534,5 +1534,12 @@ config MFD_VEXPRESS_SYSREG
 	  System Registers are the platform configuration block
 	  on the ARM Ltd. Versatile Express board.
 
+config MFD_INTEL_VUPORT
+	tristate "Intel virtual USB port controller"
+	select MFD_CORE
+	help
+	  Say Y here to enable support for Intel dual role port mux
+	  controlled by 3 GPIOs.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 0f230a6..0ccd107 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -198,3 +198,4 @@ intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC)	+= intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
 obj-$(CONFIG_MFD_MT6397)	+= mt6397-core.o
+obj-$(CONFIG_MFD_INTEL_VUPORT)	+= intel-vuport.o
diff --git a/drivers/mfd/intel-vuport.c b/drivers/mfd/intel-vuport.c
new file mode 100644
index 0000000..63391dc
--- /dev/null
+++ b/drivers/mfd/intel-vuport.c
@@ -0,0 +1,79 @@
+/*
+ * MFD driver for Intel virtual USB port
+ *
+ * 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/acpi.h>
+#include <linux/gpio.h>
+#include <linux/mfd/core.h>
+#include <linux/platform_device.h>
+
+/* ACPI GPIO Mappings */
+static const struct acpi_gpio_params id_gpio = { 0, 0, false };
+static const struct acpi_gpio_params vbus_gpio = { 1, 0, false };
+static const struct acpi_gpio_params mux_gpio = { 2, 0, false };
+static const struct acpi_gpio_mapping acpi_usb_gpios[] = {
+	{ "id-gpios", &id_gpio, 1 },
+	{ "vbus_en-gpios", &vbus_gpio, 1 },
+	{ "usb_mux-gpios", &mux_gpio, 1 },
+	{ },
+};
+
+static const struct mfd_cell intel_vuport_mfd_cells[] = {
+	{
+		.name = "extcon-usb-gpio",
+	},
+	{
+		.name = "intel-mux-gpio",
+	},
+};
+
+static int vuport_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = acpi_dev_add_driver_gpios(ACPI_COMPANION(dev), acpi_usb_gpios);
+	if (ret)
+		return ret;
+
+	return mfd_add_devices(&pdev->dev, 0, intel_vuport_mfd_cells,
+			ARRAY_SIZE(intel_vuport_mfd_cells), NULL, 0,
+			NULL);
+}
+
+static int vuport_remove(struct platform_device *pdev)
+{
+	mfd_remove_devices(&pdev->dev);
+	acpi_dev_remove_driver_gpios(ACPI_COMPANION(&pdev->dev));
+
+	return 0;
+}
+
+static struct acpi_device_id vuport_acpi_match[] = {
+	{ "INT3496" },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, vuport_acpi_match);
+
+static struct platform_driver vuport_driver = {
+	.driver = {
+		.name = "intel-vuport",
+		.owner = THIS_MODULE,
+		.acpi_match_table = ACPI_PTR(vuport_acpi_match),
+	},
+	.probe = vuport_probe,
+	.remove = vuport_remove,
+};
+
+module_platform_driver(vuport_driver);
+
+MODULE_AUTHOR("Lu Baolu <baolu.lu@linux.intel.com>");
+MODULE_DESCRIPTION("Intel virtual USB port");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1348875 — [PATCH] mfd: intel_vuport: fix platform_no_drv_owner.cocci warnings

drivers/mfd/intel-vuport.c:68:3-8: No need to set .owner here. The core will do it.

 Remove .owner field if calls are used which set it automatically

Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci

CC: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 intel-vuport.c |    1 -
 1 file changed, 1 deletion(-)

--- a/drivers/mfd/intel-vuport.c
+++ b/drivers/mfd/intel-vuport.c
@@ -65,7 +65,6 @@ MODULE_DEVICE_TABLE(acpi, vuport_acpi_ma
 static struct platform_driver vuport_driver = {
 	.driver = {
 		.name = "intel-vuport",
-		.owner = THIS_MODULE,
 		.acpi_match_table = ACPI_PTR(vuport_acpi_match),
 	},
 	.probe = vuport_probe,

[toc] | [prev] | [next] | [standalone]

#1348885 — Re: [PATCH 7/7] mfd: intel_vuport: Add Intel virtual USB port MFD Driver

Hi Lu,

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v4.5-rc6 next-20160303]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Lu-Baolu/usb-add-support-for-Intel-dual-role-port-mux/20160303-144241
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing


coccinelle warnings: (new ones prefixed by >>)

>> drivers/mfd/intel-vuport.c:68:3-8: No need to set .owner here. The core will do it.

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1348817 — [PATCH 1/7] extcon: usb-gpio: add device binding for platform device

This is needed to handle the GPIO connected USB ID pin found on
Intel Baytrail devices.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
---
 drivers/extcon/extcon-usb-gpio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index 2b2fecf..af9c8b0 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -206,6 +206,12 @@ static const struct of_device_id usb_extcon_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, usb_extcon_dt_match);
 
+static const struct platform_device_id usb_extcon_platform_ids[] = {
+	{ .name = "extcon-usb-gpio", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, usb_extcon_platform_ids);
+
 static struct platform_driver usb_extcon_driver = {
 	.probe		= usb_extcon_probe,
 	.remove		= usb_extcon_remove,
@@ -214,6 +220,7 @@ static struct platform_driver usb_extcon_driver = {
 		.pm	= &usb_extcon_pm_ops,
 		.of_match_table = usb_extcon_dt_match,
 	},
+	.id_table = usb_extcon_platform_ids,
 };
 
 module_platform_driver(usb_extcon_driver);
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1348829 — Re: [PATCH 1/7] extcon: usb-gpio: add device binding for platform device

Hell Lu,

On 2016년 03월 03일 15:37, Lu Baolu wrote:
> This is needed to handle the GPIO connected USB ID pin found on
> Intel Baytrail devices.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Felipe Balbi <balbi@kernel.org>
> ---
>  drivers/extcon/extcon-usb-gpio.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
> index 2b2fecf..af9c8b0 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -206,6 +206,12 @@ static const struct of_device_id usb_extcon_dt_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, usb_extcon_dt_match);
>  
> +static const struct platform_device_id usb_extcon_platform_ids[] = {
> +	{ .name = "extcon-usb-gpio", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, usb_extcon_platform_ids);
> +
>  static struct platform_driver usb_extcon_driver = {
>  	.probe		= usb_extcon_probe,
>  	.remove		= usb_extcon_remove,
> @@ -214,6 +220,7 @@ static struct platform_driver usb_extcon_driver = {
>  		.pm	= &usb_extcon_pm_ops,
>  		.of_match_table = usb_extcon_dt_match,
>  	},
> +	.id_table = usb_extcon_platform_ids,
>  };
>  
>  module_platform_driver(usb_extcon_driver);
> 

Looks good to me.

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Best Regards,
Chanwoo Choi

[toc] | [prev] | [next] | [standalone]

#1348830 — Re: [PATCH 1/7] extcon: usb-gpio: add device binding for platform device

On 03/03/2016 03:24 PM, Chanwoo Choi wrote:
> Hell Lu,
>
> On 2016년 03월 03일 15:37, Lu Baolu wrote:
>> This is needed to handle the GPIO connected USB ID pin found on
>> Intel Baytrail devices.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> Reviewed-by: Felipe Balbi <balbi@kernel.org>
>> ---
>>  drivers/extcon/extcon-usb-gpio.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
>> index 2b2fecf..af9c8b0 100644
>> --- a/drivers/extcon/extcon-usb-gpio.c
>> +++ b/drivers/extcon/extcon-usb-gpio.c
>> @@ -206,6 +206,12 @@ static const struct of_device_id usb_extcon_dt_match[] = {
>>  };
>>  MODULE_DEVICE_TABLE(of, usb_extcon_dt_match);
>>  
>> +static const struct platform_device_id usb_extcon_platform_ids[] = {
>> +	{ .name = "extcon-usb-gpio", },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(platform, usb_extcon_platform_ids);
>> +
>>  static struct platform_driver usb_extcon_driver = {
>>  	.probe		= usb_extcon_probe,
>>  	.remove		= usb_extcon_remove,
>> @@ -214,6 +220,7 @@ static struct platform_driver usb_extcon_driver = {
>>  		.pm	= &usb_extcon_pm_ops,
>>  		.of_match_table = usb_extcon_dt_match,
>>  	},
>> +	.id_table = usb_extcon_platform_ids,
>>  };
>>  
>>  module_platform_driver(usb_extcon_driver);
>>
> Looks good to me.
>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Thank you!

>
> Best Regards,
> Chanwoo Choi
>
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web