From: Pawel Laszczak <pawell@cadence.com>
To: 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 Chan <peter.chan@nxp.com>
Subject: RE: [PATCH] usb: cdns3: Fix: ARM core hang after connect/disconnect operation.
Date: Thu, 9 Jan 2020 08:34:12 +0000 [thread overview]
Message-ID: <BYAPR07MB4709AA109700B4BCAD1C1ED8DD390@BYAPR07MB4709.namprd07.prod.outlook.com> (raw)
In-Reply-To: <20200109063841.GA2579094@kroah.com>
>
>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.
I will talk with HW team about such cases. Maybe in the feature such improvement
will be necessary.
It's important fix for our customer so I will delete this checking for now.
>
>> For timeouts used in driver I've never see an errors. Because debugging these
>> kind of errors is very hard I decided to leave dev_err in such case to inform that
>> probably something is wrong in HW project.
>>
>> I will remove unlikely.
>>
>> >> + dev_err(priv_dev->dev, "Timeout: %s resetting failed.\n",
>> >> + priv_ep->name);
>> >> +
>> >> + priv_ep->flags |= EP_HW_RESETED;
>> >
>> >So an error happens, but you still claim the device is reset? What can
>> >a user do about this error?
>>
>> The error should never occur.
>
>Then no need to warn anyone, just wait and move on.
>
>Or properly handle it.
>
>thanks,
>
>greg k-h
Thanks,
Pawell
next prev parent reply other threads:[~2020-01-09 8:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200108113719.21551-1-pawell@cadence.com>
[not found] ` <20200108142250.GA2383861@kroah.com>
2020-01-09 4:16 ` [PATCH] usb: cdns3: Fix: ARM core hang after connect/disconnect operation Pawel Laszczak
[not found] ` <20200108142829.GB2383861@kroah.com>
2020-01-09 6:27 ` Pawel Laszczak
2020-01-09 6:38 ` Greg KH
2020-01-09 8:34 ` Pawel Laszczak [this message]
2020-01-09 9:38 ` Greg KH
2020-01-14 8:57 ` Peter Chen
2020-01-14 9:06 ` Pawel Laszczak
2020-01-14 9:09 ` Pawel Laszczak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BYAPR07MB4709AA109700B4BCAD1C1ED8DD390@BYAPR07MB4709.namprd07.prod.outlook.com \
--to=pawell@cadence.com \
--cc=felipe.balbi@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=jbergsagel@ti.com \
--cc=jpawar@cadence.com \
--cc=kurahul@cadence.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=nm@ti.com \
--cc=nsekhar@ti.com \
--cc=peter.chan@nxp.com \
--cc=rogerq@ti.com \
--cc=sparmar@cadence.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).