[U-Boot] [PATCH v3 2/6] usb: xhci: return ERR_PTR(-ETIMEDOUT) at the end of xhci_wait_for_event()

[Masahiro Yamada]

Bin, Marek,

Would you take a look at xHCI code?

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.masahiro at 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.  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.
>
> 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.masahiro at 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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot



-- 
Best Regards
Masahiro Yamada