Message ID | 52132DBE.5000804@andin.de |
---|---|
State | Not Applicable |
Delegated to: | Tom Rini |
Headers | show |
Hi, On 08/20/2013 11:50 AM, Andreas Naumann wrote: > Hi, > > Am 16.08.2013 17:30, schrieb Robert Nelson: >> 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. > > Oh, I was not aware of that. But indeed i use a patched 3.1 kernel, see below. > Some info on the history: In our design (19.2MHz crystal) we could clearly see the errata problem of high jitter on the 60MHz USB clock when (re-)booting a board that was already warmed up. > Back then I applied a slightly extended kernel patch (see below) that I found on some kernel list. This reproducably did solve the problem with the jitter. We verified this with a high quality oscilloscope and numerous powercycles at different temperatures. The U-Boot in use was 2010.09 and some old X-Loader, both of which dont touch the PLL4/5 stuff. > > Introducing the current U-Boot (to make use of SPL) brought up above described problems with the clock, probably due to setting the lock mode active. Hence this patch for DM37xx. > What are the symptoms you see when this issue triggers? What is the test case to trigger the error? Is it just running any USB I/O for long enough time? I have a beagle-xm (DM3730 ES1.2) with me and would like to reproduce the error. cheers, -roger
Hi Roger, > > What are the symptoms you see when this issue triggers? The symptoms are erroneous USB transaction, seen with a USB port analyzer. These only sometimes (not always) stall the USB communication, e.g. a USB mass storage device cant be read any longer. > > What is the test case to trigger the error? Is it just running any USB I/O for > long enough time? Our scenario to reproduce was rebooting a warmed up board (either let it run for >5min or heat up in climate chamber). However, the beagle probably uses a 26 MHz crystal oscillator (our board uses 19.2MHz), so the PLL5 dividers may be set in a way that the problem never occurs. > I have a beagle-xm (DM3730 ES1.2) with me and would like to reproduce the error. > > cheers, > -roger > regards, Andreas
On Tue, Aug 20, 2013 at 5:15 AM, Andreas Naumann <dev@andin.de> wrote: > Hi Roger, > > >> >> What are the symptoms you see when this issue triggers? > > > The symptoms are erroneous USB transaction, seen with a USB port analyzer. > These only sometimes (not always) stall the USB communication, e.g. a USB > mass storage device cant be read any longer. > > >> >> What is the test case to trigger the error? Is it just running any USB I/O >> for >> long enough time? > > > Our scenario to reproduce was rebooting a warmed up board (either let it run > for >5min or heat up in climate chamber). > > However, the beagle probably uses a 26 MHz crystal oscillator (our board > uses 19.2MHz), so the PLL5 dividers may be set in a way that the problem > never occurs. The xM uses a 26Mhz, but the errata still applies, as a number of customer boards do show the issue.. http://www.ti.com/lit/er/sprz319e/sprz319e.pdf page 113 >> I have a beagle-xm (DM3730 ES1.2) with me and would like to reproduce the >> error. Roger, this only seems to effect a small number of xM's, as it seems to vary on pll drift. So if your xM is fine, i do have a spare xM C, that pretty reliably shows the issue after transferring a large amount of data over the usb port... I had traded a good xM with a customer such that i could keep re-basing our out of tree dpll5 tweak.. Regards,
diff --git a/arch/arm/mach-omap2/clkt_clksel.c b/arch/arm/mach-omap2/clkt_clksel.c index e25364d..e378fe7 100644 --- a/arch/arm/mach-omap2/clkt_clksel.c +++ b/arch/arm/mach-omap2/clkt_clksel.c @@ -460,6 +460,21 @@ int omap2_clksel_set_rate(struct clk *clk, unsigned long rate) return 0; } +int omap2_clksel_force_divisor(struct clk *clk, int new_div) +{ + u32 field_val; + + field_val = _divisor_to_clksel(clk, new_div); + if (field_val == ~0) + return -EINVAL; + + _write_clksel_reg(clk, field_val); + + clk->rate = clk->parent->rate / new_div; + + return 0; +} + /* * Clksel parent setting function - not passed in struct clk function * pointer - instead, the OMAP clock code currently assumes that any diff --git a/arch/arm/mach-omap2/clock.h b/arch/arm/mach-omap2/clock.h index 48ac568..3d2c899 100644 --- a/arch/arm/mach-omap2/clock.h +++ b/arch/arm/mach-omap2/clock.h @@ -61,6 +61,12 @@ void omap3_dpll_allow_idle(struct clk *clk); void omap3_dpll_deny_idle(struct clk *clk); u32 omap3_dpll_autoidle_read(struct clk *clk); int omap3_noncore_dpll_set_rate(struct clk *clk, unsigned long rate); +#if CONFIG_ARCH_OMAP3 +int omap3_noncore_dpll_program(struct clk *clk, u16 m, u8 n, u16 freqsel); +/* If you are using this function and not on OMAP3, you are + * Doing It Wrong(tm), so there is no stub. + */ +#endif int omap3_noncore_dpll_enable(struct clk *clk); void omap3_noncore_dpll_disable(struct clk *clk); int omap4_dpllmx_gatectrl_read(struct clk *clk); @@ -84,6 +90,7 @@ unsigned long omap2_clksel_recalc(struct clk *clk); long omap2_clksel_round_rate(struct clk *clk, unsigned long target_rate); int omap2_clksel_set_rate(struct clk *clk, unsigned long rate); int omap2_clksel_set_parent(struct clk *clk, struct clk *new_parent); +int omap2_clksel_force_divisor(struct clk *clk, int new_div); /* clkt_iclk.c public functions */ extern void omap2_clkt_iclk_allow_idle(struct clk *clk); diff --git a/arch/arm/mach-omap2/clock3xxx.c b/arch/arm/mach-omap2/clock3xxx.c index 952c3e0..97d4192 100644 --- a/arch/arm/mach-omap2/clock3xxx.c +++ b/arch/arm/mach-omap2/clock3xxx.c @@ -40,6 +40,63 @@ /* needed by omap3_core_dpll_m2_set_rate() */ struct clk *sdrc_ick_p, *arm_fck_p; +struct dpll_settings { + int rate, m, n, f; +}; + + +static int omap3_dpll5_apply_erratum21(struct clk *clk, struct clk *dpll5_m2) +{ + struct clk *sys_clk; + int i, rv; + static const struct dpll_settings precomputed[] = { + /* From DM3730 errata (sprz319e), table 36 + * +1 is because the values in the table are register values; + * dpll_program() will subtract one from what we give it, + * so ... + */ + { 13000000, 443+1, 5+1, 8 }, + { 12000000, 80, 0+1, 8 }, + { 19200000, 50, 0+1, 8 }, + { 26000000, 443+1, 11+1, 8 }, + { 38400000, 25, 0+1, 8 }, + }; + + sys_clk = clk_get(NULL, "sys_ck"); + + for (i = 0 ; i < (sizeof(precomputed)/sizeof(struct dpll_settings)) ; + ++i) { + const struct dpll_settings *d = &precomputed[i]; + if (sys_clk->rate == d->rate) { + rv = omap3_noncore_dpll_program(clk, d->m , d->n, 0); + if (rv) + return 1; + rv = omap2_clksel_force_divisor(dpll5_m2 , d->f); + return 1; + } + } + return 0; +} + +int omap3_dpll5_set_rate(struct clk *clk, unsigned long rate) +{ + struct clk *dpll5_m2; + int rv; + dpll5_m2 = clk_get(NULL, "dpll5_m2_ck"); + + if (cpu_is_omap3630() && rate == DPLL5_FREQ_FOR_USBHOST && + omap3_dpll5_apply_erratum21(clk, dpll5_m2)) { + return 1; + } + rv = omap3_noncore_dpll_set_rate(clk, rate); + if (rv) + goto out; + rv = clk_set_rate(dpll5_m2, rate); + +out: + return rv; +} + int omap3_dpll4_set_rate(struct clk *clk, unsigned long rate) { /* @@ -59,19 +116,14 @@ int omap3_dpll4_set_rate(struct clk *clk, unsigned long rate) void __init omap3_clk_lock_dpll5(void) { struct clk *dpll5_clk; - struct clk *dpll5_m2_clk; dpll5_clk = clk_get(NULL, "dpll5_ck"); clk_set_rate(dpll5_clk, DPLL5_FREQ_FOR_USBHOST); - clk_enable(dpll5_clk); - /* Program dpll5_m2_clk divider for no division */ - dpll5_m2_clk = clk_get(NULL, "dpll5_m2_ck"); - clk_enable(dpll5_m2_clk); - clk_set_rate(dpll5_m2_clk, DPLL5_FREQ_FOR_USBHOST); + /* dpll5_m2_ck is now (grottily!) handled by dpll5_clk's set routine, + * to cope with an erratum on DM3730 + */ - clk_disable(dpll5_m2_clk); - clk_disable(dpll5_clk); return; } diff --git a/arch/arm/mach-omap2/clock3xxx.h b/arch/arm/mach-omap2/clock3xxx.h index 8bbeeaf..0ede513 100644 --- a/arch/arm/mach-omap2/clock3xxx.h +++ b/arch/arm/mach-omap2/clock3xxx.h @@ -10,6 +10,7 @@ int omap3xxx_clk_init(void); int omap3_dpll4_set_rate(struct clk *clk, unsigned long rate); +int omap3_dpll5_set_rate(struct clk *clk, unsigned long rate); int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate); void omap3_clk_lock_dpll5(void); diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c index b9b8446..33f9853 100644 --- a/arch/arm/mach-omap2/clock3xxx_data.c +++ b/arch/arm/mach-omap2/clock3xxx_data.c @@ -942,7 +942,7 @@ static struct clk dpll5_ck = { .parent = &sys_ck, .dpll_data = &dpll5_dd, .round_rate = &omap2_dpll_round_rate, - .set_rate = &omap3_noncore_dpll_set_rate, + .set_rate = &omap3_dpll5_set_rate, .clkdm_name = "dpll5_clkdm", .recalc = &omap3_dpll_recalc, }; diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c index f77022b..1909cd0 100644 --- a/arch/arm/mach-omap2/dpll3xxx.c +++ b/arch/arm/mach-omap2/dpll3xxx.c @@ -291,7 +291,7 @@ static void _lookup_sddiv(struct clk *clk, u8 *sd_div, u16 m, u8 n) * Program the DPLL with the supplied M, N values, and wait for the DPLL to * lock.. Returns -EINVAL upon error, or 0 upon success. */ -static int omap3_noncore_dpll_program(struct clk *clk, u16 m, u8 n, u16 freqsel) +int omap3_noncore_dpll_program(struct clk *clk, u16 m, u8 n, u16 freqsel)