[PATCH v2 2/4] xhci.c: Add polling support for USB keyboards

[Jason Wessel]

The xhci driver was causing intermittent 5 second delays from the USB
keyboard polling hook.  Executing something like a "sleep 1" for
example would sleep for 5 seconds, unless an event occurred on
the USB bus to shorten the delay.

Modeled after the code in the DWC2 driver, a nonblock state was added
to quickly return instead of blocking for up to 5 seconds waiting for
an event before timing out.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 26 +-
 drivers/usb/host/xhci.c  | 11 ++-
 include/usb/xhci.h   |  5 +++--
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 092ed6eaf1..607d4f715e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -432,9 +432,11 @@ static int event_ready(struct xhci_ctrl *ctrl)
  *
  * @param ctrl Host controller data structure
  * @param expected TRB type expected from Event TRB
+ * @param nonblock when true do not block waiting for response
  * @return pointer to event trb
  */
-union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected)
+union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
+   bool nonblock)
 {
trb_type type;
unsigned long ts = get_timer(0);
@@ -442,8 +444,11 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl 
*ctrl, trb_type expected)
do {
union xhci_trb *event = ctrl->event_ring->dequeue;
 
-   if (!event_ready(ctrl))
+   if (!event_ready(ctrl)) {
+   if (nonblock)
+   return NULL;
continue;
+   }
 
type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags));
if (type == expected)
@@ -493,7 +498,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
field = le32_to_cpu(event->trans_event.flags);
BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
@@ -501,7 +506,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
!= COMP_STOP)));
xhci_acknowledge_event(ctrl);
 
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -509,7 +514,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, (void *)((uintptr_t)ring->enqueue |
ring->cycle_state), udev->slot_id, ep_index, TRB_SET_DEQ);
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -552,10 +557,11 @@ static void record_transfer_result(struct usb_device 
*udev,
  * @param pipe contains the DIR_IN or OUT , devnum
  * @param length   length of the buffer
  * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
  * @return returns 0 if successful else -1 on failure
  */
 int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer)
+   int length, void *buffer, bool nonblock)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -714,8 +720,10 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
+   if (nonblock)
+   return -EINVAL;
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeout */
@@ -911,7 +919,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
if (!event)
goto abort;
field = le32_to_cpu(event->tra

[PATCH v2 3/4] xhci-ring.c: Add poll pending state to properly abort transactions

[Jason Wessel]

Both xhci_ctrl_tx() and xhci_bulk_tx() can be called synchronously by
other drivers such as the usb storage or network, while the keyboard
driver exclusively uses the polling mode.

The reason the abort needs to happen is for the case when a keyboard
poll was issue but there was no response packet.  If another driver
such as the usb mass storage is called, it could receive the response
from the keyboard because only a single TRB queue is used.

Any pending polling transactions must be aborted before switching
modes to avoid corrupting the state of the controller and the driver
which expects a series of commands and responses from a specific
device.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 80 
 include/usb/xhci.h   |  5 +++
 2 files changed, 69 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 607d4f715e..00a0491771 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -549,19 +549,8 @@ static void record_transfer_result(struct usb_device *udev,
}
 }
 
-/ Bulk and Control transfer methods /
-/**
- * Queues up the BULK Request
- *
- * @param udev pointer to the USB device structure
- * @param pipe contains the DIR_IN or OUT , devnum
- * @param length   length of the buffer
- * @param buffer   buffer to be read/written based on the request
- * @param nonblock when true do not block waiting for response
- * @return returns 0 if successful else -1 on failure
- */
-int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer, bool nonblock)
+static int _xhci_bulk_tx_queue(struct usb_device *udev, unsigned long pipe,
+  int length, void *buffer)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -575,7 +564,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
struct xhci_virt_device *virt_dev;
struct xhci_ep_ctx *ep_ctx;
struct xhci_ring *ring; /* EP transfer ring */
-   union xhci_trb *event;
 
int running_total, trb_buff_len;
unsigned int total_packet_count;
@@ -719,20 +707,73 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
} while (running_total < length);
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
+   return 0;
+}
+
+/ Bulk and Control transfer methods /
+/**
+ * Queues up the BULK Request
+ *
+ * @param udev pointer to the USB device structure
+ * @param pipe contains the DIR_IN or OUT , devnum
+ * @param length   length of the buffer
+ * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
+ * @return returns 0 if successful else -1 on failure
+ */
+int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
+int length, void *buffer, bool nonblock)
+{
+   u32 field;
+   int ret;
+   union xhci_trb *event;
+   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
+   int ep_index = usb_pipe_ep_index(pipe);
 
+   if (ctrl->poll_pend) {
+   /*
+* Abort a pending poll operation if it should have
+* timed out, or if this is a different buffer from a
+* separate request
+*/
+   if (get_timer(ctrl->bulk_tx_poll_ts) > XHCI_TIMEOUT ||
+   ctrl->last_bulk_tx_buf != buffer || ctrl->poll_last_udev != 
udev ||
+   ep_index != ctrl->poll_last_ep_index) {
+   abort_td(ctrl->poll_last_udev, 
ctrl->poll_last_ep_index);
+   ctrl->poll_last_udev->status = USB_ST_NAK_REC;  /* 
closest thing to a timeout */
+   ctrl->poll_last_udev->act_len = 0;
+   ctrl->poll_pend = false;
+   }
+   } /* No else here because poll_pend might have changed above */
+   if (!ctrl->poll_pend) {
+   ctrl->last_bulk_tx_buf = buffer;
+   ret = _xhci_bulk_tx_queue(udev, pipe, length, buffer);
+   if (ret)
+   return ret;
+   }
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
-   if (nonblock)
+   if (nonblock) {
+   if (!ctrl->poll_pend) {
+   /* Start the timer */
+   ctrl->bulk_tx_poll_ts = get_timer(0);
+   ctrl->poll_last_udev = udev;
+   ctrl->poll_last_ep_index = ep_index;
+   ctrl->poll_pend = true;
+   }
return -EINVAL;
+   }
debug("XHCI bulk transfer

[PATCH 0/4][v2] xhci fixes for rpi4

[Jason Wessel]

This is the second iteration of the xhci patch set
for making USB keyboards usable on the rpi4 board.

Patch 1 is new to deal with the USB device not
responding immediately after a port reset.

Patch 3 is refactored after the review from Bin Meng.


Jason Wessel (4):
  xhci.c: Add retry in xhci_address_device()
  xhci.c: Add polling support for USB keyboards
  xhci-ring.c: Add poll pending state to properly abort transactions
  xhci-ring.c: Fix crash when issuing "usb reset"

 drivers/usb/host/xhci-ring.c | 115 +--
 drivers/usb/host/xhci.c  |  20 ++--
 include/usb/xhci.h   |  10 +++-
 3 files changed, 111 insertions(+), 34 deletions(-)

[PATCH v2 1/4] xhci.c: Add retry in xhci_address_device()

[Jason Wessel]

After a port reset some usb keyboard devices do not respond
immediately, and instead the controller reports COMP_TX_ERR.  Adding a
retry after the first TX error is returned resolves the issue.

Without the patch u-boot prints:

   Starting the controller
   USB XHCI 1.00
   scanning bus xhci_pci for devices... Device not responding to set address.

 USB device not accepting new address (error=8000)

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 126dabc11b..9a31eba2bb 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -675,6 +675,7 @@ static int xhci_address_device(struct usb_device *udev, int 
root_portnr)
struct xhci_virt_device *virt_dev;
int slot_id = udev->slot_id;
union xhci_trb *event;
+   int retry_cnt = 0;
 
virt_dev = ctrl->devs[slot_id];
 
@@ -685,6 +686,7 @@ static int xhci_address_device(struct usb_device *udev, int 
root_portnr)
debug("Setting up addressable devices %p\n", ctrl->dcbaa);
xhci_setup_addressable_virt_dev(ctrl, udev, root_portnr);
 
+retry:
ctrl_ctx = xhci_get_input_control_ctx(virt_dev->in_ctx);
ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG | EP0_FLAG);
ctrl_ctx->drop_flags = 0;
@@ -701,6 +703,13 @@ static int xhci_address_device(struct usb_device *udev, 
int root_portnr)
ret = -EINVAL;
break;
case COMP_TX_ERR:
+   retry_cnt++;
+   if (retry_cnt < 2) {
+   /* Retry in case this was just after a port reset */
+   debug("COMP_TX_ERR retry\n");
+   xhci_acknowledge_event(ctrl);
+   goto retry;
+   }
puts("Device not responding to set address.\n");
ret = -EPROTO;
break;
-- 
2.17.1

[PATCH v2 4/4] xhci-ring.c: Fix crash when issuing "usb reset"

[Jason Wessel]

When TRB_TRANSFER is set for a call to xhci_wait_for_event, it can
return null, which causes u-boot to crash.  This is an intermittent
problem found during "usb reset" testing.

If the null return occurs it means there was a timeout, and the
abort_td() should return immediately vs crashing u-boot from a null
dereference.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 00a0491771..bbb4410908 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -499,11 +499,16 @@ static void abort_td(struct usb_device *udev, int 
ep_index)
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
-   field = le32_to_cpu(event->trans_event.flags);
-   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
-   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
-   != COMP_STOP)));
+   if (event) {
+   field = le32_to_cpu(event->trans_event.flags);
+   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
+   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
+   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
+!= COMP_STOP)));
+   } else {
+   debug("XHCI abort timeout\n");
+   return;
+   }
xhci_acknowledge_event(ctrl);
 
event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
-- 
2.17.1

Re: [PATCH 5/9] xhci-ring.c: Add the poll_pend state to properly abort transactions

[Jason Wessel]

On 8/20/20 3:26 AM, Bin Meng wrote:
> Hi Jason,
> 
> On Sat, Jul 25, 2020 at 5:51 AM Jason Wessel  
> wrote:
>>
>> xhci_trl_tx and xhchi_bulk_tx can be called synchronously by other
>> drivers such as the usb storage or network, while the keyboard driver
>> exclusively uses the polling mode.
>>
> 
> Could you provide more details as to when this will fail?


An example is when the keyboard poll happens just before loading an
a kernel image from the USB device.  The poll sets up the TRB to listen
but the packet may arrive when a keyboard event occurs, and the
existing drivers will crash when the keyboard event is received
instead of the request it has issued synchronously. 

It didn't seem to happen too often, but extended testing discovered
the problem. 


> 
>> And pending polling transactions must be aborted before switching
>> modes to avoid corrupting the state of the controller.
>>
>> Signed-off-by: Jason Wessel 
>> ---
>>  drivers/usb/host/xhci-ring.c | 86 +---
>>  1 file changed, 70 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index b7b2e16410..d6339f4f0a 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -24,6 +24,12 @@
>>
>>  #include 
>>
>> +static void *last_bulk_tx_buf;
>> +static struct usb_device *poll_last_udev;
>> +int poll_last_ep_index;
>> +static unsigned long bulk_tx_poll_ts;
>> +static bool poll_pend;
> 
> Should these variables go into struct xhci_ctrl? What happens if 2
> controllers are sending data?
>


I would say you are correct.  Because the board I was using only
had a single controller, the issue is only fixed for the single controller 
case. 

I can re-work the patches to account for the problem. 

