Re: [PATCH V4 0/5] ARM: OMAP: HOST: TLL driver implementation

[Munegowda, Keshava]

On Sat, Sep 22, 2012 at 3:37 AM, Samuel Ortiz  wrote:
> Hi Paul,
>
> On Wed, Sep 19, 2012 at 08:39:40PM +, Paul Walmsley wrote:
>>
>> Hi Samuel,
>>
>> I've queued patch 1 of this series for 3.7, which should go upstream via
>> the OMAP -> arm-soc path.  Also asked Keshava to re-send patch 5 to me
>> once patches 2-4 have been merged.
>>
>> It's up to you how to handle patches 2-4.  The series is a good step
>> forward for us in terms of cleanup.  But what is probably worthwhile at
>> some point is for that USB TLL "phy" code (added in patch 2) to be moved
>> into some place like drivers/usb/phy, since it's not MFD-related.
> That would be ideal, yes. I kept patches 2-4 as they're alreeady going in the
> right direction (I dropped patches 1 and 5).
>
> Thanks for the heads up.
>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/


thanks paul and Samuel

regards
keshava

regards
keshava
--
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] ARM: Kirkwood: ehci-orion: Add device tree binding

[Andrew Lunn]

> > +Required properties:
> > +- compatible: must be "marvell,orion-ehci"
> > +- reg: physical base address of the controller and length of memory mapped
> > +  region.
> > +- interrupts: The EHCI interrupt
> > +- phy-version: Can be one of:
> > +  "NA" - Don't touch the phy, something else has already configured it.
> > +  "orion5x" - PHY setup as specified by the Orion5x Errata
> > +
> > +Example:
> > +
> > +   ehci@5 {
> > +   compatible = "marvell,orion-ehci";
> > +   reg = <0x5 0x1000>;
> > +   interrupts = <19>;
> > +   phy-version = "NA";
> > +   };
> 
> This isn't an appropriate binding for phy. I know, it maps straight
> over from the platform data, but it doesn't focus on what the actual
> hardware is.
> 
> A couple of options. What probably makes most sense depending on how
> other phy bindings are moving ahead is to add a phy node under the
> ehci controller for the "orion5x" case, and have an appropriate
> compatible value there. No node means the same as "NA" in the above
> binding. Alternatively, have a phy phandle that points to the phy
> device if it sits on an i2c bus, etc.

I Olaf

Could i suggest a third option:

I just drop USB phy configuration all together.  Only mach-orion5x
needs this and nobody has shown any interest in moving mach-orion5x to
DT. So i would just hard code it to "NA".

If anybody does show interest in DT for orion5x, we can add a phy node
under ehci as a pure extension which does not affect backward
compatibility.

Thanks
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: [PATCH] USB: ohci-at91: fix null pointer in ohci_hcd_at91_overcurrent_irq

[Nicolas Ferre]

On 09/23/2012 10:56 PM, Joachim Eastwood :
> Fixes the following NULL pointer dereference:
> [7.74] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
> [7.81] Unable to handle kernel NULL pointer dereference at virtual 
> address 0028
> [7.81] pgd = c3a38000
> [7.81] [0028] *pgd=23a8c831, *pte=, *ppte=
> [7.81] Internal error: Oops: 17 [#1] PREEMPT ARM
> [7.81] Modules linked in: ohci_hcd(+) regmap_i2c snd_pcm usbcore 
> snd_page_alloc at91_cf snd_timer pcmcia_rsrc snd soundcore gpio_keys 
> regmap_spi pcmcia_core usb_common nls_base
> [7.81] CPU: 0Not tainted  (3.6.0-rc6-mpa+ #264)
> [7.81] PC is at __gpio_to_irq+0x18/0x40
> [7.81] LR is at ohci_hcd_at91_overcurrent_irq+0x24/0xb4 [ohci_hcd]
> [7.81] pc : []lr : []psr: 4093
> [7.81] sp : c3a11c40  ip : c3a11c50  fp : c3a11c4c
> [7.81] r10:   r9 : c02dcd6e  r8 : fefff400
> [7.81] r7 :   r6 : c02cc928  r5 : 0030  r4 : c02dd168
> [7.81] r3 : c02e7350  r2 : ffea  r1 : c02cc928  r0 : 
> [7.81] Flags: nZcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment 
> user
> [7.81] Control: c000717f  Table: 23a38000  DAC: 0015
> [7.81] Process modprobe (pid: 285, stack limit = 0xc3a10270)
> [7.81] Stack: (0xc3a11c40 to 0xc3a12000)
> [7.81] 1c40: c3a11c6c c3a11c50 bf08f694 c01392cc c3a11c84 c2c38b00 
> c3806900 0030
> [7.81] 1c60: c3a11ca4 c3a11c70 c0051264 bf08f680 c3a11cac c3a11c80 
> c003e764 c3806900
> [7.81] 1c80: c2c38b00 c02cb05c c02cb000 fefff400 c3806930 c3a11cf4 
> c3a11cbc c3a11ca8
> [7.81] 1ca0: c005142c c005123c c3806900 c3805a00 c3a11cd4 c3a11cc0 
> c0053f24 c00513e4
> [7.81] 1cc0: c3a11cf4 0030 c3a11cec c3a11cd8 c005120c c0053e88 
>  
> [7.81] 1ce0: c3a11d1c c3a11cf0 c00124d0 c00511e0 0140 0001 
> 0012 
> [7.81] 1d00:  c3a11d94 0030  c3a11d34 c3a11d20 
> c005120c c0012438
> [7.81] 1d20: c001dac4 0012 c3a11d4c c3a11d38 c0009b08 c00511e0 
> c00523fc 6013
> [7.81] 1d40: c3a11d5c c3a11d50 c0008510 c0009ab4 c3a11ddc c3a11d60 
> c0008eb4 c00084f0
> [7.81] 1d60:  0030  0080 6013 bf08f670 
> c3806900 c2c38b00
> [7.81] 1d80: 0030 c3806930  c3a11ddc c3a11d88 c3a11da8 
> c0054190 c00523fc
> [7.81] 1da0: 6013  c3a11dec c3a11db8  c2c38b00 
> bf08f670 c3806900
> [7.81] 1dc0:  0080 c02cc928 0030 c3a11e0c c3a11de0 
> c0052764 c00520d8
> [7.81] 1de0: c3a11dfc   0002 bf090f61 0004 
> c02cc930 c02cc928
> [7.81] 1e00: c3a11e4c c3a11e10 bf090978 c005269c bf090f61 c02cc928 
> bf093000 c02dd170
> [7.81] 1e20: c3a11e3c c02cc930 c02cc930 bf0911d0 bf0911d0 bf093000 
> c3a1 
> [7.81] 1e40: c3a11e5c c3a11e50 c0155b7c bf090808 c3a11e7c c3a11e60 
> c0154690 c0155b6c
> [7.81] 1e60: c02cc930 c02cc964 bf0911d0 c3a11ea0 c3a11e9c c3a11e80 
> c015484c c01545e8
> [7.81] 1e80:   c01547e4 bf0911d0 c3a11ec4 c3a11ea0 
> c0152e58 c01547f4
> [7.81] 1ea0: c381b88c c384ab10 c2c10540 bf0911d0  c02d7518 
> c3a11ed4 c3a11ec8
> [7.81] 1ec0: c01544c0 c0152e0c c3a11efc c3a11ed8 c01536cc c01544b0 
> bf091075 c3a11ee8
> [7.81] 1ee0: bf049af0 bf09120c bf0911d0  c3a11f1c c3a11f00 
> c0154e9c c0153628
> [7.81] 1f00: bf049af0 bf09120c 000ae190  c3a11f2c c3a11f20 
> c0155f58 c0154e04
> [7.81] 1f20: c3a11f44 c3a11f30 bf093054 c0155f1c  6a4f 
> c3a11f7c c3a11f48
> [7.81] 1f40: c0008638 bf093010 bf09120c 000ae190  c00093c4 
> 6a4f bf09120c
> [7.81] 1f60: 000ae190  c00093c4  c3a11fa4 c3a11f80 
> c004fdc4 c000859c
> [7.81] 1f80: c3a11fa4 000ae190 6a4f 00016eb8 000ad018 0080 
>  c3a11fa8
> [7.81] 1fa0: c0009260 c004fd58 6a4f 00016eb8 000ae190 6a4f 
> 000ae100 
> [7.81] 1fc0: 6a4f 00016eb8 000ad018 0080 000adba0 000ad208 
>  000ad3d8
> [7.81] 1fe0: beaf7ae8 beaf7ad8 000172b8 b6e4e940 2010 000ae190 
>  
> [7.81] Backtrace:
> [7.81] [] (__gpio_to_irq+0x0/0x40) from [] 
> (ohci_hcd_at91_overcurrent_irq+0x24/0xb4 [ohci_hcd])
> [7.81] [] (ohci_hcd_at91_overcurrent_irq+0x0/0xb4 
> [ohci_hcd]) from [] (handle_irq_event_percpu+0x38/0x1a8)
> [7.81]  r6:0030 r5:c3806900 r4:c2c38b00
> [7.81] [] (handle_irq_event_percpu+0x0/0x1a8) from 
> [] (handle_irq_event+0x58/0x7c)
> [7.81] [] (handle_irq_event+0x0/0x7c) from [] 
> (handle_simple_irq+0xac/0xd8)
> [7.81]  r5:c3805a00 r4:c3806900
> [7.81] [] (handle_simple_irq+0x0/0xd8) from [] 
> (generic_handle_irq+0x3c/0x48)
> [7.81]  r4:

Re: [PATCH 2/5] drivers: usb: otg: make twl6030_usb as a comparator driver to omap_usb2

[ABRAHAM, KISHON VIJAY]

Hi,

On Sat, Sep 22, 2012 at 3:03 AM, Rabin Vincent  wrote:
> 2012/9/6 Kishon Vijay Abraham I :
>> All the PHY configuration other than VBUS, ID GND and OTG SRP are removed
>> from twl6030. The phy configurations are taken care by the dedicated
>> usb2 phy driver. So twl6030 is made as comparator driver for VBUS and
>> ID detection.
>>
>> Signed-off-by: Kishon Vijay Abraham I 
>
> USB doesn't work on pandaboard on linux-next, and bisection shows this
> patch.  Unfortunately, I can't provide a dmesg log because USB is the
> only way I currently have to get one out(!), but presumably it's because
> this omap-usb2 device is never registered?  Looks like this breaks
> non-dt USB on pandaboard; is that intended?

Yes. omap-usb2 is *only* dt supported (New drivers shouldn't have the
old non-dt support).
Some patches are queued only for 3.7.

In case you want to use MUSB please use these patches on linux-next..
[PATCH v2] arm: omap: hwmod: make *phy_48m* as the main_clk of ocp2scp
[PATCH] ARM: OMAP2+: hwmod data: Fix ocp2scp_usb_phy and usb_host_hs
entries (from Benoit)
[PATCH 0/2] ARM: dts: Add subnode for ocp2scp (patch series)
[PATCH v3 0/3] ARM: dts: omap: add dt data for MUSB (patch series)

Pls note all these patches are queued for 3.7

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

Re: [PATCH 2/5] drivers: usb: otg: make twl6030_usb as a comparator driver to omap_usb2

[Rabin Vincent]

2012/9/24 ABRAHAM, KISHON VIJAY :
> On Sat, Sep 22, 2012 at 3:03 AM, Rabin Vincent  wrote:
>> USB doesn't work on pandaboard on linux-next, and bisection shows this
>> patch.  Unfortunately, I can't provide a dmesg log because USB is the
>> only way I currently have to get one out(!), but presumably it's because
>> this omap-usb2 device is never registered?  Looks like this breaks
>> non-dt USB on pandaboard; is that intended?
>
> Yes. omap-usb2 is *only* dt supported (New drivers shouldn't have the
> old non-dt support).

Well, USB used to work fine on Pandaboard without DT before the
introduction of "omap-usb2", so one would expected it to continue
working (until the board file is completely removed).

Anyway, I've moved to DT now.

> Some patches are queued only for 3.7.
>
> In case you want to use MUSB please use these patches on linux-next..
> [PATCH v2] arm: omap: hwmod: make *phy_48m* as the main_clk of ocp2scp
> [PATCH] ARM: OMAP2+: hwmod data: Fix ocp2scp_usb_phy and usb_host_hs
> entries (from Benoit)
> [PATCH 0/2] ARM: dts: Add subnode for ocp2scp (patch series)
> [PATCH v3 0/3] ARM: dts: omap: add dt data for MUSB (patch series)

I got these by merging in Benoit's for_3.7/dts_part2 on top of
next-20120921.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg

[Praveen Paneri]

Hi Kishon, Felipe,

Any further comments on these patches? Can they be merged now?

Thanks,
Praveen

On Mon, Sep 17, 2012 at 6:24 PM, Praveen Paneri  wrote:
> This driver uses usb_phy interface to interact with s3c-hsotg. Supports
> phy_init and phy_shutdown functions to enable/disable phy. Tested with
> smdk6410 and smdkv310. More SoCs can be brought under later.
>
> Signed-off-by: Praveen Paneri 
> Acked-by: Heiko Stuebner 
> ---
>  .../devicetree/bindings/usb/samsung-usbphy.txt |9 +
>  drivers/usb/phy/Kconfig|8 +
>  drivers/usb/phy/Makefile   |1 +
>  drivers/usb/phy/samsung-usbphy.c   |  360 
> 
>  include/linux/platform_data/samsung-usbphy.h   |   27 ++
>  5 files changed, 405 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>  create mode 100644 drivers/usb/phy/samsung-usbphy.c
>  create mode 100644 include/linux/platform_data/samsung-usbphy.h
>
> diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt 
> b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> new file mode 100644
> index 000..fefd9c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> @@ -0,0 +1,9 @@
> +* Samsung's usb phy transceiver
> +
> +The Samsung's phy transceiver is used for controlling usb otg phy for
> +s3c-hsotg usb device controller.
> +
> +Required properties:
> +- compatible : should be "samsung,exynos4210-usbphy"
> +- reg : base physical address of the phy registers and length of memory 
> mapped
> +   region.
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 63c339b..313685f 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -32,3 +32,11 @@ config MV_U3D_PHY
> help
>   Enable this to support Marvell USB 3.0 phy controller for Marvell
>   SoC.
> +
> +config SAMSUNG_USBPHY
> +   bool "Samsung USB PHY controller Driver"
> +   depends on USB_S3C_HSOTG
> +   select USB_OTG_UTILS
> +   help
> + Enable this to support Samsung USB phy controller for samsung
> + SoCs.
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index b069f29..55dcfc1 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_OMAP_USB2) += omap-usb2.o
>  obj-$(CONFIG_USB_ISP1301)  += isp1301.o
>  obj-$(CONFIG_MV_U3D_PHY)   += mv_u3d_phy.o
>  obj-$(CONFIG_USB_EHCI_TEGRA)   += tegra_usb_phy.o
> +obj-$(CONFIG_SAMSUNG_USBPHY)   += samsung-usbphy.o
> diff --git a/drivers/usb/phy/samsung-usbphy.c 
> b/drivers/usb/phy/samsung-usbphy.c
> new file mode 100644
> index 000..95ec4d0
> --- /dev/null
> +++ b/drivers/usb/phy/samsung-usbphy.c
> @@ -0,0 +1,360 @@
> +/* linux/drivers/usb/phy/samsung-usbphy.c
> + *
> + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> + *  http://www.samsung.com
> + *
> + * Author: Praveen Paneri 
> + *
> + * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG controller
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Register definitions */
> +
> +#define S3C_PHYPWR (0x00)
> +
> +#define S3C_PHYPWR_NORMAL_MASK (0x19 << 0)
> +#define S3C_PHYPWR_OTG_DISABLE (1 << 4)
> +#define S3C_PHYPWR_ANALOG_POWERDOWN(1 << 3)
> +#define S3C_PHYPWR_FORCE_SUSPEND   (1 << 1)
> +/* For Exynos4 */
> +#define EXYNOS4_PHYPWR_NORMAL_MASK (0x39 << 0)
> +#define EXYNOS4_PHYPWR_SLEEP   (1 << 5)
> +
> +#define S3C_PHYCLK (0x04)
> +
> +#define S3C_PHYCLK_MODE_SERIAL (1 << 6)
> +#define S3C_PHYCLK_EXT_OSC (1 << 5)
> +#define S3C_PHYCLK_COMMON_ON_N (1 << 4)
> +#define S3C_PHYCLK_ID_PULL (1 << 2)
> +#define S3C_PHYCLK_CLKSEL_MASK (0x3 << 0)
> +#define S3C_PHYCLK_CLKSEL_SHIFT(0)
> +#define S3C_PHYCLK_CLKSEL_48M  (0x0 << 0)
> +#define S3C_PHYCLK_CLKSEL_12M  (0x2 << 0)
> +#define S3C_PHYCLK_CLKSEL_24M  (0x3 << 0)
> +
> +#define S3C_RSTCON (0x08)
> +
> +#define S3C_RSTCON_PHYCLK  (1 << 2)
> +#define S3C_RSTCON_HCLK   

Re: [PATCH v6 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg

[ABRAHAM, KISHON VIJAY]

Hi,

On Mon, Sep 24, 2012 at 3:08 PM, Praveen Paneri  wrote:
> Hi Kishon, Felipe,
>
> Any further comments on these patches? Can they be merged now?

I don't have any other comments.

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

Re: [PATCH v6 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg

[Kyungmin Park]

Acked-by: Kyungmin Park 

On 9/17/12, Praveen Paneri  wrote:
> This driver uses usb_phy interface to interact with s3c-hsotg. Supports
> phy_init and phy_shutdown functions to enable/disable phy. Tested with
> smdk6410 and smdkv310. More SoCs can be brought under later.
>
> Signed-off-by: Praveen Paneri 
> Acked-by: Heiko Stuebner 
> ---
>  .../devicetree/bindings/usb/samsung-usbphy.txt |9 +
>  drivers/usb/phy/Kconfig|8 +
>  drivers/usb/phy/Makefile   |1 +
>  drivers/usb/phy/samsung-usbphy.c   |  360
> 
>  include/linux/platform_data/samsung-usbphy.h   |   27 ++
>  5 files changed, 405 insertions(+), 0 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>  create mode 100644 drivers/usb/phy/samsung-usbphy.c
>  create mode 100644 include/linux/platform_data/samsung-usbphy.h
>
> diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> new file mode 100644
> index 000..fefd9c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> @@ -0,0 +1,9 @@
> +* Samsung's usb phy transceiver
> +
> +The Samsung's phy transceiver is used for controlling usb otg phy for
> +s3c-hsotg usb device controller.
> +
> +Required properties:
> +- compatible : should be "samsung,exynos4210-usbphy"
> +- reg : base physical address of the phy registers and length of memory
> mapped
> + region.
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 63c339b..313685f 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -32,3 +32,11 @@ config MV_U3D_PHY
>   help
> Enable this to support Marvell USB 3.0 phy controller for Marvell
> SoC.
> +
> +config SAMSUNG_USBPHY
> + bool "Samsung USB PHY controller Driver"
> + depends on USB_S3C_HSOTG
> + select USB_OTG_UTILS
> + help
> +   Enable this to support Samsung USB phy controller for samsung
> +   SoCs.
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index b069f29..55dcfc1 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_OMAP_USB2)   += omap-usb2.o
>  obj-$(CONFIG_USB_ISP1301)+= isp1301.o
>  obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o
>  obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
> +obj-$(CONFIG_SAMSUNG_USBPHY) += samsung-usbphy.o
> diff --git a/drivers/usb/phy/samsung-usbphy.c
> b/drivers/usb/phy/samsung-usbphy.c
> new file mode 100644
> index 000..95ec4d0
> --- /dev/null
> +++ b/drivers/usb/phy/samsung-usbphy.c
> @@ -0,0 +1,360 @@
> +/* linux/drivers/usb/phy/samsung-usbphy.c
> + *
> + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> + *  http://www.samsung.com
> + *
> + * Author: Praveen Paneri 
> + *
> + * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG
> controller
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Register definitions */
> +
> +#define S3C_PHYPWR   (0x00)
> +
> +#define S3C_PHYPWR_NORMAL_MASK   (0x19 << 0)
> +#define S3C_PHYPWR_OTG_DISABLE   (1 << 4)
> +#define S3C_PHYPWR_ANALOG_POWERDOWN  (1 << 3)
> +#define S3C_PHYPWR_FORCE_SUSPEND (1 << 1)
> +/* For Exynos4 */
> +#define EXYNOS4_PHYPWR_NORMAL_MASK   (0x39 << 0)
> +#define EXYNOS4_PHYPWR_SLEEP (1 << 5)
> +
> +#define S3C_PHYCLK   (0x04)
> +
> +#define S3C_PHYCLK_MODE_SERIAL   (1 << 6)
> +#define S3C_PHYCLK_EXT_OSC   (1 << 5)
> +#define S3C_PHYCLK_COMMON_ON_N   (1 << 4)
> +#define S3C_PHYCLK_ID_PULL   (1 << 2)
> +#define S3C_PHYCLK_CLKSEL_MASK   (0x3 << 0)
> +#define S3C_PHYCLK_CLKSEL_SHIFT  (0)
> +#define S3C_PHYCLK_CLKSEL_48M(0x0 << 0)
> +#define S3C_PHYCLK_CLKSEL_12M(0x2 << 0)
> +#define S3C_PHYCLK_CLKSEL_24M(0x3 << 0)
> +
> +#define S3C_RSTCON   (0x08)
> +
> +#define S3C_RSTCON_PHYCLK(1 << 2)
> +#define S3C_RSTCON_HCLK  (1 << 1)
> +#define S3C_RSTCON_PHY   (1 << 0)

Re: [PATCH] usb: serial: ftdi_sio: option to hide selected interfaces of multiple interfaces convertes

[Robert Ryszard Paciorek]

Hi,

> > Maybe this is a better solution, especially considering aversion to do
> > this by the module parameters.
> >
> > Or maybe it is better to give support for I2C, SPI to ftdi_sio module and
> > use sysfs to switch betwen uart/ic2c/spi ...
>
> If can do this on interface basis per-device it looks better.

I try add support for I2C to ftdi_sio with sysfs config interface,
probably in this week.

> > > However back to the SPI/I2C
> > > thingy. You ignore one uart port and decide to use it as SPI host
> > > right? So you should get a reference somehow to this port so you can
> > > register a SPI host to the system right? How do you do this?
> >
> > I plan do this with other kernel module, matching to the same device, but
> > serving other interface. I haven't complete module yet, but in first
> > tests this concept works ok.
>
> How generic is this? I mean is this SPI mode something the chip supports or
> is this just SPI over UART since you enough wires?

This is feature of ft2232 and ft4232 chips ("Multi-Protocol Synchronous
Serial Engine Interface"). More info in data sheet and 
http://www.ftdichip.com/Support/SoftwareExamples/MPSSE.htm

> Sebastian

Regards,
Robert
--
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: removing the timer from cdc-ncm

[Alexey ORISHKO]

> -Original Message-
> From: Oliver Neukum [mailto:oneu...@suse.de]
> Sent: Friday, September 21, 2012 10:35 AM
> To: Alexey ORISHKO; bj...@mork.no; net...@vger.kernel.org; linux-
> u...@vger.kernel.org
> Subject: removing the timer from cdc-ncm
> 
> Hi,
> 
> here is the patch that does everything I consider theoretically
> necessary to have bundling of frames in usbnet and adapting cdc-ncm to
> it.
> 
> I'd appreciate any review in case I am doing something stupid.

Hi Oliver,

I've tried to apply it, but always got a message that "patch does not apply".
What is the base for this patch?

Regards,
Alexey
--
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: removing the timer from cdc-ncm

[Oliver Neukum]

On Monday 24 September 2012 14:13:50 Alexey ORISHKO wrote:

> I've tried to apply it, but always got a message that "patch does not apply".
> What is the base for this patch?

Greg KH's tree, specifically commit 6dab7ede9390d4d937cb89feca932e4fd575d2da

HTH
Oliver

--
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/1] usb: Include generic_interrupt for OMAP2_PLUS

[Felipe Balbi]

Hi,

On Mon, Sep 24, 2012 at 02:53:24PM +0300, philippedesw...@gmail.com wrote:
> From: Philippe De Swert 
> 
> Unless generic_interrupt is defined the irq setup
> in musb_init_controller fails for the omap2430 driver
> since that one does not set it's own interrupt handler.
> Which leaves usb non-functional.
> 
> Tested on N950/N9.
> 
> Signed-off-by: Philippe De Swert 

SoB's mail doesn't From mail.

> ---
>  drivers/usb/musb/musb_core.c |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index db3dff8..ff9e482 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1499,7 +1499,8 @@ static int __devinit musb_core_init(u16 musb_type, 
> struct musb *musb)
>  /*-*/
>  
>  #if defined(CONFIG_SOC_OMAP2430) || defined(CONFIG_SOC_OMAP3430) || \
> - defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_ARCH_U8500)
> + defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_ARCH_U8500) || \
> + defined(CONFIG_ARCH_OMAP2PLUS)

