Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Peter Chen]

On Thu, May 05, 2016 at 05:42:40PM -0500, Rob Herring wrote:
> On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
> > Hi,
> > 
> > This is a different, second try to fix usb3503+lan on Odroid U3 board
> > if it was initialized by bootloader (e.g. for TFTP boot).
> > 
> > First version:
> > http://www.spinics.net/lists/linux-usb/msg140042.html
> > 
> > 
> > Problem
> > ===
> > When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
> > the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
> > is required, e.g. by suspend to RAM. The actual TFTP boot does
> > not have to happen. Just "usb start" from U-Boot is sufficient.
> > 
> > From the schematics, the regulator is a supply only to LAN, however
> > without toggling it off/on, the usb3503 hub won appear neither.
> > 
> > 
> > Solution
> > 
> > This is very similar to the MMC pwrseq behavior so the idea is to:
> > 1. Move MMC pwrseq drivers to generic place,
> 
> You can do that, but I'm going to NAK any use of pwrseq bindings outside 
> of MMC. I think it is the wrong way to do things. The DT should describe 
> the devices. If they happen to be "simple" then the core can walk the 
> tree and do any setup. For example, look for "reset-gpios" and toggle 
> that GPIO. There is no need for a special node.
> 

Oh, I am doing the same thing like this patch set doing. Then, how can
we let things be generic like you mention at [1] if without common
pwrseq driver/library? The properties like "reset-gpios" under
USB child node seems can only be handled by USB driver.

> > 2. Extend the pwrseq-simple with regulator toggling,
> > 3. Add support to USB hub and port core for pwrseq,
> 
> We discussed this for USB already[1] and is why we defined how to add 
> USB child devices. The idea is not to add pwrseq to that.
> 
> Rob
> 
> [1] http://www.spinics.net/lists/linux-usb/msg134082.html

[1] http://www.spinics.net/lists/linux-usb/msg137312.html

-- 

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/2] usb: configfs: allow UDC binding rule configured as binding to *any* UDC

[Krzysztof Opasiak]

On 05/06/2016 04:46 AM, Du, Changbin wrote:
(...)
>> Well, I'm not sure if any configfs interface has been proposed as easy
>> to use from cmd line. I think they all has been proposed as  *usable*
>> from cmd line but not necessarily *easy to use*.
>>
>> That's why most of configfs clients has some support in userspace. For
>> example for target there is a taget-cli and for usb gadget we have
>> libusbg/libusbgx.
>>
> Glade to know this tool, it is much more easy to use than interact with sysfs.
> I'd like use it. Just see you are the main contributor of this project. :)
> 

That's true;) personally I would recommend you using libusbgx[1] instead
of libusbg[2] as it is far more recent and usable (292 commits vs 128;) )

(...)

>>
>> What do you mean pseudo 'busy'? If we do:
>>
>> echo  > UDC
>>
> Sorry, please ignore this. I find if no UDC available, the config will be 
> queued
> to a list, and will bind it when a UDC module install. So it is really busy.
> 
>> then gadget should be really bound to some udc and potentially really busy.
>>
>>> In a word, this patch is just an improvement, not to fix any issues or
>>> add new function.
>>
>> So it doesn't add any new functionality and breaks existing user space
>> tools.
>>

Yes, currently it's true but it's a bug which I have fixed yesterday[3]

Footnotes:
1 - https://github.com/libusbgx/libusbgx
2 - https://github.com/libusbg/libusbg
3 - http://marc.info/?l=linux-usb=146243801207458=2

Cheers,
-- 
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
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: configfs: allow UDC binding rule configured as binding to *any* UDC

[Du, Changbin]

> >>> On most platforms, there is only one device controller available.
> >>> In this case, we desn't care the UDC's name. So let's ignore the
> >>> name by setting 'UDC' to 'any'.
> >>
> >> Hmm libubsgx allows to do this for a very long time. You simply pass
> >> NULL instead of pointer to usbg_udc.
> >>
> >> It is also possible to do this from command line, just simply:
> >>
> >> $ echo `ls -1 /sys/class/udc | head -n 1` > UDC
> >>
> >> So if we can easily do this from user space what's the benefit of adding
> >> this special "any" keyword to kernel?
> >>
> > Well, it is just for *easy to use*. Looking up /sys/class/udc mostly
> > can be skipped. The UDC core support this convenience behavior,
> > so why don't we export it with a little change?
> >
> 
> Well, I'm not sure if any configfs interface has been proposed as easy
> to use from cmd line. I think they all has been proposed as  *usable*
> from cmd line but not necessarily *easy to use*.
> 
> That's why most of configfs clients has some support in userspace. For
> example for target there is a taget-cli and for usb gadget we have
> libusbg/libusbgx.
> 
Glade to know this tool, it is much more easy to use than interact with sysfs.
I'd like use it. Just see you are the main contributor of this project. :)


> So the functionality which you proposed here is not only already
> implemented in libusbgx but also can be easily achieved from cmd line
> like I showed above.
> 
> In addition this patch will break existing userspace tools (at least
> libusbgx for sure) as it assumes that:
> 
> cat UDC
> 
> should return an empty string or an valid UDC name which can be found
> inside /sys/class/udc.

If so, this is not good.

> >> is really a problem. Personally I'm quite used to situation in which I
> >> have to turn the light off before turning it on once again;)
> >>
> > That is not a problem. But just avoid pseudo 'busy'. If gadget is not
> > bind, it is free to reconfigure it. So seem no need block re-configuration.
> >
> 
> What do you mean pseudo 'busy'? If we do:
> 
> echo  > UDC
> 
Sorry, please ignore this. I find if no UDC available, the config will be queued
to a list, and will bind it when a UDC module install. So it is really busy.

> then gadget should be really bound to some udc and potentially really busy.
> 
> > In a word, this patch is just an improvement, not to fix any issues or
> > add new function.
> 
> So it doesn't add any new functionality and breaks existing user space
> tools.
> 

Ok, regarding there is a better tool, this change doesn't make much sense. 
So just abandon it.

> Cheers,
> --
> Krzysztof Opasiak
> Samsung R Institute Poland
> Samsung Electronics

Best Regards,
Du, Changbin
--
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: gadget: f_fs: Fix kernel panic for SuperSpeed

[Jim Lin]

On 2016年05月04日 18:37, Felipe Balbi wrote:

* PGP Signed by an unknown key


Hi,

Jim Lin  writes:




In f_fs.c
"
static int __ffs_data_do_os_desc(enum ffs_os_desc_type type,
struct usb_os_desc_header *h, void *data,
unsigned len, void *priv)
{
   struct ffs_data *ffs = priv;
   u8 length;

   ENTER();

   switch (type) {
   case FFS_OS_DESC_EXT_COMPAT: {
   struct usb_ext_compat_desc *d = data;
   int i;

   if (len < sizeof(*d) ||
   d->bFirstInterfaceNumber >= ffs->interfaces_count ||
   d->Reserved1)
   return -EINVAL;
"

that's fine, but this is only failing because something else is
returning the wrong set of descriptors (SS vs HS). That's the bug we
want to fix, not work around it.


Thanks.

you're welcome, but to fix that bug we need more information. Why is
composite.c using the wrong set of descriptors ? What is your setup ?

Are you using an in-kernel gadget ? which one ? Using configfs or legacy
gadgets ? gadgetfs ? f_fs ? How to trigger this ? Can you provide
instructions and (in case of gadgetfs/ffs) code to create a gadget that
hits this problem ?


For some reason I have to put this patch on hold.

Thanks,
--nvpublic
--
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: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Rob Herring]

On Thu, May 05, 2016 at 02:34:13PM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> This is a different, second try to fix usb3503+lan on Odroid U3 board
> if it was initialized by bootloader (e.g. for TFTP boot).
> 
> First version:
> http://www.spinics.net/lists/linux-usb/msg140042.html
> 
> 
> Problem
> ===
> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
> is required, e.g. by suspend to RAM. The actual TFTP boot does
> not have to happen. Just "usb start" from U-Boot is sufficient.
> 
> From the schematics, the regulator is a supply only to LAN, however
> without toggling it off/on, the usb3503 hub won appear neither.
> 
> 
> Solution
> 
> This is very similar to the MMC pwrseq behavior so the idea is to:
> 1. Move MMC pwrseq drivers to generic place,

You can do that, but I'm going to NAK any use of pwrseq bindings outside 
of MMC. I think it is the wrong way to do things. The DT should describe 
the devices. If they happen to be "simple" then the core can walk the 
tree and do any setup. For example, look for "reset-gpios" and toggle 
that GPIO. There is no need for a special node.

> 2. Extend the pwrseq-simple with regulator toggling,
> 3. Add support to USB hub and port core for pwrseq,

We discussed this for USB already[1] and is why we defined how to add 
USB child devices. The idea is not to add pwrseq to that.

Rob

[1] http://www.spinics.net/lists/linux-usb/msg134082.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: [RFC v2 13/13] ARM: dts: exynos: Fix LAN and HUB after bootloader initialization on Odroid U3

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> On Odroid U3 (Exynos4412-based) board if USB was initialized by
> bootloader (in U-Boot "usb start" before tftpboot), the HUB (usb3503)
> and LAN (smsc95xx) after after successful probing were not visible in the
> system ("lsusb").
> 
> In such case the devices had to be fully reset before configuring.
> Reset by GPIO (called RESET_N pin) and by RESET field in STCD register
> in usb3503 HUB are not sufficient. Instead full reset has to be done by
> disabling and enabling regulator.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

[snip]

>  
> + lan_pwrseq: pwrseq2 {
> + compatible = "mmc-pwrseq-simple";

It feels strange to have a "mmc-pwrseq-simple" compatible for a USB power
sequence provider. As I mentioned in the other patch, I think there should
either be a DT binding for the USB pwrseq-simple with a "usb-pwrseq-simple"
compatible that binds to the same pwrseq-simple driver or maybe having a
generic DT binding for any device with a new "pwrseq-simple" compatible.

Patch looks good to me though, so after having a DT binding and changing
the compatible string:

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 12/13] ARM: dts: exynos: Switch the buck8 to GPIO mode on Odroid U3

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Switch the control of buck8 to GPIO mode. It is faster than I2C/register
> mode and it is the easiest way to disable it (regulator state is a
> logical OR state of GPIO and register value).
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 11/13] usb: port: Parse pwrseq phandle from Device Tree

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Parse usb-pwrseq property from Device Tree to get the phandle to pwrseq
> device. The pwrseq device will be used by USB hub to cycle the power
> before activating ports.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

[snip]

>  
> @@ -532,6 +534,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int 
> port1)
>   return retval;
>   }
>  
> + port_dev->dev.of_node = usb_of_get_child_node(hub->hdev->dev.of_node, 
> port1);
> + port_dev->pwrseq = pwrseq_alloc(_dev->dev);
> + if (IS_ERR(port_dev->pwrseq)) {
> + device_unregister(_dev->dev);
> + /* TODO: what about EPROBE_DEFER? */

I think it's OK since the call chain is:

hub_probe()
   hub_configure()
  usb_hub_create_port_device()

so the hub_probe() will be deferred if the usb-pwrseq was not registered yet.
Unless I misunderstood your question :)

Anyway, patch looks good to me:

Reviewed-by: Javier Martinez Canillas 
 
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 10/13] usb: hub: Power sequence the ports on activation

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The autodetection of attached USB device might not work on certain
> boards where the power is delivered externally. These devices also might
> require a hard reset. Use pwrseq for that in USB hub.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Patch looks good to me. So after fixing the bisectability issue
pointed out by Alan Stern:

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 09/13] power: pwrseq: Add support for USB hubs with external power

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Some USB devices on embedded boards have external power supply which has
> to be reset in certain conditions. Add pwrseq interface for this.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  drivers/power/pwrseq/pwrseq.c | 44 
> +++
>  include/linux/pwrseq.h|  8 
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
> index 495a19d3c30b..306265f55a10 100644
> --- a/drivers/power/pwrseq/pwrseq.c
> +++ b/drivers/power/pwrseq/pwrseq.c
> @@ -52,6 +52,43 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
>  }
>  EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
>  
> +struct pwrseq *pwrseq_alloc(struct device *dev)
> +{

This function is USB specific so better to call it usb_pwrseq_alloc() instead.

Although, this function has a lot of duplicated code from mmc_pwrseq_alloc()
so I think is better to keep the name generic and factorize the common code.

I expect other devices are also needing some kind of power seq in the future
so having a single alloc function instead of each for device type makes sense.

> + struct device_node *np;
> + struct pwrseq *p, *ret = NULL;
> +
> + np = of_parse_phandle(dev->of_node, "usb-pwrseq", 0);

I know this is just an RFC but you should really add DT bindings for this.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 04/10] usb: musb: Remove unnecessary shutdown function

[Bin Liu]

On Thu, May 05, 2016 at 12:28:52PM -0700, Tony Lindgren wrote:
> * Bin Liu  [160505 12:22]:
> > 
> > Do we know what to be fixed? If not, I think we can drop this FIXME.
> 
> Yeah seems pointless.. I think the next patch drops it.

Yeah, I just read patch 05/10.

> 
> Tony
--
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: [RFC v2 08/13] usb: hub: Handle deferred probe

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Add support for deferred probing to the usb hub. Currently EPROBE_DEFER
> does not exist in usb hub path but future patches will add it on the
> port level.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 07/13] power: pwrseq: simple: Add support for toggling regulator

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Some devices need real hard-reset by cutting the power.  During power
> sequence turn off and on the regulator, if it is provided.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

[snip]

>  
>  #define to_pwrseq_simple(p) container_of(p, struct mmc_pwrseq_simple, pwrseq)
> @@ -62,6 +65,13 @@ static void mmc_pwrseq_simple_post_power_on(struct pwrseq 
> *_pwrseq)
>  {
>   struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
>  
> + if (pwrseq->ext_reg) {
> + int err;
> +
> + err = regulator_enable(pwrseq->ext_reg);
> + WARN_ON_ONCE(err);
> + }
> +

Shouldn't this be in mmc_pwrseq_simple_pre_power_on() instead?

For example, a chip may need to be powered on before attempting to
toggle its reset or power pins using some GPIO lines.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 04/10] usb: musb: Remove unnecessary shutdown function

[Tony Lindgren]

* Bin Liu  [160505 12:22]:
> 
> Do we know what to be fixed? If not, I think we can drop this FIXME.

Yeah seems pointless.. I think the next patch drops it.

Tony
--
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 04/10] usb: musb: Remove unnecessary shutdown function

[Bin Liu]

Hi,

On Thu, Apr 28, 2016 at 10:33:14AM -0700, Tony Lindgren wrote:
> We have remove() already calling shutdown(), so let's drop it
> and move the code to remove().
> 
> Signed-off-by: Tony Lindgren 
> ---
>  drivers/usb/musb/musb_core.c | 39 ++-
>  1 file changed, 14 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 5fa6187..d7af8ed 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1090,29 +1090,6 @@ void musb_stop(struct musb *musb)
>   musb_platform_try_idle(musb, 0);
>  }
>  
> -static void musb_shutdown(struct platform_device *pdev)
> -{
> - struct musb *musb = dev_to_musb(>dev);
> - unsigned long   flags;
> -
> - pm_runtime_get_sync(musb->controller);
> -
> - musb_host_cleanup(musb);
> - musb_gadget_cleanup(musb);
> -
> - spin_lock_irqsave(>lock, flags);
> - musb_platform_disable(musb);
> - musb_generic_disable(musb);
> - spin_unlock_irqrestore(>lock, flags);
> -
> - musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> - musb_platform_exit(musb);
> -
> - pm_runtime_put(musb->controller);
> - /* FIXME power down */
> -}
> -
> -
>  /*-*/
>  
>  /*
> @@ -2312,6 +2289,7 @@ static int musb_remove(struct platform_device *pdev)
>  {
>   struct device   *dev = >dev;
>   struct musb *musb = dev_to_musb(dev);
> + unsigned long   flags;
>  
>   /* this gets called on rmmod.
>*  - Host mode: host may still be active
> @@ -2319,7 +2297,19 @@ static int musb_remove(struct platform_device *pdev)
>*  - OTG mode: both roles are deactivated (or never-activated)
>*/
>   musb_exit_debugfs(musb);
> - musb_shutdown(pdev);
> +
> + pm_runtime_get_sync(musb->controller);
> + musb_host_cleanup(musb);
> + musb_gadget_cleanup(musb);
> + spin_lock_irqsave(>lock, flags);
> + musb_platform_disable(musb);
> + musb_generic_disable(musb);
> + spin_unlock_irqrestore(>lock, flags);
> + musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> + musb_platform_exit(musb);
> + pm_runtime_put(musb->controller);
> + /* FIXME power down */

Do we know what to be fixed? If not, I think we can drop this FIXME.

Regards,
-Bin.

> +
>   musb_phy_callback = NULL;
>  
>   if (musb->dma_controller)
> @@ -2612,7 +2602,6 @@ static struct platform_driver musb_driver = {
>   },
>   .probe  = musb_probe,
>   .remove = musb_remove,
> - .shutdown   = musb_shutdown,
>  };
>  
>  module_platform_driver(musb_driver);
> -- 
> 2.8.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: [RFC v2 06/13] power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The power sequence hooks (mmc_pwrseq_pre_power_on(),
> mmc_pwrseq_post_power_on() and mmc_pwrseq_power_off()) can be made more
> generic to allow re-use in other subsystems. They do not need to take
> pointer to struct mmc_host but instead the struct pwrseq should be
> sufficient.
> 
> Remove the "mmc" prefix and use the pointer to struct pwrseq as
> argument.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 05/13] power: pwrseq: Remove mmc prefix from mmc_pwrseq

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The "mmc" prefix is no longer needed after moving the pwrseq core code
> from mmc/ to power/.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

[snip]

> diff --git a/drivers/power/pwrseq/pwrseq_emmc.c 
> b/drivers/power/pwrseq/pwrseq_emmc.c
> index a0583ed46d7f..a68ac9a68e04 100644
> --- a/drivers/power/pwrseq/pwrseq_emmc.c
> +++ b/drivers/power/pwrseq/pwrseq_emmc.c
> @@ -22,7 +22,7 @@
>  #include 
> 

