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

From: Chanwoo Choi
Date: Thu Mar 17 2016 - 03:28:09 EST

Next message: Sherry Hurwitz: "[PATCH 1/1] linux-firmware: Update AMD microcode patch firmware"
Previous message: Maxime Ripard: "Re: [PATCH 1/2] spi: sun4i: add DMA support"
In reply to: Lu Baolu: "Re: [PATCH v4 3/7] usb: mux: add common code for Intel dual role port mux"
Next in thread: Lu Baolu: "Re: [PATCH v4 3/7] usb: mux: add common code for Intel dual role port mux"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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 <baolu.lu@xxxxxxxxxxxxxxx>
>>> Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
>>> Reviewed-by: Felipe Balbi <balbi@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxx>
>>> +Description:
>>> + In some platforms, a single USB port is shared between a USB host
>>> + controller and a device controller. A USB mux driver is needed to
>>> + handle the port mux. port_mux attribute shows and stores the mux
>>> + state.
>>> + For read:
>>> + 'peripheral' - mux switched to PERIPHERAL controller;
>>> + 'host' - mux switched to HOST controller.
>>> + For write:
>>> + 'peripheral' - mux will be switched to PERIPHERAL controller;
>>> + 'host' - mux will be switched to HOST controller.
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index da3e4d8..0dbee11 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -11399,6 +11399,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
>>> S: Maintained
>>> F: drivers/usb/phy/
>>>
>>> +USB PORT MUX DRIVER
>>> +M: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
>>> +L: linux-usb@xxxxxxxxxxxxxxx
>>> +S: Supported
>>> +F: include/linux/usb/intel-mux.h
>>> +F: drivers/usb/mux/intel-mux.c
>>> +
>>> USB PRINTER DRIVER (usblp)
>>> M: Pete Zaitcev <zaitcev@xxxxxxxxxx>
>>> L: linux-usb@xxxxxxxxxxxxxxx
>>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>>> index 8ed451d..dbd6620 100644
>>> --- a/drivers/usb/Kconfig
>>> +++ b/drivers/usb/Kconfig
>>> @@ -149,6 +149,8 @@ endif # USB
>>>
>>> source "drivers/usb/phy/Kconfig"
>>>
>>> +source "drivers/usb/mux/Kconfig"
>>> +
>>> source "drivers/usb/gadget/Kconfig"
>>>
>>> config USB_LED_TRIG
>>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>>> index d5c57f1..6433f0c 100644
>>> --- a/drivers/usb/Makefile
>>> +++ b/drivers/usb/Makefile
>>> @@ -6,6 +6,7 @@
>>>
>>> obj-$(CONFIG_USB) += core/
>>> obj-$(CONFIG_USB_SUPPORT) += phy/
>>> +obj-$(CONFIG_USB_SUPPORT) += mux/
>>>
>>> obj-$(CONFIG_USB_DWC3) += dwc3/
>>> obj-$(CONFIG_USB_DWC2) += dwc2/
>>> diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
>>> new file mode 100644
>>> index 0000000..62e2cc3
>>> --- /dev/null
>>> +++ b/drivers/usb/mux/Kconfig
>>> @@ -0,0 +1,12 @@
>>> +#
>>> +# USB port mux driver configuration
>>> +#
>>> +menu "USB Port MUX drivers"
>>> +config INTEL_USB_MUX
>>> + select EXTCON
>>> + def_bool n
>>> + help
>>> + Common code for all Intel dual role port mux drivers. All Intel
>>> + usb port mux drivers should select it.
>>> +
>>> +endmenu
>>> diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
>>> new file mode 100644
>>> index 0000000..84f0ae8
>>> --- /dev/null
>>> +++ b/drivers/usb/mux/Makefile
>>> @@ -0,0 +1,4 @@
>>> +#
>>> +# Makefile for USB port mux drivers
>>> +#
>>> +obj-$(CONFIG_INTEL_USB_MUX) += intel-mux.o
>>> diff --git a/drivers/usb/mux/intel-mux.c b/drivers/usb/mux/intel-mux.c
>>> new file mode 100644
>>> index 0000000..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@xxxxxxxxxxxxxxx>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +#include <linux/slab.h>
>>> +#include <linux/notifier.h>
>>> +#include <linux/extcon.h>
>>> +#include <linux/err.h>
>>> +
>>> +struct intel_usb_mux {
>>> + struct device *dev;
>>> + 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@xxxxxxxxxxxxxxx>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#ifndef __LINUX_USB_INTEL_MUX_H
>>> +#define __LINUX_USB_INTEL_MUX_H
>>> +
>>> +#if IS_ENABLED(CONFIG_INTEL_USB_MUX)
>>> +int intel_usb_mux_bind_cable(struct device *dev, 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
>>
>
>
>

Next message: Sherry Hurwitz: "[PATCH 1/1] linux-firmware: Update AMD microcode patch firmware"
Previous message: Maxime Ripard: "Re: [PATCH 1/2] spi: sun4i: add DMA support"
In reply to: Lu Baolu: "Re: [PATCH v4 3/7] usb: mux: add common code for Intel dual role port mux"
Next in thread: Lu Baolu: "Re: [PATCH v4 3/7] usb: mux: add common code for Intel dual role port mux"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]