2023-03-23 11:07:13

by Marco Felsch

[permalink] [raw]
Subject: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support

The i.MX8MP-EVK has a dual-role usb-type-c port marked as PORT1. By this
commit the dual-role support is added which allows the user-space to
assign usb-gadget functions to it via the configFS.

Signed-off-by: Marco Felsch <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 59 ++++++++++++++++++++
1 file changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index f2d93437084be..982fe35f09a7e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -5,7 +5,9 @@

/dts-v1/;

+#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/phy/phy-imx8-pcie.h>
+#include <dt-bindings/usb/pd.h>
#include "imx8mp.dtsi"

/ {
@@ -336,6 +338,34 @@ &i2c2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i2c2>;
status = "okay";
+
+ tcpc@50 {
+ compatible = "nxp,ptn5110";
+ reg = <0x50>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tcpc>;
+ interrupt-parent = <&gpio4>;
+ interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
+
+ connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ power-role = "dual";
+ data-role = "dual";
+ try-power-role = "sink";
+ source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
+ sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
+ PDO_VAR(5000, 20000, 3000)>;
+ op-sink-microwatt = <15000000>;
+ self-powered;
+ };
+
+ port {
+ usb_con_ss: endpoint {
+ remote-endpoint = <&usb_dwc3_0_drd>;
+ };
+ };
+ };
};

&i2c3 {
@@ -449,14 +479,37 @@ &uart2 {
status = "okay";
};

+&usb3_phy0 {
+ status = "okay";
+};
+
&usb3_phy1 {
status = "okay";
};

+&usb3_0 {
+ status = "okay";
+};
+
&usb3_1 {
status = "okay";
};

+&usb_dwc3_0 {
+ dr_mode = "otg";
+ hnp-disable;
+ srp-disable;
+ adp-disable;
+ usb-role-switch;
+ status = "okay";
+
+ port {
+ usb_dwc3_0_drd: endpoint {
+ remote-endpoint = <&usb_con_ss>;
+ };
+ };
+};
+
&usb_dwc3_1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usb1_vbus>;
@@ -666,6 +719,12 @@ MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19 0x40
>;
};

