From: Marek Vasut <marek.vasut@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] v2 i.MX28: Fix ref_cpu clock setup
Date: Mon, 6 Feb 2012 13:20:16 +0100 [thread overview]
Message-ID: <201202061320.16232.marek.vasut@gmail.com> (raw)
In-Reply-To: <6EA3E0BCC03CC34B89B01BD57ECBC718F65163@POBOX.postoffice.danego.net>
> 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@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
next parent reply other threads:[~2012-02-06 12:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <6EA3E0BCC03CC34B89B01BD57ECBC718F65163@POBOX.postoffice.danego.net>
2012-02-06 12:20 ` Marek Vasut [this message]
2012-02-06 12:33 ` [U-Boot] [PATCH 1/2] v2 i.MX28: Fix ref_cpu clock setup Robert Deliën
2012-02-06 12:49 ` Marek Vasut
2012-02-06 13:03 ` Robert Deliën
2012-02-06 13:15 ` Marek Vasut
2012-02-06 12:56 ` Marek Vasut
2012-02-06 14:28 ` Robert Deliën
2012-02-06 14:40 ` Marek Vasut
2012-02-06 12:20 ` Marek Vasut
2012-02-06 12:38 ` Robert Deliën
2012-02-06 12:49 ` Marek Vasut
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201202061320.16232.marek.vasut@gmail.com \
--to=marek.vasut@gmail.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.