[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Peter Chen]

> 
> On Tue, Nov 27, 2018 at 7:31 AM PETER CHEN <peter.chen@nxp.com> wrote:
> >
> > For USB HSIC, the data and strobe pin needs to be pulled down at
> > default, we consider it as "idle" state. When the USB host is ready to
> > be used, the strobe pin needs to be pulled up, we consider it as
> > "active" state.
> >
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > index 529e51879fb2..1e5e7ddfb1a5 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > @@ -80,7 +80,10 @@ Optional properties:
> >    controller. It's expected that a mux state of 0 indicates device mode and a
> >    mux state of 1 indicates host mode.
> >  - mux-control-names: Shall be "usb_switch" if mux-controls is specified.
> > -- pinctrl-names: Names for optional pin modes in "default", "host", "device"
> > +- pinctrl-names: Names for optional pin modes in "default", "host", "device".
> > +  In case of HSIC-mode, "idle" and "active" pin modes are mandatory.
> > +In this
> > +  case, the "idle" state needs to pull down the data and strobe pin
> > +  and the "active" state needs to pull up the strobe pin.
> >  - pinctrl-n: alternate pin modes
> >
> >  i.mx specific properties
> > @@ -110,4 +113,7 @@ Example:
> >                 phy-clkgate-delay-us = <400>;
> >                 mux-controls = <&usb_switch>;
> >                 mux-control-names = "usb_switch";
> > +               pinctrl-names = "idle", "active";
> > +               pinctrl-0 = <&pinctrl_usbh2_1>;
> > +               pinctrl-1 = <&pinctrl_usbh2_2>;
> 
> I would put the "idle" and "active" entries inside a explicit section for HSIC.
> 
> Otherwise, it may confuse people using non-HSIC bindings.

It is just an example; the user should check the meaning for optional properties before
using it, like the property "phy-clkgate-delay-us", only imx7d/7s needs it.

Peter

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Rob Herring]

On Wed, Dec 05, 2018 at 07:57:37AM +0000, PETER CHEN wrote:
>  
> > On 04.12.18 21:01, Fabio Estevam wrote:
> > > Hi Frieder,
> > >
> > > On Tue, Dec 4, 2018 at 12:31 PM Schrempf Frieder
> > > <frieder.schrempf@kontron.de> wrote:
> > >
> > >> There are many other optional properties for this driver and a lot of
> > >> them are not in the given example. Maybe we should just keep the
> > >> pinctrls for HSIC-mode out of the example, too?
> > >
> > > I am just trying to make life easier for those who want to use HSIC
> > > support with chipidea.
> > >
> > > Can we just add a real dts snippet example of your board into the
> > > binding document?
> > 
> > Sure, here is what I have in my dts:
> > 
> > &usbh2 {
> > 	pinctrl-names = "idle", "active";
> > 	pinctrl-0 = <&pinctrl_usbh2_idle>;
> > 	pinctrl-1 = <&pinctrl_usbh2_active>;
> > 	status = "okay";
> > 	#address-cells = <1>;
> > 	#size-cells = <0>;
> > 
> > 	usbnet: smsc@1 {
> > 		compatible = "usb424,9730";
> > 		reg = <1>;
> > 	};
> > };
> > 
> > @Peter: Can you add this as a second example to the binding documentation?
> > 
> 
> So, there are two examples at binding-doc, one for normal, one for HSIC? Fabio, do you
> mean that? If DT maintainer agrees it too, I will add it.

Okay.

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Fabio Estevam]

On Wed, Dec 5, 2018 at 5:57 AM PETER CHEN <peter.chen@nxp.com> wrote:

> So, there are two examples at binding-doc, one for normal, one for HSIC? Fabio, do you
> mean that? If DT maintainer agrees it too, I will add it.

Yes, I think it will makes things clearer. Thanks

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Frieder Schrempf]

On 05.12.18 08:45, Schrempf Frieder wrote:
> Hi Fabio,
> 
> On 04.12.18 21:01, Fabio Estevam wrote:
>> Hi Frieder,
>>
>> On Tue, Dec 4, 2018 at 12:31 PM Schrempf Frieder
>> <frieder.schrempf@kontron.de> wrote:
>>
>>> There are many other optional properties for this driver and a lot of
>>> them are not in the given example. Maybe we should just keep the
>>> pinctrls for HSIC-mode out of the example, too?
>>
>> I am just trying to make life easier for those who want to use HSIC
>> support with chipidea.
>>
>> Can we just add a real dts snippet example of your board into the
>> binding document?
> 
> Sure, here is what I have in my dts:
> 
> &usbh2 {
> 	pinctrl-names = "idle", "active";
> 	pinctrl-0 = <&pinctrl_usbh2_idle>;
> 	pinctrl-1 = <&pinctrl_usbh2_active>;
> 	status = "okay";
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	usbnet: smsc@1 {
> 		compatible = "usb424,9730";
> 		reg = <1>;
> 	};
> };

