Hi all,
this adds the usb gadget support to the i.MX8MP-EVK. This Series is
based on [1] and therefore it is already a v2. Thanks to Li and Andreas
for the very useful feedback.
Patch1-3: Add the mssing support for USB-SS GPIO muxes. This is required
to have proper USB-SS support on the EVK.
Patch4: Adds the devicetree integration.
[1] https://lore.kernel.org/all/[email protected]/
Regards,
Marco
---
Marco Felsch (4):
dt-bindings: usb: gpio-sbu-mux: add support for ss-data lanes mux
usb: typec: mux: gpio-sbu-mux: add support for ss data lane muxing
usb: typec: tcpci: clear the fault status bit
arm64: dts: imx8mp-evk: add dual-role usb port1 support
.../devicetree/bindings/usb/gpio-sbu-mux.yaml | 82 +++++++++++++++++---
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 88 ++++++++++++++++++++++
drivers/usb/typec/mux/Kconfig | 5 +-
drivers/usb/typec/mux/gpio-sbu-mux.c | 18 ++++-
drivers/usb/typec/tcpm/tcpci.c | 5 ++
include/linux/usb/tcpci.h | 1 +
6 files changed, 185 insertions(+), 14 deletions(-)
---
base-commit: 457391b0380335d5e9a5babdec90ac53928b23b4
change-id: 20230504-b4-v6-3-topic-boards-imx8mp-evk-dual-role-usb-8dcf6274d9df
Best regards,
--
Marco Felsch <[email protected]>
According the "USB Type-C Port Controller Interface Specification v2.0"
the TCPC sets the fault status register bit-7
(AllRegistersResetToDefault) once the registers have been reseted to
their default values.
This triggers an alert(-irq) on PTN5110 devices albeit we do mask the
fault-irq. Fix this gernally by writing a one to the correspondig
bit-7.
Signed-off-by: Marco Felsch <[email protected]>
---
drivers/usb/typec/tcpm/tcpci.c | 5 +++++
include/linux/usb/tcpci.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 8da23240afbe..15632d023e4c 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -602,6 +602,11 @@ static int tcpci_init(struct tcpc_dev *tcpc)
if (time_after(jiffies, timeout))
return -ETIMEDOUT;
+ regmap_read(tcpci->regmap, TCPC_FAULT_STATUS, ®);
+ if (reg & TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT)
+ tcpci_write16(tcpci, TCPC_FAULT_STATUS,
+ TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT);
+
/* Handle vendor init */
if (tcpci->data->init) {
ret = tcpci->data->init(tcpci, tcpci->data);
diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
index 85e95a3251d3..83376473ac76 100644
--- a/include/linux/usb/tcpci.h
+++ b/include/linux/usb/tcpci.h
@@ -103,6 +103,7 @@
#define TCPC_POWER_STATUS_SINKING_VBUS BIT(0)
#define TCPC_FAULT_STATUS 0x1f
+#define TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT BIT(7)
#define TCPC_ALERT_EXTENDED 0x21
--
2.39.2
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.
Below is the test results of a basic usbgadget-ethernet test using by
iperf3.
Accepted connection from 192.168.60.1, port 48172
[ 5] local 192.168.60.2 port 5201 connected to 192.168.60.1 port 48176
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 101 MBytes 850 Mbits/sec
[ 5] 1.00-2.00 sec 105 MBytes 883 Mbits/sec
[ 5] 2.00-3.00 sec 105 MBytes 885 Mbits/sec
...
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate
[ 5] 0.00-100.04 sec 10.3 GBytes 882 Mbits/sec receiver
Accepted connection from 192.168.60.1, port 42104
[ 5] local 192.168.60.2 port 5201 connected to 192.168.60.1 port 42110
[ ID] Interval Transfer Bitrate Retr Cwnd
[ 5] 0.00-1.00 sec 110 MBytes 925 Mbits/sec 0 540 KBytes
[ 5] 1.00-2.00 sec 112 MBytes 938 Mbits/sec 0 566 KBytes
[ 5] 2.00-3.00 sec 115 MBytes 969 Mbits/sec 0 566 KBytes
...
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-37.00 sec 4.27 GBytes 990 Mbits/sec 0 sender
Signed-off-by: Marco Felsch <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 88 ++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index 7816853162b3..20c3c41d6209 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"
/ {
@@ -83,6 +85,22 @@ reg_usdhc2_vmmc: regulator-usdhc2 {
gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
enable-active-high;
};
+
+ usb-ss-mux {
+ compatible = "gpio-ss-mux";
+ enable-gpios = <&gpio2 20 GPIO_ACTIVE_LOW>;
+ select-gpios = <&gpio4 20 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb_ss_mux>;
+
+ orientation-switch;
+
+ port {
+ usb_ss_mux: endpoint {
+ remote-endpoint = <&usb_con_ss_mux>;
+ };
+ };
+ };
};
&flexspi {
@@ -336,6 +354,40 @@ &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_mux: endpoint {
+ remote-endpoint = <&usb_ss_mux>;
+ };
+ };
+ };
+
+ port {
+ usb_con_ss: endpoint {
+ remote-endpoint = <&usb_dwc3_0_drd>;
+ };
+ };
+ };
};
&i2c3 {
@@ -442,14 +494,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>;
@@ -659,6 +734,12 @@ MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19 0x40
>;
};
+ pinctrl_tcpc: tcpcgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19 0x1e0
+ >;
+ };
+
pinctrl_uart1: uart1grp {
fsl,pins = <
MX8MP_IOMUXC_UART1_RXD__UART1_DCE_RX 0x140
@@ -690,6 +771,13 @@ MX8MP_IOMUXC_ECSPI1_MISO__UART3_DCE_CTS 0x140
>;
};
+ pinctrl_usb_ss_mux: usbssmuxgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SD2_WP__GPIO2_IO20 0x104
+ MX8MP_IOMUXC_SAI1_MCLK__GPIO4_IO20 0x144
+ >;
+ };
+
pinctrl_usdhc2: usdhc2grp {
fsl,pins = <
MX8MP_IOMUXC_SD2_CLK__USDHC2_CLK 0x190
--
2.39.2
On 5/4/23 06:46, Marco Felsch wrote:
> According the "USB Type-C Port Controller Interface Specification v2.0"
> the TCPC sets the fault status register bit-7
> (AllRegistersResetToDefault) once the registers have been reseted to
cleared ? set ?
> their default values.
>
> This triggers an alert(-irq) on PTN5110 devices albeit we do mask the
> fault-irq. Fix this gernally by writing a one to the correspondig
generically ?
corresponding
> bit-7.
>
> Signed-off-by: Marco Felsch <[email protected]>
> ---
> drivers/usb/typec/tcpm/tcpci.c | 5 +++++
> include/linux/usb/tcpci.h | 1 +
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 8da23240afbe..15632d023e4c 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -602,6 +602,11 @@ static int tcpci_init(struct tcpc_dev *tcpc)
> if (time_after(jiffies, timeout))
> return -ETIMEDOUT;
>
> + regmap_read(tcpci->regmap, TCPC_FAULT_STATUS, ®);
Needs error check.
Also, I am not sure if this is the correct place for this code. The alert
status is cleared after vendor initialization. Should the same be done
here ? Also, why not just write the bit unconditionally, similar
to TCPC_ALERT ?
Thanks,
Guenter
> + if (reg & TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT)
> + tcpci_write16(tcpci, TCPC_FAULT_STATUS,
> + TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT);
> +
> /* Handle vendor init */
> if (tcpci->data->init) {
> ret = tcpci->data->init(tcpci, tcpci->data);
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index 85e95a3251d3..83376473ac76 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -103,6 +103,7 @@
> #define TCPC_POWER_STATUS_SINKING_VBUS BIT(0)
>
> #define TCPC_FAULT_STATUS 0x1f
> +#define TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT BIT(7)
>
> #define TCPC_ALERT_EXTENDED 0x21
>
>
On 23-05-04, Guenter Roeck wrote:
> On 5/4/23 06:46, Marco Felsch wrote:
> > According the "USB Type-C Port Controller Interface Specification v2.0"
> > the TCPC sets the fault status register bit-7
> > (AllRegistersResetToDefault) once the registers have been reseted to
>
> cleared ? set ?
Sry. I don't get this.
> > their default values.
> >
> > This triggers an alert(-irq) on PTN5110 devices albeit we do mask the
> > fault-irq. Fix this gernally by writing a one to the correspondig
>
> generically ?
Sure, thanks.
> corresponding
Of course!
> > bit-7.
> >
> > Signed-off-by: Marco Felsch <[email protected]>
> > ---
> > drivers/usb/typec/tcpm/tcpci.c | 5 +++++
> > include/linux/usb/tcpci.h | 1 +
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > index 8da23240afbe..15632d023e4c 100644
> > --- a/drivers/usb/typec/tcpm/tcpci.c
> > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > @@ -602,6 +602,11 @@ static int tcpci_init(struct tcpc_dev *tcpc)
> > if (time_after(jiffies, timeout))
> > return -ETIMEDOUT;
> > + regmap_read(tcpci->regmap, TCPC_FAULT_STATUS, ®);
>
> Needs error check.
I will add this.
> Also, I am not sure if this is the correct place for this code. The alert
> status is cleared after vendor initialization. Should the same be done
> here ?
According the spec the bit must be cleared before the TCPC_ALERT is
cleared. Of course the vendor-init can (re-)trigger the bit, therefore
we should move behind the vendor init and right before the TCPC_ALERT
clear.
> Also, why not just write the bit unconditionally, similar
> to TCPC_ALERT ?
Thought about this too.. I will change it in the v3.
Thanks for the feedback,
Marco
>
> Thanks,
> Guenter
>
> > + if (reg & TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT)
> > + tcpci_write16(tcpci, TCPC_FAULT_STATUS,
> > + TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT);
> > +
> > /* Handle vendor init */
> > if (tcpci->data->init) {
> > ret = tcpci->data->init(tcpci, tcpci->data);
> > diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> > index 85e95a3251d3..83376473ac76 100644
> > --- a/include/linux/usb/tcpci.h
> > +++ b/include/linux/usb/tcpci.h
> > @@ -103,6 +103,7 @@
> > #define TCPC_POWER_STATUS_SINKING_VBUS BIT(0)
> > #define TCPC_FAULT_STATUS 0x1f
> > +#define TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT BIT(7)
> > #define TCPC_ALERT_EXTENDED 0x21
> >
>
>
On 5/4/23 07:27, Marco Felsch wrote:
> On 23-05-04, Guenter Roeck wrote:
>> On 5/4/23 06:46, Marco Felsch wrote:
>>> According the "USB Type-C Port Controller Interface Specification v2.0"
>>> the TCPC sets the fault status register bit-7
>>> (AllRegistersResetToDefault) once the registers have been reseted to
>>
>> cleared ? set ?
>
> Sry. I don't get this.
>
instead of "reseted" which isn't really a word.
>>> their default values.
>>>
>>> This triggers an alert(-irq) on PTN5110 devices albeit we do mask the
>>> fault-irq. Fix this gernally by writing a one to the correspondig
>>
>> generically ?
>
> Sure, thanks.
>
>> corresponding
>
> Of course!
>
>>> bit-7.
>>>
>>> Signed-off-by: Marco Felsch <[email protected]>
>>> ---
>>> drivers/usb/typec/tcpm/tcpci.c | 5 +++++
>>> include/linux/usb/tcpci.h | 1 +
>>> 2 files changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
>>> index 8da23240afbe..15632d023e4c 100644
>>> --- a/drivers/usb/typec/tcpm/tcpci.c
>>> +++ b/drivers/usb/typec/tcpm/tcpci.c
>>> @@ -602,6 +602,11 @@ static int tcpci_init(struct tcpc_dev *tcpc)
>>> if (time_after(jiffies, timeout))
>>> return -ETIMEDOUT;
>>> + regmap_read(tcpci->regmap, TCPC_FAULT_STATUS, ®);
>>
>> Needs error check.
>
> I will add this.
>
>> Also, I am not sure if this is the correct place for this code. The alert
>> status is cleared after vendor initialization. Should the same be done
>> here ?
>
> According the spec the bit must be cleared before the TCPC_ALERT is
> cleared. Of course the vendor-init can (re-)trigger the bit, therefore
> we should move behind the vendor init and right before the TCPC_ALERT
> clear.
>
>> Also, why not just write the bit unconditionally, similar
>> to TCPC_ALERT ?
>
> Thought about this too.. I will change it in the v3.
>
> Thanks for the feedback,
> Marco
>
>>
>> Thanks,
>> Guenter
>>
>>> + if (reg & TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT)
>>> + tcpci_write16(tcpci, TCPC_FAULT_STATUS,
>>> + TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT);
>>> +
>>> /* Handle vendor init */
>>> if (tcpci->data->init) {
>>> ret = tcpci->data->init(tcpci, tcpci->data);
>>> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
>>> index 85e95a3251d3..83376473ac76 100644
>>> --- a/include/linux/usb/tcpci.h
>>> +++ b/include/linux/usb/tcpci.h
>>> @@ -103,6 +103,7 @@
>>> #define TCPC_POWER_STATUS_SINKING_VBUS BIT(0)
>>> #define TCPC_FAULT_STATUS 0x1f
>>> +#define TCPC_FAULT_STATUS_ALL_REG_RST_TO_DEFAULT BIT(7)
>>> #define TCPC_ALERT_EXTENDED 0x21
>>>
>>
>>
On 23-05-04, Guenter Roeck wrote:
> On 5/4/23 07:27, Marco Felsch wrote:
> > On 23-05-04, Guenter Roeck wrote:
> > > On 5/4/23 06:46, Marco Felsch wrote:
> > > > According the "USB Type-C Port Controller Interface Specification v2.0"
> > > > the TCPC sets the fault status register bit-7
> > > > (AllRegistersResetToDefault) once the registers have been reseted to
> > >
> > > cleared ? set ?
> >
> > Sry. I don't get this.
> >
>
> instead of "reseted" which isn't really a word.
Sure, thanks.
Regards,
Marco
On Thu, May 04, 2023 at 03:46:49PM +0200, Marco Felsch wrote:
> Hi all,
>
> this adds the usb gadget support to the i.MX8MP-EVK. This Series is
> based on [1] and therefore it is already a v2. Thanks to Li and Andreas
> for the very useful feedback.
>
> Patch1-3: Add the mssing support for USB-SS GPIO muxes. This is required
> to have proper USB-SS support on the EVK.
>
> Patch4: Adds the devicetree integration.
Please send the DTS change separately afterwards, as we do not want
Greg's tool to pick it up into USB tree.
Shawn
>
> [1] https://lore.kernel.org/all/[email protected]/
>
> Regards,
> Marco
>
> ---
> Marco Felsch (4):
> dt-bindings: usb: gpio-sbu-mux: add support for ss-data lanes mux
> usb: typec: mux: gpio-sbu-mux: add support for ss data lane muxing
> usb: typec: tcpci: clear the fault status bit
> arm64: dts: imx8mp-evk: add dual-role usb port1 support
>
> .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 82 +++++++++++++++++---
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 88 ++++++++++++++++++++++
> drivers/usb/typec/mux/Kconfig | 5 +-
> drivers/usb/typec/mux/gpio-sbu-mux.c | 18 ++++-
> drivers/usb/typec/tcpm/tcpci.c | 5 ++
> include/linux/usb/tcpci.h | 1 +
> 6 files changed, 185 insertions(+), 14 deletions(-)
> ---
> base-commit: 457391b0380335d5e9a5babdec90ac53928b23b4
> change-id: 20230504-b4-v6-3-topic-boards-imx8mp-evk-dual-role-usb-8dcf6274d9df
>
> Best regards,
> --
> Marco Felsch <[email protected]>
>
On Sun, May 14, 2023 at 09:21:22PM +0800, Shawn Guo wrote:
> On Thu, May 04, 2023 at 03:46:49PM +0200, Marco Felsch wrote:
> > Hi all,
> >
> > this adds the usb gadget support to the i.MX8MP-EVK. This Series is
> > based on [1] and therefore it is already a v2. Thanks to Li and Andreas
> > for the very useful feedback.
> >
> > Patch1-3: Add the mssing support for USB-SS GPIO muxes. This is required
> > to have proper USB-SS support on the EVK.
> >
> > Patch4: Adds the devicetree integration.
>
> Please send the DTS change separately afterwards, as we do not want
> Greg's tool to pick it up into USB tree.
"Greg's tool" is the standard `b4` tool that lots of kernel maintainers
are now using. So this isn't some magic thing that is unique to me...
thanks,
greg k-h
On 23-08-16, Fabio Estevam wrote:
> Hi Marco,
>
> On Thu, May 4, 2023 at 11:27 AM Marco Felsch <[email protected]> wrote:
>
> > > Also, why not just write the bit unconditionally, similar
> > > to TCPC_ALERT ?
> >
> > Thought about this too.. I will change it in the v3.
>
> Was there ever a v3 for this patch?
Nope, thanks for sending it separate :)
Regards,
Marco