[PATCH 0/3] usb: dwc3: keystone: Convert binding to YAML

[PATCH 0/3] usb: dwc3: keystone: Convert binding to YAML

[Roger Quadros]

Hi Felipe,

This series converts keystone-usb.txt to YAML and prepares
for Super-Speed support for AM654 SoC.

cheers,
-roger

Roger Quadros (3):
  dt-bindings: usb: convert keystone-usb.txt to YAML
  dt-bindings: usb: ti,keystone-dwc3.yaml: Add USB3.0 PHY property
  usb: dwc3: keystone: Turn on USB3 PHY before controller

 .../devicetree/bindings/usb/keystone-usb.txt  | 56 ------------
 .../bindings/usb/ti,keystone-dwc3.yaml        | 88 +++++++++++++++++++
 drivers/usb/dwc3/dwc3-keystone.c              | 47 +++++++++-
 3 files changed, 134 insertions(+), 57 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt
 create mode 100644 Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 1/3] dt-bindings: usb: convert keystone-usb.txt to YAML

[Roger Quadros]

Convert keystone-usb documentation to YAML format.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../devicetree/bindings/usb/keystone-usb.txt  | 56 ----------------
 .../bindings/usb/ti,keystone-dwc3.yaml        | 67 +++++++++++++++++++
 2 files changed, 67 insertions(+), 56 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt
 create mode 100644 Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml

diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt
deleted file mode 100644
index 77df82e36138..000000000000
--- a/Documentation/devicetree/bindings/usb/keystone-usb.txt
+++ /dev/null
@@ -1,56 +0,0 @@
-TI Keystone Soc USB Controller
-
-DWC3 GLUE
-
-Required properties:
- - compatible: should be
-		"ti,keystone-dwc3" for Keystone 2 SoCs
-		"ti,am654-dwc3" for AM654 SoC
- - #address-cells, #size-cells : should be '1' if the device has sub-nodes
-   with 'reg' property.
- - reg : Address and length of the register set for the USB subsystem on
-   the SOC.
- - interrupts : The irq number of this device that is used to interrupt the
-   MPU.
- - ranges: allows valid 1:1 translation between child's address space and
-   parent's address space.
-
-SoC-specific Required Properties:
-The following are mandatory properties for Keystone 2 66AK2HK, 66AK2L and 66AK2E
-SoCs only:
-
-- clocks:		Clock ID for USB functional clock.
-- clock-names:		Must be "usb".
-
-
-The following are mandatory properties for 66AK2G and AM654:
-
-- power-domains:	Should contain a phandle to a PM domain provider node
-			and an args specifier containing the USB device id
-			value. This property is as per the binding,
-			Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
-
-Sub-nodes:
-The dwc3 core should be added as subnode to Keystone DWC3 glue.
-- dwc3 :
-   The binding details of dwc3 can be found in:
-   Documentation/devicetree/bindings/usb/dwc3.txt
-
-Example:
-	usb: usb@2680000 {
-		compatible = "ti,keystone-dwc3";
-		#address-cells = <1>;
-		#size-cells = <1>;
-		reg = <0x2680000 0x10000>;
-		clocks = <&clkusb>;
-		clock-names = "usb";
-		interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
-		ranges;
-
-		dwc3@2690000 {
-			compatible = "synopsys,dwc3";
-			reg = <0x2690000 0x70000>;
-			interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
-			usb-phy = <&usb_phy>, <&usb_phy>;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
new file mode 100644
index 000000000000..14d2fe329b93
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/ti,keystone-dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI Keystone Soc USB Controller
+
+maintainers:
+  - Roger Quadros <rogerq@ti.com>
+
+properties:
+  compatible:
+    oneOf:
+      - const: "ti,keystone-dwc3"
+      - const: "ti,am654-dwc3"
+
+  reg:
+    maxItems: 1
+    description: Address and length of the register set for the USB subsystem on
+      the SOC.
+
+  interrupts:
+    maxItems: 1
+    description: The irq number of this device that is used to interrupt the MPU.
+
+
+  clocks:
+    description: Clock ID for USB functional clock.
+
+  power-domains:
+    description: Should contain a phandle to a PM domain provider node
+      and an args specifier containing the USB device id
+      value. This property is as per the binding,
+      Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
+
+  dwc3:
+    description: This is the node representing the DWC3 controller instance
+      Documentation/devicetree/bindings/usb/dwc3.txt
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    usb: usb@2680000 {
+      compatible = "ti,keystone-dwc3";
+      #address-cells = <1>;
+      #size-cells = <1>;
+      reg = <0x2680000 0x10000>;
+      clocks = <&clkusb>;
+      clock-names = "usb";
+      interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
+      ranges;
+
+      dwc3@2690000 {
+        compatible = "synopsys,dwc3";
+        reg = <0x2690000 0x70000>;
+        interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
+        usb-phy = <&usb_phy>, <&usb_phy>;
+      };
+    };
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 2/3] dt-bindings: usb: ti,keystone-dwc3.yaml: Add USB3.0 PHY property

[Roger Quadros]

AM654 SoC requires USB3.0 PHY to be turned on before the USB controller.
For this SoC, the 'phys' property is used to provide the USB3.0
reference.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../bindings/usb/ti,keystone-dwc3.yaml        | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
index 14d2fe329b93..51c0ab01b0bc 100644
--- a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
@@ -34,6 +34,16 @@ properties:
       value. This property is as per the binding,
       Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
 
+  phys:
+    description:
+      PHY specifier for the USB3.0 PHY. Some SoCs need the USB3.0 PHY
+      to be turned on before the controller.
+      Documentation/devicetree/bindings/phy/phy-bindings.txt
+
+  phy-names:
+    items:
+      - const: "usb3-phy"
+
   dwc3:
     description: This is the node representing the DWC3 controller instance
       Documentation/devicetree/bindings/usb/dwc3.txt
@@ -44,6 +54,17 @@ required:
   - interrupts
   - clocks
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: "ti,am654-dwc3"
+
+then:
+  required:
+    - phys
+    - phy-names
+
 examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

[Roger Quadros]

The Local Power Sleep Controller (LPSC) dependency on AM65
requires SERDES0 to be powered on before USB.

We need to power up SERDES0 power domain and hold it on
throughout the reset, init, power on sequence.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
index 1e14a6f4884b..46d46f3507fc 100644
--- a/drivers/usb/dwc3/dwc3-keystone.c
+++ b/drivers/usb/dwc3/dwc3-keystone.c
@@ -14,6 +14,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/io.h>
 #include <linux/of_platform.h>
+#include <linux/phy/phy.h>
 #include <linux/pm_runtime.h>
 
 /* USBSS register offsets */
@@ -34,6 +35,7 @@
 struct dwc3_keystone {
 	struct device			*dev;
 	void __iomem			*usbss;
+	struct phy			*usb3_phy;
 };
 
 static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
@@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
 	if (IS_ERR(kdwc->usbss))
 		return PTR_ERR(kdwc->usbss);
 
-	pm_runtime_enable(kdwc->dev);
+	/* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
+	kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
+	if (IS_ERR(kdwc->usb3_phy)) {
+		error = PTR_ERR(kdwc->usb3_phy);
+		if (error == -ENOSYS || error == -ENODEV) {
+			kdwc->usb3_phy = NULL;
+		} else {
+			if (error != -EPROBE_DEFER) {
+				dev_err(dev, "couldn't get usb3 phy: %d\n",
+					error);
+			}
+
+			return error;
+		}
+	}
+
+	phy_pm_runtime_get_sync(kdwc->usb3_phy);
+
+	error = phy_reset(kdwc->usb3_phy);
+	if (error < 0) {
+		dev_err(dev, "usb3 phy reset failed: %d\n", error);
+		return error;
+	}
+
+	error = phy_init(kdwc->usb3_phy);
+	if (error < 0) {
+		dev_err(dev, "usb3 phy init failed: %d\n", error);
+		return error;
+	}
 
+	error = phy_power_on(kdwc->usb3_phy);
+	if (error < 0) {
+		dev_err(dev, "usb3 phy power on failed: %d\n", error);
+		phy_exit(kdwc->usb3_phy);
+		return error;
+	}
+
+	pm_runtime_enable(kdwc->dev);
 	error = pm_runtime_get_sync(kdwc->dev);
 	if (error < 0) {
 		dev_err(kdwc->dev, "pm_runtime_get_sync failed, error %d\n",
@@ -138,6 +176,9 @@ static int kdwc3_probe(struct platform_device *pdev)
 err_irq:
 	pm_runtime_put_sync(kdwc->dev);
 	pm_runtime_disable(kdwc->dev);
+	phy_power_off(kdwc->usb3_phy);
+	phy_exit(kdwc->usb3_phy);
+	phy_pm_runtime_put_sync(kdwc->usb3_phy);
 
 	return error;
 }
@@ -163,6 +204,10 @@ static int kdwc3_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(kdwc->dev);
 	pm_runtime_disable(kdwc->dev);
 
+	phy_power_off(kdwc->usb3_phy);
+	phy_exit(kdwc->usb3_phy);
+	phy_pm_runtime_put_sync(kdwc->usb3_phy);
+
 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

[Chunfeng Yun]

On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
> The Local Power Sleep Controller (LPSC) dependency on AM65
> requires SERDES0 to be powered on before USB.
> 
> We need to power up SERDES0 power domain and hold it on
> throughout the reset, init, power on sequence.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
> index 1e14a6f4884b..46d46f3507fc 100644
> --- a/drivers/usb/dwc3/dwc3-keystone.c
> +++ b/drivers/usb/dwc3/dwc3-keystone.c
> @@ -14,6 +14,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/io.h>
>  #include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
>  #include <linux/pm_runtime.h>
>  
>  /* USBSS register offsets */
> @@ -34,6 +35,7 @@
>  struct dwc3_keystone {
>  	struct device			*dev;
>  	void __iomem			*usbss;
> +	struct phy			*usb3_phy;
>  };
>  
>  static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>  	if (IS_ERR(kdwc->usbss))
>  		return PTR_ERR(kdwc->usbss);
>  
> -	pm_runtime_enable(kdwc->dev);
> +	/* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
> +	kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
Use devm_phy_optional_get() instead?

> +	if (IS_ERR(kdwc->usb3_phy)) {
> +		error = PTR_ERR(kdwc->usb3_phy);
> +		if (error == -ENOSYS || error == -ENODEV) {
> +			kdwc->usb3_phy = NULL;
> +		} else {
> +			if (error != -EPROBE_DEFER) {
> +				dev_err(dev, "couldn't get usb3 phy: %d\n",
> +					error);
> +			}
> +
> +			return error;
> +		}
> +	}
> +
> +	phy_pm_runtime_get_sync(kdwc->usb3_phy);
> +
> +	error = phy_reset(kdwc->usb3_phy);
> +	if (error < 0) {
> +		dev_err(dev, "usb3 phy reset failed: %d\n", error);
> +		return error;
> +	}
> +
> +	error = phy_init(kdwc->usb3_phy);
> +	if (error < 0) {
> +		dev_err(dev, "usb3 phy init failed: %d\n", error);
> +		return error;
> +	}
>  
> +	error = phy_power_on(kdwc->usb3_phy);
> +	if (error < 0) {
> +		dev_err(dev, "usb3 phy power on failed: %d\n", error);
> +		phy_exit(kdwc->usb3_phy);
> +		return error;
> +	}
> +
> +	pm_runtime_enable(kdwc->dev);
>  	error = pm_runtime_get_sync(kdwc->dev);
>  	if (error < 0) {
>  		dev_err(kdwc->dev, "pm_runtime_get_sync failed, error %d\n",
> @@ -138,6 +176,9 @@ static int kdwc3_probe(struct platform_device *pdev)
>  err_irq:
>  	pm_runtime_put_sync(kdwc->dev);
>  	pm_runtime_disable(kdwc->dev);
> +	phy_power_off(kdwc->usb3_phy);
> +	phy_exit(kdwc->usb3_phy);
> +	phy_pm_runtime_put_sync(kdwc->usb3_phy);
>  
>  	return error;
>  }
> @@ -163,6 +204,10 @@ static int kdwc3_remove(struct platform_device *pdev)
>  	pm_runtime_put_sync(kdwc->dev);
>  	pm_runtime_disable(kdwc->dev);
>  
> +	phy_power_off(kdwc->usb3_phy);
> +	phy_exit(kdwc->usb3_phy);
> +	phy_pm_runtime_put_sync(kdwc->usb3_phy);
> +
>  	platform_set_drvdata(pdev, NULL);
>  
>  	return 0;

Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

[Roger Quadros]

On 14/05/2020 04:37, Chunfeng Yun wrote:
> On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
>> The Local Power Sleep Controller (LPSC) dependency on AM65
>> requires SERDES0 to be powered on before USB.
>>
>> We need to power up SERDES0 power domain and hold it on
>> throughout the reset, init, power on sequence.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>>   1 file changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
>> index 1e14a6f4884b..46d46f3507fc 100644
>> --- a/drivers/usb/dwc3/dwc3-keystone.c
>> +++ b/drivers/usb/dwc3/dwc3-keystone.c
>> @@ -14,6 +14,7 @@
>>   #include <linux/dma-mapping.h>
>>   #include <linux/io.h>
>>   #include <linux/of_platform.h>
>> +#include <linux/phy/phy.h>
>>   #include <linux/pm_runtime.h>
>>   
>>   /* USBSS register offsets */
>> @@ -34,6 +35,7 @@
>>   struct dwc3_keystone {
>>   	struct device			*dev;
>>   	void __iomem			*usbss;
>> +	struct phy			*usb3_phy;
>>   };
>>   
>>   static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
>> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>>   	if (IS_ERR(kdwc->usbss))
>>   		return PTR_ERR(kdwc->usbss);
>>   
>> -	pm_runtime_enable(kdwc->dev);
>> +	/* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
>> +	kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
> Use devm_phy_optional_get() instead?

Indeed, it seems better suited.

cheers,
-roger

> 
>> +	if (IS_ERR(kdwc->usb3_phy)) {
>> +		error = PTR_ERR(kdwc->usb3_phy);
>> +		if (error == -ENOSYS || error == -ENODEV) {
>> +			kdwc->usb3_phy = NULL;
>> +		} else {
>> +			if (error != -EPROBE_DEFER) {
>> +				dev_err(dev, "couldn't get usb3 phy: %d\n",
>> +					error);
>> +			}
>> +
>> +			return error;
>> +		}
>> +	}
>> +
>> +	phy_pm_runtime_get_sync(kdwc->usb3_phy);
>> +
>> +	error = phy_reset(kdwc->usb3_phy);
>> +	if (error < 0) {
>> +		dev_err(dev, "usb3 phy reset failed: %d\n", error);
>> +		return error;
>> +	}
>> +
>> +	error = phy_init(kdwc->usb3_phy);
>> +	if (error < 0) {
>> +		dev_err(dev, "usb3 phy init failed: %d\n", error);
>> +		return error;
>> +	}
>>   
>> +	error = phy_power_on(kdwc->usb3_phy);
>> +	if (error < 0) {
>> +		dev_err(dev, "usb3 phy power on failed: %d\n", error);
>> +		phy_exit(kdwc->usb3_phy);
>> +		return error;
>> +	}
>> +
>> +	pm_runtime_enable(kdwc->dev);
>>   	error = pm_runtime_get_sync(kdwc->dev);
>>   	if (error < 0) {
>>   		dev_err(kdwc->dev, "pm_runtime_get_sync failed, error %d\n",
>> @@ -138,6 +176,9 @@ static int kdwc3_probe(struct platform_device *pdev)
>>   err_irq:
>>   	pm_runtime_put_sync(kdwc->dev);
>>   	pm_runtime_disable(kdwc->dev);
>> +	phy_power_off(kdwc->usb3_phy);
>> +	phy_exit(kdwc->usb3_phy);
>> +	phy_pm_runtime_put_sync(kdwc->usb3_phy);
>>   
>>   	return error;
>>   }
>> @@ -163,6 +204,10 @@ static int kdwc3_remove(struct platform_device *pdev)
>>   	pm_runtime_put_sync(kdwc->dev);
>>   	pm_runtime_disable(kdwc->dev);
>>   
>> +	phy_power_off(kdwc->usb3_phy);
>> +	phy_exit(kdwc->usb3_phy);
>> +	phy_pm_runtime_put_sync(kdwc->usb3_phy);
>> +
>>   	platform_set_drvdata(pdev, NULL);
>>   
>>   	return 0;
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]

Roger Quadros <rogerq@ti.com> writes:

> On 14/05/2020 04:37, Chunfeng Yun wrote:
>> On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
>>> The Local Power Sleep Controller (LPSC) dependency on AM65
>>> requires SERDES0 to be powered on before USB.
>>>
>>> We need to power up SERDES0 power domain and hold it on
>>> throughout the reset, init, power on sequence.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>   drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>>>   1 file changed, 46 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
>>> index 1e14a6f4884b..46d46f3507fc 100644
>>> --- a/drivers/usb/dwc3/dwc3-keystone.c
>>> +++ b/drivers/usb/dwc3/dwc3-keystone.c
>>> @@ -14,6 +14,7 @@
>>>   #include <linux/dma-mapping.h>
>>>   #include <linux/io.h>
>>>   #include <linux/of_platform.h>
>>> +#include <linux/phy/phy.h>
>>>   #include <linux/pm_runtime.h>
>>>   
>>>   /* USBSS register offsets */
>>> @@ -34,6 +35,7 @@
>>>   struct dwc3_keystone {
>>>   	struct device			*dev;
>>>   	void __iomem			*usbss;
>>> +	struct phy			*usb3_phy;
>>>   };
>>>   
>>>   static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
>>> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>>>   	if (IS_ERR(kdwc->usbss))
>>>   		return PTR_ERR(kdwc->usbss);
>>>   
>>> -	pm_runtime_enable(kdwc->dev);
>>> +	/* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
>>> +	kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
>> Use devm_phy_optional_get() instead?
>
> Indeed, it seems better suited.

patches 1 and 2 are in testing/next

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

[Roger Quadros]

Felipe,

On 14/05/2020 13:21, Felipe Balbi wrote:
> Roger Quadros <rogerq@ti.com> writes:
> 
>> On 14/05/2020 04:37, Chunfeng Yun wrote:
>>> On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
>>>> The Local Power Sleep Controller (LPSC) dependency on AM65
>>>> requires SERDES0 to be powered on before USB.
>>>>
>>>> We need to power up SERDES0 power domain and hold it on
>>>> throughout the reset, init, power on sequence.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>>    drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>>>>    1 file changed, 46 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
>>>> index 1e14a6f4884b..46d46f3507fc 100644
>>>> --- a/drivers/usb/dwc3/dwc3-keystone.c
>>>> +++ b/drivers/usb/dwc3/dwc3-keystone.c
>>>> @@ -14,6 +14,7 @@
>>>>    #include <linux/dma-mapping.h>
>>>>    #include <linux/io.h>
>>>>    #include <linux/of_platform.h>
>>>> +#include <linux/phy/phy.h>
>>>>    #include <linux/pm_runtime.h>
>>>>    
>>>>    /* USBSS register offsets */
>>>> @@ -34,6 +35,7 @@
>>>>    struct dwc3_keystone {
>>>>    	struct device			*dev;
>>>>    	void __iomem			*usbss;
>>>> +	struct phy			*usb3_phy;
>>>>    };
>>>>    
>>>>    static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
>>>> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>>>>    	if (IS_ERR(kdwc->usbss))
>>>>    		return PTR_ERR(kdwc->usbss);
>>>>    
>>>> -	pm_runtime_enable(kdwc->dev);
>>>> +	/* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
>>>> +	kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
>>> Use devm_phy_optional_get() instead?
>>
>> Indeed, it seems better suited.
> 
> patches 1 and 2 are in testing/next
> 

Could you please drop them as I need to make changes to make the PHY optional.
I will send v2 of entire series.

cheers,
-roger
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

[Roger Quadros]

On 25/05/2020 09:39, Roger Quadros wrote:
> Felipe,
> 
> On 14/05/2020 13:21, Felipe Balbi wrote:
>> Roger Quadros <rogerq@ti.com> writes:
>>
>>> On 14/05/2020 04:37, Chunfeng Yun wrote:
>>>> On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
>>>>> The Local Power Sleep Controller (LPSC) dependency on AM65
>>>>> requires SERDES0 to be powered on before USB.
>>>>>
>>>>> We need to power up SERDES0 power domain and hold it on
>>>>> throughout the reset, init, power on sequence.
>>>>>
>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>> ---
>>>>>    drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>>>>>    1 file changed, 46 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
>>>>> index 1e14a6f4884b..46d46f3507fc 100644
>>>>> --- a/drivers/usb/dwc3/dwc3-keystone.c
>>>>> +++ b/drivers/usb/dwc3/dwc3-keystone.c
>>>>> @@ -14,6 +14,7 @@
>>>>>    #include <linux/dma-mapping.h>
>>>>>    #include <linux/io.h>
>>>>>    #include <linux/of_platform.h>
>>>>> +#include <linux/phy/phy.h>
>>>>>    #include <linux/pm_runtime.h>
>>>>>    /* USBSS register offsets */
>>>>> @@ -34,6 +35,7 @@
>>>>>    struct dwc3_keystone {
>>>>>        struct device            *dev;
>>>>>        void __iomem            *usbss;
>>>>> +    struct phy            *usb3_phy;
>>>>>    };
>>>>>    static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
>>>>> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>>>>>        if (IS_ERR(kdwc->usbss))
>>>>>            return PTR_ERR(kdwc->usbss);
>>>>> -    pm_runtime_enable(kdwc->dev);
>>>>> +    /* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
>>>>> +    kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
>>>> Use devm_phy_optional_get() instead?
>>>
>>> Indeed, it seems better suited.
>>
>> patches 1 and 2 are in testing/next
>>
> 
> Could you please drop them as I need to make changes to make the PHY optional.
> I will send v2 of entire series.

Actually only patch 2 and 3 need to be revised. Patch 1 is fine.

cheers,
-roger
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/3] dt-bindings: usb: convert keystone-usb.txt to YAML

[Rob Herring]

On Wed, May 13, 2020 at 04:07:07PM +0300, Roger Quadros wrote:
> Convert keystone-usb documentation to YAML format.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  .../devicetree/bindings/usb/keystone-usb.txt  | 56 ----------------
>  .../bindings/usb/ti,keystone-dwc3.yaml        | 67 +++++++++++++++++++
>  2 files changed, 67 insertions(+), 56 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt
>  create mode 100644 Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt
> deleted file mode 100644
> index 77df82e36138..000000000000
> --- a/Documentation/devicetree/bindings/usb/keystone-usb.txt
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -TI Keystone Soc USB Controller
> -
> -DWC3 GLUE
> -
> -Required properties:
> - - compatible: should be
> -		"ti,keystone-dwc3" for Keystone 2 SoCs
> -		"ti,am654-dwc3" for AM654 SoC
> - - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> -   with 'reg' property.
> - - reg : Address and length of the register set for the USB subsystem on
> -   the SOC.
> - - interrupts : The irq number of this device that is used to interrupt the
> -   MPU.
> - - ranges: allows valid 1:1 translation between child's address space and
> -   parent's address space.
> -
> -SoC-specific Required Properties:
> -The following are mandatory properties for Keystone 2 66AK2HK, 66AK2L and 66AK2E
> -SoCs only:
> -
> -- clocks:		Clock ID for USB functional clock.
> -- clock-names:		Must be "usb".
> -
> -
> -The following are mandatory properties for 66AK2G and AM654:
> -
> -- power-domains:	Should contain a phandle to a PM domain provider node
> -			and an args specifier containing the USB device id
> -			value. This property is as per the binding,
> -			Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> -
> -Sub-nodes:
> -The dwc3 core should be added as subnode to Keystone DWC3 glue.
> -- dwc3 :
> -   The binding details of dwc3 can be found in:
> -   Documentation/devicetree/bindings/usb/dwc3.txt
> -
> -Example:
> -	usb: usb@2680000 {
> -		compatible = "ti,keystone-dwc3";
> -		#address-cells = <1>;
> -		#size-cells = <1>;
> -		reg = <0x2680000 0x10000>;
> -		clocks = <&clkusb>;
> -		clock-names = "usb";
> -		interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> -		ranges;
> -
> -		dwc3@2690000 {
> -			compatible = "synopsys,dwc3";
> -			reg = <0x2690000 0x70000>;
> -			interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> -			usb-phy = <&usb_phy>, <&usb_phy>;
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> new file mode 100644
> index 000000000000..14d2fe329b93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/ti,keystone-dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI Keystone Soc USB Controller
> +
> +maintainers:
> +  - Roger Quadros <rogerq@ti.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: "ti,keystone-dwc3"
> +      - const: "ti,am654-dwc3"

Use enum rather than oneOf+const.

> +
> +  reg:
> +    maxItems: 1
> +    description: Address and length of the register set for the USB subsystem on
> +      the SOC.
> +
> +  interrupts:
> +    maxItems: 1
> +    description: The irq number of this device that is used to interrupt the MPU.

No need for genericish descriptions when a single item.

> +
> +
> +  clocks:
> +    description: Clock ID for USB functional clock.

How many?

> +
> +  power-domains:
> +    description: Should contain a phandle to a PM domain provider node
> +      and an args specifier containing the USB device id
> +      value. This property is as per the binding,
> +      Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt

How many?

> +
> +  dwc3:

This doesn't work because there's a unit address. You need a pattern.

> +    description: This is the node representing the DWC3 controller instance
> +      Documentation/devicetree/bindings/usb/dwc3.txt

type: object

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks

additionalProperties: false

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    usb: usb@2680000 {
> +      compatible = "ti,keystone-dwc3";
> +      #address-cells = <1>;
> +      #size-cells = <1>;

These have to be documented.

> +      reg = <0x2680000 0x10000>;
> +      clocks = <&clkusb>;
> +      clock-names = "usb";
> +      interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> +      ranges;

This too.

> +
> +      dwc3@2690000 {
> +        compatible = "synopsys,dwc3";
> +        reg = <0x2690000 0x70000>;
> +        interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> +        usb-phy = <&usb_phy>, <&usb_phy>;
> +      };
> +    };
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

Re: [PATCH 1/3] dt-bindings: usb: convert keystone-usb.txt to YAML

[Roger Quadros]

Rob,

Thanks for the review. Since this patch was already picked up I will
address the issues in a follow up patch.

cheers,
-roger

On 27/05/2020 04:37, Rob Herring wrote:
> On Wed, May 13, 2020 at 04:07:07PM +0300, Roger Quadros wrote:
>> Convert keystone-usb documentation to YAML format.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   .../devicetree/bindings/usb/keystone-usb.txt  | 56 ----------------
>>   .../bindings/usb/ti,keystone-dwc3.yaml        | 67 +++++++++++++++++++
>>   2 files changed, 67 insertions(+), 56 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt
>>   create mode 100644 Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt
>> deleted file mode 100644
>> index 77df82e36138..000000000000
>> --- a/Documentation/devicetree/bindings/usb/keystone-usb.txt
>> +++ /dev/null
>> @@ -1,56 +0,0 @@
>> -TI Keystone Soc USB Controller
>> -
>> -DWC3 GLUE
>> -
>> -Required properties:
>> - - compatible: should be
>> -		"ti,keystone-dwc3" for Keystone 2 SoCs
>> -		"ti,am654-dwc3" for AM654 SoC
>> - - #address-cells, #size-cells : should be '1' if the device has sub-nodes
>> -   with 'reg' property.
>> - - reg : Address and length of the register set for the USB subsystem on
>> -   the SOC.
>> - - interrupts : The irq number of this device that is used to interrupt the
>> -   MPU.
>> - - ranges: allows valid 1:1 translation between child's address space and
>> -   parent's address space.
>> -
>> -SoC-specific Required Properties:
>> -The following are mandatory properties for Keystone 2 66AK2HK, 66AK2L and 66AK2E
>> -SoCs only:
>> -
>> -- clocks:		Clock ID for USB functional clock.
>> -- clock-names:		Must be "usb".
>> -
>> -
>> -The following are mandatory properties for 66AK2G and AM654:
>> -
>> -- power-domains:	Should contain a phandle to a PM domain provider node
>> -			and an args specifier containing the USB device id
>> -			value. This property is as per the binding,
>> -			Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>> -
>> -Sub-nodes:
>> -The dwc3 core should be added as subnode to Keystone DWC3 glue.
>> -- dwc3 :
>> -   The binding details of dwc3 can be found in:
>> -   Documentation/devicetree/bindings/usb/dwc3.txt
>> -
>> -Example:
>> -	usb: usb@2680000 {
>> -		compatible = "ti,keystone-dwc3";
>> -		#address-cells = <1>;
>> -		#size-cells = <1>;
>> -		reg = <0x2680000 0x10000>;
>> -		clocks = <&clkusb>;
>> -		clock-names = "usb";
>> -		interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
>> -		ranges;
>> -
>> -		dwc3@2690000 {
>> -			compatible = "synopsys,dwc3";
>> -			reg = <0x2690000 0x70000>;
>> -			interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
>> -			usb-phy = <&usb_phy>, <&usb_phy>;
>> -		};
>> -	};
>> diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
>> new file mode 100644
>> index 000000000000..14d2fe329b93
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
>> @@ -0,0 +1,67 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/ti,keystone-dwc3.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TI Keystone Soc USB Controller
>> +
>> +maintainers:
>> +  - Roger Quadros <rogerq@ti.com>
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - const: "ti,keystone-dwc3"
>> +      - const: "ti,am654-dwc3"
> 
> Use enum rather than oneOf+const.
> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description: Address and length of the register set for the USB subsystem on
>> +      the SOC.
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +    description: The irq number of this device that is used to interrupt the MPU.
> 
> No need for genericish descriptions when a single item.
> 
>> +
>> +
>> +  clocks:
>> +    description: Clock ID for USB functional clock.
> 
> How many?
> 
>> +
>> +  power-domains:
>> +    description: Should contain a phandle to a PM domain provider node
>> +      and an args specifier containing the USB device id
>> +      value. This property is as per the binding,
>> +      Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> 
> How many?
> 
>> +
>> +  dwc3:
> 
> This doesn't work because there's a unit address. You need a pattern.
> 
>> +    description: This is the node representing the DWC3 controller instance
>> +      Documentation/devicetree/bindings/usb/dwc3.txt
> 
> type: object
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
> 
> additionalProperties: false
> 
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +    usb: usb@2680000 {
>> +      compatible = "ti,keystone-dwc3";
>> +      #address-cells = <1>;
>> +      #size-cells = <1>;
> 
> These have to be documented.
> 
>> +      reg = <0x2680000 0x10000>;
>> +      clocks = <&clkusb>;
>> +      clock-names = "usb";
>> +      interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
>> +      ranges;
> 
> This too.
> 
>> +
>> +      dwc3@2690000 {
>> +        compatible = "synopsys,dwc3";
>> +        reg = <0x2690000 0x70000>;
>> +        interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
>> +        usb-phy = <&usb_phy>, <&usb_phy>;
>> +      };
>> +    };
>> -- 
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki