[U-Boot] [PATCH 1/2] v2 i.MX28: Fix ref_cpu clock setup

Robert Deliën robert at delien.nl
Mon Feb 6 13:33:39 CET 2012


Hi Marek,

> -     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.

Yeah, duplicate code didn't feel right. Besides, the code didn't check for values
larger than MXC_IOCLK0, which is does now.

> > +     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.

I came accross an enumerator in the code, but I found that too big of a change
to slip in, because it alters the external interface. But we're not done with the code
yet. Plenty of oppertunities left.

ioreg = (io == MXC_IOCLK0) is not the same. The actual value of
io != MXC_IOCLK0 it compiler implementation depending.

> Can you actually separate out the rename to register_32 and then the fixup patch
> for register_8?

I'd rather not: I'm down to my neck it work and I don't these two parts in my archive
so that means manual merging, verifying and for another two hours.

> Rest seems ok

Can you see if our server swapped tabs for spaces? It's beyond my grasp how M$
considers it a good idea to alter to contents of my email.

Cheers,

        Robert.


More information about the U-Boot mailing list