Weird, how can you build OMAP2PLUS without SOC_OMAP ??

BTW, this is also wrong as OMAP2PLUS is also used to enable AM3xxx and
TI8xxx, and those platforms don't use generic_interrupt().

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 1/1] usb: Include generic_interrupt for OMAP2_PLUS

[Felipe Balbi]

On Mon, Sep 24, 2012 at 03:15:39PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Sep 24, 2012 at 02:53:24PM +0300, philippedesw...@gmail.com wrote:
> > From: Philippe De Swert 
> > 
> > Unless generic_interrupt is defined the irq setup
> > in musb_init_controller fails for the omap2430 driver
> > since that one does not set it's own interrupt handler.
> > Which leaves usb non-functional.
> > 
> > Tested on N950/N9.
> > 
> > Signed-off-by: Philippe De Swert 
> 
> SoB's mail doesn't From mail.

doesn't 'match'.

-- 
balbi


signature.asc
Description: Digital signature

Re: USB port power off discussion

[Rafael J. Wysocki]

On Sunday, September 23, 2012, Peter Stuge wrote:
> Rafael J. Wysocki wrote:
> > > Is there already an API for power domains there? What does it look like?
> > 
> > Yes, there is, as it turns out. :-)
> > 
> > Please see drivers/base/power/domain.c and include/linux/pm_domain.h.
> > 
> > Of course, this only covers a limited set of use cases at the moment, but I
> > think it's better to extend the existing code to cover more of them than to
> > add new code of similar functionality.
> > 
> > There is no user space interface right now, because there was no demand and
> > because it is not clear what should be exposed and how.
> 
> For context I maintain libusb. There was a fork some time ago and Red
> Hat promoting the fork along with random propaganda made many distros
> all silently switch to libusbx, but libusb is still around and
> continues being developed.
> 
> For starters I would like an API that sets a power domain on or off.

Unfortunately, in general doing that from user space at random time for
some domains may render the system unusable, so I don't think we can
provide such an interface.

> I think that would be the simplest API, and maybe a good place to
> start. There is definitely room for more complex API as well, getting
> into various policies and so on, but to begin with just on/off is
> fine IMO.
> 
> As I mentioned before in the thread I have no preference whatsoever
> for what the API is, but I do want something that can give back a
> useful error in case of failure. I'm not sure if sysfs can do that.

It can in general, but that depends on the use case.  That's why I'm
generally a bit cautious about user space interfaces.

Thanks,
Rafael
--
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 port power off discussion

[Rafael J. Wysocki]

On Monday, September 24, 2012, Arjan van de Ven wrote:
> On 9/23/2012 8:33 PM, Rafael J. Wysocki wrote:
> > On Sunday, September 23, 2012, Peter Stuge wrote:
> >> Rafael J. Wysocki wrote:
> >>> Well, we actually need to handle power domains appropriately.
> >> ..
> >>> Some work in that direction has been done in the ARM space, where we have
> >>> much more direct access to hardware, and I suppose it may be extended to
> >>> things like "ganged sets of ports" (which actually are power domains).
> >>
> >> Of course.
> >>
> >> Is there already an API for power domains there? What does it look like?
> > 
> > Yes, there is, as it turns out. :-)
> > 
> > Please see drivers/base/power/domain.c and include/linux/pm_domain.h.
> > 
> > Of course, this only covers a limited set of use cases at the moment, but I
> > think it's better to extend the existing code to cover more of them than to
> > add new code of similar functionality.
> 
> yeah.
> well... we also have the voltage rail framework, which came from ARM, but
> matches the ACPI paradigm much more closely.

Well, I'm not sure about that last thing.

> Ideally we map ACPI to the VR framework, and then have a more generic mapping
> from that to power domains.

This way or another, we need to handle power domains more directly.

Thanks,
Rafael
--
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 port power off discussion

[Rafael J. Wysocki]

On Monday, September 24, 2012, Arjan van de Ven wrote:
> On 9/24/2012 12:13 AM, Alan Stern wrote:
> > Therefore all we need is an API for individual devices, not for
> > domains.  Of course, userspace may want to know which devices all
> > belong to the same power domain.
> 
> agreed on both.
> 
> I'd like PowerTOP to be able to report at least who keeps power alive and 
> such.. ;-)

Sure enough.

That's not so simple in general, though, because there may be a hierarchy of
power domains and it doesn't need to be a tree (there are domains having two
"parents", for example).  Exposing _that_ to user space in a straightforward
way may be sort of challenging.

Thanks,
Rafael
--
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/1] usb: Include generic_interrupt for OMAP2_PLUS

[Philippe De Swert]

Hi,

On 24/09/2012, Felipe Balbi  wrote:
> SoB's mail doesn't From mail.

Well still in the progress of migrating of my personal to work laptop.
Since the patch does not seem correct the replacement will have
matching addresses.

>> /*-*/
>>
>>  #if defined(CONFIG_SOC_OMAP2430) || defined(CONFIG_SOC_OMAP3430) || \
>> -defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_ARCH_U8500)
>> +defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_ARCH_U8500) || \
>> +defined(CONFIG_ARCH_OMAP2PLUS)
>
> Weird, how can you build OMAP2PLUS without SOC_OMAP ??

It seems entirely possible. I quickly tried to look if it got defined
somewhere and it does not seem to be set anywhere.  That is why I got
the impression it was replaced by CONFIG_ARCH_OMAP2PLUS. I'll dig
deeper to find out why SOC_OMAP is not set if it should be.

>From the .config I got (used menuconfig)

#
# TI OMAP2/3/4 Specific Features
#
CONFIG_ARCH_OMAP2PLUS_TYPICAL=y
CONFIG_SOC_HAS_OMAP2_SDRC=y
# CONFIG_ARCH_OMAP2 is not set
CONFIG_ARCH_OMAP3=y
# CONFIG_ARCH_OMAP4 is not set
# CONFIG_SOC_OMAP5 is not set
# CONFIG_SOC_OMAP3430 is not set
# CONFIG_SOC_TI81XX is not set
# CONFIG_SOC_AM33XX is not set
CONFIG_OMAP_PACKAGE_CBB=y

Not a mention of CONFIG_SOC_OMAP2430 and  CONFIG_SOC_OMAP3430 did not
get set (while it is not a 3430 but a 3630 I am using). Maybe
CONFIG_ARCH_OMAP3 would have been a better choice then?

> BTW, this is also wrong as OMAP2PLUS is also used to enable AM3xxx and
> TI8xxx, and those platforms don't use generic_interrupt().

It would not break them from what I could see in musb_core.c

musb->isr = generic_interrupt;
status = musb_platform_init(musb); <--- isr would be (re)set here
if (status < 0)
goto fail1;

if (!musb->isr) {
status = -ENODEV;
goto fail2;
}

The two you mention set their own interrupt routines.
drivers/usb/musb/da8xx.c:   musb->isr = da8xx_musb_interrupt;
drivers/usb/musb/am35x.c:   musb->isr = am35x_musb_interrupt;


Regards,

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

Re: [PATCH v6 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg

[Rob Herring]

On 09/17/2012 07:54 AM, Praveen Paneri wrote:
> This driver uses usb_phy interface to interact with s3c-hsotg. Supports
> phy_init and phy_shutdown functions to enable/disable phy. Tested with
> smdk6410 and smdkv310. More SoCs can be brought under later.
> 
> Signed-off-by: Praveen Paneri 
> Acked-by: Heiko Stuebner 
> ---
>  .../devicetree/bindings/usb/samsung-usbphy.txt |9 +
>  drivers/usb/phy/Kconfig|8 +
>  drivers/usb/phy/Makefile   |1 +
>  drivers/usb/phy/samsung-usbphy.c   |  360 
> 
>  include/linux/platform_data/samsung-usbphy.h   |   27 ++
>  5 files changed, 405 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/samsung-usbphy.txt
>  create mode 100644 drivers/usb/phy/samsung-usbphy.c
>  create mode 100644 include/linux/platform_data/samsung-usbphy.h
> 
> diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt 
> b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> new file mode 100644
> index 000..fefd9c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> @@ -0,0 +1,9 @@
> +* Samsung's usb phy transceiver
> +
> +The Samsung's phy transceiver is used for controlling usb otg phy for
> +s3c-hsotg usb device controller.
> +
> +Required properties:
> +- compatible : should be "samsung,exynos4210-usbphy"
> +- reg : base physical address of the phy registers and length of memory 
> mapped
> + region.

What's missing here is describing the connection of phys to host
controllers. We've got several people adding usb phy bindings and need
to define them in a common way.

Rob

> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 63c339b..313685f 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -32,3 +32,11 @@ config MV_U3D_PHY
>   help
> Enable this to support Marvell USB 3.0 phy controller for Marvell
> SoC.
> +
> +config SAMSUNG_USBPHY
> + bool "Samsung USB PHY controller Driver"
> + depends on USB_S3C_HSOTG
> + select USB_OTG_UTILS
> + help
> +   Enable this to support Samsung USB phy controller for samsung
> +   SoCs.
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index b069f29..55dcfc1 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_OMAP_USB2)   += omap-usb2.o
>  obj-$(CONFIG_USB_ISP1301)+= isp1301.o
>  obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o
>  obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
> +obj-$(CONFIG_SAMSUNG_USBPHY) += samsung-usbphy.o
> diff --git a/drivers/usb/phy/samsung-usbphy.c 
> b/drivers/usb/phy/samsung-usbphy.c
> new file mode 100644
> index 000..95ec4d0
> --- /dev/null
> +++ b/drivers/usb/phy/samsung-usbphy.c
> @@ -0,0 +1,360 @@
> +/* linux/drivers/usb/phy/samsung-usbphy.c
> + *
> + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> + *  http://www.samsung.com
> + *
> + * Author: Praveen Paneri 
> + *
> + * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG controller
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Register definitions */
> +
> +#define S3C_PHYPWR   (0x00)
> +
> +#define S3C_PHYPWR_NORMAL_MASK   (0x19 << 0)
> +#define S3C_PHYPWR_OTG_DISABLE   (1 << 4)
> +#define S3C_PHYPWR_ANALOG_POWERDOWN  (1 << 3)
> +#define S3C_PHYPWR_FORCE_SUSPEND (1 << 1)
> +/* For Exynos4 */
> +#define EXYNOS4_PHYPWR_NORMAL_MASK   (0x39 << 0)
> +#define EXYNOS4_PHYPWR_SLEEP (1 << 5)
> +
> +#define S3C_PHYCLK   (0x04)
> +
> +#define S3C_PHYCLK_MODE_SERIAL   (1 << 6)
> +#define S3C_PHYCLK_EXT_OSC   (1 << 5)
> +#define S3C_PHYCLK_COMMON_ON_N   (1 << 4)
> +#define S3C_PHYCLK_ID_PULL   (1 << 2)
> +#define S3C_PHYCLK_CLKSEL_MASK   (0x3 << 0)
> +#define S3C_PHYCLK_CLKSEL_SHIFT  (0)
> +#define S3C_PHYCLK_CLKSEL_48M(0x0 << 0)
> +#define S3C_PHYCLK_CLKSEL_12M(0x2 << 0)
> +#define S3C_PHYCLK_CLKSEL_24M(0x3 << 0)
> +
> +#define S3C_RSTCON   (0x08)
> +
> +#define S3C_RSTCON_PH

Re: [PATCH 1/1] usb: Include generic_interrupt for OMAP2_PLUS

[Felipe Balbi]

Hi,

On Mon, Sep 24, 2012 at 03:54:22PM +0300, Philippe De Swert wrote:
> Hi,
> 
> On 24/09/2012, Felipe Balbi  wrote:
> > SoB's mail doesn't From mail.
> 
> Well still in the progress of migrating of my personal to work laptop.
> Since the patch does not seem correct the replacement will have
> matching addresses.
> 
> >> /*-*/
> >>
> >>  #if defined(CONFIG_SOC_OMAP2430) || defined(CONFIG_SOC_OMAP3430) || \
> >> -  defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_ARCH_U8500)
> >> +  defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_ARCH_U8500) || \
> >> +  defined(CONFIG_ARCH_OMAP2PLUS)
> >
> > Weird, how can you build OMAP2PLUS without SOC_OMAP ??
> 
> It seems entirely possible. I quickly tried to look if it got defined
> somewhere and it does not seem to be set anywhere.  That is why I got
> the impression it was replaced by CONFIG_ARCH_OMAP2PLUS. I'll dig
> deeper to find out why SOC_OMAP is not set if it should be.
> 
> From the .config I got (used menuconfig)
> 
> #
> # TI OMAP2/3/4 Specific Features
> #
> CONFIG_ARCH_OMAP2PLUS_TYPICAL=y
> CONFIG_SOC_HAS_OMAP2_SDRC=y
> # CONFIG_ARCH_OMAP2 is not set
> CONFIG_ARCH_OMAP3=y
> # CONFIG_ARCH_OMAP4 is not set
> # CONFIG_SOC_OMAP5 is not set
> # CONFIG_SOC_OMAP3430 is not set
> # CONFIG_SOC_TI81XX is not set
> # CONFIG_SOC_AM33XX is not set
> CONFIG_OMAP_PACKAGE_CBB=y
> 
> Not a mention of CONFIG_SOC_OMAP2430 and  CONFIG_SOC_OMAP3430 did not
> get set (while it is not a 3430 but a 3630 I am using). Maybe
> CONFIG_ARCH_OMAP3 would have been a better choice then?

that's quite f**ked up. Tony, why doesn't SOC_OMAP3430 get set for all
OMAP3 boards ? And another question: why don't we have a matching
SOC_OMAP3530, SOC_OMAP3630 and so on ?

> > BTW, this is also wrong as OMAP2PLUS is also used to enable AM3xxx and
> > TI8xxx, and those platforms don't use generic_interrupt().
> 
> It would not break them from what I could see in musb_core.c
> 
> musb->isr = generic_interrupt;
> status = musb_platform_init(musb); <--- isr would be (re)set here

good point. Still that code would be hanging there with no users... Fair
enough, it's better to have extra code when it's not needed, then having
no code when it's needed :-)

That entire musb->isr crap is quite screwed up anyway and it's just
because TI's non-OMAP processors (daxxx, amxxx, ti8xxx, etc) decided
that the wrapper should read MUSB's address space (including IRQ
registers) and since MUSB's IRQ registers are clear-on-read we need to
handle the IRQ from wrapper drivers, which is pretty screwed up as well.

Ideally, we wouldn't have all of these exposed symbols flying around :-s

-- 
balbi


signature.asc
Description: Digital signature

Re: USB port power off discussion

[Rafael J. Wysocki]

On Saturday, September 22, 2012, Rafael J. Wysocki wrote:
> On Friday, September 21, 2012, Sarah Sharp wrote:
> > Two weeks ago at Linux Plumbers Conference, I presented about the Intel
> > Lynx Point USB port power off mechanism.  This email is a report out of
> > what was discussed, and a kick off point for further discussion.

[...]

> > ca9c9d0 usb : Add sysfs files to control port power.
> 
> http://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=commit;h=ca9c9d0

I have reviewed the whole series (I suppose there's too late for adding
Reviewed-by tags to it, but oh well ;-)) and the only patch I have some serious
doubts about is the one above.  In fact, I'd prefer it to be reverted for now.

First off, if the power resources manipulated through those sysfs files
are shared between ports (i.e. there are power domains), the interface isn't
really well defined, because the advertised functionality (powering off/on a
USB port) is not there in general.

Second, I'm not sure if there's any way for user space to figure out what
ports are connected to what sockets visible to user space.  If there is such
a way, I wonder what it is, if not, the proposed interface is just plain
dangerous.

Finally, it follows from my experience that interfaces of this kind often
are sources of pain and grief for distro support folks who need to handle
problems reported by users.  I used to do that and I know what kind of pain
that is.  So, in my opinion it's better not to expose low-level functionality
to user space directly, like in this case.

Of course, I understand the desire to make the feature actually useful to
someone, but what about making system suspend/hibernation use it to start
with?  The logic there should be quite straightforward.

Thanks,
Rafael
--
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/5] drivers: usb: phy: add a new driver for omap usb2 phy

[Rob Herring]

On 09/06/2012 09:57 AM, Kishon Vijay Abraham I wrote:
> All phy related programming like enabling/disabling the clocks, powering
> on/off the phy is taken care of by this driver. It is also used for OTG
> related functionality like srp.
> 
> This also includes device tree support for usb2 phy driver and
> the documentation with device tree binding information is updated.
> 
> Currently writing to control module register is taken care in this
> driver which will be removed once the control module driver is in place.
> 
> Cc: Felipe Balbi 
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  Documentation/devicetree/bindings/usb/usb-phy.txt |   17 ++
>  drivers/usb/phy/Kconfig   |9 +
>  drivers/usb/phy/Makefile  |1 +
>  drivers/usb/phy/omap-usb2.c   |  271 
> +
>  include/linux/usb/omap_usb.h  |   46 
>  include/linux/usb/phy_companion.h |   34 +++
>  6 files changed, 378 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb-phy.txt
>  create mode 100644 drivers/usb/phy/omap-usb2.c
>  create mode 100644 include/linux/usb/omap_usb.h
>  create mode 100644 include/linux/usb/phy_companion.h
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt 
> b/Documentation/devicetree/bindings/usb/usb-phy.txt
> new file mode 100644
> index 000..80d4148
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt

This is a very generic name...

> @@ -0,0 +1,17 @@
> +USB PHY
> +
> +OMAP USB2 PHY
> +
> +Required properties:
> + - compatible: Should be "ti,omap-usb2"

...for a specific phy. However, I do think a generic binding to describe
host ctrlr to phy connections is needed.

> + - reg : Address and length of the register set for the device. Also
> +add the address of control module dev conf register until a driver for
> +control module is added

The dts should describe the h/w, not what you need for the current
driver. The 2nd reg field does not belong here.

> +
> +This is usually a subnode of ocp2scp to which it is connected.

How is usb port to phy connection described?

Rob

> +
> +usb2phy@4a0ad080 {
> + compatible = "ti,omap-usb2";
> + reg = <0x4a0ad080 0x58>,
> +   <0x4a002300 0x4>;
> +};
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 2838adb..63c339b 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -4,6 +4,15 @@
>  comment "USB Physical Layer drivers"
>   depends on USB || USB_GADGET
>  
> +config OMAP_USB2
> + tristate "OMAP USB2 PHY Driver"
> + select USB_OTG_UTILS
> + help
> +   Enable this to support the transceiver that is part of SOC. This
> +   driver takes care of all the PHY functionality apart from comparator.
> +   The USB OTG controller communicates with the comparator using this
> +   driver.
> +
>  config USB_ISP1301
>   tristate "NXP ISP1301 USB transceiver support"
>   depends on USB || USB_GADGET
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index bb948fb..b069f29 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -4,6 +4,7 @@
>  
>  ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>  
> +obj-$(CONFIG_OMAP_USB2)  += omap-usb2.o
>  obj-$(CONFIG_USB_ISP1301)+= isp1301.o
>  obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o
>  obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
> new file mode 100644
> index 000..15ab3d6
> --- /dev/null
> +++ b/drivers/usb/phy/omap-usb2.c
> @@ -0,0 +1,271 @@
> +/*
> + * omap-usb2.c - USB PHY, talking to musb controller in OMAP.
> + *
> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Author: Kishon Vijay Abraham I 
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/**
> + * omap_usb2_set_comparator - links the comparator present in the sytem with
> + *   this phy
> + * @comparator - the companion phy(comparator) for this phy
> + *
> + * The phy companion driver should call this API passing the phy_companion
> + * filled with set_vbus and start_srp to be used by usb phy.
> + *
> + * For use by phy companion driver
> + */
> +int omap_usb2_set_comparator(struct phy_

Re: usb-audio: Increasing MAX_QUEUE to prevent hiccups in the audio stream?

[Matthijs Kooijman]

Hi Alan,

On Sat, Sep 15, 2012 at 11:03:39AM -0400, Alan Stern wrote:
> On Sat, 15 Sep 2012, Matthijs Kooijman wrote:
> 
> > Hi folks,
> > 
> > I've spent some time this week trying to debug frequent hiccups in my
> > audio playback, on my USB sound card. The short version is that it seems
> > the 24 ms worth of queued URBs is not enough, since the urb complete handler
> > is frequently called too late and sometimes more than 16ms (plus 8ms in
> > the urb that completed is 24ms) too late, causing a hiccup in the sound.
> 
> ...
> 
> >  - Or are the delays in the urb completion handler / interrupt by
> >definition a bug in (some other part of) the code? In that case, I
> >might be able to dig in deeper to find and fix the cause of the
> >delays, if someone can offer some suggestions about where to look for
> >the problem.
> 
> Delays that long should be looked into.  They may or may not be caused
> by bugs, but the chance that they are means that they deserve
> investigation.  The bugs don't have to like in the USB stack; they
> could be anywhere in the kernel.  Tracking them down is not necessarily
> easy to do.
Ok. I'll be happy to try your suggestions, but for some reason I don't
understand, this issue has completely disappeared for me, from one day
to the next. I'm still having some stuttering when using pulseaudio's
equalizer module, but this seems to be a seperate issue (no "delay"
messages involved).


Regardless of this, I'm still wondering if increasing the queue size
would make sense, especially from a interrupts per second / power usage
point of view. However, increasing MAX_QUEUE is not enough here: It only
causes more urbs of 8 ms/urb to get queue (up to 8 currently, for a max
buffer of 64 ms). However, since there is an interrupt for every urb,
this doesn't help to reduce the number of interrupts.

What does help, is increasing nrpacks. I tried bumping MAX_PACKS to 400
and passing nrpacks=400 on module load, which makes the interrupt rate
drop from 120/s to 3/s (since there are now always two urbs queued with
400ms worth of data each, instead of three urbs with 8ms of data each).

I'm not sure if there's a direct power usage gain, since paplay and
pulseaudio seemed to still cause a considerable number of wakeups, but
in theory, it should at least help.

I guess that just raising nrpacks by itself is not enough to make this
work, since that creates a very big buffer that alsa clients can't
touch, causing a big minimum latency. I'm not completely sure how this
stuff works, but I guess there should be some way for an alsa client to
rewind the stream, causing one or more urbs to canceled and resubmitted?

Gr.

Matthijs


signature.asc
Description: Digital signature

Re: USB port power off discussion

[Lan Tianyu]

On 2012/9/24 21:18, Rafael J. Wysocki wrote:

On Saturday, September 22, 2012, Rafael J. Wysocki wrote:

On Friday, September 21, 2012, Sarah Sharp wrote:

Two weeks ago at Linux Plumbers Conference, I presented about the Intel
Lynx Point USB port power off mechanism.  This email is a report out of
what was discussed, and a kick off point for further discussion.


[...]


ca9c9d0 usb : Add sysfs files to control port power.


http://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=commit;h=ca9c9d0


I have reviewed the whole series (I suppose there's too late for adding
Reviewed-by tags to it, but oh well ;-)) and the only patch I have some serious
doubts about is the one above.  In fact, I'd prefer it to be reverted for now.

