Hi Govindraj,
On 22.12.2011 10:55, Govindraj wrote:
Signed-off-by: Ilya Yanok yanok@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.
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.