On Thu, Jan 9, 2020 at 5:39 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, Jan 09, 2020 at 08:34:12AM +0000, Pawel Laszczak wrote: > > > > > >On Thu, Jan 09, 2020 at 06:27:02AM +0000, Pawel Laszczak wrote: > > >> >> + writel(EP_CMD_EPRST, &priv_dev->regs->ep_cmd); > > >> >> + > > >> >> + ret = readl_poll_timeout_atomic(&priv_dev->regs->ep_cmd, val, > > >> >> + !(val & (EP_CMD_CSTALL | EP_CMD_EPRST)), > > >> >> + 1, 1000); > > >> >> + > > >> >> + if (unlikely(ret)) > > >> > > > >> >Unless you can measure the difference of using/not using a > > >> >unlikely/likely mark, NEVER use it. The compiler and cpu can almost > > >> >always do better than you can, we have the tests to prove it. > > >> > > > >> > > >> The both of the above timeout should never occur. If they occurred it would be a > > >> critical controller bug. In this case driver can only inform about this event. > > > > > >"Should never occur" is a fun thing to say :) > > > > > >If it can never occur, then don't even check for it. > > > > Yes, on existing platforms it can never occur. > > > > > > > >If it can, then check for it and handle it properly. > > > > > >What about this controller in systems with removable busses (like PCI?) > > >What happens then (hint, I bet this could occur...) > > > > It's good question. Nobody from our customer currently use such system. > > The only platform with PCI is used by me for testing purpose. > > So if you do have a PCI device, then you need to handle PCI reads > failing and returning all 1s. Hopefully you can gracefully handle this :) > > Adding timeout handling here, where it is totally obvious to do so, > would be a good thing. > > thanks, > > greg k-h Hi Pawel, My email is: peter.chen@xxxxxxx, please change it when your send next version. Peter
