2018-02-22 23:06:41

by Marcel Ziswiler

[permalink] [raw]
Subject: [PATCH] clk: tegra: fix pllu rate configuration

Turns out latest upstream U-Boot does not configure/enable pllu which
leaves it at some default rate of 500 kHz:

root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep pll_u
pll_u 3 3 0 500000 0

Of course this won't quite work leading to the following messages:

[ 6.559593] usb 2-1: new full-speed USB device number 2 using tegra-
ehci
[ 11.759173] usb 2-1: device descriptor read/64, error -110
[ 27.119453] usb 2-1: device descriptor read/64, error -110
[ 27.389217] usb 2-1: new full-speed USB device number 3 using tegra-
ehci
[ 32.559454] usb 2-1: device descriptor read/64, error -110
[ 47.929777] usb 2-1: device descriptor read/64, error -110
[ 48.049658] usb usb2-port1: attempt power cycle
[ 48.759475] usb 2-1: new full-speed USB device number 4 using tegra-
ehci
[ 59.349457] usb 2-1: device not accepting address 4, error -110
[ 59.509449] usb 2-1: new full-speed USB device number 5 using tegra-
ehci
[ 70.069457] usb 2-1: device not accepting address 5, error -110
[ 70.079721] usb usb2-port1: unable to enumerate USB device

Fix this by actually allowing the rate also being set from within
the Linux kernel.

Signed-off-by: Marcel Ziswiler <[email protected]>

---

drivers/clk/tegra/clk-pll.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 7c369e21c91c..830d1c87fa7c 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -1151,6 +1151,8 @@ static const struct clk_ops tegra_clk_pllu_ops = {
.enable = clk_pllu_enable,
.disable = clk_pll_disable,
.recalc_rate = clk_pll_recalc_rate,
+ .round_rate = clk_pll_round_rate,
+ .set_rate = clk_pll_set_rate,
};

