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

[Govindraj]

Hi Igor,

On Wed, Jan 11, 2012 at 8:33 PM, Igor Grinberg <grinberg at compulab.co.il> wrote:
> Hi Guys,
>
> On 01/11/12 16:34, Marek Vasut wrote:
>>> 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).
>
> Well, it is on my list, actually,
> but I will be able to get to it only in a couple of days.
> (I'm really busy right now).
>

Could you please let me know what exactly that you will be
updating?

So that I can accordingly post my v3 of this patch fixing comments
from Marek Vasut <marek.vasut at gmail.com>

--
Thanks,
Govindraj.R