Patchwork [U-Boot,4/4] README: add documentation for CONFIG_USB_ULPI*

login
register
mail settings
Submitter Igor Grinberg
Date Dec. 12, 2011, 10:08 a.m.
Message ID <1323684515-1918-5-git-send-email-grinberg@compulab.co.il>
Download mbox | patch
Permalink /patch/130684/
State Awaiting Upstream
Headers show

Comments

Igor Grinberg - Dec. 12, 2011, 10:08 a.m.
Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
configuration options.

Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 README |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
Simon Glass - Dec. 14, 2011, 12:28 a.m.
Hi Igor,

On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
> configuration options.
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
>  README |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/README b/README
> index ff72e47..6fe1e0f 100644
> --- a/README
> +++ b/README
> @@ -1185,6 +1185,14 @@ The following options need to be configured:
>                        for your device
>                        - CONFIG_USBD_PRODUCTID 0xFFFF
>
> +- ULPI Layer Support:
> +               The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
> +               the generic ULPI layer. The generic layer accesses the ULPI PHY
> +               via the platform viewport, so you need both the genric layer and
> +               the viewport enabled. Currently only Chipidea/ARC based
> +               viewport is supported.

Where does it say that only this one is supported in the code? What is
specific to that device?

Regards,
Simon

> +               To enable the ULPI layer support, define CONFIG_USB_ULPI and
> +               CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>
>  - MMC Support:
>                The MMC controller on the Intel PXA is supported. To
> --
> 1.7.3.4
>
Igor Grinberg - Dec. 14, 2011, 5:51 a.m.
Hi Simon,

On 12/14/11 02:28, Simon Glass wrote:
> Hi Igor,
> 
> On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>> Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
>> configuration options.
>>
>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>> ---
>>  README |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/README b/README
>> index ff72e47..6fe1e0f 100644
>> --- a/README
>> +++ b/README
>> @@ -1185,6 +1185,14 @@ The following options need to be configured:
>>                        for your device
>>                        - CONFIG_USBD_PRODUCTID 0xFFFF
>>
>> +- ULPI Layer Support:
>> +               The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
>> +               the generic ULPI layer. The generic layer accesses the ULPI PHY
>> +               via the platform viewport, so you need both the genric layer and
>> +               the viewport enabled. Currently only Chipidea/ARC based
>> +               viewport is supported.
> 
> Where does it say that only this one is supported in the code?

You mean comments or the code?

> What is specific to that device?

The viewport bits? It is not a part of the ULPI spec.
Other vendors do not have to comply with those.
For example PXA310 has those bits placed and named in some other way...

> 
> Regards,
> Simon
> 
>> +               To enable the ULPI layer support, define CONFIG_USB_ULPI and
>> +               CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>>
>>  - MMC Support:
>>                The MMC controller on the Intel PXA is supported. To
>> --
>> 1.7.3.4
>>
>
Simon Glass - Dec. 14, 2011, 7:26 p.m.
Hi Igor,

On Tue, Dec 13, 2011 at 9:51 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Simon,
>
> On 12/14/11 02:28, Simon Glass wrote:
>> Hi Igor,
>>
>> On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>> Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
>>> configuration options.
>>>
>>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>>> ---
>>>  README |    8 ++++++++
>>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/README b/README
>>> index ff72e47..6fe1e0f 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -1185,6 +1185,14 @@ The following options need to be configured:
>>>                        for your device
>>>                        - CONFIG_USBD_PRODUCTID 0xFFFF
>>>
>>> +- ULPI Layer Support:
>>> +               The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
>>> +               the generic ULPI layer. The generic layer accesses the ULPI PHY
>>> +               via the platform viewport, so you need both the genric layer and
>>> +               the viewport enabled. Currently only Chipidea/ARC based
>>> +               viewport is supported.
>>
>> Where does it say that only this one is supported in the code?
>
> You mean comments or the code?

Well the filename seems generic and not specific to that chip. Are
viewports something that other chips can support?

COBJS-$(CONFIG_USB_ULPI)		+= ulpi.o
COBJS-$(CONFIG_USB_ULPI_VIEWPORT)	+= ulpi-viewport.o

It would be good if you could mention the two new CONFIG options in the README.

>
>> What is specific to that device?
>
> The viewport bits? It is not a part of the ULPI spec.
> Other vendors do not have to comply with those.
> For example PXA310 has those bits placed and named in some other way...

