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

[Dirk Behme]

Wolfgang Denk wrote:
> Dear Dirk,
> 
> In message <49A427D1.6050700 at googlemail.com> you wrote:
>> 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 ;)
> 
> 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