Patchwork [U-Boot] ARM: omap3: Implement dpll5 (HSUSB clk) workaround for OMAP36xx/AM/DM37xx according to errata sprz318e.

login
register
mail settings
Submitter Naumann Andreas
Date July 9, 2013, 7:43 a.m.
Message ID <1373355797-28758-1-git-send-email-anaumann@ultratronik.de>
Download mbox | patch
Permalink /patch/257649/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Naumann Andreas - July 9, 2013, 7:43 a.m.
In chapter 'Advisory 2.1 USB Host Clock Drift Causes USB Spec Non-compliance in Certain Configurations' of the TI Errata it is recommended to use certain div/mult values for the DPLL5 clock setup.
So far u-boot used the old 34xx values, so I added the errata recommended values specificly for 36xx init only.
Also, the FSEL registers exist no longer, so removed them from init.

Tested this on a AM3703 board with 19.2MHz oscillator, which previously couldnt lock the dpll5 (kernel complained). As a consequence the EHCI USB port wasnt usable in U-Boot and kernel. With this patch, kernel panics disappear and USB working fine in u-boot and kernel.

Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
---
 arch/arm/cpu/armv7/omap3/clock.c               | 20 +++++++++++++++++++-
 arch/arm/cpu/armv7/omap3/lowlevel_init.S       | 18 ++++++++++++++++++
 arch/arm/include/asm/arch-omap3/clocks_omap3.h | 22 ++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
Peter Bigot - Aug. 15, 2013, 2:53 a.m.
On 07/09/2013 02:43 AM, Naumann Andreas wrote:
> In chapter 'Advisory 2.1 USB Host Clock Drift Causes USB Spec Non-compliance in Certain Configurations' of the TI Errata it is recommended to use certain div/mult values for the DPLL5 clock setup.
> So far u-boot used the old 34xx values, so I added the errata recommended values specificly for 36xx init only.
> Also, the FSEL registers exist no longer, so removed them from init.
>
> Tested this on a AM3703 board with 19.2MHz oscillator, which previously couldnt lock the dpll5 (kernel complained). As a consequence the EHCI USB port wasnt usable in U-Boot and kernel. With this patch, kernel panics disappear and USB working fine in u-boot and kernel.
>
> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>

While this patch works with Linux that has been patched for this 
erratum, it will cause problems with some unpatched versions of Linux.

In particular, this patch sets CM_CLKSEL4_PLL to generate (nearly) 
960MHz, and CM_CLKSEL5_PLL to divide by 8 to produce the required 
120MHz, as recommended by sprz318e advisory 2.1.

Version 3.5 of Linux, and possibly others, configure CM_CLKSEL4_PLL 
(named "dpll5_ck") to generate 120MHz and leaves CM_CLKSEL5_PLL 
unmodified since its clock (named "dpll5_m2_ck") does not support set 
rate.  If u-boot has configured a divisor of 8, the result is that the 
actual clock speed is 15MHz and USB does not work.

Not sure how this ought to be resolved; in my case I'm going to skip the 
u-boot patch and just use the Linux patch.

Peter

