[PATCH v7 3/4] gadget: Integrate with the usb gadget supporting for usb charger

[Baolin Wang]

When the usb gadget supporting for usb charger is ready, the usb charger
should get the type by the 'get_charger_type' callback which is implemented
by the usb gadget operations, and get the usb charger pointer from struct
'usb_gadget'.

Signed-off-by: Baolin Wang 
---
 drivers/usb/gadget/charger.c |   43 --
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
index 82a9973..76e1a6f 100644
--- a/drivers/usb/gadget/charger.c
+++ b/drivers/usb/gadget/charger.c
@@ -272,7 +272,11 @@ EXPORT_SYMBOL_GPL(usb_charger_unregister_notify);
 enum usb_charger_type
 usb_charger_detect_type(struct usb_charger *uchger)
 {
-   if (uchger->psy) {
+   if (uchger->gadget && uchger->gadget->ops
+   && uchger->gadget->ops->get_charger_type) {
+   uchger->type =
+   uchger->gadget->ops->get_charger_type(uchger->gadget);
+   } else if (uchger->psy) {
union power_supply_propval val;
 
power_supply_get_property(uchger->psy,
@@ -479,6 +483,29 @@ usb_charger_plug_by_extcon(struct notifier_block *nb,
 int usb_charger_plug_by_gadget(struct usb_gadget *gadget,
   unsigned long state)
 {
+   struct usb_charger *uchger = gadget->charger;
+   enum usb_charger_state uchger_state;
+
+   if (!uchger)
+   return -EINVAL;
+
+   /* Report event to power to setting the current limitation
+* for this usb charger when one usb charger state is changed
+* with detecting by usb gadget state.
+*/
+   if (uchger->old_gadget_state != state) {
+   uchger->old_gadget_state = state;
+
+   if (state >= USB_STATE_ATTACHED)
+   uchger_state = USB_CHARGER_PRESENT;
+   else if (state == USB_STATE_NOTATTACHED)
+   uchger_state = USB_CHARGER_REMOVE;
+   else
+   uchger_state = USB_CHARGER_DEFAULT;
+
+   usb_charger_notify_others(uchger, uchger_state);
+   }
+
return 0;
 }
 EXPORT_SYMBOL_GPL(usb_charger_plug_by_gadget);
@@ -635,6 +662,7 @@ int usb_charger_init(struct usb_gadget *ugadget)
 
/* register a notifier on a usb gadget device */
uchger->gadget = ugadget;
+   ugadget->charger = uchger;
uchger->old_gadget_state = ugadget->state;
 
/* register a new usb charger */
@@ -655,7 +683,18 @@ fail:
 
 int usb_charger_exit(struct usb_gadget *ugadget)
 {
-   return 0;
+   struct usb_charger *uchger = ugadget->charger;
+
+   if (!uchger)
+   return -EINVAL;
+
+   if (uchger->extcon_dev)
+   extcon_unregister_notifier(uchger->extcon_dev,
+  EXTCON_USB, >extcon_nb.nb);
+
+   ida_simple_remove(_charger_ida, uchger->id);
+
+   return usb_charger_unregister(uchger);
 }
 
 static int __init usb_charger_sysfs_init(void)
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 1/4] gadget: Introduce the usb charger framework

[Baolin Wang]

This patch introduces the usb charger driver based on usb gadget that
makes an enhancement to a power driver. It works well in practice but
that requires a system with suitable hardware.

The basic conception of the usb charger is that, when one usb charger
is added or removed by reporting from the usb gadget state change or
the extcon device state change, the usb charger will report to power
user to set the current limitation.

The usb charger will register notifiees on the usb gadget or the extcon
device to get notified the usb charger state. It also supplies the
notification mechanism to userspace When the usb charger state is changed.

Power user will register a notifiee on the usb charger to get notified
by status changes from the usb charger. It will report to power user
to set the current limitation when detecting the usb charger is added
or removed from extcon device state or usb gadget state.

This patch doesn't yet integrate with the gadget code, so some functions
which rely on the 'gadget' are not completed, that will be implemented
in the following patches.

Signed-off-by: Baolin Wang 
---
 drivers/usb/gadget/Kconfig  |7 +
 drivers/usb/gadget/Makefile |1 +
 drivers/usb/gadget/charger.c|  669 +++
 include/linux/usb/usb_charger.h |  164 ++
 4 files changed, 841 insertions(+)
 create mode 100644 drivers/usb/gadget/charger.c
 create mode 100644 include/linux/usb/usb_charger.h

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 33834aa..8d69dca 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -127,6 +127,13 @@ config USB_GADGET_STORAGE_NUM_BUFFERS
   a module parameter as well.
   If unsure, say 2.
 
+config USB_CHARGER
+   bool "USB charger support"
+   help
+ The usb charger driver based on the usb gadget that makes an
+ enhancement to a power driver which can set the current limitation
+ when the usb charger is added or removed.
+
 source "drivers/usb/gadget/udc/Kconfig"
 
 #
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 598a67d..1e421c1 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -10,3 +10,4 @@ libcomposite-y:= usbstring.o config.o 
epautoconf.o
 libcomposite-y += composite.o functions.o configfs.o u_f.o
 
 obj-$(CONFIG_USB_GADGET)   += udc/ function/ legacy/
+obj-$(CONFIG_USB_CHARGER)  += charger.o
diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
new file mode 100644
index 000..82a9973
--- /dev/null
+++ b/drivers/usb/gadget/charger.c
@@ -0,0 +1,669 @@
+/*
+ * charger.c -- USB charger driver
+ *
+ * Copyright (C) 2015 Linaro Ltd.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DEFAULT_CUR_PROTECT(50)
+#define DEFAULT_SDP_CUR_LIMIT  (500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_DCP_CUR_LIMIT  (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_CDP_CUR_LIMIT  (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_ACA_CUR_LIMIT  (1500 - DEFAULT_CUR_PROTECT)
+#define UCHGER_STATE_LENGTH(50)
+
+static DEFINE_IDA(usb_charger_ida);
+static struct bus_type usb_charger_subsys = {
+   .name   = "usb-charger",
+   .dev_name   = "usb-charger",
+};
+
+static struct usb_charger *dev_to_uchger(struct device *udev)
+{
+   return container_of(udev, struct usb_charger, dev);
+}
+
+static ssize_t sdp_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+   struct usb_charger *uchger = dev_to_uchger(dev);
+
+   return sprintf(buf, "%d\n", uchger->cur_limit.sdp_cur_limit);
+}
+
+static ssize_t sdp_limit_store(struct device *dev,
+  struct device_attribute *attr,
+  const char *buf, size_t count)
+{
+   struct usb_charger *uchger = dev_to_uchger(dev);
+   unsigned int sdp_limit;
+   int ret;
+
+   ret = kstrtouint(buf, 10, _limit);
+   if (ret < 0)
+   return ret;
+
+   ret = usb_charger_set_cur_limit_by_type(uchger, SDP_TYPE, sdp_limit);
+   if (ret < 0)
+   return ret;
+
+   return count;
+}
+static DEVICE_ATTR_RW(sdp_limit);
+
+static ssize_t dcp_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+   struct usb_charger *uchger = dev_to_uchger(dev);
+
+   return sprintf(buf, "%d\n", uchger->cur_limit.dcp_cur_limit);
+}
+
+static ssize_t 

[PATCH v7 2/4] gadget: Support for the usb charger framework

[Baolin Wang]

For supporting the usb charger, it adds the usb_charger_init() and
usb_charger_exit() functions for usb charger initialization and exit.

It will report to the usb charger when the gadget state is changed,
then the usb charger can do the power things.

Introduce a callback 'get_charger_type' which will implemented by
user for usb gadget operations to get the usb charger type.

Signed-off-by: Baolin Wang 
---
 drivers/usb/gadget/udc/udc-core.c |   11 +++
 include/linux/usb/gadget.h|   11 +++
 2 files changed, 22 insertions(+)

diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index f660afb..2727f01 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * struct usb_udc - describes one usb device controller
@@ -226,6 +227,9 @@ static void usb_gadget_state_work(struct work_struct *work)
struct usb_gadget *gadget = work_to_gadget(work);
struct usb_udc *udc = gadget->udc;
 
+   /* when the gadget state is changed, then report to USB charger */
+   usb_charger_plug_by_gadget(gadget, gadget->state);
+
if (udc)
sysfs_notify(>dev.kobj, NULL, "state");
 }
@@ -405,8 +409,14 @@ int usb_add_gadget_udc_release(struct device *parent, 
struct usb_gadget *gadget,
 
mutex_unlock(_lock);
 
+   ret = usb_charger_init(gadget);
+   if (ret)
+   goto err5;
+
return 0;
 
+err5:
+   device_del(>dev);
 err4:
list_del(>list);
mutex_unlock(_lock);
@@ -481,6 +491,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
kobject_uevent(>dev.kobj, KOBJ_REMOVE);
flush_work(>work);
device_unregister(>dev);
+   usb_charger_exit(gadget);
device_unregister(>dev);
 }
 EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 3d583a1..52c19b1 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct usb_ep;
 
@@ -560,6 +561,7 @@ struct usb_gadget_ops {
struct usb_ep *(*match_ep)(struct usb_gadget *,
struct usb_endpoint_descriptor *,
struct usb_ss_ep_comp_descriptor *);
+   enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
 };
 
 /**
@@ -632,6 +634,8 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
struct usb_otg_caps *otg_caps;
+   /* negotiate the power with the usb charger */
+   struct usb_charger  *charger;
 
unsignedsg_supported:1;
unsignedis_otg:1;
@@ -836,10 +840,17 @@ static inline int usb_gadget_vbus_connect(struct 
usb_gadget *gadget)
  * reporting how much power the device may consume.  For example, this
  * could affect how quickly batteries are recharged.
  *
+ * It will also notify the USB charger how much power the device may
+ * consume if there is a USB charger linking with the gadget.
+ *
  * Returns zero on success, else negative errno.
  */
 static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
 {
+   if (gadget->charger)
+   usb_charger_set_cur_limit_by_type(gadget->charger,
+   usb_charger_detect_type(gadget->charger), mA);
+
if (!gadget->ops->vbus_draw)
return -EOPNOTSUPP;
return gadget->ops->vbus_draw(gadget, mA);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: misc: generic_onboard_hub: add generic onboard USB HUB driver

[Peter Chen]

On Mon, Dec 07, 2015 at 08:59:19PM -0600, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> > diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> > index 45fd4ac..da52e9a 100644
> > --- a/drivers/usb/misc/Makefile
> > +++ b/drivers/usb/misc/Makefile
> > @@ -29,3 +29,4 @@ obj-$(CONFIG_USB_CHAOSKEY)+= chaoskey.o
> >  
> >  obj-$(CONFIG_USB_SISUSBVGA)+= sisusbvga/
> >  obj-$(CONFIG_USB_LINK_LAYER_TEST)  += lvstest.o
> > +obj-$(CONFIG_USB_ONBOARD_HUB)  += generic_onboard_hub.o
> > diff --git a/drivers/usb/misc/generic_onboard_hub.c 
> > b/drivers/usb/misc/generic_onboard_hub.c
> > new file mode 100644
> > index 000..df722a0
> > --- /dev/null
> > +++ b/drivers/usb/misc/generic_onboard_hub.c
> > @@ -0,0 +1,162 @@
> > +/*
> > + * usb_hub_generic.c   The generic onboard USB HUB driver
> > + *
> > + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> > + * Author: Peter Chen 
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2  of
> > + * the License as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see .
> > + */
> > +
> > +/*
> > + * This driver is only for the USB HUB devices which need to control
> > + * their external pins(clock, reset, etc), and these USB HUB devices
> > + * are soldered at the board.
> > + */
> > +
> > +#define DEBUG
> 
> nope
> 

Will change.

> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> 
>  ?
> 

Yes, you are right, I did not know it

> > +struct usb_hub_generic_data {
> > +   struct clk *clk;
> 
> seriously ? Is this really all ? What about that reset line below ?

The clock is PHY input clock on the HUB, this clock may from SoC's PLL.
> 
> In fact, that reset line should be using a generic reset-gpio.c instead
> of a raw gpio.
> 

This reset gpio is not at mainline, I don't know what's reason

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/186693.html

Philipp, do you know the reason?

> > +static int usb_hub_generic_probe(struct platform_device *pdev)
> > +{
> > +   struct device *dev = >dev;
> > +   struct usb_hub_generic_platform_data *pdata = dev->platform_data;
> > +   struct usb_hub_generic_data *hub_data;
> > +   int reset_pol = 0, duration_us = 50, ret = 0;
> > +   struct gpio_desc *gpiod_reset = NULL;
> > +
> > +   hub_data = devm_kzalloc(dev, sizeof(*hub_data), GFP_KERNEL);
> > +   if (!hub_data)
> > +   return -ENOMEM;
> > +
> > +   if (dev->of_node) {
> > +   struct device_node *node = dev->of_node;
> > +
> > +   hub_data->clk = devm_clk_get(dev, "external_clk");
> > +   if (IS_ERR(hub_data->clk)) {
> > +   dev_dbg(dev, "Can't get external clock: %ld\n",
> > +   PTR_ERR(hub_data->clk));
> 
> how about setting clock to NULL to here ? then you don't need IS_ERR()
> checks anywhere else.
> 
> > +   }
> 
> braces shouldn't be used here, if you add NULL trick above,
> however... then you can keep them.
> 

Braces aren't needed, it may not too much useful to using NULL
as a indicator for error pointer.

> > +   /*
> > +* Try to get the information for HUB reset, the
> > +* default setting like below:
> > +*
> > +* - Reset state is low
> > +* - The duration is 50us
> > +*/
> > +   if (of_find_property(node, "hub-reset-active-high", NULL))
> > +   reset_pol = 1;
> 
> you see, this is definitely *not* generic. You should write a generic
> reset-gpio.c reset controller and describe the polarity on the gpio
> binding. This driver *always* uses reset_assert(); reset_deassert(); and
> reset-gpio implements though by gpiod_set_value() correctly.
> 
> Polarity _must_ be described elsewhere in DT.
> 
> > +   of_property_read_u32(node, "hub-reset-duration-us",
> > +   _us);
> 
> likewise, this should be described as a debounce time for the GPIO.
> 

Yes, if we are a reset gpio driver.

> > +   gpiod_reset = devm_gpiod_get_optional(dev, "hub-reset",
> > +   reset_pol ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> > +   ret = PTR_ERR_OR_ZERO(gpiod_reset);
> > +   if (ret) {
> > +   dev_err(dev, "Failed to get reset gpio, err = %d\n",
> > +   ret);

Re: [PATCH 1/3] usb: misc: generic_onboard_hub: add generic onboard USB HUB driver

[Sascha Hauer]

On Tue, Dec 08, 2015 at 05:26:56PM +0800, Peter Chen wrote:
> On Tue, Dec 08, 2015 at 07:19:05AM +0100, Sascha Hauer wrote:
> > > + hub_data = devm_kzalloc(dev, sizeof(*hub_data), GFP_KERNEL);
> > > + if (!hub_data)
> > > + return -ENOMEM;
> > > +
> > > + if (dev->of_node) {
> > > + struct device_node *node = dev->of_node;
> > > +
> > > + hub_data->clk = devm_clk_get(dev, "external_clk");
> > 
> > Please drop such _clk suffixes. The context already makes it clear that
> > it's a clock.
> > 
> 
> Will change.
> 
> > > + if (IS_ERR(hub_data->clk)) {
> > > + dev_dbg(dev, "Can't get external clock: %ld\n",
> > > + PTR_ERR(hub_data->clk));
> > > + }
> > > +
> > > + /*
> > > +  * Try to get the information for HUB reset, the
> > > +  * default setting like below:
> > > +  *
> > > +  * - Reset state is low
> > > +  * - The duration is 50us
> > > +  */
> > > + if (of_find_property(node, "hub-reset-active-high", NULL))
> > > + reset_pol = 1;
> > > +
> > > + of_property_read_u32(node, "hub-reset-duration-us",
> > > + _us);
> > > +
> > > + gpiod_reset = devm_gpiod_get_optional(dev, "hub-reset",
> > > + reset_pol ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> > > + ret = PTR_ERR_OR_ZERO(gpiod_reset);
> > > + if (ret) {
> > > + dev_err(dev, "Failed to get reset gpio, err = %d\n",
> > > + ret);
> > > + return ret;
> > > + }
> > > + } else if (pdata) {
> > > + hub_data->clk = pdata->ext_clk;
> > 
> > Passing clocks in platform_data is a no go. clocks must always be
> > retrieved with clk_get.
> 
> Yes, you are right.
> 
> > 
> > > + duration_us = pdata->gpio_reset_duration_us;
> > > + reset_pol = pdata->gpio_reset_polarity;
> > > +
> > > + if (gpio_is_valid(pdata->gpio_reset)) {
> > > + ret = devm_gpio_request_one(
> > > + dev, pdata->gpio_reset, reset_pol
> > > + ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
> > > + dev_name(dev));
> > > + if (!ret)
> > > + gpiod_reset = gpio_to_desc(pdata->gpio_reset);
> > 
> > If the gpio is valid then being unable to get it is an error.
> 
> I can't catch you, if the gpio is invalid, its descriptor will be NULL,
> then, the code will not do any gpio operation.

When the platform_data provides a gpio (gpio_is_valid() is true) then
you must use the gpio. If then devm_gpio_request_one() fails you must
bail out.

> 
> > 
> > Do you need this platform_data stuff at all?
> > 
> 
> If not, how can I cover the platform which does not use dts, but still
> uses these kinds of HUBs?

You can't, but are there any non device tree platforms which want to use
this driver?

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: dsps: handle the otg_state_a_wait_vrise_timeout case

[Gregory CLEMENT]

Hi Felipe,
 
 On lun., déc. 07 2015, Felipe Balbi  wrote:

> Hi,
>
> Gregory CLEMENT  writes:
>> Hi Felipe,
>>
>> I am going back on this subject (again :) )
>>  
>>  On mar., oct. 20 2015, Gregory CLEMENT  
>> wrote:
>>
>>> Hi Felipe,
>>>  
>>>  On lun., oct. 05 2015, Felipe Balbi  wrote:
>>>
>>>
> So after many tests on different devices, 200ms is enough for most of
> them, but for one, 2000ms (2s) was needed!
>
> I see several option:
> - adding a sysfs entry to setup the time
> - adding a debugs entry entry
> - adding configuration option in menuconfig
> - using 2000ms and hopping it was enough for everyone
>
> My preference would go to the first option, what is your opinion?

 I'm not sure if either of them is good. man 2s is just too large. If the
>>
>> Well 2s is lon I agree, but currently instead of 2s we have an infinite
>> wait, which is worse!
>>
 device isn't following the specification, I'm afraid that device needs
 to be fixed.
>>
>> I agree but these devices are for most of them USB stick that we find in
>> retail. We have no influence on them, so we have to do with them, even
>> if they do not follow the sepc.
>>
>> So what about using configfs or sysfs to let the user confgiure this
>> value if needed?
>
> iff we have to; I'm still not 100% convinced.
>
>> I go back on this because, your suggestion didn't work. At least, I
>> didn't manage to make it improve the situation.
>>
>> Thanks,
>>

 I think the real issue here is the lack of a disconnect IRQ from AM335x
 devices. But here's something I've been meaning to test but never had
 time. If you look into AM335x address space, there's a bit in the
 wrapper which tells it to use the standard MUSB registers for IRQ,
 instead of the TI-specific thing. Can you see if we get a disconnect IRQ
 with that ?

 TRM is at [1], see page 2566. Basically, if you set bit 3 in register
 offset 0x1014, then it should use Mentor IRQ registers. If that works,
 quite a few problems will actually vanish :-p
>>>
>>> So I tried it with the following patch:
>>>
>>> -
>>> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
>>> index c791ba5..c714500 100644
>>> --- a/drivers/usb/musb/musb_dsps.c
>>> +++ b/drivers/usb/musb/musb_dsps.c
>>> @@ -470,6 +470,7 @@ static int dsps_musb_init(struct musb *musb)
>>>  
>>> /* Reset the musb */
>>> dsps_writel(reg_base, wrp->control, (1 << wrp->reset));
>>> +   dsps_writel(musb->ctrl_base, wrp->control, BIT(3));
>
> overwritting reset.
>
>>> musb->isr = dsps_interrupt;
>>>  
>>> @@ -625,6 +626,7 @@ static int dsps_musb_reset(struct musb *musb)
>>> if (session_restart || !glue->sw_babble_enabled) {
>>> dev_info(musb->controller, "Restarting MUSB to recover from 
>>> Babble\n");
>>> dsps_writel(musb->ctrl_base, wrp->control, (1 << 
>>> wrp->reset));
>>> +   dsps_writel(musb->ctrl_base, wrp->control, BIT(3));
>
> here too. No wonder it won't work, right :-)
>
>>> I am not very familiar with that driver but my understanding was that
>>> the Mentor IRQ registeres are managed by the musb_interrupt() function
>>> which is called from the dsps_interrupt() interrupt handler.
>>>
>>> Am I right?
>
> yeah, however the way IRQs are reported is a different thing. AM335x
> introduced its own IRQ reporting scheme which, basically, reads MUSB
> generic IRQ status registers and reports on an AM335x specific
> register. No idea why TI did that for AM335x devices.
>
>>> if it is the case then it didn't fix the issue I had.
>>>
>>> I activated the following debug line:
>>>
>>> [musb_hdrc]musb_interrupt =_ "** IRQ %s usb%04x tx%04x rx%04x\012"
>>> [musb_dsps]dsps_interrupt =p "usbintr (%x) epintr(%x)\012"
>>>
>>> But I didn't get any interrupt while disconnecting the cable without any
>>> device connected on it (whereas I got an interrupt when I connected it).
>>>
>>> Note that I applied this patch instead of the "usb: musb: dsps: handle
>>> the otg_state_a_wait_vrise_timeout case", is what you had in mind ?
>
> yeah, that's what I had in mind. But your patch seems wrong :-)
>
> I tried writing a more correct version here and found 2 issues:
>
> a) bit 3 doesn't do anything :-p I cannot read IRQs from mentor's
> registers
>
> b) when setting RESET_ISOLATION bit, reads of CTRL register hang. Note
> that according to TRM, RESET_ISOLATION _must_ be set prior to a soft
> reset and cleared afterwards. But right after setting RESET_ISOLATION,
> if I try a read of CTRL, it'll hang forever.

The datasheet seems not very coherent about it,

on one side we have:
"This bit should be set high prior to setting bit 0 and cleared after bit 0
is cleared."

and on the other side:
"Both the soft_reset and soft_reset_isolation bits 

Re: [PATCH 2/3] doc: dt-binding: generic onboard USB HUB

[Arnd Bergmann]

On Tuesday 08 December 2015 17:20:46 Peter Chen wrote:
> On Tue, Dec 08, 2015 at 12:30:59AM -0200, Fabio Estevam wrote:
> > On Mon, Dec 7, 2015 at 11:37 PM, Peter Chen  
> > wrote:
> > > Add dt-binding documentation for generic onboard USB HUB.
> > >
> > > Signed-off-by: Peter Chen 
> > > ---
> > >  .../bindings/usb/generic-onboard-hub.txt   | 28 
> > > ++
> > >  1 file changed, 28 insertions(+)
> > >  create mode 100644 
> > > Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > >
> > > diff --git 
> > > a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt 
> > > b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > new file mode 100644
> > > index 000..ea92205
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > @@ -0,0 +1,28 @@
> > > +Generic Onboard USB HUB
> > > +
> > > +Required properties:
> > > +- compatible: should be "generic-onboard-hub"
> > > +
> > > +Optional properties:
> > > +- clocks: the input clock for HUB.
> > > +
> > > +- clock-names: Should be "external_clk"

Make this just "external", or remove the name and use it as an anonymous clock.

> > > +- hub-reset-gpios: Should specify the GPIO for reset.
> > > +
> > > +- hub-reset-active-high: the active reset signal is high, if this 
> > > property is
> > > +  not set, the active reset signal is low.
> > > +
> > > +- hub-reset-duration-us: the duration for assert reset signal, the time 
> > > unit
> > > +  is microsecond.

Remove the "hub-" prefix here.

> > > +Example:
> > > +
> > > +   usb_hub1 {
> > > +   compatible = "generic-onboard-hub";
> > > +   clocks = < IMX6QDL_CLK_CKO>;
> > > +   clock-names = "external_clk";
> > > +   hub-reset-gpios = < 12 0>;
> > > +   hub-reset-active-high;
> > 
> > I think you could drop the 'hub-reset-active-high' property and do
> > like this instead:
> > 
> > hub-reset-gpios = < 12 GPIO_ACTIVE_HIGH>;
> > 
> > or
> > 
> > hub-reset-gpios = < 12 GPIO_ACTIVE_LOW>;
> 
> Yes, you are right, would you have a test to see if can work for
> udoo board?

Yes, it should be done like this.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: misc: generic_onboard_hub: add generic onboard USB HUB driver

[Peter Chen]

On Tue, Dec 08, 2015 at 07:19:05AM +0100, Sascha Hauer wrote:
> > +   hub_data = devm_kzalloc(dev, sizeof(*hub_data), GFP_KERNEL);
> > +   if (!hub_data)
> > +   return -ENOMEM;
> > +
> > +   if (dev->of_node) {
> > +   struct device_node *node = dev->of_node;
> > +
> > +   hub_data->clk = devm_clk_get(dev, "external_clk");
> 
> Please drop such _clk suffixes. The context already makes it clear that
> it's a clock.
> 

Will change.

> > +   if (IS_ERR(hub_data->clk)) {
> > +   dev_dbg(dev, "Can't get external clock: %ld\n",
> > +   PTR_ERR(hub_data->clk));
> > +   }
> > +
> > +   /*
> > +* Try to get the information for HUB reset, the
> > +* default setting like below:
> > +*
> > +* - Reset state is low
> > +* - The duration is 50us
> > +*/
> > +   if (of_find_property(node, "hub-reset-active-high", NULL))
> > +   reset_pol = 1;
> > +
> > +   of_property_read_u32(node, "hub-reset-duration-us",
> > +   _us);
> > +
> > +   gpiod_reset = devm_gpiod_get_optional(dev, "hub-reset",
> > +   reset_pol ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> > +   ret = PTR_ERR_OR_ZERO(gpiod_reset);
> > +   if (ret) {
> > +   dev_err(dev, "Failed to get reset gpio, err = %d\n",
> > +   ret);
> > +   return ret;
> > +   }
> > +   } else if (pdata) {
> > +   hub_data->clk = pdata->ext_clk;
> 
> Passing clocks in platform_data is a no go. clocks must always be
> retrieved with clk_get.

Yes, you are right.

> 
> > +   duration_us = pdata->gpio_reset_duration_us;
> > +   reset_pol = pdata->gpio_reset_polarity;
> > +
> > +   if (gpio_is_valid(pdata->gpio_reset)) {
> > +   ret = devm_gpio_request_one(
> > +   dev, pdata->gpio_reset, reset_pol
> > +   ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
> > +   dev_name(dev));
> > +   if (!ret)
> > +   gpiod_reset = gpio_to_desc(pdata->gpio_reset);
> 
> If the gpio is valid then being unable to get it is an error.

I can't catch you, if the gpio is invalid, its descriptor will be NULL,
then, the code will not do any gpio operation.

> 
> Do you need this platform_data stuff at all?
> 

If not, how can I cover the platform which does not use dts, but still
uses these kinds of HUBs?

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Celot modem driver

[Johan Hovold]

On Wed, Nov 25, 2015 at 09:54:58AM -0600, Dan Williams wrote:
> On Wed, 2015-11-25 at 13:13 +0100, M. Hrdlička wrote:
> > Hello,
> > log script told me, that I may to tell you about add this device to
> > propper linux driver.
> > 
> > Device is combo USB modem Celot CTD-200, manufacturing date in
> > September 2010, it´s combinated GSM (GPRS/UMTS/HSPA) and CDMA modem.
> > I run this device with usbserial driver, adding this line in
> > /etc/udev/rules.d/90-celot-combo.rules:
> > ACTION=="add", SUBSYSTEM=="usb", ATTR{idVendor}=="211f",
> > ATTR{idProduct}=="6805", RUN+="/sbin/modprobe usbserial vendor=0x211f
> > product=0x6805"
> > ACTION=="add", SUBSYSTEM=="usb", ATTR{idVendor}=="211f",
> > ATTR{idProduct}=="6809", RUN+="/sbin/modprobe usbserial vendor=0x211f
> > product=0x6809"
> > 
> > In windows driver and connect program, there is choice to switch
> > between GSM and CDMA network mode. In my linux instalation only one
> > mode working - preferred is GSM mode. When I commented GSM line in
> > /etc/udev/rules.d (id 6809), then CDMA works and ModemManager make
> > ttyUSB ports.
> > 
> > I have OpenSuse 13.1 with standard kernel 3.11.10-29
> > 
> > Attached extract of /var/log/messages and lsusb command.
> > 
> 
> The modem appears to have two Qualcomm chips in it, one for CDMA and
> the other for GSM/WCDMA, but this is not a Gobi-type modem.  The
> connection manager appears to use QCDM to talk to the modem on Windows,
> but normal AT commands on Mac OS X.  The Windows CM has some mechanism
> for switching between the two modes and will even tell you if it can
> find a faster HSPA link while you're on CDMA.
> 
> In any case, I think the 'option' driver is most appropriate for this
> device's two USB IDs.  Communication traces of the Windows CM switching
> between this device's modes would be necessary to figure out switching
> between CDMA and GSM/WCDMA on Linux.

Does anyone care to write up a patch for this?

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] doc: dt-binding: generic onboard USB HUB

[Peter Chen]

On Tue, Dec 08, 2015 at 12:30:59AM -0200, Fabio Estevam wrote:
> On Mon, Dec 7, 2015 at 11:37 PM, Peter Chen  wrote:
> > Add dt-binding documentation for generic onboard USB HUB.
> >
> > Signed-off-by: Peter Chen 
> > ---
> >  .../bindings/usb/generic-onboard-hub.txt   | 28 
> > ++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> >
> > diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt 
> > b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > new file mode 100644
> > index 000..ea92205
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > @@ -0,0 +1,28 @@
> > +Generic Onboard USB HUB
> > +
> > +Required properties:
> > +- compatible: should be "generic-onboard-hub"
> > +
> > +Optional properties:
> > +- clocks: the input clock for HUB.
> > +
> > +- clock-names: Should be "external_clk"
> > +
> > +- hub-reset-gpios: Should specify the GPIO for reset.
> > +
> > +- hub-reset-active-high: the active reset signal is high, if this property 
> > is
> > +  not set, the active reset signal is low.
> > +
> > +- hub-reset-duration-us: the duration for assert reset signal, the time 
> > unit
> > +  is microsecond.
> > +
> > +Example:
> > +
> > +   usb_hub1 {
> > +   compatible = "generic-onboard-hub";
> > +   clocks = < IMX6QDL_CLK_CKO>;
> > +   clock-names = "external_clk";
> > +   hub-reset-gpios = < 12 0>;
> > +   hub-reset-active-high;
> 
> I think you could drop the 'hub-reset-active-high' property and do
> like this instead:
> 
> hub-reset-gpios = < 12 GPIO_ACTIVE_HIGH>;
> 
> or
> 
> hub-reset-gpios = < 12 GPIO_ACTIVE_LOW>;

Yes, you are right, would you have a test to see if can work for
udoo board?

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: misc: generic_onboard_hub: add generic onboard USB HUB driver

[Peter Chen]

On Tue, Dec 08, 2015 at 11:16:31AM +0800, kbuild test robot wrote:
> Hi Peter,
> 
> [auto build test ERROR on usb/usb-testing]
> [also build test ERROR on v4.4-rc4 next-20151207]
> 
> url:
> https://github.com/0day-ci/linux/commits/Peter-Chen/USB-add-generic-onboard-USB-HUB-driver/20151208-094428
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
> usb-testing
> config: tile-allmodconfig (attached as .config)
> reproduce:
> wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>  -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=tile 
> 
> All error/warnings (new ones prefixed by >>):
> 
>drivers/usb/misc/generic_onboard_hub.c: In function 
> 'usb_hub_generic_probe':
> >> drivers/usb/misc/generic_onboard_hub.c:76:3: error: implicit declaration 
> >> of function 'devm_gpiod_get_optional'
> >> drivers/usb/misc/generic_onboard_hub.c:77:16: error: 'GPIOD_OUT_HIGH' 
> >> undeclared (first use in this function)
>drivers/usb/misc/generic_onboard_hub.c:77:16: note: each undeclared 
> identifier is reported only once for each function it appears in
> >> drivers/usb/misc/generic_onboard_hub.c:77:33: error: 'GPIOD_OUT_LOW' 
> >> undeclared (first use in this function)
> >> drivers/usb/misc/generic_onboard_hub.c:95:5: error: implicit declaration 
> >> of function 'gpio_to_desc'
> >> drivers/usb/misc/generic_onboard_hub.c:95:17: warning: assignment makes 
> >> pointer from integer without a cast [enabled by default]
> >> drivers/usb/misc/generic_onboard_hub.c:114:3: error: implicit declaration 
> >> of function 'gpiod_set_value'
>cc1: some warnings being treated as errors
> 
> vim +/devm_gpiod_get_optional +76 drivers/usb/misc/generic_onboard_hub.c
> 
> 70if (of_find_property(node, 
> "hub-reset-active-high", NULL))
> 71reset_pol = 1;
> 72
> 73of_property_read_u32(node, 
> "hub-reset-duration-us",
> 74_us);
> 75
>   > 76gpiod_reset = devm_gpiod_get_optional(dev, 
> "hub-reset",
>   > 77reset_pol ? GPIOD_OUT_HIGH : 
> GPIOD_OUT_LOW);
> 78ret = PTR_ERR_OR_ZERO(gpiod_reset);
> 79if (ret) {
> 80dev_err(dev, "Failed to get reset gpio, 
> err = %d\n",
> 81ret);
> 82return ret;
> 83}
> 84} else if (pdata) {
> 85hub_data->clk = pdata->ext_clk;
> 86duration_us = pdata->gpio_reset_duration_us;
> 87reset_pol = pdata->gpio_reset_polarity;
> 88
> 89if (gpio_is_valid(pdata->gpio_reset)) {
> 90ret = devm_gpio_request_one(
> 91dev, pdata->gpio_reset, 
> reset_pol
> 92? GPIOF_OUT_INIT_HIGH : 
> GPIOF_OUT_INIT_LOW,
> 93dev_name(dev));
> 94if (!ret)
>   > 95gpiod_reset = 
> gpio_to_desc(pdata->gpio_reset);
> 96}
> 97}
> 98
> 99if (!IS_ERR(hub_data->clk)) {
>100ret = clk_prepare_enable(hub_data->clk);
>101if (ret) {
>102dev_err(dev,
>103"Can't enable external clock: 
> %d\n",
>104ret);
>105return ret;
>106}
>107}
>108
>109if (gpiod_reset) {
>110/* Sanity check */
>111if (duration_us > 100)
>112duration_us = 50;
>113usleep_range(duration_us, duration_us + 100);
>  > 114gpiod_set_value(gpiod_reset, reset_pol ? 0 : 1);
>115}
>116
>117dev_set_drvdata(dev, hub_data);
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

I will add  at header file list.


-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: misc: generic_onboard_hub: add generic onboard USB HUB driver

[Arnd Bergmann]

On Tuesday 08 December 2015 09:37:48 Peter Chen wrote:

> +struct usb_hub_generic_data {
> + struct clk *clk;
> +};
> +
> +static int usb_hub_generic_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + struct usb_hub_generic_platform_data *pdata = dev->platform_data;
> + struct usb_hub_generic_data *hub_data;
> + int reset_pol = 0, duration_us = 50, ret = 0;
> + struct gpio_desc *gpiod_reset = NULL;
> +
> + hub_data = devm_kzalloc(dev, sizeof(*hub_data), GFP_KERNEL);
> + if (!hub_data)
> + return -ENOMEM;
> +
> + if (dev->of_node) {

Let's not worry about the !DT case until someone adds a board file
that needs it. Just remove the if() here along and the whole else
block.

> +#ifdef CONFIG_OF
> +static const struct of_device_id usb_hub_generic_dt_ids[] = {
> + {.compatible = "generic-onboard-hub"},
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, usb_hub_generic_dt_ids);
> +#endif
> +
> +static struct platform_driver usb_hub_generic_driver = {
> + .probe = usb_hub_generic_probe,
> + .remove = usb_hub_generic_remove,
> + .driver = {
> + .name = "usb_hub_generic_onboard",
> + .of_match_table = usb_hub_generic_dt_ids,
> +  },
> +};

Build error when CONFIG_OF is disabled: Please remove the #ifdef around the 
device
table.

> diff --git a/include/linux/usb/generic_onboard_hub.h 
> b/include/linux/usb/generic_onboard_hub.h
> new file mode 100644
> index 000..1b70ccc
> --- /dev/null
> +++ b/include/linux/usb/generic_onboard_hub.h
> @@ -0,0 +1,11 @@
> +#ifndef __LINUX_USB_GENERIC_HUB_H
> +#define __LINUX_USB_GENERIC_HUB_H
> +
> +struct usb_hub_generic_platform_data {
> + int gpio_reset;
> + int gpio_reset_polarity;
> + int gpio_reset_duration_us;
> + struct clk *ext_clk;
> +};

Merge this structure into struct usb_hub_generic_data and remove the header.

ARnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Kuninori Morimoto]

Hi Simon

> > > Add fallback compatibility string.
> > > This is in keeping with the fallback scheme being adopted wherever
> > > appropriate for drivers for Renesas SoCs.
> > > 
> > > Signed-off-by: Simon Horman 
> > > ---
> > (snip)
> > > + {
> > > + .compatible = "renesas,usbhs",
> > > + .data = (void *)USBHS_TYPE_RCAR_GEN2,
> > > + },
> > >   { },
> > >  };
> > 
> > I think this is too much. This driver is used not only from R-Car Gen2.
> > It will work as normal mode if .data was 0.
> > see usbhs_parse_dt()
> 
> Are you suggesting that we remove USBHS_TYPE_RCAR_GEN2 from the driver?

I mean this

+   {
+   .compatible = "renesas,usbhs",
+   },

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] doc: dt-binding: generic onboard USB HUB

[Philipp Zabel]

Hi Peter,

Am Dienstag, den 08.12.2015, 09:37 +0800 schrieb Peter Chen:
> Add dt-binding documentation for generic onboard USB HUB.
> 
> Signed-off-by: Peter Chen 
> ---
>  .../bindings/usb/generic-onboard-hub.txt   | 28 
> ++
>  1 file changed, 28 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt 
> b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> new file mode 100644
> index 000..ea92205
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> @@ -0,0 +1,28 @@
> +Generic Onboard USB HUB
>+
> +Required properties:
> +- compatible: should be "generic-onboard-hub"

This something we don't have to define ad-hoc. The hub hangs off an USB
controller, right? The "Open Firmware recommended practice: USB"
document already describes how to represent USB devices in a generic
manner:
http://www.firmware.org/1275/bindings/usb/usb-1_0.ps

Is there a reason not to reuse this?

The usb hub node would be a child of the usb controller node, and it
could use
compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */


> +Optional properties:
> +- clocks: the input clock for HUB.
> +
> +- clock-names: Should be "external_clk"

Is clock-names necessary for a single clock?

> +- hub-reset-gpios: Should specify the GPIO for reset.

I'd prefer that to be just "reset-gpios", it is the only reset property
in the hub node after all. And use the GPIO_ACTIVE_HIGH/LOW flags to
indicate polarity.

> +- hub-reset-active-high: the active reset signal is high, if this property is
> +  not set, the active reset signal is low.

Then this could be dropped.

> +- hub-reset-duration-us: the duration for assert reset signal, the time unit
> +  is microsecond.

And consequently, this could just be called "reset-duration-us".
It might make sense to define the same for other reset GPIOs, too.
The Freescale FEC, for example, has "phy-reset-duration" (in ms)
already.

> +
> +Example:
> +
> + usb_hub1 {
> + compatible = "generic-onboard-hub";
> + clocks = < IMX6QDL_CLK_CKO>;
> + clock-names = "external_clk";
> + hub-reset-gpios = < 12 0>;
> + hub-reset-active-high;
> + hub-reset-duration-us = <2>;
> + };

best regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 4/4] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

Integrate with the newly added USB charger interface to limit the current
we draw from the USB input based on the input device configuration
identified by the USB stack, allowing us to charge more quickly from high
current inputs without drawing more current than specified from others.

Signed-off-by: Mark Brown 
Signed-off-by: Baolin Wang 
Acked-by: Lee Jones 
Acked-by: Charles Keepax 
Acked-by: Peter Chen 
Acked-by: Sebastian Reichel 
---
 drivers/power/wm831x_power.c |   69 ++
 include/linux/mfd/wm831x/pdata.h |3 ++
 2 files changed, 72 insertions(+)

diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
index 7082301..043f1f4 100644
--- a/drivers/power/wm831x_power.c
+++ b/drivers/power/wm831x_power.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -31,6 +32,8 @@ struct wm831x_power {
char usb_name[20];
char battery_name[20];
bool have_battery;
+   struct usb_charger *usb_charger;
+   struct notifier_block usb_notify;
 };
 
 static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 
+/* In milliamps */
+static unsigned int wm831x_usb_limits[] = {
+   0,
+   2,
+   100,
+   500,
+   900,
+   1500,
+   1800,
+   550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+  unsigned long limit, void *data)
+{
+   struct wm831x_power *wm831x_power = container_of(nb,
+struct wm831x_power,
+usb_notify);
+   int i, best;
+
+   /* Find the highest supported limit */
+   best = 0;
+   for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+   if (limit >= wm831x_usb_limits[i] &&
+   wm831x_usb_limits[best] < wm831x_usb_limits[i])
+   best = i;
+   }
+
+   dev_dbg(wm831x_power->wm831x->dev,
+   "Limiting USB current to %dmA", wm831x_usb_limits[best]);
+
+   wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+   WM831X_USB_ILIM_MASK, best);
+
+   return 0;
+}
+
 /*
  * Battery properties
  */
@@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device *pdev)
}
}
 
+   if (wm831x_pdata && wm831x_pdata->usb_gadget) {
+   power->usb_charger =
+   usb_charger_find_by_name(wm831x_pdata->usb_gadget);
+   if (IS_ERR(power->usb_charger)) {
+   ret = PTR_ERR(power->usb_charger);
+   dev_err(>dev,
+   "Failed to find USB gadget: %d\n", ret);
+   goto err_bat_irq;
+   }
+
+   power->usb_notify.notifier_call = wm831x_usb_limit_change;
+
+   ret = usb_charger_register_notify(power->usb_charger,
+ >usb_notify);
+   if (ret != 0) {
+   dev_err(>dev,
+   "Failed to register notifier: %d\n", ret);
+   goto err_usb_charger;
+   }
+   }
+
return ret;
 
+err_usb_charger:
+   /* put_device on charger */
 err_bat_irq:
--i;
for (; i >= 0; i--) {
@@ -637,6 +700,12 @@ static int wm831x_power_remove(struct platform_device 
*pdev)
struct wm831x *wm831x = wm831x_power->wm831x;
int irq, i;
 
+   if (wm831x_power->usb_charger) {
+   usb_charger_unregister_notify(wm831x_power->usb_charger,
+ _power->usb_notify);
+   /* Free charger */
+   }
+
for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
irq = wm831x_irq(wm831x, 
 platform_get_irq_byname(pdev,
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631..5af8399 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -126,6 +126,9 @@ struct wm831x_pdata {
/** The driver should initiate a power off sequence during shutdown */
bool soft_shutdown;
 
+   /** dev_name of USB charger gadget to integrate with */
+   const char *usb_gadget;
+
int irq_base;
int gpio_base;
int gpio_defaults[WM831X_GPIO_NUM];
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body 

[PATCH v7 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

Currently the Linux kernel does not provide any standard integration of this
feature that integrates the USB subsystem with the system power regulation
provided by PMICs meaning that either vendors must add this in their kernels
or USB gadget devices based on Linux (such as mobile phones) may not behave
as they should. Thus provide a standard framework for doing this in kernel.

Now introduce one user with wm831x_power to support and test the usb charger,
which is pending testing. Moreover there may be other potential users will use
it in future.

Changes since v5:
 - Remove the notifier chain things from the gadget and introduce one callback
 function to report to the usb charger when the gadget state is changed.
 - Flesh out the port type detection which combines the USB negotiation and
 PMICs detection.
 - Supply the notification mechanism to userspace when charger state is changed.
 - Integrate with the vbus staff in the gadget API.
 - Spilt up the functionality for userspace with one file per USB charger type.

Baolin Wang (4):
  gadget: Introduce the usb charger framework
  gadget: Support for the usb charger framework
  gadget: Integrate with the usb gadget supporting for usb charger
  power: wm831x_power: Support USB charger current limit management

 drivers/power/wm831x_power.c  |   69 
 drivers/usb/gadget/Kconfig|7 +
 drivers/usb/gadget/Makefile   |1 +
 drivers/usb/gadget/charger.c  |  708 +
 drivers/usb/gadget/udc/udc-core.c |   11 +
 include/linux/mfd/wm831x/pdata.h  |3 +
 include/linux/usb/gadget.h|   11 +
 include/linux/usb/usb_charger.h   |  164 +
 8 files changed, 974 insertions(+)
 create mode 100644 drivers/usb/gadget/charger.c
 create mode 100644 include/linux/usb/usb_charger.h

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: Use memdup_user to reuse the code

[Pathak, Rahul (R.)]

From: Rahul Pathak 

Fixing coccicheck warning which recommends to use memdup_user instead
to reimplement its code, using memdup_user simplifies the code

./drivers/usb/core/devio.c:1398:11-18: WARNING opportunity for memdup_user

Signed-off-by: Rahul Pathak 
---
 drivers/usb/core/devio.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 38ae877c..05266f0 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1395,11 +1395,9 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
struct usbdevfs_urb *uurb
number_of_packets = uurb->number_of_packets;
isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
   number_of_packets;
-   isopkt = kmalloc(isofrmlen, GFP_KERNEL);
-   if (!isopkt)
-   return -ENOMEM;
-   if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
-   ret = -EFAULT;
+   isopkt = memdup_user(iso_frame_desc, isofrmlen);
+   if (IS_ERR(isopkt)) {
+   ret = PTR_ERR(isopkt);
goto error;
}
for (totlen = u = 0; u < number_of_packets; u++) {
-- 
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] r8152: fix lockup when runtime PM is enabled

[Peter Wu]

When an interface is brought up which was previously suspended (via
runtime PM), it would hang. This happens because napi_disable is called
before napi_enable.

Solve this by avoiding napi_enable in the resume during open function
(netif_running is true when open is called, IFF_UP is set after a
successful open; netif_running is false when close is called, but IFF_UP
is then still set).

While at it, remove WORK_ENABLE check from rtl8152_open (introduced with
the original change) because it cannot happen:

 - After this patch, runtime resume will not set it during rtl8152_open.
 - When link is up, rtl8152_open is not called.
 - When link is down during system/auto suspend/resume, it is not set.

Fixes: 41cec84cf285 ("r8152: don't enable napi before rx ready")
Link: https://lkml.kernel.org/r/20151205105912.GA1766@al
Signed-off-by: Peter Wu 
---
 v2: moved rtl_runtime_suspend_enable from close to rtl8152_suspend

Tested with the scenario from
https://lkml.kernel.org/r/20151208111008.GA18728@al
(added debug prints to verify the suspend/resume moments)
---
 drivers/net/usb/r8152.c | 21 +++--
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index d9427ca..2e32c41 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3067,17 +3067,6 @@ static int rtl8152_open(struct net_device *netdev)
 
mutex_lock(>control);
 
-   /* The WORK_ENABLE may be set when autoresume occurs */
-   if (test_bit(WORK_ENABLE, >flags)) {
-   clear_bit(WORK_ENABLE, >flags);
-   usb_kill_urb(tp->intr_urb);
-   cancel_delayed_work_sync(>schedule);
-
-   /* disable the tx/rx, if the workqueue has enabled them. */
-   if (netif_carrier_ok(netdev))
-   tp->rtl_ops.disable(tp);
-   }
-
tp->rtl_ops.up(tp);
 
rtl8152_set_speed(tp, AUTONEG_ENABLE,
@@ -3124,12 +3113,6 @@ static int rtl8152_close(struct net_device *netdev)
} else {
mutex_lock(>control);
 
-   /* The autosuspend may have been enabled and wouldn't
-* be disable when autoresume occurs, because the
-* netif_running() would be false.
-*/
-   rtl_runtime_suspend_enable(tp, false);
-
tp->rtl_ops.down(tp);
 
mutex_unlock(>control);
@@ -3512,7 +3495,7 @@ static int rtl8152_resume(struct usb_interface *intf)
netif_device_attach(tp->netdev);
}
 
-   if (netif_running(tp->netdev)) {
+   if (netif_running(tp->netdev) && tp->netdev->flags & IFF_UP) {
if (test_bit(SELECTIVE_SUSPEND, >flags)) {
rtl_runtime_suspend_enable(tp, false);
clear_bit(SELECTIVE_SUSPEND, >flags);
@@ -3532,6 +3515,8 @@ static int rtl8152_resume(struct usb_interface *intf)
}
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
} else if (test_bit(SELECTIVE_SUSPEND, >flags)) {
+   if (tp->netdev->flags & IFF_UP)
+   rtl_runtime_suspend_enable(tp, false);
clear_bit(SELECTIVE_SUSPEND, >flags);
}
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: Use memdup_user to reuse the code

[Dan Carpenter]

On Tue, Dec 08, 2015 at 11:38:59AM +, Pathak, Rahul (R.) wrote:
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 38ae877c..05266f0 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1395,11 +1395,9 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
> struct usbdevfs_urb *uurb
>   number_of_packets = uurb->number_of_packets;
>   isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
>  number_of_packets;
> - isopkt = kmalloc(isofrmlen, GFP_KERNEL);
> - if (!isopkt)
> - return -ENOMEM;
> - if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
> - ret = -EFAULT;
> + isopkt = memdup_user(iso_frame_desc, isofrmlen);
> + if (IS_ERR(isopkt)) {
> + ret = PTR_ERR(isopkt);
>   goto error;

This introduces a one err bug.
https://plus.google.com/106378716002406849458/posts/dnanfhQ4mHQ
We can't call kfree(isopkt) when it is an ERR_PTR.

Set it to NULL:

isopkt = memdup_user(iso_frame_desc, isofrmlen);
if (IS_ERR(isopkt)) {
ret = PTR_ERR(isopkt);
isopkt = NULL;
goto error;
}

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] r8152: fix lockup when runtime PM is enabled

[Hayes Wang]

Peter Wu [mailto:pe...@lekensteyn.nl]
> Sent: Tuesday, December 08, 2015 7:18 PM
> 
> When an interface is brought up which was previously suspended (via
> runtime PM), it would hang. This happens because napi_disable is called
> before napi_enable.
> 
> Solve this by avoiding napi_enable in the resume during open function
> (netif_running is true when open is called, IFF_UP is set after a
> successful open; netif_running is false when close is called, but IFF_UP
> is then still set).
> 
> While at it, remove WORK_ENABLE check from rtl8152_open (introduced with
> the original change) because it cannot happen:
> 
>  - After this patch, runtime resume will not set it during rtl8152_open.
>  - When link is up, rtl8152_open is not called.
>  - When link is down during system/auto suspend/resume, it is not set.
> 
> Fixes: 41cec84cf285 ("r8152: don't enable napi before rx ready")
> Link: https://lkml.kernel.org/r/20151205105912.GA1766@al
> Signed-off-by: Peter Wu 
> ---
>  v2: moved rtl_runtime_suspend_enable from close to rtl8152_suspend

Acked-by: Hayes Wang 

Best Regards,
Hayes

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 1/2] extcon: add driver for Intel USB mux

[Heikki Krogerus]

Hi,

On Tue, Dec 08, 2015 at 10:17:47AM +0900, Chanwoo Choi wrote:
> I can't agree to add specific function for only one device driver.
> As I commented, it is not appropriate way.

OK, I'll prepare something else for this.


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 3/3] usb: gadget: f_midi: pre-allocate IN requests

[Felipe Ferreri Tonello]

On 01/12/15 18:31, Felipe F. Tonello wrote:
> This patch introduces pre-allocation of IN endpoint USB requests. This
> improves on latency (requires no usb request allocation on transmit) and avoid
> several potential probles on allocating too many usb requests (which involves
> DMA pool allocation problems).
> 
> This implementation also handles better multiple MIDI Gadget ports, always
> processing the last processed MIDI substream if the last USB request wasn't
> enought to handle the whole stream.
> 
> Signed-off-by: Felipe F. Tonello 
> ---
>  drivers/usb/gadget/function/f_midi.c | 166 
> +++
>  drivers/usb/gadget/legacy/gmidi.c|   2 +-
>  2 files changed, 129 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c 
> b/drivers/usb/gadget/function/f_midi.c
> index 79dc611..fb1fe96d 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -88,6 +89,9 @@ struct f_midi {
>   int index;
>   char *id;
>   unsigned int buflen, qlen;
> + /* This fifo is used as a buffer ring for pre-allocated IN usb_requests 
> */
> + DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
> + unsigned int in_last_port;
>  };
>  
>  static inline struct f_midi *func_to_midi(struct usb_function *f)
> @@ -95,7 +99,7 @@ static inline struct f_midi *func_to_midi(struct 
> usb_function *f)
>   return container_of(f, struct f_midi, func);
>  }
>  
> -static void f_midi_transmit(struct f_midi *midi, struct usb_request *req);
> +static void f_midi_transmit(struct f_midi *midi);
>  
>  DECLARE_UAC_AC_HEADER_DESCRIPTOR(1);
>  DECLARE_USB_MIDI_OUT_JACK_DESCRIPTOR(1);
> @@ -253,7 +257,8 @@ f_midi_complete(struct usb_ep *ep, struct usb_request 
> *req)
>   } else if (ep == midi->in_ep) {
>   /* Our transmit completed. See if there's more to go.
>* f_midi_transmit eats req, don't queue it again. */
> - f_midi_transmit(midi, req);
> + req->length = 0;
> + f_midi_transmit(midi);
>   return;
>   }
>   break;
> @@ -264,10 +269,12 @@ f_midi_complete(struct usb_ep *ep, struct usb_request 
> *req)
>   case -ESHUTDOWN:/* disconnect from host */
>   VDBG(cdev, "%s gone (%d), %d/%d\n", ep->name, status,
>   req->actual, req->length);
> - if (ep == midi->out_ep)
> + if (ep == midi->out_ep) {
>   f_midi_handle_out_data(ep, req);
> -
> - free_ep_req(ep, req);
> + /* We don't need to free IN requests because it's 
> handled
> +  * by the midi->in_req_fifo. */
> + free_ep_req(ep, req);
> + }
>   return;
>  
>   case -EOVERFLOW:/* buffer overrun on read means that
> @@ -334,6 +341,20 @@ static int f_midi_set_alt(struct usb_function *f, 
> unsigned intf, unsigned alt)
>   if (err)
>   return err;
>  
> + /* pre-allocate write usb requests to use on f_midi_transmit. */
> + while (kfifo_avail(>in_req_fifo)) {
> + struct usb_request *req =
> + midi_alloc_ep_req(midi->in_ep, midi->buflen);
> +
> + if (req == NULL)
> + return -ENOMEM;
> +
> + req->length = 0;
> + req->complete = f_midi_complete;
> +
> + kfifo_put(>in_req_fifo, req);
> + }
> +
>   /* allocate a bunch of read buffers and queue them all at once. */
>   for (i = 0; i < midi->qlen && err == 0; i++) {
>   struct usb_request *req =
> @@ -358,6 +379,7 @@ static void f_midi_disable(struct usb_function *f)
>  {
>   struct f_midi *midi = func_to_midi(f);
>   struct usb_composite_dev *cdev = f->config->cdev;
> + struct usb_request *req = NULL;
>  
>   DBG(cdev, "disable\n");
>  
> @@ -367,6 +389,10 @@ static void f_midi_disable(struct usb_function *f)
>*/
>   usb_ep_disable(midi->in_ep);
>   usb_ep_disable(midi->out_ep);
> +
> + /* release IN requests */
> + while (kfifo_get(>in_req_fifo, ))
> + free_ep_req(midi->in_ep, req);
>  }
>  
>  static int f_midi_snd_free(struct snd_device *device)
> @@ -488,57 +514,113 @@ static void f_midi_transmit_byte(struct usb_request 
> *req,
>   }
>  }
>  
> -static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
> +static void f_midi_drop_out_substreams(struct f_midi *midi)
>  {
> - struct usb_ep *ep = midi->in_ep;
> - int i;
> -
> - if (!ep)
> - return;
> -
> - if (!req)
> - req = midi_alloc_ep_req(ep, midi->buflen);
> -
> - if (!req) {
> - ERROR(midi, "%s: alloc_ep_request 

Re: [PATCH] r8152: fix lockup when runtime PM is enabled

[Peter Wu]

On Tue, Dec 08, 2015 at 03:18:59AM +, Hayes Wang wrote:
> Peter Wu
> > Sent: Tuesday, December 08, 2015 12:59 AM
> [...]
> > +   if (tp->netdev->flags & IFF_UP) {
> 
> Maybe you could just replace the checking of netif_running(tp->netdev)
> with this.

Simply replacing netif_running by IFF_UP does not work, it hangs during
close when the device is suspended. This patch is correct, but I have a
v2 patch that moves rtl_runtime_suspend_enable from close to suspend.

This is the evaluated scenario (run = netif_running, up = IFF_UP set):

# suspended before open
suspend (run=0, up=0)
open(run=1)
resume  (run=1, up=0)   <-- fixed by patch
(open ends)

# while up
suspend (run=1, up=1)
resume  (run=1, up=1)   <-- no issue, values match
suspend (run=1, up=1)

# close while suspended
close   (run=0, up=1)
resume  (run=0, up=1)   <-- fixed in patch v2
(close cont and ends)   <-- rtl_runtime_suspend_enable removed in v2
suspend (run=0, up=0)

# while down
resume  (run=0, up=0)
suspend (run=0, up=0)

# open while suspended, open fails
open(run=1)
resume  (run=1, up=0)   <-- fixed by patch
(open fails)
suspend (run=0, up=0)

> Excuse me. I have a question. Before the open() is finished, the
> netif_running() would be true, but the IFF_UP wouldn't be set. Is it
> right?

That is right, this happens behind the scenes:

# open
netif_running = true
open()
if open succeeded
set IFF_UP
else
netif_running = false

# close
netif_running = false
close()
clear IFF_UP
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

re: usb: gadget: tcm: factor out f_tcm

[Dan Carpenter]

Hello Andrzej Pietrasiewicz,

The patch b4b91143ec45: "usb: gadget: tcm: factor out f_tcm" from Oct
27, 2015, leads to the following static checker warning:

drivers/usb/gadget/function/f_tcm.c:1149 usbg_submit_command()
warn: bool is not less than zero.

drivers/usb/gadget/function/f_tcm.c
  1145  cmd->unpacked_lun = scsilun_to_int(_iu->lun);
  1146  
  1147  INIT_WORK(>work, usbg_cmd_work);
  1148  ret = queue_work(tpg->workqueue, >work);
  1149  if (ret < 0)

queue_work() returns type bool.  Probably just remove this check.

  1150  goto err;
  1151  
  1152  return 0;
  1153  err:
  1154  kfree(cmd);
  1155  return -EINVAL;
  1156  }

drivers/usb/gadget/function/f_tcm.c:1244 bot_submit_command() warn: bool is not 
less than zero.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] doc: dt-binding: generic onboard USB HUB

[Arnd Bergmann]

On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote:
> Am Dienstag, den 08.12.2015, 09:37 +0800 schrieb Peter Chen:
> > Add dt-binding documentation for generic onboard USB HUB.
> > 
> > Signed-off-by: Peter Chen 
> > ---
> >  .../bindings/usb/generic-onboard-hub.txt   | 28 
> > ++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt 
> > b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > new file mode 100644
> > index 000..ea92205
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > @@ -0,0 +1,28 @@
> > +Generic Onboard USB HUB
> >+
> > +Required properties:
> > +- compatible: should be "generic-onboard-hub"
> 
> This something we don't have to define ad-hoc. The hub hangs off an USB
> controller, right? The "Open Firmware recommended practice: USB"
> document already describes how to represent USB devices in a generic
> manner:
> http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
> 
> Is there a reason not to reuse this?
> 
> The usb hub node would be a child of the usb controller node, and it
> could use
>   compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */

Good point, I had not thought of that when I looked at the patches.

Yes, let's do this way. I don't know if we ever implemented the simple
patch to associate a USB device with a device_node, but if not, then
let's do it now for this driver. A lot of people have asked for it in
the past.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [TESTPATCH v2] xhci: fix usb2 resume timing and races.

[Mathias Nyman]

On 05.12.2015 06:02, Daniel J Blueman wrote:

On 1 December 2015 at 16:26, Mathias Nyman
 wrote:

usb2 ports need to signal resume for 20ms before moving to U0 state.
Both device and host can initiate resume.

On host initated resume port is set to resume state, sleep 20ms,
and finally set port to U0 state.

On device initated resume a port status interrupt with a port in resume
state in issued. The interrupt handler tags a resume_done[port]
timestamp with current time + 20ms, and kick roothub timer.
Root hub timer requests for port status, finds the port in resume state,
checks if resume_done[port] timestamp passed, and set port to U0 state.

There are a few issues with this approach,
1. A host initated resume will also generate a resume event, the event
handler will find the port in resume state, believe it's a device
initated and act accordingly.

2. A port status request might cut the 20ms resume signalling short if a
get_port_status request is handled during the 20ms host resume.
The port will be found in resume state. The timestamp is not set leading
to time_after_eq(jiffoes, timestamp) returning true, as timestamp = 0.
get_port_status will proceed with moving the port to U0.

3. If an error, or anything else happends to the port during device
initated 20ms resume signalling it will leave all device resume
parameters hanging uncleared preventing further resume.

Fix this by using the existing resuming_ports bitfield to indicate if
resume signalling timing is taken care of.
Also check if the resume_done[port] is set  before using it in time
comparison. Also clear out any resume signalling related variables if port
is not in U0 or Resume state.

v2. fix parentheses when checking for uncleared resume variables.
we want: if ((unclear1 OR unclear2 ) AND !in_resume AND !in_U3) { .. }

Signed-off-by: Mathias Nyman 


Excellent; this correctly prevents the cyclic chain of suspend
attempts, resolving the issue.

Tested-by: Daniel J Blueman 

Thanks Mathias!
   Daniel



Thanks for testing it
I'll do some minor cleanups and add it to the queue

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] usb: Use memdup_user to reuse the code

[Pathak, Rahul (R.)]

From: Rahul Pathak 

Fixing coccicheck warning which recommends to use memdup_user instead
to reimplement its code, using memdup_user simplifies the code

./drivers/usb/core/devio.c:1398:11-18: WARNING opportunity for memdup_user

Changes after v1: setting isopkt=NULL for proper kfree() call

Signed-off-by: Rahul Pathak 
---
 drivers/usb/core/devio.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 38ae877c..844407c 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1395,11 +1395,10 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
struct usbdevfs_urb *uurb
number_of_packets = uurb->number_of_packets;
isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
   number_of_packets;
-   isopkt = kmalloc(isofrmlen, GFP_KERNEL);
-   if (!isopkt)
-   return -ENOMEM;
-   if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
-   ret = -EFAULT;
+   isopkt = memdup_user(iso_frame_desc, isofrmlen);
+   if (IS_ERR(isopkt)) {
+   ret = PTR_ERR(isopkt);
+   isopkt = NULL;
goto error;
}
for (totlen = u = 0; u < number_of_packets; u++) {
-- 
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: misc: generic_onboard_hub: add generic onboard USB HUB driver

[Felipe Balbi]

Hi,

Peter Chen  writes:
>> seriously ? Is this really all ? What about that reset line below ?
>
> The clock is PHY input clock on the HUB, this clock may from SoC's
> PLL.

oh, you might have misunderstood my comment. I'm saying that there is
more than one thing you could cache in your private structure. That's
all.

>> > +static int usb_hub_generic_probe(struct platform_device *pdev)
>> > +{
>> > +  struct device *dev = >dev;
>> > +  struct usb_hub_generic_platform_data *pdata = dev->platform_data;
>> > +  struct usb_hub_generic_data *hub_data;
>> > +  int reset_pol = 0, duration_us = 50, ret = 0;
>> > +  struct gpio_desc *gpiod_reset = NULL;
>> > +
>> > +  hub_data = devm_kzalloc(dev, sizeof(*hub_data), GFP_KERNEL);
>> > +  if (!hub_data)
>> > +  return -ENOMEM;
>> > +
>> > +  if (dev->of_node) {
>> > +  struct device_node *node = dev->of_node;
>> > +
>> > +  hub_data->clk = devm_clk_get(dev, "external_clk");
>> > +  if (IS_ERR(hub_data->clk)) {
>> > +  dev_dbg(dev, "Can't get external clock: %ld\n",
>> > +  PTR_ERR(hub_data->clk));
>> 
>> how about setting clock to NULL to here ? then you don't need IS_ERR()
>> checks anywhere else.
>> 
>> > +  }
>> 
>> braces shouldn't be used here, if you add NULL trick above,
>> however... then you can keep them.
>> 
>
> Braces aren't needed, it may not too much useful to using NULL
> as a indicator for error pointer.

heh, it's not about using it as an error pointer. I'm merely trying to
make clk optional. NULL is a valid clk, meaning you won't get NULL
pointer dereferences when passing it along clk_*() calls (if you find
any, it's likely a bug in CCF), so NULL can be used to cope with
optional clocks:

 clk = clk_get(dev, "foo");
 if (IS_ERR(clk)) {
if (PTR_ERR(clk) == -EPROBE_DEFER)
return -EPROBE_DEFER;
else
clk = NULL;
 }

>> > +  /*
>> > +   * Try to get the information for HUB reset, the
>> > +   * default setting like below:
>> > +   *
>> > +   * - Reset state is low
>> > +   * - The duration is 50us
>> > +   */
>> > +  if (of_find_property(node, "hub-reset-active-high", NULL))
>> > +  reset_pol = 1;
>> 
>> you see, this is definitely *not* generic. You should write a generic
>> reset-gpio.c reset controller and describe the polarity on the gpio
>> binding. This driver *always* uses reset_assert(); reset_deassert(); and
>> reset-gpio implements though by gpiod_set_value() correctly.
>> 
>> Polarity _must_ be described elsewhere in DT.
>> 
>> > +  of_property_read_u32(node, "hub-reset-duration-us",
>> > +  _us);
>> 
>> likewise, this should be described as a debounce time for the GPIO.
>> 
>
> Yes, if we are a reset gpio driver.

even if you use a raw GPIO, polarity and duration must come through DT.

>> > +  usleep_range(duration_us, duration_us + 100);
>> > +  gpiod_set_value(gpiod_reset, reset_pol ? 0 : 1);
>> 
>> wrong. You should _not_ have polarity checks here. You should have
>> already initialized the gpio as ACTIVE_HIGH or ACTIVE_LOW and gpiolib
>> will handle the polarity for you.
>
> Yes, you are right. I did not understand ACTIVE_LOW for gpio flag
> before.

with open source code, that's a rather poor excuse, Peter.

>> > +static int __init usb_hub_generic_init(void)
>> > +{
>> > +  return platform_driver_register(_hub_generic_driver);
>> > +}
>> > +subsys_initcall(usb_hub_generic_init);
>> > +
>> > +static void __exit usb_hub_generic_exit(void)
>> > +{
>> > +  platform_driver_unregister(_hub_generic_driver);
>> > +}
>> > +module_exit(usb_hub_generic_exit);
>> 
>> module_platform_driver();
>
> I want this driver to be called before module init's.

why ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: ohci: nxp: clean up included header files

[Alan Stern]

On Tue, 8 Dec 2015, Vladimir Zapolskiy wrote:

> Remove mach/irq.h from the list of included headers, there is no
> compilation dependency on this include file, the change is needed
> to prevent a compilation failure, when mach/irq.h is removed.
> 
> Additionally remove other unneeded includes.
> 
> Signed-off-by: Vladimir Zapolskiy 
> ---
>  drivers/usb/host/ohci-nxp.c | 7 ---
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-nxp.c b/drivers/usb/host/ohci-nxp.c
> index cfa9427..ebacf97 100644
> --- a/drivers/usb/host/ohci-nxp.c
> +++ b/drivers/usb/host/ohci-nxp.c
> @@ -22,7 +22,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -32,13 +31,7 @@
>  
>  #include "ohci.h"
>  
> -
>  #include 
> -#include 
> -#include 
> -
> -#include 
> -#include 
>  
>  #define USB_CONFIG_BASE  0x3102
>  #define PWRMAN_BASE  0x40004000

If this still builds correctly, it's okay with me.  I have no way to 
test it.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: dsps: handle the otg_state_a_wait_vrise_timeout case

[Bin Liu]

Felipe,

On 12/08/2015 08:20 AM, Felipe Balbi wrote:


Hi,

Gregory CLEMENT  writes:

if it is the case then it didn't fix the issue I had.

I activated the following debug line:

[musb_hdrc]musb_interrupt =_ "** IRQ %s usb%04x tx%04x rx%04x\012"
[musb_dsps]dsps_interrupt =p "usbintr (%x) epintr(%x)\012"

But I didn't get any interrupt while disconnecting the cable without any
device connected on it (whereas I got an interrupt when I connected it).

Note that I applied this patch instead of the "usb: musb: dsps: handle
the otg_state_a_wait_vrise_timeout case", is what you had in mind ?


yeah, that's what I had in mind. But your patch seems wrong :-)

I tried writing a more correct version here and found 2 issues:

a) bit 3 doesn't do anything :-p I cannot read IRQs from mentor's
registers

b) when setting RESET_ISOLATION bit, reads of CTRL register hang. Note
that according to TRM, RESET_ISOLATION _must_ be set prior to a soft
reset and cleared afterwards. But right after setting RESET_ISOLATION,
if I try a read of CTRL, it'll hang forever.


The datasheet seems not very coherent about it,

on one side we have:
"This bit should be set high prior to setting bit 0 and cleared after bit 0
is cleared."

and on the other side:
"Both the soft_reset and soft_reset_isolation bits should be asserted
simultaneously."

The hang you saw could be explained by the following:
"Setting only the soft_reset_isolation bit will cause all USB0 output
signals to go to a known constant value via multiplexers.
This will
prevent future access to USB0."  page 2567


good catch. Setting them together makes the hang go away.

I still have the other problem, which is legacy IRQ reporting mode not
really working.



I never tried to change the IRQ mapping. The 8 MUSB interrupt will be 
the same no matter where they are reported from. What do you expect when 
switch to the MUSB IRQ reporting mode?


Regards,
-Bin.

--
Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: dsps: handle the otg_state_a_wait_vrise_timeout case

[Felipe Balbi]

Hi,

Bin Liu  writes:
> Felipe,
>
> On 12/08/2015 08:20 AM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Gregory CLEMENT  writes:
>> if it is the case then it didn't fix the issue I had.
>>
>> I activated the following debug line:
>>
>> [musb_hdrc]musb_interrupt =_ "** IRQ %s usb%04x tx%04x rx%04x\012"
>> [musb_dsps]dsps_interrupt =p "usbintr (%x) epintr(%x)\012"
>>
>> But I didn't get any interrupt while disconnecting the cable without any
>> device connected on it (whereas I got an interrupt when I connected it).
>>
>> Note that I applied this patch instead of the "usb: musb: dsps: handle
>> the otg_state_a_wait_vrise_timeout case", is what you had in mind ?

 yeah, that's what I had in mind. But your patch seems wrong :-)

 I tried writing a more correct version here and found 2 issues:

 a) bit 3 doesn't do anything :-p I cannot read IRQs from mentor's
 registers

 b) when setting RESET_ISOLATION bit, reads of CTRL register hang. Note
 that according to TRM, RESET_ISOLATION _must_ be set prior to a soft
 reset and cleared afterwards. But right after setting RESET_ISOLATION,
 if I try a read of CTRL, it'll hang forever.
>>>
>>> The datasheet seems not very coherent about it,
>>>
>>> on one side we have:
>>> "This bit should be set high prior to setting bit 0 and cleared after bit 0
>>> is cleared."
>>>
>>> and on the other side:
>>> "Both the soft_reset and soft_reset_isolation bits should be asserted
>>> simultaneously."
>>>
>>> The hang you saw could be explained by the following:
>>> "Setting only the soft_reset_isolation bit will cause all USB0 output
>>> signals to go to a known constant value via multiplexers.
>>> This will
>>> prevent future access to USB0."  page 2567
>>
>> good catch. Setting them together makes the hang go away.
>>
>> I still have the other problem, which is legacy IRQ reporting mode not
>> really working.
>>
>
> I never tried to change the IRQ mapping. The 8 MUSB interrupt will be 
> the same no matter where they are reported from. What do you expect when 
> switch to the MUSB IRQ reporting mode?

read events from MUSB's registers instead of TI's :-) so, MUSB_INTRUSB,
MUSB_INTRRX and MUSB_INTRTX.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Sergei Shtylyov]

On 12/08/2015 08:51 AM, Simon Horman wrote:


Add fallback compatibility string.
This is in keeping with the fallback scheme being adopted wherever
appropriate for drivers for Renesas SoCs.

Signed-off-by: Simon Horman 
---
  Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 4 ++--
  drivers/usb/renesas_usbhs/common.c  | 4 
  2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt 
b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
index 7d48f63db44e..8c50df8441c9 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
@@ -1,7 +1,7 @@
  Renesas Electronics USBHS driver

  Required properties:
-  - compatible: Must contain one of the following:
+  - compatible: "renesas,usbhs-", "renesas,rcar-usbhs" as fallback.
- "renesas,usbhs-r8a7790"
- "renesas,usbhs-r8a7791"
- "renesas,usbhs-r8a7794"

[...]

diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index d82fa36c3465..2a9d4f405f30 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -481,6 +481,10 @@ static const struct of_device_id usbhs_of_match[] = {
.compatible = "renesas,usbhs-r8a7795",
.data = (void *)USBHS_TYPE_RCAR_GEN2,
},
+   {
+   .compatible = "renesas,usbhs",


   You just documented "renesas,rcar-usbhs".

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb: gadget: tcm: factor out f_tcm

[Andrzej Pietrasiewicz]

Hi Dan,

W dniu 08.12.2015 o 14:15, Dan Carpenter pisze:

Hello Andrzej Pietrasiewicz,

The patch b4b91143ec45: "usb: gadget: tcm: factor out f_tcm" from Oct
27, 2015, leads to the following static checker warning:

drivers/usb/gadget/function/f_tcm.c:1149 usbg_submit_command()
warn: bool is not less than zero.

drivers/usb/gadget/function/f_tcm.c
   1145  cmd->unpacked_lun = scsilun_to_int(_iu->lun);
   1146
   1147  INIT_WORK(>work, usbg_cmd_work);
   1148  ret = queue_work(tpg->workqueue, >work);
   1149  if (ret < 0)

queue_work() returns type bool.  Probably just remove this check.

   1150  goto err;
   1151
   1152  return 0;
   1153  err:
   1154  kfree(cmd);
   1155  return -EINVAL;
   1156  }

drivers/usb/gadget/function/f_tcm.c:1244 bot_submit_command() warn: bool is not 
less than zero.



The purpose of the commit is to split tcm_usb_gadget.c into
legacy gadget proper and f_tcm.c. The newly created f_tcm.c
is a part of tcm_usb_gadget.c copied into the new place,
so this warning is a duplicate of an old warning (which of
course does not mean it will not be addressed).

AP

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: dsps: handle the otg_state_a_wait_vrise_timeout case

[Felipe Balbi]

Hi,

Gregory CLEMENT  writes:
 if it is the case then it didn't fix the issue I had.

 I activated the following debug line:

 [musb_hdrc]musb_interrupt =_ "** IRQ %s usb%04x tx%04x rx%04x\012"
 [musb_dsps]dsps_interrupt =p "usbintr (%x) epintr(%x)\012"

 But I didn't get any interrupt while disconnecting the cable without any
 device connected on it (whereas I got an interrupt when I connected it).

 Note that I applied this patch instead of the "usb: musb: dsps: handle
 the otg_state_a_wait_vrise_timeout case", is what you had in mind ?
>>
>> yeah, that's what I had in mind. But your patch seems wrong :-)
>>
>> I tried writing a more correct version here and found 2 issues:
>>
>> a) bit 3 doesn't do anything :-p I cannot read IRQs from mentor's
>> registers
>>
>> b) when setting RESET_ISOLATION bit, reads of CTRL register hang. Note
>> that according to TRM, RESET_ISOLATION _must_ be set prior to a soft
>> reset and cleared afterwards. But right after setting RESET_ISOLATION,
>> if I try a read of CTRL, it'll hang forever.
>
> The datasheet seems not very coherent about it,
>
> on one side we have:
> "This bit should be set high prior to setting bit 0 and cleared after bit 0
> is cleared."
>
> and on the other side:
> "Both the soft_reset and soft_reset_isolation bits should be asserted
> simultaneously."
>
> The hang you saw could be explained by the following:
> "Setting only the soft_reset_isolation bit will cause all USB0 output
> signals to go to a known constant value via multiplexers.
> This will
> prevent future access to USB0."  page 2567

good catch. Setting them together makes the hang go away.

I still have the other problem, which is legacy IRQ reporting mode not
really working.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH RESEND v2 2/2] usb: phy: phy-am335x: bypass first VBUS sensing for host-only mode

[Felipe Balbi]

Hi,

Bin Liu  writes:
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 5b625e2..b33e00b 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -32,7 +32,7 @@ if USB_SUPPORT
>  config USB_COMMON
>   tristate
>   default y
> - depends on USB || USB_GADGET || USB_OTG
> + depends on USB || USB_GADGET || USB_OTG || AM335X_PHY_USB

AM335X_PHY_USB can only be selected in case USB or USB_GADGET is
selected, right ? This extra depends here smells very fishy.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH RESEND v2 2/2] usb: phy: phy-am335x: bypass first VBUS sensing for host-only mode

[Bin Liu]

Felipe,

On 12/08/2015 08:59 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 5b625e2..b33e00b 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -32,7 +32,7 @@ if USB_SUPPORT
  config USB_COMMON
tristate
default y
-   depends on USB || USB_GADGET || USB_OTG
+   depends on USB || USB_GADGET || USB_OTG || AM335X_PHY_USB


AM335X_PHY_USB can only be selected in case USB or USB_GADGET is
selected, right ? This extra depends here smells very fishy.



I will have to double check it again. I forgot what exact the issue was 
when I was creating this patch. Without this depends, there was symbol 
missing error when compile, I don't remember that was due to M or Y on 
AM335x_PHY_USB or it was be disabled.


--
Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: dsps: handle the otg_state_a_wait_vrise_timeout case

[Bin Liu]

On 12/08/2015 08:35 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

Felipe,

On 12/08/2015 08:20 AM, Felipe Balbi wrote:


Hi,

Gregory CLEMENT  writes:

if it is the case then it didn't fix the issue I had.

I activated the following debug line:

[musb_hdrc]musb_interrupt =_ "** IRQ %s usb%04x tx%04x rx%04x\012"
[musb_dsps]dsps_interrupt =p "usbintr (%x) epintr(%x)\012"

But I didn't get any interrupt while disconnecting the cable without any
device connected on it (whereas I got an interrupt when I connected it).

Note that I applied this patch instead of the "usb: musb: dsps: handle
the otg_state_a_wait_vrise_timeout case", is what you had in mind ?


yeah, that's what I had in mind. But your patch seems wrong :-)

I tried writing a more correct version here and found 2 issues:

a) bit 3 doesn't do anything :-p I cannot read IRQs from mentor's
registers

b) when setting RESET_ISOLATION bit, reads of CTRL register hang. Note
that according to TRM, RESET_ISOLATION _must_ be set prior to a soft
reset and cleared afterwards. But right after setting RESET_ISOLATION,
if I try a read of CTRL, it'll hang forever.


The datasheet seems not very coherent about it,

on one side we have:
"This bit should be set high prior to setting bit 0 and cleared after bit 0
is cleared."

and on the other side:
"Both the soft_reset and soft_reset_isolation bits should be asserted
simultaneously."

The hang you saw could be explained by the following:
"Setting only the soft_reset_isolation bit will cause all USB0 output
signals to go to a known constant value via multiplexers.
This will
prevent future access to USB0."  page 2567


good catch. Setting them together makes the hang go away.

I still have the other problem, which is legacy IRQ reporting mode not
really working.



I never tried to change the IRQ mapping. The 8 MUSB interrupt will be
the same no matter where they are reported from. What do you expect when
switch to the MUSB IRQ reporting mode?


read events from MUSB's registers instead of TI's :-) so, MUSB_INTRUSB,
MUSB_INTRRX and MUSB_INTRTX.

