lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56EA59BB.4020201@linux.intel.com>
Date:	Thu, 17 Mar 2016 15:16:11 +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

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?

>
> 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

Powered by Openwall GNU/*/Linux Powered by OpenVZ