[U-Boot] [PATCH V4 1/2] ehci-omap: driver for EHCI host on OMAP3

[Igor Grinberg]

On 12/22/11 12:49, Govindraj wrote:
> On Thu, Dec 22, 2011 at 3:29 PM, Igor Grinberg <grinberg at compulab.co.il> wrote:
>> On 12/22/11 11:26, Govindraj wrote:
>>> Hi Igor,
>>>
> 
> [...]
> 
>>>
>>> probably the right method to do is as we did for panda,
>>>
>>> Add to armv7/omap3/clock.c (soc specific code) to enable usb-host clocks.
>>>
>>> as done in below patch for omap4 socs.
>>>
>>> http://patchwork.ozlabs.org/patch/131365/
>>
>> Hmmm... I don't like that patch - this is the right method...
>> but only for panda!
>> For panda it makes sense to enable the USB related clocks by default
>> because it has many of its boot important peripherals wired to USB.
>> That is not the case with majority of OMAP3 (and I bet with many
>> other OMAP4) boards.
>> Therefore, I think the USB clocks need to be turned on
>> only if the board requests them to be turned on and not as a part
>> of the default clock initialization by the PRCM subsystem
>> (unless it is configurable in the board config file).
>>
> 
> okay, I thought nothing wrong in keeping them enabled by default.
> since all un-used clocks will be gated once kernel is loaded.
> Am I missing some thing here?

May be there is nothing *too* wrong except for power consumption or
may be some other things I can't think of right now...
But, you should not rely on what will happen when kernel is loaded.
Also, you should not assume, that the Linux kernel will be loaded at all...
There are other OSes around that can also use U-Boot as a boot loader.
Also, there are stand alone applications that do not use any OS at all...
We are in the embedded world also, not just mobile...
(And if we are talking about mobile, then power consumption is important).

> 
> so as discussed earlier we can add ehci_omap3_clock_init
> into ehci-omap.c that can be used from all omap3 socs.

This is a bit confusing...
Let's try it that way:
1) ehci-omap.c should have common OMAP (OMAP3/4/5...) settings
2) ehci-omap.c should call ehci_clk_{enable|disable}() which should
   be implemented in a SoC *dependent* way (e.g. armv7/omap{3|4}/clock.c)
3) board specific stuff (e.g. PHY reset GPIO) should be passed
   to the common code to avoid code duplication.
4) probably, some other things that I've forgotten...


-- 
Regards,
Igor.