Jason. 


 
>> +
>>  /**
>>   * Is this TRB a link TRB or was the last TRB the last TRB in this event 
>> ring
>>   * segment?  I.e. would the updated event TRB pointer step off the end of 
>> the
>> @@ -549,19 +555,8 @@ static void record_transfer_result(struct usb_device 
>> *udev,
>> }
>>  }
>>
>> -/ Bulk and Control transfer methods /
>> -/**
>> - * Queues up the BULK Request
>> - *
>> - * @param udev pointer to the USB device structure
>> - * @param pipe contains the DIR_IN or OUT , devnum
>> - * @param length   length of the buffer
>> - * @param buffer   buffer to be read/written based on the request
>> - * @param nonblock when true do not block waiting for response
>> - * @return returns 0 if successful else -1 on failure
>> - */
>> -int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>> -   int length, void *buffer, bool nonblock)
>> +static int _xhci_bulk_tx_queue(struct usb_device *udev, unsigned long pipe,
>> +  int length, void *buffer)
>>  {
>> int num_trbs = 0;
>> struct xhci_generic_trb *start_trb;
>> @@ -575,7 +570,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
>> pipe,
>> struct xhci_virt_device *virt_dev;
>> struct xhci_ep_ctx *ep_ctx;
>> struct xhci_ring *ring; /* EP transfer ring */
>> -   union xhci_trb *event;
>>
>> int running_total, trb_buff_len;
>> unsigned int total_packet_count;
>> @@ -719,20 +713,73 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned 
>> long pipe,
>> } while (running_total < length);
>>
>> giveback_first_trb(udev, ep_index, start_cycle, start_trb);
>> +   return 0;
>> +}
>>
>> +/ Bulk and Control transfer methods /
>> +/**
>> + * Queues up the BULK Request
>> + *
>> + * @param udev pointer to the USB device structure
>> + * @param pipe contains the DIR_IN or OUT , devnum
>> + * @param length   length of the buffer
>> + * @param buffer   buffer to be read/written based on the request
>> + * @param nonblock when true do not block waiting for response
>> + * @return returns 0 if successful else -1 on failure
>> + */
>> +int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>> +int length, void *buffer, bool nonblock)
>> +{
>> +   u32 field;
>> +   int ret;
>> +   union xhci_trb *event;
>> +   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
>> +   int ep_index = usb_pipe_ep_index(pipe);
>> +
>> +   if (poll_pend) {
>> +   

[PATCH 0/2][v2] Raspberry pi improvements usb core

[Jason Wessel]

The original patches have been refactored.  It turns out that one of
the problems was a dwc2 specific with a stall after a port reset.
This means the generic retry code was dropped.  The xhci had a similar
problem with a retry required for COMP_TX_ERR, and will be sent to the
xhci maintainer.

The new patches allow for a wider variety of usb keyboards to be used
with u-boot.

Jason Wessel (2):
  usb_kbd: Remove the initial interrupt request
  dwc2: Retry when a usb_stall occurs.

 common/usb_kbd.c| 10 +-
 drivers/usb/host/dwc2.c |  3 +++
 2 files changed, 8 insertions(+), 5 deletions(-)

Re: [RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Jason Wessel]

On 8/18/20 12:20 PM, Marek Vasut wrote:
> On 8/18/20 6:54 PM, Jason Wessel wrote:
>>
>>
>> On 8/18/20 10:05 AM, Marek Vasut wrote:
>>> On 8/18/20 4:34 PM, Jason Wessel wrote:
>>>> After the initial configuration some USB keyboard+mouse devices never
>>>> return any kind of event on the interrupt line.  In particular, the
>>>> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>>>> 3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>>>> never returns a data packet until the first external input event.
>>>>
>>>> I found this was also true with some newer model Dell keyboards.
>>>>
>>>> When the device is plugged into a xhci controller there is also no
>>>> point in waiting 5 seconds for a device that is never going to present
>>>> data, so the call to the interrupt service was changed to a
>>>> nonblocking operation for the controllers that support this.
>>>>
>>>> With the patch applied, the rpi3 and rpi4 work well with the more
>>>> complex keyboard devices.
>>>
>>> Please extend the comment in the code, it is not clear from it or from
>>> the commit message what the problem really is that this patch tries to fix.
>>>
>>> Also, the printf() below the return 1 is never reached.
>>>
>>
>>
>> The printf() is only reached in the case of the #ifdef above it being true. 
> 
> That's pretty awful and confusing code then. The original code wasn't
> stellar either, but can this be somehow improved now ?
> 
>> The compiler in theory should optimize it away, but for clarity it can be 
>> moved 
>> with in the #ifdef but that also requires fixing it in two places because 
>> there are multiple levels to the #ifdef.
> 
> I think it's better to make it more readable if possible.
> 
>> Would the following be acceptable, which I can put in the next version.
>>
>>
>> commit 319c75ee3d1fce8a3389d1de857751504b5110cb (HEAD -> master)
>> Author: Jason Wessel 
>> Date:   Thu Jun 25 05:31:25 2020 -0700
>>
>> usb_kbd: succeed even if no interrupt is pending
>> 
>> After the initial configuration some USB keyboard+mouse devices never
>> return any kind of event on the interrupt line.  In particular, the
>> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>> 3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>> never returns a data packet until the first external input event.
>> 
>> I found this was also true with some newer model Dell keyboards.
>> 
>> Without the patch u-boot prints the following on one of keyboards and
>> leave it unable to accept input events.
>> 
>> =
>> scanning bus xhci_pci for devices...
>>   Failed to get keyboard state from device 14dd:1009
> 
> So I have to wonder, if the keyboard never returns a data packet until
> you press a key (that makes sense), how does Linux deal with this ?
> 


As far as I can tell the usb_kbd_probe() probe function in the Linux kernel 
sets up the configuration and exits right away.  The keyboard drivers state 
was zeroed out in the probe function and the kernel later processes callbacks
from the interrupt handler.  

Does that imply the other 2 code paths should also just return 1 and we get
rid of the printf() entirely? 

I don't have any boards that wanted either of these two paths through code,
so I wasn't inclined to change it.

Jason.

Re: [RESEND][PATCH 2/3] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Jason Wessel]

On 8/18/20 10:08 AM, Marek Vasut wrote:
> On 8/18/20 4:34 PM, Jason Wessel wrote:
>> When resetting the rpi3 board sometimes it will display:
>>  USB device not accepting new address (error=0)
>>
>> After the message appears, the usb keyboard will not work.  It seems
>> that the configuration actually did succeed however.  Checking the
>> device status for a return code of zero and continuing allows the usb
>> keyboard and other usb devices to work function.
>>
>> Signed-off-by: Jason Wessel 
>> ---
>>  common/usb.c | 7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/usb.c b/common/usb.c
>> index aad13fd9c5..0eb5d40a2d 100644
>> --- a/common/usb.c
>> +++ b/common/usb.c
>> @@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device 
>> *dev, int addr, bool do_read,
>>  dev->devnum = addr;
>>  
>>  err = usb_set_address(dev); /* set address */
>> -
>> -if (err < 0) {
>> +if (err < 0)
>> +debug("\n   usb_set_address return < 0\n");
> 
> Please print the return code here.

Good idea.

> 
> Is dev->status always set by usb_set_address() when err < 0 ?

Yes.  The status is filled in as far as I can tell.  I had
received other values when exceeding timing thresholds with
too many printfs() to the serial port while debugging.

The usb hardware hardware devices seem to like their initialization
to be completed in a timely manner.

Jason.

> 
>> +if (err < 0 && dev->status != 0) {
>>  printf("\n  USB device not accepting new address " \
>>  "(error=%lX)\n", dev->status);
>> -return err;
>> +return err;
>>  }
>>  
>>  mdelay(10); /* Let the SET_ADDRESS settle */
>>

Re: [RESEND][PATCH 3/3] usb.c: Add a retry in the usb_prepare_device()

[Jason Wessel]

>From the Raspberry Pi3 - With the patch removed:

U-Boot> setenv usb_pgood_delay 2000
U-Boot> usb reset
resetting USB...
Bus usb@7e98: USB DWC2
scanning bus usb@7e98 for devices... unable to get device descriptor 
(error=-22)
6 USB Device(s) found
   scanning usb for storage devices... 1 Storage Device(s) found


The only way I found to make it work reliably was to have it retry in the 
usb_prepare_device().

Jason.


On 8/18/20 10:08 AM, Marek Vasut wrote:
> On 8/18/20 4:34 PM, Jason Wessel wrote:
>> I have found through testing some USB 2 composite mouse/keyboard
>> devices do not response to the usb_set_address call immediately
>> following the port reset.  It can take anywhere from 2ms to 20ms.
> 
> Does it work if you do
> 
> => setenv usb_pgood_delay 2000
> 
> before your test ?
> 

Re: [RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Jason Wessel]

On 8/18/20 10:05 AM, Marek Vasut wrote:
> On 8/18/20 4:34 PM, Jason Wessel wrote:
>> After the initial configuration some USB keyboard+mouse devices never
>> return any kind of event on the interrupt line.  In particular, the
>> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>> 3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>> never returns a data packet until the first external input event.
>>
>> I found this was also true with some newer model Dell keyboards.
>>
>> When the device is plugged into a xhci controller there is also no
>> point in waiting 5 seconds for a device that is never going to present
>> data, so the call to the interrupt service was changed to a
>> nonblocking operation for the controllers that support this.
>>
>> With the patch applied, the rpi3 and rpi4 work well with the more
>> complex keyboard devices.
> 
> Please extend the comment in the code, it is not clear from it or from
> the commit message what the problem really is that this patch tries to fix.
> 
> Also, the printf() below the return 1 is never reached.
> 


The printf() is only reached in the case of the #ifdef above it being true. 

The compiler in theory should optimize it away, but for clarity it can be moved 
with in the #ifdef but that also requires fixing it in two places because 
there are multiple levels to the #ifdef.

Would the following be acceptable, which I can put in the next version.


commit 319c75ee3d1fce8a3389d1de857751504b5110cb (HEAD -> master)
Author: Jason Wessel 
Date:   Thu Jun 25 05:31:25 2020 -0700

usb_kbd: succeed even if no interrupt is pending

After the initial configuration some USB keyboard+mouse devices never
return any kind of event on the interrupt line.  In particular, the
device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
never returns a data packet until the first external input event.

I found this was also true with some newer model Dell keyboards.

Without the patch u-boot prints the following on one of keyboards and
leave it unable to accept input events.

=
scanning bus xhci_pci for devices...
  Failed to get keyboard state from device 14dd:1009
=
    
With the patch, all families of the Raspberry Pi boards can use this
keyboard device.

Signed-off-by: Jason Wessel 

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844..9ff008d5dc 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -514,18 +514,28 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
  USB_KBD_BOOT_REPORT_SIZE, data->new,
  data->intinterval);
if (!data->intq) {
+   printf("Failed to get keyboard state from device %04x:%04x\n",
+  dev->descriptor.idVendor, dev->descriptor.idProduct);
+   /* Abort, we don't want to use that non-functional keyboard. */
+   return 0;
+   }
 #elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP)
if (usb_get_report(dev, iface->desc.bInterfaceNumber,
   1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) {
-#else
-   if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
-   data->intinterval, false) < 0) {
-#endif
printf("Failed to get keyboard state from device %04x:%04x\n",
   dev->descriptor.idVendor, dev->descriptor.idProduct);
/* Abort, we don't want to use that non-functional keyboard. */
return 0;
}
+#else
+   /*
+* Set poll to true for initial interrupt transfer
+* because some keyboard devices do not return an
+* event until the first keypress.
+*/
+   usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
+   data->intinterval, true);
+#endif
 
/* Success. */
return 1;

[RESEND][PATCH 2/3] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Jason Wessel]

When resetting the rpi3 board sometimes it will display:
 USB device not accepting new address (error=0)

