[U-Boot] [PATCH 6/7 v2] OMAP3: Add OMAP3 core changes for MUSB

[Wolfgang Denk]

Dear Dirk,

In message <49AA9B19.6060907 at googlemail.com> you wrote:
>
> > Will it get an ACK if we change
...
> > and then call enable/disable from MUSB code at appropriate places?
> 
> Any hint if changing the patch doing something like above as proposed 
> some days ago [1] would get an ack?

I'd like to see the patch, but so far I don;t see any problems with
that.

Umm.... but this posting triggered me to look up what this function
sr32() might be doing...

I think this could need some cleanup...

"cpu/arm_cortexa8/omap3/syslib.c":

 43 /*****************************************************************
 44  * sr32 - clear & set a value in a bit range for a 32 bit address
 45  *****************************************************************/
 46 void sr32(void *addr, u32 start_bit, u32 num_bits, u32 value)
 47 {
 48         u32 tmp, msk = 0;
 49         msk = 1 << num_bits;

It makes no sense to initialize msk with 0 when you set another value
right in the next statement.

 50         --msk;

I think the code would be clearer if you wrote:

	msk = (1 << num_bits) - 1;
	
 51         tmp = readl((u32)addr) & ~(msk << start_bit);
 52         tmp |= value << start_bit;

Wouldn't it be better to make sure that "value" does not exceed the
bit range? Like that:

	tmp |= (value & ~msk) << start_bit;

?


Also, I find code that is based on bit numbers, bit widths and values
*very* difficult to read. Do you really think that for example

	sr32(&prcm_base->iclken1_core, 4, 1, 0x1);

is easier to read than

	clrsetbits(u32, &prcm_base->iclken1_core, 0x10, 0x10);

?

And actually, just to set or clear values we don't even need that, a
plain

	setbits(u32, &prcm_base->iclken1_core, 0x10);

would do as well.

[Look up the definitions in include/asm-ppc/io.h if needed].

And probably 0x10 could be even replaced by some MUSB_INTERFACE_CLK
#define from some header file?



I am aware that this is work for a future release, but I think it
would *greatly* improve the code. It would, most of all, make it
readable.

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
Space is big. You just won't believe how vastly, hugely, mind-
bogglingly big it is. I mean, you may think it's a long way down the
road to the drug store, but that's just peanuts to space.
                              -- The Hitchhiker's Guide to the Galaxy