Re: [RFC PATCH 2/2] usb: phy: Add platform driver support for ULPI PHYs

[Peter Chen]

On Thu, Sep 24, 2015 at 09:21:16AM +, Subbaraya Sundeep Bhatta wrote:
> > > +uphy->flags);
> > > +
> > > + uphy->usb_phy->set_vbus = usbphy_set_vbus;
> > 
> > When you will call it?
> 
> I am calling it in host.c and otg_fsm.c of Chipidea driver wherever
> regulator_enable/disable is called to control VBUS like below: 
> 
> if (ci->platdata->flags & CI_HDRC_PHY_VBUS_CONTROL) {
> if (enable)
> ci->usb_phy->set_vbus(ci->usb_phy, 1);
> else
> ci->usb_phy->set_vbus(ci->usb_phy, 0);
> }
> 
> After these changes host mode and otg modes works fine
> for my case.
> 

Get it, there are several vbus control ways, I may need to consolidate
them, of cos, this is another topic.

-- 

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

RE: [PATCH 1/3][v4] Documentation: dt: dwc3: Add snps,quirk-frame-length-adjustment property

[RAJESH BHAGAT]

Hi Felipe, 

Any comments on the below [v4] patches?

[PATCH 1/3][v4] Documentation: dt: dwc3: Add snps,quirk-frame-length-adjustment 
property
[PATCH 2/3][v4] drivers: usb: dwc3: Add frame length adjustment quirk
[PATCH 3/3][v4] arm: dts: ls1021a: Add quirk for Erratum A009116

I will be taking forward these patches, as Nikhil has left freescale.

Thanks,
Rajesh Bhagat

-Original Message-
From: Nikhil Badola [mailto:nikhil.bad...@freescale.com] 
Sent: Friday, September 04, 2015 10:15 AM
To: linux-ker...@vger.kernel.org; devicet...@vger.kernel.org; 
linux-usb@vger.kernel.org
Cc: ba...@ti.com; Badola Nikhil-B46172 
Subject: [PATCH 1/3][v4] Documentation: dt: dwc3: Add 
snps,quirk-frame-length-adjustment property

Add snps,quirk-frame-length-adjustment property which provides value for post 
silicon frame length adjustment

Signed-off-by: Nikhil Badola 
---
Changes for v4: None

 Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
b/Documentation/devicetree/bindings/usb/dwc3.txt
index 0815eac..8c7d585 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -40,6 +40,9 @@ Optional properties:
  - snps,hird-threshold: HIRD threshold
  - snps,hsphy_interface: High-Speed PHY interface selection between "utmi" for
UTMI+ and "ulpi" for ULPI when the DWC_USB3_HSPHY_INTERFACE has value 3.
+ - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ
+   register for post-silicon frame length adjustment when the
+   fladj_30mhz_sdbnd signal is invalid or incorrect.
 
 This is usually a subnode to DWC3 glue to which it is connected.
 
--
2.1.0

--
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 1/2] usb: doc: Add bindings for ULPI platform driver

[Subbaraya Sundeep Bhatta]

Hi Peter,

> -Original Message-
> From: Peter Chen [mailto:peter.c...@freescale.com]
> Sent: Thursday, September 24, 2015 2:41 PM
> To: Subbaraya Sundeep Bhatta
> Cc: ba...@ti.com; devicet...@vger.kernel.org; kis...@ti.com;
> gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org; Punnaiah Choudary Kalluri; Subbaraya Sundeep Bhatta;
> linux-arm-ker...@lists.infradead.org
> Subject: Re: [RFC PATCH 1/2] usb: doc: Add bindings for ULPI platform driver
> 
> On Wed, Sep 23, 2015 at 06:24:01PM +0530, Subbaraya Sundeep Bhatta
> wrote:
> > This patch adds binding doc info for generic ULPI PHYs platform
> > driver.
> >
> > Signed-off-by: Subbaraya Sundeep Bhatta 
> > ---
> >  .../devicetree/bindings/usb/ulpi-platform-phy.txt  |   34
> 
> >  1 files changed, 34 insertions(+), 0 deletions(-)  create mode 100644
> > Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> > b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> > new file mode 100644
> > index 000..7b8cbb4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> > @@ -0,0 +1,34 @@
> > +Platform driver for generic ULPI PHYs
> > +
> > +Required properties:
> > +- compatible   : Should be "ulpi-phy"
> > +- reg  : Physical base address and size of the USB
> > + controller registers map to which this PHY
> > + is connected.
> > +- view-port: Should contain viewport register offset of the
> > + USB controller to which this PHY is connected Optional
> > +properties:
> > +- drv-vbus : required if turning VBUS on/off has to be driven
> > + by writing to PHY. This feature depends on board
> > + design.
> > +
> > +Example:
> > +Below example shows the PHY binding for Chipidea USB controller which
> > +has ulpi viewport register at 0x0170
> > +
> > +   usb_phy0: phy0 {
> > +   compatible = "ulpi-phy";
> > +   reg = <0xe0002000 0x1000>;
> > +   view-port = <0x0170>;
> > +   drv-vbus;
> > +   };
> > +
> > +   usb0: usb@e0002000 {
> > +compatible = "chipidea,usb2";
> > +interrupt-parent = <>;
> > +interrupts = <0 21 4>;
> > +reg = <0xe0002000 0x1000>;
> 
> Although just call devm_ioremap twice for the same register region does not
> cause any errors, I am not sure if it will has other potential problems. Cc: 
> arm
> list.

Yes Peter I was also in doubt to call devm_ioremap twice for same register 
region.
devm_ioremap_resource complained hence modified to devm_ioremap. Thanks for
adding arm-list.

Sundeep
> 
> > +phy_type = "ulpi";
> > +   dr_mode = "host";
> > +   usb-phy = <_phy0>;
> > +};
> > --
> > 1.7.1
> >
> 
> --
> 
> Best Regards,
> Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] avr32: fix build failure

[Sudip Mukherjee]

On Wed, Sep 23, 2015 at 07:15:16PM +0200, Hans-Christian Egtvedt wrote:
> Around Wed 23 Sep 2015 21:26:01 +0530 or thereabout, Sudip Mukherjee wrote:
> > On Mon, Sep 21, 2015 at 01:31:44PM +0530, Sudip Mukherjee wrote:
> >> On Mon, Sep 21, 2015 at 09:33:00AM +0200, Hans-Christian Egtvedt wrote:
> >> > Around Mon 21 Sep 2015 12:09:01 +0530 or thereabout, Sudip Mukherjee 
> >> > wrote:
> >> > > On Mon, Sep 21, 2015 at 08:09:42AM +0200, Hans-Christian Egtvedt wrote:
> >> > >> Around Sat 19 Sep 2015 22:42:57 +0530 or thereabout, Sudip Mukherjee 
> >> > >> wrote:
> >> > >> > While building avr32 with allmodconfig, the build used to fail with 
> >> > >> > the
> >> > >> > message:
> >> > >> > error: implicit declaration of function 'pci_iomap'
> >> > >> > error: implicit declaration of function 'pci_iounmap'
> >> > >> 
> >> > >> What has changed recently that start pulling in these? AVR32 does not 
> >> > >> have
> >> > >> PCI at all, and will never have it either. Is this exposing a bug 
> >> > >> somewhere
> >> > >> else?
> >> > > It looks like pci_iomap and pci_iounmap doesnot depend on CONFIG_PCI.
> >> > > So drivers/net/ethernet/via/via-rhine.c is calling these functions even
> >> > > if PCI is disabled. The build log is at:
> >> > > https://travis-ci.org/sudipm-mukherjee/parport/jobs/81127188
> >> > > 
> >> > > You can find a similar discussion at:
> >> > > http://www.linux-mips.org/archives/linux-mips/2013-06/msg00510.html
> >> > 
> >> > If multiple architectures have similar problems, then a more global
> >> > definition of these unused functions could be added.
> >> > 
> >> > Just move the implementation for MIPS into include/asm-generic/io.h ?
> >> > 
> >> > That way we do not have to implement and identical stub for every device 
> >> > not
> >> > supporting PCI.
> >> 
> >> include/asm-generic/io.h is having:
> >> #ifndef CONFIG_GENERIC_IOMAP
> >> struct pci_dev;
> >> extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned
> >>long max);
> >> 
> >> #ifndef pci_iounmap
> >> #define pci_iounmap pci_iounmap
> >> static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p)
> >> {
> >> }
> >> #endif
> >> #endif /* CONFIG_GENERIC_IOMAP */
> >> 
> >> and CONFIG_GENERIC_IOMAP is not defined when I do allmodconfig with
> >> avr32. I have not seen this earlier, maybe in my patch pci_iounmap() was
> >> not required. But pci_iomap is declared as extern so we need have it
> >> somewhere.
> > Hi Hans,
> > Please suggest.
> 
> I can ack the original change, but it will only lead to a cluttered code
> base. The real problem is somewhere else, where it looks like the
> CONFIG_GENERIC_IOMAP symbol assumes PCI is for everybody. I would like if
> somebody addressed that problem instead.

I was having a look and it looks like generic empty pci_iomap and
pci_iounmap are already defined and is used only if CONFIG_PCI is not
defined and CONFIG_GENERIC_PCI_IOMAP is defined. So instead of adding
empty functions to each architecture where PCI is not supported maybe
the driver can have a dependency on CONFIG_GENERIC_PCI_IOMAP.
I have submitted the patch and you are in CC list. arch/sh had a similar
problem with the same driver.

regards
sudip
--
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 2/2] usb: phy: Add platform driver support for ULPI PHYs

[Subbaraya Sundeep Bhatta]

Hi Peter,

> -Original Message-
> From: Peter Chen [mailto:peter.c...@freescale.com]
> Sent: Thursday, September 24, 2015 2:37 PM
> To: Subbaraya Sundeep Bhatta
> Cc: ba...@ti.com; devicet...@vger.kernel.org; kis...@ti.com;
> gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org; Punnaiah Choudary Kalluri; Subbaraya Sundeep Bhatta
> Subject: Re: [RFC PATCH 2/2] usb: phy: Add platform driver support for ULPI
> PHYs
> 
> On Wed, Sep 23, 2015 at 06:24:33PM +0530, Subbaraya Sundeep Bhatta
> wrote:
> > Based on board design USB controller needs explicit software access to
> > ULPI PHY for controlling VBUS. This patch adds platform driver support
> > for generic ULPI PHYs and provides a USB2 PHY device to controllers.
> >
> > Signed-off-by: Subbaraya Sundeep Bhatta 
> > ---
> >  drivers/usb/phy/Kconfig |   12 +++
> >  drivers/usb/phy/Makefile|1 +
> >  drivers/usb/phy/phy-platform-ulpi.c |  143
> > +++
> >  3 files changed, 156 insertions(+), 0 deletions(-)  create mode
> > 100644 drivers/usb/phy/phy-platform-ulpi.c
> >
> > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index
> > 7d3beee..2956ad4 100644
> > --- a/drivers/usb/phy/Kconfig
> > +++ b/drivers/usb/phy/Kconfig
> > @@ -201,6 +201,18 @@ config USB_RCAR_PHY
> >   To compile this driver as a module, choose M here: the
> >   module will be called phy-rcar-usb.
> >
> > +config USB_PLATFORM_ULPI_PHY
> > +   tristate "Platform driver support for ULPI PHYs"
> > +   depends on ARCH_ZYNQ || COMPILE_TEST
> 
> If you want this as a common driver, drop platform dependency please.