After the message appears, the usb keyboard will not work.  It seems
that the configuration actually did succeed however.  Checking the
device status for a return code of zero and continuing allows the usb
keyboard and other usb devices to work function.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c5..0eb5d40a2d 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
-
-   if (err < 0) {
+   if (err < 0)
+   debug("\n   usb_set_address return < 0\n");
+   if (err < 0 && dev->status != 0) {
printf("\n  USB device not accepting new address " \
"(error=%lX)\n", dev->status);
-   return err;
+   return err;
}
 
mdelay(10); /* Let the SET_ADDRESS settle */
-- 
2.17.1

[RESEND][PATCH 3/3] usb.c: Add a retry in the usb_prepare_device()

[Jason Wessel]

I have found through testing some USB 2 composite mouse/keyboard
devices do not response to the usb_set_address call immediately
following the port reset.  It can take anywhere from 2ms to 20ms.

This patch adds a retry and delay for usb_prepare_device() and allows
all the USB keyboards I tried to function properly.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 0eb5d40a2d..39bae86a11 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1032,6 +1032,7 @@ static int usb_prepare_device(struct usb_device *dev, int 
addr, bool do_read,
  struct usb_device *parent)
 {
int err;
+   int retry_msec = 0;
 
/*
 * Allocate usb 3.0 device context.
@@ -1054,6 +1055,14 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
+   /* Retry for old composite keyboard/mouse usb2 hardware */
+   while (err < 0 && retry_msec <= 40) {
+   retry_msec += 20;
+   mdelay(20);
+   err = usb_set_address(dev); /* set address */
+   }
+   if (retry_msec > 0)
+   debug("usb_set_address delay: %i\n", retry_msec);
if (err < 0)
debug("\n   usb_set_address return < 0\n");
if (err < 0 && dev->status != 0) {
-- 
2.17.1

[RESEND][PATCH 0/3] Raspberry pi improvements usb core

[Jason Wessel]

These patches are now in use by other developers, and I would like to
get them into the upstream u-boot.  This 3 patches were part of a 9
patch series posted in July prior to the merge window closure.

-- Travis CI checks https://github.com/u-boot/u-boot/pull/35 --

Pull Request #35 Rpi master

The commit 57805f2270c4 ("net: bcmgenet: Don't set ID_MODE_DIS when
not using RGMII") needed to be extended for the case of using the
rgmii-rxid. The latest version of the Rasbperry Pi4 dtb files for the
5.4 now specify the rgmii-rxid.

Signed-off-by: Jason Wessel 

Commit 9a21770 #35: Rpi master Branch master 

-- End Travis CI checks --


Jason Wessel (3):
  usb_kbd: succeed even if no interrupt is pending
  common/usb.c: Work around keyboard reporting "USB device not accepting 
new address"
  usb.c: Add a retry in the usb_prepare_device()

 common/usb.c | 16 +---
 common/usb_kbd.c |  4 +++-
 2 files changed, 16 insertions(+), 4 deletions(-)

[RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Jason Wessel]

After the initial configuration some USB keyboard+mouse devices never
return any kind of event on the interrupt line.  In particular, the
device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
never returns a data packet until the first external input event.

I found this was also true with some newer model Dell keyboards.

When the device is plugged into a xhci controller there is also no
point in waiting 5 seconds for a device that is never going to present
data, so the call to the interrupt service was changed to a
nonblocking operation for the controllers that support this.

With the patch applied, the rpi3 and rpi4 work well with the more
complex keyboard devices.

Signed-off-by: Jason Wessel 
---
 common/usb_kbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844..3c0056e1b9 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -519,7 +519,9 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
   1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) {
 #else
if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
-   data->intinterval, false) < 0) {
+   data->intinterval, true) < 0) {
+   /* Read first packet if the device provides it, else pick it up 
later */
+   return 1;
 #endif
printf("Failed to get keyboard state from device %04x:%04x\n",
   dev->descriptor.idVendor, dev->descriptor.idProduct);
-- 
2.17.1

[PATCH 8/9] bcmgenet: fix DMA buffer management

[Jason Wessel]

This commit fixes a serious issue occurring when several network
commands are run on a raspberry pi 4 board: for instance a "dhcp"
command and then one or several "tftp" commands. In this case,
packet recv callbacks were called several times on the same packets,
and send function was failing most of the time.

note: if the boot procedure is made of a single network
command, the issue is not visible.

The issue is related to management of the packet ring buffers
(producer / consumer) and DMA.
Each time a packet is received, the ethernet device stores it
in the buffer and increments an index called RDMA_PROD_INDEX.
Each time the driver outputs a received packet, it increments
another index called RDMA_CONS_INDEX.

Between each pair of network commands, as part of the driver
'start' function, previous code tried to reset both RDMA_CONS_INDEX
and RDMA_PROD_INDEX to 0. But RDMA_PROD_INDEX cannot be written from
driver side, thus its value was actually not updated, and only
RDMA_CONS_INDEX was reset to 0. This was resulting in a major
synchronization issue between the driver and the device. Most
visible behavior was that the driver seemed to receive again the
packets from the previous commands (e.g. DHCP response packets
"received" again when performing the first TFTP command).

This fix consists in setting RDMA_CONS_INDEX to the same
value as RDMA_PROD_INDEX, when resetting the driver.

The same kind of fix was needed on the TX side, and a few variables
had to be reset accordingly (c_index, tx_index, rx_index).

The rx_index and tx_index have only 256 entries so the bottom 8 bits
must be masked off.

Originated-by: Etienne Dublé 
Signed-off-by: Jason Wessel 
---
 drivers/net/bcmgenet.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
index 11b6148ab6..1b7e7ba2bf 100644
--- a/drivers/net/bcmgenet.c
+++ b/drivers/net/bcmgenet.c
@@ -378,8 +378,6 @@ static void rx_descs_init(struct bcmgenet_eth_priv *priv)
u32 len_stat, i;
void *desc_base = priv->rx_desc_base;
 
-   priv->c_index = 0;
-
len_stat = (RX_BUF_LENGTH << DMA_BUFLENGTH_SHIFT) | DMA_OWN;
 
for (i = 0; i < RX_DESCS; i++) {
@@ -403,8 +401,11 @@ static void rx_ring_init(struct bcmgenet_eth_priv *priv)
writel(RX_DESCS * DMA_DESC_SIZE / 4 - 1,
   priv->mac_reg + RDMA_RING_REG_BASE + DMA_END_ADDR);
 
-   writel(0x0, priv->mac_reg + RDMA_PROD_INDEX);
-   writel(0x0, priv->mac_reg + RDMA_CONS_INDEX);
+   /* cannot init RDMA_PROD_INDEX to 0, so align RDMA_CONS_INDEX on it 
instead */
+   priv->c_index = readl(priv->mac_reg + RDMA_PROD_INDEX);
+   writel(priv->c_index, priv->mac_reg + RDMA_CONS_INDEX);
+   priv->rx_index = priv->c_index;
+   priv->rx_index &= 0xFF;
writel((RX_DESCS << DMA_RING_SIZE_SHIFT) | RX_BUF_LENGTH,
   priv->mac_reg + RDMA_RING_REG_BASE + DMA_RING_BUF_SIZE);
writel(DMA_FC_THRESH_VALUE, priv->mac_reg + RDMA_XON_XOFF_THRESH);
@@ -421,8 +422,10 @@ static void tx_ring_init(struct bcmgenet_eth_priv *priv)
writel(0x0, priv->mac_reg + TDMA_WRITE_PTR);
writel(TX_DESCS * DMA_DESC_SIZE / 4 - 1,
   priv->mac_reg + TDMA_RING_REG_BASE + DMA_END_ADDR);
-   writel(0x0, priv->mac_reg + TDMA_PROD_INDEX);
-   writel(0x0, priv->mac_reg + TDMA_CONS_INDEX);
+   /* cannot init TDMA_CONS_INDEX to 0, so align TDMA_PROD_INDEX on it 
instead */
+   priv->tx_index = readl(priv->mac_reg + TDMA_CONS_INDEX);
+   writel(priv->tx_index, priv->mac_reg + TDMA_PROD_INDEX);
+   priv->tx_index &= 0xFF;
writel(0x1, priv->mac_reg + TDMA_RING_REG_BASE + DMA_MBUF_DONE_THRESH);
writel(0x0, priv->mac_reg + TDMA_FLOW_PERIOD);
writel((TX_DESCS << DMA_RING_SIZE_SHIFT) | RX_BUF_LENGTH,
@@ -469,8 +472,6 @@ static int bcmgenet_gmac_eth_start(struct udevice *dev)
 
priv->tx_desc_base = priv->mac_reg + GENET_TX_OFF;
priv->rx_desc_base = priv->mac_reg + GENET_RX_OFF;
-   priv->tx_index = 0x0;
-   priv->rx_index = 0x0;
 
bcmgenet_umac_reset(priv);
 
-- 
2.17.1

[PATCH 1/9] fs/fat/fat.c: Do not perform zero block reads if there are no blocks left

[Jason Wessel]

While using u-boot with qemu's virtio driver I stumbled across a
problem reading files less than sector size.  On the real hardware the
block reader seems ok with reading zero blocks, and while we could fix
the virtio host side of qemu to deal with a zero block read instead of
crashing, the u-boot fat driver should not be doing zero block reads
in the first place.  If you ask hardware to read zero blocks you are
just going to get zero data.  There may also be other hardware that
responds similarly to the virtio interface so this is worth fixing.

Without the patch I get the following and have to restart qemu because
it dies.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
qemu-system-aarch64: virtio: zero sized buffers are not allowed
-

With the patch I get the expected results.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
30 bytes read in 11 ms (2 KiB/s)
=> md.b ${loadaddr} 0x1E
4008: 64 77 63 5f 6f 74 67 2e 6c 70 6d 5f 65 6e 61 62dwc_otg.lpm_enab
40080010: 6c 65 3d 30 20 72 6f 6f 74 77 61 69 74 0a  le=0 rootwait.

-

Signed-off-by: Jason Wessel 
Reviewed-by: Tom Rini 
---
 fs/fat/fat.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 9578b74bae..28aa5aaa9f 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -278,7 +278,10 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, 
unsigned long size)
}
} else {
idx = size / mydata->sect_size;
-   ret = disk_read(startsect, idx, buffer);
+   if (idx == 0)
+   ret = 0;
+   else
+   ret = disk_read(startsect, idx, buffer);
if (ret != idx) {
debug("Error reading data (got %d)\n", ret);
return -1;
-- 
2.17.1

[PULL][PATCH 0/9] Raspberry pi improvements qemu/usb/ethernet

[Jason Wessel]

I would like to get these patches merged for the next release, but
there has been no movement on some of them in over a month.  I would
fix any problems with the patches, but I am not aware of any.  The
patch set has been tested on all the generations of the raspberry pi
hardware and qemu and passed the travis ci checks.

-- Travis CI checks --

Pull Request #35 Rpi master

The commit 57805f2270c4 ("net: bcmgenet: Don't set ID_MODE_DIS when
not using RGMII") needed to be extended for the case of using the
rgmii-rxid. The latest version of the Rasbperry Pi4 dtb files for the
5.4 now specify the rgmii-rxid.

Signed-off-by: Jason Wessel 

Commit 9a21770 #35: Rpi master Branch master 

-- End Travis CI checks --

Prior to this patch set, I had just a small number of USB keyboards
function properly with the Rasbpberry Pi boards.  Nearly every
composite USB keyboard/mouse I had access to including the Raritan
IP/KVM devices would not initialize properly.

v1-consoldiated:
  - Consolidated all the usb and ethernet patches into a single set
  - No changes to any of the patches

v1:
  - Testing completed against all generations of raspberry pi boards
  - check patch warnings add errors have been fixed

rfc v3:
  - Add in patch 6
  - Finally got the GearHead keyboard + mouse composite USB device working

rfc v2: 
  - Minor cleanups to patches 1-3 based on prior review
  - Patch 4 & 5 are new to fix various xhchi crashes
while having additional devices plugged in along with
booting off the usb port.  The nonblocking mode turned
out to be somewhat complex.

rfc v1:

The original purpose of the patch set was to fix problems with the USB
keyboard support with the rpi4.  The generic xhci code lacks a non
blocking call to read an event.  This causes major problems for the
sleep function as well as for ethernet downloads where a keyboard poll
for interrupting the download blocks for 5 seconds.


The following changes since commit fee68b98fe3890631a9bdf8f8db328179011ee3f:

  Merge tag 'efi-2020-10-rc1-4' of 
https://gitlab.denx.de/u-boot/custodians/u-boot-efi (2020-07-16 16:35:15 -0400)

are available in the Git repository at:

  https://github.com/jwessel/u-boot 

for you to fetch changes up to fb5b89c4f2aa2a95c4024b156099c838d1199bd0:

  bcmgenet: Add support for rgmii-rxid (2020-07-17 06:35:11 -0700)

--------
Jason Wessel (9):
  fs/fat/fat.c: Do not perform zero block reads if there are no blocks left
  xhci: Add polling support for USB keyboards
  usb_kbd: succeed even if no interrupt is pending
  common/usb.c: Work around keyboard reporting "USB device not accepting 
new address"
  xhci-ring.c: Add the poll_pend state to properly abort transactions
  xhci-ring: Fix crash when issuing "usb reset"
  usb.c: Add a retry in the usb_prepare_device()
  bcmgenet: fix DMA buffer management
  bcmgenet: Add support for rgmii-rxid

 common/usb.c |  16 --
 common/usb_kbd.c |   4 +-
 drivers/net/bcmgenet.c   |  20 +++
 drivers/usb/host/xhci-ring.c | 123 +--
 drivers/usb/host/xhci.c  |  11 ++--
 fs/fat/fat.c |   5 +-
 include/usb/xhci.h   |   5 +-
 7 files changed, 136 insertions(+), 48 deletions(-)

[PATCH 7/9] usb.c: Add a retry in the usb_prepare_device()

[Jason Wessel]

I have found through testing some USB 2 composite mouse/keyboard
devices do not response to the usb_set_address call immediately
following the port reset.  It can take anywhere from 2ms to 20ms.

This patch adds a retry and delay for usb_prepare_device() and allows
all the USB keyboards I tried to function properly.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 0eb5d40a2d..39bae86a11 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1032,6 +1032,7 @@ static int usb_prepare_device(struct usb_device *dev, int 
addr, bool do_read,
  struct usb_device *parent)
 {
int err;
+   int retry_msec = 0;
 
/*
 * Allocate usb 3.0 device context.
@@ -1054,6 +1055,14 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
+   /* Retry for old composite keyboard/mouse usb2 hardware */
+   while (err < 0 && retry_msec <= 40) {
+   retry_msec += 20;
+   mdelay(20);
+   err = usb_set_address(dev); /* set address */
+   }
+   if (retry_msec > 0)
+   debug("usb_set_address delay: %i\n", retry_msec);
if (err < 0)
debug("\n   usb_set_address return < 0\n");
if (err < 0 && dev->status != 0) {
-- 
2.17.1

[PATCH 2/9] xhci: Add polling support for USB keyboards

[Jason Wessel]

The xhci driver was causing intermittent 5 second delays from the USB
keyboard polling hook.  Executing something like a "sleep 1" for
example would sleep for 5 seconds, unless an event occurred on
the USB bus to shorten the delay.

Modeled after the code in the DWC2 driver, a nonblock state was added
to quickly return instead of blocking for up to 5 seconds waiting for
an event before timing out.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 26 +-
 drivers/usb/host/xhci.c  | 11 ++-
 include/usb/xhci.h   |  5 +++--
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 86aeaab412..b7b2e16410 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -432,9 +432,11 @@ static int event_ready(struct xhci_ctrl *ctrl)
  *
  * @param ctrl Host controller data structure
  * @param expected TRB type expected from Event TRB
+ * @param nonblock when true do not block waiting for response
  * @return pointer to event trb
  */
-union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected)
+union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
+   bool nonblock)
 {
trb_type type;
unsigned long ts = get_timer(0);
@@ -442,8 +444,11 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl 
*ctrl, trb_type expected)
do {
union xhci_trb *event = ctrl->event_ring->dequeue;
 
-   if (!event_ready(ctrl))
+   if (!event_ready(ctrl)) {
+   if (nonblock)
+   return NULL;
continue;
+   }
 
type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags));
if (type == expected)
@@ -493,7 +498,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
field = le32_to_cpu(event->trans_event.flags);
BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
@@ -501,7 +506,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
!= COMP_STOP)));
xhci_acknowledge_event(ctrl);
 
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -509,7 +514,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, (void *)((uintptr_t)ring->enqueue |
ring->cycle_state), udev->slot_id, ep_index, TRB_SET_DEQ);
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -552,10 +557,11 @@ static void record_transfer_result(struct usb_device 
*udev,
  * @param pipe contains the DIR_IN or OUT , devnum
  * @param length   length of the buffer
  * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
  * @return returns 0 if successful else -1 on failure
  */
 int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer)
+   int length, void *buffer, bool nonblock)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -714,8 +720,10 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
+   if (nonblock)
+   return -EINVAL;
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeout */
@@ -911,7 +919,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
if (!event)
goto abort;
field = le32_to_cpu(event->tra

[PATCH 5/9] xhci-ring.c: Add the poll_pend state to properly abort transactions

[Jason Wessel]

xhci_trl_tx and xhchi_bulk_tx can be called synchronously by other
drivers such as the usb storage or network, while the keyboard driver
exclusively uses the polling mode.

And pending polling transactions must be aborted before switching
modes to avoid corrupting the state of the controller.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 86 +---
 1 file changed, 70 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b7b2e16410..d6339f4f0a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -24,6 +24,12 @@
 
 #include 
 
+static void *last_bulk_tx_buf;
+static struct usb_device *poll_last_udev;
+int poll_last_ep_index;
+static unsigned long bulk_tx_poll_ts;
+static bool poll_pend;
+
 /**
  * Is this TRB a link TRB or was the last TRB the last TRB in this event ring
  * segment?  I.e. would the updated event TRB pointer step off the end of the
@@ -549,19 +555,8 @@ static void record_transfer_result(struct usb_device *udev,
}
 }
 
-/ Bulk and Control transfer methods /
-/**
- * Queues up the BULK Request
- *
- * @param udev pointer to the USB device structure
- * @param pipe contains the DIR_IN or OUT , devnum
- * @param length   length of the buffer
- * @param buffer   buffer to be read/written based on the request
- * @param nonblock when true do not block waiting for response
- * @return returns 0 if successful else -1 on failure
- */
-int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer, bool nonblock)
+static int _xhci_bulk_tx_queue(struct usb_device *udev, unsigned long pipe,
+  int length, void *buffer)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -575,7 +570,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
struct xhci_virt_device *virt_dev;
struct xhci_ep_ctx *ep_ctx;
struct xhci_ring *ring; /* EP transfer ring */
-   union xhci_trb *event;
 
int running_total, trb_buff_len;
unsigned int total_packet_count;
@@ -719,20 +713,73 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
} while (running_total < length);
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
+   return 0;
+}
 
+/ Bulk and Control transfer methods /
+/**
+ * Queues up the BULK Request
+ *
+ * @param udev pointer to the USB device structure
+ * @param pipe contains the DIR_IN or OUT , devnum
+ * @param length   length of the buffer
+ * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
+ * @return returns 0 if successful else -1 on failure
+ */
+int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
+int length, void *buffer, bool nonblock)
+{
+   u32 field;
+   int ret;
+   union xhci_trb *event;
+   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
+   int ep_index = usb_pipe_ep_index(pipe);
+
+   if (poll_pend) {
+   /*
+* Abort a pending poll operation if it should have
+* timed out, or if this is a different buffer from a
+* separate request
+*/
+   if (get_timer(bulk_tx_poll_ts) > XHCI_TIMEOUT ||
+   last_bulk_tx_buf != buffer || poll_last_udev != udev ||
+   ep_index != poll_last_ep_index) {
+   abort_td(poll_last_udev, poll_last_ep_index);
+   poll_last_udev->status = USB_ST_NAK_REC;  /* closest 
thing to a timeout */
+   poll_last_udev->act_len = 0;
+   poll_pend = false;
+   }
+   } /* No else here because poll_pend might have changed above */
+   if (!poll_pend) {
+   last_bulk_tx_buf = buffer;
+   ret = _xhci_bulk_tx_queue(udev, pipe, length, buffer);
+   if (ret)
+   return ret;
+   }
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
-   if (nonblock)
+   if (nonblock) {
+   if (!poll_pend) {
+   /* Start the timer */
+   bulk_tx_poll_ts = get_timer(0);
+   poll_last_udev = udev;
+   poll_last_ep_index = ep_index;
+   poll_pend = true;
+   }
return -EINVAL;
+   }
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeout */
udev->act_len = 

[PATCH 6/9] xhci-ring: Fix crash when issuing "usb reset"

[Jason Wessel]

If a "usb reset" is issued when the poll_pend state is set the
abort_td() function will hit one of the BUG() statements in abort_td()
or the BUG() statement at the end of xhci_wait_for_event().

The controller has been reset, so the rest of the cleanup should be
skipped and poll_pend flag should be cleared.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d6339f4f0a..f2a07204cd 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -483,6 +483,8 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, 
trb_type expected,
if (expected == TRB_TRANSFER)
return NULL;
 
+   if (poll_pend)
+   return NULL;
printf("XHCI timeout on event type %d... cannot recover.\n", expected);
BUG();
 }
