From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932318AbcCQH2E (ORCPT ); Thu, 17 Mar 2016 03:28:04 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:58763 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756183AbcCQH2B (ORCPT ); Thu, 17 Mar 2016 03:28:01 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfee68d-f79646d000001355-a0-56ea5c7efd9f Content-transfer-encoding: 8BIT Message-id: <56EA5C7E.6020004@samsung.com> Date: Thu, 17 Mar 2016 16:27:58 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Lu Baolu , Felipe Balbi , Mathias Nyman , Greg Kroah-Hartman , Lee Jones , Heikki Krogerus , MyungJoo Ham Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 3/7] usb: mux: add common code for Intel dual role port mux References: <1458193581-25237-1-git-send-email-baolu.lu@linux.intel.com> <1458193581-25237-4-git-send-email-baolu.lu@linux.intel.com> <56EA49A9.6030400@samsung.com> <56EA59BB.4020201@linux.intel.com> In-reply-to: <56EA59BB.4020201@linux.intel.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrAIsWRmVeSWpSXmKPExsWyRsSkULcu5lWYwaLFFhbH2p6wW2yeuJXN onnxejaLrtU7WSzufz3KaHF51xw2i0XLWpktmjdNYbW43biCzYHTY/Gel0wem1Z1snncubaH zWPeyUCP/XPXsHv0bVnF6PF5k1wAexSXTUpqTmZZapG+XQJXxvKdjUwFW3MrNty4xNTAeDG8 i5GTQ0LAROL10d+sELaYxIV769lAbCGBFYwSh+4VwdScOruHqYuRCyi+lFHiwKPzzCAJXgFB iR+T77F0MXJwMAvISxy5lA1hqktMmZILUf6AUeJ1y2ywEl4BLYld1wVAOlkEVCU2zPzKCGKz AYX3v7jBBlIiKhAh0X2iEiQsIrCKSaJ9ET/ERGuJn/vyQcLCAiESe149YoOYfppRYvPOBywg CU4BfYmZj/uZQRISAm/ZJa7sPswIsUtA4tvkQ2AnSAjISmw6wAzxlaTEwRU3WCYwis1C8sss hF9mIfyygJF5FaNoakFyQXFSepGhXnFibnFpXrpecn7uJkZgRJ7+96x3B+PtA9aHGAU4GJV4 eBnOvQwTYk0sK67MPcRoCnTDRGYp0eR8YNznlcQbGpsZWZiamBobmVuaKYnzKkr9DBYSSE8s Sc1OTS1ILYovKs1JLT7EyMTBKdXAaLb9wFrlVLEeoWtT5VvjwvqU/Cptjv9rVTx+/q+YSIdG BV/ozk8aS6Lnlzr6ND7/zHaA4eHZs4/9DzhUi3yX2SGf6i577KvE5rkVE14lrFwdIqzw9Mmk jbP/HMwWeSY5U7PDYuVRPqF1u8JDwi4H3oz9HzDB8MyfI59/bzv8NHSbTGp7wsJr+UosxRmJ hlrMRcWJAFZr29XDAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrHIsWRmVeSWpSXmKPExsVy+t9jAd26mFdhBhNXGVoca3vCbrF54lY2 i+bF69ksulbvZLG4//Uoo8XlXXPYLBYta2W2aN40hdXiduMKNgdOj8V7XjJ5bFrVyeZx59oe No95JwM99s9dw+7Rt2UVo8fnTXIB7FENjDYZqYkpqUUKqXnJ+SmZeem2St7B8c7xpmYGhrqG lhbmSgp5ibmptkouPgG6bpk5QKcpKZQl5pQChQISi4uV9O0wTQgNcdO1gGmM0PUNCYLrMTJA AwlrGDOW72xkKtiaW7HhxiWmBsaL4V2MnBwSAiYSp87uYYKwxSQu3FvP1sXIxSEksJRR4sCj 88wgCV4BQYkfk++xdDFycDALyEscuZQNYapLTJmSC1H+gFHidctssBJeAS2JXdcFQDpZBFQl Nsz8yghiswGF97+4wQZSIioQIdF9ohIkLCKwikmifRE/xERriZ/78kHCwgIhEntePYI65jSj xOadD1hAEpwC+hIzH/czT2AUmIXktlkIt81CuG0BI/MqRonUguSC4qT0XMO81HK94sTc4tK8 dL3k/NxNjOC4fya1g/HgLvdDjAIcjEo8vCtOvwwTYk0sK67MPcQowcGsJMJbIvwqTIg3JbGy KrUoP76oNCe1+BCjKdB3E5mlRJPzgSkpryTe0NjEzMjSyNzQwsjYXEmc9/H/dWFCAumJJanZ qakFqUUwfUwcnFINjMyGfgvDxVpqjPyWr7/+J9DBvvFfxJn5SddYs7d++CPpvbPaPXx1oHmf beq0mtrX9645Sx+zYchjOxiTEbdUtvX9kUtavrKtL99+aplyYOs/U9MuMyvuU2pGN7UjI9/O cEwX2hTgurgu/c5xO8sSW7F99xb6h1lsY7Qo6IyS3Xz2Ws0up+lmSizFGYmGWsxFxYkAFF0M XxEDAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Lu, On 2016년 03월 17일 16:16, Lu Baolu wrote: > Hi Chanwoo, > > On 03/17/2016 02:07 PM, Chanwoo Choi wrote: >> Hi Lu, >> >> To handle extcon (external connector), I implemented the unique id >> for each external connector on patch[1] instead of using the ambiguous string type. >> [1] 2a9de9c0f08d6 (extcon: Use the unique id for external connector instead of string) >> >> So I recommend that you should use the unique id (ex. EXTCON_USB, EXTCON_USB_HOST) >> with extcon_register_notifier(), extcon_get_cable_state_() and extcon_set_cable_state_(). >> >> extcon_register_interest() is deprecated-> extcon_register_notifier() >> extcon_get_cable_state() is deprecated -> extcon_get_cable_state_() >> extcon_set_cable_state() is deprecated -> extcon_set_cable_state_() > > Sure. I will use the new interfaces with a refreshed patch serial later. > >> >> >> You can refer to usage for new function with unique id on patch[2] >> [2] 5960387a2fb83 (usb: dwc3: omap: Replace deprecated API of extcon) > > Thanks. That's helpful. > > By the way, is extcon_get_extcon_dev() still available? Yes. This function find the extcon device with string name. If extcon client driver use the platform_data to get the name of extcon device, the extcon client driver should use the extcon_get_extcon_dev() funtcion. Because we can not pass the point(instance) of extcon device through platform_data. But, if extcon client driver support the devicetree, we better to use the extcon_get_edev_by_phandle(). Best Regards, Chanwoo Choi > >> >> I'm sorry for late reply. I add the some comment on below. > > Never mind. Thank you for reminding me. > > Best regards, > Baolu > >> >> >> On 2016년 03월 17일 14:46, 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 >>> Reviewed-by: Heikki Krogerus >>> Reviewed-by: Felipe Balbi >>> --- >>> 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 | 180 +++++++++++++++++++++++++++ >>> include/linux/usb/intel-mux.h | 43 +++++++ >>> 8 files changed, 264 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 >>> +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 >>> +L: linux-usb@vger.kernel.org >>> +S: Supported >>> +F: include/linux/usb/intel-mux.h >>> +F: drivers/usb/mux/intel-mux.c >>> + >>> USB PRINTER DRIVER (usblp) >>> M: Pete Zaitcev >>> L: linux-usb@vger.kernel.org >>> 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..bb7b192 >>> --- /dev/null >>> +++ b/drivers/usb/mux/intel-mux.c >>> @@ -0,0 +1,180 @@ >>> +/** >>> + * intel_mux.c - USB Port Mux support >>> + * >>> + * Copyright (C) 2016 Intel Corporation >>> + * >>> + * Author: Lu Baolu >>> + * >>> + * 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 >>> +#include >>> +#include >>> +#include >>> + >>> +struct intel_usb_mux { >>> + struct device *dev; >>> + char *cable_name; >>> + int (*cable_set_cb)(struct device *dev); >>> + int (*cable_unset_cb)(struct device *dev); >>> + >>> + struct notifier_block nb; >>> + struct extcon_specific_cable_nb obj; >>> + >>> + /* >>> + * 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->obj.edev, >>> + mux->cable_name); >> Use the extcon_get_cable_stet_(). >> >>> + >>> + 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, >>> + char *extcon_name, >>> + char *cable_name, >>> + int (*cable_set_cb)(struct device *dev), >>> + int (*cable_unset_cb)(struct device *dev)) >>> +{ >>> + int ret; >>> + struct intel_usb_mux *mux; >>> + >>> + if (!cable_name) >>> + return -ENODEV; >>> + >>> + mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL); >>> + if (!mux) >>> + return -ENOMEM; >>> + >>> + mux->dev = dev; >>> + mux->cable_name = kstrdup(cable_name, GFP_KERNEL); >>> + 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_interest(&mux->obj, extcon_name, >>> + cable_name, &mux->nb); >> Use the extcon_register_notifier() >> >>> + if (ret) { >>> + kfree(mux->cable_name); >>> + 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_interest(&mux->obj); >> Use the extcon_unregister_notifier() >> >>> + kfree(mux->cable_name); >>> + 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_interest(&mux->obj); >> Use the extcon_unregister_notifier() >> >>> + kfree(mux->cable_name); >>> + >>> + 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..fd5612d >>> --- /dev/null >>> +++ b/include/linux/usb/intel-mux.h >>> @@ -0,0 +1,43 @@ >>> +/** >>> + * intel_mux.h - USB Port Mux definitions >>> + * >>> + * Copyright (C) 2016 Intel Corporation >>> + * >>> + * Author: Lu Baolu >>> + * >>> + * 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, char *extcon_name, >>> + char *cable_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 >>> +intel_usb_mux_bind_cable(struct device *dev, >>> + char *extcon_name, >>> + char *cable_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 */ >>> >> Best Regards, >> Chanwoo Choi >> > > >