Ok I will remove depends on.
> 
> > +   select USB_PHY
> > +   select USB_ULPI_VIEWPORT
> > +   help
> > + Say Y here to add support for the Platform driver for ULPI PHYs.
> > + This adds platform driver support for all generic ULPI PHYs and is
> > + typically used if usb controller driver needs explicit access to PHY.
> > +
> > + To compile this driver as a module, choose M here.
> > +
> >  config USB_ULPI
> > bool "Generic ULPI Transceiver Driver"
> > depends on ARM || ARM64
> > diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index
> > 19c0dcc..8431b6b 100644
> > --- a/drivers/usb/phy/Makefile
> > +++ b/drivers/usb/phy/Makefile
> > @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_QCOM_8X16_PHY)   += phy-
> qcom-8x16-usb.o
> >  obj-$(CONFIG_USB_MV_OTG)   += phy-mv-usb.o
> >  obj-$(CONFIG_USB_MXS_PHY)  += phy-mxs-usb.o
> >  obj-$(CONFIG_USB_RCAR_PHY) += phy-rcar-usb.o
> > +obj-$(CONFIG_USB_PLATFORM_ULPI_PHY)+= phy-platform-ulpi.o
> >  obj-$(CONFIG_USB_ULPI) += phy-ulpi.o
> >  obj-$(CONFIG_USB_ULPI_VIEWPORT)+= phy-ulpi-viewport.o
> >  obj-$(CONFIG_KEYSTONE_USB_PHY) += phy-keystone.o
> > diff --git a/drivers/usb/phy/phy-platform-ulpi.c
> > b/drivers/usb/phy/phy-platform-ulpi.c
> > new file mode 100644
> > index 000..fb89363
> > --- /dev/null
> > +++ b/drivers/usb/phy/phy-platform-ulpi.c
> > @@ -0,0 +1,143 @@
> > +/*
> > + * Platform driver for generic ULPI PHYs.
> > + *
> > + * Copyright (C) 2015 Xilinx, Inc.
> > + *
> > + * Author: Subbaraya Sundeep 
> > + *
> > + * 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.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/**
> > + * struct ulpi_phy - The ULPI PHY
> > + * @usb_phy: pointer to usb phy
> > + * @regs: base address of USB controller to which PHY is connected
> > + * @vp_offset: ulpi viewport register offset of USB controller
> > + * @flags: initial required settings of PHY  */
> > +
> > +struct ulpi_phy {
> > +   struct usb_phy  *usb_phy;
> > +   void __iomem *regs;
> > +   unsigned int vp_offset;
> > +   unsigned int flags;
> > +};
> > +
> > +/**
> > + * usbphy_set_vbus - Sets VBUS by writing to PHY.
> > + * @phy: pointer to PHY
> > + * @on: 1 - turn on VBUS
> > + * 0 - turn off VBUS
> > + * Return: 0 for success and error value on failure  */ static int
> > +usbphy_set_vbus(struct usb_phy *phy, int on) {
> > +   unsigned int flags = usb_phy_io_read(phy, ULPI_OTG_CTRL);
> > +
> > +   flags &= ~(ULPI_OTG_CTRL_DRVVBUS |
> ULPI_OTG_CTRL_DRVVBUS_EXT);
> > +
> > +   if (on) {
> > +   if (phy->flags & ULPI_OTG_DRVVBUS)
> > +   flags |= ULPI_OTG_CTRL_DRVVBUS;
> > +
> > +   if (phy->flags & ULPI_OTG_DRVVBUS_EXT)
> > +   flags |= ULPI_OTG_CTRL_DRVVBUS_EXT;
> > +   }
> > +
> > +   return usb_phy_io_write(phy, flags, ULPI_OTG_CTRL); }
> > +
> > +/**
> > + * ulpi_phy_probe - The device probe function for driver initialization.
> > + * @pdev: pointer 

Re: [RFC PATCH 1/2] usb: doc: Add bindings for ULPI platform driver

[Peter Chen]

On Wed, Sep 23, 2015 at 06:24:01PM +0530, Subbaraya Sundeep Bhatta wrote:
> This patch adds binding doc info for generic ULPI PHYs
> platform driver.
> 
> Signed-off-by: Subbaraya Sundeep Bhatta 
> ---
>  .../devicetree/bindings/usb/ulpi-platform-phy.txt  |   34 
> 
>  1 files changed, 34 insertions(+), 0 deletions(-)
>  create mode 100644 
> Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt 
> b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> new file mode 100644
> index 000..7b8cbb4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> @@ -0,0 +1,34 @@
> +Platform driver for generic ULPI PHYs
> +
> +Required properties:
> +- compatible : Should be "ulpi-phy"
> +- reg: Physical base address and size of the USB
> +   controller registers map to which this PHY
> +   is connected.
> +- view-port  : Should contain viewport register offset of the
> +   USB controller to which this PHY is connected
> +Optional properties:
> +- drv-vbus   : required if turning VBUS on/off has to be driven
> +   by writing to PHY. This feature depends on board
> +   design.
> +
> +Example:
> +Below example shows the PHY binding for Chipidea USB controller which has
> +ulpi viewport register at 0x0170
> +
> + usb_phy0: phy0 {
> + compatible = "ulpi-phy";
> + reg = <0xe0002000 0x1000>;
> + view-port = <0x0170>;
> + drv-vbus;
> + };
> +
> + usb0: usb@e0002000 {
> +compatible = "chipidea,usb2";
> +interrupt-parent = <>;
> +interrupts = <0 21 4>;
> +reg = <0xe0002000 0x1000>;

Although just call devm_ioremap twice for the same register region
does not cause any errors, I am not sure if it will has other
potential problems. Cc: arm list.

> +phy_type = "ulpi";
> + dr_mode = "host";
> + usb-phy = <_phy0>;
> +};
> -- 
> 1.7.1
> 

-- 

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

Re: [RFC PATCH 2/2] usb: phy: Add platform driver support for ULPI PHYs

[Peter Chen]

On Wed, Sep 23, 2015 at 06:24:33PM +0530, Subbaraya Sundeep Bhatta wrote:
> Based on board design USB controller needs explicit software
> access to ULPI PHY for controlling VBUS. This patch adds platform
> driver support for generic ULPI PHYs and provides a USB2 PHY device
> to controllers.
> 
> Signed-off-by: Subbaraya Sundeep Bhatta 
> ---
>  drivers/usb/phy/Kconfig |   12 +++
>  drivers/usb/phy/Makefile|1 +
>  drivers/usb/phy/phy-platform-ulpi.c |  143 
> +++
>  3 files changed, 156 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/usb/phy/phy-platform-ulpi.c
> 
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 7d3beee..2956ad4 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -201,6 +201,18 @@ config USB_RCAR_PHY
> To compile this driver as a module, choose M here: the
> module will be called phy-rcar-usb.
>  
> +config USB_PLATFORM_ULPI_PHY
> + tristate "Platform driver support for ULPI PHYs"
> + depends on ARCH_ZYNQ || COMPILE_TEST

If you want this as a common driver, drop platform dependency please.

> + select USB_PHY
> + select USB_ULPI_VIEWPORT
> + help
> +   Say Y here to add support for the Platform driver for ULPI PHYs.
> +   This adds platform driver support for all generic ULPI PHYs and is
> +   typically used if usb controller driver needs explicit access to PHY.
> +   
> +   To compile this driver as a module, choose M here.
> +
>  config USB_ULPI
>   bool "Generic ULPI Transceiver Driver"
>   depends on ARM || ARM64
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 19c0dcc..8431b6b 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_QCOM_8X16_PHY) += phy-qcom-8x16-usb.o
>  obj-$(CONFIG_USB_MV_OTG) += phy-mv-usb.o
>  obj-$(CONFIG_USB_MXS_PHY)+= phy-mxs-usb.o
>  obj-$(CONFIG_USB_RCAR_PHY)   += phy-rcar-usb.o
> +obj-$(CONFIG_USB_PLATFORM_ULPI_PHY)  += phy-platform-ulpi.o
>  obj-$(CONFIG_USB_ULPI)   += phy-ulpi.o
>  obj-$(CONFIG_USB_ULPI_VIEWPORT)  += phy-ulpi-viewport.o
>  obj-$(CONFIG_KEYSTONE_USB_PHY)   += phy-keystone.o
> diff --git a/drivers/usb/phy/phy-platform-ulpi.c 
> b/drivers/usb/phy/phy-platform-ulpi.c
> new file mode 100644
> index 000..fb89363
> --- /dev/null
> +++ b/drivers/usb/phy/phy-platform-ulpi.c
> @@ -0,0 +1,143 @@
> +/*
> + * Platform driver for generic ULPI PHYs.
> + *
> + * Copyright (C) 2015 Xilinx, Inc.
> + *
> + * Author: Subbaraya Sundeep 
> + *
> + * 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.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/**
> + * struct ulpi_phy - The ULPI PHY
> + * @usb_phy: pointer to usb phy
> + * @regs: base address of USB controller to which PHY is connected
> + * @vp_offset: ulpi viewport register offset of USB controller
> + * @flags: initial required settings of PHY
> + */
> +
> +struct ulpi_phy {
> + struct usb_phy  *usb_phy;
> + void __iomem *regs;
> + unsigned int vp_offset;
> + unsigned int flags;
> +};
> +
> +/**
> + * usbphy_set_vbus - Sets VBUS by writing to PHY.
> + * @phy: pointer to PHY
> + * @on: 1 - turn on VBUS
> + *   0 - turn off VBUS
> + * Return: 0 for success and error value on failure
> + */
> +static int usbphy_set_vbus(struct usb_phy *phy, int on)
> +{
> + unsigned int flags = usb_phy_io_read(phy, ULPI_OTG_CTRL);
> +
> + flags &= ~(ULPI_OTG_CTRL_DRVVBUS | ULPI_OTG_CTRL_DRVVBUS_EXT);
> +
> + if (on) {
> + if (phy->flags & ULPI_OTG_DRVVBUS)
> + flags |= ULPI_OTG_CTRL_DRVVBUS;
> +
> + if (phy->flags & ULPI_OTG_DRVVBUS_EXT)
> + flags |= ULPI_OTG_CTRL_DRVVBUS_EXT;
> + }
> +
> + return usb_phy_io_write(phy, flags, ULPI_OTG_CTRL);
> +}
> +
> +/**
> + * ulpi_phy_probe - The device probe function for driver initialization.
> + * @pdev: pointer to the platform device structure.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int ulpi_phy_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct resource *res;
> + struct ulpi_phy *uphy;
> + bool flag;
> + int ret;
> +
> + uphy = devm_kzalloc(>dev, sizeof(*uphy), GFP_KERNEL);
> + if (!uphy)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + uphy->regs = devm_ioremap(>dev, res->start, resource_size(res));
> + if (IS_ERR(uphy->regs))
> + return PTR_ERR(uphy->regs);

Re: [RFC][PATCH] Add spurious wakeup quirk for Lynxpoint controllers

[Mathias Nyman]

On 18.09.2015 23:25, Oliver Neukum wrote:

On Fri, 2015-09-18 at 09:56 -0700, Laura Abbott wrote:

Would you rather see a revert of the patch you gave rather than a new
one re-introducing the flag?


We need a big fat comment here saying that different tests should
different results and the quirk is needed for LynxPoint. That
suggests a new patch.



I'll take the patch from Laura and add the big fat warning to it myself

-Mathias

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

Re: [PATCH v4] usb: of: add an api to get dr_mode by the phy node

[Hans de Goede]

Hi,

On 23-09-15 22:59, Bin Liu wrote:

Hi,

On 09/23/2015 02:53 PM, Hans de Goede wrote:

Hi,

On 23-09-15 19:10, Bin Liu wrote:

Hi,

On 09/22/2015 04:18 PM, Felipe Balbi wrote:

On Tue, Sep 22, 2015 at 02:31:18PM -0500, Bin Liu wrote:

Hi,

On 09/22/2015 09:40 AM, Felipe Balbi wrote:

On Mon, Sep 21, 2015 at 10:50:56AM -0500, Bin Liu wrote:

Some USB phy drivers have different handling for the controller in
each
dr_mode. But the phy driver does not have visibility to the
dr_mode of
the controller.

This adds an api to return the dr_mode of the controller which
associates the given phy node.

Signed-off-by: Bin Liu 



doesn't apply anymore. Probably because of Heikki's series which I
just
added to testing/next.

Please rebase there.


I have to rewrite my patch. Before Heikki's patch
of_usb_get_dr_mode() takes
parameter 'struct *device_node', but now usb_get_dr_mode() takes
parameter
'struct *device'. The logic in my patch iterates over of nodes, I am
not
sure how to get the 'struct *device' from a of node yet...


okay.


There is no way to get the 'struct *device' to the controller in the
phy driver, because the controller device might not be registered yet
by the time the phy probe is called.

So I have to put back the implementation of the removed
of_usb_get_dr_mode() into this new of_usb_get_dr_mode_by_phy()
function. Please let me know if this is acceptable then I will send
the v5.


Sounds to me like it is better to revert the API change / removal of
of_usb_get_dr_mode()
as a separate patch and then stick with your v4 patch.


+1


If you agree, then the best way to do this is probably to send a patch
series with the actual revert + your v4 patch. Probably best to call
that series v5.

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 v2 2/3] usb: gadget: f_midi: free usb request when done

[Clemens Ladisch]

Peter Chen wrote:
> I can't make my aplaymidi to receive data

> # aplaymidi
> open /dev/snd/seq failed: No such file or directory

modprobe snd-seq

There are mechanisms to load it automatically, but your embedded system
might not bother about any of them.  Or CONFIG_SND_SEQUENCER isn't
enabled at all.

In any case, raw MIDI (with the amidi tool) should work.


Regards,
Clemens
--
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: Overly conservative xHCI bandwidth estimation

[Krzysztof Opasiak]

On 09/23/2015 10:08 PM, Steinar H. Gunderson wrote:

Hi again,

I'm trying to figure out why my xHCI controller refuses me to run two very
similar video cards at the same time. I'm not sure if this is a bug or if I'm
just misunderstanding, so let me see if I understand this right:

The interface of each card has two relevant alternates, 2 and 4. Number 2 has
bInterval=1, bMaxBurst=11, Mult=2, which from what I can see should be
11*(2+1)*1024 byte / 125µs, or 2.16 Gbit/sec. Number 4 has bInterval=1,
bMaxBurst=8, Mult=1, which should be 8*(1+1)*1024 byte / 125 µs, or 1.04 
Gbit/sec.

I can run one card in alternates 2 or 4, no problem. But if I put another
card in the other USB port (my Lenovo X240 has one on each side) and try to
activate that too, I get

   [  287.919173] usb 2-2: Not enough bandwidth for altsetting 4

I've tried on another machine and it works fine there, so my code should,
at least on the surface of it, be fine. I've verified that the error is
COMP_BW_ERR (not COMP_2ND_BW_ERR).

My xHCI controller is 8086:9c31 (I believe this is Intel). lsusb -v with both
devices plugged in is attached. I'm running 4.3-rc2. There are no external
USB hubs in the mix.

So this is my question: Why can I run one card at 2.16 Gbit/sec but not two
at 1.04 Gbit/sec each? (Shouldn't the USB3 bus be 5 Gbit/sec?) Is there any
way to debug these issues, or is the controller just deciding and unwilling
to give out any information? Is the information of the bandwidth domains/
bus instances available anywhere? Am I misunderstanding something
fundamentally?


Well it's not so simple. I'm not sure if this is exactly your problem 
but it may help you.


Let's start from the beginning. Your device use ISO endpoints which 
means that host allocates specific amount of bandwidth on the bus. More 
over, interfaces in your devices has many alternate settings. Probably 
each of them reserves different amount of bandwidth. When you connect 
your first device, driver selects the highest possible quality and 
allocate maximum bandwidth. When you connect second device driver tries 
to select altsetting for which there is enough bandwidth left but as you 
see this fail. So we get:


1st card - 2.16 bandwidth
2nd card - not enough bandwidth left even for lowest quality

what you should try is to force driver of the first card to use lower 
quality and allocate less bandwidth (this means choose other altsetting 
of interface) then you will get:


1st card - x bandwidth (x < 2.16)
2nd card - y bandwidth (I think that USB 3.0 should provide enough 
bandwidth for those 2 devices at lowest quality)


That's just my guess, no guarantee bud it may be worth to try.

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

Re: Overly conservative xHCI bandwidth estimation

[Krzysztof Opasiak]

On 09/24/2015 03:14 PM, Steinar H. Gunderson wrote:

On Thu, Sep 24, 2015 at 03:09:55PM +0200, Krzysztof Opasiak wrote:

But still the problem may exist. Is the 2.16 GBit bandwidth for 4th altset?


No. The 2nd altset is 2.16, the 4th altset is 1.04. Unless my calculations
are wrong.


How do you do your calculations?




Remember that according to USB spec not whole bandwidth can be used by iso
transfers (in usb 2.0 the limit is 80%, don't know how it looks in 3.0). So
if 2.16 is lowest possible bandwidth for your device you have 2.16 x 2 =
4.32 but 80% * 5 = 4...


But total bandwidth shouldn't be the issue as long as the 2nd altset
(by itself) works fine, no? It's _more_ than twice that of the 4th.



Definitely agree. Didn't get this from your previous messages.
--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Thu, Sep 24, 2015 at 03:35:10PM +0200, Krzysztof Opasiak wrote:
>> No. The 2nd altset is 2.16, the 4th altset is 1.04. Unless my calculations
>> are wrong.
> How do you do your calculations?

Like I said in my initial email:

>>> The interface of each card has two relevant alternates, 2 and 4. Number 2 
>>> has
>>> bInterval=1, bMaxBurst=11, Mult=2, which from what I can see should be
>>> 11*(2+1)*1024 byte / 125µs, or 2.16 Gbit/sec. Number 4 has bInterval=1,
>>> bMaxBurst=8, Mult=1, which should be 8*(1+1)*1024 byte / 125 µs, or 1.04 
>>> Gbit/sec.

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

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Thu, Sep 24, 2015 at 03:09:55PM +0200, Krzysztof Opasiak wrote:
> But still the problem may exist. Is the 2.16 GBit bandwidth for 4th altset?

No. The 2nd altset is 2.16, the 4th altset is 1.04. Unless my calculations
are wrong.

> Remember that according to USB spec not whole bandwidth can be used by iso
> transfers (in usb 2.0 the limit is 80%, don't know how it looks in 3.0). So
> if 2.16 is lowest possible bandwidth for your device you have 2.16 x 2 =
> 4.32 but 80% * 5 = 4...

But total bandwidth shouldn't be the issue as long as the 2nd altset
(by itself) works fine, no? It's _more_ than twice that of the 4th.

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

Re: [PATCH v4] usb: of: add an api to get dr_mode by the phy node

[Bin Liu]

Hi Felipe,

On 09/24/2015 03:37 AM, Hans de Goede wrote:

Hi,

On 23-09-15 22:59, Bin Liu wrote:

Hi,

On 09/23/2015 02:53 PM, Hans de Goede wrote:

Hi,

On 23-09-15 19:10, Bin Liu wrote:

Hi,

On 09/22/2015 04:18 PM, Felipe Balbi wrote:

On Tue, Sep 22, 2015 at 02:31:18PM -0500, Bin Liu wrote:

Hi,

On 09/22/2015 09:40 AM, Felipe Balbi wrote:

On Mon, Sep 21, 2015 at 10:50:56AM -0500, Bin Liu wrote:

Some USB phy drivers have different handling for the controller in
each
dr_mode. But the phy driver does not have visibility to the
dr_mode of
the controller.

This adds an api to return the dr_mode of the controller which
associates the given phy node.

Signed-off-by: Bin Liu 



doesn't apply anymore. Probably because of Heikki's series which I
just
added to testing/next.

Please rebase there.


I have to rewrite my patch. Before Heikki's patch
of_usb_get_dr_mode() takes
parameter 'struct *device_node', but now usb_get_dr_mode() takes
parameter
'struct *device'. The logic in my patch iterates over of nodes, I am
not
sure how to get the 'struct *device' from a of node yet...


okay.


There is no way to get the 'struct *device' to the controller in the
phy driver, because the controller device might not be registered yet
by the time the phy probe is called.

So I have to put back the implementation of the removed
of_usb_get_dr_mode() into this new of_usb_get_dr_mode_by_phy()
function. Please let me know if this is acceptable then I will send
the v5.


Sounds to me like it is better to revert the API change / removal of
of_usb_get_dr_mode()
as a separate patch and then stick with your v4 patch.


+1


If you agree, then the best way to do this is probably to send a patch
series with the actual revert + your v4 patch. Probably best to call
that series v5.


Since Heikki's patch is only in your testing/next branch, do you want to 
just drop that patch and take my v4 patch, or you want me to send the 
revert patch?


Thanks,
-Bin.



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: AM3517 usb host issue

[Ben Dooks]

On 22/05/15 14:50, Felipe Balbi wrote:
> Hi,
> 
> On Fri, May 22, 2015 at 11:04:33AM +0300, Ben Dooks wrote:
>> I am trying to get the full-speed USB host working on an custom 
>> AM3517 device with the 3.18.12 kernel. The hardware works (a 
>> 2.6.37 kernel has been used for testing).
>> 
>> Does anyone have any experience of 3.18 (or similarly recent 
>> kernel on an AM3517 system) or have any pointers as where to 
>> start debugging? The ti-linux-3.14.y does not have any patches 
>> that aren't applied to the usb on 3.18.13.
>> 
>> The cpu port 1 is connected by a TI TUSB1106 usb transceiver
>> that is directly connected to a full-speed hub (TI USB2046) hub
>> so the OHCI driver is the only one in use.
>> 
>> Note, the ohci-omap3 is loaded as a module as this is how their 
>> user application expects to be able to shut down usb when it
>> does not need it.
>> 
>> The device tree configuration for the usb host:
> 
> and what exactly doesn't work ? That old OHCI driver hasn't been 
> touched in years, it's no surprise that it stopped working :-(
> 
> Anyway, what exactly doesn't work ? No device enumerates ? Do you 
> get any IRQs by plugging a new device in ?

I just found this in my backlog, and thought I should follow up with
the issue.

It turns out that even if we're not using the EHCI unit, the 120m
functional clock needs to be enabled. This may be down to an internal
routing of port signals, or the USB front-end needing it.

I've added a local boolean fix to enable the clock, and will post it
as soon as we've had a chance to review and document.

-- 
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Thu, Sep 24, 2015 at 02:09:41PM +0200, Krzysztof Opasiak wrote:
> Let's start from the beginning. Your device use ISO endpoints which means
> that host allocates specific amount of bandwidth on the bus. More over,
> interfaces in your devices has many alternate settings. Probably each of
> them reserves different amount of bandwidth. When you connect your first
> device, driver selects the highest possible quality and allocate maximum
> bandwidth. When you connect second device driver tries to select altsetting
> for which there is enough bandwidth left but as you see this fail. So we
> get:

I'm sorry, I should have mentioned an important detail here. There is no
kernel driver for my card; I drive it from userspace with libusb-1.0. So I'm
the one setting the alternates. The cards start out in alternate 0, so the
scenario you're outlining shouldn't exist; and indeed, the setting the first
card to 4 (the low-bandwidth alternate) works just fine, it's setting the
second one that's the problem.

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

Re: Overly conservative xHCI bandwidth estimation

[Krzysztof Opasiak]

On 09/24/2015 02:50 PM, Steinar H. Gunderson wrote:

On Thu, Sep 24, 2015 at 02:09:41PM +0200, Krzysztof Opasiak wrote:

Let's start from the beginning. Your device use ISO endpoints which means
that host allocates specific amount of bandwidth on the bus. More over,
interfaces in your devices has many alternate settings. Probably each of
them reserves different amount of bandwidth. When you connect your first
device, driver selects the highest possible quality and allocate maximum
bandwidth. When you connect second device driver tries to select altsetting
for which there is enough bandwidth left but as you see this fail. So we
get:


I'm sorry, I should have mentioned an important detail here. There is no
kernel driver for my card; I drive it from userspace with libusb-1.0. So I'm
the one setting the alternates. The cards start out in alternate 0, so the
scenario you're outlining shouldn't exist; and indeed, the setting the first
card to 4 (the low-bandwidth alternate) works just fine, it's setting the
second one that's the problem.



But still the problem may exist. Is the 2.16 GBit bandwidth for 4th altset?

Remember that according to USB spec not whole bandwidth can be used by 
iso transfers (in usb 2.0 the limit is 80%, don't know how it looks in 
3.0). So if 2.16 is lowest possible bandwidth for your device you have 
2.16 x 2 = 4.32 but 80% * 5 = 4...


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

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Thu, Sep 24, 2015 at 10:17:19AM -0400, Alan Stern wrote:
> However, none of this answers the question of why you can use both
> cards on a different machine but not on yours.  It comes down to the
> implementations of the xHCI controller chips.  In USB-3, bandwidth
> allocation is handled by firmware running on the controller, not by the
> operating system's driver.  The driver presents a series of endpoints
> with all their bandwidth requirements to the controller, and the
> controller either accepts it or rejects it.

OK, I feared as much. The other machine also has an Intel controller,
but as far as I know, a newer one (and the PCI ID is different -- 8086:9cb1).

> It doesn't give any explanation for its decision, and as far as I know, it
> doesn't provide any information about the details of how it allocates the
> bandwidth.

I thought I saw something in the xHCI spec about enumerating the bandwidth
domains to try to get some more insight in what the topology looks like,
but I guess I misunderstood it? (It all wasn't very clear to me.)

I assume there's no way I can lie to the chip? Like, if I know for a fact
that the card will send less data than the alternate claims (like,
I'm using a video mode that will require only a few hundred megabits/second
in practice, but even the lowest alternate claims >1 Gbit/sec).

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

[PATCH 1/2] usb: gadget: f_sourcesink: fix function params handling

[Robert Baldyga]

Move function parameters to struct f_sourcesink to make them per instance
instead of having them as global variables. Since we can have multiple
instances of USB function we also want to have separate set of parameters
for each instance.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_sourcesink.c | 120 +++--
 1 file changed, 62 insertions(+), 58 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index 1353465..128abfb 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -50,6 +50,13 @@ struct f_sourcesink {
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
int cur_alt;
+
+   unsigned pattern;
+   unsigned isoc_interval;
+   unsigned isoc_maxpacket;
+   unsigned isoc_mult;
+   unsigned isoc_maxburst;
+   unsigned buflen;
 };
 
 static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
@@ -57,13 +64,6 @@ static inline struct f_sourcesink *func_to_ss(struct 
usb_function *f)
return container_of(f, struct f_sourcesink, function);
 }
 
-static unsigned pattern;
-static unsigned isoc_interval;
-static unsigned isoc_maxpacket;
-static unsigned isoc_mult;
-static unsigned isoc_maxburst;
-static unsigned buflen;
-
 /*-*/
 
 static struct usb_interface_descriptor source_sink_intf_alt0 = {
@@ -298,7 +298,9 @@ static struct usb_gadget_strings *sourcesink_strings[] = {
 
 static inline struct usb_request *ss_alloc_ep_req(struct usb_ep *ep, int len)
 {
-   return alloc_ep_req(ep, len, buflen);
+   struct f_sourcesink *ss = ep->driver_data;
+
+   return alloc_ep_req(ep, len, ss->buflen);
 }
 
 void free_ep_req(struct usb_ep *ep, struct usb_request *req)
@@ -357,22 +359,22 @@ autoconf_fail:
goto autoconf_fail;
 
/* sanity check the isoc module parameters */
-   if (isoc_interval < 1)
-   isoc_interval = 1;
-   if (isoc_interval > 16)
-   isoc_interval = 16;
-   if (isoc_mult > 2)
-   isoc_mult = 2;
-   if (isoc_maxburst > 15)
-   isoc_maxburst = 15;
+   if (ss->isoc_interval < 1)
+   ss->isoc_interval = 1;
+   if (ss->isoc_interval > 16)
+   ss->isoc_interval = 16;
+   if (ss->isoc_mult > 2)
+   ss->isoc_mult = 2;
+   if (ss->isoc_maxburst > 15)
+   ss->isoc_maxburst = 15;
 
/* fill in the FS isoc descriptors from the module parameters */
-   fs_iso_source_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
-   1023 : isoc_maxpacket;
-   fs_iso_source_desc.bInterval = isoc_interval;
-   fs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
-   1023 : isoc_maxpacket;
-   fs_iso_sink_desc.bInterval = isoc_interval;
+   fs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
+   1023 : ss->isoc_maxpacket;
+   fs_iso_source_desc.bInterval = ss->isoc_interval;
+   fs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
+   1023 : ss->isoc_maxpacket;
+   fs_iso_sink_desc.bInterval = ss->isoc_interval;
 
/* allocate iso endpoints */
ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, _iso_source_desc);
@@ -394,8 +396,8 @@ no_iso:
ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
}
 