I meant you expect to see any different event when switch to MUSB IRQ 
mode? The TI wrapper just reports the same 8 interrupt events. I don't 
think you would get any difference.


BTY, I think I miss some context here. This Gregory's patch is trying to 
fix the OTG state machine problem in musb_dsps, which is replicated with 
a cable without a device connected. But it also mentions about 
non-compliant MSC devices. How are the thumb drives related to this OTG 
state issue?


--
Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] usb: Use memdup_user to reuse the code

[Alan Stern]

On Tue, 8 Dec 2015, Pathak, Rahul (R.) wrote:

> From: Rahul Pathak 
> 
> Fixing coccicheck warning which recommends to use memdup_user instead
> to reimplement its code, using memdup_user simplifies the code
> 
> ./drivers/usb/core/devio.c:1398:11-18: WARNING opportunity for memdup_user
> 
> Changes after v1: setting isopkt=NULL for proper kfree() call

This line belongs below the "---" tear line, so that it doesn't show up 
in the final commit changelog.  People reading the final commit won't 
care about earlier versions of the patch.

> Signed-off-by: Rahul Pathak 
> ---
>  drivers/usb/core/devio.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 38ae877c..844407c 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1395,11 +1395,10 @@ static int proc_do_submiturb(struct usb_dev_state 
> *ps, struct usbdevfs_urb *uurb
>   number_of_packets = uurb->number_of_packets;
>   isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
>  number_of_packets;
> - isopkt = kmalloc(isofrmlen, GFP_KERNEL);
> - if (!isopkt)
> - return -ENOMEM;
> - if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
> - ret = -EFAULT;
> + isopkt = memdup_user(iso_frame_desc, isofrmlen);
> + if (IS_ERR(isopkt)) {
> + ret = PTR_ERR(isopkt);
> + isopkt = NULL;
>   goto error;
>   }
>   for (totlen = u = 0; u < number_of_packets; u++) {

Aside from that,

Acked-by: Alan Stern 

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled

[Peter Wu]

On Tue, Dec 08, 2015 at 12:39:12PM +, Hayes Wang wrote:
> Peter Wu [mailto:pe...@lekensteyn.nl]
> > Sent: Tuesday, December 08, 2015 7:18 PM
> > 
> > When an interface is brought up which was previously suspended (via
> > runtime PM), it would hang. This happens because napi_disable is called
> > before napi_enable.
> > 
> > Solve this by avoiding napi_enable in the resume during open function
> > (netif_running is true when open is called, IFF_UP is set after a
> > successful open; netif_running is false when close is called, but IFF_UP
> > is then still set).
> > 
> > While at it, remove WORK_ENABLE check from rtl8152_open (introduced with
> > the original change) because it cannot happen:
> > 
> >  - After this patch, runtime resume will not set it during rtl8152_open.
> >  - When link is up, rtl8152_open is not called.
> >  - When link is down during system/auto suspend/resume, it is not set.
> > 
> > Fixes: 41cec84cf285 ("r8152: don't enable napi before rx ready")
> > Link: https://lkml.kernel.org/r/20151205105912.GA1766@al
> > Signed-off-by: Peter Wu 
> > ---
> >  v2: moved rtl_runtime_suspend_enable from close to rtl8152_suspend
> 
> Acked-by: Hayes Wang 
> 
> Best Regards,
> Hayes

I found another problem with runtime PM. When a device is suspended via
autosuspend and a system suspend takes place, there is no network I/O
after resume. Triggering a renegotiation (ethtool -r eth1) brings back
network activity.

Can you have a look? I guess that reset_resume needs different
treatment, but don't know how to do it properly as suspend is not called
before system reset (because the device is apparently already in
suspended state).

I think that this new issue can be addressed in a different patch, this
patch solves a more severe lockup. Opinions?
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: dsps: handle the otg_state_a_wait_vrise_timeout case

[Felipe Balbi]

Hi,

Bin Liu  writes:
> "This bit should be set high prior to setting bit 0 and cleared after bit > 0
> is cleared."
>
> and on the other side:
> "Both the soft_reset and soft_reset_isolation bits should be asserted
> simultaneously."
>
> The hang you saw could be explained by the following:
> "Setting only the soft_reset_isolation bit will cause all USB0 output
> signals to go to a known constant value via multiplexers.
> This will
> prevent future access to USB0."  page 2567

 good catch. Setting them together makes the hang go away.

 I still have the other problem, which is legacy IRQ reporting mode not
 really working.

>>>
>>> I never tried to change the IRQ mapping. The 8 MUSB interrupt will be
>>> the same no matter where they are reported from. What do you expect when
>>> switch to the MUSB IRQ reporting mode?
>>
>> read events from MUSB's registers instead of TI's :-) so, MUSB_INTRUSB,
>> MUSB_INTRRX and MUSB_INTRTX.
>>
> I meant you expect to see any different event when switch to MUSB IRQ 
> mode? The TI wrapper just reports the same 8 interrupt events. I don't 
> think you would get any difference.

still valid to make sure ;-)

