[U-Boot] [PATCH v7] USB: Add generic ULPI layer and a viewport

[Igor Grinberg]

On 12/07/11 20:36, Marek Vasut wrote:
>> Hi Marek,
>>
>> On 12/07/11 19:27, Marek Vasut wrote:
>>>> On 12/07/11 03:42, Simon Glass wrote:
>>>>> Hi Igor,
>>>>>
>>>>> Looks good - a few comments from me.
>>>>>
>>>>> On Mon, Dec 5, 2011 at 1:07 AM, Igor Grinberg <grinberg at compulab.co.il>
>>>
>>> wrote:
>>>>>> From: Jana Rapava <fermata7 at gmail.com>
>>>>>>
>>>>>> Add partial ULPI specification implementation that should be enough to
>>>>>> interface the ULPI PHYs in the boot loader context.
>>>>>> Add a viewport implementation for Chipidea/ARC based controllers.
>>>>>>
>>>>>> Signed-off-by: Jana Rapava <fermata7 at gmail.com>
>>>>>> Signed-off-by: Igor Grinberg <grinberg at compulab.co.il>
>>>>>> Cc: Remy Bohmer <linux at bohmer.net>
>>>>>> Cc: Stefano Babic <sbabic at denx.de>
>>>>>> Cc: Wolfgang Grandegger <wg at denx.de>
>>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>>> ---
>>
>> [...]
>>
>>>>> Just out of interest, is it possible to test this? How would I plumb it
>>>>> in?
>>>>
>>>> Well, from my experience with ULPI hardware,
>>>> I think the controller specific glue looks like the right place
>>>> for putting the ULPI layer calls in.
>>>>
>>>> In general, the controller code knows which PHYs it supports
>>>> and board code knows which PHY is assembled on the board,
>>>> so it is not that straight simple to find the right place.
>>>>
>>>> I think, Marek has patches that supposed to use this framework on
>>>> efikamx board.
>>>
>>> I tried using the interface, but the design seems completely wrong :-(
>>> Jana was supposed to design it mainly for the efikamx board, but this
>>> interface is unusable there.
>>
>> May I ask you why?
>> Isn't it because of that nasty VBUS bug efikamx has?
>> You can't say the design is wrong if it is more generic then you want...
> 
> I think it's overengineered. Basically, to achieve what I needed, I either 
> didn't find the right function or I had to use multiple functions. Therefore I 
> had to fall back to plain simple ulpi_read/write().

So now it is plain simple multiple ulpi_read/write() calls?
This is no more as simple as ulpi_<layer function>() calls...
Instead of forging masks and writing arbitrary values, use
the functional API.

I don't think is is over engineered, because it is simple
and in the same time helps prevent mistakes and most of the cases,
you don't need to be familiar with all the ULPI bits.

>>
>>> I had to fall back to basic ulpi_read()/ulpi_write() calls :-(
>>
>> That's too bad.
>> Because ulpi_{read|write}() is only a viewport implementation and
>> it is not following the ULPI spec.
> 
> Well ... we'll need to rethink this.
>>
>>> I'm afraid we won't make it for .12 release window with this patches ...
>>> very bad :-( I'll try talking to WD if he can push the release window to
>>> allow this
>>
>> Good.
>>
>>> (or redesigned version) in, but I don't know if that's a good idea.
>>
>> I don't think it should be redesigned.
>> Currently, it is generic and abstracts the ULPI specification nicely.
> 
> Nicely maybe, but try using it on top of some hardware that has ULPI chip.

What's the problem? It is better then just using ulpi_read/write() calls...
May be you are just lazy to read the documentation...

> 
>> It can be used on any architecture.
>> I have already stated in the cover letter,
>> what IMO is missing to improve usability, but that will not be a problem.
>>
>> Do you have the efikamx patches somewhere I can look at?
> 
> Submitted to ML just a while ago.
> 
> M
> 

-- 
Regards,
Igor.