First off, if the power resources manipulated through those sysfs files
are shared between ports (i.e. there are power domains), the interface isn't
really well defined, because the advertised functionality (powering off/on a
USB port) is not there in general.
USB port power off is defined in the usb spec that when the port's 
PORT_POWER feature is removed, port power should be turn off.

But most hubs don't implement it. So the feature should be general and
not common.



Second, I'm not sure if there's any way for user space to figure out what
ports are connected to what sockets visible to user space.  If there is such
a way, I wonder what it is, if not, the proposed interface is just plain
dangerous.
ACPI _PLD method provides a lot of information to describe where the 
port located in. But currently it is not exposed to user space.


Finally, it follows from my experience that interfaces of this kind often
are sources of pain and grief for distro support folks who need to handle
problems reported by users.  I used to do that and I know what kind of pain
that is.  So, in my opinion it's better not to expose low-level functionality
to user space directly, like in this case.


You mean force power on and power off? There is a demand that if a usb 
device was abnormal, user space driver or app could make it rework via 
power off.


Of course, I understand the desire to make the feature actually useful to
someone, but what about making system suspend/hibernation use it to start
with?  The logic there should be quite straightforward.

Thanks,
Rafael




--
Best Regards
Tianyu Lan
linux kernel enabling team
--
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: [Libusbx-devel] libusbx-1.0.13 has been released

[Hans de Goede]

Hi,

On 09/20/2012 11:42 PM, Pete Batard wrote:

Hi,

It with pleasure that I would like to announce the release of libusbx
1.0.13. This version brings the following notable changes:


The Fedora packages for libusbx have been upgraded to 1.0.13 now.

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 v1 1/2] USB: check port changes before hub runtime suspend for some bug device

[Ming Lei]

On Mon, Sep 24, 2012 at 12:09 AM, Alan Stern  wrote:

> No, you can still handle the other case.  You need to report the
> condition to the PM core, say by calling pm_wakeup_event().

Good point, so kind of below code should be added to usb_suspend_both
to handle the situation centralizedly:

if (!PMSG_IS_AUTO(msg)) {
 if (hdev->do_remote_wakeup && status == -EBUSY)
  pm_wakeup_event(&hdev->dev, timeout)
 status = 0;
}

For hub devices, the 'timeout' should be a bit long since khubd has been
frozen.

Thanks,
--
Ming Lei
--
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: Remote wakeup vs. hub suspend

[Alan Stern]

On Sun, 23 Sep 2012, Oliver Neukum wrote:

> On Sunday 23 September 2012 12:23:40 Alan Stern wrote:
> > It turns out that the USB-2 spec does not take into account the
> > The race _was_ recognized in one of the errata documents published 
> > after the USB-2.0 spec came out.  The solution recommended in that
> 
> link?

This is in the May 28, 2002 errata document (2002_05_28_errata.pdf), 
which is part of the USB-2.0 ZIP file currently available at 
http://www.usb.org/developers/docs/usb_20_071012.zip.  The particular 
text is on p.3: "Chapter 10: Add host software requirement to suspend 
downstream ports before suspending a hub:".

> > document is exactly wrong!  It says that all the enabled ports on a hub 
> > should be suspended before the hub is suspended -- this just makes the 
> > race more likely to occur.
> > 
> > The only viable solution is to make sure that _none_ of a hub's ports
> > are suspended when the hub is suspended.  That way a downstream device
> > will not be able to send a remote wakeup request until after the hub is
> > fully suspended, so the race will never occur.
> 
> Only if remote wakeup is enabled.

If remote wakeup is not enabled then there is no race in the first 
place, so it certainly will never occur.

If you mean that it's okay to leave ports suspended if the devices 
attached to the ports aren't enabled for remote wakeup, then yes, I 
agree.

> > Runtime suspend is harder to handle.  The hub_suspend() routine would
> > have to make the suspend feature is turned off for all the ports
> > attached to devices that are enabled for remote wakeup before allowing
> > the hub to suspend.  The hub_resume() routine would either have to
> > re-enable the suspend feature for those ports or else cause all the
> > wakeup-enabled children to be resumed when the hub is.  No matter how 
> > we handle it, the result will be pretty inefficient.
> 
> I guess the worst case of waking up all but one is unavoidable
> as a worst case.

Do you mean that we will have N-1 unnecessary wakeups if N children are 
enabled for remote wakeup?  It's actually worse than that -- the reason 
for resuming the hub may be something other than a wakeup request.  In 
that case we would have N unnecessary wakeups.

In the end it would probably be simpler to re-enable the suspend 
feature on those ports.

> > I don't know what we should do.  Suggestions, anybody?
> 
> Merge the power transitions as much as possible if remote wakeup
> is requested. That would mean
> 
> a) divorce logically suspending a device and suspending the port

I'm not sure we should do this.  When the device is logically
suspended, the driver cancels all the outstanding URBs and relies on
the remote wakeup mechanism for notifications of important events.  If
the device isn't physically suspended then it can't send wakeup
requests.

> b) if the last port is "logically" to be suspended within a certain period
> (eg. again the autosuspend delay) of a device defer the physical suspend of 
> other devices
> c) if all devices are logically suspended suspend the uppermost possible 
> parent
> physically and logically immediately
> 
> That is the best I can come up with.

The alternative seems to be ignoring the whole issue and hope that it 
goes away in time.  I doubt this will work out well; there are people 
who require reliable wakeup handling.

Alan Stern

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

[PATCH] powerpc/usb: remove checking PHY_CLK_VALID for UTMI PHY

[Shengzhou Liu]

PHY_CLK_VALID bit doesn't work properly with UTMI PHY.
e.g. This bit is always zero on P5040, etc.
There is no need to check this bit for UTMI PHY, just keep
checking for ULPI PHY to prevent system hanging.

This patch should be squashed into previous commit 3735ba8db8e6e
"powerpc/usb: fix bug of CPU hang when missing USB PHY clock"

Signed-off-by: Shengzhou Liu 
---
 drivers/usb/host/ehci-fsl.c |3 +--
 include/linux/fsl_devices.h |2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 11ff4b4..9bfde82 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -267,8 +267,7 @@ static int ehci_fsl_setup_phy(struct usb_hcd *hcd,
break;
}
 
-   if ((pdata->controller_ver) && ((phy_mode == FSL_USB2_PHY_ULPI) ||
-   (phy_mode == FSL_USB2_PHY_UTMI))) {
+   if (pdata->controller_ver && (phy_mode == FSL_USB2_PHY_ULPI)) {
/* check PHY_CLK_VALID to get phy clk valid */
if (!spin_event_timeout(in_be32(non_ehci + FSL_SOC_USB_CTRL) &
PHY_CLK_VALID, FSL_USB_PHY_CLK_TIMEOUT, 0)) {
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index ccfc4bb..700bf31 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -19,7 +19,7 @@
 
 #define FSL_UTMI_PHY_DLY   10  /*As per P1010RM, delay for UTMI
PHY CLK to become stable - 10ms*/
-#define FSL_USB_PHY_CLK_TIMEOUT1000/* uSec */
+#define FSL_USB_PHY_CLK_TIMEOUT1   /* uSec */
 #define FSL_USB_VER_OLD0
 #define FSL_USB_VER_1_61
 #define FSL_USB_VER_2_22
-- 
1.6.4


--
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] powerpc/usb: fix bug of CPU hang when missing USB PHY clock

[Liu Shengzhou-B36685]

> -Original Message-
> From: Greg KH [mailto:g...@kroah.com]
> Sent: 2012年9月22日 22:49
> To: Kumar Gala
> Cc: Liu Shengzhou-B36685; linuxppc-...@lists.ozlabs.org; linux-
> u...@vger.kernel.org
> Subject: Re: [PATCH v3] powerpc/usb: fix bug of CPU hang when missing
> USB PHY clock
> 
> On Sat, Sep 22, 2012 at 09:39:15AM -0500, Kumar Gala wrote:
> >
> > On Sep 21, 2012, at 11:43 AM, Greg KH wrote:
> >
> > > On Tue, Sep 18, 2012 at 04:52:39PM +0800, Shengzhou Liu wrote:
> > >> when missing USB PHY clock, kernel booting up will hang during USB
> > >> initialization. We should check USBGP[PHY_CLK_VALID] bit to avoid
> > >> CPU hanging in this case.
> > >>
> > >> Signed-off-by: Shengzhou Liu 
> > >> ---
> > >> v3 change: no check for UTMI PHY.
> > >> v2 change: use spin_event_timeout() instead.
> > >>
> > >> drivers/usb/host/ehci-fsl.c |   57 +--
> ---
> > >> drivers/usb/host/ehci-fsl.h |1 +
> > >> include/linux/fsl_devices.h |1 +
> > >> 3 files changed, 41 insertions(+), 18 deletions(-)
> > >
> > > This is already applied, right?
> > >
> > > greg k-h
> >
> > It appears that v2 of the patch is applied to your usb-next branch.
> >
> > in drivers/usb/host/ehci-fsl.c
> >
> > V2:
> > @@ -262,23 +266,34 @@ static void ehci_fsl_setup_phy(struct usb_hcd
> *hcd,
> > case FSL_USB2_PHY_NONE:
> > break;
> > }
> > +
> > +   if ((pdata->controller_ver) && ((phy_mode ==
> FSL_USB2_PHY_ULPI) ||
> > +   (phy_mode == FSL_USB2_PHY_UTMI))) {
> >
> > V3:
> >
> > @@ -262,23 +266,33 @@  static void ehci_fsl_setup_phy(struct usb_hcd
> *hcd,
> >
> > case FSL_USB2_PHY_NONE:
> > break;
> > }
> >
> > +
> > +   if (pdata->controller_ver && (phy_mode == FSL_USB2_PHY_ULPI)) {
> > +   /* check PHY_CLK_VALID to get phy clk valid */
> 
> Ok, can someone please make the incremental patch that I need to apply
> here and send it to me?
> 
> thanks,
> 
> greg k-h

Hi greg,

I have sent the below incremental patch to you, please squash it into the 
previous patch v2, which had been applied in your usb-next branch. 
http://patchwork.ozlabs.org/patch/186443/

Thanks,
Shengzhou

Re: libusbx-1.0.13 has been released

[Greg KH]

On Thu, Sep 20, 2012 at 10:42:10PM +0100, Pete Batard wrote:
> Hi,
> 
> It with pleasure that I would like to announce the release of
> libusbx 1.0.13. This version brings the following notable changes:
> 
> * [MAJOR] Fix a typo in the API with struct libusb_config_descriptor
> where MaxPower was used instead of bMaxPower, as per the specs.
>   If your application was accessing the MaxPower attribute, and you
> need to maintain compatibility with libusb or older versions, please
> see APPENDIX A below.

So, you broke compatibility with existing programs, like usbutils.
That's just foolish and dumb.  I have loads of distros asking me about
this now.

Please put the work-around in the library if you really want to change
this field name, otherwise, I'm going to have to recommend that no one
use libusbx as it's developers really don't care about their users.

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

Re: [Libusbx-devel] libusbx-1.0.13 has been released

[Greg KH]

On Mon, Sep 24, 2012 at 03:52:55PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 09/20/2012 11:42 PM, Pete Batard wrote:
> >Hi,
> >
> >It with pleasure that I would like to announce the release of libusbx
> >1.0.13. This version brings the following notable changes:
> 
> The Fedora packages for libusbx have been upgraded to 1.0.13 now.

How did you handle the usbutils breakage?

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

Re: [PATCH v1 2/2] USB: set hub's default autosuspend delay as 0

[Alan Stern]

On Mon, 24 Sep 2012, Ming Lei wrote:

> On Mon, Sep 24, 2012 at 12:51 AM, Alan Stern  
> wrote:
> > On Sun, 23 Sep 2012, Ming Lei wrote:
> >
> >> > In fact, sometimes new connected device _does_ mean there will be
> >> > another new connection in the next few seconds.  This happens because
> >>
> >> Looks I should have made it clearer, and I mean the physical device
> >> connection or disconnection, eg, device plugging and unplugging.
> >
> > What difference does it make whether the connection was caused by
> > manipulating a physical plug or by electrical signalling?  The end
> > result is the same.
> >
> >> > some devices have trouble initializing when they are first plugged in.
> >> > They disconnect themselves from the bus and then reconnect a second or
> >> > so later.
> >>
> >> First, the situation should be less like to happen.
> >>
> >> Secondly, the patch may only affect the case when the new
> >> connected device is a hub.
> >
> > No, it can happen with any device.
> 
> The patch just changes the hub's autosuspend delay as 0, so
> the default 2 sec delay of other device won't make the hub
> suspend immediately.

It doesn't matter.  Here's the scenario:

New device is plugged in.
The hub wakes up.
The hub driver tries to initialize the new device and fails.
The device disconnects itself from the USB bus.
The hub is autosuspended after 0 seconds.
1 second later the device reconnects itself to the USB bus.
The hub wakes up.
The hub driver initializes the new device successfully.

This can happen for any type of newly connected device.

> If you change usb hid device's autosuspend delay as 0, you will
> find the mouse can't work basically.
> 
> The driver may not know when the URB for receiving data will
> complete, and increasing the PM counter under the situation may not
> let the device suspend.  And changing the autosuspend delay as
> 0 will kill the URB immediately which is not completed, so
> it doesn't work.

Ah, okay, now I get it.  You're right, this is an important point.  I 
would express it differently:

Unlike the usbhid driver, the hub driver does not rely on the
autosuspend delay to provide enough time to handle a wakeup event.  It
uses the PM usage counter to avoid autosuspending until the event has
been fully processed.  Setting the autosuspend delay to 0 won't cause
the hub driver to stop working correctly.

(BTW, if anyone wanted to do it, the usbhid driver could be changed so 
that it too would work correctly with 0 delay.  I don't see any reason 
for doing this, however.)

> > You are ignoring the energy required to make the transition between the
> > active and suspended states,
> 
> The current(Ct) during transition can't go above the hub's downstream port 
> limit
> (100mA or 500mA) per '7.2.1' of USB 2.0 spec, and suppose the below:
> 
>  - the transition from active to suspend current is Cw

You mean Ct here.

>  - the working(active state) current is Cw
>  - the transition time is Tt( <= 10ms, per '7.1.7.6 Suspending')

You have to add in the transition time for resuming.

>  - the suspend time is Ts(suppose 2900ms)

Suppose 50 ms.

>  - the suspend current is Cs(2.5mA)
>  - suppose VBUS voltage(V) not change
> 
> So the extra energy consumed during the transition is that E1 = V*(Ct-Cw)*Tt,
> and the saved energy during suspend period is E2 = V*(Cw-Cs)*Ts. We can
> easily get that E1 < E2 suppose Cw = 1/3 Ct or less.

Nevertheless, there is a cutoff point.  If Ts is too small then E1 >
E2.  The autosuspend delay shouldn't be much smaller than this cutoff
value for Ts (_how_ much smaller depends on how likely it is that a 
resume will be required shortly after a suspend).

> > Why do you think we have an autosuspend delay in the first place?  Why
> > not always use a delay of 0 for every device?
> 
> See the above USB HID example.

No, that's not it at all.  The real reason is performance (i.e., speed
of operation).  If we had to resume a device before every I/O operation
and suspend it afterward, performance would be terrible.

We try to mitigate this problem by avoiding autosuspends in situations
where we think an I/O operation is likely to occur in the near future.  
Since the computer can't predict the future accurately, all we can do
is assume that an I/O operation is more likely to occur if one just
finished.  That's the real reason for not suspending immediately after
each operation.

To justify an autosuspend delay of 0 seconds, you have to demonstrate 
why this argument doesn't apply.  In the case of hubs, there are three 
types of operations to consider:

1: USB traffic to/from a child device,

2: I/O requests from usbfs or sysfs,

3: Port change events (mostly connects and disconnects).

For type 1, the hub's autosuspend delay doesn't matter.  The hub can't 
suspend as long as the child is active, so the child's own autosuspend 
d

Re: usb3 fails to write when using usb3 hub in usb3 port

[Alan Stern]

You should use Reply-To-All so that your messages get sent to the 
mailing list as well as to me.

On Sun, 23 Sep 2012, Adrian Sandu wrote:

> Thanks for the fast response in the 1st place :)
> 
> > Which computer is the nettop and which is the laptop?
> 
> The nettop is an asrock 152d
> http://www.asrock.com/nettop/overview.asp?Model=ION%203D%20Series#Specifications
> 
> The laptop is a sony vaio vpcf13m1e/h
> http://www.sony.ro/product/vaio-seria-f/vpcf13m1e-h
> 
> >>
> >> An usbmon dump on 0u can be seen at 
> >> http://d3xt3r01.tk/~dexter/usbmon.trace.txt
> 
> Refreshed with some more info ..

In the new file you wrote: "launch cat of 9u, start copying a file to 
/dev/sdb1 ( using mc for example )".  Where was this file stored?  Was 
it on /dev/sdd1?

My reason for asking is because the failure did not occur during a 
write to /dev/sdb; it occurred during a read from /dev/sdd.  There is 
no apparent reason for the failure -- following a request for 122880 
bytes of data, the drive sent 2048 bytes and then stopped sending.

The computer successfully reset the drive and asked for the same 122880
bytes again.  The drive failed to respond for 30 seconds, so the
computer tried to reset the drive again.  This time, repeated resets
all failed.  That's why the drive no longer showed up in the lsusb
output.

It's hard to say exactly where the fault lies, but my guess is with the 
drive or its USB interface.

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: usb3 fails to write when using usb3 hub in usb3 port

[Adrian Sandu]

> You should use Reply-To-All so that your messages get sent to the
> mailing list as well as to me.

Will do from now on :)

>
> In the new file you wrote: "launch cat of 9u, start copying a file to
> /dev/sdb1 ( using mc for example )".  Where was this file stored?  Was
> it on /dev/sdd1?
>
> My reason for asking is because the failure did not occur during a
> write to /dev/sdb; it occurred during a read from /dev/sdd.  There is
> no apparent reason for the failure -- following a request for 122880
> bytes of data, the drive sent 2048 bytes and then stopped sending.
>
> The computer successfully reset the drive and asked for the same 122880
> bytes again.  The drive failed to respond for 30 seconds, so the
> computer tried to reset the drive again.  This time, repeated resets
> all failed.  That's why the drive no longer showed up in the lsusb
> output.
>
> It's hard to say exactly where the fault lies, but my guess is with the
> drive or its USB interface.

Will try again in about 2 - 3 hours with only 1 drive connected to the
hub so there won't be any more confussion.

All drives work just fine when connected directly to the usb ports in
the asrock. They also work just fine ( it seems ) when connected
through the usb hub to my laptop. And as I said, both have the same
usb3 hub chip that's why I think it's weird.

Anyhow, will get you more debug in a couple of hours.

Thanks again for bearing me.
--
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-audio: Increasing MAX_QUEUE to prevent hiccups in the audio stream?

[Alan Stern]

On Mon, 24 Sep 2012, Matthijs Kooijman wrote:

> Regardless of this, I'm still wondering if increasing the queue size
> would make sense, especially from a interrupts per second / power usage
> point of view. However, increasing MAX_QUEUE is not enough here: It only
> causes more urbs of 8 ms/urb to get queue (up to 8 currently, for a max
> buffer of 64 ms). However, since there is an interrupt for every urb,
> this doesn't help to reduce the number of interrupts.

That wasn't your goal.  According $SUBJECT, your goal is to prevent
hiccups in the audio stream.

> What does help, is increasing nrpacks. I tried bumping MAX_PACKS to 400
> and passing nrpacks=400 on module load, which makes the interrupt rate
> drop from 120/s to 3/s (since there are now always two urbs queued with
> 400ms worth of data each, instead of three urbs with 8ms of data each).
> 
> I'm not sure if there's a direct power usage gain, since paplay and
> pulseaudio seemed to still cause a considerable number of wakeups, but
> in theory, it should at least help.
> 
> I guess that just raising nrpacks by itself is not enough to make this
> work, since that creates a very big buffer that alsa clients can't
> touch, causing a big minimum latency. I'm not completely sure how this
> stuff works, but I guess there should be some way for an alsa client to
> rewind the stream, causing one or more urbs to canceled and resubmitted?

This is a matter for the ALSA and sound driver people, not for me.

Alan Stern

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

Re: [PATCH 1/1] usb: Include generic_interrupt for OMAP2_PLUS

[Tony Lindgren]

* Felipe Balbi  [120924 06:08]:
> Hi,
> 
> On Mon, Sep 24, 2012 at 03:54:22PM +0300, Philippe De Swert wrote:
> > Hi,
> > 
> > On 24/09/2012, Felipe Balbi  wrote:
> > > SoB's mail doesn't From mail.
> > 
> > Well still in the progress of migrating of my personal to work laptop.
> > Since the patch does not seem correct the replacement will have
> > matching addresses.
> > 
> > >> /*-*/
> > >>
> > >>  #if defined(CONFIG_SOC_OMAP2430) || defined(CONFIG_SOC_OMAP3430) || \
> > >> -defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_ARCH_U8500)
> > >> +defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_ARCH_U8500) || \
> > >> +defined(CONFIG_ARCH_OMAP2PLUS)
> > >
> > > Weird, how can you build OMAP2PLUS without SOC_OMAP ??
> > 
> > It seems entirely possible. I quickly tried to look if it got defined
> > somewhere and it does not seem to be set anywhere.  That is why I got
> > the impression it was replaced by CONFIG_ARCH_OMAP2PLUS. I'll dig
> > deeper to find out why SOC_OMAP is not set if it should be.
> > 
> > From the .config I got (used menuconfig)
> > 
> > #
> > # TI OMAP2/3/4 Specific Features
> > #
> > CONFIG_ARCH_OMAP2PLUS_TYPICAL=y
> > CONFIG_SOC_HAS_OMAP2_SDRC=y
> > # CONFIG_ARCH_OMAP2 is not set
> > CONFIG_ARCH_OMAP3=y
> > # CONFIG_ARCH_OMAP4 is not set
> > # CONFIG_SOC_OMAP5 is not set
> > # CONFIG_SOC_OMAP3430 is not set
> > # CONFIG_SOC_TI81XX is not set
> > # CONFIG_SOC_AM33XX is not set
> > CONFIG_OMAP_PACKAGE_CBB=y
> > 
> > Not a mention of CONFIG_SOC_OMAP2430 and  CONFIG_SOC_OMAP3430 did not
> > get set (while it is not a 3430 but a 3630 I am using). Maybe
> > CONFIG_ARCH_OMAP3 would have been a better choice then?
> 
> that's quite f**ked up. Tony, why doesn't SOC_OMAP3430 get set for all
> OMAP3 boards ? And another question: why don't we have a matching
> SOC_OMAP3530, SOC_OMAP3630 and so on ?

ARCH_OMAP3 will probably eventually just disappear and
be replaced with ARCH_OMAP2PLUS + SOC_. But there's
no need to do it right now as most of that should be
internal to arch/arm/mach-omap2 anyways. I would just
set the driver to depend on ARCH_OMAP2PLUS, and rely on
the platform_data and DT to do something if specified.
That means that on 2420 musb should not probe unless
tusb6010 in specified.