-   if (isoc_maxpacket > 1024)
-   isoc_maxpacket = 1024;
+   if (ss->isoc_maxpacket > 1024)
+   ss->isoc_maxpacket = 1024;
 
/* support high speed hardware */
hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
@@ -406,15 +408,15 @@ no_iso:
 * We assume that the user knows what they are doing and won't
 * give parameters that their UDC doesn't support.
 */
-   hs_iso_source_desc.wMaxPacketSize = isoc_maxpacket;
-   hs_iso_source_desc.wMaxPacketSize |= isoc_mult << 11;
-   hs_iso_source_desc.bInterval = isoc_interval;
+   hs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
+   hs_iso_source_desc.wMaxPacketSize |= ss->isoc_mult << 11;
+   hs_iso_source_desc.bInterval = ss->isoc_interval;
hs_iso_source_desc.bEndpointAddress =
fs_iso_source_desc.bEndpointAddress;
 
-   hs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket;
-   hs_iso_sink_desc.wMaxPacketSize |= isoc_mult << 11;
-   hs_iso_sink_desc.bInterval = isoc_interval;
+   hs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
+   hs_iso_sink_desc.wMaxPacketSize |= ss->isoc_mult << 11;
+   hs_iso_sink_desc.bInterval = ss->isoc_interval;

[PATCH 0/2] usb: gadget: fixes for f_sourcesink and f_loopback

[Robert Baldyga]

Hi Felipe,

There are two bugfix patches. The problem was that function parameters
in f_sourcesink and f_loopback were stored in global variables, and in
result setting parameters in one function instance caused overwriting
them in all other instances. This patchset fixes this problem by replacing
global variables with per instance parameters.

Best regards,
Robert Baldyga

