[U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

[Marek Vasut]

> On Wed, Jan 11, 2012 at 6:58 PM, Marek Vasut <marek.vasut at gmail.com> wrote:
> >> On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut <marek.vasut at gmail.com> wrote:
> >> >> On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut <marek.vasut at gmail.com> 
wrote:
> >> >> >> Hi Marek,
> >> >> >> 
> >> >> >> Thanks for you review.
> >> >> >> 
> >> >> >> On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut
> >> >> >> <marek.vasut at gmail.com>
> > 
> > wrote:
> >> >> >> >> From: "Govindraj.R" <govindraj.raja at ti.com>
> >> >> >> >> 
> >> >> >> >> Clean up added ehci-omap.c and make it generic for re-use
> >> >> >> >> across soc having same ehci ip block. Also pass the modes to
> >> >> >> >> be configured and configure the ports accordingly. All usb
> >> >> >> >> layers are not cache aligned till then keep cache off for usb
> >> >> >> >> ops as ehci will use internally dma for all usb ops.
> >> >> >> >> 
> >> >> >> >> * Add a generic common header ehci-omap.h having common ip
> >> >> >> >> block data and reg shifts.
> >> >> >> >> * Rename and modify ehci-omap3 to ehci.h retain only
> >> >> >> >> conflicting sysc reg shifts remove others and move to common
> >> >> >> >> header file.
> >> >> >> > 
> >> >> >> > Don't reimplement the ulpi stuff ... there's already some ulpi
> >> >> >> > stuff in uboot that needs fixing, so fix it and use it.
> >> >> >> 
> >> >> >> I am not implementing any ulpi stuff I am just configuring OMAP on
> >> >> >> soc usb host controller (ehci). All the configuration stuff
> >> >> >> is OMAP specific things which are done in ehci-omap.c file
> >> >> >> 
> >> >> >> stuffs done are like soft-reset, port mode to be used and putting
> >> >> >> port in no -idle mode(omap specific pm implementation) etc.
> >> >> > 
> >> >> > This stuff:
> >> >> > 
> >> >> > +/* ULPI */
> >> >> > +#define ULPI_SET(a)                                    (a + 1)
> >> >> > +#define ULPI_CLR(a)                                    (a + 2)
> >> >> > +#define ULPI_FUNC_CTRL                                 0x04
> >> >> > +#define ULPI_FUNC_CTRL_RESET                           (1 << 5)
> >> >> > 
> >> >> > is just accidentally conforming to ULPI spec?
> >> >> 
> >> >> These are for configuring INSNREG05_ULPI reg in EHCI reg map
> >> >> of omap while configuring in ulpi-phy mode.
> >> >> 
> >> >> looking into struct ulpi_regs {..}
> >> >> then it doesn't map this configuration.
> >> > 
> >> > Can you point me to some documentation about this please? It's not
> >> > that I don't trust you, I'd rather prefer to avoid unnecessary
> >> > duplication.
> >> 
> >> Yes that would be fine.
> >> 
> >> You can download the omap4460 public trm from here:
> >> 
> >> http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip
> >> 
> >> Go to chapter 23.11.6.6.1 EHCI Register Summary
> >> (page number 5171 and 5186/87)
> > 
> > Sure, but the macro above looks more like 23.11.6.3, doesn't it ? And for
> > that purpose, the struct ulpi_regs is fitting ok.
> > 
> > Actually ... can you check the ulpi_read and ulpi_write stuff that's
> > already in u-boot and explain why they can not be used with this port?
> 
> echi-omap.c is no where writing to those registers
> and the macro was used only to configure INSNREG05_ULPI reg in EHCI reg map
> 
> reg map in 23.11.6.3 used only for a external ulpi-phy communication.
> and debug purpose(to view vid, pid etc) and to hack external phy
> configuration through ulpi commands
> from omap - usb host controller point of view only needs
> INSNREG05_ULPI reg in EHCI reg configuration
> rest on soc host controller takes care of it.

Can someone else comment on this? I think I don't understand well (as I'm not 
OMAP guy).

M
> 
> --
> Thanks,
> Govindraj.R