| Message ID | 20200710132423.497230-3-philippe.schenker@toradex.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] ARM: dts: colibri-imx6: remove pinctrl-names orphan | expand |
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 >
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 = < >
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
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 = < > >
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
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 > >
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
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
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
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 = <
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(-)