It seems that things like fifo_mode and generic_interrupt
can all be set during the probe based on the platform_data
or DT passed information?
 
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

Re: [PATCH v1 2/2] USB: set hub's default autosuspend delay as 0

[Ming Lei]

On Mon, Sep 24, 2012 at 11:07 PM, Alan Stern  wrote:

>
> It doesn't matter.  Here's the scenario:
>
> New device is plugged in.
> The hub wakes up.
> The hub driver tries to initialize the new device and fails.
> The device disconnects itself from the USB bus.
> The hub is autosuspended after 0 seconds.
> 1 second later the device reconnects itself to the USB bus.
> The hub wakes up.
> The hub driver initializes the new device successfully.
>
> This can happen for any type of newly connected device.

Yes, it is the case in which the patch may introduce one extra
suspend/resume on hub.

The device might be a buggy one, or one just for firmware
downloading, and the time taken to reconnect may be more
than 2 seconds too, :-).

So the case is not a general one, we can eliminate the effect
by setting an appropriate autosuspend delay for its parent
device in the probe() if someone requires.

>
> Unlike the usbhid driver, the hub driver does not rely on the
> autosuspend delay to provide enough time to handle a wakeup event.  It
> uses the PM usage counter to avoid autosuspending until the event has
> been fully processed.  Setting the autosuspend delay to 0 won't cause
> the hub driver to stop working correctly.
>
> (BTW, if anyone wanted to do it, the usbhid driver could be changed so
> that it too would work correctly with 0 delay.  I don't see any reason
> for doing this, however.)

IMO, it might be not doable because the pressed key for remote
wakeup may be lost by the HID device, and holding the PM counter
will never put the device in suspend state.  Looks it is very common
on my USB mouse, the 1st button pressed is only for remote
wakeup and always not obtained by driver when the device is
in suspend.

>
>> > You are ignoring the energy required to make the transition between the
>> > active and suspended states,
>>
>> The current(Ct) during transition can't go above the hub's downstream port 
>> limit
>> (100mA or 500mA) per '7.2.1' of USB 2.0 spec, and suppose the below:
>>
>>  - the transition from active to suspend current is Cw
>
> You mean Ct here.
>
>>  - the working(active state) current is Cw
>>  - the transition time is Tt( <= 10ms, per '7.1.7.6 Suspending')
>
> You have to add in the transition time for resuming.
>
>>  - the suspend time is Ts(suppose 2900ms)
>
> Suppose 50 ms.
>
>>  - the suspend current is Cs(2.5mA)
>>  - suppose VBUS voltage(V) not change
>>
>> So the extra energy consumed during the transition is that E1 = V*(Ct-Cw)*Tt,
>> and the saved energy during suspend period is E2 = V*(Cw-Cs)*Ts. We can
>> easily get that E1 < E2 suppose Cw = 1/3 Ct or less.
>
> Nevertheless, there is a cutoff point.  If Ts is too small then E1 >
> E2.  The autosuspend delay shouldn't be much smaller than this cutoff
> value for Ts (_how_ much smaller depends on how likely it is that a
> resume will be required shortly after a suspend).
>
>> > Why do you think we have an autosuspend delay in the first place?  Why
>> > not always use a delay of 0 for every device?
>>
>> See the above USB HID example.
>
> No, that's not it at all.  The real reason is performance (i.e., speed
> of operation).  If we had to resume a device before every I/O operation
> and suspend it afterward, performance would be terrible.

Not only on performance, and correctness thing may be involved, still see
above HID example, :-)

>
> We try to mitigate this problem by avoiding autosuspends in situations
> where we think an I/O operation is likely to occur in the near future.
> Since the computer can't predict the future accurately, all we can do
> is assume that an I/O operation is more likely to occur if one just
> finished.  That's the real reason for not suspending immediately after
> each operation.
>
> To justify an autosuspend delay of 0 seconds, you have to demonstrate
> why this argument doesn't apply.  In the case of hubs, there are three
> types of operations to consider:
>
> 1: USB traffic to/from a child device,
>
> 2: I/O requests from usbfs or sysfs,
>
> 3: Port change events (mostly connects and disconnects).
>
> For type 1, the hub's autosuspend delay doesn't matter.  The hub can't
> suspend as long as the child is active, so the child's own autosuspend
> delay will prevent the hub from suspending too soon.
>
> Type 2 is probably too infrequent to worry about.  The lsusb example is
> one of the few places it crops up in reality.
>
> Type 3 is the one that matters most.  Here the general reasoning says
> that just after a connect or disconnect event, a new event is more
> likely to occur.  Except for the situation I outlined above
> (initializing a buggy device), this is not true.  Thus the general
> argument doesn't apply, so the only lower limit on autosuspend delays
> for hubs is the power requirement discussed above.
>
> _That_ is what you should explain in the comments.  But t

Re: [PATCH v1 1/2] USB: check port changes before hub runtime suspend for some bug device

[Alan Stern]

On Mon, 24 Sep 2012, Ming Lei wrote:

> On Mon, Sep 24, 2012 at 12:09 AM, Alan Stern  
> wrote:
> 
> > No, you can still handle the other case.  You need to report the
> > condition to the PM core, say by calling pm_wakeup_event().
> 
> Good point, so kind of below code should be added to usb_suspend_both
> to handle the situation centralizedly:
> 
> if (!PMSG_IS_AUTO(msg)) {
>  if (hdev->do_remote_wakeup && status == -EBUSY)
>   pm_wakeup_event(&hdev->dev, timeout)
>  status = 0;
> }

No.  Drivers can return -EBUSY for other reasons besides a pending 
wakeup.  They will have to handle wakeups by themselves.

> For hub devices, the 'timeout' should be a bit long since khubd has been
> frozen.

Yeah, probably around a second or two.

Alan Stern

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

Re: [PATCH] usb/core: Fix race condition when removing EHCI PCI devices

[Alan Stern]

On Mon, 17 Sep 2012, Alan Stern wrote:

> On Mon, 17 Sep 2012, Don Zickus wrote:
> 
> > I have added some in-depth analysis from our customer.
> > 
> > 
> > The problem is that the failing routine was called with a pointer to usbdev 
> > in
> > memory that has already been freed and overwritten with the pool poison 
> > pattern.
> > This causes an access violation on line 502:
> 
> The stack trace shows that the failure occurred in usb_device_dump(), 
> which was called from usb_device_read().  Since it wasn't a recursive 
> call, the usbdev argument must have pointed to the root-hub device.
> 
> I don't see how this could have happened if the kernel included the 
> patch I sent earlier.  usb_device_read() holds the usb_bus_list_lock 
> mutex throughout, and it tests that the root hub's devnum field is not 
> equal to 0 before calling usb_dump_device().
> 
> Meanwhile, usb_remove_hcd() sets hcd->self.bus.root_hub->devnum to 0
> while holding usb_bus_list_lock, and it doesn't deallocate the root hub
> device until after the mutex is dropped.
> 
> Are you certain that the test was conducted with the patch in place?
> 
> If you want to track down what's going wrong, you'll have to add some 
> debugging code to usb_device_read() and usb_remove_hcd().  By the time 
> usb_device_dump() starts running, it's already too late.

After thinking about this some more, I realized that my patch still 
leaves a race -- although the oops would occur in a different place 
(where usb_device_read checks bus->root_hub->devnum).

Here's a different patch which should work better.  It relies on the
rh_registered flag in the usb_hcd structure, which persists as long as
the usb_bus structure does, rather than on anything stored in the
root-hub device structure.

Alan Stern



Index: usb-3.6/drivers/usb/core/devices.c
===
--- usb-3.6.orig/drivers/usb/core/devices.c
+++ usb-3.6/drivers/usb/core/devices.c
@@ -624,7 +624,7 @@ static ssize_t usb_device_read(struct fi
/* print devices for all busses */
list_for_each_entry(bus, &usb_bus_list, bus_list) {
/* recurse through all children of the root hub */
-   if (!bus->root_hub)
+   if (!bus_to_hcd(bus)->rh_registered)
continue;
usb_lock_device(bus->root_hub);
ret = usb_device_dump(&buf, &nbytes, &skip_bytes, ppos,
Index: usb-3.6/drivers/usb/core/hcd.c
===
--- usb-3.6.orig/drivers/usb/core/hcd.c
+++ usb-3.6/drivers/usb/core/hcd.c
@@ -1011,10 +1011,7 @@ static int register_root_hub(struct usb_
if (retval) {
dev_err (parent_dev, "can't register root hub for %s, %d\n",
dev_name(&usb_dev->dev), retval);
-   }
-   mutex_unlock(&usb_bus_list_lock);
-
-   if (retval == 0) {
+   } else {
spin_lock_irq (&hcd_root_hub_lock);
hcd->rh_registered = 1;
spin_unlock_irq (&hcd_root_hub_lock);
@@ -1023,6 +1020,7 @@ static int register_root_hub(struct usb_
if (HCD_DEAD(hcd))
usb_hc_died (hcd);  /* This time clean up */
}
+   mutex_unlock(&usb_bus_list_lock);
 
return retval;
 }

--
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 v1 2/2] USB: set hub's default autosuspend delay as 0

[Alan Stern]

On Tue, 25 Sep 2012, Ming Lei wrote:

> IMO, it should be reasonable to set the device's autosuspend delay
> as zero when its driver is unbound because no one can predict when
> the device will be bound to a new driver.

It could be set to a low value, but it shouldn't be set to 0.  There 
aren't too many things you can do with an unbound device; lsusb is one 
of the most common.

On the other hand, we already have a usbcore module parameter for 
specifying the default autosuspend delay.  That's what the delay is set 
to when a device is first initialized, before it is bound to anything.  
Unfortunately the parameter's value is in seconds, not milliseconds, so 
we can't set it to any positive value smaller than 1 second.

Alan Stern

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

Re: [PATCH] usb/core: Fix race condition when removing EHCI PCI devices

[Don Zickus]

On Mon, Sep 24, 2012 at 12:59:51PM -0400, Alan Stern wrote:
> On Mon, 17 Sep 2012, Alan Stern wrote:
> 
> > On Mon, 17 Sep 2012, Don Zickus wrote:
> > 
> > > I have added some in-depth analysis from our customer.
> > > 
> > > 
> > > The problem is that the failing routine was called with a pointer to 
> > > usbdev in
> > > memory that has already been freed and overwritten with the pool poison 
> > > pattern.
> > > This causes an access violation on line 502:
> > 
> > The stack trace shows that the failure occurred in usb_device_dump(), 
> > which was called from usb_device_read().  Since it wasn't a recursive 
> > call, the usbdev argument must have pointed to the root-hub device.
> > 
> > I don't see how this could have happened if the kernel included the 
> > patch I sent earlier.  usb_device_read() holds the usb_bus_list_lock 
> > mutex throughout, and it tests that the root hub's devnum field is not 
> > equal to 0 before calling usb_dump_device().
> > 
> > Meanwhile, usb_remove_hcd() sets hcd->self.bus.root_hub->devnum to 0
> > while holding usb_bus_list_lock, and it doesn't deallocate the root hub
> > device until after the mutex is dropped.
> > 
> > Are you certain that the test was conducted with the patch in place?
> > 
> > If you want to track down what's going wrong, you'll have to add some 
> > debugging code to usb_device_read() and usb_remove_hcd().  By the time 
> > usb_device_dump() starts running, it's already too late.
> 
> After thinking about this some more, I realized that my patch still 
> leaves a race -- although the oops would occur in a different place 
> (where usb_device_read checks bus->root_hub->devnum).
> 
> Here's a different patch which should work better.  It relies on the
> rh_registered flag in the usb_hcd structure, which persists as long as
> the usb_bus structure does, rather than on anything stored in the
> root-hub device structure.

Thanks!  I will pass this on. Sorry for the lack of feedback.  The
customer I am coordinating with was on vacation last week.  I am still
trying to set up their machine on my end to help debug this faster.

I appreciate you still thinking about this.

Cheers,
Don

> 
> Alan Stern
> 
> 
> 
> Index: usb-3.6/drivers/usb/core/devices.c
> ===
> --- usb-3.6.orig/drivers/usb/core/devices.c
> +++ usb-3.6/drivers/usb/core/devices.c
> @@ -624,7 +624,7 @@ static ssize_t usb_device_read(struct fi
>   /* print devices for all busses */
>   list_for_each_entry(bus, &usb_bus_list, bus_list) {
>   /* recurse through all children of the root hub */
> - if (!bus->root_hub)
> + if (!bus_to_hcd(bus)->rh_registered)
>   continue;
>   usb_lock_device(bus->root_hub);
>   ret = usb_device_dump(&buf, &nbytes, &skip_bytes, ppos,
> Index: usb-3.6/drivers/usb/core/hcd.c
> ===
> --- usb-3.6.orig/drivers/usb/core/hcd.c
> +++ usb-3.6/drivers/usb/core/hcd.c
> @@ -1011,10 +1011,7 @@ static int register_root_hub(struct usb_
>   if (retval) {
>   dev_err (parent_dev, "can't register root hub for %s, %d\n",
>   dev_name(&usb_dev->dev), retval);
> - }
> - mutex_unlock(&usb_bus_list_lock);
> -
> - if (retval == 0) {
> + } else {
>   spin_lock_irq (&hcd_root_hub_lock);
>   hcd->rh_registered = 1;
>   spin_unlock_irq (&hcd_root_hub_lock);
> @@ -1023,6 +1020,7 @@ static int register_root_hub(struct usb_
>   if (HCD_DEAD(hcd))
>   usb_hc_died (hcd);  /* This time clean up */
>   }
> + mutex_unlock(&usb_bus_list_lock);
>  
>   return retval;
>  }
> 
--
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 PATCH 0/3] usbnet: support runtime PM triggered by link change

[Ming Lei]

On Thu, Sep 20, 2012 at 4:30 PM, Bjørn Mork  wrote:
> Ming Lei  writes:
>> On Mon, Sep 17, 2012 at 4:04 PM, Oliver Neukum  wrote:
>>
>>> 1) Does it actually save power? You are constantly waking up a CPU.
>>
>> Of course, it does. I don't know it will save how much power just on
>> usbnet device, but it may save power from usb hub and usb host
>> controller in the bus at least.
>>
>> Anyway we don't need to waste power if the link of usbnet is down.
>>
>> It just wake up CPU one time each 3sec. In modern linux distribution,
>> the CPU will be wakup tens times per second, so it shouldn't be a
>> big problem.
>
> I do not buy that constantly polling a device necessarily saves any
> power compared to keeping the USB link to the device alive.  You need to
> measure the savings if you want us to believe that.

Yes, good suggestion.

>
> You are not only waking the host CPU.  You are also waking the device
> CPU.

Some usbnet devices doesn't have CPU, and I don't think there are
one in smsc95xx or asix, :-)

>
> Any usbnet device will consist of more than one building block, having
> at least a network block, a USB block and a CPU.  For all you know, the
> device CPU might be in deep sleep until it has to service either the
> network or USB block, and those might also be sleeping until they see an
> interrupt.  Constantly polling the device to receive network link status
> might use considerably more power than keeping the USB link up waiting
> for a link notification.
>
> If you were to implement this feature anyway, then I would prefer a
> userspace based approach instead.  I see no reason to keep the timer in
> the kernel.  You could decide to suspend whenever the netdev is down,
> and only poll the link when userspace tries to bring up the netdev.

I am wondering if userspace knows when it should bring up the interface,
maybe it depends on the link becomes up, maybe udev always bring it up.

> That would let a userspace daemon implement the same feature by trying
> to bring up the netdev every 3 seconds (or whatever frequency the user
> selected).  And it would allow me to avoid the polling until I know I
> have plugged in a cable.

I don't see any advantage by using a userspace timer, and it will convert to
a kernel timer finally.

Also putting the interface down may introduce some side effect on user
space: the IP address may be lost, etc.

>
>>> From that perspective it is possible that leaving on the ethernet is 
>>> actually
>>> better in terms of power. Only measurements can answer that question.
>>
>> You know it is a bit difficult to test power save for this situation. And
>> most of runtime PM patches didn't provide power save data. In fact,
>> I'd like to do it, but I have not some equipment to measure it, :-(.
>
> We don't know, you don't know.  But you claim that your change saves
> power, so please provide some documentation showing that it does.

The motivation of the patch is that suspending the usbnet device may put
all usb devices in the bus into suspend.

I am trying to get some data by powertop, but looks it is not reliable enough.

I even found that the discharge rate of battery when the asix is disconnected
and let the whole bus suspend is even more than when the asix is connected
with ethernet cable link down and all devices in the whole usb bus are active.
There is about 1w difference, so odd.

I will try to get some correct power data.

>
>>> 2) Do we have many devices that would be serviced with this approach?
>>
>> At least I found asix can be serviced by this approach. Considered asix
>> is quite popular, it is worthy of the effort. Also the below devices can be
>> serviced by the patch in theory:
>>
>>dm9601.c / mcs7830.c / sierra_net.c
>
> The sierra_net.c driver is only used for wireless devices AFAIK. I
> really don't see the use case for network link based resume of that.
> There is no cable to plug.  Userspace will have to initiate a
> connection.

Wireless link is still one kind of link, :-)

Otherwise, why sierra_net.c bother to use netif_carrier_on?

>
> And the DirectIP device I've got (MC7710) supports remote wakeup.  I
> assume that will be the case for all such devices, given that this is
> mostly a firmware feature. So the correct fix for sierra_net.c is to add
> support for remote wakeup.  Then you will be able to suspend the device
> regardless of whether the link is up or down, without the constant
> polling.

In fact, the patch supports link change via remote wakeup, and it may be
useful if remote wakeup is not supported by incoming packet.

Not mention the power save data, the patch is not mature enough, for example,
ethtool_ops interface may access a suspended device.

I will update it if some progresses are got.

Thanks,
--
Ming Lei
--
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: XHCI: URB not cancelled during disconnect of a MSC device

[Ajay Gupta]

Hi,

> Ajay, which host controller are you seeing the disconnect lags on?  Can you
> send me the output of `sudo lspci -vvv -n`?

I was on leave last week so will  get the data at earliest.

Ajay
> 
> Sarah Sharp
> --
> 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
---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
--
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 v1 2/2] USB: set hub's default autosuspend delay as 0

[Ming Lei]

On Tue, Sep 25, 2012 at 1:06 AM, Alan Stern  wrote:
> On Tue, 25 Sep 2012, Ming Lei wrote:
>
>> IMO, it should be reasonable to set the device's autosuspend delay
>> as zero when its driver is unbound because no one can predict when
>> the device will be bound to a new driver.
>
> It could be set to a low value, but it shouldn't be set to 0.  There

If so, I'd rather to keep the default 2 seconds. Changing it to zero can
avoid one auto suspend timer which may increase CPU power usage.

> aren't too many things you can do with an unbound device; lsusb is one
> of the most common.

Considered 'lsusb' is a low frequency tool for final users, so could we just
ignore it for the case?

>
> On the other hand, we already have a usbcore module parameter for
> specifying the default autosuspend delay.  That's what the delay is set
> to when a device is first initialized, before it is bound to anything.
> Unfortunately the parameter's value is in seconds, not milliseconds, so
> we can't set it to any positive value smaller than 1 second.

It is a global and can't be used to just change on unbound device.

Thanks,
--
Ming Lei
--
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 v1 1/2] USB: check port changes before hub runtime suspend for some bug device

[Ming Lei]

On Tue, Sep 25, 2012 at 12:42 AM, Alan Stern  wrote:
> On Mon, 24 Sep 2012, Ming Lei wrote:
>
>> On Mon, Sep 24, 2012 at 12:09 AM, Alan Stern  
>> wrote:
>>
>> > No, you can still handle the other case.  You need to report the
>> > condition to the PM core, say by calling pm_wakeup_event().
>>
>> Good point, so kind of below code should be added to usb_suspend_both
>> to handle the situation centralizedly:
>>
>> if (!PMSG_IS_AUTO(msg)) {
>>  if (hdev->do_remote_wakeup && status == -EBUSY)
>>   pm_wakeup_event(&hdev->dev, timeout)
>>  status = 0;
>> }
>
> No.  Drivers can return -EBUSY for other reasons besides a pending
> wakeup.  They will have to handle wakeups by themselves.

If so, almost all drivers for usb devices with remote wakeup capability need
this change, could we figure out one way to handle it centralizedly?

Thanks,
--
Ming Lei
--
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: libusbx-1.0.13 has been released

[Greg KH]

On Mon, Sep 24, 2012 at 07:59:40PM +0300, Kustaa Nyholm wrote:
> On 24.9.2012 18.03, "Greg KH"  wrote:
> >libusbx as it's developers really don't care about their users.
> 
> A bit richt, I think, to say that libusbx developers do not care
> about their users.

I don't think so.  Remember, usbutils is the _one_ libusb package that
everyone has on their system.  The fact that the libusbx release wasn't
tested with that package makes me wonder how it was tested at all.

And to have distros start to blame _me_ for usbutils breaking because
libusbx changed a field of a structure in "stable" release cycle (I say
stable as I don't think they bumped the .so name of the library, did
they?)  That's acting pretty rude, don't you think?

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

Re: [PATCH v3] powerpc/usb: fix bug of CPU hang when missing USB PHY clock

[Greg KH]