> BTY, I think I miss some context here. This Gregory's patch is trying to 
> fix the OTG state machine problem in musb_dsps, which is replicated with 
> a cable without a device connected. But it also mentions about 
> non-compliant MSC devices. How are the thumb drives related to this OTG 
> state issue?

the problem seems to be caused by the missing disconnect IRQ. Not really
missing, but taking seconds to trigger.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2 2/2] usb: xhci: add Broadcom specific fake doorbell

[Sergei Shtylyov]

Hello.

On 12/08/2015 01:31 AM, Hauke Mehrtens wrote:


From: Rafał Miłecki 

This fixes problem with controller seeing devices only in some small
percentage of cold boots.
This quirk is also added to the platform data so we can activate it
when we register our platform driver.

Signed-off-by: Rafał Miłecki 
Signed-off-by: Hauke Mehrtens 


[...]

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 643d312..d49be9b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -122,6 +122,39 @@ int xhci_halt(struct xhci_hcd *xhci)
return ret;
  }

+static int xhci_fake_doorbell(struct xhci_hcd *xhci, int slot_id)
+{
+   u32 temp;
+
+   /* alloc a virt device for slot */
+   if (!xhci_alloc_virt_device(xhci, slot_id, NULL, GFP_NOIO)) {
+   xhci_warn(xhci, "Could not allocate xHCI USB device data 
structures\n");
+   return -ENOMEM;
+   }
+
+   /* ring fake doorbell for slot_id ep 0 */
+   xhci_ring_ep_doorbell(xhci, slot_id, 0, 0);
+   usleep_range(1000, 1500);
+
+   /* read the status register to check if HSE is set or not? */
+   temp = readl(>op_regs->status);
+
+   /* clear HSE if set */
+   if (temp & STS_FATAL) {
+   xhci_dbg(xhci, "HSE problem detected, status: 0x%x\n", temp);
+   temp &= ~(0x1fff);


   Parens around the literal not needed at all...

[...]

@@ -568,10 +601,25 @@ int xhci_init(struct usb_hcd *hcd)

  static int xhci_run_finished(struct xhci_hcd *xhci)
  {
-   if (xhci_start(xhci)) {
-   xhci_halt(xhci);
-   return -ENODEV;
+   int err;
+
+   err = xhci_start(xhci);
+   if (err) {
+   err = -ENODEV;
+   goto out_err;
+   }
+   if (xhci->quirks & XHCI_FAKE_DOORBELL) {
+   err = xhci_fake_doorbell(xhci, 1);
+   if (err)
+   goto out_err;
+
+   err = xhci_start(xhci);
+   if (err) {
+   err = -ENODEV;
+   goto out_err;
+   }


   Is it really necessary to call xhci_start() twice?

[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND v2 2/2] usb: phy: phy-am335x: bypass first VBUS sensing for host-only mode

[Bin Liu]

Felipe,

On 12/08/2015 09:05 AM, Bin Liu wrote:

Felipe,

On 12/08/2015 08:59 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 5b625e2..b33e00b 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -32,7 +32,7 @@ if USB_SUPPORT
  config USB_COMMON
  tristate
  default y
-depends on USB || USB_GADGET || USB_OTG
+depends on USB || USB_GADGET || USB_OTG || AM335X_PHY_USB


AM335X_PHY_USB can only be selected in case USB or USB_GADGET is
selected, right ? This extra depends here smells very fishy.



I will have to double check it again. I forgot what exact the issue was
when I was creating this patch. Without this depends, there was symbol
missing error when compile, I don't remember that was due to M or Y on
AM335x_PHY_USB or it was be disabled.



The AM335x_PHY_USB driver now calls of_usb_get_dr_mode_by_phy() defined 
in USB_COMMON. With omap2plus_defconfig, AM335x_PHY_USB=y, but 
USB_COMMON=m, which causes the phy driver is unable to find the function.


I will send v3 with the following instead.

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 793023e..f76c953 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -67,6 +67,7 @@ config AM335X_PHY_USB
select USB_PHY
select AM335X_CONTROL_USB
select NOP_USB_XCEIV
+   select USB_COMMON
help
  This driver provides PHY support for that phy which part for the
  AM335x SoC.

--
Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 2/4] gadget: Support for the usb charger framework

[kbuild test robot]

Hi Baolin,

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.4-rc4 next-20151208]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/gadget-Introduce-the-usb-charger-framework/20151208-163942
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: m68k-allmodconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   ERROR: "usb_charger_init" [drivers/usb/gadget/udc/udc-core.ko] undefined!
   ERROR: "usb_charger_plug_by_gadget" [drivers/usb/gadget/udc/udc-core.ko] 
undefined!
   ERROR: "usb_charger_exit" [drivers/usb/gadget/udc/udc-core.ko] undefined!
   ERROR: "usb_charger_set_cur_limit_by_type" 
[drivers/usb/gadget/libcomposite.ko] undefined!
>> ERROR: "usb_charger_detect_type" [drivers/usb/gadget/libcomposite.ko] 
>> undefined!
   ERROR: "usb_charger_set_cur_limit_by_type" 
[drivers/usb/gadget/legacy/gadgetfs.ko] undefined!
>> ERROR: "usb_charger_detect_type" [drivers/usb/gadget/legacy/gadgetfs.ko] 
>> undefined!

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


.config.gz
Description: Binary data

Re: [PATCH 1/3] usb: misc: generic_onboard_hub: add generic onboard USB HUB driver

[Mathieu Poirier]

On 7 December 2015 at 18:37, Peter Chen  wrote:
> Current USB HUB driver lacks of platform interfaces to configure
> external signal on HUB chip, eg, the PHY input clock and gpio reset
> pin for HUB, these kinds of HUBs are usually soldered at the board,
> and they are not hot-plug USB devices.
>
> With this patch, the user can configure the HUB's pins at platform
> part (through dts or platform), and these configuration will be
> effective before the USB bus can be used, we use subsys_initcall
> for this driver.
>
> Signed-off-by: Peter Chen 
> ---
>  MAINTAINERS |   7 ++
>  drivers/usb/misc/Kconfig|   9 ++
>  drivers/usb/misc/Makefile   |   1 +
>  drivers/usb/misc/generic_onboard_hub.c  | 162 
> 
>  include/linux/usb/generic_onboard_hub.h |  11 +++
>  5 files changed, 190 insertions(+)
>  create mode 100644 drivers/usb/misc/generic_onboard_hub.c
>  create mode 100644 include/linux/usb/generic_onboard_hub.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e9caa4b..cc1981e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11121,6 +11121,13 @@ S: Maintained
>  F: Documentation/usb/ohci.txt
>  F: drivers/usb/host/ohci*
>
> +USB Generic Onboard HUB Driver
> +M: Peter Chen 
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
> +L: linux-usb@vger.kernel.org
> +S: Maintained
> +F: drivers/usb/misc/generic_onboard_hub.c
> +
>  USB OTG FSM (Finite State Machine)
>  M: Peter Chen 
>  T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index f7a7fc2..8921cae 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -268,3 +268,12 @@ config USB_CHAOSKEY
>
>   To compile this driver as a module, choose M here: the
>   module will be called chaoskey.
> +
> +config USB_ONBOARD_HUB
> +   tristate "Generic USB Onboard HUB"
> +   help
> + Say Y here if your board has an onboard HUB, and this hub needs
> + to control its PHY clock and reset pin through external signals.
> + If you are not sure, say N.
> +
> + To compile this driver as a module, choose M here.
> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> index 45fd4ac..da52e9a 100644
> --- a/drivers/usb/misc/Makefile
> +++ b/drivers/usb/misc/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_USB_CHAOSKEY)+= chaoskey.o
>
>  obj-$(CONFIG_USB_SISUSBVGA)+= sisusbvga/
>  obj-$(CONFIG_USB_LINK_LAYER_TEST)  += lvstest.o
> +obj-$(CONFIG_USB_ONBOARD_HUB)  += generic_onboard_hub.o
> diff --git a/drivers/usb/misc/generic_onboard_hub.c 
> b/drivers/usb/misc/generic_onboard_hub.c
> new file mode 100644
> index 000..df722a0
> --- /dev/null
> +++ b/drivers/usb/misc/generic_onboard_hub.c
> @@ -0,0 +1,162 @@
> +/*
> + * usb_hub_generic.c   The generic onboard USB HUB driver
> + *
> + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> + * Author: Peter Chen 
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2  of
> + * the License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +/*
> + * This driver is only for the USB HUB devices which need to control
> + * their external pins(clock, reset, etc), and these USB HUB devices
> + * are soldered at the board.
> + */
> +
> +#define DEBUG
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct usb_hub_generic_data {
> +   struct clk *clk;
> +};
> +
> +static int usb_hub_generic_probe(struct platform_device *pdev)
> +{
> +   struct device *dev = >dev;
> +   struct usb_hub_generic_platform_data *pdata = dev->platform_data;
> +   struct usb_hub_generic_data *hub_data;
> +   int reset_pol = 0, duration_us = 50, ret = 0;
> +   struct gpio_desc *gpiod_reset = NULL;
> +
> +   hub_data = devm_kzalloc(dev, sizeof(*hub_data), GFP_KERNEL);
> +   if (!hub_data)
> +   return -ENOMEM;
> +
> +   if (dev->of_node) {
> +   struct device_node *node = dev->of_node;
> +
> +   hub_data->clk = devm_clk_get(dev, "external_clk");
> +   if (IS_ERR(hub_data->clk)) {
> +   

Re: [PATCH RESEND v2 2/2] usb: phy: phy-am335x: bypass first VBUS sensing for host-only mode

[Felipe Balbi]

Hi,

Bin Liu  writes:
> Felipe,
>
> On 12/08/2015 09:05 AM, Bin Liu wrote:
>> Felipe,
>>
>> On 12/08/2015 08:59 AM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Bin Liu  writes:
 diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
 index 5b625e2..b33e00b 100644
 --- a/drivers/usb/Kconfig
 +++ b/drivers/usb/Kconfig
 @@ -32,7 +32,7 @@ if USB_SUPPORT
   config USB_COMMON
   tristate
   default y
 -depends on USB || USB_GADGET || USB_OTG
 +depends on USB || USB_GADGET || USB_OTG || AM335X_PHY_USB
>>>
>>> AM335X_PHY_USB can only be selected in case USB or USB_GADGET is
>>> selected, right ? This extra depends here smells very fishy.
>>>
>>
>> I will have to double check it again. I forgot what exact the issue was
>> when I was creating this patch. Without this depends, there was symbol
>> missing error when compile, I don't remember that was due to M or Y on
>> AM335x_PHY_USB or it was be disabled.
>>
>
> The AM335x_PHY_USB driver now calls of_usb_get_dr_mode_by_phy() defined 
> in USB_COMMON. With omap2plus_defconfig, AM335x_PHY_USB=y, but 
> USB_COMMON=m, which causes the phy driver is unable to find the function.
>
> I will send v3 with the following instead.
>
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 793023e..f76c953 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -67,6 +67,7 @@ config AM335X_PHY_USB
>  select USB_PHY
>  select AM335X_CONTROL_USB
>  select NOP_USB_XCEIV
> +   select USB_COMMON

this is the correct way. USB_COMMON is not user-selectable.

-- 
balbi


signature.asc
Description: PGP signature

Re: USB TrackPoint mouse non-functional with dock; works if direct

[Alan Stern]

On Mon, 7 Dec 2015, Josh Triplett wrote:

> > You're looking at the wrong files.  The files to monitor are the ones
> > in /sys/devices/pci:00/:00:14.0/usb2/2-3/2-3.1/2-3.1.2/power
> > (assuming that this device really is the mouse and not something else).  
> > Handy shortcut link: /sys/bus/usb/devices/2-3.1.2/power.
> 
> Based on the idVendor and idProduct in that directory, that device is
> the keyboard/mouse combo device, yes.
> 
> That power directory has many more files, but nothing obvious:
...
> ==> /sys/devices/pci:00/:00:14.0/usb2/2-3/2-3.1/2-3.1.2/power/control 
> <==
> on

That's the important one.  It means the mouse isn't going to be 
autosuspended.

> > For more information on what's happening, try collecting a usbmon trace 
> > for bus 2 (see Documentation/usb/usbmon.txt).
> 
> Done.  I started a trace, plugged in the device, moved the mouse a
> little (which moved the pointer for a moment and then stopped
> producing any result), typed a couple of keys (which did work), moved
> the mouse a bit more (which didn't), and unplugged the device.
> 
> Trace attached.

Well, the trace shows the mouse being plugged in and enumerated.  Then
it shows the pointer being moved for about half a second, and a couple
of keys typed.  2.7 seconds later, it shows the device died and the
port was disconnected -- I assume that's when you unplugged the mouse.

During that 2.7-second interval, the usbmon trace shows nothing at all.  
No activity from the mouse (although it appears to have been
communicating okay because the computer polled it at 8-ms intervals and
didn't get any errors).  And in particular, no suspends.

It looks like there's something funny going on between the dock and the 
mouse.  For instance, maybe the dock doesn't provide quite enough 
power.  Or maybe the dock's internal hub doesn't work quite right.  

It's also possible that something strange happened in the xHCI host 
controller, but that seems less likely.  You could test it by 
removing the dock and then connecting the mouse to the computer by way 
of a USB-2.0 hub.

I don't have any other good suggestions for further debugging.  You did
check the kernel log to see if anything unusual showed up, right?  
About the only thing I can think of at this point is to use a USB
analyzer between the dock and the mouse.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[[PATCH v3]] usb: phy: phy-am335x: bypass first VBUS sensing for host-only mode

[Bin Liu]

To prevent VBUS contention, the am335x MUSB phy senses VBUS first before
transitioning to host mode. However, for host-only mode, VBUS could be
directly tied to 5V power rail which could prevent MUSB transitions to
host mode.

This change receives dr_mode of the controller then bypass the first
VBUS sensing for host-only mode, so that MUSB can work in host mode
event if VBUS is tied to 5V.

Signed-off-by: Bin Liu 
---
v3: correct module dependency in Kconfig

 drivers/usb/phy/Kconfig  |  1 +
 drivers/usb/phy/phy-am335x-control.c | 14 +++---
 drivers/usb/phy/phy-am335x-control.h |  8 +---
 drivers/usb/phy/phy-am335x.c | 15 ++-
 4 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 155694c..c690474 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -66,6 +66,7 @@ config AM335X_PHY_USB
select USB_PHY
select AM335X_CONTROL_USB
select NOP_USB_XCEIV
+   select USB_COMMON
help
  This driver provides PHY support for that phy which part for the
  AM335x SoC.
diff --git a/drivers/usb/phy/phy-am335x-control.c 
b/drivers/usb/phy/phy-am335x-control.c
index 23fca51..42a1afe 100644
--- a/drivers/usb/phy/phy-am335x-control.c
+++ b/drivers/usb/phy/phy-am335x-control.c
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "phy-am335x-control.h"
 
 struct am335x_control_usb {
@@ -58,7 +59,8 @@ static void am335x_phy_wkup(struct  phy_control *phy_ctrl, 
u32 id, bool on)
spin_unlock(_ctrl->lock);
 }
 
-static void am335x_phy_power(struct phy_control *phy_ctrl, u32 id, bool on)
+static void am335x_phy_power(struct phy_control *phy_ctrl, u32 id,
+   enum usb_dr_mode dr_mode, bool on)
 {
struct am335x_control_usb *usb_ctrl;
u32 val;
@@ -80,8 +82,14 @@ static void am335x_phy_power(struct phy_control *phy_ctrl, 
u32 id, bool on)
 
val = readl(usb_ctrl->phy_reg + reg);
if (on) {
-   val &= ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN);
-   val |= USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN;
+   if (dr_mode == USB_DR_MODE_HOST) {
+   val &= ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN |
+   USBPHY_OTGVDET_EN);
+   val |= USBPHY_OTGSESSEND_EN;
+   } else {
+   val &= ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN);
+   val |= USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN;
+   }
} else {
val |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN;
}
diff --git a/drivers/usb/phy/phy-am335x-control.h 
b/drivers/usb/phy/phy-am335x-control.h
index b96594d..e86b316 100644
--- a/drivers/usb/phy/phy-am335x-control.h
+++ b/drivers/usb/phy/phy-am335x-control.h
@@ -2,13 +2,15 @@
 #define _AM335x_PHY_CONTROL_H_
 
 struct phy_control {
-   void (*phy_power)(struct phy_control *phy_ctrl, u32 id, bool on);
+   void (*phy_power)(struct phy_control *phy_ctrl, u32 id,
+   enum usb_dr_mode dr_mode, bool on);
void (*phy_wkup)(struct phy_control *phy_ctrl, u32 id, bool on);
 };
 
-static inline void phy_ctrl_power(struct phy_control *phy_ctrl, u32 id, bool 
on)
+static inline void phy_ctrl_power(struct phy_control *phy_ctrl, u32 id,
+   enum usb_dr_mode dr_mode, bool on)
 {
-   phy_ctrl->phy_power(phy_ctrl, id, on);
+   phy_ctrl->phy_power(phy_ctrl, id, dr_mode, on);
 }
 
 static inline void phy_ctrl_wkup(struct phy_control *phy_ctrl, u32 id, bool on)
diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
index 8b6139d..39b424f 100644
--- a/drivers/usb/phy/phy-am335x.c
+++ b/drivers/usb/phy/phy-am335x.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "phy-am335x-control.h"
 #include "phy-generic.h"
@@ -16,13 +17,14 @@ struct am335x_phy {
struct usb_phy_generic usb_phy_gen;
struct phy_control *phy_ctrl;
int id;
+   enum usb_dr_mode dr_mode;
 };
 
 static int am335x_init(struct usb_phy *phy)
 {
struct am335x_phy *am_phy = dev_get_drvdata(phy->dev);
 
-   phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, true);
+   phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, am_phy->dr_mode, true);
return 0;
 }
 
@@ -30,7 +32,7 @@ static void am335x_shutdown(struct usb_phy *phy)
 {
struct am335x_phy *am_phy = dev_get_drvdata(phy->dev);
 
-   phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
+   phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, am_phy->dr_mode, false);
 }
 
 static int am335x_phy_probe(struct platform_device *pdev)
@@ -46,12 +48,15 @@ static int am335x_phy_probe(struct platform_device *pdev)
am_phy->phy_ctrl = am335x_get_phy_control(dev);
if (!am_phy->phy_ctrl)
return -EPROBE_DEFER;
+
am_phy->id = 

Re: Infrastructure for zerocopy I/O

[Alan Stern]

On Mon, 7 Dec 2015, Steinar H. Gunderson wrote:

> On Mon, Dec 07, 2015 at 10:45:55AM -0500, Alan Stern wrote:
> >> Here we are. Lightly tested, I believe all comments should be addressed.
> > This looks quite good.  I have only a couple of comments.
> 
> Excellent. I'm sorry we've needed so many rounds; this is what happens when
> you pick up someone else's patch against a code base you've never touched
> before. :-)

Of course.  And this was not a particularly simple patch.

> +static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count)
> +{
> + struct usb_dev_state *ps = usbm->ps;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>lock, flags);
> + --*count;
> + if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) {
> + list_del(>memlist);
> + spin_unlock_irqrestore(>lock, flags);
> +
> + usb_free_coherent(ps->dev, usbm->size, usbm->mem,
> + usbm->dma_handle);
> + usbfs_decrease_memory_usage(
> + usbm->size + sizeof(struct usb_memory));
> + kfree(usbm);
> + } else {
> + spin_unlock_irqrestore(>lock, flags);
> + }
> +}