@@ -505,11 +507,16 @@ static void abort_td(struct usb_device *udev, int 
ep_index)
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
-   field = le32_to_cpu(event->trans_event.flags);
-   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
-   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
-   != COMP_STOP)));
+   if (event) {
+   field = le32_to_cpu(event->trans_event.flags);
+   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
+   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
+   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
+!= COMP_STOP)));
+   } else {
+   debug("XHCI abort timeout\n");
+   return;
+   }
xhci_acknowledge_event(ctrl);
 
event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
-- 
2.17.1

[PATCH 9/9] bcmgenet: Add support for rgmii-rxid

[Jason Wessel]

The commit 57805f2270c4 ("net: bcmgenet: Don't set ID_MODE_DIS when
not using RGMII") needed to be extended for the case of using the
rgmii-rxid.  The latest version of the Rasbperry Pi4 dtb files for the
5.4 now specify the rgmii-rxid.

Signed-off-by: Jason Wessel 
---
 drivers/net/bcmgenet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
index 1b7e7ba2bf..ace1331362 100644
--- a/drivers/net/bcmgenet.c
+++ b/drivers/net/bcmgenet.c
@@ -457,7 +457,8 @@ static int bcmgenet_adjust_link(struct bcmgenet_eth_priv 
*priv)
clrsetbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, OOB_DISABLE,
RGMII_LINK | RGMII_MODE_EN);
 
-   if (phy_dev->interface == PHY_INTERFACE_MODE_RGMII)
+   if (phy_dev->interface == PHY_INTERFACE_MODE_RGMII ||
+   phy_dev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
setbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, ID_MODE_DIS);
 
writel(speed << CMD_SPEED_SHIFT, (priv->mac_reg + UMAC_CMD));
-- 
2.17.1

[PATCH 3/9] usb_kbd: succeed even if no interrupt is pending

[Jason Wessel]

After the initial configuration some USB keyboard+mouse devices never
return any kind of event on the interrupt line.  In particular, the
device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
never returns a data packet until the first external input event.

I found this was also true with some newer model Dell keyboards.

When the device is plugged into a xhci controller there is also no
point in waiting 5 seconds for a device that is never going to present
data, so the call to the interrupt service was changed to a
nonblocking operation for the controllers that support this.

With the patch applied, the rpi3 and rpi4 work well with the more
complex keyboard devices.

Signed-off-by: Jason Wessel 
---
 common/usb_kbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844..3c0056e1b9 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -519,7 +519,9 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
   1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) {
 #else
if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
-   data->intinterval, false) < 0) {
+   data->intinterval, true) < 0) {
+   /* Read first packet if the device provides it, else pick it up 
later */
+   return 1;
 #endif
printf("Failed to get keyboard state from device %04x:%04x\n",
   dev->descriptor.idVendor, dev->descriptor.idProduct);
-- 
2.17.1

[PATCH 4/9] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Jason Wessel]

When resetting the rpi3 board sometimes it will display:
 USB device not accepting new address (error=0)

