[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200109093812.GC44349@kroah.com>
Date: Thu, 9 Jan 2020 10:38:12 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Pawel Laszczak <pawell@...ence.com>
Cc: "felipe.balbi@...ux.intel.com" <felipe.balbi@...ux.intel.com>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"rogerq@...com" <rogerq@...com>,
"jbergsagel@...com" <jbergsagel@...com>,
"nsekhar@...com" <nsekhar@...com>, "nm@...com" <nm@...com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Jayshri Dajiram Pawar <jpawar@...ence.com>,
Rahul Kumar <kurahul@...ence.com>,
Sanket Parmar <sparmar@...ence.com>,
Peter Chan <peter.chan@....com>
Subject: Re: [PATCH] usb: cdns3: Fix: ARM core hang after connect/disconnect
operation.
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
Powered by blists - more mailing lists