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

[Dirk Behme]

Wolfgang Denk wrote:
> Dear Nishanth Menon,
> 
> In message <49A296F0.4000509 at gmail.com> you wrote:
>>> He probably wants to say that clocks should be enabled only when "usb
>>> start" is issued, as you might have u-boot compiled with USB defines
>>> set, but never actually use USB.
> 
> Correct.

Don't get me wrong, but I find it a little funny that we are 
speculating (?) here about what someone else might have wanted to say ;)

>> Comparing having all clock initialization at a central point (clock.c) -
>> which seems simple vs having get-put kind of clock apis in U-boot : I
> 
> But the explicit rule is only to enable interfaces (and this includes
> clocks) when they are actually being used. Enabling all clocks even if
> not needed may for example add significantly to the power consumption
> and to EMC problems.

While I totally agree, I think the point of discussion 
(misunderstanding?) is what does "_enabling_ clock only if needed" mean.

One can argue that "enabling clock only if needed" is done by

#ifdef MUSB
  enable_musbclock()
#endif

While doing this, clock is enabled if somebody _enables_ MUSB in 
config. While doing this, clock is enabled when interface is enabled 
(at compile time), assuming that user who enables interface in config 
knows why and uses it. Else it makes no sense to enable it (in 
config). And by enabling serial output over USB in upcoming patch, the 
interface is used. Seems that this is my point of view ;)

Other point of view of "enabling clock only if need" can be "enable 
clock only if code is compiled into uboot _and_ is accessed (e.g. by 
serial output over USB)" (i.e. runtime enable). I think this what 
Wolfgang's point of view is. I wonder why someone might enable an 
interface for an bootloader at compile time, getting a larger binary, 
but then not using it, though. But this might be an other topic ;)

>> u-boot is supposed to "keep it simple", enable/disable clocks on a need
>> basis is elegant, though could end up getting extended to i2c and other
> 
> Yes, indeed - if you followed the recent discussion about I2C rework
> this is indeed under consideration.
> 
>> peripherals too(if I were to stretch is pretty hard ;) ).. makes more
>> sense in kernel(which is already there) than here - my 2 cents..
> 
> Well, we're trying to improve...

I wonder a little why you try to improve code that is ideally running 
< 10s like a bootloader for e.g. power consumption aspects instead of 
keeping it small, simple and easy to maintain. But this might be an 
other topic, too ;)

Best regards

Dirk