[PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Petr Kulhavy]

This adds DT support for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver.

Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/usb/da8xx-usb.txt          | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt

diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
new file mode 100644
index 0000000..62dcc51
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
@@ -0,0 +1,47 @@
+TI DA8xx MUSB
+~~~~~~~~~~~~~
+For DA830 and DA850 platforms.
+
+Required properties:
+~~~~~~~~~~~~~~~~~~~~
+ - compatible : Should be set to "ti,da830-musb".
+
+ - reg: Offset and length of the USB controller register set.
+
+ - interrupts: The USB interrupt number.
+
+ - interrupt-names: Should be set to "mc".
+
+ - dr_mode: The USB operation mode. Should be one of "host", "peripheral" or "otg".
+
+ - mentor,power : Specifies the maximum current in milliamperes the controller can
+     supply in host mode.
+
+ - ti,phy20-refclock-frequency : Integer. Defines the USB 2.0 PHY reference clock input
+     frequency in Hz in case the clock is generated by the internal PLL.
+     Supported values are 12MHz, 13MHz, 19.2MHz, 20MHz, 24MHz, 26MHz, 38.4MHz, 40MHz, 48MHz
+
+
+Optional properties:
+~~~~~~~~~~~~~~~~~~~~
+ - ti,phy20-clkmux-pll: Boolean. Defines the USB 2.0 PHY reference clock source.
+     If present the internal PLL is used as a clock source, otherwise the external
+     USB_REFCLKIN pin is used.
+
+Example:
+
+	usb20: usb@1e00000 {
+		compatible = "ti,da830-musb";
+		reg =   <0x00200000 0x10000>;
+		interrupt-parent = <&intc>;
+		interrupts = <58>;
+		interrupt-names = "mc";
+
+		dr_mode = "host";
+		mentor,power = <500>;
+
+		ti,phy20-clkmux-pll;
+		ti,phy20-refclock-frequency = <24000000>;
+
+		status = "okay";
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Sergei Shtylyov]

Hello.

On 2/11/2016 2:01 PM, Petr Kulhavy wrote:

> This adds DT support for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver.

    This patch describes bindings, not adds support.

> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
> ---
>   .../devicetree/bindings/usb/da8xx-usb.txt          | 47 ++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> new file mode 100644
> index 0000000..62dcc51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> @@ -0,0 +1,47 @@
> +TI DA8xx MUSB
> +~~~~~~~~~~~~~
> +For DA830 and DA850 platforms.

    I'd suggest "DA8xx/OMAP-L1x/AM17xx/AM18xx" again.

> +
> +Required properties:
> +~~~~~~~~~~~~~~~~~~~~
> + - compatible : Should be set to "ti,da830-musb".
> +
> + - reg: Offset and length of the USB controller register set.
> +
> + - interrupts: The USB interrupt number.
> +
> + - interrupt-names: Should be set to "mc".
> +
> + - dr_mode: The USB operation mode. Should be one of "host", "peripheral" or "otg".
> +
> + - mentor,power : Specifies the maximum current in milliamperes the controller can
> +     supply in host mode.
> +
> + - ti,phy20-refclock-frequency : Integer. Defines the USB 2.0 PHY reference clock input

     Maybe "ti,usb2-phy-refclock-frequency"?

> +     frequency in Hz in case the clock is generated by the internal PLL.
> +     Supported values are 12MHz, 13MHz, 19.2MHz, 20MHz, 24MHz, 26MHz, 38.4MHz, 40MHz, 48MHz
> +
> +
> +Optional properties:
> +~~~~~~~~~~~~~~~~~~~~
> + - ti,phy20-clkmux-pll: Boolean. Defines the USB 2.0 PHY reference clock source.

    Maybe "ti,usb2-phy-clkmux-pll"?

> +     If present the internal PLL is used as a clock source, otherwise the external
> +     USB_REFCLKIN pin is used.
> +
> +Example:
> +
> +	usb20: usb@1e00000 {
> +		compatible = "ti,da830-musb";
> +		reg =   <0x00200000 0x10000>;
> +		interrupt-parent = <&intc>;
> +		interrupts = <58>;
> +		interrupt-names = "mc";
> +
> +		dr_mode = "host";
> +		mentor,power = <500>;
> +
> +		ti,phy20-clkmux-pll;
> +		ti,phy20-refclock-frequency = <24000000>;

    In principle these 2 should belong to the USB PHY subnode...

[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Petr Kulhavy]

On 11.02.2016 14:53, Sergei Shtylyov wrote:
>
>> +        ti,phy20-clkmux-pll;
>> +        ti,phy20-refclock-frequency = <24000000>;
>
>    In principle these 2 should belong to the USB PHY subnode...

Do you mean to define a PHY subnode even if there is currently no 
standalone PHY driver for da8xx?

Petr

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Sergei Shtylyov]

