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.
Not really, fixing the enumeration should be fine I believe.
ioreg = (io == MXC_IOCLK0) is not the same. The actual value of io != MXC_IOCLK0 it compiler implementation depending.
That's why I put the ternary operator there ?
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.
It's impossible to review like this though and much more prone to pull in bugs.
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.
It didnt.
Cheers,
Robert.