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

Dirk Behme
2009-02-25T17:39:00Z

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