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

[Govindraj]

Hi Igor,

On Sun, Jan 22, 2012 at 5:50 PM, Igor Grinberg <grinberg at compulab.co.il> wrote:
> On 01/19/12 10:15, Govindraj wrote:
>> On Wed, Jan 18, 2012 at 11:21 PM, Igor Grinberg <grinberg at compulab.co.il> wrote:
>>> Hi Govindraj,
>>>
>>> On 01/17/12 08:10, Govindraj wrote:
>>>>

[...]

>>>> +             if (get_timer(init) > CONFIG_SYS_HZ) {
>>>> +                     debug("OMAP EHCI error: timeout resetting phy\n");
>>>> +                     break;
>>>> +             }
>>>> +}
>>>
>>> Ok, this function is kind of "duplication" of the ULPI code.
>>> ulpi_reset() function in drivers/usb/ulpi/ulpi.c provides an implementation
>>> of the ULPI spec. and should be used by the drivers.
>>> What it lacks currently, is a way to pass a port number to the viewport
>>> implementation and of course the omap-ulpi-viewport(.c) implementation itself...
>>> So, IMO, the right way would be to implement ULPI accessors (omap-ulpi-viewport.c)
>>
>> so you mean add omap-ulpi-viewport.c which will do ulpi read writes
>> for ulpi implementation
>> within tll module of omap host controller.
>
> No. What I meant is that omap-ulpi-viewport.c will do the ULPI access
> to the PHY (which is not TLL), but after the above question, I think
> it can do both: TLL and non-TLL.
>
>>
>> we just need func reset to be done for ulpi which is done using ehci register
>> INSNREG05_ULPI. IMHO I don't see any use case or true requirement of
>> omap-ulpi-viewport.c framework.
>
> The fact that the reset is done by writing the ULPI_FUNC_CTRL_RESET bit
> of the ULPI_FUNC_CTRL register - is the requirement...
> For example tomorrow, you will find out that besides reset, you also need
> to set some other bit in a register inside the ULPI PHY (e.g. VBUS), so
> you will implement another "ehci" function that will do a write to ULPI
> and thus duplicate another portion of code...
>
>>
>>> and add an ability to pass some kind of private data to the viewport, which
>>> in case of OMAP will be the port number.


I started on adding omap-ulpi-viewport.c which will work with ulpi.c
if omap_ehci.c is used.

for port id can we just set a global data field that will inform the
omap_view port
on the port id, or we have to modify most api's syntax in
"drivers/usb/ulpi/ulpi.c"

Is it okay to have the port id from ehci-omap.c set and used in
"drivers/usb/ulpi/omap-ulpi-viewport.c" ?

--
Thanks,
Govindraj.R