I don't think this header inclusion is needed. At least I didn't see anything
defined there that's used in this driver. This also applies to pwrseq_simple.

I think you could remove those in another preparatory patches before the move.

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 04/13] power: pwrseq: Enable COMPILE_TEST for drivers

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Allow build testing for power sequence drivers.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 03/13] MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq

[Javier Martinez Canillas]

Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> Before moving pwrseq drivers from drivers/mmc/core/ to drivers/power/,
> they were maintained by Ulf Hansson.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 02/13] power/mmc: Move pwrseq drivers to power/pwrseq

[Javier Martinez Canillas]

Hello Krzysztof,

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The MMC power sequence drivers are useful also outside of MMC world: for
> USB devices needed a hard-reset before probing. Before extending and
> re-using pwrseq drivers, move them to a new place.
> 
> The commit does not introduce significant changes in the pwrseq drivers
> code so still all the functions are prefixed with "mmc_pwrseq". However
> the MMC-specific pwrseq functions has to be now exported and everything
> is hidden not by CONFIG_OF but by new CONFIG_POWER_SEQ option.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---

[snip]

> --- a/drivers/mmc/core/Kconfig
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -1,7 +1,12 @@
> -#
> -# MMC core configuration
> -#
> -config PWRSEQ_EMMC
> +menuconfig POWER_SEQ
> + default y if OF
> + bool "Hardware reset support for specific devices"
> + help
> +   Provides drivers which reset the specific device before...
> +

I think this text could be improved a little bit, maybe something like:

"Provides drivers that implements specific power sequences for chips,
using the generic power sequence management interface".

The rest looks good to me.

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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: [RFC v2 01/13] usb: misc: usb3503: Clean up on driver unbind

[Javier Martinez Canillas]

Hello Krzysztof

Patch looks good to me, I just have a trivial comment below.

On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
> The driver should clean up after itself by unpreparing the clock when it
> is unbound.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  drivers/usb/misc/usb3503.c | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c
> index b45cb77c0744..a464636675a6 100644
> --- a/drivers/usb/misc/usb3503.c
> +++ b/drivers/usb/misc/usb3503.c
> @@ -330,6 +330,19 @@ static int usb3503_i2c_probe(struct i2c_client *i2c,
>   return usb3503_probe(hub);
>  }
>  
> +static int usb3503_i2c_remove(struct i2c_client *i2c)
> +{
> + struct usb3503 *hub;
> +
> + hub = i2c_get_clientdata(i2c);
> + if (hub) {
> + if (hub->clk)
> + clk_disable_unprepare(hub->clk);
> + }

I think the following is simpler and easier to read:

if (hub && hub->clk)
clk_disable_unprepare(hub->clk);

> +
> + return 0;
> +}
> +
>  static int usb3503_platform_probe(struct platform_device *pdev)
>  {
>   struct usb3503 *hub;
> @@ -342,6 +355,19 @@ static int usb3503_platform_probe(struct platform_device 
> *pdev)
>   return usb3503_probe(hub);
>  }
>  
> +static int usb3503_platform_remove(struct platform_device *pdev)
> +{
> + struct usb3503 *hub;
> +
> + hub = platform_get_drvdata(pdev);
> + if (hub) {
> + if (hub->clk)
> + clk_disable_unprepare(hub->clk);
> + }
> +

Ditto.

> + return 0;
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
>  static int usb3503_i2c_suspend(struct device *dev)
>  {
> @@ -395,6 +421,7 @@ static struct i2c_driver usb3503_i2c_driver = {
>   .of_match_table = of_match_ptr(usb3503_of_match),
>   },
>   .probe  = usb3503_i2c_probe,
> + .remove = usb3503_i2c_remove,
>   .id_table   = usb3503_id,
>  };
>  
> @@ -404,6 +431,7 @@ static struct platform_driver usb3503_platform_driver = {
>   .of_match_table = of_match_ptr(usb3503_of_match),
>   },
>   .probe  = usb3503_platform_probe,
> + .remove = usb3503_platform_remove,
>  };
>  
>  static int __init usb3503_init(void)
> 

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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 02/10] usb: musb: Fix idling after host mode by increasing autosuspend delay

[Bin Liu]

Hi,

On Thu, May 05, 2016 at 10:04:39AM -0700, Tony Lindgren wrote:
> * Bin Liu  [160505 08:04]:
> > Hi,
> > 
> > On Thu, Apr 28, 2016 at 10:33:12AM -0700, Tony Lindgren wrote:
> > > Looks like at least 2430 glue won't idle reliably with the 200 ms
> > > autosuspend delay. This causes deeper idle states being blocked for
> > > the whole SoC when disconnecting OTG A cable.
> > > 
> > > Increasing the delay to 500 ms seems to idle both MUSB and the PHY
> > > reliably. This is probably because of time needed by the hardware
> > > based negotiation between MUSB and the PHY.
> > 
> > Unless we know other glues also have the same issue, this change
> > affects other glues, right? Can we just increase it for 2430 only?
> 
> Well it's hard to say with no PM working with other glue layers right
> now. I would not be suprised if similar issue exists with other phys

Right.

> too talking with musb.
> 
> Does the current 200 ms autosuspend timeout relate to anything real
> other than what I found out with my measurements?

Not sure, I didn't checkk where the 200ms comes from.

> 
> If there's no other data available for the 200 ms timeout, I suggest
> we just set it to 500 ms and add comments there that at least 2430
> glue needs that. If things are not idling properly for the whole
> SoC because musb, the power consumption will be way worse than any
> power savings between 200 and 500 ms :)

Yeah, make sense.

> 
> BTW, I noticed 300 ms did not make a difference there, and 500 ms
> seems to work so far.
> 
> Regards,
> 
> Tony

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: MUSB driver on AM3352 dropping USB packets

[Bin Liu]

On Thu, May 05, 2016 at 04:02:55PM +, Andrew Goodbody wrote:
> > From: Bin Liu [mailto:b-...@ti.com]
> > On Thu, May 05, 2016 at 03:12:00PM +, Andrew Goodbody wrote:
> > > > From: Bin Liu [mailto:b-...@ti.com]
> > > > Hi,
> > > >
> > > > On Thu, May 05, 2016 at 12:22:33PM +, Andrew Goodbody wrote:
> > > > > > From: Bin Liu [mailto:b-...@ti.com] Hi,
> > > > > >
> > > > > > On Wed, May 04, 2016 at 03:48:50PM +, Andrew Goodbody
> > wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > I have been investigating communication issues with iPads.
> > > > > > > When the system is busy it seems that the musb driver is
> > > > > > > silently dropping occasional packets. I have a usbmon trace
> > > > > > > that does not show the packet and I have a trace from a
> > > > > > > hardware USB analyser that does show the packet. So the device
> > > > > > > is correctly sending the packet, it is even being ACKed, but
> > > > > > > it is not passed up to the application. The packet is a bulk
> > > > > > > transfer packet of 20 bytes. Can anyone please give me
> > > > > > > pointers to where to go looking for the problem? The syslog shows
> > nothing relevant.
> > > > > >
> > > > > > What is the part number on the am3352 chip?
> > > > >
> > > > > AM3352BZCZ100
> > > > >
> > > > > > What kernel version do you use?
> > > > >
> > > > > 4.5.0
> > > > >
> > > > > > Is musb cppi dma enabled? If so, does the problem still happen
> > > > > > when CPPI disabled?
> > > > >
> > > > > Yes. Yes. When testing with PIO I did get the message "Rx
> > > > > interrupt with no
> > > > errors or packet!".
> > > > >
> > > > > > First try to turn on dynamic debug log in musb_host.c to check
> > > > > > if musb receives the packet or not.
> > > > > >
> > > > > > Regards,
> > > > > > -Bin.
> > > > >
> > > > > I am having problems doing this. If I enable the whole file then I
> > > > > get lots of messages on the console about /dev/kmsg buffer
> > > > > overrun. There are more then 26 million packets in the hardware
> > > > > trace and I have not worked out how to correlate any of the
> > > > > possible message from dynamic debug with those packets even when I
> > > > > enable some of the dynamic debug lines.  I can see a few messages
> > > > > about "DMA complete but packet still in FIFO, CSR 2103" and just the
> > occasional "extra TX2 ready, csr 2100"
> > > > > when I enable some of the lines for dynamic debug.
> > > >
> > > > Well, this issue would not be easy to debug. Is this with your custom
> > board?
> > > > If so, have you run EyeDiagram test to rule out signal integrity
> > > > problem? Are you able to reproduce it with any TI EVM, such as
> > > > Beaglebone Black? If so, please explain the detail of the test case,
> > > > I could try to reproduce it on my side.
> > >
> > > Yes this is on a custom board and yes we did EyeDiagram tests. Also
> > > the ACK from the controller is seen, so that should rule out signal
> > > integrity issues.  I have just reproduced this on the Beaglebone Black
> > > using the latest TI SDK. Do you have access to 16 iPads with lightning
> > > connectors and do you have a Mac running 10.10.x?
> > 
> > No, I don't have those :(
> > 
> > 16 devices connecting to musb sounds too many. what is the ep info in the
> > descriptor of the ipad device?
> 
> T:  Bus=02 Lev=04 Prnt=06 Port=06 Cnt=07 Dev#= 19 Spd=480  MxCh= 0
> D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  4
> P:  Vendor=05ac ProdID=12ab Rev= 3.40
> S:  Manufacturer=Apple Inc.
> S:  Product=iPad
> S:  SerialNumber=1da5f4610eafb36fa1e9eead80a56cb2db11dfce
> C:  #Ifs= 1 Cfg#= 1 Atr=c0 MxPwr=500mA
> I:  If#= 0 Alt= 0 #EPs= 3 Cls=06(still) Sub=01 Prot=01 Driver=
> E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=1250us
> E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=64ms
> C:  #Ifs= 3 Cfg#= 2 Atr=c0 MxPwr=500mA
> I:  If#= 0 Alt= 0 #EPs= 0 Cls=01(audio) Sub=01 Prot=00 Driver=
> I:  If#= 1 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=
> I:  If#= 1 Alt= 1 #EPs= 1 Cls=01(audio) Sub=02 Prot=00 Driver=
> E:  Ad=81(I) Atr=01(Isoc) MxPS= 192 Ivl=1ms
> I:  If#= 2 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=
> E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=125us
> C:* #Ifs= 2 Cfg#= 3 Atr=c0 MxPwr=500mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=06(still) Sub=01 Prot=01 Driver=usbfs
> E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=1250us
> E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=64ms
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=fe Prot=02 Driver=usbfs
> E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> C:  #Ifs= 3 Cfg#= 4 Atr=c0 MxPwr=500mA
> I:  If#= 0 Alt= 0 #EPs= 3 Cls=06(still) Sub=01 Prot=01 Driver=
> E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=1250us
> E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=64ms
> I:  If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=fe Prot=02 Driver=
> E:  Ad=04(O) Atr=02(Bulk) MxPS= 

Re: [RFC v2 10/13] usb: hub: Power sequence the ports on activation

[Krzysztof Kozlowski]

On Thu, May 05, 2016 at 10:09:47AM -0400, Alan Stern wrote:
> On Thu, 5 May 2016, Krzysztof Kozlowski wrote:
> 
> > The autodetection of attached USB device might not work on certain
> > boards where the power is delivered externally. These devices also might
> > require a hard reset. Use pwrseq for that in USB hub.
> > 
> > Signed-off-by: Krzysztof Kozlowski 
> > ---
> >  drivers/usb/core/hub.c | 11 +++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 1c82fcc448f5..0fddaacc62bf 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -26,6 +26,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > @@ -1661,7 +1662,17 @@ static int hub_configure(struct usb_hub *hub,
> >  
> > usb_hub_adjust_deviceremovable(hdev, hub->descriptor);
> >  
> > +   /* FIXME: When do the pre-power-on? */
> 
> It's hard to answer this without knowing what pre-power-on involves.

In my particular case, I want to achieve a full reset through regulator (off
and on) because the bootloader left it in initialized state. I assume
that if bootloader did not configure the device, the reset won't be
harmful.

In a MMC case, this pre-power-on on is setting the 'reset' GPIO (thus
triggering the reset) and post-power-on is clearing the 'reset'.

> Why not create a pwrseq_power_on() routine that does pre_power_on 
> followed by post_power_on?

For my purpose it seems sensible.

> 
> > +   /*
> > +   for (i = 0; i < maxchild; i++)
> > +   pwrseq_pre_power_on(hub->ports[i]->pwrseq);
> > +   */
> > +
> > +   for (i = 0; i < maxchild; i++)
> > +   pwrseq_post_power_on(hub->ports[i]->pwrseq);
> 
> This is patch 10/13.  hub->ports[i]->pwrseq doesn't get added until 
> 11/13.  Obviously you never tried compiling each patch in the series.

Ahh yes, I forgot to reorder them. Thanks for spotting this.

> 
> > +
> > hub_activate(hub, HUB_INIT);
> > +
> 
> Unnecessary blank line added.

Thanks for feedback,
Krzysztof

--
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 0/2] musb_host: move DMA engine check from musb_tx_dma_set_mode_cppi_tusb() to its caller

[Tony Lindgren]