After the message appears, the usb keyboard will not work.  It seems
that the configuration actually did succeed however.  Checking the
device status for a return code of zero and continuing allows the usb
keyboard and other usb devices to work function.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c5..0eb5d40a2d 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
-
-   if (err < 0) {
+   if (err < 0)
+   debug("\n   usb_set_address return < 0\n");
+   if (err < 0 && dev->status != 0) {
printf("\n  USB device not accepting new address " \
"(error=%lX)\n", dev->status);
-   return err;
+   return err;
}
 
mdelay(10); /* Let the SET_ADDRESS settle */
-- 
2.17.1

[PATCH 1/2] bcmgenet: fix DMA buffer management

[Jason Wessel]

This commit fixes a serious issue occurring when several network
commands are run on a raspberry pi 4 board: for instance a "dhcp"
command and then one or several "tftp" commands. In this case,
packet recv callbacks were called several times on the same packets,
and send function was failing most of the time.

note: if the boot procedure is made of a single network
command, the issue is not visible.

The issue is related to management of the packet ring buffers
(producer / consumer) and DMA.
Each time a packet is received, the ethernet device stores it
in the buffer and increments an index called RDMA_PROD_INDEX.
Each time the driver outputs a received packet, it increments
another index called RDMA_CONS_INDEX.

Between each pair of network commands, as part of the driver
'start' function, previous code tried to reset both RDMA_CONS_INDEX
and RDMA_PROD_INDEX to 0. But RDMA_PROD_INDEX cannot be written from
driver side, thus its value was actually not updated, and only
RDMA_CONS_INDEX was reset to 0. This was resulting in a major
synchronization issue between the driver and the device. Most
visible behavior was that the driver seemed to receive again the
packets from the previous commands (e.g. DHCP response packets
"received" again when performing the first TFTP command).

This fix consists in setting RDMA_CONS_INDEX to the same
value as RDMA_PROD_INDEX, when resetting the driver.

The same kind of fix was needed on the TX side, and a few variables
had to be reset accordingly (c_index, tx_index, rx_index).

The rx_index and tx_index have only 256 entries so the bottom 8 bits
must be masked off.

Originated-by: Etienne Dublé 
Signed-off-by: Jason Wessel 
---
 drivers/net/bcmgenet.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
index 11b6148ab6..1b7e7ba2bf 100644
--- a/drivers/net/bcmgenet.c
+++ b/drivers/net/bcmgenet.c
@@ -378,8 +378,6 @@ static void rx_descs_init(struct bcmgenet_eth_priv *priv)
u32 len_stat, i;
void *desc_base = priv->rx_desc_base;
 
-   priv->c_index = 0;
-
len_stat = (RX_BUF_LENGTH << DMA_BUFLENGTH_SHIFT) | DMA_OWN;
 
for (i = 0; i < RX_DESCS; i++) {
@@ -403,8 +401,11 @@ static void rx_ring_init(struct bcmgenet_eth_priv *priv)
writel(RX_DESCS * DMA_DESC_SIZE / 4 - 1,
   priv->mac_reg + RDMA_RING_REG_BASE + DMA_END_ADDR);
 
-   writel(0x0, priv->mac_reg + RDMA_PROD_INDEX);
-   writel(0x0, priv->mac_reg + RDMA_CONS_INDEX);
+   /* cannot init RDMA_PROD_INDEX to 0, so align RDMA_CONS_INDEX on it 
instead */
+   priv->c_index = readl(priv->mac_reg + RDMA_PROD_INDEX);
+   writel(priv->c_index, priv->mac_reg + RDMA_CONS_INDEX);
+   priv->rx_index = priv->c_index;
+   priv->rx_index &= 0xFF;
writel((RX_DESCS << DMA_RING_SIZE_SHIFT) | RX_BUF_LENGTH,
   priv->mac_reg + RDMA_RING_REG_BASE + DMA_RING_BUF_SIZE);
writel(DMA_FC_THRESH_VALUE, priv->mac_reg + RDMA_XON_XOFF_THRESH);
@@ -421,8 +422,10 @@ static void tx_ring_init(struct bcmgenet_eth_priv *priv)
writel(0x0, priv->mac_reg + TDMA_WRITE_PTR);
writel(TX_DESCS * DMA_DESC_SIZE / 4 - 1,
   priv->mac_reg + TDMA_RING_REG_BASE + DMA_END_ADDR);
-   writel(0x0, priv->mac_reg + TDMA_PROD_INDEX);
-   writel(0x0, priv->mac_reg + TDMA_CONS_INDEX);
+   /* cannot init TDMA_CONS_INDEX to 0, so align TDMA_PROD_INDEX on it 
instead */
+   priv->tx_index = readl(priv->mac_reg + TDMA_CONS_INDEX);
+   writel(priv->tx_index, priv->mac_reg + TDMA_PROD_INDEX);
+   priv->tx_index &= 0xFF;
writel(0x1, priv->mac_reg + TDMA_RING_REG_BASE + DMA_MBUF_DONE_THRESH);
writel(0x0, priv->mac_reg + TDMA_FLOW_PERIOD);
writel((TX_DESCS << DMA_RING_SIZE_SHIFT) | RX_BUF_LENGTH,
@@ -469,8 +472,6 @@ static int bcmgenet_gmac_eth_start(struct udevice *dev)
 
priv->tx_desc_base = priv->mac_reg + GENET_TX_OFF;
priv->rx_desc_base = priv->mac_reg + GENET_RX_OFF;
-   priv->tx_index = 0x0;
-   priv->rx_index = 0x0;
 
bcmgenet_umac_reset(priv);
 
-- 
2.17.1

[PATCH 2/2] bcmgenet: Add support for rgmii-rxid

[Jason Wessel]

The commit 57805f2270c4 ("net: bcmgenet: Don't set ID_MODE_DIS when
not using RGMII") needed to be extended for the case of using the
rgmii-rxid.  The latest version of the Rasbperry Pi4 dtb files for the
5.4 now specify the rgmii-rxid.

Signed-off-by: Jason Wessel 
---
 drivers/net/bcmgenet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
index 1b7e7ba2bf..ace1331362 100644
--- a/drivers/net/bcmgenet.c
+++ b/drivers/net/bcmgenet.c
@@ -457,7 +457,8 @@ static int bcmgenet_adjust_link(struct bcmgenet_eth_priv 
*priv)
clrsetbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, OOB_DISABLE,
RGMII_LINK | RGMII_MODE_EN);
 
-   if (phy_dev->interface == PHY_INTERFACE_MODE_RGMII)
+   if (phy_dev->interface == PHY_INTERFACE_MODE_RGMII ||
+   phy_dev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
setbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, ID_MODE_DIS);
 
writel(speed << CMD_SPEED_SHIFT, (priv->mac_reg + UMAC_CMD));
-- 
2.17.1

Re: [PATCH] bcmgenet: fix DMA buffer management

[Jason Wessel]

On 7/16/20 11:02 AM, Jason Wessel wrote:
> 
> 
> On 7/16/20 7:02 AM, Jason Wessel wrote:
>> On 7/9/20 3:11 AM, etienne.du...@gmail.com wrote:
>>> From: Etienne Dublé 
>>>
>>> This commit fixes a serious issue occuring when several network
>>> commands are run on a raspberry pi 4 board: for instance a "dhcp"
>>> command and then one or several "tftp" commands. In this case,
>>> packet recv callbacks were called several times on the same packets,
>>> and send function was failing most of the time.
>>>
>>> note: if the boot procedure is made of a single network
>>> command, the issue is not visible.
>>>
>>> The issue is related to management of the packet ring buffers
>>> (producer / consumer) and DMA.
>>> Each time a packet is received, the ethernet device stores it
>>> in the buffer and increments an index called RDMA_PROD_INDEX.
>>> Each time the driver outputs a received packet, it increments
>>> another index called RDMA_CONS_INDEX.
>>>
>>> Between each pair of network commands, as part of the driver
>>> 'start' function, previous code tried to reset both RDMA_CONS_INDEX
>>> and RDMA_PROD_INDEX to 0. But RDMA_PROD_INDEX cannot be written from
>>> driver side, thus its value was actually not updated, and only
>>> RDMA_CONS_INDEX was reset to 0. This was resulting in a major
>>> synchronization issue between the driver and the device. Most
>>> visible bahavior was that the driver seemed to receive again the
>>> packets from the previous commands (e.g. DHCP response packets
>>> "received" again when performing the first TFTP command).
>>>
>>> This fix consists in setting RDMA_CONS_INDEX to the same
>>> value as RDMA_PROD_INDEX, when resetting the driver.
>>>
>>> The same kind of fix was needed on the TX side, and a few variables
>>> had to be reset accordingly (c_index, tx_index, rx_index).
>>
>>
>> While there is some kind of problem with the driver, because I too
>> have observed a problem with multiple requests timing out or failing,
>> this patch makes the problem much worse.  I was only able to complete
>> a single tftp request. 
>>
>> In my case I am using a static IP address and serverip. 
>>
>> Also your patch was missing the sign-off line.  Please consider
>> running your patches through scripts/checkpatch.pl.
>>
>> Cheers,
>> Jason.
>>
>>> ---
>>>  drivers/net/bcmgenet.c | 15 +++
>>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
>>> index 11b6148ab6..a4facfd63f 100644
>>> --- a/drivers/net/bcmgenet.c
>>> +++ b/drivers/net/bcmgenet.c
>>> @@ -378,8 +378,6 @@ static void rx_descs_init(struct bcmgenet_eth_priv 
>>> *priv)
>>> u32 len_stat, i;
>>> void *desc_base = priv->rx_desc_base;
>>>  
>>> -   priv->c_index = 0;
>>> -
>>> len_stat = (RX_BUF_LENGTH << DMA_BUFLENGTH_SHIFT) | DMA_OWN;
>>>  
>>> for (i = 0; i < RX_DESCS; i++) {
>>> @@ -403,8 +401,10 @@ static void rx_ring_init(struct bcmgenet_eth_priv 
>>> *priv)
>>> writel(RX_DESCS * DMA_DESC_SIZE / 4 - 1,
>>>priv->mac_reg + RDMA_RING_REG_BASE + DMA_END_ADDR);
>>>  
>>> -   writel(0x0, priv->mac_reg + RDMA_PROD_INDEX);
>>> -   writel(0x0, priv->mac_reg + RDMA_CONS_INDEX);
>>> +   /* cannot init RDMA_PROD_INDEX to 0, so align RDMA_CONS_INDEX on it 
>>> instead */
>>> +   priv->c_index = readl(priv->mac_reg + RDMA_PROD_INDEX);
>>> +   writel(priv->c_index, priv->mac_reg + RDMA_CONS_INDEX);
>>> +   priv->rx_index = priv->c_index;
> 
> 
>   printf("before RX_IDX: 0x%x\n", priv->rx_index);
> 
> I added a printf() like above for the RX and TX to see what is going on when 
> I try and transfer a kernel Image file the second time.
> 
> 
> U-Boot> tftp ${loadaddr} bootfs/Image
> before RX_IDX: 0x0
> before TX_IDX: 0x0
> Using ethernet@7d58 device
> Filename 'bootfs/Image'.
> Load address: 0x8
> Loading: ## Warning: gatewayip needed but not set
> ##  16.8 MiB
>  6.1 MiB/s
> done
> Bytes transferred = 17615360 (10cca00 hex)
> U-Boot> tftp ${loadaddr} bootfs/Image
> before RX_IDX: 0xe4
> before TX_IDX: 0x2ee3
> Using ethernet@7d58 device
> Fil

Re: [PATCH] bcmgenet: fix DMA buffer management

[Jason Wessel]

On 7/16/20 7:02 AM, Jason Wessel wrote:
> On 7/9/20 3:11 AM, etienne.du...@gmail.com wrote:
>> From: Etienne Dublé 
>>
>> This commit fixes a serious issue occuring when several network
>> commands are run on a raspberry pi 4 board: for instance a "dhcp"
>> command and then one or several "tftp" commands. In this case,
>> packet recv callbacks were called several times on the same packets,
>> and send function was failing most of the time.
>>
>> note: if the boot procedure is made of a single network
>> command, the issue is not visible.
>>
>> The issue is related to management of the packet ring buffers
>> (producer / consumer) and DMA.
>> Each time a packet is received, the ethernet device stores it
>> in the buffer and increments an index called RDMA_PROD_INDEX.
>> Each time the driver outputs a received packet, it increments
>> another index called RDMA_CONS_INDEX.
>>
>> Between each pair of network commands, as part of the driver
>> 'start' function, previous code tried to reset both RDMA_CONS_INDEX
>> and RDMA_PROD_INDEX to 0. But RDMA_PROD_INDEX cannot be written from
>> driver side, thus its value was actually not updated, and only
>> RDMA_CONS_INDEX was reset to 0. This was resulting in a major
>> synchronization issue between the driver and the device. Most
>> visible bahavior was that the driver seemed to receive again the
>> packets from the previous commands (e.g. DHCP response packets
>> "received" again when performing the first TFTP command).
>>
>> This fix consists in setting RDMA_CONS_INDEX to the same
>> value as RDMA_PROD_INDEX, when resetting the driver.
>>
>> The same kind of fix was needed on the TX side, and a few variables
>> had to be reset accordingly (c_index, tx_index, rx_index).
> 
> 
> While there is some kind of problem with the driver, because I too
> have observed a problem with multiple requests timing out or failing,
> this patch makes the problem much worse.  I was only able to complete
> a single tftp request. 
> 
> In my case I am using a static IP address and serverip. 
> 
> Also your patch was missing the sign-off line.  Please consider
> running your patches through scripts/checkpatch.pl.
> 
> Cheers,
> Jason.
> 
>> ---
>>  drivers/net/bcmgenet.c | 15 +++
>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
>> index 11b6148ab6..a4facfd63f 100644
>> --- a/drivers/net/bcmgenet.c
>> +++ b/drivers/net/bcmgenet.c
>> @@ -378,8 +378,6 @@ static void rx_descs_init(struct bcmgenet_eth_priv *priv)
>>  u32 len_stat, i;
>>  void *desc_base = priv->rx_desc_base;
>>  
>> -priv->c_index = 0;
>> -
>>  len_stat = (RX_BUF_LENGTH << DMA_BUFLENGTH_SHIFT) | DMA_OWN;
>>  
>>  for (i = 0; i < RX_DESCS; i++) {
>> @@ -403,8 +401,10 @@ static void rx_ring_init(struct bcmgenet_eth_priv *priv)
>>  writel(RX_DESCS * DMA_DESC_SIZE / 4 - 1,
>> priv->mac_reg + RDMA_RING_REG_BASE + DMA_END_ADDR);
>>  
>> -writel(0x0, priv->mac_reg + RDMA_PROD_INDEX);
>> -writel(0x0, priv->mac_reg + RDMA_CONS_INDEX);
>> +/* cannot init RDMA_PROD_INDEX to 0, so align RDMA_CONS_INDEX on it 
>> instead */
>> +priv->c_index = readl(priv->mac_reg + RDMA_PROD_INDEX);
>> +writel(priv->c_index, priv->mac_reg + RDMA_CONS_INDEX);
>> +priv->rx_index = priv->c_index;


printf("before RX_IDX: 0x%x\n", priv->rx_index);

I added a printf() like above for the RX and TX to see what is going on when 
I try and transfer a kernel Image file the second time.


U-Boot> tftp ${loadaddr} bootfs/Image
before RX_IDX: 0x0
before TX_IDX: 0x0
Using ethernet@7d58 device
Filename 'bootfs/Image'.
Load address: 0x8
Loading: ## Warning: gatewayip needed but not set
##  16.8 MiB
 6.1 MiB/s
done
Bytes transferred = 17615360 (10cca00 hex)
U-Boot> tftp ${loadaddr} bootfs/Image
before RX_IDX: 0xe4
before TX_IDX: 0x2ee3
Using ethernet@7d58 device
Filename 'bootfs/Image'.
Load address: 0x8
Loading: ## Warning: gatewayip needed but not set



The TX_IDX is now 0x2ee3 which is definitely not going to work.

According to the driver file there are only 256 (0xFF) slots,
which is why it hangs, with your change. 

Jason.

>>  writel((RX_DESCS << DMA_RING_SIZE_SHIFT) | RX_BUF_LENGTH,
>> priv->mac_reg + RDMA_RING_REG_BASE + DMA_RING_BUF_SIZE);
>>  writel(DMA_FC_THRESH_VALUE, priv->mac_reg 

Re: [PATCH] bcmgenet: fix DMA buffer management

[Jason Wessel]

On 7/9/20 3:11 AM, etienne.du...@gmail.com wrote:
> From: Etienne Dublé 
> 
> This commit fixes a serious issue occuring when several network
> commands are run on a raspberry pi 4 board: for instance a "dhcp"
> command and then one or several "tftp" commands. In this case,
> packet recv callbacks were called several times on the same packets,
> and send function was failing most of the time.
> 
> note: if the boot procedure is made of a single network
> command, the issue is not visible.
> 
> The issue is related to management of the packet ring buffers
> (producer / consumer) and DMA.
> Each time a packet is received, the ethernet device stores it
> in the buffer and increments an index called RDMA_PROD_INDEX.
> Each time the driver outputs a received packet, it increments
> another index called RDMA_CONS_INDEX.
> 
> Between each pair of network commands, as part of the driver
> 'start' function, previous code tried to reset both RDMA_CONS_INDEX
> and RDMA_PROD_INDEX to 0. But RDMA_PROD_INDEX cannot be written from
> driver side, thus its value was actually not updated, and only
> RDMA_CONS_INDEX was reset to 0. This was resulting in a major
> synchronization issue between the driver and the device. Most
> visible bahavior was that the driver seemed to receive again the
> packets from the previous commands (e.g. DHCP response packets
> "received" again when performing the first TFTP command).
> 
> This fix consists in setting RDMA_CONS_INDEX to the same
> value as RDMA_PROD_INDEX, when resetting the driver.
> 
> The same kind of fix was needed on the TX side, and a few variables
> had to be reset accordingly (c_index, tx_index, rx_index).


While there is some kind of problem with the driver, because I too
have observed a problem with multiple requests timing out or failing,
this patch makes the problem much worse.  I was only able to complete
a single tftp request. 

In my case I am using a static IP address and serverip. 

Also your patch was missing the sign-off line.  Please consider
running your patches through scripts/checkpatch.pl.

Cheers,
Jason.

> ---
>  drivers/net/bcmgenet.c | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
> index 11b6148ab6..a4facfd63f 100644
> --- a/drivers/net/bcmgenet.c
> +++ b/drivers/net/bcmgenet.c
> @@ -378,8 +378,6 @@ static void rx_descs_init(struct bcmgenet_eth_priv *priv)
>   u32 len_stat, i;
>   void *desc_base = priv->rx_desc_base;
>  
> - priv->c_index = 0;
> -
>   len_stat = (RX_BUF_LENGTH << DMA_BUFLENGTH_SHIFT) | DMA_OWN;
>  
>   for (i = 0; i < RX_DESCS; i++) {
> @@ -403,8 +401,10 @@ static void rx_ring_init(struct bcmgenet_eth_priv *priv)
>   writel(RX_DESCS * DMA_DESC_SIZE / 4 - 1,
>  priv->mac_reg + RDMA_RING_REG_BASE + DMA_END_ADDR);
>  
> - writel(0x0, priv->mac_reg + RDMA_PROD_INDEX);
> - writel(0x0, priv->mac_reg + RDMA_CONS_INDEX);
> + /* cannot init RDMA_PROD_INDEX to 0, so align RDMA_CONS_INDEX on it 
> instead */
> + priv->c_index = readl(priv->mac_reg + RDMA_PROD_INDEX);
> + writel(priv->c_index, priv->mac_reg + RDMA_CONS_INDEX);
> + priv->rx_index = priv->c_index;
>   writel((RX_DESCS << DMA_RING_SIZE_SHIFT) | RX_BUF_LENGTH,
>  priv->mac_reg + RDMA_RING_REG_BASE + DMA_RING_BUF_SIZE);
>   writel(DMA_FC_THRESH_VALUE, priv->mac_reg + RDMA_XON_XOFF_THRESH);
> @@ -421,8 +421,9 @@ static void tx_ring_init(struct bcmgenet_eth_priv *priv)
>   writel(0x0, priv->mac_reg + TDMA_WRITE_PTR);
>   writel(TX_DESCS * DMA_DESC_SIZE / 4 - 1,
>  priv->mac_reg + TDMA_RING_REG_BASE + DMA_END_ADDR);
> - writel(0x0, priv->mac_reg + TDMA_PROD_INDEX);
> - writel(0x0, priv->mac_reg + TDMA_CONS_INDEX);
> + /* cannot init TDMA_CONS_INDEX to 0, so align TDMA_PROD_INDEX on it 
> instead */
> + priv->tx_index = readl(priv->mac_reg + TDMA_CONS_INDEX);
> + writel(priv->tx_index, priv->mac_reg + TDMA_PROD_INDEX);
>   writel(0x1, priv->mac_reg + TDMA_RING_REG_BASE + DMA_MBUF_DONE_THRESH);
>   writel(0x0, priv->mac_reg + TDMA_FLOW_PERIOD);
>   writel((TX_DESCS << DMA_RING_SIZE_SHIFT) | RX_BUF_LENGTH,
> @@ -469,8 +470,6 @@ static int bcmgenet_gmac_eth_start(struct udevice *dev)
>  
>   priv->tx_desc_base = priv->mac_reg + GENET_TX_OFF;
>   priv->rx_desc_base = priv->mac_reg + GENET_RX_OFF;
> - priv->tx_index = 0x0;
> - priv->rx_index = 0x0;
>  
>   bcmgenet_umac_reset(priv);
>  
> 

[PATCH 6/6] usb.c: Add a retry in the usb_prepare_device()

[Jason Wessel]

I have found through testing some USB 2 composite mouse/keyboard
devices do not response to the usb_set_address call immediately
following the port reset.  It can take anywhere from 2ms to 20ms.

This patch adds a retry and delay for usb_prepare_device() and allows
all the USB keyboards I tried to function properly.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 0eb5d40a2d..39bae86a11 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1032,6 +1032,7 @@ static int usb_prepare_device(struct usb_device *dev, int 
addr, bool do_read,
  struct usb_device *parent)
 {
int err;
+   int retry_msec = 0;
 
/*
 * Allocate usb 3.0 device context.
@@ -1054,6 +1055,14 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
+   /* Retry for old composite keyboard/mouse usb2 hardware */
+   while (err < 0 && retry_msec <= 40) {
+   retry_msec += 20;
+   mdelay(20);
+   err = usb_set_address(dev); /* set address */
+   }
+   if (retry_msec > 0)
+   debug("usb_set_address delay: %i\n", retry_msec);
if (err < 0)
debug("\n   usb_set_address return < 0\n");
if (err < 0 && dev->status != 0) {
-- 
2.17.1

[PATCH 5/6] xhci-ring: Fix crash when issuing "usb reset"

[Jason Wessel]

If a "usb reset" is issued when the poll_pend state is set the
abort_td() function will hit one of the BUG() statements in abort_td()
or the BUG() statement at the end of xhci_wait_for_event().

The controller has been reset, so the rest of the cleanup should be
skipped and poll_pend flag should be cleared.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d6339f4f0a..f2a07204cd 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -483,6 +483,8 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, 
trb_type expected,
if (expected == TRB_TRANSFER)
return NULL;
 
+   if (poll_pend)
+   return NULL;
printf("XHCI timeout on event type %d... cannot recover.\n", expected);
BUG();
 }
@@ -505,11 +507,16 @@ static void abort_td(struct usb_device *udev, int 
ep_index)
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
-   field = le32_to_cpu(event->trans_event.flags);
-   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
-   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
-   != COMP_STOP)));
+   if (event) {
+   field = le32_to_cpu(event->trans_event.flags);
+   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
+   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
+   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
+!= COMP_STOP)));
+   } else {
+   debug("XHCI abort timeout\n");
+   return;
+   }
xhci_acknowledge_event(ctrl);
 
event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
-- 
2.17.1

[PATCH 1/6] xhci: Add polling support for USB keyboards

[Jason Wessel]

The xhci driver was causing intermittent 5 second delays from the USB
keyboard polling hook.  Executing something like a "sleep 1" for
example would sleep for 5 seconds, unless an event occurred on
the USB bus to shorten the delay.

Modeled after the code in the DWC2 driver, a nonblock state was added
to quickly return instead of blocking for up to 5 seconds waiting for
an event before timing out.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 26 +-
 drivers/usb/host/xhci.c  | 11 ++-
 include/usb/xhci.h   |  5 +++--
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 86aeaab412..b7b2e16410 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -432,9 +432,11 @@ static int event_ready(struct xhci_ctrl *ctrl)
  *
  * @param ctrl Host controller data structure
  * @param expected TRB type expected from Event TRB
+ * @param nonblock when true do not block waiting for response
  * @return pointer to event trb
  */
-union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected)
+union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
+   bool nonblock)
 {
trb_type type;
unsigned long ts = get_timer(0);
@@ -442,8 +444,11 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl 
*ctrl, trb_type expected)
do {
union xhci_trb *event = ctrl->event_ring->dequeue;
 
-   if (!event_ready(ctrl))
+   if (!event_ready(ctrl)) {
+   if (nonblock)
+   return NULL;
continue;
+   }
 
type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags));
if (type == expected)
@@ -493,7 +498,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
field = le32_to_cpu(event->trans_event.flags);
BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
@@ -501,7 +506,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
!= COMP_STOP)));
xhci_acknowledge_event(ctrl);
 
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -509,7 +514,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, (void *)((uintptr_t)ring->enqueue |
ring->cycle_state), udev->slot_id, ep_index, TRB_SET_DEQ);
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -552,10 +557,11 @@ static void record_transfer_result(struct usb_device 
*udev,
  * @param pipe contains the DIR_IN or OUT , devnum
  * @param length   length of the buffer
  * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
  * @return returns 0 if successful else -1 on failure
  */
 int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer)
