On 01/28/2018 11:18 AM, Masahiro Yamada wrote:
> Bin, Marek,
Hi,
> Would you take a look at xHCI code?
I was kinda hoping Bin would, oh well ...
> Lots of xHCI code invokes BUG_ON()/BUG()
> when the hardware access fails,
> then halts the system completely.
> This is not a software bug.
>
>
> 2018-01-10 10:45 GMT+09:00 Masahiro Yamada <yamada.masah...@socionext.com>:
>> xhci_wait_for_event() is supposed to return a pointer to union xhci_trb,
>> but it does not return anything at the end of the function.
>>
>> This relies on that the end of the function is unreachable due to BUG().
>>
>> We are planning to make BUG() no-op for platforms with strong image size
>> constraint.
But BUG() should hang the system because it means an unrecoverable issue
happened. Changing it to nop would cause a lot of weird misbehavior, so
that is IMO a bad idea. Just change it to hang(), that should be present
even on size-constrained systems.
>> Doing so would cause compiler warning:
>>
>> drivers/usb/host/xhci-ring.c:475:1: warning: control reaches end of non-void
>> function [-Wreturn-type]
>> }
>> ^
>>
>> So, this function must return something. From the error message just
>> above, ERR_PTR(-ETIMEDOUT) seems a good choice.
This is an imported code from Linux, so you might want to check what
they do/did there.
>> The use of BUG() looks suspicious here in the first place; no response
>> from hardware is not a bug. It should be treated as a normal error.
>> So, this function must return an error pointer instead of BUG(), then
>> the caller must handle it properly.
>>
>> I am not fixing the code because this is not the only place that stops
>> the system. Just one failure of xHCI halts the system, here and there.
>>
>> I left a comment block, hoping somebody will take a look.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>> ---
>>
>> Changes in v3:
>> - newly added
>>
>> Changes in v2: None
>>
>> drivers/usb/host/xhci-ring.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index 579e670..d780367 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -471,7 +471,14 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl
>> *ctrl, trb_type expected)
>> return NULL;
>>
>> printf("XHCI timeout on event type %d... cannot recover.\n",
>> expected);
>> +
>> + /*
>> + * CHECK:
>> + * Is this software bug? Is this a good reason to halt the system?
>> + */
>> BUG();
>> +
>> + return ERR_PTR(-ETIMEDOUT);
>> }
>>
>> /*
>> --
>> 2.7.4
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>
>
>
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
