From: Stephen Warren <swarren@wwwdotorg.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Tuomas Tynkkynen <ttynkkynen@nvidia.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Felipe Balbi <balbi@ti.com>,
Philipp Zabel <p.zabel@pengutronix.de>,
linux-usb@vger.kernel.org, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, tuomas.tynkkynen@iki.fi
Subject: Re: [PATCH 0/3] Tegra USB probe order issue fix
Date: Wed, 02 Jul 2014 10:18:22 -0600 [thread overview]
Message-ID: <53B430CE.5040905@wwwdotorg.org> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1407021204570.874-100000@iolanthe.rowland.org>
On 07/02/2014 10:09 AM, Alan Stern wrote:
> On Wed, 2 Jul 2014, Stephen Warren wrote:
>
>> On 07/02/2014 08:02 AM, Alan Stern wrote:
>>> On Wed, 2 Jul 2014, Tuomas Tynkkynen wrote:
>>>
>>>> Hi all,
>>>>
>>>> This series fixes a probe order issue with the Tegra EHCI driver.
>>>> Basically, the register area of the 1st USB controller contains some
>>>> registers that are global to all of the controllers, but that are also
>>>> cleared when reset is asserted to the 1st controller. So if (say) the
>>>> 3rd controller would be the first one to finish probing successfully,
>>>> then the reset that happens during the 1st controller's probe would
>>>> result in broken USB. So the before doing anything with the USB HW,
>>>> we should reset the 1st controller once, and then never ever reset
>>>> it again.
>>>
>>> This sounds very much like the sort of thing that ought to be described
>>> in DT. It is a hardware dependence, and DT exists for the purpose of
>>> describing the hardware.
>>
>> DT is more about describing the HW, not how SW has to use the HW.
>
> Tuomas wrote: "the register area of the 1st USB controller contains
> some registers that are global to all of the controllers, but that are
> also cleared when reset is asserted to the 1st controller." That is
> very much an attribute of the hardware and so DT should describe it.
So you want to add a Boolean DT property to the first USB controller
node indicating that it has the "shared" reset? That would be fine, but
would only replace the content of tegra_find_usb1_node(); much of the
rest of the patch would remain the same.
I guess in this case, it's fine to require DT changes to enable the new
feature of fixing this issue, so long as the code continues to work the
same as it currently does with old DTs. Due to the backwards
compatibility requirement, the patch will end up slightly more complex,
but hopefully not too bad.
>> probe() ordering is a SW issue, not a HW description. It's driver
>> knowledge that the HW resets have to run in a certain order, and if the
>> driver didn't actually reset the HW ever (but rather, re-programmed all
>> registers so reset was never needed), then order wouldn't be relevant
>>
>> DT certainly doesn't have any mechanism for describing probe order or
>> anything like that, although you can fake it out by adding phandles
>> between nodes, and having SW wait for the driver for the referenced node
>> to probe first. That won't work here though, since there's no guarantee
>> that the USB1 node will actually be enabled (that USB port might not be
>> hooked up on the board, hence the DT node will be disabled), so we can't
>> rely on a driver for it ever appearing.
>
> I wasn't talking about probe order; I was talking about the fact that
> registers pertinent to the later controllers are in the reset domain of
> the first controller.
>
> Alan Stern
next prev parent reply other threads:[~2014-07-02 16:18 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-01 21:08 [PATCH 0/3] Tegra USB probe order issue fix Tuomas Tynkkynen
2014-07-01 21:08 ` [PATCH 1/3] reset: Re-export of_reset_control_get Tuomas Tynkkynen
2014-07-01 21:08 ` [PATCH 2/3] USB: EHCI: tegra: Fix probe order issue leading to broken USB Tuomas Tynkkynen
2014-07-01 21:08 ` [PATCH 3/3] USB: PHY: tegra: Call tegra_usb_phy_close only on device removal Tuomas Tynkkynen
2014-07-01 22:21 ` Stephen Warren
2014-07-02 14:02 ` [PATCH 0/3] Tegra USB probe order issue fix Alan Stern
2014-07-02 15:45 ` Stephen Warren
2014-07-02 16:09 ` Alan Stern
2014-07-02 16:18 ` Stephen Warren [this message]
2014-07-02 17:45 ` Alan Stern
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=53B430CE.5040905@wwwdotorg.org \
--to=swarren@wwwdotorg.org \
--cc=balbi@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=stern@rowland.harvard.edu \
--cc=ttynkkynen@nvidia.com \
--cc=tuomas.tynkkynen@iki.fi \
/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