From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dirk Behme <dirk.behme@googlemail.com>
Date: Wed, 25 Feb 2009 18:39:00 +0100
Subject: [U-Boot] [PATCH 6/7 v2] OMAP3: Add OMAP3 core changes for MUSB
In-Reply-To: <20090224212244.183AD832E43F@gemini.denx.de>
References: <1234464946-20400-1-git-send-email-dirk.behme@googlemail.com>
	<1234464946-20400-2-git-send-email-dirk.behme@googlemail.com>
	<1234464946-20400-3-git-send-email-dirk.behme@googlemail.com>
	<1234464946-20400-4-git-send-email-dirk.behme@googlemail.com>
	<1234464946-20400-5-git-send-email-dirk.behme@googlemail.com>
	<1234464946-20400-6-git-send-email-dirk.behme@googlemail.com>
	<1234464946-20400-7-git-send-email-dirk.behme@googlemail.com>
	<20090222155940.GD9867@game.jcrosoft.org>
	<49A17BCD.1030803@googlemail.com>
	<6ed0b2680902230413o33f98059q7239cbba0114c2b3@mail.gmail.com>
	<49A296F0.4000509@gmail.com>
	<20090223213636.8E526832E43F@gemini.denx.de>
	<49A427D1.6050700@googlemail.com>
	<20090224212244.183AD832E43F@gemini.denx.de>
Message-ID: <49A58234.2060500@googlemail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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