> +static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct usb_memory *usbm = NULL;
> + struct usb_dev_state *ps = file->private_data;
> + size_t size = vma->vm_end - vma->vm_start;
> + void *mem;
> + unsigned long flags;
> + dma_addr_t dma_handle;
> + int ret;
> +
> + ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory));
> + if (ret) {
> + return ret;
> + }
> +
> + usbm = kzalloc(sizeof(struct usb_memory), GFP_KERNEL);
> + if (!usbm) {
> + usbfs_decrease_memory_usage(
> + size + sizeof(struct usb_memory));
> + return -ENOMEM;
> + }
> +
> + mem = usb_alloc_coherent(ps->dev, size, GFP_KERNEL, _handle);
> + if (!mem) {
> + usbfs_decrease_memory_usage(
> + size + sizeof(struct usb_memory));
> + kfree(usbm);
> + return -ENOMEM;
> + }
> +
> + memset(mem, 0, size);
> +
> + usbm->mem = mem;
> + usbm->dma_handle = dma_handle;
> + usbm->size = size;
> + usbm->ps = ps;
> + usbm->vm_start = vma->vm_start;
> + usbm->vma_use_count = 1;
> +
> + if (remap_pfn_range(vma, vma->vm_start,
> + virt_to_phys(usbm->mem) >> PAGE_SHIFT,
> + size, vma->vm_page_prot) < 0) {
> + dec_usb_memory_use_count(usbm, >vma_use_count);
> + return -EAGAIN;
> + }

