[U-Boot] [PATCH 1/2] v2 i.MX28: Fix ref_cpu clock setup
Marek Vasut
marek.vasut at gmail.com
Mon Feb 6 13:20:16 CET 2012
> Hi,
>
> This patch fixes ref_cpu clock setup. This bug leads to a hanging board
> after rebooting from the Kernel, due to failing memory size detection:
> U-Boot 2011.12-svn342 (Feb 02 2012 - 17:20:00)
>
> Freescale i.MX28 family
> I2C: ready
> DRAM: 0 Bytes
>
> The cause of the bug is register hw_clkctrl_frac0 being accessed as
> a 32-bit long, whereas the manual specifically states it can be accessed
> as bytes only.
>
> This patches introduces an 8-bit wide register type, mx28_register_8.
> The already existing mx28_register has been renamed mx28_register_32.
>
> With this patch, U-Boot no longer hangs after an i.mx28 based board
> was reset from the Kernel.
>
> (PS: I hope this email is properly formatted now, after fight our exchange
> server for a whole morning and loosing in the end)
>
> Signed-off-by: Robert Delien <robert at delien.nl>
> ---
> arch/arm/cpu/arm926ejs/mx28/clock.c | 74 +++-----
> arch/arm/cpu/arm926ejs/mx28/iomux.c | 6 +-
> arch/arm/cpu/arm926ejs/mx28/mx28.c | 6 +-
> arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c | 23 +--
> arch/arm/include/asm/arch-mx28/regs-apbh.h | 254
> ++++++++++++------------ arch/arm/include/asm/arch-mx28/regs-bch.h |
> 42 ++--
> arch/arm/include/asm/arch-mx28/regs-clkctrl.h | 101 +++++------
> arch/arm/include/asm/arch-mx28/regs-common.h | 28 ++-
> arch/arm/include/asm/arch-mx28/regs-gpmi.h | 26 ++--
> arch/arm/include/asm/arch-mx28/regs-i2c.h | 28 ++--
> arch/arm/include/asm/arch-mx28/regs-ocotp.h | 86 ++++----
> arch/arm/include/asm/arch-mx28/regs-pinctrl.h | 168 ++++++++--------
> arch/arm/include/asm/arch-mx28/regs-power.h | 28 ++--
> arch/arm/include/asm/arch-mx28/regs-rtc.h | 28 ++--
> arch/arm/include/asm/arch-mx28/regs-ssp.h | 40 ++--
> arch/arm/include/asm/arch-mx28/regs-timrot.h | 38 ++--
> arch/arm/include/asm/arch-mx28/regs-usbphy.h | 20 +-
> arch/arm/include/asm/arch-mx28/sys_proto.h | 6 +-
> drivers/gpio/mxs_gpio.c | 16 +-
> drivers/usb/host/ehci-mxs.c | 8 +-
> 20 files changed, 505 insertions(+), 521 deletions(-)
>
> diff --git a/arch/arm/cpu/arm926ejs/mx28/clock.c
> b/arch/arm/cpu/arm926ejs/mx28/clock.c index f698506..c0eea9e 100644
> --- a/arch/arm/cpu/arm926ejs/mx28/clock.c
> +++ b/arch/arm/cpu/arm926ejs/mx28/clock.c
> @@ -46,8 +46,8 @@ static uint32_t mx28_get_pclk(void)
> struct mx28_clkctrl_regs *clkctrl_regs =
> (struct mx28_clkctrl_regs *)MXS_CLKCTRL_BASE;
>
> - uint32_t clkctrl, clkseq, clkfrac;
> - uint32_t frac, div;
> + uint32_t clkctrl, clkseq, div;
> + uint8_t clkfrac, frac;
>
> clkctrl = readl(&clkctrl_regs->hw_clkctrl_cpu);
>
> @@ -67,8 +67,8 @@ static uint32_t mx28_get_pclk(void)
> }
>
> /* REF Path */
> - clkfrac = readl(&clkctrl_regs->hw_clkctrl_frac0);
> - frac = clkfrac & CLKCTRL_FRAC0_CPUFRAC_MASK;
> + clkfrac = readb(&clkctrl_regs->hw_clkctrl_frac0[CLKCTRL_FRAC0_CPU]);
> + frac = clkfrac & CLKCTRL_FRAC0_FRAC_MASK;
> div = clkctrl & CLKCTRL_CPU_DIV_CPU_MASK;
> return (PLL_FREQ_MHZ * PLL_FREQ_COEF / frac) / div;
> }
> @@ -96,8 +96,8 @@ static uint32_t mx28_get_emiclk(void)
> struct mx28_clkctrl_regs *clkctrl_regs =
> (struct mx28_clkctrl_regs *)MXS_CLKCTRL_BASE;
>
> - uint32_t frac, div;
> - uint32_t clkctrl, clkseq, clkfrac;
> + uint32_t clkctrl, clkseq, div;
> + uint8_t clkfrac, frac;
>
> clkseq = readl(&clkctrl_regs->hw_clkctrl_clkseq);
> clkctrl = readl(&clkctrl_regs->hw_clkctrl_emi);
> @@ -109,11 +109,9 @@ static uint32_t mx28_get_emiclk(void)
> return XTAL_FREQ_MHZ / div;
> }
>
> - clkfrac = readl(&clkctrl_regs->hw_clkctrl_frac0);
> -
> /* REF Path */
> - frac = (clkfrac & CLKCTRL_FRAC0_EMIFRAC_MASK) >>
> - CLKCTRL_FRAC0_EMIFRAC_OFFSET;
> + clkfrac = readb(&clkctrl_regs->hw_clkctrl_frac0[CLKCTRL_FRAC0_EMI]);
> + frac = clkfrac & CLKCTRL_FRAC0_FRAC_MASK;
> div = clkctrl & CLKCTRL_EMI_DIV_EMI_MASK;
> return (PLL_FREQ_MHZ * PLL_FREQ_COEF / frac) / div;
> }
> @@ -123,8 +121,8 @@ static uint32_t mx28_get_gpmiclk(void)
> struct mx28_clkctrl_regs *clkctrl_regs =
> (struct mx28_clkctrl_regs *)MXS_CLKCTRL_BASE;
>
> - uint32_t frac, div;
> - uint32_t clkctrl, clkseq, clkfrac;
> + uint32_t clkctrl, clkseq, div;
> + uint8_t clkfrac, frac;
>
> clkseq = readl(&clkctrl_regs->hw_clkctrl_clkseq);
> clkctrl = readl(&clkctrl_regs->hw_clkctrl_gpmi);
> @@ -135,11 +133,9 @@ static uint32_t mx28_get_gpmiclk(void)
> return XTAL_FREQ_MHZ / div;
> }
>
> - clkfrac = readl(&clkctrl_regs->hw_clkctrl_frac1);
> -
> /* REF Path */
> - frac = (clkfrac & CLKCTRL_FRAC1_GPMIFRAC_MASK) >>
> - CLKCTRL_FRAC1_GPMIFRAC_OFFSET;
> + clkfrac = readb(&clkctrl_regs->hw_clkctrl_frac1[CLKCTRL_FRAC1_GPMI]);
> + frac = clkfrac & CLKCTRL_FRAC1_FRAC_MASK;
> div = clkctrl & CLKCTRL_GPMI_DIV_MASK;
> return (PLL_FREQ_MHZ * PLL_FREQ_COEF / frac) / div;
> }
> @@ -152,11 +148,12 @@ void mx28_set_ioclk(enum mxs_ioclock io, uint32_t
> freq) struct mx28_clkctrl_regs *clkctrl_regs =
> (struct mx28_clkctrl_regs *)MXS_CLKCTRL_BASE;
> uint32_t div;
> + int io_reg;
>
> if (freq == 0)
> return;
>
> - if (io > MXC_IOCLK1)
> + if ((io < MXC_IOCLK0) || (io > MXC_IOCLK1))
> return;
>
> div = (PLL_FREQ_KHZ * PLL_FREQ_COEF) / freq;
> @@ -167,23 +164,13 @@ void mx28_set_ioclk(enum mxs_ioclock io, uint32_t
> freq) if (div > 35)
> div = 35;
>
> - if (io == MXC_IOCLK0) {
> - writel(CLKCTRL_FRAC0_CLKGATEIO0,
> - &clkctrl_regs->hw_clkctrl_frac0_set);
> - clrsetbits_le32(&clkctrl_regs->hw_clkctrl_frac0,
> - CLKCTRL_FRAC0_IO0FRAC_MASK,
> - div << CLKCTRL_FRAC0_IO0FRAC_OFFSET);
> - writel(CLKCTRL_FRAC0_CLKGATEIO0,
> - &clkctrl_regs->hw_clkctrl_frac0_clr);
> - } else {
> - writel(CLKCTRL_FRAC0_CLKGATEIO1,
> - &clkctrl_regs->hw_clkctrl_frac0_set);
> - clrsetbits_le32(&clkctrl_regs->hw_clkctrl_frac0,
> - CLKCTRL_FRAC0_IO1FRAC_MASK,
> - div << CLKCTRL_FRAC0_IO1FRAC_OFFSET);
> - writel(CLKCTRL_FRAC0_CLKGATEIO1,
> - &clkctrl_regs->hw_clkctrl_frac0_clr);
> - }
I think you're mixing two things together above. This patch and some kind of a
cleanup. But ok, thinking of this, it seems context related.
> + io_reg = CLKCTRL_FRAC0_IO0 - (io - MXC_IOCLK0);
Uh ... ioreg = (io == MXC_IOCLK0) ? something : another; or stuff like that
might be better. The math above is really confusing. Or you can even enumerate
enum mxs_ioclock so that you won't need this math at all, which is even better.
> + writeb(CLKCTRL_FRAC0_CLKGATE,
> + &clkctrl_regs->hw_clkctrl_frac0_set[io_reg]);
> + writeb(CLKCTRL_FRAC0_CLKGATE | (div & CLKCTRL_FRAC0_FRAC_MASK),
> + &clkctrl_regs->hw_clkctrl_frac0[io_reg]);
> + writeb(CLKCTRL_FRAC0_CLKGATE,
> + &clkctrl_regs->hw_clkctrl_frac0_clr[io_reg]);
> }
>
> /*
> @@ -193,19 +180,16 @@ static uint32_t mx28_get_ioclk(enum mxs_ioclock io)
> {
> struct mx28_clkctrl_regs *clkctrl_regs =
> (struct mx28_clkctrl_regs *)MXS_CLKCTRL_BASE;
> - uint32_t tmp, ret;
> + uint8_t ret;
> + int io_reg;
>
> - if (io > MXC_IOCLK1)
> + if ((io < MXC_IOCLK0) || (io > MXC_IOCLK1))
> return 0;
>
> - tmp = readl(&clkctrl_regs->hw_clkctrl_frac0);
> + io_reg = CLKCTRL_FRAC0_IO0 - (io - MXC_IOCLK0);
>
> - if (io == MXC_IOCLK0)
> - ret = (tmp & CLKCTRL_FRAC0_IO0FRAC_MASK) >>
> - CLKCTRL_FRAC0_IO0FRAC_OFFSET;
> - else
> - ret = (tmp & CLKCTRL_FRAC0_IO1FRAC_MASK) >>
> - CLKCTRL_FRAC0_IO1FRAC_OFFSET;
> + ret = readb(&clkctrl_regs->hw_clkctrl_frac0[io_reg]) &
> + CLKCTRL_FRAC0_FRAC_MASK;
>
> return (PLL_FREQ_KHZ * PLL_FREQ_COEF) / ret;
> }
> @@ -223,7 +207,7 @@ void mx28_set_sspclk(enum mxs_sspclock ssp, uint32_t
> freq, int xtal) return;
>
> clkreg = (uint32_t)(&clkctrl_regs->hw_clkctrl_ssp0) +
> - (ssp * sizeof(struct mx28_register));
> + (ssp * sizeof(struct mx28_register_32));
>
> clrbits_le32(clkreg, CLKCTRL_SSP_CLKGATE);
> while (readl(clkreg) & CLKCTRL_SSP_CLKGATE)
> @@ -272,7 +256,7 @@ static uint32_t mx28_get_sspclk(enum mxs_sspclock ssp)
> return XTAL_FREQ_KHZ;
>
> clkreg = (uint32_t)(&clkctrl_regs->hw_clkctrl_ssp0) +
> - (ssp * sizeof(struct mx28_register));
> + (ssp * sizeof(struct mx28_register_32));
>
> tmp = readl(clkreg) & CLKCTRL_SSP_DIV_MASK;
>
> diff --git a/arch/arm/cpu/arm926ejs/mx28/iomux.c
> b/arch/arm/cpu/arm926ejs/mx28/iomux.c index 9ea411f..12916b6 100644
> --- a/arch/arm/cpu/arm926ejs/mx28/iomux.c
> +++ b/arch/arm/cpu/arm926ejs/mx28/iomux.c
> @@ -43,7 +43,7 @@ int mxs_iomux_setup_pad(iomux_cfg_t pad)
> {
> u32 reg, ofs, bp, bm;
> void *iomux_base = (void *)MXS_PINCTRL_BASE;
> - struct mx28_register *mxs_reg;
> + struct mx28_register_32 *mxs_reg;
>
> /* muxsel */
> ofs = 0x100;
> @@ -70,7 +70,7 @@ int mxs_iomux_setup_pad(iomux_cfg_t pad)
> /* vol */
> if (PAD_VOL_VALID(pad)) {
> bp = PAD_PIN(pad) % 8 * 4 + 2;
> - mxs_reg = (struct mx28_register *)(iomux_base + ofs);
> + mxs_reg = (struct mx28_register_32 *)(iomux_base + ofs);
> if (PAD_VOL(pad))
> writel(1 << bp, &mxs_reg->reg_set);
> else
> @@ -82,7 +82,7 @@ int mxs_iomux_setup_pad(iomux_cfg_t pad)
> ofs = PULL_OFFSET;
> ofs += PAD_BANK(pad) * 0x10;
> bp = PAD_PIN(pad);
> - mxs_reg = (struct mx28_register *)(iomux_base + ofs);
> + mxs_reg = (struct mx28_register_32 *)(iomux_base + ofs);
> if (PAD_PULL(pad))
> writel(1 << bp, &mxs_reg->reg_set);
> else
> diff --git a/arch/arm/cpu/arm926ejs/mx28/mx28.c
> b/arch/arm/cpu/arm926ejs/mx28/mx28.c index 683777f..0e69193 100644
> --- a/arch/arm/cpu/arm926ejs/mx28/mx28.c
> +++ b/arch/arm/cpu/arm926ejs/mx28/mx28.c
> @@ -63,7 +63,7 @@ void reset_cpu(ulong ignored)
> ;
> }
>
> -int mx28_wait_mask_set(struct mx28_register *reg, uint32_t mask, int
> timeout) +int mx28_wait_mask_set(struct mx28_register_32 *reg, uint32_t
Can you actually separate out the rename to register_32 and then the fixup patch
for register_8?
Rest seems ok
M
More information about the U-Boot
mailing list