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

[Igor Grinberg]

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


-- 
Regards,
Igor.