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

[Govindraj]

On Thu, Dec 22, 2011 at 4:48 PM, Igor Grinberg <grinberg at compulab.co.il> wrote:
> 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).
>

Yes Agree..

>>
>> 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...
>

Looks fine.

I will start on the above discussed stuff on top of this patch.

--
Thanks,
Govindraj.R