On Mon, Sep 24, 2012 at 02:44:19PM +, Liu Shengzhou-B36685 wrote:
> 
> > -Original Message-
> > From: Greg KH [mailto:g...@kroah.com]
> > Sent: 2012年9月22日 22:49
> > To: Kumar Gala
> > Cc: Liu Shengzhou-B36685; linuxppc-...@lists.ozlabs.org; linux-
> > u...@vger.kernel.org
> > Subject: Re: [PATCH v3] powerpc/usb: fix bug of CPU hang when missing
> > USB PHY clock
> > 
> > On Sat, Sep 22, 2012 at 09:39:15AM -0500, Kumar Gala wrote:
> > >
> > > On Sep 21, 2012, at 11:43 AM, Greg KH wrote:
> > >
> > > > On Tue, Sep 18, 2012 at 04:52:39PM +0800, Shengzhou Liu wrote:
> > > >> when missing USB PHY clock, kernel booting up will hang during USB
> > > >> initialization. We should check USBGP[PHY_CLK_VALID] bit to avoid
> > > >> CPU hanging in this case.
> > > >>
> > > >> Signed-off-by: Shengzhou Liu 
> > > >> ---
> > > >> v3 change: no check for UTMI PHY.
> > > >> v2 change: use spin_event_timeout() instead.
> > > >>
> > > >> drivers/usb/host/ehci-fsl.c |   57 +--
> > ---
> > > >> drivers/usb/host/ehci-fsl.h |1 +
> > > >> include/linux/fsl_devices.h |1 +
> > > >> 3 files changed, 41 insertions(+), 18 deletions(-)
> > > >
> > > > This is already applied, right?
> > > >
> > > > greg k-h
> > >
> > > It appears that v2 of the patch is applied to your usb-next branch.
> > >
> > > in drivers/usb/host/ehci-fsl.c
> > >
> > > V2:
> > > @@ -262,23 +266,34 @@ static void ehci_fsl_setup_phy(struct usb_hcd
> > *hcd,
> > > case FSL_USB2_PHY_NONE:
> > > break;
> > > }
> > > +
> > > +   if ((pdata->controller_ver) && ((phy_mode ==
> > FSL_USB2_PHY_ULPI) ||
> > > +   (phy_mode == FSL_USB2_PHY_UTMI))) {
> > >
> > > V3:
> > >
> > > @@ -262,23 +266,33 @@  static void ehci_fsl_setup_phy(struct usb_hcd
> > *hcd,
> > >
> > >   case FSL_USB2_PHY_NONE:
> > >   break;
> > >   }
> > >
> > > +
> > > + if (pdata->controller_ver && (phy_mode == FSL_USB2_PHY_ULPI)) {
> > > + /* check PHY_CLK_VALID to get phy clk valid */
> > 
> > Ok, can someone please make the incremental patch that I need to apply
> > here and send it to me?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi greg,
> 
> I have sent the below incremental patch to you, please squash it into the 
> previous patch v2, which had been applied in your usb-next branch. 
> http://patchwork.ozlabs.org/patch/186443/

Given that this is a git tree, "squashing" patches does not work at all.
I'll just apply it as-is.

thanks,

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

Re: libusbx-1.0.13 has been released

[Kustaa Nyholm]

On 24.9.2012 18.03, "Greg KH"  wrote:
>libusbx as it's developers really don't care about their users.

A bit richt, I think, to say that libusbx developers do not care
about their users.

To be constructive can you name the broken programs (in addition
to the already mentioned 'usbutils' ), so that everyone can asses
the magnitude of the problem.

For the record, I'm not one of the developers.

Kusti

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

Re: [PATCH] USB: ohci-at91: fix null pointer in ohci_hcd_at91_overcurrent_irq

[Greg KH]

On Mon, Sep 24, 2012 at 09:44:31AM +0200, Nicolas Ferre wrote:
> > This patches fixed the issue by adding the gpio valid check.
> > 
> > Signed-off-by: Joachim Eastwood 
> 
> Acked-by: Nicolas Ferre 
> 
> Thanks a *lot* Joachim for having chased this bug!
> 
> Alan, Greg, this one is 3.6 last minute material... I Hope that it can
> make it!
> 
> > ---
> > 
> > Note: If 6fffb77c goes into stable please either include this patch also or
> > fold it into 6fffb77c to avoid regressions.
> 
> Absolutely, I have tagged the faulty patch:
> Cc: Stable  [3.4+]
> 
> So it can be good if this one can follow the same path.

I'll queue this up for 3.7 and have it backported to 3.6.1 as it's a bit
late for 3.6 for me right now.

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

Re: USB port power off discussion

[Greg KH]

On Mon, Sep 24, 2012 at 03:18:19PM +0200, Rafael J. Wysocki wrote:
> On Saturday, September 22, 2012, Rafael J. Wysocki wrote:
> > On Friday, September 21, 2012, Sarah Sharp wrote:
> > > Two weeks ago at Linux Plumbers Conference, I presented about the Intel
> > > Lynx Point USB port power off mechanism.  This email is a report out of
> > > what was discussed, and a kick off point for further discussion.
> 
> [...]
> 
> > > ca9c9d0 usb : Add sysfs files to control port power.
> > 
> > http://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=commit;h=ca9c9d0
> 
> I have reviewed the whole series (I suppose there's too late for adding
> Reviewed-by tags to it, but oh well ;-)) and the only patch I have some 
> serious
> doubts about is the one above.  In fact, I'd prefer it to be reverted for now.

Ok, I can revert that one, I'd rather hold off on any user api changes
like this if people have reservations about them.  It's harder to revert
them later on than it is now.

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

Re: usb3 fails to write when using usb3 hub in usb3 port

[Sarah Sharp]

On Mon, Sep 24, 2012 at 06:34:51PM +0300, Adrian Sandu wrote:
> > You should use Reply-To-All so that your messages get sent to the
> > mailing list as well as to me.
> 
> Will do from now on :)
> 
> >
> > In the new file you wrote: "launch cat of 9u, start copying a file to
> > /dev/sdb1 ( using mc for example )".  Where was this file stored?  Was
> > it on /dev/sdd1?
> >
> > My reason for asking is because the failure did not occur during a
> > write to /dev/sdb; it occurred during a read from /dev/sdd.  There is
> > no apparent reason for the failure -- following a request for 122880
> > bytes of data, the drive sent 2048 bytes and then stopped sending.
> >
> > The computer successfully reset the drive and asked for the same 122880
> > bytes again.  The drive failed to respond for 30 seconds, so the
> > computer tried to reset the drive again.  This time, repeated resets
> > all failed.  That's why the drive no longer showed up in the lsusb
> > output.
> >
> > It's hard to say exactly where the fault lies, but my guess is with the
> > drive or its USB interface.
> 
> Will try again in about 2 - 3 hours with only 1 drive connected to the
> hub so there won't be any more confussion.
> 
> All drives work just fine when connected directly to the usb ports in
> the asrock. They also work just fine ( it seems ) when connected
> through the usb hub to my laptop. And as I said, both have the same
> usb3 hub chip that's why I think it's weird.

You're not the only one that's having issues with USB hard drives behind
VIA hubs.  I've seen a similar report from one of my co-workers who was
testing on an Intel chipset.

I've actually been wondering if the USB 3.0 Link PM is to blame here.
Perhaps either the USB device or the VIA hub doesn't handle enabling U1
link PM very well.  Since your device works under the roothub, I think
the VIA hub might be to blame.

I'll whip up some patches that allow you to disable USB 3.0 Link PM for
the VIA hub and we'll see if that helps.  Or you could downgrade to the
3.4 kernel and see if that helps, since USB 3.0 Link PM went into 3.5.

Sarah Sharp
--
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: usb3 fails to write when using usb3 hub in usb3 port

[Adrian Sandu]

> I'll whip up some patches that allow you to disable USB 3.0 Link PM for
> the VIA hub and we'll see if that helps.  Or you could downgrade to the
> 3.4 kernel and see if that helps, since USB 3.0 Link PM went into 3.5.

Thanks Sarah. I tried in 3.4.11 to .. about the same thing ..
http://d3xt3r01.tk/~dexter/usbmon.trace.txt contains a new trace.

1 drive plugged in the usb hub, connected to the asrock.

dexter@d3xt3r01 ~ $ cp -v /media/sdd1/en ~
‘/media/sdd1/en’ -> ‘/home/dexter/en’
cp: reading ‘/media/sdd1/eno’: Input/output error
cp: failed to extend ‘/home/dexter/en’: Input/output error



2012-09-24T21:08:35.407758+03:00 d3xt3r01 kernel: [  227.308783]
EXT4-fs (sdb1): recovery complete
2012-09-24T21:08:35.408911+03:00 d3xt3r01 kernel: [  227.309606]
EXT4-fs (sdb1): mounted filesystem with ordered data mode. Opts:
(null)
2012-09-24T21:08:57.192369+03:00 d3xt3r01 kernel: [  249.092067] usb
2-2.2: reset SuperSpeed USB device number 5 using xhci_hcd
2012-09-24T21:08:57.203739+03:00 d3xt3r01 kernel: [  249.104503] usb
2-2.2: Parent hub missing LPM exit latency info.  Power management
will be impacted.
2012-09-24T21:08:57.204902+03:00 d3xt3r01 kernel: [  249.105266]
xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep
88010b35b980
2012-09-24T21:08:57.204913+03:00 d3xt3r01 kernel: [  249.105277]
xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep
88010b35b9c0
2012-09-24T21:09:32.386789+03:00 d3xt3r01 kernel: [  284.287436] hub
2-2:1.0: Cannot enable port 2.  Maybe the USB cable is bad?
2012-09-24T21:09:32.913454+03:00 d3xt3r01 kernel: [  284.813228] sd
9:0:0:0: Device offlined - not ready after error recovery
2012-09-24T21:09:32.913478+03:00 d3xt3r01 kernel: [  284.813248] sd
9:0:0:0: [sdb] Unhandled error code
2012-09-24T21:09:32.913487+03:00 d3xt3r01 kernel: [  284.813254] sd
9:0:0:0: [sdb]
2012-09-24T21:09:32.913492+03:00 d3xt3r01 kernel: [  284.813259]
Result: hostbyte=DID_ABORT driverbyte=DRIVER_OK
2012-09-24T21:09:32.913496+03:00 d3xt3r01 kernel: [  284.813266] sd
9:0:0:0: [sdb] CDB:
2012-09-24T21:09:32.913501+03:00 d3xt3r01 kernel: [  284.813270]
Read(10): 28 00 00 07 a9 80 00 00 f0 00
2012-09-24T21:09:32.913505+03:00 d3xt3r01 kernel: [  284.813289]
end_request: I/O error, dev sdb, sector 502144
2012-09-24T21:09:32.913509+03:00 d3xt3r01 kernel: [  284.813337] sd
9:0:0:0: rejecting I/O to offline device
2012-09-24T21:09:32.913514+03:00 d3xt3r01 kernel: [  284.813347] sd
9:0:0:0: [sdb] killing request
2012-09-24T21:09:32.913518+03:00 d3xt3r01 kernel: [  284.813362] sd
9:0:0:0: rejecting I/O to offline device
2012-09-24T21:09:32.913522+03:00 d3xt3r01 kernel: [  284.813382] sd
9:0:0:0: rejecting I/O to offline device
2012-09-24T21:09:32.913526+03:00 d3xt3r01 kernel: [  284.813399] sd
9:0:0:0: rejecting I/O to offline device
2012-09-24T21:09:32.913530+03:00 d3xt3r01 kernel: [  284.813428] sd
9:0:0:0: [sdb] Unhandled error code
2012-09-24T21:09:32.913534+03:00 d3xt3r01 kernel: [  284.813435] sd
9:0:0:0: [sdb]
2012-09-24T21:09:32.913539+03:00 d3xt3r01 kernel: [  284.813439]
Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK
2012-09-24T21:09:32.913543+03:00 d3xt3r01 kernel: [  284.813446] sd
9:0:0:0: [sdb] CDB:
2012-09-24T21:09:32.913546+03:00 d3xt3r01 kernel: [  284.813449]
Read(10): 28 00 00 07 aa 70 00 00 10 00
2012-09-24T21:09:32.913550+03:00 d3xt3r01 kernel: [  284.813466]
end_request: I/O error, dev sdb, sector 502384
2012-09-24T21:09:32.929259+03:00 d3xt3r01 kernel: [  284.829046] usb
2-2.2: USB disconnect, device number 5

The drive died ( as in shut down .. I have to manually plug it out and
back in the power outlet now )...
--
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: libusbx-1.0.13 has been released

[Hans de Goede]

Hi,

On 09/24/2012 05:03 PM, Greg KH wrote:

On Thu, Sep 20, 2012 at 10:42:10PM +0100, Pete Batard wrote:

Hi,

It with pleasure that I would like to announce the release of
libusbx 1.0.13. This version brings the following notable changes:

* [MAJOR] Fix a typo in the API with struct libusb_config_descriptor
where MaxPower was used instead of bMaxPower, as per the specs.
   If your application was accessing the MaxPower attribute, and you
need to maintain compatibility with libusb or older versions, please
see APPENDIX A below.


So, you broke compatibility with existing programs, like usbutils.
That's just foolish and dumb.  I have loads of distros asking me about
this now.


First of all I'm sorry to hear that the MaxPower change has caused issues
for usbutils.

We've discussed this change on the libusbx-devel list in detail and only
one API user using the MaxPower field was identified and the author of
that was ok with the change.


Please put the work-around in the library if you really want to change
this field name,


We've considered several library level workarounds and non were deemed
really workable. Note that I was not the author of this specific change,
and that I've voiced concerns about the API breakage this entailed, but
since there seemed to be almost no users of the field and Pete felt
strong about fixing its name I went along.


otherwise, I'm going to have to recommend that no one
use libusbx as it's developers really don't care about their users.


That is a bit harsh, we do care, I would like to think that we care a
lot more then the original libusb which did not deem their users
worthy of a release for a period for more then two years.

Once more sorry for the work this is causing you, but the decision has
been made (for better or worse) and I don't think that reversing it
in a later release is going to do us any good ...

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: libusbx-1.0.13 has been released

[Pete Batard]

On 2012.09.24 18:24, Greg KH wrote:

I don't think so.  Remember, usbutils is the _one_ libusb package that
everyone has on their system.  The fact that the libusbx release wasn't
tested with that package makes me wonder how it was tested at all.


Greg,

We made the conscious decision to potentially break applications in 
order to stick to the USB specs exactly. We feel that an USB library 
that doesn't follow the specs to the letter is less trustworthy than one 
that does, and that keeping an erroneous attribute for historical reason 
it too much of a risk, as it creates a precedent for acceptable 
deviation from the specs.


If you want a fix for usbutils, I'll send you one. And no, libusbx 
wasn't tested against usbutils, though it doesn't matter here, since 
we're not discovering a problem: we did anticipate breakage with 
software that relied on MaxPower. And by the way, since our testing 
resources are limited and we have quite a few platforms to support 
besides Linux, I might as well take this opportunity to ask anyone who 
think that they could lend a hand improve testing against RCs to join 
the libusbx mailing list.


But the problem really is that libusb (and libusbx prior to v1.0.13) has 
a typo that makes it deviate from the USB specs, which left uswith 3 
choices:
1. leave the typo as is, and say "It's fine to deviate from the USB 
specs for historical reasons, even if we know our descriptor structure 
is wrong"
2. allow both MaxPower and bMaxPower to be used simultaneously, which we 
explored for a while, through anonymous structs, but decided against, 
especially as it made part of #1 still apply.
3. Treat the problem as we would treat an errata from the USB specs, and 
declare the old way of accessing the Max Power attribute erroneous and 
requiring immediate fixing.


To that extent, let me ask you this: if this was a typo from the specs 
rather than libusb/libusbx, and an errata was issued by the USB 
committee, would you not want to apply the errata, even if that meant 
possible breakage of existing apps?


