[PATCH v3 0/4] i.MX8MP: more USB3 glue layer feature support

[PATCH v3 0/4] i.MX8MP: more USB3 glue layer feature support

[Alexander Stein]

Thanks everyone for the rfeedback on v2 [1].

This patchset aims to support flags for e.g. over-current active low or port
permanantly attached which are provided in the USB3 glue layer.

In v3 the implementation is moved from USB PHY driver to USB DWC3 glue for
i.MX8MP. The confusing part is that the existing dwc3-imx8mp.c driver references
glue everywhere but is actually using HSIO BLK_CTL, which is a different
hardware block.
The implementation is still backward compatible and be used with device trees
where neither PHY clock nor (new) glue layer base address is provided.

Changes in v3:
* Rename existing member for clarity
* Moved feature implementation from phy-fsl-imx8mq-usb.c to dwc3-imx8mp.c

[1] https://lore.kernel.org/linux-arm-kernel/2245006.irdbgypaU6@steina-w/T/

Alexander Stein (4):
  usb: dwc3: imx8mp: rename iomem base pointer
  dt-bindings: usb: dwc3-imx8mp: Add imx8mp specific flags
  usb: dwc3: imx8mp: Add support for setting SOC specific flags
  arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3 nodes

 .../bindings/usb/fsl,imx8mp-dwc3.yaml         | 38 ++++++--
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     | 20 ++--
 drivers/usb/dwc3/dwc3-imx8mp.c                | 95 +++++++++++++++++--
 3 files changed, 134 insertions(+), 19 deletions(-)

-- 
2.25.1

[PATCH v3 1/4] usb: dwc3: imx8mp: rename iomem base pointer

[Alexander Stein]

Until now the iomem used is not USB glue as the name suggests, but
HSIO BLK_CTL. Rename the struct member accordingly. This is a preparing
patch for when USB glue is actually used.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/usb/dwc3/dwc3-imx8mp.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
index d328d20abfbc..1c8fe657b3a9 100644
--- a/drivers/usb/dwc3/dwc3-imx8mp.c
+++ b/drivers/usb/dwc3/dwc3-imx8mp.c
@@ -39,7 +39,7 @@
 struct dwc3_imx8mp {
 	struct device			*dev;
 	struct platform_device		*dwc3;
-	void __iomem			*glue_base;
+	void __iomem			*hsio_blk_base;
 	struct clk			*hsio_clk;
 	struct clk			*suspend_clk;
 	int				irq;
@@ -55,7 +55,7 @@ static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx)
 	if (!dwc3)
 		return;
 
-	val = readl(dwc3_imx->glue_base + USB_WAKEUP_CTRL);
+	val = readl(dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL);
 
 	if ((dwc3->current_dr_role == DWC3_GCTL_PRTCAP_HOST) && dwc3->xhci)
 		val |= USB_WAKEUP_EN | USB_WAKEUP_SS_CONN |
@@ -64,16 +64,16 @@ static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx)
 		val |= USB_WAKEUP_EN | USB_WAKEUP_VBUS_EN |
 		       USB_WAKEUP_VBUS_SRC_SESS_VAL;
 
-	writel(val, dwc3_imx->glue_base + USB_WAKEUP_CTRL);
+	writel(val, dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL);
 }
 
 static void dwc3_imx8mp_wakeup_disable(struct dwc3_imx8mp *dwc3_imx)
 {
 	u32 val;
 
-	val = readl(dwc3_imx->glue_base + USB_WAKEUP_CTRL);
+	val = readl(dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL);
 	val &= ~(USB_WAKEUP_EN | USB_WAKEUP_EN_MASK);
-	writel(val, dwc3_imx->glue_base + USB_WAKEUP_CTRL);
+	writel(val, dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL);
 }
 
 static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
@@ -115,9 +115,9 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 
 	dwc3_imx->dev = dev;
 