OK I didn't realise that.

>>> +               To enable the ULPI layer support, define CONFIG_USB_ULPI and
>>> +               CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>>>
>>>  - MMC Support:
>>>                The MMC controller on the Intel PXA is supported. To
>>> --
>>> 1.7.3.4
>>>
>>
>
> --
> Regards,
> Igor.

Regards,
Simon
Igor Grinberg - Dec. 15, 2011, 9:06 a.m.
On 12/14/11 21:26, Simon Glass wrote:
> Hi Igor,
> 
> On Tue, Dec 13, 2011 at 9:51 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>> Hi Simon,
>>
>> On 12/14/11 02:28, Simon Glass wrote:
>>> Hi Igor,
>>>
>>> On Mon, Dec 12, 2011 at 2:08 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>>> Add documentation for CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
>>>> configuration options.
>>>>
>>>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>>>> ---
>>>>  README |    8 ++++++++
>>>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/README b/README
>>>> index ff72e47..6fe1e0f 100644
>>>> --- a/README
>>>> +++ b/README
>>>> @@ -1185,6 +1185,14 @@ The following options need to be configured:
>>>>                        for your device
>>>>                        - CONFIG_USBD_PRODUCTID 0xFFFF
>>>>
>>>> +- ULPI Layer Support:
>>>> +               The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
>>>> +               the generic ULPI layer. The generic layer accesses the ULPI PHY
>>>> +               via the platform viewport, so you need both the genric layer and
>>>> +               the viewport enabled. Currently only Chipidea/ARC based
>>>> +               viewport is supported.
>>>
>>> Where does it say that only this one is supported in the code?
>>
>> You mean comments or the code?
> 
> Well the filename seems generic and not specific to that chip. Are
> viewports something that other chips can support?

Let me clarify:
1) It is not the chip it is the controller (IP block) inside the SoC.
2) viewport is just the register name inside the SoC that provides
   and interface of the controller to access the ULPI PHY.

I think every SoC that uses that controller has the viewport setup
this way, but I might be wrong (and that's why the viewport is
separated from the generic ULPI spec implementation).

Regarding the name... yeah it could be renamed, but it follows Linux
naming currently and it is the first one submitted,
so IMO it can be named that generically.

> 
> COBJS-$(CONFIG_USB_ULPI)		+= ulpi.o
> COBJS-$(CONFIG_USB_ULPI_VIEWPORT)	+= ulpi-viewport.o
> 
> It would be good if you could mention the two new CONFIG options in the README.

I did, see below...

> 
>>
>>> What is specific to that device?
>>
>> The viewport bits? It is not a part of the ULPI spec.
>> Other vendors do not have to comply with those.
>> For example PXA310 has those bits placed and named in some other way...
> 
> OK I didn't realise that.

I think same stand for OMAP, but I'm not sure.
OMAP still does arbitrary register writes for accessing ULPI.

> 
>>>> +               To enable the ULPI layer support, define CONFIG_USB_ULPI and
>>>> +               CONFIG_USB_ULPI_VIEWPORT in your board configuration file.

Here the configs are documented.
I admit, it is not that brilliant documentation...
Remy Bohmer - Dec. 16, 2011, 8:10 p.m.
Hi Igor,

2011/12/15 Igor Grinberg <grinberg@compulab.co.il>:
>>>> Where does it say that only this one is supported in the code?
>>>
>>> You mean comments or the code?
>>
>> Well the filename seems generic and not specific to that chip. Are
>> viewports something that other chips can support?
>
> Let me clarify:
> 1) It is not the chip it is the controller (IP block) inside the SoC.
> 2) viewport is just the register name inside the SoC that provides
>   and interface of the controller to access the ULPI PHY.
>
> I think every SoC that uses that controller has the viewport setup
> this way, but I might be wrong (and that's why the viewport is
> separated from the generic ULPI spec implementation).
>
> Regarding the name... yeah it could be renamed, but it follows Linux
> naming currently and it is the first one submitted,
> so IMO it can be named that generically.
>
>>
>> COBJS-$(CONFIG_USB_ULPI)              += ulpi.o
>> COBJS-$(CONFIG_USB_ULPI_VIEWPORT)     += ulpi-viewport.o
>>
>> It would be good if you could mention the two new CONFIG options in the README.
>
> I did, see below...
>
>>
>>>
>>>> What is specific to that device?
>>>
>>> The viewport bits? It is not a part of the ULPI spec.
>>> Other vendors do not have to comply with those.
>>> For example PXA310 has those bits placed and named in some other way...
>>
>> OK I didn't realise that.
>
> I think same stand for OMAP, but I'm not sure.
> OMAP still does arbitrary register writes for accessing ULPI.
>
>>
>>>>> +               To enable the ULPI layer support, define CONFIG_USB_ULPI and
>>>>> +               CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>
> Here the configs are documented.
> I admit, it is not that brilliant documentation...

