[PATCH 1/3] ARM: dts: colibri-imx6: remove pinctrl-names orphan

[PATCH 1/3] ARM: dts: colibri-imx6: remove pinctrl-names orphan

[Philippe Schenker]

This is not necessary without a pinctrl-0 statement. Remove this
orphan.

Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
---

 arch/arm/boot/dts/imx6qdl-colibri.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
index 240b86d2eb71..a4a54c82e28f 100644
--- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
@@ -362,7 +362,6 @@ &uart3 {
 };
 
 &usbotg {
-	pinctrl-names = "default";
 	disable-over-current;
 	dr_mode = "peripheral";
 	status = "disabled";
-- 
2.27.0

[PATCH 2/3] ARM: dts: colibri-imx6: add usb dual-role switch capability

[Philippe Schenker]

Since the runtime-pm wakeup bug was fixed in
drivers/usb/chipidea/core.c usb dual-role host/device switching is
working. So make use of it.

Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
---

 arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts | 8 ++++++++
 arch/arm/boot/dts/imx6qdl-colibri.dtsi       | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
index 65359aece950..c978b3f19a2d 100644
--- a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
+++ b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
@@ -45,6 +45,13 @@ clk16m: clock-16m {
 		clock-output-names = "clk16m";
 	};
 
+	extcon_usbc_det: usbc_det {
+		compatible = "linux,extcon-usb-gpio";
+		id-gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_usbc_det>;
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		pinctrl-names = "default";
@@ -220,6 +227,7 @@ &usbh1 {
 };
 
 &usbotg {
+	extcon = <0>, <&extcon_usbc_det>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
index a4a54c82e28f..de49f9faa426 100644
--- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
@@ -363,7 +363,7 @@ &uart3 {
 
 &usbotg {
 	disable-over-current;
-	dr_mode = "peripheral";
+	dr_mode = "otg";
 	status = "disabled";
 };
 
-- 
2.27.0

Re: [PATCH 2/3] ARM: dts: colibri-imx6: add usb dual-role switch capability

[Oleksandr Suvorov]

On Fri, Jul 10, 2020 at 4:26 PM Philippe Schenker
<philippe.schenker@toradex.com> wrote:
>
> Since the runtime-pm wakeup bug was fixed in
> drivers/usb/chipidea/core.c usb dual-role host/device switching is
> working. So make use of it.
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>

Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>

> ---
>
>  arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts | 8 ++++++++
>  arch/arm/boot/dts/imx6qdl-colibri.dtsi       | 2 +-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> index 65359aece950..c978b3f19a2d 100644
> --- a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> +++ b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> @@ -45,6 +45,13 @@ clk16m: clock-16m {
>                 clock-output-names = "clk16m";
>         };
>
> +       extcon_usbc_det: usbc_det {
> +               compatible = "linux,extcon-usb-gpio";
> +               id-gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pinctrl_usbc_det>;
> +       };
> +
>         gpio-keys {
>                 compatible = "gpio-keys";
>                 pinctrl-names = "default";
> @@ -220,6 +227,7 @@ &usbh1 {
>  };
>
>  &usbotg {
> +       extcon = <0>, <&extcon_usbc_det>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> index a4a54c82e28f..de49f9faa426 100644
> --- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> @@ -363,7 +363,7 @@ &uart3 {
>
>  &usbotg {
>         disable-over-current;
> -       dr_mode = "peripheral";
> +       dr_mode = "otg";
>         status = "disabled";
>  };
>
> --
> 2.27.0
>


-- 
Best regards

Oleksandr Suvorov
cryosay@gmail.com

-- 
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

[PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Philippe Schenker]

Since the runtime-pm wakeup bug was fixed in
drivers/usb/chipidea/core.c usb dual-role host/device switching is
working. So make use of it.

Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>

---

 arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi | 9 +++++++++
 arch/arm/boot/dts/imx7-colibri.dtsi         | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
index 97601375f264..db56a532a34a 100644
--- a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
@@ -20,6 +20,14 @@ clk16m: clk16m {
 		clock-frequency = <16000000>;
 	};
 
+	extcon_usbc_det: usbc_det {
+		compatible = "linux,extcon-usb-gpio";
+		id-gpio = <&gpio7 14 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_usbc_det>;
+	};
+
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		pinctrl-names = "default";
@@ -174,6 +182,7 @@ &uart3 {
 };
 
 &usbotg1 {
+	extcon = <0>, <&extcon_usbc_det>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
index e18e89dec879..caea90d2421f 100644
--- a/arch/arm/boot/dts/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri.dtsi
@@ -457,7 +457,7 @@ &uart3 {
 };
 
 &usbotg1 {
-	dr_mode = "host";
+	dr_mode = "otg";
 };
 
 &usdhc1 {
@@ -486,7 +486,7 @@ &usdhc3 {
 &iomuxc {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3 &pinctrl_gpio4
-		     &pinctrl_gpio7 &pinctrl_usbc_det>;
+		     &pinctrl_gpio7>;
 
 	pinctrl_gpio1: gpio1-grp {
 		fsl,pins = <
-- 
2.27.0

Re: [PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Oleksandr Suvorov]

On Fri, Jul 10, 2020 at 4:26 PM Philippe Schenker
<philippe.schenker@toradex.com> wrote:
>
> Since the runtime-pm wakeup bug was fixed in
> drivers/usb/chipidea/core.c usb dual-role host/device switching is
> working. So make use of it.
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>

Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>

>
> ---
>
>  arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi | 9 +++++++++
>  arch/arm/boot/dts/imx7-colibri.dtsi         | 4 ++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> index 97601375f264..db56a532a34a 100644
> --- a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> @@ -20,6 +20,14 @@ clk16m: clk16m {
>                 clock-frequency = <16000000>;
>         };
>
> +       extcon_usbc_det: usbc_det {
> +               compatible = "linux,extcon-usb-gpio";
> +               id-gpio = <&gpio7 14 GPIO_ACTIVE_HIGH>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pinctrl_usbc_det>;
> +       };
> +
> +
>         gpio-keys {
>                 compatible = "gpio-keys";
>                 pinctrl-names = "default";
> @@ -174,6 +182,7 @@ &uart3 {
>  };
>
>  &usbotg1 {
> +       extcon = <0>, <&extcon_usbc_det>;
>         status = "okay";
>  };
>
> diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
> index e18e89dec879..caea90d2421f 100644
> --- a/arch/arm/boot/dts/imx7-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
> @@ -457,7 +457,7 @@ &uart3 {
>  };
>
>  &usbotg1 {
> -       dr_mode = "host";
> +       dr_mode = "otg";
>  };
>
>  &usdhc1 {
> @@ -486,7 +486,7 @@ &usdhc3 {
>  &iomuxc {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3 &pinctrl_gpio4
> -                    &pinctrl_gpio7 &pinctrl_usbc_det>;
> +                    &pinctrl_gpio7>;
>
>         pinctrl_gpio1: gpio1-grp {
>                 fsl,pins = <
> --
> 2.27.0
>


-- 
Best regards

Oleksandr Suvorov
cryosay@gmail.com

-- 
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

Re: [PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Ahmad Fatoum]

Hello Philippe,

On 7/10/20 3:24 PM, Philippe Schenker wrote:
> Since the runtime-pm wakeup bug was fixed in
> drivers/usb/chipidea/core.c usb dual-role host/device switching is
> working. So make use of it.
> 
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> 
> ---
> 
>  arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi | 9 +++++++++
>  arch/arm/boot/dts/imx7-colibri.dtsi         | 4 ++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> index 97601375f264..db56a532a34a 100644
> --- a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> @@ -20,6 +20,14 @@ clk16m: clk16m {
>  		clock-frequency = <16000000>;
>  	};
>  
> +	extcon_usbc_det: usbc_det {
> +		compatible = "linux,extcon-usb-gpio";

According to 4602f3bff266 ("usb: common: add USB GPIO based connection detection driver"):
"the old way using extcon to support USB Dual-Role switch is now deprecated
 when use Type-B connector."

Have you considered using a compatible = "gpio-usb-b-connector" child node instead?

Cheers,
Ahmad

> +		id-gpio = <&gpio7 14 GPIO_ACTIVE_HIGH>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_usbc_det>;
> +	};
> +
> +
>  	gpio-keys {
>  		compatible = "gpio-keys";
>  		pinctrl-names = "default";
> @@ -174,6 +182,7 @@ &uart3 {
>  };
>  
>  &usbotg1 {
> +	extcon = <0>, <&extcon_usbc_det>;
>  	status = "okay";
>  };
>  
> diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
> index e18e89dec879..caea90d2421f 100644
> --- a/arch/arm/boot/dts/imx7-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
> @@ -457,7 +457,7 @@ &uart3 {
>  };
>  
>  &usbotg1 {
> -	dr_mode = "host";
> +	dr_mode = "otg";
>  };
>  
>  &usdhc1 {
> @@ -486,7 +486,7 @@ &usdhc3 {
>  &iomuxc {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3 &pinctrl_gpio4
> -		     &pinctrl_gpio7 &pinctrl_usbc_det>;
> +		     &pinctrl_gpio7>;
>  
>  	pinctrl_gpio1: gpio1-grp {
>  		fsl,pins = <
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Shawn Guo]

On Mon, Jul 13, 2020 at 11:46:04AM +0200, Ahmad Fatoum wrote:
> Hello Philippe,
> 
> On 7/10/20 3:24 PM, Philippe Schenker wrote:
> > Since the runtime-pm wakeup bug was fixed in
> > drivers/usb/chipidea/core.c usb dual-role host/device switching is
> > working. So make use of it.
> > 
> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> > 
> > ---
> > 
> >  arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi | 9 +++++++++
> >  arch/arm/boot/dts/imx7-colibri.dtsi         | 4 ++--
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> > index 97601375f264..db56a532a34a 100644
> > --- a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> > +++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> > @@ -20,6 +20,14 @@ clk16m: clk16m {
> >  		clock-frequency = <16000000>;
> >  	};
> >  
> > +	extcon_usbc_det: usbc_det {
> > +		compatible = "linux,extcon-usb-gpio";
> 
> According to 4602f3bff266 ("usb: common: add USB GPIO based connection detection driver"):
> "the old way using extcon to support USB Dual-Role switch is now deprecated
>  when use Type-B connector."
> 
> Have you considered using a compatible = "gpio-usb-b-connector" child node instead?

I dropped patch #2 and #3 for now.

Shawn

Re: [PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Philippe Schenker]

On Mon, 2020-07-13 at 11:46 +0200, Ahmad Fatoum wrote:
> Hello Philippe,
> 
> On 7/10/20 3:24 PM, Philippe Schenker wrote:
> > Since the runtime-pm wakeup bug was fixed in
> > drivers/usb/chipidea/core.c usb dual-role host/device switching is
> > working. So make use of it.
> > 
> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> > 
> > ---
> > 
> >  arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi | 9 +++++++++
> >  arch/arm/boot/dts/imx7-colibri.dtsi         | 4 ++--
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> > b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> > index 97601375f264..db56a532a34a 100644
> > --- a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> > +++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> > @@ -20,6 +20,14 @@ clk16m: clk16m {
> >  		clock-frequency = <16000000>;
> >  	};
> >  
> > +	extcon_usbc_det: usbc_det {
> > +		compatible = "linux,extcon-usb-gpio";
> 
> According to 4602f3bff266 ("usb: common: add USB GPIO based connection
> detection driver"):
> "the old way using extcon to support USB Dual-Role switch is now
> deprecated
>  when use Type-B connector."
> 
> Have you considered using a compatible = "gpio-usb-b-connector" child
> node instead?
> 
> Cheers,
> Ahmad

Thanks for the Hint Ahmad,

I already tried and just now tried again but it doesn't work on our
hardware. Are you sure this works with chipidea driver?

Should this new usb-connector stuff work in general with every old
driver?

Philippe

> 
> > +		id-gpio = <&gpio7 14 GPIO_ACTIVE_HIGH>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_usbc_det>;
> > +	};
> > +
> > +
> >  	gpio-keys {
> >  		compatible = "gpio-keys";
> >  		pinctrl-names = "default";
> > @@ -174,6 +182,7 @@ &uart3 {
> >  };
> >  
> >  &usbotg1 {
> > +	extcon = <0>, <&extcon_usbc_det>;
> >  	status = "okay";
> >  };
> >  
> > diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi
> > b/arch/arm/boot/dts/imx7-colibri.dtsi
> > index e18e89dec879..caea90d2421f 100644
> > --- a/arch/arm/boot/dts/imx7-colibri.dtsi
> > +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
> > @@ -457,7 +457,7 @@ &uart3 {
> >  };
> >  
> >  &usbotg1 {
> > -	dr_mode = "host";
> > +	dr_mode = "otg";
> >  };
> >  
> >  &usdhc1 {
> > @@ -486,7 +486,7 @@ &usdhc3 {
> >  &iomuxc {
> >  	pinctrl-names = "default";
> >  	pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3
> > &pinctrl_gpio4
> > -		     &pinctrl_gpio7 &pinctrl_usbc_det>;
> > +		     &pinctrl_gpio7>;
> >  
> >  	pinctrl_gpio1: gpio1-grp {
> >  		fsl,pins = <
> > 

Re: [PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Ahmad Fatoum]

Hello Philippe,

On 7/13/20 1:53 PM, Philippe Schenker wrote:
> On Mon, 2020-07-13 at 11:46 +0200, Ahmad Fatoum wrote:
>> Hello Philippe,
>>
>>> +	extcon_usbc_det: usbc_det {
>>> +		compatible = "linux,extcon-usb-gpio";
>>
>> According to 4602f3bff266 ("usb: common: add USB GPIO based connection
>> detection driver"):
>> "the old way using extcon to support USB Dual-Role switch is now
>> deprecated
>>  when use Type-B connector."
>>
>> Have you considered using a compatible = "gpio-usb-b-connector" child
>> node instead?
>>
>> Cheers,
>> Ahmad
> 
> Thanks for the Hint Ahmad,
> 
> I already tried and just now tried again but it doesn't work on our
> hardware. Are you sure this works with chipidea driver?

I haven't, just wanted to point its existence out in case you didn't know.
Seems we need to call of_platform_populate somewhere in the driver.
Unsure what other changes are necessary.

> Should this new usb-connector stuff work in general with every old
> driver?

If the driver support isn't there yet, I think use of extcon is fine as is.

Thanks
Ahmad

> 
> Philippe



-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Philippe Schenker]

On Tue, 2020-07-14 at 09:55 +0200, Ahmad Fatoum wrote:
> Hello Philippe,
> 
> On 7/13/20 1:53 PM, Philippe Schenker wrote:
> > On Mon, 2020-07-13 at 11:46 +0200, Ahmad Fatoum wrote:
> > > Hello Philippe,
> > > 
> > > > +	extcon_usbc_det: usbc_det {
> > > > +		compatible = "linux,extcon-usb-gpio";
> > > 
> > > According to 4602f3bff266 ("usb: common: add USB GPIO based
> > > connection
> > > detection driver"):
> > > "the old way using extcon to support USB Dual-Role switch is now
> > > deprecated
> > >  when use Type-B connector."
> > > 
> > > Have you considered using a compatible = "gpio-usb-b-connector"
> > > child
> > > node instead?
> > > 
> > > Cheers,
> > > Ahmad
> > 
> > Thanks for the Hint Ahmad,
> > 
> > I already tried and just now tried again but it doesn't work on our
> > hardware. Are you sure this works with chipidea driver?
> 
> I haven't, just wanted to point its existence out in case you didn't
> know.
> Seems we need to call of_platform_populate somewhere in the driver.
> Unsure what other changes are necessary.

Yep, thanks for that! It seems like that need to be really called as I
can't get a probe of the new usb connector driver. I quickly grepped
through code and saw that tegra USB driver is calling
of_platform_populate and they also use that gpio-usb-b-connector.

> 
> > Should this new usb-connector stuff work in general with every old
> > driver?
> 
> If the driver support isn't there yet, I think use of extcon is fine
> as is.
> 
> Thanks
> Ahmad

@Shawn: Could you pull patches #2 and #3 back in?

Thanks guys!

Philippe
> 
> > Philippe
> 
> 

Re: [PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Shawn Guo]

On Tue, Jul 14, 2020 at 08:20:34AM +0000, Philippe Schenker wrote:
> On Tue, 2020-07-14 at 09:55 +0200, Ahmad Fatoum wrote:
> > Hello Philippe,
> > 
> > On 7/13/20 1:53 PM, Philippe Schenker wrote:
> > > On Mon, 2020-07-13 at 11:46 +0200, Ahmad Fatoum wrote:
> > > > Hello Philippe,
> > > > 
> > > > > +	extcon_usbc_det: usbc_det {
> > > > > +		compatible = "linux,extcon-usb-gpio";
> > > > 
> > > > According to 4602f3bff266 ("usb: common: add USB GPIO based
> > > > connection
> > > > detection driver"):
> > > > "the old way using extcon to support USB Dual-Role switch is now
> > > > deprecated
> > > >  when use Type-B connector."
> > > > 
> > > > Have you considered using a compatible = "gpio-usb-b-connector"
> > > > child
> > > > node instead?
> > > > 
> > > > Cheers,
> > > > Ahmad
> > > 
> > > Thanks for the Hint Ahmad,
> > > 
> > > I already tried and just now tried again but it doesn't work on our
> > > hardware. Are you sure this works with chipidea driver?
> > 
> > I haven't, just wanted to point its existence out in case you didn't
> > know.
> > Seems we need to call of_platform_populate somewhere in the driver.
> > Unsure what other changes are necessary.
> 
> Yep, thanks for that! It seems like that need to be really called as I
> can't get a probe of the new usb connector driver. I quickly grepped
> through code and saw that tegra USB driver is calling
> of_platform_populate and they also use that gpio-usb-b-connector.
> 
> > 
> > > Should this new usb-connector stuff work in general with every old
> > > driver?
> > 
> > If the driver support isn't there yet, I think use of extcon is fine
> > as is.

Shouldn't we improve chipidea driver to get it work with gpio-usb-b-connector?

Shawn

Re: [PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Philippe Schenker]

On Mon, 2020-07-20 at 10:11 +0800, Shawn Guo wrote:
> On Tue, Jul 14, 2020 at 08:20:34AM +0000, Philippe Schenker wrote:
> > On Tue, 2020-07-14 at 09:55 +0200, Ahmad Fatoum wrote:
> > > Hello Philippe,
> > > 
> > > On 7/13/20 1:53 PM, Philippe Schenker wrote:
> > > > On Mon, 2020-07-13 at 11:46 +0200, Ahmad Fatoum wrote:
> > > > > Hello Philippe,
> > > > > 
> > > > > > +	extcon_usbc_det: usbc_det {
> > > > > > +		compatible = "linux,extcon-usb-gpio";
> > > > > 
> > > > > According to 4602f3bff266 ("usb: common: add USB GPIO based
> > > > > connection
> > > > > detection driver"):
> > > > > "the old way using extcon to support USB Dual-Role switch is
> > > > > now
> > > > > deprecated
> > > > >  when use Type-B connector."
> > > > > 
> > > > > Have you considered using a compatible = "gpio-usb-b-
> > > > > connector"
> > > > > child
> > > > > node instead?
> > > > > 
> > > > > Cheers,
> > > > > Ahmad
> > > > 
> > > > Thanks for the Hint Ahmad,
> > > > 
> > > > I already tried and just now tried again but it doesn't work on
> > > > our
> > > > hardware. Are you sure this works with chipidea driver?
> > > 
> > > I haven't, just wanted to point its existence out in case you
> > > didn't
> > > know.
> > > Seems we need to call of_platform_populate somewhere in the
> > > driver.
> > > Unsure what other changes are necessary.
> > 
> > Yep, thanks for that! It seems like that need to be really called as
> > I
> > can't get a probe of the new usb connector driver. I quickly grepped
> > through code and saw that tegra USB driver is calling
> > of_platform_populate and they also use that gpio-usb-b-connector.
> > 
> > > > Should this new usb-connector stuff work in general with every
> > > > old
> > > > driver?
> > > 
> > > If the driver support isn't there yet, I think use of extcon is
> > > fine
> > > as is.
> 
> Shouldn't we improve chipidea driver to get it work with gpio-usb-b-
> connector?

I know this would be nice but I don't know that code well enough to be
able to make this changes in a timely manner. And I rather like to have
support for USB dual-role support "the old way" now than never (we
actually tried this for along time on our Colibri boards).

I discovered a bug in that driver together with Peter Chen, which got
now solved by him and I would like to use the host/device switching on
colibri-imx6/7 boards now.
> 
> Shawn

Re: [PATCH 3/3] ARM: dts: colibri-imx7: add usb dual-role switch capability

[Philippe Schenker]

On Mon, 2020-07-20 at 09:37 +0200, Philippe Schenker wrote:
> On Mon, 2020-07-20 at 10:11 +0800, Shawn Guo wrote:
> > On Tue, Jul 14, 2020 at 08:20:34AM +0000, Philippe Schenker wrote:
> > > On Tue, 2020-07-14 at 09:55 +0200, Ahmad Fatoum wrote:
> > > > Hello Philippe,
> > > > 
> > > > On 7/13/20 1:53 PM, Philippe Schenker wrote:
> > > > > On Mon, 2020-07-13 at 11:46 +0200, Ahmad Fatoum wrote:
> > > > > > Hello Philippe,
> > > > > > 
> > > > > > > +	extcon_usbc_det: usbc_det {
> > > > > > > +		compatible = "linux,extcon-usb-gpio";
> > > > > > 
> > > > > > According to 4602f3bff266 ("usb: common: add USB GPIO based
> > > > > > connection
> > > > > > detection driver"):
> > > > > > "the old way using extcon to support USB Dual-Role switch is
> > > > > > now
> > > > > > deprecated
> > > > > >  when use Type-B connector."
> > > > > > 
> > > > > > Have you considered using a compatible = "gpio-usb-b-
> > > > > > connector"
> > > > > > child
> > > > > > node instead?
> > > > > > 
> > > > > > Cheers,
> > > > > > Ahmad
> > > > > 
> > > > > Thanks for the Hint Ahmad,
> > > > > 
> > > > > I already tried and just now tried again but it doesn't work
> > > > > on
> > > > > our
> > > > > hardware. Are you sure this works with chipidea driver?
> > > > 
> > > > I haven't, just wanted to point its existence out in case you
> > > > didn't
> > > > know.
> > > > Seems we need to call of_platform_populate somewhere in the
> > > > driver.
> > > > Unsure what other changes are necessary.
> > > 
> > > Yep, thanks for that! It seems like that need to be really called
> > > as
> > > I
> > > can't get a probe of the new usb connector driver. I quickly
> > > grepped
> > > through code and saw that tegra USB driver is calling
> > > of_platform_populate and they also use that gpio-usb-b-connector.
> > > 
> > > > > Should this new usb-connector stuff work in general with every
> > > > > old
> > > > > driver?
> > > > 
> > > > If the driver support isn't there yet, I think use of extcon is
> > > > fine
> > > > as is.
> > 
> > Shouldn't we improve chipidea driver to get it work with gpio-usb-b-
> > connector?
> 
> I know this would be nice but I don't know that code well enough to be
> able to make this changes in a timely manner. And I rather like to
> have
> support for USB dual-role support "the old way" now than never (we
> actually tried this for along time on our Colibri boards).
> 
> I discovered a bug in that driver together with Peter Chen, which got
> now solved by him and I would like to use the host/device switching on
> colibri-imx6/7 boards now.

Hi Shawn,

Could you let me know how I should proceed with these patches?

Thanks,
Philippe

Re: [PATCH 1/3] ARM: dts: colibri-imx6: remove pinctrl-names orphan

[Oleksandr Suvorov]

On Fri, Jul 10, 2020 at 4:25 PM Philippe Schenker
<philippe.schenker@toradex.com> wrote:
>
> This is not necessary without a pinctrl-0 statement. Remove this
> orphan.
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>

Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>

> ---
>
>  arch/arm/boot/dts/imx6qdl-colibri.dtsi | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> index 240b86d2eb71..a4a54c82e28f 100644
> --- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> @@ -362,7 +362,6 @@ &uart3 {
>  };
>
>  &usbotg {
> -       pinctrl-names = "default";
>         disable-over-current;
>         dr_mode = "peripheral";
>         status = "disabled";
> --
> 2.27.0
>


-- 
Best regards

Oleksandr Suvorov
cryosay@gmail.com

-- 
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

Re: [PATCH 1/3] ARM: dts: colibri-imx6: remove pinctrl-names orphan

[Shawn Guo]

On Fri, Jul 10, 2020 at 03:24:21PM +0200, Philippe Schenker wrote:
> This is not necessary without a pinctrl-0 statement. Remove this
> orphan.
> 
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>

Applied all, thanks.