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

[Ilya Yanok]

Hi Govindraj,

On 22.12.2011 10:55, Govindraj wrote:
>>
>> Signed-off-by: Ilya Yanok <yanok at emcraft.com>
>> ---
>> Changes from V3:
>>  - None
>> Changes from V2:
>>  - None
>> Changes from V1:
>>  - CONFIG_OMAP_EHCI_PHYx_RESET changed to CONFIG_OMAP_EHCI_PHYx_RESET_GPIO
>>  - phy reset moved to separate function
>>  - Calls to gpio_set_value after gpio_direction_output removed
>>
> 
> Here I see lot of stuff are just being moved from beagle board to new file
> ehci-omap.c , but echi-omap.c is not generic enough for re-use
> I think we should maintain clock handling and any gpio reset in board
> file itself
> and just implement stuff related to ehci only like tll reset and ulpi
> phy configuration or
> hsic mode configuration based on some data passed from board file.

I can just say that I agree with Igor on his points. If think we should
minimize board code, if something is not board-specific (like clocks) or
is generic enough (like ULPI PHY GPIO reset) we should definitely try to
put such code in some common place and only provide some values via
board configuration.

> So we can re-use most of ehci-omap.c, I have done a similar approach
> as said which
> is intending to provide generic ehci-omap.c usage, which I was planning to
> re-use for beagle board, But since you are at it already can we make ehci-omap.c
> more generic.
> 
> As we are planning to re-use ehci-omap.c for panda and other board support.
> Reference to patch done for panda initially.
> 
> http://patchwork.ozlabs.org/patch/131362/

Yes, I've seen your patches, it will be great to have common OMAP EHCI
driver.

> One thing that I planning to do in this patch is moving all reg
> offsets from asm/ehci.h to
> include/asm/arch-omap4/ehci.h and retain only generic stuff in ehci.h
> thus we can have offsets from beagle from */arch-omap3/ehci.h and

Hm. I don't really work with OMAP4 but do the offsets differ so much?
Probably it makes sense to have some common header plus SoC specific part?

>> +#include <asm/arch/ehci_omap3.h>
> 
> if this can be renamed to just ehci.h it will be better
> from re-use perspective since I can add just ehci.h for omap4/5
> in the arch folders and based on config used it will pickup the
> file used.

Surely we can rename it.

>>  #define CONFIG_USB_EHCI
>> +#define CONFIG_USB_EHCI_OMAP
>> +/*#define CONFIG_EHCI_DCACHE*/ /* leave it disabled for now */
> 
> Any reason for this?

Yes, common USB code is not cache-aware anyway so you have to disable
dcache before working with USB anyway.

I've send a patch for USB base support a while ago
(see http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/114235 )
but it only fixes common/usb.c. Upper layers have to be fixed too.

> Let me know if you intend to clean up it up or I can take up and
> re-base my stuff by
> moving back the clock and gpio stuff back to beagle board.
> or can we re-use ehci-omap.c done for panda for beagle?

Surely you can take it. I'd glad to see common OMAP EHCI driver.

Regards, Ilya.