+ pinctrl_tcpc: tcpcgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19 0x159
+ >;
+ };
+
pinctrl_uart1: uart1grp {
fsl,pins = <
MX8MP_IOMUXC_UART1_RXD__UART1_DCE_RX 0x140
--
2.30.2


2023-03-24 10:30:01

by Jun Li

[permalink] [raw]
Subject: RE: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support



> -----Original Message-----
> From: Marco Felsch <[email protected]>
> Sent: Thursday, March 23, 2023 6:58 PM
> To: [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support
>
> The i.MX8MP-EVK has a dual-role usb-type-c port marked as PORT1. By this
> commit the dual-role support is added which allows the user-space to assign
> usb-gadget functions to it via the configFS.

So just ignore the orientation switch will make this port cannot work
at super speed, this is actually why this port is not enabled at upstream.
I see the orientation switch via GPIO for SBU is already merged:
drivers/usb/typec/mux/gpio-sbu-mux.c
Do you have interest to expand this driver to support super speed
switch for this case?

Thanks
Li Jun
>
> Signed-off-by: Marco Felsch <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 59 ++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> index f2d93437084be..982fe35f09a7e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> @@ -5,7 +5,9 @@
>
> /dts-v1/;
>
> +#include <dt-bindings/interrupt-controller/irq.h>
> #include <dt-bindings/phy/phy-imx8-pcie.h>
> +#include <dt-bindings/usb/pd.h>
> #include "imx8mp.dtsi"
>
> / {
> @@ -336,6 +338,34 @@ &i2c2 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_i2c2>;
> status = "okay";
> +
> + tcpc@50 {
> + compatible = "nxp,ptn5110";
> + reg = <0x50>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_tcpc>;
> + interrupt-parent = <&gpio4>;
> + interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
> +
> + connector {
> + compatible = "usb-c-connector";
> + label = "USB-C";
> + power-role = "dual";
> + data-role = "dual";
> + try-power-role = "sink";
> + source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> + sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> + PDO_VAR(5000, 20000, 3000)>;
> + op-sink-microwatt = <15000000>;
> + self-powered;
> + };
> +
> + port {
> + usb_con_ss: endpoint {
> + remote-endpoint = <&usb_dwc3_0_drd>;
> + };
> + };
> + };
> };
>
> &i2c3 {
> @@ -449,14 +479,37 @@ &uart2 {
> status = "okay";
> };
>
> +&usb3_phy0 {
> + status = "okay";
> +};
> +
> &usb3_phy1 {
> status = "okay";
> };
>
> +&usb3_0 {
> + status = "okay";
> +};
> +
> &usb3_1 {
> status = "okay";
> };
>
> +&usb_dwc3_0 {
> + dr_mode = "otg";
> + hnp-disable;
> + srp-disable;
> + adp-disable;
> + usb-role-switch;
> + status = "okay";
> +
> + port {
> + usb_dwc3_0_drd: endpoint {
> + remote-endpoint = <&usb_con_ss>;
> + };
> + };
> +};
> +
> &usb_dwc3_1 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usb1_vbus>;
> @@ -666,6 +719,12 @@ MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19 0x40
> >;
> };
>
> + pinctrl_tcpc: tcpcgrp {
> + fsl,pins = <
> + MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19 0x159
> + >;
> + };
> +
> pinctrl_uart1: uart1grp {
> fsl,pins = <
> MX8MP_IOMUXC_UART1_RXD__UART1_DCE_RX 0x140
> --
> 2.30.2

2023-03-27 08:25:14

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support

Hi,

On 23-03-24, Jun Li wrote:
>
>
> > -----Original Message-----
> > From: Marco Felsch <[email protected]>
> > Sent: Thursday, March 23, 2023 6:58 PM
> > To: [email protected]; [email protected];
> > [email protected]; dl-linux-imx <[email protected]>; [email protected]
> > Cc: [email protected]; [email protected];
> > [email protected]
> > Subject: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support
> >
> > The i.MX8MP-EVK has a dual-role usb-type-c port marked as PORT1. By this
> > commit the dual-role support is added which allows the user-space to assign
> > usb-gadget functions to it via the configFS.
>
> So just ignore the orientation switch will make this port cannot work
> at super speed, this is actually why this port is not enabled at upstream.

I saw comments on the i.MX8MP-EVK schematic but no erratum listed. Can
you explain this a bit more in detail? Since the USBx_ID pin is
unconnected, the role is described via DTs as otg and the tcpc can be
configured via user-space. I tested this at least for the device mode.

> I see the orientation switch via GPIO for SBU is already merged:
> drivers/usb/typec/mux/gpio-sbu-mux.c
> Do you have interest to expand this driver to support super speed
> switch for this case?

My intention was to enable this port to be able to have it as device
which is useful for bootloaders. The speed doesn't matter to me, at
least not now.

Regards,
Marco


>
> Thanks
> Li Jun
> >
> > Signed-off-by: Marco Felsch <[email protected]>
> > ---
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 59 ++++++++++++++++++++
> > 1 file changed, 59 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > index f2d93437084be..982fe35f09a7e 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > @@ -5,7 +5,9 @@
> >
> > /dts-v1/;
> >
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > #include <dt-bindings/phy/phy-imx8-pcie.h>
> > +#include <dt-bindings/usb/pd.h>
> > #include "imx8mp.dtsi"
> >
> > / {
> > @@ -336,6 +338,34 @@ &i2c2 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_i2c2>;
> > status = "okay";
> > +
> > + tcpc@50 {
> > + compatible = "nxp,ptn5110";
> > + reg = <0x50>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_tcpc>;
> > + interrupt-parent = <&gpio4>;
> > + interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
> > +
> > + connector {
> > + compatible = "usb-c-connector";
> > + label = "USB-C";
> > + power-role = "dual";
> > + data-role = "dual";
> > + try-power-role = "sink";
> > + source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> > + sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> > + PDO_VAR(5000, 20000, 3000)>;
> > + op-sink-microwatt = <15000000>;
> > + self-powered;
> > + };
> > +
> > + port {
> > + usb_con_ss: endpoint {
> > + remote-endpoint = <&usb_dwc3_0_drd>;
> > + };
> > + };
> > + };
> > };
> >
> > &i2c3 {
> > @@ -449,14 +479,37 @@ &uart2 {
> > status = "okay";
> > };
> >
> > +&usb3_phy0 {
> > + status = "okay";
> > +};
> > +
> > &usb3_phy1 {
> > status = "okay";
> > };
> >
> > +&usb3_0 {
> > + status = "okay";
> > +};
> > +
> > &usb3_1 {
> > status = "okay";
> > };
> >
> > +&usb_dwc3_0 {
> > + dr_mode = "otg";
> > + hnp-disable;
> > + srp-disable;
> > + adp-disable;
> > + usb-role-switch;
> > + status = "okay";
> > +
> > + port {
> > + usb_dwc3_0_drd: endpoint {
> > + remote-endpoint = <&usb_con_ss>;
> > + };
> > + };
> > +};
> > +
> > &usb_dwc3_1 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_usb1_vbus>;
> > @@ -666,6 +719,12 @@ MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19 0x40
> > >;
> > };
> >
> > + pinctrl_tcpc: tcpcgrp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19 0x159
> > + >;
> > + };
> > +
> > pinctrl_uart1: uart1grp {
> > fsl,pins = <
> > MX8MP_IOMUXC_UART1_RXD__UART1_DCE_RX 0x140
> > --
> > 2.30.2
>
>

2023-03-27 09:07:54

by Andreas Henriksson

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support

On Fri, Mar 24, 2023 at 10:18:17AM +0000, Jun Li wrote:
>
>
> > -----Original Message-----
> > From: Marco Felsch <[email protected]>
> > Sent: Thursday, March 23, 2023 6:58 PM
> > To: [email protected]; [email protected];
> > [email protected]; dl-linux-imx <[email protected]>; [email protected]
> > Cc: [email protected]; [email protected];
> > [email protected]
> > Subject: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support
> >
> > The i.MX8MP-EVK has a dual-role usb-type-c port marked as PORT1. By this
> > commit the dual-role support is added which allows the user-space to assign
> > usb-gadget functions to it via the configFS.
>
> So just ignore the orientation switch will make this port cannot work
> at super speed, this is actually why this port is not enabled at upstream.
> I see the orientation switch via GPIO for SBU is already merged:
> drivers/usb/typec/mux/gpio-sbu-mux.c
> Do you have interest to expand this driver to support super speed
> switch for this case?
[...]

FWIW This is what I ended up with (after backporting the gpio-sbu-mux patches)
a little while ago trying to get the usb-c ports going on imx8mp-evk. I've not
yet had the time to fully test this (only done host/device, not tested: SS,
orientation, etc), so beware that it might be completely wrong.

#include "dt-bindings/usb/pd.h"

&usb3_phy0 {
vbus-power-supply = <&ptn5110>;
status = "okay";
};

&usb3_0 {
status = "okay";
};

&usb_dwc3_0 {
dr_mode = "otg";
hnp-disable;
srp-disable;
adp-disable;
usb-role-switch;
role-switch-default-mode = "peripheral";
snps,dis-u1-entry-quirk;
snps,dis-u2-entry-quirk;
status = "okay";

port {
usb3_drd_sw: endpoint {
remote-endpoint = <&typec_dr_sw>;
};
};
};

&i2c2 {
clock-frequency = <100000>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i2c2>;
status = "okay";

ptn5110: tcpc@50 {
compatible = "nxp,ptn5110";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_typec>;
reg = <0x50>;

interrupt-parent = <&gpio4>;
interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
status = "okay";

port {
typec_dr_sw: endpoint {
remote-endpoint = <&usb3_drd_sw>;
};
};

usb_con: connector {
compatible = "usb-c-connector";
label = "USB-C";
power-role = "dual";
data-role = "dual";
try-power-role = "sink";
source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
PDO_VAR(5000, 20000, 3000)>;
op-sink-microwatt = <15000000>;
self-powered;

ports {
#address-cells = <1>;
#size-cells = <0>;

port@1 {
reg = <1>;
typec_con_ss: endpoint {
remote-endpoint = <&usb3_data_ss>;
};
};
};
};
};

};

&iomuxc {
pinctrl_typec: typec1grp {
fsl,pins = <
MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19 0x1c4
>;
};

pinctrl_typec_mux: typec1muxgrp {
fsl,pins = <
MX8MP_IOMUXC_SAI1_MCLK__GPIO4_IO20 0x16
MX8MP_IOMUXC_SD2_WP__GPIO2_IO20 0x16
>;
};


pinctrl_i2c2: i2c2grp {
fsl,pins = <
MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL 0x400001c2
MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA 0x400001c2
>;
};
};

/ {
gpio-sbu-mux {
compatible = "gpio-sbu-mux";

pinctrl-names = "default";
pinctrl-0 = <&pinctrl_typec_mux>;
select-gpios = <&gpio4 20 GPIO_ACTIVE_LOW>; // (PAD_)SAI1_MCLK -> USB1_SS_SEL
enable-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>; // (PAD_)SD2_WP -> USB1_TYPEC_EN_B -> TYPEC_EN_B

//mode-switch;
orientation-switch;

port {
usb3_data_ss: endpoint {
remote-endpoint = <&typec_con_ss>;
};
};
};
};

Hope it might help.

Regards,
Andreas Henriksson

2023-03-27 11:57:10

by Jun Li

[permalink] [raw]
Subject: RE: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support



> -----Original Message-----
> From: Andreas Henriksson <[email protected]>
> Sent: Monday, March 27, 2023 4:50 PM
> To: Jun Li <[email protected]>; Marco Felsch <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; Xu Yang <[email protected]>
> Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support
>
> On Fri, Mar 24, 2023 at 10:18:17AM +0000, Jun Li wrote:
> >
> >
> > > -----Original Message-----
> > > From: Marco Felsch <[email protected]>
> > > Sent: Thursday, March 23, 2023 6:58 PM
> > > To: [email protected]; [email protected];
> > > [email protected]; dl-linux-imx <[email protected]>;
> > > [email protected]
> > > Cc: [email protected];
> > > [email protected]; [email protected]
> > > Subject: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1
> > > support
> > >
> > > The i.MX8MP-EVK has a dual-role usb-type-c port marked as PORT1. By
> > > this commit the dual-role support is added which allows the
> > > user-space to assign usb-gadget functions to it via the configFS.
> >
> > So just ignore the orientation switch will make this port cannot work
> > at super speed, this is actually why this port is not enabled at upstream.
> > I see the orientation switch via GPIO for SBU is already merged:
> > drivers/usb/typec/mux/gpio-sbu-mux.c
> > Do you have interest to expand this driver to support super speed
> > switch for this case?
> [...]
>
> FWIW This is what I ended up with (after backporting the gpio-sbu-mux patches)
> a little while ago trying to get the usb-c ports going on imx8mp-evk. I've
> not yet had the time to fully test this (only done host/device, not tested:
> SS, orientation, etc), so beware that it might be completely wrong.

Thanks for the advice.

*reuse* compatible = "gpio-sbu-mux"; can make the basic *function* work,
but that's not the right direction, SBU has its own signal in typec connector,
here what we need is the Super Speed signal switch, you can see iMX8MP EVK
use 2 GPIOs control the SS for 3 states(normal orientation, reserve orientation,
places all channels in high impedance state), but SBU will disable both channels
at TYPEC_STATE_USB, this is not correct for USB data, so logically we cannot
reuse SBU either. But I think this gpio-sbu-mux.c driver can be extended to
add support super speed signal orientation.

Li Jun

>
> #include "dt-bindings/usb/pd.h"
>
> &usb3_phy0 {
> vbus-power-supply = <&ptn5110>;
> status = "okay";
> };
>
> &usb3_0 {
> status = "okay";
> };
>
> &usb_dwc3_0 {
> dr_mode = "otg";
> hnp-disable;
> srp-disable;
> adp-disable;
> usb-role-switch;
> role-switch-default-mode = "peripheral";
> snps,dis-u1-entry-quirk;
> snps,dis-u2-entry-quirk;
> status = "okay";
>
> port {
> usb3_drd_sw: endpoint {
> remote-endpoint = <&typec_dr_sw>;
> };
> };
> };
>
> &i2c2 {
> clock-frequency = <100000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_i2c2>;
> status = "okay";
>
> ptn5110: tcpc@50 {
> compatible = "nxp,ptn5110";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_typec>;
> reg = <0x50>;
>
> interrupt-parent = <&gpio4>;
> interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
> status = "okay";
>
> port {
> typec_dr_sw: endpoint {
> remote-endpoint = <&usb3_drd_sw>;
> };
> };
>
> usb_con: connector {
> compatible = "usb-c-connector";
> label = "USB-C";
> power-role = "dual";
> data-role = "dual";
> try-power-role = "sink";
> source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> PDO_VAR(5000, 20000, 3000)>;
> op-sink-microwatt = <15000000>;
> self-powered;
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@1 {
> reg = <1>;
> typec_con_ss: endpoint {
> remote-endpoint = <&usb3_data_ss>;
> };
> };
> };
> };
> };
>
> };
>
> &iomuxc {
> pinctrl_typec: typec1grp {
> fsl,pins = <
> MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19 0x1c4
> >;
> };
>
> pinctrl_typec_mux: typec1muxgrp {
> fsl,pins = <
> MX8MP_IOMUXC_SAI1_MCLK__GPIO4_IO20 0x16
> MX8MP_IOMUXC_SD2_WP__GPIO2_IO20 0x16
> >;
> };
>
>
> pinctrl_i2c2: i2c2grp {
> fsl,pins = <
> MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL 0x400001c2
> MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA 0x400001c2
> >;
> };
> };
>
> / {
> gpio-sbu-mux {
> compatible = "gpio-sbu-mux";
>
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_typec_mux>;
> select-gpios = <&gpio4 20 GPIO_ACTIVE_LOW>; // (PAD_)SAI1_MCLK ->
> USB1_SS_SEL
> enable-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>; // (PAD_)SD2_WP ->
> USB1_TYPEC_EN_B -> TYPEC_EN_B
>
> //mode-switch;
> orientation-switch;
>
> port {
> usb3_data_ss: endpoint {
> remote-endpoint = <&typec_con_ss>;
> };
> };
> };
> };
>
> Hope it might help.
>
> Regards,
> Andreas Henriksson

2023-03-30 14:47:01

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support

Hi,

On 23-03-27, Andreas Henriksson wrote:
> On Fri, Mar 24, 2023 at 10:18:17AM +0000, Jun Li wrote:
> >
> >
> > > -----Original Message-----
> > > From: Marco Felsch <[email protected]>
> > > Sent: Thursday, March 23, 2023 6:58 PM
> > > To: [email protected]; [email protected];
> > > [email protected]; dl-linux-imx <[email protected]>; [email protected]
> > > Cc: [email protected]; [email protected];
> > > [email protected]
> > > Subject: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support
> > >
> > > The i.MX8MP-EVK has a dual-role usb-type-c port marked as PORT1. By this
> > > commit the dual-role support is added which allows the user-space to assign
> > > usb-gadget functions to it via the configFS.
> >
> > So just ignore the orientation switch will make this port cannot work
> > at super speed, this is actually why this port is not enabled at upstream.
> > I see the orientation switch via GPIO for SBU is already merged:
> > drivers/usb/typec/mux/gpio-sbu-mux.c
> > Do you have interest to expand this driver to support super speed
> > switch for this case?
> [...]
>
> FWIW This is what I ended up with (after backporting the gpio-sbu-mux patches)
> a little while ago trying to get the usb-c ports going on imx8mp-evk. I've not
> yet had the time to fully test this (only done host/device, not tested: SS,
> orientation, etc), so beware that it might be completely wrong.
>
> #include "dt-bindings/usb/pd.h"
>
> &usb3_phy0 {
> vbus-power-supply = <&ptn5110>;
> status = "okay";
> };
>
> &usb3_0 {
> status = "okay";
> };
>
> &usb_dwc3_0 {
> dr_mode = "otg";
> hnp-disable;
> srp-disable;
> adp-disable;
> usb-role-switch;
> role-switch-default-mode = "peripheral";
> snps,dis-u1-entry-quirk;
> snps,dis-u2-entry-quirk;
> status = "okay";
>
> port {
> usb3_drd_sw: endpoint {
> remote-endpoint = <&typec_dr_sw>;
> };
> };
> };
>
> &i2c2 {
> clock-frequency = <100000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_i2c2>;
> status = "okay";
>
> ptn5110: tcpc@50 {
> compatible = "nxp,ptn5110";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_typec>;
> reg = <0x50>;
>
> interrupt-parent = <&gpio4>;
> interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
> status = "okay";
>
> port {
> typec_dr_sw: endpoint {
> remote-endpoint = <&usb3_drd_sw>;
> };
> };
>
> usb_con: connector {
> compatible = "usb-c-connector";
> label = "USB-C";
> power-role = "dual";
> data-role = "dual";
> try-power-role = "sink";
> source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> PDO_VAR(5000, 20000, 3000)>;
> op-sink-microwatt = <15000000>;
> self-powered;
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@1 {
> reg = <1>;
> typec_con_ss: endpoint {
> remote-endpoint = <&usb3_data_ss>;
> };
> };
> };
> };
> };
>
> };
>
> &iomuxc {
> pinctrl_typec: typec1grp {
> fsl,pins = <
> MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19 0x1c4
> >;
> };
>
> pinctrl_typec_mux: typec1muxgrp {
> fsl,pins = <
> MX8MP_IOMUXC_SAI1_MCLK__GPIO4_IO20 0x16
> MX8MP_IOMUXC_SD2_WP__GPIO2_IO20 0x16
> >;
> };
>
>
> pinctrl_i2c2: i2c2grp {
> fsl,pins = <
> MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL 0x400001c2
> MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA 0x400001c2
> >;
> };
> };
>
> / {
> gpio-sbu-mux {
> compatible = "gpio-sbu-mux";
>
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_typec_mux>;
> select-gpios = <&gpio4 20 GPIO_ACTIVE_LOW>; // (PAD_)SAI1_MCLK -> USB1_SS_SEL
> enable-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>; // (PAD_)SD2_WP -> USB1_TYPEC_EN_B -> TYPEC_EN_B
>
> //mode-switch;
> orientation-switch;
>
> port {
> usb3_data_ss: endpoint {
> remote-endpoint = <&typec_con_ss>;
> };
> };
> };
> };
>
> Hope it might help.

I didn't tested it but at the moment I don't see the problem with my
patch. Sure the ID pin is not connected but if I understood it correctly
(please correct me) the tcpc will handle the orientation. I can set the
mode to device from user-space which worked. I didn't verified the
SuperSpeed mode nor the host mode since I don't have a USB-C flash
drive.

Without the patch the port is just unused albeit the port is really
useful for bootloaders like barebox to provide a usbgadget/fastboot
device to flash the system.

Regards,
Marco

2023-04-03 09:41:18

by Andreas Henriksson

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support

Hello Marco Felsch,

On Thu, Mar 30, 2023 at 04:38:13PM +0200, Marco Felsch wrote:
> Hi,
>
> On 23-03-27, Andreas Henriksson wrote:
[...]
> > / {
> > gpio-sbu-mux {
> > compatible = "gpio-sbu-mux";
[...]
> I didn't tested it but at the moment I don't see the problem with my
> patch.

As Jun Li helpfully explained my patch is not correct, so don't bother.

I don't have a problem with your patch, was just trying to share
what I had done.

I've since learned that the board I'll be working on will not even have
SS, so I will not pursue SBU. I'll most likely use your patches instead
as they seem simpler than what I did and should fully meet my needs for
an usb port that works in both host and device mode.

Not that it matters, but +1 from me on applying your patches.
(If people are hesitant to do it because of lack of SS, then maybe
that could be adressed by adding a comment setting the expectations?)

> Sure the ID pin is not connected but if I understood it correctly
> (please correct me) the tcpc will handle the orientation. I can set the
> mode to device from user-space which worked. I didn't verified the
> SuperSpeed mode nor the host mode since I don't have a USB-C flash
> drive.
>
> Without the patch the port is just unused albeit the port is really
> useful for bootloaders like barebox to provide a usbgadget/fastboot
> device to flash the system.
>
> Regards,
> Marco

Regards,
Andreas Henriksson

2023-04-05 13:26:46

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support

On Mon, Apr 03, 2023 at 11:38:12AM +0200, Andreas Henriksson wrote:
> Hello Marco Felsch,
>
> On Thu, Mar 30, 2023 at 04:38:13PM +0200, Marco Felsch wrote:
> > Hi,
> >
> > On 23-03-27, Andreas Henriksson wrote:
> [...]
> > > / {
> > > gpio-sbu-mux {
> > > compatible = "gpio-sbu-mux";
> [...]
> > I didn't tested it but at the moment I don't see the problem with my
> > patch.
>
> As Jun Li helpfully explained my patch is not correct, so don't bother.
>
> I don't have a problem with your patch, was just trying to share
> what I had done.
>
> I've since learned that the board I'll be working on will not even have
> SS, so I will not pursue SBU. I'll most likely use your patches instead
> as they seem simpler than what I did and should fully meet my needs for
> an usb port that works in both host and device mode.
>
> Not that it matters, but +1 from me on applying your patches.
> (If people are hesitant to do it because of lack of SS, then maybe
> that could be adressed by adding a comment setting the expectations?)

Marco,

Could you update the patch to mention a bit about Super-Speed support
as discussed here?

Shawn

2023-05-03 15:56:17

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support

Hi Andreas, Shawn,

On 23-04-05, Shawn Guo wrote:
> On Mon, Apr 03, 2023 at 11:38:12AM +0200, Andreas Henriksson wrote:
> > Hello Marco Felsch,
> >
> > On Thu, Mar 30, 2023 at 04:38:13PM +0200, Marco Felsch wrote:
> > > Hi,
> > >
> > > On 23-03-27, Andreas Henriksson wrote:
> > [...]
> > > > / {
> > > > gpio-sbu-mux {
> > > > compatible = "gpio-sbu-mux";
> > [...]
> > > I didn't tested it but at the moment I don't see the problem with my
> > > patch.
> >
> > As Jun Li helpfully explained my patch is not correct, so don't bother.
> >
> > I don't have a problem with your patch, was just trying to share
> > what I had done.

Thanks for sharing :)

> > I've since learned that the board I'll be working on will not even have
> > SS, so I will not pursue SBU. I'll most likely use your patches instead
> > as they seem simpler than what I did and should fully meet my needs for
> > an usb port that works in both host and device mode.
> >
> > Not that it matters, but +1 from me on applying your patches.
> > (If people are hesitant to do it because of lack of SS, then maybe
> > that could be adressed by adding a comment setting the expectations?)
>
> Marco,
>
> Could you update the patch to mention a bit about Super-Speed support
> as discussed here?

Yes I will update the patch accordingly.

Regards,
Marco

2023-05-03 18:54:51

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support

Hi Li, Andreas,

On 23-03-27, Jun Li wrote:
>
> > -----Original Message-----
> > From: Andreas Henriksson <[email protected]>
> > Sent: Monday, March 27, 2023 4:50 PM
> > To: Jun Li <[email protected]>; Marco Felsch <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; dl-linux-imx <[email protected]>; [email protected];
> > [email protected]; [email protected];
> > [email protected]; Xu Yang <[email protected]>
> > Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support
> >
> > On Fri, Mar 24, 2023 at 10:18:17AM +0000, Jun Li wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Marco Felsch <[email protected]>
> > > > Sent: Thursday, March 23, 2023 6:58 PM
> > > > To: [email protected]; [email protected];
> > > > [email protected]; dl-linux-imx <[email protected]>;
> > > > [email protected]
> > > > Cc: [email protected];
> > > > [email protected]; [email protected]
> > > > Subject: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1
> > > > support
> > > >
> > > > The i.MX8MP-EVK has a dual-role usb-type-c port marked as PORT1. By
> > > > this commit the dual-role support is added which allows the
> > > > user-space to assign usb-gadget functions to it via the configFS.
> > >
> > > So just ignore the orientation switch will make this port cannot work
> > > at super speed, this is actually why this port is not enabled at upstream.
> > > I see the orientation switch via GPIO for SBU is already merged:
> > > drivers/usb/typec/mux/gpio-sbu-mux.c
> > > Do you have interest to expand this driver to support super speed
> > > switch for this case?
> > [...]
> >
> > FWIW This is what I ended up with (after backporting the gpio-sbu-mux patches)
> > a little while ago trying to get the usb-c ports going on imx8mp-evk. I've
> > not yet had the time to fully test this (only done host/device, not tested:
> > SS, orientation, etc), so beware that it might be completely wrong.
>
> Thanks for the advice.
>
> *reuse* compatible = "gpio-sbu-mux"; can make the basic *function* work,
> but that's not the right direction, SBU has its own signal in typec connector,
> here what we need is the Super Speed signal switch, you can see iMX8MP EVK
> use 2 GPIOs control the SS for 3 states(normal orientation, reserve orientation,
> places all channels in high impedance state), but SBU will disable both channels
> at TYPEC_STATE_USB, this is not correct for USB data, so logically we cannot
> reuse SBU either. But I think this gpio-sbu-mux.c driver can be extended to
> add support super speed signal orientation.

Thanks for the useful input :) I was dug into the usb-c hole and now I'm
back. The "gpio-sbu-mux" should fit perfectly for our use-case, we only
have to tell the driver to act as 'orientation-switch' only. All pieces
are in place so just dts work to do. I will test my new patch and send a
new version which should support super-speed to (fingers crossed).

Regards,
Marco


>
> Li Jun
>
> >
> > #include "dt-bindings/usb/pd.h"
> >
> > &usb3_phy0 {
> > vbus-power-supply = <&ptn5110>;
> > status = "okay";
> > };
> >
> > &usb3_0 {
> > status = "okay";
> > };
> >
> > &usb_dwc3_0 {
> > dr_mode = "otg";
> > hnp-disable;
> > srp-disable;
> > adp-disable;
> > usb-role-switch;
> > role-switch-default-mode = "peripheral";
> > snps,dis-u1-entry-quirk;
> > snps,dis-u2-entry-quirk;
> > status = "okay";
> >
> > port {
> > usb3_drd_sw: endpoint {
> > remote-endpoint = <&typec_dr_sw>;
> > };
> > };
> > };
> >
> > &i2c2 {
> > clock-frequency = <100000>;
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_i2c2>;
> > status = "okay";
> >
> > ptn5110: tcpc@50 {
> > compatible = "nxp,ptn5110";
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_typec>;
> > reg = <0x50>;
> >
> > interrupt-parent = <&gpio4>;
> > interrupts = <19 IRQ_TYPE_LEVEL_LOW>;
> > status = "okay";
> >
> > port {
> > typec_dr_sw: endpoint {
> > remote-endpoint = <&usb3_drd_sw>;
> > };
> > };
> >
> > usb_con: connector {
> > compatible = "usb-c-connector";
> > label = "USB-C";
> > power-role = "dual";
> > data-role = "dual";
> > try-power-role = "sink";
> > source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> > sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> > PDO_VAR(5000, 20000, 3000)>;
> > op-sink-microwatt = <15000000>;
> > self-powered;
> >
> > ports {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > port@1 {
> > reg = <1>;
> > typec_con_ss: endpoint {
> > remote-endpoint = <&usb3_data_ss>;
> > };
> > };
> > };
> > };
> > };
> >
> > };
> >
> > &iomuxc {
> > pinctrl_typec: typec1grp {
> > fsl,pins = <
> > MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19 0x1c4
> > >;
> > };
> >
> > pinctrl_typec_mux: typec1muxgrp {
> > fsl,pins = <
> > MX8MP_IOMUXC_SAI1_MCLK__GPIO4_IO20 0x16
> > MX8MP_IOMUXC_SD2_WP__GPIO2_IO20 0x16
> > >;
> > };
> >
> >
> > pinctrl_i2c2: i2c2grp {
> > fsl,pins = <
> > MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL 0x400001c2
> > MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA 0x400001c2
> > >;
> > };
> > };
> >
> > / {
> > gpio-sbu-mux {
> > compatible = "gpio-sbu-mux";
> >
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_typec_mux>;
> > select-gpios = <&gpio4 20 GPIO_ACTIVE_LOW>; // (PAD_)SAI1_MCLK ->
> > USB1_SS_SEL
> > enable-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>; // (PAD_)SD2_WP ->
> > USB1_TYPEC_EN_B -> TYPEC_EN_B
> >
> > //mode-switch;
> > orientation-switch;
> >
> > port {
> > usb3_data_ss: endpoint {
> > remote-endpoint = <&typec_con_ss>;
> > };
> > };
> > };
> > };
> >
> > Hope it might help.
> >
> > Regards,
> > Andreas Henriksson
>

2023-05-03 19:28:48

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp-evk: add dual-role usb port1 support

On 23-05-03, Marco Felsch wrote:
> Hi Li, Andreas,

...

> > Thanks for the advice.
> >
> > *reuse* compatible = "gpio-sbu-mux"; can make the basic *function* work,
> > but that's not the right direction, SBU has its own signal in typec connector,
> > here what we need is the Super Speed signal switch, you can see iMX8MP EVK
> > use 2 GPIOs control the SS for 3 states(normal orientation, reserve orientation,
> > places all channels in high impedance state), but SBU will disable both channels
> > at TYPEC_STATE_USB, this is not correct for USB data, so logically we cannot
> > reuse SBU either. But I think this gpio-sbu-mux.c driver can be extended to
> > add support super speed signal orientation.
>
> Thanks for the useful input :) I was dug into the usb-c hole and now I'm
> back. The "gpio-sbu-mux" should fit perfectly for our use-case, we only
> have to tell the driver to act as 'orientation-switch' only. All pieces
> are in place so just dts work to do. I will test my new patch and send a
> new version which should support super-speed to (fingers crossed).

Now I see problem you mentioned, let's see if we can extent the driver.
Sorry for the noise.

Regards,
Marco