linux-kernel - Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5134D210.3060405@wwwdotorg.org>
Date:	Mon, 04 Mar 2013 09:55:44 -0700
From:	Stephen Warren <swarren@...dotorg.org>
To:	Venu Byravarasu <vbyravarasu@...dia.com>
CC:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"stern@...land.harvard.edu" <stern@...land.harvard.edu>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init

On 03/04/2013 12:55 AM, Venu Byravarasu wrote:
> Stephen Warren wrote at Thursday, February 28, 2013 11:47 PM:
>> On 02/27/2013 11:36 PM, Venu Byravarasu wrote:
>>> To clear any configurations made by U-Boot on Tegra USB controller,
>>> reset it before init in probe.
>>
>>> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
>>
>>> @@ -691,6 +692,10 @@ static int tegra_ehci_probe(struct platform_device
>> *pdev)
>>>  	if (err)
>>>  		goto fail_clk;
>>>
>>> +	tegra_periph_reset_assert(tegra->clk);
>>> +	udelay(1);
>>> +	tegra_periph_reset_deassert(tegra->clk);
>>
>> I think this patch might cause unintended consequences.
>>
>> When the Tegra PHY code is converted to a driver (i.e. has its own
>> probe), the initial order of execution of the PHY and EHCI driver probes
>> will not be guaranteed.
>>
>> In particular, since the EHCI probe will attempt to "find" the PHY
>> device, and defer the EHCI probe until it can do so, this guarantees
>> that the PHY's probe() will have completed before EHCI's probe()
>> completes (although EHCI's probe may start running first some number of
>> times, and be retried with -EPROBE_DEFERRED for a variety of reasons).
>>
>> Now, if the PHY driver's probe() actually touches HW and sets up some
>> registers, isn't this reset call going to trash any of that register
>> setup? Or, will PHY probe() not touch registers, but only do so during
>> the standard PHY open/init "op"/API calls?
>  
> Yes, PHY driver probe does not touch any registers. It just sets up the PHY API hooks.
> These APIs will be called from ehci-tegra.c as part of ehci tegra probe function, after
> getting  PHY handle, which in turn happens after issuing above reset.
> 
> Thanks to Stephen & Alan, for the review comments.

OK, in that case I have no objection to this patch.

I'd like to hold off on applying this though; I suspect I'll want to
take the Tegra USB patches through the Tegra tree rather than the USB
tree again for the 3.10 kernel cycle. I think I may have screwed the
pooch on the DT binding I set up for the USB controller clocks, and
fixing this may require some Tegra DT changes, which would be easiest
taken through the Tegra tree, and so to reduce conflicts in the USB
code, taking the rest through there migth just be easiest.

Alan, Greg, if you're OK with this patch now, or for any revised
version, an Ack so I can take it through the Tegra tree would be great,
thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/