On 02/11/2016 06:01 PM, Petr Kulhavy wrote:

>>> +        ti,phy20-clkmux-pll;
>>> +        ti,phy20-refclock-frequency = <24000000>;
>>
>>    In principle these 2 should belong to the USB PHY subnode...
>
> Do you mean to define a PHY subnode even if there is currently no standalone
> PHY driver for da8xx?

    Actually, USB PHY shouldn't be a subnode for MUSB (different register 
spaces), so nevermind. Sorry, just a stereotype from Ethernet where the MAC 
often includes MDIO-related registers...

> Petr

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Petr Kulhavy]

DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver.

Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/usb/da8xx-usb.txt          | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt

diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
new file mode 100644
index 0000000..89198ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
@@ -0,0 +1,47 @@
+TI DA8xx MUSB
+~~~~~~~~~~~~~
+For DA8xx/OMAP-L1x/AM17xx/AM18xx platforms.
+
+Required properties:
+~~~~~~~~~~~~~~~~~~~~
+ - compatible : Should be set to "ti,da830-musb".
+
+ - reg: Offset and length of the USB controller register set.
+
+ - interrupts: The USB interrupt number.
+
+ - interrupt-names: Should be set to "mc".
+
+ - dr_mode: The USB operation mode. Should be one of "host", "peripheral" or "otg".
+
+ - mentor,power : Specifies the maximum current in milliamperes the controller can
+     supply in host mode.
+
+ - ti,usb2-phy-refclock-frequency : Integer. Defines the USB 2.0 PHY reference clock input
+     frequency in Hz in case the clock is generated by the internal PLL.
+     Supported values are 12MHz, 13MHz, 19.2MHz, 20MHz, 24MHz, 26MHz, 38.4MHz, 40MHz, 48MHz
+
+
+Optional properties:
+~~~~~~~~~~~~~~~~~~~~
+ - ti,usb2-phy-clkmux-pll: Boolean. Defines the USB 2.0 PHY reference clock source.
+     If present the internal PLL is used as a clock source, otherwise the external
+     USB_REFCLKIN pin is used.
+
+Example:
+
+	usb20: usb@1e00000 {
+		compatible = "ti,da830-musb";
+		reg =   <0x00200000 0x10000>;
+		interrupt-parent = <&intc>;
+		interrupts = <58>;
+		interrupt-names = "mc";
+
+		dr_mode = "host";
+		mentor,power = <500>;
+
+		ti,usb2-phy-clkmux-pll;
+		ti,usb2-phy-refclock-frequency = <24000000>;
+
+		status = "okay";
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Rob Herring]

On Thu, Feb 11, 2016 at 12:01:06PM +0100, Petr Kulhavy wrote:
> This adds DT support for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver.
> 
> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/usb/da8xx-usb.txt          | 47 ++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> new file mode 100644
> index 0000000..62dcc51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> @@ -0,0 +1,47 @@
> +TI DA8xx MUSB
> +~~~~~~~~~~~~~
> +For DA830 and DA850 platforms.
> +
> +Required properties:
> +~~~~~~~~~~~~~~~~~~~~
> + - compatible : Should be set to "ti,da830-musb".
> +
> + - reg: Offset and length of the USB controller register set.
> +
> + - interrupts: The USB interrupt number.
> +
> + - interrupt-names: Should be set to "mc".
> +
> + - dr_mode: The USB operation mode. Should be one of "host", "peripheral" or "otg".
> +
> + - mentor,power : Specifies the maximum current in milliamperes the controller can
> +     supply in host mode.

Still a no for me. Looks like this just sets hcd->power_budget. This 
property may not be a regulator, but ultimately the value depends on 
some regulator supplying Vbus. Also, given this has nothing to do with 
MUSB h/w, however this is described should be generic.