I just noticed this -- usbm->memlist needs to be initialized before
dec_usb_memory_use_count() can be called.  INIT_LIST_HEAD before the
"if" statement will fix the problem.

The patch could use one more ingredient.  In
include/uapi/linux/usbdevicefs_h, add a #define for USBDEVFS_CAP_MMAP
and fix up proc_get_capabilities() to report the new capability.  In
theory, user programs could just try an mmap() call and see if it
works, but this seems cleaner.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Infrastructure for zerocopy I/O

[Greg KH]

On Tue, Dec 08, 2015 at 11:07:55PM +0100, Steinar H. Gunderson wrote:
> On Tue, Dec 08, 2015 at 05:01:46PM -0500, Alan Stern wrote:
> > I don't see anything else to change.  You can submit this to Greg KH 
> > and add:
> > 
> > Acked-by: Alan Stern 
> 
> Great!
> 
> How do I submit? Just do git send-email to some address?

Yes, use scripts/get_maintainer.pl to get my email and this mailing
list's address to use.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

On Tue, Dec 08, 2015 at 05:35:28PM +0300, Sergei Shtylyov wrote:
> On 12/08/2015 08:51 AM, Simon Horman wrote:
> 
> >Add fallback compatibility string.
> >This is in keeping with the fallback scheme being adopted wherever
> >appropriate for drivers for Renesas SoCs.
> >
> >Signed-off-by: Simon Horman 
> >---
> >  Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 4 ++--
> >  drivers/usb/renesas_usbhs/common.c  | 4 
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> >diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt 
> >b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> >index 7d48f63db44e..8c50df8441c9 100644
> >--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> >+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> >@@ -1,7 +1,7 @@
> >  Renesas Electronics USBHS driver
> >
> >  Required properties:
> >-  - compatible: Must contain one of the following:
> >+  - compatible: "renesas,usbhs-", "renesas,rcar-usbhs" as fallback.
> > - "renesas,usbhs-r8a7790"
> > - "renesas,usbhs-r8a7791"
> > - "renesas,usbhs-r8a7794"
> [...]
> >diff --git a/drivers/usb/renesas_usbhs/common.c 
> >b/drivers/usb/renesas_usbhs/common.c
> >index d82fa36c3465..2a9d4f405f30 100644
> >--- a/drivers/usb/renesas_usbhs/common.c
> >+++ b/drivers/usb/renesas_usbhs/common.c
> >@@ -481,6 +481,10 @@ static const struct of_device_id usbhs_of_match[] = {
> > .compatible = "renesas,usbhs-r8a7795",
> > .data = (void *)USBHS_TYPE_RCAR_GEN2,
> > },
> >+{
> >+.compatible = "renesas,usbhs",
> 
>You just documented "renesas,rcar-usbhs".

Thanks. I meant to use "renesas,rcar-usbhs" throughout the patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

On Tue, Dec 08, 2015 at 08:32:49AM +, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > > > Add fallback compatibility string.
> > > > This is in keeping with the fallback scheme being adopted wherever
> > > > appropriate for drivers for Renesas SoCs.
> > > > 
> > > > Signed-off-by: Simon Horman 
> > > > ---
> > > (snip)
> > > > +   {
> > > > +   .compatible = "renesas,usbhs",
> > > > +   .data = (void *)USBHS_TYPE_RCAR_GEN2,
> > > > +   },
> > > > { },
> > > >  };
> > > 
> > > I think this is too much. This driver is used not only from R-Car Gen2.
> > > It will work as normal mode if .data was 0.
> > > see usbhs_parse_dt()
> > 
> > Are you suggesting that we remove USBHS_TYPE_RCAR_GEN2 from the driver?
> 
> I mean this
> 
> + {
> + .compatible = "renesas,usbhs",
> + },