Or merged with the settings from imx6qdl.dtsi this will look like below. 
Maybe this is better as it is the complete node without depending on 
imx6qdl.dtsi:

usbh2: usb@2184400 {
	compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
	reg = <0x02184400 0x200>;
	interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>;
	clocks = <&clks IMX6QDL_CLK_USBOH3>;
	fsl,usbphy = <&usbphynop1>;
	fsl,usbmisc = <&usbmisc 2>;
	phy_type = "hsic";
	dr_mode = "host";
	ahb-burst-config = <0x0>;
	tx-burst-size-dword = <0x10>;
	rx-burst-size-dword = <0x10>;
	pinctrl-names = "idle", "active";
  	pinctrl-0 = <&pinctrl_usbh2_idle>;
  	pinctrl-1 = <&pinctrl_usbh2_active>;
	#address-cells = <1>;
  	#size-cells = <0>;

  	usbnet: smsc@1 {
  		compatible = "usb424,9730";
  		reg = <1>;
  	};
};

> 
> @Peter: Can you add this as a second example to the binding documentation?
> 
> Thanks,
> Frieder
>

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Peter Chen]

> On 04.12.18 21:01, Fabio Estevam wrote:
> > Hi Frieder,
> >
> > On Tue, Dec 4, 2018 at 12:31 PM Schrempf Frieder
> > <frieder.schrempf@kontron.de> wrote:
> >
> >> There are many other optional properties for this driver and a lot of
> >> them are not in the given example. Maybe we should just keep the
> >> pinctrls for HSIC-mode out of the example, too?
> >
> > I am just trying to make life easier for those who want to use HSIC
> > support with chipidea.
> >
> > Can we just add a real dts snippet example of your board into the
> > binding document?
> 
> Sure, here is what I have in my dts:
> 
> &usbh2 {
> 	pinctrl-names = "idle", "active";
> 	pinctrl-0 = <&pinctrl_usbh2_idle>;
> 	pinctrl-1 = <&pinctrl_usbh2_active>;
> 	status = "okay";
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	usbnet: smsc@1 {
> 		compatible = "usb424,9730";
> 		reg = <1>;
> 	};
> };
> 
> @Peter: Can you add this as a second example to the binding documentation?
> 

So, there are two examples at binding-doc, one for normal, one for HSIC? Fabio, do you
mean that? If DT maintainer agrees it too, I will add it.

Peter

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Frieder Schrempf]

Hi Fabio,

On 04.12.18 21:01, Fabio Estevam wrote:
> Hi Frieder,
> 
> On Tue, Dec 4, 2018 at 12:31 PM Schrempf Frieder
> <frieder.schrempf@kontron.de> wrote:
> 
>> There are many other optional properties for this driver and a lot of
>> them are not in the given example. Maybe we should just keep the
>> pinctrls for HSIC-mode out of the example, too?
> 
> I am just trying to make life easier for those who want to use HSIC
> support with chipidea.
> 
> Can we just add a real dts snippet example of your board into the
> binding document?

Sure, here is what I have in my dts:

&usbh2 {
	pinctrl-names = "idle", "active";
	pinctrl-0 = <&pinctrl_usbh2_idle>;
	pinctrl-1 = <&pinctrl_usbh2_active>;
	status = "okay";
	#address-cells = <1>;
	#size-cells = <0>;

	usbnet: smsc@1 {
		compatible = "usb424,9730";
		reg = <1>;
	};
};

@Peter: Can you add this as a second example to the binding documentation?

Thanks,
Frieder

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Fabio Estevam]

Hi Frieder,

On Tue, Dec 4, 2018 at 12:31 PM Schrempf Frieder
<frieder.schrempf@kontron.de> wrote:

> There are many other optional properties for this driver and a lot of
> them are not in the given example. Maybe we should just keep the
> pinctrls for HSIC-mode out of the example, too?

I am just trying to make life easier for those who want to use HSIC
support with chipidea.

Can we just add a real dts snippet example of your board into the
binding document?

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Frieder Schrempf]

Hi Peter, hi Fabio,

On 30.11.18 03:33, PETER CHEN wrote:
>   
>>
>> On Tue, Nov 27, 2018 at 7:31 AM PETER CHEN <peter.chen@nxp.com> wrote:
>>>
>>> For USB HSIC, the data and strobe pin needs to be pulled down at
>>> default, we consider it as "idle" state. When the USB host is ready to
>>> be used, the strobe pin needs to be pulled up, we consider it as
>>> "active" state.
>>>
>>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>>> ---
>>>   Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 8 +++++++-
>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>>> b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>>> index 529e51879fb2..1e5e7ddfb1a5 100644
>>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
>>> @@ -80,7 +80,10 @@ Optional properties:
>>>     controller. It's expected that a mux state of 0 indicates device mode and a
>>>     mux state of 1 indicates host mode.
>>>   - mux-control-names: Shall be "usb_switch" if mux-controls is specified.
>>> -- pinctrl-names: Names for optional pin modes in "default", "host", "device"
>>> +- pinctrl-names: Names for optional pin modes in "default", "host", "device".
>>> +  In case of HSIC-mode, "idle" and "active" pin modes are mandatory.
>>> +In this
>>> +  case, the "idle" state needs to pull down the data and strobe pin
>>> +  and the "active" state needs to pull up the strobe pin.
>>>   - pinctrl-n: alternate pin modes
>>>
>>>   i.mx specific properties
>>> @@ -110,4 +113,7 @@ Example:
>>>                  phy-clkgate-delay-us = <400>;
>>>                  mux-controls = <&usb_switch>;
>>>                  mux-control-names = "usb_switch";
>>> +               pinctrl-names = "idle", "active";
>>> +               pinctrl-0 = <&pinctrl_usbh2_1>;
>>> +               pinctrl-1 = <&pinctrl_usbh2_2>;
>>
>> I would put the "idle" and "active" entries inside a explicit section for HSIC.
>>
>> Otherwise, it may confuse people using non-HSIC bindings.
> 
> It is just an example; the user should check the meaning for optional properties before
> using it, like the property "phy-clkgate-delay-us", only imx7d/7s needs it.

There are many other optional properties for this driver and a lot of 
them are not in the given example. Maybe we should just keep the 
pinctrls for HSIC-mode out of the example, too?

Regards,
Frieder

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Fabio Estevam]

Hi Peter,

On Tue, Nov 27, 2018 at 7:31 AM PETER CHEN <peter.chen@nxp.com> wrote:
>
> For USB HSIC, the data and strobe pin needs to be pulled down
> at default, we consider it as "idle" state. When the USB host
> is ready to be used, the strobe pin needs to be pulled up,
> we consider it as "active" state.
>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index 529e51879fb2..1e5e7ddfb1a5 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -80,7 +80,10 @@ Optional properties:
>    controller. It's expected that a mux state of 0 indicates device mode and a
>    mux state of 1 indicates host mode.
>  - mux-control-names: Shall be "usb_switch" if mux-controls is specified.
> -- pinctrl-names: Names for optional pin modes in "default", "host", "device"
> +- pinctrl-names: Names for optional pin modes in "default", "host", "device".
> +  In case of HSIC-mode, "idle" and "active" pin modes are mandatory. In this
> +  case, the "idle" state needs to pull down the data and strobe pin
> +  and the "active" state needs to pull up the strobe pin.
>  - pinctrl-n: alternate pin modes
>
>  i.mx specific properties
> @@ -110,4 +113,7 @@ Example:
>                 phy-clkgate-delay-us = <400>;
>                 mux-controls = <&usb_switch>;
>                 mux-control-names = "usb_switch";
> +               pinctrl-names = "idle", "active";
> +               pinctrl-0 = <&pinctrl_usbh2_1>;
> +               pinctrl-1 = <&pinctrl_usbh2_2>;

I would put the "idle" and "active" entries inside a explicit section for HSIC.

Otherwise, it may confuse people using non-HSIC bindings.

[v3,4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Peter Chen]

For USB HSIC, the data and strobe pin needs to be pulled down
at default, we consider it as "idle" state. When the USB host
is ready to be used, the strobe pin needs to be pulled up,
we consider it as "active" state.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 529e51879fb2..1e5e7ddfb1a5 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -80,7 +80,10 @@ Optional properties:
   controller. It's expected that a mux state of 0 indicates device mode and a
   mux state of 1 indicates host mode.
 - mux-control-names: Shall be "usb_switch" if mux-controls is specified.
-- pinctrl-names: Names for optional pin modes in "default", "host", "device"
+- pinctrl-names: Names for optional pin modes in "default", "host", "device".
+  In case of HSIC-mode, "idle" and "active" pin modes are mandatory. In this
+  case, the "idle" state needs to pull down the data and strobe pin
+  and the "active" state needs to pull up the strobe pin.
 - pinctrl-n: alternate pin modes
 
 i.mx specific properties
@@ -110,4 +113,7 @@ Example:
 		phy-clkgate-delay-us = <400>;
 		mux-controls = <&usb_switch>;
 		mux-control-names = "usb_switch";
+		pinctrl-names = "idle", "active";
+		pinctrl-0 = <&pinctrl_usbh2_1>;
+		pinctrl-1 = <&pinctrl_usbh2_2>;
 	};