> +
> + - ti,phy20-refclock-frequency : Integer. Defines the USB 2.0 PHY reference clock input
> +     frequency in Hz in case the clock is generated by the internal PLL.
> +     Supported values are 12MHz, 13MHz, 19.2MHz, 20MHz, 24MHz, 26MHz, 38.4MHz, 40MHz, 48MHz
> +
> +
> +Optional properties:
> +~~~~~~~~~~~~~~~~~~~~
> + - ti,phy20-clkmux-pll: Boolean. Defines the USB 2.0 PHY reference clock source.
> +     If present the internal PLL is used as a clock source, otherwise the external
> +     USB_REFCLKIN pin is used.
> +
> +Example:
> +
> +	usb20: usb@1e00000 {
> +		compatible = "ti,da830-musb";
> +		reg =   <0x00200000 0x10000>;
> +		interrupt-parent = <&intc>;
> +		interrupts = <58>;
> +		interrupt-names = "mc";
> +
> +		dr_mode = "host";
> +		mentor,power = <500>;
> +
> +		ti,phy20-clkmux-pll;
> +		ti,phy20-refclock-frequency = <24000000>;
> +
> +		status = "okay";
> +	};
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Petr Kulhavy]

On 12.02.2016 17:21, Rob Herring wrote:
> On Thu, Feb 11, 2016 at 12:01:06PM +0100, Petr Kulhavy wrote:
>> + - mentor,power : Specifies the maximum current in milliamperes the controller can
>> +     supply in host mode.
> Still a no for me. Looks like this just sets hcd->power_budget. This
> property may not be a regulator, but ultimately the value depends on
> some regulator supplying Vbus. Also, given this has nothing to do with
> MUSB h/w, however this is described should be generic.

I understand your point that the description should be generic.
However the USB 2.0 specification does not define any relation between 
the bMaxPower provided by the device and the actual power control.
As far as I understand the value just serves the purpose to raise a flag 
to the user if the attached device would draw too much power, and not to 
enable it at all.
Further, the value used by the protocol is not necessarily related to 
the real current that can be supported. The maximum current supported by 
the protocol is 510mA.

For instance on my development board the real maximum current is limited 
only by the mains power-supply used.
So it cannot even be modelled in the DT as a regulator because the 
power-supply is not part of the HW and
everybody can take a different one.

So defining a regulator just to store this arbitrary USB 2.0 value is a 
bit overkill for me.

Regards
Petr
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Sergei Shtylyov]

Hello.

On 02/12/2016 07:21 PM, Rob Herring wrote:

>> This adds DT support for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver.
>>
>> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
>> ---
>>   .../devicetree/bindings/usb/da8xx-usb.txt          | 47 ++++++++++++++++++++++
>>   1 file changed, 47 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> new file mode 100644
>> index 0000000..62dcc51
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> @@ -0,0 +1,47 @@
>> +TI DA8xx MUSB
>> +~~~~~~~~~~~~~
>> +For DA830 and DA850 platforms.
>> +
>> +Required properties:
>> +~~~~~~~~~~~~~~~~~~~~
>> + - compatible : Should be set to "ti,da830-musb".
>> +
>> + - reg: Offset and length of the USB controller register set.
>> +
>> + - interrupts: The USB interrupt number.
>> +
>> + - interrupt-names: Should be set to "mc".
>> +
>> + - dr_mode: The USB operation mode. Should be one of "host", "peripheral" or "otg".
>> +
>> + - mentor,power : Specifies the maximum current in milliamperes the controller can
>> +     supply in host mode.
>
> Still a no for me.

    Note that it's been used twice already, for musb_dsps.c and omap2430.c 
glues (in the latter case the prop was called just "power"). The corresponding 
field is a part of the 'struct musb_hdrc_platform_data'.

> Looks like this just sets hcd->power_budget. This
> property may not be a regulator, but ultimately the value depends on
> some regulator supplying Vbus.

    Yes.

> Also, given this has nothing to do with MUSB h/w,

    This regulator is controlled by the DRVVBUS signal from MUSB h/w!

> however this is described should be generic.

    You mean just "power", w/o the vendor prefix?

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Petr Kulhavy]

Hello Rob,

is there any follow-up on this one?

Thanks
Petr

On 12.02.2016 18:24, Petr Kulhavy wrote:
>
> On 12.02.2016 17:21, Rob Herring wrote:
>> On Thu, Feb 11, 2016 at 12:01:06PM +0100, Petr Kulhavy wrote:
>>> + - mentor,power : Specifies the maximum current in milliamperes the 
>>> controller can
>>> +     supply in host mode.
>> Still a no for me. Looks like this just sets hcd->power_budget. This
>> property may not be a regulator, but ultimately the value depends on
>> some regulator supplying Vbus. Also, given this has nothing to do with
>> MUSB h/w, however this is described should be generic.
>
> I understand your point that the description should be generic.
> However the USB 2.0 specification does not define any relation between 
> the bMaxPower provided by the device and the actual power control.
> As far as I understand the value just serves the purpose to raise a 
> flag to the user if the attached device would draw too much power, and 
> not to enable it at all.
> Further, the value used by the protocol is not necessarily related to 
> the real current that can be supported. The maximum current supported 
> by the protocol is 510mA.
>
> For instance on my development board the real maximum current is 
> limited only by the mains power-supply used.
> So it cannot even be modelled in the DT as a regulator because the 
> power-supply is not part of the HW and
> everybody can take a different one.
>
> So defining a regulator just to store this arbitrary USB 2.0 value is 
> a bit overkill for me.
>
> Regards
> Petr

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Rob Herring]

