[PATCH v2 0/4] usb: dwc2: add optional clock used on stm32mp15

[PATCH v2 0/4] usb: dwc2: add optional clock used on stm32mp15

[Fabrice Gasnier]

This series introduces an optional utmi clock that is used on stm32mp15,
in particular when using integrated full-speed PHY, managed by GGPIO
register.
A pre-cursor change improves error handling in the platform code.
---
Changes in v2:
- "utmi_clk" renamed "utmi" as per Krzysztof comment on dt-bindings

Fabrice Gasnier (4):
  usb: dwc2: improve error handling in __dwc2_lowlevel_hw_enable
  dt-bindings: usb: dwc2: add utmi optional clock
  usb: dwc2: platform: add support for utmi optional clock
  ARM: dts: stm32: add USB OTG UTMI clock on stm32mp151

 .../devicetree/bindings/usb/dwc2.yaml         |  5 ++-
 arch/arm/boot/dts/stm32mp151.dtsi             |  4 +-
 drivers/usb/dwc2/core.h                       |  2 +
 drivers/usb/dwc2/platform.c                   | 37 ++++++++++++++++++-
 4 files changed, 43 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH v2 1/4] usb: dwc2: improve error handling in __dwc2_lowlevel_hw_enable

[Fabrice Gasnier]

Add error handling in __dwc2_lowlevel_hw_enable() that may leave the
clocks and regulators enabled upon error.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/usb/dwc2/platform.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index d1589ba7d322..c431ce6c119f 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -104,7 +104,7 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
 	if (hsotg->clk) {
 		ret = clk_prepare_enable(hsotg->clk);
 		if (ret)
-			return ret;
+			goto err_dis_reg;
 	}
 
 	if (hsotg->uphy) {
@@ -113,10 +113,25 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
 		ret = hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
 	} else {
 		ret = phy_init(hsotg->phy);
-		if (ret == 0)
+		if (ret == 0) {
 			ret = phy_power_on(hsotg->phy);
+			if (ret)
+				phy_exit(hsotg->phy);
+		}
 	}
 
+	if (ret)
+		goto err_dis_clk;
+
+	return 0;
+
+err_dis_clk:
+	if (hsotg->clk)
+		clk_disable_unprepare(hsotg->clk);
+
+err_dis_reg:
+	regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
+
 	return ret;
 }
 
-- 
2.25.1

Re: [PATCH v2 1/4] usb: dwc2: improve error handling in __dwc2_lowlevel_hw_enable

[Minas Harutyunyan]

On 4/14/23 12:41, Fabrice Gasnier wrote:
> Add error handling in __dwc2_lowlevel_hw_enable() that may leave the
> clocks and regulators enabled upon error.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Acked-by: Minas Harutyunyan <hminas@synopsys.com>

> ---
>   drivers/usb/dwc2/platform.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index d1589ba7d322..c431ce6c119f 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -104,7 +104,7 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
>   	if (hsotg->clk) {
>   		ret = clk_prepare_enable(hsotg->clk);
>   		if (ret)
> -			return ret;
> +			goto err_dis_reg;
>   	}
>   
>   	if (hsotg->uphy) {
> @@ -113,10 +113,25 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
>   		ret = hsotg->plat->phy_init(pdev, hsotg->plat->phy_type);
>   	} else {
>   		ret = phy_init(hsotg->phy);
> -		if (ret == 0)
> +		if (ret == 0) {
>   			ret = phy_power_on(hsotg->phy);
> +			if (ret)
> +				phy_exit(hsotg->phy);
> +		}
>   	}
>   
> +	if (ret)
> +		goto err_dis_clk;
> +
> +	return 0;
> +
> +err_dis_clk:
> +	if (hsotg->clk)
> +		clk_disable_unprepare(hsotg->clk);
> +
> +err_dis_reg:
> +	regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
> +
>   	return ret;
>   }
>   

[PATCH v2 2/4] dt-bindings: usb: dwc2: add utmi optional clock

[Fabrice Gasnier]

utmi clock is typically provided by PHY output. Add this optional clock,
as the core could use other clocks depending on the SoC where it's used.
This is needed on stm32mp15, when using the integrated full-speed PHY.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
Changes in v2:
- "utmi_clk" renamed "utmi" as per Krzysztof comment
---
 Documentation/devicetree/bindings/usb/dwc2.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml b/Documentation/devicetree/bindings/usb/dwc2.yaml
index 371ba93f3ce5..d3506090f8b1 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.yaml
+++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
@@ -75,11 +75,14 @@ properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   clock-names:
     items:
       - const: otg
+      - const: utmi
+    minItems: 1
 
   disable-over-current:
     type: boolean
-- 
2.25.1

Re: [PATCH v2 2/4] dt-bindings: usb: dwc2: add utmi optional clock

[Krzysztof Kozlowski]

On 14/04/2023 10:41, Fabrice Gasnier wrote:
> utmi clock is typically provided by PHY output. Add this optional clock,
> as the core could use other clocks depending on the SoC where it's used.
> This is needed on stm32mp15, when using the integrated full-speed PHY.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v2 3/4] usb: dwc2: platform: add support for utmi optional clock