+   int length, void *buffer, bool nonblock)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -714,8 +720,10 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
+   if (nonblock)
+   return -EINVAL;
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeout */
@@ -911,7 +919,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
if (!event)
goto abort;
field = le32_to_cpu(event->tra

[PATCH 3/6] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Jason Wessel]

When resetting the rpi3 board sometimes it will display:
 USB device not accepting new address (error=0)

After the message appears, the usb keyboard will not work.  It seems
that the configuration actually did succeed however.  Checking the
device status for a return code of zero and continuing allows the usb
keyboard and other usb devices to work function.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c5..0eb5d40a2d 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
-
-   if (err < 0) {
+   if (err < 0)
+   debug("\n   usb_set_address return < 0\n");
+   if (err < 0 && dev->status != 0) {
printf("\n  USB device not accepting new address " \
"(error=%lX)\n", dev->status);
-   return err;
+   return err;
}
 
mdelay(10); /* Let the SET_ADDRESS settle */
-- 
2.17.1

[PATCH 4/6] xhci-ring.c: Add the poll_pend state to properly abort transactions

[Jason Wessel]

xhci_trl_tx and xhchi_bulk_tx can be called synchronously by other
drivers such as the usb storage or network, while the keyboard driver
exclusively uses the polling mode.

And pending polling transactions must be aborted before switching
modes to avoid corrupting the state of the controller.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 86 +---
 1 file changed, 70 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b7b2e16410..d6339f4f0a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -24,6 +24,12 @@
 
 #include 
 
+static void *last_bulk_tx_buf;
+static struct usb_device *poll_last_udev;
+int poll_last_ep_index;
+static unsigned long bulk_tx_poll_ts;
+static bool poll_pend;
+
 /**
  * Is this TRB a link TRB or was the last TRB the last TRB in this event ring
  * segment?  I.e. would the updated event TRB pointer step off the end of the
@@ -549,19 +555,8 @@ static void record_transfer_result(struct usb_device *udev,
}
 }
 
-/ Bulk and Control transfer methods /
-/**
- * Queues up the BULK Request
- *
- * @param udev pointer to the USB device structure
- * @param pipe contains the DIR_IN or OUT , devnum
- * @param length   length of the buffer
- * @param buffer   buffer to be read/written based on the request
- * @param nonblock when true do not block waiting for response
- * @return returns 0 if successful else -1 on failure
- */
-int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer, bool nonblock)
+static int _xhci_bulk_tx_queue(struct usb_device *udev, unsigned long pipe,
+  int length, void *buffer)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -575,7 +570,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
struct xhci_virt_device *virt_dev;
struct xhci_ep_ctx *ep_ctx;
struct xhci_ring *ring; /* EP transfer ring */
-   union xhci_trb *event;
 
int running_total, trb_buff_len;
unsigned int total_packet_count;
@@ -719,20 +713,73 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
} while (running_total < length);
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
+   return 0;
+}
 
+/ Bulk and Control transfer methods /
+/**
+ * Queues up the BULK Request
+ *
+ * @param udev pointer to the USB device structure
+ * @param pipe contains the DIR_IN or OUT , devnum
+ * @param length   length of the buffer
+ * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
+ * @return returns 0 if successful else -1 on failure
+ */
+int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
+int length, void *buffer, bool nonblock)
+{
+   u32 field;
+   int ret;
+   union xhci_trb *event;
+   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
+   int ep_index = usb_pipe_ep_index(pipe);
+
+   if (poll_pend) {
+   /*
+* Abort a pending poll operation if it should have
+* timed out, or if this is a different buffer from a
+* separate request
+*/
+   if (get_timer(bulk_tx_poll_ts) > XHCI_TIMEOUT ||
+   last_bulk_tx_buf != buffer || poll_last_udev != udev ||
+   ep_index != poll_last_ep_index) {
+   abort_td(poll_last_udev, poll_last_ep_index);
+   poll_last_udev->status = USB_ST_NAK_REC;  /* closest 
thing to a timeout */
+   poll_last_udev->act_len = 0;
+   poll_pend = false;
+   }
+   } /* No else here because poll_pend might have changed above */
+   if (!poll_pend) {
+   last_bulk_tx_buf = buffer;
+   ret = _xhci_bulk_tx_queue(udev, pipe, length, buffer);
+   if (ret)
+   return ret;
+   }
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
-   if (nonblock)
+   if (nonblock) {
+   if (!poll_pend) {
+   /* Start the timer */
+   bulk_tx_poll_ts = get_timer(0);
+   poll_last_udev = udev;
+   poll_last_ep_index = ep_index;
+   poll_pend = true;
+   }
return -EINVAL;
+   }
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeout */
udev->act_len = 

[PATCH 2/6] usb_kbd: succeed even if no interrupt is pending

[Jason Wessel]

After the initial configuration some USB keyboard+mouse devices never
return any kind of event on the interrupt line.  In particular, the
device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
never returns a data packet until the first external input event.

I found this was also true with some newer model Dell keyboards.

When the device is plugged into a xhci controller there is also no
point in waiting 5 seconds for a device that is never going to present
data, so the call to the interrupt service was changed to a
nonblocking operation for the controllers that support this.

With the patch applied, the rpi3 and rpi4 work well with the more
complex keyboard devices.

Signed-off-by: Jason Wessel 
---
 common/usb_kbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844..3c0056e1b9 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -519,7 +519,9 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
   1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) {
 #else
if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
-   data->intinterval, false) < 0) {
+   data->intinterval, true) < 0) {
+   /* Read first packet if the device provides it, else pick it up 
later */
+   return 1;
 #endif
printf("Failed to get keyboard state from device %04x:%04x\n",
   dev->descriptor.idVendor, dev->descriptor.idProduct);
-- 
2.17.1

[PATCH 0/6] Improve USB Keyboard support for all rpi boards

[Jason Wessel]

Prior to this patch set, I had just a small number of USB keyboards
function properly with the Rasbpberry Pi boards.  Nearly every
composite USB keyboard/mouse I had access to including the Raritan
IP/KVM devices would not initialize properly.

At this point I have tested all generations of the Raspberry Pi boards
(0-4) and the USB keyboards I use day to day are working properly.

For the RPi4, I tested against the rpi-master branch which now has the
required USB patches, all the other boards were tested against the
master branch.

v1:
  - Testing completed against all generations of raspberry pi boards
  - check patch warnings add errors have been fixed

rfc v3:
  - Add in patch 6
  - Finally got the GearHead keyboard + mouse composite USB device working

rfc v2: 
  - Minor cleanups to patches 1-3 based on prior review
  - Patch 4 & 5 are new to fix various xhchi crashes
while having additional devices plugged in along with
booting off the usb port.  The nonblocking mode turned
out to be somewhat complex.

rfc v1:

The original purpose of the patch set was to fix problems with the USB
keyboard support with the rpi4.  The generic xhci code lacks a non
blocking call to read an event.  This causes major problems for the
sleep function as well as for ethernet downloads where a keyboard poll
for interrupting the download blocks for 5 seconds.

----
Jason Wessel (6):
  xhci: Add polling support for USB keyboards
  usb_kbd: succeed even if no interrupt is pending
  common/usb.c: Work around keyboard reporting "USB device not accepting 
new address"
  xhci-ring.c: Add the poll_pend state to properly abort transactions
  xhci-ring: Fix crash when issuing "usb reset"
  usb.c: Add a retry in the usb_prepare_device()

 common/usb.c |  16 +---
 common/usb_kbd.c |   4 +++-
 drivers/usb/host/xhci-ring.c | 123 
---
 drivers/usb/host/xhci.c  |  11 ++-
 include/usb/xhci.h   |   5 +++--
 5 files changed, 121 insertions(+), 38 deletions(-)

[RFC PATCH v3 5/6] xhci-ring: Fix crash when issuing "usb reset"

[Jason Wessel]

If a "usb reset" is issued when the poll_pend state is set the
abort_td() function will hit one of the BUG() statements in abort_td()
or the BUG() statement at the end of xhci_wait_for_event().

The controller has been reset, so the rest of the cleanup should be
skipped and poll_pend flag should be cleared.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1c00f2d496..ed0dea9fca 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -483,6 +483,8 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, 
trb_type expected,
if (expected == TRB_TRANSFER)
return NULL;
 
+   if (poll_pend)
+   return NULL;
printf("XHCI timeout on event type %d... cannot recover.\n", expected);
BUG();
 }
@@ -505,11 +507,16 @@ static void abort_td(struct usb_device *udev, int 
ep_index)
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
-   field = le32_to_cpu(event->trans_event.flags);
-   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
-   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
-   != COMP_STOP)));
+   if (event) {
+   field = le32_to_cpu(event->trans_event.flags);
+   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
+   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
+   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
+!= COMP_STOP)));
+   } else {
+   debug("XHCI abort timeout\n");
+   return;
+   }
xhci_acknowledge_event(ctrl);
 
event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
-- 
2.17.1

[RFC PATCH v3 0/6] Improve USB Keyboard support for rpi3/rpi4

[Jason Wessel]

At this point all the USB keyboards I had laying around now work
and I can USB boot the rpi3 and rpi4, so perhaps this series can
move beyond the RFC stage.  More testing will occur over the next
week or so. 

v3:
  - Add in patch 6
  - Finally got the GearHead keyboard + mouse composite USB device working

v2: 
  - Minor cleanups to patches 1-3 based on prior review
  - Patch 4 & 5 are new to fix various xhchi crashes
while having additional devices plugged in along with
booting off the usb port.  The nonblocking mode turned
out to be somewhat complex.

v1:

For testing this patch series, I did apply the USB patches for the
rpi4 board which had not been merged yet.  Once the rpi4 USB changes
are eventually merged the keyboard delay issues will show up and I
imagine this issue is already a problem for other boards. This series
does not depend on rpi4 patches as the changes are in the core USB
functions.

I performed tests with a number of usb modules attached including 
storage, ethernet, serial, wifi, keyboard, and
mouse without any issues.

----
Jason Wessel (6):
  xhci: Add polling support for USB keyboards
  usb_kbd: Do not fail the keyboard if it does not have an interrupt pending
  common/usb.c: Work around keyboard reporting "USB device not accepting 
new address"
  xhci-ring.c: Add the poll_pend state to properly abort transactions
  xhci-ring: Fix crash when issuing "usb reset"
  usb.c: Add a retry in the usb_prepare_device()

 common/usb.c |  18 +---
 common/usb_kbd.c |   4 ++-
 drivers/usb/host/xhci-ring.c | 123 
++--
 drivers/usb/host/xhci.c  |  11 
 include/usb/xhci.h   |   5 ++--
 5 files changed, 122 insertions(+), 39 deletions(-)

[RFC PATCH v3 1/6] xhci: Add polling support for USB keyboards

[Jason Wessel]

The xhci driver was causing intermittent 5 second delays from the USB
keyboard polling hook.  Executing something like a "sleep 1" for
example would sleep for 5 seconds, unless an event occurred on
the USB bus to shorten the delay.

Modeled after the code in the DWC2 driver, a nonblock state was added
to quickly return instead of blocking for up to 5 seconds waiting for
an event before timing out.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 26 +-
 drivers/usb/host/xhci.c  | 11 ++-
 include/usb/xhci.h   |  5 +++--
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 86aeaab412..b7b2e16410 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -432,9 +432,11 @@ static int event_ready(struct xhci_ctrl *ctrl)
  *
  * @param ctrl Host controller data structure
  * @param expected TRB type expected from Event TRB
+ * @param nonblock when true do not block waiting for response
  * @return pointer to event trb
  */
-union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected)
+union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
+   bool nonblock)
 {
trb_type type;
unsigned long ts = get_timer(0);
@@ -442,8 +444,11 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl 
*ctrl, trb_type expected)
do {
union xhci_trb *event = ctrl->event_ring->dequeue;
 
-   if (!event_ready(ctrl))
+   if (!event_ready(ctrl)) {
+   if (nonblock)
+   return NULL;
continue;
+   }
 
type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags));
if (type == expected)
@@ -493,7 +498,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
field = le32_to_cpu(event->trans_event.flags);
BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
@@ -501,7 +506,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
!= COMP_STOP)));
xhci_acknowledge_event(ctrl);
 
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -509,7 +514,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, (void *)((uintptr_t)ring->enqueue |
ring->cycle_state), udev->slot_id, ep_index, TRB_SET_DEQ);
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -552,10 +557,11 @@ static void record_transfer_result(struct usb_device 
*udev,
  * @param pipe contains the DIR_IN or OUT , devnum
  * @param length   length of the buffer
  * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
  * @return returns 0 if successful else -1 on failure
  */
 int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer)