On Fri, Feb 12, 2016 at 11:26 AM, Sergei Shtylyov
<sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> wrote:
> Hello.
>
>
> On 02/12/2016 07:21 PM, Rob Herring wrote:
>
>>> This adds DT support for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver.
>>>
>>> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
>>> ---
>>>   .../devicetree/bindings/usb/da8xx-usb.txt          | 47
>>> ++++++++++++++++++++++
>>>   1 file changed, 47 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>> b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>> new file mode 100644
>>> index 0000000..62dcc51
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>> @@ -0,0 +1,47 @@
>>> +TI DA8xx MUSB
>>> +~~~~~~~~~~~~~
>>> +For DA830 and DA850 platforms.
>>> +
>>> +Required properties:
>>> +~~~~~~~~~~~~~~~~~~~~
>>> + - compatible : Should be set to "ti,da830-musb".
>>> +
>>> + - reg: Offset and length of the USB controller register set.
>>> +
>>> + - interrupts: The USB interrupt number.
>>> +
>>> + - interrupt-names: Should be set to "mc".
>>> +
>>> + - dr_mode: The USB operation mode. Should be one of "host",
>>> "peripheral" or "otg".
>>> +
>>> + - mentor,power : Specifies the maximum current in milliamperes the
>>> controller can
>>> +     supply in host mode.
>>
>>
>> Still a no for me.
>
>
>    Note that it's been used twice already, for musb_dsps.c and omap2430.c
> glues (in the latter case the prop was called just "power"). The
> corresponding field is a part of the 'struct musb_hdrc_platform_data'.

Copied from platform_data is exactly what is wrong with this binding
and you already said those were bad examples.

>> Looks like this just sets hcd->power_budget. This
>> property may not be a regulator, but ultimately the value depends on
>> some regulator supplying Vbus.
>
>
>    Yes.
>
>> Also, given this has nothing to do with MUSB h/w,
>
>
>    This regulator is controlled by the DRVVBUS signal from MUSB h/w!

How does a single signal control amount of current? What I should say
is the max current has nothing to do with the MUSB controller. It is a
property of some regulator.

>> however this is described should be generic.
>
>
>    You mean just "power", w/o the vendor prefix?

No. I mean generic in the sense of common for all USB host bindings,
not generic as in a meaningless, unclear name.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Rob Herring]

On Fri, Feb 12, 2016 at 11:24 AM, Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org> wrote:
>
> On 12.02.2016 17:21, Rob Herring wrote:
>>
>> On Thu, Feb 11, 2016 at 12:01:06PM +0100, Petr Kulhavy wrote:
>>>
>>> + - mentor,power : Specifies the maximum current in milliamperes the
>>> controller can
>>> +     supply in host mode.
>>
>> Still a no for me. Looks like this just sets hcd->power_budget. This
>> property may not be a regulator, but ultimately the value depends on
>> some regulator supplying Vbus. Also, given this has nothing to do with
>> MUSB h/w, however this is described should be generic.
>
>
> I understand your point that the description should be generic.
> However the USB 2.0 specification does not define any relation between the
> bMaxPower provided by the device and the actual power control.
> As far as I understand the value just serves the purpose to raise a flag to
> the user if the attached device would draw too much power, and not to enable
> it at all.

That won't really work given devices lie. My bus powered USB disk
enclosure reports something like 10mA. That's pretty good considering
I have a 5W drive in it.

> Further, the value used by the protocol is not necessarily related to the
> real current that can be supported. The maximum current supported by the
> protocol is 510mA.
>
> For instance on my development board the real maximum current is limited
> only by the mains power-supply used.
> So it cannot even be modelled in the DT as a regulator because the
> power-supply is not part of the HW and
> everybody can take a different one.

Not part of which h/w? Different for everyone is exactly why Vbus
supply should be described in DT.

> So defining a regulator just to store this arbitrary USB 2.0 value is a bit
> overkill for me.