Are you planning to post an update of this patch? The rest of the
series I already pulled into the USB tree.

Kind regards,

Remy
Simon Glass - Dec. 16, 2011, 8:29 p.m.
Hi Remy,

On Fri, Dec 16, 2011 at 12:10 PM, Remy Bohmer <linux@bohmer.net> wrote:
> Hi Igor,
>
> 2011/12/15 Igor Grinberg <grinberg@compulab.co.il>:
>>>>> Where does it say that only this one is supported in the code?
>>>>
>>>> You mean comments or the code?
>>>
>>> Well the filename seems generic and not specific to that chip. Are
>>> viewports something that other chips can support?
>>
>> Let me clarify:
>> 1) It is not the chip it is the controller (IP block) inside the SoC.
>> 2) viewport is just the register name inside the SoC that provides
>>   and interface of the controller to access the ULPI PHY.
>>
>> I think every SoC that uses that controller has the viewport setup
>> this way, but I might be wrong (and that's why the viewport is
>> separated from the generic ULPI spec implementation).
>>
>> Regarding the name... yeah it could be renamed, but it follows Linux
>> naming currently and it is the first one submitted,
>> so IMO it can be named that generically.
>>
>>>
>>> COBJS-$(CONFIG_USB_ULPI)              += ulpi.o
>>> COBJS-$(CONFIG_USB_ULPI_VIEWPORT)     += ulpi-viewport.o
>>>
>>> It would be good if you could mention the two new CONFIG options in the README.
>>
>> I did, see below...
>>
>>>
>>>>
>>>>> What is specific to that device?
>>>>
>>>> The viewport bits? It is not a part of the ULPI spec.
>>>> Other vendors do not have to comply with those.
>>>> For example PXA310 has those bits placed and named in some other way...
>>>
>>> OK I didn't realise that.
>>
>> I think same stand for OMAP, but I'm not sure.
>> OMAP still does arbitrary register writes for accessing ULPI.
>>
>>>
>>>>>> +               To enable the ULPI layer support, define CONFIG_USB_ULPI and
>>>>>> +               CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
>>
>> Here the configs are documented.
>> I admit, it is not that brilliant documentation...
>
> Are you planning to post an update of this patch? The rest of the
> series I already pulled into the USB tree.

From my point of view it is fine as is.

Regards,
Simon

>
> Kind regards,
>
> Remy
Remy Bohmer - Dec. 16, 2011, 8:35 p.m.
Hi,

2011/12/16 Simon Glass <sjg@chromium.org>:
> Hi Remy,
>
> On Fri, Dec 16, 2011 at 12:10 PM, Remy Bohmer <linux@bohmer.net> wrote:
>> Hi Igor,
>>
>>> Here the configs are documented.
>>> I admit, it is not that brilliant documentation...
>>
>> Are you planning to post an update of this patch? The rest of the
>> series I already pulled into the USB tree.
>
> From my point of view it is fine as is.

In that case i will pull it in. Thanks.

Kind regards,

Remy

Patch

diff --git a/README b/README
index ff72e47..6fe1e0f 100644
--- a/README
+++ b/README
@@ -1185,6 +1185,14 @@  The following options need to be configured:
 			for your device
 			- CONFIG_USBD_PRODUCTID 0xFFFF
 
+- ULPI Layer Support:
+		The ULPI (UTMI Low Pin (count) Interface) PHYs are supported via
+		the generic ULPI layer. The generic layer accesses the ULPI PHY
+		via the platform viewport, so you need both the genric layer and
+		the viewport enabled. Currently only Chipidea/ARC based
+		viewport is supported.
+		To enable the ULPI layer support, define CONFIG_USB_ULPI and
+		CONFIG_USB_ULPI_VIEWPORT in your board configuration file.
 
 - MMC Support:
 		The MMC controller on the Intel PXA is supported. To