Robert Baldyga (2):
  usb: gadget: f_sourcesink: fix function params handling
  usb: gadget: f_loopfack: fix function params handling

 drivers/usb/gadget/function/f_loopback.c   |  22 +++---
 drivers/usb/gadget/function/f_sourcesink.c | 120 +++--
 2 files changed, 74 insertions(+), 68 deletions(-)

-- 
1.9.1

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

[PATCH 2/2] usb: gadget: f_loopfack: fix function params handling

[Robert Baldyga]

Move function parameters to struct f_loopback to make them per instance
instead of having them as global variables. Since we can have multiple
instances of USB function we also want to have separate set of parameters
for each instance.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_loopback.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c 
b/drivers/usb/gadget/function/f_loopback.c
index 300e601..8d2dac8 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -34,6 +34,9 @@ struct f_loopback {
 
struct usb_ep   *in_ep;
struct usb_ep   *out_ep;
+
+   unsigned qlen;
+   unsigned buflen;
 };
 
 static inline struct f_loopback *func_to_loop(struct usb_function *f)
@@ -41,9 +44,6 @@ static inline struct f_loopback *func_to_loop(struct 
usb_function *f)
return container_of(f, struct f_loopback, function);
 }
 
-static unsigned qlen;
-static unsigned buflen;
-
 /*-*/
 
 static struct usb_interface_descriptor loopback_intf = {
@@ -251,7 +251,7 @@ static void loopback_complete(struct usb_ep *ep, struct 
usb_request *req)
}
 
