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 330 50 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 > > --- > > 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 signature.asc Description: PGP signature
Re: [PATCH] clk: tegra: fix pllu rate configuration
On Fri, Mar 02, 2018 at 09:02:07AM +, 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 330 50 > > 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.
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 330 50 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
>
> ---
>
> 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
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 330 50 > 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
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 330 50 >> 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.
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 330 50 > 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).
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 330 50 > >>> 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.
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 330 50 >>> 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.
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 330 50 > > 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.
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 330 50 > 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.
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 330 50 > >>> 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.
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 330 50
>>> 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
>>>
>>> ---
>>>
>>> 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
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 330 50
> > 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
> >
> > ---
> >
> > 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
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 330 50 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
>
> ---
>
> 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?
Re: [PATCH] clk: tegra: fix pllu rate configuration
On February 23, 2018 12:47:56 AM GMT+01:00, Stephen Warren 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.
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.
[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 330 50 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
---
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