Unfortunately, mistakes do happen. We screwed up the header (well, 
libusb did, but we'll share the blame just as well) because MaxPower 
should never ever have been used there. Therefore we decided to treat it 
exactly as we would treat an errata from the specs and fix it by 
declaring the old way obsolete (though we did keep some provisions to 
keep using it). And yes, we're sorry about the fact that it can make 
applications accessing (b)MaxPower break, including prominent ones, but 
then again, we also expect applications to break when specs erratas are 
issued and applied. Therefore we considered that applications that are 
expected to have the flexibility to handle erratas from the specs should 
have enough flexibility to apply "erratas" from libusbx.
And we also think that erratas should be applied as soon as they are 
detected.



And to have distros start to blame _me_ for usbutils breaking because
libusbx changed a field of a structure in "stable" release cycle (I say
stable as I don't think they bumped the .so name of the library, did
they?)  That's acting pretty rude, don't you think?


Bumping to 1.1 was an option we considered (and that I actually would 
have preferred as well), but decided against it as we saw the fix as 
very straightforward, and we also tried to made sure it was prominently 
well documented. We introduced an API_VERSION macro for the sake 
allowing applications using the newer libusbx to also compile and run 
against old versions of libusbx or libusb if needed.


Once again, please understand that what decides the content of the 
libusbx library and its header is the USB specs, and if we find a 
deviation that we can address that's what we'll do. We also made the 
decision to try doing that sooner, when fewer applications were expected 
to access MaxPower, as we think it will be even more painful to leave 
that for later.


Regards,

/Pete
--
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: [Libusbx-devel] libusbx-1.0.13 has been released

[Hans de Goede]

Hi Greg,

On 09/24/2012 05:07 PM, Greg KH wrote:

On Mon, Sep 24, 2012 at 03:52:55PM +0200, Hans de Goede wrote:

Hi,

On 09/20/2012 11:42 PM, Pete Batard wrote:

Hi,

It with pleasure that I would like to announce the release of libusbx
1.0.13. This version brings the following notable changes:


The Fedora packages for libusbx have been upgraded to 1.0.13 now.


How did you handle the usbutils breakage?


I did not handle it at all, as I was not aware of it. Now that I'm I'll
file a bug against usbutils and advice the maintainer to add the
necessary #ifdef-ery to keep it building.

Note that other then breaking building from source, this does not impact
end users at all, as the ABI is unchanged. We have been very careful to
keep the ABI 100% compatible and I can guarantee you that we will keep
doing that in the future!

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: xhci: Etron XHCI_TRUST_TX_LENGTH quirk

[Matthew Hall]

Hi,

Is there anything else I could do to help figure this out?

Perhaps usbmon on some specific transactions or something?

Thanks,
Matthew.

> > On Wed, Aug 08, 2012 at 02:03:14PM -0700, Sarah Sharp wrote:
> > > Hi Matthew,
> > > 
> > > This patch does fix the error that Gary reported.  And even though it
> > > doesn't completely fix your issue, it did make the same error messages
> > > Gary was seeing go away.
> > > 
> > > I'll still be looking at your logs, but I'm leaving on vacation this
> > > Friday, so I'm not sure if we'll have time to solve your issue before I
> > > leave.  I'll be back Sept 4th.
> > > 
> > > Sarah Sharp
> > > 
> > > On Wed, Aug 08, 2012 at 12:44:39PM -0700, Matthew Hall wrote:
> > > > Hi Sarah,
> > > > 
> > > > If I am reading right, this patch is not any different from the one we 
> > > > were 
> > > > trying to get my card reader to work on USB 3.0.
> > > > 
> > > > I did some further testing last with the same card, reader, and exfat 
> > > > driver 
> > > > on a USB 2.0 port on the same machine and everything worked with no 
> > > > errors in 
> > > > dmesg, when I cped 17GB of MTS film data off the card.
> > > > 
> > > > This makes me think there must be some additional problem with the 
> > > > controller 
> > > > which needs to get worked around or resolved before this controller is 
> > > > really 
> > > > going to work.
> > > > 
> > > > Regards,
> > > > Matthew.
> > --
> > 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
> --
> 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
--
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: libusbx-1.0.13 has been released

[Greg KH]

On Mon, Sep 24, 2012 at 07:33:08PM +0100, Pete Batard wrote:
> But the problem really is that libusb (and libusbx prior to v1.0.13)
> has a typo that makes it deviate from the USB specs, which left
> uswith 3 choices:
> 1. leave the typo as is, and say "It's fine to deviate from the USB
> specs for historical reasons, even if we know our descriptor
> structure is wrong"

Yes, you don't break existing applications, do you?

> 2. allow both MaxPower and bMaxPower to be used simultaneously,
> which we explored for a while, through anonymous structs, but
> decided against, especially as it made part of #1 still apply.

Why?  It was a mistake, live with it, and provide a chance for programs
to adapt.  To just force everyone to handle this now, without a major
api change, is not ok from a library developer perspective.

Why not just do this?  That way all existing programs continue to work,
and you can provide a way that programs can, without changing, still
work properly with libusb and older library versions.  Isn't that your
goal as a library developer?

> 3. Treat the problem as we would treat an errata from the USB specs,
> and declare the old way of accessing the Max Power attribute
> erroneous and requiring immediate fixing.
> 
> To that extent, let me ask you this: if this was a typo from the
> specs rather than libusb/libusbx, and an errata was issued by the
> USB committee, would you not want to apply the errata, even if that
> meant possible breakage of existing apps?

No, I wouldn't break an API that has been used by programs.  Heck, even
if you didn't think it was used, it's part of your public API, and that
is not something, as a library developer, you can break without a .so
name change as well.

> Unfortunately, mistakes do happen. We screwed up the header (well,
> libusb did, but we'll share the blame just as well) because MaxPower
> should never ever have been used there. Therefore we decided to
> treat it exactly as we would treat an errata from the specs and fix
> it by declaring the old way obsolete (though we did keep some
> provisions to keep using it). And yes, we're sorry about the fact
> that it can make applications accessing (b)MaxPower break, including
> prominent ones, but then again, we also expect applications to break
> when specs erratas are issued and applied. Therefore we considered
> that applications that are expected to have the flexibility to
> handle erratas from the specs should have enough flexibility to
> apply "erratas" from libusbx.
> And we also think that erratas should be applied as soon as they are
> detected.
> 
> >And to have distros start to blame _me_ for usbutils breaking because
> >libusbx changed a field of a structure in "stable" release cycle (I say
> >stable as I don't think they bumped the .so name of the library, did
> >they?)  That's acting pretty rude, don't you think?
> 
> Bumping to 1.1 was an option we considered (and that I actually
> would have preferred as well), but decided against it as we saw the
> fix as very straightforward, and we also tried to made sure it was
> prominently well documented. We introduced an API_VERSION macro for
> the sake allowing applications using the newer libusbx to also
> compile and run against old versions of libusbx or libusb if needed.

No, you are forcing me to change my program to have it build with your
change, you should do it the other way around.  If you want programs to
use your library, make it so that they don't have to be changed at all.

And if I'm going to be forced to change my program, and libusbx has now
shown that they don't care about their public api, well, I might as well
just rewrite it to remove that dependancy completly, as it's obvious
they don't know how to treat their users.

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

Re: [Libusbx-devel] libusbx-1.0.13 has been released

[Greg KH]

On Mon, Sep 24, 2012 at 08:36:11PM +0200, Hans de Goede wrote:
> Hi Greg,
> 
> On 09/24/2012 05:07 PM, Greg KH wrote:
> >On Mon, Sep 24, 2012 at 03:52:55PM +0200, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 09/20/2012 11:42 PM, Pete Batard wrote:
> >>>Hi,
> >>>
> >>>It with pleasure that I would like to announce the release of libusbx
> >>>1.0.13. This version brings the following notable changes:
> >>
> >>The Fedora packages for libusbx have been upgraded to 1.0.13 now.
> >
> >How did you handle the usbutils breakage?
> 
> I did not handle it at all, as I was not aware of it. Now that I'm I'll
> file a bug against usbutils and advice the maintainer to add the
> necessary #ifdef-ery to keep it building.

So, you are going to force me (hint, I'm the usbutils maintainer), to
change my code because libusbx broke their API here?  That's what other
distros have already tried to tell me earlier today, and I'm going to
push back hard and say that it is a bug in libusbx instead.

> Note that other then breaking building from source,

Hint, that's a huge thing, as that is exactly who the users of a library
are.  If I can't build against the library, how can I use it at run-time
(ok, yes, you can drop it in later, but really, that's just foolish...)

> this does not impact end users at all, as the ABI is unchanged. We
> have been very careful to keep the ABI 100% compatible and I can
> guarantee you that we will keep doing that in the future!

Um, no, you just broke the API, my inbox is proof of that.  All the
distros just reported this to me, and as such, it's an API change in
your library, not anything I should have to do in my code.

Please fix this in libusbx, or bump the .so name so that tools can
properly know that the API has changed, and that they want to build
against the old one.

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

Re: [Libusbx-devel] libusbx-1.0.13 has been released

[Hans de Goede]

Hi,

On 09/24/2012 08:50 PM, Greg KH wrote:

On Mon, Sep 24, 2012 at 08:36:11PM +0200, Hans de Goede wrote:

Hi Greg,

On 09/24/2012 05:07 PM, Greg KH wrote:

On Mon, Sep 24, 2012 at 03:52:55PM +0200, Hans de Goede wrote:

Hi,

On 09/20/2012 11:42 PM, Pete Batard wrote:

Hi,

It with pleasure that I would like to announce the release of libusbx
1.0.13. This version brings the following notable changes:


The Fedora packages for libusbx have been upgraded to 1.0.13 now.


How did you handle the usbutils breakage?


I did not handle it at all, as I was not aware of it. Now that I'm I'll
file a bug against usbutils and advice the maintainer to add the
necessary #ifdef-ery to keep it building.


So, you are going to force me (hint, I'm the usbutils maintainer)


I know you are the usbutils maintainer.


, to
change my code because libusbx broke their API here?


Force you, no, forcing you to do anything is (luckily) not within my
power. But I can surely ask you nicely to change your code.


That's what other
distros have already tried to tell me earlier today, and I'm going to
push back hard and say that it is a bug in libusbx instead.


I can certainly understand that from your pov this seems as a libusbx
bug. Pete has already tried to explain his reasoning for the change
in his mail to you. And it seems that the problem is that there is
a difference of opinion here on priorities, Pete beliefs following
the USB spec and fixing a historic mistake is more important here, you
belief that API compatibility is more important.

I myself am somewhere in the middle here, I did not think that the
change Pete suggested would be a big deal (how wrong of me), so I agreed
to it. However your reaction shows that API compatibility is a big deal,
and in general I agree with that. So lets try to move forward with this.

I see 3 possible solutions:

1) Revert the change, and do a 1.0.14 release with just that change
right away, before anyone starts depending on the new name

2) Adapt usbutils to work with both versions of the API, as is suggested
in the release announcement. But given what you said above I guess you're
unwilling to do that ?

3) Go with the anonymous union solution discussed before, and do a 1.0.14
release with that change right away.

However we solve this I've learned from this to take API compatibility as
something very important, and the next time we discuss something like this
I will not only vigorously defend ABI but also API compatibility, and I
promise that I will do my very best to make sure that the libusbx-1.0.x
series will stay fully ABI and API compatible for all future releases!

Because we do care about our users, both developers programming against
libusbx and end-users using libusbx through various applications!

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: [Libusbx-devel] libusbx-1.0.13 has been released

[Pete Batard]

On 2012.09.24 19:50, Greg KH wrote:

Please fix this in libusbx, or bump the .so name so that tools can
properly know that the API has changed, and that they want to build
against the old one.


Well, if you leave us no other alternative, then I guess my vote will be 
for option 2, especially as we have had issues with trying to keep the 
libusb and libusbx APIs in sync in the past and we are a fork.


For the record, we will keep the LIBUSBX_API_VERSION macro in future 
versions (though we may decrease it in 1.0.14 if we do a revert), 
therefore the workaround we provide for bMaxPower and 1.0.13 will still 
work should you want to to apply it _temporarily_ to alleviate the 
1.0.13 blowback you seem to be getting.


Note that if we release 1.1, we will not be able to ensure compatibility 
with libusb, when/if libusb releases 1.1, as this will become a pure 
libusb matter then.


Regards,

/Pete


--
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: [Libusbx-devel] libusbx-1.0.13 has been released

[Kustaa Nyholm]

On 24.9.2012 21.47, "Greg KH"  wrote:
>
>And if I'm going to be forced to change my program, and libusbx has now
>shown that they don't care about their public api, well, I might as well
>just rewrite it to remove that dependancy completly, as it's obvious
>they don't know how to treat their users.

Really Greg,

I think you are overreacting and talking through your hat.

If you bothered to check the archives you'd see how much the developers
actually care about their users.

If you think rewriting to remove that dependency makes more sense
than applying a trivial fix Pete promised to deliver, go ahead,
especially if you think that is what *your* users will want.

This may have been a bad decision but there it is and now we all have
deal with flack, you, me and the developers...

Again, please note I'm talking as a hanger on who saw the struggle to
get anything done in libusb project, not as a developer of libusbx.


cheers Kusti



--
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: [Libusbx-devel] libusbx-1.0.13 has been released

[Sean McBride]

On Mon, 24 Sep 2012 11:47:27 -0700, Greg KH said:

>No, you are forcing me to change my program to have it build with your
>change, you should do it the other way around.  If you want programs to
>use your library, make it so that they don't have to be changed at all.
>
>And if I'm going to be forced to change my program, and libusbx has now
>shown that they don't care about their public api, well, I might as well
>just rewrite it to remove that dependancy completly, as it's obvious
>they don't know how to treat their users.

I hesitate to participate in this thread, as I don't like flame wars, but a 
piece of honest constructive criticism: if your project depends on libusbx (or 
any other project) why did you not try the release candidates to catch this 
problem before release?  Was the RC period too short?  Were you unaware of it? 

Cheers,

Sean


--
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 v1 2/2] USB: set hub's default autosuspend delay as 0

[Alan Stern]

On Tue, 25 Sep 2012, Ming Lei wrote:

> On Tue, Sep 25, 2012 at 1:06 AM, Alan Stern  wrote:
> > On Tue, 25 Sep 2012, Ming Lei wrote:
> >
> >> IMO, it should be reasonable to set the device's autosuspend delay
> >> as zero when its driver is unbound because no one can predict when
> >> the device will be bound to a new driver.
> >
> > It could be set to a low value, but it shouldn't be set to 0.  There
> 
> If so, I'd rather to keep the default 2 seconds. Changing it to zero can
> avoid one auto suspend timer which may increase CPU power usage.

By how much?  That is, how many times is an unbound USB device going to
be autosuspended?

Do you have any particular use cases in mind where the extra CPU power 
usage will matter?

> > aren't too many things you can do with an unbound device; lsusb is one
> > of the most common.
> 
> Considered 'lsusb' is a low frequency tool for final users, so could we just
> ignore it for the case?

It depends on the case.  What sort of device do you have in mind?

> > On the other hand, we already have a usbcore module parameter for
> > specifying the default autosuspend delay.  That's what the delay is set
> > to when a device is first initialized, before it is bound to anything.
> > Unfortunately the parameter's value is in seconds, not milliseconds, so
> > we can't set it to any positive value smaller than 1 second.
> 
> It is a global and can't be used to just change on unbound device.

That's right; it affects every device.

But think of it this way: Suppose there's no driver for a particular 
device.  Its autosuspend delay will get set to the default initially 
and then there won't be any driver to change it.  So when would the 
delay be changed to 0?

I'm not sure that this is something the kernel needs to worry about.  
Userspace can easily set the default delay to 0 whenever it wants to.  
This won't affect devices that are already plugged in... but userspace 
can also set the delays for those devices whenever it wants to.

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: [Libusbx-devel] libusbx-1.0.13 has been released

[Greg KH]

On Mon, Sep 24, 2012 at 03:20:10PM -0400, Sean McBride wrote:
> On Mon, 24 Sep 2012 11:47:27 -0700, Greg KH said:
> 
> >No, you are forcing me to change my program to have it build with your
> >change, you should do it the other way around.  If you want programs to
> >use your library, make it so that they don't have to be changed at all.
> >
> >And if I'm going to be forced to change my program, and libusbx has now
> >shown that they don't care about their public api, well, I might as well
> >just rewrite it to remove that dependancy completly, as it's obvious
> >they don't know how to treat their users.
> 
> I hesitate to participate in this thread, as I don't like flame wars,
> but a piece of honest constructive criticism: if your project depends
> on libusbx (or any other project) why did you not try the release
> candidates to catch this problem before release?  Was the RC period
> too short?  Were you unaware of it?

My program depends on "libusb", which, as it seems there are now two
different versions, one would think that they would at least not break
backward compatibility.

And yes, I was not aware of the libusbx release cycle, but I surely did
not think that a library would break its API without changing the .so
name of the library.  To do so would be madness, don't you agree?

I would ask how this library update was tested if someone didn't, at the
very least, test the most common application that uses the library?

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

[RFC] xhci: Intel Panther Point BEI quirk.

[Sarah Sharp]

When a device with an isochronous endpoint is behind a hub plugged into
the Intel Panther Point xHCI host controller, and the driver submits
multiple frames per URB, the xHCI driver will set the Block Event
Interrupt (BEI) flag on all but the last TD for the URB.  This causes
the host controller to place an event on the event ring, but not send an
interrupt.  When the last TD for the URB completes, BEI is cleared, and
we get an interrupt for the whole URB.

However, under a Panther Point xHCI host controller, if the parent hub
is unplugged when one or more events from transfers with BEI set are on
the event ring, a port status change event is placed on the event ring,
but no interrupt is generated.  This means URBs stop completing, and the
USB device disconnect is not noticed.  Something like a USB headset will
cause mplayer to hang when the device is disconnected.

If another transfer is sent (such as running `sudo lsusb -v`), the next
transfer event seems to "unstick" the event ring, the xHCI driver gets
an interrupt, and the disconnect is reported to the USB core.

The fix is not to use the BEI flag under the Panther Point xHCI host.
This will impact power consumption and system responsiveness, because
the xHCI driver will receive an interrupt for every frame in all
isochronous URBs instead of once per URB.

This patch should be backported to kernels as old as 3.0, that contain
the commit 69e848c2090aebba5698a1620604c7dccb448684 "Intel xhci: Support
EHCI/xHCI port switching."

Signed-off-by: Sarah Sharp 
Cc: sta...@vger.kernel.org
---
 drivers/usb/host/xhci-pci.c  |1 +
 drivers/usb/host/xhci-ring.c |4 +++-
 drivers/usb/host/xhci.h  |1 +
 3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 9bfd4ca11..8345d7c 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -103,6 +103,7 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
 * PPT chipsets.
 */
xhci->quirks |= XHCI_SPURIOUS_REBOOT;
+   xhci->quirks |= XHCI_AVOID_BEI;
}
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
pdev->device == PCI_DEVICE_ID_ASROCK_P67) {
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f270e70..c6ebb17 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3672,7 +3672,9 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, 
gfp_t mem_flags,
} else {
td->last_trb = ep_ring->enqueue;
field |= TRB_IOC;
-   if (xhci->hci_version == 0x100) {
+   if (xhci->hci_version == 0x100 &&
+   !(xhci->quirks &
+   XHCI_AVOID_BEI)) {
/* Set BEI bit except for the last td */
if (i < num_tds - 1)
field |= TRB_BEI;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e44e2d3..53df4e7 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1511,6 +1511,7 @@ struct xhci_hcd {
 #define XHCI_INTEL_HOST(1 << 12)
 #define XHCI_SPURIOUS_REBOOT   (1 << 13)
 #define XHCI_COMP_MODE_QUIRK   (1 << 14)
+#define XHCI_AVOID_BEI (1 << 15)
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
/* There are two roothubs to keep track of bus suspend info for */
-- 
1.7.9

--
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: [Libusbx-devel] libusbx-1.0.13 has been released

[Greg KH]

On Mon, Sep 24, 2012 at 09:12:56PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 09/24/2012 08:50 PM, Greg KH wrote:
> >On Mon, Sep 24, 2012 at 08:36:11PM +0200, Hans de Goede wrote:
> >>Hi Greg,
> >>
> >>On 09/24/2012 05:07 PM, Greg KH wrote:
> >>>On Mon, Sep 24, 2012 at 03:52:55PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 09/20/2012 11:42 PM, Pete Batard wrote:
> >Hi,
> >
> >It with pleasure that I would like to announce the release of libusbx
> >1.0.13. This version brings the following notable changes:
> 
> The Fedora packages for libusbx have been upgraded to 1.0.13 now.
> >>>
> >>>How did you handle the usbutils breakage?
> >>
> >>I did not handle it at all, as I was not aware of it. Now that I'm I'll
> >>file a bug against usbutils and advice the maintainer to add the
> >>necessary #ifdef-ery to keep it building.
> >
> >So, you are going to force me (hint, I'm the usbutils maintainer)
> 
> I know you are the usbutils maintainer.
> 
> >, to
> >change my code because libusbx broke their API here?
> 
> Force you, no, forcing you to do anything is (luckily) not within my
> power. But I can surely ask you nicely to change your code.
> 
> >That's what other
> >distros have already tried to tell me earlier today, and I'm going to
> >push back hard and say that it is a bug in libusbx instead.
> 
> I can certainly understand that from your pov this seems as a libusbx
> bug. Pete has already tried to explain his reasoning for the change
> in his mail to you. And it seems that the problem is that there is
> a difference of opinion here on priorities, Pete beliefs following
> the USB spec and fixing a historic mistake is more important here, you
> belief that API compatibility is more important.
> 
> I myself am somewhere in the middle here, I did not think that the
> change Pete suggested would be a big deal (how wrong of me), so I agreed
> to it. However your reaction shows that API compatibility is a big deal,
> and in general I agree with that. So lets try to move forward with this.
> 
> I see 3 possible solutions:
> 
> 1) Revert the change, and do a 1.0.14 release with just that change
> right away, before anyone starts depending on the new name

That would be a good first step, given the fall-out I've already gotten
from the distros who have tried to incorporate 1.0.13 into their build
systems.

> 2) Adapt usbutils to work with both versions of the API, as is suggested
> in the release announcement. But given what you said above I guess you're
> unwilling to do that ?

Why not bump the .so name of the library if you are changing the API?
How hard is that concept to people?

> 3) Go with the anonymous union solution discussed before, and do a 1.0.14
> release with that change right away.

That's fine with me as well, and solves the .so name issue.

> However we solve this I've learned from this to take API compatibility as
> something very important, and the next time we discuss something like this
> I will not only vigorously defend ABI but also API compatibility, and I
> promise that I will do my very best to make sure that the libusbx-1.0.x
> series will stay fully ABI and API compatible for all future releases!

Thank you.

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

Re: [Libusbx-devel] libusbx-1.0.13 has been released

[Greg KH]

On Mon, Sep 24, 2012 at 10:16:25PM +0300, Kustaa Nyholm wrote:
> On 24.9.2012 21.47, "Greg KH"  wrote:
> >
> >And if I'm going to be forced to change my program, and libusbx has now
> >shown that they don't care about their public api, well, I might as well
> >just rewrite it to remove that dependancy completly, as it's obvious
> >they don't know how to treat their users.
> 
> Really Greg,
> 
> I think you are overreacting and talking through your hat.
> 
> If you bothered to check the archives you'd see how much the developers
> actually care about their users.

Um, breaking existing applications is not indicative of that, don't you
agree?

> If you think rewriting to remove that dependency makes more sense
> than applying a trivial fix Pete promised to deliver, go ahead,
> especially if you think that is what *your* users will want.

No one delivered any such "fix", all I got was a bunch of bug reports
this morning from the distros saying that usbutils was suddenly broken.
That shows that libusbx is really the problem here, and that maybe I
shouldn't depend on it anymore.

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

Re: [PATCH v1 1/2] USB: check port changes before hub runtime suspend for some bug device

[Alan Stern]

On Tue, 25 Sep 2012, Ming Lei wrote:

> On Tue, Sep 25, 2012 at 12:42 AM, Alan Stern  
> wrote:
> > On Mon, 24 Sep 2012, Ming Lei wrote:
> >
> >> On Mon, Sep 24, 2012 at 12:09 AM, Alan Stern  
> >> wrote:
> >>
> >> > No, you can still handle the other case.  You need to report the
> >> > condition to the PM core, say by calling pm_wakeup_event().
> >>
> >> Good point, so kind of below code should be added to usb_suspend_both
> >> to handle the situation centralizedly:
> >>
> >> if (!PMSG_IS_AUTO(msg)) {
> >>  if (hdev->do_remote_wakeup && status == -EBUSY)
> >>   pm_wakeup_event(&hdev->dev, timeout)
> >>  status = 0;
> >> }
> >
> > No.  Drivers can return -EBUSY for other reasons besides a pending
> > wakeup.  They will have to handle wakeups by themselves.
> 
> If so, almost all drivers for usb devices with remote wakeup capability need
> this change, could we figure out one way to handle it centralizedly?

Do we really need to?  Remember, you are adding this new code only
because some Genesys Logic hubs are buggy.  Is there any reason to 
think other devices will have similar bugs?

As long as the device issues a remote wakeup request at the proper time
(i.e., whenever it is suspended with wakeup enabled and an event needs
to be processed), this workaround isn't needed.

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: [Libusbx-devel] libusbx-1.0.13 has been released

[Pete Batard]

On 2012.09.24 20:31, Greg KH wrote:

No one delivered any such "fix", all I got was a bunch of bug reports
this morning from the distros saying that usbutils was suddenly broken.
That shows that libusbx is really the problem here, and that maybe I
shouldn't depend on it anymore.


Could you please have a look at the release note that was posted on this 
list? We did our best to ensure the possibility of breakage featured 
prominently.


The full content of the post is replicated below.

Regards,

/Pete

On 2012.09.20 22:42, Pete Batard wrote:> Hi,
>
> It with pleasure that I would like to announce the release of libusbx
> 1.0.13. This version brings the following notable changes:
>
> * [MAJOR] Fix a typo in the API with struct libusb_config_descriptor
> where MaxPower was used instead of bMaxPower, as per the specs.
>If your application was accessing the MaxPower attribute, and you
> need to maintain compatibility with libusb or older versions, please see
> APPENDIX A below.
> * Fix broken support for the 0.1 -> 1.0 libusb-compat layer
> * Fix unwanted cancellation of pending timeouts as well as major timeout
> related bugs
> * Fix handling of HID and composite devices on Windows
> * Introduce LIBUSBX_API_VERSION macro
> * Add Cypress FX/FX2 firmware upload sample, based on fxload from
>http://linux-hotplug.sourceforge.net
> * Add libusb0 (libusb-win32) and libusbK driver support on Windows.
>Note that while the drivers allow it, isochronous transfers are not
> supported yet in libusbx. Also not supported yet is the use of
> libusb-win32 filter drivers on composite interfaces
> * Add support for the new get_capabilities ioctl on Linux and avoid
> unnecessary splitting of bulk transfers
> * Improve support for newer Intel and Renesas USB 3.0 controllers on
> Windows
> * Harmonize the device number for root hubs across platforms
> * Other bug fixes and improvements
>
> Release archives can be obtained from:
> https://sourceforge.net/projects/libusbx/files/releases/1.0.13/
>
> For more information, please visit: http://libusbx.org
>
> Regards,
>
> /Pete
>
> 
~~ 


>
> APPENDIX A - Maintaining code compatibility with versions of libusb and
> libusbx that use MaxPower:
>
> If you must to maintain compatibility with versions of the library that
> aren't using the bMaxPower attribute in struct libusb_config_descriptor
> the recommended way is to use the new LIBUSBX_API_VERSION macro with an
> #ifdef.
>
> For instance, if your code was written as follows:
>
>if (dev->config[0].MaxPower < 250)
>
> Then you should modify it to have:
>
> #if defined(LIBUSBX_API_VERSION) && (LIBUSBX_API_VERSION >= 0x01000100)
>if (dev->config[0].bMaxPower < 250)
> #else
>if (dev->config[0].MaxPower < 250)
> #endif
> 
~~ 


>
>


--
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: [Libusbx-devel] libusbx-1.0.13 has been released

[Greg KH]

On Mon, Sep 24, 2012 at 08:35:07PM +0100, Pete Batard wrote:
> On 2012.09.24 20:31, Greg KH wrote:
> >No one delivered any such "fix", all I got was a bunch of bug reports
> >this morning from the distros saying that usbutils was suddenly broken.
> >That shows that libusbx is really the problem here, and that maybe I
> >shouldn't depend on it anymore.
> 
> Could you please have a look at the release note that was posted on
> this list? We did our best to ensure the possibility of breakage
> featured prominently.

I saw that, but again, you are forcing users of your library, to change
their code due to your API change.  You are changing the API without
changing the library version, which is a very bad thing to do.

Yes, I understand you wanted to fix an old typo in a structure, so fix
it and bump the version number of the library if it is that important.

If it's not important enough to change the version number of the
library, then maybe you shouldn't have made that change.

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

Re: [Libusbx-devel] libusbx-1.0.13 has been released

[Kustaa Nyholm]

On 24.9.2012 22.31, "Greg KH"  wrote:

>
>Um, breaking existing applications is not indicative of that, don't you
>agree?

You never make bad calls and you think this was a deliberate
attempt not to care for users? If you think it is a bug and a bad
choice, why not say it, instead of writing down the whole project
as one that does not care for its users?

So, yes I do not agree that it was indicative of a project that
does not care about it users and I do think the way you come across is
indicative of overreacting.

>
>> If you think rewriting to remove that dependency makes more sense
>> than applying a trivial fix Pete promised to deliver, go ahead,
>> especially if you think that is what *your* users will want.
>
>No one delivered any such "fix", all I got was a bunch of bug reports
>this morning from the distros saying that usbutils was suddenly broken.

If you read what I wrote I said "Pete promised"  so if wanted all you
would have had to do was to say so. It is not nice when things break
on you but a polite and constructive approach usually is usually the
best policy to get things going again.

>That shows that libusbx is really the problem here, and that maybe I
>shouldn't depend on it anymore.

I don't think anyone has said that *the* change in the libusbx is
the source of the problem, I've just said that you could fix it fast,
probably faster that doing this email exchange, if you wanted to.


I'm sure libusbx developers would be sorry to loose you so don't let
my voice affect your decision as they seem to take the adult view
of this and are willing to co-operate with you although you seem to
be ready to fly off at the handle before the discussion has hardly
started.

br Kusti





--
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] xhci: Intel Panther Point BEI quirk.

[Alan Stern]

On Mon, 24 Sep 2012, Sarah Sharp wrote:

> When a device with an isochronous endpoint is behind a hub plugged into
> the Intel Panther Point xHCI host controller, and the driver submits
> multiple frames per URB, the xHCI driver will set the Block Event
> Interrupt (BEI) flag on all but the last TD for the URB.  This causes
> the host controller to place an event on the event ring, but not send an
> interrupt.  When the last TD for the URB completes, BEI is cleared, and
> we get an interrupt for the whole URB.
> 
> However, under a Panther Point xHCI host controller, if the parent hub
> is unplugged when one or more events from transfers with BEI set are on
> the event ring, a port status change event is placed on the event ring,
> but no interrupt is generated.  This means URBs stop completing, and the
> USB device disconnect is not noticed.  Something like a USB headset will
> cause mplayer to hang when the device is disconnected.

Won't that also cause a problem for hot-unplug detection?  How will the 
hub driver learn about the unplug event if there's no interrupt?

Also, does this same problem affect bulk transfers, in particular those
using scatter-gather?

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: [Libusbx-devel] libusbx-1.0.13 has been released

[Jure Menart]

Hi everybody,

I'd like to share one unrelated thing: I AM NOT subscribed to the libusbx 
related mailing lists and I AM NOT using it in my current development.

I do appreciate the effort of all libusbx developers and I support them in 
their decisions. I hope once I will contribute something to it... but at this 
moment in time - I am subscribed to libusb development mailing list. Why I have 
to read about (IMHO quite childish) problems of a fork of libusb?

I like how reasonable Pete and Hands are handling this so congratulations to 
them. But can you please remove 'old non-friendly libusb' thing from your CC's. 
Each maintainer/developer who will or would like to support libusb and libusbx 
should follow their thread and complain at their mailing list.


Thanks,
Jure Menart

- Original Message -
From: "Greg KH" 
To: "Kustaa Nyholm" 
Cc: libusb-de...@lists.sourceforge.net, libusbx-de...@lists.sourceforge.net, 
linux-usb@vger.kernel.org, libusb-win32-de...@lists.sourceforge.net
Sent: Monday, 24 September, 2012 9:31:29 PM
Subject: Re: [Libusbx-devel] libusbx-1.0.13 has been released

On Mon, Sep 24, 2012 at 10:16:25PM +0300, Kustaa Nyholm wrote:
> On 24.9.2012 21.47, "Greg KH"  wrote:
> >
> >And if I'm going to be forced to change my program, and libusbx has now
> >shown that they don't care about their public api, well, I might as well
> >just rewrite it to remove that dependancy completly, as it's obvious
> >they don't know how to treat their users.
> 
> Really Greg,
> 
> I think you are overreacting and talking through your hat.
> 
> If you bothered to check the archives you'd see how much the developers
> actually care about their users.

Um, breaking existing applications is not indicative of that, don't you
agree?

> If you think rewriting to remove that dependency makes more sense
> than applying a trivial fix Pete promised to deliver, go ahead,
> especially if you think that is what *your* users will want.

No one delivered any such "fix", all I got was a bunch of bug reports
this morning from the distros saying that usbutils was suddenly broken.
That shows that libusbx is really the problem here, and that maybe I
shouldn't depend on it anymore.

greg k-h

--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
libusb-devel mailing list
libusb-de...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusb-devel


--- Disclaimer: --- 
This email and contents is for use only by the intended recipient. If you are 
not the individual or entity to whom it is addressed, you are hereby formally 
notified that any use, copying or distribution of this email and attachments, 
in whole or in part, is strictly prohibited. If you have received this email in 
error, please notify the sender and delete the message and attachment(s) from 
your system. Any views, opinions or information, expressed or contained in this 
email, are those of the sender and not necessarily reflect those of ESPROS 
Photonics AG. To help protect our environment, please avoid printing out this 
information unnecessarily.

--
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: [Libusbx-devel] libusbx-1.0.13 has been released

[Pete Batard]

On 2012.09.24 20:26, Greg KH wrote:

I would ask how this library update was tested if someone didn't, at the
very least, test the most common application that uses the library?


Ad hoc, and mostly, as far as I'm concerned, using our sample xusb and 
fxload test application and a selected list of devices (about 3-4 of 
them). And since this release introduced two major developments for 
Windows (2 new drivers) as well as a new sample, the amount of work 
required for testing was significantly increased there.


We don't currently have a formal list of applications and devices we 
want to churn through during RC, but I guess we'll try to do that in the 
future (provided we have access to the relevant USB devices, but that 
shouldn't be a problem for usbutils). We will probably also try to 
increase the RC time to at least 2 weeks as it looks like this may have 
been a bit too short. Still, I will reiterate that we could use 
volunteers, especially during the RC phase, as, considering that this is 
a generic library, there is just too much to test for the handful of 
people currently doing it.


Regards,

/Pete
--
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] xhci: Intel Panther Point BEI quirk.

[Sarah Sharp]

On Mon, Sep 24, 2012 at 03:50:04PM -0400, Alan Stern wrote:
> On Mon, 24 Sep 2012, Sarah Sharp wrote:
> 
> > When a device with an isochronous endpoint is behind a hub plugged into
> > the Intel Panther Point xHCI host controller, and the driver submits
> > multiple frames per URB, the xHCI driver will set the Block Event
> > Interrupt (BEI) flag on all but the last TD for the URB.  This causes
> > the host controller to place an event on the event ring, but not send an
> > interrupt.  When the last TD for the URB completes, BEI is cleared, and
> > we get an interrupt for the whole URB.
> > 
> > However, under a Panther Point xHCI host controller, if the parent hub
> > is unplugged when one or more events from transfers with BEI set are on
> > the event ring, a port status change event is placed on the event ring,
> > but no interrupt is generated.  This means URBs stop completing, and the
> > USB device disconnect is not noticed.  Something like a USB headset will
> > cause mplayer to hang when the device is disconnected.
> 
> Won't that also cause a problem for hot-unplug detection?  How will the 
> hub driver learn about the unplug event if there's no interrupt?

There will be an interrupt if we don't use the BEI flag.  That's what
this patch fixes.

The BEI flag is only supposed to block isochronous interrupts, and only
if there was no error associated with the transfer.  But the Panther
Point xHCI host is broken, and the BEI flag stops unrelated interrupts,
like the port status change event interrupt.

> Also, does this same problem affect bulk transfers, in particular those
> using scatter-gather?

Bulk transfers, including scatter-gather transfers, are always
translated into one TD, and the BEI flag is not used.  AFAIK, this bug
only affects transfers with the BEI flag set, so bulk transfers should
be fine.

Sarah Sharp
--
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 port power off discussion

[Greg KH]

On Mon, Sep 24, 2012 at 10:46:30AM -0700, Greg KH wrote:
> On Mon, Sep 24, 2012 at 03:18:19PM +0200, Rafael J. Wysocki wrote:
> > On Saturday, September 22, 2012, Rafael J. Wysocki wrote:
> > > On Friday, September 21, 2012, Sarah Sharp wrote:
> > > > Two weeks ago at Linux Plumbers Conference, I presented about the Intel
> > > > Lynx Point USB port power off mechanism.  This email is a report out of
> > > > what was discussed, and a kick off point for further discussion.
> > 
> > [...]
> > 
> > > > ca9c9d0 usb : Add sysfs files to control port power.
> > > 
> > > http://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=commit;h=ca9c9d0
> > 
> > I have reviewed the whole series (I suppose there's too late for adding
> > Reviewed-by tags to it, but oh well ;-)) and the only patch I have some 
> > serious
> > doubts about is the one above.  In fact, I'd prefer it to be reverted for 
> > now.
> 
> Ok, I can revert that one, I'd rather hold off on any user api changes
> like this if people have reservations about them.  It's harder to revert
> them later on than it is now.