-	dwc3_imx->glue_base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(dwc3_imx->glue_base))
-		return PTR_ERR(dwc3_imx->glue_base);
+	dwc3_imx->hsio_blk_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(dwc3_imx->hsio_blk_base))
+		return PTR_ERR(dwc3_imx->hsio_blk_base);
 
 	dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio");
 	if (IS_ERR(dwc3_imx->hsio_clk)) {
-- 
2.25.1

[PATCH v3 2/4] dt-bindings: usb: dwc3-imx8mp: Add imx8mp specific flags

[Alexander Stein]

This adds bindings for features in the USB glue block. They allow
setting polarity of PWR and OC as well as disabling port power control.
Also permanently attached can be annotated as well.
Additional IO address and clock are needed.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 .../bindings/usb/fsl,imx8mp-dwc3.yaml         | 38 ++++++++++++++++---
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/fsl,imx8mp-dwc3.yaml b/Documentation/devicetree/bindings/usb/fsl,imx8mp-dwc3.yaml
index 974032b1fda0..ff48b4e8427d 100644
--- a/Documentation/devicetree/bindings/usb/fsl,imx8mp-dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/fsl,imx8mp-dwc3.yaml
@@ -15,9 +15,9 @@ properties:
     const: fsl,imx8mp-dwc3
 
   reg:
-    maxItems: 1
-    description: Address and length of the register set for the wrapper of
-      dwc3 core on the SOC.
+    items:
+      - description: Address and length of the register set for HSIO Block Control
+      - description: Address and length of the register set for the wrapper of dwc3 core on the SOC.
 
   "#address-cells":
     enum: [ 1, 2 ]
@@ -43,11 +43,35 @@ properties:
     items:
       - description: system hsio root clock.
       - description: suspend clock, used for usb wakeup logic.
+      - description: clock for the USB glue block
 
   clock-names:
     items:
       - const: hsio
       - const: suspend
+      - const: phy
+
+  fsl,permanently-attached:
+    type: boolean
+    description:
+      Indicates if the device atached to a downstream port is
+      permanently attached.
+
+  fsl,disable-port-power-control:
+    type: boolean
+    description:
+      Indicates whether the host controller implementation includes port
+      power control. Defines Bit 3 in capability register (HCCPARAMS).
+
+  fsl,over-current-active-low:
+    type: boolean
+    description:
+      Over current signal polarity is active low.
+
+  fsl,power-active-low:
+    type: boolean
+    description:
+      Power pad (PWR) polarity is active low.
 
 # Required child node:
 
@@ -74,10 +98,12 @@ examples:
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     usb3_0: usb@32f10100 {
       compatible = "fsl,imx8mp-dwc3";
-      reg = <0x32f10100 0x8>;
+      reg = <0x32f10100 0x8>,
+            <0x381f0000 0x20>;
       clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
-               <&clk IMX8MP_CLK_USB_ROOT>;
-      clock-names = "hsio", "suspend";
+               <&clk IMX8MP_CLK_USB_ROOT>,
+               <&clk IMX8MP_CLK_USB_PHY_ROOT>;
+      clock-names = "hsio", "suspend", "phy";
       interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
       #address-cells = <1>;
       #size-cells = <1>;
-- 
2.25.1

[PATCH v3 3/4] usb: dwc3: imx8mp: Add support for setting SOC specific flags

[Alexander Stein]

The i.MX8MP glue layer has support for the following flags:
* over-current polarity
* PWR pad polarity
* controlling PPC flag in HCCPARAMS register
* permanent port attach for usb2 & usb3 port

Allow setting these flags by supporting specific flags in the glue node.
In order to get this to work an additional IORESOURCE_MEM and clock is
necessary. For backward compatibility this is purely optional.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/usb/dwc3/dwc3-imx8mp.c | 81 ++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
index 1c8fe657b3a9..3df4313b3740 100644
--- a/drivers/usb/dwc3/dwc3-imx8mp.c
+++ b/drivers/usb/dwc3/dwc3-imx8mp.c
@@ -36,17 +36,66 @@
 
 #define USB_WAKEUP_EN_MASK		GENMASK(5, 0)
 
+/* USB glue registers */
+#define USB_CTRL0		0x00
+#define USB_CTRL1		0x04
+
+#define USB_CTRL0_PORTPWR_EN	BIT(12) /* 1 - PPC enabled (default) */
+#define USB_CTRL0_USB3_FIXED	BIT(22) /* 1 - USB3 permanent attached */
+#define USB_CTRL0_USB2_FIXED	BIT(23) /* 1 - USB2 permanent attached */
+
+#define USB_CTRL1_OC_POLARITY	BIT(16) /* 0 - HIGH / 1 - LOW */
+#define USB_CTRL1_PWR_POLARITY	BIT(17) /* 0 - HIGH / 1 - LOW */
+
 struct dwc3_imx8mp {
 	struct device			*dev;
 	struct platform_device		*dwc3;
 	void __iomem			*hsio_blk_base;
+	void __iomem			*glue_base;
 	struct clk			*hsio_clk;
 	struct clk			*suspend_clk;
+	struct clk			*phy_clk;
 	int				irq;
 	bool				pm_suspended;
 	bool				wakeup_pending;
 };
 
+static void imx8mp_configure_glue(struct dwc3_imx8mp *dwc3_imx)
+{
+	struct device *dev = dwc3_imx->dev;
+	u32 value;
+
+	if ((!dwc3_imx->glue_base) || (!dwc3_imx->phy_clk))
+		return;
+
+	value = readl(dwc3_imx->glue_base + USB_CTRL0);
+
+	if (device_property_read_bool(dev, "fsl,permanently-attached"))
+		value |= (USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
+	else
+		value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
+
+	if (device_property_read_bool(dev, "fsl,disable-port-power-control"))
+		value &= ~(USB_CTRL0_PORTPWR_EN);
+	else
+		value |= USB_CTRL0_PORTPWR_EN;
+
+	writel(value, dwc3_imx->glue_base + USB_CTRL0);
+
+	value = readl(dwc3_imx->glue_base + USB_CTRL1);
+	if (device_property_read_bool(dev, "fsl,over-current-active-low"))
+		value |= USB_CTRL1_OC_POLARITY;
+	else
+		value &= ~USB_CTRL1_OC_POLARITY;
+
+	if (device_property_read_bool(dev, "fsl,power-active-low"))
+		value |= USB_CTRL1_PWR_POLARITY;
+	else
+		value &= ~USB_CTRL1_PWR_POLARITY;
+
+	writel(value, dwc3_imx->glue_base + USB_CTRL1);
+}
+
 static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx)
 {
 	struct dwc3	*dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
@@ -100,6 +149,7 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 	struct device		*dev = &pdev->dev;
 	struct device_node	*dwc3_np, *node = dev->of_node;
 	struct dwc3_imx8mp	*dwc3_imx;
+	struct resource		*res;
 	int			err, irq;
 
 	if (!node) {
@@ -119,6 +169,15 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 	if (IS_ERR(dwc3_imx->hsio_blk_base))
 		return PTR_ERR(dwc3_imx->hsio_blk_base);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res) {
+		dev_warn(dev, "Base address for glue layer missing. Continuing without, some features are missing though.");
+	} else {
+		dwc3_imx->glue_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(dwc3_imx->glue_base))
+			return PTR_ERR(dwc3_imx->glue_base);
+	}
+
 	dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio");
 	if (IS_ERR(dwc3_imx->hsio_clk)) {
 		err = PTR_ERR(dwc3_imx->hsio_clk);
@@ -145,6 +204,24 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 		goto disable_hsio_clk;
 	}
 
+	dwc3_imx->phy_clk = devm_clk_get(dev, "phy");
+	if (PTR_ERR(dwc3_imx->phy_clk) == -ENOENT) {
+		dev_warn(dev, "PHY clock missing. Continuing without, some features are missing though.");
+		dwc3_imx->phy_clk = NULL;
+	} else if (IS_ERR(dwc3_imx->phy_clk)) {
+		err = PTR_ERR(dwc3_imx->phy_clk);
+		dev_err(dev, "Failed to get phy clk, err=%d\n", err);
+		goto disable_suspend_clk;
+	}
+
+	if (dwc3_imx->phy_clk) {
+		err = clk_prepare_enable(dwc3_imx->phy_clk);
+		if (err) {
+			dev_err(dev, "Failed to enable phy clk, err=%d\n", err);
+			goto disable_suspend_clk;
+		}
+	}
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		err = irq;
@@ -152,6 +229,8 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 	}
 	dwc3_imx->irq = irq;
 
+	imx8mp_configure_glue(dwc3_imx);
+
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 	err = pm_runtime_get_sync(dev);
@@ -199,6 +278,8 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 	pm_runtime_disable(dev);
 	pm_runtime_put_noidle(dev);
 disable_clks:
+	clk_disable_unprepare(dwc3_imx->phy_clk);
+disable_suspend_clk:
 	clk_disable_unprepare(dwc3_imx->suspend_clk);
 disable_hsio_clk:
 	clk_disable_unprepare(dwc3_imx->hsio_clk);
-- 
2.25.1

RE: [PATCH v3 3/4] usb: dwc3: imx8mp: Add support for setting SOC specific flags

[Jun Li]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: Tuesday, January 18, 2022 9:16 PM
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Rob Herring
> <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx
> <linux-imx@nxp.com>; linux-usb@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Jun Li
> <jun.li@nxp.com>
> Subject: [PATCH v3 3/4] usb: dwc3: imx8mp: Add support for setting SOC
> specific flags
> 
> The i.MX8MP glue layer has support for the following flags:
> * over-current polarity
> * PWR pad polarity
> * controlling PPC flag in HCCPARAMS register
> * permanent port attach for usb2 & usb3 port
> 
> Allow setting these flags by supporting specific flags in the glue node.
> In order to get this to work an additional IORESOURCE_MEM and clock is
> necessary. For backward compatibility this is purely optional.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 81 ++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c
> b/drivers/usb/dwc3/dwc3-imx8mp.c index 1c8fe657b3a9..3df4313b3740 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -36,17 +36,66 @@
> 
>  #define USB_WAKEUP_EN_MASK		GENMASK(5, 0)
> 
> +/* USB glue registers */
> +#define USB_CTRL0		0x00
> +#define USB_CTRL1		0x04
> +
> +#define USB_CTRL0_PORTPWR_EN	BIT(12) /* 1 - PPC enabled (default) */
> +#define USB_CTRL0_USB3_FIXED	BIT(22) /* 1 - USB3 permanent attached */
> +#define USB_CTRL0_USB2_FIXED	BIT(23) /* 1 - USB2 permanent attached */
> +
> +#define USB_CTRL1_OC_POLARITY	BIT(16) /* 0 - HIGH / 1 - LOW */
> +#define USB_CTRL1_PWR_POLARITY	BIT(17) /* 0 - HIGH / 1 - LOW */
> +
>  struct dwc3_imx8mp {
>  	struct device			*dev;
>  	struct platform_device		*dwc3;
>  	void __iomem			*hsio_blk_base;
> +	void __iomem			*glue_base;
>  	struct clk			*hsio_clk;
>  	struct clk			*suspend_clk;
> +	struct clk			*phy_clk;
>  	int				irq;
>  	bool				pm_suspended;
>  	bool				wakeup_pending;
>  };
> 
> +static void imx8mp_configure_glue(struct dwc3_imx8mp *dwc3_imx) {
> +	struct device *dev = dwc3_imx->dev;
> +	u32 value;
> +
> +	if ((!dwc3_imx->glue_base) || (!dwc3_imx->phy_clk))
> +		return;
> +
> +	value = readl(dwc3_imx->glue_base + USB_CTRL0);
> +
> +	if (device_property_read_bool(dev, "fsl,permanently-attached"))
> +		value |= (USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
> +	else
> +		value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
> +
> +	if (device_property_read_bool(dev,
> "fsl,disable-port-power-control"))
> +		value &= ~(USB_CTRL0_PORTPWR_EN);
> +	else
> +		value |= USB_CTRL0_PORTPWR_EN;
> +
> +	writel(value, dwc3_imx->glue_base + USB_CTRL0);
> +
> +	value = readl(dwc3_imx->glue_base + USB_CTRL1);
> +	if (device_property_read_bool(dev, "fsl,over-current-active-low"))
> +		value |= USB_CTRL1_OC_POLARITY;
> +	else
> +		value &= ~USB_CTRL1_OC_POLARITY;
> +
> +	if (device_property_read_bool(dev, "fsl,power-active-low"))
> +		value |= USB_CTRL1_PWR_POLARITY;
> +	else
> +		value &= ~USB_CTRL1_PWR_POLARITY;
> +
> +	writel(value, dwc3_imx->glue_base + USB_CTRL1); }
> +
>  static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx)  {
>  	struct dwc3	*dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
> @@ -100,6 +149,7 @@ static int dwc3_imx8mp_probe(struct platform_device
> *pdev)
>  	struct device		*dev = &pdev->dev;
>  	struct device_node	*dwc3_np, *node = dev->of_node;
>  	struct dwc3_imx8mp	*dwc3_imx;
> +	struct resource		*res;
>  	int			err, irq;
> 
>  	if (!node) {
> @@ -119,6 +169,15 @@ static int dwc3_imx8mp_probe(struct platform_device
> *pdev)
>  	if (IS_ERR(dwc3_imx->hsio_blk_base))
>  		return PTR_ERR(dwc3_imx->hsio_blk_base);
> 
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!res) {
> +		dev_warn(dev, "Base address for glue layer missing. Continuing
> without, some features are missing though.");
> +	} else {
> +		dwc3_imx->glue_base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(dwc3_imx->glue_base))
> +			return PTR_ERR(dwc3_imx->glue_base);
> +	}
> +
>  	dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio");
>  	if (IS_ERR(dwc3_imx->hsio_clk)) {
>  		err = PTR_ERR(dwc3_imx->hsio_clk);
> @@ -145,6 +204,24 @@ static int dwc3_imx8mp_probe(struct platform_device
> *pdev)
>  		goto disable_hsio_clk;
>  	}
> 
> +	dwc3_imx->phy_clk = devm_clk_get(dev, "phy");
> +	if (PTR_ERR(dwc3_imx->phy_clk) == -ENOENT) {
> +		dev_warn(dev, "PHY clock missing. Continuing without, some features
> are missing though.");

What feature needs phy clock turned on here, why phy driver turns on
this clock is not enough for you?

Thanks
Li Jun

> +		dwc3_imx->phy_clk = NULL;
> +	} else if (IS_ERR(dwc3_imx->phy_clk)) {
> +		err = PTR_ERR(dwc3_imx->phy_clk);
> +		dev_err(dev, "Failed to get phy clk, err=%d\n", err);
> +		goto disable_suspend_clk;
> +	}
> +
> +	if (dwc3_imx->phy_clk) {
> +		err = clk_prepare_enable(dwc3_imx->phy_clk);
> +		if (err) {
> +			dev_err(dev, "Failed to enable phy clk, err=%d\n", err);
> +			goto disable_suspend_clk;
> +		}
> +	}
> +
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
>  		err = irq;
> @@ -152,6 +229,8 @@ static int dwc3_imx8mp_probe(struct platform_device
> *pdev)
>  	}
>  	dwc3_imx->irq = irq;
> 
> +	imx8mp_configure_glue(dwc3_imx);
> +
>  	pm_runtime_set_active(dev);
>  	pm_runtime_enable(dev);
>  	err = pm_runtime_get_sync(dev);
> @@ -199,6 +278,8 @@ static int dwc3_imx8mp_probe(struct platform_device
> *pdev)
>  	pm_runtime_disable(dev);
>  	pm_runtime_put_noidle(dev);
>  disable_clks:
> +	clk_disable_unprepare(dwc3_imx->phy_clk);
> +disable_suspend_clk:
>  	clk_disable_unprepare(dwc3_imx->suspend_clk);
>  disable_hsio_clk:
>  	clk_disable_unprepare(dwc3_imx->hsio_clk);
> --
> 2.25.1

Re: (EXT) RE: [PATCH v3 3/4] usb: dwc3: imx8mp: Add support for setting SOC specific flags

[Alexander Stein]

Am Mittwoch, 19. Januar 2022, 15:14:05 CET schrieb Jun Li:
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: Tuesday, January 18, 2022 9:16 PM
> > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Rob Herring
> > <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> > <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com>
> > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx
> > <linux-imx@nxp.com>; linux-usb@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Jun Li
> > <jun.li@nxp.com>
> > Subject: [PATCH v3 3/4] usb: dwc3: imx8mp: Add support for setting SOC
> > specific flags
> > 
> > The i.MX8MP glue layer has support for the following flags:
> > * over-current polarity
> > * PWR pad polarity
> > * controlling PPC flag in HCCPARAMS register
> > * permanent port attach for usb2 & usb3 port
> > 
> > Allow setting these flags by supporting specific flags in the glue node.
> > In order to get this to work an additional IORESOURCE_MEM and clock is
> > necessary. For backward compatibility this is purely optional.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > 
> >  drivers/usb/dwc3/dwc3-imx8mp.c | 81 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c
> > b/drivers/usb/dwc3/dwc3-imx8mp.c index 1c8fe657b3a9..3df4313b3740 100644
> > --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> > @@ -36,17 +36,66 @@
> > 
> >  #define USB_WAKEUP_EN_MASK		GENMASK(5, 0)
> > 
> > +/* USB glue registers */
> > +#define USB_CTRL0		0x00
> > +#define USB_CTRL1		0x04
> > +
> > +#define USB_CTRL0_PORTPWR_EN	BIT(12) /* 1 - PPC enabled (default) */
> > +#define USB_CTRL0_USB3_FIXED	BIT(22) /* 1 - USB3 permanent attached */
> > +#define USB_CTRL0_USB2_FIXED	BIT(23) /* 1 - USB2 permanent attached */
> > +
> > +#define USB_CTRL1_OC_POLARITY	BIT(16) /* 0 - HIGH / 1 - LOW */
> > +#define USB_CTRL1_PWR_POLARITY	BIT(17) /* 0 - HIGH / 1 - LOW */
> > +
> > 
> >  struct dwc3_imx8mp {
> >  
> >  	struct device			*dev;
> >  	struct platform_device		*dwc3;
> >  	void __iomem			*hsio_blk_base;
> > 
> > +	void __iomem			*glue_base;
> > 
> >  	struct clk			*hsio_clk;
> >  	struct clk			*suspend_clk;
> > 
> > +	struct clk			*phy_clk;
> > 
> >  	int				irq;
> >  	bool				pm_suspended;
> >  	bool				wakeup_pending;
> >  
> >  };
> > 
> > +static void imx8mp_configure_glue(struct dwc3_imx8mp *dwc3_imx) {
> > +	struct device *dev = dwc3_imx->dev;
> > +	u32 value;
> > +
> > +	if ((!dwc3_imx->glue_base) || (!dwc3_imx->phy_clk))
> > +		return;
> > +
> > +	value = readl(dwc3_imx->glue_base + USB_CTRL0);
> > +
> > +	if (device_property_read_bool(dev, "fsl,permanently-attached"))
> > +		value |= (USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
> > +	else
> > +		value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
> > +
> > +	if (device_property_read_bool(dev,
> > "fsl,disable-port-power-control"))
> > +		value &= ~(USB_CTRL0_PORTPWR_EN);
> > +	else
> > +		value |= USB_CTRL0_PORTPWR_EN;
> > +
> > +	writel(value, dwc3_imx->glue_base + USB_CTRL0);
> > +
> > +	value = readl(dwc3_imx->glue_base + USB_CTRL1);
> > +	if (device_property_read_bool(dev, "fsl,over-current-active-low"))
> > +		value |= USB_CTRL1_OC_POLARITY;
> > +	else
> > +		value &= ~USB_CTRL1_OC_POLARITY;
> > +
> > +	if (device_property_read_bool(dev, "fsl,power-active-low"))
> > +		value |= USB_CTRL1_PWR_POLARITY;
> > +	else
> > +		value &= ~USB_CTRL1_PWR_POLARITY;
> > +
> > +	writel(value, dwc3_imx->glue_base + USB_CTRL1); }
> > +
> > 
> >  static void dwc3_imx8mp_wakeup_enable(struct dwc3_imx8mp *dwc3_imx)  {
> >  
> >  	struct dwc3	*dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
> > 
> > @@ -100,6 +149,7 @@ static int dwc3_imx8mp_probe(struct platform_device
> > *pdev)
> > 
> >  	struct device		*dev = &pdev->dev;
> >  	struct device_node	*dwc3_np, *node = dev->of_node;
> >  	struct dwc3_imx8mp	*dwc3_imx;
> > 
> > +	struct resource		*res;
> > 
> >  	int			err, irq;
> >  	
> >  	if (!node) {
> > 
> > @@ -119,6 +169,15 @@ static int dwc3_imx8mp_probe(struct platform_device
> > *pdev)
> > 
> >  	if (IS_ERR(dwc3_imx->hsio_blk_base))
> >  	
> >  		return PTR_ERR(dwc3_imx->hsio_blk_base);
> > 
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > +	if (!res) {
> > +		dev_warn(dev, "Base address for glue layer missing. 
Continuing
> > without, some features are missing though.");
> > +	} else {
> > +		dwc3_imx->glue_base = devm_ioremap_resource(dev, res);
> > +		if (IS_ERR(dwc3_imx->glue_base))
> > +			return PTR_ERR(dwc3_imx->glue_base);
> > +	}
> > +
> > 
> >  	dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio");
> >  	if (IS_ERR(dwc3_imx->hsio_clk)) {
> >  	
> >  		err = PTR_ERR(dwc3_imx->hsio_clk);
> > 
> > @@ -145,6 +204,24 @@ static int dwc3_imx8mp_probe(struct platform_device
> > *pdev)
> > 
> >  		goto disable_hsio_clk;
> >  	
> >  	}
> > 
> > +	dwc3_imx->phy_clk = devm_clk_get(dev, "phy");
> > +	if (PTR_ERR(dwc3_imx->phy_clk) == -ENOENT) {
> > +		dev_warn(dev, "PHY clock missing. Continuing without, 
some features
> > are missing though.");

Hi,

> What feature needs phy clock turned on here, why phy driver turns on
> this clock is not enough for you?

I have to admit that the clock name 'phy' might be misleading. In this case 
the actual clock is IMX8MP_CLK_USB_PHY_ROOT. Apparently this clock (or a 
dependent) is necessary to access the USB3_GLUE block in imx8mp. This block 
contains the USB3_CTRL0/USB3_CTRL1, we want to access in this case, as well as 
the PHY registers, hence the name I guess.
While it is true that phy-fsl-imx8mq-usb enables this clock as well, there is 
no guarantee this lock is enabled when we want to access the GLUE registers.
Depending on the probe order it is actually possible the clock is disabled 
while dwc3-imx8mp being probed, resulting in a system lockup.
This happens especially if there is only one USB host enabled and the drivers 
are built as modules.

Meanwhile I noticed [1] landed last week. So I gave it a try. With that in 
place there is no need to use IMX8MP_CLK_USB_PHY_ROOT in this driver anymore.
As it turns out IMX8MP_CLK_USB_ROOT is the required clock (used by imx8mp-blk-
ctrl for USB power domain).
With that, I'll send a new version based on Lucas' patchset.

Thanks for the feedback
Alexander

[1] https://patchwork.kernel.org/project/linux-arm-kernel/cover/
20220119134027.2931945-1-l.stach@pengutronix.de/

[PATCH v3 4/4] arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3 nodes

[Alexander Stein]

The USB3 glue layer has 2 areas in the register set, see RM Rev.1
section 11.2.5.2.1 GLUE_usb3 memory map:
* USB3 control/status
* PHY control/status

Provide the memory area to the usb3 nodes for accessing the features
in the USB3 control area.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 6b840c05dd77..baaa49b419fa 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -921,10 +921,14 @@ usb3_phy0: usb-phy@381f0040 {
 
 		usb3_0: usb@32f10100 {
 			compatible = "fsl,imx8mp-dwc3";
-			reg = <0x32f10100 0x8>;
+			reg = <0x32f10100 0x8>,
+			      <0x381f0000 0x20>;
 			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
-				 <&clk IMX8MP_CLK_USB_ROOT>;
-			clock-names = "hsio", "suspend";
+				 <&clk IMX8MP_CLK_USB_ROOT>,
+				 <&clk IMX8MP_CLK_USB_PHY_ROOT>;
+			clock-names = "hsio", "suspend", "phy";
+			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
+			assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
 			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
 			#address-cells = <1>;
 			#size-cells = <1>;
@@ -962,10 +966,14 @@ usb3_phy1: usb-phy@382f0040 {
 
 		usb3_1: usb@32f10108 {
 			compatible = "fsl,imx8mp-dwc3";
-			reg = <0x32f10108 0x8>;
+			reg = <0x32f10108 0x8>,
+			      <0x382f0000 0x20>;
 			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
-				 <&clk IMX8MP_CLK_USB_ROOT>;
-			clock-names = "hsio", "suspend";
+				 <&clk IMX8MP_CLK_USB_ROOT>,
+				 <&clk IMX8MP_CLK_USB_PHY_ROOT>;
+			clock-names = "hsio", "suspend", "phy";
+			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
+			assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
 			interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
 			#address-cells = <1>;
 			#size-cells = <1>;
-- 
2.25.1