[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