(As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
 with what I documented elsewhere in the patch)

My understanding is that will cause the driver to operate in a different manner
to if USBHS_TYPE_RCAR_GEN2 was set. And thus meaning
of the generic and SoC-specifc compatibility strings will cause
the driver to operate differently.

Currently the only compat strings present are as follows:

* renesas,usbhs-r8a7790
* renesas,usbhs-r8a7791
* renesas,usbhs-r8a7794
* renesas,usbhs-r8a7795

And they all set USBHS_TYPE_RCAR_GEN2 (even though r8a7795 is a Gen3 SoC).

What I am aiming is either:
* A compatibility string for R-Car or;
* Two compatibility strings, one for R-Car Gen2 and one for R-Car Gen3

And for this new compatibility string or strings to operate the same
way as the current compatibility strings.


Looking over the driver its not clear to me why the
non-USBHS_TYPE_RCAR_GEN2 case is handled by the code as
there are no compatibility strings present that trigger that mode of
operation. I feel that I must be missing something.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: ohci: nxp: clean up included header files

[Vladimir Zapolskiy]

On 08.12.2015 17:30, Alan Stern wrote:
> On Tue, 8 Dec 2015, Vladimir Zapolskiy wrote:
> 
>> Remove mach/irq.h from the list of included headers, there is no
>> compilation dependency on this include file, the change is needed
>> to prevent a compilation failure, when mach/irq.h is removed.
>>
>> Additionally remove other unneeded includes.
>>
>> Signed-off-by: Vladimir Zapolskiy 
>> ---
>>  drivers/usb/host/ohci-nxp.c | 7 ---
>>  1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/usb/host/ohci-nxp.c b/drivers/usb/host/ohci-nxp.c
>> index cfa9427..ebacf97 100644
>> --- a/drivers/usb/host/ohci-nxp.c
>> +++ b/drivers/usb/host/ohci-nxp.c
>> @@ -22,7 +22,6 @@
>>  #include 
>>  #include 
>>  #include 
>> -#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -32,13 +31,7 @@
>>  
>>  #include "ohci.h"
>>  
>> -
>>  #include 
>> -#include 
>> -#include 
>> -
>> -#include 
>> -#include 
>>  
>>  #define USB_CONFIG_BASE 0x3102
>>  #define PWRMAN_BASE 0x40004000
> 
> If this still builds correctly, it's okay with me.  I have no way to 
> test it.