+   int length, void *buffer, bool nonblock)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -714,8 +720,10 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
+   if (nonblock)
+   return -EINVAL;
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeout */
@@ -911,7 +919,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
if (!event)
goto abort;
field = le32_to_cpu(event->tra

[RFC PATCH v3 6/6] usb.c: Add a retry in the usb_prepare_device()

[Jason Wessel]

I have found through testing some USB 2 composite mouse/keyboard
devices do not response to the usb_set_address call immediately
following the port reset.  It can take anywhere from 2ms to 20ms.

This patch adds a retry and delay for usb_prepare_device() and allows
all the USB keyboards I tried to function properly.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index ba83bb960b..fb091440cc 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1054,6 +1054,15 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
+   /* Retry for old composite keyboard/mouse usb2 hardware */
+   int i = 0;
+   while (err < 0 && i <= 40) {
+   i += 20;
+   mdelay(20);
+   err = usb_set_address(dev); /* set address */
+   }
+   if (i > 0)
+   debug("usb_set_address delay: %i\n", i);
if (err < 0)
debug("\n   usb_set_address return < 0\n");
if (err < 0 && dev->status != 0) {
-- 
2.17.1

[RFC PATCH v3 4/6] xhci-ring.c: Add the poll_pend state to properly abort transactions

[Jason Wessel]

xhci_trl_tx and xhchi_bulk_tx can be called synchronously by other
drivers such as the usb storage or network, while the keyboard driver
exclusively uses the polling mode.

And pending polling transactions must be aborted before switching
modes to avoid corrupting the state of the controller.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 86 +---
 1 file changed, 70 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b7b2e16410..1c00f2d496 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -24,6 +24,12 @@
 
 #include 
 
+static void *last_bulk_tx_buf = NULL;
+static struct usb_device *poll_last_udev;
+int poll_last_ep_index;
+static unsigned long bulk_tx_poll_ts = 0;
+static bool poll_pend = false;
+
 /**
  * Is this TRB a link TRB or was the last TRB the last TRB in this event ring
  * segment?  I.e. would the updated event TRB pointer step off the end of the
@@ -549,19 +555,8 @@ static void record_transfer_result(struct usb_device *udev,
}
 }
 
-/ Bulk and Control transfer methods /
-/**
- * Queues up the BULK Request
- *
- * @param udev pointer to the USB device structure
- * @param pipe contains the DIR_IN or OUT , devnum
- * @param length   length of the buffer
- * @param buffer   buffer to be read/written based on the request
- * @param nonblock when true do not block waiting for response
- * @return returns 0 if successful else -1 on failure
- */
-int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer, bool nonblock)
+static int _xhci_bulk_tx_queue(struct usb_device *udev, unsigned long pipe,
+ int length, void *buffer)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -575,7 +570,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
struct xhci_virt_device *virt_dev;
struct xhci_ep_ctx *ep_ctx;
struct xhci_ring *ring; /* EP transfer ring */
-   union xhci_trb *event;
 
int running_total, trb_buff_len;
unsigned int total_packet_count;
@@ -719,20 +713,73 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
} while (running_total < length);
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
+   return 0;
+}
 
+/ Bulk and Control transfer methods /
+/**
+ * Queues up the BULK Request
+ *
+ * @param udev pointer to the USB device structure
+ * @param pipe contains the DIR_IN or OUT , devnum
+ * @param length   length of the buffer
+ * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
+ * @return returns 0 if successful else -1 on failure
+ */
+int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
+   int length, void *buffer, bool nonblock)
+{
+   u32 field;
+   int ret;
+   union xhci_trb *event;
+   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
+   int ep_index = usb_pipe_ep_index(pipe);
+
+   if (poll_pend) {
+   /*
+* Abort a pending poll operation if it should have
+* timed out, or if this is a different buffer from a
+* separate request
+*/
+   if (get_timer(bulk_tx_poll_ts) > XHCI_TIMEOUT ||
+   last_bulk_tx_buf != buffer || poll_last_udev != udev ||
+   ep_index != poll_last_ep_index) {
+   abort_td(poll_last_udev, poll_last_ep_index);
+   poll_last_udev->status = USB_ST_NAK_REC;  /* closest 
thing to a timeout */
+   poll_last_udev->act_len = 0;
+   poll_pend = false;
+   }
+   } /* No else here because poll_pend might have changed above */
+   if (!poll_pend) {
+   last_bulk_tx_buf = buffer;
+   ret = _xhci_bulk_tx_queue(udev, pipe, length, buffer);
+   if (ret)
+   return ret;
+   }
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
-   if (nonblock)
+   if (nonblock) {
+   if (!poll_pend) {
+   /* Start the timer */
+   bulk_tx_poll_ts = get_timer(0);
+   poll_last_udev = udev;
+   poll_last_ep_index = ep_index;
+   poll_pend = true;
+   }
return -EINVAL;
+   }
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeou

[RFC PATCH v3 3/6] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Jason Wessel]

When resetting the rpi3 board sometimes it will display:
 USB device not accepting new address (error=0)

After the message appears, the usb keyboard will not work.  It seems
that the configuration actually did succeed however.  Checking the
device status for a return code of zero and continuing allows the usb
keyboard and other usb devices to work function.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c5..ba83bb960b 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
-
-   if (err < 0) {
-   printf("\n  USB device not accepting new address " \
+   if (err < 0)
+   debug("\n   usb_set_address return < 0\n");
+   if (err < 0 && dev->status != 0) {
+   printf("\n  USB device not accepting new address "  \
"(error=%lX)\n", dev->status);
-   return err;
+   return err;
}
 
mdelay(10); /* Let the SET_ADDRESS settle */
-- 
2.17.1

[RFC PATCH v3 2/6] usb_kbd: Do not fail the keyboard if it does not have an interrupt pending

[Jason Wessel]

After the initial configuration some USB keyboard+mouse devices never
return any kind of event on the interrupt line.  In particular, the
device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
/devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
never returns a data packet until the first external input event.

I found this was also true with some newer model Dell keyboards.

When the device is plugged into a xhci controller there is also no
point in waiting 5 seconds for a device that is never going to present
data, so the call to the interrupt service was changed to a
nonblocking operation for the controllers that support this.

With the patch applied, the rpi3 and rpi4 work well with the more
complex keyboard devices.

Signed-off-by: Jason Wessel 
---
 common/usb_kbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844..3c0056e1b9 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -519,7 +519,9 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
   1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) {
 #else
if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
-   data->intinterval, false) < 0) {
+   data->intinterval, true) < 0) {
+   /* Read first packet if the device provides it, else pick it up 
later */
+   return 1;
 #endif
printf("Failed to get keyboard state from device %04x:%04x\n",
   dev->descriptor.idVendor, dev->descriptor.idProduct);
-- 
2.17.1

[RFC PATCH v2 4/5] xhci-ring.c: Add the poll_pend state to properly abort transactions

[Jason Wessel]

xhci_trl_tx and xhchi_bulk_tx can be called synchronously by other
drivers such as the usb storage or network, while the keyboard driver
exclusively uses the polling mode.

And pending polling transactions must be aborted before switching
modes to avoid corrupting the state of the controller.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 86 +---
 1 file changed, 70 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b7b2e16410..1c00f2d496 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -24,6 +24,12 @@
 
 #include 
 
+static void *last_bulk_tx_buf = NULL;
+static struct usb_device *poll_last_udev;
+int poll_last_ep_index;
+static unsigned long bulk_tx_poll_ts = 0;
+static bool poll_pend = false;
+
 /**
  * Is this TRB a link TRB or was the last TRB the last TRB in this event ring
  * segment?  I.e. would the updated event TRB pointer step off the end of the
@@ -549,19 +555,8 @@ static void record_transfer_result(struct usb_device *udev,
}
 }
 
-/ Bulk and Control transfer methods /
-/**
- * Queues up the BULK Request
- *
- * @param udev pointer to the USB device structure
- * @param pipe contains the DIR_IN or OUT , devnum
- * @param length   length of the buffer
- * @param buffer   buffer to be read/written based on the request
- * @param nonblock when true do not block waiting for response
- * @return returns 0 if successful else -1 on failure
- */
-int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer, bool nonblock)
+static int _xhci_bulk_tx_queue(struct usb_device *udev, unsigned long pipe,
+ int length, void *buffer)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -575,7 +570,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
struct xhci_virt_device *virt_dev;
struct xhci_ep_ctx *ep_ctx;
struct xhci_ring *ring; /* EP transfer ring */
-   union xhci_trb *event;
 
int running_total, trb_buff_len;
unsigned int total_packet_count;
@@ -719,20 +713,73 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
} while (running_total < length);
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
+   return 0;
+}
 
+/ Bulk and Control transfer methods /
+/**
+ * Queues up the BULK Request
+ *
+ * @param udev pointer to the USB device structure
+ * @param pipe contains the DIR_IN or OUT , devnum
+ * @param length   length of the buffer
+ * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
+ * @return returns 0 if successful else -1 on failure
+ */
+int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
+   int length, void *buffer, bool nonblock)
+{
+   u32 field;
+   int ret;
+   union xhci_trb *event;
+   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
+   int ep_index = usb_pipe_ep_index(pipe);
+
+   if (poll_pend) {
+   /*
+* Abort a pending poll operation if it should have
+* timed out, or if this is a different buffer from a
+* separate request
+*/
+   if (get_timer(bulk_tx_poll_ts) > XHCI_TIMEOUT ||
+   last_bulk_tx_buf != buffer || poll_last_udev != udev ||
+   ep_index != poll_last_ep_index) {
+   abort_td(poll_last_udev, poll_last_ep_index);
+   poll_last_udev->status = USB_ST_NAK_REC;  /* closest 
thing to a timeout */
+   poll_last_udev->act_len = 0;
+   poll_pend = false;
+   }
+   } /* No else here because poll_pend might have changed above */
+   if (!poll_pend) {
+   last_bulk_tx_buf = buffer;
+   ret = _xhci_bulk_tx_queue(udev, pipe, length, buffer);
+   if (ret)
+   return ret;
+   }
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
-   if (nonblock)
+   if (nonblock) {
+   if (!poll_pend) {
+   /* Start the timer */
+   bulk_tx_poll_ts = get_timer(0);
+   poll_last_udev = udev;
+   poll_last_ep_index = ep_index;
+   poll_pend = true;
+   }
return -EINVAL;
+   }
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeou

[RFC PATCH v2 5/5] xhci-ring: Fix crash when issuing "usb reset"

[Jason Wessel]

If a "usb reset" is issued when the poll_pend state is set the
abort_td() function will hit one of the BUG() statements in abort_td()
or the BUG() statement at the end of xhci_wait_for_event().

The controller has been reset, so the rest of the cleanup should be
skipped and poll_pend flag should be cleared.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1c00f2d496..ed0dea9fca 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -483,6 +483,8 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, 
trb_type expected,
if (expected == TRB_TRANSFER)
return NULL;
 
+   if (poll_pend)
+   return NULL;
printf("XHCI timeout on event type %d... cannot recover.\n", expected);
BUG();
 }
@@ -505,11 +507,16 @@ static void abort_td(struct usb_device *udev, int 
ep_index)
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
-   field = le32_to_cpu(event->trans_event.flags);
-   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
-   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
-   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
-   != COMP_STOP)));
+   if (event) {
+   field = le32_to_cpu(event->trans_event.flags);
+   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
+   BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
+   BUG_ON(GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len
+!= COMP_STOP)));
+   } else {
+   debug("XHCI abort timeout\n");
+   return;
+   }
xhci_acknowledge_event(ctrl);
 
event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
-- 
2.17.1

[RFC PATCH v2 3/5] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Jason Wessel]

When resetting the rpi3 board sometimes it will display:
 USB device not accepting new address (error=0)

After the message appears, the usb keyboard will not work.  It seems
that the configuration actually did succeed however.  Checking the
device status for a return code of zero and continuing allows the usb
keyboard and other usb devices to work function.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c5..ba83bb960b 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
-
-   if (err < 0) {
-   printf("\n  USB device not accepting new address " \
+   if (err < 0)
+   debug("\n   usb_set_address return < 0\n");
+   if (err < 0 && dev->status != 0) {
+   printf("\n  USB device not accepting new address "  \
"(error=%lX)\n", dev->status);
-   return err;
+   return err;
}
 
mdelay(10); /* Let the SET_ADDRESS settle */
-- 
2.17.1

[RFC PATCH v2 0/5] Improve USB Keyboard support for rpi3/rpi4

[Jason Wessel]

v2: 
  - Minor cleanups to patches 1-3 based on prior review
  - Patch 4 & 5 are new to fix various xhchi crashes
while having additional devices plugged in along with
booting off the usb port.  The nonblocking mode turned
out to be somewhat complex.

v1:

For testing this patch series, I did apply the USB patches for the
rpi4 board which had not been merged yet.  Once the rpi4 USB changes
are eventually merged the keyboard delay issues will show up and I
imagine this issue is already a problem for other boards. This series
does not depend on rpi4 patches as the changes are in the core USB
functions.

I am not entirely certain about the change to the common/usb.c which
is why this is an RFC.  I am not sure if there would be other side
effects to other USB drivers.  I did test with a number of usb modules
attached including storage, ethernet, serial, wifi, keyboard, and
mouse without any issues.

----
Jason Wessel (5):
  xhci: Add polling support for USB keyboards
  usb_kbd: Do not fail the keyboard if it does not have an interrupt pending
  common/usb.c: Work around keyboard reporting "USB device not accepting 
new address"
  xhci-ring.c: Add the poll_pend state to properly abort transactions
  xhci-ring: Fix crash when issuing "usb reset"

 common/usb.c |   9 +++---
 common/usb_kbd.c |   4 ++-
 drivers/usb/host/xhci-ring.c | 123 
++--
 drivers/usb/host/xhci.c  |  11 
 include/usb/xhci.h   |   5 ++--
 5 files changed, 113 insertions(+), 39 deletions(-)

[RFC PATCH v2 2/5] usb_kbd: Do not fail the keyboard if it does not have an interrupt pending

[Jason Wessel]

After the initial configuration some USB keyboard+mouse devices never
return any kind of event on the interrupt line.  In particular, the
device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
/devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
never returns a data packet until the first external input event.

I found this was also true with some newer model Dell keyboards.

When the device is plugged into a xhci controller there is also no
point in waiting 5 seconds for a device that is never going to present
data, so the call to the interrupt service was changed to a
nonblocking operation for the controllers that support this.

With the patch applied, the rpi3 and rpi4 work well with the more
complex keyboard devices.

Signed-off-by: Jason Wessel 
---
 common/usb_kbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844..3c0056e1b9 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -519,7 +519,9 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
   1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) {
 #else
if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
-   data->intinterval, false) < 0) {
+   data->intinterval, true) < 0) {
+   /* Read first packet if the device provides it, else pick it up 
later */
+   return 1;
 #endif
printf("Failed to get keyboard state from device %04x:%04x\n",
   dev->descriptor.idVendor, dev->descriptor.idProduct);
-- 
2.17.1

[RFC PATCH v2 1/5] xhci: Add polling support for USB keyboards

[Jason Wessel]

The xhci driver was causing intermittent 5 second delays from the USB
keyboard polling hook.  Executing something like a "sleep 1" for
example would sleep for 5 seconds, unless an event occurred on
the USB bus to shorten the delay.

Modeled after the code in the DWC2 driver, a nonblock state was added
to quickly return instead of blocking for up to 5 seconds waiting for
an event before timing out.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 26 +-
 drivers/usb/host/xhci.c  | 11 ++-
 include/usb/xhci.h   |  5 +++--
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 86aeaab412..b7b2e16410 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -432,9 +432,11 @@ static int event_ready(struct xhci_ctrl *ctrl)
  *
  * @param ctrl Host controller data structure
  * @param expected TRB type expected from Event TRB
+ * @param nonblock when true do not block waiting for response
  * @return pointer to event trb
  */
-union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected)
+union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
+   bool nonblock)
 {
trb_type type;
unsigned long ts = get_timer(0);
@@ -442,8 +444,11 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl 
*ctrl, trb_type expected)
do {
union xhci_trb *event = ctrl->event_ring->dequeue;
 
-   if (!event_ready(ctrl))
+   if (!event_ready(ctrl)) {
+   if (nonblock)
+   return NULL;
continue;
+   }
 
type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags));
if (type == expected)
@@ -493,7 +498,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
field = le32_to_cpu(event->trans_event.flags);
BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
@@ -501,7 +506,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
!= COMP_STOP)));
xhci_acknowledge_event(ctrl);
 
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -509,7 +514,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, (void *)((uintptr_t)ring->enqueue |
ring->cycle_state), udev->slot_id, ep_index, TRB_SET_DEQ);
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -552,10 +557,11 @@ static void record_transfer_result(struct usb_device 
*udev,
  * @param pipe contains the DIR_IN or OUT , devnum
  * @param length   length of the buffer
  * @param buffer   buffer to be read/written based on the request
+ * @param nonblock when true do not block waiting for response
  * @return returns 0 if successful else -1 on failure
  */
 int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer)