>
> ---
> arch/arm/cpu/armv7/omap3/clock.c               | 20 +++++++++++++++++++-
>   arch/arm/cpu/armv7/omap3/lowlevel_init.S       | 18 ++++++++++++++++++
>   arch/arm/include/asm/arch-omap3/clocks_omap3.h | 22 ++++++++++++++++++++++
>   3 files changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/cpu/armv7/omap3/clock.c b/arch/arm/cpu/armv7/omap3/clock.c
> index 81cc859..68833ba 100644
> --- a/arch/arm/cpu/armv7/omap3/clock.c
> +++ b/arch/arm/cpu/armv7/omap3/clock.c
> @@ -491,6 +491,24 @@ static void dpll4_init_36xx(u32 sil_index, u32 clk_index)
>   	wait_on_value(ST_PERIPH_CLK, 2, &prcm_base->idlest_ckgen, LDELAY);
>   }
>   
> +static void dpll5_init_36xx(u32 sil_index, u32 clk_index)
> +{
> +	struct prcm *prcm_base = (struct prcm *)PRCM_BASE;
> +	dpll_param *ptr = (dpll_param *) get_36x_per2_dpll_param();
> +
> +	/* Moving it to the right sysclk base */
> +	ptr = ptr + clk_index;
> +
> +	/* PER2 DPLL (DPLL5) */
> +	sr32(&prcm_base->clken2_pll, 0, 3, PLL_STOP);
> +	wait_on_value(1, 0, &prcm_base->idlest2_ckgen, LDELAY);
> +	sr32(&prcm_base->clksel5_pll, 0, 5, ptr->m2); /* set M2 (usbtll_fck) */
> +	sr32(&prcm_base->clksel4_pll, 8, 11, ptr->m); /* set m (11-bit multiplier) */
> +	sr32(&prcm_base->clksel4_pll, 0, 7, ptr->n); /* set n (7-bit divider)*/
> +	sr32(&prcm_base->clken2_pll, 0, 3, PLL_LOCK);   /* lock mode */
> +	wait_on_value(1, 1, &prcm_base->idlest2_ckgen, LDELAY);
> +}
> +
>   static void mpu_init_36xx(u32 sil_index, u32 clk_index)
>   {
>   	struct prcm *prcm_base = (struct prcm *)PRCM_BASE;
> @@ -595,7 +613,7 @@ void prcm_init(void)
>   
>   		dpll3_init_36xx(0, clk_index);
>   		dpll4_init_36xx(0, clk_index);
> -		dpll5_init_34xx(0, clk_index);
> +		dpll5_init_36xx(0, clk_index);
>   		iva_init_36xx(0, clk_index);
>   		mpu_init_36xx(0, clk_index);
>   
> diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> index eacfef8..66a1b48 100644
> --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
> @@ -480,6 +480,19 @@ per_36x_dpll_param:
>   .word 26000,    432,   12,     9,      16,     9,     4,      3,      1
>   .word 38400,    360,   15,     9,      16,     5,     4,      3,      1
>   
> +per2_36x_dpll_param:
> +/* 12MHz */
> +.word PER2_36XX_M_12, PER2_36XX_N_12, 0, PER2_36XX_M2_12
> +/* 13MHz */
> +.word PER2_36XX_M_13, PER2_36XX_N_13, 0, PER2_36XX_M2_13
> +/* 19.2MHz */
> +.word PER2_36XX_M_19P2, PER2_36XX_N_19P2, 0, PER2_36XX_M2_19P2
> +/* 26MHz */
> +.word PER2_36XX_M_26, PER2_36XX_N_26, 0, PER2_36XX_M2_26
> +/* 38.4MHz */
> +.word PER2_36XX_M_38P4, PER2_36XX_N_38P4, 0, PER2_36XX_M2_38P4
> +
> +
>   ENTRY(get_36x_mpu_dpll_param)
>   	adr	r0, mpu_36x_dpll_param
>   	mov	pc, lr
> @@ -499,3 +512,8 @@ ENTRY(get_36x_per_dpll_param)
>   	adr	r0, per_36x_dpll_param
>   	mov	pc, lr
>   ENDPROC(get_36x_per_dpll_param)
> +
> +ENTRY(get_36x_per2_dpll_param)
> +	adr	r0, per2_36x_dpll_param
> +	mov	pc, lr
> +ENDPROC(get_36x_per2_dpll_param)
> diff --git a/arch/arm/include/asm/arch-omap3/clocks_omap3.h b/arch/arm/include/asm/arch-omap3/clocks_omap3.h
> index 5925ac4..59e61e8 100644
> --- a/arch/arm/include/asm/arch-omap3/clocks_omap3.h
> +++ b/arch/arm/include/asm/arch-omap3/clocks_omap3.h
> @@ -336,4 +336,26 @@
>   #define PER_36XX_FSEL_38P4	0x07
>   #define PER_36XX_M2_38P4	0x09
>   
> +/* 36XX PER2 DPLL */
> +
> +#define PER2_36XX_M_12		0x50
> +#define PER2_36XX_N_12		0x00
> +#define PER2_36XX_M2_12		0x08
> +
> +#define PER2_36XX_M_13		0x1BB
> +#define PER2_36XX_N_13		0x05
> +#define PER2_36XX_M2_13		0x08
> +
> +#define PER2_36XX_M_19P2		0x32
> +#define PER2_36XX_N_19P2		0x00
> +#define PER2_36XX_M2_19P2		0x08
> +
> +#define PER2_36XX_M_26		0x1BB
> +#define PER2_36XX_N_26		0x0B
> +#define PER2_36XX_M2_26		0x08
> +
> +#define PER2_36XX_M_38P4		0x19
> +#define PER2_36XX_N_38P4		0x00
> +#define PER2_36XX_M2_38P4		0x08
> +
>   #endif	/* endif _CLOCKS_OMAP3_H_ */
>
>
Tom Rini - Aug. 16, 2013, 1:35 p.m.
On Tue, Jul 09, 2013 at 09:43:17AM +0200, Naumann Andreas wrote:

> In chapter 'Advisory 2.1 USB Host Clock Drift Causes USB Spec
> Non-compliance in Certain Configurations' of the TI Errata it is
> recommended to use certain div/mult values for the DPLL5 clock setup.
> So far u-boot used the old 34xx values, so I added the errata
> recommended values specificly for 36xx init only.  Also, the FSEL
> registers exist no longer, so removed them from init.
> 
> Tested this on a AM3703 board with 19.2MHz oscillator, which
> previously couldnt lock the dpll5 (kernel complained). As a
> consequence the EHCI USB port wasnt usable in U-Boot and kernel. With
> this patch, kernel panics disappear and USB working fine in u-boot and
> kernel.
> 
> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>

Applied to u-boot-ti/master with a prototype added to
<asm/arch-omap3/clock.h> to avoid a warning.
Tom Rini - Aug. 16, 2013, 1:38 p.m.
On Wed, Aug 14, 2013 at 09:53:16PM -0500, Peter A. Bigot wrote:
> On 07/09/2013 02:43 AM, Naumann Andreas wrote:
> >In chapter 'Advisory 2.1 USB Host Clock Drift Causes USB Spec Non-compliance in Certain Configurations' of the TI Errata it is recommended to use certain div/mult values for the DPLL5 clock setup.
> >So far u-boot used the old 34xx values, so I added the errata recommended values specificly for 36xx init only.
> >Also, the FSEL registers exist no longer, so removed them from init.
> >
> >Tested this on a AM3703 board with 19.2MHz oscillator, which previously couldnt lock the dpll5 (kernel complained). As a consequence the EHCI USB port wasnt usable in U-Boot and kernel. With this patch, kernel panics disappear and USB working fine in u-boot and kernel.
> >
> >Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
> 
> While this patch works with Linux that has been patched for this
> erratum, it will cause problems with some unpatched versions of
> Linux.

Right.  So Linux also needs to be patched for the erratum.
Peter Bigot - Aug. 16, 2013, 2:34 p.m.
On 08/16/2013 08:38 AM, Tom Rini wrote:
> On Wed, Aug 14, 2013 at 09:53:16PM -0500, Peter A. Bigot wrote:
>> On 07/09/2013 02:43 AM, Naumann Andreas wrote:
>>> In chapter 'Advisory 2.1 USB Host Clock Drift Causes USB Spec Non-compliance in Certain Configurations' of the TI Errata it is recommended to use certain div/mult values for the DPLL5 clock setup.
>>> So far u-boot used the old 34xx values, so I added the errata recommended values specificly for 36xx init only.
>>> Also, the FSEL registers exist no longer, so removed them from init.
>>>
>>> Tested this on a AM3703 board with 19.2MHz oscillator, which previously couldnt lock the dpll5 (kernel complained). As a consequence the EHCI USB port wasnt usable in U-Boot and kernel. With this patch, kernel panics disappear and USB working fine in u-boot and kernel.
>>>
>>> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
>> While this patch works with Linux that has been patched for this
>> erratum, it will cause problems with some unpatched versions of
>> Linux.
> Right.  So Linux also needs to be patched for the erratum.

Yes.  My point was that if you update u-boot alone, then try to use it 
to boot an unpatched Linux that assumes CM_CLKSEL5_PLL has its power-on 
value, USB will not work.

I think it's dangerous to assume that the mixture of an unpatched Linux 
with a patched u-boot will never occur, and the cause of the failure 
that results is pretty subtle.  So whatever gets merged would be safer 
if it restored the default setting of CM_CLKSEL5_PLL prior to handing 
off control to Linux.

Peter
robertcnelson@gmail.com - Aug. 16, 2013, 3:07 p.m.
On Fri, Aug 16, 2013 at 9:34 AM, Peter A. Bigot <pab@pabigot.com> wrote:
> On 08/16/2013 08:38 AM, Tom Rini wrote:
>>
>> On Wed, Aug 14, 2013 at 09:53:16PM -0500, Peter A. Bigot wrote:
>>>
>>> On 07/09/2013 02:43 AM, Naumann Andreas wrote:
>>>>
>>>> In chapter 'Advisory 2.1 USB Host Clock Drift Causes USB Spec
>>>> Non-compliance in Certain Configurations' of the TI Errata it is recommended
>>>> to use certain div/mult values for the DPLL5 clock setup.
>>>> So far u-boot used the old 34xx values, so I added the errata
>>>> recommended values specificly for 36xx init only.
>>>> Also, the FSEL registers exist no longer, so removed them from init.
>>>>
>>>> Tested this on a AM3703 board with 19.2MHz oscillator, which previously
>>>> couldnt lock the dpll5 (kernel complained). As a consequence the EHCI USB
>>>> port wasnt usable in U-Boot and kernel. With this patch, kernel panics
>>>> disappear and USB working fine in u-boot and kernel.
>>>>
>>>> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
>>>
>>> While this patch works with Linux that has been patched for this
>>> erratum, it will cause problems with some unpatched versions of
>>> Linux.
>>
>> Right.  So Linux also needs to be patched for the erratum.
>
>
> Yes.  My point was that if you update u-boot alone, then try to use it to
> boot an unpatched Linux that assumes CM_CLKSEL5_PLL has its power-on value,
> USB will not work.
>
> I think it's dangerous to assume that the mixture of an unpatched Linux with
> a patched u-boot will never occur, and the cause of the failure that results
> is pretty subtle.  So whatever gets merged would be safer if it restored the
> default setting of CM_CLKSEL5_PLL prior to handing off control to Linux.

Agree, we should not apply this, till we also have an 'approved' patch
for mainline linux posted.  Right now we have a set of kernel hacks,
but no agreed on method as the kernel maintainer did not have a board
that suffered from the errata..

Regards,
robertcnelson@gmail.com - Aug. 16, 2013, 3:30 p.m.
On Fri, Aug 16, 2013 at 10:07 AM, Robert Nelson <robertcnelson@gmail.com> wrote:
> On Fri, Aug 16, 2013 at 9:34 AM, Peter A. Bigot <pab@pabigot.com> wrote:
>> On 08/16/2013 08:38 AM, Tom Rini wrote:
>>>
>>> On Wed, Aug 14, 2013 at 09:53:16PM -0500, Peter A. Bigot wrote:
>>>>
>>>> On 07/09/2013 02:43 AM, Naumann Andreas wrote:
>>>>>
>>>>> In chapter 'Advisory 2.1 USB Host Clock Drift Causes USB Spec
>>>>> Non-compliance in Certain Configurations' of the TI Errata it is recommended
>>>>> to use certain div/mult values for the DPLL5 clock setup.
>>>>> So far u-boot used the old 34xx values, so I added the errata
>>>>> recommended values specificly for 36xx init only.
>>>>> Also, the FSEL registers exist no longer, so removed them from init.
>>>>>
>>>>> Tested this on a AM3703 board with 19.2MHz oscillator, which previously
>>>>> couldnt lock the dpll5 (kernel complained). As a consequence the EHCI USB
>>>>> port wasnt usable in U-Boot and kernel. With this patch, kernel panics
>>>>> disappear and USB working fine in u-boot and kernel.
>>>>>
>>>>> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
>>>>
>>>> While this patch works with Linux that has been patched for this
>>>> erratum, it will cause problems with some unpatched versions of
>>>> Linux.
>>>
>>> Right.  So Linux also needs to be patched for the erratum.
>>
>>
>> Yes.  My point was that if you update u-boot alone, then try to use it to
>> boot an unpatched Linux that assumes CM_CLKSEL5_PLL has its power-on value,
>> USB will not work.
>>
>> I think it's dangerous to assume that the mixture of an unpatched Linux with
>> a patched u-boot will never occur, and the cause of the failure that results
>> is pretty subtle.  So whatever gets merged would be safer if it restored the
>> default setting of CM_CLKSEL5_PLL prior to handing off control to Linux.
>
> Agree, we should not apply this, till we also have an 'approved' patch
> for mainline linux posted.  Right now we have a set of kernel hacks,
> but no agreed on method as the kernel maintainer did not have a board
> that suffered from the errata..

btw: here's a version that seems to work on v3.11-rc5:

https://raw.github.com/RobertCNelson/armv7-multiplatform/v3.11.x/patches/omap_sprz319_erratum_v2.1/0001-hack-omap-clockk-dpll5-apply-sprz319e-2.1-erratum-kernel-3.11-rc2.patch

Regards,

Patch

diff --git a/arch/arm/cpu/armv7/omap3/clock.c b/arch/arm/cpu/armv7/omap3/clock.c
index 81cc859..68833ba 100644
--- a/arch/arm/cpu/armv7/omap3/clock.c
+++ b/arch/arm/cpu/armv7/omap3/clock.c
@@ -491,6 +491,24 @@  static void dpll4_init_36xx(u32 sil_index, u32 clk_index)
 	wait_on_value(ST_PERIPH_CLK, 2, &prcm_base->idlest_ckgen, LDELAY);
 }
 
