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