[Fabrice Gasnier]

Add support for the utmi clock. It's needed on STM32MP15, when using
the integrated full-speed PHY. This clock is an output of USBPHYC, but
HS USBPHYC is not attached as PHY in this case: Full-Speed PHY is directly
managed in dwc2 glue, through GGPIO register. Typical DT when using FS PHY
&usbotg_hs {
	compatible = "st,stm32mp15-fsotg", "snps,dwc2";
	pinctrl-names = "default";
	pinctrl-0 = <&usbotg_hs_pins_a &usbotg_fs_dp_dm_pins_a>;
	vbus-supply = <&vbus_otg>;
	status = "okay";
};

In this configuration, USBPHYC clock output must be defined, so it can
be properly enabled as a clock provider:
	clocks = <&rcc USBO_K>, <&usbphyc>;
	clock-names = "otg", "utmi";

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
Changes in v2:
- "utmi_clk" renamed "utmi" as per Krzysztof comment on dt-bindings
---
 drivers/usb/dwc2/core.h     |  2 ++
 drivers/usb/dwc2/platform.c | 20 +++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 40cf2880d7e5..0bb4c0c845bf 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1003,6 +1003,7 @@ struct dwc2_hregs_backup {
  * @ctrl_out_desc:	EP0 OUT data phase desc chain pointer
  * @irq:		Interrupt request line number
  * @clk:		Pointer to otg clock
+ * @utmi_clk:		Pointer to utmi_clk clock
  * @reset:		Pointer to dwc2 reset controller
  * @reset_ecc:          Pointer to dwc2 optional reset controller in Stratix10.
  * @regset:		A pointer to a struct debugfs_regset32, which contains
@@ -1065,6 +1066,7 @@ struct dwc2_hsotg {
 	void *priv;
 	int     irq;
 	struct clk *clk;
+	struct clk *utmi_clk;
 	struct reset_control *reset;
 	struct reset_control *reset_ecc;
 
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index c431ce6c119f..5aee284018c0 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -101,10 +101,16 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
 	if (ret)
 		return ret;
 
+	if (hsotg->utmi_clk) {
+		ret = clk_prepare_enable(hsotg->utmi_clk);
+		if (ret)
+			goto err_dis_reg;
+	}
+
 	if (hsotg->clk) {
 		ret = clk_prepare_enable(hsotg->clk);
 		if (ret)
-			goto err_dis_reg;
+			goto err_dis_utmi_clk;
 	}
 
 	if (hsotg->uphy) {
@@ -129,6 +135,10 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
 	if (hsotg->clk)
 		clk_disable_unprepare(hsotg->clk);
 
+err_dis_utmi_clk:
+	if (hsotg->utmi_clk)
+		clk_disable_unprepare(hsotg->utmi_clk);
+
 err_dis_reg:
 	regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
 
@@ -171,6 +181,9 @@ static int __dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg)
 	if (hsotg->clk)
 		clk_disable_unprepare(hsotg->clk);
 
+	if (hsotg->utmi_clk)
+		clk_disable_unprepare(hsotg->utmi_clk);
+
 	return regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
 }
 
@@ -247,6 +260,11 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
 	if (IS_ERR(hsotg->clk))
 		return dev_err_probe(hsotg->dev, PTR_ERR(hsotg->clk), "cannot get otg clock\n");
 
+	hsotg->utmi_clk = devm_clk_get_optional(hsotg->dev, "utmi");
+	if (IS_ERR(hsotg->utmi_clk))
+		return dev_err_probe(hsotg->dev, PTR_ERR(hsotg->utmi_clk),
+				     "cannot get utmi clock\n");
+
 	/* Regulators */
 	for (i = 0; i < ARRAY_SIZE(hsotg->supplies); i++)
 		hsotg->supplies[i].supply = dwc2_hsotg_supply_names[i];
-- 
2.25.1

Re: [PATCH v2 3/4] usb: dwc2: platform: add support for utmi optional clock

[Minas Harutyunyan]

On 4/14/23 12:41, Fabrice Gasnier wrote:
> Add support for the utmi clock. It's needed on STM32MP15, when using
> the integrated full-speed PHY. This clock is an output of USBPHYC, but
> HS USBPHYC is not attached as PHY in this case: Full-Speed PHY is directly
> managed in dwc2 glue, through GGPIO register. Typical DT when using FS PHY
> &usbotg_hs {
> 	compatible = "st,stm32mp15-fsotg", "snps,dwc2";
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&usbotg_hs_pins_a &usbotg_fs_dp_dm_pins_a>;
> 	vbus-supply = <&vbus_otg>;
> 	status = "okay";
> };
> 
> In this configuration, USBPHYC clock output must be defined, so it can
> be properly enabled as a clock provider:
> 	clocks = <&rcc USBO_K>, <&usbphyc>;
> 	clock-names = "otg", "utmi";
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Acked-by: Minas Harutyunyan <hminas@synopsys.com>

> ---
> Changes in v2:
> - "utmi_clk" renamed "utmi" as per Krzysztof comment on dt-bindings
> ---
>   drivers/usb/dwc2/core.h     |  2 ++
>   drivers/usb/dwc2/platform.c | 20 +++++++++++++++++++-
>   2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 40cf2880d7e5..0bb4c0c845bf 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -1003,6 +1003,7 @@ struct dwc2_hregs_backup {
>    * @ctrl_out_desc:	EP0 OUT data phase desc chain pointer
>    * @irq:		Interrupt request line number
>    * @clk:		Pointer to otg clock
> + * @utmi_clk:		Pointer to utmi_clk clock
>    * @reset:		Pointer to dwc2 reset controller
>    * @reset_ecc:          Pointer to dwc2 optional reset controller in Stratix10.
>    * @regset:		A pointer to a struct debugfs_regset32, which contains
> @@ -1065,6 +1066,7 @@ struct dwc2_hsotg {
>   	void *priv;
>   	int     irq;
>   	struct clk *clk;
> +	struct clk *utmi_clk;
>   	struct reset_control *reset;
>   	struct reset_control *reset_ecc;
>   
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index c431ce6c119f..5aee284018c0 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -101,10 +101,16 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
>   	if (ret)
>   		return ret;
>   
> +	if (hsotg->utmi_clk) {
> +		ret = clk_prepare_enable(hsotg->utmi_clk);
> +		if (ret)
> +			goto err_dis_reg;
> +	}
> +
>   	if (hsotg->clk) {
>   		ret = clk_prepare_enable(hsotg->clk);
>   		if (ret)
> -			goto err_dis_reg;
> +			goto err_dis_utmi_clk;
>   	}
>   
>   	if (hsotg->uphy) {
> @@ -129,6 +135,10 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg)
>   	if (hsotg->clk)
>   		clk_disable_unprepare(hsotg->clk);
>   
> +err_dis_utmi_clk:
> +	if (hsotg->utmi_clk)
> +		clk_disable_unprepare(hsotg->utmi_clk);
> +
>   err_dis_reg:
>   	regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
>   
> @@ -171,6 +181,9 @@ static int __dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg)
>   	if (hsotg->clk)
>   		clk_disable_unprepare(hsotg->clk);
>   
> +	if (hsotg->utmi_clk)
> +		clk_disable_unprepare(hsotg->utmi_clk);
> +
>   	return regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
>   }
>   
> @@ -247,6 +260,11 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
>   	if (IS_ERR(hsotg->clk))
>   		return dev_err_probe(hsotg->dev, PTR_ERR(hsotg->clk), "cannot get otg clock\n");
>   
> +	hsotg->utmi_clk = devm_clk_get_optional(hsotg->dev, "utmi");
> +	if (IS_ERR(hsotg->utmi_clk))
> +		return dev_err_probe(hsotg->dev, PTR_ERR(hsotg->utmi_clk),
> +				     "cannot get utmi clock\n");
> +
>   	/* Regulators */
>   	for (i = 0; i < ARRAY_SIZE(hsotg->supplies); i++)
>   		hsotg->supplies[i].supply = dwc2_hsotg_supply_names[i];

[PATCH v2 4/4] ARM: dts: stm32: add USB OTG UTMI clock on stm32mp151

[Fabrice Gasnier]

It's needed on STM32MP15, when using the integrated full-speed PHY. This
clock is an output of USBPHYC, and the HS USBPHYC is not attached as PHY
in this case (managed directly by dwc2 ggpio glue):

    &usbotg_hs {
    	compatible = "st,stm32mp15-fsotg", "snps,dwc2";
    	pinctrl-names = "default";
    	pinctrl-0 = <&usbotg_hs_pins_a &usbotg_fs_dp_dm_pins_a>;
    	vbus-supply = <&vbus_otg>;
    	status = "okay";
    };

USBPHYC clock output must be used, so it can be properly enabled as a
clock provider.

Without this, currently, when the dualport High-Speed USBPHYC isn't
requested by either USBH or OTG, it remains uninitialized when probing
OTG: OTG configured with full-speed PHY isn't properly clocked, resulting
in error log like:
[    2.383138] dwc2 49000000.usb-otg: dwc2_core_reset: HANG! Soft Reset
timeout GRSTCTL_CSFTRST.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
Changes in v2:
- "utmi_clk" renamed "utmi" as per Krzysztof comment on dt-bindings
---
 arch/arm/boot/dts/stm32mp151.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi
index 4e437d3f2ed6..63f4c78fcc1d 100644
--- a/arch/arm/boot/dts/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/stm32mp151.dtsi
@@ -1130,8 +1130,8 @@ sdmmc3: mmc@48004000 {
 		usbotg_hs: usb-otg@49000000 {
 			compatible = "st,stm32mp15-hsotg", "snps,dwc2";
 			reg = <0x49000000 0x10000>;
-			clocks = <&rcc USBO_K>;
-			clock-names = "otg";
+			clocks = <&rcc USBO_K>, <&usbphyc>;
+			clock-names = "otg", "utmi";
 			resets = <&rcc USBO_R>;
 			reset-names = "dwc2";
 			interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.25.1