+static void dpll5_init_36xx(u32 sil_index, u32 clk_index)
+{
+	struct prcm *prcm_base = (struct prcm *)PRCM_BASE;
+	dpll_param *ptr = (dpll_param *) get_36x_per2_dpll_param();
+
+	/* Moving it to the right sysclk base */
+	ptr = ptr + clk_index;
+
+	/* PER2 DPLL (DPLL5) */
+	sr32(&prcm_base->clken2_pll, 0, 3, PLL_STOP);
+	wait_on_value(1, 0, &prcm_base->idlest2_ckgen, LDELAY);
+	sr32(&prcm_base->clksel5_pll, 0, 5, ptr->m2); /* set M2 (usbtll_fck) */
+	sr32(&prcm_base->clksel4_pll, 8, 11, ptr->m); /* set m (11-bit multiplier) */
+	sr32(&prcm_base->clksel4_pll, 0, 7, ptr->n); /* set n (7-bit divider)*/
+	sr32(&prcm_base->clken2_pll, 0, 3, PLL_LOCK);   /* lock mode */
+	wait_on_value(1, 1, &prcm_base->idlest2_ckgen, LDELAY);
+}
+
 static void mpu_init_36xx(u32 sil_index, u32 clk_index)
 {
 	struct prcm *prcm_base = (struct prcm *)PRCM_BASE;
@@ -595,7 +613,7 @@  void prcm_init(void)
 
 		dpll3_init_36xx(0, clk_index);
 		dpll4_init_36xx(0, clk_index);
-		dpll5_init_34xx(0, clk_index);
+		dpll5_init_36xx(0, clk_index);
 		iva_init_36xx(0, clk_index);
 		mpu_init_36xx(0, clk_index);
 
diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
index eacfef8..66a1b48 100644
--- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
@@ -480,6 +480,19 @@  per_36x_dpll_param:
 .word 26000,    432,   12,     9,      16,     9,     4,      3,      1
 .word 38400,    360,   15,     9,      16,     5,     4,      3,      1
 
+per2_36x_dpll_param:
+/* 12MHz */
+.word PER2_36XX_M_12, PER2_36XX_N_12, 0, PER2_36XX_M2_12
+/* 13MHz */
+.word PER2_36XX_M_13, PER2_36XX_N_13, 0, PER2_36XX_M2_13
+/* 19.2MHz */
+.word PER2_36XX_M_19P2, PER2_36XX_N_19P2, 0, PER2_36XX_M2_19P2
+/* 26MHz */
+.word PER2_36XX_M_26, PER2_36XX_N_26, 0, PER2_36XX_M2_26
+/* 38.4MHz */
+.word PER2_36XX_M_38P4, PER2_36XX_N_38P4, 0, PER2_36XX_M2_38P4
+
+
 ENTRY(get_36x_mpu_dpll_param)
 	adr	r0, mpu_36x_dpll_param
 	mov	pc, lr
@@ -499,3 +512,8 @@  ENTRY(get_36x_per_dpll_param)
 	adr	r0, per_36x_dpll_param
 	mov	pc, lr
 ENDPROC(get_36x_per_dpll_param)
+
+ENTRY(get_36x_per2_dpll_param)
+	adr	r0, per2_36x_dpll_param
+	mov	pc, lr
+ENDPROC(get_36x_per2_dpll_param)
diff --git a/arch/arm/include/asm/arch-omap3/clocks_omap3.h b/arch/arm/include/asm/arch-omap3/clocks_omap3.h
index 5925ac4..59e61e8 100644
--- a/arch/arm/include/asm/arch-omap3/clocks_omap3.h
+++ b/arch/arm/include/asm/arch-omap3/clocks_omap3.h
@@ -336,4 +336,26 @@ 
 #define PER_36XX_FSEL_38P4	0x07
 #define PER_36XX_M2_38P4	0x09
 
+/* 36XX PER2 DPLL */
+
+#define PER2_36XX_M_12		0x50
+#define PER2_36XX_N_12		0x00
+#define PER2_36XX_M2_12		0x08
+
+#define PER2_36XX_M_13		0x1BB
+#define PER2_36XX_N_13		0x05
+#define PER2_36XX_M2_13		0x08
+
+#define PER2_36XX_M_19P2		0x32
+#define PER2_36XX_N_19P2		0x00
+#define PER2_36XX_M2_19P2		0x08
+
+#define PER2_36XX_M_26		0x1BB
+#define PER2_36XX_N_26		0x0B
+#define PER2_36XX_M2_26		0x08
+
+#define PER2_36XX_M_38P4		0x19
+#define PER2_36XX_N_38P4		0x00
+#define PER2_36XX_M2_38P4		0x08
+
 #endif	/* endif _CLOCKS_OMAP3_H_ */