Re: [PATCH] clk: tegra: fix pllu rate configuration

[Dmitry Osipenko]

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 <marcel.ziswiler@xxxxxxxxxxx>
>>>
>>> ---
>>>
>>>  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