Thanks for review.

If you have an ARMv5 toolchain, then you can test building with
lpc32xx_defconfig:

make ARCH=arm CROSS_COMPILE=${CCPREFIX} lpc32xx_defconfig
make ARCH=arm CROSS_COMPILE=${CCPREFIX} drivers/usb/host/ohci-nxp.o

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Kuninori Morimoto]

Hi Simon

> > > (As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
> > >  with what I documented elsewhere in the patch)
> > 
> > "renesas,rcar-usbhs" is better,
> > but I guess you want to have "renesas,rcar-gen2-usbhs" ?
> > 
> > My understanding is these
> > 
> >  * renesas,usbhs-r8a77xx# SoC specific
> >  * renesas,rcar-usbhs   # R-Car common
> >  * renesas,rcar-gen2-usbhs  # R-Car Gen2 common
> >  * renesas,rcar-gen3-usbhs  # R-Car Gen3 common
> >  * renesas,usbhs# Renesas USBHS common
> > 
> 
> I was intentionally including gen3 as well. So I think we have two options:
> 
> 1. renesas,rcar-usbhs
> 2. renesas,rcar-gen2-usbhs and renesas,rcar-gen3-usbhs

Renesas USB always have pick feature/settings.
Thus, generic name (= "renesas,rcar-usbhs") is very risky IMO.
I think 2 is more safety.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

On Wed, Dec 09, 2015 at 06:29:07AM +, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > > > (As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
> > > >  with what I documented elsewhere in the patch)
> > > 
> > > "renesas,rcar-usbhs" is better,
> > > but I guess you want to have "renesas,rcar-gen2-usbhs" ?
> > > 
> > > My understanding is these
> > > 
> > >  * renesas,usbhs-r8a77xx  # SoC specific
> > >  * renesas,rcar-usbhs # R-Car common
> > >  * renesas,rcar-gen2-usbhs# R-Car Gen2 common
> > >  * renesas,rcar-gen3-usbhs# R-Car Gen3 common
> > >  * renesas,usbhs  # Renesas USBHS common
> > > 
> > 
> > I was intentionally including gen3 as well. So I think we have two options:
> > 
> > 1. renesas,rcar-usbhs
> > 2. renesas,rcar-gen2-usbhs and renesas,rcar-gen3-usbhs
> 
> Renesas USB always have pick feature/settings.
> Thus, generic name (= "renesas,rcar-usbhs") is very risky IMO.
> I think 2 is more safety.

Sure, better safe than sorry.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] usb: renesas_usbhs: add device tree support for r8a779[23]

[Rob Herring]

On Tue, Dec 08, 2015 at 02:51:51PM +0900, Simon Horman wrote:
> Simply document new compatibility string.
> As a previous patch adds a generic R-Car Gen2 compatibility string
> there appears to be no need for a driver updates.
> 
> Also add names for SoCs.
> 
> Signed-off-by: Simon Horman 