I've now reverted this.  If there's anything else you don't like in the
series, please let me know.

thanks,

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

Re: USB port power off discussion

[Rafael J. Wysocki]

On Monday, September 24, 2012, Greg KH wrote:
> On Mon, Sep 24, 2012 at 10:46:30AM -0700, Greg KH wrote:
> > On Mon, Sep 24, 2012 at 03:18:19PM +0200, Rafael J. Wysocki wrote:
> > > On Saturday, September 22, 2012, Rafael J. Wysocki wrote:
> > > > On Friday, September 21, 2012, Sarah Sharp wrote:
> > > > > Two weeks ago at Linux Plumbers Conference, I presented about the 
> > > > > Intel
> > > > > Lynx Point USB port power off mechanism.  This email is a report out 
> > > > > of
> > > > > what was discussed, and a kick off point for further discussion.
> > > 
> > > [...]
> > > 
> > > > > ca9c9d0 usb : Add sysfs files to control port power.
> > > > 
> > > > http://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=commit;h=ca9c9d0
> > > 
> > > I have reviewed the whole series (I suppose there's too late for adding
> > > Reviewed-by tags to it, but oh well ;-)) and the only patch I have some 
> > > serious
> > > doubts about is the one above.  In fact, I'd prefer it to be reverted for 
> > > now.
> > 
> > Ok, I can revert that one, I'd rather hold off on any user api changes
> > like this if people have reservations about them.  It's harder to revert
> > them later on than it is now.
> 
> I've now reverted this.

Thanks!

> If there's anything else you don't like in the series, please let me know.

As I said, all of the other commits in this series are fine by me.

Rafael
--
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: usb3 fails to write when using usb3 hub in usb3 port

[Sarah Sharp]

On Mon, Sep 24, 2012 at 09:13:42PM +0300, Adrian Sandu wrote:
> > I'll whip up some patches that allow you to disable USB 3.0 Link PM for
> > the VIA hub and we'll see if that helps.  Or you could downgrade to the
> > 3.4 kernel and see if that helps, since USB 3.0 Link PM went into 3.5.
> 
> Thanks Sarah. I tried in 3.4.11 to .. about the same thing ..
> http://d3xt3r01.tk/~dexter/usbmon.trace.txt contains a new trace.

Uh, are you sure you're running 3.4.11?  Because this output from the
log snippet you sent:

> 2012-09-24T21:08:57.203739+03:00 d3xt3r01 kernel: [  249.104503] usb 2-2.2: 
> Parent hub missing LPM exit latency info.  Power management will be impacted.

Is not present in the USB core until 3.5-rc1:

sarah@xanatos:~/git/kernels/xhci$ git blame drivers/usb/core/hub.c
51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  214)   /* All 
USB 3.0 must support LPM, but we need their max exit latency
51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  215)* 
information from the SuperSpeed Extended Capabilities BOS descriptor.
51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  216)*/
51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  217)   if 
(!udev->bos->ss_cap) {
51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  218)   
dev_warn(&udev->dev, "No LPM exit latency info found.  "
51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  219)   
"Power management will be impacted.\n");
51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  220)   
return 0;
51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  221)   }

sarah@xanatos:~/git/kernels/xhci$ git describe --contains --match="v*" 51e0a01
v3.5-rc1~149^2~6^2~9

So can you double check you were running 3.4.11 on your last test?

Sarah Sharp
--
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 port power off discussion

[Rafael J. Wysocki]

On Monday, September 24, 2012, Lan Tianyu wrote:
> On 2012/9/24 21:18, Rafael J. Wysocki wrote:
> > On Saturday, September 22, 2012, Rafael J. Wysocki wrote:
> >> On Friday, September 21, 2012, Sarah Sharp wrote:
> >>> Two weeks ago at Linux Plumbers Conference, I presented about the Intel
> >>> Lynx Point USB port power off mechanism.  This email is a report out of
> >>> what was discussed, and a kick off point for further discussion.
> >
> > [...]
> >
> >>> ca9c9d0 usb : Add sysfs files to control port power.
> >>
> >> http://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=commit;h=ca9c9d0
> >
> > I have reviewed the whole series (I suppose there's too late for adding
> > Reviewed-by tags to it, but oh well ;-)) and the only patch I have some 
> > serious
> > doubts about is the one above.  In fact, I'd prefer it to be reverted for 
> > now.
> >
> > First off, if the power resources manipulated through those sysfs files
> > are shared between ports (i.e. there are power domains), the interface isn't
> > really well defined, because the advertised functionality (powering off/on a
> > USB port) is not there in general.
> USB port power off is defined in the usb spec that when the port's 
> PORT_POWER feature is removed, port power should be turn off.
> But most hubs don't implement it. So the feature should be general and
> not common.

I don't mean this.

Suppose that there are two ports on the hub, A and B, and there's only one
power resource used to put A _and_ B into D3cold.  Then, when you call
acpi_bus_set_power(A, D3cold) on A alone, what really happens is that the
power resource's reference counter is decremented and power isn't really
removed from the port.  To actually remove power from A you'd need to
call acpi_bus_set_power(B, D3cold) too, but then it would be removed from
_both_ A and B simultaneously.

Your simple sysfs interface doesn't match this use case.

> > Second, I'm not sure if there's any way for user space to figure out what
> > ports are connected to what sockets visible to user space.  If there is such
> > a way, I wonder what it is, if not, the proposed interface is just plain
> > dangerous.
> ACPI _PLD method provides a lot of information to describe where the 
> port located in. But currently it is not exposed to user space.

Well, precisely.  Which means that the user would have to apply trial-and-error
to figure out which sysfs file corresponds to which physical port on his/her
machine.

That doesn't sound really user friendly.

> > Finally, it follows from my experience that interfaces of this kind often
> > are sources of pain and grief for distro support folks who need to handle
> > problems reported by users.  I used to do that and I know what kind of pain
> > that is.  So, in my opinion it's better not to expose low-level 
> > functionality
> > to user space directly, like in this case.
> 
> You mean force power on and power off? There is a demand that if a usb 
> device was abnormal, user space driver or app could make it rework via 
> power off.

Well, then implement it as a "hard reset" attribute on the device.  Namely,
if the device is attached to a power-manageable port, writing 1 (for example)
to its "hard reset" attribute will cause the port to be power-cycled (as
long as the port has its own power resource, that is).

Thanks,
Rafael
--
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: [Libusbx-devel] libusbx-1.0.13 has been released

[Nathan Hjelm]

Agreed. Please move this to the libusbx mailing list. I couldn't care less 
about libusbx after the hid fiasco.

-Nathan

On Sep 24, 2012, at 9:42 PM, Jure Menart  wrote:

> Hi everybody,
> 
> I'd like to share one unrelated thing: I AM NOT subscribed to the libusbx 
> related mailing lists and I AM NOT using it in my current development.
> 
> I do appreciate the effort of all libusbx developers and I support them in 
> their decisions. I hope once I will contribute something to it... but at this 
> moment in time - I am subscribed to libusb development mailing list. Why I 
> have to read about (IMHO quite childish) problems of a fork of libusb?
> 
> I like how reasonable Pete and Hands are handling this so congratulations to 
> them. But can you please remove 'old non-friendly libusb' thing from your 
> CC's. Each maintainer/developer who will or would like to support libusb and 
> libusbx should follow their thread and complain at their mailing list.
> 
> 
> Thanks,
> Jure Menart
> 
> - Original Message -
> From: "Greg KH" 
> To: "Kustaa Nyholm" 
> Cc: libusb-de...@lists.sourceforge.net, libusbx-de...@lists.sourceforge.net, 
> linux-usb@vger.kernel.org, libusb-win32-de...@lists.sourceforge.net
> Sent: Monday, 24 September, 2012 9:31:29 PM
> Subject: Re: [Libusbx-devel] libusbx-1.0.13 has been released
> 
> On Mon, Sep 24, 2012 at 10:16:25PM +0300, Kustaa Nyholm wrote:
>> On 24.9.2012 21.47, "Greg KH"  wrote:
>>> 
>>> And if I'm going to be forced to change my program, and libusbx has now
>>> shown that they don't care about their public api, well, I might as well
>>> just rewrite it to remove that dependancy completly, as it's obvious
>>> they don't know how to treat their users.
>> 
>> Really Greg,
>> 
>> I think you are overreacting and talking through your hat.
>> 
>> If you bothered to check the archives you'd see how much the developers
>> actually care about their users.
> 
> Um, breaking existing applications is not indicative of that, don't you
> agree?
> 
>> If you think rewriting to remove that dependency makes more sense
>> than applying a trivial fix Pete promised to deliver, go ahead,
>> especially if you think that is what *your* users will want.
> 
> No one delivered any such "fix", all I got was a bunch of bug reports
> this morning from the distros saying that usbutils was suddenly broken.
> That shows that libusbx is really the problem here, and that maybe I
> shouldn't depend on it anymore.
> 
> greg k-h
> 
> --
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and 
> threat landscape has changed and how IT managers can respond. Discussions 
> will include endpoint security, mobile security and the latest in malware 
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> ___
> libusb-devel mailing list
> libusb-de...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/libusb-devel
> 
> 
> --- Disclaimer: --- 
> This email and contents is for use only by the intended recipient. If you are 
> not the individual or entity to whom it is addressed, you are hereby formally 
> notified that any use, copying or distribution of this email and attachments, 
> in whole or in part, is strictly prohibited. If you have received this email 
> in error, please notify the sender and delete the message and attachment(s) 
> from your system. Any views, opinions or information, expressed or contained 
> in this email, are those of the sender and not necessarily reflect those of 
> ESPROS Photonics AG. To help protect our environment, please avoid printing 
> out this information unnecessarily.
> 
> 
> --
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and 
> threat landscape has changed and how IT managers can respond. Discussions 
> will include endpoint security, mobile security and the latest in malware 
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> ___
> libusb-devel mailing list
> libusb-de...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/libusb-devel
--
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 port power off discussion

[Alan Stern]

On Mon, 24 Sep 2012, Rafael J. Wysocki wrote:

> I don't mean this.
> 
> Suppose that there are two ports on the hub, A and B, and there's only one
> power resource used to put A _and_ B into D3cold.  Then, when you call
> acpi_bus_set_power(A, D3cold) on A alone, what really happens is that the
> power resource's reference counter is decremented and power isn't really
> removed from the port.  To actually remove power from A you'd need to
> call acpi_bus_set_power(B, D3cold) too, but then it would be removed from
> _both_ A and B simultaneously.
> 
> Your simple sysfs interface doesn't match this use case.

It's not quite that simple.  The USB spec does require that a port
essentially stop functioning when its power feature is disabled -- even
if the port is ganged with others and therefore remains at full power.

I don't know if we need to worry about such subtleties.

> > > Second, I'm not sure if there's any way for user space to figure out what
> > > ports are connected to what sockets visible to user space.  If there is 
> > > such
> > > a way, I wonder what it is, if not, the proposed interface is just plain
> > > dangerous.
> > ACPI _PLD method provides a lot of information to describe where the 
> > port located in. But currently it is not exposed to user space.
> 
> Well, precisely.  Which means that the user would have to apply 
> trial-and-error
> to figure out which sysfs file corresponds to which physical port on his/her
> machine.
> 
> That doesn't sound really user friendly.

It doesn't have to be trial-and-error.  We should add symlinks between
the sysfs directory for a USB device and the directory for the port it
is plugged into.

In fact, Tianyu, that would be a good patch to do next.

> > > Finally, it follows from my experience that interfaces of this kind often
> > > are sources of pain and grief for distro support folks who need to handle
> > > problems reported by users.  I used to do that and I know what kind of 
> > > pain
> > > that is.  So, in my opinion it's better not to expose low-level 
> > > functionality
> > > to user space directly, like in this case.
> > 
> > You mean force power on and power off? There is a demand that if a usb 
> > device was abnormal, user space driver or app could make it rework via 
> > power off.
> 
> Well, then implement it as a "hard reset" attribute on the device.  Namely,
> if the device is attached to a power-manageable port, writing 1 (for example)
> to its "hard reset" attribute will cause the port to be power-cycled (as
> long as the port has its own power resource, that is).

A few people want to use their USB ports as digital output signals.  
For this purpose they want to be able to control the port power 
directly.  However, this is relatively rare.

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

Linux xHCI driver problems

[Yuliya T]

Dear Sarah,
We noticed a couple of problems with the Linux xHCI driver:

1. There is a problem with URB cancellation for USB 2.0 device on USB
3.0 host.  When we disable our device, we cancel all pending URBs by
calling ioctl with the request code of USBDEVFS_DISCARDURB.  When we
re-enable our device, we submit new URBs, but when we try to reap
them, the program hangs (not every time, but pretty often).  The same
code works if the device is plugged into the EHCI port, and we also
don't see any hanging with xHCI host if we don't do URB cancellation,
but have our device terminate URBs by sending zero-length packets
instead.

2. Clear Halt of EP does not reset the Sequence number if the device
is a USB 3.0 device, while it seems that clear halt properly resets
the toggle bits if the device is 2.0 even on an xHCI host.

Thank you
Yuliya
--
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: usb3 fails to write when using usb3 hub in usb3 port

[Sarah Sharp]

Ok, so 3.4.11 doesn't work, and the log file was from 3.5.

If it's not LPM, I'm really out of ideas.  I would suggest getting
another USB 3.0 hub, but I know that basically the VIA chipset is the
one out there, so that wouldn't be likely to help.

You might go see if your USB 3.0 hub has a firmware update for it.  I
know other people were having problems with storage devices that stopped
responding to a Set Address command after the USB storage driver tried
to reset it.  Maybe your issue would also go away after a firmware
update?

Sarah Sharp

On Mon, Sep 24, 2012 at 11:23:12PM +0300, Adrian Sandu wrote:
> Oh, sorry about that. Yes, that "LPM exit latency info" only appears
> in my 3.5.4, just took a look at 3.4.11 and it doesn't.
> 
> On Mon, Sep 24, 2012 at 11:13 PM, Sarah Sharp
>  wrote:
> > On Mon, Sep 24, 2012 at 09:13:42PM +0300, Adrian Sandu wrote:
> >> > I'll whip up some patches that allow you to disable USB 3.0 Link PM for
> >> > the VIA hub and we'll see if that helps.  Or you could downgrade to the
> >> > 3.4 kernel and see if that helps, since USB 3.0 Link PM went into 3.5.
> >>
> >> Thanks Sarah. I tried in 3.4.11 to .. about the same thing ..
> >> http://d3xt3r01.tk/~dexter/usbmon.trace.txt contains a new trace.
> >
> > Uh, are you sure you're running 3.4.11?  Because this output from the
> > log snippet you sent:
> >
> >> 2012-09-24T21:08:57.203739+03:00 d3xt3r01 kernel: [  249.104503] usb 
> >> 2-2.2: Parent hub missing LPM exit latency info.  Power management will be 
> >> impacted.
> >
> > Is not present in the USB core until 3.5-rc1:
> >
> > sarah@xanatos:~/git/kernels/xhci$ git blame drivers/usb/core/hub.c
> > 51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  214)   /* 
> > All USB 3.0 must support LPM, but we need their max exit latency
> > 51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  215)* 
> > information from the SuperSpeed Extended Capabilities BOS descriptor.
> > 51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  216)*/
> > 51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  217)   if 
> > (!udev->bos->ss_cap) {
> > 51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  218)   
> > dev_warn(&udev->dev, "No LPM exit latency info found.  "
> > 51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  219)   
> > "Power management will be impacted.\n");
> > 51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  220)   
> > return 0;
> > 51e0a012 (Sarah Sharp 2012-02-20 12:02:19 -0800  221)   }
> >
> > sarah@xanatos:~/git/kernels/xhci$ git describe --contains --match="v*" 
> > 51e0a01
> > v3.5-rc1~149^2~6^2~9
> >
> > So can you double check you were running 3.4.11 on your last test?
> >
> > Sarah Sharp
--
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] HID: keep dev_rdesc unmodified and use it for comparisons

[Randy Levensalor]

On 12/31/1969 05:00 PM, wrote:
usbhid's hid_post_reset checks the report descriptor currently 
returned by the

device against a descriptor that may have been modified by a driver's
report_fixup method. That leaves some devices nonfunctional after a 
resume, with
a "reset_resume error 1" reported. This patch checks the new 
descriptor against
the unmodified dev_rdesc instead and uses the original, instead of 
modified,

report size.

Thanks for the patch.  I had the same problem with a Logitech mouse and 
this fixed it.


Tested-by: Randy Levensalor 


BugLink: http://bugs.launchpad.net/bugs/1049623
Signed-off-by: Kevin Daughtridge 
---
 drivers/hid/hid-core.c|   16 +---
 drivers/hid/usbhid/hid-core.c |6 +++---
 2 files changed, 16 insertions(+), 6 deletions(-)

Changed in this version: added a temporary buffer to work around 
report_fixup

inconsistencies; using dev_rsize instead of rsize to allocate and read new
descriptor.

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8bcd168..5de3bb3 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -757,6 +757,7 @@ int hid_open_report(struct hid_device *device)
 struct hid_item item;
 unsigned int size;
 __u8 *start;
+__u8 *buf;
 __u8 *end;
 int ret;
 static int (*dispatch_type[])(struct hid_parser *parser,
@@ -775,12 +776,21 @@ int hid_open_report(struct hid_device *device)
 return -ENODEV;
 size = device->dev_rsize;

+buf = kmemdup(start, size, GFP_KERNEL);
+if (buf == NULL)
+return -ENOMEM;
+
 if (device->driver->report_fixup)
-start = device->driver->report_fixup(device, start, 
&size);
+start = device->driver->report_fixup(device, buf, 
&size);

+else
+start = buf;

-device->rdesc = kmemdup(start, size, GFP_KERNEL);
-if (device->rdesc == NULL)
+start = kmemdup(start, size, GFP_KERNEL);
+kfree(buf);
+if (start == NULL)
 return -ENOMEM;
+
+device->rdesc = start;
 device->rsize = size;

 parser = vzalloc(sizeof(struct hid_parser));
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index dedd8e4..8e0c4bf94 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1415,20 +1415,20 @@ static int hid_post_reset(struct usb_interface 
*intf)

  * configuration descriptors passed, we already know that
  * the size of the HID report descriptor has not changed.
  */
-rdesc = kmalloc(hid->rsize, GFP_KERNEL);
+rdesc = kmalloc(hid->dev_rsize, GFP_KERNEL);
 if (!rdesc) {
 dbg_hid("couldn't allocate rdesc memory (post_reset)\n");
 return 1;
 }
 status = hid_get_class_descriptor(dev,
 interface->desc.bInterfaceNumber,
-HID_DT_REPORT, rdesc, hid->rsize);
+HID_DT_REPORT, rdesc, hid->dev_rsize);
 if (status < 0) {
 dbg_hid("reading report descriptor failed (post_reset)\n");
 kfree(rdesc);
 return 1;
 }
