Re: USB to Serial converter code pl2303
Am 20.02.2014 19:19, schrieb Frank Schäfer: We have tested different baud rates with the PL2303HX china clones. All are working including non standard baud rates. Urgh... Karsten... don't confuse the people. :/ Yes, _your_ clones behave exactly like HX originals, but others don't. O.K. - all of them is not correct. My meaning was all i have tested from the PL2303HX clones. At least all standard driver only support the standard baud rates. That's sufficient for nearly all purposes. But if someone needs to support non standard baud rates, it might be good to know that this is possible with some chips. Cheers Karsten -- 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 v10 01/15] usb: doc: phy-mxs: Add more compatible strings
On Thu, Feb 20, 2014 at 05:14:30AM +, Peter Chen wrote: > Add "fsl,imx6q-usbphy" for imx6dq and imx6dl, add > "fsl,imx6sl-usbphy" for imx6sl. > > Signed-off-by: Peter Chen > --- > Documentation/devicetree/bindings/usb/mxs-phy.txt |3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt > b/Documentation/devicetree/bindings/usb/mxs-phy.txt > index 5835b27..b43d4c9e 100644 > --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt > +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt > @@ -1,7 +1,8 @@ > * Freescale MXS USB Phy Device > > Required properties: > -- compatible: Should be "fsl,imx23-usbphy" > +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q-usbphy" > + for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl Minor nit, but could we restructure this as something like the following, with each string on a new line: - compatible: should contain: * "fsl,imx23-usbphy" for imx23 and imx28 * "fsl,imx6q-usbphy" for imx6dq and imx6dl * "fsl,imx6sl-usbphy" for imx6sl It makes it a bit easier to read. I see the existing "fsl,imx23-usbphy" is used as a fallback for "fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in existing DTs. Is this expected going forward? It might be worth mentioning. Otherwise this looks fine to me. Thanks, Mark. -- 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 v10 04/15] usb: doc: phy-mxs: update binding for adding anatop phandle
On Thu, Feb 20, 2014 at 05:14:33AM +, Peter Chen wrote:
> Add anatop phandle which is used to access anatop registers to
> control PHY's power and other USB operations.
>
> Signed-off-by: Peter Chen
> ---
> Documentation/devicetree/bindings/usb/mxs-phy.txt |2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> index b43d4c9e..fc6659d 100644
> --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> @@ -5,10 +5,12 @@ Required properties:
>for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl
> - reg: Should contain registers location and length
> - interrupts: Should contain phy interrupt
> +- fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
This is now required?
What happens to those existing DTBs that claim compatibility with
"fsl,imx6q-usbphy" but don't have an fsl,anatop property?
Thanks,
Mark.
>
> Example:
> usbphy1: usbphy@020c9000 {
> compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> reg = <0x020c9000 0x1000>;
> interrupts = <0 44 0x04>;
> + fsl,anatop = <&anatop>;
> };
> --
> 1.7.8
>
>
>
--
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 v10 10/15] usb: phy-mxs: Add implementation of set_wakeup
On Thu, Feb 20, 2014 at 05:14:39AM +, Peter Chen wrote:
> When we need the PHY can be waken up by external signals,
> we can call this API. Besides, we call mxs_phy_disconnect_line
> at this API to close the connection between USB PHY and
> controller, after that, the line state from controller is SE0.
> Once the PHY is out of power, without calling mxs_phy_disconnect_line,
> there are unknown wakeups due to dp/dm floating at device mode.
>
> Signed-off-by: Peter Chen
> ---
> drivers/usb/phy/phy-mxs-usb.c | 116
> +
> 1 files changed, 116 insertions(+), 0 deletions(-)
[...]
> +static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
> +{
> + bool vbus_is_on = false;
> +
> + /* If the SoCs don't need to disconnect line without vbus, quit */
> + if (!(mxs_phy->data->flags & MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS))
> + return;
> +
> + /* If the SoCs don't have anatop, quit */
> + if (!mxs_phy->regmap_anatop)
> + return;
So it looks like fsl,anatop is truly optional.
Thanks,
Mark.
--
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 v10 01/15] usb: doc: phy-mxs: Add more compatible strings
> > > > Required properties: > > -- compatible: Should be "fsl,imx23-usbphy" > > +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q- > usbphy" > > + for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl > > Minor nit, but could we restructure this as something like the following, > with each string on a new line: > > - compatible: should contain: > * "fsl,imx23-usbphy" for imx23 and imx28 > * "fsl,imx6q-usbphy" for imx6dq and imx6dl > * "fsl,imx6sl-usbphy" for imx6sl > > It makes it a bit easier to read. Thanks, will change like above. > > I see the existing "fsl,imx23-usbphy" is used as a fallback for > "fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in > existing DTs. > > Is this expected going forward? It might be worth mentioning. > These SoCs used the same FSL imx PHY, but different versions. imx23/imx28 are the first version, more improvements are at later SoCs (like imx6x) version. Keep "fsl,imx23-usbphy" at imx6 dts will be user know it is from imx23's. If you think it does not need, I can delete "fsl,imx23-usbphy" from imx6 dts. Peter -- 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 v10 01/15] usb: doc: phy-mxs: Add more compatible strings
On 02/21/2014 10:40 AM, Peter Chen wrote: > >>> >>> Required properties: >>> -- compatible: Should be "fsl,imx23-usbphy" >>> +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q- >> usbphy" >>> + for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl >> >> Minor nit, but could we restructure this as something like the following, >> with each string on a new line: >> >> - compatible: should contain: >> * "fsl,imx23-usbphy" for imx23 and imx28 >> * "fsl,imx6q-usbphy" for imx6dq and imx6dl >> * "fsl,imx6sl-usbphy" for imx6sl >> >> It makes it a bit easier to read. > > Thanks, will change like above. > >> >> I see the existing "fsl,imx23-usbphy" is used as a fallback for >> "fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in >> existing DTs. >> >> Is this expected going forward? It might be worth mentioning. >> > > These SoCs used the same FSL imx PHY, but different versions. > imx23/imx28 are the first version, more improvements are at > later SoCs (like imx6x) version. Keep "fsl,imx23-usbphy" at > imx6 dts will be user know it is from imx23's. If you think > it does not need, I can delete "fsl,imx23-usbphy" from imx6 dts. You should go after compatibility here. List (all) phys that are comaptible, start with most specific end with most generic. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v10 04/15] usb: doc: phy-mxs: update binding for adding anatop phandle
On Fri, Feb 21, 2014 at 09:14:44AM +, Mark Rutland wrote: > On Thu, Feb 20, 2014 at 05:14:33AM +, Peter Chen wrote: > > Add anatop phandle which is used to access anatop registers to > > control PHY's power and other USB operations. > > > > Signed-off-by: Peter Chen > > --- > > Documentation/devicetree/bindings/usb/mxs-phy.txt |2 ++ > > 1 files changed, 2 insertions(+), 0 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt > > b/Documentation/devicetree/bindings/usb/mxs-phy.txt > > index b43d4c9e..fc6659d 100644 > > --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt > > +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt > > @@ -5,10 +5,12 @@ Required properties: > >for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl > > - reg: Should contain registers location and length > > - interrupts: Should contain phy interrupt > > +- fsl,anatop: phandle for anatop register, it is only for imx6 SoC series > > This is now required? Yes, it is required. > > What happens to those existing DTBs that claim compatibility with > "fsl,imx6q-usbphy" but don't have an fsl,anatop property? > The flag stands for the SoC has anatop will be 0, no anatop operation will be done, the old-version dts can work with update driver, but less features and one or two bug exists. Peter -- 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 v10 10/15] usb: phy-mxs: Add implementation of set_wakeup
On Fri, Feb 21, 2014 at 09:21:27AM +, Mark Rutland wrote:
> On Thu, Feb 20, 2014 at 05:14:39AM +, Peter Chen wrote:
> > When we need the PHY can be waken up by external signals,
> > we can call this API. Besides, we call mxs_phy_disconnect_line
> > at this API to close the connection between USB PHY and
> > controller, after that, the line state from controller is SE0.
> > Once the PHY is out of power, without calling mxs_phy_disconnect_line,
> > there are unknown wakeups due to dp/dm floating at device mode.
> >
> > Signed-off-by: Peter Chen
> > ---
> > drivers/usb/phy/phy-mxs-usb.c | 116
> > +
> > 1 files changed, 116 insertions(+), 0 deletions(-)
>
> [...]
>
> > +static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
> > +{
> > + bool vbus_is_on = false;
> > +
> > + /* If the SoCs don't need to disconnect line without vbus, quit */
> > + if (!(mxs_phy->data->flags & MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS))
> > + return;
> > +
> > + /* If the SoCs don't have anatop, quit */
> > + if (!mxs_phy->regmap_anatop)
> > + return;
>
> So it looks like fsl,anatop is truly optional.
>
Like I said at 04/15, if the user wants full features and less bugs,
it is required.
--
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 v10 01/15] usb: doc: phy-mxs: Add more compatible strings
On Fri, Feb 21, 2014 at 10:46:41AM +0100, Marc Kleine-Budde wrote: > On 02/21/2014 10:40 AM, Peter Chen wrote: > > > >>> > >>> Required properties: > >>> -- compatible: Should be "fsl,imx23-usbphy" > >>> +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q- > >> usbphy" > >>> + for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl > >> > >> Minor nit, but could we restructure this as something like the following, > >> with each string on a new line: > >> > >> - compatible: should contain: > >> * "fsl,imx23-usbphy" for imx23 and imx28 > >> * "fsl,imx6q-usbphy" for imx6dq and imx6dl > >> * "fsl,imx6sl-usbphy" for imx6sl > >> > >> It makes it a bit easier to read. > > > > Thanks, will change like above. > > > >> > >> I see the existing "fsl,imx23-usbphy" is used as a fallback for > >> "fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in > >> existing DTs. > >> > >> Is this expected going forward? It might be worth mentioning. > >> > > > > These SoCs used the same FSL imx PHY, but different versions. > > imx23/imx28 are the first version, more improvements are at > > later SoCs (like imx6x) version. Keep "fsl,imx23-usbphy" at > > imx6 dts will be user know it is from imx23's. If you think > > it does not need, I can delete "fsl,imx23-usbphy" from imx6 dts. > > You should go after compatibility here. List (all) phys that are > comaptible, start with most specific end with most generic. > > Marc > Then, I should keep imx6 dts unchanging. Then, what I need to mention at this binding doc? -- 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: musb - babble interrupt recovery
Thank you for reply, On 21.2.2014 2:03, Felipe Balbi wrote: heh, you can drop the Mr. ;-) :-) We have another version in TI's tree (git.ti.com) which will be sent soonish for mainline. Just hashing out a few extra details. That sounds great and i didn't know about that repository. I'll check patches there and possibly try it on BBB, maybe there is also something, that could also address other issue, i'm having with musb (DMA errors and high CPU load) and then possibly post some field report. George and Roger have been involved in all the work and deserve all the credit for getting all of that working fine (well, plus Ravi, Sebastian and few others). As an user I also would like to join with thanks for work there. Best regards, Michal -- 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 4/7] ECHI Platform: Merge ppc-of EHCI driver into the ehci-platform driver
[Adding Tony Prisk to Cc]
On Fri, Feb 21, 2014 at 06:31:30AM +, Alistair Popple wrote:
> Currently the ppc-of driver uses the compatibility string
> "usb-ehci". This means platforms that use device-tree and implement an
> EHCI compatible interface have to either use the ppc-of driver or add
> a compatible line to the ehci-platform driver. It would be more
> appropriate for the platform driver to be compatible with "usb-ehci"
> as non-powerpc platforms are also beginning to utilise device-tree.
>
> This patch merges the device tree property parsing from ehci-ppc-of
> into the platform driver and adds a "usb-ehci" compatibility
> string. The existing ehci-ppc-of driver is removed and the 440EPX
> specific quirks are added to the ehci-platform driver.
>
> Signed-off-by: Alistair Popple
> ---
> drivers/usb/host/Kconfig |7 +-
> drivers/usb/host/ehci-hcd.c |5 -
> drivers/usb/host/ehci-platform.c | 87 +-
> drivers/usb/host/ehci-ppc-of.c | 238
> --
> 4 files changed, 89 insertions(+), 248 deletions(-)
> delete mode 100644 drivers/usb/host/ehci-ppc-of.c
[...]
> + /* Device tree properties if available will override platform data. */
> + dn = hcd_to_bus(hcd)->controller->of_node;
> + if (dn) {
> + if (of_get_property(dn, "big-endian", NULL)) {
> + ehci->big_endian_mmio = 1;
> + ehci->big_endian_desc = 1;
> + }
> + if (of_get_property(dn, "big-endian-regs", NULL))
> + ehci->big_endian_mmio = 1;
> + if (of_get_property(dn, "big-endian-desc", NULL))
> + ehci->big_endian_desc = 1;
> + }
Please use of_property_read_bool for these.
This driver already handles "via,vt8500-ehci" and "wm,prizm-ehci" which
aren't documented to handle these properties, but now gain support for
them. It might be worth unifying the binding documents if there's
nothing special about those two host controllers.
We seem to have two binding documents for "via,vt8500-ehci", so some
cleanup is definitely in order.
Tony, you seem to have written both documents judging by 95e9fd10f06c
and 8ad551d150e3. Do you have any issue with merging both of these into
a common usb-ehci document?
> +
> + np = of_find_compatible_node(NULL, NULL, "ibm,usb-ohci-440epx");
> + if (np != NULL) {
> + /* claim we really affected by usb23 erratum */
> + if (!of_address_to_resource(np, 0, &res))
> + ehci->ohci_hcctrl_reg =
> + devm_ioremap(&pdev->dev,
> +res.start + OHCI_HCCTRL_OFFSET,
> +OHCI_HCCTRL_LEN);
> + else
> + ehci_dbg(ehci, "%s: no ohci offset in fdt\n",
> __FILE__);
> + if (!ehci->ohci_hcctrl_reg) {
> + ehci_dbg(ehci, "%s: ioremap for ohci hcctrl failed\n",
> + __FILE__);
> + } else {
> + ehci->has_amcc_usb23 = 1;
> + }
> + }
As this is being dropped into a generic driver, it would be nice to have
a comment explaining why we need to do this -- not everyone using this
will know the powerpc history. It certainly seems odd to look for
another node in the tree that this driver isn't necessarily handling,
and that should probably be explained.
As this bit of fixup is only needed for powerpc, it would be nice to not
have to do it elsewhere. Perhaps these could be factored out into a
ppc_platform_reset function that could be empty stub for other
architectures.
> +
> + if (of_device_is_compatible(dn, "ibm,usb-ehci-440epx")) {
> + retval = ppc44x_enable_bmt(dn);
> + ehci_dbg(ehci, "Break Memory Transfer (BMT) is %senabled!\n",
> + retval ? "NOT " : "");
> + }
> +
Likewise.
[...]
> + /* use request_mem_region to test if the ohci driver is loaded. if so
> +* ensure the ohci core is operational.
> +*/
Nit: comment code style (should have a newline after the /* according to
Documentation/CodingStyle).
> + if (ehci->has_amcc_usb23) {
> + np = of_find_compatible_node(NULL, NULL,
> "ibm,usb-ohci-440epx");
> + if (np != NULL) {
> + if (!of_address_to_resource(np, 0, &res))
> + if (!request_mem_region(res.start,
> + 0x4, hcd_name))
> + set_ohci_hcfs(ehci, 1);
> + else
> + release_mem_region(res.start, 0x4);
> + else
> + ehci_dbg(ehci, "%s: no ohci offset in fdt\n",
> +
Re: musb - babble interrupt recovery
Hi Michal, On 02/21/2014 01:44 PM, Michal Šmucr wrote: > Thank you for reply, > > On 21.2.2014 2:03, Felipe Balbi wrote: >> >> heh, you can drop the Mr. ;-) >> > :-) > >> >> We have another version in TI's tree (git.ti.com) which will be sent >> soonish for mainline. Just hashing out a few extra details. >> > That sounds great and i didn't know about that repository. I'll check patches > there and possibly try it on BBB, maybe there is also something, that could > also address other issue, i'm having with musb (DMA errors and high CPU load) > and then possibly post some field report. > >> George and Roger have been involved in all the work and deserve all the >> credit for getting all of that working fine (well, plus Ravi, Sebastian >> and few others). >> > As an user I also would like to join with thanks for work there. > Could you please try the 2 patches posted here [1] for the babble problem, esp. patch 2. This is not about babble recovery but to prevent the babble from happening in the first place. The patches are on their way to linux-next and 3.14-rc as well. [1] - http://thread.gmane.org/gmane.linux.kernel/1640032 cheers, -roger -- 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 RFC] usb: gadget: Add xilinx axi usb2 device support
On Thu, Feb 20, 2014 at 06:23:13PM +, Felipe Balbi wrote:
> Hi,
>
> On Wed, Feb 19, 2014 at 03:10:45PM +0530, Subbaraya Sundeep Bhatta wrote:
> > This patch adds xilinx axi usb2 device driver support
> >
> > Signed-off-by: Subbaraya Sundeep Bhatta
> > ---
> > .../devicetree/bindings/usb/xilinx_usb.txt | 21 +
> > drivers/usb/gadget/Kconfig | 14 +
> > drivers/usb/gadget/Makefile|1 +
> > drivers/usb/gadget/xilinx_udc.c| 2045
> >
> > 4 files changed, 2081 insertions(+), 0 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/usb/xilinx_usb.txt
> > create mode 100644 drivers/usb/gadget/xilinx_udc.c
> >
> > diff --git a/Documentation/devicetree/bindings/usb/xilinx_usb.txt
> > b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
> > new file mode 100644
> > index 000..acf03ab
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
> > @@ -0,0 +1,21 @@
> > +Xilinx AXI USB2 device controller
> > +
> > +Required properties:
> > +- compatible : Should be "xlnx,axi-usb2-device-4.00.a"
Is "axi-usb2-device" the official device name?
> > +- reg : Physical base address and size of the Axi USB2
> > + device registers map.
> > +- interrupts : Property with a value describing the interrupt
> > + number.
Does the device only have a single interrupt?
> > +- interrupt-parent : Must be core interrupt controller
Is this strictly necessary?
> > +- xlnx,include-dma : Must be 1 or 0 based on whether DMA is included
> > + in IP or not.
Perhaps xlnx,has-builtin-dma would better describe this?
> > +
> > +Example:
> > + axi-usb2-device@42e0 {
> > +compatible = "xlnx,axi-usb2-device-4.00.a";
> > +interrupt-parent = <0x1>;
> > +interrupts = <0x0 0x39 0x1>;
> > +reg = <0x42e0 0x1>;
> > +xlnx,include-dma = <0x1>;
> > +};
> > +
>
> you need to Cc devicet...@vger.kernel.org for this.
Cheers Filipe; thanks for adding us to Cc :)
[...]
> > + /* Map the registers */
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + udc->base_address = devm_ioremap_nocache(&pdev->dev, res->start,
> > +resource_size(res));
>
> use devm_ioremap_resource() instead.
Also, res might be NULL. You should check that before dereferencing it.
Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi Roger, On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: > Hi, > > On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: >> On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: >>> Hi, >>> >>> On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: > On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: > For the controller drivers the PHYs are just a resource like any > other. The controller drivers can't have any responsibility of > them. They should not care if PHY drivers are available for them or > not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. >>> >>> Yes, true. In my head I was comparing the PHY only to resources like >>> gpios, clocks, dma channels, etc. that are often optional to the >>> drivers. >>> But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. >>> >>> Not true in our case. Platforms using Intel's SoCs and chip sets may >>> or may not have controllable USB PHY. Quite often they don't. The >>> Baytrails have usually ULPI PHY for USB2, but that does not mean they >>> provide any vendor specific functions or any need for a driver in any >>> case. >> >> that's different from what I heard. > > I don't know where you got that impression, but it's not true. The > Baytrail SoCs for example don't have internal USB PHYs, which means > the manufacturers using it can select what they want. So we have > boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. >>> >>> No, only the USB2 PHY is external. I'm giving you wrong information, >>> I'm sorry about that. Need to concentrate on what I'm writing. >>> >>> >>> > This is really good to get. We have some projects where we are dealing > with more embedded environments, like IVI, where the kernel should be > stripped of everything useless. Since the PHYs are autonomous, we > should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? >>> >>> OMAP has it's own glue driver, so shouldn't it depend on the PHY >>> layer? >> >> right, but the PHY is connected to the dwc3 core and not to the glue. >>> I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) >>> >>> I don't think dwc3 (core) should care about that. The PHY layer needs >>> to tell us that. If the PHY driver that the platform depends is not >>> available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up >>> returning -EPROBE_DEFER. >> >> I don't think the PHY layer can 'reliably' tell if PHY driver is available or >> not. Consider when the phy_provider_register fails, there is no way to know >> if >> PHY driver is available or not. There are a few cases where PHY layer returns >> -EPROBE_DEFER but none of them can tell for sure that PHY driver is either >> available and failed or not available at all. It would be best for us to >> leave >> that to the platforms if we want to be sure if the platform needs a PHY or >> not. >> > > Just to summarize this thread on what we need Thanks for summarizing. > > 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed or > not. > It should be as generic as possible. > > 2) dwc3 core should continue probe even if PHY layer is not enabled, as not > all platforms need it. > > 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) > > 4) dwc3 core should error out on any error condition if PHY device is > available and caused some error, > e.g. init error. > > 5) dwc3 core should return EPROBE_DEFER if PH
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: > Hi Roger, > > On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: >> Hi, >> >> On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: >>> On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: Hi, On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: > On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: >> On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: >> For the controller drivers the PHYs are just a resource like any >> other. The controller drivers can't have any responsibility of >> them. They should not care if PHY drivers are available for them or >> not, or even if the PHY framework is available or not. > > huh? If memory isn't available you don't continue probing, right ? If > your IORESOURCE_MEM is missing, you also don't continue probing, if your > IRQ line is missing, you bail too. Those are also nothing but resources > to the driver, what you're asking here is to treat PHY as a _different_ > resource; which might be fine, but we need to make sure we don't > continue probing when a PHY is missing in a platform that certainly > needs a PHY. Yes, true. In my head I was comparing the PHY only to resources like gpios, clocks, dma channels, etc. that are often optional to the drivers. > But I really want to see the argument against using no-op. As far as I > could see, everybody needs a PHY driver one way or another, some > platforms just haven't sent any PHY driver upstream and have their own > hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. >>> >>> that's different from what I heard. >> >> I don't know where you got that impression, but it's not true. The >> Baytrail SoCs for example don't have internal USB PHYs, which means >> the manufacturers using it can select what they want. So we have >> boards where PHY driver(s) is needed and boards where it isn't. > > alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? > You have an external PIPE3 interface ? That's quite an achievement, > kudos to your HW designers. Getting timing closure on PIPE3 is a > difficult task. No, only the USB2 PHY is external. I'm giving you wrong information, I'm sorry about that. Need to concentrate on what I'm writing. >> This is really good to get. We have some projects where we are dealing >> with more embedded environments, like IVI, where the kernel should be >> stripped of everything useless. Since the PHYs are autonomous, we >> should be able to disable the PHY libraries/frameworks. > > hmmm, in that case it's a lot easier to treat. We can use > ERR_PTR(-ENXIO) as an indication that the framework is disabled, or > something like that. > > The difficult is really reliably supporting e.g. OMAP5 (which won't work > without a PHY) and your BayTrail with autonomous PHYs. What can we use > as an indication ? OMAP has it's own glue driver, so shouldn't it depend on the PHY layer? >>> >>> right, but the PHY is connected to the dwc3 core and not to the glue. > I mean, I need to know that a particular platform depends on a PHY > driver before I decide to return -EPROBE_DEFER or just assume the PHY > isn't needed ;-) I don't think dwc3 (core) should care about that. The PHY layer needs to tell us that. If the PHY driver that the platform depends is not available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up returning -EPROBE_DEFER. >>> >>> I don't think the PHY layer can 'reliably' tell if PHY driver is available >>> or >>> not. Consider when the phy_provider_register fails, there is no way to know >>> if >>> PHY driver is available or not. There are a few cases where PHY layer >>> returns >>> -EPROBE_DEFER but none of them can tell for sure that PHY driver is either >>> available and failed or not available at all. It would be best for us to >>> leave >>> that to the platforms if we want to be sure if the platform needs a PHY or >>> not. >>> >> >> Just to summarize this thread on what we need > > Thanks for summarizing. >> >> 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed >> or not. >> It should be as generic as possible. >> >> 2) dwc3 core should continue probe even if PHY layer is not enabled, as not >> all platforms need it. >> >> 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) >> >> 4)
Re: [PATCH v10 01/15] usb: doc: phy-mxs: Add more compatible strings
On Fri, Feb 21, 2014 at 09:40:29AM +, Peter Chen wrote: > > > > > > > Required properties: > > > -- compatible: Should be "fsl,imx23-usbphy" > > > +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q- > > usbphy" > > > + for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl > > > > Minor nit, but could we restructure this as something like the following, > > with each string on a new line: > > > > - compatible: should contain: > > * "fsl,imx23-usbphy" for imx23 and imx28 > > * "fsl,imx6q-usbphy" for imx6dq and imx6dl > > * "fsl,imx6sl-usbphy" for imx6sl > > > > It makes it a bit easier to read. > > Thanks, will change like above. > > > > > I see the existing "fsl,imx23-usbphy" is used as a fallback for > > "fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in > > existing DTs. > > > > Is this expected going forward? It might be worth mentioning. > > > > These SoCs used the same FSL imx PHY, but different versions. > imx23/imx28 are the first version, more improvements are at > later SoCs (like imx6x) version. Keep "fsl,imx23-usbphy" at > imx6 dts will be user know it is from imx23's. If you think > it does not need, I can delete "fsl,imx23-usbphy" from imx6 dts. I'm not arguing to remove it, I'm suggesting it might be worth mentioning that it's not mutually exclusive, and can be a fallback for the other strings. Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH] xhci: Prevent runtime pm from autosuspending during initialization
If autosuspend is set to zero the usb-2 roothub will try to suspend
the controller before usb-3 parts are initialized.
Prevent this by incrementing the usage counter before usb-2 registers
its roothub. Decrement the counter when usb-3 roothub is allocated.
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6fe577d..9301aff 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4753,6 +4753,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t
get_quirks)
* companion controller.
*/
hcd->has_tt = 1;
+ /* prevent USB2 runtime pm until USB3 parts are initialized */
+ pm_runtime_get_noresume(hcd->self.controller);
} else {
/* xHCI private pointer was set in xhci_pci_probe for the second
* registered roothub.
@@ -4765,6 +4767,9 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t
get_quirks)
if (xhci->hci_version < 0x100)
hcd->self.no_sg_constraint = 1;
+ /* USB3 initialized far enough to allow runtime pm suspend */
+ pm_runtime_put_noidle(hcd->self.controller);
+
return 0;
}
--
1.8.1.2
--
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] usb/xhci: avoid kernel panic on xhci_suspend()
On 01/08/2014 09:53 PM, David Cohen wrote: On Wed, Jan 08, 2014 at 10:48:06AM -0500, Alan Stern wrote: On Tue, 7 Jan 2014, Greg KH wrote: On Tue, Jan 07, 2014 at 05:44:26PM -0800, David Cohen wrote: From: jianqian There is a possible kernel panic faced on xhci_suspend(). Due to kernel modified the hub autosupend_delay to 0s, after usb1 root hub finishes initialization, it will trigger runtime_suspend and then it will trigger xhci runtime suspend. But at that time, if xhci->shared_hcd is still doing initialization, it is possible to face null pointer kernel panic in xhci_suspend() function. This patch checks if xhci->shared_hcd is null to avoid panic. That sounds like this is a race that should be fixed properly, not just papered over, right? That was my reaction too. The best way to solve the problem is to prevent the USB-2 root hub from suspending until after the USB-3 root hub has been registered. That makes sense. Thanks for the feedback. I'll check for a new approach. Could you check if this patch works for you?: http://marc.info/?l=linux-usb&m=139298822514995&w=2 I'm not able to reproduce the original issue 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 RFC] usb: gadget: Add xilinx axi usb2 device support
Hi Mark,
On 02/21/2014 01:04 PM, Mark Rutland wrote:
>
> On Thu, Feb 20, 2014 at 06:23:13PM +, Felipe Balbi wrote:
>> Hi,
>>
>> On Wed, Feb 19, 2014 at 03:10:45PM +0530, Subbaraya Sundeep Bhatta wrote:
>>> This patch adds xilinx axi usb2 device driver support
>>>
>>> Signed-off-by: Subbaraya Sundeep Bhatta
>>> ---
>>> .../devicetree/bindings/usb/xilinx_usb.txt | 21 +
>>> drivers/usb/gadget/Kconfig | 14 +
>>> drivers/usb/gadget/Makefile|1 +
>>> drivers/usb/gadget/xilinx_udc.c| 2045
>>>
>>> 4 files changed, 2081 insertions(+), 0 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/usb/xilinx_usb.txt
>>> create mode 100644 drivers/usb/gadget/xilinx_udc.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/xilinx_usb.txt
>>> b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
>>> new file mode 100644
>>> index 000..acf03ab
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
>>> @@ -0,0 +1,21 @@
>>> +Xilinx AXI USB2 device controller
>>> +
>>> +Required properties:
>>> +- compatible : Should be "xlnx,axi-usb2-device-4.00.a"
>
> Is "axi-usb2-device" the official device name?
It is the name of IP which Xilinx have and we are using names in this format.
>>> +- reg : Physical base address and size of the Axi USB2
>>> + device registers map.
>>> +- interrupts : Property with a value describing the interrupt
>>> + number.
>
> Does the device only have a single interrupt?
I believe so.
>>> +- interrupt-parent : Must be core interrupt controller
>
> Is this strictly necessary?
I am not sure what do you mean by that. If you mean that interrupt-parent
can be written to the root of DTS file then we can setup system with more
interrupt controllers that's why it is required.
If we can point to standard interrupt description then please point me to
exact description you would like to see here and we can change it.
>>> +- xlnx,include-dma : Must be 1 or 0 based on whether DMA is included
>>> + in IP or not.
>
> Perhaps xlnx,has-builtin-dma would better describe this?
No opinion.
>>> +
>>> +Example:
>>> + axi-usb2-device@42e0 {
>>> +compatible = "xlnx,axi-usb2-device-4.00.a";
>>> +interrupt-parent = <0x1>;
>>> +interrupts = <0x0 0x39 0x1>;
>>> +reg = <0x42e0 0x1>;
>>> +xlnx,include-dma = <0x1>;
>>> +};
>>> +
>>
>> you need to Cc devicet...@vger.kernel.org for this.
>
> Cheers Filipe; thanks for adding us to Cc :)
>
Sundeep with CC devicetree list in next patch version.
>>> + /* Map the registers */
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + udc->base_address = devm_ioremap_nocache(&pdev->dev, res->start,
>>> +resource_size(res));
>>
>> use devm_ioremap_resource() instead.
>
> Also, res might be NULL. You should check that before dereferencing it.
yes it is necessary for both cases with devm_ioremap_nocache
or with devm_ioremap_resource.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
signature.asc
Description: OpenPGP digital signature
Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support
On Fri, Feb 21, 2014 at 01:41:03PM +, Michal Simek wrote: > Hi Mark, > > On 02/21/2014 01:04 PM, Mark Rutland wrote: > > > > On Thu, Feb 20, 2014 at 06:23:13PM +, Felipe Balbi wrote: > >> Hi, > >> > >> On Wed, Feb 19, 2014 at 03:10:45PM +0530, Subbaraya Sundeep Bhatta wrote: > >>> This patch adds xilinx axi usb2 device driver support > >>> > >>> Signed-off-by: Subbaraya Sundeep Bhatta > >>> --- > >>> .../devicetree/bindings/usb/xilinx_usb.txt | 21 + > >>> drivers/usb/gadget/Kconfig | 14 + > >>> drivers/usb/gadget/Makefile|1 + > >>> drivers/usb/gadget/xilinx_udc.c| 2045 > >>> > >>> 4 files changed, 2081 insertions(+), 0 deletions(-) > >>> create mode 100644 Documentation/devicetree/bindings/usb/xilinx_usb.txt > >>> create mode 100644 drivers/usb/gadget/xilinx_udc.c > >>> > >>> diff --git a/Documentation/devicetree/bindings/usb/xilinx_usb.txt > >>> b/Documentation/devicetree/bindings/usb/xilinx_usb.txt > >>> new file mode 100644 > >>> index 000..acf03ab > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/usb/xilinx_usb.txt > >>> @@ -0,0 +1,21 @@ > >>> +Xilinx AXI USB2 device controller > >>> + > >>> +Required properties: > >>> +- compatible : Should be "xlnx,axi-usb2-device-4.00.a" > > > > Is "axi-usb2-device" the official device name? > > It is the name of IP which Xilinx have and we are using names in this format. > > > >>> +- reg: Physical base address and size of the Axi USB2 > >>> + device registers map. > >>> +- interrupts : Property with a value describing the interrupt > >>> + number. > > > > Does the device only have a single interrupt? > > I believe so. > > > >>> +- interrupt-parent : Must be core interrupt controller > > > > Is this strictly necessary? > > I am not sure what do you mean by that. If you mean that interrupt-parent > can be written to the root of DTS file then we can setup system with more > interrupt controllers that's why it is required. At which point it's not a required property of the node... > If we can point to standard interrupt description then please point me to > exact description you would like to see here and we can change it. Unfortunately I'm not aware of a generic interrupts document. I just don't see the point in each document listing interrupt-parent as a requiredp roeprty when it's not. That said this is a trivial detail and not really a blocker. Cheers, Mark -- 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 RFC] usb: gadget: Add xilinx axi usb2 device support
>> If we can point to standard interrupt description then please point me to >> exact description you would like to see here and we can change it. > > Unfortunately I'm not aware of a generic interrupts document. I just > don't see the point in each document listing interrupt-parent as a > requiredp roeprty when it's not. That said this is a trivial detail and > not really a blocker. I agree with you that copying this part again and again is just problematic. Time to time I see that IRQs doesn't need to be described too. I am also not sure if kernel can work without interrupt-parent at all. I expect that it won't work and because we have interrupt parent in every node (which is generated) it is probably required in our setup. As you said it is just trivial detail for me too. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: musb - babble interrupt recovery
Hi Roger, thanks for those patches, actually i briefly approached them during my googling, but at first i didn't relate subject with Babble situation. Closely reading description about SUSPENDM bit behavior during resume makes sense. On 21.2.2014 13:03, Roger Quadros wrote: Could you please try the 2 patches posted here [1] for the babble problem, esp. patch 2. This is not about babble recovery but to prevent the babble from happening in the first place. I tried both patches separately on 3.14-rc3 and second patch alone helped here. I wasn't able to reproduce babble interrupt anymore, no matter, what i did with my hub and USB peripherals. Great! Best regards, Michal -- 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/7] IBM Akebono: Add support to the OHCI platform driver for PPC476GTR
On Friday 21 February 2014 17:31:29 Alistair Popple wrote:
>
> +static const struct of_device_id ohci_of_match[] = {
> + { .compatible = "usb-ohci", },
> + {},
> +};
> +
> static const struct platform_device_id ohci_platform_table[] = {
> { "ohci-platform", 0 },
> { }
> @@ -198,6 +209,7 @@ static struct platform_driver ohci_platform_driver = {
> .owner = THIS_MODULE,
> .name = "ohci-platform",
> .pm = &ohci_platform_pm_ops,
> + .of_match_table = ohci_of_match,
> }
> };
>
>
Linux-next already has a patch to add an of_match_table in this driver,
using
static const struct of_device_id ohci_platform_ids[] = {
{ .compatible = "generic-ohci", },
{ }
};
I think you should just use that string on your platform.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] ECHI Platform: Merge ppc-of EHCI driver into the ehci-platform driver
On Friday 21 February 2014 11:48:03 Mark Rutland wrote:
> > +
> > + np = of_find_compatible_node(NULL, NULL, "ibm,usb-ohci-440epx");
> > + if (np != NULL) {
> > + /* claim we really affected by usb23 erratum */
> > + if (!of_address_to_resource(np, 0, &res))
> > + ehci->ohci_hcctrl_reg =
> > + devm_ioremap(&pdev->dev,
> > +res.start + OHCI_HCCTRL_OFFSET,
> > +OHCI_HCCTRL_LEN);
> > + else
> > + ehci_dbg(ehci, "%s: no ohci offset in fdt\n",
> > __FILE__);
> > + if (!ehci->ohci_hcctrl_reg) {
> > + ehci_dbg(ehci, "%s: ioremap for ohci hcctrl
> > failed\n",
> > + __FILE__);
> > + } else {
> > + ehci->has_amcc_usb23 = 1;
> > + }
> > + }
>
> As this is being dropped into a generic driver, it would be nice to have
> a comment explaining why we need to do this -- not everyone using this
> will know the powerpc history. It certainly seems odd to look for
> another node in the tree that this driver isn't necessarily handling,
> and that should probably be explained.
>
> As this bit of fixup is only needed for powerpc, it would be nice to not
> have to do it elsewhere. Perhaps these could be factored out into a
> ppc_platform_reset function that could be empty stub for other
> architectures.
How about using the .data field of the of_device_id array to point to
a structure of functions? That way you don't even have to call
of_device_is_compatible() here. Note that using of_find_compatible_node()
is the wrong approach anyway, you want to check the current device
for compatibility, not just any device I assume.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] usb: don't use PREPARE_DELAYED_WORK
On Thu, 20 Feb 2014, Greg Kroah-Hartman wrote: > On Thu, Feb 20, 2014 at 03:44:27PM -0500, Tejun Heo wrote: > > PREPARE_[DELAYED_]WORK() are being phased out. They have few users > > and a nasty surprise in terms of reentrancy guarantee as workqueue > > considers work items to be different if they don't have the same work > > function. > > > > usb_hub->init_work is multiplexed with multiple work functions. > > Introduce hub_init_workfn() which invokes usb_hub->init_workfn and > > always use it as the work function and update the users to set the > > ->init_workfn field instead of overriding the work function using > > PREPARE_DELAYED_WORK(). > > > > It looks like that the work items are never queued while in-flight, so They aren't. But one work item does get queued by the previous one's work function. > > simply using INIT_DELAYED_WORK() before each queueing could be enough. > > This patch performs equivalent conversion just in case but we probably > > wanna clean it up later if that's the case. > > I think it should be fine to use INIT_DELAYED_WORK(), but Alan would > know best. Alan? That's right; INIT_DELAYED_WORK() should be fine. Provided there's no problem doing it from within the previous work routine. 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 5/9] usb: don't use PREPARE_DELAYED_WORK
On Fri, Feb 21, 2014 at 10:06:05AM -0500, Alan Stern wrote: > > I think it should be fine to use INIT_DELAYED_WORK(), but Alan would > > know best. Alan? > > That's right; INIT_DELAYED_WORK() should be fine. Provided there's no > problem doing it from within the previous work routine. Yeah, that's completely fine. Once a work item starts execution, workqueue doesn't care about it at all - even freeing and recycling it is fine. Thanks. -- tejun -- 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: Warning from USC core on kernel 3.13
On Thu, 20 Feb 2014, Larry Finger wrote: > Alan, > > The Lenovo Yogi 13 tablet comes with a Realtek RTL8723AU wireless device > built > in. Realtek sent me a driver that I modified so that it would build on new > kernels, and created a GitHub repo so that it would be available to the > community. One of the users of this driver is reporting intermittent warnings > that say > > WARNING: CPU: 1 PID: 0 at drivers/usb/core/urb.c:452 > usb_submit_urb+0x205/0x5d0() > usb 1-1.4: BOGUS urb xfer, pipe 3 != type 1 > > The full splat is available at http://pastebin.com/VcFPd4Yt. > > As no other user has reported this problem, I think this is some kind of > problem > with the wireless device or the adapter on this box. Even so, I would like to > know more about what is happening. In addition, Jes Sorensen at RedHat is > working on making the driver suitable for submission to the staging part of > the > tree. Just in case other uses run into the problem, it would be good to know > if > a fix is possible. > > Can you suggest any addition logging for when this situation occurs? The user > is > generating his own kernels, thus we can add any patches we want. You probably don't need any additional logging. The WARNING is telling you that the usb_read_interrupt_complete() routine in the 8723au driver called usb_submit_urb() with an incorrect pipe value. The URB specified a BULK pipe, but the endpoint in question is actually an INTERRUPT endpoint. 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: usb audio breaks ohci-pci
On Thu, 20 Feb 2014, Dennis New wrote: > > Anyway, I finally got around to writing a diagnostic patch for you to > > try. This should be applied with no other patches present. It > > requires CONFIG_USB_DEBUG to be enabled, and it should add a fair > > amount of debugging information to the dmesg log when your problem > > occurs. > > > > Let's see what shows up. > > I didn't get any special output this time, with the applied patch. I > have CONFIG_USB_DEBUG=y. Is there some other kind of verbosity/debugging > setting that I should have enabled? :s No. > As usual, after I unplug the usb audio device, > no other usb device will even get power when I try to plug it in -- > i.e. the whole usb system is effectively "shutdown". > > The last relevant dmesg output is: > > [22436.857502] input: Logitech Logitech Wireless Headset as > > /devices/pci:00/:00:13.1/usb3/3-1/3-1:1.3/0003:046D:0A29.0002/input/input26 > [22436.857951] hid-generic 0003:046D:0A29.0002: input: USB HID v1.11 Device > [Logitech Logitech Wireless Headset] on usb-:00:13.1-1/input3 > [25540.988369] usb 3-1: USB disconnect, device number 3 > [25556.224967] usb 3-1: new full-speed USB device number 4 using ohci-pci > [25556.477964] usb 3-1: New USB device found, idVendor=046d, idProduct=0a29 > [25556.477974] usb 3-1: New USB device strings: Mfr=1, Product=2, > SerialNumber=3 > [25556.477980] usb 3-1: Product: Logitech Wireless Headset > [25556.477984] usb 3-1: Manufacturer: Logitech > [25556.477988] usb 3-1: SerialNumber: 000D44B9714E > [25556.627203] input: Logitech Logitech Wireless Headset as > > /devices/pci:00/:00:13.1/usb3/3-1/3-1:1.3/0003:046D:0A29.0003/input/input27 > [25556.627658] hid-generic 0003:046D:0A29.0003: input: USB HID v1.11 Device > [Logitech Logitech Wireless Headset] on usb-:00:13.1-1/input3 > [25876.723713] usb 3-1: USB disconnect, device number 4 > [25877.722992] timeout: still 3 active urbs on EP #3 That's weird. Are you sure you were running the patched kernel? There very definitely should have been a bunch of debugging output, at the point where the headset was unplugged if not before. > Should I enable CONFIG_DEBUG_KERNEL as well? That's not necessary. 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/7] IBM Akebono: Add support to the OHCI platform driver for PPC476GTR
On Fri, 21 Feb 2014, Alistair Popple wrote: > The IBM Akebono board uses the PPC476GTR SoC which has a OHCI > compliant USB host interface. This patch adds support for it to the > OHCI platform driver. > > As we use device tree to pass platform specific data instead of > platform data we remove the check for platform data and instead > provide reasonable defaults if no platform data is present. This is > similar to what is currently done in ehci-platform.c. > > Signed-off-by: Alistair Popple > Acked-by: Alan Stern As Arnd pointed out, this patch is out of date. See https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/drivers/usb/host/ohci-platform.c?h=usb-next&id=ca52a17ba975dbf47e87c9bc63086aca0cf92806 and https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/drivers/usb/host/ohci-platform.c?h=usb-next&id=ce149c30b9f89d0c9addd1d71ccdb57c1051553b 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 RFC] usb: gadget: Add xilinx axi usb2 device support
Hi, On Fri, Feb 21, 2014 at 11:27:07AM +, Subbaraya Sundeep Bhatta wrote: > > From the looks of it, I doubt this was actually tested, you need a lot > > of work on this driver. > Tested on both ARM and Microblaze architectures with Mass storage gadget. > Will send a v2 after addressing all your comments. clearly you didn't try to remove and reinsert the module or you would see a whole bunch of errors. > This email and any attachments are intended for the sole use of the > named recipient(s) and contain(s) confidential information that may be > proprietary, privileged or copyrighted under applicable law. If you > are not the intended recipient, do not read, copy, or forward this > email message or any attachments. Delete this email message and any > attachments immediately. remove this footer message, btw, when sending emails to public mailing lists. -- balbi signature.asc Description: Digital signature
Re: [PATCH 4/7] ECHI Platform: Merge ppc-of EHCI driver into the ehci-platform driver
On Fri, 21 Feb 2014, Alistair Popple wrote: > Currently the ppc-of driver uses the compatibility string > "usb-ehci". This means platforms that use device-tree and implement an > EHCI compatible interface have to either use the ppc-of driver or add > a compatible line to the ehci-platform driver. It would be more > appropriate for the platform driver to be compatible with "usb-ehci" > as non-powerpc platforms are also beginning to utilise device-tree. > > This patch merges the device tree property parsing from ehci-ppc-of > into the platform driver and adds a "usb-ehci" compatibility > string. The existing ehci-ppc-of driver is removed and the 440EPX > specific quirks are added to the ehci-platform driver. > > Signed-off-by: Alistair Popple This patch is also out of date. The compatibility string used by the ehci-platform driver is "generic-ehci". There remains the question of whether to merge ehci-ppc-of into ehci-platform. This would be a rather invasive change, but I suppose we could do it. With adjustments along the lines suggested by Mark Rutland. 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 RFC] usb: gadget: Add xilinx axi usb2 device support
Hi,
On Fri, Feb 21, 2014 at 12:04:54PM +, Mark Rutland wrote:
> > > +Example:
> > > + axi-usb2-device@42e0 {
> > > +compatible = "xlnx,axi-usb2-device-4.00.a";
> > > +interrupt-parent = <0x1>;
> > > +interrupts = <0x0 0x39 0x1>;
> > > +reg = <0x42e0 0x1>;
> > > +xlnx,include-dma = <0x1>;
> > > +};
> > > +
> >
> > you need to Cc devicet...@vger.kernel.org for this.
>
> Cheers Filipe; thanks for adding us to Cc :)
sure, but it's "Felipe" ;-)
> > > + /* Map the registers */
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + udc->base_address = devm_ioremap_nocache(&pdev->dev, res->start,
> > > + resource_size(res));
> >
> > use devm_ioremap_resource() instead.
>
> Also, res might be NULL. You should check that before dereferencing it.
not needed when using devm_ioremap_resource(), see the implementation.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support
Hi,
On Fri, Feb 21, 2014 at 02:41:03PM +0100, Michal Simek wrote:
> >>> + /* Map the registers */
> >>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>> + udc->base_address = devm_ioremap_nocache(&pdev->dev, res->start,
> >>> + resource_size(res));
> >>
> >> use devm_ioremap_resource() instead.
> >
> > Also, res might be NULL. You should check that before dereferencing it.
>
> yes it is necessary for both cases with devm_ioremap_nocache
> or with devm_ioremap_resource.
read the source Luke:
| void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res)
| {
| resource_size_t size;
| const char *name;
| void __iomem *dest_ptr;
|
| BUG_ON(!dev);
|
| if (!res || resource_type(res) != IORESOURCE_MEM) {
already done for you
| dev_err(dev, "invalid resource\n");
| return ERR_PTR(-EINVAL);
| }
|
| size = resource_size(res);
| name = res->name ?: dev_name(dev);
|
| if (!devm_request_mem_region(dev, res->start, size, name)) {
| dev_err(dev, "can't request region for resource %pR\n", res);
| return ERR_PTR(-EBUSY);
| }
|
| if (res->flags & IORESOURCE_CACHEABLE)
| dest_ptr = devm_ioremap(dev, res->start, size);
| else
| dest_ptr = devm_ioremap_nocache(dev, res->start, size);
|
| if (!dest_ptr) {
| dev_err(dev, "ioremap failed for resource %pR\n", res);
| devm_release_mem_region(dev, res->start, size);
| dest_ptr = ERR_PTR(-ENOMEM);
| }
|
| return dest_ptr;
| }
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support
On 02/21/2014 04:42 PM, Felipe Balbi wrote:
> Hi,
>
> On Fri, Feb 21, 2014 at 02:41:03PM +0100, Michal Simek wrote:
> + /* Map the registers */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + udc->base_address = devm_ioremap_nocache(&pdev->dev, res->start,
> + resource_size(res));
use devm_ioremap_resource() instead.
>>>
>>> Also, res might be NULL. You should check that before dereferencing it.
>>
>> yes it is necessary for both cases with devm_ioremap_nocache
>> or with devm_ioremap_resource.
>
> read the source Luke:
>
> | void __iomem *devm_ioremap_resource(struct device *dev, struct resource
> *res)
> | {
> | resource_size_t size;
> | const char *name;
> | void __iomem *dest_ptr;
> |
> | BUG_ON(!dev);
> |
> | if (!res || resource_type(res) != IORESOURCE_MEM) {
>
> already done for you
>
> | dev_err(dev, "invalid resource\n");
> | return ERR_PTR(-EINVAL);
> | }
> |
> | size = resource_size(res);
> | name = res->name ?: dev_name(dev);
> |
> | if (!devm_request_mem_region(dev, res->start, size, name)) {
> | dev_err(dev, "can't request region for resource %pR\n", res);
> | return ERR_PTR(-EBUSY);
> | }
> |
> | if (res->flags & IORESOURCE_CACHEABLE)
> | dest_ptr = devm_ioremap(dev, res->start, size);
> | else
> | dest_ptr = devm_ioremap_nocache(dev, res->start, size);
I have read it just not sure if IORESOURCE_CACHEABLE is setup by default
or not.
If yes, then you have to setup res->flags in your driver and have to
check it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
signature.asc
Description: OpenPGP digital signature
Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support
On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote: > BTW: u-boot started to use SPDX-License-Identifier > which will be nice to start to use. I agree, feel free to start sending patches to use this type of identifier for drivers. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support
On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote: > On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote: >> BTW: u-boot started to use SPDX-License-Identifier >> which will be nice to start to use. > > I agree, feel free to start sending patches to use this type of > identifier for drivers. But we probably need to add that Licenses to one location. Documentation/Licenses? Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support
On Fri, Feb 21, 2014 at 04:51:07PM +0100, Michal Simek wrote:
> On 02/21/2014 04:42 PM, Felipe Balbi wrote:
> > Hi,
> >
> > On Fri, Feb 21, 2014 at 02:41:03PM +0100, Michal Simek wrote:
> > + /* Map the registers */
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + udc->base_address = devm_ioremap_nocache(&pdev->dev, res->start,
> > +resource_size(res));
>
> use devm_ioremap_resource() instead.
> >>>
> >>> Also, res might be NULL. You should check that before dereferencing it.
> >>
> >> yes it is necessary for both cases with devm_ioremap_nocache
> >> or with devm_ioremap_resource.
> >
> > read the source Luke:
> >
> > | void __iomem *devm_ioremap_resource(struct device *dev, struct resource
> > *res)
> > | {
> > | resource_size_t size;
> > | const char *name;
> > | void __iomem *dest_ptr;
> > |
> > | BUG_ON(!dev);
> > |
> > | if (!res || resource_type(res) != IORESOURCE_MEM) {
> >
> > already done for you
> >
> > | dev_err(dev, "invalid resource\n");
> > | return ERR_PTR(-EINVAL);
> > | }
> > |
> > | size = resource_size(res);
> > | name = res->name ?: dev_name(dev);
> > |
> > | if (!devm_request_mem_region(dev, res->start, size, name)) {
> > | dev_err(dev, "can't request region for resource %pR\n", res);
> > | return ERR_PTR(-EBUSY);
> > | }
> > |
> > | if (res->flags & IORESOURCE_CACHEABLE)
> > | dest_ptr = devm_ioremap(dev, res->start, size);
> > | else
> > | dest_ptr = devm_ioremap_nocache(dev, res->start, size);
>
> I have read it just not sure if IORESOURCE_CACHEABLE is setup by default
> or not.
> If yes, then you have to setup res->flags in your driver and have to
> check it.
you don't need IORESOURCe_CACHEABLE. It's fine the way it is, just use
the helper function ;-).
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support
On Fri, Feb 21, 2014 at 03:41:03PM +, Felipe Balbi wrote:
> Hi,
>
> On Fri, Feb 21, 2014 at 12:04:54PM +, Mark Rutland wrote:
> > > > +Example:
> > > > + axi-usb2-device@42e0 {
> > > > +compatible = "xlnx,axi-usb2-device-4.00.a";
> > > > +interrupt-parent = <0x1>;
> > > > +interrupts = <0x0 0x39 0x1>;
> > > > +reg = <0x42e0 0x1>;
> > > > +xlnx,include-dma = <0x1>;
> > > > +};
> > > > +
> > >
> > > you need to Cc devicet...@vger.kernel.org for this.
> >
> > Cheers Filipe; thanks for adding us to Cc :)
>
> sure, but it's "Felipe" ;-)
Whoops; sorry Felipe :)
>
> > > > + /* Map the registers */
> > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > + udc->base_address = devm_ioremap_nocache(&pdev->dev, res->start,
> > > > +resource_size(res));
> > >
> > > use devm_ioremap_resource() instead.
> >
> > Also, res might be NULL. You should check that before dereferencing it.
>
> not needed when using devm_ioremap_resource(), see the implementation.
Ah, good to know.
Cheers,
Mark.
--
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
SPDX-License-Identifier (was: Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support)
On Fri, Feb 21, 2014 at 05:04:26PM +0100, Michal Simek wrote: > On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote: > > On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote: > >> BTW: u-boot started to use SPDX-License-Identifier > >> which will be nice to start to use. > > > > I agree, feel free to start sending patches to use this type of > > identifier for drivers. > > But we probably need to add that Licenses to one location. > Documentation/Licenses? Just add to the drivers themselves, just like u-boot is doing. A simple: $ git grep -e SPDX-License-Identifier will tell you filename and license. Or did I misunderstand your question ? -- balbi signature.asc Description: Digital signature
Re: SPDX-License-Identifier
On 02/21/2014 05:12 PM, Felipe Balbi wrote: > On Fri, Feb 21, 2014 at 05:04:26PM +0100, Michal Simek wrote: >> On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote: >>> On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote: BTW: u-boot started to use SPDX-License-Identifier which will be nice to start to use. >>> >>> I agree, feel free to start sending patches to use this type of >>> identifier for drivers. >> >> But we probably need to add that Licenses to one location. >> Documentation/Licenses? > > Just add to the drivers themselves, just like u-boot is doing. A simple: > > $ git grep -e SPDX-License-Identifier > > will tell you filename and license. Or did I misunderstand your question ? But for doing this it is probably necessary to have location where you will place that licenses and you will explain what it is how it is done by Wolfgang in this commit. http://git.denx.de/?p=u-boot.git;a=commitdiff;h=eca3aeb352c964bdb28b8e191d6326370245e03f Then yes, adding one line is enough. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: SPDX-License-Identifier
Hi, On Fri, Feb 21, 2014 at 05:18:39PM +0100, Michal Simek wrote: > On 02/21/2014 05:12 PM, Felipe Balbi wrote: > > On Fri, Feb 21, 2014 at 05:04:26PM +0100, Michal Simek wrote: > >> On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote: > >>> On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote: > BTW: u-boot started to use SPDX-License-Identifier > which will be nice to start to use. > >>> > >>> I agree, feel free to start sending patches to use this type of > >>> identifier for drivers. > >> > >> But we probably need to add that Licenses to one location. > >> Documentation/Licenses? > > > > Just add to the drivers themselves, just like u-boot is doing. A simple: > > > > $ git grep -e SPDX-License-Identifier > > > > will tell you filename and license. Or did I misunderstand your question ? > > But for doing this it is probably necessary to have location where > you will place that licenses and you will explain what it is how > it is done by Wolfgang in this commit. > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=eca3aeb352c964bdb28b8e191d6326370245e03f > > Then yes, adding one line is enough. spdx.org has all licenses, why don't we just rely on that instead of adding every other license to the kernel source ? cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support
On Friday 21 February 2014 16:51:07 Michal Simek wrote: > > | > > | if (res->flags & IORESOURCE_CACHEABLE) > > | dest_ptr = devm_ioremap(dev, res->start, size); > > | else > > | dest_ptr = devm_ioremap_nocache(dev, res->start, size); > > I have read it just not sure if IORESOURCE_CACHEABLE is setup by default > or not. > If yes, then you have to setup res->flags in your driver and have to > check it. ioremap() and ioremap_nocache() are the same on all architectures. If you want a cacheable mapping, you need to call ioremap_cache(), which unfortunately doesn't exist on all architectures and also doesn't have a devm_* variant. I don't know how the above code made it into devm_ioremap_resource(), it's clearly bogus. The only time we ever set IORESOURCE_CACHEABLE is for ROM BARs on PCI, which in turn are rarely used in the kernel. It's never set for any platform devices, aside from one use in arch/arm/mach-clps711x/board-cdb89712.c. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] xhci: Use pci_enable_msix_exact() instead of pci_enable_msix()
As result of deprecation of MSI-X/MSI enablement functions
pci_enable_msix() and pci_enable_msi_block() all drivers
using these two interfaces need to be updated to use the
new pci_enable_msi_range() or pci_enable_msi_exact()
and pci_enable_msix_range() or pci_enable_msix_exact()
interfaces.
This update also cleans up a bit xhci_setup_msi() and
xhci_setup_msix() returning of success.
Signed-off-by: Alexander Gordeev
Cc: Sarah Sharp
Cc: Greg Kroah-Hartman
Cc: linux-usb@vger.kernel.org
Cc: linux-...@vger.kernel.org
---
drivers/usb/host/xhci.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6fe577d..dc7cfb5 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -232,9 +232,10 @@ static int xhci_setup_msi(struct xhci_hcd *xhci)
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"disable MSI interrupt");
pci_disable_msi(pdev);
+ return ret;
}
- return ret;
+ return 0;
}
/*
@@ -291,7 +292,7 @@ static int xhci_setup_msix(struct xhci_hcd *xhci)
xhci->msix_entries[i].vector = 0;
}
- ret = pci_enable_msix(pdev, xhci->msix_entries, xhci->msix_count);
+ ret = pci_enable_msix_exact(pdev, xhci->msix_entries, xhci->msix_count);
if (ret) {
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"Failed to enable MSI-X");
@@ -307,7 +308,7 @@ static int xhci_setup_msix(struct xhci_hcd *xhci)
}
hcd->msix_enabled = 1;
- return ret;
+ return 0;
disable_msix:
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "disable MSI-X interrupt");
--
1.7.7.6
--
Regards,
Alexander Gordeev
agord...@redhat.com
--
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 RFC] usb: gadget: Add xilinx axi usb2 device support
On Fri, Feb 21, 2014 at 05:04:26PM +0100, Michal Simek wrote: > On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote: > > On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote: > >> BTW: u-boot started to use SPDX-License-Identifier > >> which will be nice to start to use. > > > > I agree, feel free to start sending patches to use this type of > > identifier for drivers. > > But we probably need to add that Licenses to one location. No, why would we need to do that? -- 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: SPDX-License-Identifier
On Fri, Feb 21, 2014 at 10:20:45AM -0600, Felipe Balbi wrote: > Hi, > > On Fri, Feb 21, 2014 at 05:18:39PM +0100, Michal Simek wrote: > > On 02/21/2014 05:12 PM, Felipe Balbi wrote: > > > On Fri, Feb 21, 2014 at 05:04:26PM +0100, Michal Simek wrote: > > >> On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote: > > >>> On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote: > > BTW: u-boot started to use SPDX-License-Identifier > > which will be nice to start to use. > > >>> > > >>> I agree, feel free to start sending patches to use this type of > > >>> identifier for drivers. > > >> > > >> But we probably need to add that Licenses to one location. > > >> Documentation/Licenses? > > > > > > Just add to the drivers themselves, just like u-boot is doing. A simple: > > > > > > $ git grep -e SPDX-License-Identifier > > > > > > will tell you filename and license. Or did I misunderstand your question ? > > > > But for doing this it is probably necessary to have location where > > you will place that licenses and you will explain what it is how > > it is done by Wolfgang in this commit. > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=eca3aeb352c964bdb28b8e191d6326370245e03f > > > > Then yes, adding one line is enough. > > spdx.org has all licenses, why don't we just rely on that instead of > adding every other license to the kernel source ? Yes, all that will be acceptable is a one-line identifier in the file. No need to have all the different licenses in the kernel source, that's crazy and not needed at all. I've told the SPDX people this in the past, and they keep promising that they will do the comment work, but it's been months and I have yet to see a single patch... greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SPDX-License-Identifier
On 02/21/2014 05:56 PM, Greg Kroah-Hartman wrote: > On Fri, Feb 21, 2014 at 10:20:45AM -0600, Felipe Balbi wrote: >> Hi, >> >> On Fri, Feb 21, 2014 at 05:18:39PM +0100, Michal Simek wrote: >>> On 02/21/2014 05:12 PM, Felipe Balbi wrote: On Fri, Feb 21, 2014 at 05:04:26PM +0100, Michal Simek wrote: > On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote: >> On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote: >>> BTW: u-boot started to use SPDX-License-Identifier >>> which will be nice to start to use. >> >> I agree, feel free to start sending patches to use this type of >> identifier for drivers. > > But we probably need to add that Licenses to one location. > Documentation/Licenses? Just add to the drivers themselves, just like u-boot is doing. A simple: $ git grep -e SPDX-License-Identifier will tell you filename and license. Or did I misunderstand your question ? >>> >>> But for doing this it is probably necessary to have location where >>> you will place that licenses and you will explain what it is how >>> it is done by Wolfgang in this commit. >>> >>> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=eca3aeb352c964bdb28b8e191d6326370245e03f >>> >>> Then yes, adding one line is enough. >> >> spdx.org has all licenses, why don't we just rely on that instead of >> adding every other license to the kernel source ? > > Yes, all that will be acceptable is a one-line identifier in the file. > No need to have all the different licenses in the kernel source, that's > crazy and not needed at all. > > I've told the SPDX people this in the past, and they keep promising that > they will do the comment work, but it's been months and I have yet to > see a single patch... But shouldn't we at least write somewhere that it has connection to spdx.org where you can find out that licenses. I have no problem to use this one-line identifier at all. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform signature.asc Description: OpenPGP digital signature
Re: [RFC PATCH] xhci: Prevent runtime pm from autosuspending during initialization
On Fri, Feb 21, 2014 at 5:19 AM, Mathias Nyman
wrote:
> If autosuspend is set to zero the usb-2 roothub will try to suspend
> the controller before usb-3 parts are initialized.
>
> Prevent this by incrementing the usage counter before usb-2 registers
> its roothub. Decrement the counter when usb-3 roothub is allocated.
>
> Signed-off-by: Mathias Nyman
> ---
> drivers/usb/host/xhci.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 6fe577d..9301aff 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -4753,6 +4753,8 @@ int xhci_gen_setup(struct usb_hcd *hcd,
> xhci_get_quirks_t get_quirks)
> * companion controller.
> */
> hcd->has_tt = 1;
> + /* prevent USB2 runtime pm until USB3 parts are initialized */
> + pm_runtime_get_noresume(hcd->self.controller);
> } else {
> /* xHCI private pointer was set in xhci_pci_probe for the
> second
> * registered roothub.
> @@ -4765,6 +4767,9 @@ int xhci_gen_setup(struct usb_hcd *hcd,
> xhci_get_quirks_t get_quirks)
> if (xhci->hci_version < 0x100)
> hcd->self.no_sg_constraint = 1;
>
> + /* USB3 initialized far enough to allow runtime pm suspend */
> + pm_runtime_put_noidle(hcd->self.controller);
> +
Why the noresume and noidle versions? Shouldn't these be the _sync versions?
--
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: SPDX-License-Identifier
On Fri, Feb 21, 2014 at 06:26:08PM +0100, Michal Simek wrote: > On 02/21/2014 05:56 PM, Greg Kroah-Hartman wrote: > > On Fri, Feb 21, 2014 at 10:20:45AM -0600, Felipe Balbi wrote: > >> Hi, > >> > >> On Fri, Feb 21, 2014 at 05:18:39PM +0100, Michal Simek wrote: > >>> On 02/21/2014 05:12 PM, Felipe Balbi wrote: > On Fri, Feb 21, 2014 at 05:04:26PM +0100, Michal Simek wrote: > > On 02/21/2014 05:04 PM, Greg Kroah-Hartman wrote: > >> On Fri, Feb 21, 2014 at 07:38:16AM +0100, Michal Simek wrote: > >>> BTW: u-boot started to use SPDX-License-Identifier > >>> which will be nice to start to use. > >> > >> I agree, feel free to start sending patches to use this type of > >> identifier for drivers. > > > > But we probably need to add that Licenses to one location. > > Documentation/Licenses? > > Just add to the drivers themselves, just like u-boot is doing. A simple: > > $ git grep -e SPDX-License-Identifier > > will tell you filename and license. Or did I misunderstand your question > ? > >>> > >>> But for doing this it is probably necessary to have location where > >>> you will place that licenses and you will explain what it is how > >>> it is done by Wolfgang in this commit. > >>> > >>> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=eca3aeb352c964bdb28b8e191d6326370245e03f > >>> > >>> Then yes, adding one line is enough. > >> > >> spdx.org has all licenses, why don't we just rely on that instead of > >> adding every other license to the kernel source ? > > > > Yes, all that will be acceptable is a one-line identifier in the file. > > No need to have all the different licenses in the kernel source, that's > > crazy and not needed at all. > > > > I've told the SPDX people this in the past, and they keep promising that > > they will do the comment work, but it's been months and I have yet to > > see a single patch... > > But shouldn't we at least write somewhere > that it has connection to spdx.org where you can find out that licenses. Why? Are these licenses so unknown that no one knows what they are? And, as part of the kernel-as-a-whole-work, they all resolve to GPLv2 anyway, and we have that license in the source tree, so nothing else should be needed. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] xhci: Prevent runtime pm from autosuspending during initialization
@@ -4753,6 +4753,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t
get_quirks)
* companion controller.
*/
hcd->has_tt = 1;
+ /* prevent USB2 runtime pm until USB3 parts are initialized */
+ pm_runtime_get_noresume(hcd->self.controller);
} else {
/* xHCI private pointer was set in xhci_pci_probe for the
second
* registered roothub.
@@ -4765,6 +4767,9 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t
get_quirks)
if (xhci->hci_version < 0x100)
hcd->self.no_sg_constraint = 1;
+ /* USB3 initialized far enough to allow runtime pm suspend */
+ pm_runtime_put_noidle(hcd->self.controller);
+
Why the noresume and noidle versions? Shouldn't these be the _sync versions?
As I understood it the _sync versions may actually run pm_runtime_resume
/ pm_runtime_idle. Which is not what I'm looking for here.
Noresume and noidle will just increase / decrease the usage count and
that way prevent usb2 roothub register from calling xhci_suspend.
This is how I gather it works:
Round 1: add usb2 hcd
usb_add_hcd() // primary usb2 hcd
hcd->self.root_hub = usb_alloc_dev()
hcd->driver->reset(hcd) -> xhci_gen_setup()
if(primary_hcd) // true
pm_runtime_get_noresume(hcd->self.controller) // +1 usage
register_root_hub(hcd) -> usb_new_device(usb_dev)
pm_runtime_set_active(&udev->dev);
pm_runtime_get_noresume(&udev->dev);
pm_runtime_use_autosuspend(&udev->dev);
pm_runtime_enable(&udev->dev);
...
pm_runtime_put_sync_autosuspend(&udev->dev); this would trigger
xhci_suspend if all usages were 0. (for both roothub and controller)
which oopses as usb3 stuff is not yet initialized
Round 2: add usb 3 hcd
usb_add_hcd() // secondary usb3 hcd
hcd->self.root_hub = usb_alloc_dev()
hcd->driver->reset(hcd) -> xhci_gen_setup()
if(primary_hcd) // false
..
else
pm_runtime_put_noidle(hcd->self.controller) // -1 usage, *
register_root_hub(hcd) -> usb_new_device(usb_dev)
pm_runtime_set_active(&udev->dev);
pm_runtime_get_noresume(&udev->dev);
pm_runtime_use_autosuspend(&udev->dev);
pm_runtime_enable(&udev->dev);
...
pm_runtime_put_sync_autosuspend(&udev->dev); // ok to trigger would
trigger xhci_suspend if all usages were 0.
*) unnecessary to trigger any suspend as we're just about to register
the usb3 roothub. but we need to decrement the usage counter.
-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: [RFC PATCH] xhci: Prevent runtime pm from autosuspending during initialization
On Fri, 21 Feb 2014, Mathias Nyman wrote:
> >> @@ -4753,6 +4753,8 @@ int xhci_gen_setup(struct usb_hcd *hcd,
> >> xhci_get_quirks_t get_quirks)
> >> * companion controller.
> >> */
> >> hcd->has_tt = 1;
> >> + /* prevent USB2 runtime pm until USB3 parts are
> >> initialized */
> >> + pm_runtime_get_noresume(hcd->self.controller);
> >> } else {
> >> /* xHCI private pointer was set in xhci_pci_probe for the
> >> second
> >> * registered roothub.
> >> @@ -4765,6 +4767,9 @@ int xhci_gen_setup(struct usb_hcd *hcd,
> >> xhci_get_quirks_t get_quirks)
> >> if (xhci->hci_version < 0x100)
> >> hcd->self.no_sg_constraint = 1;
> >>
> >> + /* USB3 initialized far enough to allow runtime pm suspend
> >> */
> >> + pm_runtime_put_noidle(hcd->self.controller);
> >> +
> >
> > Why the noresume and noidle versions? Shouldn't these be the _sync
> > versions?
> >
>
> As I understood it the _sync versions may actually run pm_runtime_resume
> / pm_runtime_idle. Which is not what I'm looking for here.
The _sync versions won't run those routines if the usage counter isn't
0.
> Noresume and noidle will just increase / decrease the usage count and
> that way prevent usb2 roothub register from calling xhci_suspend.
>
> This is how I gather it works:
>
> Round 1: add usb2 hcd
>
> usb_add_hcd() // primary usb2 hcd
>hcd->self.root_hub = usb_alloc_dev()
>hcd->driver->reset(hcd) -> xhci_gen_setup()
> if(primary_hcd) // true
>pm_runtime_get_noresume(hcd->self.controller) // +1 usage
At this point, we know the controller's usage counter is already > 0.
Therefore it doesn't matter whether the _sync or _noresume version is
used.
But is this really what you want to do? That is, do you want to
prevent the controller from being suspended, or do you want to prevent
the root hub from being suspended?
>register_root_hub(hcd) -> usb_new_device(usb_dev)
> pm_runtime_set_active(&udev->dev);
> pm_runtime_get_noresume(&udev->dev);
> pm_runtime_use_autosuspend(&udev->dev);
> pm_runtime_enable(&udev->dev);
> ...
> pm_runtime_put_sync_autosuspend(&udev->dev); this would trigger
> xhci_suspend if all usages were 0. (for both roothub and controller)
> which oopses as usb3 stuff is not yet initialized
Why would this trigger xhci_suspend? Isn't we still running in the
context of the probe routine? The controller won't be suspended until
the probe call returns.
> Round 2: add usb 3 hcd
>
> usb_add_hcd() // secondary usb3 hcd
>hcd->self.root_hub = usb_alloc_dev()
>hcd->driver->reset(hcd) -> xhci_gen_setup()
> if(primary_hcd) // false
> ..
> else
>pm_runtime_put_noidle(hcd->self.controller) // -1 usage, *
Again, is this really what you want? I thought you were trying to
prevent runtime suspends until the USB-3 root hub was registered. In
that case, you shouldn't do the _put until after the register_root_hub
call.
>register_root_hub(hcd) -> usb_new_device(usb_dev)
> pm_runtime_set_active(&udev->dev);
> pm_runtime_get_noresume(&udev->dev);
> pm_runtime_use_autosuspend(&udev->dev);
> pm_runtime_enable(&udev->dev);
> ...
> pm_runtime_put_sync_autosuspend(&udev->dev); // ok to trigger would
> trigger xhci_suspend if all usages were 0.
>
> *) unnecessary to trigger any suspend as we're just about to register
> the usb3 roothub. but we need to decrement the usage counter.
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: SPDX-License-Identifier
On Fri, Feb 21, 2014 at 09:57:20AM -0800, Greg Kroah-Hartman wrote: > > But shouldn't we at least write somewhere > > that it has connection to spdx.org where you can find out that licenses. > > Why? Are these licenses so unknown that no one knows what they are? > And, as part of the kernel-as-a-whole-work, they all resolve to GPLv2 > anyway, and we have that license in the source tree, so nothing else > should be needed. Note that not all lawyers are in agreement about this, so if this is a driver being developed by a company, you may want to ask your corporate counsel if they have an opinion about this. I've received advice of the form that it's not obvious that regardless of whether or not us *engineers* understand what all of the licensing terms mean, what's important is whether someone who is accused of "borrowing" GPL'ed code and dropping it in a driver for some other OS can convince a judge whether or not it's considered "obvious" from a legal perspective what an SPDX header means, and what is implied by an SPDX license identifer. Also note that with the advent of web sites that allow people to do web searches and turn up a singleton file via some gitweb interface, the fact that the full license text is distributed alongside the tarball might or might have as much legal significance as it once had. But of course, I'm not a lawyer, and if your company has is paying for the development of the driver, the Golden Rule applies (he who has the Gold, makes the Rules), and each of our respective corporate lawyers may have different opinions about what might happen if the question was ever to be adjudicated in court. Cheers, - Ted -- 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 to add Cressi Leonardo PID
Hello,
the following patch adds an entry for the PID of a Cressi Leonardo
diving computer interface to kernel 3.13.0.
It is detected as FT232RL.
Works with subsurface.
Signed-off-by: Joerg Dorchain
--- ftdi_sio.c.orig 2014-02-20 21:06:01.824231733 +0100
+++ ftdi_sio.c 2014-02-20 21:09:35.278398183 +0100
@@ -905,6 +905,8 @@
/* Crucible Devices */
{ USB_DEVICE(FTDI_VID, FTDI_CT_COMET_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_Z3X_PID) },
+ /* Cressi Devices */
+ { USB_DEVICE(FTDI_VID, FTDI_CRESSI_PID) },
{ } /* Terminating entry */
};
--- ftdi_sio_ids.h.orig 2014-02-20 21:02:11.505168421 +0100
+++ ftdi_sio_ids.h 2014-02-20 21:08:19.702069888 +0100
@@ -1313,3 +1313,9 @@
* Manufacturer: Smart GSM Team
*/
#define FTDI_Z3X_PID 0x0011
+
+/*
+ * Product: Cressi PC Interface
+ * Manufacturer: Cressi
+ */
+#define FTDI_CRESSI_PID0x87d0
signature.asc
Description: Digital signature
Re: [RFC PATCH] xhci: Prevent runtime pm from autosuspending during initialization
On Fri, Feb 21, 2014 at 01:57:25PM -0500, Alan Stern wrote:
> On Fri, 21 Feb 2014, Mathias Nyman wrote:
> > This is how I gather it works:
> >
> > Round 1: add usb2 hcd
> >
> > usb_add_hcd() // primary usb2 hcd
> >hcd->self.root_hub = usb_alloc_dev()
> >hcd->driver->reset(hcd) -> xhci_gen_setup()
> > if(primary_hcd) // true
> >pm_runtime_get_noresume(hcd->self.controller) // +1 usage
>
> At this point, we know the controller's usage counter is already > 0.
> Therefore it doesn't matter whether the _sync or _noresume version is
> used.
>
> But is this really what you want to do? That is, do you want to
> prevent the controller from being suspended, or do you want to prevent
> the root hub from being suspended?
Either one would work. Mathias is just trying to prevent xhci_suspend
from being called before the xHCI driver finishes registering both
roothubs. It could be better to prevent the root hub from suspending,
in case there's other issues with bus suspend we haven't found yet.
> >register_root_hub(hcd) -> usb_new_device(usb_dev)
> > pm_runtime_set_active(&udev->dev);
> > pm_runtime_get_noresume(&udev->dev);
> > pm_runtime_use_autosuspend(&udev->dev);
> > pm_runtime_enable(&udev->dev);
> > ...
> > pm_runtime_put_sync_autosuspend(&udev->dev); this would trigger
> > xhci_suspend if all usages were 0. (for both roothub and controller)
> > which oopses as usb3 stuff is not yet initialized
>
> Why would this trigger xhci_suspend? Isn't we still running in the
> context of the probe routine? The controller won't be suspended until
> the probe call returns.
Maybe it helps to look at the context of the bug report Mathias is
trying to fix?
http://marc.info/?l=linux-usb&m=138914518219334&w=2
http://marc.info/?l=linux-kernel&m=138919609832200&w=2
It's pretty clear from the oops that xhci_suspend gets called before the
second roothub is registered. From the group it came from, I suspect
they have lots of random patches applied on top of their 3.10 kernel,
but let's assume that's not the issue and explore whether xhci_suspend
can be called before probe completes.
The xHCI driver registers its own PCI probe function, and then it calls
usb_hcd_pci_probe(), and then registers the second roothub. So if the
USB core is somehow preventing PCI suspend in usb_hcd_pci_probe(), it
won't help for our special init flow. If the PCI core is supposed to
prevent PCI suspend until probe completes, then yes, xhci_suspend
shouldn't ever be called until both buses are registered.
What about the xHCI platform case? xhci_plat_suspend is registered as a
system sleep PM operation, and it calls xhci_suspend:
static int xhci_plat_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
return xhci_suspend(xhci);
}
static int xhci_plat_resume(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
return xhci_resume(xhci, 0);
}
static const struct dev_pm_ops xhci_plat_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume)
};
So could we have a system sleep event in the middle of platform probe?
> > Round 2: add usb 3 hcd
> >
> > usb_add_hcd() // secondary usb3 hcd
> >hcd->self.root_hub = usb_alloc_dev()
> >hcd->driver->reset(hcd) -> xhci_gen_setup()
> > if(primary_hcd) // false
> > ..
> > else
> >pm_runtime_put_noidle(hcd->self.controller) // -1 usage, *
>
> Again, is this really what you want? I thought you were trying to
> prevent runtime suspends until the USB-3 root hub was registered. In
> that case, you shouldn't do the _put until after the register_root_hub
> call.
>
> >register_root_hub(hcd) -> usb_new_device(usb_dev)
> > pm_runtime_set_active(&udev->dev);
> > pm_runtime_get_noresume(&udev->dev);
> > pm_runtime_use_autosuspend(&udev->dev);
> > pm_runtime_enable(&udev->dev);
> > ...
> > pm_runtime_put_sync_autosuspend(&udev->dev); // ok to trigger would
> > trigger xhci_suspend if all usages were 0.
> >
> > *) unnecessary to trigger any suspend as we're just about to register
> > the usb3 roothub. but we need to decrement the usage counter.
Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: musb: USB_MUSB_DUAL_ROLE/USB_MUSB_GADGET should depend on HAS_DMA
If NO_DMA=y: drivers/built-in.o: In function `txstate': musb_gadget.c:(.text+0x35955a): undefined reference to `dma_unmap_single' musb_gadget.c:(.text+0x35957e): undefined reference to `dma_sync_single_for_cpu' drivers/built-in.o: In function `musb_g_giveback': (.text+0x359672): undefined reference to `dma_mapping_error' drivers/built-in.o: In function `musb_g_giveback': (.text+0x3596ba): undefined reference to `dma_unmap_single' drivers/built-in.o: In function `musb_g_giveback': (.text+0x3596e0): undefined reference to `dma_sync_single_for_cpu' drivers/built-in.o: In function `rxstate': musb_gadget.c:(.text+0x3599d0): undefined reference to `dma_unmap_single' musb_gadget.c:(.text+0x3599f6): undefined reference to `dma_sync_single_for_cpu' drivers/built-in.o: In function `musb_gadget_queue': musb_gadget.c:(.text+0x35a8c0): undefined reference to `dma_map_single' musb_gadget.c:(.text+0x35a8d0): undefined reference to `dma_mapping_error' musb_gadget.c:(.text+0x35a906): undefined reference to `dma_sync_single_for_cpu' musb_gadget.c:(.text+0x35a9a0): undefined reference to `dma_unmap_single' musb_gadget.c:(.text+0x35a9c8): undefined reference to `dma_sync_single_for_cpu' Signed-off-by: Geert Uytterhoeven --- drivers/usb/musb/Kconfig |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index 688dc8bb192d..8b789792f6fa 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -43,6 +43,7 @@ config USB_MUSB_HOST config USB_MUSB_GADGET bool "Gadget only mode" depends on USB_GADGET=y || USB_GADGET=USB_MUSB_HDRC + depends on HAS_DMA help Select this when you want to use MUSB in gadget mode only, thereby the host feature will be regressed. @@ -50,6 +51,7 @@ config USB_MUSB_GADGET config USB_MUSB_DUAL_ROLE bool "Dual Role mode" depends on ((USB=y || USB=USB_MUSB_HDRC) && (USB_GADGET=y || USB_GADGET=USB_MUSB_HDRC)) + depends on HAS_DMA help This is the default mode of working of MUSB controller where both host and gadget features are enabled. -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] xhci: Prevent runtime pm from autosuspending during initialization
On Fri, Feb 21, 2014 at 11:45 AM, Sarah Sharp
wrote:
> On Fri, Feb 21, 2014 at 01:57:25PM -0500, Alan Stern wrote:
>> On Fri, 21 Feb 2014, Mathias Nyman wrote:
>> > This is how I gather it works:
>> >
>> > Round 1: add usb2 hcd
>> >
>> > usb_add_hcd() // primary usb2 hcd
>> >hcd->self.root_hub = usb_alloc_dev()
>> >hcd->driver->reset(hcd) -> xhci_gen_setup()
>> > if(primary_hcd) // true
>> >pm_runtime_get_noresume(hcd->self.controller) // +1 usage
>>
>> At this point, we know the controller's usage counter is already > 0.
>> Therefore it doesn't matter whether the _sync or _noresume version is
>> used.
>>
>> But is this really what you want to do? That is, do you want to
>> prevent the controller from being suspended, or do you want to prevent
>> the root hub from being suspended?
>
> Either one would work. Mathias is just trying to prevent xhci_suspend
> from being called before the xHCI driver finishes registering both
> roothubs. It could be better to prevent the root hub from suspending,
> in case there's other issues with bus suspend we haven't found yet.
>
>> >register_root_hub(hcd) -> usb_new_device(usb_dev)
>> > pm_runtime_set_active(&udev->dev);
>> > pm_runtime_get_noresume(&udev->dev);
>> > pm_runtime_use_autosuspend(&udev->dev);
>> > pm_runtime_enable(&udev->dev);
>> > ...
>> > pm_runtime_put_sync_autosuspend(&udev->dev); this would trigger
>> > xhci_suspend if all usages were 0. (for both roothub and controller)
>> > which oopses as usb3 stuff is not yet initialized
>>
>> Why would this trigger xhci_suspend? Isn't we still running in the
>> context of the probe routine? The controller won't be suspended until
>> the probe call returns.
>
> Maybe it helps to look at the context of the bug report Mathias is
> trying to fix?
>
> http://marc.info/?l=linux-usb&m=138914518219334&w=2
> http://marc.info/?l=linux-kernel&m=138919609832200&w=2
>
> It's pretty clear from the oops that xhci_suspend gets called before the
> second roothub is registered. From the group it came from, I suspect
> they have lots of random patches applied on top of their 3.10 kernel,
> but let's assume that's not the issue and explore whether xhci_suspend
> can be called before probe completes.
>
> The xHCI driver registers its own PCI probe function, and then it calls
> usb_hcd_pci_probe(), and then registers the second roothub. So if the
> USB core is somehow preventing PCI suspend in usb_hcd_pci_probe(), it
> won't help for our special init flow. If the PCI core is supposed to
> prevent PCI suspend until probe completes, then yes, xhci_suspend
> shouldn't ever be called until both buses are registered.
>
> What about the xHCI platform case? xhci_plat_suspend is registered as a
> system sleep PM operation, and it calls xhci_suspend:
>
> static int xhci_plat_suspend(struct device *dev)
> {
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>
> return xhci_suspend(xhci);
> }
>
> static int xhci_plat_resume(struct device *dev)
> {
> struct usb_hcd *hcd = dev_get_drvdata(dev);
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>
> return xhci_resume(xhci, 0);
> }
>
> static const struct dev_pm_ops xhci_plat_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume)
> };
>
> So could we have a system sleep event in the middle of platform probe?
No, not system sleep. dpm_suspend() is performed under the
device_lock and so is ->probe(). The driver core does not hold the
lock for runtime pm operations.
--
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] xhci: Prevent runtime pm from autosuspending during initialization
On Fri, Feb 21, 2014 at 10:45 AM, Mathias Nyman
wrote:
>>> @@ -4753,6 +4753,8 @@ int xhci_gen_setup(struct usb_hcd *hcd,
>>> xhci_get_quirks_t get_quirks)
>>> * companion controller.
>>> */
>>> hcd->has_tt = 1;
>>> + /* prevent USB2 runtime pm until USB3 parts are
>>> initialized */
>>> + pm_runtime_get_noresume(hcd->self.controller);
>>> } else {
>>> /* xHCI private pointer was set in xhci_pci_probe for
>>> the second
>>> * registered roothub.
>>> @@ -4765,6 +4767,9 @@ int xhci_gen_setup(struct usb_hcd *hcd,
>>> xhci_get_quirks_t get_quirks)
>>> if (xhci->hci_version < 0x100)
>>> hcd->self.no_sg_constraint = 1;
>>>
>>> + /* USB3 initialized far enough to allow runtime pm
>>> suspend */
>>> + pm_runtime_put_noidle(hcd->self.controller);
>>> +
>>
>>
>> Why the noresume and noidle versions? Shouldn't these be the _sync
>> versions?
>>
>
> As I understood it the _sync versions may actually run pm_runtime_resume /
> pm_runtime_idle. Which is not what I'm looking for here.
>
> Noresume and noidle will just increase / decrease the usage count and that
> way prevent usb2 roothub register from calling xhci_suspend.
>
> This is how I gather it works:
>
> Round 1: add usb2 hcd
>
> usb_add_hcd() // primary usb2 hcd
> hcd->self.root_hub = usb_alloc_dev()
> hcd->driver->reset(hcd) -> xhci_gen_setup()
> if(primary_hcd) // true
> pm_runtime_get_noresume(hcd->self.controller) // +1 usage
> register_root_hub(hcd) -> usb_new_device(usb_dev)
> pm_runtime_set_active(&udev->dev);
> pm_runtime_get_noresume(&udev->dev);
> pm_runtime_use_autosuspend(&udev->dev);
> pm_runtime_enable(&udev->dev);
> ...
> pm_runtime_put_sync_autosuspend(&udev->dev); this would trigger
> xhci_suspend if all usages were 0. (for both roothub and controller)
> which oopses as usb3 stuff is not yet initialized
>
> Round 2: add usb 3 hcd
>
> usb_add_hcd() // secondary usb3 hcd
> hcd->self.root_hub = usb_alloc_dev()
> hcd->driver->reset(hcd) -> xhci_gen_setup()
> if(primary_hcd) // false
> ..
> else
> pm_runtime_put_noidle(hcd->self.controller) // -1 usage, *
> register_root_hub(hcd) -> usb_new_device(usb_dev)
> pm_runtime_set_active(&udev->dev);
> pm_runtime_get_noresume(&udev->dev);
> pm_runtime_use_autosuspend(&udev->dev);
> pm_runtime_enable(&udev->dev);
> ...
> pm_runtime_put_sync_autosuspend(&udev->dev); // ok to trigger would
> trigger xhci_suspend if all usages were 0.
>
> *) unnecessary to trigger any suspend as we're just about to register the
> usb3 roothub. but we need to decrement the usage counter.
Right, but I assume you'd want to hold the reference until after the
hub is registered, otherwise there's still a chance we suspend right
before register. So I'm saying hold the reference until the
registration process takes its own.
--
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] xhci: Prevent runtime pm from autosuspending during initialization
On Fri, 21 Feb 2014, Sarah Sharp wrote: > > Why would this trigger xhci_suspend? Isn't we still running in the > > context of the probe routine? The controller won't be suspended until > > the probe call returns. > > Maybe it helps to look at the context of the bug report Mathias is > trying to fix? > > http://marc.info/?l=linux-usb&m=138914518219334&w=2 > http://marc.info/?l=linux-kernel&m=138919609832200&w=2 > > It's pretty clear from the oops that xhci_suspend gets called before the > second roothub is registered. From the group it came from, I suspect > they have lots of random patches applied on top of their 3.10 kernel, > but let's assume that's not the issue and explore whether xhci_suspend > can be called before probe completes. > > The xHCI driver registers its own PCI probe function, and then it calls > usb_hcd_pci_probe(), and then registers the second roothub. So if the > USB core is somehow preventing PCI suspend in usb_hcd_pci_probe(), it > won't help for our special init flow. If the PCI core is supposed to > prevent PCI suspend until probe completes, then yes, xhci_suspend > shouldn't ever be called until both buses are registered. Ah, I see the problem. After calling usb_hcd_pci_probe to register the primary bus, the driver registers the secondary bus by hand. The thing is, usb_hcd_pci_probe does a pm_runtime_put_noidle just before returning. Therefore a runtime suspend can occur before the driver is fully ready. However, Mathias's patch is incorrect because it ignores the possibility of errors during probing. An error would leave the controller with an elevated usage counter. Instead of calling the pm_runtime routines for hcd->self.controller, it should use &hcd->self.root_hub->dev in the "if" clause and &hcd->primary_hcd->self.root_hub->dev in the "else" clause. Then errors won't matter, because the USB-2 root-hub structure will be destroyed if anything goes wrong. 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: [RFC PATCH] xhci: Prevent runtime pm from autosuspending during initialization
On Fri, 21 Feb 2014, Dan Williams wrote: > > Round 2: add usb 3 hcd > > > > usb_add_hcd() // secondary usb3 hcd > > hcd->self.root_hub = usb_alloc_dev() > > hcd->driver->reset(hcd) -> xhci_gen_setup() > > if(primary_hcd) // false > > .. > > else > > pm_runtime_put_noidle(hcd->self.controller) // -1 usage, * > > register_root_hub(hcd) -> usb_new_device(usb_dev) > > pm_runtime_set_active(&udev->dev); > > pm_runtime_get_noresume(&udev->dev); > > pm_runtime_use_autosuspend(&udev->dev); > > pm_runtime_enable(&udev->dev); > > ... > > pm_runtime_put_sync_autosuspend(&udev->dev); // ok to trigger would > > trigger xhci_suspend if all usages were 0. > > > > *) unnecessary to trigger any suspend as we're just about to register the > > usb3 roothub. but we need to decrement the usage counter. > > Right, but I assume you'd want to hold the reference until after the > hub is registered, otherwise there's still a chance we suspend right > before register. So I'm saying hold the reference until the > registration process takes its own. To be really safe about it, you should call pm_runtime_get_noresume at the start of xhci_pci_probe, and pm_runtime_put_noidle at the end (as well as in the error return pathways). 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
USB kernel oops
$ uname -a Linux hellcat 3.12.9-gentoo #1 SMP PREEMPT Mon Jan 27 08:32:22 MST 2014 x86_64 Intel(R) Core(TM) i7-3770 CPU @ 3.40GHz GenuineIntel GNU/Linux distro: gentoo kernel: sys-kernel/gentoo-sources-3.12.9 oops: [531193.073318] usb 4-4.3.3: Failed to set U1 timeout to 0x0,error code -71 [531193.073326] BUG: unable to handle kernel NULL pointer dereference at 0010 [531193.073772] IP: [] usb_enable_link_state+0x38/0x350 [531193.074149] PGD 60ee9b067 PUD 5cc3b6067 PMD 0 [531193.074640] Oops: [#1] PREEMPT SMP [531193.075126] Modules linked in: snd_pcm_oss snd_mixer_oss snd_hda_codec_hdmi snd_hda_codec_realtek nvidia(PO) fglrx(PO) snd_hda_intel x86_pkg_temp_thermal snd_hda_codec coretemp snd_hwdep snd_pcm snd_page_alloc e1000e snd_timer ptp snd pps_core [531193.077140] CPU: 0 PID: 25835 Comm: usb-storage Tainted: P O 3.12.9-gentoo #1 [531193.077461] Hardware name: Dell Inc. OptiPlex 7010/0KRC95, BIOS A12 01/10/2013 [531193.09] task: 8806111d0d00 ti: 8805cc3ee000 task.ti: 8805cc3ee000 [531193.078097] RIP: 0010:[] [] usb_enable_link_state+0x38/0x350 [531193.078716] RSP: :8805cc3efc08 EFLAGS: 00010246 [531193.079025] RAX: RBX: 8805cc3dc800 RCX: [531193.079341] RDX: 0001 RSI: 8805cc3dc801 RDI: 880611fede00 [531193.079656] RBP: 0001 R08: 0400 R09: 81a108e4 [531193.079972] R10: 034a R11: 0349 R12: 880611fede00 [531193.080290] R13: 880610da4800 R14: 8805cc3dc800 R15: [531193.080605] FS: () GS:88062dc0() knlGS: [531193.080925] CS: 0010 DS: ES: CR0: 80050033 [531193.081235] CR2: 0010 CR3: 0005cc14b000 CR4: 001427e0 [531193.081551] Stack: [531193.081846] 817f824f ffb9 dead00100100 [531193.082407] 0010 8147e84c 8805cc3dc800 [531193.082973] 880611fede00 0003 880610da4800 8805cc3dc800 [531193.083534] Call Trace: [531193.083835] [] ? usb_set_lpm_timeout+0x12c/0x140 [531193.084145] [] ? usb_enable_lpm+0x88/0xb0 [531193.084452] [] ? usb_disable_lpm+0xb2/0xc0 [531193.084759] [] ? usb_unlocked_disable_lpm+0x2e/0x60 [531193.085066] [] ? usb_reset_and_verify_device+0x9f/0x5f0 [531193.085376] [] ? usb_reset_device+0xcf/0x190 [531193.085688] [] ? usb_stor_port_reset+0x61/0x70 [531193.086002] [] ? usb_stor_invoke_transport+0x92/0x4b0 [531193.086317] [] ? usb_stor_control_thread+0x147/0x250 [531193.086630] [] ? fill_inquiry_response+0x170/0x170 [531193.086946] [] ? fill_inquiry_response+0x170/0x170 [531193.087260] [] ? kthread+0xb3/0xc0 [531193.087568] [] ? copy_namespaces+0xa0/0xc0 [531193.087877] [] ? __kthread_parkme+0x80/0x80 [531193.088190] [] ? ret_from_fork+0x7c/0xb0 [531193.088501] [] ? __kthread_parkme+0x80/0x80 [531193.088808] Code: 89 6c 24 40 89 d5 4c 89 64 24 48 83 fd 01 49 89 fc 4c 89 6c 24 50 4c 89 74 24 58 4c 89 7c 24 60 48 8b 86 e0 02 00 00 40 0f 94 c6 <48> 8b 40 10 0f b7 50 08 0f 84 ba 00 00 00 83 fd 02 40 0f 94 c7 [531193.093613] RIP [] usb_enable_link_state+0x38/0x350 [531193.093991] RSP [531193.094290] CR2: 0010 [531193.100655] ---[ end trace 9332546a3bef43c9 ]--- $ lsmod Module Size Used by snd_pcm_oss37730 0 snd_mixer_oss 14308 1 snd_pcm_oss snd_hda_codec_hdmi 28786 2 snd_hda_codec_realtek39897 1 nvidia 10612283 84 fglrx7261959 0 snd_hda_intel 27117 10 x86_pkg_temp_thermal 2920 0 snd_hda_codec 124872 3 snd_hda_codec_realtek,snd_hda_codec_hdmi,snd_hda_intel coretemp4728 0 snd_hwdep 5805 1 snd_hda_codec snd_pcm74063 6 snd_pcm_oss,snd_hda_codec_hdmi,snd_hda_codec,snd_hda_intel snd_page_alloc 6746 2 snd_pcm,snd_hda_intel e1000e183723 0 snd_timer 18097 3 snd_pcm ptp 7788 1 e1000e snd56343 25 snd_hda_codec_realtek,snd_pcm_oss,snd_hwdep,snd_timer,snd_hda_codec_hdmi,snd_pcm,snd_hda_codec,snd_hda_intel,snd_mixer_oss pps_core6593 1 ptp description: Fairly regularly (within a week's time), yet so far unpredictably, my office desktop hits this oops. After which my USB keyboard no longer responds. Physically disconnecting and reconnecting it has no effect--even into other usb ports. Running 'reboot' hangs somewhere attempting to shut down. I have to do a hard reset to reboot it. My keyboard is a Kinesis Advantage MPC/USB if that makes any difference. I have the same keyboard on three different machines, and this has only happened on this machine. However it has done so on this machine with several different kernel versions: 3.13.0 3.12.9 3.12.2 3.10.25 3.10.17 In case tainting is an issue, this m
Re: More Info
On Thu, Feb 20, 2014 at 04:02:43PM -0800, Jay S wrote: > I tried yet another USB 3.0 external HDD enclosure with the 1 TB WD > drive that has been the source of all these problems. This one is > made by Icy Dock. It's a MB080U35-1SB, and has USB 3.0 as well as > eSATA. I figured the eSATA would work even if the USB 3.0 failed. > > Eureka! It works via USB 3.0. The output of dmesg is as follows: ... > This one uses a different chipset than any of the others I've tried. > I hope this helps. I am even more hopeful that this wasn't a wild > goose chase I led you on due to problems with these external > docks/enclosures and not something in the kernel. Great, I'm glad it's not an xHCI host controller issue. Thanks for letting us know, Jay! Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] storage: accept some UAS devices if streams are unavailable
On Tue, Feb 11, 2014 at 08:36:04PM +0100, oli...@neukum.org wrote:
> From: Oliver Neukum
>
> On some older XHCIs streams are not supported and the UAS driver
> will fail at probe time. For those devices storage should try
> to bind to UAS devices.
> This patch adds a flag for stream support to HCDs and evaluates
> it.
Oliver, the logic in this patch is off. When I plug it into an Intel
Panther Point xHCI host that has hcc_params set to 0x20007181, which
translates to 2^(7+1) stream IDs, the device comes up as usb-storage,
not uas:
sarah@xanatos:~$ lsusb -t
/: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
|__ Port 2: Dev 2, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
> Signed-off-by: Oliver Neukum
> ---
> drivers/usb/host/xhci-pci.c | 3 +++
> drivers/usb/host/xhci-plat.c | 3 +++
> drivers/usb/storage/uas-detect.h | 4
> include/linux/usb/hcd.h | 1 +
> 4 files changed, 11 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 73f5208..93d4895 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -223,6 +223,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const
> struct pci_device_id *id)
> if (xhci->quirks & XHCI_LPM_SUPPORT)
> hcd_to_bus(xhci->shared_hcd)->root_hub->lpm_capable = 1;
>
> + if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> + hcd->can_do_streams = 1;
> +
> return 0;
If you look at the code before that, hcd is set to the host controller
driver structure for the USB 2.0 hcd:
/* USB 2.0 roothub is stored in the PCI device now. */
hcd = dev_get_drvdata(&dev->dev);
xhci = hcd_to_xhci(hcd);
xhci->shared_hcd = usb_create_shared_hcd(driver, &dev->dev,
pci_name(dev), hcd);
if (!xhci->shared_hcd) {
retval = -ENOMEM;
goto dealloc_usb2_hcd;
}
/* Set the xHCI pointer before xhci_pci_setup() (aka hcd_driver.reset)
* is called by usb_add_hcd().
*/
*((struct xhci_hcd **) xhci->shared_hcd->hcd_priv) = xhci;
retval = usb_add_hcd(xhci->shared_hcd, dev->irq,
IRQF_SHARED);
if (retval)
goto put_usb3_hcd;
/* Roothub already marked as USB 3.0 speed */
So your patch should set xhci->shared_hcd->can_do_streams instead.
> put_usb3_hcd:
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index d9c169f..6e328ec 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -156,6 +156,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
>*/
> *((struct xhci_hcd **) xhci->shared_hcd->hcd_priv) = xhci;
>
> + if (HCC_MAX_PSA(xhci->hcc_params) >=4)
> + hcd->can_do_streams = 1;
> +
> ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> if (ret)
> goto put_usb3_hcd;
Same here.
> diff --git a/drivers/usb/storage/uas-detect.h
> b/drivers/usb/storage/uas-detect.h
> index b8a02e1..bb05b98 100644
> --- a/drivers/usb/storage/uas-detect.h
> +++ b/drivers/usb/storage/uas-detect.h
> @@ -72,6 +72,7 @@ static int uas_use_uas_driver(struct usb_interface *intf,
> {
> struct usb_host_endpoint *eps[4] = { };
> struct usb_device *udev = interface_to_usbdev(intf);
> + struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> unsigned long flags = id->driver_info;
> int r, alt;
>
> @@ -80,6 +81,9 @@ static int uas_use_uas_driver(struct usb_interface *intf,
> if (flags & US_FL_IGNORE_UAS)
> return 0;
>
> + if (udev->speed >= USB_SPEED_SUPER && !hcd->can_do_streams)
> + return 0;
> +
You probably want to add some debugging to let the user know that their
xHCI host doesn't support streams, and that's required for USB 3.0 UAS
devices.
Can you fix these issues?
Thanks,
Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 01/16] usb: disable port power control if not supported in wHubCharacteristics
A hub indicates whether it supports per-port power control via the
wHubCharacteristics field in its descriptor. If it is not supported
a hub will still emulate ClearPortPower(PORT_POWER) requests by
stopping the link state machine. However, since this does not save
power do not bother suspending.
This also consolidates support checks into a
hub_is_port_power_switchable() helper.
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c |8 ++--
drivers/usb/core/hub.h | 10 ++
drivers/usb/core/port.c | 12 +++-
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 519f2c3594b2..4e967f613e70 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -810,8 +810,6 @@ static unsigned hub_power_on(struct usb_hub *hub, bool
do_delay)
int port1;
unsigned pgood_delay = hub->descriptor->bPwrOn2PwrGood * 2;
unsigned delay;
- u16 wHubCharacteristics =
- le16_to_cpu(hub->descriptor->wHubCharacteristics);
/* Enable power on each port. Some hubs have reserved values
* of LPSM (> 2) in their descriptors, even though they are
@@ -819,7 +817,7 @@ static unsigned hub_power_on(struct usb_hub *hub, bool
do_delay)
* but only emulate it. In all cases, the ports won't work
* unless we send these messages to the hub.
*/
- if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2)
+ if (hub_is_port_power_switchable(hub))
dev_dbg(hub->intfdev, "enabling power on all ports\n");
else
dev_dbg(hub->intfdev, "trying to enable port power on "
@@ -4383,8 +4381,6 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
struct usb_device *hdev = hub->hdev;
struct device *hub_dev = hub->intfdev;
struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
- unsigned wHubCharacteristics =
- le16_to_cpu(hub->descriptor->wHubCharacteristics);
struct usb_device *udev;
int status, i;
unsigned unit_load;
@@ -4469,7 +4465,7 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
test_bit(port1, hub->removed_bits)) {
/* maybe switch power back on (e.g. root hub was reset) */
- if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2
+ if (hub_is_port_power_switchable(hub)
&& !port_is_power_on(hub, portstatus))
set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index df629a310e44..baf5b48b79f7 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -111,6 +111,16 @@ extern int hub_port_debounce(struct usb_hub *hub, int
port1,
extern int usb_clear_port_feature(struct usb_device *hdev,
int port1, int feature);
+static inline bool hub_is_port_power_switchable(struct usb_hub *hub)
+{
+ __le16 hcs;
+
+ if (!hub)
+ return false;
+ hcs = hub->descriptor->wHubCharacteristics;
+ return (le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM;
+}
+
static inline int hub_port_debounce_be_connected(struct usb_hub *hub,
int port1)
{
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 51542f852393..9ae8a499b70f 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -171,12 +171,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int
port1)
pm_runtime_set_active(&port_dev->dev);
- /* It would be dangerous if user space couldn't
-* prevent usb device from being powered off. So don't
-* enable port runtime pm if failed to expose port's pm qos.
+ /* It would be dangerous if user space couldn't prevent usb
+* device from being powered off. So don't enable port runtime
+* pm if failed to expose port's pm qos, or if the hub does not
+* support power switching
*/
- if (!dev_pm_qos_expose_flags(&port_dev->dev,
- PM_QOS_FLAG_NO_POWER_OFF))
+ if (hub_is_port_power_switchable(hub)
+ && dev_pm_qos_expose_flags(&port_dev->dev,
+ PM_QOS_FLAG_NO_POWER_OFF) == 0)
pm_runtime_enable(&port_dev->dev);
device_enable_async_suspend(&port_dev->dev);
--
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 v5 02/16] usb: assign default peer ports for root hubs
Assume that the peer of a superspeed port is the port with the same id
on the shared_hcd root hub. This identification scheme is required of
external hubs by the USB3 spec [1]. However, for root hubs, tier mismatch
may be in effect [2]. Tier mismatch can only be enumerated via platform
firmware. For now, simply perform the nominal association.
[1]: usb 3.1 section 10.3.3
[2]: xhci 1.1 appendix D
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.h |2 +
drivers/usb/core/port.c | 98 ---
2 files changed, 94 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index baf5b48b79f7..d51feb68165b 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -81,6 +81,7 @@ struct usb_hub {
* @child: usb device attached to the port
* @dev: generic device interface
* @port_owner: port's owner
+ * @peer: related usb2 and usb3 ports (share the same connector)
* @connect_type: port's connect type
* @portnum: port index num based one
* @power_is_on: port's power state
@@ -90,6 +91,7 @@ struct usb_port {
struct usb_device *child;
struct device dev;
struct dev_state *port_owner;
+ struct usb_port *peer;
enum usb_port_connect_type connect_type;
u8 portnum;
unsigned power_is_on:1;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 9ae8a499b70f..d57fb01bbc1c 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -21,6 +21,7 @@
#include "hub.h"
+static DEFINE_MUTEX(peer_lock);
static const struct attribute_group *port_dev_group[];
static ssize_t connect_type_show(struct device *dev,
@@ -146,9 +147,83 @@ struct device_type usb_port_device_type = {
.pm = &usb_port_pm_ops,
};
+/*
+ * Set the default peer port for root hubs. Platform firmware will have
+ * already set the peer if tier-mismatch is present. Assumes the
+ * primary_hcd is registered first
+ */
+static struct usb_port *find_default_peer(struct usb_hub *hub, int port1)
+{
+ struct usb_device *hdev = hub->hdev;
+ struct usb_port *peer = NULL;
+
+ if (!hdev->parent) {
+ struct usb_hub *peer_hub;
+ struct usb_device *peer_hdev;
+ struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
+ struct usb_hcd *peer_hcd = hcd->primary_hcd;
+
+ if (!peer_hcd || hcd == peer_hcd)
+ return NULL;
+
+ peer_hdev = peer_hcd->self.root_hub;
+ peer_hub = usb_hub_to_struct_hub(peer_hdev);
+ if (peer_hub && port1 <= peer_hdev->maxchild)
+ peer = peer_hub->ports[port1 - 1];
+ }
+
+ return peer;
+}
+
+static void link_peers(struct usb_port *left, struct usb_port *right)
+{
+ struct device *rdev;
+ struct device *ldev;
+
+ if (left->peer) {
+ right = left->peer;
+ ldev = left->dev.parent->parent;
+ rdev = right->dev.parent->parent;
+
+ WARN_ONCE(1, "%s port%d already peered with %s %d\n",
+ dev_name(ldev), left->portnum, dev_name(rdev),
+ right->portnum);
+ return;
+ } else if (right->peer) {
+ left = right->peer;
+ ldev = left->dev.parent->parent;
+ rdev = right->dev.parent->parent;
+
+ WARN_ONCE(1, "%s port%d already peered with %s %d\n",
+ dev_name(rdev), right->portnum, dev_name(ldev),
+ left->portnum);
+ return;
+ }
+
+ get_device(&right->dev);
+ left->peer = right;
+ get_device(&left->dev);
+ right->peer = left;
+}
+
+static void unlink_peers(struct usb_port *left, struct usb_port *right)
+{
+ struct device *rdev = right->dev.parent->parent;
+ struct device *ldev = left->dev.parent->parent;
+
+ WARN_ONCE(right->peer != left || left->peer != right,
+ "%s port%d and %s port%d are not peers?\n",
+ dev_name(ldev), left->portnum, dev_name(rdev), right->portnum);
+
+ put_device(&left->dev);
+ right->peer = NULL;
+ put_device(&right->dev);
+ left->peer = NULL;
+}
+
int usb_hub_create_port_device(struct usb_hub *hub, int port1)
{
- struct usb_port *port_dev = NULL;
+ struct usb_port *port_dev, *peer;
int retval;
port_dev = kzalloc(sizeof(*port_dev), GFP_KERNEL);
@@ -164,11 +239,16 @@ int usb_hub_create_port_device(struct usb_hub *hub, int
port1)
port_dev->dev.groups = port_dev_group;
port_dev->dev.type = &usb_port_device_type;
dev_set_name(&port_dev->dev, "port%d", port1);
-
retval = device_register(&port_dev->dev);
if (retval)
goto error_register;
+ mutex_lock(&peer_lock);
+ peer = find_default_peer(hub, port1);
+ if (peer)
+
[PATCH v5 03/16] usb: assign usb3 external hub port peers
Given that root hub port peers are already established, external hub peer
ports can be determined by traversing the device topology:
1/ ascend to the parent hub and find the upstream port_dev
2/ walk ->peer to find the peer port
3/ descend to the peer hub via ->child
4/ find the port with the matching port id
Note that this assumes the port labelling scheme required by the
specification [1].
[1]: usb3 3.1 section 10.3.3
Signed-off-by: Dan Williams
---
drivers/usb/core/port.c | 22 --
1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index d57fb01bbc1c..068d495007e1 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -155,11 +155,11 @@ struct device_type usb_port_device_type = {
static struct usb_port *find_default_peer(struct usb_hub *hub, int port1)
{
struct usb_device *hdev = hub->hdev;
+ struct usb_device *peer_hdev;
struct usb_port *peer = NULL;
+ struct usb_hub *peer_hub;
if (!hdev->parent) {
- struct usb_hub *peer_hub;
- struct usb_device *peer_hdev;
struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
struct usb_hcd *peer_hcd = hcd->primary_hcd;
@@ -170,6 +170,24 @@ static struct usb_port *find_default_peer(struct usb_hub
*hub, int port1)
peer_hub = usb_hub_to_struct_hub(peer_hdev);
if (peer_hub && port1 <= peer_hdev->maxchild)
peer = peer_hub->ports[port1 - 1];
+ } else {
+ struct usb_port *upstream;
+ struct usb_device *parent = hdev->parent;
+ struct usb_hub *parent_hub = usb_hub_to_struct_hub(parent);
+
+ if (!parent_hub)
+ return NULL;
+
+ upstream = parent_hub->ports[hdev->portnum - 1];
+ if (!upstream->peer)
+ return NULL;
+
+ peer_hdev = upstream->peer->child;
+ peer_hub = usb_hub_to_struct_hub(peer_hdev);
+ if (!peer_hub || port1 > peer_hdev->maxchild)
+ return NULL;
+
+ peer = peer_hub->ports[port1 - 1];
}
return peer;
--
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 v5 06/16] usb: defer suspension of superspeed port while peer is powered
ClearPortFeature(PORT_POWER) on a usb3 port places the port in either a
DSPORT.Powered-off-detect / DSPORT.Powered-off-reset loop, or the
DSPORT.Powered-off state. There is no way to ensure that RX
terminations will persist in this state, so it is possible a device will
degrade to its usb2 connection. Prevent this by blocking power-off of a
usb3 port while its usb2 peer is active, and powering on a usb3 port
before its usb2 peer.
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c |5 -
drivers/usb/core/hub.h |5 +
drivers/usb/core/port.c | 50 +++
3 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 4e967f613e70..479abbe0ba09 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -36,11 +36,6 @@
#define USB_VENDOR_GENESYS_LOGIC 0x05e3
#define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND 0x01
-static inline int hub_is_superspeed(struct usb_device *hdev)
-{
- return (hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS);
-}
-
/* Protect struct usb_device->state and ->children members
* Note: Both are also protected by ->dev.sem, except that ->state can
* change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index e7a9666e7cd6..87efea1424d3 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -129,6 +129,11 @@ static inline bool hub_is_port_power_switchable(struct
usb_hub *hub)
return (le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM;
}
+static inline int hub_is_superspeed(struct usb_device *hdev)
+{
+ return hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS;
+}
+
static inline int hub_port_debounce_be_connected(struct usb_hub *hub,
int port1)
{
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 86e78bbd2e4e..1e38f123ed8c 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -77,12 +77,20 @@ static int usb_port_runtime_resume(struct device *dev)
struct usb_device *hdev = to_usb_device(dev->parent->parent);
struct usb_interface *intf = to_usb_interface(dev->parent);
struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+ struct usb_port *peer = port_dev->peer;
int port1 = port_dev->portnum;
int retval;
if (!hub)
return -EINVAL;
+ /*
+* Power on our usb3 peer before this usb2 port to prevent a usb3
+* device from degrading to its usb2 connection
+*/
+ if (!hub_is_superspeed(hdev) && peer)
+ pm_runtime_get_sync(&peer->dev);
+
usb_autopm_get_interface(intf);
set_bit(port1, hub->busy_bits);
@@ -104,6 +112,7 @@ static int usb_port_runtime_resume(struct device *dev)
clear_bit(port1, hub->busy_bits);
usb_autopm_put_interface(intf);
+
return retval;
}
@@ -113,6 +122,7 @@ static int usb_port_runtime_suspend(struct device *dev)
struct usb_device *hdev = to_usb_device(dev->parent->parent);
struct usb_interface *intf = to_usb_interface(dev->parent);
struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+ struct usb_port *peer = port_dev->peer;
int port1 = port_dev->portnum;
int retval;
@@ -130,6 +140,15 @@ static int usb_port_runtime_suspend(struct device *dev)
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
clear_bit(port1, hub->busy_bits);
usb_autopm_put_interface(intf);
+
+ /*
+* Our peer usb3 port may now be able to suspend, asynchronously
+* queue a suspend request to observe that this usb2 peer port
+* is now off.
+*/
+ if (!hub_is_superspeed(hdev) && peer)
+ pm_runtime_put(&peer->dev);
+
return retval;
}
#endif
@@ -198,13 +217,12 @@ static struct usb_port *find_default_peer(struct usb_hub
*hub, int port1)
static int link_peers(struct usb_port *left, struct usb_port *right)
{
- struct device *rdev;
- struct device *ldev;
+ struct device *ldev = left->dev.parent->parent;
+ struct device *rdev = right->dev.parent->parent;
int rc;
if (left->peer) {
right = left->peer;
- ldev = left->dev.parent->parent;
rdev = right->dev.parent->parent;
WARN_ONCE(1, "%s port%d already peered with %s %d\n",
@@ -214,7 +232,6 @@ static int link_peers(struct usb_port *left, struct
usb_port *right)
} else if (right->peer) {
left = right->peer;
ldev = left->dev.parent->parent;
- rdev = right->dev.parent->parent;
WARN_ONCE(1, "%s port%d already peered with %s %d\n",
dev_name(rdev), right->portnum, dev_name(ldev),
@@ -236,6 +253,19 @@ static int link_peers(struct usb_port *left, struct
usb_po
[PATCH v5 08/16] usb: usb3 ports do not support FEAT_C_ENABLE
The port pm_runtime implementation unconditionally clears FEAT_C_ENABLE after clearing PORT_POWER, but the bit is reserved on usb3 hub ports. We expect khubd to be prevented from running because the port state is not RPM_ACTIVE, so we need to clear any errors for usb2 ports. Signed-off-by: Dan Williams --- drivers/usb/core/port.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 921cce9dd808..0ce07a517c14 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -136,7 +136,8 @@ static int usb_port_runtime_suspend(struct device *dev) set_bit(port1, hub->busy_bits); retval = usb_hub_set_port_power(hdev, hub, port1, false); usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION); - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); + if (!hub_is_superspeed(hdev)) + usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); clear_bit(port1, hub->busy_bits); usb_autopm_put_interface(intf); -- 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 v5 07/16] usb: don't clear FEAT_C_ENABLE on usb_port_runtime_resume failure
Three reasons: 1/ It's an invalid operation on usb3 ports 2/ There's no guarantee of when / if a usb2 port has entered an error state relative to PORT_POWER request 3/ The port is active / powered at this point, so khubd will clear it as a matter of course Signed-off-by: Dan Williams --- drivers/usb/core/port.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 1e38f123ed8c..921cce9dd808 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -106,7 +106,6 @@ static int usb_port_runtime_resume(struct device *dev) if (retval < 0) dev_dbg(&port_dev->dev, "can't get reconnection after setting port power on, status %d\n", retval); - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); retval = 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
[PATCH v5 04/16] usb: find internal hub tier mismatch via acpi
ACPI identifies peer ports by setting their 'group_token' and
'group_position' _PLD data to the same value. If a platform has tier
mismatch [1] , ACPI can override the default (USB3 defined) peer port
association for internal hubs. External hubs follow the default peer
association scheme.
Location data is cached as an opaque cookie in usb_port_location data.
Note that we only consider the group_token and group_position attributes
from the _PLD data as ACPI specifies that group_token is a unique
identifier.
The bulk of the implementation is recursively fixing up peer
associations once we detect tier mismatch. Due to the way acpi data is
associated to a usb_port device (via a callback triggered by
device_register()) we do not discover tier mismatch until after the port
has been registered. This leads to a question about what happens when a
pm runtime event occurs while the peer associations are wrong, and can
we prevent that from occurring? The result of wrong peer associations
is always the possibility that a USB3 device switches to its USB2
connection upon detecting the USB3 port being turned off. As far as
closing the race there are 2 considerations:
1/ the acpi data may be wrong so the risk of wrong peer associations
cannot be entirely eliminated by the kernel
2/ if the acpi data is good then we can potentially close the race by
waiting for all the initial hub discovery to complete before processing
the first pm runtime suspend event. (not implemented in this patch).
[1]: xhci 1.1 appendix D figure 131
[2]: acpi 5 section 6.1.8
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.h |6 ++
drivers/usb/core/port.c | 161 ++-
drivers/usb/core/usb-acpi.c | 35 +++--
drivers/usb/core/usb.h |2 +
4 files changed, 192 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index d51feb68165b..e7a9666e7cd6 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -76,6 +76,10 @@ struct usb_hub {
struct usb_port **ports;
};
+struct usb_port_location {
+ u32 cookie;
+};
+
/**
* struct usb port - kernel's representation of a usb port
* @child: usb device attached to the port
@@ -83,6 +87,7 @@ struct usb_hub {
* @port_owner: port's owner
* @peer: related usb2 and usb3 ports (share the same connector)
* @connect_type: port's connect type
+ * @location: opaque representation of platform connector location
* @portnum: port index num based one
* @power_is_on: port's power state
* @did_runtime_put: port has done pm_runtime_put().
@@ -93,6 +98,7 @@ struct usb_port {
struct dev_state *port_owner;
struct usb_port *peer;
enum usb_port_connect_type connect_type;
+ struct usb_port_location location;
u8 portnum;
unsigned power_is_on:1;
unsigned did_runtime_put:1;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 068d495007e1..0b8ae73b0466 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -154,11 +154,14 @@ struct device_type usb_port_device_type = {
*/
static struct usb_port *find_default_peer(struct usb_hub *hub, int port1)
{
- struct usb_device *hdev = hub->hdev;
+ struct usb_device *hdev = hub ? hub->hdev : NULL;
struct usb_device *peer_hdev;
struct usb_port *peer = NULL;
struct usb_hub *peer_hub;
+ if (!hub)
+ return NULL;
+
if (!hdev->parent) {
struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
struct usb_hcd *peer_hcd = hcd->primary_hcd;
@@ -239,9 +242,154 @@ static void unlink_peers(struct usb_port *left, struct
usb_port *right)
left->peer = NULL;
}
+/**
+ * for_each_child_port() - invoke 'fn' on all usb_port instances beneath 'hdev'
+ * @hdev: potential hub usb_device (validated by usb_hub_to_struct_hub)
+ * @level: track recursion level to stop after reaching USB spec max depth
+ * @p: parameter to pass to 'fn'
+ * @fn: routine to invoke on each port
+ *
+ * Recursively iterate all ports (depth-first) beneath 'hdev' until 'fn'
+ * returns a non-NULL usb_port or all ports have been visited.
+ */
+static struct usb_port *for_each_child_port(struct usb_device *hdev, int level,
+ void *p, struct usb_port * (*fn)(struct usb_port *, void *))
+{
+ struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+ int port1;
+
+#define MAX_HUB_DEPTH 5
+ if (!hub || level > MAX_HUB_DEPTH)
+ return NULL;
+
+ level++;
+ for (port1 = 1; port1 <= hdev->maxchild; port1++) {
+ struct usb_port *ret, *port_dev = hub->ports[port1 - 1];
+
+ ret = fn(port_dev, p);
+ if (ret)
+ return ret;
+ ret = for_each_child_port(port_dev->child, level, p, fn);
+ if (ret)
+ return ret;
+ }
+
+ return NULL;
+}
+
+stati
[PATCH v5 14/16] usb: documentation for usb port power off mechanisms
From: Lan Tianyu describe the mechanisms for controlling port power policy and discovering the port power state. Cc: Oliver Neukum Signed-off-by: Lan Tianyu [sarah]: wordsmithing [djbw]: updates for peer port changes Signed-off-by: Dan Williams --- Documentation/usb/power-management.txt | 237 1 files changed, 237 insertions(+), 0 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index 1392b61d6ebe..e67c1d4d1994 100644 --- a/Documentation/usb/power-management.txt +++ b/Documentation/usb/power-management.txt @@ -5,6 +5,25 @@ October 28, 2010 + Contents: + - + * What is Power Management? + * What is Remote Wakeup? + * When is a USB device idle? + * Forms of dynamic PM + * The user interface for dynamic PM + * Changing the default idle-delay time + * Warnings + * The driver interface for Power Management + * The driver interface for autosuspend and autoresume + * Other parts of the driver interface + * Mutual exclusion + * Interaction between dynamic PM and system PM + * xHCI hardware link PM + * USB Port Power Control + * User Interface for Port Power Control + * Suggested Userspace Port Power Policy + What is Power Management? - @@ -516,3 +535,221 @@ relevant attribute files is usb2_hardware_lpm. driver will enable hardware LPM for the device. You can write y/Y/1 or n/N/0 to the file to enable/disable USB2 hardware LPM manually. This is for test purpose mainly. + + + USB Port Power Control + -- + +In addition to suspending endpoint devices and enabling hardware +controlled link power management, the USB subsystem also has the +capability to disable power to individual ports. Power is controlled +through Set/ClearPortFeature(PORT_POWER) requests to a hub. In the case +of a root or platform-internal hub the host controller driver translates +PORT_POWER requests into platform firmware (ACPI) method calls to set +the port power state. For more background see the Linux Plumbers +Conference 2012 slides [1] and video [2]: + +Upon receiving a ClearPortFeature(PORT_POWER) request a USB port is +logically off, and may trigger the actual loss of VBUS to the port [3]. +VBUS may be maintained in the case where a hub gangs multiple ports into +a shared power well causing power to remain until all ports in the gang +are turned off. VBUS may also be maintained by hub ports configured for +a charging application. In any event a logically off port will lose +connection with its device, not respond to hotplug events, and not +respond to remote wakeup events*. + +WARNING: turning off a port may result in the inability to hot add a device. +Please see "User Interface for Port Power Control" for details. + +As far as the effect on the device itself it is similar to what a device +goes through during system suspend, i.e. the power session is lost. Any +USB device or driver that misbehaves with system suspend will be +similarly affected by a port power cycle event. For this reason the +implementation shares the same device recovery path (and honors the same +quirks) as the system resume path for the hub. + +[1]: http://dl.dropbox.com/u/96820575/sarah-sharp-lpt-port-power-off2-mini.pdf +[2]: http://linuxplumbers.ubicast.tv/videos/usb-port-power-off-kerneluserspace-api/ +[3]: USB 3.1 Section 10.12 +* wakeup note: the implementation does not allow a port connected to a + device with wakeup capability to be powered off. + + + User Interface for Port Power Control + - + +The port power control mechanism uses the PM runtime system. Poweroff is +requested by clearing the power/pm_qos_no_power_off flag of the port device +(defaults to 1). If the port is disconnected it will immediately receive a +ClearPortFeature(PORT_POWER) request. Otherwise, it will honor the pm runtime +rules and require the attached child device and all descendants to be suspended. +This mechanism is dependent on the hub advertising port power switching in its +hub descriptor (wHubCharacteristics logical power switching mode field). + +Note, some interface devices/drivers do not support autosuspend. Userspace may +need to unbind the interface drivers before the usb_device will suspend. An +unbound interface device is suspended by default. When unbinding, be careful +to unbind interface drivers, not the driver of the parent usb device. Also, +leave hub interface drivers bound. If the driver for the usb device (not +interface) is unbound the kernel is no longer able to resume the device. If a +hub interface driver is unbound, control of its child ports is lost and all +attached child-devices will disconnect. A good rule of thumb is that if t
[PATCH v5 12/16] usb: resume (wakeup) child device when port is powered on
Unconditionally wake up the child device when the power session is
recovered.
This address the following scenarios:
1/ The device may need a reset on power-session loss, without this
change port power-on recovery exposes khubd to scenarios that
usb_port_resume() is set to handle. Prior to port power control the
only time a power session would be lost is during dpm_suspend of the
hub. In that scenario usb_port_resume() is guaranteed to be called
prior to khubd running for that port. With this change we wakeup the
child device as soon as possible (prior to khubd running again for this
port).
Although khubd has facilities to wake a child device it will only do
so if the portstatus / portchange indicates a suspend state. In the
case of port power control we are not coming from a hub-port-suspend
state. This implemenation extends usb_wake_notification() for the
port rpm_resume case.
2/ This mechanism rate limits port power toggling. The minimum port
power on/off period is now gated by the child device suspend/resume
latency. Empirically this mitigates devices downgrading their connection
on perceived instability of the host connection. This ratelimiting is
really only relevant to port power control testing, but it is a nice
side effect of closing the above race. Namely, the race of khubd for
the given port running while a usb_port_resume() event is pending.
3/ Going forward we are finding that power-session recovery requires
warm-resets (http://marc.info/?t=13865923293&r=1&w=2). This
mechanism allows for warm-resets to be requested at the same point in
the resume path for hub dpm_suspend power session losses, or port
rpm_suspend power session losses.
4/ If the device *was* disconnected the only time we'll know for sure is
after a failed resume, so it's necessary for usb_port_runtime_resume()
to expedite a usb_port_resume() to clean up the removed device. The
reasoning for this is "least surprise" for the user. Turning on a port
means that hotplug detection is again enabled for the port, it is
surprising that devices that were removed while the port was off are not
disconnected until they are attempted to be used. As a user "why would
I try to use a device I removed from the system?"
1, 2, and 4 are not a problem in the system dpm_resume case because,
although the power-session is lost, khubd is frozen until after device
resume. For the port rpm_resume use runtime-pm-synchronization to
guarantee the same sequence of events. When a usb_wakeup_notification()
is invoked the port device is in the RPM_RESUMING state. khubd in turn
performs a pm_runtime_barrier() on the port device to flush the port
recovery, holds the port active while it resumes the child, and
completes child device resume before acting on the current portstatus.
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 39 ---
drivers/usb/core/port.c |2 ++
2 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5b4a5e7b3762..1aa1f14bbe22 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -590,9 +590,11 @@ void usb_kick_khubd(struct usb_device *hdev)
* USB 3.0 hubs do not report the port link state change from U3 to U0 when the
* device initiates resume, so the USB core will not receive notice of the
* resume through the normal hub interrupt URB.
+ *
+ * This is also called by usb_port_runtime_resume() to arrange for the child
+ * device to be woken up as part of the power session recovery for the port.
*/
-void usb_wakeup_notification(struct usb_device *hdev,
- unsigned int portnum)
+void usb_wakeup_notification(struct usb_device *hdev, unsigned int port1)
{
struct usb_hub *hub;
@@ -601,7 +603,10 @@ void usb_wakeup_notification(struct usb_device *hdev,
hub = usb_hub_to_struct_hub(hdev);
if (hub) {
- set_bit(portnum, hub->wakeup_bits);
+ struct usb_port *port_dev = hub->ports[port1 - 1];
+
+ if (!test_and_set_bit(port1, hub->wakeup_bits))
+ pm_runtime_get(&port_dev->dev);
kick_khubd(hub);
}
}
@@ -4648,28 +4653,33 @@ static void hub_port_connect_change(struct usb_hub
*hub, int port1,
/* Returns 1 if there was a remote wakeup and a connect status change. */
static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
- u16 portstatus, u16 portchange)
+ u16 portstatus, u16 portchange, int wakeup_change)
__must_hold(&port_dev->status_lock)
{
struct usb_port *port_dev = hub->ports[port - 1];
+ int connect_change = 0, do_wakeup = 1;
struct usb_device *hdev;
struct usb_device *udev;
- int connect_change = 0;
int ret;
hdev = hub->hdev;
udev = port_dev->child;
[RFC PATCH v5 16/16] usb, xhci: flush initial hub discovery to gate port power control
Until all root hubs have been discovered and tier mismatch identified,
port power control is unreliable. When a USB3 port is paired with an
incorrect peer port there is chance a connected device will downgrade
its connection to its USB2 pins. The downgrade occurs when the USB3
port is powered off while the USB2 peer port is powered. Once the peer
ports are correctly assigned the kernel will prevent the disconnect
scenario. So, wait for the peer ports to be correctly assigned before
allowing a USB3 port to power off.
Now that khubd is a workqueue, and provided that khubd arranges to
re-queue enumeration work until all hubs (connected at driver load) are
enumerated, a drain_workqueue() operation will wait for all initial
discovery to complete. This requires the delayed hub->init_work to move
to khubd context. Care is taken to maintain parallel waiting for all
root hubs power on delays. However, since hub_quiesce() is called with
the device lock held it can no longer synchronously wait for init_work
to flush.
The workqueue subsystem enforces that no un-chained work (work queued
outside the workqueue context, e.g. hub_irq) may be queued for the
duration of the drain. Add infrastructure to defer and replay
kick_khubd() requests.
Not Signed-off, pending resolution of the locking horror in hub_quiesce()
---
drivers/usb/core/hcd.c |1
drivers/usb/core/hub.c | 88 ++
drivers/usb/core/hub.h |4 +-
drivers/usb/core/port.c |4 ++
drivers/usb/host/xhci-pci.c |5 ++
drivers/usb/host/xhci-plat.c |4 ++
drivers/usb/host/xhci.c | 10 +
drivers/usb/host/xhci.h |5 ++
include/linux/device.h |5 ++
include/linux/usb.h |1
include/linux/usb/hcd.h |3 +
11 files changed, 112 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index b3ecaf32f2aa..f79bd06edc5f 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2424,6 +2424,7 @@ struct usb_hcd *usb_create_shared_hcd(const struct
hc_driver *driver,
dev_dbg (dev, "hcd alloc failed\n");
return NULL;
}
+ INIT_LIST_HEAD(&hcd->khubd_defer);
if (primary_hcd == NULL) {
hcd->bandwidth_mutex = kmalloc(sizeof(*hcd->bandwidth_mutex),
GFP_KERNEL);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index e0518af66af9..894d0e47b563 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -562,14 +562,20 @@ static void hub_release(struct kref *kref)
kfree(hub);
}
-static void kick_khubd(struct usb_hub *hub)
+static int khubd_draining;
+
+static void kick_khubd_unlocked(struct usb_hub *hub)
{
struct usb_interface *intf = to_usb_interface(hub->intfdev);
- unsigned long flags;
+ struct usb_hcd *hcd = bus_to_hcd(hub->hdev->bus);
- /* Suppress autosuspend until khubd runs */
- spin_lock_irqsave(&hub_event_lock, flags);
- if (!hub->disconnected) {
+ if (hub->disconnected)
+ return;
+
+ if (khubd_draining) {
+ list_move_tail(&hub->defer, &hcd->khubd_defer);
+ } else {
+ /* Suppress autosuspend until khubd runs */
usb_autopm_get_interface_no_resume(intf);
kref_get(&hub->kref);
if (!queue_work(khubd_wq, &hub->event_work)) {
@@ -577,6 +583,14 @@ static void kick_khubd(struct usb_hub *hub)
kref_put(&hub->kref, hub_release);
}
}
+}
+
+static void kick_khubd(struct usb_hub *hub)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&hub_event_lock, flags);
+ kick_khubd_unlocked(hub);
spin_unlock_irqrestore(&hub_event_lock, flags);
}
@@ -588,6 +602,33 @@ void usb_kick_khubd(struct usb_device *hdev)
kick_khubd(hub);
}
+void usb_drain_khubd(struct usb_hcd *hcd)
+{
+ static DEFINE_MUTEX(drain_mutex);
+ struct usb_hub *hub, *_h;
+
+ /* prevent concurrent drainers */
+ mutex_lock(&drain_mutex);
+
+ /* flag khubd to start deferring work */
+ spin_lock_irq(&hub_event_lock);
+ khubd_draining = 1;
+ spin_unlock_irq(&hub_event_lock);
+
+ drain_workqueue(khubd_wq);
+
+ spin_lock_irq(&hub_event_lock);
+ khubd_draining = 0;
+ list_for_each_entry_safe(hub, _h, &hcd->khubd_defer, defer) {
+ list_del_init(&hub->defer);
+ kick_khubd_unlocked(hub);
+ }
+ spin_unlock_irq(&hub_event_lock);
+
+ mutex_unlock(&drain_mutex);
+}
+EXPORT_SYMBOL_GPL(usb_drain_khubd);
+
/*
* Let the USB core know that a USB 3.0 device has sent a Function Wake Device
* Notification, which indicates it had initiated remote wakeup.
@@ -1044,10 +1085,9 @@ static void hub_activate(struct usb_hub *hub, enum
hub_activation_type type)
*/
[PATCH v5 05/16] usb: sysfs link peer ports
The usb topology after this change will have symlinks between usb3 ports
and their usb2 peers, for example:
usb2/2-1/2-1:1.0/port1/peer => ../../../../usb3/3-1/3-1:1.0/port1
usb2/2-1/2-1:1.0/port2/peer => ../../../../usb3/3-1/3-1:1.0/port2
usb2/2-1/2-1:1.0/port3/peer => ../../../../usb3/3-1/3-1:1.0/port3
usb2/2-1/2-1:1.0/port4/peer => ../../../../usb3/3-1/3-1:1.0/port4
usb2/2-0:1.0/port1/peer => ../../../usb3/3-0:1.0/port1
usb2/2-0:1.0/port2/peer => ../../../usb3/3-0:1.0/port2
usb2/2-0:1.0/port3/peer => ../../../usb3/3-0:1.0/port3
usb2/2-0:1.0/port4/peer => ../../../usb3/3-0:1.0/port4
usb3/3-1/3-1:1.0/port1/peer => ../../../../usb2/2-1/2-1:1.0/port1
usb3/3-1/3-1:1.0/port2/peer => ../../../../usb2/2-1/2-1:1.0/port2
usb3/3-1/3-1:1.0/port3/peer => ../../../../usb2/2-1/2-1:1.0/port3
usb3/3-1/3-1:1.0/port4/peer => ../../../../usb2/2-1/2-1:1.0/port4
usb3/3-0:1.0/port1/peer => ../../../usb2/2-0:1.0/port1
usb3/3-0:1.0/port2/peer => ../../../usb2/2-0:1.0/port2
usb3/3-0:1.0/port3/peer => ../../../usb2/2-0:1.0/port3
usb3/3-0:1.0/port4/peer => ../../../usb2/2-0:1.0/port4
Signed-off-by: Dan Williams
---
drivers/usb/core/port.c | 45 +++--
1 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 0b8ae73b0466..86e78bbd2e4e 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -196,10 +196,11 @@ static struct usb_port *find_default_peer(struct usb_hub
*hub, int port1)
return peer;
}
-static void link_peers(struct usb_port *left, struct usb_port *right)
+static int link_peers(struct usb_port *left, struct usb_port *right)
{
struct device *rdev;
struct device *ldev;
+ int rc;
if (left->peer) {
right = left->peer;
@@ -209,7 +210,7 @@ static void link_peers(struct usb_port *left, struct
usb_port *right)
WARN_ONCE(1, "%s port%d already peered with %s %d\n",
dev_name(ldev), left->portnum, dev_name(rdev),
right->portnum);
- return;
+ return -EBUSY;
} else if (right->peer) {
left = right->peer;
ldev = left->dev.parent->parent;
@@ -218,13 +219,43 @@ static void link_peers(struct usb_port *left, struct
usb_port *right)
WARN_ONCE(1, "%s port%d already peered with %s %d\n",
dev_name(rdev), right->portnum, dev_name(ldev),
left->portnum);
- return;
+ return -EBUSY;
+ }
+
+ rc = sysfs_create_link(&left->dev.kobj, &right->dev.kobj, "peer");
+ if (rc)
+ return rc;
+ rc = sysfs_create_link(&right->dev.kobj, &left->dev.kobj, "peer");
+ if (rc) {
+ sysfs_remove_link(&left->dev.kobj, "peer");
+ return rc;
}
get_device(&right->dev);
left->peer = right;
get_device(&left->dev);
right->peer = left;
+
+ return 0;
+}
+
+static void link_peers_report(struct usb_port *left, struct usb_port *right)
+{
+ struct device *rdev = right->dev.parent->parent;
+ struct device *ldev = left->dev.parent->parent;
+ int rc;
+
+ rc = link_peers(left, right);
+ if (rc == 0) {
+ pr_debug("usb: link %s port%d to %s port%d\n",
+ dev_name(ldev), left->portnum,
+ dev_name(rdev), right->portnum);
+ } else {
+ pr_warn("usb: failed to link %s port%d to %s port%d (%d)\n",
+ dev_name(ldev), left->portnum,
+ dev_name(rdev), right->portnum, rc);
+ pr_warn_once("usb: port power management may be unreliable\n");
+ }
}
static void unlink_peers(struct usb_port *left, struct usb_port *right)
@@ -236,8 +267,10 @@ static void unlink_peers(struct usb_port *left, struct
usb_port *right)
"%s port%d and %s port%d are not peers?\n",
dev_name(ldev), left->portnum, dev_name(rdev), right->portnum);
+ sysfs_remove_link(&left->dev.kobj, "peer");
put_device(&left->dev);
right->peer = NULL;
+ sysfs_remove_link(&right->dev.kobj, "peer");
put_device(&right->dev);
left->peer = NULL;
}
@@ -306,7 +339,7 @@ static struct usb_port *do_default_link(struct usb_port
*port_dev, void *p)
* set
*/
if (peer && !port_dev->peer)
- link_peers(port_dev, peer);
+ link_peers_report(port_dev, peer);
return NULL;
}
@@ -372,7 +405,7 @@ void usb_set_hub_port_location(struct usb_device *hdev, int
port1,
enum_peer = 1;
}
- link_peers(port_dev, peer);
+ link_peers_report(port_dev, peer);
/*
* If a peer relationship was invalidated then we need to
@@ -414,7 +447,7 @@
[PATCH v5 09/16] usb: refactor port handling in hub_events()
In preparation for synchronizing port handling with pm_runtime
transitions refactor port handling into its own subroutine.
We expect that clearing some status flags will be required regardless of
the port state, so handle those first and group all non-trivial actions
at the bottom of the routine.
This also splits off the bottom half of hub_port_connect_change() into
hub_port_reconnect() in prepartion for introducing a port->status_lock.
hub_port_reconnect() will expect the port lock to not be held while
hub_port_connect_change() expects to enter with it held.
Other cleanups include:
1/ reflowing to 80 columns
2/ replacing redundant usages of 'hub->hdev' with 'hdev'
3/ baseline debug prints on a common format of "port%d: "
4/ consolidate clearing of ->change_bits() in hub_port_connect_change
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 368
1 files changed, 182 insertions(+), 186 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 479abbe0ba09..d4fab7caaf40 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4362,76 +4362,24 @@ hub_power_remaining (struct usb_hub *hub)
return remaining;
}
-/* Handle physical or logical connection change events.
- * This routine is called when:
- * a port connection-change occurs;
- * a port enable-change occurs (often caused by EMI);
- * usb_reset_and_verify_device() encounters changed descriptors (as from
- * a firmware download)
- * caller already locked the hub
- */
-static void hub_port_connect_change(struct usb_hub *hub, int port1,
- u16 portstatus, u16 portchange)
+static void hub_port_reconnect(struct usb_hub *hub, int port1, u16 portstatus,
+ u16 portchange)
{
+ int status, i;
+ unsigned unit_load;
struct usb_device *hdev = hub->hdev;
struct device *hub_dev = hub->intfdev;
struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
- struct usb_device *udev;
- int status, i;
- unsigned unit_load;
-
- dev_dbg (hub_dev,
- "port %d, status %04x, change %04x, %s\n",
- port1, portstatus, portchange, portspeed(hub, portstatus));
-
- if (hub->has_indicators) {
- set_port_led(hub, port1, HUB_LED_AUTO);
- hub->indicator[port1-1] = INDICATOR_AUTO;
- }
-
-#ifdef CONFIG_USB_OTG
- /* during HNP, don't repeat the debounce */
- if (hdev->bus->is_b_host)
- portchange &= ~(USB_PORT_STAT_C_CONNECTION |
- USB_PORT_STAT_C_ENABLE);
-#endif
-
- /* Try to resuscitate an existing device */
- udev = hub->ports[port1 - 1]->child;
- if ((portstatus & USB_PORT_STAT_CONNECTION) && udev &&
- udev->state != USB_STATE_NOTATTACHED) {
- usb_lock_device(udev);
- if (portstatus & USB_PORT_STAT_ENABLE) {
- status = 0; /* Nothing to do */
-
-#ifdef CONFIG_PM_RUNTIME
- } else if (udev->state == USB_STATE_SUSPENDED &&
- udev->persist_enabled) {
- /* For a suspended device, treat this as a
-* remote wakeup event.
-*/
- status = usb_remote_wakeup(udev);
-#endif
-
- } else {
- status = -ENODEV; /* Don't resuscitate */
- }
- usb_unlock_device(udev);
-
- if (status == 0) {
- clear_bit(port1, hub->change_bits);
- return;
- }
- }
+ struct usb_port *port_dev = hub->ports[port1 - 1];
+ struct usb_device *udev = port_dev->child;
/* Disconnect any existing devices under this port */
if (udev) {
if (hcd->phy && !hdev->parent &&
!(portstatus & USB_PORT_STAT_CONNECTION))
usb_phy_notify_disconnect(hcd->phy, udev->speed);
- usb_disconnect(&hub->ports[port1 - 1]->child);
+ usb_disconnect(&port_dev->child);
}
- clear_bit(port1, hub->change_bits);
/* We can forget about a "removed" device when there's a physical
* disconnect or the connect status changes.
@@ -4565,7 +4513,7 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
if (hdev->state == USB_STATE_NOTATTACHED)
status = -ENOTCONN;
else
- hub->ports[port1 - 1]->child = udev;
+ port_dev->child = udev;
spin_unlock_irq(&device_state_lock);
/* Run it through the hoops (find a driver, etc) */
@@ -4573,7 +4521,7 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
status = usb_new
[PATCH v5 00/16] port power control rework
Toggling port power currently leads to three unintended disconnect
scenarios that are addressed by this rework of port power recovery and
usb device resume:
1/ Superspeed devices downgrade to their hispeed connection when rx-detection
fails on the superspeed pins. Address this by preventing superspeed port
poweroff/suspension until the peer port is suspended. This depends on the
ability to identify peer ports (patches 2-5), and then patch 6 implements the
policy.
2/ khubd prematurely disconnects ports that are in the process of being
resumed or reset. After this series khubd will ignore ports in the
pm-runtime-suspended state (patch 10) and holds a new port status lock
to synchronize the port status changes of usb_port_{suspend|resume}
(patch 11).
3/ Superspeed devices fail to reconnect after a 2 second timeout This
event has two causes:
3.1/ Repeated {Set|Clear}PortFeature(PORT_POWER) toggles caused the
device to switch to its hispeed connection (perceived
instability of the superspeed connection). Address this by
arranging for the child device to be woken up when the parent
port resumes. (patch 12)
3.2/ Devices may require a warm reset when recovering the power
session. When the child device is woken up per above and the
port timed out on reconnect, force a warm-reset during the
child's reset-resume (patch 13).
Changes since v4 [1]:
Lots of updates thanks to Alan's thorough review, much appreciated Alan!
Relative diffstat:
7 files changed, 452 insertions(+), 347 deletions(-)
* Deleted "xhci: cancel in-flight resume requests when the port is
powered off" (patch 10 in v4). The new usb_wakeup_notification()
closes the failure window my tests were hitting.
* Patch 1: "usb: disable port power control if not supported in
wHubCharacteristics"
* Added to prevent port runtime power management from being
enabled if not supported by the hub.
* Patch 2: "usb: assign default peer ports for root hubs"
* peer_lock now covers both finding and linking peers
* Made find_default_peer() independent of the order shared_hcd is
registered
* Undid the device_initialize/device_register split
* Patch 4: "usb: find internal hub tier mismatch via acpi"
* Rewrote the recursive walk through the USB topology to be more
readable and safer with a recursion limit
* Added detail to the changelog
* Patch 5: "usb: sysfs link peer ports"
* Clarified the changelog
* Killed "do if" usage
* Patch 6: "usb: defer suspension of superspeed port while peer is powered"
* Cleaned up how the USB2 port holds a pm runtime reference on behalf
of the USB3 port
* Patch 8: "usb: usb3 ports do not support FEAT_C_ENABLE"
* Clarified the changelog
* Patch 9: "usb: refactor port handling in hub_events()"
* Refactored hub_port_connect_change() to split off the bottom portion
into a new hub_port_reconnect() routine that can be called without
the port status lock held at entry.
* Patch 10: "usb: synchronize port poweroff and khubd"
* Moved pm runtime synchronization outside of port_event() to prevent
needing to re-read portstatus
* Patch 11: "usb: introduce port status lock"
* Clarified the changelog
* Pushed usb_lock_device() into usb_remote_wakeup()
* Sparse annotations
* Take usb_lock_port() around usb_reset_and_verify_device()
* Killed hub->busy_bits
* Killed hub_port_connect_change_unlock() in favor of introducing
hub_port_reconnect() called without the port lock held.
* Patch 12: "usb: resume (wakeup) child device when port is powered on"
* Clarified the changelog
* Repurposed usb_wakeup_notification() and wakeup_change for handling
these child device resume requests.
* Patch 13: "usb: force warm reset to break link re-connect livelock"
* Clarified the changelog
* Made the warm-reset request unconditional. If re-connect times out a
warm-reset is the last stop before force disconnecting the device.
* Patch 14: "usb: documentation for usb port power off mechanisms"
* Clarified that port power control will be disabled when the hub does
not advertise port power control capabilities.
* Patch 15 / 16 (New and optional)
* This is a RFC implementation of the mechanism Alan and I discussed to
close the window of rpm_suspend() event occurring while we are still
resolving tier mismatch [2]. RFC-only due to the locking horror in
hub_quiesce() this causes.
[1]: v4: http://marc.info/?l=linux-usb&m=139121878519367&w=2
[2]: http://marc.info/?l=linux-usb&m=139180783415065&w=2
---
[PATCH v5 01/16] usb: disable port power control if not supported in
wHubCharacteristics
[PATCH v5 02/16] usb: assign default peer ports for root hubs
[PATCH v5 03/16] usb: assign usb3 external hub port peers
[PATCH v5 04/16] usb: find internal hub tier mismatch via acpi
[PATCH v5 05/16] usb: sysfs link peer ports
[PATCH v5 06/16] usb: defer suspension of superspeed port wh
[PATCH v5 10/16] usb: synchronize port poweroff and khubd
If a port is powered-off, or in the process of being powered-off, prevent
khubd from operating on it. Otherwise, the following sequence of events
leading to an unintended disconnect may occur:
Events:
(0)
(1) hub 2-2:1.0: hub_resume
(2) hub 2-2:1.0: port 1: status 0301 change
(3) hub 2-2:1.0: state 7 ports 4 chg 0002 evt
(4) hub 2-2:1.0: port 1, power off status , change , 12 Mb/s
(5) usb 2-2.1: USB disconnect, device number 5
Description:
(1) hub is resumed before sending a ClearPortFeature request
(2) hub_activate() notices the port is connected and sets
hub->change_bits for the port
(3) hub_events() starts, but at the same time the port suspends
(4) hub_connect_change() sees the disabled port and triggers disconnect
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 62 +---
1 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d4fab7caaf40..bb6ebde85192 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4736,33 +4736,39 @@ static void port_event(struct usb_hub *hub, int port1)
USB_PORT_FEAT_C_PORT_CONFIG_ERROR);
}
- if (hub_handle_remote_wakeup(hub, port1, portstatus, portchange))
- connect_change = 1;
-
- /*
-* Warm reset a USB3 protocol port if it's in
-* SS.Inactive state.
-*/
- if (hub_port_warm_reset_required(hub, portstatus)) {
- int status;
+ /* take port actions that require the port to be powered on */
+ if (pm_runtime_active(&port_dev->dev)) {
+ if (hub_handle_remote_wakeup(hub, port1,
+ portstatus, portchange))
+ connect_change = 1;
- dev_dbg(hub_dev, "port%d: do warm reset\n", port1);
- if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION)
- || udev->state == USB_STATE_NOTATTACHED) {
- status = hub_port_reset(hub, port1, NULL,
- HUB_BH_RESET_TIME, true);
- if (status < 0)
- hub_port_disable(hub, port1, 1);
- } else {
- usb_lock_device(udev);
- status = usb_reset_device(udev);
- usb_unlock_device(udev);
- connect_change = 0;
+ /*
+* Warm reset a USB3 protocol port if it's in
+* SS.Inactive state.
+*/
+ if (hub_port_warm_reset_required(hub, portstatus)) {
+ int status;
+
+ dev_dbg(hub_dev, "port%d: do warm reset\n", port1);
+ if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION)
+ || udev->state ==
USB_STATE_NOTATTACHED) {
+ status = hub_port_reset(hub, port1, NULL,
+ HUB_BH_RESET_TIME,
+ true);
+ if (status < 0)
+ hub_port_disable(hub, port1, 1);
+ } else {
+ usb_lock_device(udev);
+ status = usb_reset_device(udev);
+ usb_unlock_device(udev);
+ connect_change = 0;
+ }
}
- }
- if (connect_change)
- hub_port_connect_change(hub, port1, portstatus, portchange);
+ if (connect_change)
+ hub_port_connect_change(hub, port1,
+ portstatus, portchange);
+ }
}
@@ -4849,11 +4855,17 @@ static void hub_events(void)
/* deal with port status changes */
for (i = 1; i <= hdev->maxchild; i++) {
+ struct usb_port *port_dev = hub->ports[i - 1];
+
if (!test_bit(i, hub->busy_bits)
&& (test_and_clear_bit(i,
hub->event_bits)
|| test_bit(i, hub->change_bits)
- || test_bit(i, hub->wakeup_bits)))
+ || test_bit(i, hub->wakeup_bits))) {
+ pm_runtime_get_noresume(&port_dev->dev);
+ pm_runtime_barrier(&port_dev->dev);
port_event(hub, i);
+ pm_runtime_put_sync(&port_dev->dev);
+ }
}
/* deal with hub status changes */
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kern
[PATCH v5 15/16] usb: convert khubd to a workqueue
Both a cleanup, as khubd open codes several facilities that are provided
by workqueue, and an enabling step for flushing initial port discovery
operations. A do { } while (0) loop in hub_event() is used to minimize
code thrash.
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 146
drivers/usb/core/hub.h |2 -
2 files changed, 51 insertions(+), 97 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3c109924d2ae..e0518af66af9 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -22,11 +22,10 @@
#include
#include
#include
-#include
#include
-#include
#include
#include
+#include
#include
#include
@@ -41,14 +40,10 @@
* change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
static DEFINE_SPINLOCK(device_state_lock);
-/* khubd's worklist and its lock */
+/* sync event queueing vs hub disconnects */
static DEFINE_SPINLOCK(hub_event_lock);
-static LIST_HEAD(hub_event_list); /* List of hubs needing servicing */
-/* Wakes up khubd */
-static DECLARE_WAIT_QUEUE_HEAD(khubd_wait);
-
-static struct task_struct *khubd_task;
+static struct workqueue_struct *khubd_wq;
/* cycle leds on hubs that aren't blinking for attention */
static bool blinkenlights = 0;
@@ -559,18 +554,28 @@ static int hub_port_status(struct usb_hub *hub, int port1,
return ret;
}
+static void hub_release(struct kref *kref)
+{
+ struct usb_hub *hub = container_of(kref, struct usb_hub, kref);
+
+ usb_put_intf(to_usb_interface(hub->intfdev));
+ kfree(hub);
+}
+
static void kick_khubd(struct usb_hub *hub)
{
- unsigned long flags;
+ struct usb_interface *intf = to_usb_interface(hub->intfdev);
+ unsigned long flags;
+ /* Suppress autosuspend until khubd runs */
spin_lock_irqsave(&hub_event_lock, flags);
- if (!hub->disconnected && list_empty(&hub->event_list)) {
- list_add_tail(&hub->event_list, &hub_event_list);
-
- /* Suppress autosuspend until khubd runs */
- usb_autopm_get_interface_no_resume(
- to_usb_interface(hub->intfdev));
- wake_up(&khubd_wait);
+ if (!hub->disconnected) {
+ usb_autopm_get_interface_no_resume(intf);
+ kref_get(&hub->kref);
+ if (!queue_work(khubd_wq, &hub->event_work)) {
+ usb_autopm_put_interface_async(intf);
+ kref_put(&hub->kref, hub_release);
+ }
}
spin_unlock_irqrestore(&hub_event_lock, flags);
}
@@ -1584,14 +1589,6 @@ fail_keep_maxchild:
return ret;
}
-static void hub_release(struct kref *kref)
-{
- struct usb_hub *hub = container_of(kref, struct usb_hub, kref);
-
- usb_put_intf(to_usb_interface(hub->intfdev));
- kfree(hub);
-}
-
static unsigned highspeed_hubs;
static void hub_disconnect(struct usb_interface *intf)
@@ -1600,13 +1597,13 @@ static void hub_disconnect(struct usb_interface *intf)
struct usb_device *hdev = interface_to_usbdev(intf);
int port1;
- /* Take the hub off the event list and don't let it be added again */
- spin_lock_irq(&hub_event_lock);
- if (!list_empty(&hub->event_list)) {
- list_del_init(&hub->event_list);
- usb_autopm_put_interface_no_suspend(intf);
- }
+ /*
+* Disable hub event processing, note that we can't flush the work
+* since we may be holding a device lock that khubd wants to acquire
+* (lockdep is prevented from flagging this)
+*/
hub->disconnected = 1;
+ spin_lock_irq(&hub_event_lock);
spin_unlock_irq(&hub_event_lock);
/* Disconnect all children and quiesce the hub */
@@ -1636,6 +1633,8 @@ static void hub_disconnect(struct usb_interface *intf)
kref_put(&hub->kref, hub_release);
}
+static void hub_event(struct work_struct *);
+
static int hub_probe(struct usb_interface *intf, const struct usb_device_id
*id)
{
struct usb_host_interface *desc;
@@ -1726,7 +1725,7 @@ descriptor_error:
}
kref_init(&hub->kref);
- INIT_LIST_HEAD(&hub->event_list);
+ INIT_WORK(&hub->event_work, hub_event);
hub->intfdev = &intf->dev;
hub->hdev = hdev;
INIT_DELAYED_WORK(&hub->leds, led_work);
@@ -4828,42 +4827,16 @@ static void port_event(struct usb_hub *hub, int port1)
}
-static void hub_events(void)
+static void hub_event(struct work_struct *w)
{
- struct list_head *tmp;
- struct usb_device *hdev;
- struct usb_interface *intf;
- struct usb_hub *hub;
- struct device *hub_dev;
- u16 hubstatus;
- u16 hubchange;
+ struct usb_hub *hub = container_of(w, struct usb_hub, event_work);
int i, ret;
+ u16 hubstatus, hubchange;
+ struct usb_device *hdev = hub->hdev;
+
[PATCH v5 13/16] usb: force warm reset to break link re-connect livelock
Resuming a powered down port sometimes results in the port state being
stuck in the training sequence.
hub 3-0:1.0: debounce: port 1: total 2000ms stable 0ms status 0x2e0
port1: can't get reconnection after setting port power on, status -110
hub 3-0:1.0: port 1 status .02e0 after resume, -19
usb 3-1: can't resume, status -19
hub 3-0:1.0: logical disconnect on port 1
In the case above we wait for the port re-connect timeout of 2 seconds
and observe that the port status is USB_SS_PORT_LS_POLLING (although it
is likely toggling between this state and USB_SS_PORT_LS_RX_DETECT).
This is indicative of a case where the device is failing to progress the
link training state machine.
It is resolved by issuing a warm reset to get the hub and device link
state machines back in sync.
hub 3-0:1.0: debounce: port 1: total 2000ms stable 0ms status 0x2e0
usb usb3: port1 usb_port_runtime_resume requires warm reset
hub 3-0:1.0: port 1 not warm reset yet, waiting 50ms
usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd
After a reconnect timeout when we expect the device to be present, force
a warm reset of the device. Note that we can not simply look at the
link status to determine if a warm reset is required as any of the
training states USB_SS_PORT_LS_POLLING, USB_SS_PORT_LS_RX_DETECT, or
USB_SS_PORT_LS_COMP_MOD are valid states that do not indicate the need
for warm reset by themselves.
Cc: Julius Werner
Cc: Alan Stern
Cc: Vikas Sajjan
Cc: Kukjin Kim
Cc: Vincent Palatin
Cc: Lan Tianyu
Cc: Ksenia Ragiadakou
Cc: Vivek Gautam
Cc: Douglas Anderson
Cc: Felipe Balbi
Cc: Sunil Joshi
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 23 ---
drivers/usb/core/hub.h |2 ++
drivers/usb/core/port.c | 22 ++
3 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 1aa1f14bbe22..3c109924d2ae 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2514,10 +2514,12 @@ static int hub_port_reset(struct usb_hub *hub, int
port1,
/* Is a USB 3.0 port in the Inactive or Compliance Mode state?
* Port worm reset is required to recover
*/
-static bool hub_port_warm_reset_required(struct usb_hub *hub, u16 portstatus)
+static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1,
+u16 portstatus)
{
return hub_is_superspeed(hub->hdev) &&
- (((portstatus & USB_PORT_STAT_LINK_STATE) ==
+ (test_bit(port1, hub->warm_reset_bits) ||
+ ((portstatus & USB_PORT_STAT_LINK_STATE) ==
USB_SS_PORT_LS_SS_INACTIVE) ||
((portstatus & USB_PORT_STAT_LINK_STATE) ==
USB_SS_PORT_LS_COMP_MOD)) ;
@@ -2557,7 +2559,7 @@ static int hub_port_wait_reset(struct usb_hub *hub, int
port1,
if ((portstatus & USB_PORT_STAT_RESET))
return -EBUSY;
- if (hub_port_warm_reset_required(hub, portstatus))
+ if (hub_port_warm_reset_required(hub, port1, portstatus))
return -ENOTCONN;
/* Device went away? */
@@ -2656,8 +2658,9 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
if (status < 0)
goto done;
- if (hub_port_warm_reset_required(hub, portstatus))
+ if (hub_port_warm_reset_required(hub, port1, portstatus))
warm = true;
+ clear_bit(port1, hub->warm_reset_bits);
}
/* Reset the port */
@@ -2695,7 +2698,8 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
&portstatus, &portchange) < 0)
goto done;
- if (!hub_port_warm_reset_required(hub, portstatus))
+ if (!hub_port_warm_reset_required(hub, port1,
+ portstatus))
goto done;
/*
@@ -2782,8 +2786,13 @@ static int check_port_resume_type(struct usb_device
*udev,
struct usb_hub *hub, int port1,
int status, unsigned portchange, unsigned portstatus)
{
+ /* Is a warm reset needed to recover the connection? */
+ if (udev->reset_resume
+ && hub_port_warm_reset_required(hub, port1, portstatus)) {
+ /* pass */;
+ }
/* Is the device still present? */
- if (status || port_is_suspended(hub, portstatus) ||
+ else if (status || port_is_suspended(hub, portstatus) ||
!port_is_power_on(hub, portstatus) ||
!(portstatus & USB_PORT_STAT_CONNECTION)) {
if (status >= 0)
@@ -4791,7 +4800,7 @@ static void port_event(struct usb_hub *hub, int port1)
* Warm reset a USB3 protocol port if it's in
* SS.Inactive state.
*/
[PATCH v5 11/16] usb: introduce port status lock
In general we do not want khubd to act on port status changes that are
the result of in progress resets or USB runtime PM operations.
Specifically port power control testing has been able to trigger an
unintended disconnect in hub_port_connect_change(), paraphrasing:
if ((portstatus & USB_PORT_STAT_CONNECTION) && udev &&
udev->state != USB_STATE_NOTATTACHED) {
if (portstatus & USB_PORT_STAT_ENABLE) {
/* Nothing to do */
} else if (udev->state == USB_STATE_SUSPENDED &&
udev->persist_enabled) {
...
} else {
/* Don't resuscitate */;
}
}
...by falling to the "Don't resuscitate" path or missing
USB_PORT_STAT_CONNECTION because usb_port_resume() was in the middle of
modifying the port status.
So, we want a new lock to hold off khubd for a given port while the
child device is being suspended, resumed, or reset. The lock ordering
rules are now usb_lock_device() => usb_lock_port(). This is mandated by
the device core which may hold the device_lock on the usb_device before
invoking usb_port_{suspend|resume} which in turn take the status_lock on
the usb_port. We attempt to hold the status_lock for the duration of a
port_event() run, and drop/re-acquire it when needing to take the
device_lock. The lock is also dropped/re-acquired during
hub_port_reconnect().
This patch also deletes hub->busy_bits as all use cases are now covered
by port PM runtime synchronization or the port->status_lock.
Signed-off-by: Dan Williams
---
drivers/usb/core/hcd.c |2 -
drivers/usb/core/hub.c | 102 +--
drivers/usb/core/hub.h |4 +-
drivers/usb/core/port.c |7 +--
drivers/usb/core/usb.h |6 +++
5 files changed, 82 insertions(+), 39 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 2518c3250750..b3ecaf32f2aa 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2246,9 +2246,7 @@ static void hcd_resume_work(struct work_struct *work)
struct usb_hcd *hcd = container_of(work, struct usb_hcd, wakeup_work);
struct usb_device *udev = hcd->self.root_hub;
- usb_lock_device(udev);
usb_remote_wakeup(udev);
- usb_unlock_device(udev);
}
/**
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bb6ebde85192..5b4a5e7b3762 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2737,6 +2737,20 @@ static int port_is_power_on(struct usb_hub *hub,
unsigned portstatus)
return ret;
}
+static void usb_lock_port(struct usb_port *port_dev)
+ __acquires(&port_dev->status_lock)
+{
+ mutex_lock(&port_dev->status_lock);
+ __acquire(&port_dev->status_lock);
+}
+
+static void usb_unlock_port(struct usb_port *port_dev)
+ __releases(&port_dev->status_lock)
+{
+ mutex_unlock(&port_dev->status_lock);
+ __release(&port_dev->status_lock);
+}
+
#ifdef CONFIG_PM
/* Check if a port is suspended(USB2.0 port) or in U3 state(USB3.0 port) */
@@ -2958,6 +2972,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
int status;
boolreally_suspend = true;
+ usb_lock_port(port_dev);
+
/* enable remote wakeup when appropriate; this lets the device
* wake up the upstream hub (including maybe the root hub).
*
@@ -3053,6 +3069,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
}
usb_mark_last_busy(hub->hdev);
+
+ usb_unlock_port(port_dev);
return status;
}
@@ -3192,6 +3210,8 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
}
}
+ usb_lock_port(port_dev);
+
/* Skip the initial Clear-Suspend step for a remote wakeup */
status = hub_port_status(hub, port1, &portstatus, &portchange);
if (status == 0 && !port_is_suspended(hub, portstatus))
@@ -3199,8 +3219,6 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
/* dev_dbg(hub->intfdev, "resume port %d\n", port1); */
- set_bit(port1, hub->busy_bits);
-
/* see 7.1.7.7; affects power usage, but not budgeting */
if (hub_is_superspeed(hub->hdev))
status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U0);
@@ -3240,8 +3258,6 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
}
}
- clear_bit(port1, hub->busy_bits);
-
status = check_port_resume_type(udev,
hub, port1, status, portchange, portstatus);
if (status == 0)
@@ -3259,16 +3275,18 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
usb_unlocked_enable_lpm(udev);
}
+ usb_unlock_port(port_dev);
+
return status;
}
#ifdef CONFIG_PM_RUN
[PATCH] phy-rcar-gen2-usb: always use 'dev' variable in probe() method
The probe() method has the 'dev' local variable declared and used but strangely
not in all cases where it should be...
Signed-off-by: Sergei Shtylyov
---
This patch is against the 'next' branch of Felipe Balbi's 'usb.git' repo.
drivers/usb/phy/phy-rcar-gen2-usb.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Index: usb/drivers/usb/phy/phy-rcar-gen2-usb.c
===
--- usb.orig/drivers/usb/phy/phy-rcar-gen2-usb.c
+++ usb/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -177,15 +177,15 @@ static int rcar_gen2_usb_phy_probe(struc
struct clk *clk;
int retval;
- pdata = dev_get_platdata(&pdev->dev);
+ pdata = dev_get_platdata(dev);
if (!pdata) {
dev_err(dev, "No platform data\n");
return -EINVAL;
}
- clk = devm_clk_get(&pdev->dev, "usbhs");
+ clk = devm_clk_get(dev, "usbhs");
if (IS_ERR(clk)) {
- dev_err(&pdev->dev, "Can't get the clock\n");
+ dev_err(dev, "Can't get the clock\n");
return PTR_ERR(clk);
}
--
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
sisusb: Use static const, fix typo
Reduce text a bit by using static const.
Fix a symmetric typo and neaten a dev_info call
to 80 columns.
$ size drivers/usb/misc/sisusbvga/sisusb.o*
textdata bss dec hex filename
3001648419180 44037ac05 drivers/usb/misc/sisusbvga/sisusb.o.new
3008748419180 44108ac4c drivers/usb/misc/sisusbvga/sisusb.o.old
Signed-off-by: Joe Perches
---
drivers/usb/misc/sisusbvga/sisusb.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/misc/sisusbvga/sisusb.c
b/drivers/usb/misc/sisusbvga/sisusb.c
index de98906..1f89633 100644
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -2123,8 +2123,9 @@ sisusb_get_ramconfig(struct sisusb_usb_data *sisusb)
u8 tmp8, tmp82, ramtype;
int bw = 0;
char *ramtypetext1 = NULL;
- const char *ramtypetext2[] = { "SDR SDRAM", "SDR SGRAM",
- "DDR SDRAM", "DDR SGRAM" };
+ static const char *ramtypetext2[] = {
+ "SDR SDRAM", "SDR SGRAM", "DDR SDRAM", "DDR SGRAM"
+ };
static const int busSDR[4] = {64, 64, 128, 128};
static const int busDDR[4] = {32, 32, 64, 64};
static const int busDDRA[4] = {64+32, 64+32 , (64+32)*2, (64+32)*2};
@@ -2146,7 +2147,7 @@ sisusb_get_ramconfig(struct sisusb_usb_data *sisusb)
sisusb->vramsize <<= 1;
bw = busSDR[(tmp8 & 0x03)];
break;
- case 2: ramtypetext1 = "asymmeric";
+ case 2: ramtypetext1 = "asymmetric";
sisusb->vramsize += sisusb->vramsize/2;
bw = busDDRA[(tmp8 & 0x03)];
break;
@@ -2156,8 +2157,9 @@ sisusb_get_ramconfig(struct sisusb_usb_data *sisusb)
break;
}
- dev_info(&sisusb->sisusb_dev->dev, "%dMB %s %s, bus width %d\n",
(sisusb->vramsize >> 20), ramtypetext1,
- ramtypetext2[ramtype], bw);
+ dev_info(&sisusb->sisusb_dev->dev, "%dMB %s %s, bus width %d\n",
+sisusb->vramsize >> 20, ramtypetext1, ramtypetext2[ramtype],
+bw);
}
static int
--
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 4/7] ECHI Platform: Merge ppc-of EHCI driver into the ehci-platform driver
On 22/02/14 00:48, Mark Rutland wrote: [Adding Tony Prisk to Cc] On Fri, Feb 21, 2014 at 06:31:30AM +, Alistair Popple wrote: Currently the ppc-of driver uses the compatibility string "usb-ehci". This means platforms that use device-tree and implement an EHCI compatible interface have to either use the ppc-of driver or add a compatible line to the ehci-platform driver. It would be more appropriate for the platform driver to be compatible with "usb-ehci" as non-powerpc platforms are also beginning to utilise device-tree. This patch merges the device tree property parsing from ehci-ppc-of into the platform driver and adds a "usb-ehci" compatibility string. The existing ehci-ppc-of driver is removed and the 440EPX specific quirks are added to the ehci-platform driver. Signed-off-by: Alistair Popple --- drivers/usb/host/Kconfig |7 +- drivers/usb/host/ehci-hcd.c |5 - drivers/usb/host/ehci-platform.c | 87 +- drivers/usb/host/ehci-ppc-of.c | 238 -- 4 files changed, 89 insertions(+), 248 deletions(-) delete mode 100644 drivers/usb/host/ehci-ppc-of.c Please use of_property_read_bool for these. This driver already handles "via,vt8500-ehci" and "wm,prizm-ehci" which aren't documented to handle these properties, but now gain support for them. It might be worth unifying the binding documents if there's nothing special about those two host controllers. We seem to have two binding documents for "via,vt8500-ehci", so some cleanup is definitely in order. Tony, you seem to have written both documents judging by 95e9fd10f06c and 8ad551d150e3. Do you have any issue with merging both of these into a common usb-ehci document? .. Cheers, Mark. I'm not sure how we ended up with two bindings for the driver anyway. I think this was an error on my part somewhere. None of the in-tree dts files use "wm,prizm-ehci". I have no issue with merging all the documentation into a single usb-ehci binding. Regards Tony Prisk -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB kernel oops
On Fri, Feb 21, 2014 at 02:30:18PM -0700, Nicholas Leippe wrote: > $ uname -a > Linux hellcat 3.12.9-gentoo #1 SMP PREEMPT Mon Jan 27 08:32:22 MST 2014 > x86_64 Intel(R) Core(TM) i7-3770 CPU @ 3.40GHz GenuineIntel GNU/Linux > > > distro: gentoo > kernel: sys-kernel/gentoo-sources-3.12.9 > > oops: > > > [531193.073318] usb 4-4.3.3: Failed to set U1 timeout to 0x0,error code -71 > [531193.073326] BUG: unable to handle kernel NULL pointer dereference at > 0010 > [531193.073772] IP: [] usb_enable_link_state+0x38/0x350 > [531193.074149] PGD 60ee9b067 PUD 5cc3b6067 PMD 0 > [531193.074640] Oops: [#1] PREEMPT SMP > [531193.075126] Modules linked in: snd_pcm_oss snd_mixer_oss > snd_hda_codec_hdmi snd_hda_codec_realtek nvidia(PO) fglrx(PO) snd_hda_intel > x86_pkg_temp_thermal snd_hda_codec coretemp snd_hwdep snd_pcm snd_page_alloc > e1000e snd_timer ptp snd pps_core > [531193.077140] CPU: 0 PID: 25835 Comm: usb-storage Tainted: P O > 3.12.9-gentoo #1 Both the nvidia and AMD closed kernel drivers loaded? You are brave, and on your own :( Can you duplicate this on 3.13 without any closed drivers loaded? There have been a number of lpr fixes recently that should help with this. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] u_ether: move hardware transmit to RX workqueue
In order to reduce the interrupt times in the embedded system,
a receiving workqueue is introduced.
This modification also enhanced the overall throughput as the
benefits of reducing interrupt occurrence.
Signed-off-by: Clanlab (Taiwan) Linux Project
Cc: David Brownell
Cc: David S. Miller
Cc: Stephen Hemminger
Cc: Felipe Balbi
Cc: Greg Kroah-Hartman
---
Changes for v2:
- Remove the whitespace trailer.
- Reorganize the setup/destroy gether_wq work queue procedure
into APIs gether_setup and gether_cleanup
drivers/usb/gadget/u_ether.c | 111 +--
1 file changed, 76 insertions(+), 35 deletions(-)
diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
index b7d4f82..506f16d 100644
--- a/drivers/usb/gadget/u_ether.c
+++ b/drivers/usb/gadget/u_ether.c
@@ -72,6 +72,7 @@ struct eth_dev {
struct sk_buff_head *list);
struct work_struct work;
+ struct work_struct rx_work;
unsigned long todo;
#defineWORK_RX_MEMORY 0
@@ -81,6 +82,8 @@ struct eth_dev {
u8 dev_mac[ETH_ALEN];
};
+static struct workqueue_struct *gether_wq;
+
/*-*/
#define RX_EXTRA 20 /* bytes guarding against rx overflows */
@@ -253,18 +256,16 @@ enomem:
DBG(dev, "rx submit --> %d\n", retval);
if (skb)
dev_kfree_skb_any(skb);
- spin_lock_irqsave(&dev->req_lock, flags);
- list_add(&req->list, &dev->rx_reqs);
- spin_unlock_irqrestore(&dev->req_lock, flags);
}
return retval;
}
static void rx_complete(struct usb_ep *ep, struct usb_request *req)
{
- struct sk_buff *skb = req->context, *skb2;
+ struct sk_buff *skb = req->context;
struct eth_dev *dev = ep->driver_data;
int status = req->status;
+ boolrx_queue = 0;
switch (status) {
@@ -288,30 +289,8 @@ static void rx_complete(struct usb_ep *ep, struct
usb_request *req)
} else {
skb_queue_tail(&dev->rx_frames, skb);
}
- skb = NULL;
-
- skb2 = skb_dequeue(&dev->rx_frames);
- while (skb2) {
- if (status < 0
- || ETH_HLEN > skb2->len
- || skb2->len > VLAN_ETH_FRAME_LEN) {
- dev->net->stats.rx_errors++;
- dev->net->stats.rx_length_errors++;
- DBG(dev, "rx length %d\n", skb2->len);
- dev_kfree_skb_any(skb2);
- goto next_frame;
- }
- skb2->protocol = eth_type_trans(skb2, dev->net);
- dev->net->stats.rx_packets++;
- dev->net->stats.rx_bytes += skb2->len;
-
- /* no buffer copies needed, unless hardware can't
-* use skb buffers.
-*/
- status = netif_rx(skb2);
-next_frame:
- skb2 = skb_dequeue(&dev->rx_frames);
- }
+ if (!status)
+ rx_queue = 1;
break;
/* software-driven interface shutdown */
@@ -334,22 +313,20 @@ quiesce:
/* FALLTHROUGH */
default:
+ rx_queue = 1;
+ dev_kfree_skb_any(skb);
dev->net->stats.rx_errors++;
DBG(dev, "rx status %d\n", status);
break;
}
- if (skb)
- dev_kfree_skb_any(skb);
- if (!netif_running(dev->net)) {
clean:
spin_lock(&dev->req_lock);
list_add(&req->list, &dev->rx_reqs);
spin_unlock(&dev->req_lock);
- req = NULL;
- }
- if (req)
- rx_submit(dev, req, GFP_ATOMIC);
+
+ if (rx_queue)
+ queue_work(gether_wq, &dev->rx_work);
}
static int prealloc(struct list_head *list, struct usb_ep *ep, unsigned n)
@@ -414,16 +391,24 @@ static void rx_fill(struct eth_dev *dev, gfp_t gfp_flags)
{
struct usb_request *req;
unsigned long flags;
+ int rx_counts = 0;
/* fill unused rxq slots with some skb */
spin_lock_irqsave(&dev->req_lock, flags);
while (!list_empty(&dev->rx_reqs)) {
+
+ if (++rx_counts > qlen(dev->gadget, dev->qmult))
+ break;
+
req = container_of(dev->rx_reqs.next,
struct usb_request, list);
list_del_init(&req->list);
spin_unlock_irqrestore(&dev->req_lo
Re: [PATCH v2] u_ether: move hardware transmit to RX workqueue
On Sat, Feb 22, 2014 at 01:41:52PM +0800, Clanlab (Taiwan) Linux Project wrote: > In order to reduce the interrupt times in the embedded system, > a receiving workqueue is introduced. > This modification also enhanced the overall throughput as the > benefits of reducing interrupt occurrence. > > Signed-off-by: Clanlab (Taiwan) Linux Project You need a person to have a "From" and a "signed-off-by:", not a "project", sorry, that doesn't work at all and means the patch must be rejected. Please use a real name for kernel development, it is required by what you are agreeing to when you use the line "Signed-off-by:", didn't you read it? greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