If it is just arbitrary, then put it into the driver. I would do 500mA
and be done with it. I'd guess there is nothing real behind the
current default of 250mA.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Petr Kulhavy]

Hello Rob,

On 16.02.2016 21:05, Rob Herring wrote:
> On Fri, Feb 12, 2016 at 11:24 AM, Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org> wrote:
>> I understand your point that the description should be generic.
>> However the USB 2.0 specification does not define any relation between the
>> bMaxPower provided by the device and the actual power control.
>> As far as I understand the value just serves the purpose to raise a flag to
>> the user if the attached device would draw too much power, and not to enable
>> it at all.
> That won't really work given devices lie. My bus powered USB disk
> enclosure reports something like 10mA. That's pretty good considering
> I have a 5W drive in it.
Yes, that's exactly what's happening. The USB 2.0 is in that matter 
naive and assumes that the devices don't lie :-)
But at the same time gives them the possibility to lie by not binding 
this value to the physical current.
>> For instance on my development board the real maximum current is limited
>> only by the mains power-supply used.
>> So it cannot even be modelled in the DT as a regulator because the
>> power-supply is not part of the HW and
>> everybody can take a different one.
> Not part of which h/w? Different for everyone is exactly why Vbus
> supply should be described in DT.
I mean the regulator is in the AC power supply, which is standalone and 
can be changed. Therefore not described in the DT.
The DT describes only the board.

>> So defining a regulator just to store this arbitrary USB 2.0 value is a bit
>> overkill for me.
> If it is just arbitrary, then put it into the driver. I would do 500mA
> and be done with it. I'd guess there is nothing real behind the
> current default of 250mA.
Still there is some sense behind. The power_budget is used to possibly 
print a warning message and/or
disable a self powered device that announces too much power. So I would 
leave it configurable.

What would be your proposal then? Something like this?

     usb_reg: fixedregulator@0 {
         compatible = "regulator-fixed";
         regulator-name = "USB Vbus";
         regulator-always-on;
         regulator-max-microamp = <500000>;    /* 500mA */
     };

or a real regulator if there is one. And then reference it as:

     usb20: ... {
         ...
         vbus-supply = <&usb_reg>;
     }

As I said for me it's an overkill, knowing that this monster needs to be 
set-up and reside in memory just to write the power_budget variable for 
devices which anyway may lie.
But if you prefer that and Sergei and the guys from USB agree to it I 
don't really mind.--

Regards
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Sergei Shtylyov]

Hello.

On 02/16/2016 10:53 PM, Rob Herring wrote:

>>>> This adds DT support for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver.
>>>>
>>>> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>    .../devicetree/bindings/usb/da8xx-usb.txt          | 47
>>>> ++++++++++++++++++++++
>>>>    1 file changed, 47 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>>> b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>>> new file mode 100644
>>>> index 0000000..62dcc51
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>>> @@ -0,0 +1,47 @@
>>>> +TI DA8xx MUSB
>>>> +~~~~~~~~~~~~~
>>>> +For DA830 and DA850 platforms.
>>>> +
>>>> +Required properties:
>>>> +~~~~~~~~~~~~~~~~~~~~
>>>> + - compatible : Should be set to "ti,da830-musb".
>>>> +
>>>> + - reg: Offset and length of the USB controller register set.
>>>> +
>>>> + - interrupts: The USB interrupt number.
>>>> +
>>>> + - interrupt-names: Should be set to "mc".
>>>> +
>>>> + - dr_mode: The USB operation mode. Should be one of "host",
>>>> "peripheral" or "otg".
>>>> +
>>>> + - mentor,power : Specifies the maximum current in milliamperes the
>>>> controller can
>>>> +     supply in host mode.
>>>
>>> Still a no for me.
>>
>>     Note that it's been used twice already, for musb_dsps.c and omap2430.c
>> glues (in the latter case the prop was called just "power"). The
>> corresponding field is a part of the 'struct musb_hdrc_platform_data'.
>
> Copied from platform_data is exactly what is wrong with this binding

    Why? This is a usual way for the platform device to DT migration, no?..

> and you already said those were bad examples.

    No, I said that about the members of 'struct musb_hdrc_config' (referenced 
from the MUSB platform data) that describes the MUSB hardware config. and 
should have been filled based on the "compatible" prop instead of being 
represented in the device tree props. I didn't complain about the "true" 
platform data IIRC...

