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

[Igor Grinberg]

On 02/03/12 14:50, Govindraj wrote:

[...]

>>>
>>>  COBJS := $(COBJS-y)
>>>  SRCS  := $(COBJS:.o=.c)
>>> diff --git a/drivers/usb/ulpi/omap-ulpi-viewport.c
>>> b/drivers/usb/ulpi/omap-ulpi-viewport.c
>>> new file mode 100644
>>> index 0000000..1718788
>>> --- /dev/null
>>> +++ b/drivers/usb/ulpi/omap-ulpi-viewport.c
>>> @@ -0,0 +1,110 @@
>>
>>
>> [...]
>>
>>> +/* ULPI viewport control bits */
>>> +#define OMAP_ULPI_PORT_SHIFT   24
>>> +
>>> +#define OMAP_ULPI_WR_OPSEL     (3<<  21)
>>> +#define OMAP_ULPI_ACCESS       (1<<  31)
>>> +
>>> +/*
>>> + * Wait for the ULPI Access to complete
>>> + */
>>> +static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask)
>>> +{
>>> +       int timeout = CONFIG_USB_ULPI_TIMEOUT;
>>> +
>>> +       while (--timeout) {
>>> +               if ((readl(ulpi_vp->viewport_addr)&  mask))
>>>
>>> +                       return 0;
>>> +
>>> +               udelay(1);
>>> +       }
>>> +
>>> +       return ULPI_ERROR;
>>> +}
>>> +
>>> +/*
>>> + * Wake the ULPI PHY up for communication
>>> + *
>>> + * returns 0 on success.
>>> + */
>>> +static int ulpi_wakeup(struct ulpi_viewport *ulpi_vp)
>>> +{
>>> +       int err;
>>> +
>>> +       if (readl(ulpi_vp->viewport_addr)&  OMAP_ULPI_ACCESS)
>>>
>>> +               return 0; /* already awake */
>>> +
>>> +       writel(OMAP_ULPI_ACCESS, ulpi_vp->viewport_addr);
>>> +
>>> +       err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS);
>>> +       if (err)
>>> +               debug("ULPI wakeup timed out\n");
>>> +
>>> +       return err;
>>> +}
>>
>>
>> This function looks really weird...
>> You write the OMAP_ULPI_ACCESS bit, but you don't specify the port...
>> Now, if you will specify the port, what access type (read/write) should it
>> be?
>> So, I don't really know what whould be the consequence of that access...
>>
>> TRM does not talk about waking the ULPI PHY at all or have I missed it...
>> I think this function is unneeded on OMAP - it looks like OMAP wakes
>> the ULPI PHY automatically as soon as you access it for read/write,
>> but the TRM does not talk too much on that...
>>
>> Have you tried without this function?
>> Just calling ulpi_wait() to verify that the previous access has completed,
>> should do the job, no? Can you, please, check that?
>>
> 
> Actually we have to start the ulpi access first, then we can write
> port number and mode to be used and after writing we can check for access
> done bit. (checking read/write op was successful can be polled for
> access done bit)
> 
> without ulpi_wake access done was not happening and timing out.

This is really weird... because we actually don't know which port (PHY)
is being wakened (may be all of them...).
But if you say it does not work without it, then I'm fine with it.

Thanks Govindraj.

-- 
Regards,
Igor.