-status = memcmp(rdesc, hid->rdesc, hid->rsize);
+status = memcmp(rdesc, hid->dev_rdesc, hid->dev_rsize);
 kfree(rdesc);
 if (status != 0) {
 dbg_hid("report descriptor changed\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





--
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 port power off discussion

[Rafael J. Wysocki]

On Monday, September 24, 2012, Alan Stern wrote:
> On Mon, 24 Sep 2012, Rafael J. Wysocki wrote:
> 
> > I don't mean this.
> > 
> > Suppose that there are two ports on the hub, A and B, and there's only one
> > power resource used to put A _and_ B into D3cold.  Then, when you call
> > acpi_bus_set_power(A, D3cold) on A alone, what really happens is that the
> > power resource's reference counter is decremented and power isn't really
> > removed from the port.  To actually remove power from A you'd need to
> > call acpi_bus_set_power(B, D3cold) too, but then it would be removed from
> > _both_ A and B simultaneously.
> > 
> > Your simple sysfs interface doesn't match this use case.
> 
> It's not quite that simple.  The USB spec does require that a port
> essentially stop functioning when its power feature is disabled -- even
> if the port is ganged with others and therefore remains at full power.
> 
> I don't know if we need to worry about such subtleties.

OK

> > > > Second, I'm not sure if there's any way for user space to figure out 
> > > > what
> > > > ports are connected to what sockets visible to user space.  If there is 
> > > > such
> > > > a way, I wonder what it is, if not, the proposed interface is just plain
> > > > dangerous.
> > > ACPI _PLD method provides a lot of information to describe where the 
> > > port located in. But currently it is not exposed to user space.
> > 
> > Well, precisely.  Which means that the user would have to apply 
> > trial-and-error
> > to figure out which sysfs file corresponds to which physical port on his/her
> > machine.
> > 
> > That doesn't sound really user friendly.
> 
> It doesn't have to be trial-and-error.  We should add symlinks between
> the sysfs directory for a USB device and the directory for the port it
> is plugged into.
> 
> In fact, Tianyu, that would be a good patch to do next.

Well, in my opinion it's rather requisite for adding the direct port power
control interface.

What about hubs connected to such ports?  I don't think they're going to
work when power is removed from it, are they?

> > > > Finally, it follows from my experience that interfaces of this kind 
> > > > often
> > > > are sources of pain and grief for distro support folks who need to 
> > > > handle
> > > > problems reported by users.  I used to do that and I know what kind of 
> > > > pain
> > > > that is.  So, in my opinion it's better not to expose low-level 
> > > > functionality
> > > > to user space directly, like in this case.
> > > 
> > > You mean force power on and power off? There is a demand that if a usb 
> > > device was abnormal, user space driver or app could make it rework via 
> > > power off.
> > 
> > Well, then implement it as a "hard reset" attribute on the device.  Namely,
> > if the device is attached to a power-manageable port, writing 1 (for 
> > example)
> > to its "hard reset" attribute will cause the port to be power-cycled (as
> > long as the port has its own power resource, that is).
> 
> A few people want to use their USB ports as digital output signals.  
> For this purpose they want to be able to control the port power 
> directly.  However, this is relatively rare.

Out of curiosity, does it apply to empty ports or ports with devices connected
to them?

Rafael
--
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: Linux xHCI driver problems

[Sarah Sharp]

On Mon, Sep 24, 2012 at 02:38:05PM -0700, Yuliya T wrote:
> Dear Sarah,
> We noticed a couple of problems with the Linux xHCI driver:
> 
> 1. There is a problem with URB cancellation for USB 2.0 device on USB
> 3.0 host.  When we disable our device, we cancel all pending URBs by
> calling ioctl with the request code of USBDEVFS_DISCARDURB.  When we
> re-enable our device, we submit new URBs, but when we try to reap
> them, the program hangs (not every time, but pretty often).  The same
> code works if the device is plugged into the EHCI port, and we also
> don't see any hanging with xHCI host if we don't do URB cancellation,
> but have our device terminate URBs by sending zero-length packets
> instead.

Which host controller are you running under?  Please send the output of
`sudo lspci -vvv` and `sudo lspci -vvv -n`.

Also, which version of libusb are you using?  Is it libusb or libusb-x?

What kind of endpoints are being used by your device?  There was a
problem with the bulk continuation flag under USB 3.0 devices, but I'm
not sure if that was fixed in libusb/libusbx yet.

Which kernel version are you using?  Have you tested with the latest
stable kernel (currently 3.5.4)?  If so, can you recompile your kernel
with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING and send me the
dmesg from one of the runs when the program hangs?

> 2. Clear Halt of EP does not reset the Sequence number if the device
> is a USB 3.0 device, while it seems that clear halt properly resets
> the toggle bits if the device is 2.0 even on an xHCI host.

The xHCI driver will issue a Reset Endpoint command that should reset
the sequence number.  If you have xHCI debugging turned on, you should
see output something like "Ignoring reset ep completion code" when the
Reset Endpoint command completes.  If you don't see that output, then
the USB core or the xHCI driver isn't resetting the endpoints when you
ask it to.

Also note that the xHCI hardware will only allow the Reset Endpoint to
complete if the endpoint was actually halted due to a stall, babble,
transfer error, etc.  It won't reset the endpoint toggles or sequence
number at arbitrary points, so we can't reset the endpoint after a new
alternate interface setting is installed.

Sarah Sharp
--
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 port power off discussion

[Sarah Sharp]

On Tue, Sep 25, 2012 at 12:09:06AM +0200, Rafael J. Wysocki wrote:
> On Monday, September 24, 2012, Alan Stern wrote:
> > On Mon, 24 Sep 2012, Rafael J. Wysocki wrote:
> > > > > Second, I'm not sure if there's any way for user space to figure out 
> > > > > what
> > > > > ports are connected to what sockets visible to user space.  If there 
> > > > > is such
> > > > > a way, I wonder what it is, if not, the proposed interface is just 
> > > > > plain
> > > > > dangerous.
> > > > ACPI _PLD method provides a lot of information to describe where the 
> > > > port located in. But currently it is not exposed to user space.
> > > 
> > > Well, precisely.  Which means that the user would have to apply 
> > > trial-and-error
> > > to figure out which sysfs file corresponds to which physical port on 
> > > his/her
> > > machine.
> > > 
> > > That doesn't sound really user friendly.
> > 
> > It doesn't have to be trial-and-error.  We should add symlinks between
> > the sysfs directory for a USB device and the directory for the port it
> > is plugged into.
> > 
> > In fact, Tianyu, that would be a good patch to do next.
> 
> Well, in my opinion it's rather requisite for adding the direct port power
> control interface.
> 
> What about hubs connected to such ports?  I don't think they're going to
> work when power is removed from it, are they?

USB hubs would have remote wakeup enabled, so we would never power off
their port with the "auto" policy.

> > > > > Finally, it follows from my experience that interfaces of this kind 
> > > > > often
> > > > > are sources of pain and grief for distro support folks who need to 
> > > > > handle
> > > > > problems reported by users.  I used to do that and I know what kind 
> > > > > of pain
> > > > > that is.  So, in my opinion it's better not to expose low-level 
> > > > > functionality
> > > > > to user space directly, like in this case.
> > > > 
> > > > You mean force power on and power off? There is a demand that if a usb 
> > > > device was abnormal, user space driver or app could make it rework via 
> > > > power off.
> > > 
> > > Well, then implement it as a "hard reset" attribute on the device.  
> > > Namely,
> > > if the device is attached to a power-manageable port, writing 1 (for 
> > > example)
> > > to its "hard reset" attribute will cause the port to be power-cycled (as
> > > long as the port has its own power resource, that is).
> > 
> > A few people want to use their USB ports as digital output signals.  
> > For this purpose they want to be able to control the port power 
> > directly.  However, this is relatively rare.
> 
> Out of curiosity, does it apply to empty ports or ports with devices connected
> to them?

There are things like USB lights or fans that pull port power, but don't
actually enumerate as a USB device.  So to the OS, those would be
"empty" ports.  But they would be external ports, so the default "auto"
policy would never power off external ports.  The user could write a
small script to PWM the fan on and off to control the speed.  It would
just be a novelty though, as Alan said.

Sarah Sharp
--
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] xhci: Intel Panther Point BEI quirk.

[Sarah Sharp]

On Mon, Sep 24, 2012 at 01:05:08PM -0700, Sarah Sharp wrote:
> On Mon, Sep 24, 2012 at 03:50:04PM -0400, Alan Stern wrote:
> > On Mon, 24 Sep 2012, Sarah Sharp wrote:
> > 
> > > When a device with an isochronous endpoint is behind a hub plugged into
> > > the Intel Panther Point xHCI host controller, and the driver submits
> > > multiple frames per URB, the xHCI driver will set the Block Event
> > > Interrupt (BEI) flag on all but the last TD for the URB.  This causes
> > > the host controller to place an event on the event ring, but not send an
> > > interrupt.  When the last TD for the URB completes, BEI is cleared, and
> > > we get an interrupt for the whole URB.
> > > 
> > > However, under a Panther Point xHCI host controller, if the parent hub
> > > is unplugged when one or more events from transfers with BEI set are on
> > > the event ring, a port status change event is placed on the event ring,
> > > but no interrupt is generated.  This means URBs stop completing, and the
> > > USB device disconnect is not noticed.  Something like a USB headset will
> > > cause mplayer to hang when the device is disconnected.
> > 
> > Won't that also cause a problem for hot-unplug detection?  How will the 
> > hub driver learn about the unplug event if there's no interrupt?
> 
> There will be an interrupt if we don't use the BEI flag.  That's what
> this patch fixes.
> 
> The BEI flag is only supposed to block isochronous interrupts, and only
> if there was no error associated with the transfer.  But the Panther
> Point xHCI host is broken, and the BEI flag stops unrelated interrupts,
> like the port status change event interrupt.
> 
> > Also, does this same problem affect bulk transfers, in particular those
> > using scatter-gather?
> 
> Bulk transfers, including scatter-gather transfers, are always
> translated into one TD, and the BEI flag is not used.  AFAIK, this bug
> only affects transfers with the BEI flag set, so bulk transfers should
> be fine.

Alan, any more questions on this patch?  I'd like to get it (and a
couple other bug fixes) pushed to Greg this week before the 3.7 merge
window.

Sarah Sharp
--
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 v4] ezusb: move ezusb.c from drivers/usb/serial to drivers/usb/misc

[Rene Buergel]

> ERROR: "ezusb_fx1_ihex_firmware_download"
> [drivers/usb/serial/whiteheat.ko] undefined!
> ERROR: "ezusb_fx1_ihex_firmware_download"
> [drivers/usb/serial/keyspan_pda.ko] undefined!
> ERROR: "ezusb_fx1_set_reset" [drivers/usb/serial/keyspan_pda.ko]
> undefined!
> ERROR: "ezusb_fx1_ihex_firmware_download"
> [drivers/usb/serial/keyspan.ko] undefined!
> 
> What I did was:
>   - build the kernel without this patch.
>   - apply the patch
>   - run make to build the kernel again.
> 
> Something isn't getting rebuilt properly, and this is going to hit
> lots of other kernel developers, so I can't take this as-is.
> 
> I've attached my .config file below, maybe something there can help
> with figuring it out?

thanks!

i managed to reproduce the error with your config.
I'm still trying to figure out why my config worked fine and how to fix this...

This might take me a while.


René Bürgel
--
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: Remote wakeup vs. hub suspend

[Peter Chen]

On Mon, Sep 24, 2012 at 12:23 AM, Alan Stern  wrote:
> It turns out that the USB-2 spec does not take into account the
> possibility of the race between a hub being suspended and one of its
> ports receiving a remote wakeup request from downstream.  The
> flowcharts and discussions in Chapter 11 ignore the possibility
> completely.  Depending on how closely a particular hub's implementation
> follows the treatment in the spec, if the two events happen at about
> the same time then it is possible for the hub to lose the wakeup
> request, and it is even possible for the hub to think the child device
> has been disconnected.  I believe (though I'm not certain) that people
> have observed both these things happen during testing.
>
> The race _was_ recognized in one of the errata documents published
> after the USB-2.0 spec came out.  The solution recommended in that
> document is exactly wrong!  It says that all the enabled ports on a hub
> should be suspended before the hub is suspended -- this just makes the
> race more likely to occur.
>
> The only viable solution is to make sure that _none_ of a hub's ports
> are suspended when the hub is suspended.  That way a downstream device
> will not be able to send a remote wakeup request until after the hub is
> fully suspended, so the race will never occur.
>
> (The situation for USB-3 hubs is somewhat different.  Right now I'm
> only talking about USB-2 hubs, or the HS/FS/LS parts of USB-3 hubs.)
>
> For us to do this would be a little difficult.  System suspend is easy
> enough to handle because we don't actually need to put any external
> ports into suspend; the entire bus will go into global suspend when the
> root hub is suspended.  In fact, this is what we should be doing now
> (but we aren't).
>
Do you think is it OK to set ehci->no_selective_suspend = 1 for single
port controller?

Currently, both ehci_hub_control and ehci_bus_suspend will go bus suspend
routine when CONFIG_USB_SUSPEND is defined, and I am not sure if
PSE and ASE is cleared before we let bus go to suspend (at ehci_hub_control).

> Runtime suspend is harder to handle.  The hub_suspend() routine would
> have to make the suspend feature is turned off for all the ports
> attached to devices that are enabled for remote wakeup before allowing
> the hub to suspend.  The hub_resume() routine would either have to
> re-enable the suspend feature for those ports or else cause all the
> wakeup-enabled children to be resumed when the hub is.  No matter how
> we handle it, the result will be pretty inefficient.
>
> I don't know what we should do.  Suggestions, anybody?
>
> 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
--
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 v1 1/2] USB: check port changes before hub runtime suspend for some bug device

[Ming Lei]

On Tue, Sep 25, 2012 at 3:34 AM, Alan Stern  wrote:
>
> Do we really need to?  Remember, you are adding this new code only
> because some Genesys Logic hubs are buggy.  Is there any reason to
> think other devices will have similar bugs?

Hub is a bit special because the event is handled in two steps:

  - report the change by interrupt endpoint
  - handle the change by get status/change and clear the change

So hub can send its remote wakeup if the change hasn't been cleared
even the event has been reported to driver(urb completed).

For other drivers, it is common that one event is handled in just one step:

 - read the transfer_buffer in urb complete and handle the data

So once the urb is completed, the device may not send remote resume
any more. Or just simply don't send it until key is pressed during suspend.

For example on usb HID, see below code in hid_suspend():

if (PMSG_IS_AUTO(message) && test_bit(HID_KEYS_PRESSED,
&usbhid->iofl)) {
 status = -EBUSY;
 goto failed;
}
The comment said that most keyboards won't wakeup if a key is released.

If the urb is just completed during suspend(), don't we need to call
pm_wakeup_event() for it? I don't think there will be remote wakeup
for the handled action. In fact, it can be observed easily if some delay
is added before hid_cancel_delayed_stuff() inside hid_suspend().

>
> As long as the device issues a remote wakeup request at the proper time
> (i.e., whenever it is suspended with wakeup enabled and an event needs
> to be processed), this workaround isn't needed.

Also IMO,  during system sleep, it is better to handle the wakeup
asap even though the remote wakeup can be generated later
suppose someone triggered one event during or before the
driver's suspend(), right?


Thanks
--
Ming Lei
--
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] xhci: Intel Panther Point BEI quirk.

[Andiry Xu]

On Mon, Sep 24, 2012 at 3:35 PM, Sarah Sharp
 wrote:
> On Mon, Sep 24, 2012 at 01:05:08PM -0700, Sarah Sharp wrote:
>> On Mon, Sep 24, 2012 at 03:50:04PM -0400, Alan Stern wrote:
>> > On Mon, 24 Sep 2012, Sarah Sharp wrote:
>> >
>> > > When a device with an isochronous endpoint is behind a hub plugged into
>> > > the Intel Panther Point xHCI host controller, and the driver submits
>> > > multiple frames per URB, the xHCI driver will set the Block Event
>> > > Interrupt (BEI) flag on all but the last TD for the URB.  This causes
>> > > the host controller to place an event on the event ring, but not send an
>> > > interrupt.  When the last TD for the URB completes, BEI is cleared, and
>> > > we get an interrupt for the whole URB.
>> > >
>> > > However, under a Panther Point xHCI host controller, if the parent hub
>> > > is unplugged when one or more events from transfers with BEI set are on
>> > > the event ring, a port status change event is placed on the event ring,
>> > > but no interrupt is generated.  This means URBs stop completing, and the
>> > > USB device disconnect is not noticed.  Something like a USB headset will
>> > > cause mplayer to hang when the device is disconnected.
>> >
>> > Won't that also cause a problem for hot-unplug detection?  How will the
>> > hub driver learn about the unplug event if there's no interrupt?
>>
>> There will be an interrupt if we don't use the BEI flag.  That's what
>> this patch fixes.
>>
>> The BEI flag is only supposed to block isochronous interrupts, and only
>> if there was no error associated with the transfer.  But the Panther
>> Point xHCI host is broken, and the BEI flag stops unrelated interrupts,
>> like the port status change event interrupt.
>>
>> > Also, does this same problem affect bulk transfers, in particular those
>> > using scatter-gather?
>>
>> Bulk transfers, including scatter-gather transfers, are always
>> translated into one TD, and the BEI flag is not used.  AFAIK, this bug
>> only affects transfers with the BEI flag set, so bulk transfers should
>> be fine.
>
> Alan, any more questions on this patch?  I'd like to get it (and a
> couple other bug fixes) pushed to Greg this week before the 3.7 merge
> window.
>

Does this bug only affect isoc devices behind a hub? Can the
workaround be limited so devices connected to the root hub can still
use BEI?

Thanks,
Andiry

> --
> 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
--
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 v1 2/2] USB: set hub's default autosuspend delay as 0

[Ming Lei]

On Tue, Sep 25, 2012 at 3:26 AM, Alan Stern  wrote:
>
> By how much?  That is, how many times is an unbound USB device going to
> be autosuspended?

I mean if we can remove the timer for the unbound devices, why not do it
to decrease wakeup counts for CPU?

In fact, the CPU should be busy(udev things, related application, ...) during
driver unbind, so kind of msleep() inside auto-suspend path may not cause
extra CPU wakeup if the timer is disabled.

But, 2 seconds later, CPU may become idle, so the autosuspend may
introduce some extra wakeups.

>
> Do you have any particular use cases in mind where the extra CPU power
> usage will matter?

Not, just try to decrease unnecessary cpu wakeup by disabling the
autosuspend timer for unbound devices.

>
>> > aren't too many things you can do with an unbound device; lsusb is one
>> > of the most common.
>>
>> Considered 'lsusb' is a low frequency tool for final users, so could we just
>> ignore it for the case?
>
> It depends on the case.  What sort of device do you have in mind?

I mean for final users, 'lsusb' will be seldom used.  Also, non-hub devices
should be woke up only one time by 'lsusb -v' no matter the autosuspend
delay change.

>
>> > On the other hand, we already have a usbcore module parameter for
>> > specifying the default autosuspend delay.  That's what the delay is set
>> > to when a device is first initialized, before it is bound to anything.
>> > Unfortunately the parameter's value is in seconds, not milliseconds, so
>> > we can't set it to any positive value smaller than 1 second.
>>
>> It is a global and can't be used to just change on unbound device.
>
> That's right; it affects every device.
>
> But think of it this way: Suppose there's no driver for a particular
> device.  Its autosuspend delay will get set to the default initially
> and then there won't be any driver to change it.  So when would the
> delay be changed to 0?

How about just keeping the default delay for this device?

>
> I'm not sure that this is something the kernel needs to worry about.
> Userspace can easily set the default delay to 0 whenever it wants to.
> This won't affect devices that are already plugged in... but userspace
> can also set the delays for those devices whenever it wants to.

If we can do it in kernel safely and consistently, looks not necessary to
bother userspace.

Thanks
--
Ming Lei
--
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 v1 2/2] USB: set hub's default autosuspend delay as 0

[Ming Lei]

On Tue, Sep 25, 2012 at 12:29 PM, Ming Lei  wrote:
>
> I mean for final users, 'lsusb' will be seldom used.  Also, non-hub devices
> should be woke up only one time by 'lsusb -v' no matter the autosuspend
> delay change.

For hub devices, we can restore its original 2 seconds delay in open(), and
set the 0 delay back in its release() to eliminate the extra suspend/resume
during 'lsusb -v'.

I will prepare -v2 of the patch for review.

Thanks,
--
Ming Lei
--
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: usb3 fails to write when using usb3 hub in usb3 port

[Adrian Sandu]

On Tue, Sep 25, 2012 at 12:38 AM, Sarah Sharp
 wrote:
> Ok, so 3.4.11 doesn't work, and the log file was from 3.5.

If you want I can provide a 3.4 log...

> If it's not LPM, I'm really out of ideas.  I would suggest getting
> another USB 3.0 hub, but I know that basically the VIA chipset is the
> one out there, so that wouldn't be likely to help.
>
> You might go see if your USB 3.0 hub has a firmware update for it.  I
> know other people were having problems with storage devices that stopped
> responding to a Set Address command after the USB storage driver tried
> to reset it.  Maybe your issue would also go away after a firmware
> update?

I got a gembird and a  manhatan..none show firmware updaes on their
websites (both seem to be using the same via chipset)... manhattan
even mailed replied to my mail:
"Dear sirs, as this hub is not designed for Linux-OS, you may
encounter such driver issues. If you want, you can test it on a
Windows-PC. If you are still experiencing problems, let us know."
--
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 1/2] usb/xhci: release xhci->lock during turning on/off usb port's acpi power resource

[Lan Tianyu]

When setting usb port's acpi power resource, there will be some xhci hub 
requests.
This will cause dead lock since xhci->lock has been held before setting acpi 
power
resource in the xhci_hub_control().

Signed-off-by: Lan Tianyu 
---
 drivers/usb/host/xhci-hub.c |   10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index aa90ad4..25a1a5e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -811,9 +811,12 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 
wValue,
 
temp = usb_acpi_power_manageable(hcd->self.root_hub,
wIndex);
-   if (temp)
+   if (temp) {
+   spin_unlock_irqrestore(&xhci->lock, flags);
usb_acpi_set_power_state(hcd->self.root_hub,
wIndex, true);
+   spin_lock_irqsave(&xhci->lock, flags);
+   }
break;
case USB_PORT_FEAT_RESET:
temp = (temp | PORT_RESET);
@@ -919,9 +922,12 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 
wValue,
 
temp = usb_acpi_power_manageable(hcd->self.root_hub,
wIndex);
-   if (temp)
+   if (temp) {
+   spin_unlock_irqrestore(&xhci->lock, flags);
usb_acpi_set_power_state(hcd->self.root_hub,
wIndex, false);
+   spin_lock_irqsave(&xhci->lock, flags);
+   }
break;
default:
goto error;
-- 
1.7.9.5

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

[PATCH 2/2] usb/xhci: Remove (__force__ __u16) before assigning DeviceRemovable and assign directly.

[Lan Tianyu]

Struct usb_hub_descriptor.ss.DeviceRemovable has been defined as __le16
and (__force__ __u16) doesn't need.

Signed-off-by: Lan Tianyu 
---
 drivers/usb/host/xhci-hub.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 25a1a5e..52a5ec3 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -151,9 +151,8 @@ static void xhci_usb3_hub_descriptor(struct usb_hcd *hcd, 
struct xhci_hcd *xhci,
if (portsc & PORT_DEV_REMOVE)
port_removable |= 1 << (i + 1);
}
-   memset(&desc->u.ss.DeviceRemovable,
-   (__force __u16) cpu_to_le16(port_removable),
-   sizeof(__u16));
+
+   desc->u.ss.DeviceRemovable = cpu_to_le16(port_removable);
 }
 
 static void xhci_hub_descriptor(struct usb_hcd *hcd, struct xhci_hcd *xhci,
-- 
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