static int _pll_fixed_mdiv(struct tegra_clk_pll_params *pll_params,
--
2.14.3



2018-02-22 23:49:27

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On 02/22/2018 04:04 PM, Marcel Ziswiler wrote:
> Turns out latest upstream U-Boot does not configure/enable pllu which
> leaves it at some default rate of 500 kHz:

I assume this is only because U-Boot just happened not to access any USB
devices. In other words, if you break into the U-Boot boot flow and
explicitly initialize/access USB, then USB works both in U-Boot and in
the kernel even without this patch? If that's not the case, it seems
there's a bug in U-Boot.

Either way, I have no issue with this patch. I just want to make sure
there weren't other bugs in U-Boot that needed fixing.

2018-02-23 06:31:10

by Marcel Ziswiler

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration



On February 23, 2018 12:47:56 AM GMT+01:00, Stephen Warren <[email protected]> wrote:
>On 02/22/2018 04:04 PM, Marcel Ziswiler wrote:
>> Turns out latest upstream U-Boot does not configure/enable pllu which
>> leaves it at some default rate of 500 kHz:
>
>I assume this is only because U-Boot just happened not to access any
>USB
>devices. In other words, if you break into the U-Boot boot flow and
>explicitly initialize/access USB, then USB works both in U-Boot and in
>the kernel even without this patch?

Exactly.

> If that's not the case, it seems
>there's a bug in U-Boot.
>
>Either way, I have no issue with this patch. I just want to make sure
>there weren't other bugs in U-Boot that needed fixing.

No, rest assured U-Boot is working just fine on the legacy Tegras. The only thing in need of fixing lately was NAND on Colibri T20 and Harmony.

2018-02-26 12:45:11

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On 23.02.2018 02:04, Marcel Ziswiler wrote:
> Turns out latest upstream U-Boot does not configure/enable pllu which
> leaves it at some default rate of 500 kHz:
>
> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep pll_u
> pll_u 3 3 0 500000 0
>
> Of course this won't quite work leading to the following messages:
>
> [ 6.559593] usb 2-1: new full-speed USB device number 2 using tegra-
> ehci
> [ 11.759173] usb 2-1: device descriptor read/64, error -110
> [ 27.119453] usb 2-1: device descriptor read/64, error -110
> [ 27.389217] usb 2-1: new full-speed USB device number 3 using tegra-
> ehci
> [ 32.559454] usb 2-1: device descriptor read/64, error -110
> [ 47.929777] usb 2-1: device descriptor read/64, error -110
> [ 48.049658] usb usb2-port1: attempt power cycle
> [ 48.759475] usb 2-1: new full-speed USB device number 4 using tegra-
> ehci
> [ 59.349457] usb 2-1: device not accepting address 4, error -110
> [ 59.509449] usb 2-1: new full-speed USB device number 5 using tegra-
> ehci
> [ 70.069457] usb 2-1: device not accepting address 5, error -110
> [ 70.079721] usb usb2-port1: unable to enumerate USB device
>
> Fix this by actually allowing the rate also being set from within
> the Linux kernel.
>
> Signed-off-by: Marcel Ziswiler <[email protected]>
>
> ---
>
> drivers/clk/tegra/clk-pll.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
> index 7c369e21c91c..830d1c87fa7c 100644
> --- a/drivers/clk/tegra/clk-pll.c
> +++ b/drivers/clk/tegra/clk-pll.c
> @@ -1151,6 +1151,8 @@ static const struct clk_ops tegra_clk_pllu_ops = {
> .enable = clk_pllu_enable,
> .disable = clk_pll_disable,
> .recalc_rate = clk_pll_recalc_rate,
> + .round_rate = clk_pll_round_rate,
> + .set_rate = clk_pll_set_rate,
> };
>
> static int _pll_fixed_mdiv(struct tegra_clk_pll_params *pll_params,
>

Tegra's USB PHY driver only enables clock and clk driver doesn't specify the
clock rate in the init table. Could you please clarify where in the kernels code
PLL_U rate is getting set?

2018-02-26 23:05:37

by Marcel Ziswiler

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
> On 23.02.2018 02:04, Marcel Ziswiler wrote:
> > Turns out latest upstream U-Boot does not configure/enable pllu
> > which
> > leaves it at some default rate of 500 kHz:
> >
> > root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
> > pll_u
> > pll_u 3 3 0 500000
> > 0
> >
> > Of course this won't quite work leading to the following messages:
> >
> > [ 6.559593] usb 2-1: new full-speed USB device number 2 using
> > tegra-
> > ehci
> > [ 11.759173] usb 2-1: device descriptor read/64, error -110
> > [ 27.119453] usb 2-1: device descriptor read/64, error -110
> > [ 27.389217] usb 2-1: new full-speed USB device number 3 using
> > tegra-
> > ehci
> > [ 32.559454] usb 2-1: device descriptor read/64, error -110
> > [ 47.929777] usb 2-1: device descriptor read/64, error -110
> > [ 48.049658] usb usb2-port1: attempt power cycle
> > [ 48.759475] usb 2-1: new full-speed USB device number 4 using
> > tegra-
> > ehci
> > [ 59.349457] usb 2-1: device not accepting address 4, error -110
> > [ 59.509449] usb 2-1: new full-speed USB device number 5 using
> > tegra-
> > ehci
> > [ 70.069457] usb 2-1: device not accepting address 5, error -110
> > [ 70.079721] usb usb2-port1: unable to enumerate USB device
> >
> > Fix this by actually allowing the rate also being set from within
> > the Linux kernel.
> >
> > Signed-off-by: Marcel Ziswiler <[email protected]>
> >
> > ---
> >
> > drivers/clk/tegra/clk-pll.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-
> > pll.c
> > index 7c369e21c91c..830d1c87fa7c 100644
> > --- a/drivers/clk/tegra/clk-pll.c
> > +++ b/drivers/clk/tegra/clk-pll.c
> > @@ -1151,6 +1151,8 @@ static const struct clk_ops
> > tegra_clk_pllu_ops = {
> > .enable = clk_pllu_enable,
> > .disable = clk_pll_disable,
> > .recalc_rate = clk_pll_recalc_rate,
> > + .round_rate = clk_pll_round_rate,
> > + .set_rate = clk_pll_set_rate,
> > };
> >
> > static int _pll_fixed_mdiv(struct tegra_clk_pll_params
> > *pll_params,
> >
>
> Tegra's USB PHY driver only enables clock and clk driver doesn't
> specify the
> clock rate in the init table. Could you please clarify where in the
> kernels code
> PLL_U rate is getting set?

I guess that would be according to the following table isn't it:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree
/drivers/clk/tegra/clk-tegra30.c?h=v4.16-rc3#n287

2018-02-27 12:00:45

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On 27.02.2018 02:04, Marcel Ziswiler wrote:
> On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
>> On 23.02.2018 02:04, Marcel Ziswiler wrote:
>>> Turns out latest upstream U-Boot does not configure/enable pllu
>>> which
>>> leaves it at some default rate of 500 kHz:
>>>
>>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
>>> pll_u
>>> pll_u 3 3 0 500000
>>> 0
>>>
>>> Of course this won't quite work leading to the following messages:
>>>
>>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
>>> tegra-
>>> ehci
>>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
>>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
>>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
>>> tegra-
>>> ehci
>>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
>>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
>>> [ 48.049658] usb usb2-port1: attempt power cycle
>>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
>>> tegra-
>>> ehci
>>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
>>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
>>> tegra-
>>> ehci
>>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
>>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
>>>
>>> Fix this by actually allowing the rate also being set from within
>>> the Linux kernel.
>>>
>>> Signed-off-by: Marcel Ziswiler <[email protected]>
>>>
>>> ---
>>>
>>> drivers/clk/tegra/clk-pll.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-
>>> pll.c
>>> index 7c369e21c91c..830d1c87fa7c 100644
>>> --- a/drivers/clk/tegra/clk-pll.c
>>> +++ b/drivers/clk/tegra/clk-pll.c
>>> @@ -1151,6 +1151,8 @@ static const struct clk_ops
>>> tegra_clk_pllu_ops = {
>>> .enable = clk_pllu_enable,
>>> .disable = clk_pll_disable,
>>> .recalc_rate = clk_pll_recalc_rate,
>>> + .round_rate = clk_pll_round_rate,
>>> + .set_rate = clk_pll_set_rate,
>>> };
>>>
>>> static int _pll_fixed_mdiv(struct tegra_clk_pll_params
>>> *pll_params,
>>>
>>
>> Tegra's USB PHY driver only enables clock and clk driver doesn't
>> specify the
>> clock rate in the init table. Could you please clarify where in the
>> kernels code
>> PLL_U rate is getting set?
>
> I guess that would be according to the following table isn't it:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree
> /drivers/clk/tegra/clk-tegra30.c?h=v4.16-rc3#n287
>

I see now that T30 has PLL_U in the init table [0], but T20 doesn't.

There is only one 480MHz rate defined in PLL_U lookup table. PLL_U also provides
12MHz and 60MHz outputs using fixed dividers and hence PLL_U shouldn't need
set/round_rate(), we can't change PLL_U rate at all and only enable / disable it.

Looks like for some reason PLL_U enabling fails without set/round_rate, but then
PHY driver should fail to probe [1]. Is it the case? Do you know why PLL_U isn't
actually getting enabled in HW? It is quite fishy, seems like clk driver /
framework bug or I'm missing something.

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/tegra/clk-tegra30.c?h=v4.16-rc3#n1274

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/phy/phy-tegra-usb.c?h=v4.16-rc3#n777

2018-02-28 09:37:28

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
> On 27.02.2018 02:04, Marcel Ziswiler wrote:
> > On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
> >> On 23.02.2018 02:04, Marcel Ziswiler wrote:
> >>> Turns out latest upstream U-Boot does not configure/enable pllu
> >>> which
> >>> leaves it at some default rate of 500 kHz:
> >>>
> >>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
> >>> pll_u
> >>> pll_u 3 3 0 500000
> >>> 0
> >>>
> >>> Of course this won't quite work leading to the following messages:
> >>>
> >>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
> >>> tegra-
> >>> ehci
> >>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
> >>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
> >>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
> >>> tegra-
> >>> ehci
> >>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
> >>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
> >>> [ 48.049658] usb usb2-port1: attempt power cycle
> >>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
> >>> tegra-
> >>> ehci
> >>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
> >>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
> >>> tegra-
> >>> ehci
> >>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
> >>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
> >>>
> >>> Fix this by actually allowing the rate also being set from within
> >>> the Linux kernel.

I think the best solution to this problem would be to make pll_u a fixed
clock and enable it and program the rate if it's not enabled at boot.
This is how it's done for Tegra210. The reason is that the USB IP blocks
can control the pll_u state in hw. This means that if sw would disable
and then re-enable the pll_u clock, but there is no USB activity, pll_u
will still be disable and therefor not lock, causing an error. Today
this is worked around by not polling the lock bit for pll_u, but a better
solution would be to just remove all sw controls for pll_u.

Peter.

2018-02-28 12:01:36

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On 28.02.2018 12:36, Peter De Schrijver wrote:
> On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
>> On 27.02.2018 02:04, Marcel Ziswiler wrote:
>>> On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
>>>> On 23.02.2018 02:04, Marcel Ziswiler wrote:
>>>>> Turns out latest upstream U-Boot does not configure/enable pllu
>>>>> which
>>>>> leaves it at some default rate of 500 kHz:
>>>>>
>>>>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
>>>>> pll_u
>>>>> pll_u 3 3 0 500000
>>>>> 0
>>>>>
>>>>> Of course this won't quite work leading to the following messages:
>>>>>
>>>>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
>>>>> tegra-
>>>>> ehci
>>>>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
>>>>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
>>>>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
>>>>> tegra-
>>>>> ehci
>>>>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
>>>>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
>>>>> [ 48.049658] usb usb2-port1: attempt power cycle
>>>>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
>>>>> tegra-
>>>>> ehci
>>>>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
>>>>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
>>>>> tegra-
>>>>> ehci
>>>>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
>>>>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
>>>>>
>>>>> Fix this by actually allowing the rate also being set from within
>>>>> the Linux kernel.
>
> I think the best solution to this problem would be to make pll_u a fixed
> clock and enable it and program the rate if it's not enabled at boot.

Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
yesterday.. So set/round_rate() for PLL_U are actually needed and the patch is
correct. Seems only T20 misses PLL_U in the init table, probably worth to add it
there.

> This is how it's done for Tegra210. The reason is that the USB IP blocks
> can control the pll_u state in hw. This means that if sw would disable
> and then re-enable the pll_u clock, but there is no USB activity, pll_u
> will still be disable and therefor not lock, causing an error. Today
> this is worked around by not polling the lock bit for pll_u, but a better
> solution would be to just remove all sw controls for pll_u.

SW controls could be removed, but I don't think it is really necessary as in our
case SW is the PHY driver and we know what it does. Alternatively we can enable
PLL_U in the init table to keep it "always" enabled.

2018-02-28 14:16:04

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On Wed, Feb 28, 2018 at 03:00:23PM +0300, Dmitry Osipenko wrote:
> On 28.02.2018 12:36, Peter De Schrijver wrote:
> > On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
> >> On 27.02.2018 02:04, Marcel Ziswiler wrote:
> >>> On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
> >>>> On 23.02.2018 02:04, Marcel Ziswiler wrote:
> >>>>> Turns out latest upstream U-Boot does not configure/enable pllu
> >>>>> which
> >>>>> leaves it at some default rate of 500 kHz:
> >>>>>
> >>>>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
> >>>>> pll_u
> >>>>> pll_u 3 3 0 500000
> >>>>> 0
> >>>>>
> >>>>> Of course this won't quite work leading to the following messages:
> >>>>>
> >>>>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
> >>>>> tegra-
> >>>>> ehci
> >>>>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
> >>>>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
> >>>>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
> >>>>> tegra-
> >>>>> ehci
> >>>>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
> >>>>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
> >>>>> [ 48.049658] usb usb2-port1: attempt power cycle
> >>>>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
> >>>>> tegra-
> >>>>> ehci
> >>>>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
> >>>>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
> >>>>> tegra-
> >>>>> ehci
> >>>>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
> >>>>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
> >>>>>
> >>>>> Fix this by actually allowing the rate also being set from within
> >>>>> the Linux kernel.
> >
> > I think the best solution to this problem would be to make pll_u a fixed
> > clock and enable it and program the rate if it's not enabled at boot.
>
> Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
> yesterday.. So set/round_rate() for PLL_U are actually needed and the patch is
> correct. Seems only T20 misses PLL_U in the init table, probably worth to add it
> there.
>

AFAIK we only use one rate ever?

> > This is how it's done for Tegra210. The reason is that the USB IP blocks
> > can control the pll_u state in hw. This means that if sw would disable
> > and then re-enable the pll_u clock, but there is no USB activity, pll_u
> > will still be disable and therefor not lock, causing an error. Today
> > this is worked around by not polling the lock bit for pll_u, but a better
> > solution would be to just remove all sw controls for pll_u.
>
> SW controls could be removed, but I don't think it is really necessary as in our
> case SW is the PHY driver and we know what it does. Alternatively we can enable
> PLL_U in the init table to keep it "always" enabled.

2018-02-28 17:22:42

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On 28.02.2018 17:14, Peter De Schrijver wrote:
> On Wed, Feb 28, 2018 at 03:00:23PM +0300, Dmitry Osipenko wrote:
>> On 28.02.2018 12:36, Peter De Schrijver wrote:
>>> On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
>>>> On 27.02.2018 02:04, Marcel Ziswiler wrote:
>>>>> On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
>>>>>> On 23.02.2018 02:04, Marcel Ziswiler wrote:
>>>>>>> Turns out latest upstream U-Boot does not configure/enable pllu
>>>>>>> which
>>>>>>> leaves it at some default rate of 500 kHz:
>>>>>>>
>>>>>>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
>>>>>>> pll_u
>>>>>>> pll_u 3 3 0 500000
>>>>>>> 0
>>>>>>>
>>>>>>> Of course this won't quite work leading to the following messages:
>>>>>>>
>>>>>>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
>>>>>>> tegra-
>>>>>>> ehci
>>>>>>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
>>>>>>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
>>>>>>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
>>>>>>> tegra-
>>>>>>> ehci
>>>>>>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
>>>>>>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
>>>>>>> [ 48.049658] usb usb2-port1: attempt power cycle
>>>>>>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
>>>>>>> tegra-
>>>>>>> ehci
>>>>>>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
>>>>>>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
>>>>>>> tegra-
>>>>>>> ehci
>>>>>>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
>>>>>>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
>>>>>>>
>>>>>>> Fix this by actually allowing the rate also being set from within
>>>>>>> the Linux kernel.
>>>
>>> I think the best solution to this problem would be to make pll_u a fixed
>>> clock and enable it and program the rate if it's not enabled at boot.
>>
>> Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
>> yesterday.. So set/round_rate() for PLL_U are actually needed and the patch is
>> correct. Seems only T20 misses PLL_U in the init table, probably worth to add it
>> there.
>>
>
> AFAIK we only use one rate ever?

IIUC, PLL_U has 3 outputs and output dividers are fixed in HW. So yes, we are
setting PLL_U to one rate - 480MHz to get out1-480MHz, out2-60MHz and out3-12MHz.

>>> This is how it's done for Tegra210. The reason is that the USB IP blocks
>>> can control the pll_u state in hw. This means that if sw would disable
>>> and then re-enable the pll_u clock, but there is no USB activity, pll_u
>>> will still be disable and therefor not lock, causing an error. Today
>>> this is worked around by not polling the lock bit for pll_u, but a better
>>> solution would be to just remove all sw controls for pll_u.
>>
>> SW controls could be removed, but I don't think it is really necessary as in our
>> case SW is the PHY driver and we know what it does. Alternatively we can enable
>> PLL_U in the init table to keep it "always" enabled.


2018-03-01 07:44:05

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On Wed, Feb 28, 2018 at 08:20:47PM +0300, Dmitry Osipenko wrote:
> On 28.02.2018 17:14, Peter De Schrijver wrote:
> > On Wed, Feb 28, 2018 at 03:00:23PM +0300, Dmitry Osipenko wrote:
> >> On 28.02.2018 12:36, Peter De Schrijver wrote:
> >>> On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
> >>>> On 27.02.2018 02:04, Marcel Ziswiler wrote:
> >>>>> On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
> >>>>>> On 23.02.2018 02:04, Marcel Ziswiler wrote:
> >>>>>>> Turns out latest upstream U-Boot does not configure/enable pllu
> >>>>>>> which
> >>>>>>> leaves it at some default rate of 500 kHz:
> >>>>>>>
> >>>>>>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
> >>>>>>> pll_u
> >>>>>>> pll_u 3 3 0 500000
> >>>>>>> 0
> >>>>>>>
> >>>>>>> Of course this won't quite work leading to the following messages:
> >>>>>>>
> >>>>>>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
> >>>>>>> tegra-
> >>>>>>> ehci
> >>>>>>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
> >>>>>>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
> >>>>>>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
> >>>>>>> tegra-
> >>>>>>> ehci
> >>>>>>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
> >>>>>>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
> >>>>>>> [ 48.049658] usb usb2-port1: attempt power cycle
> >>>>>>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
> >>>>>>> tegra-
> >>>>>>> ehci
> >>>>>>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
> >>>>>>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
> >>>>>>> tegra-
> >>>>>>> ehci
> >>>>>>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
> >>>>>>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
> >>>>>>>
> >>>>>>> Fix this by actually allowing the rate also being set from within
> >>>>>>> the Linux kernel.
> >>>
> >>> I think the best solution to this problem would be to make pll_u a fixed
> >>> clock and enable it and program the rate if it's not enabled at boot.
> >>
> >> Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
> >> yesterday.. So set/round_rate() for PLL_U are actually needed and the patch is
> >> correct. Seems only T20 misses PLL_U in the init table, probably worth to add it
> >> there.
> >>
> >
> > AFAIK we only use one rate ever?
>
> IIUC, PLL_U has 3 outputs and output dividers are fixed in HW. So yes, we are
> setting PLL_U to one rate - 480MHz to get out1-480MHz, out2-60MHz and out3-12MHz.
>

Indeed. And given that it's hw controlled anyway, I don't see why we can't make
it a fixed clock and handle the init at kernel boot depending on what the
bootloader has done.

Peter.


2018-03-01 13:20:16

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On 01.03.2018 10:41, Peter De Schrijver wrote:
> On Wed, Feb 28, 2018 at 08:20:47PM +0300, Dmitry Osipenko wrote:
>> On 28.02.2018 17:14, Peter De Schrijver wrote:
>>> On Wed, Feb 28, 2018 at 03:00:23PM +0300, Dmitry Osipenko wrote:
>>>> On 28.02.2018 12:36, Peter De Schrijver wrote:
>>>>> On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
>>>>>> On 27.02.2018 02:04, Marcel Ziswiler wrote:
>>>>>>> On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
>>>>>>>> On 23.02.2018 02:04, Marcel Ziswiler wrote:
>>>>>>>>> Turns out latest upstream U-Boot does not configure/enable pllu
>>>>>>>>> which
>>>>>>>>> leaves it at some default rate of 500 kHz:
>>>>>>>>>
>>>>>>>>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
>>>>>>>>> pll_u
>>>>>>>>> pll_u 3 3 0 500000
>>>>>>>>> 0
>>>>>>>>>
>>>>>>>>> Of course this won't quite work leading to the following messages:
>>>>>>>>>
>>>>>>>>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
>>>>>>>>> tegra-
>>>>>>>>> ehci
>>>>>>>>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
>>>>>>>>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
>>>>>>>>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
>>>>>>>>> tegra-
>>>>>>>>> ehci
>>>>>>>>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
>>>>>>>>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
>>>>>>>>> [ 48.049658] usb usb2-port1: attempt power cycle
>>>>>>>>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
>>>>>>>>> tegra-
>>>>>>>>> ehci
>>>>>>>>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
>>>>>>>>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
>>>>>>>>> tegra-
>>>>>>>>> ehci
>>>>>>>>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
>>>>>>>>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
>>>>>>>>>
>>>>>>>>> Fix this by actually allowing the rate also being set from within
>>>>>>>>> the Linux kernel.
>>>>>
>>>>> I think the best solution to this problem would be to make pll_u a fixed
>>>>> clock and enable it and program the rate if it's not enabled at boot.
>>>>
>>>> Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
>>>> yesterday.. So set/round_rate() for PLL_U are actually needed and the patch is
>>>> correct. Seems only T20 misses PLL_U in the init table, probably worth to add it
>>>> there.
>>>>
>>>
>>> AFAIK we only use one rate ever?
>>
>> IIUC, PLL_U has 3 outputs and output dividers are fixed in HW. So yes, we are
>> setting PLL_U to one rate - 480MHz to get out1-480MHz, out2-60MHz and out3-12MHz.
>>
>
> Indeed. And given that it's hw controlled anyway, I don't see why we can't make
> it a fixed clock and handle the init at kernel boot depending on what the
> bootloader has done.

We can, I just don't think you can demand from Mark to do it. This patch is fine
on its own, everything else could be done later.

On the other hand, it's USB driver responsibility to do the right thing. Why
just not to correct the driver? It would be kinda weird to mask one driver bug
by adding workaround to another. So I'd even suggest to go other way round by
implementing PLL lock polling and removing PLL_U enabling/disabling from the USB
PHY driver (if it's really fully-controlled by HW).

2018-03-01 13:46:26

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On 01.03.2018 16:19, Dmitry Osipenko wrote:
> On 01.03.2018 10:41, Peter De Schrijver wrote:
>> On Wed, Feb 28, 2018 at 08:20:47PM +0300, Dmitry Osipenko wrote:
>>> On 28.02.2018 17:14, Peter De Schrijver wrote:
>>>> On Wed, Feb 28, 2018 at 03:00:23PM +0300, Dmitry Osipenko wrote:
>>>>> On 28.02.2018 12:36, Peter De Schrijver wrote:
>>>>>> On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
>>>>>>> On 27.02.2018 02:04, Marcel Ziswiler wrote:
>>>>>>>> On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
>>>>>>>>> On 23.02.2018 02:04, Marcel Ziswiler wrote:
>>>>>>>>>> Turns out latest upstream U-Boot does not configure/enable pllu
>>>>>>>>>> which
>>>>>>>>>> leaves it at some default rate of 500 kHz:
>>>>>>>>>>
>>>>>>>>>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
>>>>>>>>>> pll_u
>>>>>>>>>> pll_u 3 3 0 500000
>>>>>>>>>> 0
>>>>>>>>>>
>>>>>>>>>> Of course this won't quite work leading to the following messages:
>>>>>>>>>>
>>>>>>>>>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
>>>>>>>>>> tegra-
>>>>>>>>>> ehci
>>>>>>>>>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
>>>>>>>>>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
>>>>>>>>>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
>>>>>>>>>> tegra-
>>>>>>>>>> ehci
>>>>>>>>>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
>>>>>>>>>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
>>>>>>>>>> [ 48.049658] usb usb2-port1: attempt power cycle
>>>>>>>>>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
>>>>>>>>>> tegra-
>>>>>>>>>> ehci
>>>>>>>>>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
>>>>>>>>>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
>>>>>>>>>> tegra-
>>>>>>>>>> ehci
>>>>>>>>>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
>>>>>>>>>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
>>>>>>>>>>
>>>>>>>>>> Fix this by actually allowing the rate also being set from within
>>>>>>>>>> the Linux kernel.
>>>>>>
>>>>>> I think the best solution to this problem would be to make pll_u a fixed
>>>>>> clock and enable it and program the rate if it's not enabled at boot.
>>>>>
>>>>> Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
>>>>> yesterday.. So set/round_rate() for PLL_U are actually needed and the patch is
>>>>> correct. Seems only T20 misses PLL_U in the init table, probably worth to add it
>>>>> there.
>>>>>
>>>>
>>>> AFAIK we only use one rate ever?
>>>
>>> IIUC, PLL_U has 3 outputs and output dividers are fixed in HW. So yes, we are
>>> setting PLL_U to one rate - 480MHz to get out1-480MHz, out2-60MHz and out3-12MHz.
>>>
>>
>> Indeed. And given that it's hw controlled anyway, I don't see why we can't make
>> it a fixed clock and handle the init at kernel boot depending on what the
>> bootloader has done.
>
> We can, I just don't think you can demand from Mark to do it. This patch is fine
> on its own, everything else could be done later.
I meant Marcel.

2018-03-02 09:46:19

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration


On 01/03/18 07:41, Peter De Schrijver wrote:
> On Wed, Feb 28, 2018 at 08:20:47PM +0300, Dmitry Osipenko wrote:
>> On 28.02.2018 17:14, Peter De Schrijver wrote:
>>> On Wed, Feb 28, 2018 at 03:00:23PM +0300, Dmitry Osipenko wrote:
>>>> On 28.02.2018 12:36, Peter De Schrijver wrote:
>>>>> On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
>>>>>> On 27.02.2018 02:04, Marcel Ziswiler wrote:
>>>>>>> On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
>>>>>>>> On 23.02.2018 02:04, Marcel Ziswiler wrote:
>>>>>>>>> Turns out latest upstream U-Boot does not configure/enable pllu
>>>>>>>>> which
>>>>>>>>> leaves it at some default rate of 500 kHz:
>>>>>>>>>
>>>>>>>>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
>>>>>>>>> pll_u
>>>>>>>>> pll_u 3 3 0 500000
>>>>>>>>> 0
>>>>>>>>>
>>>>>>>>> Of course this won't quite work leading to the following messages:
>>>>>>>>>
>>>>>>>>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
>>>>>>>>> tegra-
>>>>>>>>> ehci
>>>>>>>>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
>>>>>>>>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
>>>>>>>>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
>>>>>>>>> tegra-
>>>>>>>>> ehci
>>>>>>>>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
>>>>>>>>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
>>>>>>>>> [ 48.049658] usb usb2-port1: attempt power cycle
>>>>>>>>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
>>>>>>>>> tegra-
>>>>>>>>> ehci
>>>>>>>>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
>>>>>>>>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
>>>>>>>>> tegra-
>>>>>>>>> ehci
>>>>>>>>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
>>>>>>>>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
>>>>>>>>>
>>>>>>>>> Fix this by actually allowing the rate also being set from within
>>>>>>>>> the Linux kernel.
>>>>>
>>>>> I think the best solution to this problem would be to make pll_u a fixed
>>>>> clock and enable it and program the rate if it's not enabled at boot.
>>>>
>>>> Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
>>>> yesterday.. So set/round_rate() for PLL_U are actually needed and the patch is
>>>> correct. Seems only T20 misses PLL_U in the init table, probably worth to add it
>>>> there.
>>>>
>>>
>>> AFAIK we only use one rate ever?
>>
>> IIUC, PLL_U has 3 outputs and output dividers are fixed in HW. So yes, we are
>> setting PLL_U to one rate - 480MHz to get out1-480MHz, out2-60MHz and out3-12MHz.
>>
>
> Indeed. And given that it's hw controlled anyway, I don't see why we can't make
> it a fixed clock and handle the init at kernel boot depending on what the
> bootloader has done.

Peter, are you suggesting we implement the equivalent to
tegra210_init_pllu()? This does look a bit more involved that what this
change is doing. Is there a simple way to do what you are suggesting?

Cheers
Jon

--
nvpublic

2018-03-02 10:50:14

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration


On 22/02/18 23:04, Marcel Ziswiler wrote:
> Turns out latest upstream U-Boot does not configure/enable pllu which
> leaves it at some default rate of 500 kHz:
>
> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep pll_u
> pll_u 3 3 0 500000 0
>
> Of course this won't quite work leading to the following messages:
>
> [ 6.559593] usb 2-1: new full-speed USB device number 2 using tegra-
> ehci
> [ 11.759173] usb 2-1: device descriptor read/64, error -110
> [ 27.119453] usb 2-1: device descriptor read/64, error -110
> [ 27.389217] usb 2-1: new full-speed USB device number 3 using tegra-
> ehci
> [ 32.559454] usb 2-1: device descriptor read/64, error -110
> [ 47.929777] usb 2-1: device descriptor read/64, error -110
> [ 48.049658] usb usb2-port1: attempt power cycle
> [ 48.759475] usb 2-1: new full-speed USB device number 4 using tegra-
> ehci
> [ 59.349457] usb 2-1: device not accepting address 4, error -110
> [ 59.509449] usb 2-1: new full-speed USB device number 5 using tegra-
> ehci
> [ 70.069457] usb 2-1: device not accepting address 5, error -110
> [ 70.079721] usb usb2-port1: unable to enumerate USB device
>
> Fix this by actually allowing the rate also being set from within
> the Linux kernel.
>
> Signed-off-by: Marcel Ziswiler <[email protected]>
>
> ---
>
> drivers/clk/tegra/clk-pll.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
> index 7c369e21c91c..830d1c87fa7c 100644
> --- a/drivers/clk/tegra/clk-pll.c
> +++ b/drivers/clk/tegra/clk-pll.c
> @@ -1151,6 +1151,8 @@ static const struct clk_ops tegra_clk_pllu_ops = {
> .enable = clk_pllu_enable,
> .disable = clk_pll_disable,
> .recalc_rate = clk_pll_recalc_rate,
> + .round_rate = clk_pll_round_rate,
> + .set_rate = clk_pll_set_rate,
> };
>
> static int _pll_fixed_mdiv(struct tegra_clk_pll_params *pll_params,

Thanks for the fix. I have tested this on Tegra30 cardhu and this
resolves a problem booting this board with the upstream uboot bootloader.

We just need to align with Peter on the best way to fix.

Cheers,
Jon

--
nvpublic

2018-03-02 11:26:59

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On Fri, Mar 02, 2018 at 09:02:07AM +0000, Jon Hunter wrote:
>
> On 01/03/18 07:41, Peter De Schrijver wrote:
> > On Wed, Feb 28, 2018 at 08:20:47PM +0300, Dmitry Osipenko wrote:
> >> On 28.02.2018 17:14, Peter De Schrijver wrote:
> >>> On Wed, Feb 28, 2018 at 03:00:23PM +0300, Dmitry Osipenko wrote:
> >>>> On 28.02.2018 12:36, Peter De Schrijver wrote:
> >>>>> On Tue, Feb 27, 2018 at 02:59:11PM +0300, Dmitry Osipenko wrote:
> >>>>>> On 27.02.2018 02:04, Marcel Ziswiler wrote:
> >>>>>>> On Mon, 2018-02-26 at 15:42 +0300, Dmitry Osipenko wrote:
> >>>>>>>> On 23.02.2018 02:04, Marcel Ziswiler wrote:
> >>>>>>>>> Turns out latest upstream U-Boot does not configure/enable pllu
> >>>>>>>>> which
> >>>>>>>>> leaves it at some default rate of 500 kHz:
> >>>>>>>>>
> >>>>>>>>> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep
> >>>>>>>>> pll_u
> >>>>>>>>> pll_u 3 3 0 500000
> >>>>>>>>> 0
> >>>>>>>>>
> >>>>>>>>> Of course this won't quite work leading to the following messages:
> >>>>>>>>>
> >>>>>>>>> [ 6.559593] usb 2-1: new full-speed USB device number 2 using
> >>>>>>>>> tegra-
> >>>>>>>>> ehci
> >>>>>>>>> [ 11.759173] usb 2-1: device descriptor read/64, error -110
> >>>>>>>>> [ 27.119453] usb 2-1: device descriptor read/64, error -110
> >>>>>>>>> [ 27.389217] usb 2-1: new full-speed USB device number 3 using
> >>>>>>>>> tegra-
> >>>>>>>>> ehci
> >>>>>>>>> [ 32.559454] usb 2-1: device descriptor read/64, error -110
> >>>>>>>>> [ 47.929777] usb 2-1: device descriptor read/64, error -110
> >>>>>>>>> [ 48.049658] usb usb2-port1: attempt power cycle
> >>>>>>>>> [ 48.759475] usb 2-1: new full-speed USB device number 4 using
> >>>>>>>>> tegra-
> >>>>>>>>> ehci
> >>>>>>>>> [ 59.349457] usb 2-1: device not accepting address 4, error -110
> >>>>>>>>> [ 59.509449] usb 2-1: new full-speed USB device number 5 using
> >>>>>>>>> tegra-
> >>>>>>>>> ehci
> >>>>>>>>> [ 70.069457] usb 2-1: device not accepting address 5, error -110
> >>>>>>>>> [ 70.079721] usb usb2-port1: unable to enumerate USB device
> >>>>>>>>>
> >>>>>>>>> Fix this by actually allowing the rate also being set from within
> >>>>>>>>> the Linux kernel.
> >>>>>
> >>>>> I think the best solution to this problem would be to make pll_u a fixed
> >>>>> clock and enable it and program the rate if it's not enabled at boot.
> >>>>
> >>>> Oh, right. PLL_U rate is actually configurable, somehow I missed it in TRM
> >>>> yesterday.. So set/round_rate() for PLL_U are actually needed and the patch is
> >>>> correct. Seems only T20 misses PLL_U in the init table, probably worth to add it
> >>>> there.
> >>>>
> >>>
> >>> AFAIK we only use one rate ever?
> >>
> >> IIUC, PLL_U has 3 outputs and output dividers are fixed in HW. So yes, we are
> >> setting PLL_U to one rate - 480MHz to get out1-480MHz, out2-60MHz and out3-12MHz.
> >>
> >
> > Indeed. And given that it's hw controlled anyway, I don't see why we can't make
> > it a fixed clock and handle the init at kernel boot depending on what the
> > bootloader has done.
>
> Peter, are you suggesting we implement the equivalent to
> tegra210_init_pllu()? This does look a bit more involved that what this
> change is doing. Is there a simple way to do what you are suggesting?
>

Basically yes.

Peter.

2018-03-08 14:59:36

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH] clk: tegra: fix pllu rate configuration

On Fri, Feb 23, 2018 at 12:04:51AM +0100, Marcel Ziswiler wrote:
> Turns out latest upstream U-Boot does not configure/enable pllu which
> leaves it at some default rate of 500 kHz:
>
> root@apalis-t30:~# cat /sys/kernel/debug/clk/clk_summary | grep pll_u
> pll_u 3 3 0 500000 0
>
> Of course this won't quite work leading to the following messages:
>
> [ 6.559593] usb 2-1: new full-speed USB device number 2 using tegra-
> ehci
> [ 11.759173] usb 2-1: device descriptor read/64, error -110
> [ 27.119453] usb 2-1: device descriptor read/64, error -110
> [ 27.389217] usb 2-1: new full-speed USB device number 3 using tegra-
> ehci
> [ 32.559454] usb 2-1: device descriptor read/64, error -110
> [ 47.929777] usb 2-1: device descriptor read/64, error -110
> [ 48.049658] usb usb2-port1: attempt power cycle
> [ 48.759475] usb 2-1: new full-speed USB device number 4 using tegra-
> ehci
> [ 59.349457] usb 2-1: device not accepting address 4, error -110
> [ 59.509449] usb 2-1: new full-speed USB device number 5 using tegra-
> ehci
> [ 70.069457] usb 2-1: device not accepting address 5, error -110
> [ 70.079721] usb usb2-port1: unable to enumerate USB device
>
> Fix this by actually allowing the rate also being set from within
> the Linux kernel.
>
> Signed-off-by: Marcel Ziswiler <[email protected]>
>
> ---
>
> drivers/clk/tegra/clk-pll.c | 2 ++
> 1 file changed, 2 insertions(+)

I'm aware of the discussion about how best to implement this, but this
seems like a reasonable fix until we have something better.

Peter, Jon: if you want to fix this more properly, please take point on
this.

Applied this to for-4.17/clk, thanks.

Thierry


Attachments:
(No filename) (1.75 kB)
signature.asc (849.00 B)
Download all attachments