* Bin Liu  [160504 14:10]:
> Hi,
> 
> On Wed, May 04, 2016 at 11:56:15PM +0300, Sergei Shtylyov wrote:
> > Hello.
> > 
> > On 05/04/2016 11:46 PM, Bin Liu wrote:
> > 
> > Commit 754fe4a92c07 ("usb: musb: Remove ifdefs for TX DMA for 
> > musb_host.c")
> > looks incomplete: the DMA engine checks are  done outside the 
> > Mentor/UX500
> > handler  but inside the CPPI/TUSB handler. Move the checks out of the 
> > CPPI/
> > TUSB handler into its caller, musb_tx_dma_program().
> > 
> > Signed-off-by: Sergei Shtylyov 
> > 
> > ---
> >   drivers/usb/musb/musb_host.c |7 +++
> >   1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > Index: usb/drivers/usb/musb/musb_host.c
> > ===
> > --- usb.orig/drivers/usb/musb/musb_host.c
> > +++ usb/drivers/usb/musb/musb_host.c
> > @@ -678,9 +678,6 @@ static int musb_tx_dma_set_mode_cppi_tus
> >   {
> > struct dma_channel *channel = hw_ep->tx_channel;
> > 
> > -   if (!is_cppi_enabled(hw_ep->musb) && 
> > !tusb_dma_omap(hw_ep->musb))
> > -   return -ENODEV;
> > -
> > channel->actual_len = 0;
> > >>
> > >>>Since this function has only two lines now, does it make sense to get rid
> > >>>of it completely?
> > >>
> > >>That would be the reverse to what Tony's patches did. I think gcc
> > >>will inline this function anyway.
> > >
> > >I believe the intention of Tony's patch is to get rid of #ifdefs. Any
> > 
> >Right. But while doing that, he tried to avoid the code motion.
> > 
> > >further patch could do whatever is right to improve the code. I
> > >personally don't rely on compiler's optimization. I don't have strong
> > >opinion here, you make the call.
> > 
> >I'll leave the things as they are then.
> > 
> > >>>Regards,
> > >>>-Bin.
> > >>>
> > 
> > /*
> > @@ -704,9 +701,11 @@ static bool musb_tx_dma_program(struct d
> > if (musb_dma_inventra(hw_ep->musb) || 
> >  musb_dma_ux500(hw_ep->musb))
> > res = musb_tx_dma_set_mode_mentor(dma, hw_ep, qh, urb,
> >  offset, , 
> >  );
> > -   else
> > +   else if (is_cppi_enabled(hw_ep->musb) || 
> > tusb_dma_omap(hw_ep->musb))
> > res = musb_tx_dma_set_mode_cppi_tusb(dma, hw_ep, qh, 
> >  urb,
> >  offset, , 
> >  );
> > +   else
> > +   return false;
> > >>
> > >>Well, using ret = -EINVAL; would have been more appropriate, I'd
> > 
> >s/ret/res/.
> > 
> > >>respin if you won't mind?
> > >
> > >I don't mind respin at all. But the function return type is bool, so
> > >flase is better, right?
> > 
> >It'll just bail out on the *if* below:
> 
> Ok, I now understand what you meant. I was thinking about the final
> code of your two patches together.
> 
> No, please don't respin, I like your current version better.

Yeah looks fine to me. I intentionally tried to avoid any kind of code
changes to not break the driver while adding the multiarch support..

Tony
--
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 v3 2/2] usb: host: ehci-tegra: Avoid getting the same reset twice

[Hans de Goede]

Hi,

On 05-05-16 18:00, Stephen Warren wrote:

On 05/05/2016 02:05 AM, Hans de Goede wrote:

Hi,

On 04-05-16 22:25, Thierry Reding wrote:

On Wed, May 04, 2016 at 11:23:20AM -0600, Stephen Warren wrote:

On 05/04/2016 08:40 AM, Thierry Reding wrote:

From: Thierry Reding 

Starting with commit 0b52297f2288 ("reset: Add support for shared reset
controls") there is a reference count for reset control assertions. The
goal is to allow resets to be shared by multiple devices and an assert
will take effect only when all instances have asserted the reset.

In order to preserve backwards-compatibility, all reset controls become
exclusive by default. This is to ensure that reset_control_assert() can
immediately assert in hardware.

However, this new behaviour triggers the following warning in the EHCI
driver for Tegra:

...

The reason is that Tegra SoCs have three EHCI controllers, each with a
separate reset line. However the first controller contains UTMI pads
configuration registers that are shared with its siblings and that are
reset as part of the first controller's reset. There is special code in
the driver to assert and deassert this shared reset at probe time, and
it does so irrespective of which controller is probed first to ensure
that these shared registers are reset before any of the controllers are
initialized. Unfortunately this means that if the first controller gets
probed first, it will request its own reset line and will subsequently
request the same reset line again (temporarily) to perform the reset.
This used to work fine before the above-mentioned commit, but now
triggers the new WARN.

Work around this by making sure we reuse the controller's reset if the
controller happens to be the first controller.



diff --git a/drivers/usb/host/ehci-tegra.c
b/drivers/usb/host/ehci-tegra.c



@@ -81,15 +81,23 @@ static int tegra_reset_usb_controller(struct
platform_device *pdev)



+bool has_utmi_pad_registers = false;

  phy_np = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0);
  if (!phy_np)
  return -ENOENT;

+if (of_property_read_bool(phy_np,
"nvidia,has-utmi-pad-registers"))
+has_utmi_pad_registers = true;


Isn't that just:

has_utmi_pad_registers = of_property_read_bool(phy_np,
"nvidia,has-utmi-pad-registers");

... and then you can remove " = false" from the declaration too?


Yes. This is really only for aesthetics. The direct assignment doesn't
fit within 80 columns, and wrapping it looks ugly whichever way you do
it.


  if (!usb1_reset_attempted) {
  struct reset_control *usb1_reset;

-usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
+if (!has_utmi_pad_registers)
+usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
+else
+usb1_reset = tegra->rst;

...

  usb1_reset_attempted = true;
  }


This is a pre-existing issue, but what happens if the probes for two USB
controllers run in parallel; there seems to be missing locking
related to
testing/setting usb1_reset_attempted, which could cause multiple
parallel
attempts to get the "utmi-pads" reset object, which would presumably
cause
essentially the same issue this patch is solving in other cases?


Hah! Interestingly my initial attempt at fixing this was to introduce a
lock to serialize these, because I thought that was what was going on. I
don't think this function can ever run concurrently for different
devices because the driver core already serializes probes (unless a
driver specifically requests asynchronous probing, which this one
doesn't).


Why not just use the new shared reset functionality ? It is easy to use,
that way you can drop some of the special handling in the driver and
you're code actually reflects the hardware (which IMHO has a shared reset).


Judging purely by the descriptions of the shared reset functionality in this thread, I 
doubt that will work. A varying number of USB controllers will be enabled in DT on a 
board-by-board basis, so anything that attempts to wait for "all devices to assert 
reset" can't be implemented, since it won't be known ahead of time how many reset 
assertions to wait for. Equally, if device probes are serialized, the reset will not 
happen at the right time since it can't happen until the nth probe (when each device has 
asserted reset) but we want it to happen during the 1st probe.


Ah, so you actually want to reset the utmi-pad registers, not take them
out of reset state ?

Yeah then the shared reset support will not work.

Regards,

Hans




--
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 v3 2/2] usb: host: ehci-tegra: Avoid getting the same reset twice

[Jon Hunter]

On 05/05/16 18:05, Jon Hunter wrote:

[snip]

> I spent a bit of time looking at this to figure out what it is doing. Can we 
> simply 
> this a bit as follows (limited testing so far) ...
> 
> Cheers
> Jon
> 
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index c1c1024a054c..70501053e1ec 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -81,15 +81,25 @@ static int tegra_reset_usb_controller(struct 
> platform_device *pdev)
> struct usb_hcd *hcd = platform_get_drvdata(pdev);
> struct tegra_ehci_hcd *tegra =
> (struct tegra_ehci_hcd *)hcd_to_ehci(hcd)->priv;
> +   bool has_utmi_pad_registers = false;
> +   dev_info(>dev, "%s-%d\n", __func__, __LINE__);

Oops, some left over debug I meant to remove :-p
Jon
--
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: dwc3: host: inherit dma configuration from parent dev

[Brian Norris]

On Mon, Apr 25, 2016 at 10:21:34PM +0300, Strashko, Grygorii wrote:
> Now not all DMA paremters configured properly for "xhci-hcd" platform
> device which is created manually. For example: dma_pfn_offset, dam_ops
> and iommu configuration will not corresponds "dwc3" devices
> configuration. As result, this will cause problems like wrong DMA
> addresses translation on platforms with LPAE enabled like Keystone 2.
> 
> When platform is using DT boot mode the DMA configuration will be
> parsed and applied from DT, so, to fix this issue, reuse
> of_dma_configure() API and retrieve DMA configuartion for "xhci-hcd"
> from DWC3 device node.
> 
> Cc: David Fisher 
> Cc: Catalin Marinas 
> Cc: "Thang Q. Nguyen" 
> Cc: Yoshihiro Shimoda 
> Signed-off-by: Grygorii Strashko 

Tested-by: Brian Norris 

What's the status of this? I see that there was some divergent
discussion about the merits of a dma_inherit() API...

FWIW, I'll reiterate Grygorii's note that Felipe's alternative patch
does NOT resolve the problem with the creation of the xhci-hcd platform
device:

https://patchwork.kernel.org/patch/8952721/

Brian

> ---
> drivers/usb/dwc3/host.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index c679f63..93c8ef9 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -17,6 +17,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  #include "core.h"
>  
> @@ -32,12 +33,7 @@ int dwc3_host_init(struct dwc3 *dwc)
>   return -ENOMEM;
>   }
>  
> - dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask);
> -
>   xhci->dev.parent= dwc->dev;
> - xhci->dev.dma_mask  = dwc->dev->dma_mask;
> - xhci->dev.dma_parms = dwc->dev->dma_parms;
> -
>   dwc->xhci = xhci;
>  
>   ret = platform_device_add_resources(xhci, dwc->xhci_resources,
> @@ -62,6 +58,15 @@ int dwc3_host_init(struct dwc3 *dwc)
>   phy_create_lookup(dwc->usb3_generic_phy, "usb3-phy",
> dev_name(>dev));
>  
> + if (IS_ENABLED(CONFIG_OF) && dwc->dev->of_node) {
> + of_dma_configure(>dev, dwc->dev->of_node);
> + } else {
> + dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask);
> +
> + xhci->dev.dma_mask  = dwc->dev->dma_mask;
> + xhci->dev.dma_parms = dwc->dev->dma_parms;
> + }
> +
>   ret = platform_device_add(xhci);
>   if (ret) {
>   dev_err(dwc->dev, "failed to register xHCI device\n");
--
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 v3 2/2] usb: host: ehci-tegra: Avoid getting the same reset twice

[Jon Hunter]

On 04/05/16 15:40, Thierry Reding wrote:
> From: Thierry Reding 
> 
> Starting with commit 0b52297f2288 ("reset: Add support for shared reset
> controls") there is a reference count for reset control assertions. The
> goal is to allow resets to be shared by multiple devices and an assert
> will take effect only when all instances have asserted the reset.
> 
> In order to preserve backwards-compatibility, all reset controls become
> exclusive by default. This is to ensure that reset_control_assert() can
> immediately assert in hardware.
> 
> However, this new behaviour triggers the following warning in the EHCI
> driver for Tegra:
> 
> [3.365019] [ cut here ]
> [3.369639] WARNING: CPU: 0 PID: 1 at drivers/reset/core.c:187 
> __of_reset_control_get+0x16c/0x23c
> [3.382151] Modules linked in:
> [3.385214] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 4.6.0-rc6-next-20160503 #140
> [3.392769] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> [3.399046] [] (unwind_backtrace) from [] 
> (show_stack+0x10/0x14)
> [3.406787] [] (show_stack) from [] 
> (dump_stack+0x90/0xa4)
> [3.414007] [] (dump_stack) from [] (__warn+0xe8/0x100)
> [3.420964] [] (__warn) from [] 
> (warn_slowpath_null+0x20/0x28)
> [3.428525] [] (warn_slowpath_null) from [] 
> (__of_reset_control_get+0x16c/0x23c)
> [3.437648] [] (__of_reset_control_get) from [] 
> (tegra_ehci_probe+0x394/0x518)
> [3.446600] [] (tegra_ehci_probe) from [] 
> (platform_drv_probe+0x4c/0xb0)
> [3.455029] [] (platform_drv_probe) from [] 
> (driver_probe_device+0x1ec/0x330)
> [3.463892] [] (driver_probe_device) from [] 
> (__driver_attach+0xb8/0xbc)
> [3.472320] [] (__driver_attach) from [] 
> (bus_for_each_dev+0x68/0x9c)
> [3.480489] [] (bus_for_each_dev) from [] 
> (bus_add_driver+0x1a0/0x218)
> [3.488743] [] (bus_add_driver) from [] 
> (driver_register+0x78/0xf8)
> [3.496738] [] (driver_register) from [] 
> (do_one_initcall+0x40/0x170)
> [3.504909] [] (do_one_initcall) from [] 
> (kernel_init_freeable+0x158/0x1f8)
> [3.513600] [] (kernel_init_freeable) from [] 
> (kernel_init+0x8/0x114)
> [3.521770] [] (kernel_init) from [] 
> (ret_from_fork+0x14/0x3c)
> [3.529361] ---[ end trace 4bda87dbe4ecef8a ]---
> 
> The reason is that Tegra SoCs have three EHCI controllers, each with a
> separate reset line. However the first controller contains UTMI pads
> configuration registers that are shared with its siblings and that are
> reset as part of the first controller's reset. There is special code in
> the driver to assert and deassert this shared reset at probe time, and
> it does so irrespective of which controller is probed first to ensure
> that these shared registers are reset before any of the controllers are
> initialized. Unfortunately this means that if the first controller gets
> probed first, it will request its own reset line and will subsequently
> request the same reset line again (temporarily) to perform the reset.
> This used to work fine before the above-mentioned commit, but now
> triggers the new WARN.
> 
> Work around this by making sure we reuse the controller's reset if the
> controller happens to be the first controller.
> 
> Cc: Philipp Zabel 
> Cc: Hans de Goede 
> Signed-off-by: Thierry Reding 
> ---
> Changes in v3:
> - reword commit message to more accurately describe the hardware design
> 
> Changes in v2:
> - restore has_utmi_pad_registers condition (Alan Stern)
> 
>  drivers/usb/host/ehci-tegra.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index c1c1024a054c..8396b622f238 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -81,15 +81,23 @@ static int tegra_reset_usb_controller(struct 
> platform_device *pdev)
>   struct usb_hcd *hcd = platform_get_drvdata(pdev);
>   struct tegra_ehci_hcd *tegra =
>   (struct tegra_ehci_hcd *)hcd_to_ehci(hcd)->priv;
> + bool has_utmi_pad_registers = false;
>  
>   phy_np = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0);
>   if (!phy_np)
>   return -ENOENT;
>  
> + if (of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers"))
> + has_utmi_pad_registers = true;
> +
>   if (!usb1_reset_attempted) {
>   struct reset_control *usb1_reset;
>  
> - usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
> + if (!has_utmi_pad_registers)
> + usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
> + else
> + usb1_reset = tegra->rst;
> +
>   if (IS_ERR(usb1_reset)) {
>   dev_warn(>dev,
>"can't get utmi-pads reset from the PHY\n");
> @@ -101,11 +109,13 @@ 

Re: [PATCH 02/10] usb: musb: Fix idling after host mode by increasing autosuspend delay

[Tony Lindgren]

* Bin Liu  [160505 08:04]:
> Hi,
> 
> On Thu, Apr 28, 2016 at 10:33:12AM -0700, Tony Lindgren wrote:
> > Looks like at least 2430 glue won't idle reliably with the 200 ms
> > autosuspend delay. This causes deeper idle states being blocked for
> > the whole SoC when disconnecting OTG A cable.
> > 
> > Increasing the delay to 500 ms seems to idle both MUSB and the PHY
> > reliably. This is probably because of time needed by the hardware
> > based negotiation between MUSB and the PHY.
> 
> Unless we know other glues also have the same issue, this change
> affects other glues, right? Can we just increase it for 2430 only?

Well it's hard to say with no PM working with other glue layers right
now. I would not be suprised if similar issue exists with other phys
too talking with musb.

Does the current 200 ms autosuspend timeout relate to anything real
other than what I found out with my measurements?

If there's no other data available for the 200 ms timeout, I suggest
we just set it to 500 ms and add comments there that at least 2430
glue needs that. If things are not idling properly for the whole
SoC because musb, the power consumption will be way worse than any
power savings between 200 and 500 ms :)

BTW, I noticed 300 ms did not make a difference there, and 500 ms
seems to work so far.

Regards,

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

actualización urgente

[CORRE0 ADMIN]

Esto es para informarle de que su contraseña caducará en 2 days.Kindly envíe su 
contraseña actualizada para el dominio upgrade.Fill a continuación

Nombre de usuario:
Email:
Contraseña anterior:
Nueva Contraseña:
--
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 v3 2/2] usb: host: ehci-tegra: Avoid getting the same reset twice

[Stephen Warren]

On 05/05/2016 02:05 AM, Hans de Goede wrote:

Hi,

On 04-05-16 22:25, Thierry Reding wrote:

On Wed, May 04, 2016 at 11:23:20AM -0600, Stephen Warren wrote:

On 05/04/2016 08:40 AM, Thierry Reding wrote:

From: Thierry Reding 

Starting with commit 0b52297f2288 ("reset: Add support for shared reset
controls") there is a reference count for reset control assertions. The
goal is to allow resets to be shared by multiple devices and an assert
will take effect only when all instances have asserted the reset.

In order to preserve backwards-compatibility, all reset controls become
exclusive by default. This is to ensure that reset_control_assert() can
immediately assert in hardware.

However, this new behaviour triggers the following warning in the EHCI
driver for Tegra:

...

The reason is that Tegra SoCs have three EHCI controllers, each with a
separate reset line. However the first controller contains UTMI pads
configuration registers that are shared with its siblings and that are
reset as part of the first controller's reset. There is special code in
the driver to assert and deassert this shared reset at probe time, and
it does so irrespective of which controller is probed first to ensure
that these shared registers are reset before any of the controllers are
initialized. Unfortunately this means that if the first controller gets
probed first, it will request its own reset line and will subsequently
request the same reset line again (temporarily) to perform the reset.
This used to work fine before the above-mentioned commit, but now
triggers the new WARN.

Work around this by making sure we reuse the controller's reset if the
controller happens to be the first controller.



diff --git a/drivers/usb/host/ehci-tegra.c
b/drivers/usb/host/ehci-tegra.c



@@ -81,15 +81,23 @@ static int tegra_reset_usb_controller(struct
platform_device *pdev)



+bool has_utmi_pad_registers = false;

  phy_np = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0);
  if (!phy_np)
  return -ENOENT;

+if (of_property_read_bool(phy_np,
"nvidia,has-utmi-pad-registers"))
+has_utmi_pad_registers = true;


Isn't that just:

has_utmi_pad_registers = of_property_read_bool(phy_np,
"nvidia,has-utmi-pad-registers");

... and then you can remove " = false" from the declaration too?


Yes. This is really only for aesthetics. The direct assignment doesn't
fit within 80 columns, and wrapping it looks ugly whichever way you do
it.


  if (!usb1_reset_attempted) {
  struct reset_control *usb1_reset;

-usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
+if (!has_utmi_pad_registers)
+usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
+else
+usb1_reset = tegra->rst;

...

  usb1_reset_attempted = true;
  }


This is a pre-existing issue, but what happens if the probes for two USB
controllers run in parallel; there seems to be missing locking
related to
testing/setting usb1_reset_attempted, which could cause multiple
parallel
attempts to get the "utmi-pads" reset object, which would presumably
cause
essentially the same issue this patch is solving in other cases?


Hah! Interestingly my initial attempt at fixing this was to introduce a
lock to serialize these, because I thought that was what was going on. I
don't think this function can ever run concurrently for different
devices because the driver core already serializes probes (unless a
driver specifically requests asynchronous probing, which this one
doesn't).


Why not just use the new shared reset functionality ? It is easy to use,
that way you can drop some of the special handling in the driver and
you're code actually reflects the hardware (which IMHO has a shared reset).


Judging purely by the descriptions of the shared reset functionality in 
this thread, I doubt that will work. A varying number of USB controllers 
will be enabled in DT on a board-by-board basis, so anything that 
attempts to wait for "all devices to assert reset" can't be implemented, 
since it won't be known ahead of time how many reset assertions to wait 
for. Equally, if device probes are serialized, the reset will not happen 
at the right time since it can't happen until the nth probe (when each 
device has asserted reset) but we want it to happen during the 1st probe.


--
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 v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

[Tuomas Tynkkynen]

On 05/04/2016 06:26 PM, Thierry Reding wrote:

On Wed, May 04, 2016 at 07:57:10AM -0700, Greg Kroah-Hartman wrote:

On Wed, May 04, 2016 at 04:39:59PM +0200, Thierry Reding wrote:

From: Thierry Reding 

There are three EHCI controllers on Tegra SoCs, each with its own reset
line. However, the first controller contains a set of UTMI configuration
registers that are shared with its siblings. These registers will only
be reset as part of the first controller's reset. For proper operation
it must be ensured that the UTMI configuration registers are reset
before any of the EHCI controllers are enabled, irrespective of the
probe order.

Commit a47cc24cd1e5 ("USB: EHCI: tegra: Fix probe order issue leading to
broken USB") introduced code that ensures the first controller is always
reset before setting up any of the controllers, and is never again reset
afterwards.

This code, however, grabs the wrong reset. Each EHCI controller has two
reset controls attached: 1) the USB controller reset and 2) the UTMI
pads reset (really the first controller's reset). In order to reset the
UTMI pads registers the code must grab the second reset, but instead it
grabbing the first.

Signed-off-by: Thierry Reding 

...snip ...


While at it, adding Tuomas who wrote the original probe order fix.
Tuomas, does this patch look correct to you? Here's the patch in full if
you don't have it in your inbox:

http://patchwork.ozlabs.org/patch/618488/



D'oh! Yes, that patch looks correct.

- Tuomas

--
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: MUSB driver on AM3352 dropping USB packets

[Andrew Goodbody]

> From: Bin Liu [mailto:b-...@ti.com]
> On Thu, May 05, 2016 at 03:12:00PM +, Andrew Goodbody wrote:
> > > From: Bin Liu [mailto:b-...@ti.com]
> > > Hi,
> > >
> > > On Thu, May 05, 2016 at 12:22:33PM +, Andrew Goodbody wrote:
> > > > > From: Bin Liu [mailto:b-...@ti.com] Hi,
> > > > >
> > > > > On Wed, May 04, 2016 at 03:48:50PM +, Andrew Goodbody
> wrote:
> > > > > > Hi,
> > > > > >
> > > > > > I have been investigating communication issues with iPads.
> > > > > > When the system is busy it seems that the musb driver is
> > > > > > silently dropping occasional packets. I have a usbmon trace
> > > > > > that does not show the packet and I have a trace from a
> > > > > > hardware USB analyser that does show the packet. So the device
> > > > > > is correctly sending the packet, it is even being ACKed, but
> > > > > > it is not passed up to the application. The packet is a bulk
> > > > > > transfer packet of 20 bytes. Can anyone please give me
> > > > > > pointers to where to go looking for the problem? The syslog shows
> nothing relevant.
> > > > >
> > > > > What is the part number on the am3352 chip?
> > > >
> > > > AM3352BZCZ100
> > > >
> > > > > What kernel version do you use?
> > > >
> > > > 4.5.0
> > > >
> > > > > Is musb cppi dma enabled? If so, does the problem still happen
> > > > > when CPPI disabled?
> > > >
> > > > Yes. Yes. When testing with PIO I did get the message "Rx
> > > > interrupt with no
> > > errors or packet!".
> > > >
> > > > > First try to turn on dynamic debug log in musb_host.c to check
> > > > > if musb receives the packet or not.
> > > > >
> > > > > Regards,
> > > > > -Bin.
> > > >
> > > > I am having problems doing this. If I enable the whole file then I
> > > > get lots of messages on the console about /dev/kmsg buffer
> > > > overrun. There are more then 26 million packets in the hardware
> > > > trace and I have not worked out how to correlate any of the
> > > > possible message from dynamic debug with those packets even when I
> > > > enable some of the dynamic debug lines.  I can see a few messages
> > > > about "DMA complete but packet still in FIFO, CSR 2103" and just the
> occasional "extra TX2 ready, csr 2100"
> > > > when I enable some of the lines for dynamic debug.
> > >
> > > Well, this issue would not be easy to debug. Is this with your custom
> board?
> > > If so, have you run EyeDiagram test to rule out signal integrity
> > > problem? Are you able to reproduce it with any TI EVM, such as
> > > Beaglebone Black? If so, please explain the detail of the test case,
> > > I could try to reproduce it on my side.
> >
> > Yes this is on a custom board and yes we did EyeDiagram tests. Also
> > the ACK from the controller is seen, so that should rule out signal
> > integrity issues.  I have just reproduced this on the Beaglebone Black
> > using the latest TI SDK. Do you have access to 16 iPads with lightning
> > connectors and do you have a Mac running 10.10.x?
> 
> No, I don't have those :(
> 
> 16 devices connecting to musb sounds too many. what is the ep info in the
> descriptor of the ipad device?

T:  Bus=02 Lev=04 Prnt=06 Port=06 Cnt=07 Dev#= 19 Spd=480  MxCh= 0
D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  4
P:  Vendor=05ac ProdID=12ab Rev= 3.40
S:  Manufacturer=Apple Inc.
S:  Product=iPad
S:  SerialNumber=1da5f4610eafb36fa1e9eead80a56cb2db11dfce
C:  #Ifs= 1 Cfg#= 1 Atr=c0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=06(still) Sub=01 Prot=01 Driver=
E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=1250us
E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=64ms
C:  #Ifs= 3 Cfg#= 2 Atr=c0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 0 Cls=01(audio) Sub=01 Prot=00 Driver=
I:  If#= 1 Alt= 0 #EPs= 0 Cls=01(audio) Sub=02 Prot=00 Driver=
I:  If#= 1 Alt= 1 #EPs= 1 Cls=01(audio) Sub=02 Prot=00 Driver=
E:  Ad=81(I) Atr=01(Isoc) MxPS= 192 Ivl=1ms
I:  If#= 2 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=
E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=125us
C:* #Ifs= 2 Cfg#= 3 Atr=c0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=06(still) Sub=01 Prot=01 Driver=usbfs
E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=1250us
E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=64ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=fe Prot=02 Driver=usbfs
E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
C:  #Ifs= 3 Cfg#= 4 Atr=c0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=06(still) Sub=01 Prot=01 Driver=
E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=1250us
E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=64ms
I:  If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=fe Prot=02 Driver=
E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:  If#= 2 Alt= 0 #EPs= 0 Cls=ff(vend.) Sub=fd Prot=01 Driver=
I:  If#= 2 Alt= 1 #EPs= 2 Cls=ff(vend.) Sub=fd Prot=01 Driver=
E:  Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:  

Re: MUSB driver on AM3352 dropping USB packets

[Bin Liu]

On Thu, May 05, 2016 at 03:12:00PM +, Andrew Goodbody wrote:
> > From: Bin Liu [mailto:b-...@ti.com]
> > Hi,
> > 
> > On Thu, May 05, 2016 at 12:22:33PM +, Andrew Goodbody wrote:
> > > > From: Bin Liu [mailto:b-...@ti.com]
> > > > Hi,
> > > >
> > > > On Wed, May 04, 2016 at 03:48:50PM +, Andrew Goodbody wrote:
> > > > > Hi,
> > > > >
> > > > > I have been investigating communication issues with iPads. When
> > > > > the system is busy it seems that the musb driver is silently
> > > > > dropping occasional packets. I have a usbmon trace that does not
> > > > > show the packet and I have a trace from a hardware USB analyser
> > > > > that does show the packet. So the device is correctly sending the
> > > > > packet, it is even being ACKed, but it is not passed up to the
> > > > > application. The packet is a bulk transfer packet of 20 bytes. Can
> > > > > anyone please give me pointers to where to go looking for the
> > > > > problem? The syslog shows nothing relevant.
> > > >
> > > > What is the part number on the am3352 chip?
> > >
> > > AM3352BZCZ100
> > >
> > > > What kernel version do you use?
> > >
> > > 4.5.0
> > >
> > > > Is musb cppi dma enabled? If so, does the problem still happen when
> > > > CPPI disabled?
> > >
> > > Yes. Yes. When testing with PIO I did get the message "Rx interrupt with 
> > > no
> > errors or packet!".
> > >
> > > > First try to turn on dynamic debug log in musb_host.c to check if
> > > > musb receives the packet or not.
> > > >
> > > > Regards,
> > > > -Bin.
> > >
> > > I am having problems doing this. If I enable the whole file then I get
> > > lots of messages on the console about /dev/kmsg buffer overrun. There
> > > are more then 26 million packets in the hardware trace and I have not
> > > worked out how to correlate any of the possible message from dynamic
> > > debug with those packets even when I enable some of the dynamic debug
> > > lines.  I can see a few messages about "DMA complete but packet still
> > > in FIFO, CSR 2103" and just the occasional "extra TX2 ready, csr 2100"
> > > when I enable some of the lines for dynamic debug.
> > 
> > Well, this issue would not be easy to debug. Is this with your custom board?
> > If so, have you run EyeDiagram test to rule out signal integrity problem? 
> > Are
> > you able to reproduce it with any TI EVM, such as Beaglebone Black? If so,
> > please explain the detail of the test case, I could try to reproduce it on 
> > my
> > side.
> 
> Yes this is on a custom board and yes we did EyeDiagram tests. Also
> the ACK from the controller is seen, so that should rule out signal
> integrity issues.  I have just reproduced this on the Beaglebone Black
> using the latest TI SDK. Do you have access to 16 iPads with lightning
> connectors and do you have a Mac running 10.10.x?

No, I don't have those :(

16 devices connecting to musb sounds too many. what is the ep info in
the descriptor of the ipad device?

> 
> > >
> > > Andrew
> > 
> > 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: MUSB driver on AM3352 dropping USB packets

[Andrew Goodbody]

> From: Bin Liu [mailto:b-...@ti.com]
> Hi,
> 
> On Thu, May 05, 2016 at 12:22:33PM +, Andrew Goodbody wrote:
> > > From: Bin Liu [mailto:b-...@ti.com]
> > > Hi,
> > >
> > > On Wed, May 04, 2016 at 03:48:50PM +, Andrew Goodbody wrote:
> > > > Hi,
> > > >
> > > > I have been investigating communication issues with iPads. When
> > > > the system is busy it seems that the musb driver is silently
> > > > dropping occasional packets. I have a usbmon trace that does not
> > > > show the packet and I have a trace from a hardware USB analyser
> > > > that does show the packet. So the device is correctly sending the
> > > > packet, it is even being ACKed, but it is not passed up to the
> > > > application. The packet is a bulk transfer packet of 20 bytes. Can
> > > > anyone please give me pointers to where to go looking for the
> > > > problem? The syslog shows nothing relevant.
> > >
> > > What is the part number on the am3352 chip?
> >
> > AM3352BZCZ100
> >
> > > What kernel version do you use?
> >
> > 4.5.0
> >
> > > Is musb cppi dma enabled? If so, does the problem still happen when
> > > CPPI disabled?
> >
> > Yes. Yes. When testing with PIO I did get the message "Rx interrupt with no
> errors or packet!".
> >
> > > First try to turn on dynamic debug log in musb_host.c to check if
> > > musb receives the packet or not.
> > >
> > > Regards,
> > > -Bin.
> >
> > I am having problems doing this. If I enable the whole file then I get
> > lots of messages on the console about /dev/kmsg buffer overrun. There
> > are more then 26 million packets in the hardware trace and I have not
> > worked out how to correlate any of the possible message from dynamic
> > debug with those packets even when I enable some of the dynamic debug
> > lines.  I can see a few messages about "DMA complete but packet still
> > in FIFO, CSR 2103" and just the occasional "extra TX2 ready, csr 2100"
> > when I enable some of the lines for dynamic debug.
> 
> Well, this issue would not be easy to debug. Is this with your custom board?
> If so, have you run EyeDiagram test to rule out signal integrity problem? Are
> you able to reproduce it with any TI EVM, such as Beaglebone Black? If so,
> please explain the detail of the test case, I could try to reproduce it on my
> side.

Yes this is on a custom board and yes we did EyeDiagram tests. Also the ACK 
from the controller is seen, so that should rule out signal integrity issues.
I have just reproduced this on the Beaglebone Black using the latest TI SDK. Do 
you have access to 16 iPads with lightning connectors and do you have a Mac 
running 10.10.x?

> >
> > Andrew
> 
> 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 02/10] usb: musb: Fix idling after host mode by increasing autosuspend delay

[Bin Liu]

Hi,

On Thu, Apr 28, 2016 at 10:33:12AM -0700, Tony Lindgren wrote:
> Looks like at least 2430 glue won't idle reliably with the 200 ms
> autosuspend delay. This causes deeper idle states being blocked for
> the whole SoC when disconnecting OTG A cable.
> 
> Increasing the delay to 500 ms seems to idle both MUSB and the PHY
> reliably. This is probably because of time needed by the hardware
> based negotiation between MUSB and the PHY.

Unless we know other glues also have the same issue, this change
affects other glues, right? Can we just increase it for 2430 only?

Regards,
-Bin.

> 
> Signed-off-by: Tony Lindgren 
> ---
>  drivers/usb/musb/musb_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 39fd958..5fa6187 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -2030,7 +2030,7 @@ musb_init_controller(struct device *dev, int nIrq, void 
> __iomem *ctrl)
>  
>   /* We need musb_read/write functions initialized for PM */
>   pm_runtime_use_autosuspend(musb->controller);
> - pm_runtime_set_autosuspend_delay(musb->controller, 200);
> + pm_runtime_set_autosuspend_delay(musb->controller, 500);
>   pm_runtime_enable(musb->controller);
>  
>   /* The musb_platform_init() call:
> -- 
> 2.8.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: [RFC v2 10/13] usb: hub: Power sequence the ports on activation

[Alan Stern]

On Thu, 5 May 2016, Krzysztof Kozlowski wrote:

> The autodetection of attached USB device might not work on certain
> boards where the power is delivered externally. These devices also might
> require a hard reset. Use pwrseq for that in USB hub.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  drivers/usb/core/hub.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 1c82fcc448f5..0fddaacc62bf 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -1661,7 +1662,17 @@ static int hub_configure(struct usb_hub *hub,
>  
>   usb_hub_adjust_deviceremovable(hdev, hub->descriptor);
>  
> + /* FIXME: When do the pre-power-on? */

It's hard to answer this without knowing what pre-power-on involves.

Why not create a pwrseq_power_on() routine that does pre_power_on 
followed by post_power_on?

> + /*
> + for (i = 0; i < maxchild; i++)
> + pwrseq_pre_power_on(hub->ports[i]->pwrseq);
> + */
> +
> + for (i = 0; i < maxchild; i++)
> + pwrseq_post_power_on(hub->ports[i]->pwrseq);

This is patch 10/13.  hub->ports[i]->pwrseq doesn't get added until 
11/13.  Obviously you never tried compiling each patch in the series.

> +
>   hub_activate(hub, HUB_INIT);
> +

Unnecessary blank line added.

>   return 0;
>  
>  fail:

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: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

[Bin Liu]

On Thu, May 05, 2016 at 04:39:06PM +0300, Sergei Shtylyov wrote:
> On 5/5/2016 4:31 PM, Bin Liu wrote:
> 
> yes, it also works with that reset and go to finish:
> 
> diff --git a/drivers/usb/musb/musb_host.c 
> b/drivers/usb/musb/musb_host.c
> index c3d5fc9..8cd98e7 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -1599,6 +1599,10 @@ void musb_host_rx(struct musb *musb, u8 
> epnum)
>    status = -EPROTO;
>    musb_writeb(epio, MUSB_RXINTERVAL, 0);
> 
> +   rx_csr &= ~MUSB_RXCSR_H_ERROR;
> +   musb_writew(epio, MUSB_RXCSR, rx_csr);
> +
> +   goto finish;
>    } else if (rx_csr & MUSB_RXCSR_DATAERROR) {
> 
>    if (USB_ENDPOINT_XFER_ISOC != qh->type) {
> 
> >>>
> >>>Thanks for testing it.
> >>
> >>Have tested your patch and now both FT4232 and Huawei don't freeze 
> >>on removal.
> >>
> >>Bin, Max thanks for fixing this issue.
> >>
> >>Tested-by: Yegor Yefremov 
> >
> >Thanks for testing.
> >
> >Can you please test the patch [1] instead? I'd like to use it as the
> >fix.
> >
> >[1] http://marc.info/?l=linux-usb=146222355213935=2
> 
> The patch behaves the same as the previous one.
> 
> Kernel: 4.6-rc6
> >>>
> >>>Thanks for testing. I will add your Tested-by.
> >>
> >>If you'll resend this patch, it would be good to add it to stable
> >>kernels. I've tested 3.18.32 and it fixes the error too.
> 
> >Thanks for testing.
> >
> >My plan is to not rush it into stable, but let it sit in v4.7 for a
> >while first.
> 
>   Are you serious? Fixing interrupt storm due to not cleared
> interrupt bit will only be done in 4.7?
> >>>
> >>>Well, I am new to maintianer's role, and thought there is only one week
> >>>away to v4.7 merge window, there is no big difference to let this patch
> >>>get into v4.7-rc1. If getting the fix into upstream as soon as possible
> >>>is important, I will send it for 4.6-rc7.
> >>>
> >>>BTY, the issue is not because of not clearing interrupt bit, but the hub
> >>>has no chance to report the disconnect event, which causes the
> >>>controller keeps generating the interrupt for every new rx urb.
> >>
> >>   Sorry, looking at the Mentor manuals, I got the impression that
> >>whenever the RXCSR.Error is set, there's interrupt. Probably they
> >
> >This is my understanding of the manual too.
> >
> >>meant that the interrupt is generated only on transition from 0 to
> >>1
> >
> >What transition? the RXCSR bit?
> 
>Of course.
> 
> >'set' means from 0 to 1, 'clear' means 1 -> 0, right?
> 
>Well, in my understanding "set" means 1 and "clear" means 0.

This is right. I didn't mean raising or falling edge when mentioning
0->1 or 1->0. We don't know when exactly the controller generates the
interrupt when CSR bit changes, and we/driver don't care.

> 
> >I don't see you have any misunderstanding.
> 
> >>>Regards,
> >>>-Bin.
> 
> 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: MUSB driver on AM3352 dropping USB packets

[Bin Liu]

Hi,

On Thu, May 05, 2016 at 12:22:33PM +, Andrew Goodbody wrote:
> > From: Bin Liu [mailto:b-...@ti.com]
> > Hi,
> > 
> > On Wed, May 04, 2016 at 03:48:50PM +, Andrew Goodbody wrote:
> > > Hi,
> > >
> > > I have been investigating communication issues with iPads. When the
> > > system is busy it seems that the musb driver is silently dropping
> > > occasional packets. I have a usbmon trace that does not show the
> > > packet and I have a trace from a hardware USB analyser that does show
> > > the packet. So the device is correctly sending the packet, it is even
> > > being ACKed, but it is not passed up to the application. The packet is
> > > a bulk transfer packet of 20 bytes. Can anyone please give me pointers
> > > to where to go looking for the problem? The syslog shows nothing
> > > relevant.
> > 
> > What is the part number on the am3352 chip?
> 
> AM3352BZCZ100
> 
> > What kernel version do you use?
> 
> 4.5.0
> 
> > Is musb cppi dma enabled? If so, does the problem still happen when CPPI
> > disabled?
> 
> Yes. Yes. When testing with PIO I did get the message "Rx interrupt with no 
> errors or packet!".
> 
> > First try to turn on dynamic debug log in musb_host.c to check if musb
> > receives the packet or not.
> > 
> > Regards,
> > -Bin.
> 
> I am having problems doing this. If I enable the whole file then I get
> lots of messages on the console about /dev/kmsg buffer overrun. There
> are more then 26 million packets in the hardware trace and I have not
> worked out how to correlate any of the possible message from dynamic
> debug with those packets even when I enable some of the dynamic debug
> lines.  I can see a few messages about "DMA complete but packet still
> in FIFO, CSR 2103" and just the occasional "extra TX2 ready, csr 2100"
> when I enable some of the lines for dynamic debug.

Well, this issue would not be easy to debug. Is this with your custom
board? If so, have you run EyeDiagram test to rule out signal integrity
problem? Are you able to reproduce it with any TI EVM, such as
Beaglebone Black? If so, please explain the detail of the test case, I
could try to reproduce it on my side.

> 
> Andrew

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: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

[Sergei Shtylyov]

On 5/5/2016 4:31 PM, Bin Liu wrote:


yes, it also works with that reset and go to finish:

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index c3d5fc9..8cd98e7 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1599,6 +1599,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
   status = -EPROTO;
   musb_writeb(epio, MUSB_RXINTERVAL, 0);

+   rx_csr &= ~MUSB_RXCSR_H_ERROR;
+   musb_writew(epio, MUSB_RXCSR, rx_csr);
+
+   goto finish;
   } else if (rx_csr & MUSB_RXCSR_DATAERROR) {

   if (USB_ENDPOINT_XFER_ISOC != qh->type) {



Thanks for testing it.


Have tested your patch and now both FT4232 and Huawei don't freeze on removal.

Bin, Max thanks for fixing this issue.

Tested-by: Yegor Yefremov 


Thanks for testing.

Can you please test the patch [1] instead? I'd like to use it as the
fix.

[1] http://marc.info/?l=linux-usb=146222355213935=2


The patch behaves the same as the previous one.

Kernel: 4.6-rc6


Thanks for testing. I will add your Tested-by.


If you'll resend this patch, it would be good to add it to stable
kernels. I've tested 3.18.32 and it fixes the error too.



Thanks for testing.

My plan is to not rush it into stable, but let it sit in v4.7 for a
while first.


  Are you serious? Fixing interrupt storm due to not cleared
interrupt bit will only be done in 4.7?


Well, I am new to maintianer's role, and thought there is only one week
away to v4.7 merge window, there is no big difference to let this patch
get into v4.7-rc1. If getting the fix into upstream as soon as possible
is important, I will send it for 4.6-rc7.

BTY, the issue is not because of not clearing interrupt bit, but the hub
has no chance to report the disconnect event, which causes the
controller keeps generating the interrupt for every new rx urb.


   Sorry, looking at the Mentor manuals, I got the impression that
whenever the RXCSR.Error is set, there's interrupt. Probably they


This is my understanding of the manual too.


meant that the interrupt is generated only on transition from 0 to
1


What transition? the RXCSR bit?


   Of course.


'set' means from 0 to 1, 'clear' means 1 -> 0, right?


   Well, in my understanding "set" means 1 and "clear" means 0.


I don't see you have any misunderstanding.



Regards,
-Bin.


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: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

[Bin Liu]

Hi,

On Thu, May 05, 2016 at 04:21:23PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 5/4/2016 10:17 PM, Bin Liu wrote:
> 
> >>yes, it also works with that reset and go to finish:
> >>
> >>diff --git a/drivers/usb/musb/musb_host.c 
> >>b/drivers/usb/musb/musb_host.c
> >>index c3d5fc9..8cd98e7 100644
> >>--- a/drivers/usb/musb/musb_host.c
> >>+++ b/drivers/usb/musb/musb_host.c
> >>@@ -1599,6 +1599,10 @@ void musb_host_rx(struct musb *musb, u8 
> >>epnum)
> >>status = -EPROTO;
> >>musb_writeb(epio, MUSB_RXINTERVAL, 0);
> >>
> >>+   rx_csr &= ~MUSB_RXCSR_H_ERROR;
> >>+   musb_writew(epio, MUSB_RXCSR, rx_csr);
> >>+
> >>+   goto finish;
> >>} else if (rx_csr & MUSB_RXCSR_DATAERROR) {
> >>
> >>if (USB_ENDPOINT_XFER_ISOC != qh->type) {
> >>
> >
> >Thanks for testing it.
> 
> Have tested your patch and now both FT4232 and Huawei don't freeze on 
> removal.
> 
> Bin, Max thanks for fixing this issue.
> 
> Tested-by: Yegor Yefremov 
> >>>
> >>>Thanks for testing.
> >>>
> >>>Can you please test the patch [1] instead? I'd like to use it as the
> >>>fix.
> >>>
> >>>[1] http://marc.info/?l=linux-usb=146222355213935=2
> >>
> >>The patch behaves the same as the previous one.
> >>
> >>Kernel: 4.6-rc6
> >
> >Thanks for testing. I will add your Tested-by.
> 
> If you'll resend this patch, it would be good to add it to stable
> kernels. I've tested 3.18.32 and it fixes the error too.
> >>
> >>>Thanks for testing.
> >>>
> >>>My plan is to not rush it into stable, but let it sit in v4.7 for a
> >>>while first.
> >>
> >>   Are you serious? Fixing interrupt storm due to not cleared
> >>interrupt bit will only be done in 4.7?
> >
> >Well, I am new to maintianer's role, and thought there is only one week
> >away to v4.7 merge window, there is no big difference to let this patch
> >get into v4.7-rc1. If getting the fix into upstream as soon as possible
> >is important, I will send it for 4.6-rc7.
> >
> >BTY, the issue is not because of not clearing interrupt bit, but the hub
> >has no chance to report the disconnect event, which causes the
> >controller keeps generating the interrupt for every new rx urb.
> 
>Sorry, looking at the Mentor manuals, I got the impression that
> whenever the RXCSR.Error is set, there's interrupt. Probably they

This is my understanding of the manual too.

> meant that the interrupt is generated only on transition from 0 to
> 1

What transition? the RXCSR bit? 'set' means from 0 to 1, 'clear' means 1
-> 0, right? I don't see you have any misunderstanding.

> 
> >Regards,
> >-Bin.
> 
> 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: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

[Sergei Shtylyov]

Hello.

On 5/4/2016 10:17 PM, Bin Liu wrote:


yes, it also works with that reset and go to finish:

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index c3d5fc9..8cd98e7 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1599,6 +1599,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
status = -EPROTO;
musb_writeb(epio, MUSB_RXINTERVAL, 0);

+   rx_csr &= ~MUSB_RXCSR_H_ERROR;
+   musb_writew(epio, MUSB_RXCSR, rx_csr);
+
+   goto finish;
} else if (rx_csr & MUSB_RXCSR_DATAERROR) {

if (USB_ENDPOINT_XFER_ISOC != qh->type) {



Thanks for testing it.


Have tested your patch and now both FT4232 and Huawei don't freeze on removal.

Bin, Max thanks for fixing this issue.

Tested-by: Yegor Yefremov 


Thanks for testing.

Can you please test the patch [1] instead? I'd like to use it as the
fix.

[1] http://marc.info/?l=linux-usb=146222355213935=2


The patch behaves the same as the previous one.

Kernel: 4.6-rc6


Thanks for testing. I will add your Tested-by.


If you'll resend this patch, it would be good to add it to stable
kernels. I've tested 3.18.32 and it fixes the error too.



Thanks for testing.

My plan is to not rush it into stable, but let it sit in v4.7 for a
while first.


   Are you serious? Fixing interrupt storm due to not cleared
interrupt bit will only be done in 4.7?


Well, I am new to maintianer's role, and thought there is only one week
away to v4.7 merge window, there is no big difference to let this patch
get into v4.7-rc1. If getting the fix into upstream as soon as possible
is important, I will send it for 4.6-rc7.

BTY, the issue is not because of not clearing interrupt bit, but the hub
has no chance to report the disconnect event, which causes the
controller keeps generating the interrupt for every new rx urb.


   Sorry, looking at the Mentor manuals, I got the impression that whenever 
the RXCSR.Error is set, there's interrupt. Probably they meant that the 
interrupt is generated only on transition from 0 to 1



Regards,
-Bin.


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 v2] xhci: Cleanup only when releasing primary hcd.

[Joel Stanley]

On Tue, May 3, 2016 at 12:01 AM, Mathias Nyman  wrote:
> On 28.04.2016 11:33, Roger Quadros wrote:
>> This looks good to me. Are you going to pick this up for v4.6-rc cycle?
>> We should copy this to v4.3+ stable as well.
>>
>> cheers,
>> -roger
>>
>
> Looks good to me too, but I think we're too late for 4.6-rc cycle, I'll send
> it
> forward after 4.7-rc1 and add the stable tags.
>

Thanks all.

Without this patch we lose USB after kexec on our OpenPower machines.
I tested it on a Palmetto machine and it resolves the issue. Please
add my

Tested-by: Joel Stanley 

Cheers,

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

[RFC v2 03/13] MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq

[Krzysztof Kozlowski]

Before moving pwrseq drivers from drivers/mmc/core/ to drivers/power/,
they were maintained by Ulf Hansson.

Signed-off-by: Krzysztof Kozlowski 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b016f447c6c9..2c501b795d18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8874,6 +8874,14 @@ F:   include/linux/power_supply.h
 F: drivers/power/
 X: drivers/power/avs/
 
+POWER SEQ CORE and DRIVERS
+M: Ulf Hansson 
+L: linux-...@vger.kernel.org
+T: git git://git.linaro.org/people/ulf.hansson/mmc.git
+S: Maintained
+F: drivers/power/pwrseq/
+F: include/linux/pwrseq.h
+
 POWER STATE COORDINATION INTERFACE (PSCI)
 M: Mark Rutland 
 M: Lorenzo Pieralisi 
-- 
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

[RFC v2 02/13] power/mmc: Move pwrseq drivers to power/pwrseq

[Krzysztof Kozlowski]

The MMC power sequence drivers are useful also outside of MMC world: for
USB devices needed a hard-reset before probing. Before extending and
re-using pwrseq drivers, move them to a new place.

The commit does not introduce significant changes in the pwrseq drivers
code so still all the functions are prefixed with "mmc_pwrseq". However
the MMC-specific pwrseq functions has to be now exported and everything
is hidden not by CONFIG_OF but by new CONFIG_POWER_SEQ option.

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/mmc/Kconfig|  2 --
 drivers/mmc/core/Makefile  |  3 ---
 drivers/mmc/core/core.c|  2 +-
 drivers/mmc/core/host.c|  2 +-
 drivers/power/Kconfig  |  1 +
 drivers/power/Makefile |  1 +
 drivers/{mmc/core => power/pwrseq}/Kconfig | 17 -
 drivers/power/pwrseq/Makefile  |  3 +++
 drivers/{mmc/core => power/pwrseq}/pwrseq.c|  8 ++--
 drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c   |  3 +--
 drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c |  3 +--
 {drivers/mmc/core => include/linux}/pwrseq.h   |  6 +++---
 12 files changed, 30 insertions(+), 21 deletions(-)
 rename drivers/{mmc/core => power/pwrseq}/Kconfig (71%)
 create mode 100644 drivers/power/pwrseq/Makefile
 rename drivers/{mmc/core => power/pwrseq}/pwrseq.c (90%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c (99%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c (99%)
 rename {drivers/mmc/core => include/linux}/pwrseq.h (94%)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index f2eeb38efa65..7ade379e0634 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -21,8 +21,6 @@ config MMC_DEBUG
 
 if MMC
 
-source "drivers/mmc/core/Kconfig"
-
 source "drivers/mmc/card/Kconfig"
 
 source "drivers/mmc/host/Kconfig"
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index f007151dfdc6..a901d3cd09d3 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -8,7 +8,4 @@ mmc_core-y  := core.o bus.o host.o \
   sdio.o sdio_ops.o sdio_bus.o \
   sdio_cis.o sdio_io.o sdio_irq.o \
   quirks.o slot-gpio.o
-mmc_core-$(CONFIG_OF)  += pwrseq.o
-obj-$(CONFIG_PWRSEQ_SIMPLE)+= pwrseq_simple.o
-obj-$(CONFIG_PWRSEQ_EMMC)  += pwrseq_emmc.o
 mmc_core-$(CONFIG_DEBUG_FS)+= debugfs.o
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 99275e40bf2f..0f145ff6e4f1 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -43,7 +44,6 @@
 #include "bus.h"
 #include "host.h"
 #include "sdio_bus.h"
-#include "pwrseq.h"
 
 #include "mmc_ops.h"
 #include "sd_ops.h"
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index e0a3ee16c0d3..98164a352dfb 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -29,7 +30,6 @@
 #include "core.h"
 #include "host.h"
 #include "slot-gpio.h"
-#include "pwrseq.h"
 
 #define cls_dev_to_mmc_host(d) container_of(d, struct mmc_host, class_dev)
 
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 421770ddafa3..2702aca6cd2c 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -511,5 +511,6 @@ config AXP20X_POWER
 
 endif # POWER_SUPPLY
 
+source "drivers/power/pwrseq/Kconfig"
 source "drivers/power/reset/Kconfig"
 source "drivers/power/avs/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e46b75d448a5..02f9d5da2e76 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_POWER_AVS)   += avs/
 obj-$(CONFIG_CHARGER_SMB347)   += smb347-charger.o
 obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o
 obj-$(CONFIG_CHARGER_TPS65217) += tps65217_charger.o
+obj-$(CONFIG_POWER_SEQ)+= pwrseq/
 obj-$(CONFIG_POWER_RESET)  += reset/
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)   += axp288_charger.o
diff --git a/drivers/mmc/core/Kconfig b/drivers/power/pwrseq/Kconfig
similarity index 71%
rename from drivers/mmc/core/Kconfig
rename to drivers/power/pwrseq/Kconfig
index 250f223aaa80..b5d2d6c65f28 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/power/pwrseq/Kconfig
@@ -1,7 +1,12 @@
-#
-# MMC core configuration
-#
-config PWRSEQ_EMMC
+menuconfig POWER_SEQ
+   default y if OF
+   bool "Hardware reset support for specific devices"
+   help
+ Provides drivers which reset the specific device before...
+
+if POWER_SEQ
+
+config POWER_SEQ_EMMC
tristate "HW reset support for eMMC"
default 

[RFC v2 01/13] usb: misc: usb3503: Clean up on driver unbind

[Krzysztof Kozlowski]

The driver should clean up after itself by unpreparing the clock when it
is unbound.

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/usb/misc/usb3503.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c
index b45cb77c0744..a464636675a6 100644
--- a/drivers/usb/misc/usb3503.c
+++ b/drivers/usb/misc/usb3503.c
@@ -330,6 +330,19 @@ static int usb3503_i2c_probe(struct i2c_client *i2c,
return usb3503_probe(hub);
 }
 
+static int usb3503_i2c_remove(struct i2c_client *i2c)
+{
+   struct usb3503 *hub;
+
+   hub = i2c_get_clientdata(i2c);
+   if (hub) {
+   if (hub->clk)
+   clk_disable_unprepare(hub->clk);
+   }
+
+   return 0;
+}
+
 static int usb3503_platform_probe(struct platform_device *pdev)
 {
struct usb3503 *hub;
@@ -342,6 +355,19 @@ static int usb3503_platform_probe(struct platform_device 
*pdev)
return usb3503_probe(hub);
 }
 
+static int usb3503_platform_remove(struct platform_device *pdev)
+{
+   struct usb3503 *hub;
+
+   hub = platform_get_drvdata(pdev);
+   if (hub) {
+   if (hub->clk)
+   clk_disable_unprepare(hub->clk);
+   }
+
+   return 0;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int usb3503_i2c_suspend(struct device *dev)
 {
@@ -395,6 +421,7 @@ static struct i2c_driver usb3503_i2c_driver = {
.of_match_table = of_match_ptr(usb3503_of_match),
},
.probe  = usb3503_i2c_probe,
+   .remove = usb3503_i2c_remove,
.id_table   = usb3503_id,
 };
 
@@ -404,6 +431,7 @@ static struct platform_driver usb3503_platform_driver = {
.of_match_table = of_match_ptr(usb3503_of_match),
},
.probe  = usb3503_platform_probe,
+   .remove = usb3503_platform_remove,
 };
 
 static int __init usb3503_init(void)
-- 
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

[RFC v2 07/13] power: pwrseq: simple: Add support for toggling regulator

[Krzysztof Kozlowski]

Some devices need real hard-reset by cutting the power.  During power
sequence turn off and on the regulator, if it is provided.

Signed-off-by: Krzysztof Kozlowski 
---
 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt  |  2 +
 drivers/power/pwrseq/pwrseq_simple.c   | 50 ++
 2 files changed, 52 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt 
b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
index ce0e76749671..176ff831e7f1 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -16,6 +16,7 @@ Optional properties:
   See ../clocks/clock-bindings.txt for details.
 - clock-names : Must include the following entry:
   "ext_clock" (External clock provided to the card).
+- ext-supply : External regulator supply
 
 Example:
 
@@ -24,4 +25,5 @@ Example:
reset-gpios = < 12 GPIO_ACTIVE_LOW>;
clocks = <_32768_ck>;
clock-names = "ext_clock";
+   ext-supply = <>;
}
diff --git a/drivers/power/pwrseq/pwrseq_simple.c 
b/drivers/power/pwrseq/pwrseq_simple.c
index ab0098412690..4d5ea53d3ead 100644
--- a/drivers/power/pwrseq/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -16,7 +16,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 
 #include 
 
@@ -25,6 +27,7 @@ struct mmc_pwrseq_simple {
bool clk_enabled;
struct clk *ext_clk;
struct gpio_descs *reset_gpios;
+   struct regulator *ext_reg;
 };
 
 #define to_pwrseq_simple(p) container_of(p, struct mmc_pwrseq_simple, pwrseq)
@@ -62,6 +65,13 @@ static void mmc_pwrseq_simple_post_power_on(struct pwrseq 
*_pwrseq)
 {
struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
 
+   if (pwrseq->ext_reg) {
+   int err;
+
+   err = regulator_enable(pwrseq->ext_reg);
+   WARN_ON_ONCE(err);
+   }
+
mmc_pwrseq_simple_set_gpios_value(pwrseq, 0);
 }
 
@@ -75,6 +85,13 @@ static void mmc_pwrseq_simple_power_off(struct pwrseq 
*_pwrseq)
clk_disable_unprepare(pwrseq->ext_clk);
pwrseq->clk_enabled = false;
}
+
+   if (pwrseq->ext_reg) {
+   int err;
+
+   err = regulator_disable(pwrseq->ext_reg);
+   WARN_ON_ONCE(err);
+   }
 }
 
 static const struct pwrseq_ops mmc_pwrseq_simple_ops = {
@@ -102,6 +119,32 @@ static int mmc_pwrseq_simple_probe(struct platform_device 
*pdev)
if (IS_ERR(pwrseq->ext_clk) && PTR_ERR(pwrseq->ext_clk) != -ENOENT)
return PTR_ERR(pwrseq->ext_clk);
 
+   /* FIXME: regulator_get_exclusive? */
+   pwrseq->ext_reg = devm_regulator_get_optional(dev, "ext");
+   if (IS_ERR(pwrseq->ext_reg)) {
+   if (PTR_ERR(pwrseq->ext_reg) == -ENODEV)
+   pwrseq->ext_reg = NULL;
+   else
+   return PTR_ERR(pwrseq->ext_reg);
+   } else {
+   int err;
+   /*
+* Be sure that regulator is off, before the driver will start
+* power sequence. It is likely that regulator is on by default
+* and it without toggling it here, it would be disabled much
+* later by the core.
+*/
+
+   err = regulator_enable(pwrseq->ext_reg);
+   WARN_ON_ONCE(err);
+
+   /* FIXME: handle this in a more sensible way */
+   mdelay(10);
+
+   err = regulator_disable(pwrseq->ext_reg);
+   WARN_ON_ONCE(err);
+   }
+
pwrseq->reset_gpios = devm_gpiod_get_array(dev, "reset",
GPIOD_OUT_HIGH);
if (IS_ERR(pwrseq->reset_gpios) &&
@@ -124,6 +167,13 @@ static int mmc_pwrseq_simple_remove(struct platform_device 
*pdev)
 
pwrseq_unregister(>pwrseq);
 
+   if (pwrseq->ext_reg) {
+   int err;
+
+   err = regulator_disable(pwrseq->ext_reg);
+   WARN_ON_ONCE(err);
+   }
+
return 0;
 }
 
-- 
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

[RFC v2 06/13] power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix

[Krzysztof Kozlowski]

The power sequence hooks (mmc_pwrseq_pre_power_on(),
mmc_pwrseq_post_power_on() and mmc_pwrseq_power_off()) can be made more
generic to allow re-use in other subsystems. They do not need to take
pointer to struct mmc_host but instead the struct pwrseq should be
sufficient.

Remove the "mmc" prefix and use the pointer to struct pwrseq as
argument.

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/mmc/core/core.c  |  6 +++---
 drivers/power/pwrseq/pwrseq.c| 24 +---
 drivers/power/pwrseq/pwrseq_emmc.c   |  4 ++--
 drivers/power/pwrseq/pwrseq_simple.c | 12 ++--
 include/linux/pwrseq.h   | 18 +-
 5 files changed, 29 insertions(+), 35 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 0f145ff6e4f1..dfc4681054a8 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1719,7 +1719,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
if (host->ios.power_mode == MMC_POWER_ON)
return;
 
-   mmc_pwrseq_pre_power_on(host);
+   pwrseq_pre_power_on(host->pwrseq);
 
host->ios.vdd = fls(ocr) - 1;
host->ios.power_mode = MMC_POWER_UP;
@@ -1740,7 +1740,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 */
mmc_delay(10);
 
-   mmc_pwrseq_post_power_on(host);
+   pwrseq_post_power_on(host->pwrseq);
 
host->ios.clock = host->f_init;
 
@@ -1759,7 +1759,7 @@ void mmc_power_off(struct mmc_host *host)
if (host->ios.power_mode == MMC_POWER_OFF)
return;
 
-   mmc_pwrseq_power_off(host);
+   pwrseq_power_off(host->pwrseq);
 
host->ios.clock = 0;
host->ios.vdd = 0;
diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
index 9c665821f890..495a19d3c30b 100644
--- a/drivers/power/pwrseq/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -52,32 +52,26 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
-void mmc_pwrseq_pre_power_on(struct mmc_host *host)
+void pwrseq_pre_power_on(struct pwrseq *pwrseq)
 {
-   struct pwrseq *pwrseq = host->pwrseq;
-
if (pwrseq && pwrseq->ops->pre_power_on)
-   pwrseq->ops->pre_power_on(host);
+   pwrseq->ops->pre_power_on(pwrseq);
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_pre_power_on);
+EXPORT_SYMBOL_GPL(pwrseq_pre_power_on);
 
-void mmc_pwrseq_post_power_on(struct mmc_host *host)
+void pwrseq_post_power_on(struct pwrseq *pwrseq)
 {
-   struct pwrseq *pwrseq = host->pwrseq;
-
if (pwrseq && pwrseq->ops->post_power_on)
-   pwrseq->ops->post_power_on(host);
+   pwrseq->ops->post_power_on(pwrseq);
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_post_power_on);
+EXPORT_SYMBOL_GPL(pwrseq_post_power_on);
 
-void mmc_pwrseq_power_off(struct mmc_host *host)
+void pwrseq_power_off(struct pwrseq *pwrseq)
 {
-   struct pwrseq *pwrseq = host->pwrseq;
-
if (pwrseq && pwrseq->ops->power_off)
-   pwrseq->ops->power_off(host);
+   pwrseq->ops->power_off(pwrseq);
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_power_off);
+EXPORT_SYMBOL_GPL(pwrseq_power_off);
 
 void mmc_pwrseq_free(struct mmc_host *host)
 {
diff --git a/drivers/power/pwrseq/pwrseq_emmc.c 
b/drivers/power/pwrseq/pwrseq_emmc.c
index a68ac9a68e04..82327d0223f2 100644
--- a/drivers/power/pwrseq/pwrseq_emmc.c
+++ b/drivers/power/pwrseq/pwrseq_emmc.c
@@ -37,9 +37,9 @@ static void __mmc_pwrseq_emmc_reset(struct mmc_pwrseq_emmc 
*pwrseq)
udelay(200);
 }
 
-static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
+static void mmc_pwrseq_emmc_reset(struct pwrseq *_pwrseq)
 {
-   struct mmc_pwrseq_emmc *pwrseq =  to_pwrseq_emmc(host->pwrseq);
+   struct mmc_pwrseq_emmc *pwrseq = to_pwrseq_emmc(_pwrseq);
 
__mmc_pwrseq_emmc_reset(pwrseq);
 }
diff --git a/drivers/power/pwrseq/pwrseq_simple.c 
b/drivers/power/pwrseq/pwrseq_simple.c
index d5fbd653153e..ab0098412690 100644
--- a/drivers/power/pwrseq/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -46,9 +46,9 @@ static void mmc_pwrseq_simple_set_gpios_value(struct 
mmc_pwrseq_simple *pwrseq,
}
 }
 
-static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
+static void mmc_pwrseq_simple_pre_power_on(struct pwrseq *_pwrseq)
 {
-   struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
+   struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
 
if (!IS_ERR(pwrseq->ext_clk) && !pwrseq->clk_enabled) {
clk_prepare_enable(pwrseq->ext_clk);
@@ -58,16 +58,16 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host 
*host)
mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
 }
 
-static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
+static void mmc_pwrseq_simple_post_power_on(struct pwrseq *_pwrseq)
 {
-   struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
+   struct 

[RFC v2 09/13] power: pwrseq: Add support for USB hubs with external power

[Krzysztof Kozlowski]

Some USB devices on embedded boards have external power supply which has
to be reset in certain conditions. Add pwrseq interface for this.

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/power/pwrseq/pwrseq.c | 44 +++
 include/linux/pwrseq.h|  8 
 2 files changed, 52 insertions(+)

diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
index 495a19d3c30b..306265f55a10 100644
--- a/drivers/power/pwrseq/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -52,6 +52,43 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
+struct pwrseq *pwrseq_alloc(struct device *dev)
+{
+   struct device_node *np;
+   struct pwrseq *p, *ret = NULL;
+
+   np = of_parse_phandle(dev->of_node, "usb-pwrseq", 0);
+   if (!np)
+   return NULL;
+
+   mutex_lock(_list_mutex);
+   list_for_each_entry(p, _list, pwrseq_node) {
+   if (p->dev->of_node == np) {
+   if (!try_module_get(p->owner))
+   dev_err(dev,
+   "increasing module refcount failed\n");
+   else
+   ret = p;
+
+   break;
+   }
+   }
+
+   of_node_put(np);
+   mutex_unlock(_list_mutex);
+
+   /* FIXME: this path can be simplified... */
+   if (!ret) {
+   dev_info(dev, "usb-pwrseq defer\n");
+   return ERR_PTR(-EPROBE_DEFER);
+   }
+
+   dev_info(dev, "allocated usb-pwrseq\n");
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(pwrseq_alloc);
+
 void pwrseq_pre_power_on(struct pwrseq *pwrseq)
 {
if (pwrseq && pwrseq->ops->pre_power_on)
@@ -84,6 +121,13 @@ void mmc_pwrseq_free(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_free);
 
+void pwrseq_free(const struct pwrseq *pwrseq)
+{
+   if (pwrseq)
+   module_put(pwrseq->owner);
+}
+EXPORT_SYMBOL_GPL(pwrseq_free);
+
 int pwrseq_register(struct pwrseq *pwrseq)
 {
if (!pwrseq || !pwrseq->ops || !pwrseq->dev)
diff --git a/include/linux/pwrseq.h b/include/linux/pwrseq.h
index fcc8fd855d4c..c3c91f50e4cb 100644
--- a/include/linux/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -31,9 +31,13 @@ void pwrseq_unregister(struct pwrseq *pwrseq);
 void pwrseq_pre_power_on(struct pwrseq *pwrseq);
 void pwrseq_post_power_on(struct pwrseq *pwrseq);
 void pwrseq_power_off(struct pwrseq *pwrseq);
+
 int mmc_pwrseq_alloc(struct mmc_host *host);
 void mmc_pwrseq_free(struct mmc_host *host);
 
+struct pwrseq *pwrseq_alloc(struct device *dev);
+void pwrseq_free(const struct pwrseq *pwrseq);
+
 #else /* CONFIG_POWER_SEQ */
 
 static inline int pwrseq_register(struct pwrseq *pwrseq)
@@ -44,9 +48,13 @@ static inline void pwrseq_unregister(struct pwrseq *pwrseq) 
{}
 static inline void pwrseq_pre_power_on(struct pwrseq *pwrseq) {}
 static inline void pwrseq_post_power_on(struct pwrseq *pwrseq) {}
 static inline void pwrseq_power_off(struct pwrseq *pwrseq) {}
+
 static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
 static inline void mmc_pwrseq_free(struct mmc_host *host) {}
 
+static inline struct pwrseq *pwrseq_alloc(struct device *dev) { return NULL; }
+static inline void pwrseq_free(const struct pwrseq *pwrseq) {}
+
 #endif /* CONFIG_POWER_SEQ */
 
 #endif /* _LINUX_PWRSEQ_H */
-- 
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

[RFC v2 08/13] usb: hub: Handle deferred probe

[Krzysztof Kozlowski]

Add support for deferred probing to the usb hub. Currently EPROBE_DEFER
does not exist in usb hub path but future patches will add it on the
port level.

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/usb/core/hub.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 38cc4bae0a82..1c82fcc448f5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1731,6 +1731,7 @@ static int hub_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
struct usb_endpoint_descriptor *endpoint;
struct usb_device *hdev;
struct usb_hub *hub;
+   int ret;
 
desc = intf->cur_altsetting;
hdev = interface_to_usbdev(intf);
@@ -1850,11 +1851,12 @@ descriptor_error:
if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
hub->quirk_check_port_auto_suspend = 1;
 
-   if (hub_configure(hub, endpoint) >= 0)
+   ret = hub_configure(hub, endpoint);
+   if (ret >= 0)
return 0;
 
hub_disconnect(intf);
-   return -ENODEV;
+   return ret;
 }
 
 static int
-- 
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

[RFC v2 13/13] ARM: dts: exynos: Fix LAN and HUB after bootloader initialization on Odroid U3

[Krzysztof Kozlowski]

On Odroid U3 (Exynos4412-based) board if USB was initialized by
bootloader (in U-Boot "usb start" before tftpboot), the HUB (usb3503)
and LAN (smsc95xx) after after successful probing were not visible in the
system ("lsusb").

In such case the devices had to be fully reset before configuring.
Reset by GPIO (called RESET_N pin) and by RESET field in STCD register
in usb3503 HUB are not sufficient. Instead full reset has to be done by
disabling and enabling regulator.

Signed-off-by: Krzysztof Kozlowski 
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 2 +-
 arch/arm/boot/dts/exynos4412-odroidu3.dts   | 6 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi 
b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 14e653e32e0f..efa204a85c83 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -67,7 +67,7 @@
};
};
 
-   emmc_pwrseq: pwrseq {
+   emmc_pwrseq: pwrseq1 {
pinctrl-0 = <_cd>;
pinctrl-names = "default";
compatible = "mmc-pwrseq-emmc";
diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts 
b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index 31cdc036fda4..3da0e6b3c32a 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -41,6 +41,11 @@
cooling-levels = <0 102 170 230>;
};
 
+   lan_pwrseq: pwrseq2 {
+   compatible = "mmc-pwrseq-simple";
+   ext-supply = <_reg>;
+   };
+
thermal-zones {
cpu_thermal: cpu-thermal {
cooling-maps {
@@ -104,6 +109,7 @@
  {
port@1 {
status = "okay";
+   usb-pwrseq = <_pwrseq>;
};
port@2 {
status = "okay";
-- 
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

[RFC v2 10/13] usb: hub: Power sequence the ports on activation

[Krzysztof Kozlowski]

The autodetection of attached USB device might not work on certain
boards where the power is delivered externally. These devices also might
require a hard reset. Use pwrseq for that in USB hub.

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/usb/core/hub.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 1c82fcc448f5..0fddaacc62bf 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1661,7 +1662,17 @@ static int hub_configure(struct usb_hub *hub,
 
usb_hub_adjust_deviceremovable(hdev, hub->descriptor);
 
+   /* FIXME: When do the pre-power-on? */
+   /*
+   for (i = 0; i < maxchild; i++)
+   pwrseq_pre_power_on(hub->ports[i]->pwrseq);
+   */
+
+   for (i = 0; i < maxchild; i++)
+   pwrseq_post_power_on(hub->ports[i]->pwrseq);
+
hub_activate(hub, HUB_INIT);
+
return 0;
 
 fail:
-- 
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

[RFC v2 12/13] ARM: dts: exynos: Switch the buck8 to GPIO mode on Odroid U3

[Krzysztof Kozlowski]

Switch the control of buck8 to GPIO mode. It is faster than I2C/register
mode and it is the easiest way to disable it (regulator state is a
logical OR state of GPIO and register value).

Signed-off-by: Krzysztof Kozlowski 
---
 arch/arm/boot/dts/exynos4412-odroidu3.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts 
b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index d73aa6c58fe3..31cdc036fda4 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -74,6 +74,7 @@
regulator-name = "BUCK8_P3V3";
regulator-min-microvolt = <330>;
regulator-max-microvolt = <330>;
+   maxim,ena-gpios = < 1 GPIO_ACTIVE_HIGH>;
 };
 
 /* VDDQ for MSHC (eMMC card) */
-- 
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

[RFC v2 04/13] power: pwrseq: Enable COMPILE_TEST for drivers

[Krzysztof Kozlowski]

Allow build testing for power sequence drivers.

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/power/pwrseq/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
index b5d2d6c65f28..4731ba01a958 100644
--- a/drivers/power/pwrseq/Kconfig
+++ b/drivers/power/pwrseq/Kconfig
@@ -9,7 +9,7 @@ if POWER_SEQ
 config POWER_SEQ_EMMC
tristate "HW reset support for eMMC"
default y
-   depends on OF
+   depends on OF || COMPILE_TEST
help
  This selects Hardware reset support aka pwrseq-emmc for eMMC
  devices. By default this option is set to y.
@@ -20,7 +20,7 @@ config POWER_SEQ_EMMC
 config POWER_SEQ_SIMPLE
tristate "Simple HW reset support for MMC"
default y
-   depends on OF
+   depends on OF || COMPILE_TEST
help
  This selects simple hardware reset support aka pwrseq-simple for MMC
  devices. By default this option is set to y.
-- 
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

[RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting

[Krzysztof Kozlowski]

Hi,

This is a different, second try to fix usb3503+lan on Odroid U3 board
if it was initialized by bootloader (e.g. for TFTP boot).

First version:
http://www.spinics.net/lists/linux-usb/msg140042.html


Problem
===
When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
is required, e.g. by suspend to RAM. The actual TFTP boot does
not have to happen. Just "usb start" from U-Boot is sufficient.

>From the schematics, the regulator is a supply only to LAN, however
without toggling it off/on, the usb3503 hub won appear neither.


Solution

This is very similar to the MMC pwrseq behavior so the idea is to:
1. Move MMC pwrseq drivers to generic place,
2. Extend the pwrseq-simple with regulator toggling,
3. Add support to USB hub and port core for pwrseq,
4. Toggle the regulator when needed.


Issues
==
I am not familiar with USB subsystem, so please kindly guide me
where USB related code should be placed.

In the code there are still some issues to solve (FIXME/TODO notes).
If the approach is okay, I will improve the patchset. However at this
point - IT WORKS, which is nice. :)


Best regards,
Krzysztof

Krzysztof Kozlowski (13):
  usb: misc: usb3503: Clean up on driver unbind
  power/mmc: Move pwrseq drivers to power/pwrseq
  MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq
  power: pwrseq: Enable COMPILE_TEST for drivers
  power: pwrseq: Remove mmc prefix from mmc_pwrseq
  power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix
  power: pwrseq: simple: Add support for toggling regulator
  usb: hub: Handle deferred probe
  power: pwrseq: Add support for USB hubs with external power
  usb: hub: Power sequence the ports on activation
  usb: port: Parse pwrseq phandle from Device Tree
  ARM: dts: exynos: Switch the buck8 to GPIO mode on Odroid U3
  ARM: dts: exynos: Fix LAN and HUB after bootloader initialization on
Odroid U3

 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt  |  2 +
 MAINTAINERS|  8 +++
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi|  2 +-
 arch/arm/boot/dts/exynos4412-odroidu3.dts  |  7 ++
 drivers/mmc/Kconfig|  2 -
 drivers/mmc/core/Makefile  |  3 -
 drivers/mmc/core/core.c|  8 +--
 drivers/mmc/core/host.c|  2 +-
 drivers/mmc/core/pwrseq.h  | 52 --
 drivers/power/Kconfig  |  1 +
 drivers/power/Makefile |  1 +
 drivers/{mmc/core => power/pwrseq}/Kconfig | 21 --
 drivers/power/pwrseq/Makefile  |  3 +
 drivers/{mmc/core => power/pwrseq}/pwrseq.c| 80 +-
 drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c   | 15 ++--
 drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c | 73 
 drivers/usb/core/hub.c | 17 -
 drivers/usb/core/hub.h |  3 +
 drivers/usb/core/port.c| 15 
 drivers/usb/misc/usb3503.c | 28 
 include/linux/mmc/host.h   |  4 +-
 include/linux/pwrseq.h | 60 
 22 files changed, 294 insertions(+), 113 deletions(-)
 delete mode 100644 drivers/mmc/core/pwrseq.h
 rename drivers/{mmc/core => power/pwrseq}/Kconfig (65%)
 create mode 100644 drivers/power/pwrseq/Makefile
 rename drivers/{mmc/core => power/pwrseq}/pwrseq.c (50%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c (89%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c (64%)
 create mode 100644 include/linux/pwrseq.h

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

[RFC v2 11/13] usb: port: Parse pwrseq phandle from Device Tree

[Krzysztof Kozlowski]

Parse usb-pwrseq property from Device Tree to get the phandle to pwrseq
device. The pwrseq device will be used by USB hub to cycle the power
before activating ports.

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/usb/core/hub.h  |  3 +++
 drivers/usb/core/port.c | 15 +++
 2 files changed, 18 insertions(+)

diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e22aae..68ca89780d26 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -24,6 +24,8 @@
 #include 
 #include "usb.h"
 
+struct pwrseq;
+
 struct usb_hub {
struct device   *intfdev;   /* the "interface" device */
struct usb_device   *hdev;
@@ -101,6 +103,7 @@ struct usb_port {
struct usb_dev_state *port_owner;
struct usb_port *peer;
struct dev_pm_qos_request *req;
+   struct pwrseq *pwrseq;
enum usb_port_connect_type connect_type;
usb_port_location_t location;
struct mutex status_lock;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 14718a9ffcfb..a875bd342452 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -18,6 +18,8 @@
 
 #include 
 #include 
+#include 
+#include 
 
 #include "hub.h"
 
@@ -532,6 +534,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int 
port1)
return retval;
}
 
+   port_dev->dev.of_node = usb_of_get_child_node(hub->hdev->dev.of_node, 
port1);
+   port_dev->pwrseq = pwrseq_alloc(_dev->dev);
+   if (IS_ERR(port_dev->pwrseq)) {
+   device_unregister(_dev->dev);
+   /* TODO: what about EPROBE_DEFER? */
+   return PTR_ERR(port_dev->pwrseq);
+   }
+
find_and_link_peer(hub, port1);
 
/*
@@ -573,8 +583,13 @@ void usb_hub_remove_port_device(struct usb_hub *hub, int 
port1)
struct usb_port *port_dev = hub->ports[port1 - 1];
struct usb_port *peer;
 
+   pwrseq_power_off(port_dev->pwrseq);
+
peer = port_dev->peer;
if (peer)
unlink_peers(port_dev, peer);
+
+   pwrseq_free(port_dev->pwrseq);
+   port_dev->pwrseq = NULL;
device_unregister(_dev->dev);
 }
-- 
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

[RFC v2 05/13] power: pwrseq: Remove mmc prefix from mmc_pwrseq

[Krzysztof Kozlowski]

The "mmc" prefix is no longer needed after moving the pwrseq core code
from mmc/ to power/.

Signed-off-by: Krzysztof Kozlowski 
---
 drivers/power/pwrseq/pwrseq.c| 18 +-
 drivers/power/pwrseq/pwrseq_emmc.c   |  8 
 drivers/power/pwrseq/pwrseq_simple.c |  8 
 include/linux/mmc/host.h |  4 ++--
 include/linux/pwrseq.h   | 20 ++--
 5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
index 66310d7643cc..9c665821f890 100644
--- a/drivers/power/pwrseq/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -21,7 +21,7 @@ static LIST_HEAD(pwrseq_list);
 int mmc_pwrseq_alloc(struct mmc_host *host)
 {
struct device_node *np;
-   struct mmc_pwrseq *p;
+   struct pwrseq *p;
 
np = of_parse_phandle(host->parent->of_node, "mmc-pwrseq", 0);
if (!np)
@@ -54,7 +54,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
 void mmc_pwrseq_pre_power_on(struct mmc_host *host)
 {
-   struct mmc_pwrseq *pwrseq = host->pwrseq;
+   struct pwrseq *pwrseq = host->pwrseq;
 
if (pwrseq && pwrseq->ops->pre_power_on)
pwrseq->ops->pre_power_on(host);
@@ -63,7 +63,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_pre_power_on);
 
 void mmc_pwrseq_post_power_on(struct mmc_host *host)
 {
-   struct mmc_pwrseq *pwrseq = host->pwrseq;
+   struct pwrseq *pwrseq = host->pwrseq;
 
if (pwrseq && pwrseq->ops->post_power_on)
pwrseq->ops->post_power_on(host);
@@ -72,7 +72,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_post_power_on);
 
 void mmc_pwrseq_power_off(struct mmc_host *host)
 {
-   struct mmc_pwrseq *pwrseq = host->pwrseq;
+   struct pwrseq *pwrseq = host->pwrseq;
 
if (pwrseq && pwrseq->ops->power_off)
pwrseq->ops->power_off(host);
@@ -81,7 +81,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_power_off);
 
 void mmc_pwrseq_free(struct mmc_host *host)
 {
-   struct mmc_pwrseq *pwrseq = host->pwrseq;
+   struct pwrseq *pwrseq = host->pwrseq;
 
if (pwrseq) {
module_put(pwrseq->owner);
@@ -90,7 +90,7 @@ void mmc_pwrseq_free(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_free);
 
-int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
+int pwrseq_register(struct pwrseq *pwrseq)
 {
if (!pwrseq || !pwrseq->ops || !pwrseq->dev)
return -EINVAL;
@@ -101,9 +101,9 @@ int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
 
return 0;
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_register);
+EXPORT_SYMBOL_GPL(pwrseq_register);
 
-void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq)
+void pwrseq_unregister(struct pwrseq *pwrseq)
 {
if (pwrseq) {
mutex_lock(_list_mutex);
@@ -111,4 +111,4 @@ void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq)
mutex_unlock(_list_mutex);
}
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_unregister);
+EXPORT_SYMBOL_GPL(pwrseq_unregister);
diff --git a/drivers/power/pwrseq/pwrseq_emmc.c 
b/drivers/power/pwrseq/pwrseq_emmc.c
index a0583ed46d7f..a68ac9a68e04 100644
--- a/drivers/power/pwrseq/pwrseq_emmc.c
+++ b/drivers/power/pwrseq/pwrseq_emmc.c
@@ -22,7 +22,7 @@
 #include 
 
 struct mmc_pwrseq_emmc {
-   struct mmc_pwrseq pwrseq;
+   struct pwrseq pwrseq;
struct notifier_block reset_nb;
struct gpio_desc *reset_gpio;
 };
@@ -54,7 +54,7 @@ static int mmc_pwrseq_emmc_reset_nb(struct notifier_block 
*this,
return NOTIFY_DONE;
 }
 
-static const struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
+static const struct pwrseq_ops mmc_pwrseq_emmc_ops = {
.post_power_on = mmc_pwrseq_emmc_reset,
 };
 
@@ -85,7 +85,7 @@ static int mmc_pwrseq_emmc_probe(struct platform_device *pdev)
pwrseq->pwrseq.owner = THIS_MODULE;
platform_set_drvdata(pdev, pwrseq);
 
-   return mmc_pwrseq_register(>pwrseq);
+   return pwrseq_register(>pwrseq);
 }
 
 static int mmc_pwrseq_emmc_remove(struct platform_device *pdev)
@@ -93,7 +93,7 @@ static int mmc_pwrseq_emmc_remove(struct platform_device 
*pdev)
struct mmc_pwrseq_emmc *pwrseq = platform_get_drvdata(pdev);
 
unregister_restart_handler(>reset_nb);
-   mmc_pwrseq_unregister(>pwrseq);
+   pwrseq_unregister(>pwrseq);
 
return 0;
 }
diff --git a/drivers/power/pwrseq/pwrseq_simple.c 
b/drivers/power/pwrseq/pwrseq_simple.c
index 786f1db53a3f..d5fbd653153e 100644
--- a/drivers/power/pwrseq/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -21,7 +21,7 @@
 #include 
 
 struct mmc_pwrseq_simple {
-   struct mmc_pwrseq pwrseq;
+   struct pwrseq pwrseq;
bool clk_enabled;
struct clk *ext_clk;
struct gpio_descs *reset_gpios;
@@ -77,7 +77,7 @@ static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
}
 }
 
-static const struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
+static const struct pwrseq_ops mmc_pwrseq_simple_ops = {
.pre_power_on 

RE: MUSB driver on AM3352 dropping USB packets

[Andrew Goodbody]

> From: Bin Liu [mailto:b-...@ti.com]
> Hi,
> 
> On Wed, May 04, 2016 at 03:48:50PM +, Andrew Goodbody wrote:
> > Hi,
> >
> > I have been investigating communication issues with iPads. When the
> > system is busy it seems that the musb driver is silently dropping
> > occasional packets. I have a usbmon trace that does not show the
> > packet and I have a trace from a hardware USB analyser that does show
> > the packet. So the device is correctly sending the packet, it is even
> > being ACKed, but it is not passed up to the application. The packet is
> > a bulk transfer packet of 20 bytes. Can anyone please give me pointers
> > to where to go looking for the problem? The syslog shows nothing
> > relevant.
> 
> What is the part number on the am3352 chip?

AM3352BZCZ100

> What kernel version do you use?

4.5.0

> Is musb cppi dma enabled? If so, does the problem still happen when CPPI
> disabled?

Yes. Yes. When testing with PIO I did get the message "Rx interrupt with no 
errors or packet!".

> First try to turn on dynamic debug log in musb_host.c to check if musb
> receives the packet or not.
> 
> Regards,
> -Bin.

I am having problems doing this. If I enable the whole file then I get lots of 
messages on the console about /dev/kmsg buffer overrun. There are more then 26 
million packets in the hardware trace and I have not worked out how to 
correlate any of the possible message from dynamic debug with those packets 
even when I enable some of the dynamic debug lines.
I can see a few messages about "DMA complete but packet still in FIFO, CSR 
2103" and just the occasional "extra TX2 ready, csr 2100" when I enable some of 
the lines for dynamic debug.

Andrew
--
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: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions

[Guodong Xu]

On 5 May 2016 at 16:11, Dean Jenkins  wrote:
> On 05/05/16 00:45, John Stultz wrote:
>>
>> On Tue, May 3, 2016 at 3:54 AM, Dean Jenkins 
>> wrote:
>>>
>>> On 03/05/16 11:04, Guodong Xu wrote:

 did you test on ARM 64-bit system or ARM 32-bit? I ask because HiKey
 is an ARM 64-bit system. I suggest we should be careful on that. I saw
 similar issues when transferring to a 64-bit system in other net
 drivers.
>>>
>>> We used 32-bit ARM and never tested on 64-bit ARM so I suggest that the
>>> commits need to be reviewed with 64-bit OS in mind.


 Do you have any suggestion on this regard?
>>>
>>> Try testing on a Linux PC x86 32-bit OS which has has a kernel containing
>>> my
>>> ASIX commits. This will help to confirm whether the failure is related to
>>> 32-bit or 64-bit OS. Then try with Linux PC x86 64-bit OS, this should
>>> fail
>>> otherwise it points to something specific in your ARM 64-bit platform.
>>
>> Just as a sample point, I have managed to reproduce exactly this issue
>> on an x86_64 system by simply scp'ing a large file.
>
> Please tell us the x86_64 kernel version number that you used and which
> Linux Distribution it was ? This allows other people a chance to reproduce
> your observations.
>
>>
>> [  417.819276] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
>> synchronisation was lost, remaining 988
>
> It is interesting that the reported "remaining" value is 988. Is 988 always
> shown ? I mean that do you see any other "remaining" values for the "Data
> Header synchronisation was lost" error message ?
>
>> [  417.823415] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0xef830347, offset 4
>
> The gap in the timestamps shows 417.823415 - 417.819276 = 0.004139 = 4ms
> which is a large gap in terms of USB 2.0 high speed communications. This gap
> is expected to be in the 100us range for consecutive URBs. So 4ms is
> strange.
>
> The expectation is that the "Data Header synchronisation was lost" error
> message resets the 32-bit header word synchronisation to the start of the
> next URB buffer. The "Bad Header Length, offset 4" is the expected outcome
> for the next URB because it is unlikely the 32-bit header word is at the
> start of URB buffer due to Ethernet frames spanning across URBs.
>>
>> [  417.827502] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0x31e2b348, offset 4
>
> Timestamps show the gap to be 4ms which is strange for USB 2.0 high speed,
> are you sure high speed mode is being used ?
>>
>> [  417.843779] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
>> synchronisation was lost, remaining 988
>> [  417.847921] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0x8af91035, offset 4
>> [  417.852004] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0x8521fa03, offset 4
>> [  418.273394] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
>> synchronisation was lost, remaining 988
>> [  418.277532] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0x33cd9c7c, offset 4
>> [  418.281683] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0x3d850896, offset 4
>> [  418.286227] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0x86443357, offset 4
>> [  418.290319] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0xee6c81d1, offset 4
>>
>> I don't have any 32bit x86 installs around so I'm not sure I can easly
>> test there, but its clear its not arm64 specific.
>
> I agree the issue is not specific to your ARM 64 bit platform.
>
> Please can you supply the output of ifconfig for the USB to Ethernet
> adaptor, your example above shows eth1 as the device.
>
> Please show the output of ifconfig eth1 before and after the issue is seen.
> This will show us whether the kernel logs any network errors and how many
> bytes have been transferred.
>
> After the issue is seen, please can you show us the output of "dmesg | grep
> asix" so that we can see status messages from the ASIX driver that the USB
> to Ethernet adaptor is using. In particular we need to check that USB high
> speed operation (480Mbps) is being used and not full speed operation
> (12Mbps).

Hi, Dean

I am not sure why do you insist 'not full speed'. Actually, the tests
I run on ARM-64bit is at USB full speed mode. I pasted my log here:
http://paste.ubuntu.com/16236442/
, which includes the information you requested above, ifconfig, dmesg.
The interval between two consecutive errors varies from 10 to 40ms.

> It is interesting that the reported "remaining" value is 988. Is 988 always
> shown ? I mean that do you see any other "remaining" values for the "Data
> Header synchronisation was lost" error message ?

Yes and No. When doing iperf test in TCP mode, always 988. I have
never seen other "remaining" value.

But,
1. I tried "ping -f -s 1400 [my.arm.64bit.board.ip]", but this cannot
trigger the error.
2. Tried iperf in UDP mode, I saw "Data Header synchronisation was
lost" remaining value is 984 

Re: [PATCH] usb: gadget: f_fs: Fix kernel panic for SuperSpeed

[Jim Lin]

On 2016年05月04日 18:37, Felipe Balbi wrote:

* PGP Signed by an unknown key


Hi,

Jim Lin  writes:




In f_fs.c
"
static int __ffs_data_do_os_desc(enum ffs_os_desc_type type,
struct usb_os_desc_header *h, void *data,
unsigned len, void *priv)
{
   struct ffs_data *ffs = priv;
   u8 length;

   ENTER();

   switch (type) {
   case FFS_OS_DESC_EXT_COMPAT: {
   struct usb_ext_compat_desc *d = data;
   int i;

   if (len < sizeof(*d) ||
   d->bFirstInterfaceNumber >= ffs->interfaces_count ||
   d->Reserved1)
   return -EINVAL;
"

that's fine, but this is only failing because something else is
returning the wrong set of descriptors (SS vs HS). That's the bug we
want to fix, not work around it.


Thanks.

you're welcome, but to fix that bug we need more information. Why is
composite.c using the wrong set of descriptors ? What is your setup ?

Are you using an in-kernel gadget ? which one ?

No, our gadget driver is on the way to submit.

Using configfs or legacy
gadgets ? gadgetfs ? f_fs ?



  How to trigger this ? Can you provide
instructions and (in case of gadgetfs/ffs) code to create a gadget that
hits this problem ?


Please refer to
https://android.googlesource.com/platform/system/core/+/master/adb/usb_linux_client.cpp
https://android.googlesource.com/device/google/dragon/+/android-6.0.1_r4/init.dragon.usb.rc
https://android.googlesource.com/platform/system/core/+/master/rootdir/init.usb.configfs.rc


Also this is a thought coming from another engineer for your reference.
"

I think Microsoft and linux are contradicting the requirements. 
According MSFT's os descriptor definition, one of the reserved fields 
needs to be set to 1 whereas seems like f_fs.c expects them to be 0. 
(copy pasting from the spec downloaded from: 
https://msdn.microsoft.com/en-us/library/windows/hardware/gg463179.aspx) 
What does upstream think ? Requires some conflict resolution I guess !! 
Since the OS descriptors are from MSFT, I believe upstream has to drop 
the check and I think this patch might be valid..


bFirstInterfaceNumber This field specifies the interface or function 
that is associated with the IDs in this section. To use this function 
section to associate a single-function group of interfaces with a single 
pair of IDs, set bFirstInterfaceNumber to the first interface in the 
group. Then use an IAD in that interface’s interface descriptor to 
specify which additional interfaces should be included in the group. The 
interfaces in the group must be consecutively numbered. For details, see 
“Support for USB Interface Association Descriptor in Windows.”


RESERVED Reserved for system use. Set this value to 0x01.

compatibleID This field contains the value of the compatible ID to be 
associated with this function. Any unused bytes should be filled with 
NULLs. If the function does not have a compatible ID, fill the entire 
field with NULLs.


subCompatibleID This field contains the value of the subcompatible ID to 
be associated with this function. Any remaining bytes should be filled 
with NULLs. If the function does not have a subcompatible ID, fill the 
entire field with NULLs.


RESERVED Reserved. Fill this value with NULLs.
"

--nvpublic

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

[GIT PULL] USB-serial fixes for v4.6-rc7

[Johan Hovold]

Hi Greg,

Here are some new modem device ids for the option driver. These have all
been in linux-next over night and could go into -rc7 unless you prefer
to hold them off for v4.7.

Thanks,
Johan


The following changes since commit 04974df8049fc4240d22759a91e035082ccd18b4:

  Linux 4.6-rc6 (2016-05-01 15:52:31 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git 
tags/usb-serial-4.6-rc7

for you to fetch changes up to 74d2a91aec97ab832790c9398d320413ad185321:

  USB: serial: option: add even more ZTE device ids (2016-05-04 14:16:05 +0200)


USB-serial fixes for v4.6-rc7

Here are some more new device ids.

Signed-off-by: Johan Hovold 


Lei Liu (1):
  USB: serial: option: add even more ZTE device ids

Schemmel Hans-Christoph (1):
  USB: serial: option: add support for Cinterion PH8 and AHxx

lei liu (1):
  USB: serial: option: add more ZTE device ids

 drivers/usb/serial/option.c | 155 ++--
 1 file changed, 148 insertions(+), 7 deletions(-)
--
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 v5 0/3] USB: serial: cp210x: Bugfixes and cleanup in CRTSCTS flag code

[Johan Hovold]

On Wed, May 04, 2016 at 04:56:38PM -0500, Konstantin Shkolnyy wrote:
> This patch series fixes bugs and replaces magic numbers with symbolic
> names in CRTSCTS flag code.
> 
> v5:
> Fixed 2 compile errors in PATCH 2/3, otherwise no change.
> v4:
> Same series of patches, fixed names and defines by feedback.
> v3:
> Regenerated the patches correctly against the latest usb-next branch.
> v2
> Improved CRTSCTS fix by feedback. Dropped get_termios error handling fix.
> 
> Konstantin Shkolnyy (3):
>   USB: serial: cp210x: Fixed RTS mode setting by the CRTSCTS flag.
>   USB: serial: cp210x: Got rid of magic numbers in CRTSCTS flag code.
>   USB: serial: cp210x: Cleaned up CRTSCTS flag code.
> 
>  drivers/usb/serial/cp210x.c | 102 
> 
>  1 file changed, 74 insertions(+), 28 deletions(-)

Thanks for the v5. All now applied for -next.

I reworded the first commit message slightly and added a stable tag
since it clearly fixes a bug even if it happens to have been around
since 2005 without anyone noticing.

And thanks again for doing this work. The code is indeed much easier to
read now, and provides a good foundation for further improvements.

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

[PATCH] usb: gadget: Fix binding to UDC via configfs interface

[Krzysztof Opasiak]

By default user could store only valid UDC name in configfs UDC
attr by doing:

echo $UDC_NAME > UDC

Commit (855ed04 "usb: gadget: udc-core: independent registration of
gadgets and gadget drivers") broke this behavior and allowed to store
any arbitrary string in UDC file and udc core was waiting for such
controller to appear.

echo "any arbitrary string here" > UDC

This commit fix this by adding a flag which prevents configfs
gadget from being added to list of pending drivers if UDC with
given name has not been found.

Signed-off-by: Krzysztof Opasiak 
---
 drivers/usb/gadget/configfs.c |1 +
 drivers/usb/gadget/udc/udc-core.c |   12 
 include/linux/usb/gadget.h|3 +++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index b6f60ca..70cf347 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1401,6 +1401,7 @@ static const struct usb_gadget_driver 
configfs_driver_template = {
.owner  = THIS_MODULE,
.name   = "configfs-gadget",
},
+   .match_existing_only = 1,
 };
 
 static struct config_group *gadgets_make(
diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index e4e70e1..08ff65d 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -591,11 +591,15 @@ int usb_gadget_probe_driver(struct usb_gadget_driver 
*driver)
}
}
 
-   list_add_tail(>pending, _driver_pending_list);
-   pr_info("udc-core: couldn't find an available UDC - added [%s] to list 
of pending drivers\n",
-   driver->function);
+   if (!driver->match_existing_only) {
+   list_add_tail(>pending, _driver_pending_list);
+   pr_info("udc-core: couldn't find an available UDC - added [%s] 
to list of pending drivers\n",
+   driver->function);
+   ret = 0;
+   }
+
mutex_unlock(_lock);
-   return 0;
+   return ret;
 found:
ret = udc_bind_to_driver(udc, driver);
mutex_unlock(_lock);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 5d4e151..5edb37b 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1034,6 +1034,8 @@ static inline int usb_gadget_activate(struct usb_gadget 
*gadget)
  * @udc_name: A name of UDC this driver should be bound to. If udc_name is 
NULL,
  * this driver will be bound to any available UDC.
  * @pending: UDC core private data used for deferred probe of this driver.
+ * @match_existing_only: If udc is not found, return an error and don't add 
this
+ *  gadget driver to list of pending driver
  *
  * Devices are disabled till a gadget driver successfully bind()s, which
  * means the driver will handle setup() requests needed to enumerate (and
@@ -1097,6 +1099,7 @@ struct usb_gadget_driver {
 
char*udc_name;
struct list_headpending;
+   unsignedmatch_existing_only:1;
 };
 
 
-- 
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] usb: dwc3: usb/dwc3: fake dissconnect event when turn off pullup

[Peter Chen]

On Thu, Apr 28, 2016 at 09:46:15AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> (we don't top-post on this forum ;-)
> 
> "Du, Changbin"  writes:
> > Hi, Balbi,
> >
> > The step to reproduce this issue is:
> > 1) connect device to a host and wait its enumeration.
> > 2) trigger software disconnect by calling function
> > usb_gadget_disconnect(), which finally call
> >dwc3_gadget_pullup(false). Do not reconnect device
> >   (I mean no enumeration go on, keep bit Run/Stop 0.).
> >
> > At here, gadget driver's disconnect callback should be
> > Called, right? We has been disconnected. But no, as
> > You said " not generating disconnect IRQ after you
> > drop Run/Stop is expected".
> >
> > And I am testing on an Android device, Android only
> > use dwc3_gadget_pullup(false) to issue a soft disconnection.
> > This confused user that the UI still show usb as connected
> > State, caused by missing a disconnect event.
> 
> okay, so I know what this is. This is caused by Android gadget itself
> not notifying the gadget that a disconnect has happened. Just look at
> udc-core's soft_connect implementation for the sysfs interface, and
> you'll see what I mean.
> 
> This should be fixed at Android gadget itself. The only thing we could
> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the
> logic so it's easier for Android gadget to use; but even that I'm a
> little bit reluctant to do because Android should be using our
> soft_connect interface instead of reimplementing it (wrongly) by its
> own.
> 

If it is a gadget driver, it can call its disconnect explicitly.
Another thing is the gadget driver should not call usb_gadget_disconnect
directly, it should call usb_gadget_deactivate or usb_function_deactivate.

Since currently, calling usb_gadget_disconnect may not do real pull down
dp, Felipe, will you consider adding gadget_driver->disconnect into
usb_gadget_disconnect after pull down dp?

-- 

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: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions

[Dean Jenkins]

On 05/05/16 00:45, John Stultz wrote:

On Tue, May 3, 2016 at 3:54 AM, Dean Jenkins  wrote:

On 03/05/16 11:04, Guodong Xu wrote:

did you test on ARM 64-bit system or ARM 32-bit? I ask because HiKey
is an ARM 64-bit system. I suggest we should be careful on that. I saw
similar issues when transferring to a 64-bit system in other net
drivers.

We used 32-bit ARM and never tested on 64-bit ARM so I suggest that the
commits need to be reviewed with 64-bit OS in mind.


Do you have any suggestion on this regard?

Try testing on a Linux PC x86 32-bit OS which has has a kernel containing my
ASIX commits. This will help to confirm whether the failure is related to
32-bit or 64-bit OS. Then try with Linux PC x86 64-bit OS, this should fail
otherwise it points to something specific in your ARM 64-bit platform.

Just as a sample point, I have managed to reproduce exactly this issue
on an x86_64 system by simply scp'ing a large file.
Please tell us the x86_64 kernel version number that you used and which 
Linux Distribution it was ? This allows other people a chance to 
reproduce your observations.




[  417.819276] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
It is interesting that the reported "remaining" value is 988. Is 988 
always shown ? I mean that do you see any other "remaining" values for 
the "Data Header synchronisation was lost" error message ?



[  417.823415] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xef830347, offset 4
The gap in the timestamps shows 417.823415 - 417.819276 = 0.004139 = 4ms 
which is a large gap in terms of USB 2.0 high speed communications. This 
gap is expected to be in the 100us range for consecutive URBs. So 4ms is 
strange.


The expectation is that the "Data Header synchronisation was lost" error 
message resets the 32-bit header word synchronisation to the start of 
the next URB buffer. The "Bad Header Length, offset 4" is the expected 
outcome for the next URB because it is unlikely the 32-bit header word 
is at the start of URB buffer due to Ethernet frames spanning across URBs.

[  417.827502] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x31e2b348, offset 4
Timestamps show the gap to be 4ms which is strange for USB 2.0 high 
speed, are you sure high speed mode is being used ?

[  417.843779] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  417.847921] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x8af91035, offset 4
[  417.852004] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x8521fa03, offset 4
[  418.273394] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  418.277532] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x33cd9c7c, offset 4
[  418.281683] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x3d850896, offset 4
[  418.286227] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x86443357, offset 4
[  418.290319] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xee6c81d1, offset 4

I don't have any 32bit x86 installs around so I'm not sure I can easly
test there, but its clear its not arm64 specific.

I agree the issue is not specific to your ARM 64 bit platform.

Please can you supply the output of ifconfig for the USB to Ethernet 
adaptor, your example above shows eth1 as the device.


Please show the output of ifconfig eth1 before and after the issue is 
seen. This will show us whether the kernel logs any network errors and 
how many bytes have been transferred.


After the issue is seen, please can you show us the output of "dmesg | 
grep asix" so that we can see status messages from the ASIX driver that 
the USB to Ethernet adaptor is using. In particular we need to check 
that USB high speed operation (480Mbps) is being used and not full speed 
operation (12Mbps).


Thanks,

Regards,
Dean



thanks
-john



--
Dean Jenkins
Embedded Software Engineer
Linux Transportation Solutions
Mentor Embedded Software Division
Mentor Graphics (UK) Ltd.

--
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 v3 2/2] usb: host: ehci-tegra: Avoid getting the same reset twice

[Hans de Goede]

Hi,

On 04-05-16 22:25, Thierry Reding wrote:

On Wed, May 04, 2016 at 11:23:20AM -0600, Stephen Warren wrote:

On 05/04/2016 08:40 AM, Thierry Reding wrote:

From: Thierry Reding 

Starting with commit 0b52297f2288 ("reset: Add support for shared reset
controls") there is a reference count for reset control assertions. The
goal is to allow resets to be shared by multiple devices and an assert
will take effect only when all instances have asserted the reset.

In order to preserve backwards-compatibility, all reset controls become
exclusive by default. This is to ensure that reset_control_assert() can
immediately assert in hardware.

However, this new behaviour triggers the following warning in the EHCI
driver for Tegra:

...

The reason is that Tegra SoCs have three EHCI controllers, each with a
separate reset line. However the first controller contains UTMI pads
configuration registers that are shared with its siblings and that are
reset as part of the first controller's reset. There is special code in
the driver to assert and deassert this shared reset at probe time, and
it does so irrespective of which controller is probed first to ensure
that these shared registers are reset before any of the controllers are
initialized. Unfortunately this means that if the first controller gets
probed first, it will request its own reset line and will subsequently
request the same reset line again (temporarily) to perform the reset.
This used to work fine before the above-mentioned commit, but now
triggers the new WARN.

Work around this by making sure we reuse the controller's reset if the
controller happens to be the first controller.



diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c



@@ -81,15 +81,23 @@ static int tegra_reset_usb_controller(struct 
platform_device *pdev)



+   bool has_utmi_pad_registers = false;

phy_np = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0);
if (!phy_np)
return -ENOENT;

+   if (of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers"))
+   has_utmi_pad_registers = true;


Isn't that just:

has_utmi_pad_registers = of_property_read_bool(phy_np,
"nvidia,has-utmi-pad-registers");

... and then you can remove " = false" from the declaration too?


Yes. This is really only for aesthetics. The direct assignment doesn't
fit within 80 columns, and wrapping it looks ugly whichever way you do
it.


if (!usb1_reset_attempted) {
struct reset_control *usb1_reset;

-   usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
+   if (!has_utmi_pad_registers)
+   usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
+   else
+   usb1_reset = tegra->rst;

...

usb1_reset_attempted = true;
}


This is a pre-existing issue, but what happens if the probes for two USB
controllers run in parallel; there seems to be missing locking related to
testing/setting usb1_reset_attempted, which could cause multiple parallel
attempts to get the "utmi-pads" reset object, which would presumably cause
essentially the same issue this patch is solving in other cases?


Hah! Interestingly my initial attempt at fixing this was to introduce a
lock to serialize these, because I thought that was what was going on. I
don't think this function can ever run concurrently for different
devices because the driver core already serializes probes (unless a
driver specifically requests asynchronous probing, which this one
doesn't).


Why not just use the new shared reset functionality ? It is easy to use,
that way you can drop some of the special handling in the driver and
you're code actually reflects the hardware (which IMHO has a shared reset).

Regards,

Hans

--
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 v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

[Jon Hunter]

On 04/05/16 15:39, Thierry Reding wrote:
> From: Thierry Reding 
> 
> There are three EHCI controllers on Tegra SoCs, each with its own reset
> line. However, the first controller contains a set of UTMI configuration
> registers that are shared with its siblings. These registers will only
> be reset as part of the first controller's reset. For proper operation
> it must be ensured that the UTMI configuration registers are reset
> before any of the EHCI controllers are enabled, irrespective of the
> probe order.
> 
> Commit a47cc24cd1e5 ("USB: EHCI: tegra: Fix probe order issue leading to
> broken USB") introduced code that ensures the first controller is always
> reset before setting up any of the controllers, and is never again reset
> afterwards.
> 
> This code, however, grabs the wrong reset. Each EHCI controller has two
> reset controls attached: 1) the USB controller reset and 2) the UTMI
> pads reset (really the first controller's reset). In order to reset the
> UTMI pads registers the code must grab the second reset, but instead it
> grabbing the first.
> 
> Signed-off-by: Thierry Reding 
> ---
> Stephen, Alex, Jon, have you ever encountered cases where UTMI might not
> have worked correctly? It seems that this code was pulsing the wrong
> reset line and therefore the UTMI pads would never be reset unless the
> first USB controller was probed before all others. I've never seen any
> such problems myself, so I'm unsure about whether it's worth Cc'ing the
> patch to sta...@vger.kernel.org.
> 
>  drivers/usb/host/ehci-tegra.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index 4031b372008e..c1c1024a054c 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -89,7 +89,7 @@ static int tegra_reset_usb_controller(struct 
> platform_device *pdev)
>   if (!usb1_reset_attempted) {
>   struct reset_control *usb1_reset;
>  
> - usb1_reset = of_reset_control_get(phy_np, "usb");
> + usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
>   if (IS_ERR(usb1_reset)) {
>   dev_warn(>dev,
>"can't get utmi-pads reset from the PHY\n");
> 

I have not seen any issues either, but may be we were getting lucky. The
change makes sense to me.

Acked-by: Jon Hunter 

Cheers
Jon
--
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: configfs: allow UDC binding rule configured as binding to *any* UDC

[Krzysztof Opasiak]

Hi,

On 05/05/2016 07:46 AM, Du, Changbin wrote:
> Hi,
>>> On most platforms, there is only one device controller available.
>>> In this case, we desn't care the UDC's name. So let's ignore the
>>> name by setting 'UDC' to 'any'.
>>
>> Hmm libubsgx allows to do this for a very long time. You simply pass
>> NULL instead of pointer to usbg_udc.
>>
>> It is also possible to do this from command line, just simply:
>>
>> $ echo `ls -1 /sys/class/udc | head -n 1` > UDC
>>
>> So if we can easily do this from user space what's the benefit of adding
>> this special "any" keyword to kernel?
>>
> Well, it is just for *easy to use*. Looking up /sys/class/udc mostly
> can be skipped. The UDC core support this convenience behavior, 
> so why don't we export it with a little change?
> 

Well, I'm not sure if any configfs interface has been proposed as easy
to use from cmd line. I think they all has been proposed as  *usable*
from cmd line but not necessarily *easy to use*.

That's why most of configfs clients has some support in userspace. For
example for target there is a taget-cli and for usb gadget we have
libusbg/libusbgx.

So the functionality which you proposed here is not only already
implemented in libusbgx but also can be easily achieved from cmd line
like I showed above.

In addition this patch will break existing userspace tools (at least
libusbgx for sure) as it assumes that:

cat UDC

should return an empty string or an valid UDC name which can be found
inside /sys/class/udc.

After this patch the kernel can return some kind of magic string "any"
which obviously will cannot be found in udc dir.

>>> And also we can change UDC name
>>> at any time if it is not binded (no need set to "" first).
>>>
>>
>> Not sure if:
>>
>> $ echo "" > UDC
>>
>> is really a problem. Personally I'm quite used to situation in which I
>> have to turn the light off before turning it on once again;)
>>
> That is not a problem. But just avoid pseudo 'busy'. If gadget is not 
> bind, it is free to reconfigure it. So seem no need block re-configuration.
> 

What do you mean pseudo 'busy'? If we do:

echo  > UDC

then gadget should be really bound to some udc and potentially really busy.

> In a word, this patch is just an improvement, not to fix any issues or
> add new function.

So it doesn't add any new functionality and breaks existing user space
tools.

Cheers,
-- 
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
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