[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <56EA60AF.2030401@linux.intel.com>
Date: Thu, 17 Mar 2016 15:45:51 +0800
From: Lu Baolu <baolu.lu@...ux.intel.com>
To: Chanwoo Choi <cw00.choi@...sung.com>,
Felipe Balbi <balbi@...nel.org>,
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>,
MyungJoo Ham <myungjoo.ham@...sung.com>
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/7] usb: mux: add common code for Intel dual role port
mux
On 03/17/2016 03:27 PM, Chanwoo Choi wrote:
> 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().
That's clear. Thank you!
Best regards,
Baolu
>
> 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 <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 | 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 <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. 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@...ux.intel.com>
>>>> +L: linux-usb@...r.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 <zaitcev@...hat.com>
>>>> L: linux-usb@...r.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 <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/notifier.h>
>>>> +#include <linux/extcon.h>
>>>> +#include <linux/err.h>
>>>> +
>>>> +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 <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_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
>>>
>>
>>
>
Powered by blists - more mailing lists