/* queue the buffer for some later OUT packet */
-   req->length = buflen;
+   req->length = loop->buflen;
status = usb_ep_queue(ep, req, GFP_ATOMIC);
if (status == 0)
return;
@@ -288,7 +288,9 @@ static void disable_loopback(struct f_loopback *loop)
 
 static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
 {
-   return alloc_ep_req(ep, len, buflen);
+   struct f_loopback   *loop = ep->driver_data;
+
+   return alloc_ep_req(ep, len, loop->buflen);
 }
 
 static int enable_endpoint(struct usb_composite_dev *cdev, struct f_loopback 
*loop,
@@ -315,7 +317,7 @@ static int enable_endpoint(struct usb_composite_dev *cdev, 
struct f_loopback *lo
 * we buffer at most 'qlen' transfers; fewer if any need more
 * than 'buflen' bytes each.
 */
-   for (i = 0; i < qlen && result == 0; i++) {
+   for (i = 0; i < loop->qlen && result == 0; i++) {
req = lb_alloc_ep_req(ep, 0);
if (!req)
goto fail1;
@@ -388,10 +390,10 @@ static struct usb_function *loopback_alloc(struct 
usb_function_instance *fi)
lb_opts->refcnt++;
mutex_unlock(_opts->lock);
 
-   buflen = lb_opts->bulk_buflen;
-   qlen = lb_opts->qlen;
-   if (!qlen)
-   qlen = 32;
+   loop->buflen = lb_opts->bulk_buflen;
+   loop->qlen = lb_opts->qlen;
+   if (!loop->qlen)
+   loop->qlen = 32;
 
loop->function.name = "loopback";
loop->function.bind = loopback_bind;
-- 
1.9.1

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

Re: Overly conservative xHCI bandwidth estimation

[Alan Stern]

On Wed, 23 Sep 2015, Steinar H. Gunderson wrote:

> Hi again,
> 
> I'm trying to figure out why my xHCI controller refuses me to run two very
> similar video cards at the same time. I'm not sure if this is a bug or if I'm
> just misunderstanding, so let me see if I understand this right:
> 
> The interface of each card has two relevant alternates, 2 and 4. Number 2 has 
> bInterval=1, bMaxBurst=11, Mult=2, which from what I can see should be
> 11*(2+1)*1024 byte / 125µs, or 2.16 Gbit/sec. Number 4 has bInterval=1,
> bMaxBurst=8, Mult=1, which should be 8*(1+1)*1024 byte / 125 µs, or 1.04 
> Gbit/sec.
> 
> I can run one card in alternates 2 or 4, no problem. But if I put another
> card in the other USB port (my Lenovo X240 has one on each side) and try to
> activate that too, I get
> 
>   [  287.919173] usb 2-2: Not enough bandwidth for altsetting 4
> 
> I've tried on another machine and it works fine there, so my code should,
> at least on the surface of it, be fine. I've verified that the error is
> COMP_BW_ERR (not COMP_2ND_BW_ERR).
> 
> My xHCI controller is 8086:9c31 (I believe this is Intel). lsusb -v with both
> devices plugged in is attached. I'm running 4.3-rc2. There are no external
> USB hubs in the mix.
> 
> So this is my question: Why can I run one card at 2.16 Gbit/sec but not two
> at 1.04 Gbit/sec each? (Shouldn't the USB3 bus be 5 Gbit/sec?) Is there any
> way to debug these issues, or is the controller just deciding and unwilling
> to give out any information? Is the information of the bandwidth domains/
> bus instances available anywhere? Am I misunderstanding something
> fundamentally?

Like Krzysztof said, it's more complicated than this.  No more than 80% 
of the total bus capacity can be allocated for periodic transfers.  In 
addition, USB-3.0 doesn't really handle 5 Gb/s.  That's the bit rate of 
the bus itself, but data on the bus gets encoded in way that requires 
10 bits of bus traffic for every 8 bits of data transferred.  Thus the 
available bandwidth is really 4 Gb/s -- and part of that is used up by 
overhead.

However, none of this answers the question of why you can use both
cards on a different machine but not on yours.  It comes down to the
implementations of the xHCI controller chips.  In USB-3, bandwidth
allocation is handled by firmware running on the controller, not by the
operating system's driver.  The driver presents a series of endpoints
with all their bandwidth requirements to the controller, and the
controller either accepts it or rejects it.  It doesn't give any
explanation for its decision, and as far as I know, it doesn't provide 
any information about the details of how it allocates the bandwidth.

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: Overly conservative xHCI bandwidth estimation

[Krzysztof Opasiak]

On 09/24/2015 03:50 PM, Steinar H. Gunderson wrote:

On Thu, Sep 24, 2015 at 03:35:10PM +0200, Krzysztof Opasiak wrote:

No. The 2nd altset is 2.16, the 4th altset is 1.04. Unless my calculations
are wrong.

How do you do your calculations?


Like I said in my initial email:


The interface of each card has two relevant alternates, 2 and 4. Number 2 has
bInterval=1, bMaxBurst=11, Mult=2, which from what I can see should be
11*(2+1)*1024 byte / 125µs, or 2.16 Gbit/sec. Number 4 has bInterval=1,
bMaxBurst=8, Mult=1, which should be 8*(1+1)*1024 byte / 125 µs, or 1.04 
Gbit/sec.


As far I can see in tutorials the correct equation is:

(bMaxBurst + 1)*(Mult + 1)*wMaxPacketSize

So for 2nd you should get:

12*3*1024 = 36864 /125µs what gives 2.95 GBit/sec.

and for 4th one:

9*2*1024 = 18432 /125µs what gives 1.47 GBit/sec.

But this should still fit.

Sorry... no other ideas...

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

[PATCH v2] usb: gadget: f_sourcesink: fix function params handling

[Robert Baldyga]

Move function parameters to struct f_sourcesink to make them per instance
instead of having them as global variables. Since we can have multiple
instances of USB function we also want to have separate set of parameters
for each instance.

Cc:  # 3.10+
Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_sourcesink.c | 120 +++--
 1 file changed, 62 insertions(+), 58 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index 1353465..128abfb 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -50,6 +50,13 @@ struct f_sourcesink {
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
int cur_alt;
+
+   unsigned pattern;
+   unsigned isoc_interval;
+   unsigned isoc_maxpacket;
+   unsigned isoc_mult;
+   unsigned isoc_maxburst;
+   unsigned buflen;
 };
 
 static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
@@ -57,13 +64,6 @@ static inline struct f_sourcesink *func_to_ss(struct 
usb_function *f)
return container_of(f, struct f_sourcesink, function);
 }
 
-static unsigned pattern;
-static unsigned isoc_interval;
-static unsigned isoc_maxpacket;
-static unsigned isoc_mult;
-static unsigned isoc_maxburst;
-static unsigned buflen;
-
 /*-*/
 
 static struct usb_interface_descriptor source_sink_intf_alt0 = {
@@ -298,7 +298,9 @@ static struct usb_gadget_strings *sourcesink_strings[] = {
 
 static inline struct usb_request *ss_alloc_ep_req(struct usb_ep *ep, int len)
 {
-   return alloc_ep_req(ep, len, buflen);
+   struct f_sourcesink *ss = ep->driver_data;
+
+   return alloc_ep_req(ep, len, ss->buflen);
 }
 
 void free_ep_req(struct usb_ep *ep, struct usb_request *req)
@@ -357,22 +359,22 @@ autoconf_fail:
goto autoconf_fail;
 
/* sanity check the isoc module parameters */
-   if (isoc_interval < 1)
-   isoc_interval = 1;
-   if (isoc_interval > 16)
-   isoc_interval = 16;
-   if (isoc_mult > 2)
-   isoc_mult = 2;
-   if (isoc_maxburst > 15)
-   isoc_maxburst = 15;
+   if (ss->isoc_interval < 1)
+   ss->isoc_interval = 1;
+   if (ss->isoc_interval > 16)
+   ss->isoc_interval = 16;
+   if (ss->isoc_mult > 2)
+   ss->isoc_mult = 2;
+   if (ss->isoc_maxburst > 15)
+   ss->isoc_maxburst = 15;
 
/* fill in the FS isoc descriptors from the module parameters */
-   fs_iso_source_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
-   1023 : isoc_maxpacket;
-   fs_iso_source_desc.bInterval = isoc_interval;
-   fs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
-   1023 : isoc_maxpacket;
-   fs_iso_sink_desc.bInterval = isoc_interval;
+   fs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
+   1023 : ss->isoc_maxpacket;
+   fs_iso_source_desc.bInterval = ss->isoc_interval;
+   fs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
+   1023 : ss->isoc_maxpacket;
+   fs_iso_sink_desc.bInterval = ss->isoc_interval;
 
/* allocate iso endpoints */
ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, _iso_source_desc);
@@ -394,8 +396,8 @@ no_iso:
ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
}
 
-   if (isoc_maxpacket > 1024)
-   isoc_maxpacket = 1024;
+   if (ss->isoc_maxpacket > 1024)
+   ss->isoc_maxpacket = 1024;
 
/* support high speed hardware */
hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
@@ -406,15 +408,15 @@ no_iso:
 * We assume that the user knows what they are doing and won't
 * give parameters that their UDC doesn't support.
 */
-   hs_iso_source_desc.wMaxPacketSize = isoc_maxpacket;
-   hs_iso_source_desc.wMaxPacketSize |= isoc_mult << 11;
-   hs_iso_source_desc.bInterval = isoc_interval;
+   hs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
+   hs_iso_source_desc.wMaxPacketSize |= ss->isoc_mult << 11;
+   hs_iso_source_desc.bInterval = ss->isoc_interval;
hs_iso_source_desc.bEndpointAddress =
fs_iso_source_desc.bEndpointAddress;
 
-   hs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket;
-   hs_iso_sink_desc.wMaxPacketSize |= isoc_mult << 11;
-   hs_iso_sink_desc.bInterval = isoc_interval;
+   hs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
+   hs_iso_sink_desc.wMaxPacketSize |= ss->isoc_mult << 11;
+   

Re: [PATCH 2/2] usb: gadget: f_loopfack: fix function params handling

[Robert Baldyga]

Hi Felipe,

I see that Krzysztof Opasiak made the same change in patch
https://lkml.org/lkml/2015/9/22/578, so please just ignore my patch.

My first patch is still needed - I will resend it with CC: stable.

Thanks,
Robert Baldyga

On 09/24/2015 05:02 PM, Robert Baldyga wrote:
> Move function parameters to struct f_loopback to make them per instance
> instead of having them as global variables. Since we can have multiple
> instances of USB function we also want to have separate set of parameters
> for each instance.
> 
> Signed-off-by: Robert Baldyga 
> ---
>  drivers/usb/gadget/function/f_loopback.c | 22 --
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_loopback.c 
> b/drivers/usb/gadget/function/f_loopback.c
> index 300e601..8d2dac8 100644
> --- a/drivers/usb/gadget/function/f_loopback.c
> +++ b/drivers/usb/gadget/function/f_loopback.c
> @@ -34,6 +34,9 @@ struct f_loopback {
>  
>   struct usb_ep   *in_ep;
>   struct usb_ep   *out_ep;
> +
> + unsigned qlen;
> + unsigned buflen;
>  };
>  
>  static inline struct f_loopback *func_to_loop(struct usb_function *f)
> @@ -41,9 +44,6 @@ static inline struct f_loopback *func_to_loop(struct 
> usb_function *f)
>   return container_of(f, struct f_loopback, function);
>  }
>  
> -static unsigned qlen;
> -static unsigned buflen;
> -
>  /*-*/
>  
>  static struct usb_interface_descriptor loopback_intf = {
> @@ -251,7 +251,7 @@ static void loopback_complete(struct usb_ep *ep, struct 
> usb_request *req)
>   }
>  
>   /* queue the buffer for some later OUT packet */
> - req->length = buflen;
> + req->length = loop->buflen;
>   status = usb_ep_queue(ep, req, GFP_ATOMIC);
>   if (status == 0)
>   return;
> @@ -288,7 +288,9 @@ static void disable_loopback(struct f_loopback *loop)
>  
>  static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
>  {
> - return alloc_ep_req(ep, len, buflen);
> + struct f_loopback   *loop = ep->driver_data;
> +
> + return alloc_ep_req(ep, len, loop->buflen);
>  }
>  
>  static int enable_endpoint(struct usb_composite_dev *cdev, struct f_loopback 
> *loop,
> @@ -315,7 +317,7 @@ static int enable_endpoint(struct usb_composite_dev 
> *cdev, struct f_loopback *lo
>* we buffer at most 'qlen' transfers; fewer if any need more
>* than 'buflen' bytes each.
>*/
> - for (i = 0; i < qlen && result == 0; i++) {
> + for (i = 0; i < loop->qlen && result == 0; i++) {
>   req = lb_alloc_ep_req(ep, 0);
>   if (!req)
>   goto fail1;
> @@ -388,10 +390,10 @@ static struct usb_function *loopback_alloc(struct 
> usb_function_instance *fi)
>   lb_opts->refcnt++;
>   mutex_unlock(_opts->lock);
>  
> - buflen = lb_opts->bulk_buflen;
> - qlen = lb_opts->qlen;
> - if (!qlen)
> - qlen = 32;
> + loop->buflen = lb_opts->bulk_buflen;
> + loop->qlen = lb_opts->qlen;
> + if (!loop->qlen)
> + loop->qlen = 32;
>  
>   loop->function.name = "loopback";
>   loop->function.bind = loopback_bind;
> 

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

Re: [PATCH v3 1/2] usb: gadget: loopback: fix: Don't share qlen and buflen between instances

[Robert Baldyga]

On 09/22/2015 08:40 PM, Krzysztof Opasiak wrote:
> Each instance of loopback function may have different qlen
> and buflen attributes values. When linking function to
> configuration those values had been assigned to global
> variables. Linking other instance to config overwrites those
> values.
> 
> This commit moves those values to f_loopback structure
> to avoid overwriting. Now each function has its own instance
> of those values.
> 
> Cc:  # 3.10+
> Signed-off-by: Krzysztof Opasiak 

Reviewed-by: Robert Baldyga 

> ---
>  drivers/usb/gadget/function/f_loopback.c |   24 +---
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_loopback.c 
> b/drivers/usb/gadget/function/f_loopback.c
> index 6e2fe63..e4bfed4 100644
> --- a/drivers/usb/gadget/function/f_loopback.c
> +++ b/drivers/usb/gadget/function/f_loopback.c
> @@ -34,6 +34,9 @@ struct f_loopback {
>  
>   struct usb_ep   *in_ep;
>   struct usb_ep   *out_ep;
> +
> + unsignedqlen;
> + unsignedbuflen;
>  };
>  
>  static inline struct f_loopback *func_to_loop(struct usb_function *f)
> @@ -41,13 +44,10 @@ static inline struct f_loopback *func_to_loop(struct 
> usb_function *f)
>   return container_of(f, struct f_loopback, function);
>  }
>  
> -static unsigned qlen;
> -static unsigned buflen;
> -
>  /*-*/
>  
>  static struct usb_interface_descriptor loopback_intf = {
> - .bLength =  sizeof loopback_intf,
> + .bLength =  sizeof(loopback_intf),
>   .bDescriptorType =  USB_DT_INTERFACE,
>  
>   .bNumEndpoints =2,
> @@ -253,7 +253,7 @@ static void loopback_complete(struct usb_ep *ep, struct 
> usb_request *req)
>   }
>  
>   /* queue the buffer for some later OUT packet */
> - req->length = buflen;
> + req->length = loop->buflen;
>   status = usb_ep_queue(ep, req, GFP_ATOMIC);
>   if (status == 0)
>   return;
> @@ -290,7 +290,9 @@ static void disable_loopback(struct f_loopback *loop)
>  
>  static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
>  {
> - return alloc_ep_req(ep, len, buflen);
> + struct f_loopback   *loop = ep->driver_data;
> +
> + return alloc_ep_req(ep, len, loop->buflen);
>  }
>  
>  static int enable_endpoint(struct usb_composite_dev *cdev, struct f_loopback 
> *loop,
> @@ -317,7 +319,7 @@ static int enable_endpoint(struct usb_composite_dev 
> *cdev, struct f_loopback *lo
>* we buffer at most 'qlen' transfers; fewer if any need more
>* than 'buflen' bytes each.
>*/
> - for (i = 0; i < qlen && result == 0; i++) {
> + for (i = 0; i < loop->qlen && result == 0; i++) {
>   req = lb_alloc_ep_req(ep, 0);
>   if (!req)
>   goto fail1;
> @@ -391,10 +393,10 @@ static struct usb_function *loopback_alloc(struct 
> usb_function_instance *fi)
>   lb_opts->refcnt++;
>   mutex_unlock(_opts->lock);
>  
> - buflen = lb_opts->bulk_buflen;
> - qlen = lb_opts->qlen;
> - if (!qlen)
> - qlen = 32;
> + loop->buflen = lb_opts->bulk_buflen;
> + loop->qlen = lb_opts->qlen;
> + if (!loop->qlen)
> + loop->qlen = 32;
>  
>   loop->function.name = "loopback";
>   loop->function.bind = loopback_bind;
> 

--
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: Overly conservative xHCI bandwidth estimation

[Alan Stern]

On Thu, 24 Sep 2015, Steinar H. Gunderson wrote:

> On Thu, Sep 24, 2015 at 10:17:19AM -0400, Alan Stern wrote:
> > However, none of this answers the question of why you can use both
> > cards on a different machine but not on yours.  It comes down to the
> > implementations of the xHCI controller chips.  In USB-3, bandwidth
> > allocation is handled by firmware running on the controller, not by the
> > operating system's driver.  The driver presents a series of endpoints
> > with all their bandwidth requirements to the controller, and the
> > controller either accepts it or rejects it.
> 
> OK, I feared as much. The other machine also has an Intel controller,
> but as far as I know, a newer one (and the PCI ID is different -- 8086:9cb1).
> 
> > It doesn't give any explanation for its decision, and as far as I know, it
> > doesn't provide any information about the details of how it allocates the
> > bandwidth.
> 
> I thought I saw something in the xHCI spec about enumerating the bandwidth
> domains to try to get some more insight in what the topology looks like,
> but I guess I misunderstood it? (It all wasn't very clear to me.)

Ah, you're referring to the Get Port Bandwidth command, documented in
section 4.6.15 of the xHCI-1.1 spec.  Yes, that does provide some
information on what bandwidth remains available.  Apparently the
xhci-hcd driver doesn't support it.  Adding it would be a good way to
improve the driver's debugging facilities.

> I assume there's no way I can lie to the chip? Like, if I know for a fact
> that the card will send less data than the alternate claims (like,
> I'm using a video mode that will require only a few hundred megabits/second
> in practice, but even the lowest alternate claims >1 Gbit/sec).

You can always patch the kernel to do whatever you want.  For instance, 
you could reduce the bMaxBurst value in the descriptor after the kernel 
gets it from the card.  Short of that, there is no way to affect the 
outcome.

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 3/3] page-flags: rectify forward declaration

[Sudip Mukherjee]

> > 
> > Also, I'm finding that the patch series introduces a pretty large
> > bisection hole:
> > 
> > include/linux/page-flags.h: In function 'PageYoung':
> > include/linux/page-flags.h:327: error: implicit declaration of function 
> > 'PF_ANY'
> > include/linux/page-flags.h:327: error: invalid type argument of '->' (have 
> > 'int')
> > include/linux/page-flags.h:327: error: invalid type argument of '->' (have 
> > 'int')
> > 
> > which later gets fixed up by
> > page-flags-rectify-forward-declaration.patch.
> How to test this? Should I apply them on top of v4.2 and bisect? And I
> don't see any relation between the first two patches and this patch of
> the series, then how does it fail in bisect? Am I missing something?
> Confused.. :(
> > 
> > Maybe it's time to do a wholesale refactoring of the patchset?
> If this patch is the first in the series will that help?
> And besides I got the auto mail from you that the patch is applied.
> Now totally confused.. :(
Ohhh ..all the time you were talking about Kirill's patch set and I kept
thinking you are mentioning about this patchset. :)

regards
sudip
--
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 1/2] usb: doc: Add bindings for ULPI platform driver

[Rob Herring]

On Wed, Sep 23, 2015 at 7:54 AM, Subbaraya Sundeep Bhatta
 wrote:
> This patch adds binding doc info for generic ULPI PHYs
> platform driver.
>
> Signed-off-by: Subbaraya Sundeep Bhatta 
> ---
>  .../devicetree/bindings/usb/ulpi-platform-phy.txt  |   34 
> 
>  1 files changed, 34 insertions(+), 0 deletions(-)
>  create mode 100644 
> Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt 
> b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> new file mode 100644
> index 000..7b8cbb4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> @@ -0,0 +1,34 @@
> +Platform driver for generic ULPI PHYs

Platform driver has nothing to do with DT bindings.

> +
> +Required properties:
> +- compatible   : Should be "ulpi-phy"

This needs a more specific compatible in addition.

> +- reg  : Physical base address and size of the USB
> + controller registers map to which this PHY
> + is connected.
> +- view-port: Should contain viewport register offset of the
> + USB controller to which this PHY is connected

This is a property of the controller, not the phy.

> +Optional properties:
> +- drv-vbus : required if turning VBUS on/off has to be driven
> + by writing to PHY. This feature depends on board
> + design.
> +
> +Example:
> +Below example shows the PHY binding for Chipidea USB controller which has
> +ulpi viewport register at 0x0170
> +
> +   usb_phy0: phy0 {
> +   compatible = "ulpi-phy";
> +   reg = <0xe0002000 0x1000>;
> +   view-port = <0x0170>;
> +   drv-vbus;
> +   };
> +
> +   usb0: usb@e0002000 {
> +compatible = "chipidea,usb2";
> +interrupt-parent = <>;
> +interrupts = <0 21 4>;
> +reg = <0xe0002000 0x1000>;
> +phy_type = "ulpi";
> +   dr_mode = "host";
> +   usb-phy = <_phy0>;

usb-phy is deprecated. Use phys and the generic phy binding instead.

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

Re: [RFC PATCH 1/2] usb: doc: Add bindings for ULPI platform driver

[Rob Herring]

On Thu, Sep 24, 2015 at 4:26 AM, Subbaraya Sundeep Bhatta
 wrote:
> Hi Peter,
>
>> -Original Message-
>> From: Peter Chen [mailto:peter.c...@freescale.com]
>> Sent: Thursday, September 24, 2015 2:41 PM
>> To: Subbaraya Sundeep Bhatta
>> Cc: ba...@ti.com; devicet...@vger.kernel.org; kis...@ti.com;
>> gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
>> ker...@vger.kernel.org; Punnaiah Choudary Kalluri; Subbaraya Sundeep Bhatta;
>> linux-arm-ker...@lists.infradead.org
>> Subject: Re: [RFC PATCH 1/2] usb: doc: Add bindings for ULPI platform driver
>>
>> On Wed, Sep 23, 2015 at 06:24:01PM +0530, Subbaraya Sundeep Bhatta
>> wrote:
>> > This patch adds binding doc info for generic ULPI PHYs platform
>> > driver.
>> >
>> > Signed-off-by: Subbaraya Sundeep Bhatta 
>> > ---
>> >  .../devicetree/bindings/usb/ulpi-platform-phy.txt  |   34
>> 
>> >  1 files changed, 34 insertions(+), 0 deletions(-)  create mode 100644
>> > Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
>> >
>> > diff --git
>> > a/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
>> > b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
>> > new file mode 100644
>> > index 000..7b8cbb4
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
>> > @@ -0,0 +1,34 @@
>> > +Platform driver for generic ULPI PHYs
>> > +
>> > +Required properties:
>> > +- compatible   : Should be "ulpi-phy"
>> > +- reg  : Physical base address and size of the USB
>> > + controller registers map to which this PHY
>> > + is connected.
>> > +- view-port: Should contain viewport register offset of 
>> > the
>> > + USB controller to which this PHY is connected 
>> > Optional
>> > +properties:
>> > +- drv-vbus : required if turning VBUS on/off has to be driven
>> > + by writing to PHY. This feature depends on board
>> > + design.
>> > +
>> > +Example:
>> > +Below example shows the PHY binding for Chipidea USB controller which
>> > +has ulpi viewport register at 0x0170
>> > +
>> > +   usb_phy0: phy0 {
>> > +   compatible = "ulpi-phy";
>> > +   reg = <0xe0002000 0x1000>;
>> > +   view-port = <0x0170>;
>> > +   drv-vbus;
>> > +   };
>> > +
>> > +   usb0: usb@e0002000 {
>> > +compatible = "chipidea,usb2";
>> > +interrupt-parent = <>;
>> > +interrupts = <0 21 4>;
>> > +reg = <0xe0002000 0x1000>;
>>
>> Although just call devm_ioremap twice for the same register region does not
>> cause any errors, I am not sure if it will has other potential problems. Cc: 
>> arm
>> list.
>
> Yes Peter I was also in doubt to call devm_ioremap twice for same register 
> region.
> devm_ioremap_resource complained hence modified to devm_ioremap. Thanks for
> adding arm-list.

Don't put overlapping resources in the DT. Having 2 drivers accessing
the same registers is not a clean or safe design.

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

Re: [PATCH v2] usb: gadget: f_sourcesink: fix function params handling

[Krzysztof Opasiak]

On 09/24/2015 06:49 PM, Felipe Balbi wrote:

On Thu, Sep 24, 2015 at 05:23:09PM +0200, Robert Baldyga wrote:

Move function parameters to struct f_sourcesink to make them per instance
instead of having them as global variables. Since we can have multiple
instances of USB function we also want to have separate set of parameters
for each instance.

Cc:  # 3.10+
Signed-off-by: Robert Baldyga 


why do you think this is stable material ? Looks like it's not
fixing anything to me; just implementing a brand new requirement.



Please see my answer in loopback patch[1]. It's totally the same case. 
Both those functions have the same bug.


Footnotes:
1 - http://marc.info/?l=linux-usb=144311466118170=2
--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode

[Roman Bacik]

> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: September-23-15 9:21 PM
> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> u...@vger.kernel.org; Roman Bacik
> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
> Subject: Re: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode
> 
> On 9/10/2015 6:14 PM, Scott Branden wrote:
> > This patch contains a fix for a real world interop problem found when
> > using the Synopsis DWC2 USB controller with isochronous audio as
> > detailed in the commit message.
> >
> > Changes from v2:
> >  - created s2c_hsotg_chage_ep_iso_parity function to call function in
> > 3 places in code
> >  - used hsotg->num_of_eps instead of MAX_EPS_CHANNELS
> >
> > Changes from v1:
> >  - Address code review comments as per previous responses:
> >  - renamed parity_set to has_correct_parity and reorder some logic
> >
> >
> > Roman Bacik (1):
> >   usb: dwc2: gadget: parity fix in isochronous mode
> >
> >  drivers/usb/dwc2/core.h   |  1 +
> >  drivers/usb/dwc2/gadget.c | 58
> ++-
> >  drivers/usb/dwc2/hw.h |  1 +
> >  3 files changed, 54 insertions(+), 6 deletions(-)
> >
> 
> This seems to break slave mode on my platform. It seems to be dropping a
> lot of packets. I tried bInterval=4,5,6, ISO OUT.
> I'll need to take a closer look to determine why. Probably later this week.
> 
> Are you able to run in slave mode on your platform? If so can you try it out?
> 
> Regards,
> John

Our current test procedure is as follows:

Build Linux kernel with: 

Device Drivers
- Sound Card Support 'SOUND=y': ALSA 'SND=y'
- Generic sound devices: Dummy (/dev/null) soundcard 'SND_DUMMY=y'
- USB support 'USB_SUPPORT=y':
DesignWare USB2 DRD Core support 'USB_DWC2=y'
- Gadget only mode 'USB_DWC2_PERIPHERAL=y'
DWC2 Platform 'USB_DWC2_PLATFORM=y'
- USB Gadget Support 'USB_GADGET=y': Audio Gadget 'USB_AUDIO=y'
- PHY Subsystem: Broadcom Kona USB2 PHY Driver 'BCM_KONA_USB2_PHY=y'

If you have only even hosts, you can change the default micro frame parity to 
odd:

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index ad45b0b..80bde75 100644
@@ -2709,7 +2709,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
 switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
 case USB_ENDPOINT_XFER_ISOC:
 epctrl |= DXEPCTL_EPTYPE_ISO;
-epctrl |= DXEPCTL_SETEVENFR;
+epctrl |= DXEPCTL_SETODDFR;
 hs_ep->isochronous = 1;
 if (dir_in)
 hs_ep->periodic = 1;
@@ -2777,7 +2777,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
  
 /* for non control endpoints, set PID to D0 */
 if (index)
-epctrl |= DXEPCTL_SETD0PID;
+epctrl |= DXEPCTL_SETD1PID;
  
 dev_dbg(hsotg->dev, "%s: write DxEPCTL=0x%08x\n",
 __func__, epctrl);

To test OUT direction:
Host:
aplay -D plughw:2 -r 48000 -f S16_LE tone_48stereo.wav
Device:
arecord -D plughw:0 -r 48000 -f S16_LE -c 1 /tmp/rec.pcm

To test IN direction:
Host:
arecord -D plughw:2 -r 48000 -f S16_LE -c 1 /tmp/rec.pcm
Device:
aplay -D plughw:0 -r 48000 -f S16_LE /tmp/rec.pcm

If you would like, we can try to test your procedure provided you send us 
enough details.
Regards,

Roman

--
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: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M

[Roland Weber]

Hi Alan,

> > Frankly, I don't dare to upgrade the BIOS right now.
> 
> All right.  Maybe at some point in the future.  The thing is, I don't 
> want to patch the kernel only to find that the problem has already been 
> fixed in a newer BIOS.

Fair enough. I'll see if I can make time for setting things up over
the next few weeks. Andrew's report on this matter is encouraging.

> The unavoidable conclusion is that your system doesn't work right when 
> this root hub is suspended.  (I still think it's the BIOS's fault.)

I've been using the machine in legacy BIOS mode ever since I wasted
three days trying to get an installed Linux booting in UEFI mode. I'm
planning a clean installation anyway, this weekend or early next week.
I'll give UEFI mode (without secure boot) another chance. Maybe it
works when I get rid of the factory boot entries in that menu.
At the very least, I can boot a Linux live disk with UEFI to see
if that one freezes, and what the EHCI registers have to say.

> Which suggests yet another test: Under a kernel with CONFIG_PM and
> CONFIG_PM_SLEEP enabled, what happens when you suspend the system
> (i.e., suspend-to-RAM) and then wake it up?

Well, there's the second big problem I have with the machine. It has
an eMMC rather than a real hard disk or SSD. AIUI, the Linux MMC driver
has no way of distinguishing removable from non-removable MMC media.
So it treats all MMC as removable and expects that the disk contents
may have changed while it was asleep. Kind of an unmount on sleep or so.
Which means that when the machine wakes up, the root filesystem is gone.
IIRC, it did wake up but then hung up when I entered a command. It's
been a while since I tried.
There is a boot parameter to tell the MMC driver that all media are to
be treated as non-removable. I gave it a try back then and it didn't work.
But I'll give it another try a.s.a.p. Maybe I just had the prefix wrong.

Thanks a lot, Alan. I'll provide more information as soon as I can
gather it.

cheers,
  Roland
--
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 v4 0/5] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

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

Providing a standard framework for doing this in the kernel.

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

Changes since v3:
 - Re-order the patch and split the patch to avoid breaking build.
 - Other modifications.

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

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

-- 
1.9.1

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

[PATCH v4 5/5] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

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

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

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

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

[PATCH v4 2/5] gadget: Introduce the usb charger framework

[Baolin Wang]

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

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

The usb charger will register notifiees on the usb gadget or the extcon
device to get notified the usb charger state.

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

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

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index bcf83c0..3d2b959 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -127,6 +127,13 @@ config USB_GADGET_STORAGE_NUM_BUFFERS
   a module parameter as well.
   If unsure, say 2.
 
+config USB_CHARGER
+   bool "USB charger support"
+   help
+ The usb charger driver based on the usb gadget that makes an
+ enhancement to a power driver which can set the current limitation
+ when the usb charger is added or removed.
+
 source "drivers/usb/gadget/udc/Kconfig"
 
 #
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 598a67d..1e421c1 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -10,3 +10,4 @@ libcomposite-y:= usbstring.o config.o 
epautoconf.o
 libcomposite-y += composite.o functions.o configfs.o u_f.o
 
 obj-$(CONFIG_USB_GADGET)   += udc/ function/ legacy/
+obj-$(CONFIG_USB_CHARGER)  += charger.o
diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
new file mode 100644
index 000..35b46c1
--- /dev/null
+++ b/drivers/usb/gadget/charger.c
@@ -0,0 +1,529 @@
+/*
+ * charger.c -- USB charger driver
+ *
+ * Copyright (C) 2015 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DEFAULT_CUR_PROTECT(50)
+#define DEFAULT_SDP_CUR_LIMIT  (500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_DCP_CUR_LIMIT  (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_CDP_CUR_LIMIT  (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_ACA_CUR_LIMIT  (1500 - DEFAULT_CUR_PROTECT)
+
+static DEFINE_IDA(usb_charger_ida);
+static struct bus_type usb_charger_subsys = {
+   .name   = "usb-charger",
+   .dev_name   = "usb-charger",
+};
+
+static struct usb_charger *dev_to_uchger(struct device *udev)
+{
+   return container_of(udev, struct usb_charger, dev);
+}
+
+static ssize_t cur_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+   struct usb_charger *uchger = dev_to_uchger(dev);
+
+   return scnprintf(buf, PAGE_SIZE, "%d %d %d %d\n",
+uchger->cur_limit.sdp_cur_limit,
+uchger->cur_limit.dcp_cur_limit,
+uchger->cur_limit.cdp_cur_limit,
+uchger->cur_limit.aca_cur_limit);
+}
+
+static ssize_t cur_limit_store(struct device *dev,
+  struct device_attribute *attr,
+  const char *buf, size_t count)
+{
+   struct usb_charger *uchger = dev_to_uchger(dev);
+   struct usb_charger_cur_limit cur;
+   int ret;
+
+   ret = sscanf(buf, "%d %d %d %d",
+_cur_limit, _cur_limit,
+_cur_limit, _cur_limit);
+   if (ret == 0)
+   return -EINVAL;
+
+   ret = usb_charger_set_cur_limit(uchger, );
+   if (ret < 0)
+   return ret;
+
+   return count;
+}
+static DEVICE_ATTR_RW(cur_limit);
+
+static struct attribute *usb_charger_attrs[] = {
+   _attr_cur_limit.attr,
+   NULL
+};
+ATTRIBUTE_GROUPS(usb_charger);
+
+/*
+ * usb_charger_find_by_name - Get the usb charger device by name.
+ * @name - usb charger device name.
+ *
+ * return the instance of usb charger device, the device must be
+ * released with 

[PATCH v4 1/5] gadget: Introduce the notifier functions

[Baolin Wang]

The usb charger framework is based on usb gadget. The usb charger
need to be notified the state changing of usb gadget to confirm the
usb charger state.

Thus this patch adds a notifier mechanism for usb gadget to report a
event to usb charger when the usb gadget state is changed.

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

diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index f660afb..4238fc3 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -129,6 +129,32 @@ void usb_gadget_giveback_request(struct usb_ep *ep,
 }
 EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
 
+int usb_gadget_register_notify(struct usb_gadget *gadget,
+  struct notifier_block *nb)
+{
+   int ret;
+
+   mutex_lock(>lock);
+   ret = raw_notifier_chain_register(>nh, nb);
+   mutex_unlock(>lock);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_register_notify);
+
+int usb_gadget_unregister_notify(struct usb_gadget *gadget,
+struct notifier_block *nb)
+{
+   int ret;
+
+   mutex_lock(>lock);
+   ret = raw_notifier_chain_unregister(>nh, nb);
+   mutex_unlock(>lock);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_unregister_notify);
+
 /* - */
 
 /**
@@ -226,6 +252,10 @@ static void usb_gadget_state_work(struct work_struct *work)
struct usb_gadget *gadget = work_to_gadget(work);
struct usb_udc *udc = gadget->udc;
 
+   mutex_lock(>lock);
+   raw_notifier_call_chain(>nh, gadget->state, gadget);
+   mutex_unlock(>lock);
+
if (udc)
sysfs_notify(>dev.kobj, NULL, "state");
 }
@@ -364,6 +394,8 @@ int usb_add_gadget_udc_release(struct device *parent, 
struct usb_gadget *gadget,
 
dev_set_name(>dev, "gadget");
INIT_WORK(>work, usb_gadget_state_work);
+   RAW_INIT_NOTIFIER_HEAD(>nh);
+   mutex_init(>lock);
gadget->dev.parent = parent;
 
 #ifdef CONFIG_HAS_DMA
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c14a69b..755e8bc 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -609,6 +609,8 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
struct usb_otg_caps *otg_caps;
+   struct raw_notifier_headnh;
+   struct mutexlock;
 
unsignedsg_supported:1;
unsignedis_otg:1;
@@ -1183,6 +1185,22 @@ extern void usb_gadget_unmap_request(struct usb_gadget 
*gadget,
 
 /*-*/
 
+/**
+ * Register a notifiee to get notified by any attach status changes from
+ * the usb gadget
+ */
+int usb_gadget_register_notify(struct usb_gadget *gadget,
+  struct notifier_block *nb);
+
+/*-*/
+
+
+/* Unregister a notifiee from the usb gadget */
+int usb_gadget_unregister_notify(struct usb_gadget *gadget,
+struct notifier_block *nb);
+
+/*-*/
+
 /* utility to set gadget state properly */
 
 extern void usb_gadget_set_state(struct usb_gadget *gadget,
-- 
1.9.1

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

[PATCH v4 3/5] gadget: Support for the usb charger framework

[Baolin Wang]

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

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

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

diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index 4238fc3..370376e 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * struct usb_udc - describes one usb device controller
@@ -437,8 +438,14 @@ int usb_add_gadget_udc_release(struct device *parent, 
struct usb_gadget *gadget,
 
mutex_unlock(_lock);
 
+   ret = usb_charger_init(gadget);
+   if (ret)
+   goto err5;
+
return 0;
 
+err5:
+   device_del(>dev);
 err4:
list_del(>list);
mutex_unlock(_lock);
@@ -513,6 +520,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
kobject_uevent(>dev.kobj, KOBJ_REMOVE);
flush_work(>work);
device_unregister(>dev);
+   usb_charger_exit(gadget);
device_unregister(>dev);
 }
 EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 755e8bc..451ad32 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -537,6 +537,7 @@ struct usb_gadget_ops {
struct usb_ep *(*match_ep)(struct usb_gadget *,
struct usb_endpoint_descriptor *,
struct usb_ss_ep_comp_descriptor *);
+   enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
 };
 
 /**
@@ -611,6 +612,7 @@ struct usb_gadget {
struct usb_otg_caps *otg_caps;
struct raw_notifier_headnh;
struct mutexlock;
+   struct usb_charger  *charger;
 
unsignedsg_supported:1;
unsignedis_otg:1;
-- 
1.9.1

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

[PATCH v4 4/5] gadget: Integrate with the usb gadget supporting for usb charger

[Baolin Wang]

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

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

diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
index 35b46c1..a919f38 100644
--- a/drivers/usb/gadget/charger.c
+++ b/drivers/usb/gadget/charger.c
@@ -181,6 +181,13 @@ int usb_charger_unregister_notify(struct usb_charger 
*uchger,
 enum usb_charger_type
 usb_charger_detect_type(struct usb_charger *uchger)
 {
+   if (uchger->gadget && uchger->gadget->ops
+   && uchger->gadget->ops->get_charger_type)
+   uchger->type =
+   uchger->gadget->ops->get_charger_type(uchger->gadget);
+   else
+   uchger->type = UNKNOWN_TYPE;
+
return uchger->type;
 }
 
@@ -313,7 +320,8 @@ static int
 usb_charger_plug_by_gadget(struct notifier_block *nb,
   unsigned long state, void *data)
 {
-   struct usb_charger *uchger = NULL;
+   struct usb_gadget *gadget = (struct usb_gadget *)data;
+   struct usb_charger *uchger = gadget->charger;
enum usb_charger_state uchger_state;
 
if (!uchger)
@@ -480,6 +488,7 @@ int usb_charger_init(struct usb_gadget *ugadget)
 
/* register a notifier on a usb gadget device */
uchger->gadget = ugadget;
+   ugadget->charger = uchger;
uchger->old_gadget_state = ugadget->state;
uchger->gadget_nb.notifier_call = usb_charger_plug_by_gadget;
usb_gadget_register_notify(ugadget, >gadget_nb);
@@ -503,7 +512,7 @@ fail:
 
 int usb_charger_exit(struct usb_gadget *ugadget)
 {
-   struct usb_charger *uchger = NULL;
+   struct usb_charger *uchger = ugadget->charger;
 
if (!uchger)
return -EINVAL;
-- 
1.9.1

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

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Thu, Sep 24, 2015 at 11:22:57AM -0400, Alan Stern wrote:
>> I assume there's no way I can lie to the chip? Like, if I know for a fact
>> that the card will send less data than the alternate claims (like,
>> I'm using a video mode that will require only a few hundred megabits/second
>> in practice, but even the lowest alternate claims >1 Gbit/sec).
> You can always patch the kernel to do whatever you want.  For instance, 
> you could reduce the bMaxBurst value in the descriptor after the kernel 
> gets it from the card.  Short of that, there is no way to affect the 
> outcome.

I tried, but it didn't work; I bMaxBurst to 4 instead of to 15
(in usb_parse_ss_endpoint_companion). It didn't change the outcome.
Perhaps this doesn't actually change what the xHCI controller sees,
though.

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

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Thu, Sep 24, 2015 at 02:33:06PM -0700, Paul Zimmerman wrote:
> IIRC, at least some of the Intel controllers require the bandwidth
> calculations to be done by the xHCI driver, instead of doing it
> themselves in hardware. Perhaps you're tripping over a bug in the xHCI
> driver? Mathias is probably best the one to comment on this.

FWIW, I've checked that the XHCI_SW_BW_CHECKING quirk isn't set for my
device. (I suppose that's the one you're thinking about?)

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

Re: Overly conservative xHCI bandwidth estimation

[Paul Zimmerman]

IIRC, at least some of the Intel controllers require the bandwidth
calculations to be done by the xHCI driver, instead of doing it
themselves in hardware. Perhaps you're tripping over a bug in the xHCI
driver? Mathias is probably best the one to comment on this.

-- 
Paul
--
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: Overly conservative xHCI bandwidth estimation

[Paul Zimmerman]

Yep, that's the one.

-- 
Paul

On Thu, Sep 24, 2015 at 2:37 PM, Steinar H. Gunderson
 wrote:
> On Thu, Sep 24, 2015 at 02:33:06PM -0700, Paul Zimmerman wrote:
>> IIRC, at least some of the Intel controllers require the bandwidth
>> calculations to be done by the xHCI driver, instead of doing it
>> themselves in hardware. Perhaps you're tripping over a bug in the xHCI
>> driver? Mathias is probably best the one to comment on this.
>
> FWIW, I've checked that the XHCI_SW_BW_CHECKING quirk isn't set for my
> device. (I suppose that's the one you're thinking about?)
>
> /* Steinar */
> --
> Homepage: http://www.sesse.net/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Fri, Sep 25, 2015 at 12:42:22AM +0200, Steinar H. Gunderson wrote:
> I downgraded to Debian's 3.16 kernel. Both cards came up without a hitch.
> But I only seem to get frames back from the second one.

...and after a quick app fix, I have capture from both cards.

Does this mean I have a long bisect ahead of me?

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

Re: Overly conservative xHCI bandwidth estimation

[Alan Stern]

On Thu, 24 Sep 2015, Steinar H. Gunderson wrote:

> On Thu, Sep 24, 2015 at 11:22:57AM -0400, Alan Stern wrote:
> >> I assume there's no way I can lie to the chip? Like, if I know for a fact
> >> that the card will send less data than the alternate claims (like,
> >> I'm using a video mode that will require only a few hundred megabits/second
> >> in practice, but even the lowest alternate claims >1 Gbit/sec).
> > You can always patch the kernel to do whatever you want.  For instance, 
> > you could reduce the bMaxBurst value in the descriptor after the kernel 
> > gets it from the card.  Short of that, there is no way to affect the 
> > outcome.
> 
> I tried, but it didn't work; I bMaxBurst to 4 instead of to 15
> (in usb_parse_ss_endpoint_companion). It didn't change the outcome.
> Perhaps this doesn't actually change what the xHCI controller sees,
> though.

It does.  Grep for max_burst in drivers/usb/host/x*.c to see where it 
gets used.  (Note that in a couple of places involving USB-2 devices, 
the code uses max_burst where it really means multiplicity.)

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: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Wed, Sep 23, 2015 at 10:08:05PM +0200, Steinar H. Gunderson wrote:
> I've tried on another machine and it works fine there, so my code should,
> at least on the surface of it, be fine.

PLOT TWIST:

I downgraded to Debian's 3.16 kernel. Both cards came up without a hitch.
But I only seem to get frames back from the second one.

???

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

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Thu, Sep 24, 2015 at 04:46:22PM -0400, Alan Stern wrote:
> It does.  Grep for max_burst in drivers/usb/host/x*.c to see where it 
> gets used.  (Note that in a couple of places involving USB-2 devices, 
> the code uses max_burst where it really means multiplicity.)

OK, so this is very curious. I hacked the endpoint descriptors to set
max_burst=1, max_esit_payload=1024, mult=0 (in xhci_endpoint_init, which
seems to be the thing that actually produces the structures the device 
consumed, so I'm reasonably sure nothing else is modifying it on the way).
Still it says COMP_BW_ERR. Setting interval=3 (it used to be 0) has no
effect. Either my override isn't working, or this just isn't about bandwidth,
but something else.

I also tried ignoring the three other endpoints (early-out in
xhci_add_endpoint) just to check that they weren't taking up some resource
I was needing, and it didn't change anything. If I also ignore the video
endpoint itself (0x83), the altsetting _does_ go through, but of course then
I can't get any data from it.

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

Re: [PATCH v2 00/32] usb: dwc2: various bug fixes

[John Youn]

On 9/22/2015 6:15 AM, Mian Yousaf Kaukab wrote:
> Hi,
> This series consists of various bug fixes for both host and gadget
> sides. All patches are verified on dwc2 v3.0a with dedicated fifos.
> It would be good to get some Tested-bys for other platforms.
> 
> It is based on testing/next branch in Felipe's git.
> 
> Thank you,
> 
> Best regards,
> Yousaf
> 
> History:
> v2:
>  - Rebase to latest testing/next
>  - Fix lock issue found by John Youn when calling
>dwc2_hsotg_core_init_disconnected() from dwc2_conn_id_status_change()
>  - Fix comments from John Youn
>  - Fix comments from Doug Anderson
> 
> v1:
>  - Fix following comments from David Cohen (received on an internal
>list):
> - Fix build when "usb: dwc2: host: create a function to handle-
>   port_resume" is applied.
> - Take spin locks within if statements in "usb: dwc2: host: enter-
>   hibernation during bus suspend"
> - Remove extra 100us delay in "usb: dwc2: host: enter-
>   hibernation during bus suspend"
> - Fix spelling mistakes in "usb: dwc2: host: update hcd and-
>   lx_state during start/stop callbacks"
> - Change dev_warn to dev_dbg in "usb: dwc2: host: reset frame-
>number after suspend"
> - Change mdelay to usleep_range in "usb: dwc2: host: wait 3ms for-
>   controller stabilization"
>  - Fix comments from Sergei Shtylyov
> 
> Gregory Herrero (22):
>   usb: dwc2: host: don't clear hprt0 status bits when exiting
> hibernation
>   usb: dwc2: host: create a function to handle port_resume
>   usb: dwc2: host: add flag to reflect bus state
>   usb: dwc2: host: enter hibernation during bus suspend
>   usb: dwc2: host: update hcd and lx_state during start/stop callbacks
>   usb: dwc2: host: avoid resetting lx_state to L3 during disconnect
>   usb: dwc2: host: ignore wakeup interrupt if hibernation supported
>   usb: dwc2: host: resume only if bus is suspended
>   usb: dwc2: host: reset frame number after suspend
>   usb: dwc2: host: disconnect hcd prior stopping it
>   usb: dwc2: host: disable interrupt during stop
>   usb: dwc2: host: clear pending interrupts prior hibernation
>   usb: dwc2: host: wait 3ms for controller stabilization
>   usb: dwc2: host: correctly dump urb isochronous descriptors
>   usb: dwc2: host: use correct frame number during qh init
>   usb: dwc2: host: kill remaining urbs using -ECONNRESET status
>   usb: dwc2: gadget: ensure lx_state corresponds to current state
>   usb: dwc2: gadget: don't modify pullup state in host mode
>   usb: dwc2: gadget: set op_state in vbus_session call
>   usb: dwc2: gadget: abort core init if core_reset fails
>   usb: dwc2: gadget: unmask idstschng interrupt only if controller
> supports it
>   usb: dwc2: gadget: exit hibernation before power down
> 
> Mian Yousaf Kaukab (10):
>   usb: dwc2: host: add disconnect interrupt to host only interrupts
>   usb: dwc2: gadget: initialize op_state for peripheral only
> configuration
>   usb: dwc2: force dr_mode in case of configuration mismatch
>   usb: dwc2: gadget: ignore stall check for ep0
>   usb: dwc2: gadget: print complete setup packet
>   usb: dwc2: gadget: stop current transfer on dequeue
>   usb: dwc2: gadget: kill ep0 requests before reinitializing core
>   usb: dwc2: gadget: only reset core after addressed state
>   usb: dwc2: gadget: handle reset interrupt before endpoint interrupts
>   usb: dwc2: exit hibernation on session request
> 
>  drivers/usb/dwc2/core.c  |  14 ++-
>  drivers/usb/dwc2/core.h  |   3 +-
>  drivers/usb/dwc2/core_intr.c |  29 --
>  drivers/usb/dwc2/gadget.c| 210 
> ++-
>  drivers/usb/dwc2/hcd.c   | 209 --
>  drivers/usb/dwc2/hcd_queue.c |  11 +++
>  drivers/usb/dwc2/platform.c  |  11 +++
>  7 files changed, 386 insertions(+), 101 deletions(-)
> 


For this series:

Tested-by: John Youn 
Acked-by: John Youn 

Regards,
John


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

Re: [PATCH v2] usb: gadget: f_sourcesink: fix function params handling

[Felipe Balbi]

On Thu, Sep 24, 2015 at 05:23:09PM +0200, Robert Baldyga wrote:
> Move function parameters to struct f_sourcesink to make them per instance
> instead of having them as global variables. Since we can have multiple
> instances of USB function we also want to have separate set of parameters
> for each instance.
> 
> Cc:  # 3.10+
> Signed-off-by: Robert Baldyga 

why do you think this is stable material ? Looks like it's not
fixing anything to me; just implementing a brand new requirement.

> ---
>  drivers/usb/gadget/function/f_sourcesink.c | 120 
> +++--
>  1 file changed, 62 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
> b/drivers/usb/gadget/function/f_sourcesink.c
> index 1353465..128abfb 100644
> --- a/drivers/usb/gadget/function/f_sourcesink.c
> +++ b/drivers/usb/gadget/function/f_sourcesink.c
> @@ -50,6 +50,13 @@ struct f_sourcesink {
>   struct usb_ep   *iso_in_ep;
>   struct usb_ep   *iso_out_ep;
>   int cur_alt;
> +
> + unsigned pattern;
> + unsigned isoc_interval;
> + unsigned isoc_maxpacket;
> + unsigned isoc_mult;
> + unsigned isoc_maxburst;
> + unsigned buflen;
>  };
>  
>  static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
> @@ -57,13 +64,6 @@ static inline struct f_sourcesink *func_to_ss(struct 
> usb_function *f)
>   return container_of(f, struct f_sourcesink, function);
>  }
>  
> -static unsigned pattern;
> -static unsigned isoc_interval;
> -static unsigned isoc_maxpacket;
> -static unsigned isoc_mult;
> -static unsigned isoc_maxburst;
> -static unsigned buflen;
> -
>  /*-*/
>  
>  static struct usb_interface_descriptor source_sink_intf_alt0 = {
> @@ -298,7 +298,9 @@ static struct usb_gadget_strings *sourcesink_strings[] = {
>  
>  static inline struct usb_request *ss_alloc_ep_req(struct usb_ep *ep, int len)
>  {
> - return alloc_ep_req(ep, len, buflen);
> + struct f_sourcesink *ss = ep->driver_data;
> +
> + return alloc_ep_req(ep, len, ss->buflen);
>  }
>  
>  void free_ep_req(struct usb_ep *ep, struct usb_request *req)
> @@ -357,22 +359,22 @@ autoconf_fail:
>   goto autoconf_fail;
>  
>   /* sanity check the isoc module parameters */
> - if (isoc_interval < 1)
> - isoc_interval = 1;
> - if (isoc_interval > 16)
> - isoc_interval = 16;
> - if (isoc_mult > 2)
> - isoc_mult = 2;
> - if (isoc_maxburst > 15)
> - isoc_maxburst = 15;
> + if (ss->isoc_interval < 1)
> + ss->isoc_interval = 1;
> + if (ss->isoc_interval > 16)
> + ss->isoc_interval = 16;
> + if (ss->isoc_mult > 2)
> + ss->isoc_mult = 2;
> + if (ss->isoc_maxburst > 15)
> + ss->isoc_maxburst = 15;
>  
>   /* fill in the FS isoc descriptors from the module parameters */
> - fs_iso_source_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
> - 1023 : isoc_maxpacket;
> - fs_iso_source_desc.bInterval = isoc_interval;
> - fs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
> - 1023 : isoc_maxpacket;
> - fs_iso_sink_desc.bInterval = isoc_interval;
> + fs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
> + 1023 : ss->isoc_maxpacket;
> + fs_iso_source_desc.bInterval = ss->isoc_interval;
> + fs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
> + 1023 : ss->isoc_maxpacket;
> + fs_iso_sink_desc.bInterval = ss->isoc_interval;
>  
>   /* allocate iso endpoints */
>   ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, _iso_source_desc);
> @@ -394,8 +396,8 @@ no_iso:
>   ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
>   }
>  
> - if (isoc_maxpacket > 1024)
> - isoc_maxpacket = 1024;
> + if (ss->isoc_maxpacket > 1024)
> + ss->isoc_maxpacket = 1024;
>  
>   /* support high speed hardware */
>   hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
> @@ -406,15 +408,15 @@ no_iso:
>* We assume that the user knows what they are doing and won't
>* give parameters that their UDC doesn't support.
>*/
> - hs_iso_source_desc.wMaxPacketSize = isoc_maxpacket;
> - hs_iso_source_desc.wMaxPacketSize |= isoc_mult << 11;
> - hs_iso_source_desc.bInterval = isoc_interval;
> + hs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
> + hs_iso_source_desc.wMaxPacketSize |= ss->isoc_mult << 11;
> + hs_iso_source_desc.bInterval = ss->isoc_interval;
>   hs_iso_source_desc.bEndpointAddress =
>   fs_iso_source_desc.bEndpointAddress;
>  
> - 

Re: [PATCH v3 1/2] usb: gadget: loopback: fix: Don't share qlen and buflen between instances

[Felipe Balbi]

On Thu, Sep 24, 2015 at 05:19:12PM +0200, Robert Baldyga wrote:
> On 09/22/2015 08:40 PM, Krzysztof Opasiak wrote:
> > Each instance of loopback function may have different qlen
> > and buflen attributes values. When linking function to
> > configuration those values had been assigned to global
> > variables. Linking other instance to config overwrites those
> > values.
> > 
> > This commit moves those values to f_loopback structure
> > to avoid overwriting. Now each function has its own instance
> > of those values.
> > 
> > Cc:  # 3.10+
> > Signed-off-by: Krzysztof Opasiak 
> 
> Reviewed-by: Robert Baldyga 

doesn't seem to fit stable IMO. Care to explain why you think we need
to backport this to v3.10+ ?

> 
> > ---
> >  drivers/usb/gadget/function/f_loopback.c |   24 +---
> >  1 file changed, 13 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/function/f_loopback.c 
> > b/drivers/usb/gadget/function/f_loopback.c
> > index 6e2fe63..e4bfed4 100644
> > --- a/drivers/usb/gadget/function/f_loopback.c
> > +++ b/drivers/usb/gadget/function/f_loopback.c
> > @@ -34,6 +34,9 @@ struct f_loopback {
> >  
> > struct usb_ep   *in_ep;
> > struct usb_ep   *out_ep;
> > +
> > +   unsignedqlen;
> > +   unsignedbuflen;
> >  };
> >  
> >  static inline struct f_loopback *func_to_loop(struct usb_function *f)
> > @@ -41,13 +44,10 @@ static inline struct f_loopback *func_to_loop(struct 
> > usb_function *f)
> > return container_of(f, struct f_loopback, function);
> >  }
> >  
> > -static unsigned qlen;
> > -static unsigned buflen;
> > -
> >  
> > /*-*/
> >  
> >  static struct usb_interface_descriptor loopback_intf = {
> > -   .bLength =  sizeof loopback_intf,
> > +   .bLength =  sizeof(loopback_intf),
> > .bDescriptorType =  USB_DT_INTERFACE,
> >  
> > .bNumEndpoints =2,
> > @@ -253,7 +253,7 @@ static void loopback_complete(struct usb_ep *ep, struct 
> > usb_request *req)
> > }
> >  
> > /* queue the buffer for some later OUT packet */
> > -   req->length = buflen;
> > +   req->length = loop->buflen;
> > status = usb_ep_queue(ep, req, GFP_ATOMIC);
> > if (status == 0)
> > return;
> > @@ -290,7 +290,9 @@ static void disable_loopback(struct f_loopback *loop)
> >  
> >  static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int 
> > len)
> >  {
> > -   return alloc_ep_req(ep, len, buflen);
> > +   struct f_loopback   *loop = ep->driver_data;
> > +
> > +   return alloc_ep_req(ep, len, loop->buflen);
> >  }
> >  
> >  static int enable_endpoint(struct usb_composite_dev *cdev, struct 
> > f_loopback *loop,
> > @@ -317,7 +319,7 @@ static int enable_endpoint(struct usb_composite_dev 
> > *cdev, struct f_loopback *lo
> >  * we buffer at most 'qlen' transfers; fewer if any need more
> >  * than 'buflen' bytes each.
> >  */
> > -   for (i = 0; i < qlen && result == 0; i++) {
> > +   for (i = 0; i < loop->qlen && result == 0; i++) {
> > req = lb_alloc_ep_req(ep, 0);
> > if (!req)
> > goto fail1;
> > @@ -391,10 +393,10 @@ static struct usb_function *loopback_alloc(struct 
> > usb_function_instance *fi)
> > lb_opts->refcnt++;
> > mutex_unlock(_opts->lock);
> >  
> > -   buflen = lb_opts->bulk_buflen;
> > -   qlen = lb_opts->qlen;
> > -   if (!qlen)
> > -   qlen = 32;
> > +   loop->buflen = lb_opts->bulk_buflen;
> > +   loop->qlen = lb_opts->qlen;
> > +   if (!loop->qlen)
> > +   loop->qlen = 32;
> >  
> > loop->function.name = "loopback";
> > loop->function.bind = loopback_bind;
> > 
> 

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 1/2] usb: gadget: loopback: fix: Don't share qlen and buflen between instances

[Krzysztof Opasiak]

On 09/24/2015 06:51 PM, Felipe Balbi wrote:

On Thu, Sep 24, 2015 at 05:19:12PM +0200, Robert Baldyga wrote:

On 09/22/2015 08:40 PM, Krzysztof Opasiak wrote:

Each instance of loopback function may have different qlen
and buflen attributes values. When linking function to
configuration those values had been assigned to global
variables. Linking other instance to config overwrites those
values.

This commit moves those values to f_loopback structure
to avoid overwriting. Now each function has its own instance
of those values.

Cc:  # 3.10+
Signed-off-by: Krzysztof Opasiak 


Reviewed-by: Robert Baldyga 


doesn't seem to fit stable IMO. Care to explain why you think we need
to backport this to v3.10+ ?



Let's consider following set of commands:

cd $CFS_ROOT/usb_gadget
mkdir g1
cd g1
mkdir functions/SourceSink.1
mkdir functions/SourceSink.2
echo 1024 > functions/SourceSink.1/buflen
echo 512 > functions/SourceSink.2/buflen
mkdir configs/c.1
ln -s functions/SourceSink.1 configs/c.1/
ln -s functions/SourceSink.2 configs/c.1/
echo $UDC > UDC

You assume that after executing this script you will get two instances 
of SourceSink functions and one of them will alloc requests buffers 
which has 1024 and second one which has only 512.


Unfortunately due to using global variables both of them are going to 
alloc only 512 bytes for each request. The same situation is with qlen. 
All instances are going to use attributes values from the last one in 
the system, even if they are in multiple gadgets.


In my opinion second patch in this series is much more important as it 
makes loopback function working again after almost year of being only 
/dev/null and /dev/zero. But that one should to go only to 3.18.


Best regards,
--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M

[Roland Weber]

Hello Andrew,

> ES1-111M running BIOS v1.13 and have had no problems booting from an 
> external USB stick to install Debian (using legacy mode -- I haven't 
> tried UEFI).  It also worked fine under v1.10 (factory-installed).

Thanks, that's good to know. Btw, I'm also using legacy mode.
Maybe I should try UEFI to see if it makes a difference.

> Thanks so much to you all for looking into the USB issues on this 
> system. My own 111M freezes at shutdown if 'noapic' is not specified on 
> the (stock Debian 3.16.0-4) kernel command line and throws all sorts of 
> EHCI errors otherwise so I look forward to the results of your efforts 
> in due time.

The feeling that I might be the only one in the world having this
kind of problem with this model has been nagging at me for a while.
Thanks for letting me know that there are others.

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

Re: [PATCH 2/2] Doc: usb: ci-hdrc-usb2: Add phy-clkgate-delay-us entry

[Fabio Estevam]

Hi Peter,

On Tue, Sep 8, 2015 at 11:27 PM, Peter Chen  wrote:

> I will queue both for next rc, thanks.

It would be nice if these patches could appear in linux-next soon.
--
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 0/1] USB DWC2 parity fix in isochronous mode

[John Youn]

On 9/24/2015 10:28 AM, Roman Bacik wrote:
>> -Original Message-
>> From: John Youn [mailto:john.y...@synopsys.com]
>> Sent: September-23-15 9:21 PM
>> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
>> u...@vger.kernel.org; Roman Bacik
>> Cc: linux-ker...@vger.kernel.org; bcm-kernel-feedback-list
>> Subject: Re: [PATCH v3 0/1] USB DWC2 parity fix in isochronous mode
>>
>> On 9/10/2015 6:14 PM, Scott Branden wrote:
>>> This patch contains a fix for a real world interop problem found when
>>> using the Synopsis DWC2 USB controller with isochronous audio as
>>> detailed in the commit message.
>>>
>>> Changes from v2:
>>>  - created s2c_hsotg_chage_ep_iso_parity function to call function in
>>> 3 places in code
>>>  - used hsotg->num_of_eps instead of MAX_EPS_CHANNELS
>>>
>>> Changes from v1:
>>>  - Address code review comments as per previous responses:
>>>  - renamed parity_set to has_correct_parity and reorder some logic
>>>
>>>
>>> Roman Bacik (1):
>>>   usb: dwc2: gadget: parity fix in isochronous mode
>>>
>>>  drivers/usb/dwc2/core.h   |  1 +
>>>  drivers/usb/dwc2/gadget.c | 58
>> ++-
>>>  drivers/usb/dwc2/hw.h |  1 +
>>>  3 files changed, 54 insertions(+), 6 deletions(-)
>>>
>>
>> This seems to break slave mode on my platform. It seems to be dropping a
>> lot of packets. I tried bInterval=4,5,6, ISO OUT.
>> I'll need to take a closer look to determine why. Probably later this week.
>>
>> Are you able to run in slave mode on your platform? If so can you try it out?
>>
>> Regards,
>> John
> 
> Our current test procedure is as follows:
> 
> Build Linux kernel with: 
> 
> Device Drivers
> - Sound Card Support 'SOUND=y': ALSA 'SND=y'
> - Generic sound devices: Dummy (/dev/null) soundcard 'SND_DUMMY=y'
> - USB support 'USB_SUPPORT=y':
> DesignWare USB2 DRD Core support 'USB_DWC2=y'
> - Gadget only mode 'USB_DWC2_PERIPHERAL=y'
> DWC2 Platform 'USB_DWC2_PLATFORM=y'
> - USB Gadget Support 'USB_GADGET=y': Audio Gadget 'USB_AUDIO=y'
> - PHY Subsystem: Broadcom Kona USB2 PHY Driver 'BCM_KONA_USB2_PHY=y'
> 
> If you have only even hosts, you can change the default micro frame parity to 
> odd:
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index ad45b0b..80bde75 100644
> @@ -2709,7 +2709,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
>  switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
>  case USB_ENDPOINT_XFER_ISOC:
>  epctrl |= DXEPCTL_EPTYPE_ISO;
> -epctrl |= DXEPCTL_SETEVENFR;
> +epctrl |= DXEPCTL_SETODDFR;
>  hs_ep->isochronous = 1;
>  if (dir_in)
>  hs_ep->periodic = 1;
> @@ -2777,7 +2777,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
>   
>  /* for non control endpoints, set PID to D0 */
>  if (index)
> -epctrl |= DXEPCTL_SETD0PID;
> +epctrl |= DXEPCTL_SETD1PID;
>   
>  dev_dbg(hsotg->dev, "%s: write DxEPCTL=0x%08x\n",
>  __func__, epctrl);
> 
> To test OUT direction:
> Host:
> aplay -D plughw:2 -r 48000 -f S16_LE tone_48stereo.wav
> Device:
> arecord -D plughw:0 -r 48000 -f S16_LE -c 1 /tmp/rec.pcm
> 
> To test IN direction:
> Host:
> arecord -D plughw:2 -r 48000 -f S16_LE -c 1 /tmp/rec.pcm
> Device:
> aplay -D plughw:0 -r 48000 -f S16_LE /tmp/rec.pcm
> 
> If you would like, we can try to test your procedure provided you send us 
> enough details.
> Regards,
> 
> Roman
> 
> 

I looked at it a bit more and I think there are two issues.

In slave-mode, the xfercomplete interrupt is not used for OUT
transfers, so the has_correct_parity flag is never set.

The other issue is that we occasionally get the incomplete
interrupt twice in a microframe causing the parity to be set
incorrectly for the next frame. Thus we will continuously miss
until this occurs again, correcting it.

These two problems in combination cause dropped packets
throughout operation.

If you give me some time I can see if I can fix up this patch to
work with slave mode.

I've tried all our hosts here and they are all even hosts, so I
tried flipping the default parity. The issue exists in either
case.

Regards,
John


--
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: Problems with printk logs and my driver

[Alan Stern]

On Thu, 24 Sep 2015, Jiri Kosina wrote:

> On Wed, 23 Sep 2015, Alan Stern wrote:
> 
> > Your mistake was thinking that the driver for your keyboard is usbkbd.  
> > It isn't.  It's usbhid, as you can see in the "lsusb -t" output above.
> 
> As Eric is absolutely not the first person ever who got confused by this 
> (and I can certainly understand the reasons for this confusion), I've been 
> thinking for quite some time already about renaming this driver (and 
> usbmouse as well). We'd probably want to make obvious from the name that 
> this isn't regular usb keyboard driver.
> 
> Any opinions on usbkbd-simple or usbkbd-dummy? The most accurate would of 
> course be usbkbd-boot, but that might be equally confusing.

I prefer -simple over -dummy.  Even better would be usbkbd-alt.

> The drawback I can see in renaming the driver is various embedded folks 
> having he name hardcoded in their scripts.

Yes, that's a real problem.  Similar considerations apply to .config 
files, if you also change the Kconfig symbol name.

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