Acked-by: Rob Herring 

> ---
>  Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt 
> b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> index 8c50df8441c9..b31f036d2171 100644
> --- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> +++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> @@ -2,10 +2,12 @@ Renesas Electronics USBHS driver
>  
>  Required properties:
>- compatible: "renesas,usbhs-", "renesas,rcar-usbhs" as fallback.
> - - "renesas,usbhs-r8a7790"
> - - "renesas,usbhs-r8a7791"
> - - "renesas,usbhs-r8a7794"
> - - "renesas,usbhs-r8a7795"
> + - "renesas,usbhs-r8a7790" (R-Car H2)
> + - "renesas,usbhs-r8a7791" (R-Car M2-W)
> + - "renesas,usbhs-r8a7792" (R-Car V2H)
> + - "renesas,usbhs-r8a7793" (R-Car M2-N)
> + - "renesas,usbhs-r8a7794" (R-Car E2)
> + - "renesas,usbhs-r8a7795" (R-Car H3)
>- reg: Base address and length of the register for the USBHS
>- interrupts: Interrupt specifier for the USBHS
>- clocks: A list of phandle + clock specifier pairs
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Kuninori Morimoto]

Hi Simon

> (As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
>  with what I documented elsewhere in the patch)

"renesas,rcar-usbhs" is better,
but I guess you want to have "renesas,rcar-gen2-usbhs" ?

My understanding is these

 * renesas,usbhs-r8a77xx# SoC specific
 * renesas,rcar-usbhs   # R-Car common
 * renesas,rcar-gen2-usbhs  # R-Car Gen2 common
 * renesas,rcar-gen3-usbhs  # R-Car Gen3 common
 * renesas,usbhs# Renesas USBHS common
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] doc: dt-binding: generic onboard USB HUB

[Rob Herring]

On Tue, Dec 08, 2015 at 10:58:48AM +0100, Arnd Bergmann wrote:
> On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote:
> > Am Dienstag, den 08.12.2015, 09:37 +0800 schrieb Peter Chen:
> > > Add dt-binding documentation for generic onboard USB HUB.
> > > 
> > > Signed-off-by: Peter Chen 
> > > ---
> > >  .../bindings/usb/generic-onboard-hub.txt   | 28 
> > > ++
> > >  1 file changed, 28 insertions(+)
> > >  create mode 100644 
> > > Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > 
> > > diff --git 
> > > a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt 
> > > b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > new file mode 100644
> > > index 000..ea92205
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > @@ -0,0 +1,28 @@
> > > +Generic Onboard USB HUB
> > >+
> > > +Required properties:
> > > +- compatible: should be "generic-onboard-hub"
> > 
> > This something we don't have to define ad-hoc. The hub hangs off an USB
> > controller, right? The "Open Firmware recommended practice: USB"
> > document already describes how to represent USB devices in a generic
> > manner:
> > http://www.firmware.org/1275/bindings/usb/usb-1_0.ps
> > 
> > Is there a reason not to reuse this?
> > 
> > The usb hub node would be a child of the usb controller node, and it
> > could use
> > compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */
> 
> Good point, I had not thought of that when I looked at the patches.
>  
> Yes, let's do this way. I don't know if we ever implemented the simple
> patch to associate a USB device with a device_node, but if not, then
> let's do it now for this driver. A lot of people have asked for it in
> the past.

Agreed. Also, some hubs have I2C buses as well, but I still think under 
the USB bus is the right place.

However, one complication here is often (probably this case) these 
addtional signals need to be controlled before the device enumerates.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled

[David Miller]

From: Peter Wu 
Date: Tue,  8 Dec 2015 12:17:42 +0100

> When an interface is brought up which was previously suspended (via
> runtime PM), it would hang. This happens because napi_disable is called
> before napi_enable.
> 
> Solve this by avoiding napi_enable in the resume during open function
> (netif_running is true when open is called, IFF_UP is set after a
> successful open; netif_running is false when close is called, but IFF_UP
> is then still set).
> 
> While at it, remove WORK_ENABLE check from rtl8152_open (introduced with
> the original change) because it cannot happen:
> 
>  - After this patch, runtime resume will not set it during rtl8152_open.
>  - When link is up, rtl8152_open is not called.
>  - When link is down during system/auto suspend/resume, it is not set.
> 
> Fixes: 41cec84cf285 ("r8152: don't enable napi before rx ready")
> Link: https://lkml.kernel.org/r/20151205105912.GA1766@al
> Signed-off-by: Peter Wu 
> ---
>  v2: moved rtl_runtime_suspend_enable from close to rtl8152_suspend

Applied, and queued up for -stable, thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

On Wed, Dec 09, 2015 at 04:48:47AM +, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > (As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
> >  with what I documented elsewhere in the patch)
> 
> "renesas,rcar-usbhs" is better,
> but I guess you want to have "renesas,rcar-gen2-usbhs" ?
> 
> My understanding is these
> 
>  * renesas,usbhs-r8a77xx  # SoC specific
>  * renesas,rcar-usbhs # R-Car common
>  * renesas,rcar-gen2-usbhs# R-Car Gen2 common
>  * renesas,rcar-gen3-usbhs# R-Car Gen3 common
>  * renesas,usbhs  # Renesas USBHS common
> 

I was intentionally including gen3 as well. So I think we have two options:

1. renesas,rcar-usbhs
2. renesas,rcar-gen2-usbhs and renesas,rcar-gen3-usbhs

Which do you prefer?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: USB TrackPoint mouse non-functional with dock; works if direct

[Josh Triplett]

On Tue, Dec 08, 2015 at 11:04:00AM -0500, Alan Stern wrote:
> On Mon, 7 Dec 2015, Josh Triplett wrote:
> 
> > > You're looking at the wrong files.  The files to monitor are the ones
> > > in /sys/devices/pci:00/:00:14.0/usb2/2-3/2-3.1/2-3.1.2/power
> > > (assuming that this device really is the mouse and not something else).  
> > > Handy shortcut link: /sys/bus/usb/devices/2-3.1.2/power.
> > 
> > Based on the idVendor and idProduct in that directory, that device is
> > the keyboard/mouse combo device, yes.
> > 
> > That power directory has many more files, but nothing obvious:
> ...
> > ==> 
> > /sys/devices/pci:00/:00:14.0/usb2/2-3/2-3.1/2-3.1.2/power/control 
> > <==
> > on
> 
> That's the important one.  It means the mouse isn't going to be 
> autosuspended.

So it doesn't matter that the other sysfs directory (the one listed by
dmesg for the mouse input device, containing the :1.1) showed up with
autosuspend enabled and the device suspending after a moment, roughly
matching the "works for a second and then stops" behavior?  That seems
entirely too coincidental.

What do that other sysfs directory's properties mean, if not the actual
suspend state of the device?

> > > For more information on what's happening, try collecting a usbmon trace 
> > > for bus 2 (see Documentation/usb/usbmon.txt).
> > 
> > Done.  I started a trace, plugged in the device, moved the mouse a
> > little (which moved the pointer for a moment and then stopped
> > producing any result), typed a couple of keys (which did work), moved
> > the mouse a bit more (which didn't), and unplugged the device.
> > 
> > Trace attached.
> 
> Well, the trace shows the mouse being plugged in and enumerated.  Then
> it shows the pointer being moved for about half a second, and a couple
> of keys typed.  2.7 seconds later, it shows the device died and the
> port was disconnected -- I assume that's when you unplugged the mouse.
> 
> During that 2.7-second interval, the usbmon trace shows nothing at all.  
> No activity from the mouse (although it appears to have been
> communicating okay because the computer polled it at 8-ms intervals and
> didn't get any errors).  And in particular, no suspends.

I moved the mouse again during that interval, but the cursor did not
respond.

> It looks like there's something funny going on between the dock and the 
> mouse.  For instance, maybe the dock doesn't provide quite enough 
> power.

This is just a keyboard/mouse; it doesn't draw significant power from
the port, and doesn't require a powered port.

>  Or maybe the dock's internal hub doesn't work quite right.  

> It's also possible that something strange happened in the xHCI host 
> controller, but that seems less likely.  You could test it by 
> removing the dock and then connecting the mouse to the computer by way 
> of a USB-2.0 hub.

If it makes any difference, the dock has both USB 2 and USB 3 ports; I
observe the same behavior in both.

Would a USB 2 A-to-A extension cable, without the USB 3 pins, suffice
for such testing?  I have one of those.

> I don't have any other good suggestions for further debugging.  You did
> check the kernel log to see if anything unusual showed up, right?  

Yes, and nothing did.  dmesg shows the input devices showing up, but no
USB-related messages after that.

> About the only thing I can think of at this point is to use a USB
> analyzer between the dock and the mouse.

Not something I have handy.

- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Infrastructure for zerocopy I/O

[Alan Stern]

On Tue, 8 Dec 2015, Steinar H. Gunderson wrote:

> On Tue, Dec 08, 2015 at 03:26:28PM -0500, Alan Stern wrote:
> > I just noticed this -- usbm->memlist needs to be initialized before
> > dec_usb_memory_use_count() can be called.  INIT_LIST_HEAD before the
> > "if" statement will fix the problem.
> 
> Done.
> 
> > The patch could use one more ingredient.  In
> > include/uapi/linux/usbdevicefs_h, add a #define for USBDEVFS_CAP_MMAP
> > and fix up proc_get_capabilities() to report the new capability.  In
> > theory, user programs could just try an mmap() call and see if it
> > works, but this seems cleaner.
> 
> Done.
> 
> /* Steinar */
> 
> From 03318604cc6ad9def451784da407e6fcd6af4705 Mon Sep 17 00:00:00 2001
> From: "Steinar H. Gunderson" 
> Date: Thu, 26 Nov 2015 01:19:13 +0100
> Subject: [PATCH] Add support for usbfs zerocopy.
> 
> Add a new interface for userspace to preallocate memory that can be
> used with usbfs. This gives two primary benefits:
> 
>  - Zerocopy; data no longer needs to be copied between the userspace
>and the kernel, but can instead be read directly by the driver from
>userspace's buffers. This works for all kinds of transfers (even if
>nonsensical for control and interrupt transfers); isochronous also
>no longer need to memset() the buffer to zero to avoid leaking kernel data.
> 
>  - Once the buffers are allocated, USB transfers can no longer fail due to
>memory fragmentation; previously, long-running programs could run into
>problems finding a large enough contiguous memory chunk, especially on
>embedded systems or at high rates.
> 
> Memory is allocated by using mmap() against the usbfs file descriptor,
> and similarly deallocated by munmap(). Once memory has been allocated,
> using it as pointers to a bulk or isochronous operation means you will
> automatically get zerocopy behavior. Note that this also means you cannot
> modify outgoing data until the transfer is complete. The same holds for
> data on the same cache lines as incoming data; DMA modifying them at the
> same time could lead to your changes being overwritten.
> 
> There's a new capability USBDEVFS_CAP_MMAP that userspace can query to see
> if the running kernel supports this functionality, if just trying mmap() is
> not acceptable.
> 
> Largely based on a patch by Markus Rechberger with some updates. The original
> patch can be found at:
> 
>   http://sundtek.de/support/devio_mmap_v0.4.diff
> 
> Signed-off-by: Steinar H. Gunderson 
> Signed-off-by: Markus Rechberger 

I don't see anything else to change.  You can submit this to Greg KH 
and add:

Acked-by: Alan Stern 

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Infrastructure for zerocopy I/O

[Steinar H. Gunderson]

On Tue, Dec 08, 2015 at 05:01:46PM -0500, Alan Stern wrote:
> I don't see anything else to change.  You can submit this to Greg KH 
> and add:
> 
> Acked-by: Alan Stern 

Great!

How do I submit? Just do git send-email to some address?

Do you know what the status is for the LPM blacklisting patch we discussed
earlier?

/* Steinar */
-- 
Homepage: https://www.sesse.net/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Infrastructure for zerocopy I/O

[Steinar H. Gunderson]

On Tue, Dec 08, 2015 at 03:26:28PM -0500, Alan Stern wrote:
> I just noticed this -- usbm->memlist needs to be initialized before
> dec_usb_memory_use_count() can be called.  INIT_LIST_HEAD before the
> "if" statement will fix the problem.

Done.

> The patch could use one more ingredient.  In
> include/uapi/linux/usbdevicefs_h, add a #define for USBDEVFS_CAP_MMAP
> and fix up proc_get_capabilities() to report the new capability.  In
> theory, user programs could just try an mmap() call and see if it
> works, but this seems cleaner.

Done.

/* Steinar */

>From 03318604cc6ad9def451784da407e6fcd6af4705 Mon Sep 17 00:00:00 2001
From: "Steinar H. Gunderson" 
Date: Thu, 26 Nov 2015 01:19:13 +0100
Subject: [PATCH] Add support for usbfs zerocopy.

Add a new interface for userspace to preallocate memory that can be
used with usbfs. This gives two primary benefits:

 - Zerocopy; data no longer needs to be copied between the userspace
   and the kernel, but can instead be read directly by the driver from
   userspace's buffers. This works for all kinds of transfers (even if
   nonsensical for control and interrupt transfers); isochronous also
   no longer need to memset() the buffer to zero to avoid leaking kernel data.

 - Once the buffers are allocated, USB transfers can no longer fail due to
   memory fragmentation; previously, long-running programs could run into
   problems finding a large enough contiguous memory chunk, especially on
   embedded systems or at high rates.

Memory is allocated by using mmap() against the usbfs file descriptor,
and similarly deallocated by munmap(). Once memory has been allocated,
using it as pointers to a bulk or isochronous operation means you will
automatically get zerocopy behavior. Note that this also means you cannot
modify outgoing data until the transfer is complete. The same holds for
data on the same cache lines as incoming data; DMA modifying them at the
same time could lead to your changes being overwritten.

There's a new capability USBDEVFS_CAP_MMAP that userspace can query to see
if the running kernel supports this functionality, if just trying mmap() is
not acceptable.

Largely based on a patch by Markus Rechberger with some updates. The original
patch can be found at:

  http://sundtek.de/support/devio_mmap_v0.4.diff

Signed-off-by: Steinar H. Gunderson 
Signed-off-by: Markus Rechberger 
---
 drivers/usb/core/devio.c  | 230 +-
 include/uapi/linux/usbdevice_fs.h |   1 +
 2 files changed, 206 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 38ae877c..3349222 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -69,6 +70,7 @@ struct usb_dev_state {
spinlock_t lock;/* protects the async urb lists */
struct list_head async_pending;
struct list_head async_completed;
+   struct list_head memory_list;
wait_queue_head_t wait; /* wake up if a request completed */
unsigned int discsignr;
struct pid *disc_pid;
@@ -79,6 +81,17 @@ struct usb_dev_state {
u32 disabled_bulk_eps;
 };
 
+struct usb_memory {
+   struct list_head memlist;
+   int vma_use_count;
+   int urb_use_count;
+   u32 size;
+   void *mem;
+   dma_addr_t dma_handle;
+   unsigned long vm_start;
+   struct usb_dev_state *ps;
+};
+
 struct async {
struct list_head asynclist;
struct usb_dev_state *ps;
@@ -89,6 +102,7 @@ struct async {
void __user *userbuffer;
void __user *userurb;
struct urb *urb;
+   struct usb_memory *usbm;
unsigned int mem_usage;
int status;
u32 secid;
@@ -157,6 +171,108 @@ static int connected(struct usb_dev_state *ps)
ps->dev->state != USB_STATE_NOTATTACHED);
 }
 
+static void dec_usb_memory_use_count(struct usb_memory *usbm, int *count)
+{
+   struct usb_dev_state *ps = usbm->ps;
+   unsigned long flags;
+
+   spin_lock_irqsave(>lock, flags);
+   --*count;
+   if (usbm->urb_use_count == 0 && usbm->vma_use_count == 0) {
+   list_del(>memlist);
+   spin_unlock_irqrestore(>lock, flags);
+
+   usb_free_coherent(ps->dev, usbm->size, usbm->mem,
+   usbm->dma_handle);
+   usbfs_decrease_memory_usage(
+   usbm->size + sizeof(struct usb_memory));
+   kfree(usbm);
+   } else {
+   spin_unlock_irqrestore(>lock, flags);
+   }
+}
+
+static void usbdev_vm_open(struct vm_area_struct *vma)
+{
+   struct usb_memory *usbm = vma->vm_private_data;
+   unsigned long flags;
+
+   spin_lock_irqsave(>ps->lock, flags);
+   ++usbm->vma_use_count;
+   spin_unlock_irqrestore(>ps->lock, flags);
+}
+