Re: [PATCH 1/2] USB: at91: fix the number of endpoint parameter
Hi J,
On 01/21/2014 01:49 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 11:39 Mon 20 Jan , Bo Shen wrote:
Hi J,
On 01/18/2014 01:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 10:59 Fri 17 Jan , Bo Shen wrote:
In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
Signed-off-by: Bo Shen
---
drivers/usb/gadget/atmel_usba_udc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c
b/drivers/usb/gadget/atmel_usba_udc.c
index 2cb52e0..7e67a81 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (ep_status) {
int i;
- for (i = 0; i < USBA_NR_ENDPOINTS; i++)
+ for (i = 0; i < udc->num_ep; i++)
no the limit need to specified in the driver as a checkpoint by the compatible
or platform driver id
You mean, we should not trust the data passed from dt node or
platform data? Or do you think we should do double confirm?
no base on the driver name or the compatible you will known the MAX EP
not based on the dt ep description
as we do on pinctrl-at91
I am sorry, I am not fully get it after reading the code of
pinctrl-at91.c, can you give the example code in pinctrl-at91.c?
Btw, the udc->num_ep is get from the following code.
for dt
--->8---
while ((pp = of_get_next_child(np, pp)))
udc->num_ep++;
---<8---
for non-dt
--->8---
udc->num_ep = pdata->num_ep;
---8<---
Best Regards,
J.
if (ep_status & (1 << i)) {
if (ep_is_control(&udc->usba_ep[i]))
usba_control_irq(udc, &udc->usba_ep[i]);
--
1.8.5.2
Best Regards,
Bo Shen
Best Regards,
Bo Shen
--
To unsubscribe from this list: send the line "unsubscribe 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 03/10] usb: find internal hub tier mismatch via acpi
On Sat, 2014-01-18 at 08:59 -0500, Alan Stern wrote:
> On Fri, 17 Jan 2014, Dan Williams wrote:
>
> > > The basic idea of using runtime PM synchronization to prevent khubd and
> > > port power operations from interfering sounds good, and it is simpler
> > > than adding a new mutex. And I think we can also use it to prevent
> > > port power operations and port resets from interfering. But that
> > > leaves open the question of how to prevent usb_device_reset from
> > > interfering with khubd.
> > >
> >
> > Yes, patch 9 narrowly addresses a problem that needs a wider solution.
> >
> > We want to:
> >
> > 1/ prevent khubd from running while reset is in progress
>
> This is the open question mentioned above, if you add in the
> requirement that we also want to prevent a reset from starting while
> khubd is running (except when khubd performs the reset itself).
>
> > 2/ prevent khubd from seeing an intermediate device state during
> > usb_port_resume
>
> khubd doesn't pay all that much attention to device states; mostly it
> is concerned with port statuses. We don't want khubd to see an
> intermediate port status during usb_port_resume. Basically that means
> we don't want khubd to run while usb_port_resume is in progress, and we
> don't want usb_port_resume to start while khubd is running unless khubd
> performs the resume itself.
Yes, it does not pay all that much attention to device states, but it is
critical that it not read ->state while usb_port_resume() is changing
it. Specifically this hole in hub_port_connect_change():
> if ((portstatus & USB_PORT_STAT_CONNECTION) && udev &&
> udev->state != USB_STATE_NOTATTACHED) {
> if (portstatus & USB_PORT_STAT_ENABLE) {
> status = 0; /* Nothing to do */
>
> #ifdef CONFIG_PM_RUNTIME
...usb_port_resume() can change the state at this point and cause a
disconnect.
> } else if (udev->state == USB_STATE_SUSPENDED &&
> udev->persist_enabled) {
> /* For a suspended device, treat this as a
> * remote wakeup event.
> */
> status = usb_remote_wakeup(udev);
> #endif
> } else {
> /* Don't resuscitate */;
> }
> }
> > 3/ prevent khubd from running in the interval between completion of
> > ubs_port_runtime_resume() completing and usb_port_resume() running.
>
> Hmmm. I don't remember what usb_port_runtime_resume does about a
> connected USB-2 child device. It can't simply do a runtime resume of
> the child. Maybe it should tell khubd to resume the child? Then khubd
> would never see the intermediate state; the next time it looked at the
> port, it would issue the runtime resume. (Provided that
> usb_port_runtime_resume didn't complete after khubd was already
> running.)
Yes, that's what I had in the patch, more straightforward than the work
queue.
> > All these scenarios to point to problems with ->device_state collisions
> > not necessarily the port state.
>
> I don't think so, for the reason mentioned above.
I'll rephrase, checking USB_STATE_SUSPENDED needs usb_port_resume()
synchronization.
> And it is starting
> to seem less likely that we can rely on runtime PM synchronization to
> do everything we want. For example, if something is suspended then
> there is no way to prevent a runtime resume from occurring at any
> moment.
I propose we use usb_port pm runtime synchronization to block khubd when
we know the portstatus is not valid (powered off), and we need a new
lock to synchronize with usb_device pm operations before checking the
device state.
> > How about a new lock that synchronizes
> > device state changes with state checks? This replaces patch 9, only
> > compile tested:
>
> When considering overall strategies for solving a problem, I find that
> posting patches is almost never helpful. A patch focuses the mind on
> low-level coding details, many of which are irrelevant to the problem
> at hand, preventing you from concentrating on the overall picture --
> which is the most important thing at this stage of planning.
> Pseudo-code can be better in this regard, and I sometimes use it when a
> degree of precision is necessary in the discussion. However, I don't
> think we have reached that point yet.
Getting closer...
> Therefore I would greatly prefer if you could describe in prose what
> you want to do.
Ok.
I think we agree that khubd needs to not look at the portstatus while
the port is down. pm runtime synchronization takes care of that and
prevents khubd from starting while the port is inactive. With that in
place we can now make requests in usb_port_runtime_resume() that are
later serviced in khubd when the port is marked active (pm runtime
enforces that ->runtime_resume()
Re: [PATCH 1/2] USB: at91: fix the number of endpoint parameter
On 21/01/2014 09:12, Bo Shen :
> Hi J,
>
> On 01/21/2014 01:49 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 11:39 Mon 20 Jan , Bo Shen wrote:
>>> Hi J,
>>>
>>> On 01/18/2014 01:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 10:59 Fri 17 Jan , Bo Shen wrote:
> In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
> is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
>
> Signed-off-by: Bo Shen
> ---
>
> drivers/usb/gadget/atmel_usba_udc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/atmel_usba_udc.c
> b/drivers/usb/gadget/atmel_usba_udc.c
> index 2cb52e0..7e67a81 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> @@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void
> *devid)
> if (ep_status) {
> int i;
>
> - for (i = 0; i < USBA_NR_ENDPOINTS; i++)
> + for (i = 0; i < udc->num_ep; i++)
no the limit need to specified in the driver as a checkpoint by the
compatible
or platform driver id
>>>
>>> You mean, we should not trust the data passed from dt node or
>>> platform data? Or do you think we should do double confirm?
>>
>> no base on the driver name or the compatible you will known the MAX EP
>>
>> not based on the dt ep description
>>
>> as we do on pinctrl-at91
>
> I am sorry, I am not fully get it after reading the code of
> pinctrl-at91.c, can you give the example code in pinctrl-at91.c?
>
> Btw, the udc->num_ep is get from the following code.
> for dt
> --->8---
> while ((pp = of_get_next_child(np, pp)))
> udc->num_ep++;
> ---<8---
>
> for non-dt
> --->8---
> udc->num_ep = pdata->num_ep;
> ---8<---
It seems to me pretty valid to use num_ep in this driver and not have to
rely on another compatibility string just for this.
The information is here, it is retrieved pretty cleanly so I vote for a
simple use of it: if we introduce another information we will have to
double check the cross errors that would happen...
Bye,
> if (ep_status & (1 << i)) {
> if (ep_is_control(&udc->usba_ep[i]))
> usba_control_irq(udc,
> &udc->usba_ep[i]);
> --
> 1.8.5.2
>
>>>
>>> Best Regards,
>>> Bo Shen
>
> Best Regards,
> Bo Shen
>
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
USB EHCI broken on Celeron N2920 (Bay Trail)
Here is kernel message: [2.939251] usb 1-1: new high-speed USB device number 2 using ehci-pci [ 18.490987] usb 1-1: device not accepting address 2, error -110 [ 18.595095] usb 1-1: new high-speed USB device number 3 using ehci-pci [ 34.182233] usb 1-1: device not accepting address 3, error -110 [ 34.286352] usb 1-1: new high-speed USB device number 4 using ehci-pci [ 44.744589] usb 1-1: device not accepting address 4, error -110 [ 44.849590] usb 1-1: new high-speed USB device number 5 using ehci-pci [ 55.307858] usb 1-1: device not accepting address 5, error -110 [ 55.310239] hub 1-0:1.0: unable to enumerate USB device on port 1 Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 03/10] usb: find internal hub tier mismatch via acpi
> The lock hangs off the usb_device rather than the the usb_port since it
> is meant to prevent unintended disconnects. Any portstatus vs khubd
> collisions are handled by pm runtime synchronization since there is no
> expectation that khubd resumes a usb_port.
>
> Once khubd has made a determination that it wants to resume or reset a
> device it will of course need to drop the state lock. Those actions
> will implicitly take the state lock.
>
> Here's the patch (to replace patch 9), now tested. There are of course
> cleanups that can follow this, but holding off until we have agreement
> on closing these races.
>
[..]
Sarah,
I had meant to send this hack out separately. I believe this is the
infinite loop that Tianyu had seen previously. I had not been able to
reproduce until now (new timings now that khubd flushes device pm
ops). This works around a remote wakeup vs port poweroff request
race. A wakeup request sets the timer, but by the time it expires the
port has been turned off, so we hit:
if ((raw_port_status & PORT_RESET) ||
!(raw_port_status & PORT_PE))
return 0x;
...in xhci_get_port_status. Cancelling the resume seems the right
choice given that userspace has stated "I don't care about remote
wakeups on this port" by clearing pm_qos_no_poweroff.
Thoughts?
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 9992fbfec85f..2333ec573594 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -1004,9 +1004,16 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq,
> u16 wValue,
> xhci_disable_port(hcd, xhci, wIndex,
> port_array[wIndex], temp);
> break;
> - case USB_PORT_FEAT_POWER:
> - writel(temp & ~PORT_POWER, port_array[wIndex]);
> + case USB_PORT_FEAT_POWER: {
> + struct xhci_bus_state *bus_state;
>
> + bus_state = &xhci->bus_state[hcd_index(hcd)];
> + writel(temp & ~PORT_POWER, port_array[wIndex]);
> + if (test_and_clear_bit(wIndex,
> &bus_state->resuming_ports)) {
> + bus_state->resume_done[wIndex] = 0;
> + xhci_dbg(xhci, "port%d: resume cancelled\n",
> +wIndex);
> + }
> spin_unlock_irqrestore(&xhci->lock, flags);
> temp = usb_acpi_power_manageable(hcd->self.root_hub,
> wIndex);
> @@ -1015,6 +1022,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq,
> u16 wValue,
> wIndex, false);
> spin_lock_irqsave(&xhci->lock, flags);
> break;
> + }
> default:
> goto error;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
USB serial gadget: Interchanged order of TX packets
Hi,
When using the USB serial gadget it regularly occurs that USB packets
arrive at the host in the wrong order. I am using kernel version 3.4
with the corresponding RT patch on an ARM Cortex-A8 platform.
The problem appears to be in file 'drivers/usb/gadget/u_serial.c'.
It seems that kernel thread "irq/92-musb-hdr" and a userspace thread
have a race when sending data, in function 'gs_start_tx'. The kernel
thread calls 'gs_start_tx' from the callback function
'gs_write_complete', a userspace thread through 'gs_write' and
'gs_flush_chars'. The problem is triggered by the fact that the
spinlock 'port_lock' is dropped temporarily in 'gs_start_tx' during
the time 'usb_ep_queue' is executed. There might be good reasons,
which I am unaware off, to do that. So, suppose that the userspace
thread has just released the spinlock and the scheduler selects at
that moment the (normally) higher prioritized kernel thread, which was
waiting at the 'port_lock' in 'gs_write_complete' to run. This will
allow the kernel thread to overtake the sleeping userspace thread and
send out the "next" USB packet first!
I can't reproduce the problem if I disable the RT patch, but I think
that the problem might as well appear, if for instance an interrupt
just happens during the time the lock is dropped in 'gs_start_rx'. But
if you prove me wrong, I'm happy to post it in the RT mailing list
instead.
The provided patch is a diff to the mainline. I've introduced a flag
to mark when there is already a thread busy in the loop emptying the
pool in 'gs_start_tx'. In that case another thread would just skip the
loop. This seems to work for me but it might not be foolproof and not
elegant enough.
By checking the code of the mainline I didn't find any relevant change
that would indicate that the issue has been already solved in the
meantime. For the moment it would be too much work for me to make the
mainline runnable on my platform.
Please CC me and arn...@mind.be as we are not subscribed.
Philip
diff --git a/drivers/usb/gadget/u_serial.c b/drivers/usb/gadget/u_serial.c
index b369292..d2682f5 100644
--- a/drivers/usb/gadget/u_serial.c
+++ b/drivers/usb/gadget/u_serial.c
@@ -116,6 +116,7 @@ struct gs_port {
int write_allocated;
struct gs_buf port_write_buf;
wait_queue_head_t drain_wait; /* wait while writes drain */
+ boolwrite_busy;
/* REVISIT this state ... */
struct usb_cdc_line_coding port_line_coding; /* 8-N-1 etc */
@@ -366,7 +367,7 @@ __acquires(&port->port_lock)
int status = 0;
bool do_tty_wake = false;
- while (!list_empty(pool)) {
+ while (!port->write_busy && !list_empty(pool)) {
struct usb_request *req;
int len;
@@ -396,9 +397,11 @@ __acquires(&port->port_lock)
* NOTE that we may keep sending data for a while after
* the TTY closed (dev->ioport->port_tty is NULL).
*/
+ port->write_busy = true;
spin_unlock(&port->port_lock);
status = usb_ep_queue(in, req, GFP_ATOMIC);
spin_lock(&port->port_lock);
+ port->write_busy = false;
if (status) {
pr_debug("%s: %s %s err %d\n",
@@ -770,6 +773,7 @@ static int gs_open(struct tty_struct *tty, struct file *file)
/* Do the "real open" */
spin_lock_irq(&port->port_lock);
+ port->write_busy = false;
/* allocate circular buffer on first open */
if (port->port_write_buf.buf_buf == NULL) {
RE: FunctionFS: Provide device and configdescriptor from user mode application?
Hi, > -Original Message- > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > ow...@vger.kernel.org] On Behalf Of Marco Zamponi > > Hi! > Sorry, I posted before but forgot the subject. > I am running the test application "ffs-test.c" in user space and I > want to > write the descriptors to ep0. > However, it seems that I can only write interface descriptors > and enpoint descriptors. > What about device and configdescriptors? When I put those in the > "descriptor" struct I get an EINVAL (22, invalid parameter) error: > > "ffs-test: info: ep0: writing descriptors" > "ffs-test: crit: ep0: write: descriptors: (-22) Invalid argument" This is intended behavior. FunctionFS is kernel-userspace interface which allow to provide implementation of *single* usb function (interface). Each usb device consist of several functions which are placed in configuration. FunctionFS allow you only to provide descriptors for this *one particular* function which is implemented by you. Information related to whole usb device may be set using two ways: 1) Using ConfigFS to create your own gadget [1] 2) Using modprobe g_ffs and provide arguments which are suitable for you. ConfigFS is more flexible and will allow you to set all values from usb device/configuration descriptors using file system. Footnotes: [1] https://wiki.tizen.org/wiki/USB/Linux_USB_Layers/Configfs_Composite_Gadg et -- BR's Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst [NEW HARDWARE]
From: Sarah Sharp > On Mon, Jan 20, 2014 at 11:21:14AM +, David Laight wrote: ... > > A guess... > > > > In queue_bulk_sg_tx() try calling xhci_v1_0_td_remainder() instead > > of xhci_td_remainder(). > > Why? Walt has a 0.96 xHCI host controller, and the format for how to > calculate the TD remainder changed between the 0.96 and the 1.0 spec. > That's why we have xhci_v1_0_td_remainder() and xhci_td_remainder(). I just wonder how many of those differences are just differences in the specification, rather than differences in the hardware implementation. In some cases it might be that the old hardware just ignored the value. I know that the xhci hardware on my ivy bridge cpu does look at that value (at least checking for zero), since things failed in subtle ways when I got it wrong. In this case it was just something easy to change that might be worth trying. I didn't necessarily expect it to make a positive difference. 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: xhci ASMedia lockups - a theory and a patch
From: Sarah Sharp [ > On Thu, Jan 16, 2014 at 10:21:11AM +, David Laight wrote: > > From: David Laight > > > From: David Laight ... > > > Below is a possible patch, I've only compile tested it. > > > I've minimalised the patch by not removing all the code that saves > > > 'start_trb' and modifies the TRB_CYCLE bit. > > > If the patch works those parts can also be tidied up. > > > > I've had some feedback (in a private email) that the patch helps. > > This was using the ASMedia controller with the asx88179_178a > > ethernet device. > > David, please post the contents of that private email to the list and Cc > me. We don't debug in private in the kernel. I'm not sure I should copy private emails to the list without permission of the person who wrote them. > > So the theory was definitely on the right lines. > > And I managed to write the patch without any silly mistakes! > > Your theory about the ASMedia host may be correct. There are other host > controllers (Synopsys, I think) that needed us to not give a link TRB > over to the host until the TRB it pointed had the cycle bit set to > hardware owned. ISTR that this was only triggered with USB 3.0 streams, > at least on that Synopsys host. > > (BTW, you can find out why any part of the code was written by using > `git blame filename`, although the original commit that fixed this bug > has an entirely unhelpful description.) > > So, I agree that this needs to get fixed, especially since full UAS and > streams support will be landing in the 3.15 kernel. However, I like the > simple patch you posted much better than the second patch. The second > patch is much too big to be back ported to stable, and we need to get > this backported to stable. Please resend your previous patch. Ok, I'll send it in parts. Probably as a patch set that must be applied in sequence. 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 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs),
do not return from probe if the USB PHY library returns -ENODEV as that
indicates the platform does not have PHY.
Signed-off-by: Kishon Vijay Abraham I
---
drivers/usb/dwc3/core.c | 34 ++
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index a49217a..e009d4e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -411,32 +411,26 @@ static int dwc3_probe(struct platform_device *pdev)
if (IS_ERR(dwc->usb2_phy)) {
ret = PTR_ERR(dwc->usb2_phy);
-
- /*
-* if -ENXIO is returned, it means PHY layer wasn't
-* enabled, so it makes no sense to return -EPROBE_DEFER
-* in that case, since no PHY driver will ever probe.
-*/
- if (ret == -ENXIO)
+ if (ret == -ENXIO || ret == -ENODEV) {
+ dwc->usb2_phy = NULL;
+ } else if (ret == -EPROBE_DEFER) {
return ret;
-
- dev_err(dev, "no usb2 phy configured\n");
- return -EPROBE_DEFER;
+ } else {
+ dev_err(dev, "no usb2 phy configured\n");
+ return ret;
+ }
}
if (IS_ERR(dwc->usb3_phy)) {
ret = PTR_ERR(dwc->usb3_phy);
-
- /*
-* if -ENXIO is returned, it means PHY layer wasn't
-* enabled, so it makes no sense to return -EPROBE_DEFER
-* in that case, since no PHY driver will ever probe.
-*/
- if (ret == -ENXIO)
+ if (ret == -ENXIO || ret == -ENODEV) {
+ dwc->usb3_phy = NULL;
+ } else if (ret == -EPROBE_DEFER) {
return ret;
-
- dev_err(dev, "no usb3 phy configured\n");
- return -EPROBE_DEFER;
+ } else {
+ dev_err(dev, "no usb3 phy configured\n");
+ return ret;
+ }
}
dwc->xhci_resources[0].start = res->start;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/2] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
power_on and power_off the following APIs are used phy_init(), phy_exit(),
phy_power_on() and phy_power_off().
However using the old USB phy library wont be removed till the PHYs of all
other SoC's using dwc3 core is adapted to the Generic PHY Framework.
Signed-off-by: Kishon Vijay Abraham I
---
Changes from v3:
* avoided using quirks
Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
drivers/usb/dwc3/core.c| 60
drivers/usb/dwc3/core.h|7 +++
3 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
b/Documentation/devicetree/bindings/usb/dwc3.txt
index e807635..471366d 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -6,11 +6,13 @@ Required properties:
- compatible: must be "snps,dwc3"
- reg : Address and length of the register set for the device
- interrupts: Interrupts used by the dwc3 controller.
+
+Optional properties:
- usb-phy : array of phandle for the PHY device. The first element
in the array is expected to be a handle to the USB2/HS PHY and
the second element is expected to be a handle to the USB3/SS PHY
-
-Optional properties:
+ - phys: from the *Generic PHY* bindings
+ - phy-names: from the *Generic PHY* bindings
- tx-fifo-resize: determines if the FIFO *has* to be reallocated.
This is usually a subnode to DWC3 glue to which it is connected.
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index e009d4e..036d589 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -82,6 +82,11 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
usb_phy_init(dwc->usb2_phy);
usb_phy_init(dwc->usb3_phy);
+ if (dwc->usb2_generic_phy)
+ phy_init(dwc->usb2_generic_phy);
+ if (dwc->usb3_generic_phy)
+ phy_init(dwc->usb3_generic_phy);
+
mdelay(100);
/* Clear USB3 PHY reset */
@@ -343,6 +348,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
{
usb_phy_shutdown(dwc->usb2_phy);
usb_phy_shutdown(dwc->usb3_phy);
+ if (dwc->usb2_generic_phy)
+ phy_exit(dwc->usb2_generic_phy);
+ if (dwc->usb3_generic_phy)
+ phy_exit(dwc->usb3_generic_phy);
+
}
#define DWC3_ALIGN_MASK(16 - 1)
@@ -433,6 +443,32 @@ static int dwc3_probe(struct platform_device *pdev)
}
}
+ dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
+ if (IS_ERR(dwc->usb2_generic_phy)) {
+ ret = PTR_ERR(dwc->usb2_generic_phy);
+ if (ret == -ENOSYS || ret == -ENODEV) {
+ dwc->usb2_generic_phy = NULL;
+ } else if (ret == -EPROBE_DEFER) {
+ return ret;
+ } else {
+ dev_err(dev, "no usb2 phy configured\n");
+ return ret;
+ }
+ }
+
+ dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
+ if (IS_ERR(dwc->usb3_generic_phy)) {
+ ret = PTR_ERR(dwc->usb3_generic_phy);
+ if (ret == -ENOSYS || ret == -ENODEV) {
+ dwc->usb3_generic_phy = NULL;
+ } else if (ret == -EPROBE_DEFER) {
+ return ret;
+ } else {
+ dev_err(dev, "no usb3 phy configured\n");
+ return ret;
+ }
+ }
+
dwc->xhci_resources[0].start = res->start;
dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
DWC3_XHCI_REGS_END;
@@ -482,6 +518,11 @@ static int dwc3_probe(struct platform_device *pdev)
usb_phy_set_suspend(dwc->usb2_phy, 0);
usb_phy_set_suspend(dwc->usb3_phy, 0);
+ if (dwc->usb2_generic_phy)
+ phy_power_on(dwc->usb2_generic_phy);
+ if (dwc->usb3_generic_phy)
+ phy_power_on(dwc->usb3_generic_phy);
+
ret = dwc3_event_buffers_setup(dwc);
if (ret) {
dev_err(dwc->dev, "failed to setup event buffers\n");
@@ -565,6 +606,10 @@ err2:
err1:
usb_phy_set_suspend(dwc->usb2_phy, 1);
usb_phy_set_suspend(dwc->usb3_phy, 1);
+ if (dwc->usb2_generic_phy)
+ phy_power_off(dwc->usb2_generic_phy);
+ if (dwc->usb3_generic_phy)
+ phy_power_off(dwc->usb3_generic_phy);
dwc3_core_exit(dwc);
err0:
@@ -580,6 +625,11 @@ static int dwc3_remove(struct platform_device *pdev)
usb_phy_set_suspend(dwc->usb2_phy, 1);
usb_phy_set_suspend(dwc->usb3_phy, 1);
+ if (dwc->usb2_generic_phy)
+ phy_power_off(dwc->usb2_generic_phy);
+ if (dwc->usb3_generic_phy)
+ phy_power_off(dwc->usb3_generic_phy);
+
pm_runtime_put_syn
Re: [PATCH 1/1] usb: chipidea: udc: delete wrong warning message for ep0in
On Tue, Jan 21, 2014 at 01:21:51PM +0800, Peter Chen wrote:
> On Mon, Jan 20, 2014 at 09:13:11AM +0800, Peter Chen wrote:
> > The idea of this message is: the non-ep0 should not use ctrl
> > transfer. But it does not cover the ep0in which uses ctrl
> > transfer too.
> >
> > This commit deletes the warning message at interrupt handler,
> > and adds judgement at ep_enable, if non-ep0 requests ctrl transfer,
> > it will indicate an error.
> >
> > Reported-by: Michael Grzeschik
> > Signed-off-by: Peter Chen
> > ---
> > drivers/usb/chipidea/udc.c | 10 +-
> > 1 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > index 09366b4..646d958 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -1001,11 +1001,6 @@ __acquires(ci->lock)
> > !hw_test_and_clear_setup_status(ci, i))
> > continue;
> >
> > - if (i != 0) {
> > - dev_warn(ci->dev, "ctrl traffic at endpoint %d\n", i);
> > - continue;
> > - }
> > -
> > /*
> > * Flush data and handshake transactions of previous
> > * setup packet.
> > @@ -1201,6 +1196,11 @@ static int ep_enable(struct usb_ep *ep,
> > retval |= hw_ep_enable(hwep->ci, hwep->num, hwep->dir,
> >hwep->type);
> >
> > + if (hwep->num != 0 && hwep->type == USB_ENDPOINT_XFER_CONTROL) {
> > + dev_err(hwep->ci->dev, "Set control xfer at non-ep0\n");
> > + retval = -EINVAL;
> > + }
> > +
> > spin_unlock_irqrestore(hwep->lock, flags);
> > return retval;
> > }
> > --
> > 1.7.8
> >
> >
>
> Michael, this patch may not fix your problem totally, as a temp solution,
> you can use the matthieu's patch at your email thread.
Yes, I have seen it. But for this patch I partly agree. The check
in ep_enable should be done before calling hw_ep_enable.
Beside that:
Acked-by: Michael Grzeschik
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] xhci: Don't trace all short/incomplete receives
Don't trace short receives if URB_SHORT_NOT_OK is set.
Short receives are normal for USB ethernet devices.
Don't trace unexpected incomplete receives if XHCI_TRUST_TX_LENGTH is set.
Ratelimit the trace.
Signed-off-by: David Laight
---
If these two traces ever happen, then they will happen for every receive
packet when using USB ethernet.
If you need to enable the xhci_warn or xhci_dgb traces you don't want to
be spammed with trace (syslogd will soon will the disk).
These patches won't apply to 3.12 because the trace texts have changed,
however 3.12 also needs a kernel recompile to enable the traces and
anyone doing that can probably manage to patch them out.
Changes for v2:
Fixed so that it applies to Linus's current tree.
drivers/usb/host/xhci-ring.c | 38 ++
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a0b248c..0b3dd16 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2301,36 +2301,34 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci,
struct xhci_td *td,
switch (trb_comp_code) {
case COMP_SUCCESS:
/* Double check that the HW transferred everything. */
- if (event_trb != td->last_trb ||
- EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) {
- xhci_warn(xhci, "WARN Successful completion "
- "on short TX\n");
- if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
- *status = -EREMOTEIO;
- else
- *status = 0;
- if ((xhci->quirks & XHCI_TRUST_TX_LENGTH))
- trb_comp_code = COMP_SHORT_TX;
- } else {
+ if (event_trb == td->last_trb &&
+ EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) == 0) {
*status = 0;
+ break;
}
- break;
+ if (xhci->quirks & XHCI_TRUST_TX_LENGTH)
+ trb_comp_code = COMP_SHORT_TX;
+ else
+ xhci_warn_ratelimited(xhci,
+ "WARN Successful completion on short TX\n");
+ /* FALLTHROUGH */
case COMP_SHORT_TX:
- if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
+ if (td->urb->transfer_flags & URB_SHORT_NOT_OK) {
+ xhci_dbg(xhci,
+ "ep %#x - asked for %d bytes, %d bytes
untransferred\n",
+ td->urb->ep->desc.bEndpointAddress,
+ td->urb->transfer_buffer_length,
+ EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)));
*status = -EREMOTEIO;
- else
+ } else {
*status = 0;
+ }
break;
default:
/* Others already handled above */
break;
}
- if (trb_comp_code == COMP_SHORT_TX)
- xhci_dbg(xhci, "ep %#x - asked for %d bytes, "
- "%d bytes untransferred\n",
- td->urb->ep->desc.bEndpointAddress,
- td->urb->transfer_buffer_length,
-
EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)));
+
/* Fast path - was this the last TRB in the TD for this URB? */
if (event_trb == td->last_trb) {
if (EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) {
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On 01/21/2014 12:11 PM, Kishon Vijay Abraham I wrote:
> Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs),
> do not return from probe if the USB PHY library returns -ENODEV as that
> indicates the platform does not have PHY.
>
> Signed-off-by: Kishon Vijay Abraham I
Reviewed-by: Roger Quadros
cheers,
-roger
> ---
> drivers/usb/dwc3/core.c | 34 ++
> 1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index a49217a..e009d4e 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -411,32 +411,26 @@ static int dwc3_probe(struct platform_device *pdev)
>
> if (IS_ERR(dwc->usb2_phy)) {
> ret = PTR_ERR(dwc->usb2_phy);
> -
> - /*
> - * if -ENXIO is returned, it means PHY layer wasn't
> - * enabled, so it makes no sense to return -EPROBE_DEFER
> - * in that case, since no PHY driver will ever probe.
> - */
> - if (ret == -ENXIO)
> + if (ret == -ENXIO || ret == -ENODEV) {
> + dwc->usb2_phy = NULL;
> + } else if (ret == -EPROBE_DEFER) {
> return ret;
> -
> - dev_err(dev, "no usb2 phy configured\n");
> - return -EPROBE_DEFER;
> + } else {
> + dev_err(dev, "no usb2 phy configured\n");
> + return ret;
> + }
> }
>
> if (IS_ERR(dwc->usb3_phy)) {
> ret = PTR_ERR(dwc->usb3_phy);
> -
> - /*
> - * if -ENXIO is returned, it means PHY layer wasn't
> - * enabled, so it makes no sense to return -EPROBE_DEFER
> - * in that case, since no PHY driver will ever probe.
> - */
> - if (ret == -ENXIO)
> + if (ret == -ENXIO || ret == -ENODEV) {
> + dwc->usb3_phy = NULL;
> + } else if (ret == -EPROBE_DEFER) {
> return ret;
> -
> - dev_err(dev, "no usb3 phy configured\n");
> - return -EPROBE_DEFER;
> + } else {
> + dev_err(dev, "no usb3 phy configured\n");
> + return ret;
> + }
> }
>
> dwc->xhci_resources[0].start = res->start;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi Kishon,
On 01/21/2014 12:11 PM, Kishon Vijay Abraham I wrote:
> Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
> power_on and power_off the following APIs are used phy_init(), phy_exit(),
> phy_power_on() and phy_power_off().
>
> However using the old USB phy library wont be removed till the PHYs of all
> other SoC's using dwc3 core is adapted to the Generic PHY Framework.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> Changes from v3:
> * avoided using quirks
>
> Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
> drivers/usb/dwc3/core.c| 60
>
> drivers/usb/dwc3/core.h|7 +++
> 3 files changed, 71 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
> b/Documentation/devicetree/bindings/usb/dwc3.txt
> index e807635..471366d 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -6,11 +6,13 @@ Required properties:
> - compatible: must be "snps,dwc3"
> - reg : Address and length of the register set for the device
> - interrupts: Interrupts used by the dwc3 controller.
> +
> +Optional properties:
> - usb-phy : array of phandle for the PHY device. The first element
> in the array is expected to be a handle to the USB2/HS PHY and
> the second element is expected to be a handle to the USB3/SS PHY
> -
> -Optional properties:
> + - phys: from the *Generic PHY* bindings
> + - phy-names: from the *Generic PHY* bindings
> - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
>
> This is usually a subnode to DWC3 glue to which it is connected.
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index e009d4e..036d589 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -82,6 +82,11 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
>
> usb_phy_init(dwc->usb2_phy);
> usb_phy_init(dwc->usb3_phy);
> + if (dwc->usb2_generic_phy)
> + phy_init(dwc->usb2_generic_phy);
What if phy_init() fails? You need to report and fail. Same applies for all PHY
apis in this patch.
> + if (dwc->usb3_generic_phy)
> + phy_init(dwc->usb3_generic_phy);
> +
> mdelay(100);
>
> /* Clear USB3 PHY reset */
> @@ -343,6 +348,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
> {
> usb_phy_shutdown(dwc->usb2_phy);
> usb_phy_shutdown(dwc->usb3_phy);
> + if (dwc->usb2_generic_phy)
> + phy_exit(dwc->usb2_generic_phy);
> + if (dwc->usb3_generic_phy)
> + phy_exit(dwc->usb3_generic_phy);
> +
> }
>
> #define DWC3_ALIGN_MASK (16 - 1)
> @@ -433,6 +443,32 @@ static int dwc3_probe(struct platform_device *pdev)
> }
> }
>
> + dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
> + if (IS_ERR(dwc->usb2_generic_phy)) {
> + ret = PTR_ERR(dwc->usb2_generic_phy);
> + if (ret == -ENOSYS || ret == -ENODEV) {
> + dwc->usb2_generic_phy = NULL;
> + } else if (ret == -EPROBE_DEFER) {
> + return ret;
> + } else {
> + dev_err(dev, "no usb2 phy configured\n");
> + return ret;
> + }
> + }
> +
> + dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
> + if (IS_ERR(dwc->usb3_generic_phy)) {
> + ret = PTR_ERR(dwc->usb3_generic_phy);
> + if (ret == -ENOSYS || ret == -ENODEV) {
> + dwc->usb3_generic_phy = NULL;
> + } else if (ret == -EPROBE_DEFER) {
> + return ret;
> + } else {
> + dev_err(dev, "no usb3 phy configured\n");
> + return ret;
> + }
> + }
> +
> dwc->xhci_resources[0].start = res->start;
> dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
> DWC3_XHCI_REGS_END;
> @@ -482,6 +518,11 @@ static int dwc3_probe(struct platform_device *pdev)
> usb_phy_set_suspend(dwc->usb2_phy, 0);
> usb_phy_set_suspend(dwc->usb3_phy, 0);
>
> + if (dwc->usb2_generic_phy)
> + phy_power_on(dwc->usb2_generic_phy);
> + if (dwc->usb3_generic_phy)
> + phy_power_on(dwc->usb3_generic_phy);
> +
Is it OK to power on the phy before phy_init()?
I suggest to move phy_init() from core_soft_reset() to here, just before
phy_power_on().
> ret = dwc3_event_buffers_setup(dwc);
> if (ret) {
> dev_err(dwc->dev, "failed to setup event buffers\n");
> @@ -565,6 +606,10 @@ err2:
> err1:
> usb_phy_set_suspend(dwc->usb2_phy, 1);
> usb_phy_set_suspend(dwc->usb3_phy, 1);
> + if (dwc->usb2_generic_phy)
> + phy_power_off(dwc->usb2_generic_phy);
> + if (dwc->usb3_generic_phy)
> + phy_power_off(dwc->
Re: USB EHCI broken on Celeron N2920 (Bay Trail)
I'm sorry forgot kernel version: 3.13.0 . -- Chris Cheng 鄭宇廷 Atrust Computer Corp. 冠信電腦股份有限公司 http://www.atrustcorp.com Tel: +886-3-3288837 ext.1074 3F., No.361, Fusing 1st Rd., Gueishan Township, Taoyuan County 333, Taiwan 333 桃園縣龜山鄉復興一路361號3F 2014/1/21 Chris Cheng : > Here is kernel message: > > [2.939251] usb 1-1: new high-speed USB device number 2 using ehci-pci > [ 18.490987] usb 1-1: device not accepting address 2, error -110 > [ 18.595095] usb 1-1: new high-speed USB device number 3 using ehci-pci > [ 34.182233] usb 1-1: device not accepting address 3, error -110 > [ 34.286352] usb 1-1: new high-speed USB device number 4 using ehci-pci > [ 44.744589] usb 1-1: device not accepting address 4, error -110 > [ 44.849590] usb 1-1: new high-speed USB device number 5 using ehci-pci > [ 55.307858] usb 1-1: device not accepting address 5, error -110 > [ 55.310239] hub 1-0:1.0: unable to enumerate USB device on port 1 > > Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
libusbg 0.1.0 (formerly libgadget)
libusbg 0.1.0 is released. libusbg is a library that provides a C API to the kernel USB gadget configfs API. It simplifies creation and management of USB gadget devices from C applications. Get it at: git://github.com/libusbg/libusbg.git API docs at: http://libusbg.github.com/group__libusbg.html http://libusbg.github.com/examples.html Changes since libgadget 0.0.1: - libusbg now has a new name to avoid conflicts with an existing libgadget libary - use the standard /sys/kernel/config mount path in examples - a new project home on github Going forward we have a couple contributors with clean up and bug fixes that will be rolled into libusbg once comments are addressed. Support for FunctionFS (merged for 3.14) will also be added. -Matt -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/2] libusbg: clean up and error handling
On Thu, Nov 07, 2013 at 12:55:04PM -0500, Alan Ott wrote: > On 11/06/2013 08:04 AM, Stanislaw Wadas wrote: > >In reference to the message sent by Andrzej Pietrasiewicz > >(about libusbg (formerly libgadget)) I would like to propose > >some changes to libusbg. > > > >Creating directories is now performed after successful memory > >allocation and gadget function creation. Hard coded values are > >replaced with constants. Error handling added to functions that > >operates on strings. > > > >Changes since v3: > > - changes are now in four separate files > > - fixed code indentation > > > >Changes since v2: > > - fixed code indentation > > - removed unused variable ret > > > >Changes since v1: > > - fixed typos in MAX_LENGTH throughout > > > >Stanislaw Wadas (2): > > libusbg: Moved mkdir functions, added MAX_LENGTH & MAX_PATH_LENGTH > > libusbg: added fputs()/fgets() error handling > > > > It's preferred to use present tense in commit messages: > "Move mkdir functions and add " > "Add puts()/gets() error handling" Yes, thank you ;) Stanislaw: can you incorporate Alan's comment please? Except for the remaining comment on 2/2 and changing to use present tense in the commit messages, I will be able to merge this series. Thanks, Matt -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] libusbg: Path length replaced with MAX_LENGTH & MAX_PATH_LENGTH
On Wed, Nov 06, 2013 at 02:04:40PM +0100, Stanislaw Wadas wrote:
> 256 hard coded value has been replaced by two defined
> constants MAX_LENGTH and MAX_PATH_LENGTH
In both the patch summary and commit message, please remember to use
present tense as Alan pointed out. e.g.:
"
Subject: [PATCH v4 1/4" libusbg: replace array lengths with defines
Replace hard coded value of 256 by two constant defines,
MAX_PATH_LENGTH and MAX_STR_LENGTH.
"
>
> Signed-off-by: Stanislaw Wadas
> ---
> Changes since v1:
> - fixed typos in MAX_LENGTH throughout
>
> include/gadget/gadget.h | 27 +++
> src/gadget.c| 46 +++---
> 2 files changed, 38 insertions(+), 35 deletions(-)
>
> diff --git a/include/gadget/gadget.h b/include/gadget/gadget.h
> index 6a32c39..9bca97e 100644
> --- a/include/gadget/gadget.h
> +++ b/include/gadget/gadget.h
> @@ -33,13 +33,16 @@
> #define DEFAULT_UDC NULL
> #define LANG_US_ENG 0x0409
>
> +#define MAX_LENGTH 256
> +#define MAX_PATH_LENGTH 256
> +
> /**
> * @struct state
> * @brief State of the gadget devices in the system
> */
> struct state
> {
> - char path[256];
> + char path[MAX_PATH_LENGTH];
>
> TAILQ_HEAD(ghead, gadget) gadgets;
> };
> @@ -51,8 +54,8 @@ struct state
> struct gadget
> {
> char name[40];
> - char path[256];
> - char udc[256];
> + char path[MAX_PATH_LENGTH];
> + char udc[MAX_LENGTH];
As David is suggesting, MAX_LENGTH needs a better name. I'm fine with
MAX_PATH_LENGTH. s/MAX_LENGTH/MAX_STR_LENGTH/ would be much better.
-Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/2] libusbg: clean up and error handling
Also, there's something wrong with how you are sending your series. This cover letter is broken and showing only a 2 part series but you've posted a 4 part series. Please fix this for v5. -Matt On Wed, Nov 06, 2013 at 02:04:39PM +0100, Stanislaw Wadas wrote: > In reference to the message sent by Andrzej Pietrasiewicz > (about libusbg (formerly libgadget)) I would like to propose > some changes to libusbg. > > Creating directories is now performed after successful memory > allocation and gadget function creation. Hard coded values are > replaced with constants. Error handling added to functions that > operates on strings. > > Changes since v3: > - changes are now in four separate files > - fixed code indentation > > Changes since v2: > - fixed code indentation > - removed unused variable ret > > Changes since v1: > - fixed typos in MAX_LENGTH throughout > > Stanislaw Wadas (2): > libusbg: Moved mkdir functions, added MAX_LENGTH & MAX_PATH_LENGTH > libusbg: added fputs()/fgets() error handling > > include/gadget/gadget.h | 27 -- > src/gadget.c| 95 > ++- > 2 files changed, 67 insertions(+), 55 deletions(-) > > -- > 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 00/12] libusbg: Cleanup, bug fix and refactoring.
On Thu, Jan 02, 2014 at 05:13:30PM +0100, Krzysztof Opasiak wrote: > Dear Matt, > > Please excuse me my passivity after discussuon about libusbg some time ago. > I had to close some other issues before taking up this one. > > Recently I looked into code of libusbg, build it and found some errors > which are fixed in attached patches. Moreover I have done some clean up > and minor refactoring. > > I hope that you will accept enclosed patched and update the repository. Hi Krzysztof, Thanks, these all look pretty good. A few things: 1) This v3 series didn't get marked as [PATCH v3 ...] so it was a bit confusing when I was applying to test. 2) In the future, please mention any dependencies in your cover letter. In this case, you have some dependencies on Stanislaw's series: http://www.spinics.net/lists/linux-usb/msg96859.html There's some open comments on that series atm that I need addressed. 3) The series will need to be rebased on the current libusbg. It was necessary to do a flag day api change early on to get rid of the libgadget-induced naming. It should be an easy rebase on top of the tip of libusbg, now at git://github.com/libusbg/libusbg.git Thanks, Matt > Changes since v2: > - replace // comments with /* */ > - add missing parenthesis in commit message > - fix minor spelling mistakes in commit messages > > Changes since v1: > - replace memcpy with direct structure assignment > - Change goto lables on more meaningful in example > - Remove additional check in gadget_read_string > > > Krzysztof Opasiak (12): > libusbg: Surround header with include guards. > libusbg: Add missing return statement in non-void functions. > libusbg: Fix gadget-acm-ecm example to cleanup at exit. > libusbg: Move directory creation before writing attributes. > libusbg: Fix memory leak when unable to create directory. > libusbg: Add error handling to gadget_read_string(). > libusbg: Add missing config attrs parsing while new config creation. > libusbg: Separate parsing gadget attributes and strings. > libusbg: Initialize gadget attributes and strings while gadget > creation. > libusbg: Move symlink creation after memory allocation. > libusbg: Replace memcpy with structure assignment. > libusbg: Replace directory names with defines. > > examples/gadget-acm-ecm.c | 20 - > include/gadget/gadget.h |4 ++ > src/gadget.c | 110 > + > 3 files changed, 86 insertions(+), 48 deletions(-) > > -- > 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 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On Tue, Jan 21, 2014 at 03:41:38PM +0530, Kishon Vijay Abraham I wrote: > Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs), > do not return from probe if the USB PHY library returns -ENODEV as that this isn't correct, they all have PHYs, some of them might not be controllable. > indicates the platform does not have PHY. not really, that indicates the current platform tried to grab a PHY and the PHY doesn't exist. If there's anybody with a non-controllable PHY and someone gives me a really good reason for not using the generic no-op PHY, then we should add a flag and we could: if (!likely(dwc->flags & DWC3_USB2PHY_DRIVER_NOT_NEEDED)) dwc3_grab_phys(dwc); But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Your commit log really needs quite some extra work. What are you trying to achieve ? Is this related to AM437x which doesn't have a USB3 PHY due to the way the HW was wired up ? If so, please mention it. Explain how AM437x HW was wired up, why it lacks a USB3 PHY and why we should support it the way you want. That sort of detail needs to be clear in the commit log, specially considering the peculiar nature of AM43xx which uses a USB3 IP in USB2-only mode, that needs to be documented in the commit log :-) cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 01/12] libusbg: Surround header with include guards.
On Thu, Jan 02, 2014 at 05:13:31PM +0100, Krzysztof Opasiak wrote: > Surround header with include guards to protect against > multiple inclusion. > > Signed-off-by: Krzysztof Opasiak > --- > include/gadget/gadget.h |4 > 1 file changed, 4 insertions(+) > > diff --git a/include/gadget/gadget.h b/include/gadget/gadget.h > index 9bca97e..f3c08e9 100644 > --- a/include/gadget/gadget.h > +++ b/include/gadget/gadget.h > @@ -14,6 +14,9 @@ > * Lesser General Public License for more details. > */ > > +#ifndef __GADGET_H__ > +#define __GADGET_H__ Please rename these to __USBG_H__ due to the library rename. > + > #include > #include > #include > @@ -438,3 +441,4 @@ extern void gadget_set_net_qmult(struct function *f, int > qmult); > /** > * @} > */ > +#endif //__GADGET_H__ > -- > 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: USB serial gadget: Interchanged order of TX packets
Hi,
On Tue, Jan 21, 2014 at 10:35:54AM +0100, Philip Philippe wrote:
> Hi,
>
> When using the USB serial gadget it regularly occurs that USB packets
> arrive at the host in the wrong order. I am using kernel version 3.4
> with the corresponding RT patch on an ARM Cortex-A8 platform.
>
> The problem appears to be in file 'drivers/usb/gadget/u_serial.c'.
>
> It seems that kernel thread "irq/92-musb-hdr" and a userspace thread
> have a race when sending data, in function 'gs_start_tx'. The kernel
> thread calls 'gs_start_tx' from the callback function
> 'gs_write_complete', a userspace thread through 'gs_write' and
> 'gs_flush_chars'. The problem is triggered by the fact that the
> spinlock 'port_lock' is dropped temporarily in 'gs_start_tx' during
> the time 'usb_ep_queue' is executed. There might be good reasons,
> which I am unaware off, to do that. So, suppose that the userspace
> thread has just released the spinlock and the scheduler selects at
> that moment the (normally) higher prioritized kernel thread, which was
> waiting at the 'port_lock' in 'gs_write_complete' to run. This will
> allow the kernel thread to overtake the sleeping userspace thread and
> send out the "next" USB packet first!
>
> I can't reproduce the problem if I disable the RT patch, but I think
> that the problem might as well appear, if for instance an interrupt
> just happens during the time the lock is dropped in 'gs_start_rx'. But
> if you prove me wrong, I'm happy to post it in the RT mailing list
> instead.
>
> The provided patch is a diff to the mainline. I've introduced a flag
> to mark when there is already a thread busy in the loop emptying the
> pool in 'gs_start_tx'. In that case another thread would just skip the
> loop. This seems to work for me but it might not be foolproof and not
> elegant enough.
>
> By checking the code of the mainline I didn't find any relevant change
> that would indicate that the issue has been already solved in the
> meantime. For the moment it would be too much work for me to make the
> mainline runnable on my platform.
>
> Please CC me and arn...@mind.be as we are not subscribed.
cool, please have a look at Documentation/SubmittingPatches in order to
send it in the proper format. You have information in your commit log
which shouldn't be in the commit log.
Also, let us know how you tested this and, if you have a simple test
application, it would be nice to share it.
cheers
> diff --git a/drivers/usb/gadget/u_serial.c b/drivers/usb/gadget/u_serial.c
> index b369292..d2682f5 100644
> --- a/drivers/usb/gadget/u_serial.c
> +++ b/drivers/usb/gadget/u_serial.c
> @@ -116,6 +116,7 @@ struct gs_port {
> int write_allocated;
> struct gs_buf port_write_buf;
> wait_queue_head_t drain_wait; /* wait while writes drain */
> + boolwrite_busy;
>
> /* REVISIT this state ... */
> struct usb_cdc_line_coding port_line_coding;/* 8-N-1 etc */
> @@ -366,7 +367,7 @@ __acquires(&port->port_lock)
> int status = 0;
> booldo_tty_wake = false;
>
> - while (!list_empty(pool)) {
> + while (!port->write_busy && !list_empty(pool)) {
> struct usb_request *req;
> int len;
>
> @@ -396,9 +397,11 @@ __acquires(&port->port_lock)
>* NOTE that we may keep sending data for a while after
>* the TTY closed (dev->ioport->port_tty is NULL).
>*/
> + port->write_busy = true;
> spin_unlock(&port->port_lock);
> status = usb_ep_queue(in, req, GFP_ATOMIC);
> spin_lock(&port->port_lock);
> + port->write_busy = false;
>
> if (status) {
> pr_debug("%s: %s %s err %d\n",
> @@ -770,6 +773,7 @@ static int gs_open(struct tty_struct *tty, struct file
> *file)
> /* Do the "real open" */
> spin_lock_irq(&port->port_lock);
>
> + port->write_busy = false;
seems like you should make sure to clear this during close too.
--
balbi
signature.asc
Description: Digital signature
[PATCH 0/2] target refcounting infrastructure fixes for usb
After about 3 iterations as an RFC, I'm elevating this to a patch. If it works, I'll try for this merge window with a delayed backport to stable (after the next kernel release to make sure no bugs got introduced). This set should fix our target problems with USB by making the target visibility properly reference counted. James Bottomley (2): [SCSI] fix our current target reap infrastructure. [SCSI] dual scan thread bug fix drivers/scsi/scsi_scan.c | 112 - drivers/scsi/scsi_sysfs.c | 20 +--- include/scsi/scsi_device.h | 3 +- 3 files changed, 86 insertions(+), 49 deletions(-) -- 1.8.4.3 -- To unsubscribe from this list: send the line "unsubscribe 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] dual scan thread bug fix
In the highly unusual case where two threads are running concurrently through
the scanning code scanning the same target, we run into the situation where
one may allocate the target while the other is still using it. In this case,
because the reap checks for STARGET_CREATED and kills the target without
reference counting, the second thread will do the wrong thing on reap.
Fix this by reference counting even creates and doing the STARGET_CREATED
check in the final put.
Signed-off-by: James Bottomley
---
drivers/scsi/scsi_scan.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 5fad646..4109530 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -320,6 +320,7 @@ static void scsi_target_destroy(struct scsi_target *starget)
struct Scsi_Host *shost = dev_to_shost(dev->parent);
unsigned long flags;
+ starget->state = STARGET_DEL;
transport_destroy_device(dev);
spin_lock_irqsave(shost->host_lock, flags);
if (shost->hostt->target_destroy)
@@ -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_CREATED) {
+ transport_remove_device(&starget->dev);
+ device_del(&starget->dev);
+ }
scsi_target_destroy(starget);
}
@@ -506,11 +513,13 @@ static struct scsi_target *scsi_alloc_target(struct
device *parent,
*/
void scsi_target_reap(struct scsi_target *starget)
{
+ /*
+* serious problem if this triggers: STARGET_DEL is only set in the if
+* the reap_ref drops to zero, so we're trying to do another final put
+* on an already released kref
+*/
BUG_ON(starget->state == STARGET_DEL);
- if (starget->state == STARGET_CREATED)
- scsi_target_destroy(starget);
- else
- scsi_target_reap_ref_put(starget);
+ scsi_target_reap_ref_put(starget);
}
/**
--
1.8.4.3
--
To unsubscribe from this list: send the line "unsubscribe 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] fix our current target reap infrastructure
This patch eliminates the reap_ref and replaces it with a proper kref.
On last put of this kref, the target is removed from visibility in
sysfs. The final call to scsi_target_reap() for the device is done from
__scsi_remove_device() and only if the device was made visible. This
ensures that the target disappears as soon as the last device is gone
rather than waiting until final release of the device (which is often
too long).
Reviewed-by: Alan Stern
Signed-off-by: James Bottomley
---
drivers/scsi/scsi_scan.c | 99 --
drivers/scsi/scsi_sysfs.c | 20 +++---
include/scsi/scsi_device.h | 3 +-
3 files changed, 75 insertions(+), 47 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 307a811..5fad646 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -371,6 +371,31 @@ static struct scsi_target *__scsi_find_target(struct
device *parent,
}
/**
+ * scsi_target_reap_ref_release - remove target from visibility
+ * @kref: the reap_ref in the target being released
+ *
+ * Called on last put of reap_ref, which is the indication that no device
+ * under this target is visible anymore, so render the target invisible in
+ * sysfs. Note: we have to be in user context here because the target reaps
+ * should be done in places where the scsi device visibility is being removed.
+ */
+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;
+ scsi_target_destroy(starget);
+}
+
+static void scsi_target_reap_ref_put(struct scsi_target *starget)
+{
+ kref_put(&starget->reap_ref, scsi_target_reap_ref_release);
+}
+
+/**
* scsi_alloc_target - allocate a new or find an existing target
* @parent:parent of the target (need not be a scsi host)
* @channel: target channel number (zero if no channels)
@@ -392,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) {
@@ -401,7 +426,7 @@ static struct scsi_target *scsi_alloc_target(struct device
*parent,
}
dev = &starget->dev;
device_initialize(dev);
- starget->reap_ref = 1;
+ kref_init(&starget->reap_ref);
dev->parent = get_device(parent);
dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id);
dev->bus = &scsi_bus_type;
@@ -441,29 +466,36 @@ static struct scsi_target *scsi_alloc_target(struct
device *parent,
return starget;
found:
- found_target->reap_ref++;
+ /*
+* 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;
}
- /* Unfortunately, we found a dying target; need to
-* wait until it's dead before we can get a new one */
+ /*
+* 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 being released, it's
+* already invisible and the reap_ref is irrelevant. If we call
+* scsi_target_reap() we might spuriously do another device_del() on
+* an already invisible target.
+*/
put_device(&found_target->dev);
- flush_scheduled_work();
+ /*
+* length of time is irrelevant here, we just want to yield the CPU
+* for a tick to avoid busy waiting for the target to die.
+*/
+ msleep(1);
goto retry;
}
-static void scsi_target_reap_usercontext(struct work_struct *work)
-{
- struct scsi_target *starget =
- container_of(work, struct scsi_target, ew.work);
-
- transport_remove_device(&starget->dev);
- device_del(&starget->dev);
- scsi_target_destroy(starget);
-}
-
/**
* scsi_target_reap - check to see if target is in use and destroy if not
* @starget: target to be checked
@@ -474,28 +506,11 @@ static void scsi_target_reap_usercontext(struct
work_struct *work)
*/
void scsi_target_reap(struct scsi_target *starget)
{
- struct Scsi_Host *shost = dev_to_shost(sta
Re: usb audio breaks ohci-pci
On Wed, 15 Jan 2014 17:04:05 -0500 (EST), Alan Stern wrote: > On Wed, 15 Jan 2014, Dennis New wrote: > > > When listening to my usb-bluetooth audio headset, the usb subsystem > > (I think) on my laptop crashes, the bluetooth/audio is lost, and my > > mplayer program is left in an "uninterruptible sleep" (D) state and > > cannot be killed. The only way to get my usb-audio working again is > > to reboot. > > ... > > > The crash is difficult to reproduce, sometimes it can occur after a > > few minutes of a clean boot, and sometimes after a few days. After > > the audio/usb abruptly stops, the syslog simply says: > > > > kernel: ALSA sound/usb/endpoint.c:501 timeout: still 3 active > > urbs on EP #3 > > > > And then after a few minutes (after enabling some kernel debugging > > options), I get some call traces of the hung task(s), which have > > "ohci_urb_dequeue" in common. > > > > http://dennisn.dyndns.org/guest/pubstuff/debug-usbaudio/crash1.log > > http://dennisn.dyndns.org/guest/pubstuff/debug-usbaudio/crash2.log > > http://dennisn.dyndns.org/guest/pubstuff/debug-usbaudio/crash3.log > > The logs show that the khubd thread is hung, waiting for the ohci-hcd > driver to finish cancelling a transfer. > > > The problem is not with my device, since it occurs with another > > identical piece of hardware. It maybe be a problem with my laptop, > > since my friend does not seem to be experiencing this problem, > > although her laptop does not use ohci-pci. > > > > My usb controllers are: > > > > 1002:4374 00:13.0 USB controller: Advanced Micro Devices [AMD] > > nee ATI IXP SB400 USB Host Controller (prog-if 10 [OHCI]) > > 1002:4375 00:13.1 USB controller: Advanced Micro Devices [AMD] > > nee ATI IXP SB400 USB Host Controller (prog-if 10 [OHCI]) > > 1002:4373 00:13.2 USB controller: Advanced Micro Devices [AMD] > > nee ATI IXP SB400 USB2 Host Controller (prog-if 20 [EHCI]) > > Your first step in attacking this problem should be to use the most > up-to-date kernel available, which at the moment is 3.13-rc8. Be > sure to enable CONFIG_FRAME_POINTER along with CONFIG_USB_DEBUG. > > If the problem occurs again, collect a copy of the output from the > dmesg command (_not_ a copy of the system log). Then go into > /sys/kernel/debug/usb/ohci and collect copies of all the files in the > two subdirectories. > > It may be necessary to apply some debugging patches to get more > information. Erase my last post, the problem still exists with 3.13-rc8. I did enable FRAME_POINTER and USB_DEBUG, but forgot DEBUG_FS. (I will do that now.) There doesn't appear to be anything more informative in the dmesg/syslog outputs. (My dmesg output was truncated, but basically the same as my syslog.) http://dennisn.dyndns.org/guest/pubstuff/debug-usbaudio/crash4.log Although, as you can see, it's difficult to reproduce -- my 3.13-rc8 was running just fine for almost a week! -- To unsubscribe from this list: send the line "unsubscribe 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] ehci-platform: Add support for controllers with big-endian regs / descriptors
This uses the already documented devicetree booleans for this.
Signed-off-by: Hans de Goede
---
drivers/usb/host/Kconfig | 3 +++
drivers/usb/host/ehci-platform.c | 33 +++--
2 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 237d7b1..4af41f3 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -256,6 +256,9 @@ config USB_EHCI_ATH79
config USB_EHCI_HCD_PLATFORM
tristate "Generic EHCI driver for a platform device"
depends on !PPC_OF
+ # Support BE on architectures which have readl_be
+ select USB_EHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE ||
SPARC || PPC32 || PPC64)
+ select USB_EHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE ||
SPARC || PPC32 || PPC64)
default n
---help---
Adds an EHCI host driver for a generic platform device, which
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index d8aebc0..5888abb 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -55,8 +55,10 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
hcd->has_tt = pdata->has_tt;
ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
- ehci->big_endian_desc = pdata->big_endian_desc;
- ehci->big_endian_mmio = pdata->big_endian_mmio;
+ if (pdata->big_endian_desc)
+ ehci->big_endian_desc = 1;
+ if (pdata->big_endian_mmio)
+ ehci->big_endian_mmio = 1;
if (pdata->pre_setup) {
retval = pdata->pre_setup(hcd);
@@ -142,6 +144,7 @@ static int ehci_platform_probe(struct platform_device *dev)
struct resource *res_mem;
struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
struct ehci_platform_priv *priv;
+ struct ehci_hcd *ehci;
int err, irq, clk = 0;
if (usb_disabled())
@@ -177,8 +180,34 @@ static int ehci_platform_probe(struct platform_device *dev)
platform_set_drvdata(dev, hcd);
dev->dev.platform_data = pdata;
priv = hcd_to_ehci_priv(hcd);
+ ehci = hcd_to_ehci(hcd);
if (pdata == &ehci_platform_defaults && dev->dev.of_node) {
+ if (of_property_read_bool(dev->dev.of_node, "big-endian-regs"))
+ ehci->big_endian_mmio = 1;
+
+ if (of_property_read_bool(dev->dev.of_node, "big-endian-desc"))
+ ehci->big_endian_desc = 1;
+
+ if (of_property_read_bool(dev->dev.of_node, "big-endian"))
+ ehci->big_endian_mmio = ehci->big_endian_desc = 1;
+
+#ifndef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
+ if (ehci->big_endian_mmio) {
+ dev_err(&dev->dev,
+ "Error big-endian-regs not compiled in\n");
+ err = -EINVAL;
+ goto err_put_hcd;
+ }
+#endif
+#ifndef CONFIG_USB_EHCI_BIG_ENDIAN_DESC
+ if (ehci->big_endian_desc) {
+ dev_err(&dev->dev,
+ "Error big-endian-desc not compiled in\n");
+ err = -EINVAL;
+ goto err_put_hcd;
+ }
+#endif
priv->phy = devm_phy_get(&dev->dev, "usb");
if (IS_ERR(priv->phy)) {
err = PTR_ERR(priv->phy);
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe 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] ohci-platform: Add support for controllers with big-endian regs / descriptors
Note this commit uses the same devicetree booleans for this as the ones
already existing in the usb-ehci bindings.
Signed-off-by: Hans de Goede
---
Documentation/devicetree/bindings/usb/usb-ohci.txt | 3 +++
drivers/usb/host/Kconfig | 4
drivers/usb/host/ohci-platform.c | 27 ++
3 files changed, 34 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt
b/Documentation/devicetree/bindings/usb/usb-ohci.txt
index f9d6c73..aa1d765 100644
--- a/Documentation/devicetree/bindings/usb/usb-ohci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
@@ -6,6 +6,9 @@ Required properties:
- interrupts : ohci controller interrupt
Optional properties:
+- big-endian-regs : boolean, set this for hcds with big-endian registers
+- big-endian-desc : boolean, set this for hcds with big-endian descriptors
+- big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
- clocks : a list of phandle + clock specifier pairs
- phys : phandle + phy specifier pair
- phy-names : "usb"
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index e28cbe0..237d7b1 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -512,6 +512,10 @@ config USB_CNS3XXX_OHCI
config USB_OHCI_HCD_PLATFORM
tristate "Generic OHCI driver for a platform device"
+ # Always support LE, support BE on architectures which have readl_be
+ select USB_OHCI_LITTLE_ENDIAN
+ select USB_OHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE ||
SPARC || PPC32 || PPC64)
+ select USB_OHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE ||
SPARC || PPC32 || PPC64)
default n
---help---
Adds an OHCI host driver for a generic platform device, which
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index b2d0e1e..71e9d8e 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -128,6 +128,7 @@ static int ohci_platform_probe(struct platform_device *dev)
struct resource *res_mem;
struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev);
struct ohci_platform_priv *priv;
+ struct ohci_hcd *ohci;
int err, irq, clk = 0;
if (usb_disabled())
@@ -164,8 +165,34 @@ static int ohci_platform_probe(struct platform_device *dev)
platform_set_drvdata(dev, hcd);
dev->dev.platform_data = pdata;
priv = hcd_to_ohci_priv(hcd);
+ ohci = hcd_to_ohci(hcd);
if (pdata == &ohci_platform_defaults && dev->dev.of_node) {
+ if (of_property_read_bool(dev->dev.of_node, "big-endian-regs"))
+ ohci->flags |= OHCI_QUIRK_BE_MMIO;
+
+ if (of_property_read_bool(dev->dev.of_node, "big-endian-desc"))
+ ohci->flags |= OHCI_QUIRK_BE_DESC;
+
+ if (of_property_read_bool(dev->dev.of_node, "big-endian"))
+ ohci->flags |= OHCI_QUIRK_BE_MMIO | OHCI_QUIRK_BE_DESC;
+
+#ifndef CONFIG_USB_OHCI_BIG_ENDIAN_MMIO
+ if (ohci->flags & OHCI_QUIRK_BE_MMIO) {
+ dev_err(&dev->dev,
+ "Error big-endian-regs not compiled in\n");
+ err = -EINVAL;
+ goto err_put_hcd;
+ }
+#endif
+#ifndef CONFIG_USB_OHCI_BIG_ENDIAN_DESC
+ if (ohci->flags & OHCI_QUIRK_BE_DESC) {
+ dev_err(&dev->dev,
+ "Error big-endian-desc not compiled in\n");
+ err = -EINVAL;
+ goto err_put_hcd;
+ }
+#endif
priv->phy = devm_phy_get(&dev->dev, "usb");
if (IS_ERR(priv->phy)) {
err = PTR_ERR(priv->phy);
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe 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 controllers with big-endian regs / descriptors
On Tue, 21 Jan 2014, Hans de Goede wrote: > Note this commit uses the same devicetree booleans for this as the ones > already existing in the usb-ehci bindings. > > Signed-off-by: Hans de Goede > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -512,6 +512,10 @@ config USB_CNS3XXX_OHCI > > config USB_OHCI_HCD_PLATFORM > tristate "Generic OHCI driver for a platform device" > + # Always support LE, support BE on architectures which have readl_be > + select USB_OHCI_LITTLE_ENDIAN > + select USB_OHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || > SPARC || PPC32 || PPC64) > + select USB_OHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || > SPARC || PPC32 || PPC64) > default n The comment line above is slightly misleading. USB_OHCI_LITTLE_ENDIAN doesn't exactly mean to include support for little-endian controllers; it means include mixed-endian support if either USB_OHCI_BIG_ENDIAN_DESC or USB_OHCI_BIG_ENDIAN_MMIO is set. That is, the driver determines at runtime whether a particular controller is big-endian or little-endian, rather than choosing to support one or the other at compile time. In any case, the style we have adopted is that these select lines go in the arch-specific defconfig, not here. For example, check out the occurrences of EHCI in arch/mips/Kconfig. Also, I'm not sure how you came up with that list of architectures for the two selects; it's hard to say if they are right. For instance, why did you include AVR32? The changes to the driver itself look fine. Similar comments apply to the ehci-platform 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
Re: [PATCH v7 1/2] ohci-platform: Add support for devicetree instantiation
Hello.
On 01/15/2014 10:24 PM, 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
Acked-by: Alan Stern
[...]
Have only found time to fully read the patches just now...
diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt
b/Documentation/devicetree/bindings/usb/usb-ohci.txt
new file mode 100644
index 000..f9d6c73
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
@@ -0,0 +1,22 @@
+USB OHCI controllers
+
+Required properties:
+- compatible : "usb-ohci"
+- reg : ohci controller register range (address and length)
+- interrupts : ohci controller interrupt
+
+Optional properties:
+- clocks : a list of phandle + clock specifier pairs
+- phys : phandle + phy specifier pair
+- phy-names : "usb"
+
+Example:
+
+ ohci0: ohci@0x01c14400 {
Two minor nits: there should be no "0x" in the address part of the node
name. And according to ePAPR [1], "the name of a node should be somewhat
generic, reflecting the function of the device and not its precise programming
model. If appropriate, the name should be one of the following choices:
[...]
- usb".
Same comments for "usb-ehci" binding.
+ compatible = "allwinner,sun4i-a10-ohci", "usb-ohci";
+ reg = <0x01c14400 0x100>;
+ interrupts = <64>;
+ clocks = <&usb_clk 6>, <&ahb_gates 2>;
+ phys = <&usbphy 1>;
+ phy-names = "usb";
+ };
[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ohci-platform: Add support for controllers with big-endian regs / descriptors
Hi, On 01/21/2014 05:40 PM, Alan Stern wrote: On Tue, 21 Jan 2014, Hans de Goede wrote: Note this commit uses the same devicetree booleans for this as the ones already existing in the usb-ehci bindings. Signed-off-by: Hans de Goede --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -512,6 +512,10 @@ config USB_CNS3XXX_OHCI config USB_OHCI_HCD_PLATFORM tristate "Generic OHCI driver for a platform device" + # Always support LE, support BE on architectures which have readl_be + select USB_OHCI_LITTLE_ENDIAN + select USB_OHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64) + select USB_OHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64) default n The comment line above is slightly misleading. USB_OHCI_LITTLE_ENDIAN doesn't exactly mean to include support for little-endian controllers; it means include mixed-endian support if either USB_OHCI_BIG_ENDIAN_DESC or USB_OHCI_BIG_ENDIAN_MMIO is set. That is, the driver determines at runtime whether a particular controller is big-endian or little-endian, rather than choosing to support one or the other at compile time. Right I already knew that, that is what the "Always" tried to summarize. In any case, the style we have adopted is that these select lines go in the arch-specific defconfig, not here. Ok, so I should drop the Kconfig parts of both patches ? For example, check out the occurrences of EHCI in arch/mips/Kconfig. Also, I'm not sure how you came up with that list of architectures for the two selects; it's hard to say if they are right. For instance, why did you include AVR32? I included all archs which defines readl_be in asm/io.h 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
[PATCH 5/5] xhci: Simpify processing LINK TRBs when writing trb
The ring write pointer has to be left pointing at any LINK TRB
when all the TRB for a TD have been added.
This is currently acheived by advancing past a LINK in prepare_ring()
and conditonally by queue_trb() (actually inc_enq) after writing the TRB
if the caller passed 'more_trbs_coming = true'.
Instead just advance the pointer past a LINK TRB in queue_trb() before
writing the required entry. Invert ring->cycle_state when the new
ring segment is the first one.
The value saved in 'td->last_trb' is now incorrect (it could refer to
a LINK TRB). Correct by saving 'ring->enqueue - 1' after queue_trb()
has been called for the last fragment.
Done via a static inline function since ity is very implementation
specific.
It doesn't matter if td->first_trb points to a LINK, it will be skipped.
Remove the now-unused more_trbs_coming parameter to queue_trb().
The code never generates LINK TRB pointing to LINK TRB - so the loops
in the old code are not needed. Similarly inc_enq() has never been
called for event rings - so those checks aren't needed.
Signed-off-by: David Laight
---
drivers/usb/host/xhci-ring.c | 211 ---
drivers/usb/host/xhci.h | 7 ++
2 files changed, 64 insertions(+), 154 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0d0bd7f..639704e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -117,12 +117,6 @@ static int last_trb(struct xhci_hcd *xhci, struct
xhci_ring *ring,
return TRB_TYPE_LINK_LE32(trb->link.control);
}
-static int enqueue_is_link_trb(struct xhci_ring *ring)
-{
- struct xhci_link_trb *link = &ring->enqueue->link;
- return TRB_TYPE_LINK_LE32(link->control);
-}
-
union xhci_trb *xhci_find_next_enqueue(struct xhci_ring *ring)
{
/* Enqueue pointer can be left pointing to the link TRB,
@@ -187,81 +181,6 @@ static void inc_deq(struct xhci_hcd *xhci, struct
xhci_ring *ring)
}
/*
- * See Cycle bit rules. SW is the consumer for the event ring only.
- * Don't make a ring full of link TRBs. That would be dumb and this would
loop.
- *
- * If we've just enqueued a TRB that is in the middle of a TD (meaning the
- * chain bit is set), then set the chain bit in all the following link TRBs.
- * If we've enqueued the last TRB in a TD, make sure the following link TRBs
- * have their chain bit cleared (so that each Link TRB is a separate TD).
- *
- * Section 6.4.4.1 of the 0.95 spec says link TRBs cannot have the chain bit
- * set, but other sections talk about dealing with the chain bit set. This was
- * fixed in the 0.96 specification errata, but we have to assume that all 0.95
- * xHCI hardware can't handle the chain bit being cleared on a link TRB.
- *
- * @more_trbs_coming: Will you enqueue more TRBs before calling
- * prepare_transfer()?
- */
-static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring,
- bool more_trbs_coming)
-{
- u32 chain;
- union xhci_trb *next;
-
- chain = le32_to_cpu(ring->enqueue->generic.field[3]) & TRB_CHAIN;
- /* If this is not event ring, there is one less usable TRB */
- if (ring->type != TYPE_EVENT &&
- !last_trb(xhci, ring, ring->enq_seg, ring->enqueue))
- ring->num_trbs_free--;
- next = ++(ring->enqueue);
-
- ring->enq_updates++;
- /* Update the dequeue pointer further if that was a link TRB or we're at
-* the end of an event ring segment (which doesn't have link TRBS)
-*/
- while (last_trb(xhci, ring, ring->enq_seg, next)) {
- if (ring->type != TYPE_EVENT) {
- /*
-* If the caller doesn't plan on enqueueing more
-* TDs before ringing the doorbell, then we
-* don't want to give the link TRB to the
-* hardware just yet. We'll give the link TRB
-* back in prepare_ring() just before we enqueue
-* the TD at the top of the ring.
-*/
- if (!chain && !more_trbs_coming)
- break;
-
- /* If we're not dealing with 0.95 hardware or
-* isoc rings on AMD 0.96 host,
-* carry over the chain bit of the previous TRB
-* (which may mean the chain bit is cleared).
-*/
- if (!(ring->type == TYPE_ISOC &&
- (xhci->quirks & XHCI_AMD_0x96_HOST))
- && !xhci_link_trb_quirk(xhci)) {
- next->link.control &=
- cpu_to_le32(~TRB_CHAIN);
- next->link.control |=
- cpu_to_le3
[PATCH 0/5] xhci: fixes/changes to trb writing code
Some xhci controllers (eg ASMedia) don't like processing a LINK TRB when they don't own the linked-to TRB. I think this can happen due to timing races between the host cpu and the controller even when the driver is single-threading transfers. This happens a lot more often with the patch that pads the ring with NOP TRB, since it is then much more likely that a transfer will start with a LINK TRB. The first patch just changes the code that writes the ownership of ring entries to ensure that the ownership of the correct TRB is set last. It leaves a lot of code that sets up variables and values that are not used. The 'cleanup' code for partially queued isoc transfers has not been fixed, but since it is never executed (patch 2 removes it) it doesn't matter. The first patch is a candiate for back-porting to 3.12 and 3.13. The later patches remove the unused variables and values as well as a moderate amount of code from 'normal' paths. They must be applied in order, the driver should work with any of them applied - but I've only tested with just patch 1 and all 5 patches. I did consider making giveback_first_trb() ring the command doorbell, but that opened another bag of worms. David Laight (5): xhci: Don't change the ownership of LINK TRB until all the TRB are written xhci: isoc transfers can't fail when partially queued - remove cleanup code. xhci: Pass the 'ep_ring' to prepare_transfer(). xhci: Don't pass TRB_CYCLE value to queue_trb() xhci: Simpify processing LINK TRBs when writing trb. drivers/usb/host/xhci-ring.c | 409 --- drivers/usb/host/xhci.h | 8 + 2 files changed, 117 insertions(+), 300 deletions(-) -- 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 2/5] xhci: isoc transfers can't fail when partially queued - remove cleanup code.
The 'cleanup' code at the bottom of xhci_queue_isoc_tx() can never be executed.
For 'i != 0' on an ISOC ring prepare_transfer can only fail if the endpoint
ring cannot be determined, but it was found only moments earlier (it could
be passed as a parameter).
The 'running_total' check is just checking that the code is correct
and can just be deleted now the code is working.
Remove the 'flip_cycle' parameter to td_to_noop() since it is no longer
ever set.
Signed-off-by: David Laight
---
drivers/usb/host/xhci-ring.c | 56
1 file changed, 5 insertions(+), 51 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 62049e5..ae5d32c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -617,12 +617,8 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
(unsigned long long) addr);
}
-/* flip_cycle means flip the cycle bit of all but the first and last TRB.
- * (The last TRB actually points to the ring enqueue pointer, which is not part
- * of this TD.) This is used to remove partially enqueued isoc TDs from a
ring.
- */
static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
- struct xhci_td *cur_td, bool flip_cycle)
+ struct xhci_td *cur_td)
{
struct xhci_segment *cur_seg;
union xhci_trb *cur_trb;
@@ -635,12 +631,6 @@ static void td_to_noop(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
* leave the pointers intact.
*/
cur_trb->generic.field[3] &= cpu_to_le32(~TRB_CHAIN);
- /* Flip the cycle bit (link TRBs can't be the first
-* or last TRB).
-*/
- if (flip_cycle)
- cur_trb->generic.field[3] ^=
- cpu_to_le32(TRB_CYCLE);
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Cancel (unchain) link TRB");
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
@@ -656,11 +646,6 @@ static void td_to_noop(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
cur_trb->generic.field[2] = 0;
/* Preserve only the cycle bit of this TRB */
cur_trb->generic.field[3] &= cpu_to_le32(TRB_CYCLE);
- /* Flip the cycle bit except on the first or last TRB */
- if (flip_cycle && cur_trb != cur_td->first_trb &&
- cur_trb != cur_td->last_trb)
- cur_trb->generic.field[3] ^=
- cpu_to_le32(TRB_CYCLE);
cur_trb->generic.field[3] |= cpu_to_le32(
TRB_TYPE(TRB_TR_NOOP));
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
@@ -834,7 +819,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
int slot_id,
cur_td->urb->stream_id,
cur_td, &deq_state);
else
- td_to_noop(xhci, ep_ring, cur_td, false);
+ td_to_noop(xhci, ep_ring, cur_td);
remove_finished_td:
/*
* The event handler won't see a completion for this TD anymore,
@@ -3834,11 +3819,9 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
ret = prepare_transfer(xhci, xhci->devs[slot_id], ep_index,
urb->stream_id, trbs_per_td, urb, i, mem_flags);
- if (ret < 0) {
- if (i == 0)
- return ret;
- goto cleanup;
- }
+ if (ret < 0)
+ /* This can only happen when for the first TD */
+ return ret;
td = urb_priv->td[i];
for (j = 0; j < trbs_per_td; j++) {
@@ -3922,13 +3905,6 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
addr += trb_buff_len;
td_remain_len -= trb_buff_len;
}
-
- /* Check TD length */
- if (running_total != td_len) {
- xhci_err(xhci, "ISOC TD length unmatch\n");
- ret = -EINVAL;
- goto cleanup;
- }
}
if (xhci_to_hcd(xhci)->self.bandwidth_isoc_reqs == 0) {
@@ -3939,28 +3915,6 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
giveback_first_trb(xhci, slot_id, ep_index, ep_ring);
return 0;
-cleanup:
- /* Clean up a partially enqueued isoc transfer. */
-
- for (i--; i >= 0; i--)
-
[PATCH 3/5] xhci: Pass the 'ep_ring' to prepare_transfer()
prepare_transfer() looks up the endpoint ring structure, however
the caller already knows it.
Save code and an error test that can never fail by passing the ring
address instead of the stream_id.
Signed-off-by: David Laight
---
drivers/usb/host/xhci-ring.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ae5d32c..c1ba8ca 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3072,7 +3072,7 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
static int prepare_transfer(struct xhci_hcd *xhci,
struct xhci_virt_device *xdev,
unsigned int ep_index,
- unsigned int stream_id,
+ struct xhci_ring *ep_ring,
unsigned int num_trbs,
struct urb *urb,
unsigned int td_index,
@@ -3081,16 +3081,8 @@ static int prepare_transfer(struct xhci_hcd *xhci,
int ret;
struct urb_priv *urb_priv;
struct xhci_td *td;
- struct xhci_ring *ep_ring;
struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx,
ep_index);
- ep_ring = xhci_stream_id_to_ring(xdev, ep_index, stream_id);
- if (!ep_ring) {
- xhci_dbg(xhci, "Can't prepare ring for bad stream ID %u\n",
- stream_id);
- return -EINVAL;
- }
-
/* xhci_queue_isoc_tx_prepare() called prepare ring earlier. */
if (ep_ring->type != TYPE_ISOC) {
ret = prepare_ring(xhci, ep_ring,
@@ -3309,7 +3301,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
usb_endpoint_maxp(&urb->ep->desc));
trb_buff_len = prepare_transfer(xhci, xhci->devs[slot_id],
- ep_index, urb->stream_id,
+ ep_index, ep_ring,
num_trbs, urb, 0, mem_flags);
if (trb_buff_len < 0)
return trb_buff_len;
@@ -3479,7 +3471,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
/* FIXME: this doesn't deal with URB_ZERO_PACKET - need one more */
ret = prepare_transfer(xhci, xhci->devs[slot_id],
- ep_index, urb->stream_id,
+ ep_index, ep_ring,
num_trbs, urb, 0, mem_flags);
if (ret < 0)
return ret;
@@ -3608,7 +3600,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
if (urb->transfer_buffer_length > 0)
num_trbs++;
ret = prepare_transfer(xhci, xhci->devs[slot_id],
- ep_index, urb->stream_id,
+ ep_index, ep_ring,
num_trbs, urb, 0, mem_flags);
if (ret < 0)
return ret;
@@ -3818,7 +3810,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
trbs_per_td = count_isoc_trbs_needed(xhci, urb, i);
ret = prepare_transfer(xhci, xhci->devs[slot_id], ep_index,
- urb->stream_id, trbs_per_td, urb, i, mem_flags);
+ ep_ring, trbs_per_td, urb, i, mem_flags);
if (ret < 0)
/* This can only happen when for the first TD */
return ret;
--
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 4/5] xhci: Don't pass TRB_CYCLE value to queue_trb()
Now that queue_trb() generates the correct TRB_CYCLE value for all
commands, there is no need for the callers to worry about it.
Remove all the code from xhci_queue_bulk_tx() that remembered
the location of the first TRB and its cycle bit.
Also remove related variables that used to be passed to giveback_first_trb.
There are no references left to ring->cycle_state or TRB_CYCLE
(or 0x1 meaing TRB_CYCLE) in the code that generates TRB.
Signed-off-by: David Laight
---
drivers/usb/host/xhci-ring.c | 78 +++-
1 file changed, 5 insertions(+), 73 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c1ba8ca..0d0bd7f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2906,12 +2906,8 @@ static void queue_trb(struct xhci_hcd *xhci, struct
xhci_ring *ring,
trb = &ring->enqueue->generic;
- /*
-* Ignore the caller's CYCLE bit.
-* The caller doesn't know whether the real first TRB is
-* actually a LINK (or NOP) TRB.
-*/
- field4 = (field4 & ~TRB_CYCLE) | ring->cycle_state;
+ /* Give the controller ownership of all but the first TRB */
+ field4 |= ring->cycle_state;
if (trb == &ring->enqueue_first->generic)
field4 ^= TRB_CYCLE;
@@ -3284,13 +3280,9 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
int num_sgs;
int trb_buff_len, this_sg_len, running_total;
unsigned int total_packet_count;
- bool first_trb;
u64 addr;
bool more_trbs_coming;
- struct xhci_generic_trb *start_trb;
- int start_cycle;
-
ep_ring = xhci_urb_to_transfer_ring(xhci, urb);
if (!ep_ring)
return -EINVAL;
@@ -3309,14 +3301,6 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
urb_priv = urb->hcpriv;
td = urb_priv->td[0];
- /*
-* Don't give the first TRB to the hardware (by toggling the cycle bit)
-* until we've finished creating all the other TRBs. The ring's cycle
-* state may change as we enqueue the other TRBs, so save it too.
-*/
- start_trb = &ep_ring->enqueue->generic;
- start_cycle = ep_ring->cycle_state;
-
running_total = 0;
/*
* How much data is in the first TRB?
@@ -3335,21 +3319,12 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
if (trb_buff_len > urb->transfer_buffer_length)
trb_buff_len = urb->transfer_buffer_length;
- first_trb = true;
/* Queue the first TRB, even if it's zero-length */
do {
u32 field = 0;
u32 length_field = 0;
u32 remainder = 0;
- /* Don't change the cycle bit of the first TRB until later */
- if (first_trb) {
- first_trb = false;
- if (start_cycle == 0)
- field |= 0x1;
- } else
- field |= ep_ring->cycle_state;
-
/* Chain all the TRBs together; clear the chain bit in the last
* TRB to indicate it's the last TRB in the chain.
*/
@@ -3435,10 +3410,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
struct urb_priv *urb_priv;
struct xhci_td *td;
int num_trbs;
- struct xhci_generic_trb *start_trb;
- bool first_trb;
bool more_trbs_coming;
- int start_cycle;
u32 field, length_field;
int running_total, trb_buff_len, ret;
@@ -3479,14 +3451,6 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
urb_priv = urb->hcpriv;
td = urb_priv->td[0];
- /*
-* Don't give the first TRB to the hardware (by toggling the cycle bit)
-* until we've finished creating all the other TRBs. The ring's cycle
-* state may change as we enqueue the other TRBs, so save it too.
-*/
- start_trb = &ep_ring->enqueue->generic;
- start_cycle = ep_ring->cycle_state;
-
running_total = 0;
total_packet_count = DIV_ROUND_UP(urb->transfer_buffer_length,
usb_endpoint_maxp(&urb->ep->desc));
@@ -3497,21 +3461,11 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
if (trb_buff_len > urb->transfer_buffer_length)
trb_buff_len = urb->transfer_buffer_length;
- first_trb = true;
-
/* Queue the first TRB, even if it's zero-length */
do {
u32 remainder = 0;
field = 0;
- /* Don't change the cycle bit of the first TRB until later */
- if (first_trb) {
- first_trb = false;
- if (start_cycle == 0)
- field |= 0x1;
- } else
-
[PATCH 1/5] xhci: Don't change the ownership of LINK TRB until all the TRB are written
Some xhci controllers (eg ASMedia) don't like processing a LINK TRB and
then finding that they can't process the next TRB.
Instead of flipping the cycle bit on the first data TRB, remember the
real first TRB in prepare_ring() and flip that in giveback_first_trb().
In queue_trb() ignore the cycle bit value set by xhci_queue_bulk_tx() etc,
and changes the ownership of all but the first TRB.
If prepare_ring() adds any NOP TRB, the ownership of all but the first
and the LINK are changed. The wmb() is no longer needed before changing
the ownership of a LINK trb since is is preceeded by a NOP.
Since the 'first trb' might be a LINK trb queue_command() must also now
call giveback_first_trb(). Don't ring the doorbell here though.
The isoc code calls prepare_ring() right at the start, this ensures that
there is enough space for all the TRB and advances the write-ptr past
any LINK TRB. It then calls prepare_transfer() multiple times.
However the prepare_ring() calls must now be matched with those to
giveback_first_trb().
Don't call prepare_ring() from prepare_transfer() for isoc rings.
Set 'more_trbs_coming' (it means 'advance past a LINK TRB) on all
the queue_trb() calls except the very last one.
Signed-off-by: David Laight
---
Note that this differs to any previous similar patches I've sent
because it contains a fix to the isoc code.
However I've only tsted it with USB3 ethernet.
drivers/usb/host/xhci-ring.c | 84 +---
drivers/usb/host/xhci.h | 1 +
2 files changed, 57 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a0b248c..62049e5 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2920,6 +2920,16 @@ static void queue_trb(struct xhci_hcd *xhci, struct
xhci_ring *ring,
struct xhci_generic_trb *trb;
trb = &ring->enqueue->generic;
+
+ /*
+* Ignore the caller's CYCLE bit.
+* The caller doesn't know whether the real first TRB is
+* actually a LINK (or NOP) TRB.
+*/
+ field4 = (field4 & ~TRB_CYCLE) | ring->cycle_state;
+ if (trb == &ring->enqueue_first->generic)
+ field4 ^= TRB_CYCLE;
+
trb->field[0] = cpu_to_le32(field1);
trb->field[1] = cpu_to_le32(field2);
trb->field[2] = cpu_to_le32(field3);
@@ -2964,6 +2974,8 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
return -EINVAL;
}
+ /* Save entry whose cycle bit needs flipping at the end */
+ ep_ring->enqueue_first = ep_ring->enqueue;
while (1) {
if (room_on_ring(xhci, ep_ring, num_trbs)) {
union xhci_trb *trb = ep_ring->enqueue;
@@ -3006,13 +3018,18 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
ep_ring->cycle_state);
ep_ring->num_trbs_free -= usable;
- do {
+ /* Leave TRB_CYCLE unchanged on first NOP */
+ trb->generic.field[3] = nop_cmd ^
+ cpu_to_le32(TRB_CYCLE);
+ for (;;) {
trb->generic.field[0] = 0;
trb->generic.field[1] = 0;
trb->generic.field[2] = 0;
- trb->generic.field[3] = nop_cmd;
trb++;
- } while (--usable);
+ if (!--usable)
+ break;
+ trb->generic.field[3] = nop_cmd;
+ }
ep_ring->enqueue = trb;
if (room_on_ring(xhci, ep_ring, num_trbs))
break;
@@ -3050,8 +3067,9 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
else
next->link.control |= cpu_to_le32(TRB_CHAIN);
- wmb();
- next->link.control ^= cpu_to_le32(TRB_CYCLE);
+ /* If LINK follows a NOP, invert cycle */
+ if (next != ep_ring->enqueue_first)
+ next->link.control ^= cpu_to_le32(TRB_CYCLE);
/* Toggle the cycle bit after the last ring segment. */
if (last_trb_on_last_seg(xhci, ring, ring->enq_seg,
next)) {
@@ -3088,11 +3106,14 @@ static int prepare_transfer(struct xhci_hcd *xhci,
return -EINVAL;
}
- ret = prepare_ring(xhci, ep_ring,
- le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK,
- num_trbs, mem_flags);
- if (ret)
- return ret;
+
Re: [PATCH v6 00/10] USB Host support for OMAP5 uEVM (for 3.14)
> This patchset brings up USB Host ports and Ethernet port on > the OMAP5 uEVM board. I'd keep hold of this and send it out again when the merge-window is closed. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe 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: host: xhci: fix HALTED endpoint handling
On Mon, Jan 20, 2014 at 02:45:11PM -0600, Felipe Balbi wrote: > If the HW reports the endpoint as Halted, there's > no need to start any URB for that endpoint, we can > simply return -EPIPE and this will tell upper layers > that the endpoint is, indeed, halted. > > This patch fixes usbtest's set/clear halt test #13 > on xHCI-based hosts. It looks like that test assumes that the xHCI driver should not allow URBs to be enqueued to the endpoint when the driver needs to call usb_clear_halt(). Or is it expecting that the host will attempt the transfer, and the device will stall the transfer? > Cc: > Signed-off-by: Felipe Balbi > --- > > Hi Sarah, > > seems like this has been broken for quite a long > time. > > I tested the patch on an AM437x which has XHCI Host and > the same IP configured as device as well (two dwc3 instances, > basically). > > It has been running for the past hour or so without any failures > when running testusb -t 13 -c 5000 -s 2048 -a in a loop, but > if you think there's a better way to fix this, let me know. I just > didn't understand why xhci-hcd still queues the URB even though > HW already told us the endpoint is halted. Any comments ? The reason the driver still queues URBs is because the xHC halts the endpoint for events that are outside of stalls. For instance, it will halt the endpoint on a babble error, a transfer error, or a split transaction error. The upper layers don't call usb_clear_halt() for those cases, so the xHCI driver handles the halted endpoint internally. Look for calls to xhci_requires_manual_halt_cleanup(). The xHC also halts the endpoint on a control transfer stall. The upper layers don't ever call usb_clear_halt on the control endpoint, because the bus spec says the next control transfer will clear the halted condition. Therefore the xHCI driver has to handle the halt manually. The way the driver does that is to set a flag for the endpoint (EP_HALTED), issue a Reset Endpoint command, and then a Set TR Dequeue command to move the dequeue pointer past the transfer that failed. The driver still accepts URBs submitted to the endpoint (because from the upper layer's perspective, we should in the cases where we manually handle the halt), but it does not ring the doorbell for that endpoint. Once the command to set the dequeue pointer completes, the EP_HALTED flag is cleared, the doorbell is rung for the endpoint, and the queued URBs are processed. The end result is that the xHCI driver *needs* to allow URBs to be queued when the endpoint is halted and it needs to manual cleanup the ring. Otherwise, the driver might queue an URB while the two commands are still being processed, the driver will reject the URBs, and the behavior in these corner cases will be different than other host controllers. So, no, I don't think this is quite the right solution. You could try to differentiate between an endpoint halt that requires a manual cleanup, and ones that the upper layer should handle, perhaps by adding a new EP_HALTED_MANUAL flag. The driver could accept URBs for the manual cleanup case, and reject URBs with -EPIPE for the ones the upper layers should handle. Code that refuses to ring the endpoint doorbell would have to look for both EP_HALTED and EP_HALTED_MANUAL. There might be other implications as well; grep for EP_HALTED and see what code checks it. Sarah Sharp > drivers/usb/host/xhci-ring.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index 53c2e29..e9df61a 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -2959,7 +2959,8 @@ static int prepare_ring(struct xhci_hcd *xhci, struct > xhci_ring *ep_ring, > /* XXX not sure if this should be -ENOENT or not */ > return -EINVAL; > case EP_STATE_HALTED: > - xhci_dbg(xhci, "WARN halted endpoint, queueing URB anyway.\n"); > + xhci_dbg(xhci, "WARN halted endpoint.\n"); > + return -EPIPE; > case EP_STATE_STOPPED: > case EP_STATE_RUNNING: > break; > -- > 1.8.4.GIT > -- To unsubscribe from this list: send the line "unsubscribe 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 v7 1/2] ohci-platform: Add support for devicetree instantiation
Hi,
On 01/21/2014 06:59 PM, Sergei Shtylyov wrote:
Hello.
On 01/15/2014 10:24 PM, 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
Acked-by: Alan Stern
[...]
Have only found time to fully read the patches just now...
diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt
b/Documentation/devicetree/bindings/usb/usb-ohci.txt
new file mode 100644
index 000..f9d6c73
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
@@ -0,0 +1,22 @@
+USB OHCI controllers
+
+Required properties:
+- compatible : "usb-ohci"
+- reg : ohci controller register range (address and length)
+- interrupts : ohci controller interrupt
+
+Optional properties:
+- clocks : a list of phandle + clock specifier pairs
+- phys : phandle + phy specifier pair
+- phy-names : "usb"
+
+Example:
+
+ohci0: ohci@0x01c14400 {
Two minor nits: there should be no "0x" in the address part of the node name.
And according to ePAPR [1], "the name of a node should be somewhat generic, reflecting
the function of the device and not its precise programming model. If appropriate, the name
should be one of the following choices:
[...]
- usb".
Same comments for "usb-ehci" binding.
You're right on both accounts, I'll do a v8 tomorrow, including a re-spin of the
big-endian patches.
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] usb: host: xhci: fix HALTED endpoint handling
On Tue, Jan 21, 2014 at 09:39:12AM -0800, Sarah Sharp wrote: > On Mon, Jan 20, 2014 at 02:45:11PM -0600, Felipe Balbi wrote: > > If the HW reports the endpoint as Halted, there's > > no need to start any URB for that endpoint, we can > > simply return -EPIPE and this will tell upper layers > > that the endpoint is, indeed, halted. > > > > This patch fixes usbtest's set/clear halt test #13 > > on xHCI-based hosts. > > It looks like that test assumes that the xHCI driver should not allow > URBs to be enqueued to the endpoint when the driver needs to call > usb_clear_halt(). Or is it expecting that the host will attempt the > transfer, and the device will stall the transfer? I suppose it doesn't make a difference, considering it wait_for_completion_interruptible(). As long as the transfer actually completes within 10 seconds, it doesn't matter. What I have seen, though, is the transfer never completes and the 10 second timeout always kicks in when trying two consecutive usb_submit_urb(). Here's, in a nut-shell, what happens: -> SetFeature(ENDPOINT_HALT) -> GetStatus() -> usb_submit_urb() -> this one completes with -EPIPE as it should -> usb_submit_urb() -> this one always times out :-( > > Cc: > > Signed-off-by: Felipe Balbi > > --- > > > > Hi Sarah, > > > > seems like this has been broken for quite a long > > time. > > > > I tested the patch on an AM437x which has XHCI Host and > > the same IP configured as device as well (two dwc3 instances, > > basically). > > > > It has been running for the past hour or so without any failures > > when running testusb -t 13 -c 5000 -s 2048 -a in a loop, but > > if you think there's a better way to fix this, let me know. I just > > didn't understand why xhci-hcd still queues the URB even though > > HW already told us the endpoint is halted. Any comments ? > > The reason the driver still queues URBs is because the xHC halts the > endpoint for events that are outside of stalls. For instance, it will > halt the endpoint on a babble error, a transfer error, or a split > transaction error. The upper layers don't call usb_clear_halt() for > those cases, so the xHCI driver handles the halted endpoint internally. > Look for calls to xhci_requires_manual_halt_cleanup(). alright. > The xHC also halts the endpoint on a control transfer stall. The upper > layers don't ever call usb_clear_halt on the control endpoint, because > the bus spec says the next control transfer will clear the halted > condition. Therefore the xHCI driver has to handle the halt manually. > > The way the driver does that is to set a flag for the endpoint > (EP_HALTED), issue a Reset Endpoint command, and then a Set TR Dequeue > command to move the dequeue pointer past the transfer that failed. The > driver still accepts URBs submitted to the endpoint (because from the > upper layer's perspective, we should in the cases where we manually > handle the halt), but it does not ring the doorbell for that endpoint. > Once the command to set the dequeue pointer completes, the EP_HALTED > flag is cleared, the doorbell is rung for the endpoint, and the queued > URBs are processed. > > The end result is that the xHCI driver *needs* to allow URBs to be > queued when the endpoint is halted and it needs to manual cleanup the > ring. Otherwise, the driver might queue an URB while the two commands > are still being processed, the driver will reject the URBs, and the > behavior in these corner cases will be different than other host > controllers. > > So, no, I don't think this is quite the right solution. > > You could try to differentiate between an endpoint halt that requires a > manual cleanup, and ones that the upper layer should handle, perhaps by > adding a new EP_HALTED_MANUAL flag. The driver could accept URBs for > the manual cleanup case, and reject URBs with -EPIPE for the ones the > upper layers should handle. > > Code that refuses to ring the endpoint doorbell would have to look for > both EP_HALTED and EP_HALTED_MANUAL. There might be other implications > as well; grep for EP_HALTED and see what code checks it. will do. So what's supposed to actually happen with that test ? I'd expect the URB to actually be started (meaning we should ring the ep doorbell ?) and the device would reply with a Stall which, in turn, would return -EPIPE to upper layers. Currently, that URB seems like it's never started to begin with, so we never see a Stall from the device side and the test fails. Surprisingly, this only happens on the second usb_submit_urb(). From what I could gather yesterday, EP_STATE_HALTED isn't yet set when the first usb_submit_urb() is called. Likely that because of that, the doorbell is rung and the device has a chance to Stall the transfer. Or am I missing something ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: host: xhci: fix HALTED endpoint handling
On Tue, 21 Jan 2014, Sarah Sharp wrote: > On Mon, Jan 20, 2014 at 02:45:11PM -0600, Felipe Balbi wrote: > > If the HW reports the endpoint as Halted, there's > > no need to start any URB for that endpoint, we can > > simply return -EPIPE and this will tell upper layers > > that the endpoint is, indeed, halted. > > > > This patch fixes usbtest's set/clear halt test #13 > > on xHCI-based hosts. > > It looks like that test assumes that the xHCI driver should not allow > URBs to be enqueued to the endpoint when the driver needs to call > usb_clear_halt(). Or is it expecting that the host will attempt the > transfer, and the device will stall the transfer? It expects that the computer will attempt the transfer and the device will reply with a STALL. > The reason the driver still queues URBs is because the xHC halts the > endpoint for events that are outside of stalls. For instance, it will > halt the endpoint on a babble error, a transfer error, or a split > transaction error. The upper layers don't call usb_clear_halt() for > those cases, so the xHCI driver handles the halted endpoint internally. > Look for calls to xhci_requires_manual_halt_cleanup(). > > The xHC also halts the endpoint on a control transfer stall. The upper > layers don't ever call usb_clear_halt on the control endpoint, because > the bus spec says the next control transfer will clear the halted > condition. Therefore the xHCI driver has to handle the halt manually. It sounds like there's a little confusion here. To say that the endpoint is halted is somewhat ambiguous; it could refer to a halt on the host side or a halt on the device side. usb_clear_halt() is meant to clear a halt on the device side. It also tells the HCD that it has done so, in case the HCD is keeping track, but it is not meant for clearing a halt on the host side. usb_reset_endpoint() does that (among other things). > You could try to differentiate between an endpoint halt that requires a > manual cleanup, and ones that the upper layer should handle, perhaps by > adding a new EP_HALTED_MANUAL flag. The driver could accept URBs for > the manual cleanup case, and reject URBs with -EPIPE for the ones the > upper layers should handle. That should not be necessary. The HCD should always accept URB submissions. Nothing will get sent to the device until the HCD clears any host-side halts, but the upper layers don't worry about that because the HCD takes care of it automatically. Contrariwise, the upper layers are responsible for clearing device-side halts. The HCD should ignore that end of things as much as it can. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ohci-platform: Add support for controllers with big-endian regs / descriptors
On Tue, 21 Jan 2014, Hans de Goede wrote: > Hi, > > On 01/21/2014 05:40 PM, Alan Stern wrote: > > On Tue, 21 Jan 2014, Hans de Goede wrote: > > > >> Note this commit uses the same devicetree booleans for this as the ones > >> already existing in the usb-ehci bindings. > >> > >> Signed-off-by: Hans de Goede > > > >> --- a/drivers/usb/host/Kconfig > >> +++ b/drivers/usb/host/Kconfig > >> @@ -512,6 +512,10 @@ config USB_CNS3XXX_OHCI > >> > >> config USB_OHCI_HCD_PLATFORM > >>tristate "Generic OHCI driver for a platform device" > >> + # Always support LE, support BE on architectures which have readl_be > >> + select USB_OHCI_LITTLE_ENDIAN > >> + select USB_OHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || > >> SPARC || PPC32 || PPC64) > >> + select USB_OHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || > >> SPARC || PPC32 || PPC64) > >>default n > > In any case, the style we have adopted is that these select lines go in > > the arch-specific defconfig, not here. > > Ok, so I should drop the Kconfig parts of both patches ? That's rigt. They are likely to cause trouble, and if the selects were needed then they should already be present somewhere else. 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: host: xhci: fix HALTED endpoint handling
Hi, On Tue, Jan 21, 2014 at 01:06:19PM -0500, Alan Stern wrote: > > You could try to differentiate between an endpoint halt that requires a > > manual cleanup, and ones that the upper layer should handle, perhaps by > > adding a new EP_HALTED_MANUAL flag. The driver could accept URBs for > > the manual cleanup case, and reject URBs with -EPIPE for the ones the > > upper layers should handle. > > That should not be necessary. The HCD should always accept URB > submissions. Nothing will get sent to the device until the HCD clears > any host-side halts, but the upper layers don't worry about that > because the HCD takes care of it automatically. > > Contrariwise, the upper layers are responsible for clearing device-side > halts. The HCD should ignore that end of things as much as it can. right, but there's still the bug that usb_submit_urb() times out with xhci and it doesn't with ehci. So there _is_ a bug in xhci. I'll try to debug more and figure out what's really going on, but one thing is for sure. usbtest issued a SetFeature(ENDPOINT_HALT) as part of the test and it tries to usb_submit_urb() as means to verify that the device is really responding with Stalls. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] xhci: Don't trace all short/incomplete receives
On Tue, Jan 21, 2014 at 12:02:56PM +, David Laight wrote:
> Don't trace short receives if URB_SHORT_NOT_OK is set.
> Short receives are normal for USB ethernet devices.
>
> Don't trace unexpected incomplete receives if XHCI_TRUST_TX_LENGTH is set.
> Ratelimit the trace.
Your patch does more than what you wrote here.
> Signed-off-by: David Laight
> ---
> If these two traces ever happen, then they will happen for every receive
> packet when using USB ethernet.
> If you need to enable the xhci_warn or xhci_dgb traces you don't want to
> be spammed with trace (syslogd will soon will the disk).
>
> These patches won't apply to 3.12 because the trace texts have changed,
> however 3.12 also needs a kernel recompile to enable the traces and
> anyone doing that can probably manage to patch them out.
This is a cleanup, so it won't go into 3.12. Only bug fixes get
backported to the stable kernels. The messages are annoying, but they
don't trigger a bug. People can work around them by turning off
CONFIG_USB_DEBUG.
> Changes for v2:
> Fixed so that it applies to Linus's current tree.
>
> drivers/usb/host/xhci-ring.c | 38 ++
> 1 file changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index a0b248c..0b3dd16 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2301,36 +2301,34 @@ static int process_bulk_intr_td(struct xhci_hcd
> *xhci, struct xhci_td *td,
> switch (trb_comp_code) {
> case COMP_SUCCESS:
> /* Double check that the HW transferred everything. */
> - if (event_trb != td->last_trb ||
> - EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) {
> - xhci_warn(xhci, "WARN Successful completion "
> - "on short TX\n");
> - if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> - *status = -EREMOTEIO;
> - else
> - *status = 0;
> - if ((xhci->quirks & XHCI_TRUST_TX_LENGTH))
> - trb_comp_code = COMP_SHORT_TX;
> - } else {
> + if (event_trb == td->last_trb &&
> + EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) == 0) {
> *status = 0;
> + break;
Your patch changes the behavior of the code here, for when the status
variable is set to either zero or -EREMOTEIO. The code is hard to
reason about, so this really needs to be a separate patch from the one
that removes the traces. Please send a patchset with two patches: one
to remove the trace, and another to clean up setting *status, in
whichever order makes the code clearest in the *status patch.
> }
> - break;
> + if (xhci->quirks & XHCI_TRUST_TX_LENGTH)
> + trb_comp_code = COMP_SHORT_TX;
I don't think changing the trb_comp_code is a good idea. There's a lot
of code that relies on it later, and it would take me a bit to figure
out if changing it is safe.
Sarah Sharp
> + else
> + xhci_warn_ratelimited(xhci,
> + "WARN Successful completion on short TX\n");
> + /* FALLTHROUGH */
> case COMP_SHORT_TX:
> - if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> + if (td->urb->transfer_flags & URB_SHORT_NOT_OK) {
> + xhci_dbg(xhci,
> + "ep %#x - asked for %d bytes, %d bytes
> untransferred\n",
> + td->urb->ep->desc.bEndpointAddress,
> + td->urb->transfer_buffer_length,
> + EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)));
> *status = -EREMOTEIO;
> - else
> + } else {
> *status = 0;
> + }
> break;
> default:
> /* Others already handled above */
> break;
> }
> - if (trb_comp_code == COMP_SHORT_TX)
> - xhci_dbg(xhci, "ep %#x - asked for %d bytes, "
> - "%d bytes untransferred\n",
> - td->urb->ep->desc.bEndpointAddress,
> - td->urb->transfer_buffer_length,
> -
> EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)));
> +
> /* Fast path - was this the last TRB in the TD for this URB? */
> if (event_trb == td->last_trb) {
> if (EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) {
> --
> 1.8.1.2
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci ASMedia lockups - a theory and a patch
On Tue, Jan 21, 2014 at 10:00:40AM +, David Laight wrote: > From: Sarah Sharp [ > > On Thu, Jan 16, 2014 at 10:21:11AM +, David Laight wrote: > > > From: David Laight > > > > From: David Laight > ... > > > > Below is a possible patch, I've only compile tested it. > > > > I've minimalised the patch by not removing all the code that saves > > > > 'start_trb' and modifies the TRB_CYCLE bit. > > > > If the patch works those parts can also be tidied up. > > > > > > I've had some feedback (in a private email) that the patch helps. > > > This was using the ASMedia controller with the asx88179_178a > > > ethernet device. > > > > David, please post the contents of that private email to the list and Cc > > me. We don't debug in private in the kernel. > > I'm not sure I should copy private emails to the list without > permission of the person who wrote them. Then please get their permission to repost it. As is, I only have Walt's report that your second patch doesn't help his ASMedia host. Unless I get a report that your second patch actually fixes a bug, I can't mark it for stable in good faith. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB EHCI broken on Celeron N2920 (Bay Trail)
Hi Chris, We think the Baytrail BIOS has an issue where it doesn't set the EHCI PCI interrupt enable bit when the computer is in "EHCI only mode". Please try this patch: http://marc.info/?l=linux-usb&m=138998295909608&w=2 Sarah Sharp On Tue, Jan 21, 2014 at 10:04:41PM +0800, Chris Cheng wrote: > I'm sorry forgot kernel version: 3.13.0 . > -- > Chris Cheng 鄭宇廷 > Atrust Computer Corp. 冠信電腦股份有限公司 > http://www.atrustcorp.com > Tel: +886-3-3288837 ext.1074 > 3F., No.361, Fusing 1st Rd., Gueishan Township, Taoyuan County 333, Taiwan > 333 桃園縣龜山鄉復興一路361號3F > > > 2014/1/21 Chris Cheng : > > Here is kernel message: > > > > [2.939251] usb 1-1: new high-speed USB device number 2 using ehci-pci > > [ 18.490987] usb 1-1: device not accepting address 2, error -110 > > [ 18.595095] usb 1-1: new high-speed USB device number 3 using ehci-pci > > [ 34.182233] usb 1-1: device not accepting address 3, error -110 > > [ 34.286352] usb 1-1: new high-speed USB device number 4 using ehci-pci > > [ 44.744589] usb 1-1: device not accepting address 4, error -110 > > [ 44.849590] usb 1-1: new high-speed USB device number 5 using ehci-pci > > [ 55.307858] usb 1-1: device not accepting address 5, error -110 > > [ 55.310239] hub 1-0:1.0: unable to enumerate USB device on port 1 > > > > Thanks. > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: bcm63xx_udc: fix build failure on DMA channel code
2014/1/14 Florian Fainelli :
> Commit 3dc6475 ("bcm63xx_enet: add support Broadcom BCM6345 Ethernet")
> changed the ENETDMA[CS] macros such that they are no longer macros, but
> actual register offset definitions. The bcm63xx_udc driver was not
> updated, and as a result, causes the following build error to pop up:
Felipe, Greg, any feedback on this patch? Can we get this merged since
this fixes a build failure?
Thanks!
>
> CC drivers/usb/gadget/u_ether.o
> drivers/usb/gadget/bcm63xx_udc.c: In function 'iudma_write':
> drivers/usb/gadget/bcm63xx_udc.c:642:24: error: called object '0' is not
> a function
> drivers/usb/gadget/bcm63xx_udc.c: In function 'iudma_reset_channel':
> drivers/usb/gadget/bcm63xx_udc.c:698:46: error: called object '0' is not
> a function
> drivers/usb/gadget/bcm63xx_udc.c:700:49: error: called object '0' is not
> a function
>
> Fix this by updating usb_dmac_{read,write}l and usb_dmas_{read,write}l to
> take an extra channel argument, and use the channel width
> (ENETDMA_CHAN_WIDTH) to offset the register we want to access, hence
> doing again what the macro implicitely did for us.
>
> CC: Kevin Cernekee
> CC: Jonas Gorski
> CC: sta...@vger.kernel.org
> Signed-off-by: Florian Fainelli
> ---
> Felipe,
>
> This is against your branch as balbi/usb.git, and this fix should be applied
> to
> stable 3.11 onwards.
>
> Thanks!
>
> drivers/usb/gadget/bcm63xx_udc.c | 58
> ++--
> 1 file changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/usb/gadget/bcm63xx_udc.c
> b/drivers/usb/gadget/bcm63xx_udc.c
> index 2ac7a8f..4c7b47f 100644
> --- a/drivers/usb/gadget/bcm63xx_udc.c
> +++ b/drivers/usb/gadget/bcm63xx_udc.c
> @@ -361,24 +361,30 @@ static inline void usb_dma_writel(struct bcm63xx_udc
> *udc, u32 val, u32 off)
> bcm_writel(val, udc->iudma_regs + off);
> }
>
> -static inline u32 usb_dmac_readl(struct bcm63xx_udc *udc, u32 off)
> +static inline u32 usb_dmac_readl(struct bcm63xx_udc *udc, u32 off, int chan)
> {
> - return bcm_readl(udc->iudma_regs + IUDMA_DMAC_OFFSET + off);
> + return bcm_readl(udc->iudma_regs + IUDMA_DMAC_OFFSET + off +
> + (ENETDMA_CHAN_WIDTH * chan));
> }
>
> -static inline void usb_dmac_writel(struct bcm63xx_udc *udc, u32 val, u32 off)
> +static inline void usb_dmac_writel(struct bcm63xx_udc *udc, u32 val, u32 off,
> + int chan)
> {
> - bcm_writel(val, udc->iudma_regs + IUDMA_DMAC_OFFSET + off);
> + bcm_writel(val, udc->iudma_regs + IUDMA_DMAC_OFFSET + off +
> + (ENETDMA_CHAN_WIDTH * chan));
> }
>
> -static inline u32 usb_dmas_readl(struct bcm63xx_udc *udc, u32 off)
> +static inline u32 usb_dmas_readl(struct bcm63xx_udc *udc, u32 off, int chan)
> {
> - return bcm_readl(udc->iudma_regs + IUDMA_DMAS_OFFSET + off);
> + return bcm_readl(udc->iudma_regs + IUDMA_DMAS_OFFSET + off +
> + (ENETDMA_CHAN_WIDTH * chan));
> }
>
> -static inline void usb_dmas_writel(struct bcm63xx_udc *udc, u32 val, u32 off)
> +static inline void usb_dmas_writel(struct bcm63xx_udc *udc, u32 val, u32 off,
> + int chan)
> {
> - bcm_writel(val, udc->iudma_regs + IUDMA_DMAS_OFFSET + off);
> + bcm_writel(val, udc->iudma_regs + IUDMA_DMAS_OFFSET + off +
> + (ENETDMA_CHAN_WIDTH * chan));
> }
>
> static inline void set_clocks(struct bcm63xx_udc *udc, bool is_enabled)
> @@ -639,7 +645,7 @@ static void iudma_write(struct bcm63xx_udc *udc, struct
> iudma_ch *iudma,
> } while (!last_bd);
>
> usb_dmac_writel(udc, ENETDMAC_CHANCFG_EN_MASK,
> - ENETDMAC_CHANCFG_REG(iudma->ch_idx));
> + ENETDMAC_CHANCFG_REG, iudma->ch_idx);
> }
>
> /**
> @@ -695,9 +701,9 @@ static void iudma_reset_channel(struct bcm63xx_udc *udc,
> struct iudma_ch *iudma)
> bcm63xx_fifo_reset_ep(udc, max(0, iudma->ep_num));
>
> /* stop DMA, then wait for the hardware to wrap up */
> - usb_dmac_writel(udc, 0, ENETDMAC_CHANCFG_REG(ch_idx));
> + usb_dmac_writel(udc, 0, ENETDMAC_CHANCFG_REG, ch_idx);
>
> - while (usb_dmac_readl(udc, ENETDMAC_CHANCFG_REG(ch_idx)) &
> + while (usb_dmac_readl(udc, ENETDMAC_CHANCFG_REG, ch_idx) &
>ENETDMAC_CHANCFG_EN_MASK) {
> udelay(1);
>
> @@ -714,10 +720,10 @@ static void iudma_reset_channel(struct bcm63xx_udc
> *udc, struct iudma_ch *iudma)
> dev_warn(udc->dev, "forcibly halting IUDMA channel
> %d\n",
> ch_idx);
> usb_dmac_writel(udc, ENETDMAC_CHANCFG_BUFHALT_MASK,
> - ENETDMAC_CHANCFG_REG(ch_idx));
> + ENETDMAC_CHANCFG_REG, ch_idx);
> }
> }
> - usb_dmac_writel(udc, ~0, ENETDMAC_IR_REG(
Re: [PATCH] usb: host: xhci: fix HALTED endpoint handling
On Tue, 21 Jan 2014, Felipe Balbi wrote: > Hi, > > On Tue, Jan 21, 2014 at 01:06:19PM -0500, Alan Stern wrote: > > > You could try to differentiate between an endpoint halt that requires a > > > manual cleanup, and ones that the upper layer should handle, perhaps by > > > adding a new EP_HALTED_MANUAL flag. The driver could accept URBs for > > > the manual cleanup case, and reject URBs with -EPIPE for the ones the > > > upper layers should handle. > > > > That should not be necessary. The HCD should always accept URB > > submissions. Nothing will get sent to the device until the HCD clears > > any host-side halts, but the upper layers don't worry about that > > because the HCD takes care of it automatically. > > > > Contrariwise, the upper layers are responsible for clearing device-side > > halts. The HCD should ignore that end of things as much as it can. > > right, but there's still the bug that usb_submit_urb() times out with > xhci and it doesn't with ehci. So there _is_ a bug in xhci. Oh, definitely. I was just trying to help make sure that the bug wasn't "fixed" incorrectly. 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 2/2] ehci-platform: Add support for controllers with big-endian regs / descriptors
2014/1/21 Hans de Goede :
> This uses the already documented devicetree booleans for this.
(I would greatly appreciate if you could CC people who gave you
feedback on this before)
A more informative commit message would be welcome, along with a
reference to which Device Tree binding documentation you are referring
to.
>
> Signed-off-by: Hans de Goede
> ---
> drivers/usb/host/Kconfig | 3 +++
> drivers/usb/host/ehci-platform.c | 33 +++--
> 2 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 237d7b1..4af41f3 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -256,6 +256,9 @@ config USB_EHCI_ATH79
> config USB_EHCI_HCD_PLATFORM
> tristate "Generic EHCI driver for a platform device"
> depends on !PPC_OF
> + # Support BE on architectures which have readl_be
> + select USB_EHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE ||
> SPARC || PPC32 || PPC64)
> + select USB_EHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE ||
> SPARC || PPC32 || PPC64)
I do not think this is that simple nor correct for at least Microblaze
and MIPS since they can run in either BE or LE mode, and those
specific platforms should already do the proper select at the
board/SoC level. This *might* be correct for SPARC, PPC32 and PPC64,
although I believe some specific PPC64 boards can run in little-endian
mode like the P-series, SPARC might too.
It seems to me that you should not touch this and keep the existing
selects in place, if it turns out that the selects are missing the
error messages you added below are catching those misuses.
> default n
> ---help---
> Adds an EHCI host driver for a generic platform device, which
> diff --git a/drivers/usb/host/ehci-platform.c
> b/drivers/usb/host/ehci-platform.c
> index d8aebc0..5888abb 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -55,8 +55,10 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>
> hcd->has_tt = pdata->has_tt;
> ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
> - ehci->big_endian_desc = pdata->big_endian_desc;
> - ehci->big_endian_mmio = pdata->big_endian_mmio;
> + if (pdata->big_endian_desc)
> + ehci->big_endian_desc = 1;
> + if (pdata->big_endian_mmio)
> + ehci->big_endian_mmio = 1;
>
> if (pdata->pre_setup) {
> retval = pdata->pre_setup(hcd);
> @@ -142,6 +144,7 @@ static int ehci_platform_probe(struct platform_device
> *dev)
> struct resource *res_mem;
> struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
> struct ehci_platform_priv *priv;
> + struct ehci_hcd *ehci;
> int err, irq, clk = 0;
>
> if (usb_disabled())
> @@ -177,8 +180,34 @@ static int ehci_platform_probe(struct platform_device
> *dev)
> platform_set_drvdata(dev, hcd);
> dev->dev.platform_data = pdata;
> priv = hcd_to_ehci_priv(hcd);
> + ehci = hcd_to_ehci(hcd);
>
> if (pdata == &ehci_platform_defaults && dev->dev.of_node) {
> + if (of_property_read_bool(dev->dev.of_node,
> "big-endian-regs"))
> + ehci->big_endian_mmio = 1;
> +
> + if (of_property_read_bool(dev->dev.of_node,
> "big-endian-desc"))
> + ehci->big_endian_desc = 1;
> +
> + if (of_property_read_bool(dev->dev.of_node, "big-endian"))
> + ehci->big_endian_mmio = ehci->big_endian_desc = 1;
Ok, so I am confused now, should you update
pdata->ehci_big_endian_{desc,mmio} here or is it valid to directly
modify ehci->big_endian_{desc,mmio}, is not there any risk to undo
what is done in ehci_platform_reset(), or is ehci_platform_reset()
only called for non-DT cases?
> +
> +#ifndef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
> + if (ehci->big_endian_mmio) {
> + dev_err(&dev->dev,
> + "Error big-endian-regs not compiled in\n");
I do not think using the Device Tree property name would be very
informative since this is supposed to guard against misconfigurations
for both DT and non-DT enabled platforms, how about something like the
following:
"support for big-endian MMIO registers not enabled".
> + err = -EINVAL;
> + goto err_put_hcd;
> + }
> +#endif
> +#ifndef CONFIG_USB_EHCI_BIG_ENDIAN_DESC
> + if (ehci->big_endian_desc) {
> + dev_err(&dev->dev,
> + "Error big-endian-desc not compiled in\n");
> + err = -EINVAL;
> + goto err_put_hcd;
And here "support for big-endian descriptors not enabled".
> + }
> +#endif
> priv->phy = devm_phy_get(&dev->dev, "usb");
[PATCH] staging/usbip: remove vhci_hcd vhci_hub_status change message
When vhci_hcd is enabled, the following message floods the dmesg buffer.
This message doesn't provide any useful information other than cluttering
the dmesg buffer. Fix it by removing the message. There is another debug
message in this routine that dumps detailed port status change information.
[ 4062.716662] vhci_hcd: changed 0
Signed-off-by: Shuah Khan
---
drivers/staging/usbip/vhci_hcd.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
index 72391ef..adb6201 100644
--- a/drivers/staging/usbip/vhci_hcd.c
+++ b/drivers/staging/usbip/vhci_hcd.c
@@ -205,8 +205,6 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf)
}
}
- pr_info("changed %d\n", changed);
-
if ((hcd->state == HC_STATE_SUSPENDED) && (changed == 1))
usb_hcd_resume_root_hub(hcd);
--
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 1/2] ohci-platform: Add support for controllers with big-endian regs / descriptors
2014/1/21 Alan Stern : > On Tue, 21 Jan 2014, Hans de Goede wrote: > >> Hi, >> >> On 01/21/2014 05:40 PM, Alan Stern wrote: >> > On Tue, 21 Jan 2014, Hans de Goede wrote: >> > >> >> Note this commit uses the same devicetree booleans for this as the ones >> >> already existing in the usb-ehci bindings. >> >> >> >> Signed-off-by: Hans de Goede >> > >> >> --- a/drivers/usb/host/Kconfig >> >> +++ b/drivers/usb/host/Kconfig >> >> @@ -512,6 +512,10 @@ config USB_CNS3XXX_OHCI >> >> >> >> config USB_OHCI_HCD_PLATFORM >> >>tristate "Generic OHCI driver for a platform device" >> >> + # Always support LE, support BE on architectures which have readl_be >> >> + select USB_OHCI_LITTLE_ENDIAN >> >> + select USB_OHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || >> >> SPARC || PPC32 || PPC64) >> >> + select USB_OHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || >> >> SPARC || PPC32 || PPC64) >> >>default n > >> > In any case, the style we have adopted is that these select lines go in >> > the arch-specific defconfig, not here. >> >> Ok, so I should drop the Kconfig parts of both patches ? > > That's rigt. They are likely to cause trouble, and if the selects were > needed then they should already be present somewhere else. Absolutely, they will actually break platforms. Since you added some guards for when these properties are set, but proper support in the kernel is not enabled, this is already catching misuses, and as such is already an improvement. -- Florian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb disk recognized but fails
I have applied the patches to a0fa1dd3cdbccec9597fe53b6177a9aa6e20f2f8:
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 53c2e29..64c36fe 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3008,7 +3008,7 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
if (num_trbs >= TRBS_PER_SEGMENT) {
xhci_err(xhci, "Too many fragments %d, max
%d\n",
num_trbs, TRBS_PER_SEGMENT - 1);
- return -ENOMEM;
+ return -EINVAL;
}
nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4265b48..d45a0d5 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4713,8 +4713,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t
get_quirks)
struct device *dev = hcd->self.controller;
int retval;
- /* Accept arbitrarily long scatter-gather lists */
- hcd->self.sg_tablesize = ~0;
+ /* Limit the block layer scatter-gather lists to half a segment. */
+ hcd->self.sg_tablesize = TRBS_PER_SEGMENT / 2;
/* support to build packet from discontinuous buffers */
hcd->self.no_sg_constraint = 1;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 03c74b7..c283cf1 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1260,7 +1260,7 @@ union xhci_trb {
* since the command ring is 64-byte aligned.
* It must also be greater than 16.
*/
-#define TRBS_PER_SEGMENT 64
+#define TRBS_PER_SEGMENT 256
/* Allow two commands + a link TRB, along with any reserved command TRBs */
#define MAX_RSVD_CMD_TRBS (TRBS_PER_SEGMENT - 3)
#define TRB_SEGMENT_SIZE (TRBS_PER_SEGMENT*16)
The output of lspci -vvv -n
00:14.0 0c03: 8086:1e31 (rev 04) (prog-if 30 [XHCI])
Subsystem: 1028:0534
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
SERR- Od: Sarah Sharp
> Komu: Milan Svoboda
> Datum: 20.01.2014 20:45
> Předmět: Re: usb disk recognized but fails
>
> CC: linux-usb@vger.kernel.org, "USB Storage List"
> , linux-s...@vger.kernel.org
>On Mon, Jan 20, 2014 at 08:00:23PM +0100, Milan Svoboda wrote:
>> Non-working:
>>
>> 3.12.7-2-ARCH #1 SMP PREEMPT Sun Jan 12 13:09:09 CET 2014 x86_64 GNU/Linux
>>
>> I'm sorry, I don't remember exactly which version worked, I have had the
>> disk connected to TV for
>> a long time and only recently I wanted to copy some movies on it. I guess it
>> worked something like
>> 6 or 9 months ago.
>
>If you're up for compiling a custom kernel, can you try 3.12.7 with
>these two patches applied?
>
>http://marc.info/?l=linux-usb&m=138921117504911&w=2
>http://marc.info/?l=linux-usb&m=138921117604912&w=2
>
>I'm concerned the I/O errors are caused by commit 459d3c146117 "usb:
>xhci: Link TRB must not occur within a USB payload burst".
>
>Can you also send me the output of `sudo lspci -vvv -n`? I need the
>output for the xHCI host only.
>
>Sarah Sharp
>
>> __
>> > Od: Sarah Sharp
>> > Komu: Milan Svoboda
>> > Datum: 20.01.2014 19:23
>> > Předmět: Re: usb disk recognized but fails
>> >
>> > CC: linux-usb@vger.kernel.org, "USB Storage List"
>> > , linux-s...@vger.kernel.org
>> >Cc-ing the USB storage list, in case this device needs some quirk.
>> >
>> >On Wed, Jan 15, 2014 at 03:47:39PM +0100, Milan Svoboda wrote:
>> >> The SATA adapter has a power input, but it is not used, it is connected
>> >> to the
>> >> PC with Y-type USB cable.
>> >>
>> >> The disk works with my "smart" TV, it works with Windows on the same
>> >> machine,
>> >> it also had worked some time ago with Linux, but now it shows the
>> >> following:
>> >
>> >Do you know which kernel version it worked with? What version are you
>> >currently running? Please run `uname -a`.
>> >
>> >> [ 2192.027213] usb 1-3: new high-speed USB device number 5 using xhci_hcd
>> >> [ 2192.042323] ums-cypress 1-3:1.0: USB Mass Storage device detected
>> >> [ 2192.042500] scsi9 : usb-storage 1-3:1.0
>> >> [ 2193.044219] scsi 9:0:0:0: Direct-Access WDC WD64 00BPVT-00HXZT0
>> >> PQ: 0 ANSI: 0
>> >> [ 2193.045083] sd 9:0:0:0: [sdb] 1250263728 512-byte logical blocks: (640
>> >> GB/596 GiB)
>> >> [ 2193.045240] sd 9:0:0:0: [sdb] Write Protect is off
>> >> [ 2193.045245] sd 9:0:0:0: [sdb] Mode Sense: 03 00 00 00
>> >> [ 2193.045384] sd 9:0:0:0: [sdb] No Caching mode page found
>> >> [ 2193.045388] sd 9:0:0:0: [sdb] Assuming drive cache: write through
>> >> [ 2193.046259] sd 9:0:0:0: [sdb] No Caching mode page found
>> >> [ 2193.046264] sd 9:0:0:0: [sdb] Assumi
Re: Strange behavior with ZTE MF821D (and possible other MDM9200 devices)
Hi all, After spending more time experimenting and trying to solve this problem, we have sort-of come to a conclusion. In case anyone else finds it interesting, I thought I should share it here. Our findings can be summarized as follows: - The problem is triggered by the switch from 3G to 2G. I can reliably trigger it by sending AT+ZSNT=1,0,0 to the modem or setting system selection to 0x0004 (both locking MF821D to 2G). - The root cause is the modem, but I can trigger the error on different USB hosts. I have been able to replicate it on the earlier mention router, my two laptops (with Intel-chipsets), a Raspberry Pi and some commercial mobile broadband routers. However, some hosts seem more resilliant than others. One for example the TP-Link I can trigger it all the time, while on my laptop it is more seldom. - One theory that we have not been able to verify is that the problem is somehow related to ISP. I got some friends in another country to try the AT-command with MF821D and the TP-Link WDR4300, and they did not see it. - I tried the same with another modem I have, the Huawei E392. I had no problems locking to 2G or moving between 3G/2G. - Our current work-around is to disable 2G. After doing this we have not seen this problem. -Kristian -- To unsubscribe from this list: send the line "unsubscribe 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] FL1009: xHCI host not responding to stop endpoint command.
On Sat, Jan 18, 2014 at 10:49:17PM +0100, Arnaud Ebalard wrote:
> Hi,
>
> I have added Thomas in the recipients, because I guess he may be of some
> help debugging the issue further. Thomas, the beginning of the thread is
> here: http://thread.gmane.org/gmane.linux.usb.general/101531
...
> I started suspecting the introduction of MSI support in Marvell PCIe
> host controller driver (FL1009 is on the PCIe bus) and compiled a
> a 3.13.0-rc8 w/ CONFIG_PCI_MSI disabled (it was enabled in all my
> previous tests): I did not manage to reproduce the issue with this
> kernel. As a side note, commits 5b4deb6526bd, 31f614edb726 and
> 627dfcc249e2 are
>
> ATM, I do not know if the problem is related to a bug in introduced MSI
> support or some weird incompatibility of that functionality with the
> FL1009 which would require some quirk in XHCI stack.
We've actually had issues in the past with Fresco Logic hosts not
supporting MSI properly, even though the PCI devices claim to have MSI
support. So turning off CONFIG_PCI_MSI may actually mean the Fresco
Logic host is to blame, rather than the Marvell patches. I assume MSI
wouldn't have been turned on for the Fresco Logic host unless the parent
PCI host controller supported it.
Let's see if the Fresco Logic host is really the root cause. Please
apply the this patch to 3.13.0-rc8 and recompile with CONFIG_PCI_MSI
enabled:
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 6c03584ac15f..74748444c040 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -30,6 +30,7 @@
/* Device for a quirk */
#define PCI_VENDOR_ID_FRESCO_LOGIC 0x1b73
#define PCI_DEVICE_ID_FRESCO_LOGIC_PDK 0x1000
+#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1009 0x1009
#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1400 0x1400
#define PCI_VENDOR_ID_ETRON0x1b6f
@@ -63,6 +64,9 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
/* Look for vendor-specific quirks */
if (pdev->vendor == PCI_VENDOR_ID_FRESCO_LOGIC &&
+ pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_FL1009)
+ xhci->quirks |= XHCI_BROKEN_MSI;
+ if (pdev->vendor == PCI_VENDOR_ID_FRESCO_LOGIC &&
(pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK ||
pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_FL1400)) {
if (pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK &&
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] usb: host: xhci: fix HALTED endpoint handling
On Tue, Jan 21, 2014 at 12:11:39PM -0600, Felipe Balbi wrote: > Hi, > > On Tue, Jan 21, 2014 at 01:06:19PM -0500, Alan Stern wrote: > > > You could try to differentiate between an endpoint halt that requires a > > > manual cleanup, and ones that the upper layer should handle, perhaps by > > > adding a new EP_HALTED_MANUAL flag. The driver could accept URBs for > > > the manual cleanup case, and reject URBs with -EPIPE for the ones the > > > upper layers should handle. > > > > That should not be necessary. The HCD should always accept URB > > submissions. Nothing will get sent to the device until the HCD clears > > any host-side halts, but the upper layers don't worry about that > > because the HCD takes care of it automatically. > > > > Contrariwise, the upper layers are responsible for clearing device-side > > halts. The HCD should ignore that end of things as much as it can. > > right, but there's still the bug that usb_submit_urb() times out with > xhci and it doesn't with ehci. So there _is_ a bug in xhci. > > I'll try to debug more and figure out what's really going on, but one > thing is for sure. usbtest issued a SetFeature(ENDPOINT_HALT) as part of > the test and it tries to usb_submit_urb() as means to verify that the > device is really responding with Stalls. ... > I suppose it doesn't make a difference, considering it > wait_for_completion_interruptible(). As long as the transfer actually > completes within 10 seconds, it doesn't matter. > > What I have seen, though, is the transfer never completes and the 10 > second timeout always kicks in when trying two consecutive > usb_submit_urb(). > > Here's, in a nut-shell, what happens: > > -> SetFeature(ENDPOINT_HALT) > -> GetStatus() > -> usb_submit_urb() > -> this one completes with -EPIPE as it should > -> usb_submit_urb() > -> this one always times out :-( Ah, I think I see what's happening. The xHCI driver is not designed to execute transfers after a stall on a non-control endpoint until xhci_endpoint_reset is called and the driver fixes up the endpoint ring. That will happen in usb_clear_halt(). I expect that if an URB is submitted to a stalled non-control endpoint before usb_clear_halt is called, the xHCI driver should see EP_HALTED is set and refuse to ring the doorbell in that case. Once usb_clear_halt calls endpoint_reset, the xHCI driver will clean up the endpoint ring. The way the code was designed doesn't account for the device driver telling the endpoint to halt via a control transfer. The xHC doesn't snoop that particular control transfer, so it allows the first URB to be submitted. When that transfer stalls, the driver sets EP_HALTED. Then the second URB hangs as I expect, because the xHCI driver is waiting for usb_clear_halt() to be called. If it really matters that a driver be able to get an immediate -EPIPE response back from a device when the endpoint is halted, I can rip the code out of xhci_endpoint_reset() and make the driver manually clean up the ring at the time of the stall, like it does for control endpoints and babble/transfer/split TX errors. If it doesn't matter, test #13 should be changed to only submit one URB and get back an -EPIPE response. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 03/10] usb: find internal hub tier mismatch via acpi
On Tue, 21 Jan 2014, Dan Williams wrote:
> > > We want to:
> > >
> > > 1/ prevent khubd from running while reset is in progress
> >
> > This is the open question mentioned above, if you add in the
> > requirement that we also want to prevent a reset from starting while
> > khubd is running (except when khubd performs the reset itself).
> >
> > > 2/ prevent khubd from seeing an intermediate device state during
> > > usb_port_resume
> >
> > khubd doesn't pay all that much attention to device states; mostly it
> > is concerned with port statuses. We don't want khubd to see an
> > intermediate port status during usb_port_resume. Basically that means
> > we don't want khubd to run while usb_port_resume is in progress, and we
> > don't want usb_port_resume to start while khubd is running unless khubd
> > performs the resume itself.
>
> Yes, it does not pay all that much attention to device states, but it is
> critical that it not read ->state while usb_port_resume() is changing
> it. Specifically this hole in hub_port_connect_change():
It doesn't matter. We don't want khubd and usb_port_resume to run at
the same time, for other reasons, and therefore we don't have to worry
about khubd seeing an intermediate device state caused by
usb_port_resume.
It is true, however, that the region of usb_port_resume currently
protected by hub->busy_bits isn't big enough.
> > > 3/ prevent khubd from running in the interval between completion of
> > > ubs_port_runtime_resume() completing and usb_port_resume() running.
> >
> > Hmmm. I don't remember what usb_port_runtime_resume does about a
> > connected USB-2 child device. It can't simply do a runtime resume of
> > the child. Maybe it should tell khubd to resume the child? Then khubd
> > would never see the intermediate state; the next time it looked at the
> > port, it would issue the runtime resume. (Provided that
> > usb_port_runtime_resume didn't complete after khubd was already
> > running.)
>
> Yes, that's what I had in the patch, more straightforward than the work
> queue.
Okay.
> > > All these scenarios to point to problems with ->device_state collisions
> > > not necessarily the port state.
> >
> > I don't think so, for the reason mentioned above.
>
> I'll rephrase, checking USB_STATE_SUSPENDED needs usb_port_resume()
> synchronization.
Which we need anyway, so the device_state aspect doesn't matter. (You
may have overlooked the port-status tests in hub_handle_remote_wakeup.)
> I propose we use usb_port pm runtime synchronization to block khubd when
> we know the portstatus is not valid (powered off),
Suppose, when you power the port back on, you find that the device is
no longer plugged in. How do you prevent the port from being powered
off again before khubd has a chance to finalize the device? I guess
you would have to do a pm_runtime_get on the port whenever the port's
bit in hub->change_bits gets set.
> and we need a new
> lock to synchronize with usb_device pm operations before checking the
> device state.
Forget about the device state. It's a distraction.
> I think we agree that khubd needs to not look at the portstatus while
> the port is down. pm runtime synchronization takes care of that and
> prevents khubd from starting while the port is inactive. With that in
> place we can now make requests in usb_port_runtime_resume() that are
> later serviced in khubd when the port is marked active (pm runtime
> enforces that ->runtime_resume() return 0 before the port is marked
> active).
How do you know, when you make the request, that khubd won't start
running while the port is still down? If that happened then khubd
would know to avoid looking at the port, of course, but the request
would be used up and lost.
With a lock, this problem doesn't arise.
> This mechanism can be used for forcing at least one child
> resume for each port resume, and rate-limiting port power requests.
What's the connection with rate-limiting? Under what circumstances
would you need to rate-limit the port power changes?
> For the next case, fixing khubd vs device_state changes, we can't use
> pm_runtime synchronization because khubd is expected to be able to
> resume a suspended usb_device. So we need to prevent khubd from
> evaluating the device_state (USB_STATE_SUSPENDED) while usb_device pm
> operations are in flight. In the case of dpm_{suspend|resume} khubd is
> frozen so there's no collision, in the case of rpm_{suspend|resume} we
> need to take a lock.
Agreed.
> Let's wrap usb_runtime_{resume|suspend} in a new usb_device->state_lock
> and then take that lock in khubd whenever it needs to check ->state or
> portstatus values associated with suspend. Specifically:
>
> @@ -1765,10 +1774,14 @@ int usb_runtime_resume(struct device *dev)
> struct usb_device *udev = to_usb_device(dev);
> int status;
>
> - /* Runtime resume for a USB device means resuming both the device
> -
Re: [PATCH 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst [NEW HARDWARE]
On 01/21/2014 01:51 AM, David Laight wrote:
> From: Sarah Sharp
>> On Mon, Jan 20, 2014 at 11:21:14AM +, David Laight wrote:
> ...
>>> A guess...
>>>
>>> In queue_bulk_sg_tx() try calling xhci_v1_0_td_remainder() instead
>>> of xhci_td_remainder().
>>
>> Why? Walt has a 0.96 xHCI host controller, and the format for how to
>> calculate the TD remainder changed between the 0.96 and the 1.0 spec.
>> That's why we have xhci_v1_0_td_remainder() and xhci_td_remainder().
>
> I just wonder how many of those differences are just differences in the
> specification, rather than differences in the hardware implementation.
> In some cases it might be that the old hardware just ignored the value.
>
> I know that the xhci hardware on my ivy bridge cpu does look at that
> value (at least checking for zero), since things failed in subtle ways
> when I got it wrong.
>
> In this case it was just something easy to change that might be worth
> trying. I didn't necessarily expect it to make a positive difference.
David, I tried the one-liner below, which changed nothing AFAICS, but
then I'm not sure it's the change you intended:
--- xhci-ring.c.orig2014-01-21 13:28:36.396278813 -0800
+++ xhci-ring.c 2014-01-21 13:35:11.410312814 -0800
@@ -3335,7 +3335,7 @@
}
/* Set the TRB length, TD size, and interrupter fields. */
- if (xhci->hci_version < 0x100) {
+ if (xhci->hci_version > 0x100) {
remainder = xhci_td_remainder(
urb->transfer_buffer_length -
running_total);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] Re: usb disk recognized but fails
On Tue, 21 Jan 2014, Milan Svoboda wrote: > I have applied the patches to a0fa1dd3cdbccec9597fe53b6177a9aa6e20f2f8: > The kernel logs: > > [ 114.854464] usb 3-1: new high-speed USB device number 2 using xhci_hcd > [ 114.920229] usbcore: registered new interface driver usb-storage > [ 114.920647] ums-cypress 3-1:1.0: USB Mass Storage device detected > [ 114.920705] scsi6 : usb-storage 3-1:1.0 > [ 114.920816] usbcore: registered new interface driver ums-cypress > [ 115.921301] scsi 6:0:0:0: Direct-Access WDC WD64 00BPVT-00HXZT0 > PQ: 0 ANSI: 0 > [ 115.921991] sd 6:0:0:0: [sdb] 1250263728 512-byte logical blocks: (640 > GB/596 GiB) > [ 115.922142] sd 6:0:0:0: [sdb] Write Protect is off > [ 115.922145] sd 6:0:0:0: [sdb] Mode Sense: 03 00 00 00 > [ 115.922269] sd 6:0:0:0: [sdb] No Caching mode page found > [ 115.922271] sd 6:0:0:0: [sdb] Assuming drive cache: write through > [ 115.922929] sd 6:0:0:0: [sdb] No Caching mode page found > [ 115.922931] sd 6:0:0:0: [sdb] Assuming drive cache: write through > [ 115.938479] sdb: sdb1 > [ 115.939112] sd 6:0:0:0: [sdb] No Caching mode page found > [ 115.939115] sd 6:0:0:0: [sdb] Assuming drive cache: write through > [ 115.939117] sd 6:0:0:0: [sdb] Attached SCSI disk > [ 124.241874] usb 3-1: reset high-speed USB device number 2 using xhci_hcd This reset is unexplained. It happened almost 70 seconds before the error below. > [ 124.255232] xhci_hcd :00:14.0: xHCI xhci_drop_endpoint called with > disabled ep 8801f899f700 > [ 124.255237] xhci_hcd :00:14.0: xHCI xhci_drop_endpoint called with > disabled ep 8801f899f740 > [ 128.552357] vboxdrv: Found 4 processor cores. > [ 128.552802] vboxdrv: fAsync=0 offMin=0x1dd offMax=0x16bc > [ 128.552862] vboxdrv: TSC mode is 'synchronous', kernel timer mode is > 'normal'. > [ 128.552864] vboxdrv: Successfully loaded version 4.3.6_OSE (interface > 0x001a0007). > [ 128.556554] vboxpci: IOMMU not found (not registered) > [ 191.463779] sd 6:0:0:0: [sdb] Unhandled sense code > [ 191.463786] sd 6:0:0:0: [sdb] > [ 191.463788] Result: hostbyte=0x07 driverbyte=0x08 > [ 191.463791] sd 6:0:0:0: [sdb] > [ 191.463793] Sense Key : 0x4 [current] > [ 191.463797] sd 6:0:0:0: [sdb] > [ 191.463799] ASC=0x0 ASCQ=0x0 > [ 191.463802] sd 6:0:0:0: [sdb] CDB: > [ 191.463803] cdb[0]=0x28: 28 00 4a 85 6e af 00 00 08 00 > [ 191.463813] end_request: I/O error, dev sdb, sector 1250258607 Sense Key 4 means Hardware Error. It's possible that something is wrong with your disk. In any case, can you post a usbmon trace showing what happens starting from when you plug in the USB disk? Instructions are in the kernel source file Documentation/usb/usbmon.txt. 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: host: xhci: fix HALTED endpoint handling
On Tue, 21 Jan 2014, Sarah Sharp wrote: > Ah, I think I see what's happening. > > The xHCI driver is not designed to execute transfers after a stall on a > non-control endpoint until xhci_endpoint_reset is called and the driver > fixes up the endpoint ring. That will happen in usb_clear_halt(). > > I expect that if an URB is submitted to a stalled non-control endpoint > before usb_clear_halt is called, the xHCI driver should see EP_HALTED is > set and refuse to ring the doorbell in that case. Once usb_clear_halt > calls endpoint_reset, the xHCI driver will clean up the endpoint ring. > > The way the code was designed doesn't account for the device driver > telling the endpoint to halt via a control transfer. The xHC doesn't > snoop that particular control transfer, so it allows the first URB to be > submitted. When that transfer stalls, the driver sets EP_HALTED. Then > the second URB hangs as I expect, because the xHCI driver is waiting for > usb_clear_halt() to be called. > > If it really matters that a driver be able to get an immediate -EPIPE > response back from a device when the endpoint is halted, I can rip the > code out of xhci_endpoint_reset() and make the driver manually clean up > the ring at the time of the stall, like it does for control endpoints > and babble/transfer/split TX errors. If it doesn't matter, test #13 > should be changed to only submit one URB and get back an -EPIPE > response. In theory, errors caused by the device sending a STALL should be handled just like other sorts of errors. After the URB which got the STALL has been given back, the HCD should automatically restart the endpoint ring. It should not wait for usb_clear_halt. 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 v3 03/10] usb: find internal hub tier mismatch via acpi
On Tue, Jan 21, 2014 at 2:05 PM, Alan Stern wrote:
> On Tue, 21 Jan 2014, Dan Williams wrote:
>
>> > > We want to:
>> > >
>> > > 1/ prevent khubd from running while reset is in progress
>> >
>> > This is the open question mentioned above, if you add in the
>> > requirement that we also want to prevent a reset from starting while
>> > khubd is running (except when khubd performs the reset itself).
>> >
>> > > 2/ prevent khubd from seeing an intermediate device state during
>> > > usb_port_resume
>> >
>> > khubd doesn't pay all that much attention to device states; mostly it
>> > is concerned with port statuses. We don't want khubd to see an
>> > intermediate port status during usb_port_resume. Basically that means
>> > we don't want khubd to run while usb_port_resume is in progress, and we
>> > don't want usb_port_resume to start while khubd is running unless khubd
>> > performs the resume itself.
>>
>> Yes, it does not pay all that much attention to device states, but it is
>> critical that it not read ->state while usb_port_resume() is changing
>> it. Specifically this hole in hub_port_connect_change():
>
> It doesn't matter. We don't want khubd and usb_port_resume to run at
> the same time, for other reasons, and therefore we don't have to worry
> about khubd seeing an intermediate device state caused by
> usb_port_resume.
I won't argue this anymore. Protecting khubd against portstatus
changes makes the device state moot.
[..]
>> > > All these scenarios to point to problems with ->device_state collisions
>> > > not necessarily the port state.
>> >
>> > I don't think so, for the reason mentioned above.
>>
>> I'll rephrase, checking USB_STATE_SUSPENDED needs usb_port_resume()
>> synchronization.
>
> Which we need anyway, so the device_state aspect doesn't matter. (You
> may have overlooked the port-status tests in hub_handle_remote_wakeup.)
No, I considered it.
I think we only need protection vs usb_port_{suspend|resume} in three cases
1/ hub_handle_remote_wakeup() where we check if the link/device is suspended
2/ hub_connect_change() where we check for connection
3/ in hub_events() where we check if warm reset is required as
usb_port_resume may also want to perform warm resets
->state and portstatus are intertwined in these checks, but I can see
your point that it really is more about portstatus. Sychronizing
->state comes along for the ride.
>> I propose we use usb_port pm runtime synchronization to block khubd when
>> we know the portstatus is not valid (powered off),
>
> Suppose, when you power the port back on, you find that the device is
> no longer plugged in. How do you prevent the port from being powered
> off again before khubd has a chance to finalize the device? I guess
> you would have to do a pm_runtime_get on the port whenever the port's
> bit in hub->change_bits gets set.
Simple, take a pm reference in usb_port_runtime_resume and flag the
port as "needs child resume". Drop the reference once the child
resume completes. The port is !active until it returns from
usb_port_runtime_resume, and khubd performs a pm barrier
>> and we need a new
>> lock to synchronize with usb_device pm operations before checking the
>> device state.
>
> Forget about the device state. It's a distraction.
Done.
>> I think we agree that khubd needs to not look at the portstatus while
>> the port is down. pm runtime synchronization takes care of that and
>> prevents khubd from starting while the port is inactive. With that in
>> place we can now make requests in usb_port_runtime_resume() that are
>> later serviced in khubd when the port is marked active (pm runtime
>> enforces that ->runtime_resume() return 0 before the port is marked
>> active).
>
> How do you know, when you make the request, that khubd won't start
> running while the port is still down? If that happened then khubd
> would know to avoid looking at the port, of course, but the request
> would be used up and lost.
Per above it requires khubd to barrier pm operations to finish
bringing the port up. It does this while holding a reference to
ensure the port does not go down while checking. I also expect a
reference to be taken when the request is queued and dropped when
khubd services it.
> With a lock, this problem doesn't arise.
A lock for this scenario would effectively duplicate dev->power.lock,
or be wider than necessary if I understand where you are considering
holding it.
>> This mechanism can be used for forcing at least one child
>> resume for each port resume, and rate-limiting port power requests.
>
> What's the connection with rate-limiting? Under what circumstances
> would you need to rate-limit the port power changes?
Only testing scenarios where the port is turning on and off at a
potentially high frequency. Forcing child resume limits that period
to the latency of device recovery.
>> For the next case, fixing khubd vs device_state changes, we can't use
>> pm_runtime synchronization because khubd is e
174c:5106 1 TB External USB 3.0 Drive Fails to Automount through USB 3.0 dock with XHCI Enabled
[1.] 174c:5106 1 TB External USB 3.0 Drive Fails to Automount through USB 3.0 dock with XHCI Enabled [2.] I have a 1 TB Western Digital drive in a USB 3.0 HDD dock that will not automount while XHCI is enabled in the BIOS. If I disable XHCI in the BIOS, it automounts normally. I've tried an external USB 3.0 enclosure and two different brands of USB 3.0 HDD docks (that use different chipsets) with the 1 TB drive and it still will not automount. Dmesg shows the following when the drive is turned on: [10813.786820] usb 4-5.3: new SuperSpeed USB device number 4 using xhci_hcd [10823.816236] usb 4-5.3: New USB device found, idVendor=174c, idProduct=5106 [10823.816245] usb 4-5.3: New USB device strings: Mfr=2, Product=3, SerialNumber=1 [10823.816250] usb 4-5.3: Product: AS2105 [10823.816255] usb 4-5.3: Manufacturer: ASMedia [10828.823853] usb 4-5.3: can't set config #1, error -110 WORKAROUND: Disable XHCI in the BIOS, but this runs at USB 2.0 speeds using EHCI. I actually thought the 1 TB drive was bad, and I RMA'd it to Western Digital, but the replacement drive fails with the same error message. I have a couple of Western Digital 500 GB drives that work just fine in the same HDD dock with XHCI enabled. All the drives were formatted in using LUKS. I have tried various settings in the BIOS (XHCI set at Smart Auto, Auto, and Manual) but they make no difference. I found an old bug report with the same "can't set config #1, error -110" error message that recommended adding "pci=nomsi" to Grub, but that did not work either. [3.] [4.] Linux version 3.13.0-031300-generic (apw@gomeisa) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #201401192235 SMP Mon Jan 20 03:36:48 UTC 2014 [5.] Not applicable [6.] None [7.] lsb_release -rd Description:Ubuntu 13.10 Release:13.10 [7.1] sh ver_linux Linux username 3.13.0-031300-generic #201401192235 SMP Mon Jan 20 03:36:48 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Gnu C 4.8 Gnu make 3.81 binutils 2.23.52.20130913 util-linux 2.20.1 mount support module-init-tools 9 e2fsprogs 1.42.8 pcmciautils018 PPP2.4.5 Linux C Library2.17 Dynamic linker (ldd) 2.17 Procps 3.3.3 Net-tools 1.60 Kbd1.15.5 Sh-utils 8.20 wireless-tools 30 Modules Loaded parport_pc ppdev rfcomm bnep bluetooth snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_intel snd_hda_codec nls_iso8859_1 snd_hwdep x86_pkg_temp_thermal intel_powerclamp snd_pcm snd_page_alloc coretemp snd_seq_midi snd_seq_midi_event snd_rawmidi kvm_intel snd_seq lp snd_seq_device snd_timer mei_me kvm mei snd parport soundcore serio_raw lpc_ich microcode mac_hid hid_generic usbhid hid dm_crypt crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel nouveau aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd ahci mxm_wmi libahci i2c_algo_bit ttm e1000e drm_kms_helper drm ptp pps_core wmi video [7.2] cat /proc/cpuinfo processor : 0 vendor_id : GenuineIntel cpu family : 6 model : 60 model name : Intel(R) Core(TM) i7-4770K CPU @ 3.50GHz stepping: 3 microcode : 0x9 cpu MHz : 3900.722 cache size : 8192 KB physical id : 0 siblings: 8 core id : 0 cpu cores : 4 apicid : 0 initial apicid : 0 fpu : yes fpu_exception : yes cpuid level : 13 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm ida arat xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid bogomips: 7000.12 clflush size: 64 cache_alignment : 64 address sizes : 39 bits physical, 48 bits virtual power management: processor : 1 vendor_id : GenuineIntel cpu family : 6 model : 60 model name : Intel(R) Core(TM) i7-4770K CPU @ 3.50GHz stepping: 3 microcode : 0x9 cpu MHz : 3900.039 cache size : 8192 KB physical id : 0 siblings: 8 core id : 1 cpu cores : 4 apicid : 2 initial apicid : 2 fpu : yes fpu_exception : yes cpuid level : 13 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3
Re: [PATCH] usb: gadget: bcm63xx_udc: fix build failure on DMA channel code
On Tue, Jan 21, 2014 at 11:20:09AM -0800, Florian Fainelli wrote:
> 2014/1/14 Florian Fainelli :
> > Commit 3dc6475 ("bcm63xx_enet: add support Broadcom BCM6345 Ethernet")
> > changed the ENETDMA[CS] macros such that they are no longer macros, but
> > actual register offset definitions. The bcm63xx_udc driver was not
> > updated, and as a result, causes the following build error to pop up:
>
> Felipe, Greg, any feedback on this patch? Can we get this merged since
> this fixes a build failure?
It must be a pretty rare build failure as this is the first I've seen
it. :)
It can wait for the "normal" bugfix process (i.e. through Felipe's tree
after 3.14-rc1 is out) as no one else has had the problem for the past
year or so...
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: gadget: bcm63xx_udc: fix build failure on DMA channel code
2014/1/21 Greg KH :
> On Tue, Jan 21, 2014 at 11:20:09AM -0800, Florian Fainelli wrote:
>> 2014/1/14 Florian Fainelli :
>> > Commit 3dc6475 ("bcm63xx_enet: add support Broadcom BCM6345 Ethernet")
>> > changed the ENETDMA[CS] macros such that they are no longer macros, but
>> > actual register offset definitions. The bcm63xx_udc driver was not
>> > updated, and as a result, causes the following build error to pop up:
>>
>> Felipe, Greg, any feedback on this patch? Can we get this merged since
>> this fixes a build failure?
>
> It must be a pretty rare build failure as this is the first I've seen
> it. :)
It requires building with MIPS && BCM63XX and even there, this driver
is not part of the defconfig (which I intend to update with
bcm63xx_udc once this is fixed), hence the reason why nobody noticed.
>
> It can wait for the "normal" bugfix process (i.e. through Felipe's tree
> after 3.14-rc1 is out) as no one else has had the problem for the past
> year or so...
Sure, that can wait. Thanks for the reply!
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB EHCI broken on Celeron N2920 (Bay Trail)
Hi Sarah, The patch is useful, thanks for your help. Thanks. -- Chris Cheng 鄭宇廷 Atrust Computer Corp. 冠信電腦股份有限公司 http://www.atrustcorp.com Tel: +886-3-3288837 ext.1074 3F., No.361, Fusing 1st Rd., Gueishan Township, Taoyuan County 333, Taiwan 333 桃園縣龜山鄉復興一路361號3F 2014/1/22 Sarah Sharp : > Hi Chris, > > We think the Baytrail BIOS has an issue where it doesn't set the EHCI > PCI interrupt enable bit when the computer is in "EHCI only mode". > Please try this patch: > > http://marc.info/?l=linux-usb&m=138998295909608&w=2 > > Sarah Sharp > > On Tue, Jan 21, 2014 at 10:04:41PM +0800, Chris Cheng wrote: >> I'm sorry forgot kernel version: 3.13.0 . >> -- >> Chris Cheng 鄭宇廷 >> Atrust Computer Corp. 冠信電腦股份有限公司 >> http://www.atrustcorp.com >> Tel: +886-3-3288837 ext.1074 >> 3F., No.361, Fusing 1st Rd., Gueishan Township, Taoyuan County 333, Taiwan >> 333 桃園縣龜山鄉復興一路361號3F >> >> >> 2014/1/21 Chris Cheng : >> > Here is kernel message: >> > >> > [2.939251] usb 1-1: new high-speed USB device number 2 using ehci-pci >> > [ 18.490987] usb 1-1: device not accepting address 2, error -110 >> > [ 18.595095] usb 1-1: new high-speed USB device number 3 using ehci-pci >> > [ 34.182233] usb 1-1: device not accepting address 3, error -110 >> > [ 34.286352] usb 1-1: new high-speed USB device number 4 using ehci-pci >> > [ 44.744589] usb 1-1: device not accepting address 4, error -110 >> > [ 44.849590] usb 1-1: new high-speed USB device number 5 using ehci-pci >> > [ 55.307858] usb 1-1: device not accepting address 5, error -110 >> > [ 55.310239] hub 1-0:1.0: unable to enumerate USB device on port 1 >> > >> > Thanks. >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe 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: 174c:5106 1 TB External USB 3.0 Drive Fails to Automount through USB 3.0 dock with XHCI Enabled
On 01/21/2014 04:51 PM, Jay S wrote: > [1.] 174c:5106 1 TB External USB 3.0 Drive Fails to Automount through USB 3.0 > dock with XHCI Enabled > [10823.816255] usb 4-5.3: Manufacturer: ASMedia I'm interested in ASMedia usb3 bugs :) > [4.] Linux version 3.13.0-031300-generic You're running a bleeding-edge kernel, I see. I know nothing about Linaro, unfortunately -- can you test older kernel versions so as to tell us which older kernel version can use the usb3 drive in the normal and expected way? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi,
On Tue, Jan 21, 2014 at 7:30 PM, Roger Quadros wrote:
> Hi Kishon,
>
> On 01/21/2014 12:11 PM, Kishon Vijay Abraham I wrote:
>> Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
>> power_on and power_off the following APIs are used phy_init(), phy_exit(),
>> phy_power_on() and phy_power_off().
>>
>> However using the old USB phy library wont be removed till the PHYs of all
>> other SoC's using dwc3 core is adapted to the Generic PHY Framework.
>>
>> Signed-off-by: Kishon Vijay Abraham I
>> ---
>> Changes from v3:
>> * avoided using quirks
>>
>> Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
>> drivers/usb/dwc3/core.c| 60
>>
>> drivers/usb/dwc3/core.h|7 +++
>> 3 files changed, 71 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
>> b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index e807635..471366d 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -6,11 +6,13 @@ Required properties:
>> - compatible: must be "snps,dwc3"
>> - reg : Address and length of the register set for the device
>> - interrupts: Interrupts used by the dwc3 controller.
>> +
>> +Optional properties:
>> - usb-phy : array of phandle for the PHY device. The first element
>> in the array is expected to be a handle to the USB2/HS PHY and
>> the second element is expected to be a handle to the USB3/SS PHY
>> -
>> -Optional properties:
>> + - phys: from the *Generic PHY* bindings
>> + - phy-names: from the *Generic PHY* bindings
>> - tx-fifo-resize: determines if the FIFO *has* to be reallocated.
>>
>> This is usually a subnode to DWC3 glue to which it is connected.
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index e009d4e..036d589 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -82,6 +82,11 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
>>
>> usb_phy_init(dwc->usb2_phy);
>> usb_phy_init(dwc->usb3_phy);
>> + if (dwc->usb2_generic_phy)
>> + phy_init(dwc->usb2_generic_phy);
>
> What if phy_init() fails? You need to report and fail. Same applies for all
> PHY apis in this patch.
>
>> + if (dwc->usb3_generic_phy)
>> + phy_init(dwc->usb3_generic_phy);
>> +
>> mdelay(100);
>>
>> /* Clear USB3 PHY reset */
>> @@ -343,6 +348,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
>> {
>> usb_phy_shutdown(dwc->usb2_phy);
>> usb_phy_shutdown(dwc->usb3_phy);
>> + if (dwc->usb2_generic_phy)
>> + phy_exit(dwc->usb2_generic_phy);
>> + if (dwc->usb3_generic_phy)
>> + phy_exit(dwc->usb3_generic_phy);
>> +
>> }
>>
>> #define DWC3_ALIGN_MASK (16 - 1)
>> @@ -433,6 +443,32 @@ static int dwc3_probe(struct platform_device *pdev)
>> }
>> }
>>
>> + dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
>> + if (IS_ERR(dwc->usb2_generic_phy)) {
>> + ret = PTR_ERR(dwc->usb2_generic_phy);
>> + if (ret == -ENOSYS || ret == -ENODEV) {
>> + dwc->usb2_generic_phy = NULL;
>> + } else if (ret == -EPROBE_DEFER) {
>> + return ret;
>> + } else {
>> + dev_err(dev, "no usb2 phy configured\n");
>> + return ret;
>> + }
>> + }
>> +
>> + dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
>> + if (IS_ERR(dwc->usb3_generic_phy)) {
>> + ret = PTR_ERR(dwc->usb3_generic_phy);
>> + if (ret == -ENOSYS || ret == -ENODEV) {
>> + dwc->usb3_generic_phy = NULL;
>> + } else if (ret == -EPROBE_DEFER) {
>> + return ret;
>> + } else {
>> + dev_err(dev, "no usb3 phy configured\n");
>> + return ret;
>> + }
>> + }
>> +
>> dwc->xhci_resources[0].start = res->start;
>> dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
>> DWC3_XHCI_REGS_END;
>> @@ -482,6 +518,11 @@ static int dwc3_probe(struct platform_device *pdev)
>> usb_phy_set_suspend(dwc->usb2_phy, 0);
>> usb_phy_set_suspend(dwc->usb3_phy, 0);
>>
>> + if (dwc->usb2_generic_phy)
>> + phy_power_on(dwc->usb2_generic_phy);
>> + if (dwc->usb3_generic_phy)
>> + phy_power_on(dwc->usb3_generic_phy);
>> +
>
> Is it OK to power on the phy before phy_init()?
Isn't phy_init() being done before phy_power_on() in the
core_soft_reset() in this patch ?
Isn't that what you want here ?
>
> I suggest to move phy_init() from core_soft_reset() to here, just before
> phy_power_on().
core_soft_reset() is called before phy_power_on() itself from
dwc3_core_init(), right ?
will moving the phy_inti() here make na
