[RFT PATCH v1 5/5] usb: Handle QT_TOKEN_STATUS_XACTERR error when sending data

[Marek Vasut]

On 3/23/20 1:58 PM, Lukasz Majewski wrote:
> Hi Marek,

Hi,

>> On 3/23/20 8:18 AM, Lukasz Majewski wrote:
>>> Hi Marek,  
>>
>> Hi,
>>
>>>> On 3/22/20 2:00 PM, Lukasz Majewski wrote:  
>>>>> This code adds check if QT_TOKEN_STATUS_XACTERR error occurred.
>>>>> When it is detected the token is reconfigured and transmission is
>>>>> retried.
>>>>>
>>>>> This code is the port to newest U-Boot of the fix from - "rayvt"
>>>>> (from [1]).
>>>>>
>>>>> Links:
>>>>> [1] - https://forum.doozan.com/read.php?3,35295,35295#msg-35295
>>>>> [2] -
>>>>> https://www.dropbox.com/s/nrkrd1no63viuu8/uboot-bodhi-2016.05-timeoutTD.patch?dl=0
>>>>>
>>>>>
>>>>> Signed-off-by: Lukasz Majewski <lukma at denx.de>
>>>>> [Unfortunately, the original patch [2] did not contain S-o-B from
>>>>> the original author - "rayvt"]
>>>>> ---
>>>>>
>>>>>  drivers/usb/host/ehci-hcd.c | 27 ++++++++++++++++++++++++++-
>>>>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/usb/host/ehci-hcd.c
>>>>> b/drivers/usb/host/ehci-hcd.c index 0a77111f80..45eda7ad24 100644
>>>>> --- a/drivers/usb/host/ehci-hcd.c
>>>>> +++ b/drivers/usb/host/ehci-hcd.c
>>>>> @@ -315,6 +315,7 @@ ehci_submit_async(struct usb_device *dev,
>>>>> unsigned long pipe, void *buffer, int timeout;
>>>>>  	int ret = 0;
>>>>>  	struct ehci_ctrl *ctrl = ehci_get_ctrl(dev);
>>>>> +	int trynum;
>>>>>  
>>>>>  	debug("dev=%p, pipe=%lx, buffer=%p, length=%d, req=%p\n",
>>>>> dev, pipe, buffer, length, req);
>>>>> @@ -560,6 +561,10 @@ ehci_submit_async(struct usb_device *dev,
>>>>> unsigned long pipe, void *buffer,
>>>>> ehci_writel(&ctrl->hcor->or_usbsts, (usbsts & 0x3f)); 
>>>>>  	/* Enable async. schedule. */
>>>>> +	trynum = 1;	/* No more than 2 tries, in case of
>>>>> XACTERR. */ +retry_xacterr:;
>>>>> +	vtd = &qtd[qtd_counter - 1];
>>>>> +
>>>>>  	cmd = ehci_readl(&ctrl->hcor->or_usbcmd);
>>>>>  	cmd |= CMD_ASE;
>>>>>  	ehci_writel(&ctrl->hcor->or_usbcmd, cmd);    
>>>>
>>>> Are you sure always retrying the transfer if you get XACTERR is the
>>>> right thing to do, even if you get that e.g. on filesystem writes
>>>> ?  
>>>
>>> Please correct me if I'm wrong, but this function -
>>> ehci_submit_async
>>> - doesn't work with filesystem. It just setups proper EHCI
>>> descriptor and checks if it was sent with or without errors.  
>>
>> If you're writing into a block device, this function is used. If you
>> get XACTERR during the transfer and you retry, is there a chance the
>> data get actually written to the device ?
> 
> As fair as I can tell this code snippet doesn't touch file systems.

It could be any sort of write into block storage, it does not have to be
filesystem. The filesystem was just an example of write type we trigger
more often.

>>> When the XACTERR happens, proper flag is cleared and the
>>> transmission is retried.  
>>
>> What happens to the payload if it's a host-to-device transfer, do the
>> data get discarded or do they get written to the storage ?
>>
> 
> As state above - the resent shall only result is data being to some
> in-memory buffer. 

If you trigger a write from device memory to remote storage, then it is
the other way around.