Re: gadgetfs USB2.0 Chapter 9 Tests: Test after "Addressed State" fails
Thank You Michal, I am testing now the unchanged usb.c example (www.linux-usb.org/gadget/usb.c) as user space application on my ARM Cortex A5 with atmel_usba_udc driver. When I run the Command Verifier, all Tests up to "Interface Descriptor Test" are passed. "Endpoint Descriptor Test - Configured State" fails while the application usb.c does not give output til the Test "Suspend/Resume" Test. This is the output during "Suspend/Resume" Test: fd 5, unclaimed = 0 fd 4, unclaimed = 0 reset source fd: No such device reset sink fd: Bad file descriptor ... protocol stall 01.0b ep0 stall: Identifier removed DISCONNECT CONNECT high speed SETUP bReqType 00 bReq 09 value 0003 index length 0 CONFIG #3 simple_sink_thread start -1234979728 fd 4 simple_source_thread start -1226591120 fd 5 The following tests "Remote Wakeup Test - Enabled" and "Remote Wakeup Test - Disabled" both are passed fine as are all the others. But when I repeat the tests, at some point, I always get simple_sink_thread open ep2 error 16 (Device or resource busy) simple_source_thread open ep1 error 16 (Device or resource busy) while the Tests are still marked as "passed" excpept for the "Halt Endpoint Test". That one results in udc: ep0: Invalid setup request: 82.00 v i0081 l2, halting endpoint... udc: ep0: Invalid setup request: 02.01 v i0081 l0, halting endpoint... on the serial console output. I thought the usb.c example was user space application that would be "conform" so the Command Verifier Tests would all pass? Where Is my mistake here? Thanks in advance for any hints! I'm really stuck on this issue since I don't really know the origin of the failure. On Fri, Jan 3, 2014 at 4:32 PM, Michal Nazarewicz wrote: > On Fri, Jan 03 2014, Marco Zamponi wrote: >> Thank you Roshan for the patch and thank you Michal for the discussion! >> Appearently I am not as familiar with USB as you guys are. But what I >> understood from the discussion is, that if I apply the patch from Roshan to >> f_fs.c and composite.c, the CV Test should not fail? > > That may be true, but it will change the behaviour of f_fs.c and thus > may fail existing user space code, so it's better to actually fix user > space code that uses f_fs.c with incorrect assumptions. > > -- > Best regards, _ _ > .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o > ..o | Computer Science, Michał “mina86” Nazarewicz(o o) > ooo +--ooO--(_)--Ooo-- > > -- To unsubscribe from this list: send the line "unsubscribe 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: Allow to blacklist xhci_hcd module and use ports with EHCI
On Mon, Dec 23, 2013 at 11:21 AM, Holger Hans Peter Freyther wrote: > From: Holger Hans Peter Freyther > > xhci_hcd does not work with the Canon Lide scanners and has issues > with the suspend/resume handling. My Acer Aspire S5 notebook only > exposes USB3.0 ports and the distribution kernels generally have > xhci_hcd enabled and I don't have any USB3.0 hardware either. Hi Holger, Is there another thread somewhere that details the issues this device has with xhci? -- To unsubscribe from this list: send the line "unsubscribe 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: Bug#733826: [PATCH] xhci: Set scatter-gather limit to avoid failed block writes.
It only happens to me once in a full moon too. Not something I can reproduce at will. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] usb: chipidea: need to mask INT_STATUS when write otgsc
For otgsc, both enable bits and status bits are in it. So we need
to make sure the status bits are not be cleared when write enable
bits. It can fix one bug that we plug in/out Micro AB cable fast,
and sometimes, the IDIS will be cleared wrongly when handle last
ID interrupt (ID 0->1), so the current interrupt will not occur.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/otg.h |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 2d9f090..449bee0 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ * Copyright (C) 2013-2014 Freescale Semiconductor, Inc.
*
* Author: Peter Chen
*
@@ -19,12 +19,12 @@ static inline void ci_clear_otg_interrupt(struct ci_hdrc
*ci, u32 bits)
static inline void ci_enable_otg_interrupt(struct ci_hdrc *ci, u32 bits)
{
- hw_write(ci, OP_OTGSC, bits, bits);
+ hw_write(ci, OP_OTGSC, bits | OTGSC_INT_STATUS_BITS, bits);
}
static inline void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits)
{
- hw_write(ci, OP_OTGSC, bits, 0);
+ hw_write(ci, OP_OTGSC, bits | OTGSC_INT_STATUS_BITS, 0);
}
int ci_hdrc_otg_init(struct ci_hdrc *ci);
--
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] RFC: Allow to blacklist xhci_hcd module and use ports with EHCI
On Tue, Jan 07, 2014 at 12:19:58AM -0800, Dan Williams wrote: > Hi Holger, > > Is there another thread somewhere that details the issues this device > has with xhci? The initial report from 2012 is here[1]. I asked again[2] in May 2013 and now in December[3]. From time to time I have issues with immediate resume as well but the mail has not been answered[4]. I looked at the FreeBSD driver for xhci and saw that on for intel PCI implementations I can change the routing of the ports and cooked up the RFC patch to do that. My goal is to do the paper work of my company using my main machine and using EHCI for that is good enough. At the same time I would like to have the perspective of being able to use the distribution kernel again (hence blacklisting, boot options, whatever makes it work). cheers holger [1] http://comments.gmane.org/gmane.linux.usb.general/72749 [2] http://www.spinics.net/lists/linux-usb/msg86615.html [3] http://www.spinics.net/lists/linux-usb/msg99787.html [4] https://lkml.org/lkml/2013/12/21/84 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb/core: fix NULL pointer dereference in recursively_mark_NOTATTACHED
> > Changbin, after looking more closely I realized there was a second > > aspect to this race: recursively_mark_NOTATTACHED uses hub->ports[i] > > while hub_disconnect removes the port devices. You ought to be able > > to cause an oops by inserting a delay just after the loop where > > usb_hub_remove_port_device is called. > > > > The updated patch below should fix both problems. Can you test it? > > > > Alan Stern > > > > Ok, I'll test it today or tomorrow. Please wait my response. Alan, I cannot cause a panic after inserting a delay just after usb_hub_remove_port_device is called, even move the delay after kfree(hub->ports). recursively_mark_NOTATTACHED will not access hub->ports[i] since maxchild has been set to 0. Alan, I think your last patch can fix this issue. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] xhci: Allocate the td array and urb_priv together.
> From: 'David Cohen' > On Mon, Jan 06, 2014 at 09:26:20AM +, David Laight wrote: > > > From: David Cohen > > > On Fri, Dec 20, 2013 at 09:26:35AM -, David Laight wrote: > > > > > From: David Cohen > > > > The effect of this change is really to remove the first allocation and > > > > add 8 bytes (or maybe a pointer) to the start of the second one. > > > > So it is extremely unlikely to fail when the old code would work. > > > > > > Currently struct urb_priv has a dynamic array of pointers to struct > > > xhci_td. You're replacing the pointer by structs itself. Now, instead of > > > 2 kmallocs() (1 for urb_priv and another for size * xhci_td) we've 1 > > > kmalloc() with urb_priv + size * xhci_td. > > > > You misread the old code. > > The first malloc was for the header and 'n' pointers. > > The second malloc was for 'n' copies of the structure. > > That's exactly what I described but in a more complicated way :) > > The new kmalloc is going to be "n * sizeof(struct) - n * sizeof(pointer)" > bigger. I don't know what is the usual range of values for "n", but my > experience with android devices with non-abundant memory size is that > they are sensible to kmalloc > PAGE_SIZE. It is much easier to assume that we are keeping the second malloc are getting rid of the first one. The header is only one pointer. > Back to your patch description, you mention new kmalloc size may be > PAGE_SIZE + 8 bytes, which means kmalloc may have to find 2 consecutive > free pages. Of course it is not a big issue, but I don't see a reason to > unnecessarily change 2 kmalloc < PAGE_SIZE by one possibly > PAGE_SIZE. The values of 'n' are unknown. I doubt that the value that just exceeds a page happens any more often than those either side of it. David -- To unsubscribe from this list: send the line "unsubscribe 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: dwc3: fix wrong bit mask in dwc3_event_devt
Per dwc3 2.70a spec in the Device-Specific Event (DEVT), the field of
Event Information Bits(EvtInfo) uses [24:16] bits, and it has 9 bits
not 8 bits. And the following reserved field uses [31:25] bits not
[31:24] bits, and it has 7 bits.
So in dwc3_event_devt, the bit mask should be:
event_info [24:16] 9 bits
reserved31_25 [31:25] 7 bits
This patch should be backported to kernels as old as 3.2, that contain
the commit 72246da40f3719af3bfd104a2365b32537c27d83 "usb: Introduce
DesignWare USB3 DRD Driver".
Signed-off-by: Huang Rui
---
drivers/usb/dwc3/core.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index f8af8d4..69c4583 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -815,15 +815,15 @@ struct dwc3_event_depevt {
* 12 - VndrDevTstRcved
* @reserved15_12: Reserved, not used
* @event_info: Information about this event
- * @reserved31_24: Reserved, not used
+ * @reserved31_25: Reserved, not used
*/
struct dwc3_event_devt {
u32 one_bit:1;
u32 device_event:7;
u32 type:4;
u32 reserved15_12:4;
- u32 event_info:8;
- u32 reserved31_24:8;
+ u32 event_info:9;
+ u32 reserved31_25:7;
} __packed;
/**
--
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
[PATCH v2] usb: dwc3: fix wrong bit mask in dwc3_event_devt
Per dwc3 2.70a spec in the Device-Specific Event (DEVT), the field of
Event Information Bits(EvtInfo) uses [24:16] bits, and it has 9 bits
not 8 bits. And the following reserved field uses [31:25] bits not
[31:24] bits, and it has 7 bits.
So in dwc3_event_devt, the bit mask should be:
event_info [24:16] 9 bits
reserved31_25 [31:25] 7 bits
This patch should be backported to kernels as old as 3.2, that contain
the commit 72246da40f3719af3bfd104a2365b32537c27d83 "usb: Introduce
DesignWare USB3 DRD Driver".
Cc:
Signed-off-by: Huang Rui
---
Changes from v1 -> v2:
- Add CC stable mail list.
drivers/usb/dwc3/core.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index f8af8d4..69c4583 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -815,15 +815,15 @@ struct dwc3_event_depevt {
* 12 - VndrDevTstRcved
* @reserved15_12: Reserved, not used
* @event_info: Information about this event
- * @reserved31_24: Reserved, not used
+ * @reserved31_25: Reserved, not used
*/
struct dwc3_event_devt {
u32 one_bit:1;
u32 device_event:7;
u32 type:4;
u32 reserved15_12:4;
- u32 event_info:8;
- u32 reserved31_24:8;
+ u32 event_info:9;
+ u32 reserved31_25:7;
} __packed;
/**
--
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: [PATCH] RFC: Allow to blacklist xhci_hcd module and use ports with EHCI
> From: Of Holger Freyther > > xhci_hcd does not work with the Canon Lide scanners and has issues > > with the suspend/resume handling. My Acer Aspire S5 notebook only > > exposes USB3.0 ports and the distribution kernels generally have > > xhci_hcd enabled and I don't have any USB3.0 hardware either. > > I am using the laptop with the ports routed to the EHCI and I didn't > have any suspend/resume issues and I could even do the book keeping > of sysmocom on my laptop now. > > Could we please get to a situation were users that only have USB3.0 > ports can either drop to EHCI mode by unloading the xhci_hcd module > or preferable be able to blacklist the xhci_hcd so I could even use > the stock Debian kernel? Are these scanners using USB3 speeds, or USB2 speeds using xhci? And/or is there a way of stopping the xhci hardware attempting USB3 operation even for a USB3 device connected with a USB3 cable? David -- To unsubscribe from this list: send the line "unsubscribe 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 1/4] phy: Add new Exynos5 USB 3.0 PHY driver
Hi, On Monday 30 December 2013 03:13 PM, Vivek Gautam wrote: > Hi Kishon, > > > On Tue, Dec 24, 2013 at 11:15 PM, Kishon Vijay Abraham I > wrote: >> Hi, >> >> >> On Thursday 05 December 2013 01:44 PM, Vivek Gautam wrote: >>> >>> Hi Kishon, >>> >>> >>> On Wed, Dec 4, 2013 at 7:58 PM, Kishon Vijay Abraham I >>> wrote: Hi Vivek, On Wednesday 20 November 2013 09:14 PM, Kishon Vijay Abraham I wrote: > > Hi, > > On Wednesday 20 November 2013 03:02 PM, Vivek Gautam wrote: >> >> On Wed, Nov 20, 2013 at 2:34 PM, Kishon Vijay Abraham I >> wrote: >>> >>> On Wednesday 20 November 2013 02:27 PM, Vivek Gautam wrote: Hi Kishon, On Mon, Nov 11, 2013 at 4:41 PM, Kishon Vijay Abraham I wrote: > > Hi, sorry for the delayed response. > > On Wednesday 06 November 2013 05:37 AM, Jingoo Han wrote: >> >> On Wednesday, November 06, 2013 2:58 AM, Vivek Gautam wrote: >>> >>> On Tue, Nov 5, 2013 at 5:33 PM, Jingoo Han >>> wrote: >> >> >> [.] >> USB3.0 PHY consists of two blocks such as 3.0 block and 2.0 block. This USB3.0 PHY can support UTMI+ and PIPE3 interface for 3.0 block and 2.0 block, respectively. Conclusion: 1) USB2.0 PHY: USB2.0 HOST, USB2.0 Device Base address: 0x1213 2) USB3.0 PHY: USB3.0 DRD (3.0 HOST & 3.0 Device) Base address: 0x1210 2.0 block(UTMI+) & 3.0 block(PIPE3) >>> >>> >>> And this is of course the PHY used by DWC3 controller, which works >>> at >>> both High speed as well as Super Speed. >>> Right ? >> >> >> Right. >> >> While 3.0 block(PIPE3) can be used for Super Speed, 2.0 >> block(UTMI+) >> can be used for High speed. > > > It should then come under *single IP muliple PHY* category similar > to what > Sylwester has done. Do you mean that i should be including PHY IDs for UTMI+ phy and PIPE3 phy present in this PHY block ? AFAICS the two phys (UTMI+ and PIPE3) do not really have separate registers to program, and that's the reason we program the entire PHY in a shot. >>> >>> >>> you mean you program the same set of bits for UTMI+ and PIPE3? >> >> >> No, looking closely into PHY datasheet as well as Exynos5250 manual, i >> can see that UTMI+ and PIPE3 >> phys have separate bit settings. So i think we should be able to >> segregate the two PHYs (UTMI+ and PIPE3). >> Pardon me for my earlier observations. > > > no problem.. >> >> Let me clarify more with our h/w team also on this and then i will >> confirm with this. Did you get more information on this? >>> >>> >>> Yes, i have been in contact with our hardware team. >>> The functionality of setting up UTMI+ and PIPE3 phys separately, and >>> thereby using only one functionality of the two >>> at some point of time (either high speed or super speed) hasn't been >>> tested so far. >> >> >> Irrespective of whether we are able to test the functionality separately or >> not, we should model it as multiple PHYs since you have separate bit >> settings for UTMI+ and PIPE3. >> >> (I'll review your next patch version shortly). > > Thanks Kishon, i know i am disturbing you in the holiday season. :-) > But there's one concern, on Exynos5 platforms we have only one bit to > power control > the entire PHY (irrespective of the two PHYs present in the USB 3.0 > PHY controller). > So anyways we won't be able to save anything on the power front even > if we program only > one PHY (UTMI/PIPE3). > Although there are PHY settings register bits which seem separate for > the two phys. r > What do you suggest in that case ? The idea is to model the driver as close to the hardware though I understand there won't be any advantages w.r.t power or performance. maybe in later versions of the IP we'll have separate bits to control usb3 and usb2. I think for power control we should have both usb3 and usb2 power-on callback calling a single function that controls the power bit. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] xhci: Switch Intel Lynx Point ports to EHCI on shutdown
At Mon, 06 Jan 2014 14:34:28 +0200, Denis Turischev wrote: > > Hi Sarah, > > On 01/03/2014 02:03 AM, Sarah Sharp wrote: > > Denis, do all of Compulab's Haswell systems reboot on shutdown? Are > > they all running a Phoenix BIOS? Can you send me the output of `sudo > > lspci -vvv -s` for the xHCI host? > > oem@oem-Intense-PC2 ~ $ sudo lspci -vvv -s 00:14.0 > 00:14.0 USB controller: Intel Corporation Lynx Point-LP USB xHCI HC (rev 04) > (prog-if 30 [XHCI]) > Subsystem: Intel Corporation Device 7270 > Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- > Stepping- SERR- FastB2B- DisINTx+ > Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- > SERR-Latency: 0 > Interrupt: pin A routed to IRQ 59 > Region 0: Memory at f062 (64-bit, non-prefetchable) [size=64K] > Capabilities: [70] Power Management version 2 > Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA > PME(D0-,D1-,D2-,D3hot+,D3cold+) > Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- > Capabilities: [80] MSI: Enable+ Count=1/8 Maskable- 64bit+ > Address: fee0200c Data: 41b1 > Kernel driver in use: xhci_hcd > > > Basically, I'm trying to find a common variable to key off. I suspect > > BIOS vendor is probably the right thing, instead of system vendor. > > By the way the quirk introduced by commit > e95829f474f0db3a4d940cae1423783edd966027 "xhci: Switch PPT > ports to EHCI on shutdown." works for Lynx Point as well at least on > Intense-PC2. I mean we can add > XHCI_SPURIOUS_REBOOT flag that invokes usb_disable_xhci_ports(). > > May be this solution works for HP and other systems without side effects? No, we already tested it at first, but didn't fix the behavior on HP machines. It was harmless as far as we've tested, though. Takashi -- To unsubscribe from this list: send the line "unsubscribe 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: gadgetfs USB2.0 Chapter 9 Tests: Test after "Addressed State" fails
On Tue, Jan 07 2014, Marco Zamponi wrote: > I am testing now the unchanged usb.c example > (www.linux-usb.org/gadget/usb.c) as user space application on my ARM > Cortex A5 with atmel_usba_udc driver. Note that usb.c uses GadgetFS *not* FunctionFS. Unfortunately there are slight incompatibilities between the two, and user space application written for GadgetFS won't work with FunctionFS out of the box. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Re: gadgetfs USB2.0 Chapter 9 Tests: Test after "Addressed State" fails
Actually, I was referring to gadgetFS all along. Shouldn't the usb.c user space application be compliant (e.g. pass all the CV Tests) ? On Tue, Jan 7, 2014 at 11:10 AM, Michal Nazarewicz wrote: > On Tue, Jan 07 2014, Marco Zamponi wrote: >> I am testing now the unchanged usb.c example >> (www.linux-usb.org/gadget/usb.c) as user space application on my ARM >> Cortex A5 with atmel_usba_udc driver. > > Note that usb.c uses GadgetFS *not* FunctionFS. Unfortunately there are > slight incompatibilities between the two, and user space application > written for GadgetFS won't work with FunctionFS out of the box. > > -- > Best regards, _ _ > .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o > ..o | Computer Science, Michał “mina86” Nazarewicz(o o) > ooo +--ooO--(_)--Ooo-- > > -- To unsubscribe from this list: send the line "unsubscribe 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 1/4] phy: Add new Exynos5 USB 3.0 PHY driver
HI Kishon On Tue, Jan 7, 2014 at 3:19 PM, Kishon Vijay Abraham I wrote: > Hi, > > On Monday 30 December 2013 03:13 PM, Vivek Gautam wrote: >> Hi Kishon, >> >> >> On Tue, Dec 24, 2013 at 11:15 PM, Kishon Vijay Abraham I >> wrote: >>> Hi, >>> >>> >>> On Thursday 05 December 2013 01:44 PM, Vivek Gautam wrote: Hi Kishon, On Wed, Dec 4, 2013 at 7:58 PM, Kishon Vijay Abraham I wrote: > > Hi Vivek, > > On Wednesday 20 November 2013 09:14 PM, Kishon Vijay Abraham I wrote: >> >> Hi, >> >> On Wednesday 20 November 2013 03:02 PM, Vivek Gautam wrote: >>> >>> On Wed, Nov 20, 2013 at 2:34 PM, Kishon Vijay Abraham I >>> wrote: On Wednesday 20 November 2013 02:27 PM, Vivek Gautam wrote: > > Hi Kishon, > > > On Mon, Nov 11, 2013 at 4:41 PM, Kishon Vijay Abraham I > wrote: >> >> Hi, > > sorry for the delayed response. > >> >> On Wednesday 06 November 2013 05:37 AM, Jingoo Han wrote: >>> >>> On Wednesday, November 06, 2013 2:58 AM, Vivek Gautam wrote: On Tue, Nov 5, 2013 at 5:33 PM, Jingoo Han wrote: >>> >>> >>> [.] >>> > USB3.0 PHY consists of two blocks such as 3.0 block and 2.0 > block. > This USB3.0 PHY can support UTMI+ and PIPE3 interface for 3.0 > block > and 2.0 block, respectively. > > Conclusion: > > 1) USB2.0 PHY: USB2.0 HOST, USB2.0 Device > Base address: 0x1213 > > 2) USB3.0 PHY: USB3.0 DRD (3.0 HOST & 3.0 Device) > Base address: 0x1210 > 2.0 block(UTMI+) & 3.0 block(PIPE3) And this is of course the PHY used by DWC3 controller, which works at both High speed as well as Super Speed. Right ? >>> >>> >>> Right. >>> >>> While 3.0 block(PIPE3) can be used for Super Speed, 2.0 >>> block(UTMI+) >>> can be used for High speed. >> >> >> It should then come under *single IP muliple PHY* category similar >> to what >> Sylwester has done. > > > Do you mean that i should be including PHY IDs for UTMI+ phy and > PIPE3 > phy present in this PHY block ? > AFAICS the two phys (UTMI+ and PIPE3) do not really have separate > registers to program, and that's the reason > we program the entire PHY in a shot. you mean you program the same set of bits for UTMI+ and PIPE3? >>> >>> >>> No, looking closely into PHY datasheet as well as Exynos5250 manual, i >>> can see that UTMI+ and PIPE3 >>> phys have separate bit settings. So i think we should be able to >>> segregate the two PHYs (UTMI+ and PIPE3). >>> Pardon me for my earlier observations. >> >> >> no problem.. >>> >>> Let me clarify more with our h/w team also on this and then i will >>> confirm with this. > > > Did you get more information on this? Yes, i have been in contact with our hardware team. The functionality of setting up UTMI+ and PIPE3 phys separately, and thereby using only one functionality of the two at some point of time (either high speed or super speed) hasn't been tested so far. >>> >>> >>> Irrespective of whether we are able to test the functionality separately or >>> not, we should model it as multiple PHYs since you have separate bit >>> settings for UTMI+ and PIPE3. >>> >>> (I'll review your next patch version shortly). >> >> Thanks Kishon, i know i am disturbing you in the holiday season. :-) >> But there's one concern, on Exynos5 platforms we have only one bit to >> power control >> the entire PHY (irrespective of the two PHYs present in the USB 3.0 >> PHY controller). >> So anyways we won't be able to save anything on the power front even >> if we program only >> one PHY (UTMI/PIPE3). >> Although there are PHY settings register bits which seem separate for >> the two phys. r >> What do you suggest in that case ? > > The idea is to model the driver as close to the hardware though I understand > there won't be any advantages w.r.t power or performance. maybe in later > versions of the IP we'll have separate bits to control usb3 and usb2. Ok, i will prepare the next patchset for separating out the possible code based on the UTMI+ or PIPE3 phys. Though when experimenting with the PHY settings i can see there's little of such code :-) > > I think for power control we should have both usb3 and usb2 power-on callback > calling a single function that controls t
Asymmetric speed results with testusb/usbtest/g_zero
Hi, I'm seeing peak rates of about 50 Mbps write speeds, and 150 Mbps read. Setup is host AMD SB700/SB800/Hudson-2/3 and Atheros SoC, using testusb tool with usbtest module and g_zero gadget module on TI AM335x. Is this to be expected? Is this a limitation of the controllers or the test setup? Can anyone point me at speed numbers for testusb/usbtest/g_zero? This is using tests 5 and 6 from testusb, which read/write various chained block sizes for various counts. The speeds mentioned are for the largest 32KB block size, on both the AMD and Atheros. Thanks, Conor -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
ohci-at91 mismerge build error
After commit 99f14bd4d1 "Merge 3.13-rc5 into usb-next" (in linux-next as of
today), I'm getting this error building any at91 kernel:
drivers/usb/host/ohci-at91.c: In function 'usb_hcd_at91_probe':
drivers/usb/host/ohci-at91.c:190:4: error: label 'err' used but not defined
goto err;
^
drivers/usb/host/ohci-at91.c: At top level:
drivers/usb/host/ohci-at91.c:206:2: warning: data definition has no type or
storage class [enabled by default]
at91_stop_hc(pdev);
^
...
The problem is obviously a mismerge between two unrelated changes that
resulted in missing opening braces.
Signed-off-by: Arnd Bergmann
---
Please just ignore if this has been reported before
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 2d0ee5e..091ae49 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -197,7 +197,7 @@ static int usb_hcd_at91_probe(const struct hc_driver
*driver,
at91_start_hc(pdev);
retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
- if (retval == 0)
+ if (retval == 0) {
device_wakeup_enable(hcd->self.controller);
return retval;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ohci-at91 mismerge build error
Hello Arnd,
On 07/01/2014 12:54, Arnd Bergmann wrote:
After commit 99f14bd4d1 "Merge 3.13-rc5 into usb-next" (in linux-next as of
today), I'm getting this error building any at91 kernel:
drivers/usb/host/ohci-at91.c: In function 'usb_hcd_at91_probe':
drivers/usb/host/ohci-at91.c:190:4: error: label 'err' used but not defined
goto err;
^
drivers/usb/host/ohci-at91.c: At top level:
drivers/usb/host/ohci-at91.c:206:2: warning: data definition has no type or
storage class [enabled by default]
at91_stop_hc(pdev);
^
...
The problem is obviously a mismerge between two unrelated changes that
resulted in missing opening braces.
Thanks for fixing this: I was about to propose the same patch to resolve
the issue introduced by this merge (reported by Olof yesterday).
Signed-off-by: Arnd Bergmann
Acked-by: Boris BREZILLON
---
Please just ignore if this has been reported before
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 2d0ee5e..091ae49 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -197,7 +197,7 @@ static int usb_hcd_at91_probe(const struct hc_driver
*driver,
at91_start_hc(pdev);
retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
- if (retval == 0)
+ if (retval == 0) {
device_wakeup_enable(hcd->self.controller);
return retval;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module
USB Host driver (drivers/mfd/omap-usb-host.c) expects the 60MHz
reference clock to be named "init_60m_fclk". Provide this
information.
Signed-off-by: Roger Quadros
---
arch/arm/boot/dts/omap5.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 2f12a47..e0ab379 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -765,6 +765,8 @@
#address-cells = <1>;
#size-cells = <1>;
ranges;
+ clocks = <&l3init_60m_fclk>;
+ clock-names = "init_60m_fclk";
usbhsohci: ohci@4a064800 {
compatible = "ti,ohci-omap3", "usb-ohci";
--
1.8.3.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
[PATCH v3 3/4] ARM: dts: omap5-uevm: Provide USB PHY clock
The HS USB 2 PHY gets its clock from AUXCLK1. Provide this
information.
Signed-off-by: Roger Quadros
---
arch/arm/boot/dts/omap5-uevm.dts | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts
index 002fa70..3b99ec2 100644
--- a/arch/arm/boot/dts/omap5-uevm.dts
+++ b/arch/arm/boot/dts/omap5-uevm.dts
@@ -31,12 +31,8 @@
hsusb2_phy: hsusb2_phy {
compatible = "usb-nop-xceiv";
reset-gpios = <&gpio3 16 GPIO_ACTIVE_LOW>; /* gpio3_80
HUB_NRESET */
- /**
- * FIXME
- * Put the right clock phandle here when available
- * clocks = <&auxclk1>;
- * clock-names = "main_clk";
- */
+ clocks = <&auxclk1_ck>;
+ clock-names = "main_clk";
clock-frequency = <1920>;
};
--
1.8.3.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
[PATCH v3 0/4] USB Host support for OMAP5 uEVM (for 3.14)
Hi Benoit & Tony, This patchset brings up USB Host ports and Ethernet port on the OMAP5 uEVM board. It depends on the TI Clock DT conversion patches [1]. [1] - http://article.gmane.org/gmane.linux.ports.arm.kernel/289895 cheers, -roger Roger Quadros (4): ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module ARM: dts: omap4-panda: Provide USB PHY clock ARM: dts: omap5-uevm: Provide USB PHY clock ARM: OMAP2+: Remove legacy_init_ehci_clk() arch/arm/boot/dts/omap4-panda-common.dtsi | 8 ++-- arch/arm/boot/dts/omap5-uevm.dts | 8 ++-- arch/arm/boot/dts/omap5.dtsi | 2 ++ arch/arm/mach-omap2/pdata-quirks.c| 16 4 files changed, 6 insertions(+), 28 deletions(-) -- 1.8.3.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
[PATCH v3 2/4] ARM: dts: omap4-panda: Provide USB PHY clock
The USB PHY gets its clock from AUXCLK3. Provide this information. Signed-off-by: Roger Quadros --- arch/arm/boot/dts/omap4-panda-common.dtsi | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi index 88c6a05..50b72966 100644 --- a/arch/arm/boot/dts/omap4-panda-common.dtsi +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi @@ -83,12 +83,8 @@ compatible = "usb-nop-xceiv"; reset-gpios = <&gpio2 30 GPIO_ACTIVE_LOW>; /* gpio_62 */ vcc-supply = <&hsusb1_power>; - /** -* FIXME: -* put the right clock phandle here when available -* clocks = <&auxclk3>; -* clock-names = "main_clk"; -*/ + clocks = <&auxclk3_ck>; + clock-names = "main_clk"; clock-frequency = <1920>; }; -- 1.8.3.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
[PATCH v3 4/4] ARM: OMAP2+: Remove legacy_init_ehci_clk()
The necessary clock phandle for the EHCI clock is now provided
via device tree so we no longer need this legacy method.
Signed-off-by: Roger Quadros
---
arch/arm/mach-omap2/pdata-quirks.c | 16
1 file changed, 16 deletions(-)
diff --git a/arch/arm/mach-omap2/pdata-quirks.c
b/arch/arm/mach-omap2/pdata-quirks.c
index 39f020c..6a4e2d1 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -26,20 +26,6 @@ struct pdata_init {
void (*fn)(void);
};
-/*
- * Create alias for USB host PHY clock.
- * Remove this when clock phandle can be provided via DT
- */
-static void __init __used legacy_init_ehci_clk(char *clkname)
-{
- int ret;
-
- ret = clk_add_alias("main_clk", NULL, clkname, NULL);
- if (ret)
- pr_err("%s:Failed to add main_clk alias to %s :%d\n",
- __func__, clkname, ret);
-}
-
#if IS_ENABLED(CONFIG_WL12XX)
static struct wl12xx_platform_data wl12xx __initdata;
@@ -105,7 +91,6 @@ static void __init omap4_sdp_legacy_init(void)
static void __init omap4_panda_legacy_init(void)
{
omap4_panda_display_init_of();
- legacy_init_ehci_clk("auxclk3_ck");
legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
}
#endif
@@ -113,7 +98,6 @@ static void __init omap4_panda_legacy_init(void)
#ifdef CONFIG_SOC_OMAP5
static void __init omap5_uevm_legacy_init(void)
{
- legacy_init_ehci_clk("auxclk1_ck");
}
#endif
--
1.8.3.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: [PATCH net-next v2 6/6] r8152: support RTL8153
Bjørn Mork [mailto:bj...@mork.no] > Sent: Monday, January 06, 2014 5:22 PM > To: Hayeswang > Cc: oli...@neukum.org; net...@vger.kernel.org; nic_swsd; > linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org > Subject: Re: [PATCH net-next v2 6/6] r8152: support RTL8153 [...] > Exactly the same device, but now cfg #1 is active and a > different set of > drivers have bound to the interfaces. This is possible > because none of > the involved drivers disable the support for this device at > build-time. > Instead they use the available interface descriptors for matching and > probing supported functions. > > End users will of course normally not go around writing stuff to sysfs > attributes like this. Creating an udev rule to select a specific > counfiguration when the device is plugged is more useful for normal > usage. Thanks for your answer. I would study udev rule first. Does the udev alwayes exist for all Linux system, such as Android, embedded system, and so on? Best Regards, Hayes -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst
On 01/06/2014 04:31 PM, Sarah Sharp wrote: > On Fri, Jan 03, 2014 at 03:29:29PM -0800, Sarah Sharp wrote: >> With the dmesg, I can finally see what happened: >> >> [ 188.703059] xhci_hcd :03:00.0: Cancel URB 8800b7d2e0c0, dev 1, ep >> 0x2, starting at offset 0xbb7b9000 >> [ 188.703072] xhci_hcd :03:00.0: // Ding dong! >> [ 193.711022] xhci_hcd :03:00.0: xHCI host not responding to stop >> endpoint command. >> [ 193.711029] xhci_hcd :03:00.0: Assuming host is dying, halting host. >> [ 193.711046] xhci_hcd :03:00.0: // Halt the HC >> [ 193.711060] xhci_hcd :03:00.0: Killing URBs for slot ID 1, ep index 0 >> [ 193.711066] xhci_hcd :03:00.0: Killing URBs for slot ID 1, ep index 2 >> [ 193.711078] xhci_hcd :03:00.0: Killing URBs for slot ID 1, ep index 3 >> [ 193.711096] xhci_hcd :03:00.0: Calling usb_hc_died() >> [ 193.711103] xhci_hcd :03:00.0: HC died; cleaning up >> [ 193.76] xhci_hcd :03:00.0: xHCI host controller is dead. >> >> It seems that the xHCI driver tried to stop the endpoint ring in order >> to cancel a SCSI transfer, and the driver never got a response for that. >> >> The offset is rather suspicious (0xbb7b9000), and it probably means the >> driver attempted to cancel a transfer that had been moved to the >> beginning of the ring segment, with no-op TRBs before the link TRB. >> >> I suspect David's patch triggers a bug in the command cancellation code. >> There's also the unlikely possibility that the no-op TRBs did indeed >> cause the host to hang. Either way, I'll have to look into it. >> >> I'll let you know when I have some diagnostic patches ready. > > Hi Walt, > > I have a couple of patches for you to test. > Please only apply the first patch (which is diagnostic only), trigger > your issue, and send me the resulting dmesg. Then try applying the > other two patches, and see if the issue goes away. (I suspect it won't > but I can't be sure.) Thanks Sarah. dmesg0 is from the diagnostic patch only. dmesg1 has all three patches applied. Some of the messages in dmesg1 fell off the end of the kernel buffer, so I may need to make the buffer larger next time but I'll need a reminder of how to do it. As you suspected, the patches didn't fix the problem, sorry. I find that I can tell in advance whether the copy is going to succeed, just by watching the light flicker on the usb3 drive. When the flicker is absolutely regular, with no variation whatever, I can tell in 10 or 15 seconds that the copy will fail. At the same time the light on the main drive goes dark after 10 seconds, implying that the usb3 drive stops receiving any data from the main drive after 10 seconds, yet the light on the usb3 drive continues to flicker as if writing data -- even after the cp officially fails. The light on the usb3 drive never stops flickering until I reboot the machine or unplug the usb cable. dmesg0.gz Description: application/gzip dmesg1.gz Description: application/gzip
[PATCH -next] usb: gadget: remove unused variable in gr_queue_int()
From: Wei Yongjun
The variable 'dev' is initialized but never used
otherwise, so remove the unused variable.
Signed-off-by: Wei Yongjun
---
drivers/usb/gadget/gr_udc.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index 5f9c659..483d5a8 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -663,9 +663,6 @@ static inline int gr_queue_int(struct gr_ep *ep, struct
gr_request *req,
static void gr_ep_nuke(struct gr_ep *ep)
{
struct gr_request *req;
- struct gr_udc *dev;
-
- dev = ep->dev;
ep->stopped = 1;
ep->dma_start = 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 -next] usb: gadget: s3c-hsotg: remove duplicated include from s3c-hsotg.c
From: Wei Yongjun Remove duplicated include. Signed-off-by: Wei Yongjun --- drivers/usb/gadget/s3c-hsotg.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c index c0ff1cb..1172eae 100644 --- a/drivers/usb/gadget/s3c-hsotg.c +++ b/drivers/usb/gadget/s3c-hsotg.c @@ -31,7 +31,6 @@ #include #include #include -#include #include #include -- To unsubscribe from this list: send the line "unsubscribe 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.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst
> From: walt ... > Thanks Sarah. dmesg0 is from the diagnostic patch only. dmesg1 has all > three patches applied. Some of the messages in dmesg1 fell off the end of > the kernel buffer, so I may need to make the buffer larger next time but > I'll need a reminder of how to do it. > > As you suspected, the patches didn't fix the problem, sorry. I think Sarah will want the traces of the transfer being setup (ie just before the error). Some 'normal' completing transfers are also useful. You might also find the full trace output in one of the files in /var/log. David
RE: [PATCH 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst
The dmesg contains: [ 538.728064] EXT4-fs warning (device dm-0): ext4_end_bio:316: I/O error writing to inode 23330865 (offset 0 size 8388608 starting block 812628) An 8MB transfer will need at least 128 ring entries (TRB) even if the request is a single contiguous memory block. Are you using the patch that increases the ring size from 64 to 256? David
Re: [PATCH v3 1/4] ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module
On Tuesday 07 January 2014, Roger Quadros wrote:
> USB Host driver (drivers/mfd/omap-usb-host.c) expects the 60MHz
> reference clock to be named "init_60m_fclk". Provide this
> information.
>
> Signed-off-by: Roger Quadros
> ---
> arch/arm/boot/dts/omap5.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index 2f12a47..e0ab379 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -765,6 +765,8 @@
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> + clocks = <&l3init_60m_fclk>;
> + clock-names = "init_60m_fclk";
>
> usbhsohci: ohci@4a064800 {
> compatible = "ti,ohci-omap3", "usb-ohci";
The bindings/mfd/omap-usb-host.txt file doesn't document any clocks.
Please create another patch to document the clock names in this binding
before you start putting them into the dtsi file. So far the clock
names are an implementation detail of Linux as they are not part
of the binding, and with your patch it becomes part of the ABI.
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] usb:hub set hub->change_bits when over-current happens
On Tue, Jan 07, 2014 at 02:38:36PM +0800, Shen Guang wrote: > On Tue, Jan 7, 2014 at 11:53 AM, Greg KH wrote: > > On Tue, Jan 07, 2014 at 11:35:50AM +0800, 沈光 wrote: > >> On Tue, Jan 7, 2014 at 10:40 AM, Greg KH > >> wrote: > >> > On Tue, Jan 07, 2014 at 10:33:14AM +0800, 沈光 wrote: > >> >> set hub->change_bits when we plug in a device which causes > >> >> over-current condition, so that hub_events() will check it. > >> > > >> > Why? > >> > > >> > What does this solve? Is this a bug with existing devices that needs to > >> > be backported to older kernels? > >> > > >> > thanks, > >> > > >> > greg k-h > >> > >> This is something that we found when we are doing compliance test with > >> xHCI. > >> If we enable CONFIG_USB_SUSPEND, and plug in a bad device which causes > >> over-current condition to the root port, the hub->change_bits will not > >> be set in current code in the function of hub_activate. > > > > Now that's something that should go in the changelog, don't you think? > > > > thanks, > > > > greg k-h > > Sure, I agree. > I am sorry I didn't make it clear enough. > And the reason why over-current can be detected if CONFIG_USB_SUSPEND > is disabled, is that the interrupt pipe of the hub will report the > change and set hub->event_bits, and then hub_events() will check what > events happened. Great, care to resend the patch with all of this information added to it? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] RFC: Allow to blacklist xhci_hcd module and use ports with EHCI
On Tue, 7 Jan 2014, Holger Hans Peter Freyther wrote: > On Mon, Jan 06, 2014 at 04:44:51PM -0500, Alan Stern wrote: > > > Finally, blacklisting xhci-hcd won't solve the problem at hand, because > > the ports get switched from EHCI to xHCI during early PCI processing, > > before xhci-hcd is loaded. The only check is for whether > > CONFIG_USB_XHCI_HCD is enabled, which isn't affected by blacklisting. > > > > As far as I can see, the code that switches the ports back to EHCI gets > > run only when the computer is turned off (and then only for some types > > of machines). > > I think in my case quirk_usb_handoff_xhci is called during boot that > will switch the ports to the XHCI. That's what I said. Early PCI processing occurs during boot. > The question is about defaults. My RFC patch proposed to route the > ports to the EHCI until the XHCI module is loaded. I had modified a > comment to highlight a potential issue with this approach. The comment > mentioned that the code tries to avoid taking away an already enumerated > device (e.g. usb storage already mounted) from the EHCI. Indeed, port switching should be avoided once the USB buses are active. > In my case this did not happen. ehci-hcd starts after xhci-hcd but this > might be sheer luck. The question is if this can be made deterministic > or not. No, it cannot. > > The best way to solve this problem would be a boot command-line option. > > Do you have a proposal for a name? One possibility: noxhci-port-switch Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb/core: fix NULL pointer dereference in recursively_mark_NOTATTACHED
On Tue, 7 Jan 2014, Du, ChangbinX wrote: > > > Changbin, after looking more closely I realized there was a second > > > aspect to this race: recursively_mark_NOTATTACHED uses hub->ports[i] > > > while hub_disconnect removes the port devices. You ought to be able > > > to cause an oops by inserting a delay just after the loop where > > > usb_hub_remove_port_device is called. > > > > > > The updated patch below should fix both problems. Can you test it? > > > > > > Alan Stern > > > > > > > Ok, I'll test it today or tomorrow. Please wait my response. > > Alan, I cannot cause a panic after inserting a delay just after > usb_hub_remove_port_device is called, even move the delay after > kfree(hub->ports). recursively_mark_NOTATTACHED will not access > hub->ports[i] since maxchild has been set to 0. > > Alan, I think your last patch can fix this issue. Okay, thanks for testing. I will submit the patch. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: fix race between hub_disconnect and recursively_mark_NOTATTACHED
There is a race in the hub driver between hub_disconnect() and
recursively_mark_NOTATTACHED(). This race can be triggered if the
driver is unbound from a device at the same time as the bus's root hub
is removed. When the race occurs, it can cause an oops:
BUG: unable to handle kernel NULL pointer dereference at 015c
IP: [] recursively_mark_NOTATTACHED+0x20/0x60
Call Trace:
[] recursively_mark_NOTATTACHED+0x34/0x60
[] recursively_mark_NOTATTACHED+0x34/0x60
[] recursively_mark_NOTATTACHED+0x34/0x60
[] recursively_mark_NOTATTACHED+0x34/0x60
[] usb_set_device_state+0x92/0x120
[] usb_disconnect+0x2b/0x1a0
[] usb_remove_hcd+0xb0/0x160
[] ? _raw_spin_unlock_irqrestore+0x26/0x50
[] ehci_mid_remove+0x1c/0x30
[] ehci_mid_stop_host+0x16/0x30
[] penwell_otg_work+0xd28/0x3520
[] ? __schedule+0x39b/0x7f0
[] ? sub_preempt_count+0x3d/0x50
[] process_one_work+0x11d/0x3d0
[] ? mutex_unlock+0xd/0x10
[] ? manage_workers.isra.24+0x1b5/0x270
[] worker_thread+0xf9/0x320
[] ? _raw_spin_unlock_irqrestore+0x26/0x50
[] ? rescuer_thread+0x2b0/0x2b0
[] kthread+0x94/0xa0
[] ret_from_kernel_thread+0x1b/0x28
[] ? kthread_create_on_node+0xc0/0xc0
One problem is that recursively_mark_NOTATTACHED() uses the intfdata
value and hub->hdev->maxchild while hub_disconnect() is clearing them.
Another problem is that it uses hub->ports[i] while the port device is
being released.
To fix this race, we need to hold the device_state_lock while
hub_disconnect() changes the values. (Note that usb_disconnect()
and hub_port_connect_change() already acquire this lock at similar
critical times during a USB device's life cycle.) We also need to
remove the port devices after maxchild has been set to 0, instead of
before.
Signed-off-by: Alan Stern
Reported-by: "Du, Changbin"
Tested-by: "Du, Changbin"
CC:
---
[as1733]
drivers/usb/core/hub.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
Index: usb-3.13/drivers/usb/core/hub.c
===
--- usb-3.13.orig/drivers/usb/core/hub.c
+++ usb-3.13/drivers/usb/core/hub.c
@@ -1607,7 +1607,7 @@ static void hub_disconnect(struct usb_in
{
struct usb_hub *hub = usb_get_intfdata(intf);
struct usb_device *hdev = interface_to_usbdev(intf);
- int i;
+ int port1;
/* Take the hub off the event list and don't let it be added again */
spin_lock_irq(&hub_event_lock);
@@ -1622,11 +1622,15 @@ static void hub_disconnect(struct usb_in
hub->error = 0;
hub_quiesce(hub, HUB_DISCONNECT);
- usb_set_intfdata (intf, NULL);
+ /* Avoid races with recursively_mark_NOTATTACHED() */
+ spin_lock_irq(&device_state_lock);
+ port1 = hdev->maxchild;
+ hdev->maxchild = 0;
+ usb_set_intfdata(intf, NULL);
+ spin_unlock_irq(&device_state_lock);
- for (i = 0; i < hdev->maxchild; i++)
- usb_hub_remove_port_device(hub, i + 1);
- hub->hdev->maxchild = 0;
+ for (; port1 > 0; --port1)
+ usb_hub_remove_port_device(hub, port1);
if (hub->hdev->speed == USB_SPEED_HIGH)
highspeed_hubs--;
--
To unsubscribe from this list: send the line "unsubscribe 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: gadgetfs USB2.0 Chapter 9 Tests: Test after "Addressed State" fails
On Tue, Jan 07 2014, Marco Zamponi wrote: > Actually, I was referring to gadgetFS all along. In that case everything I've written may be inapplicable. > Shouldn't the usb.c user space application be compliant (e.g. pass all > the CV Tests) ? Yes it should. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
[PATCH] staging: usbip: userspace: add support for viewing imported devices
As of Matt Mooney's major refactoring in 2011, usbip port
option was left out. Add support for this option in
a manner similar to the old implementation.
Sample output:
Imported USB devices
Port 00: at Full Speed(12Mbps)
unknown vendor : unknown product (1687:6211)
2-1 -> usbip://192.168.122.152:3240/1-1
-> remote bus/dev 001/002
Signed-off-by: Valentina Manea
Reviewed-by: Ilija Hadzic
---
Resubmitting this patch as I still didn't get any reply from Greg
after 2 weeks.
.../staging/usbip/userspace/libsrc/vhci_driver.c | 67 ++
.../staging/usbip/userspace/libsrc/vhci_driver.h | 2 +
drivers/staging/usbip/userspace/src/Makefile.am| 2 +-
drivers/staging/usbip/userspace/src/usbip.c| 6 ++
drivers/staging/usbip/userspace/src/usbip.h| 1 +
drivers/staging/usbip/userspace/src/usbip_port.c | 57 ++
6 files changed, 134 insertions(+), 1 deletion(-)
create mode 100644 drivers/staging/usbip/userspace/src/usbip_port.c
diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
index 241006a..209df9b 100644
--- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
+++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
@@ -4,6 +4,8 @@
#include "usbip_common.h"
#include "vhci_driver.h"
+#include
+#include
#undef PROGNAME
#define PROGNAME "libusbip"
@@ -337,6 +339,29 @@ err:
return -1;
}
+static int read_record(int rhport, char *host, char *port, char *busid)
+{
+ FILE *file;
+ char path[PATH_MAX+1];
+
+ snprintf(path, PATH_MAX, VHCI_STATE_PATH"/port%d", rhport);
+
+ file = fopen(path, "r");
+ if (!file) {
+ err("fopen");
+ return -1;
+ }
+
+ if (fscanf(file, "%s %s %s\n", host, port, busid) != 3) {
+ err("fscanf");
+ fclose(file);
+ return -1;
+ }
+
+ fclose(file);
+
+ return 0;
+}
/* -- */
@@ -535,3 +560,45 @@ int usbip_vhci_detach_device(uint8_t port)
return 0;
}
+
+int usbip_vhci_imported_device_dump(struct usbip_imported_device *idev)
+{
+ char product_name[100];
+ char host[NI_MAXHOST] = "unknown host";
+ char serv[NI_MAXSERV] = "unknown port";
+ char remote_busid[SYSFS_BUS_ID_SIZE];
+ int ret;
+ int read_record_error = 0;
+
+ if (idev->status == VDEV_ST_NULL || idev->status == VDEV_ST_NOTASSIGNED)
+ return 0;
+
+ ret = read_record(idev->port, host, serv, remote_busid);
+ if (ret) {
+ err("read_record");
+ read_record_error = 1;
+ }
+
+ printf("Port %02d: <%s> at %s\n", idev->port,
+ usbip_status_string(idev->status),
+ usbip_speed_string(idev->udev.speed));
+
+ usbip_names_get_product(product_name, sizeof(product_name),
+ idev->udev.idVendor, idev->udev.idProduct);
+
+ printf(" %s\n", product_name);
+
+ if (!read_record_error) {
+ printf("%10s -> usbip://%s:%s/%s\n", idev->udev.busid,
+ host, serv, remote_busid);
+ printf("%10s -> remote bus/dev %03d/%03d\n", " ",
+ idev->busnum, idev->devnum);
+ } else {
+ printf("%10s -> unknown host, remote port and remote busid\n",
+ idev->udev.busid);
+ printf("%10s -> remote bus/dev %03d/%03d\n", " ",
+ idev->busnum, idev->devnum);
+ }
+
+ return 0;
+}
diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
b/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
index 89949aa..e071f80 100644
--- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
+++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
@@ -64,4 +64,6 @@ int usbip_vhci_attach_device(uint8_t port, int sockfd,
uint8_t busnum,
int usbip_vhci_detach_device(uint8_t port);
+int usbip_vhci_imported_device_dump(struct usbip_imported_device *idev);
+
#endif /* __VHCI_DRIVER_H */
diff --git a/drivers/staging/usbip/userspace/src/Makefile.am
b/drivers/staging/usbip/userspace/src/Makefile.am
index a113003..b4f8c4b 100644
--- a/drivers/staging/usbip/userspace/src/Makefile.am
+++ b/drivers/staging/usbip/userspace/src/Makefile.am
@@ -6,7 +6,7 @@ sbin_PROGRAMS := usbip usbipd
usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \
usbip_attach.c usbip_detach.c usbip_list.c \
-usbip_bind.c usbip_unbind.c
+usbip_bind.c usbip_unbind.c usbip_port.c
usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c
diff --git a/drivers/staging/usbip/userspace/src/usbip.c
b/drivers/staging/usbip/userspace/src/usbip.c
index 04a5f20..d7599d9 100644
--- a/drivers/staging/usbip/userspace/src/usbip
Re: [PATCH 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst
On 01/07/2014 05:58 AM, David Laight wrote: > The dmesg contains: > > [ 538.728064] EXT4-fs warning (device dm-0): ext4_end_bio:316: I/O error > writing to inode 23330865 (offset 0 size 8388608 starting block 812628) > > An 8MB transfer will need at least 128 ring entries (TRB) even if the request > is a single contiguous memory block. > > Are you using the patch that increases the ring size from 64 to 256? Yes, 256. I'll work on getting more debugging output. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] xhci: Allocate the td array and urb_priv together.
On Tue, Jan 07, 2014 at 09:29:30AM +, David Laight wrote:
> > From: 'David Cohen'
> > On Mon, Jan 06, 2014 at 09:26:20AM +, David Laight wrote:
> > > > From: David Cohen
> > > > On Fri, Dec 20, 2013 at 09:26:35AM -, David Laight wrote:
> > > > > > From: David Cohen
> > > > > The effect of this change is really to remove the first allocation and
> > > > > add 8 bytes (or maybe a pointer) to the start of the second one.
> > > > > So it is extremely unlikely to fail when the old code would work.
> > > >
> > > > Currently struct urb_priv has a dynamic array of pointers to struct
> > > > xhci_td. You're replacing the pointer by structs itself. Now, instead of
> > > > 2 kmallocs() (1 for urb_priv and another for size * xhci_td) we've 1
> > > > kmalloc() with urb_priv + size * xhci_td.
> > >
> > > You misread the old code.
> > > The first malloc was for the header and 'n' pointers.
> > > The second malloc was for 'n' copies of the structure.
> >
> > That's exactly what I described but in a more complicated way :)
> >
> > The new kmalloc is going to be "n * sizeof(struct) - n * sizeof(pointer)"
> > bigger. I don't know what is the usual range of values for "n", but my
> > experience with android devices with non-abundant memory size is that
> > they are sensible to kmalloc > PAGE_SIZE.
>
> It is much easier to assume that we are keeping the second malloc
> are getting rid of the first one. The header is only one pointer.
The header is not only one pointer. I believe the most relevant changes
from your patch happen here:
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1363,7 +1363,7 @@ struct xhci_scratchpad {
struct urb_priv {
int length;
int td_cnt;
- struct xhci_td *td[0];
+ struct xhci_td td[0];
};
This is a dynamic array. It's not 1 pointer, it's how many you want it
to be. See below:
/*
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1275,21 +1274,10 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb
*urb, gfp_t mem_flags)
size = 1;
urb_priv = kzalloc(sizeof(struct urb_priv) +
- size * sizeof(struct xhci_td *), mem_flags);
+ size * sizeof(struct xhci_td), mem_flags);
if (!urb_priv)
return -ENOMEM;
"sizeof(struct urb_priv)" does not consider the dynamic array at all, then
you have the second value "size * sizeof(struct xhci_td *)" where "size"
is the number of pointers you're going to have in the dynamic array.
By doing your change you're increasing in the size of kmalloc in
size * (sizeof(struct xhci_td) - sizeof(struct xhci_td *)) bytes.
>
> > Back to your patch description, you mention new kmalloc size may be
> > PAGE_SIZE + 8 bytes, which means kmalloc may have to find 2 consecutive
> > free pages. Of course it is not a big issue, but I don't see a reason to
> > unnecessarily change 2 kmalloc < PAGE_SIZE by one possibly > PAGE_SIZE.
>
> The values of 'n' are unknown. I doubt that the value that just exceeds
> a page happens any more often than those either side of it.
Knowing the regular range of values for "n" (AKA "size" in code above)
does matter in order to know if it's worth to replace 2 kmallocs by one.
SLAB/SLUB provide pretty fast way for smaller kmallocs.
Br, David Cohen
>
> David
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe 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.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst
Okay, I used log_buf_len to make dmesg bigger and now I think I have the whole thing. It's attached. dmesg2.gz Description: application/gzip
[PATCH v3 01/10] 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 may be a lie if tier mismatch is in effect. Platform firmware can
fix it after the port is registered.
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c |5
drivers/usb/core/hub.h |6
drivers/usb/core/port.c | 64 +++
3 files changed, 64 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 162e94dbed53..d86548edcc36 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -43,11 +43,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 4e4790dea343..b5efc713498e 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -81,6 +81,7 @@ struct usb_hub {
* @child: usb device attatched 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;
@@ -123,3 +125,7 @@ static inline int hub_port_debounce_be_stable(struct
usb_hub *hub,
return hub_port_debounce(hub, port1, false);
}
+static inline int hub_is_superspeed(struct usb_device *hdev)
+{
+ return hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS;
+}
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 51542f852393..8d3ec2daf6fe 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -21,6 +21,7 @@
#include "hub.h"
+DEFINE_SPINLOCK(peer_lock);
static const struct attribute_group *port_dev_group[];
static ssize_t connect_type_show(struct device *dev,
@@ -146,9 +147,37 @@ struct device_type usb_port_device_type = {
.pm = &usb_port_pm_ops,
};
+static struct usb_port *find_peer_port(struct usb_hub *hub, int port1)
+{
+ struct usb_device *hdev = hub->hdev;
+ struct usb_port *peer = NULL;
+
+ /* set the default peer port for root hubs. Platform firmware
+* can later fix this up if tier-mismatch is present. Assumes
+* the primary_hcd is usb2.0 and registered first
+*/
+ 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 (!hub_is_superspeed(hdev)
+ || WARN_ON_ONCE(!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;
+}
+
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,8 +193,21 @@ 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);
+ device_initialize(&port_dev->dev);
+
+ peer = find_peer_port(hub, port1);
+ dev_dbg(&hub->hdev->dev, "port%d peer = %s\n",
+ port1, peer ? dev_name(peer->dev.parent->parent) : "[none]");
+ if (peer) {
+ spin_lock(&peer_lock);
+ get_device(&peer->dev);
+ port_dev->peer = peer;
+ get_device(&port_dev->dev);
+ peer->peer = port_dev;
+ spin_unlock(&peer_lock);
+ }
- retval = device_register(&port_dev->dev);
+ retval = device_add(&port_dev->dev);
if (retval)
goto error_register;
@@ -188,9 +230,19 @@ exit:
return retval;
}
-void usb_hub_remove_port_device(struct usb_hub *hub,
- int port1)
+void usb_hub_remove_port_device(struct usb_hub *hub, int port1)
{
- device_unregister(&hub->ports[port1
[PATCH v3 02/10] usb: find 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
Signed-off-by: Dan Williams
---
drivers/usb/core/port.c | 23 +--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 8d3ec2daf6fe..7fd22020d7ee 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -150,15 +150,15 @@ struct device_type usb_port_device_type = {
static struct usb_port *find_peer_port(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;
/* set the default peer port for root hubs. Platform firmware
* can later fix this up if tier-mismatch is present. Assumes
* the primary_hcd is usb2.0 and registered first
*/
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_peer_port(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;
@@ -202,6 +220,7 @@ int usb_hub_create_port_device(struct usb_hub *hub, int
port1)
spin_lock(&peer_lock);
get_device(&peer->dev);
port_dev->peer = peer;
+ WARN_ON(peer->peer);
get_device(&port_dev->dev);
peer->peer = port_dev;
spin_unlock(&peer_lock);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 00/10] Just the essential port power control fixes for 3.14
Alan, Sarah, This revision boils down the port power control fixes to the bare minimum to get the implementation functional and reliable. Data structure changes are constrained to struct usb_port and gone are the clumsier attempts at wider reworks from v1 [1] and v2 [2]. No device model changes to consider or changes to the meaning of 'runtime_status' for port devices. Three disconnect bugs are fixed: 1/ Superspeed devices downgrade to their hi-speed connection: fix this by preventing superspeed poweroff until the peer port is suspended. See patch 5. 2/ khubd taking disconnect action on ports that are in the process of being recovered: khubd now ignores ports in the pm-runtime-suspended state. Alan, per your comment [3] this effectively uses the pm_usage counter and state as a lock against khubd. See patch 7. 3/ Superspeed devices fail to reconnect: Seen during repeated toggles of the port power state. Fixed by forcing a full recovery cycle of the device before allowing the next suspend, and blocking khubd while the resume is in progress. See patch 9. Patch overview: [PATCH 01/10] usb: assign default peer ports for root hubs [PATCH 02/10] usb: find external hub port peers [PATCH 03/10] usb: find internal hub tier mismatch via acpi [PATCH 04/10] usb: sysfs link peer ports * Per our discussions of v1 these patches implement a simple algorithm for associating peer ports across internal and external hubs. [PATCH 05/10] usb: defer suspension of superspeed port while peer is powered * Fix case 1 [PATCH 06/10] usb: gate clearing FEAT_C_ENABLE to usb2 hubs * Cleanup misuse of ClearPortFeature(PORT_C_ENABLE) [PATCH 07/10] usb: synchronize port poweroff and khubd * Fix case 2 [PATCH 08/10] usb: cleanup straggling C_PORT_RESET C_PORT_LINK_STATE notifications * Handle some unexpected hub events encountered during testing [PATCH 09/10] usb: make khubd and subsequent suspension wait for port recovery * Fix case 3 [PATCH 10/10] usb: documentation for usb port power off mechanisms These patches were tested by repeatedly power toggling all 35 ports in the following topology: /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/4p, 5000M |__ Port 1: Dev 3, If 0, Class=vend., Driver=ax88179_178a, 5000M |__ Port 2: Dev 4, If 0, Class=stor., Driver=usb-storage, 5000M |__ Port 4: Dev 5, If 0, Class=stor., Driver=usb-storage, 5000M /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/9p, 480M |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/4p, 480M |__ Port 3: Dev 4, If 0, Class=HID, Driver=usbhid, 12M |__ Port 2: Dev 3, If 0, Class=hub, Driver=hub/4p, 480M |__ Port 1: Dev 5, If 0, Class=HID, Driver=usbhid, 1.5M |__ Port 2: Dev 6, If 0, Class=HID, Driver=usbhid, 1.5M /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/2p, 480M |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/8p, 480M Each iteration of the test verifies that no disconnects occur and that all ports reach the 'suspended' state. To force device suspension interface drivers are unbound for power-off and then rebound. Note that since the hub drivers are never unbound their parent ports remain active due to ->do_remote_wakeup for the hub device, but the the 30 other ports reliably suspend and resume now with these patches. The proposed warm reset changes [4] do not appear to be required as long as superspeed hub parent ports remain powered. [1] http://marc.info/?l=linux-usb&m=138260013707007&w=2 [2] http://marc.info/?l=linux-usb&m=138511124910669&w=2 [3] http://marc.info/?l=linux-usb&m=138775577717546&w=2 [4] http://marc.info/?l=linux-usb&m=138759482824618&w=2 --- Dan Williams (9): usb: assign default peer ports for root hubs usb: find external hub port peers usb: find internal hub tier mismatch via acpi usb: sysfs link peer ports usb: don't suspend port while peer is powered usb: gate clearing FEAT_C_ENABLE to usb2 hubs usb: synchronize port poweroff and khubd usb: cleanup straggling C_PORT_RESET C_PORT_LINK_STATE notifications usb: make khubd and subsequent suspension wait for port recovery Lan Tianyu (1): USB: Documentation for USB port power off mechanisms Documentation/usb/power-management.txt | 210 +++ drivers/usb/core/hub.c | 112 ++ drivers/usb/core/hub.h | 14 ++ drivers/usb/core/port.c| 252 +++- drivers/usb/core/usb-acpi.c| 35 drivers/usb/core/usb.h |2 6 files changed, 574 insertions(+), 51 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://
[PATCH v3 04/10] usb: sysfs link peer ports
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 | 40 +++-
1 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 8fae3cd03305..d3aacf093aa1 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -21,7 +21,7 @@
#include "hub.h"
-DEFINE_SPINLOCK(peer_lock);
+DEFINE_MUTEX(peer_lock);
static const struct attribute_group *port_dev_group[];
static ssize_t connect_type_show(struct device *dev,
@@ -201,16 +201,20 @@ static void reset_peer(struct usb_port *port_dev, struct
usb_port *peer)
if (!peer)
return;
- spin_lock(&peer_lock);
- if (port_dev->peer)
+ mutex_lock(&peer_lock);
+ if (port_dev->peer) {
put_device(&port_dev->peer->dev);
- if (peer->peer)
+ sysfs_remove_link(&port_dev->dev.kobj, "peer");
+ }
+ if (peer->peer) {
put_device(&peer->peer->dev);
+ sysfs_remove_link(&peer->dev.kobj, "peer");
+ }
port_dev->peer = peer;
peer->peer = port_dev;
get_device(&peer->dev);
get_device(&port_dev->dev);
- spin_unlock(&peer_lock);
+ mutex_unlock(&peer_lock);
}
/* assumes that location data is only set for external connectors and that
@@ -311,19 +315,33 @@ int usb_hub_create_port_device(struct usb_hub *hub, int
port1)
dev_dbg(&hub->hdev->dev, "port%d peer = %s\n",
port1, peer ? dev_name(peer->dev.parent->parent) : "[none]");
if (peer) {
- spin_lock(&peer_lock);
+ mutex_lock(&peer_lock);
get_device(&peer->dev);
port_dev->peer = peer;
WARN_ON(peer->peer);
get_device(&port_dev->dev);
peer->peer = port_dev;
- spin_unlock(&peer_lock);
+ mutex_unlock(&peer_lock);
}
retval = device_add(&port_dev->dev);
if (retval)
goto error_register;
+ mutex_lock(&peer_lock);
+ peer = port_dev->peer;
+ do if (peer) {
+ retval = sysfs_create_link(&port_dev->dev.kobj,
+ &peer->dev.kobj, "peer");
+ if (retval)
+ break;
+ retval = sysfs_create_link(&peer->dev.kobj,
+ &port_dev->dev.kobj, "peer");
+ } while (0);
+ mutex_unlock(&peer_lock);
+ if (retval)
+ goto error_links;
+
pm_runtime_set_active(&port_dev->dev);
/* It would be dangerous if user space couldn't
@@ -339,6 +357,8 @@ int usb_hub_create_port_device(struct usb_hub *hub, int
port1)
error_register:
put_device(&port_dev->dev);
+error_links:
+ device_unregister(&port_dev->dev);
exit:
return retval;
}
@@ -348,14 +368,16 @@ void usb_hub_remove_port_device(struct usb_hub *hub, int
port1)
struct usb_port *port_dev = hub->ports[port1 - 1];
struct usb_port *peer = port_dev->peer;
- spin_lock(&peer_lock);
+ mutex_lock(&peer_lock);
if (peer) {
peer->peer = NULL;
port_dev->peer = NULL;
put_device(&port_dev->dev);
put_device(&peer->dev);
+ sysfs_remove_link(&port_dev->dev.kobj, "peer");
+ sysfs_remove_link(&peer->dev.kobj, "peer");
}
- spin_unlock(&peer_lock);
+ mutex_unlock(&peer_lock);
device_unregister(&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 v3 05/10] 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/port.c | 22 ++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index d3aacf093aa1..217a3c6df29e 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -77,12 +77,16 @@ 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;
+ 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 +108,10 @@ static int usb_port_runtime_resume(struct device *dev)
clear_bit(port1, hub->busy_bits);
usb_autopm_put_interface(intf);
+
+ if (!hub_is_superspeed(hdev) && peer)
+ pm_runtime_put(&peer->dev);
+
return retval;
}
@@ -113,6 +121,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;
@@ -123,6 +132,12 @@ static int usb_port_runtime_suspend(struct device *dev)
== PM_QOS_FLAGS_ALL)
return -EAGAIN;
+ /* block poweroff of superspeed ports while highspeed peer is on */
+ dev_WARN_ONCE(&hdev->dev, hub_is_superspeed(hdev) && !peer,
+ "port%d missing peer info\n", port1);
+ if (hub_is_superspeed(hdev) && (!peer || peer->power_is_on))
+ return -EBUSY;
+
usb_autopm_get_interface(intf);
set_bit(port1, hub->busy_bits);
retval = usb_hub_set_port_power(hdev, hub, port1, false);
@@ -130,6 +145,13 @@ 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);
+
+ /* bounce our peer now that we are down */
+ if (!hub_is_superspeed(hdev) && peer) {
+ pm_runtime_get(&peer->dev);
+ pm_runtime_put(&peer->dev);
+ }
+
return retval;
}
#endif
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 03/10] 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 (see Appendix D
figure 131 in the xhci spec), ACPI can override the default peer port
association (for internal hubs).
Location data is cached as an opaque cookie in usb_port_location data.
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.h |6 +++
drivers/usb/core/port.c | 96 +++
drivers/usb/core/usb-acpi.c | 35 +---
drivers/usb/core/usb.h |2 +
4 files changed, 131 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index b5efc713498e..2ba10798c943 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 attatched 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 7fd22020d7ee..8fae3cd03305 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -149,11 +149,14 @@ struct device_type usb_port_device_type = {
static struct usb_port *find_peer_port(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;
+
/* set the default peer port for root hubs. Platform firmware
* can later fix this up if tier-mismatch is present. Assumes
* the primary_hcd is usb2.0 and registered first
@@ -193,6 +196,97 @@ static struct usb_port *find_peer_port(struct usb_hub
*hub, int port1)
return peer;
}
+static void reset_peer(struct usb_port *port_dev, struct usb_port *peer)
+{
+ if (!peer)
+ return;
+
+ spin_lock(&peer_lock);
+ if (port_dev->peer)
+ put_device(&port_dev->peer->dev);
+ if (peer->peer)
+ put_device(&peer->peer->dev);
+ port_dev->peer = peer;
+ peer->peer = port_dev;
+ get_device(&peer->dev);
+ get_device(&port_dev->dev);
+ spin_unlock(&peer_lock);
+}
+
+/* assumes that location data is only set for external connectors and that
+ * external hubs never have tier mismatch
+ */
+static int redo_find_peer_port(struct device *dev, void *data)
+{
+ struct usb_port *port_dev = to_usb_port(dev);
+
+ if (is_usb_port(dev)) {
+ struct usb_device *hdev = to_usb_device(dev->parent->parent);
+ struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+ int port1 = port_dev->portnum;
+ struct usb_port *peer;
+
+ peer = find_peer_port(hub, port1);
+ reset_peer(port_dev, peer);
+ }
+
+ return device_for_each_child(dev, NULL, redo_find_peer_port);
+}
+
+static int pair_port(struct device *dev, void *data)
+{
+ struct usb_port *peer = data;
+ struct usb_port *port_dev = to_usb_port(dev);
+
+ if (!is_usb_port(dev)
+ || port_dev->location.cookie != peer->location.cookie)
+ return device_for_each_child(dev, peer, pair_port);
+
+ dev_dbg(dev->parent->parent, "port%d peer = %s port%d (by location)\n",
+ port_dev->portnum, dev_name(peer->dev.parent->parent),
+ peer->portnum);
+ if (port_dev->peer != peer) {
+ /* Sigh, tier mismatch has invalidated our ancestry.
+* This should not be too onerous even in deep hub
+* topologies as we will discover tier mismatch early
+* (after platform internal hubs have been enumerated),
+* before external hubs are probed.
+*/
+ reset_peer(port_dev, peer);
+ device_for_each_child(dev, NULL, redo_find_peer_port);
+ }
+
+ return true;
+}
+
+void usb_set_hub_port_location(struct usb_device *hdev, int port1,
+ u32 cookie)
+{
+ struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
+ struct usb_hcd *hcd = bus_to_hcd(hd
[PATCH v3 06/10] usb: gate clearing FEAT_C_ENABLE to usb2 hubs
The port pm_runtime implementation unconditionally clears FEAT_C_ENABLE after clearing PORT_POWER, but the bit is reserved on usb3 hub ports. It also takes steps to re-disable the port if it fails to reconnect which again is broken on usb3 ports and unnecessary on usb2 ports. Signed-off-by: Dan Williams --- drivers/usb/core/port.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 217a3c6df29e..97e4939fee1a 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -102,7 +102,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; } @@ -142,7 +141,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 v3 09/10] usb: make khubd and subsequent suspension wait for port recovery
While a device is going through reset-resume khubd may run and trigger a
disconnect since it sees the device not in the suspended state. This does not
happen in the hub_suspend() case because the hub->urb is dead and we clear the
connect status prior to restarting khubd. In the port suspend case khubd
remains active.
To close this race window arrange for port resume to trigger a child
device resume and then have khubd wait for that resume to complete
before taking action on the port. Similar to the hub_resume() case we
request a reset_resume to recover the power session.
This mechanism is also used to rate limit power toggling as the port will not
suspend again until the child device has been given a chance to wake up. This
mitigates devices downgrading their connection on perceived instability of
the host connection.
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c |1 +
drivers/usb/core/hub.h |2 ++
drivers/usb/core/port.c | 27 +++
3 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 80661e20de9e..5db2956c9a22 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4700,6 +4700,7 @@ static struct usb_port *next_active_port(struct usb_hub
*hub, int *port1)
pm_runtime_get_noresume(&port_dev->dev);
pm_runtime_barrier(&port_dev->dev);
+ flush_work(&port_dev->resume_work);
if (pm_runtime_active(&port_dev->dev))
return port_dev;
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 2ba10798c943..333d21495459 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -88,6 +88,7 @@ struct usb_port_location {
* @peer: related usb2 and usb3 ports (share the same connector)
* @connect_type: port's connect type
* @location: opaque representation of platform connector location
+ * @resume_work: arrange for child device resume when port resumes
* @portnum: port index num based one
* @power_is_on: port's power state
* @did_runtime_put: port has done pm_runtime_put().
@@ -99,6 +100,7 @@ struct usb_port {
struct usb_port *peer;
enum usb_port_connect_type connect_type;
struct usb_port_location location;
+ struct work_struct resume_work;
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 97e4939fee1a..773663312a95 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -67,9 +67,29 @@ static void usb_port_device_release(struct device *dev)
{
struct usb_port *port_dev = to_usb_port(dev);
+ flush_work(&port_dev->resume_work);
kfree(port_dev);
}
+static void port_dev_wake_child(struct work_struct *w)
+{
+ struct usb_port *port_dev;
+ struct usb_device *udev;
+
+ port_dev = container_of(w, typeof(*port_dev), resume_work);
+ udev = port_dev->child;
+
+ if (udev) {
+#ifdef CONFIG_PM
+ if (udev->persist_enabled)
+ udev->reset_resume = 1;
+#endif
+ usb_autoresume_device(udev);
+ usb_autosuspend_device(udev);
+ }
+ pm_runtime_put_sync(&port_dev->dev);
+}
+
#ifdef CONFIG_PM_RUNTIME
static int usb_port_runtime_resume(struct device *dev)
{
@@ -111,6 +131,12 @@ static int usb_port_runtime_resume(struct device *dev)
if (!hub_is_superspeed(hdev) && peer)
pm_runtime_put(&peer->dev);
+ /* keep this port awake until we have had a chance to recover
+* the child
+*/
+ pm_runtime_get_noresume(&port_dev->dev);
+ schedule_work(&port_dev->resume_work);
+
return retval;
}
@@ -330,6 +356,7 @@ int usb_hub_create_port_device(struct usb_hub *hub, int
port1)
port_dev->dev.parent = hub->intfdev;
port_dev->dev.groups = port_dev_group;
port_dev->dev.type = &usb_port_device_type;
+ INIT_WORK(&port_dev->resume_work, port_dev_wake_child);
dev_set_name(&port_dev->dev, "port%d", port1);
device_initialize(&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 v3 07/10] 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 | 38 +-
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d86548edcc36..0fa13a52b52a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4654,6 +4654,35 @@ static int hub_handle_remote_wakeup(struct usb_hub *hub,
unsigned int port,
return connect_change;
}
+static void put_port(struct usb_port *port_dev, int *port1)
+{
+ if (!port_dev)
+ return;
+
+ pm_runtime_put_sync(&port_dev->dev);
+ (*port1)++;
+}
+
+static struct usb_port *next_active_port(struct usb_hub *hub, int *port1)
+{
+ struct usb_device *hdev = hub->hdev;
+
+ while (*port1 <= hdev->maxchild) {
+ struct usb_port *port_dev = hub->ports[*port1 - 1];
+
+ pm_runtime_get_noresume(&port_dev->dev);
+ pm_runtime_barrier(&port_dev->dev);
+ if (pm_runtime_active(&port_dev->dev))
+ return port_dev;
+ put_port(port_dev, port1);
+ }
+
+ return NULL;
+}
+
+#define for_each_pm_active_port(i, p, h) \
+ for (i = 1; (p = next_active_port(h, &i)); put_port(p, &i))
+
static void hub_events(void)
{
struct list_head *tmp;
@@ -4661,6 +4690,7 @@ static void hub_events(void)
struct usb_interface *intf;
struct usb_hub *hub;
struct device *hub_dev;
+ struct usb_port *port_dev;
u16 hubstatus;
u16 hubchange;
u16 portstatus;
@@ -4739,7 +4769,7 @@ static void hub_events(void)
}
/* deal with port status changes */
- for (i = 1; i <= hdev->maxchild; i++) {
+ for_each_pm_active_port(i, port_dev, hub) {
if (test_bit(i, hub->busy_bits))
continue;
connect_change = test_bit(i, hub->change_bits);
@@ -4775,8 +4805,7 @@ static void hub_events(void)
* Works at least with mouse driver.
*/
if (!(portstatus & USB_PORT_STAT_ENABLE)
- && !connect_change
- && hub->ports[i - 1]->child) {
+ && !connect_change && port_dev->child) {
dev_err (hub_dev,
"port %i "
"disabled by hub (EMI?), "
@@ -4838,8 +4867,7 @@ static void hub_events(void)
*/
if (hub_port_warm_reset_required(hub, portstatus)) {
int status;
- struct usb_device *udev =
- hub->ports[i - 1]->child;
+ struct usb_device *udev = port_dev->child;
dev_dbg(hub_dev, "warm reset port %d\n", i);
if (!udev ||
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 10/10] 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 Signed-off-by: Sarah Sharp Signed-off-by: Dan Williams --- Documentation/usb/power-management.txt | 210 1 files changed, 210 insertions(+), 0 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index 1392b61d6ebe..e5e77a67abb7 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,194 @@ 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 requrire the attached child device and all descendants to be +suspended. + +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. + +Example of the relevant files for port power control. + + child device link + + port device + | +parent hub + | | + v v v + /sys/bus/devices/usb2/2-0:1.0/port1/device + + /sys/bus/devices/usb2/2-0:1.0/port1/power/pm_qos_no_power_off + /sys/bus/devices/usb2/2-0:1.0/port1/device/power/control + /sys/bus/devices/usb2/2-0:1.0/port1/device//driver/unbind + +In addition to these files some ports may have a 'peer' link to a port on +another hub. The expectation is that a
[PATCH v3 08/10] usb: cleanup straggling C_PORT_RESET C_PORT_LINK_STATE notifications
Port power testing sometimes result in a port being powered-off while
C_PORT_RESET and C_PORT_LINK_STATE are active. Per the spec these bits
should be automatically cleared by being in the logical powered off
state. Handle this for hubs that do not follow the spec.
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 68 ++--
1 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 0fa13a52b52a..80661e20de9e 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4663,6 +4663,34 @@ static void put_port(struct usb_port *port_dev, int
*port1)
(*port1)++;
}
+static void hub_clear_misc_change(struct usb_hub *hub, int port1,
+ u16 portstatus, u16 portchange)
+{
+ struct device *hub_dev = hub->intfdev;
+ struct usb_device *hdev = hub->hdev;
+
+ if (portchange & USB_PORT_STAT_C_RESET) {
+ dev_dbg(hub_dev, "reset change on port %d\n", port1);
+ usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_RESET);
+ }
+ if ((portchange & USB_PORT_STAT_C_BH_RESET)
+ && hub_is_superspeed(hdev)) {
+ dev_dbg(hub_dev, "warm reset change on port %d\n", port1);
+ usb_clear_port_feature(hdev, port1,
+ USB_PORT_FEAT_C_BH_PORT_RESET);
+ }
+ if (portchange & USB_PORT_STAT_C_LINK_STATE) {
+ dev_dbg(hub_dev, "link state change on port %d\n", port1);
+ usb_clear_port_feature(hdev, port1,
+ USB_PORT_FEAT_C_PORT_LINK_STATE);
+ }
+ if (portchange & USB_PORT_STAT_C_CONFIG_ERROR) {
+ dev_warn(hub_dev, "config error on port %d\n", port1);
+ usb_clear_port_feature(hdev, port1,
+ USB_PORT_FEAT_C_PORT_CONFIG_ERROR);
+ }
+}
+
static struct usb_port *next_active_port(struct usb_hub *hub, int *port1)
{
struct usb_device *hdev = hub->hdev;
@@ -4674,6 +4702,19 @@ static struct usb_port *next_active_port(struct usb_hub
*hub, int *port1)
pm_runtime_barrier(&port_dev->dev);
if (pm_runtime_active(&port_dev->dev))
return port_dev;
+
+ /* handle out of spec hubs that continue to signal reset and
+* link-state change events while the port is powered-off
+*/
+ if (test_bit(*port1, hub->event_bits)) {
+ u16 stat, change;
+ int ret;
+
+ ret = hub_port_status(hub, *port1, &stat, &change);
+ if (ret == 0)
+ hub_clear_misc_change(hub, *port1, stat,
+ change);
+ }
put_port(port_dev, port1);
}
@@ -4835,32 +4876,7 @@ static void hub_events(void)
"condition on port %d\n", i);
}
- if (portchange & USB_PORT_STAT_C_RESET) {
- dev_dbg (hub_dev,
- "reset change on port %d\n",
- i);
- usb_clear_port_feature(hdev, i,
- USB_PORT_FEAT_C_RESET);
- }
- if ((portchange & USB_PORT_STAT_C_BH_RESET) &&
- hub_is_superspeed(hub->hdev)) {
- dev_dbg(hub_dev,
- "warm reset change on port %d\n",
- i);
- usb_clear_port_feature(hdev, i,
- USB_PORT_FEAT_C_BH_PORT_RESET);
- }
- if (portchange & USB_PORT_STAT_C_LINK_STATE) {
- usb_clear_port_feature(hub->hdev, i,
-
USB_PORT_FEAT_C_PORT_LINK_STATE);
- }
- if (portchange & USB_PORT_STAT_C_CONFIG_ERROR) {
- dev_warn(hub_dev,
- "config error on port %d\n",
- i);
- usb_clear_port_feature(hub->hdev, i,
-
USB_PORT_FEAT_C_PORT_CONFIG_ERROR);
- }
+ hub_clear_misc_change(hub, i, portstatus, portchange);
/* Warm reset a USB3 protocol port if it's in
* SS.Inactive state.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info a
Re: [Bug 68161] Unstable work of xhci with USB3.0 card reader and UDMA7 CompactFlash card.
Since reporting this bug I've invested some time to get myself familiar with USB protocol and analyzed attached capture files. It seems like device resetoccurs after device returns urb_status=-75 (-EOVERFLOW). This can be seen in attachment https://bugzilla.kernel.org/attachment.cgi?id=120901 in packet #1987. Also I've noticed that host tries to read device by chunks of 240 sectors while device returns on each query no more than 120 sectors (61440 bytes). From traffic it is clearly seen that EOVERFLOW occurs after the device is already mounted and while software tries to browse it's content. When I do something like 'dd if=/dev/sdb of=/dev/null' where sdb is CF card or mount and copy with shell commands host<->device communication scheme is the same (240 sectors requested, 120 returned), but this doesn't lead to EOVERFLOW. In that cases read speed is at about 80Mb/s. So I suppose that something wrong happens only while software like thunar or midnight commander tries to browse the contents of card (maybe parallel threads trying to access card simultaneously?). With that knowledge I've tried to tweak some device parameters in /sys filesystem. When I put value 60 in /sys/block/sd?/queue/max_sectors_kb then all operates correctly without any resets. However in this case read speed of card drops down by factor of two at around 40Mb/s. When I set max_sectors_kb to 64 then device does reset upon mount in thunar, however, surprisingly, this doesn't lead to dropping of device mount, like in case of default value of 120. In this case read speed is at about 89.5Mb/s, as expected by card specs. So I've added udev rule that corrects value of max_sectors_kb to 64 upon device connection. For now I can live with this 10 seconds latency of device mounting if latter it works properly. However I think that the reason of this issue must be clarified and fixed. Also tried to set queue/scheduler to noop with no effect. In case of USB2.0 host<->device traffic looks pretty the same way as in case of USB3.0. Host also tries to read device by chunks of 240 sectors and device returns only 120. However for some reason this doesn't lead to EOVERFLOW. Main difference I've managed to find between usb 2.0 and 3.0 traffic is the device initialization. In case of 3.0 there are some CLEAR_FEATURE/SET_FEATURE requests that are missing in case of 2.0, so maybe device operates differently by that reason. I'm going to investigate further. ps My main kernel for now is 3.10.17-gentoo, all that written above is also true for this version too. -- To unsubscribe from this list: send the line "unsubscribe 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] ohci-platform: Add support for devicetree instantiation
Hi, On 01/06/2014 04:49 PM, Alan Stern wrote: On Mon, 6 Jan 2014, Hans de Goede wrote: Add support for ohci-platform instantiation from devicetree, including optionally getting clks and a phy from devicetree, and enabling / disabling those on power_on / off. This should allow using ohci-platform from devicetree in various cases. Specifically after this commit it can be used for the ohci controller found on Allwinner sunxi SoCs. Signed-off-by: Hans de Goede --- .../devicetree/bindings/usb/platform-ohci.txt | 25 drivers/usb/host/ohci-platform.c | 146 ++--- 2 files changed, 151 insertions(+), 20 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/platform-ohci.txt diff --git a/Documentation/devicetree/bindings/usb/platform-ohci.txt b/Documentation/devicetree/bindings/usb/platform-ohci.txt new file mode 100644 index 000..6846f1c --- /dev/null +++ b/Documentation/devicetree/bindings/usb/platform-ohci.txt @@ -0,0 +1,25 @@ +Generic Platform OHCI controller + +Required properties: + - compatible: Should be "platform-ohci" + - reg: Address range of the ohci registers. + - interrupts: Should contain the ohci interrupt. + +Optional properties: + - clocks: array of clocks + - clock-names: clock names "ahb" and/or "ohci" Where does "ahb" come from, what does it mean, and how is it relevant to generic platforms? ahb is an ARM specific thing, so your right it does not belong in a generic driver. I'll use clk1 and clk2 as names in my next version. What about platforms that use 3 clocks? Ah yes I see some platforms have 3 clocks, I'll also add a clk3. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ohci-platform: Add support for devicetree instantiation
Hi, On 01/06/2014 04:45 PM, Mark Rutland wrote: On Sun, Jan 05, 2014 at 11:04:39PM +, Hans de Goede wrote: Add support for ohci-platform instantiation from devicetree, including optionally getting clks and a phy from devicetree, and enabling / disabling those on power_on / off. This should allow using ohci-platform from devicetree in various cases. Specifically after this commit it can be used for the ohci controller found on Allwinner sunxi SoCs. Signed-off-by: Hans de Goede --- .../devicetree/bindings/usb/platform-ohci.txt | 25 drivers/usb/host/ohci-platform.c | 146 ++--- 2 files changed, 151 insertions(+), 20 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/platform-ohci.txt diff --git a/Documentation/devicetree/bindings/usb/platform-ohci.txt b/Documentation/devicetree/bindings/usb/platform-ohci.txt new file mode 100644 index 000..6846f1c --- /dev/null +++ b/Documentation/devicetree/bindings/usb/platform-ohci.txt @@ -0,0 +1,25 @@ +Generic Platform OHCI controller + +Required properties: + - compatible: Should be "platform-ohci" To me, "platform-ohci" seems rather Linux-specific. Platform devices are a Linux abstraction that don't correspond to any particular bus type in reality. We have some "*-generic" bindings. A name of that form might be more appropriate. Or we could choose an arbitrary OHCI implementation's vendor,ochi string and everything else can declare compatibility with that. With the ehci patch I'll go for your suggestion to simply keep via,vt8500-ehci as compat string for it, without adding a new generic name. So for ohci I'll also go with the first platform to use it and thus "allwinner,sun4i-ohci" + - reg: Address range of the ohci registers. + - interrupts: Should contain the ohci interrupt. + +Optional properties: + - clocks: array of clocks + - clock-names: clock names "ahb" and/or "ohci" A description of what the clocks are might be helpful. How about something like: - clocks: a list of phandle + clock specifier pairs, one for each entry in clock-names. - clock-names: Should contain: * "ahb" - some description here. * "ohci" - some description here. As Alan pointed out by asking "what is ahb" these names are too platform specific (arm platform in this case), for a generic driver. So I'm going to call them clk1, clk2 and instead. It may seem a bit silly to use names at all in this case, but having names makes things a lot easier implementation wise. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ohci-platform: Add support for devicetree instantiation
On Tuesday 07 January 2014 22:03:11 Hans de Goede wrote: > >> + > >> +Optional properties: > >> + - clocks: array of clocks > >> + - clock-names: clock names "ahb" and/or "ohci" > > > > Where does "ahb" come from, what does it mean, and how is it relevant > > to generic platforms? > > ahb is an ARM specific thing, so your right it does not belong in a > generic driver. I'll use clk1 and clk2 as names in my next version. While AHB is a bus created by ARM Ltd, it's not actually specific to the ARM architecture. My guess is that it is in fact used on 95% of all SoCs, so I would leave it at that. For the other clock, I think that's actually the bus clock for the USB interface, so I would not call it "ohci" but rather just "usb" or "phy". I think it's important to distinguish the names and not just use "clk1" and "clk2", because the driver may actually want to access a particular clock in some scenario. > > What about platforms that use 3 clocks? > > Ah yes I see some platforms have 3 clocks, I'll also add a clk3. I guess we should try to find at least one hardware data sheet for an actual ohci implementation and look at what the clock inputs are really called. A lot of the drivers seem to incorrectly use the name for the clock signal inside of the soc, which tends to be named after who provides it, not what it's used for. 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 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst
On Tue, Jan 07, 2014 at 05:29:48AM -0800, walt wrote:
> On 01/06/2014 04:31 PM, Sarah Sharp wrote:
> > Hi Walt,
> >
> > I have a couple of patches for you to test.
>
> > Please only apply the first patch (which is diagnostic only), trigger
> > your issue, and send me the resulting dmesg. Then try applying the
> > other two patches, and see if the issue goes away. (I suspect it won't
> > but I can't be sure.)
>
> Thanks Sarah. dmesg0 is from the diagnostic patch only. dmesg1 has all
> three patches applied. Some of the messages in dmesg1 fell off the end of
> the kernel buffer, so I may need to make the buffer larger next time but
> I'll need a reminder of how to do it.
Set CONFIG_LOG_BUF_SHIFT to 21.
> As you suspected, the patches didn't fix the problem, sorry.
Yep, I thought so. I did glean one bit of information from the logs: it
seems that your host does handle no-op TRBs, at least for a while.
However, after a bigger chunk of TRBs, it goes off into la-la-land.
Assuming one of the rings is comprised of two segments:
0xbb711000 (start)
0xbb7113f0 (end)
0xbb711400 (start)
0xbb7117f0 (end)
The log show no-ops were inserted at:
0xbb7207d0
0xbb7206a0
0xbb720be0
0xbb720be0
0xbb720bd0
0xbb7207e0
0xbb711370 = 8 no-ops
0xbb7117c0 = 3
0xbb7113b0 = 4
0xbb7113a0 = 5
0xbb7117d0 = 2
0xbb711340 = 11
0xbb711770 = 8
0xbb711230 = 28
0xbb7117e0 = 1
0xbb7117b0 = 4
0xbb7113d0 = 2
0xbb7117b0 = 4
0xbb711340 = 11
0xbb711690 = 22
So the host was able to process 28 no-op TRBs, but failed on 22 no-ops
later. The event ring debugging shows the last event was for
0xbb711680, which is the last TRB before the first no-op inserted before
the host died. There's no Stop Endpoint Command completion, and it
looks like the command was correctly put on the command ring, so it
seems the host is actually hanging for some reason.
Unfortunately, I made a mistake in the debugging patch I sent
you, so it didn't print out the endpoint rings when the host died. I
need that info, to see whether the link TRB was still intact, or if we
over-wrote it and caused the host to go fetch some invalid memory.
Can you please try the attached patch, on top of the previous three
patches, and send me dmesg?
> I find that I can tell in advance whether the copy is going to succeed,
> just by watching the light flicker on the usb3 drive. When the flicker
> is absolutely regular, with no variation whatever, I can tell in 10 or
> 15 seconds that the copy will fail.
>
> At the same time the light on the main drive goes dark after 10 seconds,
> implying that the usb3 drive stops receiving any data from the main drive
> after 10 seconds, yet the light on the usb3 drive continues to flicker as
> if writing data -- even after the cp officially fails. The light on the
> usb3 drive never stops flickering until I reboot the machine or unplug
> the usb cable.
Interesting. Without a USB analyzer, we can't really tell what's
happening. However, one hypothesis could be that the blinking light is
triggered by an active SCSI command (read/write, etc).
There are three phases of the command: setup, data, and status. I think
your device is getting the setup phase, and the host is dying before it
sends the data phase. If the light blinks when it gets a setup phase,
and turns off when the devices sends a status phase, that would explain
its behavior.
But that's just a hypothesis, I have no idea whether it's correct.
Sarah Sharp
>From d085fb5b9630e935d7954fe5947b48402e43bdc1 Mon Sep 17 00:00:00 2001
From: Sarah Sharp
Date: Tue, 7 Jan 2014 12:39:47 -0800
Subject: [PATCH] More debugging.
Signed-off-by: Sarah Sharp
---
drivers/usb/host/xhci-ring.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2afaf15009e8..228ab8cf868e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -993,6 +993,9 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
for (i = 0; i < MAX_HC_SLOTS; i++) {
if (!xhci->devs[i])
continue;
+
+ xhci_dbg(xhci, "Slot %d output context\n", i);
+ xhci_dbg_ctx(xhci, xhci->devs[i]->out_ctx, 30);
for (j = 0; j < 31; j++) {
temp_ep = &xhci->devs[i]->eps[j];
ring = temp_ep->ring;
@@ -1001,6 +1004,10 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Killing URBs for slot ID %u, "
"ep index %u", i, j);
+ xhci_dbg(xhci, "Dev %i Ep 0x%x:\n", i,
+ xhci_get_endpoint_address(j));
+ xhci_debug_ring(xhci, ring);
+ xhci_dbg_ring_ptrs(xhci, ring);
while (!list_empty(&ring->td_list)) {
cur_td = list_first_entry(&ring->td_list,
struct xhci_td,
@@ -1011,12 +1018,6 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
xhci_giveback_urb_in_irq(xhci, cur_td,
-ESHUTDOWN, "killed");
}
- if (!list_empty(&temp_ep->cancelled_td_list)) {
-xhci_dbg(xhci, "Dev %i Ep 0x%x:\n", i,
Re: [PATCH] usb: core: get config and string descriptors for unauthorized devices
Hi, has there been any movement on this patch? It happens to fix a problem that we have been experiencing with races between authorize/deauthorize and device removal. I'd like to know if this is going to be considered for merge or if I'll have to look elsewhere for a solution. -- To unsubscribe from this list: send the line "unsubscribe 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] ohci-platform: Add support for devicetree instantiation
Hi, On 01/07/2014 10:16 PM, Arnd Bergmann wrote: On Tuesday 07 January 2014 22:03:11 Hans de Goede wrote: + +Optional properties: + - clocks: array of clocks + - clock-names: clock names "ahb" and/or "ohci" Where does "ahb" come from, what does it mean, and how is it relevant to generic platforms? ahb is an ARM specific thing, so your right it does not belong in a generic driver. I'll use clk1 and clk2 as names in my next version. While AHB is a bus created by ARM Ltd, it's not actually specific to the ARM architecture. My guess is that it is in fact used on 95% of all SoCs, so I would leave it at that. For the other clock, I think that's actually the bus clock for the USB interface, so I would not call it "ohci" but rather just "usb" or "phy". I think it's important to distinguish the names and not just use "clk1" and "clk2", because the driver may actually want to access a particular clock in some scenario. The idea here is to have a generic driver, if a driver needs to know about a specific clock, it will likely be another device specific driver and it can use its own dt-bindings and clock names. I believe that for a generic driver meant to cover common hardware configs, simply having X clks and then on power_on enabling clk1, then clk2, then clk3, etc. and on power off do the same in reverse other is a good approach. What about platforms that use 3 clocks? Ah yes I see some platforms have 3 clocks, I'll also add a clk3. I guess we should try to find at least one hardware data sheet for an actual ohci implementation and look at what the clock inputs are really called. A lot of the drivers seem to incorrectly use the name for the clock signal inside of the soc, which tends to be named after who provides it, not what it's used for. I don't know about data-sheets, but an example of a driver with 3 clocks is drivers/usb/host/ohci-at91.c fwiw it uses "uhpck", "hclk" and "usb_clk" as clk names. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst
On Tue, Jan 07, 2014 at 01:58:32PM +, David Laight wrote: > The dmesg contains: > > [ 538.728064] EXT4-fs warning (device dm-0): ext4_end_bio:316: I/O error > writing to inode 23330865 (offset 0 size 8388608 starting block 812628) > > An 8MB transfer will need at least 128 ring entries (TRB) even if the request > is a single contiguous memory block. > > Are you using the patch that increases the ring size from 64 to 256? It's likely that the block layer is breaking up the EXT4 write into several transfers, since usb-storage limits overall transfer size to 120 KB. In any case, I added more debugging in the last patch to print the number of TRBs necessary. That way we can verify the patch to limit the number of scatter-gather list entries is working. 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] RFC: Allow to blacklist xhci_hcd module and use ports with EHCI
On Tue, Jan 07, 2014 at 10:32:08AM -0500, Alan Stern wrote: > > I think in my case quirk_usb_handoff_xhci is called during boot that > > will switch the ports to the XHCI. > > That's what I said. Early PCI processing occurs during boot. Sure, I was referring to "run only when the computer is turned off". > > One possibility: noxhci-port-switch Okay. Any opinion of having xhci-hcd switch the routing during module unloading? I will prepare a boot param patch this week. holger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: core: get config and string descriptors for unauthorized devices
On Tue, Jan 07, 2014 at 01:21:15PM -0800, Julius Werner wrote: > Hi, has there been any movement on this patch? It happens to fix a problem > that we have been experiencing with races between authorize/deauthorize and > device removal. You are using wireless usb? > I'd like to know if this is going to be considered for merge or if > I'll have to look elsewhere for a solution. It's queued up for 3.14-rc1 (you can always check linux-next for this type of thing...) Do you also need/want this in older (i.e. stable) kernel releases? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: core: get config and string descriptors for unauthorized devices
>> I'd like to know if this is going to be considered for merge or if >> I'll have to look elsewhere for a solution. > > It's queued up for 3.14-rc1 (you can always check linux-next for this > type of thing...) Do you also need/want this in older (i.e. stable) > kernel releases? Oh right... sorry, I should've had a closer look first. (I guess Google doesn't index git.kernel.org that well or I would've found it.) I personally don't care about the stable backport, but it does work around a rare use-after-free bug of the udev->config pointer (when the usb_set_configuration(-1) in usb_deauthorize_device() fails because the device is already unplugged, but it still runs usb_destroy_configuration() afterwards), so you might want to consider it. > You are using wireless usb? Nope, but we are using the sysfs "authorized" node to force a reconfiguration and driver rebinding in certain cases. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci_hcd and Canon Lide 110 not playing well together
On Wed, Dec 25, 2013 at 09:51:28PM -0500, Alan Stern wrote: > Okay, now we know that usb_enable_interface takes a long time. That > routine does nothing but call usb_enable_endpoint, which does nothing > but call usb_hcd_reset_endpoint, which calls the xhci_endpoint_reset > routine. > > So we have traced the problem into xhci-hcd. You could trace it > farther down if you want, but at this point it probably would be more > productive to see what Sarah has to say. She may know about some new > changes that are not yet available in the development kernel. I suspect the patch I asked Matthias and Holger to apply [1] has some issues, and it's entirely possible that there's deadlocks. One other tester (Michal) confirms it fixes his issue with a Samsung scanner, but the patch leads to list corruption in some xHCI structures: https://bugzilla.kernel.org/show_bug.cgi?id=47421 In any case, all three testers (Matthias, Holger, and Michal) confirm the patch fixes the underlying issue. So we just need to get the remaining race conditions and locking issues fixed up. Xenia, would you mind if Dan or I finished your patch? I know you don't have much time for xHCI work since you started your new job. Sarah Sharp [1] http://marc.info/?l=linux-usb&m=138116117104619&w=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: [PATCH] usb: core: get config and string descriptors for unauthorized devices
On Tue, Jan 07, 2014 at 01:45:29PM -0800, Julius Werner wrote: > >> I'd like to know if this is going to be considered for merge or if > >> I'll have to look elsewhere for a solution. > > > > It's queued up for 3.14-rc1 (you can always check linux-next for this > > type of thing...) Do you also need/want this in older (i.e. stable) > > kernel releases? > > Oh right... sorry, I should've had a closer look first. (I guess > Google doesn't index git.kernel.org that well or I would've found it.) > I personally don't care about the stable backport, but it does work > around a rare use-after-free bug of the udev->config pointer (when the > usb_set_configuration(-1) in usb_deauthorize_device() fails because > the device is already unplugged, but it still runs > usb_destroy_configuration() afterwards), so you might want to consider > it. Ok, as you need it, it makes sense to add it to the stable kernels, I'll do it after 3.14-rc1. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/10] Just the essential port power control fixes for 3.14
On Tue, Jan 07, 2014 at 12:29:28PM -0800, Dan Williams wrote: > Alan, Sarah, > > This revision boils down the port power control fixes to the > bare minimum to get the implementation functional and reliable. > Data structure changes are constrained to struct usb_port and > gone are the clumsier attempts at wider reworks from v1 [1] and > v2 [2]. No device model changes to consider or changes to the > meaning of 'runtime_status' for port devices. Three disconnect > bugs are fixed: It's too late for 3.14, sorry. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/10] Just the essential port power control fixes for 3.14
On Tue, Jan 7, 2014 at 1:58 PM, Greg KH wrote: > On Tue, Jan 07, 2014 at 12:29:28PM -0800, Dan Williams wrote: >> Alan, Sarah, >> >> This revision boils down the port power control fixes to the >> bare minimum to get the implementation functional and reliable. >> Data structure changes are constrained to struct usb_port and >> gone are the clumsier attempts at wider reworks from v1 [1] and >> v2 [2]. No device model changes to consider or changes to the >> meaning of 'runtime_status' for port devices. Three disconnect >> bugs are fixed: > > It's too late for 3.14, sorry. > C'est la vie. The bulk of these changes do not rise to the level of regression fix since port power off was unreliable from the beginning. However, please consider cherry-picking "[PATCH 06/10] usb: gate clearing FEAT_C_ENABLE to usb2 hubs" out of this series. We're causing request errors unnecessarily by sending this reserved value to usb3 hub ports. -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] xhci: Switch Intel Lynx Point ports to EHCI on shutdown
On Tue, Jan 07, 2014 at 11:03:00AM +0100, Takashi Iwai wrote: > At Mon, 06 Jan 2014 14:34:28 +0200, > Denis Turischev wrote: > > > > Hi Sarah, > > > > On 01/03/2014 02:03 AM, Sarah Sharp wrote: > > > Denis, do all of Compulab's Haswell systems reboot on shutdown? Are > > > they all running a Phoenix BIOS? Can you send me the output of `sudo > > > lspci -vvv -s` for the xHCI host? > > > > oem@oem-Intense-PC2 ~ $ sudo lspci -vvv -s 00:14.0 > > 00:14.0 USB controller: Intel Corporation Lynx Point-LP USB xHCI HC (rev > > 04) (prog-if 30 [XHCI]) > > Subsystem: Intel Corporation Device 7270 > > Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- > > Stepping- SERR- FastB2B- DisINTx+ > > Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- > > SERR- > Latency: 0 > > Interrupt: pin A routed to IRQ 59 > > Region 0: Memory at f062 (64-bit, non-prefetchable) [size=64K] > > Capabilities: [70] Power Management version 2 > > Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA > > PME(D0-,D1-,D2-,D3hot+,D3cold+) > > Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- > > Capabilities: [80] MSI: Enable+ Count=1/8 Maskable- 64bit+ > > Address: fee0200c Data: 41b1 > > Kernel driver in use: xhci_hcd > > > > > Basically, I'm trying to find a common variable to key off. I suspect > > > BIOS vendor is probably the right thing, instead of system vendor. Hmm, since Compulab isn't the subsystem vendor, we can't enable the same HP quirk using that piece of information. We can't enable the quirk to put the host into D3 for all Lynx Point-LP hosts, since that quirk breaks other vendors' systems. Does this impact any Lynx Point (non-LP) systems as well? So far, two of the other systems that don't react well to the quirk are both ASRock systems with American Megatrends BIOSes, based on info provided by Art and Meng. I can see from Giorgos' posted lspci that his xHCI also lists ASRock as the Subsystem vendor, although I don't know what the BIOS manufacturer is. Niklas's xHCI subsystem VID:PID is 1558:7410, which is CLEVO/KAPOK Computer Device. Looks like Clevo is a laptop manufacturer. Giorgos and Niklas, can you post output from `sudo dmidecode` please? > > By the way the quirk introduced by commit > > e95829f474f0db3a4d940cae1423783edd966027 "xhci: Switch PPT > > ports to EHCI on shutdown." works for Lynx Point as well at least on > > Intense-PC2. I mean we can add > > XHCI_SPURIOUS_REBOOT flag that invokes usb_disable_xhci_ports(). > > May be this solution works for HP and other systems without side effects? > > No, we already tested it at first, but didn't fix the behavior on HP > machines. It was harmless as far as we've tested, though. Denis, what do you mean by "works for Lynx Point"? Do you mean that adding the quirk to switch the ports on EHCI on shutdown (e95829f474) for the Intense-PC2 *instead of* the commit to put the host in D3 on shutdown (638298dc66) works? Or do you mean you need both patches for your system? If you only need the quirk to switch the ports to EHCI on shutdown, then we could apply that broadly to Lynx Point LP, and see whether other BIOSes tolerate that quirk. The alternative would be to turn on the D3 quirk for systems with an HP or Phoenix BIOS, by checking dmi_name_in_vendors() for those strings. 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 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst
On Tue, Jan 07, 2014 at 12:00:00PM -0800, walt wrote: > Okay, I used log_buf_len to make dmesg bigger and now I think I have > the whole thing. It's attached. Walt, can you make sure the patch I sent you was applied? The output doesn't look like it is. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/2] dual scan thread bug fix
On Sat, 2014-01-04 at 11:27 -0500, Alan Stern wrote:
> On Fri, 3 Jan 2014, James Bottomley wrote:
>
> > > I'm still concerned about one thing. The previous patch does this in
> > > scsi_alloc_target():
> > >
> > > > found:
> > > > - found_target->reap_ref++;
> > > > + if (!kref_get_unless_zero(&found_target->reap_ref))
> > > > + /*
> > > > +* release routine already fired. Target is dead, but
> > > > +* STARGET_DEL may not yet be set (set in the release
> > > > +* routine), so set here as well, just in case
> > > > +*/
> > > > + found_target->state = STARGET_DEL;
> > > > spin_unlock_irqrestore(shost->host_lock, flags);
> > >
> > > As a result, the two comments in this patch aren't right:
> > >
> > > > @@ -384,9 +385,15 @@ static void scsi_target_reap_ref_release(struct
> > > > kref *kref)
> > > > struct scsi_target *starget
> > > > = container_of(kref, struct scsi_target, reap_ref);
> > > >
> > > > - transport_remove_device(&starget->dev);
> > > > - device_del(&starget->dev);
> > > > - starget->state = STARGET_DEL;
> > > > + /*
> > > > +* if we get here and the target is still in the CREATED state
> > > > that
> > > > +* means it was allocated but never made visible (because a scan
> > > > +* turned up no LUNs), so don't call device_del() on it.
> > > > +*/
> > > > + if (starget->state == STARGET_RUNNING) {
> > > > + transport_remove_device(&starget->dev);
> > > > + device_del(&starget->dev);
> > > > + }
> > >
> > > Here the state could already be STARGET_DEL, even though the target is
> > > still visible.
> >
> > Well, I agree with the theory. In practise, there are only a few
> > machine instructions between the kref going to zero and us reaching that
> > point, because kref_release will jump into this routine next, so the
> > condition would be very hard to see.
>
> It's true that the window is very small and not at all likely to be
> hit. Still, I prefer eliminating such things entirely.
>
> > However, I suppose it's easy to
> > close by checking for != STARGET_CREATED and there's no reason not to do
> > that, so I'll change it.
>
> Checking for != STARGET_CREATED is also wrong in principle. The state
> could already be STARGET_DEL even though the target was never made
> visible.
>
> The basic problem is that you are relying on the state to be an
> accurate description of the target's visibility, but
> scsi_alloc_target() changes the state without changing the visibility.
> I really think you should get rid of that extra assignment in
> scsi_alloc_target().
OK, Agreed, but that means modifying the 1/2 patch with the below. This
should make the proposed diff to 2/2 correct.
James
---
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index ef3f958..5fad646 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -417,7 +417,7 @@ static struct scsi_target *scsi_alloc_target(struct device
*parent,
+ shost->transportt->target_size;
struct scsi_target *starget;
struct scsi_target *found_target;
- int error;
+ int error, ref_got;
starget = kzalloc(size, GFP_KERNEL);
if (!starget) {
@@ -466,15 +466,15 @@ static struct scsi_target *scsi_alloc_target(struct
device *parent,
return starget;
found:
- if (!kref_get_unless_zero(&found_target->reap_ref))
- /*
-* release routine already fired. Target is dead, but
-* STARGET_DEL may not yet be set (set in the release
-* routine), so set here as well, just in case
-*/
- found_target->state = STARGET_DEL;
+ /*
+* release routine already fired if kref is zero, so if we can still
+* take the reference, the target must be alive. If we can't, it must
+* be dying and we need to wait for a new target
+*/
+ ref_got = kref_get_unless_zero(&found_target->reap_ref);
+
spin_unlock_irqrestore(shost->host_lock, flags);
- if (found_target->state != STARGET_DEL) {
+ if (ref_got) {
put_device(dev);
return found_target;
}
@@ -482,8 +482,8 @@ static struct scsi_target *scsi_alloc_target(struct device
*parent,
* Unfortunately, we found a dying target; need to wait until it's
* dead before we can get a new one. There is an anomaly here. We
* *should* call scsi_target_reap() to balance the kref_get() of the
-* reap_ref above. However, since the target is in state STARGET_DEL,
-* it's already invisible and the reap_ref is irrelevant. If we call
+* reap_ref above. However, since the target being released, it's
+* already invisible and the reap_ref is irrelevant. If we ca
Re: [PATCH v3 00/10] Just the essential port power control fixes for 3.14
On Tue, Jan 07, 2014 at 02:21:08PM -0800, Dan Williams wrote: > On Tue, Jan 7, 2014 at 1:58 PM, Greg KH wrote: > > On Tue, Jan 07, 2014 at 12:29:28PM -0800, Dan Williams wrote: > >> Alan, Sarah, > >> > >> This revision boils down the port power control fixes to the > >> bare minimum to get the implementation functional and reliable. > >> Data structure changes are constrained to struct usb_port and > >> gone are the clumsier attempts at wider reworks from v1 [1] and > >> v2 [2]. No device model changes to consider or changes to the > >> meaning of 'runtime_status' for port devices. Three disconnect > >> bugs are fixed: > > > > It's too late for 3.14, sorry. > > > > C'est la vie. > > The bulk of these changes do not rise to the level of regression fix > since port power off was unreliable from the beginning. However, > please consider cherry-picking "[PATCH 06/10] usb: gate clearing > FEAT_C_ENABLE to usb2 hubs" out of this series. We're causing request > errors unnecessarily by sending this reserved value to usb3 hub ports. I'll let Sarah deal with that, as they can come through her tree if they are needed for 3.14-final. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] usb: ehci: add freescale imx28 special write register method
On Mon, Jan 06, 2014 at 09:42:26AM +0100, Marc Kleine-Budde wrote: > Hello Peter and Greg, > > On 01/06/2014 03:10 AM, Peter Chen wrote: > > According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB > > register error issue", All USB register write operations must > > use the ARM SWP instruction. So, we implement a special ehci_write > > for imx28. > > > > Discussion for it at below: > > http://marc.info/?l=linux-usb&m=137996395529294&w=2 > > > > Signed-off-by: Peter Chen > > Acked-by: Alan Stern > > Signed-off-by: Marc Kleine-Budde > > Tested-by: Marc Kleine-Budde > > please add stable on Cc for this and the next two patches: > > [PATCH 4/8] usb: ehci: add freescale imx28 special write register method > [PATCH 5/8] usb: chipidea: add freescale imx28 special write register method > [PATCH 6/8] usb: chipidea: imx: set CI_HDRC_IMX28_WRITE_FIX for imx28 How do those patches meet the Documentation/stable_kernel_rules.txt guidelines? -- To unsubscribe from this list: send the line "unsubscribe 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/8] usb: ehci: add freescale imx28 special write register method
On Mon, Jan 06, 2014 at 10:10:41AM +0800, Peter Chen wrote:
> According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB
> register error issue", All USB register write operations must
> use the ARM SWP instruction. So, we implement a special ehci_write
> for imx28.
>
> Discussion for it at below:
> http://marc.info/?l=linux-usb&m=137996395529294&w=2
>
> Signed-off-by: Peter Chen
> Acked-by: Alan Stern
> Signed-off-by: Marc Kleine-Budde
> Tested-by: Marc Kleine-Budde
> ---
> drivers/usb/host/ehci.h | 18 +-
> 1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index c35a6e2b..e26099b 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -225,6 +225,7 @@ struct ehci_hcd { /* one per controller */
> unsignedhas_synopsys_hc_bug:1; /* Synopsys HC */
> unsignedframe_index_bug:1; /* MosChip (AKA NetMos) */
> unsignedneed_oc_pp_cycle:1; /* MPC834X port power */
> + unsignedimx28_write_fix:1; /* For Freescale i.MX28 */
>
> /* required for usb32 quirk */
> #define OHCI_CTRL_HCFS (3 << 6)
> @@ -728,6 +729,18 @@ static inline unsigned int ehci_readl(const struct
> ehci_hcd *ehci,
> #endif
> }
>
> +#ifdef CONFIG_SOC_IMX28
> +static inline void imx28_ehci_writel(const unsigned int val,
> + volatile __u32 __iomem *addr)
> +{
> + __asm__ ("swp %0, %0, [%1]" : : "r"(val), "r"(addr));
> +}
> +#else
> +static inline void imx28_ehci_writel(const unsigned int val,
> + volatile __u32 __iomem *addr)
> +{
> +}
> +#endif
> static inline void ehci_writel(const struct ehci_hcd *ehci,
> const unsigned int val, __u32 __iomem *regs)
> {
> @@ -736,7 +749,10 @@ static inline void ehci_writel(const struct ehci_hcd
> *ehci,
> writel_be(val, regs) :
> writel(val, regs);
> #else
> - writel(val, regs);
> + if (IS_ENABLED(CONFIG_SOC_IMX28) && ehci->imx28_write_fix)
> + imx28_ehci_writel(val, regs);
> + else
> + writel(val, regs);
> #endif
This IS_ENABLED() isn't needed at all, so please remove it.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/8] usb: chipidea: add freescale imx28 special write register method
On Mon, Jan 06, 2014 at 10:10:42AM +0800, Peter Chen wrote:
> According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB
> register error issue", All USB register write operations must
> use the ARM SWP instruction. So, we implement special hw_write
> and hw_test_and_clear for imx28.
>
> Discussion for it at below:
> http://marc.info/?l=linux-usb&m=137996395529294&w=2
>
> Signed-off-by: Peter Chen
> Signed-off-by: Marc Kleine-Budde
> Tested-by: Marc Kleine-Budde
> ---
> drivers/usb/chipidea/ci.h| 26 --
> drivers/usb/chipidea/core.c |2 ++
> drivers/usb/chipidea/host.c |1 +
> include/linux/usb/chipidea.h |1 +
> 4 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index a71dc1c..e5506ce 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -164,6 +164,7 @@ struct hw_bank {
> * @id_event: indicates there is an id event, and handled at ci_otg_work
> * @b_sess_valid_event: indicates there is a vbus event, and handled
> * at ci_otg_work
> + * @imx28_write_fix: Freescale imx28 needs swp instruction for writing
> */
> struct ci_hdrc {
> struct device *dev;
> @@ -202,6 +203,7 @@ struct ci_hdrc {
> struct dentry *debugfs;
> boolid_event;
> boolb_sess_valid_event;
> + boolimx28_write_fix;
> };
>
> static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> @@ -250,6 +252,26 @@ static inline u32 hw_read(struct ci_hdrc *ci, enum
> ci_hw_regs reg, u32 mask)
> return ioread32(ci->hw_bank.regmap[reg]) & mask;
> }
>
> +#ifdef CONFIG_SOC_IMX28
> +static inline void imx28_ci_writel(u32 val, volatile void __iomem *addr)
> +{
> + __asm__ ("swp %0, %0, [%1]" : : "r"(val), "r"(addr));
> +}
> +#else
> +static inline void imx28_ci_writel(u32 val, volatile void __iomem *addr)
> +{
> +}
> +#endif
> +
> +static inline void __hw_write(struct ci_hdrc *ci, u32 val,
> + void __iomem *addr)
> +{
> + if (IS_ENABLED(CONFIG_SOC_IMX28) && ci->imx28_write_fix)
> + imx28_ci_writel(val, addr);
Same here, this IS_ENABLED() isn't needed, right?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] usb: chipidea: put hw_phymode_configure before ci_usb_phy_init
On Mon, Jan 06, 2014 at 10:10:45AM +0800, Peter Chen wrote: > From: Chris Ruehl > > hw_phymode_configure configures the PORTSC registers and allow the > following phy_inits to operate on the right parameters. This fix a problem > where the UPLI (ISP1504) could not be detected, because the Viewport was not > available and read the viewport return 0's only. > > Signed-off-by: Chris Ruehl > Signed-off-by: Peter Chen > --- > drivers/usb/chipidea/core.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) Why isn't this ok for the stable trees? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
On Mon, Jan 06, 2014 at 10:10:44AM +0800, Peter Chen wrote: > From: Chris Ruehl > > * init the sts flag to 0 (missed) > * fix write the real bit not sts value > * Set PORTCS_STS and DEVLC_STS only if sts = 1 > > Signed-off-by: Chris Ruehl > Signed-off-by: Peter Chen > --- > drivers/usb/chipidea/core.c |8 +--- > 1 files changed, 5 insertions(+), 3 deletions(-) Why isn't this patch ok for the -stable trees? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usbcore: fix BABBLE failed enumeration of legacy USB2 devices on USB3 bus
On Tue, Dec 24, 2013 at 04:19:18AM +, Marius Silaghi wrote: > From: Marius C Silaghi > > This patch is generated against the last kernel version in the github kernel > repository. We work off of the git.kernel.org trees, not github :) > Some older families of USB2 cameras (STC-XUSB) do not support querying > only the first 8 bytes of their > device descriptor and therefore fail at enumeration on USB3 HCDs, with babble > error -75 as they send more than > the expected 8 bytes. The proposed patch extends the mechanism used for non > USB3 HCDs in the first part of > the same function, and successively tries to query both the 8 byte prefix of > the device descriptor, as well as > the whole device descriptor (in case the old style query of the 8 byte prefix > fails). > In fact, for the cameras I try to fix, the preferred condition is the > negation of the one in the proposed patch, > "if (!USE_NEW_SCHEME(retry_counter))", to try first the version successful on > this case, but I keep the > current order of the "if" branches to ensure clean continuation of support > for other supported devices. > > Signed-off-by: Marius C Silaghi I'll let Sarah take this patch, if it passes her testing. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst
On Tue, Jan 07, 2014 at 03:57:00PM -0800, walt wrote: > On 01/07/2014 01:21 PM, Sarah Sharp wrote: > > > Can you please try the attached patch, on top of the previous three > > patches, and send me dmesg? > > Hi Sarah, I just now finished running 0001-More-debugging.patch for the > first time. The previous dmesg didn't include that patch, but this one > does. > > I read through this dmesg but I nodded off somewhere around line 500. > I hope you can stay awake :) Well, it has all the info I need, but the results don't make me too happy. Everything I've checked seems consistent, and I don't know why the host stopped. The link TRBs are intact, the dequeue pointer for the endpoint was pointing to the transfer that timed out and it had the cycle bit set correctly, etc. Perhaps the no-op TRBs are really the issue. I'll have to take a look at the log again tomorrow. I posted the dmesg on pastebin if David wants to check it out as well: http://pastebin.com/a4AUpsL1 Can you send me the output of `sudo lspci -vvv -n`? Maybe we can just turn off scatter-gather for your host controller until we get a proper fix in that uses link TRBs instead of no-op TRBs. 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
[RFC/PATCH] usb/xhci: avoid kernel panic on xhci_suspend()
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. Signed-off-by: jianqian Signed-off-by: David Cohen --- This is the kernel panic. The bug was discovered on current LTS kernel 3.10, as showed on logs. But the problem does not seem to be fixed so far. Maybe we should consider apply it on kernel >= 3.10? [5.037841] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002 [5.037854] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [5.037864] usb usb1: Product: xHCI Host Controller [5.037875] usb usb1: Manufacturer: Linux 3.10.16-261428-g07aa93a xhci_hcd [5.037885] usb usb1: SerialNumber: :00:14.0 [5.038358] xHCI xhci_add_endpoint called for root hub [5.038365] xHCI xhci_check_bandwidth called for root hub [5.038553] hub 1-0:1.0: USB hub found [5.038587] hub 1-0:1.0: 6 ports detected [5.189460] BUG: unable to handle kernel NULL pointer dereference at 0198 [5.189485] IP: [] xhci_suspend+0x22/0x2e0 [5.189499] *pdpt = *pde = f000f3b5f000f3b0- [5.189514] Oops: [#1] PREEMPT SMP- [5.189524] Modules linked in: [5.189539] CPU: 3 PID: 67 Comm: kworker/3:1 Not tainted 3.10.16-261428-g07aa93a #39 [5.189558] Workqueue: pm pm_runtime_work [5.189565] task: f2be5fa0 ti: f26d task.ti: f26d [5.189576] EIP: 0060:[] EFLAGS: 00010246 CPU: 3 [5.189587] EIP is at xhci_suspend+0x22/0x2e0 [5.189594] EAX: EBX: f2738a00 ECX: c17eb820 EDX: 0001 [5.189602] ESI: f2721000 EDI: f3054064 EBP: f26d1da4 ESP: f26d1d84 [5.189611] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 [5.189619] CR0: 8005003b CR2: 0198 CR3: 020fa000 CR4: 001007f0 [5.189626] DR0: DR1: DR2: DR3: [5.189632] DR6: 0ff0 DR7: 0400 [5.189636] Stack: [5.189668] f26b0b90 f26d1da4 c1af6a35 f2738a00 c1fdcba4 f3054064 f2721000 f3054064 [5.189697] f26d1db4 c17eb84d f2738a00 f26d1dd0 c17c40d8 0001 f26d1dd8 [5.189726] f3054064 f30540d4 c1b8fbc0 f26d1df4 c17c41b3 f26d1e0c c1255fbc [5.189730] Call Trace: [5.189752] [] ? sub_preempt_count+0x55/0xe0 [5.189773] [] xhci_pci_suspend+0x2d/0x40 [5.189792] [] suspend_common+0x98/0x150 [5.189810] [] hcd_pci_runtime_suspend+0x23/0x60 [5.189827] [] ? __queue_work+0x10c/0x2f0 [5.189842] [] ? usb_suspend_both+0x135/0x1a0 [5.189861] [] pci_pm_runtime_suspend+0x54/0x110 [5.189875] [] ? sub_preempt_count+0x55/0xe0 [5.189894] [] ? pci_pm_runtime_resume+0xb0/0xb0 [5.189907] [] __rpm_callback+0x2f/0x80 [5.189922] [] rpm_callback+0x28/0x80 [5.189937] [] rpm_suspend+0x100/0x5f0 [5.189956] [] __pm_runtime_suspend+0x41/0x80 [5.189972] [] ? pci_uevent+0x120/0x120 [5.189988] [] pci_pm_runtime_idle+0x38/0x50 [5.190002] [] __rpm_callback+0x2f/0x80 [5.190016] [] rpm_idle+0x101/0x230 [5.190032] [] pm_runtime_work+0x92/0xb0 [5.190048] [] process_one_work+0x11d/0x3d0 [5.190062] [] ? add_preempt_count+0x7d/0xf0 [5.190076] [] ? sub_preempt_count+0x55/0xe0 [5.190093] [] worker_thread+0xf9/0x320 [5.190109] [] ? _raw_spin_unlock_irqrestore+0x26/0x50 [5.190126] [] ? rescuer_thread+0x2b0/0x2b0 [5.190140] [] kthread+0x94/0xa0 [5.190154] [] ? sub_preempt_count+0x55/0xe0 [5.190177] [] ret_from_kernel_thread+0x1b/0x28 [5.190190] [] ? kthread_create_on_node+0xc0/0xc0 [5.190377] Code: 00 00 8d bc 27 00 00 00 00 55 89 e5 57 56 53 83 ec 14 3e 8d 74 26 00 8b 18 89 c6 83 bb 98 01 00 00 04 0f 85 79 02 00 00 8b 40 04 <83> b8 98 01 00 00 04 0f 85 69 02 00 00 f0 80 a3 58 01 00 00 fb [5.190396] EIP: [] xhci_suspend+0x22/0x2e0 SS:ESP 0068:f26d1d84 [5.190401] CR2: 0198 [5.190440] ---[ end trace b828c61ee573eafd ]--- [5.196507] Kernel panic - not syncing: Fatal exception [5.567347] Rebooting in 40 seconds.. [ 45.589915] ACPI MEMORY or I/O RESET_REG. drivers/usb/host/xhci.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index f052272b25c8..97c1d58f7baf 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -853,7 +853,14 @@ int xhci_suspend(struct xhci_hcd *xhci) struct usb_hcd *hcd = xhci_to_hcd(xhci); u32 command; - if (hcd->state != HC_STATE_SUSPENDED || + /* +* Need to check if xhci->shared_hcd is null to avoid kernel panic when +* boot up and xhci did not allocate xh
Re: [RFC/PATCH] usb/xhci: avoid kernel panic on xhci_suspend()
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? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] usb/xhci: avoid kernel panic on xhci_suspend()
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. > > Signed-off-by: jianqian > Signed-off-by: David Cohen > --- > > This is the kernel panic. The bug was discovered on current LTS kernel 3.10, > as > showed on logs. But the problem does not seem to be fixed so far. > Maybe we should consider apply it on kernel >= 3.10? How do you trigger this? I've never seen anyone report this problem before, is there something different in the hardware you are using that enables this to be triggered easier? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC/PATCH] usb/xhci: avoid kernel panic on xhci_suspend()
Hi, 1) I met this issue one time just boot up our Linux Platform(Kernel3.10) with XHCI driver, then kernel panic happen. And this issue reported once by other internal team. Nothing special of reproduce step and do not need special Hardware I think. Just random issue which will happen when meet the timing condition. 2) This issue is introduced by this patch: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=596d789a211d134dc5f94d1e5957248c204ef850 which set all hub autosuspend delay to 0. This causes race condition during XHCI driver initialization, After USB2 hcd and USB2 root hub finish the initialization, USB2 root hub is functional and auto suspend right now, hence trigger XHCI runtime suspend flow; At the same time, XHCI driver continue to initialize the USB3 hcd and assign to xhci->shared_hcd after finish the initialization; Since xhci_suspend() use the xhci->shared_hcd, so there is race condition that when XHCI runtime suspend called, xhci->shared_hcd still NULL. I think this patch is a fix solution since before XHCI finish the whole initialization, USB2 root hub triggered runtime suspend is mean less and do not need to handle. Thanks! -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Wednesday, January 08, 2014 9:47 AM To: David Cohen Cc: st...@rowland.harvard.edu; sarah.a.sh...@linux.intel.com; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Tang, Jianqiang Subject: Re: [RFC/PATCH] usb/xhci: avoid kernel panic on xhci_suspend() 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. > > Signed-off-by: jianqian > Signed-off-by: David Cohen > --- > > This is the kernel panic. The bug was discovered on current LTS kernel > 3.10, as showed on logs. But the problem does not seem to be fixed so far. > Maybe we should consider apply it on kernel >= 3.10? How do you trigger this? I've never seen anyone report this problem before, is there something different in the hardware you are using that enables this to be triggered easier? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: xHCI driver Bug report (Babble reported at enumeration for cam that works in Windows)
I tried the patch you pointed and it has a similar effect. The cameras work
with it as well.
From: Sarah Sharp [sarah.a.sh...@linux.intel.com]
Sent: Friday, January 03, 2014 15:06
To: Marius Silaghi
Cc: linux-usb@vger.kernel.org
Subject: Re: xHCI driver Bug report (Babble reported at enumeration for cam
that works in Windows)
On Fri, Jan 03, 2014 at 12:48:26AM +, Marius Silaghi wrote:
> Thanks
> Running 3.12.4
>
> During the holiday I managed to solve my problem with the following small
> patch (could it be added to the mainstream kernel such that it would be
> solved when we upgrade to the future kernels?):
Hi Marius,
Please try the patch I pointed you to instead of your patch. Dan's
patch may have the same impact yours does, and I need to know whether
his patch helps, since that's what's currently queued for the upcoming
3.14 kernel.
Sarah Sharp
>
> Signed-off-by: Marius C Silaghi
> ---
> --- linux/drivers/usb/core/hub.c.orig 2013-12-23 22:09:50.545093470 -0500
> +++ linux/drivers/usb/core/hub.c2013-12-23 22:29:40.521043702 -0500
> @@ -4197,7 +4197,19 @@ hub_port_init (struct usb_hub *hub, stru
> break;
> }
>
> - retval = usb_get_device_descriptor(udev, 8);
> + /* Try first the old 8-byte query (since it may be expected
> +* by some devices), but then give at least a chance to the
> +* new form (retrieving the whole descriptor)!
> +* Querying the content of the whole structure is required
> +* for older USB2 video cameras which do not support partial
> +* descriptor queries, like the STC-XXXUSB family.
> +* Windows also supports them.
> +*/
> + if (USE_NEW_SCHEME(retry_counter))
> + retval = usb_get_device_descriptor(udev, 8);
> + else
> + retval = usb_get_device_descriptor(udev,
> + sizeof(struct usb_device_descriptor));
> if (retval < 8) {
> if (retval != -ENODEV)
> dev_err(&udev->dev,--
>
>
> From: Sarah Sharp [sarah.a.sh...@linux.intel.com]
> Sent: Thursday, January 02, 2014 19:24
> To: Marius Silaghi
> Cc: linux-usb-ow...@vger.kernel.org
> Subject: Re: xHCI driver Bug report (Babble reported at enumeration for cam
> that works in Windows)
>
> On Sat, Dec 21, 2013 at 04:40:21PM +, Marius Silaghi wrote:
> >
> Hi Marius,
>
> > There is a bug in xHCI. The camera STC-MC33USB that is recognized on the
> > same computer&port with Windows fails to be enumerated with Babble (-75).
> >
> > I recompiled the kernel with DEBUG on for XHCI and DEBUG_USB.
> >
> > I attach the following files:
> > - dmesg_cam_connect (cat /proc/kmsg output during plugging the USB
> > connector)
> > - dmesg_cam_disconnect_and_poll (cat /proc/kmsg during a normal xhci ring
> > poll and then a camera usb unplugging)
> > -lsusb_ehci_Sentech (lsusb -v on a system with ahci where the camera is
> > recognized)
> > -lsusb_ehci_nothing (lsusb -v on the same system with ahci with nothing
> > plugged in)
> > -lsusb_xhci_Sentech (lsusb -v on the system with xhci where the camera is
> > recognized)
> > -lsusb_xhci_nothing (lsusb -v on the same system with xhci with nothing
> > plugged in)
>
> Thanks for the bug report. Which kernel version are you running?
>
> I think this issue might be fixed in a patch that's queued for the 3.14
> kernel. Can you apply the following patch to 3.13-rc6 and see if it
> helps?
>
> http://marc.info/?l=linux-usb&m=138672064109890&w=2
>
> If you need directions on how to compile a custom kernel, please see
> http://kernelnewbies.org/KernelBuild
>
> 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] usbcore: fix BABBLE failed enumeration of legacy USB2 devices on USB3 bus
Great observation,
Sarah located a patch that is queued for the 3.14 kernel and that has a similar
effect. So future kernels could work with that one as well.
The patch I provided (being very small and safe) can still be suggested for
maintainers of older kernels in various long-term maintained distributions (if
you know who is doing that).
Here are some versions of the patch I made for current kernels:
The next one was tested on Ubuntu, applied to the source for 3.5.0-17-generic
(Ubuntu)
--- linux-3.5.0/drivers/usb/core/hub.c.orig 2014-01-07 18:16:01.997031650
-0500
+++ linux-3.5.0/drivers/usb/core/hub.c 2014-01-07 18:19:41.617022465 -0500
@@ -4043,7 +4043,11 @@
break;
}
- retval = usb_get_device_descriptor(udev, 8);
+ if (!USE_NEW_SCHEME(retry_counter))
+ retval = usb_get_device_descriptor(udev, 8);
+ else
+ retval = usb_get_device_descriptor(udev,
+ sizeof(struct usb_device_descriptor));
if (retval < 8) {
dev_err(&udev->dev,
"device descriptor read/8, error %d\n",
For kernel 3.9.0-0.4
--- linux-3.5.0/drivers/usb/core/hub.c.orig 2014-01-07 18:16:01.997031650
-0500
+++ linux-3.5.0/drivers/usb/core/hub.c 2014-01-07 18:19:41.617022465 -0500
@@ -4043,7 +4043,11 @@
break;
}
- retval = usb_get_device_descriptor(udev, 8);
+ if (!USE_NEW_SCHEME(retry_counter))
+ retval = usb_get_device_descriptor(udev, 8);
+ else
+ retval = usb_get_device_descriptor(udev,
+ sizeof(struct usb_device_descriptor));
if (retval < 8) {
dev_err(&udev->dev,
"device descriptor read/8, error %d\n",
For kernel 3.10.0-5.15
--- ubuntu-saucy/drivers/usb/core/hub.c.orig2014-01-07 16:52:41.300835262
-0500
+++ ubuntu-saucy/drivers/usb/core/hub.c 2014-01-07 16:54:53.612829730 -0500
@@ -4126,8 +4126,11 @@
if (USE_NEW_SCHEME(retry_counter) &&
!(hcd->driver->flags & HCD_USB3))
break;
}
-
- retval = usb_get_device_descriptor(udev, 8);
+ if (!USE_NEW_SCHEME(retry_counter))
+ retval = usb_get_device_descriptor(udev, 8);
+ else
+ retval = usb_get_device_descriptor(udev,
+ sizeof(struct usb_device_descriptor));
if (retval < 8) {
if (retval != -ENODEV)
dev_err(&udev->dev,
For kernel 3.11
--- linux-3.11/drivers/usb/core/hub.c.orig 2014-01-07 16:57:16.352823760
-0500
+++ linux-3.11/drivers/usb/core/hub.c 2014-01-07 16:58:10.168821508 -0500
@@ -4161,7 +4161,11 @@
break;
}
- retval = usb_get_device_descriptor(udev, 8);
+ if (!USE_NEW_SCHEME(retry_counter))
+ retval = usb_get_device_descriptor(udev, 8);
+ else
+ retval = usb_get_device_descriptor(udev,
+ sizeof(struct usb_device_descriptor));
if (retval < 8) {
if (retval != -ENODEV)
dev_err(&udev->dev,
From: linux-usb-ow...@vger.kernel.org [linux-usb-ow...@vger.kernel.org] on
behalf of Greg Kroah-Hartman [gre...@linuxfoundation.org]
Sent: Tuesday, January 07, 2014 19:32
To: Marius Silaghi
Cc: Sarah Sharp; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Alan
Stern; Lan Tianyu; Xenia Ragiadakou; Jiri Kosina
Subject: Re: [PATCH] usbcore: fix BABBLE failed enumeration of legacy USB2
devices on USB3 bus
On Tue, Dec 24, 2013 at 04:19:18AM +, Marius Silaghi wrote:
> From: Marius C Silaghi
>
> This patch is generated against the last kernel version in the github kernel
> repository.
We work off of the git.kernel.org trees, not github :)
> Some older families of USB2 cameras (STC-XUSB) do not support querying
> only the first 8 bytes of their
> device descriptor and therefore fail at enumeration on USB3 HCDs, with babble
> error -75 as they send more than
> the expected 8 bytes. The proposed patch extends the mechanism used for non
> USB3 HCDs in the first part of
> the same function, and successively tries to query both the 8 byte prefix of
> the device descriptor, as well as
> the whole device descriptor (in case the old style query of the 8 byte prefix
> fails).
> In fact, for the cameras I try to fix, the preferred condition is the
> negation of the one in the proposed
Re: [RFC/PATCH] usb/xhci: avoid kernel panic on xhci_suspend()
A: No. Q: Should I include quotations after my reply? http://daringfireball.net/2007/07/on_top On Wed, Jan 08, 2014 at 03:49:07AM +, Tang, Jianqiang wrote: > Hi, > 1) I met this issue one time just boot up our Linux Platform(Kernel3.10) > with XHCI driver, then kernel panic happen. > >And this issue reported once by other internal team. > >Nothing special of reproduce step and do not need special Hardware I think. > >Just random issue which will happen when meet the timing condition. > > 2) This issue is introduced by this patch: > > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=596d789a211d134dc5f94d1e5957248c204ef850 > >which set all hub autosuspend delay to 0. That patch was released in a kernel almost a full year ago, yet we have never had a report of this happening before, so are you sure this patch is the root cause? >This causes race condition during XHCI driver initialization, > > After USB2 hcd and USB2 root hub finish the initialization, USB2 root hub > is functional and auto suspend right now, hence trigger XHCI runtime suspend > flow; > > At the same time, XHCI driver continue to initialize the USB3 hcd and > assign to xhci->shared_hcd after finish the initialization; > > Since xhci_suspend() use the xhci->shared_hcd, so there is race condition > that when XHCI runtime suspend called, xhci->shared_hcd still NULL. > > I think this patch is a fix solution since before XHCI finish the whole > initialization, USB2 root hub triggered runtime suspend is mean less and do > not need to handle. With this patch applied, does the crash go away? -- To unsubscribe from this list: send the line "unsubscribe 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: Asymmetric speed results with testusb/usbtest/g_zero
On Tue, Jan 07, 2014 at 07:41:27PM +0800, Conor O'Gorman wrote: > Hi, > > I'm seeing peak rates of about 50 Mbps write speeds, and 150 Mbps read. > Setup is host AMD SB700/SB800/Hudson-2/3 and Atheros SoC, using testusb > tool with usbtest module and g_zero gadget module on TI AM335x. > > Is this to be expected? > Is this a limitation of the controllers or the test setup? Most likely should be limitation of driver. > Can anyone point me at speed numbers for testusb/usbtest/g_zero? I can provide you figures with dwc3 in high speed mode. Bulk In - 257 Mbps (testusb -a -c 10 -t 2 -s 512000 completes in 0.159 sec) Bulk out - 130 Mbps (testusb -a -c 10 -t 1 -s 512000 completes in 0.315 sec) pattern=2 option does not verify data.If you use pattern=2 with both g_zero and usbtest module, you can achieve Bulk Out also as 257 Mbps, while for Bulk In you can reach to 290 Mbps. But if I remember well, there was some minor bug with pattern=2 implementation. I had fixed it locally and probably forgot to upstream. Using bigger buflen with g_zero (may be buflen=409600) you can reach to 344 Mbps while Bulk In and a slight improvement with Bulk Out (271). Regards Pratyush > > This is using tests 5 and 6 from testusb, which read/write various > chained block sizes for various counts. The speeds mentioned are for the > largest 32KB block size, on both the AMD and Atheros. > > Thanks, > Conor > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] usb/xhci: avoid kernel panic on xhci_suspend()
Hi Greg, On Tue, Jan 07, 2014 at 08:16:20PM -0800, Greg KH wrote: > > A: No. > Q: Should I include quotations after my reply? > > http://daringfireball.net/2007/07/on_top > > On Wed, Jan 08, 2014 at 03:49:07AM +, Tang, Jianqiang wrote: > > Hi, > > 1) I met this issue one time just boot up our Linux Platform(Kernel3.10) > > with XHCI driver, then kernel panic happen. > > > >And this issue reported once by other internal team. > > > >Nothing special of reproduce step and do not need special Hardware I > > think. > > > >Just random issue which will happen when meet the timing condition. > > > > 2) This issue is introduced by this patch: > > > > > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=596d789a211d134dc5f94d1e5957248c204ef850 > > > >which set all hub autosuspend delay to 0. > > That patch was released in a kernel almost a full year ago, yet we have > never had a report of this happening before, so are you sure this patch > is the root cause? This bug happened in a platform with 1 usb3 host controller + 1 usb3 OTG controller (Jianqiang, please correct me if I'm wrong). How common is this configuration out there? Br, David Cohen > > >This causes race condition during XHCI driver initialization, > > > > After USB2 hcd and USB2 root hub finish the initialization, USB2 root > > hub is functional and auto suspend right now, hence trigger XHCI runtime > > suspend flow; > > > > At the same time, XHCI driver continue to initialize the USB3 hcd and > > assign to xhci->shared_hcd after finish the initialization; > > > > Since xhci_suspend() use the xhci->shared_hcd, so there is race > > condition that when XHCI runtime suspend called, xhci->shared_hcd still > > NULL. > > > > I think this patch is a fix solution since before XHCI finish the whole > > initialization, USB2 root hub triggered runtime suspend is mean less and do > > not need to handle. > > With this patch applied, does the crash go away? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module
On 01/07/2014 08:43 PM, Arnd Bergmann wrote:
> On Tuesday 07 January 2014, Roger Quadros wrote:
>> USB Host driver (drivers/mfd/omap-usb-host.c) expects the 60MHz
>> reference clock to be named "init_60m_fclk". Provide this
>> information.
>>
>> Signed-off-by: Roger Quadros
>> ---
>> arch/arm/boot/dts/omap5.dtsi | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
>> index 2f12a47..e0ab379 100644
>> --- a/arch/arm/boot/dts/omap5.dtsi
>> +++ b/arch/arm/boot/dts/omap5.dtsi
>> @@ -765,6 +765,8 @@
>> #address-cells = <1>;
>> #size-cells = <1>;
>> ranges;
>> + clocks = <&l3init_60m_fclk>;
>> + clock-names = "init_60m_fclk";
>>
>> usbhsohci: ohci@4a064800 {
>> compatible = "ti,ohci-omap3", "usb-ohci";
>
> The bindings/mfd/omap-usb-host.txt file doesn't document any clocks.
> Please create another patch to document the clock names in this binding
> before you start putting them into the dtsi file. So far the clock
> names are an implementation detail of Linux as they are not part
> of the binding, and with your patch it becomes part of the ABI.
>
Right. I'll re-post the series.
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
[PATCH v4 0/5] USB Host support for OMAP5 uEVM (for 3.14)
Hi Benoit & Tony, This patchset brings up USB Host ports and Ethernet port on the OMAP5 uEVM board. It depends on the TI Clock DT conversion patches [1] and is based on 3.13-rc7 [1] - http://article.gmane.org/gmane.linux.ports.arm.kernel/289895 Changelog: v4: - Updated DT binding document for clock binding v3: - Rebased on top of 3.13-rc7 cheers, -roger Roger Quadros (5): mfd: omap-usb-host: Update DT clock binding information ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module ARM: dts: omap4-panda: Provide USB PHY clock ARM: dts: omap5-uevm: Provide USB PHY clock ARM: OMAP2+: Remove legacy_init_ehci_clk() Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 arch/arm/boot/dts/omap4-panda-common.dtsi | 8 ++-- arch/arm/boot/dts/omap5-uevm.dts| 8 ++-- arch/arm/boot/dts/omap5.dtsi| 2 ++ arch/arm/mach-omap2/pdata-quirks.c | 16 5 files changed, 10 insertions(+), 28 deletions(-) -- 1.8.3.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
[PATCH v4 5/5] ARM: OMAP2+: Remove legacy_init_ehci_clk()
The necessary clock phandle for the EHCI clock is now provided
via device tree so we no longer need this legacy method.
Signed-off-by: Roger Quadros
---
arch/arm/mach-omap2/pdata-quirks.c | 16
1 file changed, 16 deletions(-)
diff --git a/arch/arm/mach-omap2/pdata-quirks.c
b/arch/arm/mach-omap2/pdata-quirks.c
index 39f020c..6a4e2d1 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -26,20 +26,6 @@ struct pdata_init {
void (*fn)(void);
};
-/*
- * Create alias for USB host PHY clock.
- * Remove this when clock phandle can be provided via DT
- */
-static void __init __used legacy_init_ehci_clk(char *clkname)
-{
- int ret;
-
- ret = clk_add_alias("main_clk", NULL, clkname, NULL);
- if (ret)
- pr_err("%s:Failed to add main_clk alias to %s :%d\n",
- __func__, clkname, ret);
-}
-
#if IS_ENABLED(CONFIG_WL12XX)
static struct wl12xx_platform_data wl12xx __initdata;
@@ -105,7 +91,6 @@ static void __init omap4_sdp_legacy_init(void)
static void __init omap4_panda_legacy_init(void)
{
omap4_panda_display_init_of();
- legacy_init_ehci_clk("auxclk3_ck");
legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
}
#endif
@@ -113,7 +98,6 @@ static void __init omap4_panda_legacy_init(void)
#ifdef CONFIG_SOC_OMAP5
static void __init omap5_uevm_legacy_init(void)
{
- legacy_init_ehci_clk("auxclk1_ck");
}
#endif
--
1.8.3.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
[PATCH v4 2/5] ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module
USB Host driver (drivers/mfd/omap-usb-host.c) expects the 60MHz
reference clock to be named "init_60m_fclk". Provide this
information.
Signed-off-by: Roger Quadros
---
arch/arm/boot/dts/omap5.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 2f12a47..e0ab379 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -765,6 +765,8 @@
#address-cells = <1>;
#size-cells = <1>;
ranges;
+ clocks = <&l3init_60m_fclk>;
+ clock-names = "init_60m_fclk";
usbhsohci: ohci@4a064800 {
compatible = "ti,ohci-omap3", "usb-ohci";
--
1.8.3.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
[PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information
The omap-usb-host driver expects the 60MHz functional clock of the USB host module to be named as "init_60m_fclk". Add this information in the DT binding document. CC: Lee Jones CC: Samuel Ortiz Signed-off-by: Roger Quadros --- Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt index b381fa6..5635202 100644 --- a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt @@ -32,6 +32,10 @@ Optional properties: - single-ulpi-bypass: Must be present if the controller contains a single ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1 +- clocks: phandle to 60MHz functional clock to the USB Host module. + +- clock-names: must be "init_60m_fclk" + Required properties if child node exists: - #address-cells: Must be 1 -- 1.8.3.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
[PATCH v4 3/5] ARM: dts: omap4-panda: Provide USB PHY clock
The USB PHY gets its clock from AUXCLK3. Provide this information. Signed-off-by: Roger Quadros --- arch/arm/boot/dts/omap4-panda-common.dtsi | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi index 88c6a05..50b72966 100644 --- a/arch/arm/boot/dts/omap4-panda-common.dtsi +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi @@ -83,12 +83,8 @@ compatible = "usb-nop-xceiv"; reset-gpios = <&gpio2 30 GPIO_ACTIVE_LOW>; /* gpio_62 */ vcc-supply = <&hsusb1_power>; - /** -* FIXME: -* put the right clock phandle here when available -* clocks = <&auxclk3>; -* clock-names = "main_clk"; -*/ + clocks = <&auxclk3_ck>; + clock-names = "main_clk"; clock-frequency = <1920>; }; -- 1.8.3.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
[PATCH v4 4/5] ARM: dts: omap5-uevm: Provide USB PHY clock
The HS USB 2 PHY gets its clock from AUXCLK1. Provide this
information.
Signed-off-by: Roger Quadros
---
arch/arm/boot/dts/omap5-uevm.dts | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts
index 002fa70..3b99ec2 100644
--- a/arch/arm/boot/dts/omap5-uevm.dts
+++ b/arch/arm/boot/dts/omap5-uevm.dts
@@ -31,12 +31,8 @@
hsusb2_phy: hsusb2_phy {
compatible = "usb-nop-xceiv";
reset-gpios = <&gpio3 16 GPIO_ACTIVE_LOW>; /* gpio3_80
HUB_NRESET */
- /**
- * FIXME
- * Put the right clock phandle here when available
- * clocks = <&auxclk1>;
- * clock-names = "main_clk";
- */
+ clocks = <&auxclk1_ck>;
+ clock-names = "main_clk";
clock-frequency = <1920>;
};
--
1.8.3.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
[PATCH 2/2] usb/gadget: fix printer memory leak
From: "wenlin.kang"
When read data from g_printer, we see a Segmentation fault. eg:
Unable to handle kernel paging request at virtual address bf048000 pgd
= cf038000 [bf048000] *pgd=8e8cf811, *pte=, *ppte=
Internal error: Oops: 7 [#1] PREEMPT ARM Modules linked in: bluetooth
rfcomm g_printer
CPU: 0Not tainted (3.4.43-WR5.0.1.9_standard #1)
PC is at __copy_to_user_std+0x310/0x3a8 LR is at 0x4c808010
pc : []lr : [<4c808010>]psr: 2013
sp : cf883ea8 ip : 80801018 fp : cf883f24
r10: bf04706c r9 : 18a21205 r8 : 21953888
r7 : 201588aa r6 : 5109aa16 r5 : 0705aaa2 r4 : 5140aa8a
r3 : 004c r2 : 0fdc r1 : bf048000 r0 : bef5fc3c
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 10c5387d Table: 8f038019 DAC: 0015 Process
g_printer_test. (pid: 661, stack limit = 0xcf8822e8)
Stack: (0xcf883ea8 to 0xcf884000)
3ea0: bf047068 1fff bef5ecb9 cf882000 1fff bef5ecb9
3ec0: 1fff cf2e8724 bf044d3c 8013 8013 0001
bf04706c
3ee0: cf883f24 cf883ef0 c012e5ac c0324388 c007c8ac c0046298 8180
cf29b900
3f00: 2000 bef5ecb8 cf883f68 0003 cf882000 cf29b900 cf883f54
cf883f28
3f20: c012ea08 bf044b0c c000eb88 cf883f7c
2000
3f40: bef5ecb8 0003 cf883fa4 cf883f58 c012eae8 c012e960 0001
bef60cb8
3f60: 00a8 c000eb88 cf883fa4 c014329c
3f80: 00d4 41af63f0 0003 c000eb88 cf882000
cf883fa8
3fa0: c000e920 c012eaa4 00d4 0003 bef5ecb8 2000
bef5ecb8
3fc0: 00d4 41af63f0 0003 b6f534c0 419f9000
3fe0: bef5ecac 86d9 41a986bc 6010 0003 0109608a
0088828a
Code: f5d1f07c e8b100f0 e1a03c2e e2522020 (e8b15300) ---[ end trace
97e2618e250e3377 ]--- Segmentation fault
The root cause is the dev->rx_buffers list has been broken.
When we call printer_read(), the following call tree is triggered:
printer_read()
|
+---setup_rx_reqs(req)
| |
| +---usb_ep_queue(req)
| | |
| | +---...
| | |
| | +---rx_complete(req).
| |
| +---add the req to dev->rx_reqs_active
|
+---while(!list_empty(&dev->rx_buffers)))
The route happens when we don't use DMA or fail to start DMA in USB
driver. We can see: in the case, in rx_complete() it will add the req
to dev->rx_buffers. meanwhile we see that we will also add the req to
dev->rx_reqs_active after usb_ep_queue() return, so this adding will
break the dev->rx_buffers out.
After, when we call list_empty() to check dev->rx_buffers in while(),
due to can't check correctly dev->rx_buffers, so the Segmentation fault
occurs when copy_to_user() is called.
Signed-off-by: wenlin.kang
---
drivers/usb/gadget/printer.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/printer.c b/drivers/usb/gadget/printer.c
index f4b4fa8..1f15ad0 100644
--- a/drivers/usb/gadget/printer.c
+++ b/drivers/usb/gadget/printer.c
@@ -534,7 +534,9 @@ setup_rx_reqs(struct printer_dev *dev)
DBG(dev, "rx submit --> %d\n", error);
list_add(&req->list, &dev->rx_reqs);
break;
- } else {
+ }
+ /* if the req is empty, then add it into dev->rx_reqs_active. */
+ else if (list_empty(&req->list)) {
list_add(&req->list, &dev->rx_reqs_active);
}
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mfd: omap-usb-tll: Don't hold lock during pm_runtime_get/put_sync()
Hi,
On 12/18/2013 04:06 PM, Lee Jones wrote:
>> pm_runtime_get/put_sync() can sleep so don't hold spinlock while
>> calling them.
>>
>> This patch prevents a BUG() when CONFIG_DEBUG_ATOMIC_SLEEP is enabled.
>> Bug is present in Kernel versions v3.9 onwards.
>>
>> Reported-by: Tomi Valkeinen
>> Signed-off-by: Roger Quadros
>> Tested-by: Tomi Valkeinen
>> Cc: # 3.9+
>> ---
>> drivers/mfd/omap-usb-tll.c | 11 ++-
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
>> index 0d946ae1..248004c 100644
>> --- a/drivers/mfd/omap-usb-tll.c
>> +++ b/drivers/mfd/omap-usb-tll.c
>> @@ -346,7 +346,9 @@ int omap_tll_init(struct usbhs_omap_platform_data *pdata)
>> for (i = 0; i < tll->nch; i++)
>> needs_tll |= omap_usb_mode_needs_tll(pdata->port_mode[i]);
>>
>> +spin_unlock(&tll_lock);
>> pm_runtime_get_sync(tll_dev);
>> +spin_lock(&tll_lock);
>
> This is pretty ugly. Can't you move it above the spin_lock() instead?
OK. I'll also remove NULL check for tll_dev outside the spin_lock().
>
>
>
>> tll = dev_get_drvdata(tll_dev);
>>
>> +spin_unlock(&tll_lock);
>> pm_runtime_get_sync(tll_dev);
>> +spin_lock(&tll_lock);
>
> Same here?
Yes.
>
>> for (i = 0; i < tll->nch; i++) {
>> if (omap_usb_mode_needs_tll(pdata->port_mode[i])) {
>> @@ -438,7 +441,6 @@ int omap_tll_enable(struct usbhs_omap_platform_data
>> *pdata)
>> }
>>
>> spin_unlock(&tll_lock);
>> -
>
> This doesn't belong in this patch and is now inconsistent with the
> other functions in the driver.
OK.
>
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(omap_tll_enable);
>> @@ -464,9 +466,8 @@ int omap_tll_disable(struct usbhs_omap_platform_data
>> *pdata)
>> }
>> }
>>
>> -pm_runtime_put_sync(tll_dev);
>> -
>> spin_unlock(&tll_lock);
>> +pm_runtime_put_sync(tll_dev);
>>
>> return 0;
>> }
>
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
[PATCH 1/2] usb/gadget: fix printer deadlock
From: "wenlin.kang"
The problem occurs in follow path.
printer_read()
|
+---setup_rx_reqs()
|
+---usb_ep_queue()
|
+---...
|
+---rx_complete()
Although it is clear from code, we can't get it normally.
only when we enable some spin_lock debug config option, we can find it.
eg:
BUG: spinlock lockup on CPU#0, g_printer_test_/584
lock: bf05e158, .magic: dead4ead, .owner: g_printer_test_/584, .owner_cpu: 0
[] (unwind_backtrace+0x0/0x104) from []
(dump_stack+0x20/0x24)
[] (dump_stack+0x20/0x24) from [] (spin_dump+0x8c/0x94)
[] (spin_dump+0x8c/0x94) from []
(do_raw_spin_lock+0x128/0x154)
[] (do_raw_spin_lock+0x128/0x154) from []
(_raw_spin_lock_irqsave+0x64/0x70)
[] (_raw_spin_lock_irqsave+0x64/0x70) from []
(rx_complete+0x54/0x10c [g_printer])
[] (rx_complete+0x54/0x10c [g_printer]) from []
(musb_g_giveback+0x78/0x88)
[] (musb_g_giveback+0x78/0x88) from [] (rxstate+0xa0/0x10c)
[] (rxstate+0xa0/0x10c) from [] (musb_ep_restart+0x44/0x70)
[] (musb_ep_restart+0x44/0x70) from []
(musb_gadget_queue+0xe8/0xf8)
[] (musb_gadget_queue+0xe8/0xf8) from []
(setup_rx_reqs+0xa4/0x178 [g_printer])
[] (setup_rx_reqs+0xa4/0x178 [g_printer]) from []
(printer_read+0x9c/0x3f4 [g_printer])
[] (printer_read+0x9c/0x3f4 [g_printer]) from []
(vfs_read+0xb4/0x144)
[] (vfs_read+0xb4/0x144) from [] (sys_read+0x50/0x124)
[] (sys_read+0x50/0x124) from [] (ret_fast_syscall+0x0/0x3c)
The root cause is that we use the same lock two time in a path, so to avoid
the deadlock, we need to unlock in setup_rx_reqs(), and only unlock.
Signed-off-by: wenlin.kang
---
drivers/usb/gadget/printer.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/printer.c b/drivers/usb/gadget/printer.c
index 4e4dc1f..f4b4fa8 100644
--- a/drivers/usb/gadget/printer.c
+++ b/drivers/usb/gadget/printer.c
@@ -526,7 +526,10 @@ setup_rx_reqs(struct printer_dev *dev)
req->length = USB_BUFSIZE;
req->complete = rx_complete;
+ /* here, we unlock, and only unlock, to avoid deadlock. */
+ spin_unlock(&dev->lock);
error = usb_ep_queue(dev->out_ep, req, GFP_ATOMIC);
+ spin_lock(&dev->lock);
if (error) {
DBG(dev, "rx submit --> %d\n", error);
list_add(&req->list, &dev->rx_reqs);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb:hub set hub->change_bits when over-current happens
When we are doing compliance test with xHCI, we found that if we
enable CONFIG_USB_SUSPEND and plug in a bad device which causes
over-current condition to the root port, software will not be noticed.
The reason is that current code don't set hub->change_bits in
hub_activate() when over-current happens, and then hub_events() will
not check the port status because it thinks nothing changed.
If CONFIG_USB_SUSPEND is disabled, the interrupt pipe of the hub will
report the change and set hub->event_bits, and then hub_events() will
check what events happened.In this case over-current can be detected.
Signed-off-by: Shen Guang
---
drivers/usb/core/hub.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bd9dc35..98b5679 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1154,7 +1154,8 @@ static void hub_activate(struct usb_hub *hub,
enum hub_activation_type type)
/* Tell khubd to disconnect the device or
* check for a new connection
*/
- if (udev || (portstatus & USB_PORT_STAT_CONNECTION))
+ if (udev || (portstatus & USB_PORT_STAT_CONNECTION) ||
+ (portstatus & USB_PORT_STAT_OVERCURRENT))
set_bit(port1, hub->change_bits);
} else if (portstatus & USB_PORT_STAT_ENABLE) {
--
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
--
To unsubscribe from this list: send the line "unsubscribe 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] mfd: omap-usb-tll: Don't hold lock during pm_runtime_get/put_sync()
pm_runtime_get/put_sync() can sleep so don't hold spinlock while
calling them.
This patch prevents a BUG() during system suspend when
CONFIG_DEBUG_ATOMIC_SLEEP is enabled.
Bug is present in Kernel versions v3.9 onwards.
Reported-by: Tomi Valkeinen
Signed-off-by: Roger Quadros
Tested-by: Tomi Valkeinen
Cc: # 3.9+
---
drivers/mfd/omap-usb-tll.c | 36
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
index 0d946ae1..c409be9 100644
--- a/drivers/mfd/omap-usb-tll.c
+++ b/drivers/mfd/omap-usb-tll.c
@@ -333,21 +333,17 @@ int omap_tll_init(struct usbhs_omap_platform_data *pdata)
unsigned reg;
struct usbtll_omap *tll;
- spin_lock(&tll_lock);
-
- if (!tll_dev) {
- spin_unlock(&tll_lock);
+ if (!tll_dev)
return -ENODEV;
- }
- tll = dev_get_drvdata(tll_dev);
+ pm_runtime_get_sync(tll_dev);
+ spin_lock(&tll_lock);
+ tll = dev_get_drvdata(tll_dev);
needs_tll = false;
for (i = 0; i < tll->nch; i++)
needs_tll |= omap_usb_mode_needs_tll(pdata->port_mode[i]);
- pm_runtime_get_sync(tll_dev);
-
if (needs_tll) {
void __iomem *base = tll->base;
@@ -398,9 +394,8 @@ int omap_tll_init(struct usbhs_omap_platform_data *pdata)
}
}
- pm_runtime_put_sync(tll_dev);
-
spin_unlock(&tll_lock);
+ pm_runtime_put_sync(tll_dev);
return 0;
}
@@ -411,17 +406,14 @@ int omap_tll_enable(struct usbhs_omap_platform_data
*pdata)
int i;
struct usbtll_omap *tll;
- spin_lock(&tll_lock);
-
- if (!tll_dev) {
- spin_unlock(&tll_lock);
+ if (!tll_dev)
return -ENODEV;
- }
-
- tll = dev_get_drvdata(tll_dev);
pm_runtime_get_sync(tll_dev);
+ spin_lock(&tll_lock);
+ tll = dev_get_drvdata(tll_dev);
+
for (i = 0; i < tll->nch; i++) {
if (omap_usb_mode_needs_tll(pdata->port_mode[i])) {
int r;
@@ -448,13 +440,10 @@ int omap_tll_disable(struct usbhs_omap_platform_data
*pdata)
int i;
struct usbtll_omap *tll;
- spin_lock(&tll_lock);
-
- if (!tll_dev) {
- spin_unlock(&tll_lock);
+ if (!tll_dev)
return -ENODEV;
- }
+ spin_lock(&tll_lock);
tll = dev_get_drvdata(tll_dev);
for (i = 0; i < tll->nch; i++) {
@@ -464,9 +453,8 @@ int omap_tll_disable(struct usbhs_omap_platform_data *pdata)
}
}
- pm_runtime_put_sync(tll_dev);
-
spin_unlock(&tll_lock);
+ pm_runtime_put_sync(tll_dev);
return 0;
}
--
1.8.3.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
