Wolfgang Denk wrote:
Dear Dirk,
In message 49A427D1.6050700@googlemail.com you wrote:
Wolfgang Denk wrote:
Dear Nishanth Menon,
In message 49A296F0.4000509@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 ;)
That was not my intention. What I meant was that the statement was correct. U-Boot design rules say "Initialize devices only when they are needed within U-Boot" (see for example http://www.denx.de/wiki/view/U-Boot/DesignPrinciples#2_Keep_it_Fast), and applied to clocks that means we should enable clocks only when we really need them. There are many reasons for that, power consumption being one of them.
While I totally agree, I think the point of discussion (misunderstanding?) is what does "_enabling_ clock only if needed" mean.
If "needed" means that the clocks are needed to execute a specific command within U-Boot.
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
One can argue like that, but it's wrong. The intention is to enable interfaces only when really *used* by U-Boot to run some command. So if nobody ever executes an USB command in U-Boot, then the clocks should NOT be enabled.
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 ;)
Only if serial output over USB is permanently enabled this would make sense. Otherwise, there is no reason to turn the clocks on before a command in U-Boot turns on the serial output over USB.
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
Yes, that is the intention.
Will it get an ACK if we change
--- u-boot-main.orig/cpu/arm_cortexa8/omap3/clock.c +++ u-boot-main/cpu/arm_cortexa8/omap3/clock.c @@ -377,5 +377,10 @@ void per_clocks_enable(void) sr32(&prcm_base->fclken_per, 0, 32, FCK_PER_ON); sr32(&prcm_base->iclken_per, 0, 32, ICK_PER_ON);
+#ifdef CONFIG_MUSB + /* Enable the MUSB interface clock */ + sr32(&prcm_base->iclken1_core, 4, 1, 0x1); +#endif
to something like
+++ u-boot-main/cpu/arm_cortexa8/omap3/clock.c #ifdef CONFIG_MUSB void enable_musb_clock(void) { sr32(&prcm_base->iclken1_core, 4, 1, 0x1); }
void disable_musb_clock(void) { sr32(&prcm_base->iclken1_core, 4, 1, 0x0); } #endif
and then call enable/disable from MUSB code at appropriate places?
Best regards
Dirk