RE: [PATCH] usb: cdns3: Fix: ARM core hang after connect/disconnect operation.

From: Pawel Laszczak <pawell@cadence.com>
To: Peter Chen <hzpeterchen@gmail.com>, Greg KH <gregkh@linuxfoundation.org>
Cc: "felipe.balbi@linux.intel.com" <felipe.balbi@linux.intel.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"rogerq@ti.com" <rogerq@ti.com>,
	"jbergsagel@ti.com" <jbergsagel@ti.com>,
	"nsekhar@ti.com" <nsekhar@ti.com>, "nm@ti.com" <nm@ti.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Jayshri Dajiram Pawar <jpawar@cadence.com>,
	Rahul Kumar <kurahul@cadence.com>,
	Sanket Parmar <sparmar@cadence.com>,
	"peter.chen@nxp.com" <peter.chen@nxp.com>
Subject: RE: [PATCH] usb: cdns3: Fix: ARM core hang after connect/disconnect operation.
Date: Tue, 14 Jan 2020 09:09:41 +0000	[thread overview]
Message-ID: <BYAPR07MB470980796F605EF38D9E1C60DD340@BYAPR07MB4709.namprd07.prod.outlook.com> (raw)
In-Reply-To: <CAL411-pE7QheAEQf+ki8auChWP6MOWxaaAA3xkkdweWn=q_0wA@mail.gmail.com>

>
>On Thu, Jan 9, 2020 at 5:39 PM Greg KH <gregkh@linuxfoundation.org> 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@nxp.com, please change it when your send next version.

Hi Peter, 

I'm sorry for that.

BTW. I need postpone sending the next version of this patch because there is some issue with synchronization of repositories  regarding the patch "usb: cdns3: Add streams support to cadence USB3 DRD driver"
I don't want generate new conflicts. 

Regards,
Pawell

>
>Peter