On 3/23/20 2:03 PM, Lukasz Majewski wrote: > Hi Marek, Hi,
>> On 3/23/20 8:00 AM, Lukasz Majewski wrote: >>> Hi Marek, >>> >>>> On 3/22/20 2:00 PM, Lukasz Majewski wrote: >>>>> This change brings support for handling XACTERR error during DATA >>>>> phase of USB BBB (bulk) transmission. >>>>> >>>>> The fix is to clear stall'ed endpoint and reset recovery on the >>>>> USB mass storage class. >>>>> >>>>> 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 <lu...@denx.de> >>>>> [Unfortunately, the original patch [2] did not contain S-o-B from >>>>> the original author - "rayvt"] >>>>> --- >>>>> >>>>> common/usb_storage.c | 10 ++++++++++ >>>>> include/usb_defs.h | 1 + >>>>> 2 files changed, 11 insertions(+) >>>>> >>>>> diff --git a/common/usb_storage.c b/common/usb_storage.c >>>>> index 097b6729c1..92e1e54d1c 100644 >>>>> --- a/common/usb_storage.c >>>>> +++ b/common/usb_storage.c >>>>> @@ -740,6 +740,16 @@ static int usb_stor_BBB_transport(struct >>>>> scsi_cmd *srb, struct us_data *us) >>>>> result = usb_bulk_msg(us->pusb_dev, pipe, srb->pdata, >>>>> srb->datalen, &data_actlen, USB_CNTL_TIMEOUT * 5); >>>>> + >>>>> + /* special handling of XACTERR in DATA phase */ >>>>> + if (result < 0 && (us->pusb_dev->status & >>>>> USB_ST_XACTERR)) { >>>>> + debug("XACTERR in data phase - clr, reset, and >>>>> return fail.\n"); >>>>> + usb_stor_BBB_clear_endpt_stall(us, dir_in ? >>>>> + us->ep_in : >>>>> us->ep_out); >>>>> + usb_stor_BBB_reset(us); >>>>> + return USB_STOR_TRANSPORT_FAILED; >>>>> + } >>>>> + >>>> >>>> Can resetting the endpoint result in data corruption of some sort >>>> here ? >>> >>> Please correct me if I'm wrong, but this code is executed when we do >>> receive data, not writing them. Those operations shall (and are?) >>> orthogonal. >>> >>>> Especially if this happens on filesystem write for example ? >>> >>> Do we write data here? >> >> I only did a very quick look into the code, but there I see >> >> 1082 static int usb_write_10(struct scsi_cmd *srb, struct us_data *ss, >> ... >> 1096 return ss->transport(srb, ss); >> >> 1338 case US_PR_BULK: >> 1339 debug("Bulk/Bulk/Bulk\n"); >> 1340 ss->transport = usb_stor_BBB_transport; >> >> So I would say, the answer is -- yes. >> > > A few lines down (usb_storage.c @ L757) you have the USB_ST_STALLED > status handled in the same way (it also aborts after a single retry). But stall isn't xfer error. Which makes me wonder, why is the xfer error here handled the same way as a stall, shouldn't the handling be different ? Also, this doesn't answer the question whether such handling can cause data corruption during write.
