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

[Govindraj]

On Thu, Jan 12, 2012 at 2:53 PM, Igor Grinberg <grinberg at compulab.co.il> wrote:
> Hi Govindraj,
>
> On 01/12/12 07:45, Govindraj wrote:
>> 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>
>
> Well, I did not say, I'm going to update anything.
> What I meant is that I'm going to look into TI's documentation
> regarding EHCI and the ULPI to understand the dependencies and see
> how your code meets those and if the generic ULPI layer can be used
> for that.
>

okay, Thanks,

> So to lower the work load from from you, I'd suggest you to wait
> till Monday (if you can of course) to let me look into this.

Yes Sure.

--
Thanks,
Govindraj.R