> -----Original Message-----
> From: Stephen Warren [mailto:[email protected]]
> Sent: Thursday, February 28, 2013 11:47 PM
> To: Venu Byravarasu
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
>
> 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.
>
> I think the way to solve this is to put the reset call into the PHY
> driver. I assume it has access to the appropriate clock object.
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.
On Mon, 4 Mar 2013, Stephen Warren wrote:
> 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);
> 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.
I have no other objections.
Acked-by: Alan Stern <[email protected]>
Alan Stern
On Mon, Mar 04, 2013 at 09:55:44AM -0700, Stephen Warren wrote:
> 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.
Acked-by: Greg Kroah-Hartman <[email protected]>
> -----Original Message-----
> From: [email protected] [mailto:[email protected]]
> Sent: Tuesday, March 05, 2013 6:04 AM
> To: Stephen Warren
> Cc: Venu Byravarasu; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
>
> On Mon, Mar 04, 2013 at 09:55:44AM -0700, Stephen Warren wrote:
> > 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);
> > >>
> > 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.
>
> Acked-by: Greg Kroah-Hartman <[email protected]>
Hi Greg,
Stephen initially thought that the patch can be taken through tegra tree, along with
other patches posted at http://marc.info/?l=linux-tegra&m=136361016003625&w=2 .
As some other issues are blocking that patch series, can this change be merged
independent of that patch series via USB tree?
The original change can be seen at http://marc.info/?l=linux-usb&m=136203353205291&w=2
Thanks,
Venu
On 04/02/2013 01:12 AM, Venu Byravarasu wrote:
>> [email protected] wrote at Tuesday, March 05, 2013 6:04 AM:
>> On Mon, Mar 04, 2013 at 09:55:44AM -0700, Stephen Warren wrote:
>>> 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.
...
>> Acked-by: Greg Kroah-Hartman <[email protected]>
>
> Hi Greg,
>
> Stephen initially thought that the patch can be taken through tegra tree, along with
> other patches posted at http://marc.info/?l=linux-tegra&m=136361016003625&w=2 .
> As some other issues are blocking that patch series, can this change be merged
> independent of that patch series via USB tree?
>
> The original change can be seen at http://marc.info/?l=linux-usb&m=136203353205291&w=2
Venu, you should simply repost the patch; I'm sure its not in anyone's
email inbox any more.
> -----Original Message-----
> From: Stephen Warren [mailto:[email protected]]
> Sent: Tuesday, April 02, 2013 9:02 PM
> To: Venu Byravarasu
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
>
> On 04/02/2013 01:12 AM, Venu Byravarasu wrote:
> >> [email protected] wrote at Tuesday, March 05, 2013 6:04 AM:
> >> On Mon, Mar 04, 2013 at 09:55:44AM -0700, Stephen Warren wrote:
> >>> 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.
> ...
> >> Acked-by: Greg Kroah-Hartman <[email protected]>
> >
> > Hi Greg,
> >
> > Stephen initially thought that the patch can be taken through tegra tree,
> along with
> > other patches posted at http://marc.info/?l=linux-
> tegra&m=136361016003625&w=2 .
> > As some other issues are blocking that patch series, can this change be
> merged
> > independent of that patch series via USB tree?
> >
> > The original change can be seen at http://marc.info/?l=linux-
> usb&m=136203353205291&w=2
>
> Venu, you should simply repost the patch; I'm sure its not in anyone's
> email inbox any more.
Resent the patch, with ACKs added.
Latest patch can be seen at: http://marc.info/?l=linux-tegra&m=136498598329081&w=2
Thanks,
Venu