[U-Boot] [PATCH] Davinci: add a pin multiplexer configuration API

Wolfgang Denk wd at denx.de
Fri Oct 30 09:26:12 CET 2009


Dear Kim Phillips,

In message <20091029182304.469c9f7f.kim.phillips at freescale.com> you wrote:
>
> > +		if (field < PIN_MUX_NUM_FIELDS &&
> > +			(value & ~PIN_MUX_FIELD_MASK) == 0) {
> 
> the second line should not be indented as though it is the code
> subblock; it should fall directly underneath the column where 'field
> <..' starts, like this:

Agreed.

> if (field < PIN_MUX_NUM_FIELDS &&
>     (value & ~PIN_MUX_FIELD_MASK) == 0) {
> 
> > +			int offset = field * PIN_MUX_FIELD_SIZE;
> > +			unsigned int mux = pins[i].mux;
> > +			unsigned int mask = PIN_MUX_FIELD_MASK << offset;
> 
> also please just declare everything at the top of the function - same
> for value and field declarations above.

No! Why should that be needed? It would be just a waste of stack
space (except that recent compilers don't care abouyt this anyway),
and keeping variables as localized as possible seems to be a good
thing to me.

> > +			value <<= offset;
> > +			writel(value | (readl(mux) & (~mask)), mux);
> 
> I guess arm doesn't have setbits32 and friends, huh.

Not yet. Patches welcome!

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Evil does seek to maintain power by suppressing the truth."
"Or by misleading the innocent."
	-- Spock and McCoy, "And The Children Shall Lead",
	   stardate 5029.5.


More information about the U-Boot mailing list