>>> Looks like this just sets hcd->power_budget. This
>>> property may not be a regulator, but ultimately the value depends on
>>> some regulator supplying Vbus.
>>
>>     Yes.
>>
>>> Also, given this has nothing to do with MUSB h/w,
>>
>>     This regulator is controlled by the DRVVBUS signal from MUSB h/w!
>
> How does a single signal control amount of current? What I should say

    It controls the VBUS voltage (0/+5 V), not the current. The amount of 
current that can be sourced by the bus seems to be an inherent feature of the 
regualtor as far as I remember the corresponding chip datasheets...

> is the max current has nothing to do with the MUSB controller. It is a
> property of some regulator.

    But it's not the current regulator...

>>> however this is described should be generic.
>>
>>     You mean just "power", w/o the vendor prefix?
>
> No. I mean generic in the sense of common for all USB host bindings,

    OK, it's just that currently it's conveyed to the USB stack by each host 
driver's individual platform data, not only MUSB's but also Chipidea's, etc.

> not generic as in a meaningless, unclear name.

    I'm seeing the generically named "hub-power-budget" prop parsed by the 
FHCI driver though, maybe we should stick to that.

> Rob

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Sergei Shtylyov]

On 02/16/2016 11:05 PM, Rob Herring wrote:

>>>> + - mentor,power : Specifies the maximum current in milliamperes the
>>>> controller can
>>>> +     supply in host mode.
>>>
>>> Still a no for me. Looks like this just sets hcd->power_budget. This
>>> property may not be a regulator, but ultimately the value depends on
>>> some regulator supplying Vbus. Also, given this has nothing to do with
>>> MUSB h/w, however this is described should be generic.
>>
>>
>> I understand your point that the description should be generic.
>> However the USB 2.0 specification does not define any relation between the
>> bMaxPower provided by the device and the actual power control.
>> As far as I understand the value just serves the purpose to raise a flag to
>> the user if the attached device would draw too much power, and not to enable
>> it at all.
>
> That won't really work given devices lie. My bus powered USB disk
> enclosure reports something like 10mA. That's pretty good considering
> I have a 5W drive in it.

>> Further, the value used by the protocol is not necessarily related to the
>> real current that can be supported. The maximum current supported by the
>> protocol is 510mA.
>>
>> For instance on my development board the real maximum current is limited
>> only by the mains power-supply used.
>> So it cannot even be modelled in the DT as a regulator because the
>> power-supply is not part of the HW and
>> everybody can take a different one.
>
> Not part of which h/w? Different for everyone is exactly why Vbus
> supply should be described in DT.

    I'm afraid you're misunderstanding the nature of VBUS regulator still.
It's a voltage regulator with some maximum value of the current that can be 
sourced from it when it's powered (+5 V). The power chip can typically detect 
and report the over-current condition inj which case VBUS will be turned off).

>> So defining a regulator just to store this arbitrary USB 2.0 value is a bit
>> overkill for me.
>
> If it is just arbitrary, then put it into the driver. I would do 500mA

    You clearly misunderstand things. The regulator used depends on the board 
design, the driver (or glue in this case) doesn't, it's just SoC specific. 
That's why this power budget thing ended up in the platform data in the first 
place...

> and be done with it. I'd guess there is nothing real behind the
> current default of 250mA.

    500 mA actually, it's multiplied by 2.

> Rob

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Rob Herring]

On Wed, Feb 17, 2016 at 12:14 PM, Sergei Shtylyov
<sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> wrote:
> On 02/16/2016 11:05 PM, Rob Herring wrote:
>
>>>>> + - mentor,power : Specifies the maximum current in milliamperes the
>>>>> controller can
>>>>> +     supply in host mode.
>>>>
>>>>
>>>> Still a no for me. Looks like this just sets hcd->power_budget. This
>>>> property may not be a regulator, but ultimately the value depends on
>>>> some regulator supplying Vbus. Also, given this has nothing to do with
>>>> MUSB h/w, however this is described should be generic.
>>>
>>>
>>>
>>> I understand your point that the description should be generic.
>>> However the USB 2.0 specification does not define any relation between
>>> the
>>> bMaxPower provided by the device and the actual power control.
>>> As far as I understand the value just serves the purpose to raise a flag
>>> to
>>> the user if the attached device would draw too much power, and not to
>>> enable
>>> it at all.
>>
>>
>> That won't really work given devices lie. My bus powered USB disk
>> enclosure reports something like 10mA. That's pretty good considering
>> I have a 5W drive in it.
>
>
>>> Further, the value used by the protocol is not necessarily related to the
>>> real current that can be supported. The maximum current supported by the
>>> protocol is 510mA.
>>>
>>> For instance on my development board the real maximum current is limited
>>> only by the mains power-supply used.
>>> So it cannot even be modelled in the DT as a regulator because the
>>> power-supply is not part of the HW and
>>> everybody can take a different one.
>>
>>
>> Not part of which h/w? Different for everyone is exactly why Vbus
>> supply should be described in DT.
>
>
>    I'm afraid you're misunderstanding the nature of VBUS regulator still.
> It's a voltage regulator with some maximum value of the current that can be
> sourced from it when it's powered (+5 V). The power chip can typically
> detect and report the over-current condition inj which case VBUS will be
> turned off).

