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

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

[Alexander Stein]

Hi all,

Thanks for the feedback on v3 [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 v4 the patchset depends on Lucas' i.MX8MP power-domains patchset [2].
Because of that the 'phy' clock usage can be dropped, as now power domains
take care of the clocks. Due to powering up/down the glue layer settings
will have to be reapplied during resume.

Changes in v4:
* Depends on Lucas' i.MX8MP power-domains patchset
* removed 'phy' clock usage from dwc3-imx8mp.c
* Reapply glue layer settings upon resume

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

[1] https://patchwork.kernel.org/project/linux-arm-kernel/cover/20220118131626.926394-1-alexander.stein@ew.tq-group.com/
[2] https://patchwork.kernel.org/project/linux-arm-kernel/cover/20220119134027.2931945-1-l.stach@pengutronix.de/

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         | 31 +++++++-
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     |  6 +-
 drivers/usb/dwc3/dwc3-imx8mp.c                | 76 +++++++++++++++++--
 3 files changed, 100 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH v4 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

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

[Jun Li]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: Wednesday, January 26, 2022 10:14 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 v4 1/4] usb: dwc3: imx8mp: rename iomem base pointer
> 
> 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>

Reviewed-by: Li Jun <jun.li@nxp.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 v4 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         | 31 ++++++++++++++++---
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/fsl,imx8mp-dwc3.yaml b/Documentation/devicetree/bindings/usb/fsl,imx8mp-dwc3.yaml
index 974032b1fda0..01ab0f922ae8 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 ]
@@ -49,6 +49,28 @@ properties:
       - const: hsio
       - const: suspend
 
+  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:
 
 patternProperties:
@@ -74,7 +96,8 @@ 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";
-- 
2.25.1

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

[Rob Herring]

On Wed, 26 Jan 2022 15:13:38 +0100, Alexander Stein wrote:
> 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         | 31 ++++++++++++++++---
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1584490


usb@32f10100: reg: [[854655232, 8]] is too short
	arch/arm64/boot/dts/freescale/imx8mp-evk.dt.yaml
	arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dt.yaml

usb@32f10108: reg: [[854655240, 8]] is too short
	arch/arm64/boot/dts/freescale/imx8mp-evk.dt.yaml
	arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dt.yaml

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

[Rob Herring]

On Wed, 26 Jan 2022 15:13:38 +0100, Alexander Stein wrote:
> 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         | 31 ++++++++++++++++---
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v4 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 | 62 ++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
index 1c8fe657b3a9..5a96b66bf237 100644
--- a/drivers/usb/dwc3/dwc3-imx8mp.c
+++ b/drivers/usb/dwc3/dwc3-imx8mp.c
@@ -36,10 +36,22 @@
 
 #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;
 	int				irq;
@@ -47,6 +59,42 @@ struct dwc3_imx8mp {
 	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)
+		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 +148,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 +168,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);
@@ -152,6 +210,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);
@@ -252,6 +312,8 @@ static int __maybe_unused dwc3_imx8mp_resume(struct dwc3_imx8mp *dwc3_imx,
 	dwc3_imx8mp_wakeup_disable(dwc3_imx);
 	dwc3_imx->pm_suspended = false;
 
+	imx8mp_configure_glue(dwc3_imx);
+
 	if (dwc3_imx->wakeup_pending) {
 		dwc3_imx->wakeup_pending = false;
 		if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE) {
-- 
2.25.1

RE: [PATCH v4 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: Wednesday, January 26, 2022 10:14 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 v4 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 | 62 ++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c
> b/drivers/usb/dwc3/dwc3-imx8mp.c
> index 1c8fe657b3a9..5a96b66bf237 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -36,10 +36,22 @@
> 
>  #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;
>  	int				irq;
> @@ -47,6 +59,42 @@ struct dwc3_imx8mp {
>  	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)
> +		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;

I think you don't need a value update&write if property is not present.

> +
> +	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 +148,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;

Looks like this is not used anywhere, remove it.

>  	int			err, irq;
> 
>  	if (!node) {
> @@ -119,6 +168,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);
> @@ -152,6 +210,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);
> @@ -252,6 +312,8 @@ static int __maybe_unused dwc3_imx8mp_resume(struct
> dwc3_imx8mp *dwc3_imx,
>  	dwc3_imx8mp_wakeup_disable(dwc3_imx);
>  	dwc3_imx->pm_suspended = false;
> 
> +	imx8mp_configure_glue(dwc3_imx);
> +

I guess this is to restore the settings if we experienced a power lost,
Please add some comments.

thanks
Li Jun

>  	if (dwc3_imx->wakeup_pending) {
>  		dwc3_imx->wakeup_pending = false;
>  		if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE) {
> --
> 2.25.1

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

[Alexander Stein]

Hi Li Jun,

thanks for your feedback.

Am Freitag, 18. Februar 2022, 07:57:33 CET schrieb Jun Li:
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: Wednesday, January 26, 2022 10:14 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 v4 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 | 62 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c
> > b/drivers/usb/dwc3/dwc3-imx8mp.c
> > index 1c8fe657b3a9..5a96b66bf237 100644
> > --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> > @@ -36,10 +36,22 @@
> > 
> >  #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;
> >  	int				irq;
> > 
> > @@ -47,6 +59,42 @@ struct dwc3_imx8mp {
> > 
> >  	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)
> > +		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;
> 
> I think you don't need a value update&write if property is not present.

I get what you intend here, but this assumes some (sane) default which might 
be true in every case. But I prefer configuring this without any assumption 
about previous settings.

> > +
> > +	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 +148,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;
> 
> Looks like this is not used anywhere, remove it.

It actually is, please refer to the next hunk. Both is inside 
dwc3_imx8mp_probe(). Probably the context of the diff is a bit misleading.

> >  	int			err, irq;
> >  	
> >  	if (!node) {
> > 
> > @@ -119,6 +168,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);
> > 
> > @@ -152,6 +210,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);
> > 
> > @@ -252,6 +312,8 @@ static int __maybe_unused dwc3_imx8mp_resume(struct
> > dwc3_imx8mp *dwc3_imx,
> > 
> >  	dwc3_imx8mp_wakeup_disable(dwc3_imx);
> >  	dwc3_imx->pm_suspended = false;
> > 
> > +	imx8mp_configure_glue(dwc3_imx);
> > +
> 
> I guess this is to restore the settings if we experienced a power lost,
> Please add some comments.

You are right, makes sense. I'll add a comment.

Regards,
Alexander

> >  	if (dwc3_imx->wakeup_pending) {
> >  	
> >  		dwc3_imx->wakeup_pending = false;
> >  		if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE) {
> > 
> > --
> > 2.25.1

[PATCH v4 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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index e61ac5f136ad..d4aadb434d36 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1030,7 +1030,8 @@ 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";
@@ -1070,7 +1071,8 @@ 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";
-- 
2.25.1

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

[Jun Li]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: Wednesday, January 26, 2022 10:14 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 v4 4/4] arm64: dts: imx8mp: Add memory for USB3 glue layer
> to usb3 nodes
> 
> 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>

Reviewed-by: Li Jun <jun.li@nxp.com>

> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index e61ac5f136ad..d4aadb434d36 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1030,7 +1030,8 @@ 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";
> @@ -1070,7 +1071,8 @@ 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";
> --
> 2.25.1