| Message ID | 1373355797-28758-1-git-send-email-anaumann@ultratronik.de |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
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_ */ > >
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.
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.
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
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,
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,
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_ */
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(-)