Yes, I understand that exactly. I've seen and reviewed a USB circuit
or two.  You're describing a regulator, I'm saying describe the
regulator in DT and Petr is saying that's too complex.

>>> So defining a regulator just to store this arbitrary USB 2.0 value is a
>>> bit
>>> overkill for me.
>>
>>
>> If it is just arbitrary, then put it into the driver. I would do 500mA
>
>
>    You clearly misunderstand things. The regulator used depends on the board
> design, the driver (or glue in this case) doesn't, it's just SoC specific.
> That's why this power budget thing ended up in the platform data in the
> first place...

The driver needs to be able to query the supply and get the current
limit. We have the technology.

>
>> and be done with it. I'd guess there is nothing real behind the
>> current default of 250mA.
>
>
>    500 mA actually, it's multiplied by 2.

Ok. For 2 ports?

In any case, if there is effectively no limit (Vbus comes from the
wall), then yes a regulator is probably an overkill. So make the case
with no Vbus regulator (or regulator with no limit set) in DT be the
max current. However, if the controller has a signal to turn on/off
Vbus, then that should be modeled as a regulator.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Sergei Shtylyov]

Hello.

    Sorry about late reply, I kept forgetting about this mail.

On 02/17/2016 09:51 PM, Rob Herring wrote:

>>>>>> + - mentor,power : Specifies the maximum current in milliamperes the
>>>>>> controller can
>>>>>> +     supply in host mode.
>>>>>
>>>>>
>>>>> Still a no for me. Looks like this just sets hcd->power_budget. This
>>>>> property may not be a regulator, but ultimately the value depends on
>>>>> some regulator supplying Vbus. Also, given this has nothing to do with
>>>>> MUSB h/w, however this is described should be generic.
>>>>
>>>>
>>>>
>>>> I understand your point that the description should be generic.
>>>> However the USB 2.0 specification does not define any relation between
>>>> the
>>>> bMaxPower provided by the device and the actual power control.
>>>> As far as I understand the value just serves the purpose to raise a flag
>>>> to
>>>> the user if the attached device would draw too much power, and not to
>>>> enable
>>>> it at all.
>>>
>>>
>>> That won't really work given devices lie. My bus powered USB disk
>>> enclosure reports something like 10mA. That's pretty good considering
>>> I have a 5W drive in it.
>>
>>
>>>> Further, the value used by the protocol is not necessarily related to the
>>>> real current that can be supported. The maximum current supported by the
>>>> protocol is 510mA.
>>>>
>>>> For instance on my development board the real maximum current is limited
>>>> only by the mains power-supply used.
>>>> So it cannot even be modelled in the DT as a regulator because the
>>>> power-supply is not part of the HW and
>>>> everybody can take a different one.
>>>
>>> Not part of which h/w? Different for everyone is exactly why Vbus
>>> supply should be described in DT.
>>
>>     I'm afraid you're misunderstanding the nature of VBUS regulator still.
>> It's a voltage regulator with some maximum value of the current that can be
>> sourced from it when it's powered (+5 V). The power chip can typically
>> detect and report the over-current condition inj which case VBUS will be
>> turned off).
>
> Yes, I understand that exactly. I've seen and reviewed a USB circuit
> or two.  You're describing a regulator, I'm saying describe the
> regulator in DT and Petr is saying that's too complex.

    The why ask about the Vbus current regulation if you know it's a voltage 
regulator? :-)

>>>> So defining a regulator just to store this arbitrary USB 2.0 value is a
>>>> bit
>>>> overkill for me.
>>>
>>> If it is just arbitrary, then put it into the driver. I would do 500mA
>>
>>     You clearly misunderstand things. The regulator used depends on the board
>> design, the driver (or glue in this case) doesn't, it's just SoC specific.
>> That's why this power budget thing ended up in the platform data in the
>> first place...
>
> The driver needs to be able to query the supply and get the current
> limit. We have the technology.

    Looking at the docs, I guess you mean the 
"regulator-input-current-limit-microamp" prop...

