[PATCH 1/3] usb: dwc2: Add support for STM32F429/439/469 USB OTG in FS mode with internal PHY

[PATCH 1/3] usb: dwc2: Add support for STM32F429/439/469 USB OTG in FS mode with internal PHY

[Bruno Herrera]

Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
---
 drivers/usb/dwc2/core.c     | 18 ++++++++++++++++++
 drivers/usb/dwc2/core.h     |  5 +++++
 drivers/usb/dwc2/hcd.c      | 12 +++++++++++-
 drivers/usb/dwc2/hw.h       |  2 ++
 drivers/usb/dwc2/platform.c | 37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 4135a5f..83fbed6 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -1276,6 +1276,23 @@ static void dwc2_set_param_hibernation(struct dwc2_hsotg *hsotg,
 	hsotg->core_params->hibernation = val;
 }
 
+static void dwc2_set_param_stm32_powerdown(struct dwc2_hsotg *hsotg,
+		int val)
+{
+	if (DWC2_OUT_OF_BOUNDS(val, 0, 1)) {
+		if (val >= 0) {
+			dev_err(hsotg->dev,
+				"'%d' invalid for parameter power down\n",
+				val);
+			dev_err(hsotg->dev, "power down must be 0 or 1\n");
+		}
+		val = 0;
+		dev_dbg(hsotg->dev, "Setting power down to %d\n", val);
+	}
+
+	hsotg->core_params->stm32_powerdown = val;
+}
+
 /*
  * This function is called during module intialization to pass module parameters
  * for the DWC_otg core.
@@ -1323,6 +1340,7 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
 	dwc2_set_param_uframe_sched(hsotg, params->uframe_sched);
 	dwc2_set_param_external_id_pin_ctl(hsotg, params->external_id_pin_ctl);
 	dwc2_set_param_hibernation(hsotg, params->hibernation);
+	dwc2_set_param_stm32_powerdown(hsotg, params->stm32_powerdown);
 }
 
 /*
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 3c58d63..d3e4fcb 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -386,6 +386,10 @@ enum dwc2_ep0_state {
  *			needed.
  *			0 - No (default)
  *			1 - Yes
+ * @stm32_powerdown:	Enable STM32 specific USB FS transceiver power down
+ *			control.
+ *			0 = USB FS transceiver disabled (default)
+ *			1 = USB FS transceiver enabled
  *
  * The following parameters may be specified when starting the module. These
  * parameters define how the DWC_otg controller should be configured. A
@@ -426,6 +430,7 @@ struct dwc2_core_params {
 	int uframe_sched;
 	int external_id_pin_ctl;
 	int hibernation;
+	int stm32_powerdown;
 };
 
 /**
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 2df3d04..4f9bb93 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -118,7 +118,7 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg)
 
 static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
 {
-	u32 usbcfg, i2cctl;
+	u32 usbcfg, usbgpio, i2cctl;
 	int retval = 0;
 
 	/*
@@ -142,6 +142,16 @@ static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
 				return retval;
 			}
 		}
+
+		if (hsotg->core_params->stm32_powerdown > 0) {
+			dev_dbg(hsotg->dev, "STM32 FS PHY enabling transceiver\n");
+			/* STM32 uses the GGPIO register as general core
+			 * configuration register.
+			 */
+			usbgpio = dwc2_readl(hsotg->regs + GGPIO);
+			usbgpio |= STM32_OTG_GCCFG_PWRDWN;
+			dwc2_writel(usbgpio, hsotg->regs + GGPIO);
+		}
 	}
 
 	/*
diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 281b57b..d5f9294 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -224,6 +224,8 @@
 
 #define GPVNDCTL			HSOTG_REG(0x0034)
 #define GGPIO				HSOTG_REG(0x0038)
+#define STM32_OTG_GCCFG_PWRDWN		(1 << 16)
+
 #define GUID				HSOTG_REG(0x003c)
 #define GSNPSID				HSOTG_REG(0x0040)
 #define GHWCFG1				HSOTG_REG(0x0044)
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index fc6f525..d806b94 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -84,6 +84,7 @@ static const struct dwc2_core_params params_hi6220 = {
 	.uframe_sched			= 0,
 	.external_id_pin_ctl		= -1,
 	.hibernation			= -1,
+	.stm32_powerdown		= 0,
 };
 
 static const struct dwc2_core_params params_bcm2835 = {
@@ -115,6 +116,7 @@ static const struct dwc2_core_params params_bcm2835 = {
 	.uframe_sched			= 0,
 	.external_id_pin_ctl		= -1,
 	.hibernation			= -1,
+	.stm32_powerdown		= 0,
 };
 
 static const struct dwc2_core_params params_rk3066 = {
@@ -147,6 +149,7 @@ static const struct dwc2_core_params params_rk3066 = {
 	.uframe_sched			= -1,
 	.external_id_pin_ctl		= -1,
 	.hibernation			= -1,
+	.stm32_powerdown		= 0,
 };
 
 static const struct dwc2_core_params params_ltq = {
@@ -179,6 +182,39 @@ static const struct dwc2_core_params params_ltq = {
 	.uframe_sched			= -1,
 	.external_id_pin_ctl		= -1,
 	.hibernation			= -1,
+	.stm32_powerdown		= 0,
+};
+
+static const struct dwc2_core_params params_stm32fs = {
+	.otg_cap			= 2,	/* non-HNP/non-SRP */
+	.otg_ver			= -1,
+	.dma_enable			= 0,
+	.dma_desc_enable		= 0,
+	.dma_desc_fs_enable		= 0,
+	.speed				= 1,	/* Full Speed */
+	.enable_dynamic_fifo		= -1,
+	.en_multiple_tx_fifo		= -1,
+	.host_rx_fifo_size		= 128,	/* 128 DWORDs */
+	.host_nperio_tx_fifo_size	= 96,	/* 96 DWORDs */
+	.host_perio_tx_fifo_size	= 96,	/* 96 DWORDs */
+	.max_transfer_size		= -1,
+	.max_packet_count		= 256,
+	.host_channels			= -1,
+	.phy_type			= 0,	/* Full Speed PHY */
+	.phy_utmi_width			= 0,
+	.phy_ulpi_ddr			= 0,
+	.phy_ulpi_ext_vbus		= 0,
+	.i2c_enable			= 0,
+	.ulpi_fs_ls			= 0,
+	.host_support_fs_ls_low_power	= 0,
+	.host_ls_low_power_phy_clk	= 0,
+	.ts_dline			= 0,
+	.reload_ctl			= 1,
+	.ahbcfg				= 0,
+	.uframe_sched			= 0,
+	.external_id_pin_ctl		= 0,
+	.hibernation			= 0,
+	.stm32_powerdown		= 1,
 };
 
 /*
@@ -464,6 +500,7 @@ static const struct of_device_id dwc2_of_match_table[] = {
 	{ .compatible = "lantiq,xrx200-usb", .data = &params_ltq },
 	{ .compatible = "snps,dwc2", .data = NULL },
 	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
+	{ .compatible = "st,stm32-fsotg", .data = &params_stm32fs},
 	{},
 };
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
-- 
2.7.4 (Apple Git-66)

[PATCH 2/3] ARM: dts: STM32 Add USB FS host mode support

[Bruno Herrera]

Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
---
 arch/arm/boot/dts/stm32f429-disco.dts | 30 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/stm32f429.dtsi      | 33 ++++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/stm32f469-disco.dts | 30 ++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/stm32f429-disco.dts b/arch/arm/boot/dts/stm32f429-disco.dts
index 0140807..8e0114b 100644
--- a/arch/arm/boot/dts/stm32f429-disco.dts
+++ b/arch/arm/boot/dts/stm32f429-disco.dts
@@ -75,6 +75,16 @@
 			linux,default-trigger = "heartbeat";
 		};
 	};
+
+	/* This turns on vbus for otg for host mode (dwc2) */
+	vcc5v_otg: vcc5v-otg-regulator {
+		compatible = "regulator-fixed";
+		gpio = <&gpioc 4 0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usbotg_pwren_h>;
+		regulator-name = "vcc5_host1";
+		regulator-always-on;
+	};
 };
 
 &clk_hse {
@@ -86,3 +96,23 @@
 	pinctrl-names = "default";
 	status = "okay";
 };
+
+&usbotg_hs {
+	compatible = "st,stm32-fsotg", "snps,dwc2";
+	dr_mode = "host";
+	pinctrl-0 = <&usbotg_fs_pins_b>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&pinctrl {
+	usb-host {
+		usbotg_pwren_h: usbotg-pwren-h {
+			pins {
+				pinmux = <STM32F429_PC4_FUNC_GPIO>;
+				bias-disable;
+				drive-push-pull;
+			};
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 35df462..cb33aa2 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -176,7 +176,7 @@
 			reg = <0x40013800 0x400>;
 		};
 
-		pin-controller {
+		pinctrl: pin-controller {
 			#address-cells = <1>;
 			#size-cells = <1>;
 			compatible = "st,stm32f429-pinctrl";
@@ -284,6 +284,28 @@
 				};
 			};
 
+			usbotg_fs_pins_a: usbotg_fs@0 {
+				pins {
+					pinmux = <STM32F429_PA10_FUNC_OTG_FS_ID>,
+						 <STM32F429_PA11_FUNC_OTG_FS_DM>,
+						 <STM32F429_PA12_FUNC_OTG_FS_DP>;
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <2>;
+				};
+			};
+
+			usbotg_fs_pins_b: usbotg_fs@1 {
+				pins {
+					pinmux = <STM32F429_PB12_FUNC_OTG_HS_ID>,
+						 <STM32F429_PB14_FUNC_OTG_HS_DM>,
+						 <STM32F429_PB15_FUNC_OTG_HS_DP>;
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <2>;
+				};
+			};
+
 			usbotg_hs_pins_a: usbotg_hs@0 {
 				pins {
 					pinmux = <STM32F429_PH4_FUNC_OTG_HS_ULPI_NXT>,
@@ -388,6 +410,15 @@
 			status = "disabled";
 		};
 
+		usbotg_fs: usb@50000000 {
+			compatible = "st,stm32-fsotg", "snps,dwc2";
+			reg = <0x50000000 0x40000>;
+			interrupts = <67>;
+			clocks = <&rcc 0 39>;
+			clock-names = "otg";
+			status = "disabled";
+		};
+
 		rng: rng@50060800 {
 			compatible = "st,stm32-rng";
 			reg = <0x50060800 0x400>;
diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
index 83ee90d..46e5279 100644
--- a/arch/arm/boot/dts/stm32f469-disco.dts
+++ b/arch/arm/boot/dts/stm32f469-disco.dts
@@ -64,6 +64,17 @@
 	aliases {
 		serial0 = &usart3;
 	};
+
+	/* This turns on vbus for otg for host mode (dwc2) */
+	vcc5v_otg: vcc5v-otg-regulator {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpiob 2 0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usbotg_pwren_h>;
+		regulator-name = "vcc5_host1";
+		regulator-always-on;
+	};
 };
 
 &clk_hse {
@@ -73,3 +84,22 @@
 &usart3 {
 	status = "okay";
 };
+
+&usbotg_fs {
+	dr_mode = "host";
+	pinctrl-0 = <&usbotg_fs_pins_a>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&pinctrl {
+	usb-host {
+		usbotg_pwren_h: usbotg-pwren-h {
+			pins {
+				pinmux = <STM32F429_PB2_FUNC_GPIO>;
+				bias-disable;
+				drive-push-pull;
+			};
+		};
+	};
+};
-- 
2.7.4 (Apple Git-66)

[PATCH 3/3] dt-bindings: Document the STM32 USB OTG DWC2 core binding

[Bruno Herrera]

Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 20a68bf..79e5370 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -11,6 +11,7 @@ Required properties:
   - "lantiq,arx100-usb": The DWC2 USB controller instance in Lantiq ARX SoCs;
   - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
   - snps,dwc2: A generic DWC2 USB controller with default parameters.
+  - st,stm32-fsotg: The DWC2 USB controller instance in STM32F4 SoCs in FS mode;
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
 - clocks: clock provider specifier
-- 
2.7.4 (Apple Git-66)

Re: [PATCH 3/3] dt-bindings: Document the STM32 USB OTG DWC2 core binding

[Rob Herring]

On Tue, Jun 21, 2016 at 11:25:49PM -0300, Bruno Herrera wrote:
> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> index 20a68bf..79e5370 100644
> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> @@ -11,6 +11,7 @@ Required properties:
>    - "lantiq,arx100-usb": The DWC2 USB controller instance in Lantiq ARX SoCs;
>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
> +  - st,stm32-fsotg: The DWC2 USB controller instance in STM32F4 SoCs in FS mode;

This should go above snps,dwc2.

What determines FS mode vs. HS?

>  - reg : Should contain 1 register range (address and length)
>  - interrupts : Should contain 1 interrupt
>  - clocks: clock provider specifier
> -- 
> 2.7.4 (Apple Git-66)
> 

Re: [PATCH 3/3] dt-bindings: Document the STM32 USB OTG DWC2 core binding

[Bruno Herrera]

On Fri, Jun 24, 2016 at 12:41 PM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Jun 21, 2016 at 11:25:49PM -0300, Bruno Herrera wrote:
>> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>> index 20a68bf..79e5370 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>> @@ -11,6 +11,7 @@ Required properties:
>>    - "lantiq,arx100-usb": The DWC2 USB controller instance in Lantiq ARX SoCs;
>>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>> +  - st,stm32-fsotg: The DWC2 USB controller instance in STM32F4 SoCs in FS mode;
>
> This should go above snps,dwc2.
>
Ok, tks!

> What determines FS mode vs. HS?
>
Its more HW design decision.
STM32F429/439/469 has two OTG controllers, one that is FS (internal
phy) and other that is HS (but can also work in FS mode with
internal/external phy)
This bind work with both cores FS and HS working with the internal PHY.

I tested the following configurations:
1 - STM32F429I-DISCOv1 board (OTG HS working in FS mode internal PHY)
2 - STM32F469I-DISCO board (OTG FS)

I did not tested OTG HS core working in FS mode with external PHY (I2C).

>>  - reg : Should contain 1 register range (address and length)
>>  - interrupts : Should contain 1 interrupt
>>  - clocks: clock provider specifier
>> --
>> 2.7.4 (Apple Git-66)
>>

Re: [PATCH 1/3] usb: dwc2: Add support for STM32F429/439/469 USB OTG in FS mode with internal PHY

[John Youn]

On 6/21/2016 7:26 PM, Bruno Herrera wrote:
> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>

Please add a commit message describing the purpose of your changes,
some information about the platform you're adding, and the special
handling of the GGPIO.

> ---
>  drivers/usb/dwc2/core.c     | 18 ++++++++++++++++++
>  drivers/usb/dwc2/core.h     |  5 +++++
>  drivers/usb/dwc2/hcd.c      | 12 +++++++++++-
>  drivers/usb/dwc2/hw.h       |  2 ++
>  drivers/usb/dwc2/platform.c | 37 +++++++++++++++++++++++++++++++++++++
>  5 files changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index 4135a5f..83fbed6 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -1276,6 +1276,23 @@ static void dwc2_set_param_hibernation(struct dwc2_hsotg *hsotg,
>  	hsotg->core_params->hibernation = val;
>  }
>  
> +static void dwc2_set_param_stm32_powerdown(struct dwc2_hsotg *hsotg,
> +		int val)
> +{
> +	if (DWC2_OUT_OF_BOUNDS(val, 0, 1)) {
> +		if (val >= 0) {
> +			dev_err(hsotg->dev,
> +				"'%d' invalid for parameter power down\n",
> +				val);
> +			dev_err(hsotg->dev, "power down must be 0 or 1\n");
> +		}
> +		val = 0;
> +		dev_dbg(hsotg->dev, "Setting power down to %d\n", val);
> +	}
> +
> +	hsotg->core_params->stm32_powerdown = val;
> +}
> +
>  /*
>   * This function is called during module intialization to pass module parameters
>   * for the DWC_otg core.
> @@ -1323,6 +1340,7 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
>  	dwc2_set_param_uframe_sched(hsotg, params->uframe_sched);
>  	dwc2_set_param_external_id_pin_ctl(hsotg, params->external_id_pin_ctl);
>  	dwc2_set_param_hibernation(hsotg, params->hibernation);
> +	dwc2_set_param_stm32_powerdown(hsotg, params->stm32_powerdown);
>  }
>  
>  /*
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 3c58d63..d3e4fcb 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -386,6 +386,10 @@ enum dwc2_ep0_state {
>   *			needed.
>   *			0 - No (default)
>   *			1 - Yes
> + * @stm32_powerdown:	Enable STM32 specific USB FS transceiver power down
> + *			control.
> + *			0 = USB FS transceiver disabled (default)
> + *			1 = USB FS transceiver enabled
>   *
>   * The following parameters may be specified when starting the module. These
>   * parameters define how the DWC_otg controller should be configured. A
> @@ -426,6 +430,7 @@ struct dwc2_core_params {
>  	int uframe_sched;
>  	int external_id_pin_ctl;
>  	int hibernation;
> +	int stm32_powerdown;
>  };
>  
>  /**
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 2df3d04..4f9bb93 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -118,7 +118,7 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg)
>  
>  static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
>  {
> -	u32 usbcfg, i2cctl;
> +	u32 usbcfg, usbgpio, i2cctl;

The convention in this driver would be to just call 'usbgpio' -> 'ggpio'

>  	int retval = 0;
>  
>  	/*
> @@ -142,6 +142,16 @@ static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
>  				return retval;
>  			}
>  		}
> +
> +		if (hsotg->core_params->stm32_powerdown > 0) {
> +			dev_dbg(hsotg->dev, "STM32 FS PHY enabling transceiver\n");
> +			/* STM32 uses the GGPIO register as general core
> +			 * configuration register.
> +			 */
> +			usbgpio = dwc2_readl(hsotg->regs + GGPIO);
> +			usbgpio |= STM32_OTG_GCCFG_PWRDWN;
> +			dwc2_writel(usbgpio, hsotg->regs + GGPIO);
> +		}
>  	}
>  
>  	/*
> diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
> index 281b57b..d5f9294 100644
> --- a/drivers/usb/dwc2/hw.h
> +++ b/drivers/usb/dwc2/hw.h
> @@ -224,6 +224,8 @@
>  
>  #define GPVNDCTL			HSOTG_REG(0x0034)
>  #define GGPIO				HSOTG_REG(0x0038)
> +#define STM32_OTG_GCCFG_PWRDWN		(1 << 16)
> +

Also, bitfields for GGPIO should be named: GGPIO_xxx


>  #define GUID				HSOTG_REG(0x003c)
>  #define GSNPSID				HSOTG_REG(0x0040)
>  #define GHWCFG1				HSOTG_REG(0x0044)
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index fc6f525..d806b94 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -84,6 +84,7 @@ static const struct dwc2_core_params params_hi6220 = {
>  	.uframe_sched			= 0,
>  	.external_id_pin_ctl		= -1,
>  	.hibernation			= -1,
> +	.stm32_powerdown		= 0,

You can probably just leave this out of unrelated configurations since
it will default to 0.

Regards,
John

Re: [PATCH 3/3] dt-bindings: Document the STM32 USB OTG DWC2 core binding

[Rob Herring]

On Fri, Jun 24, 2016 at 03:51:18PM -0300, Bruno Herrera wrote:
> On Fri, Jun 24, 2016 at 12:41 PM, Rob Herring <robh@kernel.org> wrote:
> > On Tue, Jun 21, 2016 at 11:25:49PM -0300, Bruno Herrera wrote:
> >> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
> >> ---
> >>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> index 20a68bf..79e5370 100644
> >> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> >> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> @@ -11,6 +11,7 @@ Required properties:
> >>    - "lantiq,arx100-usb": The DWC2 USB controller instance in Lantiq ARX SoCs;
> >>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
> >>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
> >> +  - st,stm32-fsotg: The DWC2 USB controller instance in STM32F4 SoCs in FS mode;
> >
> > This should go above snps,dwc2.
> >
> Ok, tks!
> 
> > What determines FS mode vs. HS?
> >
> Its more HW design decision.
> STM32F429/439/469 has two OTG controllers, one that is FS (internal
> phy) and other that is HS (but can also work in FS mode with
> internal/external phy)
> This bind work with both cores FS and HS working with the internal PHY.
> 
> I tested the following configurations:
> 1 - STM32F429I-DISCOv1 board (OTG HS working in FS mode internal PHY)
> 2 - STM32F469I-DISCO board (OTG FS)
> 
> I did not tested OTG HS core working in FS mode with external PHY (I2C).

You shouldn't be setting the compatible string based on which mode you 
want. So for the HS block, you need a different compatible string than 
the FS block and set the speed in another way (not sure if we have a 
standard way). Or perhaps the phy should determine the speed.

Rob

Re: [PATCH 3/3] dt-bindings: Document the STM32 USB OTG DWC2 core binding

[Bruno Herrera]

On Tue, Jun 28, 2016 at 5:54 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Jun 24, 2016 at 03:51:18PM -0300, Bruno Herrera wrote:
>> On Fri, Jun 24, 2016 at 12:41 PM, Rob Herring <robh@kernel.org> wrote:
>> > On Tue, Jun 21, 2016 at 11:25:49PM -0300, Bruno Herrera wrote:
>> >> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> index 20a68bf..79e5370 100644
>> >> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> @@ -11,6 +11,7 @@ Required properties:
>> >>    - "lantiq,arx100-usb": The DWC2 USB controller instance in Lantiq ARX SoCs;
>> >>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>> >>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>> >> +  - st,stm32-fsotg: The DWC2 USB controller instance in STM32F4 SoCs in FS mode;
>> >
>> > This should go above snps,dwc2.
>> >
>> Ok, tks!
>>
>> > What determines FS mode vs. HS?
>> >
>> Its more HW design decision.
>> STM32F429/439/469 has two OTG controllers, one that is FS (internal
>> phy) and other that is HS (but can also work in FS mode with
>> internal/external phy)
>> This bind work with both cores FS and HS working with the internal PHY.
>>
>> I tested the following configurations:
>> 1 - STM32F429I-DISCOv1 board (OTG HS working in FS mode internal PHY)
>> 2 - STM32F469I-DISCO board (OTG FS)
>>
>> I did not tested OTG HS core working in FS mode with external PHY (I2C).
>
> You shouldn't be setting the compatible string based on which mode you
> want. So for the HS block, you need a different compatible string than
> the FS block and set the speed in another way (not sure if we have a
> standard way). Or perhaps the phy should determine the speed.

I understand but I dont see how to fix it properly unless we add a
some specific STM32 properties in the DT or in the dwc2_core_params.
In fact there are two cores, and they are different in terms of
functionality (despite of the type of the PHY).
One core is for FS and other is for HS, so they could/should have
different compatible strings because they have different
configurations and are different piece of IP/Hardware(The buffer size
are different, the number of end points and so one, DMA)

But the problem is that the HS core can also support the the FS mode
and in this case is misleading to have a HS core with an FS compatible
string.
Something like that (real case for STM32F429I-DISCO):

&usbotg_hs {
compatible = "st,stm32-fsotg", "snps,dwc2";
dr_mode = "host";
pinctrl-0 = <&usbotg_fs_pins_b>;
pinctrl-names = "default";
status = "okay";
};

Even if the decision is phy based it would lead to a STM32 specific
logic and we would need to figure out how to represent the internal
PHY.

Bruno
>
> Rob

Re: [PATCH 1/3] usb: dwc2: Add support for STM32F429/439/469 USB OTG in FS mode with internal PHY

[Bruno Herrera]

On Mon, Jun 27, 2016 at 7:51 PM, John Youn <John.Youn@synopsys.com> wrote:
> On 6/21/2016 7:26 PM, Bruno Herrera wrote:
>> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
>
> Please add a commit message describing the purpose of your changes,
> some information about the platform you're adding, and the special
> handling of the GGPIO.
>
Ok, no problem.

>> ---
>>  drivers/usb/dwc2/core.c     | 18 ++++++++++++++++++
>>  drivers/usb/dwc2/core.h     |  5 +++++
>>  drivers/usb/dwc2/hcd.c      | 12 +++++++++++-
>>  drivers/usb/dwc2/hw.h       |  2 ++
>>  drivers/usb/dwc2/platform.c | 37 +++++++++++++++++++++++++++++++++++++
>>  5 files changed, 73 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
>> index 4135a5f..83fbed6 100644
>> --- a/drivers/usb/dwc2/core.c
>> +++ b/drivers/usb/dwc2/core.c
>> @@ -1276,6 +1276,23 @@ static void dwc2_set_param_hibernation(struct dwc2_hsotg *hsotg,
>>       hsotg->core_params->hibernation = val;
>>  }
>>
>> +static void dwc2_set_param_stm32_powerdown(struct dwc2_hsotg *hsotg,
>> +             int val)
>> +{
>> +     if (DWC2_OUT_OF_BOUNDS(val, 0, 1)) {
>> +             if (val >= 0) {
>> +                     dev_err(hsotg->dev,
>> +                             "'%d' invalid for parameter power down\n",
>> +                             val);
>> +                     dev_err(hsotg->dev, "power down must be 0 or 1\n");
>> +             }
>> +             val = 0;
>> +             dev_dbg(hsotg->dev, "Setting power down to %d\n", val);
>> +     }
>> +
>> +     hsotg->core_params->stm32_powerdown = val;
>> +}
>> +
>>  /*
>>   * This function is called during module intialization to pass module parameters
>>   * for the DWC_otg core.
>> @@ -1323,6 +1340,7 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
>>       dwc2_set_param_uframe_sched(hsotg, params->uframe_sched);
>>       dwc2_set_param_external_id_pin_ctl(hsotg, params->external_id_pin_ctl);
>>       dwc2_set_param_hibernation(hsotg, params->hibernation);
>> +     dwc2_set_param_stm32_powerdown(hsotg, params->stm32_powerdown);
>>  }
>>
>>  /*
>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>> index 3c58d63..d3e4fcb 100644
>> --- a/drivers/usb/dwc2/core.h
>> +++ b/drivers/usb/dwc2/core.h
>> @@ -386,6 +386,10 @@ enum dwc2_ep0_state {
>>   *                   needed.
>>   *                   0 - No (default)
>>   *                   1 - Yes
>> + * @stm32_powerdown: Enable STM32 specific USB FS transceiver power down
>> + *                   control.
>> + *                   0 = USB FS transceiver disabled (default)
>> + *                   1 = USB FS transceiver enabled
>>   *
>>   * The following parameters may be specified when starting the module. These
>>   * parameters define how the DWC_otg controller should be configured. A
>> @@ -426,6 +430,7 @@ struct dwc2_core_params {
>>       int uframe_sched;
>>       int external_id_pin_ctl;
>>       int hibernation;
>> +     int stm32_powerdown;
>>  };
>>
>>  /**
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index 2df3d04..4f9bb93 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -118,7 +118,7 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg)
>>
>>  static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
>>  {
>> -     u32 usbcfg, i2cctl;
>> +     u32 usbcfg, usbgpio, i2cctl;
>
> The convention in this driver would be to just call 'usbgpio' -> 'ggpio'

OK.
>
>>       int retval = 0;
>>
>>       /*
>> @@ -142,6 +142,16 @@ static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
>>                               return retval;
>>                       }
>>               }
>> +
>> +             if (hsotg->core_params->stm32_powerdown > 0) {
>> +                     dev_dbg(hsotg->dev, "STM32 FS PHY enabling transceiver\n");
>> +                     /* STM32 uses the GGPIO register as general core
>> +                      * configuration register.
>> +                      */
>> +                     usbgpio = dwc2_readl(hsotg->regs + GGPIO);
>> +                     usbgpio |= STM32_OTG_GCCFG_PWRDWN;
>> +                     dwc2_writel(usbgpio, hsotg->regs + GGPIO);
>> +             }
>>       }
>>
>>       /*
>> diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
>> index 281b57b..d5f9294 100644
>> --- a/drivers/usb/dwc2/hw.h
>> +++ b/drivers/usb/dwc2/hw.h
>> @@ -224,6 +224,8 @@
>>
>>  #define GPVNDCTL                     HSOTG_REG(0x0034)
>>  #define GGPIO                                HSOTG_REG(0x0038)
>> +#define STM32_OTG_GCCFG_PWRDWN               (1 << 16)
>> +
>
> Also, bitfields for GGPIO should be named: GGPIO_xxx

Could it be -> GGPIO_STM32_OTG_GCCFG_PWRDWN
>From my understand the GGPIO are platform/vendor dependent so in the
future some other vendor could use the same bit for other purpose.
Should we keep it defined here or only in the scope it is used?

>
>
>>  #define GUID                         HSOTG_REG(0x003c)
>>  #define GSNPSID                              HSOTG_REG(0x0040)
>>  #define GHWCFG1                              HSOTG_REG(0x0044)
>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>> index fc6f525..d806b94 100644
>> --- a/drivers/usb/dwc2/platform.c
>> +++ b/drivers/usb/dwc2/platform.c
>> @@ -84,6 +84,7 @@ static const struct dwc2_core_params params_hi6220 = {
>>       .uframe_sched                   = 0,
>>       .external_id_pin_ctl            = -1,
>>       .hibernation                    = -1,
>> +     .stm32_powerdown                = 0,
>
> You can probably just leave this out of unrelated configurations since
> it will default to 0.
OK.
>
> Regards,
> John

Re: [PATCH 1/3] usb: dwc2: Add support for STM32F429/439/469 USB OTG in FS mode with internal PHY

[John Youn]

On 6/29/2016 12:47 PM, Bruno Herrera wrote:
> On Mon, Jun 27, 2016 at 7:51 PM, John Youn <John.Youn@synopsys.com> wrote:
>> On 6/21/2016 7:26 PM, Bruno Herrera wrote:
>>> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
>>
>> Please add a commit message describing the purpose of your changes,
>> some information about the platform you're adding, and the special
>> handling of the GGPIO.
>>
> Ok, no problem.
> 
>>> ---
>>>  drivers/usb/dwc2/core.c     | 18 ++++++++++++++++++
>>>  drivers/usb/dwc2/core.h     |  5 +++++
>>>  drivers/usb/dwc2/hcd.c      | 12 +++++++++++-
>>>  drivers/usb/dwc2/hw.h       |  2 ++
>>>  drivers/usb/dwc2/platform.c | 37 +++++++++++++++++++++++++++++++++++++
>>>  5 files changed, 73 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
>>> index 4135a5f..83fbed6 100644
>>> --- a/drivers/usb/dwc2/core.c
>>> +++ b/drivers/usb/dwc2/core.c
>>> @@ -1276,6 +1276,23 @@ static void dwc2_set_param_hibernation(struct dwc2_hsotg *hsotg,
>>>       hsotg->core_params->hibernation = val;
>>>  }
>>>
>>> +static void dwc2_set_param_stm32_powerdown(struct dwc2_hsotg *hsotg,
>>> +             int val)
>>> +{
>>> +     if (DWC2_OUT_OF_BOUNDS(val, 0, 1)) {
>>> +             if (val >= 0) {
>>> +                     dev_err(hsotg->dev,
>>> +                             "'%d' invalid for parameter power down\n",
>>> +                             val);
>>> +                     dev_err(hsotg->dev, "power down must be 0 or 1\n");
>>> +             }
>>> +             val = 0;
>>> +             dev_dbg(hsotg->dev, "Setting power down to %d\n", val);
>>> +     }
>>> +
>>> +     hsotg->core_params->stm32_powerdown = val;
>>> +}
>>> +
>>>  /*
>>>   * This function is called during module intialization to pass module parameters
>>>   * for the DWC_otg core.
>>> @@ -1323,6 +1340,7 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
>>>       dwc2_set_param_uframe_sched(hsotg, params->uframe_sched);
>>>       dwc2_set_param_external_id_pin_ctl(hsotg, params->external_id_pin_ctl);
>>>       dwc2_set_param_hibernation(hsotg, params->hibernation);
>>> +     dwc2_set_param_stm32_powerdown(hsotg, params->stm32_powerdown);
>>>  }
>>>
>>>  /*
>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>>> index 3c58d63..d3e4fcb 100644
>>> --- a/drivers/usb/dwc2/core.h
>>> +++ b/drivers/usb/dwc2/core.h
>>> @@ -386,6 +386,10 @@ enum dwc2_ep0_state {
>>>   *                   needed.
>>>   *                   0 - No (default)
>>>   *                   1 - Yes
>>> + * @stm32_powerdown: Enable STM32 specific USB FS transceiver power down
>>> + *                   control.
>>> + *                   0 = USB FS transceiver disabled (default)
>>> + *                   1 = USB FS transceiver enabled
>>>   *
>>>   * The following parameters may be specified when starting the module. These
>>>   * parameters define how the DWC_otg controller should be configured. A
>>> @@ -426,6 +430,7 @@ struct dwc2_core_params {
>>>       int uframe_sched;
>>>       int external_id_pin_ctl;
>>>       int hibernation;
>>> +     int stm32_powerdown;
>>>  };
>>>
>>>  /**
>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>> index 2df3d04..4f9bb93 100644
>>> --- a/drivers/usb/dwc2/hcd.c
>>> +++ b/drivers/usb/dwc2/hcd.c
>>> @@ -118,7 +118,7 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg)
>>>
>>>  static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
>>>  {
>>> -     u32 usbcfg, i2cctl;
>>> +     u32 usbcfg, usbgpio, i2cctl;
>>
>> The convention in this driver would be to just call 'usbgpio' -> 'ggpio'
> 
> OK.
>>
>>>       int retval = 0;
>>>
>>>       /*
>>> @@ -142,6 +142,16 @@ static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
>>>                               return retval;
>>>                       }
>>>               }
>>> +
>>> +             if (hsotg->core_params->stm32_powerdown > 0) {
>>> +                     dev_dbg(hsotg->dev, "STM32 FS PHY enabling transceiver\n");
>>> +                     /* STM32 uses the GGPIO register as general core
>>> +                      * configuration register.
>>> +                      */
>>> +                     usbgpio = dwc2_readl(hsotg->regs + GGPIO);
>>> +                     usbgpio |= STM32_OTG_GCCFG_PWRDWN;
>>> +                     dwc2_writel(usbgpio, hsotg->regs + GGPIO);
>>> +             }
>>>       }
>>>
>>>       /*
>>> diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
>>> index 281b57b..d5f9294 100644
>>> --- a/drivers/usb/dwc2/hw.h
>>> +++ b/drivers/usb/dwc2/hw.h
>>> @@ -224,6 +224,8 @@
>>>
>>>  #define GPVNDCTL                     HSOTG_REG(0x0034)
>>>  #define GGPIO                                HSOTG_REG(0x0038)
>>> +#define STM32_OTG_GCCFG_PWRDWN               (1 << 16)
>>> +
>>
>> Also, bitfields for GGPIO should be named: GGPIO_xxx
> 
> Could it be -> GGPIO_STM32_OTG_GCCFG_PWRDWN
> From my understand the GGPIO are platform/vendor dependent so in the
> future some other vendor could use the same bit for other purpose.
> Should we keep it defined here or only in the scope it is used?

Here is fine. We already have overlapping bitfield definitions for
some registers due to differences in host or device mode.

Regards,
John

Re: [PATCH 3/3] dt-bindings: Document the STM32 USB OTG DWC2 core binding

[Rob Herring]

On Wed, Jun 29, 2016 at 03:56:23PM -0300, Bruno Herrera wrote:
> On Tue, Jun 28, 2016 at 5:54 PM, Rob Herring <robh@kernel.org> wrote:
> > On Fri, Jun 24, 2016 at 03:51:18PM -0300, Bruno Herrera wrote:
> >> On Fri, Jun 24, 2016 at 12:41 PM, Rob Herring <robh@kernel.org> wrote:
> >> > On Tue, Jun 21, 2016 at 11:25:49PM -0300, Bruno Herrera wrote:
> >> >> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
> >> >> ---
> >> >>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
> >> >>  1 file changed, 1 insertion(+)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> >> index 20a68bf..79e5370 100644
> >> >> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> >> >> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> >> @@ -11,6 +11,7 @@ Required properties:
> >> >>    - "lantiq,arx100-usb": The DWC2 USB controller instance in Lantiq ARX SoCs;
> >> >>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
> >> >>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
> >> >> +  - st,stm32-fsotg: The DWC2 USB controller instance in STM32F4 SoCs in FS mode;
> >> >
> >> > This should go above snps,dwc2.
> >> >
> >> Ok, tks!
> >>
> >> > What determines FS mode vs. HS?
> >> >
> >> Its more HW design decision.
> >> STM32F429/439/469 has two OTG controllers, one that is FS (internal
> >> phy) and other that is HS (but can also work in FS mode with
> >> internal/external phy)
> >> This bind work with both cores FS and HS working with the internal PHY.
> >>
> >> I tested the following configurations:
> >> 1 - STM32F429I-DISCOv1 board (OTG HS working in FS mode internal PHY)
> >> 2 - STM32F469I-DISCO board (OTG FS)
> >>
> >> I did not tested OTG HS core working in FS mode with external PHY (I2C).
> >
> > You shouldn't be setting the compatible string based on which mode you
> > want. So for the HS block, you need a different compatible string than
> > the FS block and set the speed in another way (not sure if we have a
> > standard way). Or perhaps the phy should determine the speed.
> 
> I understand but I dont see how to fix it properly unless we add a
> some specific STM32 properties in the DT or in the dwc2_core_params.
> In fact there are two cores, and they are different in terms of
> functionality (despite of the type of the PHY).
> One core is for FS and other is for HS, so they could/should have
> different compatible strings because they have different
> configurations and are different piece of IP/Hardware(The buffer size
> are different, the number of end points and so one, DMA)

Then it is perfectly fine to have 2 compatible strings: one for the FS 
block and one for the HS block.

> 
> But the problem is that the HS core can also support the the FS mode
> and in this case is misleading to have a HS core with an FS compatible
> string.

But as you said, there are other differences in the 2 blocks, so just 
changing to the FS compatible string would not work.

You could just have a high-speed-disable property or something, but look 
thru existing bindings and see if one exists already. If not, propose a 
common one.

> Something like that (real case for STM32F429I-DISCO):
> 
> &usbotg_hs {
> compatible = "st,stm32-fsotg", "snps,dwc2";
> dr_mode = "host";
> pinctrl-0 = <&usbotg_fs_pins_b>;
> pinctrl-names = "default";
> status = "okay";
> };
> 
> Even if the decision is phy based it would lead to a STM32 specific
> logic and we would need to figure out how to represent the internal
> PHY.

Most SoCs end up describing the phys at least for USB 2 and 3 as they 
vary a lot eventhough everyone licenses one of the few IP blocks.

Rob