+   int length, void *buffer, bool nonblock)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -714,8 +720,10 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
+   if (nonblock)
+   return -EINVAL;
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeout */
@@ -911,7 +919,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
if (!event)
goto abort;
field = le32_to_cpu(event->tra

[RFC PATCH 2/3] usb_kbd: Do not fail the keyboard if it does not have an interrupt pending

[Jason Wessel]

After the initial configuration some USB keyboard+mouse devices never
return any kind of event on the interrupt line.  In particular, the
device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
/devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
never returns a data packet until the first external input event.

I found this was also true with some newer model Dell keyboards.

When the device is plugged into a xhci controller there is also no
point in waiting 5 seconds for a device that is never going to present
data, so the call to the interrupt service was changed to a
nonblocking operation for the controllers that support this.

With the patch applied, the rpi3 and rpi4 work well with the more
complex keyboard devices.

Signed-off-by: Jason Wessel 
---
 common/usb_kbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844..3c0056e1b9 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -519,7 +519,9 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
   1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) {
 #else
if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
-   data->intinterval, false) < 0) {
+   data->intinterval, true) < 0) {
+   /* Read first packet if the device provides it, else pick it up 
later */
+   return 1;
 #endif
printf("Failed to get keyboard state from device %04x:%04x\n",
   dev->descriptor.idVendor, dev->descriptor.idProduct);
-- 
2.17.1

[RFC PATCH 1/3] xhci: Add polling support for USB keyboards

[Jason Wessel]

The xhci driver was causing intermittent 5 second delays from the USB
keyboard polling hook.  Executing something like a "sleep 1" for
example would sleep for 5 seconds, unless an event occurred on
the USB bus to shorten the delay.

Modeled after the code in the DWC2 driver, a nonblock state was added
to quickly return instead of blocking for up to 5 seconds waiting for
an event before timing out.

Signed-off-by: Jason Wessel 
---
 drivers/usb/host/xhci-ring.c | 24 +++-
 drivers/usb/host/xhci.c  | 10 +-
 include/usb/xhci.h   |  5 +++--
 3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 86aeaab412..1f7b3a8e0b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -434,7 +434,8 @@ static int event_ready(struct xhci_ctrl *ctrl)
  * @param expected TRB type expected from Event TRB
  * @return pointer to event trb
  */
-union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected)
+union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,
+   bool nonblock)
 {
trb_type type;
unsigned long ts = get_timer(0);
@@ -442,8 +443,11 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl 
*ctrl, trb_type expected)
do {
union xhci_trb *event = ctrl->event_ring->dequeue;
 
-   if (!event_ready(ctrl))
+   if (!event_ready(ctrl)) {
+   if (nonblock)
+   return NULL;
continue;
+   }
 
type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags));
if (type == expected)
@@ -493,7 +497,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
field = le32_to_cpu(event->trans_event.flags);
BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
@@ -501,7 +505,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
!= COMP_STOP)));
xhci_acknowledge_event(ctrl);
 
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -509,7 +513,7 @@ static void abort_td(struct usb_device *udev, int ep_index)
 
xhci_queue_command(ctrl, (void *)((uintptr_t)ring->enqueue |
ring->cycle_state), udev->slot_id, ep_index, TRB_SET_DEQ);
-   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false);
BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
!= udev->slot_id || GET_COMP_CODE(le32_to_cpu(
event->event_cmd.status)) != COMP_SUCCESS);
@@ -555,7 +559,7 @@ static void record_transfer_result(struct usb_device *udev,
  * @return returns 0 if successful else -1 on failure
  */
 int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
-   int length, void *buffer)
+   int length, void *buffer, bool nonblock)
 {
int num_trbs = 0;
struct xhci_generic_trb *start_trb;
@@ -714,8 +718,10 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock);
if (!event) {
+   if (nonblock)
+   return -EINVAL;
debug("XHCI bulk transfer timed out, aborting...\n");
abort_td(udev, ep_index);
udev->status = USB_ST_NAK_REC;  /* closest thing to a timeout */
@@ -911,7 +917,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
giveback_first_trb(udev, ep_index, start_cycle, start_trb);
 
-   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
+   event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
if (!event)
goto abort;
field = le32_to_cpu(event->trans_event.flags);
@@ -929,7 +935,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
if (GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len))
== COMP_SHORT_TX) {
/* Short data stage, clear up additional status stage event */
-   event = xhci_wait_for_event(ctrl, TRB_TRANSF

[RFC PATCH 3/3] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Jason Wessel]

When resetting the rpi3 board sometimes it will display:
 USB device not accepting new address (error=0)

After the message appears, the usb keyboard will not work.  It seems
that the configuration actually did succeed however.  Checking the
device status for a return code of zero and continuing allows the usb
keyboard and other usb devices to work function.

Signed-off-by: Jason Wessel 
---
 common/usb.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c5..2f7f205444 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1054,11 +1054,13 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
dev->devnum = addr;
 
err = usb_set_address(dev); /* set address */
-
-   if (err < 0) {
-   printf("\n  USB device not accepting new address " \
+   if (err < 0)
+   debug("\n   usb_set_address return < 0\n");
+   if (err < 0 && dev->status != 0) {
+   printf("\n  USB device not accepting new address "  \
"(error=%lX)\n", dev->status);
-   return err;
+   if (dev->status != 0)
+   return err;
}
 
mdelay(10); /* Let the SET_ADDRESS settle */
-- 
2.17.1

[RFC PATCH 0/3] Improve USB Keyboard support for rpi3/rpi4

[Jason Wessel]

For testing this patch series, I did apply the USB patches for the
rpi4 board which had not been merged yet.  Once the rpi4 USB changes
are eventually merged the keyboard delay issues will show up and I
imagine this issue is already a problem for other boards. This series
does not depend on rpi4 patches as the changes are in the core USB
functions.

I am not entirely certain about the change to the common/usb.c which
is why this is an RFC.  I am not sure if there would be other side
effects to other USB drivers.  I did test with a number of usb modules
attached including storage, ethernet, serial, wifi, keyboard, and
mouse without any issues.


Jason Wessel (3):
  xhci: Add polling support for USB keyboards
  usb_kbd: Do not fail the keyboard if it does not have an interrupt pending
  common/usb.c: Work around keyboard reporting "USB device not accepting 
new address"

 common/usb.c | 10 ++
 common/usb_kbd.c |  4 +++-
 drivers/usb/host/xhci-ring.c | 24 +++-
 drivers/usb/host/xhci.c  | 10 +-
 include/usb/xhci.h   |  5 +++--
 5 files changed, 32 insertions(+), 21 deletions(-)

[PATCH v3] fs/fat/fat.c: Do not perform zero block reads if there are no blocks left

[Jason Wessel]

While using u-boot with qemu's virtio driver I stumbled across a
problem reading files less than sector size.  On the real hardware the
block reader seems ok with reading zero blocks, and while we could fix
the virtio host side of qemu to deal with a zero block read instead of
crashing, the u-boot fat driver should not be doing zero block reads
in the first place.  If you ask hardware to read zero blocks you are
just going to get zero data.  There may also be other hardware that
responds similarly to the virtio interface so this is worth fixing.

Without the patch I get the following and have to restart qemu because
it dies.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
qemu-system-aarch64: virtio: zero sized buffers are not allowed
-

With the patch I get the expected results.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
30 bytes read in 11 ms (2 KiB/s)
=> md.b ${loadaddr} 0x1E
4008: 64 77 63 5f 6f 74 67 2e 6c 70 6d 5f 65 6e 61 62dwc_otg.lpm_enab
40080010: 6c 65 3d 30 20 72 6f 6f 74 77 61 69 74 0a  le=0 rootwait.

-

Signed-off-by: Jason Wessel 

Signed-off-by: Jason Wessel 
---
 fs/fat/fat.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 7fd29470c1..dfad1e910b 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -278,7 +278,10 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, 
unsigned long size)
}
} else {
idx = size / mydata->sect_size;
-   ret = disk_read(startsect, idx, buffer);
+   if (idx == 0)
+   ret = 0;
+   else
+   ret = disk_read(startsect, idx, buffer);
if (ret != idx) {
debug("Error reading data (got %d)\n", ret);
return -1;
-- 
2.17.1

[PATCH] fs/fat/fat.c: Do not perform zero block reads if there are no blocks left

[Jason Wessel]

v2: Fix indentation and remove unneeded braces.

While using u-boot with qemu's virtio driver I stumbled across a
problem reading files less than sector size.  On the real hardware the
block reader seems ok with reading zero blocks, and while we could fix
the virtio host side of qemu to deal with a zero block read instead of
crashing, the u-boot fat driver should not be doing zero block reads
in the first place.  If you ask hardware to read zero blocks you are
just going to get zero data.  There may also be other hardware that
responds similarly to the virtio interface so this is worth fixing.

Without the patch I get the following and have to restart qemu because
it dies.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
qemu-system-aarch64: virtio: zero sized buffers are not allowed
-

With the patch I get the expected results.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
30 bytes read in 11 ms (2 KiB/s)
=> md.b ${loadaddr} 0x1E
4008: 64 77 63 5f 6f 74 67 2e 6c 70 6d 5f 65 6e 61 62dwc_otg.lpm_enab
40080010: 6c 65 3d 30 20 72 6f 6f 74 77 61 69 74 0a  le=0 rootwait.

-

Signed-off-by: Jason Wessel 
---
 fs/fat/fat.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 7fd29470c1..8233a74620 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -278,7 +278,11 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, 
unsigned long size)
}
} else {
idx = size / mydata->sect_size;
-   ret = disk_read(startsect, idx, buffer);
+   if (idx == 0) {
+ ret = 0;
+   } else {
+ ret = disk_read(startsect, idx, buffer);
+   }
if (ret != idx) {
debug("Error reading data (got %d)\n", ret);
return -1;
-- 
2.17.1

[PATCH] fs/fat.c: Do not perform zero block reads if there are no blocks left

[Jason Wessel]

While using u-boot with qemu's virtio driver I stumbled across a
problem reading files less than sector size.  On the real hardware the
block reader seems ok with reading zero blocks, and while we could fix
the virtio host side of qemu to deal with a zero block read instead of
crashing, the u-boot fat driver should not be doing zero block reads
in the first place.  If you ask hardware to read zero blocks you are
just going to get zero data.  There may also be other hardware that
responds similarly to the virtio interface so this is worth fixing.

Without the patch I get the following and have to restart qemu because
it dies.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
qemu-system-aarch64: virtio: zero sized buffers are not allowed
-

With the patch I get the expected results.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
30 bytes read in 11 ms (2 KiB/s)
=> md.b ${loadaddr} 0x1E
4008: 64 77 63 5f 6f 74 67 2e 6c 70 6d 5f 65 6e 61 62dwc_otg.lpm_enab
40080010: 6c 65 3d 30 20 72 6f 6f 74 77 61 69 74 0a  le=0 rootwait.

-

Signed-off-by: Jason Wessel 
---
 fs/fat/fat.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 7fd29470c1..8233a74620 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -278,7 +278,11 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, 
unsigned long size)
}
} else {
idx = size / mydata->sect_size;
-   ret = disk_read(startsect, idx, buffer);
+   if (idx == 0) {
+ ret = 0;
+   } else {
+ ret = disk_read(startsect, idx, buffer);
+   }
if (ret != idx) {
debug("Error reading data (got %d)\n", ret);
return -1;
-- 
2.17.1

[PATCH] fs/fat.c: Do not perform zero block reads if there are no blocks left

[Jason Wessel]

While using u-boot with qemu's virtio driver I stumbled across a
problem reading files less than sector size.  On the real hardware the
block reader seems ok with reading zero blocks, and while we could fix
the virtio host side of qemu to deal with a zero block read instead of
crashing, the u-boot fat driver should not be doing zero block reads
in the first place.  If you ask hardware to read zero blocks you are
just going to get zero data.  There may also be other hardware that
responds similarly to the virtio interface so this is worth fixing.

Without the patch I get the following and have to restart qemu because
it dies.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
qemu-system-aarch64: virtio: zero sized buffers are not allowed
-

With the patch I get the expected results.
-
=> fatls virtio 0:1
   30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
30 bytes read in 11 ms (2 KiB/s)
=> md.b ${loadaddr} 0x1E
4008: 64 77 63 5f 6f 74 67 2e 6c 70 6d 5f 65 6e 61 62dwc_otg.lpm_enab
40080010: 6c 65 3d 30 20 72 6f 6f 74 77 61 69 74 0a  le=0 rootwait.

-

Signed-off-by: Jason Wessel 
---
 fs/fat/fat.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 7fd29470c1..8233a74620 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -278,7 +278,11 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, 
unsigned long size)
}
} else {
idx = size / mydata->sect_size;
-   ret = disk_read(startsect, idx, buffer);
+   if (idx == 0) {
+ ret = 0;
+   } else {
+ ret = disk_read(startsect, idx, buffer);
+   }
if (ret != idx) {
debug("Error reading data (got %d)\n", ret);
return -1;
-- 
2.17.1