>>> and be done with it. I'd guess there is nothing real behind the
>>> current default of 250mA.
>>
>>
>>     500 mA actually, it's multiplied by 2.
>
> Ok. For 2 ports?

   No, MUSB has only a single port, the unit was 2 mA.

> In any case, if there is effectively no limit (Vbus comes from the
> wall), then yes a regulator is probably an overkill. So make the case
> with no Vbus regulator (or regulator with no limit set) in DT be the
> max current. However, if the controller has a signal to turn on/off
> Vbus, then that should be modeled as a regulator.

    Well, actually DRVVBUS controls the VBUS charge-pump in the TI MUSB 
implementations, it's not something described by the Mentor's docs...

> Rob

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] dt/bindings: Add binding for the DA8xx MUSB driver

[Bin Liu]

Hi Rob,

On Wed, Feb 17, 2016 at 12:51:34PM -0600, Rob Herring wrote:
> On Wed, Feb 17, 2016 at 12:14 PM, Sergei Shtylyov
> <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> wrote:
> > On 02/16/2016 11:05 PM, Rob Herring wrote:
> >
> >>>>> + - mentor,power : Specifies the maximum current in milliamperes the
> >>>>> controller can
> >>>>> +     supply in host mode.
> >>>>
> >>>>
> >>>> Still a no for me. Looks like this just sets hcd->power_budget. This
> >>>> property may not be a regulator, but ultimately the value depends on
> >>>> some regulator supplying Vbus. Also, given this has nothing to do with
> >>>> MUSB h/w, however this is described should be generic.
> >>>
> >>>
> >>>
> >>> I understand your point that the description should be generic.
> >>> However the USB 2.0 specification does not define any relation between
> >>> the
> >>> bMaxPower provided by the device and the actual power control.
> >>> As far as I understand the value just serves the purpose to raise a flag
> >>> to
> >>> the user if the attached device would draw too much power, and not to
> >>> enable
> >>> it at all.
> >>
> >>
> >> That won't really work given devices lie. My bus powered USB disk
> >> enclosure reports something like 10mA. That's pretty good considering
> >> I have a 5W drive in it.
> >
> >
> >>> Further, the value used by the protocol is not necessarily related to the
> >>> real current that can be supported. The maximum current supported by the
> >>> protocol is 510mA.
> >>>
> >>> For instance on my development board the real maximum current is limited
> >>> only by the mains power-supply used.
> >>> So it cannot even be modelled in the DT as a regulator because the
> >>> power-supply is not part of the HW and
> >>> everybody can take a different one.
> >>
> >>
> >> Not part of which h/w? Different for everyone is exactly why Vbus
> >> supply should be described in DT.
> >
> >
> >    I'm afraid you're misunderstanding the nature of VBUS regulator still.
> > It's a voltage regulator with some maximum value of the current that can be
> > sourced from it when it's powered (+5 V). The power chip can typically
> > detect and report the over-current condition inj which case VBUS will be
> > turned off).
> 
> Yes, I understand that exactly. I've seen and reviewed a USB circuit
> or two.  You're describing a regulator, I'm saying describe the
> regulator in DT and Petr is saying that's too complex.
> 
> >>> So defining a regulator just to store this arbitrary USB 2.0 value is a
> >>> bit
> >>> overkill for me.
> >>
> >>
> >> If it is just arbitrary, then put it into the driver. I would do 500mA
> >
> >
> >    You clearly misunderstand things. The regulator used depends on the board
> > design, the driver (or glue in this case) doesn't, it's just SoC specific.
> > That's why this power budget thing ended up in the platform data in the
> > first place...
> 
> The driver needs to be able to query the supply and get the current
> limit. We have the technology.
> 
> >
> >> and be done with it. I'd guess there is nothing real behind the
> >> current default of 250mA.
> >
> >
> >    500 mA actually, it's multiplied by 2.
> 
> Ok. For 2 ports?
> 
> In any case, if there is effectively no limit (Vbus comes from the
> wall), then yes a regulator is probably an overkill. So make the case
> with no Vbus regulator (or regulator with no limit set) in DT be the
> max current. However, if the controller has a signal to turn on/off
> Vbus, then that should be modeled as a regulator.

The TI implemention of the controller does have a signal to turn on/off
Vbus, but this signal is controlled by hardware internal state machine
and transparent to software.

>From software perspective, the driver only needs to get the board
defined current limit and passes it to usb core. I would think a
regulator is an overkill in here.

What is your final comments on this so Petr can revise the patch
accordingly?

Thanks,
-Bin.

> 
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html