[PATCH v3 0/5] Add JH7110 USB and USB PHY driver support

[PATCH v3 0/5] Add JH7110 USB and USB PHY driver support

[Minda Chen]

This patchset adds USB driver and USB PHY for the StarFive JH7110 SoC.
USB work mode is peripheral and using USB 2.0 PHY in VisionFive 2 board.
The patch has been tested on the VisionFive 2 board.

This patchset should be applied after the patchset [1], [2] and[3]:
[1] https://lore.kernel.org/all/20230221083323.302471-1-xingyu.wu@starfivetech.com/
[2] https://lore.kernel.org/all/20230215113249.47727-4-william.qiu@starfivetech.com/
[3] https://lore.kernel.org/all/20230223015952.201841-1-changhuang.liang@starfivetech.com/

This patchset is base on v6.3-rc1

patch 1 is usb phy and pcie PHY layer dt-binding document.
patch 2 is USB 2.0 PHY and PCIe PHY driver.
patch 3 is usb wrapper layer dt-binding document.
patch 4 is the wrapper module driver of Cadence USB3. USB controller IP is Cadence USB3.
patch 5 is USB device tree configuration.

previous version
---
v1: https://patchwork.kernel.org/project/linux-usb/cover/20230306095212.25840-1-minda.chen@starfivetech.com/
v2: https://patchwork.kernel.org/project/linux-usb/cover/20230308082800.3008-1-minda.chen@starfivetech.com/

changes
v3:
  1. Add patch 1 and patch2. Add USB driver. USB PHY codes are moved to patch 2.
  2. (patch 3) USB wrapper module dts document is moved to usb directory.
     Remove the 'dr_mode' and 'starfive,usb2-only' setting.
     Some dts format changes. dts binding check pass.
  3. (patch 4) Remove the PHY codes. 
     Search 'dr_mode' and phy setting from Cadence subnode.
  4. (patch 5) Add USB PHY dts configurion. 
     'dr_mode' is moved to Cadence controller submode.

v2:
  1. (patch 3) dt-binding changes. The document example is the same as dts config.
  2. (patch 4) using dev_err_probe and syscon_regmap_lookup_by_phandle_args function. Some formats changes
  3. (patch 5) dts nodes sorted by the address after @


Minda Chen (5):
  dt-bindings: phy: Add StarFive JH7110 USB/PCIe document
  phy: starfive: add JH7110 PCIE 2.0 and USB 2.0 PHY driver.
  dt-binding: Add JH7110 USB wrapper layer doc.
  usb: cdns3: add StarFive JH7110 USB driver.
  dts: usb: add StarFive JH7110 USB dts configuration.

 .../phy/starfive,jh7110-usb-pcie-phy.yaml     |  62 ++++
 .../bindings/usb/starfive,jh7110-usb.yaml     | 119 +++++++
 MAINTAINERS                                   |  15 +
 .../jh7110-starfive-visionfive-2.dtsi         |   7 +
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  54 ++++
 drivers/phy/starfive/Kconfig                  |  22 ++
 drivers/phy/starfive/Makefile                 |   2 +
 drivers/phy/starfive/phy-jh7110-pcie.c        | 136 ++++++++
 drivers/phy/starfive/phy-jh7110-usb.c         | 167 ++++++++++
 drivers/usb/cdns3/Kconfig                     |  11 +
 drivers/usb/cdns3/Makefile                    |   1 +
 drivers/usb/cdns3/cdns3-starfive.c            | 305 ++++++++++++++++++
 12 files changed, 901 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
 create mode 100644 Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
 create mode 100644 drivers/phy/starfive/phy-jh7110-pcie.c
 create mode 100644 drivers/phy/starfive/phy-jh7110-usb.c
 create mode 100644 drivers/usb/cdns3/cdns3-starfive.c


base-commit: 8ca09d5fa3549d142c2080a72a4c70ce389163cd
prerequisite-patch-id: 46cc850aa0e9e03ccf5ed23d8458babfca3d71af
prerequisite-patch-id: a6975e61ee5803fbd74b1c21ab925fd81c3c0eab
prerequisite-patch-id: ac150a8c622e858e088df8121093d448df49c245
prerequisite-patch-id: 044263ef2fb9f1e5a586edbf85d5f67814a28430
prerequisite-patch-id: 89f049f951e5acf75aab92541992f816fd0acc0d
prerequisite-patch-id: 9f3dbc9073eee89134e68977e941e457593c2757
prerequisite-patch-id: 8600b156a235be2b3db53be3f834e7a370e2cfb9
prerequisite-patch-id: 1b2d0982b18da060c82134f05bf3ce16425bac8d
prerequisite-patch-id: 090ba4b78d47bc19204916e76fdbc70021785388
prerequisite-patch-id: a5d9e0f7d4f8163f566678894cf693015119f2d9
prerequisite-patch-id: 4c12d958e3a3d629d86dddb1e4f099d8909393e0
prerequisite-patch-id: bb939c0c7c26b08addfccd890f9d3974b6eaec53
prerequisite-patch-id: 8f5c66dfb14403424044192f6fa05b347ad356a7
prerequisite-patch-id: fd93763b95469912bde9bdfa4cd827c8d5dba9c6
prerequisite-patch-id: 6987950c2eb4b3773b2df8f7934eff434244aeab
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: dbb0c0151b8bdf093e6ce79fd2fe3f60791a6e0b
prerequisite-patch-id: e7773c977a7b37692e9792b21cc4f17fa58f9215
prerequisite-patch-id: d57e95d31686772abc4c4d5aa1cadc344dc293cd
prerequisite-patch-id: 9f911969d0a550648493952c99096d26e05d4d83
prerequisite-patch-id: f9ce88e490c2473c3c94ad63fa26bc91829ce2cc
prerequisite-patch-id: 2c6b9fe2b00d9c1812d26fcffb4925335b359ede
prerequisite-patch-id: 51c1ca0fa8e1e8910c4d6aba147b8a75fd90a07f
prerequisite-patch-id: be0d260ac6eea34de411bf028828fea9f9fa0a9f
prerequisite-patch-id: aaff99c0bdd1604048d0713b44084ad4604816e1
prerequisite-patch-id: d7d5f5f35ecc3d66be8e3d8b0662788f875adc32
prerequisite-patch-id: b3362bb851a0efaa848104bc0c2a1a264ba7904b
prerequisite-patch-id: 6c25cbf9fe08218ee952d2202c5f6b645ea4b6e4
-- 
2.17.1

[PATCH v3 1/5] dt-bindings: phy: Add StarFive JH7110 USB/PCIe document

[Minda Chen]

Add StarFive JH7110 SoC USB 2.0/3.0 and PCIe 2.0 PHY dt-binding.
PCIe 2.0 phy can use as USB 3.0 PHY.

Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
 .../phy/starfive,jh7110-usb-pcie-phy.yaml     | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
new file mode 100644
index 000000000000..aa1c3fe93100
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/starfive,jh7110-usb-pcie-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive USB 2.0 and PCIe 2.0 PHY
+
+maintainers:
+  - Minda Chen <minda.chen@starfivetech.com>
+
+properties:
+  compatible:
+    enum:
+      - starfive,jh7110-usb-phy
+      - starfive,jh7110-pcie-phy
+
+  reg:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 0
+
+  clocks:
+    items:
+      - description: usb 125m clock
+      - description: app 125m clock
+
+  clock-names:
+    items:
+      - const: 125m
+      - const: app_125
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    phy@10200000 {
+      compatible = "starfive,jh7110-usb-phy";
+      reg = <0x10200000 0x10000>;
+      clocks = <&syscrg 95>,
+               <&stgcrg 6>;
+      clock-names = "125m", "app_125";
+      #phy-cells = <0>;
+    };
+
+    phy@10210000 {
+      compatible = "starfive,jh7110-pcie-phy";
+      reg = <0x10210000 0x10000>;
+      #phy-cells = <0>;
+    };
+
+    phy@10220000 {
+      compatible = "starfive,jh7110-pcie-phy";
+      reg = <0x10220000 0x10000>;
+      #phy-cells = <0>;
+    };
-- 
2.17.1

Re: [PATCH v3 1/5] dt-bindings: phy: Add StarFive JH7110 USB/PCIe document

[Krzysztof Kozlowski]

On 15/03/2023 11:44, Minda Chen wrote:
> Add StarFive JH7110 SoC USB 2.0/3.0 and PCIe 2.0 PHY dt-binding.
> PCIe 2.0 phy can use as USB 3.0 PHY.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  .../phy/starfive,jh7110-usb-pcie-phy.yaml     | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
> new file mode 100644
> index 000000000000..aa1c3fe93100
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/starfive,jh7110-usb-pcie-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive USB 2.0 and PCIe 2.0 PHY
> +
> +maintainers:
> +  - Minda Chen <minda.chen@starfivetech.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - starfive,jh7110-usb-phy
> +      - starfive,jh7110-pcie-phy
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#phy-cells":
> +    const: 0
> +
> +  clocks:
> +    items:
> +      - description: usb 125m clock
> +      - description: app 125m clock
> +
> +  clock-names:
> +    items:
> +      - const: 125m
> +      - const: app_125
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +

It seems pci phy does not take these clocks, thus you should have
allOf:if:then which will customize it per variant. Otherwise binding is
incorrect for the pci.

> +additionalProperties: false
> +
> +examples:
> +  - |
> +    phy@10200000 {
> +      compatible = "starfive,jh7110-usb-phy";
> +      reg = <0x10200000 0x10000>;
> +      clocks = <&syscrg 95>,
> +               <&stgcrg 6>;
> +      clock-names = "125m", "app_125";
> +      #phy-cells = <0>;
> +    };
> +
> +    phy@10210000 {
> +      compatible = "starfive,jh7110-pcie-phy";
> +      reg = <0x10210000 0x10000>;
> +      #phy-cells = <0>;
> +    };
> +
> +    phy@10220000 {
> +      compatible = "starfive,jh7110-pcie-phy";
> +      reg = <0x10220000 0x10000>;
> +      #phy-cells = <0>;
> +    };

Drop duplicated examples. Keep usb and maybe one phy.

Best regards,
Krzysztof

Re: [PATCH v3 1/5] dt-bindings: phy: Add StarFive JH7110 USB/PCIe document

[Minda Chen]

On 2023/3/17 16:39, Krzysztof Kozlowski wrote:
> On 15/03/2023 11:44, Minda Chen wrote:
>> Add StarFive JH7110 SoC USB 2.0/3.0 and PCIe 2.0 PHY dt-binding.
>> PCIe 2.0 phy can use as USB 3.0 PHY.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  .../phy/starfive,jh7110-usb-pcie-phy.yaml     | 62 +++++++++++++++++++
>>  1 file changed, 62 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>> new file mode 100644
>> index 000000000000..aa1c3fe93100
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/starfive,jh7110-usb-pcie-phy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive USB 2.0 and PCIe 2.0 PHY
>> +
>> +maintainers:
>> +  - Minda Chen <minda.chen@starfivetech.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - starfive,jh7110-usb-phy
>> +      - starfive,jh7110-pcie-phy
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  "#phy-cells":
>> +    const: 0
>> +
>> +  clocks:
>> +    items:
>> +      - description: usb 125m clock
>> +      - description: app 125m clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: 125m
>> +      - const: app_125
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - "#phy-cells"
>> +
> 
> It seems pci phy does not take these clocks, thus you should have
> allOf:if:then which will customize it per variant. Otherwise binding is
> incorrect for the pci.
> 
ok
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    phy@10200000 {
>> +      compatible = "starfive,jh7110-usb-phy";
>> +      reg = <0x10200000 0x10000>;
>> +      clocks = <&syscrg 95>,
>> +               <&stgcrg 6>;
>> +      clock-names = "125m", "app_125";
>> +      #phy-cells = <0>;
>> +    };
>> +
>> +    phy@10210000 {
>> +      compatible = "starfive,jh7110-pcie-phy";
>> +      reg = <0x10210000 0x10000>;
>> +      #phy-cells = <0>;
>> +    };
>> +
>> +    phy@10220000 {
>> +      compatible = "starfive,jh7110-pcie-phy";
>> +      reg = <0x10220000 0x10000>;
>> +      #phy-cells = <0>;
>> +    };
> 
> Drop duplicated examples. Keep usb and maybe one phy.
> 
ok, thanks
> Best regards,
> Krzysztof
> 

[PATCH v3 2/5] phy: starfive: add JH7110 PCIE 2.0 and USB 2.0 PHY driver.

[Minda Chen]

Add Starfive JH7110 SoC PCIe 2.0 and USB 2.0 PHY driver support.
PCIe 2.0 PHY can used as USB 3.0 PHY

Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
 MAINTAINERS                            |   8 ++
 drivers/phy/starfive/Kconfig           |  22 ++++
 drivers/phy/starfive/Makefile          |   2 +
 drivers/phy/starfive/phy-jh7110-pcie.c | 136 ++++++++++++++++++++
 drivers/phy/starfive/phy-jh7110-usb.c  | 167 +++++++++++++++++++++++++
 5 files changed, 335 insertions(+)
 create mode 100644 drivers/phy/starfive/phy-jh7110-pcie.c
 create mode 100644 drivers/phy/starfive/phy-jh7110-usb.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8361b8e710ca..4263c005e45c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19977,6 +19977,14 @@ S:	Supported
 F:	Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
 F:	drivers/phy/starfive/phy-starfive-dphy-rx.c
 
+STARFIVE JH71X0 PCIE AND USB PHY DRIVER
+M:	Emil Renner Berthing <kernel@esmil.dk>
+M:	Minda Chen <minda.chen@starfivetech.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
+F:	drivers/phy/starfive/phy-jh7110-pcie.c
+F:	drivers/phy/starfive/phy-jh7110-usb.c
+
 STATIC BRANCH/CALL
 M:	Peter Zijlstra <peterz@infradead.org>
 M:	Josh Poimboeuf <jpoimboe@kernel.org>
diff --git a/drivers/phy/starfive/Kconfig b/drivers/phy/starfive/Kconfig
index e449a662acf5..dd0f139b5bfb 100644
--- a/drivers/phy/starfive/Kconfig
+++ b/drivers/phy/starfive/Kconfig
@@ -11,3 +11,25 @@ config PHY_STARFIVE_DPHY_RX
 	  Choose this option if you have a Starfive D-PHY in your
 	  system. If M is selected, the module will be called
 	  phy-starfive-dphy-rx.
+
+config PHY_STARFIVE_JH7110_USB
+	tristate "Starfive JH7110 USB 2.0 PHY support"
+	depends on USB_SUPPORT
+	select GENERIC_PHY
+	select USB_PHY
+	help
+	  Enable this to support the StarFive USB 2.0 PHY,
+	  used with the Cadence USB controller.
+	  If M is selected, the module will be called
+	  phy-jh7110-usb.ko.
+
+config PHY_STARFIVE_JH7110_PCIE
+	tristate "Starfive JH7110 PCIE 2.0/USB 3.0 PHY support"
+	depends on USB_SUPPORT
+	select GENERIC_PHY
+	select USB_PHY
+	help
+	  Enable this to support the StarFive PCIe 2.0 PHY,
+	  or used as USB 3.0 PHY.
+	  If M is selected, the module will be called
+	  phy-jh7110-pcie.ko.
diff --git a/drivers/phy/starfive/Makefile b/drivers/phy/starfive/Makefile
index 7ec576cb30ae..c3eaf1b34cbb 100644
--- a/drivers/phy/starfive/Makefile
+++ b/drivers/phy/starfive/Makefile
@@ -1,2 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_PHY_STARFIVE_DPHY_RX)      += phy-starfive-dphy-rx.o
+obj-$(CONFIG_PHY_STARFIVE_JH7110_USB)	+= phy-jh7110-usb.o
+obj-$(CONFIG_PHY_STARFIVE_JH7110_PCIE)	+= phy-jh7110-pcie.o
diff --git a/drivers/phy/starfive/phy-jh7110-pcie.c b/drivers/phy/starfive/phy-jh7110-pcie.c
new file mode 100644
index 000000000000..30a8fa1f580d
--- /dev/null
+++ b/drivers/phy/starfive/phy-jh7110-pcie.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * StarFive JH7110 PCIe 2.0 PHY driver
+ *
+ * Copyright (C) 2023 Minda Chen <minda.chen@starfivetech.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define PCIE_KVCO_LEVEL_OFF		(0x28)
+#define PCIE_USB3_PHY_PLL_CTL_OFF	(0x7c)
+#define PCIE_KVCO_TUNE_SIGNAL_OFF	(0x80)
+#define PCIE_USB3_PHY_ENABLE		BIT(4)
+#define PHY_KVCO_FINE_TUNE_LEVEL	0x91
+#define PHY_KVCO_FINE_TUNE_SIGNALS	0xc
+
+struct jh7110_pcie_phy {
+	struct phy *phy;
+	void __iomem *regs;
+	enum phy_mode mode;
+};
+
+static void jh7110_usb3_mode_set(struct jh7110_pcie_phy *phy)
+{
+	/* Configuare spread-spectrum mode: down-spread-spectrum */
+	writel(PCIE_USB3_PHY_ENABLE, phy->regs + PCIE_USB3_PHY_PLL_CTL_OFF);
+}
+
+static void jh7110_pcie_mode_set(struct jh7110_pcie_phy *phy)
+{
+	/* PCIe Multi-PHY PLL KVCO Gain fine tune settings: */
+	writel(PHY_KVCO_FINE_TUNE_LEVEL, phy->regs + PCIE_KVCO_LEVEL_OFF);
+	writel(PHY_KVCO_FINE_TUNE_SIGNALS, phy->regs + PCIE_KVCO_TUNE_SIGNAL_OFF);
+}
+
+static int jh7110_pcie_phy_set_mode(struct phy *_phy,
+				  enum phy_mode mode, int submode)
+{
+	struct jh7110_pcie_phy *phy = phy_get_drvdata(_phy);
+
+	if (mode != phy->mode) {
+		switch (mode) {
+		case PHY_MODE_USB_HOST:
+		case PHY_MODE_USB_DEVICE:
+		case PHY_MODE_USB_OTG:
+			jh7110_usb3_mode_set(phy);
+			break;
+		case PHY_MODE_PCIE:
+			jh7110_pcie_mode_set(phy);
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		dev_info(&_phy->dev, "Changing phy mode to %d\n", mode);
+		phy->mode = mode;
+	}
+
+	return 0;
+}
+
+static int jh7110_pcie_phy_init(struct phy *_phy)
+{
+	return 0;
+}
+
+static int jh7110_pcie_phy_exit(struct phy *_phy)
+{
+	return 0;
+}
+
+static const struct phy_ops jh7110_pcie_phy_ops = {
+	.init		= jh7110_pcie_phy_init,
+	.exit		= jh7110_pcie_phy_exit,
+	.set_mode	= jh7110_pcie_phy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static int jh7110_pcie_phy_probe(struct platform_device *pdev)
+{
+	struct jh7110_pcie_phy *phy;
+	struct device *dev = &pdev->dev;
+	struct phy_provider *phy_provider;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(phy->regs))
+		return PTR_ERR(phy->regs);
+
+	phy->phy = devm_phy_create(dev, NULL, &jh7110_pcie_phy_ops);
+	if (IS_ERR(phy->phy))
+		return dev_err_probe(dev, PTR_ERR(phy->regs),
+			"Failed to map phy base\n");
+
+	platform_set_drvdata(pdev, phy);
+	phy_set_drvdata(phy->phy, phy);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static int jh7110_pcie_phy_remove(struct platform_device *pdev)
+{
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static const struct of_device_id jh7110_pcie_phy_of_match[] = {
+	{ .compatible = "starfive,jh7110-pcie-phy" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, jh7110_pcie_phy_of_match);
+
+static struct platform_driver jh7110_pcie_phy_driver = {
+	.probe	= jh7110_pcie_phy_probe,
+	.remove	= jh7110_pcie_phy_remove,
+	.driver = {
+		.of_match_table	= jh7110_pcie_phy_of_match,
+		.name  = "jh7110-pcie-phy",
+	}
+};
+module_platform_driver(jh7110_pcie_phy_driver);
+
+MODULE_DESCRIPTION("StarFive JH7110 PCIe 2.0 PHY driver");
+MODULE_AUTHOR("Minda Chen <minda.chen@starfivetech.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/phy/starfive/phy-jh7110-usb.c b/drivers/phy/starfive/phy-jh7110-usb.c
new file mode 100644
index 000000000000..89db0b7b1224
--- /dev/null
+++ b/drivers/phy/starfive/phy-jh7110-usb.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * StarFive JH7110 USB 2.0 PHY driver
+ *
+ * Copyright (C) 2023 Minda Chen <minda.chen@starfivetech.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/usb/of.h>
+
+#define USB_125M_CLK_RATE		125000000
+#define USB_LS_KEEPALIVE_OFF		0x4
+#define USB_LS_KEEPALIVE_ENABLE		BIT(4)
+
+struct jh7110_usb2_phy {
+	struct phy *phy;
+	void __iomem *regs;
+	struct clk *usb_125m_clk;
+	struct clk *app_125;
+	enum usb_dr_mode dr_mode;
+};
+
+static void jh7110_usb2_mode_set(struct jh7110_usb2_phy *phy)
+{
+	unsigned int val;
+
+	if (phy->dr_mode != USB_DR_MODE_PERIPHERAL) {
+		/* Enable the LS speed keep-alive signal */
+		val = readl(phy->regs + USB_LS_KEEPALIVE_OFF);
+		val |= USB_LS_KEEPALIVE_ENABLE;
+		writel(val, phy->regs + USB_LS_KEEPALIVE_OFF);
+	}
+}
+
+static int jh7110_usb2_phy_set_mode(struct phy *_phy,
+				  enum phy_mode mode, int submode)
+{
+	struct jh7110_usb2_phy *phy = phy_get_drvdata(_phy);
+	int new_mode;
+
+	switch (mode) {
+	case PHY_MODE_USB_HOST:
+		new_mode = USB_DR_MODE_HOST;
+		break;
+	case PHY_MODE_USB_DEVICE:
+		new_mode = USB_DR_MODE_PERIPHERAL;
+		break;
+	case PHY_MODE_USB_OTG:
+		new_mode = USB_DR_MODE_OTG;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (new_mode != phy->dr_mode) {
+		dev_info(&_phy->dev, "Changing dr_mode to %d\n", new_mode);
+		phy->dr_mode = new_mode;
+		jh7110_usb2_mode_set(phy);
+	}
+
+	return 0;
+}
+
+static int jh7110_usb2_phy_init(struct phy *_phy)
+{
+	struct jh7110_usb2_phy *phy = phy_get_drvdata(_phy);
+	int ret;
+
+	ret = clk_set_rate(phy->usb_125m_clk, USB_125M_CLK_RATE);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(phy->app_125);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int jh7110_usb2_phy_exit(struct phy *_phy)
+{
+	struct jh7110_usb2_phy *phy = phy_get_drvdata(_phy);
+
+	clk_disable_unprepare(phy->app_125);
+
+	return 0;
+}
+
+static const struct phy_ops jh7110_usb2_phy_ops = {
+	.init		= jh7110_usb2_phy_init,
+	.exit		= jh7110_usb2_phy_exit,
+	.set_mode	= jh7110_usb2_phy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static int jh7110_usb_phy_probe(struct platform_device *pdev)
+{
+	struct jh7110_usb2_phy *phy;
+	struct device *dev = &pdev->dev;
+	struct phy_provider *phy_provider;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->usb_125m_clk = devm_clk_get(dev, "125m");
+	if (IS_ERR(phy->usb_125m_clk))
+		return dev_err_probe(dev, PTR_ERR(phy->usb_125m_clk),
+			"Failed to get 125m clock\n");
+
+	phy->app_125 = devm_clk_get(dev, "app_125");
+	if (IS_ERR(phy->app_125))
+		return dev_err_probe(dev, PTR_ERR(phy->app_125),
+			"Failed to get app 125m clock\n");
+
+	phy->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(phy->regs))
+		return dev_err_probe(dev, PTR_ERR(phy->regs),
+			"Failed to map phy base\n");
+
+	phy->phy = devm_phy_create(dev, NULL, &jh7110_usb2_phy_ops);
+	if (IS_ERR(phy->phy))
+		return dev_err_probe(dev, PTR_ERR(phy->phy),
+			"Failed to create phy\n");
+
+	platform_set_drvdata(pdev, phy);
+	phy_set_drvdata(phy->phy, phy);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static int jh7110_usb_phy_remove(struct platform_device *pdev)
+{
+	struct jh7110_usb2_phy *phy = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(phy->app_125);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static const struct of_device_id jh7110_usb_phy_of_match[] = {
+	{ .compatible = "starfive,jh7110-usb-phy" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, jh7110_usb_phy_of_match);
+
+static struct platform_driver jh7110_usb_phy_driver = {
+	.probe	= jh7110_usb_phy_probe,
+	.remove	= jh7110_usb_phy_remove,
+	.driver = {
+		.of_match_table	= jh7110_usb_phy_of_match,
+		.name  = "jh7110-usb-phy",
+	}
+};
+module_platform_driver(jh7110_usb_phy_driver);
+
+MODULE_DESCRIPTION("StarFive JH7110 USB 2.0 PHY driver");
+MODULE_AUTHOR("Minda Chen <minda.chen@starfivetech.com>");
+MODULE_LICENSE("GPL");
-- 
2.17.1

Re: [PATCH v3 2/5] phy: starfive: add JH7110 PCIE 2.0 and USB 2.0 PHY driver.

[Vinod Koul]

On 15-03-23, 18:44, Minda Chen wrote:
> Add Starfive JH7110 SoC PCIe 2.0 and USB 2.0 PHY driver support.
> PCIe 2.0 PHY can used as USB 3.0 PHY
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  MAINTAINERS                            |   8 ++
>  drivers/phy/starfive/Kconfig           |  22 ++++
>  drivers/phy/starfive/Makefile          |   2 +
>  drivers/phy/starfive/phy-jh7110-pcie.c | 136 ++++++++++++++++++++
>  drivers/phy/starfive/phy-jh7110-usb.c  | 167 +++++++++++++++++++++++++
>  5 files changed, 335 insertions(+)
>  create mode 100644 drivers/phy/starfive/phy-jh7110-pcie.c
>  create mode 100644 drivers/phy/starfive/phy-jh7110-usb.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8361b8e710ca..4263c005e45c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19977,6 +19977,14 @@ S:	Supported
>  F:	Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
>  F:	drivers/phy/starfive/phy-starfive-dphy-rx.c
>  
> +STARFIVE JH71X0 PCIE AND USB PHY DRIVER
> +M:	Emil Renner Berthing <kernel@esmil.dk>
> +M:	Minda Chen <minda.chen@starfivetech.com>
> +S:	Supported
> +F:	Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
> +F:	drivers/phy/starfive/phy-jh7110-pcie.c
> +F:	drivers/phy/starfive/phy-jh7110-usb.c
> +
>  STATIC BRANCH/CALL
>  M:	Peter Zijlstra <peterz@infradead.org>
>  M:	Josh Poimboeuf <jpoimboe@kernel.org>
> diff --git a/drivers/phy/starfive/Kconfig b/drivers/phy/starfive/Kconfig
> index e449a662acf5..dd0f139b5bfb 100644
> --- a/drivers/phy/starfive/Kconfig
> +++ b/drivers/phy/starfive/Kconfig
> @@ -11,3 +11,25 @@ config PHY_STARFIVE_DPHY_RX
>  	  Choose this option if you have a Starfive D-PHY in your
>  	  system. If M is selected, the module will be called
>  	  phy-starfive-dphy-rx.
> +
> +config PHY_STARFIVE_JH7110_USB
> +	tristate "Starfive JH7110 USB 2.0 PHY support"
> +	depends on USB_SUPPORT
> +	select GENERIC_PHY
> +	select USB_PHY
> +	help
> +	  Enable this to support the StarFive USB 2.0 PHY,
> +	  used with the Cadence USB controller.
> +	  If M is selected, the module will be called
> +	  phy-jh7110-usb.ko.
> +
> +config PHY_STARFIVE_JH7110_PCIE

Sorted alphabetically please

> +	tristate "Starfive JH7110 PCIE 2.0/USB 3.0 PHY support"
> +	depends on USB_SUPPORT
> +	select GENERIC_PHY
> +	select USB_PHY
> +	help
> +	  Enable this to support the StarFive PCIe 2.0 PHY,
> +	  or used as USB 3.0 PHY.
> +	  If M is selected, the module will be called
> +	  phy-jh7110-pcie.ko.
> diff --git a/drivers/phy/starfive/Makefile b/drivers/phy/starfive/Makefile
> index 7ec576cb30ae..c3eaf1b34cbb 100644
> --- a/drivers/phy/starfive/Makefile
> +++ b/drivers/phy/starfive/Makefile
> @@ -1,2 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_PHY_STARFIVE_DPHY_RX)      += phy-starfive-dphy-rx.o
> +obj-$(CONFIG_PHY_STARFIVE_JH7110_USB)	+= phy-jh7110-usb.o
> +obj-$(CONFIG_PHY_STARFIVE_JH7110_PCIE)	+= phy-jh7110-pcie.o

Here also

> diff --git a/drivers/phy/starfive/phy-jh7110-pcie.c b/drivers/phy/starfive/phy-jh7110-pcie.c
> new file mode 100644
> index 000000000000..30a8fa1f580d
> --- /dev/null
> +++ b/drivers/phy/starfive/phy-jh7110-pcie.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * StarFive JH7110 PCIe 2.0 PHY driver
> + *
> + * Copyright (C) 2023 Minda Chen <minda.chen@starfivetech.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +
> +#define PCIE_KVCO_LEVEL_OFF		(0x28)
> +#define PCIE_USB3_PHY_PLL_CTL_OFF	(0x7c)
> +#define PCIE_KVCO_TUNE_SIGNAL_OFF	(0x80)
> +#define PCIE_USB3_PHY_ENABLE		BIT(4)
> +#define PHY_KVCO_FINE_TUNE_LEVEL	0x91
> +#define PHY_KVCO_FINE_TUNE_SIGNALS	0xc
> +
> +struct jh7110_pcie_phy {
> +	struct phy *phy;
> +	void __iomem *regs;
> +	enum phy_mode mode;
> +};
> +
> +static void jh7110_usb3_mode_set(struct jh7110_pcie_phy *phy)
> +{
> +	/* Configuare spread-spectrum mode: down-spread-spectrum */
> +	writel(PCIE_USB3_PHY_ENABLE, phy->regs + PCIE_USB3_PHY_PLL_CTL_OFF);
> +}
> +
> +static void jh7110_pcie_mode_set(struct jh7110_pcie_phy *phy)
> +{
> +	/* PCIe Multi-PHY PLL KVCO Gain fine tune settings: */
> +	writel(PHY_KVCO_FINE_TUNE_LEVEL, phy->regs + PCIE_KVCO_LEVEL_OFF);
> +	writel(PHY_KVCO_FINE_TUNE_SIGNALS, phy->regs + PCIE_KVCO_TUNE_SIGNAL_OFF);
> +}
> +
> +static int jh7110_pcie_phy_set_mode(struct phy *_phy,
> +				  enum phy_mode mode, int submode)
> +{
> +	struct jh7110_pcie_phy *phy = phy_get_drvdata(_phy);
> +
> +	if (mode != phy->mode) {
> +		switch (mode) {
> +		case PHY_MODE_USB_HOST:
> +		case PHY_MODE_USB_DEVICE:
> +		case PHY_MODE_USB_OTG:
> +			jh7110_usb3_mode_set(phy);
> +			break;
> +		case PHY_MODE_PCIE:
> +			jh7110_pcie_mode_set(phy);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		dev_info(&_phy->dev, "Changing phy mode to %d\n", mode);
> +		phy->mode = mode;
> +	}
> +
> +	return 0;
> +}
> +
> +static int jh7110_pcie_phy_init(struct phy *_phy)
> +{
> +	return 0;
> +}
> +
> +static int jh7110_pcie_phy_exit(struct phy *_phy)
> +{
> +	return 0;
> +}
> +
> +static const struct phy_ops jh7110_pcie_phy_ops = {
> +	.init		= jh7110_pcie_phy_init,
> +	.exit		= jh7110_pcie_phy_exit,
> +	.set_mode	= jh7110_pcie_phy_set_mode,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int jh7110_pcie_phy_probe(struct platform_device *pdev)
> +{
> +	struct jh7110_pcie_phy *phy;
> +	struct device *dev = &pdev->dev;
> +	struct phy_provider *phy_provider;
> +
> +	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> +	if (!phy)
> +		return -ENOMEM;
> +
> +	phy->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(phy->regs))
> +		return PTR_ERR(phy->regs);
> +
> +	phy->phy = devm_phy_create(dev, NULL, &jh7110_pcie_phy_ops);
> +	if (IS_ERR(phy->phy))
> +		return dev_err_probe(dev, PTR_ERR(phy->regs),
> +			"Failed to map phy base\n");
> +
> +	platform_set_drvdata(pdev, phy);
> +	phy_set_drvdata(phy->phy, phy);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static int jh7110_pcie_phy_remove(struct platform_device *pdev)
> +{
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id jh7110_pcie_phy_of_match[] = {
> +	{ .compatible = "starfive,jh7110-pcie-phy" },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, jh7110_pcie_phy_of_match);
> +
> +static struct platform_driver jh7110_pcie_phy_driver = {
> +	.probe	= jh7110_pcie_phy_probe,
> +	.remove	= jh7110_pcie_phy_remove,
> +	.driver = {
> +		.of_match_table	= jh7110_pcie_phy_of_match,
> +		.name  = "jh7110-pcie-phy",
> +	}
> +};
> +module_platform_driver(jh7110_pcie_phy_driver);
> +
> +MODULE_DESCRIPTION("StarFive JH7110 PCIe 2.0 PHY driver");
> +MODULE_AUTHOR("Minda Chen <minda.chen@starfivetech.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/phy/starfive/phy-jh7110-usb.c b/drivers/phy/starfive/phy-jh7110-usb.c
> new file mode 100644
> index 000000000000..89db0b7b1224
> --- /dev/null
> +++ b/drivers/phy/starfive/phy-jh7110-usb.c
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * StarFive JH7110 USB 2.0 PHY driver

Since these two seem to be different driver, pls split the patches to
two, one for each phy driver

> + *
> + * Copyright (C) 2023 Minda Chen <minda.chen@starfivetech.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/usb/of.h>
> +
> +#define USB_125M_CLK_RATE		125000000
> +#define USB_LS_KEEPALIVE_OFF		0x4
> +#define USB_LS_KEEPALIVE_ENABLE		BIT(4)
> +
> +struct jh7110_usb2_phy {
> +	struct phy *phy;
> +	void __iomem *regs;
> +	struct clk *usb_125m_clk;
> +	struct clk *app_125;
> +	enum usb_dr_mode dr_mode;
> +};
> +
> +static void jh7110_usb2_mode_set(struct jh7110_usb2_phy *phy)
> +{
> +	unsigned int val;
> +
> +	if (phy->dr_mode != USB_DR_MODE_PERIPHERAL) {
> +		/* Enable the LS speed keep-alive signal */
> +		val = readl(phy->regs + USB_LS_KEEPALIVE_OFF);
> +		val |= USB_LS_KEEPALIVE_ENABLE;
> +		writel(val, phy->regs + USB_LS_KEEPALIVE_OFF);
> +	}
> +}
> +
> +static int jh7110_usb2_phy_set_mode(struct phy *_phy,
> +				  enum phy_mode mode, int submode)
> +{
> +	struct jh7110_usb2_phy *phy = phy_get_drvdata(_phy);
> +	int new_mode;
> +
> +	switch (mode) {
> +	case PHY_MODE_USB_HOST:
> +		new_mode = USB_DR_MODE_HOST;
> +		break;
> +	case PHY_MODE_USB_DEVICE:
> +		new_mode = USB_DR_MODE_PERIPHERAL;
> +		break;
> +	case PHY_MODE_USB_OTG:
> +		new_mode = USB_DR_MODE_OTG;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (new_mode != phy->dr_mode) {
> +		dev_info(&_phy->dev, "Changing dr_mode to %d\n", new_mode);
> +		phy->dr_mode = new_mode;
> +		jh7110_usb2_mode_set(phy);
> +	}
> +
> +	return 0;
> +}
> +
> +static int jh7110_usb2_phy_init(struct phy *_phy)
> +{
> +	struct jh7110_usb2_phy *phy = phy_get_drvdata(_phy);
> +	int ret;
> +
> +	ret = clk_set_rate(phy->usb_125m_clk, USB_125M_CLK_RATE);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(phy->app_125);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int jh7110_usb2_phy_exit(struct phy *_phy)
> +{
> +	struct jh7110_usb2_phy *phy = phy_get_drvdata(_phy);
> +
> +	clk_disable_unprepare(phy->app_125);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops jh7110_usb2_phy_ops = {
> +	.init		= jh7110_usb2_phy_init,
> +	.exit		= jh7110_usb2_phy_exit,
> +	.set_mode	= jh7110_usb2_phy_set_mode,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int jh7110_usb_phy_probe(struct platform_device *pdev)
> +{
> +	struct jh7110_usb2_phy *phy;
> +	struct device *dev = &pdev->dev;
> +	struct phy_provider *phy_provider;
> +
> +	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> +	if (!phy)
> +		return -ENOMEM;
> +
> +	phy->usb_125m_clk = devm_clk_get(dev, "125m");
> +	if (IS_ERR(phy->usb_125m_clk))
> +		return dev_err_probe(dev, PTR_ERR(phy->usb_125m_clk),
> +			"Failed to get 125m clock\n");
> +
> +	phy->app_125 = devm_clk_get(dev, "app_125");
> +	if (IS_ERR(phy->app_125))
> +		return dev_err_probe(dev, PTR_ERR(phy->app_125),
> +			"Failed to get app 125m clock\n");
> +
> +	phy->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(phy->regs))
> +		return dev_err_probe(dev, PTR_ERR(phy->regs),
> +			"Failed to map phy base\n");
> +
> +	phy->phy = devm_phy_create(dev, NULL, &jh7110_usb2_phy_ops);
> +	if (IS_ERR(phy->phy))
> +		return dev_err_probe(dev, PTR_ERR(phy->phy),
> +			"Failed to create phy\n");
> +
> +	platform_set_drvdata(pdev, phy);
> +	phy_set_drvdata(phy->phy, phy);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static int jh7110_usb_phy_remove(struct platform_device *pdev)
> +{
> +	struct jh7110_usb2_phy *phy = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(phy->app_125);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id jh7110_usb_phy_of_match[] = {
> +	{ .compatible = "starfive,jh7110-usb-phy" },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, jh7110_usb_phy_of_match);
> +
> +static struct platform_driver jh7110_usb_phy_driver = {
> +	.probe	= jh7110_usb_phy_probe,
> +	.remove	= jh7110_usb_phy_remove,
> +	.driver = {
> +		.of_match_table	= jh7110_usb_phy_of_match,
> +		.name  = "jh7110-usb-phy",
> +	}
> +};
> +module_platform_driver(jh7110_usb_phy_driver);
> +
> +MODULE_DESCRIPTION("StarFive JH7110 USB 2.0 PHY driver");
> +MODULE_AUTHOR("Minda Chen <minda.chen@starfivetech.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.17.1

-- 
~Vinod

Re: [PATCH v3 2/5] phy: starfive: add JH7110 PCIE 2.0 and USB 2.0 PHY driver.

[Minda Chen]

On 2023/3/20 16:55, Vinod Koul wrote:
> On 15-03-23, 18:44, Minda Chen wrote:
>> Add Starfive JH7110 SoC PCIe 2.0 and USB 2.0 PHY driver support.
>> PCIe 2.0 PHY can used as USB 3.0 PHY
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  MAINTAINERS                            |   8 ++
>>  drivers/phy/starfive/Kconfig           |  22 ++++
>>  drivers/phy/starfive/Makefile          |   2 +
>>  drivers/phy/starfive/phy-jh7110-pcie.c | 136 ++++++++++++++++++++
>>  drivers/phy/starfive/phy-jh7110-usb.c  | 167 +++++++++++++++++++++++++
>>  5 files changed, 335 insertions(+)
>>  create mode 100644 drivers/phy/starfive/phy-jh7110-pcie.c
>>  create mode 100644 drivers/phy/starfive/phy-jh7110-usb.c
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 8361b8e710ca..4263c005e45c 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19977,6 +19977,14 @@ S:	Supported
>>  F:	Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
>>  F:	drivers/phy/starfive/phy-starfive-dphy-rx.c
>>  
>> +STARFIVE JH71X0 PCIE AND USB PHY DRIVER
>> +M:	Emil Renner Berthing <kernel@esmil.dk>
>> +M:	Minda Chen <minda.chen@starfivetech.com>
>> +S:	Supported
>> +F:	Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>> +F:	drivers/phy/starfive/phy-jh7110-pcie.c
>> +F:	drivers/phy/starfive/phy-jh7110-usb.c
>> +
>>  STATIC BRANCH/CALL
>>  M:	Peter Zijlstra <peterz@infradead.org>
>>  M:	Josh Poimboeuf <jpoimboe@kernel.org>
>> diff --git a/drivers/phy/starfive/Kconfig b/drivers/phy/starfive/Kconfig
>> index e449a662acf5..dd0f139b5bfb 100644
>> --- a/drivers/phy/starfive/Kconfig
>> +++ b/drivers/phy/starfive/Kconfig
>> @@ -11,3 +11,25 @@ config PHY_STARFIVE_DPHY_RX
>>  	  Choose this option if you have a Starfive D-PHY in your
>>  	  system. If M is selected, the module will be called
>>  	  phy-starfive-dphy-rx.
>> +
>> +config PHY_STARFIVE_JH7110_USB
>> +	tristate "Starfive JH7110 USB 2.0 PHY support"
>> +	depends on USB_SUPPORT
>> +	select GENERIC_PHY
>> +	select USB_PHY
>> +	help
>> +	  Enable this to support the StarFive USB 2.0 PHY,
>> +	  used with the Cadence USB controller.
>> +	  If M is selected, the module will be called
>> +	  phy-jh7110-usb.ko.
>> +
>> +config PHY_STARFIVE_JH7110_PCIE
> 
> Sorted alphabetically please
> 
>> +	tristate "Starfive JH7110 PCIE 2.0/USB 3.0 PHY support"
>> +	depends on USB_SUPPORT
>> +	select GENERIC_PHY
>> +	select USB_PHY
>> +	help
>> +	  Enable this to support the StarFive PCIe 2.0 PHY,
>> +	  or used as USB 3.0 PHY.
>> +	  If M is selected, the module will be called
>> +	  phy-jh7110-pcie.ko.
>> diff --git a/drivers/phy/starfive/Makefile b/drivers/phy/starfive/Makefile
>> index 7ec576cb30ae..c3eaf1b34cbb 100644
>> --- a/drivers/phy/starfive/Makefile
>> +++ b/drivers/phy/starfive/Makefile
>> @@ -1,2 +1,4 @@
>>  # SPDX-License-Identifier: GPL-2.0
>>  obj-$(CONFIG_PHY_STARFIVE_DPHY_RX)      += phy-starfive-dphy-rx.o
>> +obj-$(CONFIG_PHY_STARFIVE_JH7110_USB)	+= phy-jh7110-usb.o
>> +obj-$(CONFIG_PHY_STARFIVE_JH7110_PCIE)	+= phy-jh7110-pcie.o
> 
> Here also
> 
ok， thanks
>> diff --git a/drivers/phy/starfive/phy-jh7110-pcie.c b/drivers/phy/starfive/phy-jh7110-pcie.c
>> new file mode 100644
>> index 000000000000..30a8fa1f580d
>> --- /dev/null
>> +++ b/drivers/phy/starfive/phy-jh7110-pcie.c
>> @@ -0,0 +1,136 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * StarFive JH7110 PCIe 2.0 PHY driver
>> + *
>> + * Copyright (C) 2023 Minda Chen <minda.chen@starfivetech.com>
>> + */
>> +
>> +#include <linux/bits.h>
>> +#include <linux/clk.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define PCIE_KVCO_LEVEL_OFF		(0x28)
>> +#define PCIE_USB3_PHY_PLL_CTL_OFF	(0x7c)
>> +#define PCIE_KVCO_TUNE_SIGNAL_OFF	(0x80)
>> +#define PCIE_USB3_PHY_ENABLE		BIT(4)
>> +#define PHY_KVCO_FINE_TUNE_LEVEL	0x91
>> +#define PHY_KVCO_FINE_TUNE_SIGNALS	0xc
>> +
>> +struct jh7110_pcie_phy {
>> +	struct phy *phy;
>> +	void __iomem *regs;
>> +	enum phy_mode mode;
>> +};
>> +
>> +static void jh7110_usb3_mode_set(struct jh7110_pcie_phy *phy)
>> +{
>> +	/* Configuare spread-spectrum mode: down-spread-spectrum */
>> +	writel(PCIE_USB3_PHY_ENABLE, phy->regs + PCIE_USB3_PHY_PLL_CTL_OFF);
>> +}
>> +
>> +static void jh7110_pcie_mode_set(struct jh7110_pcie_phy *phy)
>> +{
>> +	/* PCIe Multi-PHY PLL KVCO Gain fine tune settings: */
>> +	writel(PHY_KVCO_FINE_TUNE_LEVEL, phy->regs + PCIE_KVCO_LEVEL_OFF);
>> +	writel(PHY_KVCO_FINE_TUNE_SIGNALS, phy->regs + PCIE_KVCO_TUNE_SIGNAL_OFF);
>> +}
>> +
>> +static int jh7110_pcie_phy_set_mode(struct phy *_phy,
>> +				  enum phy_mode mode, int submode)
>> +{
>> +	struct jh7110_pcie_phy *phy = phy_get_drvdata(_phy);
>> +
>> +	if (mode != phy->mode) {
>> +		switch (mode) {
>> +		case PHY_MODE_USB_HOST:
>> +		case PHY_MODE_USB_DEVICE:
>> +		case PHY_MODE_USB_OTG:
>> +			jh7110_usb3_mode_set(phy);
>> +			break;
>> +		case PHY_MODE_PCIE:
>> +			jh7110_pcie_mode_set(phy);
>> +			break;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +
>> +		dev_info(&_phy->dev, "Changing phy mode to %d\n", mode);
>> +		phy->mode = mode;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int jh7110_pcie_phy_init(struct phy *_phy)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int jh7110_pcie_phy_exit(struct phy *_phy)
>> +{
>> +	return 0;
>> +}
>> +
>> +static const struct phy_ops jh7110_pcie_phy_ops = {
>> +	.init		= jh7110_pcie_phy_init,
>> +	.exit		= jh7110_pcie_phy_exit,
>> +	.set_mode	= jh7110_pcie_phy_set_mode,
>> +	.owner		= THIS_MODULE,
>> +};
>> +
>> +static int jh7110_pcie_phy_probe(struct platform_device *pdev)
>> +{
>> +	struct jh7110_pcie_phy *phy;
>> +	struct device *dev = &pdev->dev;
>> +	struct phy_provider *phy_provider;
>> +
>> +	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
>> +	if (!phy)
>> +		return -ENOMEM;
>> +
>> +	phy->regs = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(phy->regs))
>> +		return PTR_ERR(phy->regs);
>> +
>> +	phy->phy = devm_phy_create(dev, NULL, &jh7110_pcie_phy_ops);
>> +	if (IS_ERR(phy->phy))
>> +		return dev_err_probe(dev, PTR_ERR(phy->regs),
>> +			"Failed to map phy base\n");
>> +
>> +	platform_set_drvdata(pdev, phy);
>> +	phy_set_drvdata(phy->phy, phy);
>> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +
>> +	return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static int jh7110_pcie_phy_remove(struct platform_device *pdev)
>> +{
>> +	platform_set_drvdata(pdev, NULL);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id jh7110_pcie_phy_of_match[] = {
>> +	{ .compatible = "starfive,jh7110-pcie-phy" },
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, jh7110_pcie_phy_of_match);
>> +
>> +static struct platform_driver jh7110_pcie_phy_driver = {
>> +	.probe	= jh7110_pcie_phy_probe,
>> +	.remove	= jh7110_pcie_phy_remove,
>> +	.driver = {
>> +		.of_match_table	= jh7110_pcie_phy_of_match,
>> +		.name  = "jh7110-pcie-phy",
>> +	}
>> +};
>> +module_platform_driver(jh7110_pcie_phy_driver);
>> +
>> +MODULE_DESCRIPTION("StarFive JH7110 PCIe 2.0 PHY driver");
>> +MODULE_AUTHOR("Minda Chen <minda.chen@starfivetech.com>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/phy/starfive/phy-jh7110-usb.c b/drivers/phy/starfive/phy-jh7110-usb.c
>> new file mode 100644
>> index 000000000000..89db0b7b1224
>> --- /dev/null
>> +++ b/drivers/phy/starfive/phy-jh7110-usb.c
>> @@ -0,0 +1,167 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * StarFive JH7110 USB 2.0 PHY driver
> 
> Since these two seem to be different driver, pls split the patches to
> two, one for each phy driver
> 
ok
>> + *
>> + * Copyright (C) 2023 Minda Chen <minda.chen@starfivetech.com>
>> + */
>> +
>> +#include <linux/bits.h>
>> +#include <linux/clk.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/usb/of.h>
>> +
>> +#define USB_125M_CLK_RATE		125000000
>> +#define USB_LS_KEEPALIVE_OFF		0x4
>> +#define USB_LS_KEEPALIVE_ENABLE		BIT(4)
>> +
>> +struct jh7110_usb2_phy {
>> +	struct phy *phy;
>> +	void __iomem *regs;
>> +	struct clk *usb_125m_clk;
>> +	struct clk *app_125;
>> +	enum usb_dr_mode dr_mode;
>> +};
>> +
>> +static void jh7110_usb2_mode_set(struct jh7110_usb2_phy *phy)
>> +{
>> +	unsigned int val;
>> +
>> +	if (phy->dr_mode != USB_DR_MODE_PERIPHERAL) {
>> +		/* Enable the LS speed keep-alive signal */
>> +		val = readl(phy->regs + USB_LS_KEEPALIVE_OFF);
>> +		val |= USB_LS_KEEPALIVE_ENABLE;
>> +		writel(val, phy->regs + USB_LS_KEEPALIVE_OFF);
>> +	}
>> +}
>> +
>> +static int jh7110_usb2_phy_set_mode(struct phy *_phy,
>> +				  enum phy_mode mode, int submode)
>> +{
>> +	struct jh7110_usb2_phy *phy = phy_get_drvdata(_phy);
>> +	int new_mode;
>> +
>> +	switch (mode) {
>> +	case PHY_MODE_USB_HOST:
>> +		new_mode = USB_DR_MODE_HOST;
>> +		break;
>> +	case PHY_MODE_USB_DEVICE:
>> +		new_mode = USB_DR_MODE_PERIPHERAL;
>> +		break;
>> +	case PHY_MODE_USB_OTG:
>> +		new_mode = USB_DR_MODE_OTG;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (new_mode != phy->dr_mode) {
>> +		dev_info(&_phy->dev, "Changing dr_mode to %d\n", new_mode);
>> +		phy->dr_mode = new_mode;
>> +		jh7110_usb2_mode_set(phy);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int jh7110_usb2_phy_init(struct phy *_phy)
>> +{
>> +	struct jh7110_usb2_phy *phy = phy_get_drvdata(_phy);
>> +	int ret;
>> +
>> +	ret = clk_set_rate(phy->usb_125m_clk, USB_125M_CLK_RATE);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = clk_prepare_enable(phy->app_125);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int jh7110_usb2_phy_exit(struct phy *_phy)
>> +{
>> +	struct jh7110_usb2_phy *phy = phy_get_drvdata(_phy);
>> +
>> +	clk_disable_unprepare(phy->app_125);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct phy_ops jh7110_usb2_phy_ops = {
>> +	.init		= jh7110_usb2_phy_init,
>> +	.exit		= jh7110_usb2_phy_exit,
>> +	.set_mode	= jh7110_usb2_phy_set_mode,
>> +	.owner		= THIS_MODULE,
>> +};
>> +
>> +static int jh7110_usb_phy_probe(struct platform_device *pdev)
>> +{
>> +	struct jh7110_usb2_phy *phy;
>> +	struct device *dev = &pdev->dev;
>> +	struct phy_provider *phy_provider;
>> +
>> +	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
>> +	if (!phy)
>> +		return -ENOMEM;
>> +
>> +	phy->usb_125m_clk = devm_clk_get(dev, "125m");
>> +	if (IS_ERR(phy->usb_125m_clk))
>> +		return dev_err_probe(dev, PTR_ERR(phy->usb_125m_clk),
>> +			"Failed to get 125m clock\n");
>> +
>> +	phy->app_125 = devm_clk_get(dev, "app_125");
>> +	if (IS_ERR(phy->app_125))
>> +		return dev_err_probe(dev, PTR_ERR(phy->app_125),
>> +			"Failed to get app 125m clock\n");
>> +
>> +	phy->regs = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(phy->regs))
>> +		return dev_err_probe(dev, PTR_ERR(phy->regs),
>> +			"Failed to map phy base\n");
>> +
>> +	phy->phy = devm_phy_create(dev, NULL, &jh7110_usb2_phy_ops);
>> +	if (IS_ERR(phy->phy))
>> +		return dev_err_probe(dev, PTR_ERR(phy->phy),
>> +			"Failed to create phy\n");
>> +
>> +	platform_set_drvdata(pdev, phy);
>> +	phy_set_drvdata(phy->phy, phy);
>> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +
>> +	return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static int jh7110_usb_phy_remove(struct platform_device *pdev)
>> +{
>> +	struct jh7110_usb2_phy *phy = platform_get_drvdata(pdev);
>> +
>> +	clk_disable_unprepare(phy->app_125);
>> +	platform_set_drvdata(pdev, NULL);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id jh7110_usb_phy_of_match[] = {
>> +	{ .compatible = "starfive,jh7110-usb-phy" },
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, jh7110_usb_phy_of_match);
>> +
>> +static struct platform_driver jh7110_usb_phy_driver = {
>> +	.probe	= jh7110_usb_phy_probe,
>> +	.remove	= jh7110_usb_phy_remove,
>> +	.driver = {
>> +		.of_match_table	= jh7110_usb_phy_of_match,
>> +		.name  = "jh7110-usb-phy",
>> +	}
>> +};
>> +module_platform_driver(jh7110_usb_phy_driver);
>> +
>> +MODULE_DESCRIPTION("StarFive JH7110 USB 2.0 PHY driver");
>> +MODULE_AUTHOR("Minda Chen <minda.chen@starfivetech.com>");
>> +MODULE_LICENSE("GPL");
>> -- 
>> 2.17.1
> 

[PATCH v3 3/5] dt-binding: Add JH7110 USB wrapper layer doc.

[Minda Chen]

The dt-binding doc of Cadence USBSS-DRD controller wrapper
layer.

Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
 .../bindings/usb/starfive,jh7110-usb.yaml     | 119 ++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml

diff --git a/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
new file mode 100644
index 000000000000..b1a8dc6d7b4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
@@ -0,0 +1,119 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/starfive,jh7110-usb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 wrapper module for the Cadence USBSS-DRD controller
+
+maintainers:
+  - Minda Chen <minda.chen@starfivetech.com>
+
+properties:
+  compatible:
+    const: starfive,jh7110-usb
+
+  clocks:
+    items:
+      - description: lpm clock
+      - description: stb clock
+      - description: apb clock
+      - description: axi clock
+      - description: utmi apb clock
+
+  clock-names:
+    items:
+      - const: lpm
+      - const: stb
+      - const: apb
+      - const: axi
+      - const: utmi_apb
+
+  resets:
+    items:
+      - description: PWRUP reset
+      - description: APB reset
+      - description: AXI reset
+      - description: UTMI_APB reset
+
+  starfive,sys-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle to System Register Controller sys_syscon node.
+        - description: offset of SYS_SYSCONSAIF__SYSCFG register for USB.
+    description:
+      The phandle to System Register Controller syscon node and the offset
+      of SYS_SYSCONSAIF__SYSCFG register for USB.
+
+  starfive,stg-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle to System Register Controller stg_syscon node.
+        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for USB.
+        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for USB.
+        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for USB.
+        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for USB.
+    description:
+      The phandle to System Register Controller syscon node and the offset
+      of STG_SYSCONSAIF__SYSCFG register for USB. Total 4 regsisters offset
+      for USB.
+
+  "#address-cells":
+    maximum: 2
+
+  "#size-cells":
+    maximum: 2
+
+  ranges: true
+
+patternProperties:
+  "^usb@[0-9a-f]+$":
+    type: object
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - resets
+  - starfive,sys-syscon
+  - starfive,stg-syscon
+  - "#address-cells"
+  - "#size-cells"
+  - ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    usb@10100000 {
+      compatible = "starfive,jh7110-usb";
+      clocks = <&syscrg 4>,
+               <&stgcrg 5>,
+               <&stgcrg 1>,
+               <&stgcrg 3>,
+               <&stgcrg 2>;
+      clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
+      resets = <&stgcrg 10>,
+               <&stgcrg 8>,
+               <&stgcrg 7>,
+               <&stgcrg 9>;
+      starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
+      starfive,sys-syscon = <&sys_syscon 0x18>;
+      #address-cells = <1>;
+      #size-cells = <1>;
+      ranges = <0x0 0x10100000 0x100000>;
+
+      usb@0 {
+        compatible = "cdns,usb3";
+        reg = <0x0 0x10000>,
+              <0x10000 0x10000>,
+              <0x20000 0x10000>;
+        reg-names = "otg", "xhci", "dev";
+        interrupts = <100>, <108>, <110>;
+        interrupt-names = "host", "peripheral", "otg";
+        maximum-speed = "super-speed";
+        dr_mode = "host";
+      };
+    };
-- 
2.17.1

Re: [PATCH v3 3/5] dt-binding: Add JH7110 USB wrapper layer doc.

[Peter Chen]

On 23-03-15 18:44:09, Minda Chen wrote:
> The dt-binding doc of Cadence USBSS-DRD controller wrapper
> layer.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>

Reviewed-by: Peter Chen <peter.chen@kernel.org>

> ---
>  .../bindings/usb/starfive,jh7110-usb.yaml     | 119 ++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> new file mode 100644
> index 000000000000..b1a8dc6d7b4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/starfive,jh7110-usb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 wrapper module for the Cadence USBSS-DRD controller
> +
> +maintainers:
> +  - Minda Chen <minda.chen@starfivetech.com>
> +
> +properties:
> +  compatible:
> +    const: starfive,jh7110-usb
> +
> +  clocks:
> +    items:
> +      - description: lpm clock
> +      - description: stb clock
> +      - description: apb clock
> +      - description: axi clock
> +      - description: utmi apb clock
> +
> +  clock-names:
> +    items:
> +      - const: lpm
> +      - const: stb
> +      - const: apb
> +      - const: axi
> +      - const: utmi_apb
> +
> +  resets:
> +    items:
> +      - description: PWRUP reset
> +      - description: APB reset
> +      - description: AXI reset
> +      - description: UTMI_APB reset
> +
> +  starfive,sys-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      items:
> +        - description: phandle to System Register Controller sys_syscon node.
> +        - description: offset of SYS_SYSCONSAIF__SYSCFG register for USB.
> +    description:
> +      The phandle to System Register Controller syscon node and the offset
> +      of SYS_SYSCONSAIF__SYSCFG register for USB.
> +
> +  starfive,stg-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      items:
> +        - description: phandle to System Register Controller stg_syscon node.
> +        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for USB.
> +        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for USB.
> +        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for USB.
> +        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for USB.
> +    description:
> +      The phandle to System Register Controller syscon node and the offset
> +      of STG_SYSCONSAIF__SYSCFG register for USB. Total 4 regsisters offset
> +      for USB.
> +
> +  "#address-cells":
> +    maximum: 2
> +
> +  "#size-cells":
> +    maximum: 2
> +
> +  ranges: true
> +
> +patternProperties:
> +  "^usb@[0-9a-f]+$":
> +    type: object
> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - resets
> +  - starfive,sys-syscon
> +  - starfive,stg-syscon
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    usb@10100000 {
> +      compatible = "starfive,jh7110-usb";
> +      clocks = <&syscrg 4>,
> +               <&stgcrg 5>,
> +               <&stgcrg 1>,
> +               <&stgcrg 3>,
> +               <&stgcrg 2>;
> +      clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
> +      resets = <&stgcrg 10>,
> +               <&stgcrg 8>,
> +               <&stgcrg 7>,
> +               <&stgcrg 9>;
> +      starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
> +      starfive,sys-syscon = <&sys_syscon 0x18>;
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +      ranges = <0x0 0x10100000 0x100000>;
> +
> +      usb@0 {
> +        compatible = "cdns,usb3";
> +        reg = <0x0 0x10000>,
> +              <0x10000 0x10000>,
> +              <0x20000 0x10000>;
> +        reg-names = "otg", "xhci", "dev";
> +        interrupts = <100>, <108>, <110>;
> +        interrupt-names = "host", "peripheral", "otg";
> +        maximum-speed = "super-speed";
> +        dr_mode = "host";
> +      };
> +    };
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen

Re: [PATCH v3 3/5] dt-binding: Add JH7110 USB wrapper layer doc.

[Krzysztof Kozlowski]

On 15/03/2023 11:44, Minda Chen wrote:
> The dt-binding doc of Cadence USBSS-DRD controller wrapper
> layer.

Subject: drop full stop. It's not a sentence.

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).


> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  .../bindings/usb/starfive,jh7110-usb.yaml     | 119 ++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> new file mode 100644
> index 000000000000..b1a8dc6d7b4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/starfive,jh7110-usb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 wrapper module for the Cadence USBSS-DRD controller
> +
> +maintainers:
> +  - Minda Chen <minda.chen@starfivetech.com>
> +
> +properties:
> +  compatible:
> +    const: starfive,jh7110-usb
> +
> +  clocks:
> +    items:
> +      - description: lpm clock
> +      - description: stb clock
> +      - description: apb clock
> +      - description: axi clock
> +      - description: utmi apb clock
> +
> +  clock-names:
> +    items:
> +      - const: lpm
> +      - const: stb
> +      - const: apb
> +      - const: axi
> +      - const: utmi_apb
> +
> +  resets:
> +    items:
> +      - description: PWRUP reset
> +      - description: APB reset
> +      - description: AXI reset
> +      - description: UTMI_APB reset
> +
> +  starfive,sys-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      items:
> +        - description: phandle to System Register Controller sys_syscon node.
> +        - description: offset of SYS_SYSCONSAIF__SYSCFG register for USB.
> +    description:
> +      The phandle to System Register Controller syscon node and the offset
> +      of SYS_SYSCONSAIF__SYSCFG register for USB.
> +
> +  starfive,stg-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      items:
> +        - description: phandle to System Register Controller stg_syscon node.
> +        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for USB.
> +        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for USB.
> +        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for USB.
> +        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for USB.
> +    description:
> +      The phandle to System Register Controller syscon node and the offset
> +      of STG_SYSCONSAIF__SYSCFG register for USB. Total 4 regsisters offset
> +      for USB.
> +
> +  "#address-cells":
> +    maximum: 2

enum: [ 1, 2 ]
(because 0 should not be valid for you)

> +
> +  "#size-cells":
> +    maximum: 2

ditto

> +
> +  ranges: true
> +
> +patternProperties:
> +  "^usb@[0-9a-f]+$":
> +    type: object

missing $ref and unevaluatedProperties: false

> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - resets
> +  - starfive,sys-syscon
> +  - starfive,stg-syscon
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
> +
> +additionalProperties: false


Best regards,
Krzysztof

Re: [PATCH v3 3/5] dt-binding: Add JH7110 USB wrapper layer doc.

[Minda Chen]

On 2023/3/17 16:43, Krzysztof Kozlowski wrote:
> On 15/03/2023 11:44, Minda Chen wrote:
>> The dt-binding doc of Cadence USBSS-DRD controller wrapper
>> layer.
> 
> Subject: drop full stop. It's not a sentence.
> 
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
> 
> 
Thanks. I should check all the commits title and commit messages.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  .../bindings/usb/starfive,jh7110-usb.yaml     | 119 ++++++++++++++++++
>>  1 file changed, 119 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> new file mode 100644
>> index 000000000000..b1a8dc6d7b4b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> @@ -0,0 +1,119 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/starfive,jh7110-usb.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive JH7110 wrapper module for the Cadence USBSS-DRD controller
>> +
>> +maintainers:
>> +  - Minda Chen <minda.chen@starfivetech.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: starfive,jh7110-usb
>> +
>> +  clocks:
>> +    items:
>> +      - description: lpm clock
>> +      - description: stb clock
>> +      - description: apb clock
>> +      - description: axi clock
>> +      - description: utmi apb clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: lpm
>> +      - const: stb
>> +      - const: apb
>> +      - const: axi
>> +      - const: utmi_apb
>> +
>> +  resets:
>> +    items:
>> +      - description: PWRUP reset
>> +      - description: APB reset
>> +      - description: AXI reset
>> +      - description: UTMI_APB reset
>> +
>> +  starfive,sys-syscon:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    items:
>> +      items:
>> +        - description: phandle to System Register Controller sys_syscon node.
>> +        - description: offset of SYS_SYSCONSAIF__SYSCFG register for USB.
>> +    description:
>> +      The phandle to System Register Controller syscon node and the offset
>> +      of SYS_SYSCONSAIF__SYSCFG register for USB.
>> +
>> +  starfive,stg-syscon:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    items:
>> +      items:
>> +        - description: phandle to System Register Controller stg_syscon node.
>> +        - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for USB.
>> +        - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for USB.
>> +        - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for USB.
>> +        - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for USB.
>> +    description:
>> +      The phandle to System Register Controller syscon node and the offset
>> +      of STG_SYSCONSAIF__SYSCFG register for USB. Total 4 regsisters offset
>> +      for USB.
>> +
>> +  "#address-cells":
>> +    maximum: 2
> 
> enum: [ 1, 2 ]
> (because 0 should not be valid for you)
> 
>> +
>> +  "#size-cells":
>> +    maximum: 2
> 
> ditto
> 
ok
>> +
>> +  ranges: true
>> +
>> +patternProperties:
>> +  "^usb@[0-9a-f]+$":
>> +    type: object
> 
> missing $ref and unevaluatedProperties: false
> 
ok, thanks
>> +
>> +required:
>> +  - compatible
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - starfive,sys-syscon
>> +  - starfive,stg-syscon
>> +  - "#address-cells"
>> +  - "#size-cells"
>> +  - ranges
>> +
>> +additionalProperties: false
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 3/5] dt-binding: Add JH7110 USB wrapper layer doc.

[Philipp Zabel]

On Mi, 2023-03-15 at 18:44 +0800, Minda Chen wrote:
> The dt-binding doc of Cadence USBSS-DRD controller wrapper
> layer.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  .../bindings/usb/starfive,jh7110-usb.yaml     | 119 ++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> new file mode 100644
> index 000000000000..b1a8dc6d7b4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/starfive,jh7110-usb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 wrapper module for the Cadence USBSS-DRD controller
> +
> +maintainers:
> +  - Minda Chen <minda.chen@starfivetech.com>
> +
> +properties:
> +  compatible:
> +    const: starfive,jh7110-usb
> +
> +  clocks:
> +    items:
> +      - description: lpm clock
> +      - description: stb clock
> +      - description: apb clock
> +      - description: axi clock
> +      - description: utmi apb clock
> +
> +  clock-names:
> +    items:
> +      - const: lpm
> +      - const: stb
> +      - const: apb
> +      - const: axi
> +      - const: utmi_apb
> +
> +  resets:
> +    items:
> +      - description: PWRUP reset
> +      - description: APB reset
> +      - description: AXI reset
> +      - description: UTMI_APB reset

I'd add a "reset-names" property, just in case there is ever a reason
to trigger any of the resets independently from the others.

regards
Philipp

Re: [PATCH v3 3/5] dt-binding: Add JH7110 USB wrapper layer doc.

[Minda Chen]

On 2023/3/23 17:23, Philipp Zabel wrote:
> On Mi, 2023-03-15 at 18:44 +0800, Minda Chen wrote:
>> The dt-binding doc of Cadence USBSS-DRD controller wrapper
>> layer.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  .../bindings/usb/starfive,jh7110-usb.yaml     | 119 ++++++++++++++++++
>>  1 file changed, 119 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> new file mode 100644
>> index 000000000000..b1a8dc6d7b4b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> @@ -0,0 +1,119 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/starfive,jh7110-usb.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive JH7110 wrapper module for the Cadence USBSS-DRD controller
>> +
>> +maintainers:
>> +  - Minda Chen <minda.chen@starfivetech.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: starfive,jh7110-usb
>> +
>> +  clocks:
>> +    items:
>> +      - description: lpm clock
>> +      - description: stb clock
>> +      - description: apb clock
>> +      - description: axi clock
>> +      - description: utmi apb clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: lpm
>> +      - const: stb
>> +      - const: apb
>> +      - const: axi
>> +      - const: utmi_apb
>> +
>> +  resets:
>> +    items:
>> +      - description: PWRUP reset
>> +      - description: APB reset
>> +      - description: AXI reset
>> +      - description: UTMI_APB reset
> 
> I'd add a "reset-names" property, just in case there is ever a reason
> to trigger any of the resets independently from the others.
> 
OK, Thanks
> regards
> Philipp

[PATCH v3 4/5] usb: cdns3: add StarFive JH7110 USB driver.

[Minda Chen]

There is a Cadence USB3 core for JH7110 SoCs, the cdns
core is the child of this USB wrapper module device.

Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
 MAINTAINERS                        |   7 +
 drivers/usb/cdns3/Kconfig          |  11 ++
 drivers/usb/cdns3/Makefile         |   1 +
 drivers/usb/cdns3/cdns3-starfive.c | 305 +++++++++++++++++++++++++++++
 4 files changed, 324 insertions(+)
 create mode 100644 drivers/usb/cdns3/cdns3-starfive.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4263c005e45c..c530c966ab26 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19985,6 +19985,13 @@ F:	Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
 F:	drivers/phy/starfive/phy-jh7110-pcie.c
 F:	drivers/phy/starfive/phy-jh7110-usb.c
 
+STARFIVE JH71X0 USB DRIVERS
+M:	Emil Renner Berthing <kernel@esmil.dk>
+M:	Minda Chen <minda.chen@starfivetech.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
+F:	drivers/usb/cdns3/cdns3-starfive.c
+
 STATIC BRANCH/CALL
 M:	Peter Zijlstra <peterz@infradead.org>
 M:	Josh Poimboeuf <jpoimboe@kernel.org>
diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
index b98ca0a1352a..0a514b591527 100644
--- a/drivers/usb/cdns3/Kconfig
+++ b/drivers/usb/cdns3/Kconfig
@@ -78,6 +78,17 @@ config USB_CDNS3_IMX
 
 	  For example, imx8qm and imx8qxp.
 
+config USB_CDNS3_STARFIVE
+	tristate "Cadence USB3 support on StarFive SoC platforms"
+	depends on ARCH_STARFIVE || COMPILE_TEST
+	help
+	  Say 'Y' or 'M' here if you are building for StarFive SoCs
+	  platforms that contain Cadence USB3 controller core.
+
+	  e.g. JH7110.
+
+	  If you choose to build this driver as module it will
+	  be dynamically linked and module will be called cdns3-starfive.ko
 endif
 
 if USB_CDNS_SUPPORT
diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
index 61edb2f89276..48dfae75b5aa 100644
--- a/drivers/usb/cdns3/Makefile
+++ b/drivers/usb/cdns3/Makefile
@@ -24,6 +24,7 @@ endif
 obj-$(CONFIG_USB_CDNS3_PCI_WRAP)		+= cdns3-pci-wrap.o
 obj-$(CONFIG_USB_CDNS3_TI)			+= cdns3-ti.o
 obj-$(CONFIG_USB_CDNS3_IMX)			+= cdns3-imx.o
+obj-$(CONFIG_USB_CDNS3_STARFIVE)		+= cdns3-starfive.o
 
 cdnsp-udc-pci-y					:= cdnsp-pci.o
 
diff --git a/drivers/usb/cdns3/cdns3-starfive.c b/drivers/usb/cdns3/cdns3-starfive.c
new file mode 100644
index 000000000000..a99f98f85235
--- /dev/null
+++ b/drivers/usb/cdns3/cdns3-starfive.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * cdns3-starfive.c - StarFive specific Glue layer for Cadence USB Controller
+ *
+ * Copyright (C) 2022 Starfive, Inc.
+ * Author:	Yanhong Wang <yanhong.wang@starfivetech.com>
+ * Author:	Mason Huo <mason.huo@starfivetech.com>
+ * Author:	Minda Chen <minda.chen@starfivetech.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/usb/otg.h>
+#include "core.h"
+
+#define USB_STRAP_HOST			BIT(17)
+#define USB_STRAP_DEVICE		BIT(18)
+#define USB_STRAP_MASK			GENMASK(18, 16)
+
+#define USB_SUSPENDM_HOST		BIT(19)
+#define USB_SUSPENDM_MASK		BIT(19)
+
+#define USB_SUSPENDM_BYPS		BIT(20)
+#define USB_REFCLK_MODE			BIT(23)
+#define USB_PLL_EN			BIT(22)
+#define USB_PDRSTN_SPLIT		BIT(17)
+
+#define PCIE_CKREF_SRC_MASK		GENMASK(19, 18)
+#define PCIE_CLK_SEL_MASK		GENMASK(21, 20)
+#define PCIE_PHY_MODE			BIT(20)
+#define PCIE_PHY_MODE_MASK		GENMASK(21, 20)
+#define PCIE_USB3_BUS_WIDTH_MASK	GENMASK(3, 2)
+#define PCIE_USB3_RATE_MASK		GENMASK(6, 5)
+#define PCIE_USB3_RX_STANDBY_MASK	BIT(7)
+#define PCIE_USB3_PHY_ENABLE		BIT(4)
+
+struct cdns_starfive {
+	struct device *dev;
+	struct regmap *stg_syscon;
+	struct regmap *sys_syscon;
+	struct reset_control *resets;
+	struct clk_bulk_data *clks;
+	int num_clks;
+	u32 sys_offset;
+	u32 stg_offset_4;
+	u32 stg_offset_196;
+	u32 stg_offset_328;
+	u32 stg_offset_500;
+	bool usb2_only;
+};
+
+static int cdns_mode_init(struct platform_device *pdev,
+				struct cdns_starfive *data, const char **out_mode)
+{
+	struct device_node *child;
+	const char *dr_mode = NULL;
+
+	child = of_get_compatible_child(pdev->dev.of_node, "cdns,usb3");
+	if (!child) {
+		return dev_err_probe(&pdev->dev, -ENODEV,
+			"Failed to find child node\n");
+	}
+
+	/* Init usb 2.0 utmi phy */
+	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
+		USB_SUSPENDM_BYPS, USB_SUSPENDM_BYPS);
+	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
+		USB_PLL_EN, USB_PLL_EN);
+	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
+		USB_REFCLK_MODE, USB_REFCLK_MODE);
+
+	if (!of_find_property(child, "cdns3,usb3-phy", NULL)) {
+		/* Disconnect usb 3.0 phy mode */
+		regmap_update_bits(data->sys_syscon, data->sys_offset,
+			USB_PDRSTN_SPLIT, USB_PDRSTN_SPLIT);
+		data->usb2_only = 1;
+	} else {
+		/* Config usb 3.0 pipe phy */
+		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
+			PCIE_CKREF_SRC_MASK, 0);
+		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
+			PCIE_CLK_SEL_MASK, 0);
+		regmap_update_bits(data->stg_syscon, data->stg_offset_328,
+			PCIE_PHY_MODE_MASK, PCIE_PHY_MODE);
+		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
+			PCIE_USB3_BUS_WIDTH_MASK, 0);
+		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
+			PCIE_USB3_RATE_MASK, 0);
+		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
+			PCIE_USB3_RX_STANDBY_MASK, 0);
+		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
+			PCIE_USB3_PHY_ENABLE, PCIE_USB3_PHY_ENABLE);
+
+		/* Connect usb 3.0 phy mode */
+		regmap_update_bits(data->sys_syscon, data->sys_offset,
+			USB_PDRSTN_SPLIT, 0);
+	}
+
+	if (!of_property_read_string(child, "dr_mode", &dr_mode)) {
+		if (!strcmp(dr_mode, "host")) {
+			regmap_update_bits(data->stg_syscon,
+				data->stg_offset_4,
+				USB_STRAP_MASK,
+				USB_STRAP_HOST);
+			regmap_update_bits(data->stg_syscon,
+				data->stg_offset_4,
+				USB_SUSPENDM_MASK,
+				USB_SUSPENDM_HOST);
+		} else if (!strcmp(dr_mode, "peripheral")) {
+			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
+				USB_STRAP_MASK, USB_STRAP_DEVICE);
+			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
+				USB_SUSPENDM_MASK, 0);
+		}
+	}
+
+	if (out_mode)
+		*out_mode = dr_mode;
+
+	return 0;
+}
+
+static int cdns_clk_rst_init(struct cdns_starfive *data)
+{
+	int ret;
+
+	data->num_clks = devm_clk_bulk_get_all(data->dev, &data->clks);
+	if (data->num_clks < 0)
+		return dev_err_probe(data->dev, -ENODEV,
+			"Failed to get clocks\n");
+
+	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
+	if (ret)
+		return dev_err_probe(data->dev, ret,
+			"failed to enable clocks\n");
+
+	data->resets = devm_reset_control_array_get_exclusive(data->dev);
+	if (IS_ERR(data->resets)) {
+		ret = dev_err_probe(data->dev, PTR_ERR(data->resets),
+			"Failed to get resets");
+		goto err_clk_init;
+	}
+
+	ret = reset_control_deassert(data->resets);
+	if (ret) {
+		ret = dev_err_probe(data->dev, ret,
+			"failed to reset clocks\n");
+		goto err_clk_init;
+	}
+
+	return ret;
+
+err_clk_init:
+	clk_bulk_disable_unprepare(data->num_clks, data->clks);
+	return ret;
+}
+
+static int cdns_starfive_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = pdev->dev.of_node;
+	struct cdns_starfive *data;
+	unsigned int args[4];
+	const char *dr_mode;
+	int ret;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, data);
+
+	data->dev = dev;
+
+	data->stg_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
+		"starfive,stg-syscon", 4, args);
+
+	if (IS_ERR(data->stg_syscon))
+		return dev_err_probe(dev, PTR_ERR(data->stg_syscon),
+			"Failed to parse starfive,stg-syscon\n");
+
+	data->stg_offset_4 = args[0];
+	data->stg_offset_196 = args[1];
+	data->stg_offset_328 = args[2];
+	data->stg_offset_500 = args[3];
+
+	data->sys_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
+		"starfive,sys-syscon", 1, args);
+	if (IS_ERR(data->sys_syscon))
+		return dev_err_probe(dev, PTR_ERR(data->sys_syscon),
+			"Failed to parse starfive,sys-syscon\n");
+
+	data->sys_offset = args[0];
+
+	ret = cdns_mode_init(pdev, data, &dr_mode);
+	if (ret)
+		return ret;
+
+	ret = cdns_clk_rst_init(data);
+	if (ret)
+		return ret;
+
+	ret = of_platform_populate(node, NULL, NULL, dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to create children\n");
+
+	device_set_wakeup_capable(dev, true);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
+	dev_info(dev, "usb mode %s %s probe success\n",
+		dr_mode ? dr_mode : "unknown", data->usb2_only ? "2.0" : "3.0");
+
+	return 0;
+}
+
+static int cdns_starfive_remove_core(struct device *dev, void *c)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	platform_device_unregister(pdev);
+
+	return 0;
+}
+
+static int cdns_starfive_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cdns_starfive *data = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(dev);
+	device_for_each_child(dev, NULL, cdns_starfive_remove_core);
+
+	reset_control_assert(data->resets);
+	clk_bulk_disable_unprepare(data->num_clks, data->clks);
+	pm_runtime_disable(dev);
+	pm_runtime_put_noidle(dev);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int cdns_starfive_resume(struct device *dev)
+{
+	struct cdns_starfive *data = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(data->resets);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int cdns_starfive_suspend(struct device *dev)
+{
+	struct cdns_starfive *data = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(data->num_clks, data->clks);
+	reset_control_assert(data->resets);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops cdns_starfive_pm_ops = {
+	SET_RUNTIME_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume)
+};
+
+static const struct of_device_id cdns_starfive_of_match[] = {
+	{ .compatible = "starfive,jh7110-usb", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, cdns_starfive_of_match);
+
+static struct platform_driver cdns_starfive_driver = {
+	.probe		= cdns_starfive_probe,
+	.remove		= cdns_starfive_remove,
+	.driver		= {
+		.name	= "cdns3-starfive",
+		.of_match_table	= cdns_starfive_of_match,
+		.pm	= &cdns_starfive_pm_ops,
+	},
+};
+module_platform_driver(cdns_starfive_driver);
+
+MODULE_ALIAS("platform:cdns3-starfive");
+MODULE_AUTHOR("YanHong Wang <yanhong.wang@starfivetech.com>");
+MODULE_AUTHOR("Mason Huo <mason.huo@starfivetech.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
-- 
2.17.1

Re: [PATCH v3 4/5] usb: cdns3: add StarFive JH7110 USB driver.

[Dongliang Mu]

.

On Wed, Mar 15, 2023 at 6:48 PM Minda Chen <minda.chen@starfivetech.com> wrote:
>
> There is a Cadence USB3 core for JH7110 SoCs, the cdns
> core is the child of this USB wrapper module device.
>
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  MAINTAINERS                        |   7 +
>  drivers/usb/cdns3/Kconfig          |  11 ++
>  drivers/usb/cdns3/Makefile         |   1 +
>  drivers/usb/cdns3/cdns3-starfive.c | 305 +++++++++++++++++++++++++++++
>  4 files changed, 324 insertions(+)
>  create mode 100644 drivers/usb/cdns3/cdns3-starfive.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4263c005e45c..c530c966ab26 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19985,6 +19985,13 @@ F:     Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>  F:     drivers/phy/starfive/phy-jh7110-pcie.c
>  F:     drivers/phy/starfive/phy-jh7110-usb.c
>
> +STARFIVE JH71X0 USB DRIVERS
> +M:     Emil Renner Berthing <kernel@esmil.dk>
> +M:     Minda Chen <minda.chen@starfivetech.com>
> +S:     Maintained
> +F:     Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> +F:     drivers/usb/cdns3/cdns3-starfive.c
> +
>  STATIC BRANCH/CALL
>  M:     Peter Zijlstra <peterz@infradead.org>
>  M:     Josh Poimboeuf <jpoimboe@kernel.org>
> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
> index b98ca0a1352a..0a514b591527 100644
> --- a/drivers/usb/cdns3/Kconfig
> +++ b/drivers/usb/cdns3/Kconfig
> @@ -78,6 +78,17 @@ config USB_CDNS3_IMX
>
>           For example, imx8qm and imx8qxp.
>
> +config USB_CDNS3_STARFIVE
> +       tristate "Cadence USB3 support on StarFive SoC platforms"
> +       depends on ARCH_STARFIVE || COMPILE_TEST
> +       help
> +         Say 'Y' or 'M' here if you are building for StarFive SoCs
> +         platforms that contain Cadence USB3 controller core.
> +
> +         e.g. JH7110.
> +
> +         If you choose to build this driver as module it will
> +         be dynamically linked and module will be called cdns3-starfive.ko
>  endif
>
>  if USB_CDNS_SUPPORT
> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
> index 61edb2f89276..48dfae75b5aa 100644
> --- a/drivers/usb/cdns3/Makefile
> +++ b/drivers/usb/cdns3/Makefile
> @@ -24,6 +24,7 @@ endif
>  obj-$(CONFIG_USB_CDNS3_PCI_WRAP)               += cdns3-pci-wrap.o
>  obj-$(CONFIG_USB_CDNS3_TI)                     += cdns3-ti.o
>  obj-$(CONFIG_USB_CDNS3_IMX)                    += cdns3-imx.o
> +obj-$(CONFIG_USB_CDNS3_STARFIVE)               += cdns3-starfive.o
>
>  cdnsp-udc-pci-y                                        := cdnsp-pci.o
>
> diff --git a/drivers/usb/cdns3/cdns3-starfive.c b/drivers/usb/cdns3/cdns3-starfive.c
> new file mode 100644
> index 000000000000..a99f98f85235
> --- /dev/null
> +++ b/drivers/usb/cdns3/cdns3-starfive.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * cdns3-starfive.c - StarFive specific Glue layer for Cadence USB Controller
> + *
> + * Copyright (C) 2022 Starfive, Inc.
> + * Author:     Yanhong Wang <yanhong.wang@starfivetech.com>
> + * Author:     Mason Huo <mason.huo@starfivetech.com>
> + * Author:     Minda Chen <minda.chen@starfivetech.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/usb/otg.h>
> +#include "core.h"
> +
> +#define USB_STRAP_HOST                 BIT(17)
> +#define USB_STRAP_DEVICE               BIT(18)
> +#define USB_STRAP_MASK                 GENMASK(18, 16)
> +
> +#define USB_SUSPENDM_HOST              BIT(19)
> +#define USB_SUSPENDM_MASK              BIT(19)
> +
> +#define USB_SUSPENDM_BYPS              BIT(20)
> +#define USB_REFCLK_MODE                        BIT(23)
> +#define USB_PLL_EN                     BIT(22)
> +#define USB_PDRSTN_SPLIT               BIT(17)
> +
> +#define PCIE_CKREF_SRC_MASK            GENMASK(19, 18)
> +#define PCIE_CLK_SEL_MASK              GENMASK(21, 20)
> +#define PCIE_PHY_MODE                  BIT(20)
> +#define PCIE_PHY_MODE_MASK             GENMASK(21, 20)
> +#define PCIE_USB3_BUS_WIDTH_MASK       GENMASK(3, 2)
> +#define PCIE_USB3_RATE_MASK            GENMASK(6, 5)
> +#define PCIE_USB3_RX_STANDBY_MASK      BIT(7)
> +#define PCIE_USB3_PHY_ENABLE           BIT(4)
> +
> +struct cdns_starfive {
> +       struct device *dev;
> +       struct regmap *stg_syscon;
> +       struct regmap *sys_syscon;
> +       struct reset_control *resets;
> +       struct clk_bulk_data *clks;
> +       int num_clks;
> +       u32 sys_offset;
> +       u32 stg_offset_4;
> +       u32 stg_offset_196;
> +       u32 stg_offset_328;
> +       u32 stg_offset_500;
> +       bool usb2_only;
> +};
> +
> +static int cdns_mode_init(struct platform_device *pdev,
> +                               struct cdns_starfive *data, const char **out_mode)
> +{
> +       struct device_node *child;
> +       const char *dr_mode = NULL;
> +
> +       child = of_get_compatible_child(pdev->dev.of_node, "cdns,usb3");
> +       if (!child) {
> +               return dev_err_probe(&pdev->dev, -ENODEV,
> +                       "Failed to find child node\n");
> +       }
> +
> +       /* Init usb 2.0 utmi phy */
> +       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +               USB_SUSPENDM_BYPS, USB_SUSPENDM_BYPS);
> +       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +               USB_PLL_EN, USB_PLL_EN);
> +       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +               USB_REFCLK_MODE, USB_REFCLK_MODE);
> +
> +       if (!of_find_property(child, "cdns3,usb3-phy", NULL)) {
> +               /* Disconnect usb 3.0 phy mode */
> +               regmap_update_bits(data->sys_syscon, data->sys_offset,
> +                       USB_PDRSTN_SPLIT, USB_PDRSTN_SPLIT);
> +               data->usb2_only = 1;
> +       } else {
> +               /* Config usb 3.0 pipe phy */
> +               regmap_update_bits(data->stg_syscon, data->stg_offset_196,
> +                       PCIE_CKREF_SRC_MASK, 0);
> +               regmap_update_bits(data->stg_syscon, data->stg_offset_196,
> +                       PCIE_CLK_SEL_MASK, 0);
> +               regmap_update_bits(data->stg_syscon, data->stg_offset_328,
> +                       PCIE_PHY_MODE_MASK, PCIE_PHY_MODE);
> +               regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +                       PCIE_USB3_BUS_WIDTH_MASK, 0);
> +               regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +                       PCIE_USB3_RATE_MASK, 0);
> +               regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +                       PCIE_USB3_RX_STANDBY_MASK, 0);
> +               regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +                       PCIE_USB3_PHY_ENABLE, PCIE_USB3_PHY_ENABLE);
> +
> +               /* Connect usb 3.0 phy mode */
> +               regmap_update_bits(data->sys_syscon, data->sys_offset,
> +                       USB_PDRSTN_SPLIT, 0);
> +       }
> +
> +       if (!of_property_read_string(child, "dr_mode", &dr_mode)) {
> +               if (!strcmp(dr_mode, "host")) {
> +                       regmap_update_bits(data->stg_syscon,
> +                               data->stg_offset_4,
> +                               USB_STRAP_MASK,
> +                               USB_STRAP_HOST);
> +                       regmap_update_bits(data->stg_syscon,
> +                               data->stg_offset_4,
> +                               USB_SUSPENDM_MASK,
> +                               USB_SUSPENDM_HOST);
> +               } else if (!strcmp(dr_mode, "peripheral")) {
> +                       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +                               USB_STRAP_MASK, USB_STRAP_DEVICE);
> +                       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +                               USB_SUSPENDM_MASK, 0);
> +               }
> +       }
> +
> +       if (out_mode)
> +               *out_mode = dr_mode;
> +
> +       return 0;
> +}
> +
> +static int cdns_clk_rst_init(struct cdns_starfive *data)
> +{
> +       int ret;
> +
> +       data->num_clks = devm_clk_bulk_get_all(data->dev, &data->clks);
> +       if (data->num_clks < 0)
> +               return dev_err_probe(data->dev, -ENODEV,
> +                       "Failed to get clocks\n");
> +
> +       ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
> +       if (ret)
> +               return dev_err_probe(data->dev, ret,
> +                       "failed to enable clocks\n");
> +
> +       data->resets = devm_reset_control_array_get_exclusive(data->dev);
> +       if (IS_ERR(data->resets)) {
> +               ret = dev_err_probe(data->dev, PTR_ERR(data->resets),
> +                       "Failed to get resets");
> +               goto err_clk_init;
> +       }
> +
> +       ret = reset_control_deassert(data->resets);
> +       if (ret) {
> +               ret = dev_err_probe(data->dev, ret,
> +                       "failed to reset clocks\n");
> +               goto err_clk_init;
> +       }
> +
> +       return ret;
> +
> +err_clk_init:
> +       clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +       return ret;
> +}
> +
> +static int cdns_starfive_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *node = pdev->dev.of_node;
> +       struct cdns_starfive *data;
> +       unsigned int args[4];
> +       const char *dr_mode;
> +       int ret;
> +
> +       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +
> +       platform_set_drvdata(pdev, data);
> +
> +       data->dev = dev;
> +
> +       data->stg_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
> +               "starfive,stg-syscon", 4, args);
> +
> +       if (IS_ERR(data->stg_syscon))
> +               return dev_err_probe(dev, PTR_ERR(data->stg_syscon),
> +                       "Failed to parse starfive,stg-syscon\n");
> +
> +       data->stg_offset_4 = args[0];
> +       data->stg_offset_196 = args[1];
> +       data->stg_offset_328 = args[2];
> +       data->stg_offset_500 = args[3];
> +
> +       data->sys_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
> +               "starfive,sys-syscon", 1, args);
> +       if (IS_ERR(data->sys_syscon))
> +               return dev_err_probe(dev, PTR_ERR(data->sys_syscon),
> +                       "Failed to parse starfive,sys-syscon\n");
> +
> +       data->sys_offset = args[0];
> +
> +       ret = cdns_mode_init(pdev, data, &dr_mode);
> +       if (ret)
> +               return ret;
> +
> +       ret = cdns_clk_rst_init(data);
> +       if (ret)
> +               return ret;
> +
> +       ret = of_platform_populate(node, NULL, NULL, dev);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "Failed to create children\n");

For this error handling, you need to add some paired undo operations
for cdns_clk_rst_init, i.e., reset_control_assert and
clk_bulk_disable_unprepare.

> +
> +       device_set_wakeup_capable(dev, true);
> +       pm_runtime_set_active(dev);
> +       pm_runtime_enable(dev);
> +
> +       dev_info(dev, "usb mode %s %s probe success\n",
> +               dr_mode ? dr_mode : "unknown", data->usb2_only ? "2.0" : "3.0");
> +
> +       return 0;
> +}
> +
> +static int cdns_starfive_remove_core(struct device *dev, void *c)
> +{
> +       struct platform_device *pdev = to_platform_device(dev);
> +
> +       platform_device_unregister(pdev);
> +
> +       return 0;
> +}
> +
> +static int cdns_starfive_remove(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct cdns_starfive *data = dev_get_drvdata(dev);
> +
> +       pm_runtime_get_sync(dev);
> +       device_for_each_child(dev, NULL, cdns_starfive_remove_core);
> +
> +       reset_control_assert(data->resets);
> +       clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +       pm_runtime_disable(dev);
> +       pm_runtime_put_noidle(dev);
> +       platform_set_drvdata(pdev, NULL);
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int cdns_starfive_resume(struct device *dev)
> +{
> +       struct cdns_starfive *data = dev_get_drvdata(dev);
> +       int ret;
> +
> +       ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
> +       if (ret)
> +               return ret;
> +
> +       ret = reset_control_deassert(data->resets);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static int cdns_starfive_suspend(struct device *dev)
> +{
> +       struct cdns_starfive *data = dev_get_drvdata(dev);
> +
> +       clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +       reset_control_assert(data->resets);
> +
> +       return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops cdns_starfive_pm_ops = {
> +       SET_RUNTIME_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume, NULL)
> +       SET_SYSTEM_SLEEP_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume)
> +};
> +
> +static const struct of_device_id cdns_starfive_of_match[] = {
> +       { .compatible = "starfive,jh7110-usb", },
> +       { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, cdns_starfive_of_match);
> +
> +static struct platform_driver cdns_starfive_driver = {
> +       .probe          = cdns_starfive_probe,
> +       .remove         = cdns_starfive_remove,
> +       .driver         = {
> +               .name   = "cdns3-starfive",
> +               .of_match_table = cdns_starfive_of_match,
> +               .pm     = &cdns_starfive_pm_ops,
> +       },
> +};
> +module_platform_driver(cdns_starfive_driver);
> +
> +MODULE_ALIAS("platform:cdns3-starfive");
> +MODULE_AUTHOR("YanHong Wang <yanhong.wang@starfivetech.com>");
> +MODULE_AUTHOR("Mason Huo <mason.huo@starfivetech.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
> --
> 2.17.1
>

Re: [PATCH v3 4/5] usb: cdns3: add StarFive JH7110 USB driver.

[Minda Chen]

On 2023/3/15 21:32, Dongliang Mu wrote:
> .
> 
> On Wed, Mar 15, 2023 at 6:48 PM Minda Chen <minda.chen@starfivetech.com> wrote:
>>
>> There is a Cadence USB3 core for JH7110 SoCs, the cdns
>> core is the child of this USB wrapper module device.
>>
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  MAINTAINERS                        |   7 +
>>  drivers/usb/cdns3/Kconfig          |  11 ++
>>  drivers/usb/cdns3/Makefile         |   1 +
>>  drivers/usb/cdns3/cdns3-starfive.c | 305 +++++++++++++++++++++++++++++
>>  4 files changed, 324 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/cdns3-starfive.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 4263c005e45c..c530c966ab26 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19985,6 +19985,13 @@ F:     Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>>  F:     drivers/phy/starfive/phy-jh7110-pcie.c
>>  F:     drivers/phy/starfive/phy-jh7110-usb.c
>>
>> +STARFIVE JH71X0 USB DRIVERS
>> +M:     Emil Renner Berthing <kernel@esmil.dk>
>> +M:     Minda Chen <minda.chen@starfivetech.com>
>> +S:     Maintained
>> +F:     Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> +F:     drivers/usb/cdns3/cdns3-starfive.c
>> +
>>  STATIC BRANCH/CALL
>>  M:     Peter Zijlstra <peterz@infradead.org>
>>  M:     Josh Poimboeuf <jpoimboe@kernel.org>
>> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
>> index b98ca0a1352a..0a514b591527 100644
>> --- a/drivers/usb/cdns3/Kconfig
>> +++ b/drivers/usb/cdns3/Kconfig
>> @@ -78,6 +78,17 @@ config USB_CDNS3_IMX
>>
>>           For example, imx8qm and imx8qxp.
>>
>> +config USB_CDNS3_STARFIVE
>> +       tristate "Cadence USB3 support on StarFive SoC platforms"
>> +       depends on ARCH_STARFIVE || COMPILE_TEST
>> +       help
>> +         Say 'Y' or 'M' here if you are building for StarFive SoCs
>> +         platforms that contain Cadence USB3 controller core.
>> +
>> +         e.g. JH7110.
>> +
>> +         If you choose to build this driver as module it will
>> +         be dynamically linked and module will be called cdns3-starfive.ko
>>  endif
>>
>>  if USB_CDNS_SUPPORT
>> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>> index 61edb2f89276..48dfae75b5aa 100644
>> --- a/drivers/usb/cdns3/Makefile
>> +++ b/drivers/usb/cdns3/Makefile
>> @@ -24,6 +24,7 @@ endif
>>  obj-$(CONFIG_USB_CDNS3_PCI_WRAP)               += cdns3-pci-wrap.o
>>  obj-$(CONFIG_USB_CDNS3_TI)                     += cdns3-ti.o
>>  obj-$(CONFIG_USB_CDNS3_IMX)                    += cdns3-imx.o
>> +obj-$(CONFIG_USB_CDNS3_STARFIVE)               += cdns3-starfive.o
>>
>>  cdnsp-udc-pci-y                                        := cdnsp-pci.o
>>
>> diff --git a/drivers/usb/cdns3/cdns3-starfive.c b/drivers/usb/cdns3/cdns3-starfive.c
>> new file mode 100644
>> index 000000000000..a99f98f85235
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/cdns3-starfive.c
>> @@ -0,0 +1,305 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/**
>> + * cdns3-starfive.c - StarFive specific Glue layer for Cadence USB Controller
>> + *
>> + * Copyright (C) 2022 Starfive, Inc.
>> + * Author:     Yanhong Wang <yanhong.wang@starfivetech.com>
>> + * Author:     Mason Huo <mason.huo@starfivetech.com>
>> + * Author:     Minda Chen <minda.chen@starfivetech.com>
>> + */
>> +
>> +#include <linux/bits.h>
>> +#include <linux/clk.h>
>> +#include <linux/module.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/io.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +#include <linux/usb/otg.h>
>> +#include "core.h"
>> +
>> +#define USB_STRAP_HOST                 BIT(17)
>> +#define USB_STRAP_DEVICE               BIT(18)
>> +#define USB_STRAP_MASK                 GENMASK(18, 16)
>> +
>> +#define USB_SUSPENDM_HOST              BIT(19)
>> +#define USB_SUSPENDM_MASK              BIT(19)
>> +
>> +#define USB_SUSPENDM_BYPS              BIT(20)
>> +#define USB_REFCLK_MODE                        BIT(23)
>> +#define USB_PLL_EN                     BIT(22)
>> +#define USB_PDRSTN_SPLIT               BIT(17)
>> +
>> +#define PCIE_CKREF_SRC_MASK            GENMASK(19, 18)
>> +#define PCIE_CLK_SEL_MASK              GENMASK(21, 20)
>> +#define PCIE_PHY_MODE                  BIT(20)
>> +#define PCIE_PHY_MODE_MASK             GENMASK(21, 20)
>> +#define PCIE_USB3_BUS_WIDTH_MASK       GENMASK(3, 2)
>> +#define PCIE_USB3_RATE_MASK            GENMASK(6, 5)
>> +#define PCIE_USB3_RX_STANDBY_MASK      BIT(7)
>> +#define PCIE_USB3_PHY_ENABLE           BIT(4)
>> +
>> +struct cdns_starfive {
>> +       struct device *dev;
>> +       struct regmap *stg_syscon;
>> +       struct regmap *sys_syscon;
>> +       struct reset_control *resets;
>> +       struct clk_bulk_data *clks;
>> +       int num_clks;
>> +       u32 sys_offset;
>> +       u32 stg_offset_4;
>> +       u32 stg_offset_196;
>> +       u32 stg_offset_328;
>> +       u32 stg_offset_500;
>> +       bool usb2_only;
>> +};
>> +
>> +static int cdns_mode_init(struct platform_device *pdev,
>> +                               struct cdns_starfive *data, const char **out_mode)
>> +{
>> +       struct device_node *child;
>> +       const char *dr_mode = NULL;
>> +
>> +       child = of_get_compatible_child(pdev->dev.of_node, "cdns,usb3");
>> +       if (!child) {
>> +               return dev_err_probe(&pdev->dev, -ENODEV,
>> +                       "Failed to find child node\n");
>> +       }
>> +
>> +       /* Init usb 2.0 utmi phy */
>> +       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +               USB_SUSPENDM_BYPS, USB_SUSPENDM_BYPS);
>> +       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +               USB_PLL_EN, USB_PLL_EN);
>> +       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +               USB_REFCLK_MODE, USB_REFCLK_MODE);
>> +
>> +       if (!of_find_property(child, "cdns3,usb3-phy", NULL)) {
>> +               /* Disconnect usb 3.0 phy mode */
>> +               regmap_update_bits(data->sys_syscon, data->sys_offset,
>> +                       USB_PDRSTN_SPLIT, USB_PDRSTN_SPLIT);
>> +               data->usb2_only = 1;
>> +       } else {
>> +               /* Config usb 3.0 pipe phy */
>> +               regmap_update_bits(data->stg_syscon, data->stg_offset_196,
>> +                       PCIE_CKREF_SRC_MASK, 0);
>> +               regmap_update_bits(data->stg_syscon, data->stg_offset_196,
>> +                       PCIE_CLK_SEL_MASK, 0);
>> +               regmap_update_bits(data->stg_syscon, data->stg_offset_328,
>> +                       PCIE_PHY_MODE_MASK, PCIE_PHY_MODE);
>> +               regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +                       PCIE_USB3_BUS_WIDTH_MASK, 0);
>> +               regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +                       PCIE_USB3_RATE_MASK, 0);
>> +               regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +                       PCIE_USB3_RX_STANDBY_MASK, 0);
>> +               regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +                       PCIE_USB3_PHY_ENABLE, PCIE_USB3_PHY_ENABLE);
>> +
>> +               /* Connect usb 3.0 phy mode */
>> +               regmap_update_bits(data->sys_syscon, data->sys_offset,
>> +                       USB_PDRSTN_SPLIT, 0);
>> +       }
>> +
>> +       if (!of_property_read_string(child, "dr_mode", &dr_mode)) {
>> +               if (!strcmp(dr_mode, "host")) {
>> +                       regmap_update_bits(data->stg_syscon,
>> +                               data->stg_offset_4,
>> +                               USB_STRAP_MASK,
>> +                               USB_STRAP_HOST);
>> +                       regmap_update_bits(data->stg_syscon,
>> +                               data->stg_offset_4,
>> +                               USB_SUSPENDM_MASK,
>> +                               USB_SUSPENDM_HOST);
>> +               } else if (!strcmp(dr_mode, "peripheral")) {
>> +                       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +                               USB_STRAP_MASK, USB_STRAP_DEVICE);
>> +                       regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +                               USB_SUSPENDM_MASK, 0);
>> +               }
>> +       }
>> +
>> +       if (out_mode)
>> +               *out_mode = dr_mode;
>> +
>> +       return 0;
>> +}
>> +
>> +static int cdns_clk_rst_init(struct cdns_starfive *data)
>> +{
>> +       int ret;
>> +
>> +       data->num_clks = devm_clk_bulk_get_all(data->dev, &data->clks);
>> +       if (data->num_clks < 0)
>> +               return dev_err_probe(data->dev, -ENODEV,
>> +                       "Failed to get clocks\n");
>> +
>> +       ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
>> +       if (ret)
>> +               return dev_err_probe(data->dev, ret,
>> +                       "failed to enable clocks\n");
>> +
>> +       data->resets = devm_reset_control_array_get_exclusive(data->dev);
>> +       if (IS_ERR(data->resets)) {
>> +               ret = dev_err_probe(data->dev, PTR_ERR(data->resets),
>> +                       "Failed to get resets");
>> +               goto err_clk_init;
>> +       }
>> +
>> +       ret = reset_control_deassert(data->resets);
>> +       if (ret) {
>> +               ret = dev_err_probe(data->dev, ret,
>> +                       "failed to reset clocks\n");
>> +               goto err_clk_init;
>> +       }
>> +
>> +       return ret;
>> +
>> +err_clk_init:
>> +       clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +       return ret;
>> +}
>> +
>> +static int cdns_starfive_probe(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct device_node *node = pdev->dev.of_node;
>> +       struct cdns_starfive *data;
>> +       unsigned int args[4];
>> +       const char *dr_mode;
>> +       int ret;
>> +
>> +       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> +       if (!data)
>> +               return -ENOMEM;
>> +
>> +       platform_set_drvdata(pdev, data);
>> +
>> +       data->dev = dev;
>> +
>> +       data->stg_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
>> +               "starfive,stg-syscon", 4, args);
>> +
>> +       if (IS_ERR(data->stg_syscon))
>> +               return dev_err_probe(dev, PTR_ERR(data->stg_syscon),
>> +                       "Failed to parse starfive,stg-syscon\n");
>> +
>> +       data->stg_offset_4 = args[0];
>> +       data->stg_offset_196 = args[1];
>> +       data->stg_offset_328 = args[2];
>> +       data->stg_offset_500 = args[3];
>> +
>> +       data->sys_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
>> +               "starfive,sys-syscon", 1, args);
>> +       if (IS_ERR(data->sys_syscon))
>> +               return dev_err_probe(dev, PTR_ERR(data->sys_syscon),
>> +                       "Failed to parse starfive,sys-syscon\n");
>> +
>> +       data->sys_offset = args[0];
>> +
>> +       ret = cdns_mode_init(pdev, data, &dr_mode);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = cdns_clk_rst_init(data);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = of_platform_populate(node, NULL, NULL, dev);
>> +       if (ret)
>> +               return dev_err_probe(dev, ret, "Failed to create children\n");
> 
> For this error handling, you need to add some paired undo operations
> for cdns_clk_rst_init, i.e., reset_control_assert and
> clk_bulk_disable_unprepare.
> 
ok, I will change. Thanks
>> +
>> +       device_set_wakeup_capable(dev, true);
>> +       pm_runtime_set_active(dev);
>> +       pm_runtime_enable(dev);
>> +
>> +       dev_info(dev, "usb mode %s %s probe success\n",
>> +               dr_mode ? dr_mode : "unknown", data->usb2_only ? "2.0" : "3.0");
>> +
>> +       return 0;
>> +}
>> +
>> +static int cdns_starfive_remove_core(struct device *dev, void *c)
>> +{
>> +       struct platform_device *pdev = to_platform_device(dev);
>> +
>> +       platform_device_unregister(pdev);
>> +
>> +       return 0;
>> +}
>> +
>> +static int cdns_starfive_remove(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct cdns_starfive *data = dev_get_drvdata(dev);
>> +
>> +       pm_runtime_get_sync(dev);
>> +       device_for_each_child(dev, NULL, cdns_starfive_remove_core);
>> +
>> +       reset_control_assert(data->resets);
>> +       clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +       pm_runtime_disable(dev);
>> +       pm_runtime_put_noidle(dev);
>> +       platform_set_drvdata(pdev, NULL);
>> +
>> +       return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int cdns_starfive_resume(struct device *dev)
>> +{
>> +       struct cdns_starfive *data = dev_get_drvdata(dev);
>> +       int ret;
>> +
>> +       ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = reset_control_deassert(data->resets);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return 0;
>> +}
>> +
>> +static int cdns_starfive_suspend(struct device *dev)
>> +{
>> +       struct cdns_starfive *data = dev_get_drvdata(dev);
>> +
>> +       clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +       reset_control_assert(data->resets);
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops cdns_starfive_pm_ops = {
>> +       SET_RUNTIME_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume, NULL)
>> +       SET_SYSTEM_SLEEP_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume)
>> +};
>> +
>> +static const struct of_device_id cdns_starfive_of_match[] = {
>> +       { .compatible = "starfive,jh7110-usb", },
>> +       { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, cdns_starfive_of_match);
>> +
>> +static struct platform_driver cdns_starfive_driver = {
>> +       .probe          = cdns_starfive_probe,
>> +       .remove         = cdns_starfive_remove,
>> +       .driver         = {
>> +               .name   = "cdns3-starfive",
>> +               .of_match_table = cdns_starfive_of_match,
>> +               .pm     = &cdns_starfive_pm_ops,
>> +       },
>> +};
>> +module_platform_driver(cdns_starfive_driver);
>> +
>> +MODULE_ALIAS("platform:cdns3-starfive");
>> +MODULE_AUTHOR("YanHong Wang <yanhong.wang@starfivetech.com>");
>> +MODULE_AUTHOR("Mason Huo <mason.huo@starfivetech.com>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
>> --
>> 2.17.1
>>

Re: [PATCH v3 4/5] usb: cdns3: add StarFive JH7110 USB driver.

[Peter Chen]

On 23-03-15 18:44:10, Minda Chen wrote:
> There is a Cadence USB3 core for JH7110 SoCs, the cdns
> core is the child of this USB wrapper module device.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  MAINTAINERS                        |   7 +
>  drivers/usb/cdns3/Kconfig          |  11 ++
>  drivers/usb/cdns3/Makefile         |   1 +
>  drivers/usb/cdns3/cdns3-starfive.c | 305 +++++++++++++++++++++++++++++
>  4 files changed, 324 insertions(+)
>  create mode 100644 drivers/usb/cdns3/cdns3-starfive.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4263c005e45c..c530c966ab26 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19985,6 +19985,13 @@ F:	Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>  F:	drivers/phy/starfive/phy-jh7110-pcie.c
>  F:	drivers/phy/starfive/phy-jh7110-usb.c
>  
> +STARFIVE JH71X0 USB DRIVERS
> +M:	Emil Renner Berthing <kernel@esmil.dk>
> +M:	Minda Chen <minda.chen@starfivetech.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> +F:	drivers/usb/cdns3/cdns3-starfive.c
> +
>  STATIC BRANCH/CALL
>  M:	Peter Zijlstra <peterz@infradead.org>
>  M:	Josh Poimboeuf <jpoimboe@kernel.org>
> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
> index b98ca0a1352a..0a514b591527 100644
> --- a/drivers/usb/cdns3/Kconfig
> +++ b/drivers/usb/cdns3/Kconfig
> @@ -78,6 +78,17 @@ config USB_CDNS3_IMX
>  
>  	  For example, imx8qm and imx8qxp.
>  
> +config USB_CDNS3_STARFIVE
> +	tristate "Cadence USB3 support on StarFive SoC platforms"
> +	depends on ARCH_STARFIVE || COMPILE_TEST
> +	help
> +	  Say 'Y' or 'M' here if you are building for StarFive SoCs
> +	  platforms that contain Cadence USB3 controller core.
> +
> +	  e.g. JH7110.
> +
> +	  If you choose to build this driver as module it will
> +	  be dynamically linked and module will be called cdns3-starfive.ko
>  endif
>  
>  if USB_CDNS_SUPPORT
> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
> index 61edb2f89276..48dfae75b5aa 100644
> --- a/drivers/usb/cdns3/Makefile
> +++ b/drivers/usb/cdns3/Makefile
> @@ -24,6 +24,7 @@ endif
>  obj-$(CONFIG_USB_CDNS3_PCI_WRAP)		+= cdns3-pci-wrap.o
>  obj-$(CONFIG_USB_CDNS3_TI)			+= cdns3-ti.o
>  obj-$(CONFIG_USB_CDNS3_IMX)			+= cdns3-imx.o
> +obj-$(CONFIG_USB_CDNS3_STARFIVE)		+= cdns3-starfive.o
>  
>  cdnsp-udc-pci-y					:= cdnsp-pci.o
>  
> diff --git a/drivers/usb/cdns3/cdns3-starfive.c b/drivers/usb/cdns3/cdns3-starfive.c
> new file mode 100644
> index 000000000000..a99f98f85235
> --- /dev/null
> +++ b/drivers/usb/cdns3/cdns3-starfive.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * cdns3-starfive.c - StarFive specific Glue layer for Cadence USB Controller
> + *
> + * Copyright (C) 2022 Starfive, Inc.
> + * Author:	Yanhong Wang <yanhong.wang@starfivetech.com>
> + * Author:	Mason Huo <mason.huo@starfivetech.com>
> + * Author:	Minda Chen <minda.chen@starfivetech.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/usb/otg.h>
> +#include "core.h"
> +
> +#define USB_STRAP_HOST			BIT(17)
> +#define USB_STRAP_DEVICE		BIT(18)
> +#define USB_STRAP_MASK			GENMASK(18, 16)
> +
> +#define USB_SUSPENDM_HOST		BIT(19)
> +#define USB_SUSPENDM_MASK		BIT(19)
> +
> +#define USB_SUSPENDM_BYPS		BIT(20)
> +#define USB_REFCLK_MODE			BIT(23)
> +#define USB_PLL_EN			BIT(22)
> +#define USB_PDRSTN_SPLIT		BIT(17)
> +
> +#define PCIE_CKREF_SRC_MASK		GENMASK(19, 18)
> +#define PCIE_CLK_SEL_MASK		GENMASK(21, 20)
> +#define PCIE_PHY_MODE			BIT(20)
> +#define PCIE_PHY_MODE_MASK		GENMASK(21, 20)
> +#define PCIE_USB3_BUS_WIDTH_MASK	GENMASK(3, 2)
> +#define PCIE_USB3_RATE_MASK		GENMASK(6, 5)
> +#define PCIE_USB3_RX_STANDBY_MASK	BIT(7)
> +#define PCIE_USB3_PHY_ENABLE		BIT(4)
> +
> +struct cdns_starfive {
> +	struct device *dev;
> +	struct regmap *stg_syscon;
> +	struct regmap *sys_syscon;
> +	struct reset_control *resets;
> +	struct clk_bulk_data *clks;
> +	int num_clks;
> +	u32 sys_offset;
> +	u32 stg_offset_4;
> +	u32 stg_offset_196;
> +	u32 stg_offset_328;
> +	u32 stg_offset_500;
> +	bool usb2_only;
> +};
> +
> +static int cdns_mode_init(struct platform_device *pdev,
> +				struct cdns_starfive *data, const char **out_mode)
> +{
> +	struct device_node *child;
> +	const char *dr_mode = NULL;
> +
> +	child = of_get_compatible_child(pdev->dev.of_node, "cdns,usb3");
> +	if (!child) {
> +		return dev_err_probe(&pdev->dev, -ENODEV,
> +			"Failed to find child node\n");
> +	}
> +
> +	/* Init usb 2.0 utmi phy */
> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +		USB_SUSPENDM_BYPS, USB_SUSPENDM_BYPS);
> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +		USB_PLL_EN, USB_PLL_EN);
> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +		USB_REFCLK_MODE, USB_REFCLK_MODE);
> +
> +	if (!of_find_property(child, "cdns3,usb3-phy", NULL)) {
> +		/* Disconnect usb 3.0 phy mode */
> +		regmap_update_bits(data->sys_syscon, data->sys_offset,
> +			USB_PDRSTN_SPLIT, USB_PDRSTN_SPLIT);
> +		data->usb2_only = 1;
> +	} else {
> +		/* Config usb 3.0 pipe phy */
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
> +			PCIE_CKREF_SRC_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
> +			PCIE_CLK_SEL_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_328,
> +			PCIE_PHY_MODE_MASK, PCIE_PHY_MODE);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +			PCIE_USB3_BUS_WIDTH_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +			PCIE_USB3_RATE_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +			PCIE_USB3_RX_STANDBY_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +			PCIE_USB3_PHY_ENABLE, PCIE_USB3_PHY_ENABLE);
> +
> +		/* Connect usb 3.0 phy mode */
> +		regmap_update_bits(data->sys_syscon, data->sys_offset,
> +			USB_PDRSTN_SPLIT, 0);
> +	}
> +
> +	if (!of_property_read_string(child, "dr_mode", &dr_mode)) {
> +		if (!strcmp(dr_mode, "host")) {
> +			regmap_update_bits(data->stg_syscon,
> +				data->stg_offset_4,
> +				USB_STRAP_MASK,
> +				USB_STRAP_HOST);
> +			regmap_update_bits(data->stg_syscon,
> +				data->stg_offset_4,
> +				USB_SUSPENDM_MASK,
> +				USB_SUSPENDM_HOST);
> +		} else if (!strcmp(dr_mode, "peripheral")) {
> +			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +				USB_STRAP_MASK, USB_STRAP_DEVICE);
> +			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +				USB_SUSPENDM_MASK, 0);
> +		}
> +	}
> +
> +	if (out_mode)
> +		*out_mode = dr_mode;
> +
> +	return 0;
> +}
> +
> +static int cdns_clk_rst_init(struct cdns_starfive *data)
> +{
> +	int ret;
> +
> +	data->num_clks = devm_clk_bulk_get_all(data->dev, &data->clks);
> +	if (data->num_clks < 0)
> +		return dev_err_probe(data->dev, -ENODEV,
> +			"Failed to get clocks\n");
> +
> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +			"failed to enable clocks\n");
> +
> +	data->resets = devm_reset_control_array_get_exclusive(data->dev);
> +	if (IS_ERR(data->resets)) {
> +		ret = dev_err_probe(data->dev, PTR_ERR(data->resets),
> +			"Failed to get resets");
> +		goto err_clk_init;
> +	}
> +
> +	ret = reset_control_deassert(data->resets);
> +	if (ret) {
> +		ret = dev_err_probe(data->dev, ret,
> +			"failed to reset clocks\n");
> +		goto err_clk_init;
> +	}
> +
> +	return ret;
> +
> +err_clk_init:
> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +	return ret;
> +}
> +
> +static int cdns_starfive_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = pdev->dev.of_node;
> +	struct cdns_starfive *data;
> +	unsigned int args[4];
> +	const char *dr_mode;
> +	int ret;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	data->dev = dev;
> +
> +	data->stg_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
> +		"starfive,stg-syscon", 4, args);
> +
> +	if (IS_ERR(data->stg_syscon))
> +		return dev_err_probe(dev, PTR_ERR(data->stg_syscon),
> +			"Failed to parse starfive,stg-syscon\n");
> +
> +	data->stg_offset_4 = args[0];
> +	data->stg_offset_196 = args[1];
> +	data->stg_offset_328 = args[2];
> +	data->stg_offset_500 = args[3];
> +
> +	data->sys_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
> +		"starfive,sys-syscon", 1, args);
> +	if (IS_ERR(data->sys_syscon))
> +		return dev_err_probe(dev, PTR_ERR(data->sys_syscon),
> +			"Failed to parse starfive,sys-syscon\n");
> +
> +	data->sys_offset = args[0];
> +
> +	ret = cdns_mode_init(pdev, data, &dr_mode);
> +	if (ret)
> +		return ret;
> +
> +	ret = cdns_clk_rst_init(data);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_platform_populate(node, NULL, NULL, dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to create children\n");
> +
> +	device_set_wakeup_capable(dev, true);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +
> +	dev_info(dev, "usb mode %s %s probe success\n",
> +		dr_mode ? dr_mode : "unknown", data->usb2_only ? "2.0" : "3.0");
> +
> +	return 0;
> +}
> +
> +static int cdns_starfive_remove_core(struct device *dev, void *c)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	platform_device_unregister(pdev);
> +
> +	return 0;
> +}
> +
> +static int cdns_starfive_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cdns_starfive *data = dev_get_drvdata(dev);
> +
> +	pm_runtime_get_sync(dev);
> +	device_for_each_child(dev, NULL, cdns_starfive_remove_core);
> +
> +	reset_control_assert(data->resets);
> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +	pm_runtime_disable(dev);
> +	pm_runtime_put_noidle(dev);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int cdns_starfive_resume(struct device *dev)
> +{
> +	struct cdns_starfive *data = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
> +	if (ret)
> +		return ret;
> +
> +	ret = reset_control_deassert(data->resets);
> +	if (ret)
> +		return ret;
> +

So, you clk enable and reset has no sequence requirements? I see the
sequence is not the same between resume and suspend routine.

Peter
> +	return 0;
> +}
> +
> +static int cdns_starfive_suspend(struct device *dev)
> +{
> +	struct cdns_starfive *data = dev_get_drvdata(dev);
> +
> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +	reset_control_assert(data->resets);
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops cdns_starfive_pm_ops = {
> +	SET_RUNTIME_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume)
> +};
> +
> +static const struct of_device_id cdns_starfive_of_match[] = {
> +	{ .compatible = "starfive,jh7110-usb", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, cdns_starfive_of_match);
> +
> +static struct platform_driver cdns_starfive_driver = {
> +	.probe		= cdns_starfive_probe,
> +	.remove		= cdns_starfive_remove,
> +	.driver		= {
> +		.name	= "cdns3-starfive",
> +		.of_match_table	= cdns_starfive_of_match,
> +		.pm	= &cdns_starfive_pm_ops,
> +	},
> +};
> +module_platform_driver(cdns_starfive_driver);
> +
> +MODULE_ALIAS("platform:cdns3-starfive");
> +MODULE_AUTHOR("YanHong Wang <yanhong.wang@starfivetech.com>");
> +MODULE_AUTHOR("Mason Huo <mason.huo@starfivetech.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen

Re: [PATCH v3 4/5] usb: cdns3: add StarFive JH7110 USB driver.

[Minda Chen]

On 2023/3/16 10:46, Peter Chen wrote:
> On 23-03-15 18:44:10, Minda Chen wrote:
>> There is a Cadence USB3 core for JH7110 SoCs, the cdns
>> core is the child of this USB wrapper module device.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  MAINTAINERS                        |   7 +
>>  drivers/usb/cdns3/Kconfig          |  11 ++
>>  drivers/usb/cdns3/Makefile         |   1 +
>>  drivers/usb/cdns3/cdns3-starfive.c | 305 +++++++++++++++++++++++++++++
>>  4 files changed, 324 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/cdns3-starfive.c
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 4263c005e45c..c530c966ab26 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19985,6 +19985,13 @@ F:	Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>>  F:	drivers/phy/starfive/phy-jh7110-pcie.c
>>  F:	drivers/phy/starfive/phy-jh7110-usb.c
>>  
>> +STARFIVE JH71X0 USB DRIVERS
>> +M:	Emil Renner Berthing <kernel@esmil.dk>
>> +M:	Minda Chen <minda.chen@starfivetech.com>
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> +F:	drivers/usb/cdns3/cdns3-starfive.c
>> +
>>  STATIC BRANCH/CALL
>>  M:	Peter Zijlstra <peterz@infradead.org>
>>  M:	Josh Poimboeuf <jpoimboe@kernel.org>
>> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
>> index b98ca0a1352a..0a514b591527 100644
>> --- a/drivers/usb/cdns3/Kconfig
>> +++ b/drivers/usb/cdns3/Kconfig
>> @@ -78,6 +78,17 @@ config USB_CDNS3_IMX
>>  
>>  	  For example, imx8qm and imx8qxp.
>>  
>> +config USB_CDNS3_STARFIVE
>> +	tristate "Cadence USB3 support on StarFive SoC platforms"
>> +	depends on ARCH_STARFIVE || COMPILE_TEST
>> +	help
>> +	  Say 'Y' or 'M' here if you are building for StarFive SoCs
>> +	  platforms that contain Cadence USB3 controller core.
>> +
>> +	  e.g. JH7110.
>> +
>> +	  If you choose to build this driver as module it will
>> +	  be dynamically linked and module will be called cdns3-starfive.ko
>>  endif
>>  
>>  if USB_CDNS_SUPPORT
>> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>> index 61edb2f89276..48dfae75b5aa 100644
>> --- a/drivers/usb/cdns3/Makefile
>> +++ b/drivers/usb/cdns3/Makefile
>> @@ -24,6 +24,7 @@ endif
>>  obj-$(CONFIG_USB_CDNS3_PCI_WRAP)		+= cdns3-pci-wrap.o
>>  obj-$(CONFIG_USB_CDNS3_TI)			+= cdns3-ti.o
>>  obj-$(CONFIG_USB_CDNS3_IMX)			+= cdns3-imx.o
>> +obj-$(CONFIG_USB_CDNS3_STARFIVE)		+= cdns3-starfive.o
>>  
>>  cdnsp-udc-pci-y					:= cdnsp-pci.o
>>  
>> diff --git a/drivers/usb/cdns3/cdns3-starfive.c b/drivers/usb/cdns3/cdns3-starfive.c
>> new file mode 100644
>> index 000000000000..a99f98f85235
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/cdns3-starfive.c
>> @@ -0,0 +1,305 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/**
>> + * cdns3-starfive.c - StarFive specific Glue layer for Cadence USB Controller
>> + *
>> + * Copyright (C) 2022 Starfive, Inc.
>> + * Author:	Yanhong Wang <yanhong.wang@starfivetech.com>
>> + * Author:	Mason Huo <mason.huo@starfivetech.com>
>> + * Author:	Minda Chen <minda.chen@starfivetech.com>
>> + */
>> +
>> +#include <linux/bits.h>
>> +#include <linux/clk.h>
>> +#include <linux/module.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/io.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +#include <linux/usb/otg.h>
>> +#include "core.h"
>> +
>> +#define USB_STRAP_HOST			BIT(17)
>> +#define USB_STRAP_DEVICE		BIT(18)
>> +#define USB_STRAP_MASK			GENMASK(18, 16)
>> +
>> +#define USB_SUSPENDM_HOST		BIT(19)
>> +#define USB_SUSPENDM_MASK		BIT(19)
>> +
>> +#define USB_SUSPENDM_BYPS		BIT(20)
>> +#define USB_REFCLK_MODE			BIT(23)
>> +#define USB_PLL_EN			BIT(22)
>> +#define USB_PDRSTN_SPLIT		BIT(17)
>> +
>> +#define PCIE_CKREF_SRC_MASK		GENMASK(19, 18)
>> +#define PCIE_CLK_SEL_MASK		GENMASK(21, 20)
>> +#define PCIE_PHY_MODE			BIT(20)
>> +#define PCIE_PHY_MODE_MASK		GENMASK(21, 20)
>> +#define PCIE_USB3_BUS_WIDTH_MASK	GENMASK(3, 2)
>> +#define PCIE_USB3_RATE_MASK		GENMASK(6, 5)
>> +#define PCIE_USB3_RX_STANDBY_MASK	BIT(7)
>> +#define PCIE_USB3_PHY_ENABLE		BIT(4)
>> +
>> +struct cdns_starfive {
>> +	struct device *dev;
>> +	struct regmap *stg_syscon;
>> +	struct regmap *sys_syscon;
>> +	struct reset_control *resets;
>> +	struct clk_bulk_data *clks;
>> +	int num_clks;
>> +	u32 sys_offset;
>> +	u32 stg_offset_4;
>> +	u32 stg_offset_196;
>> +	u32 stg_offset_328;
>> +	u32 stg_offset_500;
>> +	bool usb2_only;
>> +};
>> +
>> +static int cdns_mode_init(struct platform_device *pdev,
>> +				struct cdns_starfive *data, const char **out_mode)
>> +{
>> +	struct device_node *child;
>> +	const char *dr_mode = NULL;
>> +
>> +	child = of_get_compatible_child(pdev->dev.of_node, "cdns,usb3");
>> +	if (!child) {
>> +		return dev_err_probe(&pdev->dev, -ENODEV,
>> +			"Failed to find child node\n");
>> +	}
>> +
>> +	/* Init usb 2.0 utmi phy */
>> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +		USB_SUSPENDM_BYPS, USB_SUSPENDM_BYPS);
>> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +		USB_PLL_EN, USB_PLL_EN);
>> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +		USB_REFCLK_MODE, USB_REFCLK_MODE);
>> +
>> +	if (!of_find_property(child, "cdns3,usb3-phy", NULL)) {
>> +		/* Disconnect usb 3.0 phy mode */
>> +		regmap_update_bits(data->sys_syscon, data->sys_offset,
>> +			USB_PDRSTN_SPLIT, USB_PDRSTN_SPLIT);
>> +		data->usb2_only = 1;
>> +	} else {
>> +		/* Config usb 3.0 pipe phy */
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
>> +			PCIE_CKREF_SRC_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
>> +			PCIE_CLK_SEL_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_328,
>> +			PCIE_PHY_MODE_MASK, PCIE_PHY_MODE);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +			PCIE_USB3_BUS_WIDTH_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +			PCIE_USB3_RATE_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +			PCIE_USB3_RX_STANDBY_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +			PCIE_USB3_PHY_ENABLE, PCIE_USB3_PHY_ENABLE);
>> +
>> +		/* Connect usb 3.0 phy mode */
>> +		regmap_update_bits(data->sys_syscon, data->sys_offset,
>> +			USB_PDRSTN_SPLIT, 0);
>> +	}
>> +
>> +	if (!of_property_read_string(child, "dr_mode", &dr_mode)) {
>> +		if (!strcmp(dr_mode, "host")) {
>> +			regmap_update_bits(data->stg_syscon,
>> +				data->stg_offset_4,
>> +				USB_STRAP_MASK,
>> +				USB_STRAP_HOST);
>> +			regmap_update_bits(data->stg_syscon,
>> +				data->stg_offset_4,
>> +				USB_SUSPENDM_MASK,
>> +				USB_SUSPENDM_HOST);
>> +		} else if (!strcmp(dr_mode, "peripheral")) {
>> +			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +				USB_STRAP_MASK, USB_STRAP_DEVICE);
>> +			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +				USB_SUSPENDM_MASK, 0);
>> +		}
>> +	}
>> +
>> +	if (out_mode)
>> +		*out_mode = dr_mode;
>> +
>> +	return 0;
>> +}
>> +
>> +static int cdns_clk_rst_init(struct cdns_starfive *data)
>> +{
>> +	int ret;
>> +
>> +	data->num_clks = devm_clk_bulk_get_all(data->dev, &data->clks);
>> +	if (data->num_clks < 0)
>> +		return dev_err_probe(data->dev, -ENODEV,
>> +			"Failed to get clocks\n");
>> +
>> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
>> +	if (ret)
>> +		return dev_err_probe(data->dev, ret,
>> +			"failed to enable clocks\n");
>> +
>> +	data->resets = devm_reset_control_array_get_exclusive(data->dev);
>> +	if (IS_ERR(data->resets)) {
>> +		ret = dev_err_probe(data->dev, PTR_ERR(data->resets),
>> +			"Failed to get resets");
>> +		goto err_clk_init;
>> +	}
>> +
>> +	ret = reset_control_deassert(data->resets);
>> +	if (ret) {
>> +		ret = dev_err_probe(data->dev, ret,
>> +			"failed to reset clocks\n");
>> +		goto err_clk_init;
>> +	}
>> +
>> +	return ret;
>> +
>> +err_clk_init:
>> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +	return ret;
>> +}
>> +
>> +static int cdns_starfive_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *node = pdev->dev.of_node;
>> +	struct cdns_starfive *data;
>> +	unsigned int args[4];
>> +	const char *dr_mode;
>> +	int ret;
>> +
>> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	platform_set_drvdata(pdev, data);
>> +
>> +	data->dev = dev;
>> +
>> +	data->stg_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
>> +		"starfive,stg-syscon", 4, args);
>> +
>> +	if (IS_ERR(data->stg_syscon))
>> +		return dev_err_probe(dev, PTR_ERR(data->stg_syscon),
>> +			"Failed to parse starfive,stg-syscon\n");
>> +
>> +	data->stg_offset_4 = args[0];
>> +	data->stg_offset_196 = args[1];
>> +	data->stg_offset_328 = args[2];
>> +	data->stg_offset_500 = args[3];
>> +
>> +	data->sys_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
>> +		"starfive,sys-syscon", 1, args);
>> +	if (IS_ERR(data->sys_syscon))
>> +		return dev_err_probe(dev, PTR_ERR(data->sys_syscon),
>> +			"Failed to parse starfive,sys-syscon\n");
>> +
>> +	data->sys_offset = args[0];
>> +
>> +	ret = cdns_mode_init(pdev, data, &dr_mode);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = cdns_clk_rst_init(data);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = of_platform_populate(node, NULL, NULL, dev);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to create children\n");
>> +
>> +	device_set_wakeup_capable(dev, true);
>> +	pm_runtime_set_active(dev);
>> +	pm_runtime_enable(dev);
>> +
>> +	dev_info(dev, "usb mode %s %s probe success\n",
>> +		dr_mode ? dr_mode : "unknown", data->usb2_only ? "2.0" : "3.0");
>> +
>> +	return 0;
>> +}
>> +
>> +static int cdns_starfive_remove_core(struct device *dev, void *c)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +
>> +	platform_device_unregister(pdev);
>> +
>> +	return 0;
>> +}
>> +
>> +static int cdns_starfive_remove(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct cdns_starfive *data = dev_get_drvdata(dev);
>> +
>> +	pm_runtime_get_sync(dev);
>> +	device_for_each_child(dev, NULL, cdns_starfive_remove_core);
>> +
>> +	reset_control_assert(data->resets);
>> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +	pm_runtime_disable(dev);
>> +	pm_runtime_put_noidle(dev);
>> +	platform_set_drvdata(pdev, NULL);
>> +
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int cdns_starfive_resume(struct device *dev)
>> +{
>> +	struct cdns_starfive *data = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = reset_control_deassert(data->resets);
>> +	if (ret)
>> +		return ret;
>> +
> 
> So, you clk enable and reset has no sequence requirements? I see the
> sequence is not the same between resume and suspend routine.
> 
> Peter
Thanks, suspend seq should be inverted. I will change the suspend reset and enable sequence.
>> +	return 0;
>> +}
>> +
>> +static int cdns_starfive_suspend(struct device *dev)
>> +{
>> +	struct cdns_starfive *data = dev_get_drvdata(dev);
>> +
>> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +	reset_control_assert(data->resets);
I will invert this sequence. But these still can work.
>> +
>> +	return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops cdns_starfive_pm_ops = {
>> +	SET_RUNTIME_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume, NULL)
>> +	SET_SYSTEM_SLEEP_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume)
>> +};
>> +
>> +static const struct of_device_id cdns_starfive_of_match[] = {
>> +	{ .compatible = "starfive,jh7110-usb", },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, cdns_starfive_of_match);
>> +
>> +static struct platform_driver cdns_starfive_driver = {
>> +	.probe		= cdns_starfive_probe,
>> +	.remove		= cdns_starfive_remove,
>> +	.driver		= {
>> +		.name	= "cdns3-starfive",
>> +		.of_match_table	= cdns_starfive_of_match,
>> +		.pm	= &cdns_starfive_pm_ops,
>> +	},
>> +};
>> +module_platform_driver(cdns_starfive_driver);
>> +
>> +MODULE_ALIAS("platform:cdns3-starfive");
>> +MODULE_AUTHOR("YanHong Wang <yanhong.wang@starfivetech.com>");
>> +MODULE_AUTHOR("Mason Huo <mason.huo@starfivetech.com>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
>> -- 
>> 2.17.1
>> 
> 

Re: [PATCH v3 4/5] usb: cdns3: add StarFive JH7110 USB driver.

[Rob Herring]

On Wed, Mar 15, 2023 at 06:44:10PM +0800, Minda Chen wrote:
> There is a Cadence USB3 core for JH7110 SoCs, the cdns
> core is the child of this USB wrapper module device.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  MAINTAINERS                        |   7 +
>  drivers/usb/cdns3/Kconfig          |  11 ++
>  drivers/usb/cdns3/Makefile         |   1 +
>  drivers/usb/cdns3/cdns3-starfive.c | 305 +++++++++++++++++++++++++++++
>  4 files changed, 324 insertions(+)
>  create mode 100644 drivers/usb/cdns3/cdns3-starfive.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4263c005e45c..c530c966ab26 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19985,6 +19985,13 @@ F:	Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>  F:	drivers/phy/starfive/phy-jh7110-pcie.c
>  F:	drivers/phy/starfive/phy-jh7110-usb.c
>  
> +STARFIVE JH71X0 USB DRIVERS
> +M:	Emil Renner Berthing <kernel@esmil.dk>
> +M:	Minda Chen <minda.chen@starfivetech.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> +F:	drivers/usb/cdns3/cdns3-starfive.c
> +
>  STATIC BRANCH/CALL
>  M:	Peter Zijlstra <peterz@infradead.org>
>  M:	Josh Poimboeuf <jpoimboe@kernel.org>
> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
> index b98ca0a1352a..0a514b591527 100644
> --- a/drivers/usb/cdns3/Kconfig
> +++ b/drivers/usb/cdns3/Kconfig
> @@ -78,6 +78,17 @@ config USB_CDNS3_IMX
>  
>  	  For example, imx8qm and imx8qxp.
>  
> +config USB_CDNS3_STARFIVE
> +	tristate "Cadence USB3 support on StarFive SoC platforms"
> +	depends on ARCH_STARFIVE || COMPILE_TEST
> +	help
> +	  Say 'Y' or 'M' here if you are building for StarFive SoCs
> +	  platforms that contain Cadence USB3 controller core.
> +
> +	  e.g. JH7110.
> +
> +	  If you choose to build this driver as module it will
> +	  be dynamically linked and module will be called cdns3-starfive.ko
>  endif
>  
>  if USB_CDNS_SUPPORT
> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
> index 61edb2f89276..48dfae75b5aa 100644
> --- a/drivers/usb/cdns3/Makefile
> +++ b/drivers/usb/cdns3/Makefile
> @@ -24,6 +24,7 @@ endif
>  obj-$(CONFIG_USB_CDNS3_PCI_WRAP)		+= cdns3-pci-wrap.o
>  obj-$(CONFIG_USB_CDNS3_TI)			+= cdns3-ti.o
>  obj-$(CONFIG_USB_CDNS3_IMX)			+= cdns3-imx.o
> +obj-$(CONFIG_USB_CDNS3_STARFIVE)		+= cdns3-starfive.o
>  
>  cdnsp-udc-pci-y					:= cdnsp-pci.o
>  
> diff --git a/drivers/usb/cdns3/cdns3-starfive.c b/drivers/usb/cdns3/cdns3-starfive.c
> new file mode 100644
> index 000000000000..a99f98f85235
> --- /dev/null
> +++ b/drivers/usb/cdns3/cdns3-starfive.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * cdns3-starfive.c - StarFive specific Glue layer for Cadence USB Controller
> + *
> + * Copyright (C) 2022 Starfive, Inc.
> + * Author:	Yanhong Wang <yanhong.wang@starfivetech.com>
> + * Author:	Mason Huo <mason.huo@starfivetech.com>
> + * Author:	Minda Chen <minda.chen@starfivetech.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/usb/otg.h>
> +#include "core.h"
> +
> +#define USB_STRAP_HOST			BIT(17)
> +#define USB_STRAP_DEVICE		BIT(18)
> +#define USB_STRAP_MASK			GENMASK(18, 16)
> +
> +#define USB_SUSPENDM_HOST		BIT(19)
> +#define USB_SUSPENDM_MASK		BIT(19)
> +
> +#define USB_SUSPENDM_BYPS		BIT(20)
> +#define USB_REFCLK_MODE			BIT(23)
> +#define USB_PLL_EN			BIT(22)
> +#define USB_PDRSTN_SPLIT		BIT(17)
> +
> +#define PCIE_CKREF_SRC_MASK		GENMASK(19, 18)
> +#define PCIE_CLK_SEL_MASK		GENMASK(21, 20)
> +#define PCIE_PHY_MODE			BIT(20)
> +#define PCIE_PHY_MODE_MASK		GENMASK(21, 20)
> +#define PCIE_USB3_BUS_WIDTH_MASK	GENMASK(3, 2)
> +#define PCIE_USB3_RATE_MASK		GENMASK(6, 5)
> +#define PCIE_USB3_RX_STANDBY_MASK	BIT(7)
> +#define PCIE_USB3_PHY_ENABLE		BIT(4)
> +
> +struct cdns_starfive {
> +	struct device *dev;
> +	struct regmap *stg_syscon;
> +	struct regmap *sys_syscon;
> +	struct reset_control *resets;
> +	struct clk_bulk_data *clks;
> +	int num_clks;
> +	u32 sys_offset;
> +	u32 stg_offset_4;
> +	u32 stg_offset_196;
> +	u32 stg_offset_328;
> +	u32 stg_offset_500;
> +	bool usb2_only;
> +};
> +
> +static int cdns_mode_init(struct platform_device *pdev,
> +				struct cdns_starfive *data, const char **out_mode)
> +{
> +	struct device_node *child;
> +	const char *dr_mode = NULL;
> +
> +	child = of_get_compatible_child(pdev->dev.of_node, "cdns,usb3");
> +	if (!child) {
> +		return dev_err_probe(&pdev->dev, -ENODEV,
> +			"Failed to find child node\n");
> +	}
> +
> +	/* Init usb 2.0 utmi phy */
> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +		USB_SUSPENDM_BYPS, USB_SUSPENDM_BYPS);
> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +		USB_PLL_EN, USB_PLL_EN);
> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +		USB_REFCLK_MODE, USB_REFCLK_MODE);
> +
> +	if (!of_find_property(child, "cdns3,usb3-phy", NULL)) {

No such property defined in the binding. And there shouldn't be...

> +		/* Disconnect usb 3.0 phy mode */
> +		regmap_update_bits(data->sys_syscon, data->sys_offset,
> +			USB_PDRSTN_SPLIT, USB_PDRSTN_SPLIT);
> +		data->usb2_only = 1;
> +	} else {
> +		/* Config usb 3.0 pipe phy */
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
> +			PCIE_CKREF_SRC_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
> +			PCIE_CLK_SEL_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_328,
> +			PCIE_PHY_MODE_MASK, PCIE_PHY_MODE);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +			PCIE_USB3_BUS_WIDTH_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +			PCIE_USB3_RATE_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +			PCIE_USB3_RX_STANDBY_MASK, 0);
> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
> +			PCIE_USB3_PHY_ENABLE, PCIE_USB3_PHY_ENABLE);
> +
> +		/* Connect usb 3.0 phy mode */
> +		regmap_update_bits(data->sys_syscon, data->sys_offset,
> +			USB_PDRSTN_SPLIT, 0);
> +	}

This all looks like PHY configuration. It should be in the PHY driver 
instead. If there's some USB controller specific config for the PHY, 
then look into having phy cells for that. Multi-mode PHYs sometimes put 
the PHY mode into phy cells for example.

> +
> +	if (!of_property_read_string(child, "dr_mode", &dr_mode)) {
> +		if (!strcmp(dr_mode, "host")) {
> +			regmap_update_bits(data->stg_syscon,
> +				data->stg_offset_4,
> +				USB_STRAP_MASK,
> +				USB_STRAP_HOST);
> +			regmap_update_bits(data->stg_syscon,
> +				data->stg_offset_4,
> +				USB_SUSPENDM_MASK,
> +				USB_SUSPENDM_HOST);
> +		} else if (!strcmp(dr_mode, "peripheral")) {
> +			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +				USB_STRAP_MASK, USB_STRAP_DEVICE);
> +			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
> +				USB_SUSPENDM_MASK, 0);
> +		}
> +	}
> +
> +	if (out_mode)
> +		*out_mode = dr_mode;
> +
> +	return 0;
> +}
> +
> +static int cdns_clk_rst_init(struct cdns_starfive *data)
> +{
> +	int ret;
> +
> +	data->num_clks = devm_clk_bulk_get_all(data->dev, &data->clks);
> +	if (data->num_clks < 0)
> +		return dev_err_probe(data->dev, -ENODEV,
> +			"Failed to get clocks\n");
> +
> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +			"failed to enable clocks\n");
> +
> +	data->resets = devm_reset_control_array_get_exclusive(data->dev);
> +	if (IS_ERR(data->resets)) {
> +		ret = dev_err_probe(data->dev, PTR_ERR(data->resets),
> +			"Failed to get resets");
> +		goto err_clk_init;
> +	}
> +
> +	ret = reset_control_deassert(data->resets);
> +	if (ret) {
> +		ret = dev_err_probe(data->dev, ret,
> +			"failed to reset clocks\n");
> +		goto err_clk_init;
> +	}
> +
> +	return ret;
> +
> +err_clk_init:
> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +	return ret;
> +}
> +
> +static int cdns_starfive_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = pdev->dev.of_node;
> +	struct cdns_starfive *data;
> +	unsigned int args[4];
> +	const char *dr_mode;
> +	int ret;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	data->dev = dev;
> +
> +	data->stg_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
> +		"starfive,stg-syscon", 4, args);
> +
> +	if (IS_ERR(data->stg_syscon))
> +		return dev_err_probe(dev, PTR_ERR(data->stg_syscon),
> +			"Failed to parse starfive,stg-syscon\n");
> +
> +	data->stg_offset_4 = args[0];
> +	data->stg_offset_196 = args[1];
> +	data->stg_offset_328 = args[2];
> +	data->stg_offset_500 = args[3];
> +
> +	data->sys_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
> +		"starfive,sys-syscon", 1, args);
> +	if (IS_ERR(data->sys_syscon))
> +		return dev_err_probe(dev, PTR_ERR(data->sys_syscon),
> +			"Failed to parse starfive,sys-syscon\n");
> +
> +	data->sys_offset = args[0];
> +
> +	ret = cdns_mode_init(pdev, data, &dr_mode);
> +	if (ret)
> +		return ret;
> +
> +	ret = cdns_clk_rst_init(data);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_platform_populate(node, NULL, NULL, dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to create children\n");
> +
> +	device_set_wakeup_capable(dev, true);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +
> +	dev_info(dev, "usb mode %s %s probe success\n",
> +		dr_mode ? dr_mode : "unknown", data->usb2_only ? "2.0" : "3.0");
> +
> +	return 0;
> +}
> +
> +static int cdns_starfive_remove_core(struct device *dev, void *c)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +
> +	platform_device_unregister(pdev);
> +
> +	return 0;
> +}
> +
> +static int cdns_starfive_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cdns_starfive *data = dev_get_drvdata(dev);
> +
> +	pm_runtime_get_sync(dev);
> +	device_for_each_child(dev, NULL, cdns_starfive_remove_core);
> +
> +	reset_control_assert(data->resets);
> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +	pm_runtime_disable(dev);
> +	pm_runtime_put_noidle(dev);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int cdns_starfive_resume(struct device *dev)
> +{
> +	struct cdns_starfive *data = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
> +	if (ret)
> +		return ret;
> +
> +	ret = reset_control_deassert(data->resets);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int cdns_starfive_suspend(struct device *dev)
> +{
> +	struct cdns_starfive *data = dev_get_drvdata(dev);
> +
> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +	reset_control_assert(data->resets);
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops cdns_starfive_pm_ops = {
> +	SET_RUNTIME_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume)
> +};
> +
> +static const struct of_device_id cdns_starfive_of_match[] = {
> +	{ .compatible = "starfive,jh7110-usb", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, cdns_starfive_of_match);
> +
> +static struct platform_driver cdns_starfive_driver = {
> +	.probe		= cdns_starfive_probe,
> +	.remove		= cdns_starfive_remove,
> +	.driver		= {
> +		.name	= "cdns3-starfive",
> +		.of_match_table	= cdns_starfive_of_match,
> +		.pm	= &cdns_starfive_pm_ops,
> +	},
> +};
> +module_platform_driver(cdns_starfive_driver);
> +
> +MODULE_ALIAS("platform:cdns3-starfive");
> +MODULE_AUTHOR("YanHong Wang <yanhong.wang@starfivetech.com>");
> +MODULE_AUTHOR("Mason Huo <mason.huo@starfivetech.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
> -- 
> 2.17.1
> 

Re: [PATCH v3 4/5] usb: cdns3: add StarFive JH7110 USB driver.

[Minda Chen]

On 2023/3/20 23:26, Rob Herring wrote:
> On Wed, Mar 15, 2023 at 06:44:10PM +0800, Minda Chen wrote:
>> There is a Cadence USB3 core for JH7110 SoCs, the cdns
>> core is the child of this USB wrapper module device.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  MAINTAINERS                        |   7 +
>>  drivers/usb/cdns3/Kconfig          |  11 ++
>>  drivers/usb/cdns3/Makefile         |   1 +
>>  drivers/usb/cdns3/cdns3-starfive.c | 305 +++++++++++++++++++++++++++++
>>  4 files changed, 324 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/cdns3-starfive.c
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 4263c005e45c..c530c966ab26 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19985,6 +19985,13 @@ F:	Documentation/devicetree/bindings/phy/starfive,jh7110-usb-pcie-phy.yaml
>>  F:	drivers/phy/starfive/phy-jh7110-pcie.c
>>  F:	drivers/phy/starfive/phy-jh7110-usb.c
>>  
>> +STARFIVE JH71X0 USB DRIVERS
>> +M:	Emil Renner Berthing <kernel@esmil.dk>
>> +M:	Minda Chen <minda.chen@starfivetech.com>
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> +F:	drivers/usb/cdns3/cdns3-starfive.c
>> +
>>  STATIC BRANCH/CALL
>>  M:	Peter Zijlstra <peterz@infradead.org>
>>  M:	Josh Poimboeuf <jpoimboe@kernel.org>
>> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
>> index b98ca0a1352a..0a514b591527 100644
>> --- a/drivers/usb/cdns3/Kconfig
>> +++ b/drivers/usb/cdns3/Kconfig
>> @@ -78,6 +78,17 @@ config USB_CDNS3_IMX
>>  
>>  	  For example, imx8qm and imx8qxp.
>>  
>> +config USB_CDNS3_STARFIVE
>> +	tristate "Cadence USB3 support on StarFive SoC platforms"
>> +	depends on ARCH_STARFIVE || COMPILE_TEST
>> +	help
>> +	  Say 'Y' or 'M' here if you are building for StarFive SoCs
>> +	  platforms that contain Cadence USB3 controller core.
>> +
>> +	  e.g. JH7110.
>> +
>> +	  If you choose to build this driver as module it will
>> +	  be dynamically linked and module will be called cdns3-starfive.ko
>>  endif
>>  
>>  if USB_CDNS_SUPPORT
>> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>> index 61edb2f89276..48dfae75b5aa 100644
>> --- a/drivers/usb/cdns3/Makefile
>> +++ b/drivers/usb/cdns3/Makefile
>> @@ -24,6 +24,7 @@ endif
>>  obj-$(CONFIG_USB_CDNS3_PCI_WRAP)		+= cdns3-pci-wrap.o
>>  obj-$(CONFIG_USB_CDNS3_TI)			+= cdns3-ti.o
>>  obj-$(CONFIG_USB_CDNS3_IMX)			+= cdns3-imx.o
>> +obj-$(CONFIG_USB_CDNS3_STARFIVE)		+= cdns3-starfive.o
>>  
>>  cdnsp-udc-pci-y					:= cdnsp-pci.o
>>  
>> diff --git a/drivers/usb/cdns3/cdns3-starfive.c b/drivers/usb/cdns3/cdns3-starfive.c
>> new file mode 100644
>> index 000000000000..a99f98f85235
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/cdns3-starfive.c
>> @@ -0,0 +1,305 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/**
>> + * cdns3-starfive.c - StarFive specific Glue layer for Cadence USB Controller
>> + *
>> + * Copyright (C) 2022 Starfive, Inc.
>> + * Author:	Yanhong Wang <yanhong.wang@starfivetech.com>
>> + * Author:	Mason Huo <mason.huo@starfivetech.com>
>> + * Author:	Minda Chen <minda.chen@starfivetech.com>
>> + */
>> +
>> +#include <linux/bits.h>
>> +#include <linux/clk.h>
>> +#include <linux/module.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/io.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +#include <linux/usb/otg.h>
>> +#include "core.h"
>> +
>> +#define USB_STRAP_HOST			BIT(17)
>> +#define USB_STRAP_DEVICE		BIT(18)
>> +#define USB_STRAP_MASK			GENMASK(18, 16)
>> +
>> +#define USB_SUSPENDM_HOST		BIT(19)
>> +#define USB_SUSPENDM_MASK		BIT(19)
>> +
>> +#define USB_SUSPENDM_BYPS		BIT(20)
>> +#define USB_REFCLK_MODE			BIT(23)
>> +#define USB_PLL_EN			BIT(22)
>> +#define USB_PDRSTN_SPLIT		BIT(17)
>> +
>> +#define PCIE_CKREF_SRC_MASK		GENMASK(19, 18)
>> +#define PCIE_CLK_SEL_MASK		GENMASK(21, 20)
>> +#define PCIE_PHY_MODE			BIT(20)
>> +#define PCIE_PHY_MODE_MASK		GENMASK(21, 20)
>> +#define PCIE_USB3_BUS_WIDTH_MASK	GENMASK(3, 2)
>> +#define PCIE_USB3_RATE_MASK		GENMASK(6, 5)
>> +#define PCIE_USB3_RX_STANDBY_MASK	BIT(7)
>> +#define PCIE_USB3_PHY_ENABLE		BIT(4)
>> +
>> +struct cdns_starfive {
>> +	struct device *dev;
>> +	struct regmap *stg_syscon;
>> +	struct regmap *sys_syscon;
>> +	struct reset_control *resets;
>> +	struct clk_bulk_data *clks;
>> +	int num_clks;
>> +	u32 sys_offset;
>> +	u32 stg_offset_4;
>> +	u32 stg_offset_196;
>> +	u32 stg_offset_328;
>> +	u32 stg_offset_500;
>> +	bool usb2_only;
>> +};
>> +
>> +static int cdns_mode_init(struct platform_device *pdev,
>> +				struct cdns_starfive *data, const char **out_mode)
>> +{
>> +	struct device_node *child;
>> +	const char *dr_mode = NULL;
>> +
>> +	child = of_get_compatible_child(pdev->dev.of_node, "cdns,usb3");
>> +	if (!child) {
>> +		return dev_err_probe(&pdev->dev, -ENODEV,
>> +			"Failed to find child node\n");
>> +	}
>> +
>> +	/* Init usb 2.0 utmi phy */
>> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +		USB_SUSPENDM_BYPS, USB_SUSPENDM_BYPS);
>> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +		USB_PLL_EN, USB_PLL_EN);
>> +	regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +		USB_REFCLK_MODE, USB_REFCLK_MODE);
>> +
>> +	if (!of_find_property(child, "cdns3,usb3-phy", NULL)) {
> 
> No such property defined in the binding. And there shouldn't be...
> 
>> +		/* Disconnect usb 3.0 phy mode */
>> +		regmap_update_bits(data->sys_syscon, data->sys_offset,
>> +			USB_PDRSTN_SPLIT, USB_PDRSTN_SPLIT);
>> +		data->usb2_only = 1;
>> +	} else {
>> +		/* Config usb 3.0 pipe phy */
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
>> +			PCIE_CKREF_SRC_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_196,
>> +			PCIE_CLK_SEL_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_328,
>> +			PCIE_PHY_MODE_MASK, PCIE_PHY_MODE);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +			PCIE_USB3_BUS_WIDTH_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +			PCIE_USB3_RATE_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +			PCIE_USB3_RX_STANDBY_MASK, 0);
>> +		regmap_update_bits(data->stg_syscon, data->stg_offset_500,
>> +			PCIE_USB3_PHY_ENABLE, PCIE_USB3_PHY_ENABLE);
>> +
>> +		/* Connect usb 3.0 phy mode */
>> +		regmap_update_bits(data->sys_syscon, data->sys_offset,
>> +			USB_PDRSTN_SPLIT, 0);
>> +	}
> 
> This all looks like PHY configuration. It should be in the PHY driver 
> instead. If there's some USB controller specific config for the PHY, 
> then look into having phy cells for that. Multi-mode PHYs sometimes put 
> the PHY mode into phy cells for example.
> 
I think so. But actually they are not PHY register setting. They are SOC system-control
registers set which shared by other device. OK, They can be moved to PHY driver. 
>> +
>> +	if (!of_property_read_string(child, "dr_mode", &dr_mode)) {
>> +		if (!strcmp(dr_mode, "host")) {
>> +			regmap_update_bits(data->stg_syscon,
>> +				data->stg_offset_4,
>> +				USB_STRAP_MASK,
>> +				USB_STRAP_HOST);
>> +			regmap_update_bits(data->stg_syscon,
>> +				data->stg_offset_4,
>> +				USB_SUSPENDM_MASK,
>> +				USB_SUSPENDM_HOST);
>> +		} else if (!strcmp(dr_mode, "peripheral")) {
>> +			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +				USB_STRAP_MASK, USB_STRAP_DEVICE);
>> +			regmap_update_bits(data->stg_syscon, data->stg_offset_4,
>> +				USB_SUSPENDM_MASK, 0);
>> +		}
>> +	}
>> +
>> +	if (out_mode)
>> +		*out_mode = dr_mode;
>> +
>> +	return 0;
>> +}
>> +
>> +static int cdns_clk_rst_init(struct cdns_starfive *data)
>> +{
>> +	int ret;
>> +
>> +	data->num_clks = devm_clk_bulk_get_all(data->dev, &data->clks);
>> +	if (data->num_clks < 0)
>> +		return dev_err_probe(data->dev, -ENODEV,
>> +			"Failed to get clocks\n");
>> +
>> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
>> +	if (ret)
>> +		return dev_err_probe(data->dev, ret,
>> +			"failed to enable clocks\n");
>> +
>> +	data->resets = devm_reset_control_array_get_exclusive(data->dev);
>> +	if (IS_ERR(data->resets)) {
>> +		ret = dev_err_probe(data->dev, PTR_ERR(data->resets),
>> +			"Failed to get resets");
>> +		goto err_clk_init;
>> +	}
>> +
>> +	ret = reset_control_deassert(data->resets);
>> +	if (ret) {
>> +		ret = dev_err_probe(data->dev, ret,
>> +			"failed to reset clocks\n");
>> +		goto err_clk_init;
>> +	}
>> +
>> +	return ret;
>> +
>> +err_clk_init:
>> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +	return ret;
>> +}
>> +
>> +static int cdns_starfive_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *node = pdev->dev.of_node;
>> +	struct cdns_starfive *data;
>> +	unsigned int args[4];
>> +	const char *dr_mode;
>> +	int ret;
>> +
>> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	platform_set_drvdata(pdev, data);
>> +
>> +	data->dev = dev;
>> +
>> +	data->stg_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
>> +		"starfive,stg-syscon", 4, args);
>> +
>> +	if (IS_ERR(data->stg_syscon))
>> +		return dev_err_probe(dev, PTR_ERR(data->stg_syscon),
>> +			"Failed to parse starfive,stg-syscon\n");
>> +
>> +	data->stg_offset_4 = args[0];
>> +	data->stg_offset_196 = args[1];
>> +	data->stg_offset_328 = args[2];
>> +	data->stg_offset_500 = args[3];
>> +
>> +	data->sys_syscon = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
>> +		"starfive,sys-syscon", 1, args);
>> +	if (IS_ERR(data->sys_syscon))
>> +		return dev_err_probe(dev, PTR_ERR(data->sys_syscon),
>> +			"Failed to parse starfive,sys-syscon\n");
>> +
>> +	data->sys_offset = args[0];
>> +
>> +	ret = cdns_mode_init(pdev, data, &dr_mode);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = cdns_clk_rst_init(data);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = of_platform_populate(node, NULL, NULL, dev);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to create children\n");
>> +
>> +	device_set_wakeup_capable(dev, true);
>> +	pm_runtime_set_active(dev);
>> +	pm_runtime_enable(dev);
>> +
>> +	dev_info(dev, "usb mode %s %s probe success\n",
>> +		dr_mode ? dr_mode : "unknown", data->usb2_only ? "2.0" : "3.0");
>> +
>> +	return 0;
>> +}
>> +
>> +static int cdns_starfive_remove_core(struct device *dev, void *c)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +
>> +	platform_device_unregister(pdev);
>> +
>> +	return 0;
>> +}
>> +
>> +static int cdns_starfive_remove(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct cdns_starfive *data = dev_get_drvdata(dev);
>> +
>> +	pm_runtime_get_sync(dev);
>> +	device_for_each_child(dev, NULL, cdns_starfive_remove_core);
>> +
>> +	reset_control_assert(data->resets);
>> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +	pm_runtime_disable(dev);
>> +	pm_runtime_put_noidle(dev);
>> +	platform_set_drvdata(pdev, NULL);
>> +
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int cdns_starfive_resume(struct device *dev)
>> +{
>> +	struct cdns_starfive *data = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = reset_control_deassert(data->resets);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int cdns_starfive_suspend(struct device *dev)
>> +{
>> +	struct cdns_starfive *data = dev_get_drvdata(dev);
>> +
>> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +	reset_control_assert(data->resets);
>> +
>> +	return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops cdns_starfive_pm_ops = {
>> +	SET_RUNTIME_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume, NULL)
>> +	SET_SYSTEM_SLEEP_PM_OPS(cdns_starfive_suspend, cdns_starfive_resume)
>> +};
>> +
>> +static const struct of_device_id cdns_starfive_of_match[] = {
>> +	{ .compatible = "starfive,jh7110-usb", },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, cdns_starfive_of_match);
>> +
>> +static struct platform_driver cdns_starfive_driver = {
>> +	.probe		= cdns_starfive_probe,
>> +	.remove		= cdns_starfive_remove,
>> +	.driver		= {
>> +		.name	= "cdns3-starfive",
>> +		.of_match_table	= cdns_starfive_of_match,
>> +		.pm	= &cdns_starfive_pm_ops,
>> +	},
>> +};
>> +module_platform_driver(cdns_starfive_driver);
>> +
>> +MODULE_ALIAS("platform:cdns3-starfive");
>> +MODULE_AUTHOR("YanHong Wang <yanhong.wang@starfivetech.com>");
>> +MODULE_AUTHOR("Mason Huo <mason.huo@starfivetech.com>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
>> -- 
>> 2.17.1
>> 

Re: [PATCH v3 4/5] usb: cdns3: add StarFive JH7110 USB driver.

[Philipp Zabel]

On Mi, 2023-03-15 at 18:44 +0800, Minda Chen wrote:
> There is a Cadence USB3 core for JH7110 SoCs, the cdns
> core is the child of this USB wrapper module device.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
[...]
> diff --git a/drivers/usb/cdns3/cdns3-starfive.c b/drivers/usb/cdns3/cdns3-starfive.c
> new file mode 100644
> index 000000000000..a99f98f85235
> --- /dev/null
> +++ b/drivers/usb/cdns3/cdns3-starfive.c
> @@ -0,0 +1,305 @@
[...]
> +static int cdns_clk_rst_init(struct cdns_starfive *data)
> +{
> +	int ret;
> +
> +	data->num_clks = devm_clk_bulk_get_all(data->dev, &data->clks);
> +	if (data->num_clks < 0)
> +		return dev_err_probe(data->dev, -ENODEV,
> +			"Failed to get clocks\n");
> +
> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +			"failed to enable clocks\n");

In general, it's better to acquire all resources first and only then
start interacting with them, and to order all devm_ calls before non-
devm calls to make sure cleanup is done in reverse order.

In this case you can switch clk_bulk_prepare_enable() with
devm_reset_control_array_get_exclusive() and simplify the error path.

> +	data->resets = devm_reset_control_array_get_exclusive(data->dev);
> +	if (IS_ERR(data->resets)) {
> +		ret = dev_err_probe(data->dev, PTR_ERR(data->resets),
> +			"Failed to get resets");
> +		goto err_clk_init;
> +	}
> +
> +	ret = reset_control_deassert(data->resets);
> +	if (ret) {
> +		ret = dev_err_probe(data->dev, ret,
> +			"failed to reset clocks\n");
> +		goto err_clk_init;
> +	}
> +
> +	return ret;
> +
> +err_clk_init:
> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
> +	return ret;
> +}

regards
Philipp

Re: [PATCH v3 4/5] usb: cdns3: add StarFive JH7110 USB driver.

[Minda Chen]

On 2023/3/23 17:29, Philipp Zabel wrote:
> On Mi, 2023-03-15 at 18:44 +0800, Minda Chen wrote:
>> There is a Cadence USB3 core for JH7110 SoCs, the cdns
>> core is the child of this USB wrapper module device.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
> [...]
>> diff --git a/drivers/usb/cdns3/cdns3-starfive.c b/drivers/usb/cdns3/cdns3-starfive.c
>> new file mode 100644
>> index 000000000000..a99f98f85235
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/cdns3-starfive.c
>> @@ -0,0 +1,305 @@
> [...]
>> +static int cdns_clk_rst_init(struct cdns_starfive *data)
>> +{
>> +	int ret;
>> +
>> +	data->num_clks = devm_clk_bulk_get_all(data->dev, &data->clks);
>> +	if (data->num_clks < 0)
>> +		return dev_err_probe(data->dev, -ENODEV,
>> +			"Failed to get clocks\n");
>> +
>> +	ret = clk_bulk_prepare_enable(data->num_clks, data->clks);
>> +	if (ret)
>> +		return dev_err_probe(data->dev, ret,
>> +			"failed to enable clocks\n");
> 
> In general, it's better to acquire all resources first and only then
> start interacting with them, and to order all devm_ calls before non-
> devm calls to make sure cleanup is done in reverse order.
> 
> In this case you can switch clk_bulk_prepare_enable() with
> devm_reset_control_array_get_exclusive() and simplify the error path.
> 
OK, thanks
>> +	data->resets = devm_reset_control_array_get_exclusive(data->dev);
>> +	if (IS_ERR(data->resets)) {
>> +		ret = dev_err_probe(data->dev, PTR_ERR(data->resets),
>> +			"Failed to get resets");
>> +		goto err_clk_init;
>> +	}
>> +
>> +	ret = reset_control_deassert(data->resets);
>> +	if (ret) {
>> +		ret = dev_err_probe(data->dev, ret,
>> +			"failed to reset clocks\n");
>> +		goto err_clk_init;
>> +	}
>> +
>> +	return ret;
>> +
>> +err_clk_init:
>> +	clk_bulk_disable_unprepare(data->num_clks, data->clks);
>> +	return ret;
>> +}
> 
> regards
> Philipp

[PATCH v3 5/5] dts: usb: add StarFive JH7110 USB dts configuration.

[Minda Chen]

USB Glue layer and Cadence USB subnode configuration,
also includes USB and PCIe phy dts configuration.

Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
 .../jh7110-starfive-visionfive-2.dtsi         |  7 +++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 54 +++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index a132debb9b53..c64476aebc1a 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -236,3 +236,10 @@
 	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
+
+&usb0 {
+	status = "okay";
+	usbdrd_cdns3: usb@0 {
+		dr_mode = "peripheral";
+	};
+};
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index f70a4ed47eb4..17722fd1be62 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -362,6 +362,60 @@
 			status = "disabled";
 		};
 
+		usb0: usb@10100000 {
+			compatible = "starfive,jh7110-usb";
+			clocks = <&stgcrg JH7110_STGCLK_USB0_LPM>,
+				 <&stgcrg JH7110_STGCLK_USB0_STB>,
+				 <&stgcrg JH7110_STGCLK_USB0_APB>,
+				 <&stgcrg JH7110_STGCLK_USB0_AXI>,
+				 <&stgcrg JH7110_STGCLK_USB0_UTMI_APB>;
+			clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
+			resets = <&stgcrg JH7110_STGRST_USB0_PWRUP>,
+				 <&stgcrg JH7110_STGRST_USB0_APB>,
+				 <&stgcrg JH7110_STGRST_USB0_AXI>,
+				 <&stgcrg JH7110_STGRST_USB0_UTMI_APB>;
+			starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
+			starfive,sys-syscon = <&sys_syscon 0x18>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0x0 0x0 0x10100000 0x100000>;
+
+			usbdrd_cdns3: usb@0 {
+				compatible = "cdns,usb3";
+				reg = <0x0 0x10000>,
+				      <0x10000 0x10000>,
+				      <0x20000 0x10000>;
+				reg-names = "otg", "xhci", "dev";
+				interrupts = <100>, <108>, <110>;
+				interrupt-names = "host", "peripheral", "otg";
+				phys = <&usbphy0>;
+				phy-names = "cdns3,usb2-phy";
+				maximum-speed = "super-speed";
+			};
+		};
+
+		usbphy0: phy@10200000 {
+			compatible = "starfive,jh7110-usb-phy";
+			reg = <0x0 0x10200000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
+				 <&stgcrg JH7110_STGCLK_USB0_APP_125>;
+			clock-names = "125m", "app_125";
+			#phy-cells = <0>;
+		};
+
+		pciephy0: phy@10210000 {
+			compatible = "starfive,jh7110-pcie-phy";
+			reg = <0x0 0x10210000 0x0 0x10000>;
+			#phy-cells = <0>;
+		};
+
+		pciephy1: phy@10220000 {
+			compatible = "starfive,jh7110-pcie-phy";
+			reg = <0x0 0x10220000 0x0 0x10000>;
+			#phy-cells = <0>;
+		};
+
 		stgcrg: clock-controller@10230000 {
 			compatible = "starfive,jh7110-stgcrg";
 			reg = <0x0 0x10230000 0x0 0x10000>;
-- 
2.17.1

Re: [PATCH v3 5/5] dts: usb: add StarFive JH7110 USB dts configuration.

[Peter Chen]

On 23-03-15 18:44:11, Minda Chen wrote:
> USB Glue layer and Cadence USB subnode configuration,
> also includes USB and PCIe phy dts configuration.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  .../jh7110-starfive-visionfive-2.dtsi         |  7 +++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 54 +++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index a132debb9b53..c64476aebc1a 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -236,3 +236,10 @@
>  	pinctrl-0 = <&uart0_pins>;
>  	status = "okay";
>  };
> +
> +&usb0 {
> +	status = "okay";
> +	usbdrd_cdns3: usb@0 {
> +		dr_mode = "peripheral";
> +	};
> +};
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index f70a4ed47eb4..17722fd1be62 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -362,6 +362,60 @@
>  			status = "disabled";
>  		};
>  
> +		usb0: usb@10100000 {
> +			compatible = "starfive,jh7110-usb";
> +			clocks = <&stgcrg JH7110_STGCLK_USB0_LPM>,
> +				 <&stgcrg JH7110_STGCLK_USB0_STB>,
> +				 <&stgcrg JH7110_STGCLK_USB0_APB>,
> +				 <&stgcrg JH7110_STGCLK_USB0_AXI>,
> +				 <&stgcrg JH7110_STGCLK_USB0_UTMI_APB>;
> +			clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
> +			resets = <&stgcrg JH7110_STGRST_USB0_PWRUP>,
> +				 <&stgcrg JH7110_STGRST_USB0_APB>,
> +				 <&stgcrg JH7110_STGRST_USB0_AXI>,
> +				 <&stgcrg JH7110_STGRST_USB0_UTMI_APB>;
> +			starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
> +			starfive,sys-syscon = <&sys_syscon 0x18>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0x0 0x0 0x10100000 0x100000>;

Why it is four entry at ranges? Your address-cells and size-cells are
both 1, and your binding-doc is also three?

Peter
> +
> +			usbdrd_cdns3: usb@0 {
> +				compatible = "cdns,usb3";
> +				reg = <0x0 0x10000>,
> +				      <0x10000 0x10000>,
> +				      <0x20000 0x10000>;
> +				reg-names = "otg", "xhci", "dev";
> +				interrupts = <100>, <108>, <110>;
> +				interrupt-names = "host", "peripheral", "otg";
> +				phys = <&usbphy0>;
> +				phy-names = "cdns3,usb2-phy";
> +				maximum-speed = "super-speed";
> +			};
> +		};
> +
> +		usbphy0: phy@10200000 {
> +			compatible = "starfive,jh7110-usb-phy";
> +			reg = <0x0 0x10200000 0x0 0x10000>;
> +			clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
> +				 <&stgcrg JH7110_STGCLK_USB0_APP_125>;
> +			clock-names = "125m", "app_125";
> +			#phy-cells = <0>;
> +		};
> +
> +		pciephy0: phy@10210000 {
> +			compatible = "starfive,jh7110-pcie-phy";
> +			reg = <0x0 0x10210000 0x0 0x10000>;
> +			#phy-cells = <0>;
> +		};
> +
> +		pciephy1: phy@10220000 {
> +			compatible = "starfive,jh7110-pcie-phy";
> +			reg = <0x0 0x10220000 0x0 0x10000>;
> +			#phy-cells = <0>;
> +		};
> +
>  		stgcrg: clock-controller@10230000 {
>  			compatible = "starfive,jh7110-stgcrg";
>  			reg = <0x0 0x10230000 0x0 0x10000>;
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen

Re: [PATCH v3 5/5] dts: usb: add StarFive JH7110 USB dts configuration.

[Minda Chen]

On 2023/3/16 10:43, Peter Chen wrote:
> On 23-03-15 18:44:11, Minda Chen wrote:
>> USB Glue layer and Cadence USB subnode configuration,
>> also includes USB and PCIe phy dts configuration.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  .../jh7110-starfive-visionfive-2.dtsi         |  7 +++
>>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 54 +++++++++++++++++++
>>  2 files changed, 61 insertions(+)
>> 
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index a132debb9b53..c64476aebc1a 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -236,3 +236,10 @@
>>  	pinctrl-0 = <&uart0_pins>;
>>  	status = "okay";
>>  };
>> +
>> +&usb0 {
>> +	status = "okay";
>> +	usbdrd_cdns3: usb@0 {
>> +		dr_mode = "peripheral";
>> +	};
>> +};
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> index f70a4ed47eb4..17722fd1be62 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -362,6 +362,60 @@
>>  			status = "disabled";
>>  		};
>>  
>> +		usb0: usb@10100000 {
>> +			compatible = "starfive,jh7110-usb";
>> +			clocks = <&stgcrg JH7110_STGCLK_USB0_LPM>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_STB>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_APB>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_AXI>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_UTMI_APB>;
>> +			clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
>> +			resets = <&stgcrg JH7110_STGRST_USB0_PWRUP>,
>> +				 <&stgcrg JH7110_STGRST_USB0_APB>,
>> +				 <&stgcrg JH7110_STGRST_USB0_AXI>,
>> +				 <&stgcrg JH7110_STGRST_USB0_UTMI_APB>;
>> +			starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
>> +			starfive,sys-syscon = <&sys_syscon 0x18>;
>> +			status = "disabled";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges = <0x0 0x0 0x10100000 0x100000>;
> 
> Why it is four entry at ranges? Your address-cells and size-cells are
> both 1, and your binding-doc is also three?
> 
> Peter
Because the parent soc node address-cells is 2. So the local address is 2 entry.
  	
	soc {
                compatible = "simple-bus";
                interrupt-parent = <&plic>;
                #address-cells = <2>;
                #size-cells = <2>;
                ranges;
                ...

So should I change the binding-doc ?
>> +
>> +			usbdrd_cdns3: usb@0 {
>> +				compatible = "cdns,usb3";
>> +				reg = <0x0 0x10000>,
>> +				      <0x10000 0x10000>,
>> +				      <0x20000 0x10000>;
>> +				reg-names = "otg", "xhci", "dev";
>> +				interrupts = <100>, <108>, <110>;
>> +				interrupt-names = "host", "peripheral", "otg";
>> +				phys = <&usbphy0>;
>> +				phy-names = "cdns3,usb2-phy";
>> +				maximum-speed = "super-speed";
>> +			};
>> +		};
>> +
>> +		usbphy0: phy@10200000 {
>> +			compatible = "starfive,jh7110-usb-phy";
>> +			reg = <0x0 0x10200000 0x0 0x10000>;
>> +			clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_APP_125>;
>> +			clock-names = "125m", "app_125";
>> +			#phy-cells = <0>;
>> +		};
>> +
>> +		pciephy0: phy@10210000 {
>> +			compatible = "starfive,jh7110-pcie-phy";
>> +			reg = <0x0 0x10210000 0x0 0x10000>;
>> +			#phy-cells = <0>;
>> +		};
>> +
>> +		pciephy1: phy@10220000 {
>> +			compatible = "starfive,jh7110-pcie-phy";
>> +			reg = <0x0 0x10220000 0x0 0x10000>;
>> +			#phy-cells = <0>;
>> +		};
>> +
>>  		stgcrg: clock-controller@10230000 {
>>  			compatible = "starfive,jh7110-stgcrg";
>>  			reg = <0x0 0x10230000 0x0 0x10000>;
>> -- 
>> 2.17.1
>> 
> 

Re: [PATCH v3 5/5] dts: usb: add StarFive JH7110 USB dts configuration.

[Krzysztof Kozlowski]

On 15/03/2023 11:44, Minda Chen wrote:
> USB Glue layer and Cadence USB subnode configuration,
> also includes USB and PCIe phy dts configuration.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

> ---
>  .../jh7110-starfive-visionfive-2.dtsi         |  7 +++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 54 +++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index a132debb9b53..c64476aebc1a 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -236,3 +236,10 @@
>  	pinctrl-0 = <&uart0_pins>;
>  	status = "okay";
>  };
> +
> +&usb0 {
> +	status = "okay";
> +	usbdrd_cdns3: usb@0 {

You should rather override by phandle this as well.

> +		dr_mode = "peripheral";
> +	};
> +};
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index f70a4ed47eb4..17722fd1be62 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -362,6 +362,60 @@
>  			status = "disabled";
>  		};
>  
> +		usb0: usb@10100000 {
> +			compatible = "starfive,jh7110-usb";
> +			clocks = <&stgcrg JH7110_STGCLK_USB0_LPM>,
> +				 <&stgcrg JH7110_STGCLK_USB0_STB>,
> +				 <&stgcrg JH7110_STGCLK_USB0_APB>,
> +				 <&stgcrg JH7110_STGCLK_USB0_AXI>,
> +				 <&stgcrg JH7110_STGCLK_USB0_UTMI_APB>;
> +			clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
> +			resets = <&stgcrg JH7110_STGRST_USB0_PWRUP>,
> +				 <&stgcrg JH7110_STGRST_USB0_APB>,
> +				 <&stgcrg JH7110_STGRST_USB0_AXI>,
> +				 <&stgcrg JH7110_STGRST_USB0_UTMI_APB>;
> +			starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
> +			starfive,sys-syscon = <&sys_syscon 0x18>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0x0 0x0 0x10100000 0x100000>;

reg and ranges should be second property. This also applies to your
binding example.


Best regards,
Krzysztof

Re: [PATCH v3 5/5] dts: usb: add StarFive JH7110 USB dts configuration.

[Minda Chen]

On 2023/3/17 16:44, Krzysztof Kozlowski wrote:
> On 15/03/2023 11:44, Minda Chen wrote:
>> USB Glue layer and Cadence USB subnode configuration,
>> also includes USB and PCIe phy dts configuration.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> 
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
>ok
>> ---
>>  .../jh7110-starfive-visionfive-2.dtsi         |  7 +++
>>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 54 +++++++++++++++++++
>>  2 files changed, 61 insertions(+)
>> 
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index a132debb9b53..c64476aebc1a 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -236,3 +236,10 @@
>>  	pinctrl-0 = <&uart0_pins>;
>>  	status = "okay";
>>  };
>> +
>> +&usb0 {
>> +	status = "okay";
>> +	usbdrd_cdns3: usb@0 {
> 
> You should rather override by phandle this as well.
> 
I will remove the wrapper node 'usb0'. 

Just like this :
+&usbdrd_cdns3 {
+       dr_mode = "peripheral";
+};


（similar configuration in arch/arm64/boot/dts/ti/k3-j721e-sk.dts, usb1 node)
   &usb1 {
        dr_mode = "host";
        ....
  
>> +		dr_mode = "peripheral";
>> +	};
>> +};
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> index f70a4ed47eb4..17722fd1be62 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -362,6 +362,60 @@
>>  			status = "disabled";
>>  		};
>>  
>> +		usb0: usb@10100000 {
>> +			compatible = "starfive,jh7110-usb";
>> +			clocks = <&stgcrg JH7110_STGCLK_USB0_LPM>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_STB>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_APB>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_AXI>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_UTMI_APB>;
>> +			clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
>> +			resets = <&stgcrg JH7110_STGRST_USB0_PWRUP>,
>> +				 <&stgcrg JH7110_STGRST_USB0_APB>,
>> +				 <&stgcrg JH7110_STGRST_USB0_AXI>,
>> +				 <&stgcrg JH7110_STGRST_USB0_UTMI_APB>;
>> +			starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
>> +			starfive,sys-syscon = <&sys_syscon 0x18>;
>> +			status = "disabled";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges = <0x0 0x0 0x10100000 0x100000>;
> 
> reg and ranges should be second property. This also applies to your
> binding example.
> 
> 
ok, thanks
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 5/5] dts: usb: add StarFive JH7110 USB dts configuration.

[Rob Herring]

On Wed, Mar 15, 2023 at 06:44:11PM +0800, Minda Chen wrote:
> USB Glue layer and Cadence USB subnode configuration,
> also includes USB and PCIe phy dts configuration.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  .../jh7110-starfive-visionfive-2.dtsi         |  7 +++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 54 +++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index a132debb9b53..c64476aebc1a 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -236,3 +236,10 @@
>  	pinctrl-0 = <&uart0_pins>;
>  	status = "okay";
>  };
> +
> +&usb0 {
> +	status = "okay";
> +	usbdrd_cdns3: usb@0 {
> +		dr_mode = "peripheral";
> +	};
> +};
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index f70a4ed47eb4..17722fd1be62 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -362,6 +362,60 @@
>  			status = "disabled";
>  		};
>  
> +		usb0: usb@10100000 {
> +			compatible = "starfive,jh7110-usb";
> +			clocks = <&stgcrg JH7110_STGCLK_USB0_LPM>,
> +				 <&stgcrg JH7110_STGCLK_USB0_STB>,
> +				 <&stgcrg JH7110_STGCLK_USB0_APB>,
> +				 <&stgcrg JH7110_STGCLK_USB0_AXI>,
> +				 <&stgcrg JH7110_STGCLK_USB0_UTMI_APB>;
> +			clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
> +			resets = <&stgcrg JH7110_STGRST_USB0_PWRUP>,
> +				 <&stgcrg JH7110_STGRST_USB0_APB>,
> +				 <&stgcrg JH7110_STGRST_USB0_AXI>,
> +				 <&stgcrg JH7110_STGRST_USB0_UTMI_APB>;
> +			starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
> +			starfive,sys-syscon = <&sys_syscon 0x18>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0x0 0x0 0x10100000 0x100000>;
> +
> +			usbdrd_cdns3: usb@0 {
> +				compatible = "cdns,usb3";

This pattern of USB wrapper and then a "generic" IP node is discouraged 
if it is just clocks, resets, power-domains, etc. IOW, unless there's an 
actual wrapper h/w block with its own registers, then don't do this 
split. Merge it all into a single node.

> +				reg = <0x0 0x10000>,
> +				      <0x10000 0x10000>,
> +				      <0x20000 0x10000>;
> +				reg-names = "otg", "xhci", "dev";
> +				interrupts = <100>, <108>, <110>;
> +				interrupt-names = "host", "peripheral", "otg";
> +				phys = <&usbphy0>;
> +				phy-names = "cdns3,usb2-phy";

No need for *-names when there is only 1 entry. Names are local to the 
device and only to distinguish entries, so 'usb2' would be sufficient 
here.

> +				maximum-speed = "super-speed";
> +			};
> +		};
> +
> +		usbphy0: phy@10200000 {
> +			compatible = "starfive,jh7110-usb-phy";
> +			reg = <0x0 0x10200000 0x0 0x10000>;
> +			clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
> +				 <&stgcrg JH7110_STGCLK_USB0_APP_125>;
> +			clock-names = "125m", "app_125";
> +			#phy-cells = <0>;
> +		};
> +
> +		pciephy0: phy@10210000 {
> +			compatible = "starfive,jh7110-pcie-phy";
> +			reg = <0x0 0x10210000 0x0 0x10000>;
> +			#phy-cells = <0>;
> +		};
> +
> +		pciephy1: phy@10220000 {
> +			compatible = "starfive,jh7110-pcie-phy";
> +			reg = <0x0 0x10220000 0x0 0x10000>;
> +			#phy-cells = <0>;
> +		};
> +
>  		stgcrg: clock-controller@10230000 {
>  			compatible = "starfive,jh7110-stgcrg";
>  			reg = <0x0 0x10230000 0x0 0x10000>;
> -- 
> 2.17.1
> 

Re: [PATCH v3 5/5] dts: usb: add StarFive JH7110 USB dts configuration.

[Minda Chen]

On 2023/3/20 23:34, Rob Herring wrote:
> On Wed, Mar 15, 2023 at 06:44:11PM +0800, Minda Chen wrote:
>> USB Glue layer and Cadence USB subnode configuration,
>> also includes USB and PCIe phy dts configuration.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  .../jh7110-starfive-visionfive-2.dtsi         |  7 +++
>>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 54 +++++++++++++++++++
>>  2 files changed, 61 insertions(+)
>> 
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index a132debb9b53..c64476aebc1a 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -236,3 +236,10 @@
>>  	pinctrl-0 = <&uart0_pins>;
>>  	status = "okay";
>>  };
>> +
>> +&usb0 {
>> +	status = "okay";
>> +	usbdrd_cdns3: usb@0 {
>> +		dr_mode = "peripheral";
>> +	};
>> +};
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> index f70a4ed47eb4..17722fd1be62 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -362,6 +362,60 @@
>>  			status = "disabled";
>>  		};
>>  
>> +		usb0: usb@10100000 {
>> +			compatible = "starfive,jh7110-usb";
>> +			clocks = <&stgcrg JH7110_STGCLK_USB0_LPM>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_STB>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_APB>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_AXI>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_UTMI_APB>;
>> +			clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
>> +			resets = <&stgcrg JH7110_STGRST_USB0_PWRUP>,
>> +				 <&stgcrg JH7110_STGRST_USB0_APB>,
>> +				 <&stgcrg JH7110_STGRST_USB0_AXI>,
>> +				 <&stgcrg JH7110_STGRST_USB0_UTMI_APB>;
>> +			starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
>> +			starfive,sys-syscon = <&sys_syscon 0x18>;
>> +			status = "disabled";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges = <0x0 0x0 0x10100000 0x100000>;
>> +
>> +			usbdrd_cdns3: usb@0 {
>> +				compatible = "cdns,usb3";
> 
> This pattern of USB wrapper and then a "generic" IP node is discouraged 
> if it is just clocks, resets, power-domains, etc. IOW, unless there's an 
> actual wrapper h/w block with its own registers, then don't do this 
> split. Merge it all into a single node.
> 
I am afraid it is difficult to merge in one single node. 

1.If cadence3 usb device is still the sub device. All the dts setting are in
StarFive node. This can not work.
StarFive driver code Using platform_device_add generate cadenc3 usb platform device. 
Even IO memory space setting can be passed to cadence3 USB, PHY setting can not be passed.
For the PHY driver using dts now. But in this case, Cadence3 USB no dts configure.

2. Just one USB Cadence platform device.
Maybe this can work. But Cadence USB driver code cdns3-plat.c required to changed.

Hi Peter Pawel and Roger
   There is a "platform_suspend" function pointer in "struct cdns3_platform_data"，
   Add "platform_init" and "platform_exit" for our JH7110 platform. Maybe it can work.
   Is it OK?   
>> +				reg = <0x0 0x10000>,
>> +				      <0x10000 0x10000>,
>> +				      <0x20000 0x10000>;
>> +				reg-names = "otg", "xhci", "dev";
>> +				interrupts = <100>, <108>, <110>;
>> +				interrupt-names = "host", "peripheral", "otg";
>> +				phys = <&usbphy0>;
>> +				phy-names = "cdns3,usb2-phy";
> 
> No need for *-names when there is only 1 entry. Names are local to the 
> device and only to distinguish entries, so 'usb2' would be sufficient 
> here.
> 
The PHY name 'cdns3,usb2-phy'  is defined in cadence3 usb driver code.
Cadence USB3 driver code using this name to get PHY instance.
And all the PHY ops used in Cadence3 USB sub device. 
>> +				maximum-speed = "super-speed";
>> +			};
>> +		};
>> +
>> +		usbphy0: phy@10200000 {
>> +			compatible = "starfive,jh7110-usb-phy";
>> +			reg = <0x0 0x10200000 0x0 0x10000>;
>> +			clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
>> +				 <&stgcrg JH7110_STGCLK_USB0_APP_125>;
>> +			clock-names = "125m", "app_125";
>> +			#phy-cells = <0>;
>> +		};
>> +
>> +		pciephy0: phy@10210000 {
>> +			compatible = "starfive,jh7110-pcie-phy";
>> +			reg = <0x0 0x10210000 0x0 0x10000>;
>> +			#phy-cells = <0>;
>> +		};
>> +
>> +		pciephy1: phy@10220000 {
>> +			compatible = "starfive,jh7110-pcie-phy";
>> +			reg = <0x0 0x10220000 0x0 0x10000>;
>> +			#phy-cells = <0>;
>> +		};
>> +
>>  		stgcrg: clock-controller@10230000 {
>>  			compatible = "starfive,jh7110-stgcrg";
>>  			reg = <0x0 0x10230000 0x0 0x10000>;
>> -- 
>> 2.17.1
>> 

Re: [PATCH v3 5/5] dts: usb: add StarFive JH7110 USB dts configuration.

[Roger Quadros]

Hi Minda,

On 21/03/2023 14:35, Minda Chen wrote:
> 
> 
> On 2023/3/20 23:34, Rob Herring wrote:
>> On Wed, Mar 15, 2023 at 06:44:11PM +0800, Minda Chen wrote:
>>> USB Glue layer and Cadence USB subnode configuration,
>>> also includes USB and PCIe phy dts configuration.
>>>
>>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>>> ---
>>>  .../jh7110-starfive-visionfive-2.dtsi         |  7 +++
>>>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 54 +++++++++++++++++++
>>>  2 files changed, 61 insertions(+)
>>>
>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>>> index a132debb9b53..c64476aebc1a 100644
>>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>>> @@ -236,3 +236,10 @@
>>>  	pinctrl-0 = <&uart0_pins>;
>>>  	status = "okay";
>>>  };
>>> +
>>> +&usb0 {
>>> +	status = "okay";
>>> +	usbdrd_cdns3: usb@0 {
>>> +		dr_mode = "peripheral";
>>> +	};
>>> +};
>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>> index f70a4ed47eb4..17722fd1be62 100644
>>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>> @@ -362,6 +362,60 @@
>>>  			status = "disabled";
>>>  		};
>>>  
>>> +		usb0: usb@10100000 {
>>> +			compatible = "starfive,jh7110-usb";
>>> +			clocks = <&stgcrg JH7110_STGCLK_USB0_LPM>,
>>> +				 <&stgcrg JH7110_STGCLK_USB0_STB>,
>>> +				 <&stgcrg JH7110_STGCLK_USB0_APB>,
>>> +				 <&stgcrg JH7110_STGCLK_USB0_AXI>,
>>> +				 <&stgcrg JH7110_STGCLK_USB0_UTMI_APB>;
>>> +			clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
>>> +			resets = <&stgcrg JH7110_STGRST_USB0_PWRUP>,
>>> +				 <&stgcrg JH7110_STGRST_USB0_APB>,
>>> +				 <&stgcrg JH7110_STGRST_USB0_AXI>,
>>> +				 <&stgcrg JH7110_STGRST_USB0_UTMI_APB>;
>>> +			starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
>>> +			starfive,sys-syscon = <&sys_syscon 0x18>;
>>> +			status = "disabled";
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +			ranges = <0x0 0x0 0x10100000 0x100000>;
>>> +
>>> +			usbdrd_cdns3: usb@0 {
>>> +				compatible = "cdns,usb3";
>>
>> This pattern of USB wrapper and then a "generic" IP node is discouraged 
>> if it is just clocks, resets, power-domains, etc. IOW, unless there's an 
>> actual wrapper h/w block with its own registers, then don't do this 
>> split. Merge it all into a single node.
>>
> I am afraid it is difficult to merge in one single node. 
> 
> 1.If cadence3 usb device is still the sub device. All the dts setting are in
> StarFive node. This can not work.
> StarFive driver code Using platform_device_add generate cadenc3 usb platform device. 
> Even IO memory space setting can be passed to cadence3 USB, PHY setting can not be passed.
> For the PHY driver using dts now. But in this case, Cadence3 USB no dts configure.
> 
> 2. Just one USB Cadence platform device.
> Maybe this can work. But Cadence USB driver code cdns3-plat.c required to changed.
> 
> Hi Peter Pawel and Roger
>    There is a "platform_suspend" function pointer in "struct cdns3_platform_data"，
>    Add "platform_init" and "platform_exit" for our JH7110 platform. Maybe it can work.
>    Is it OK?   

Once you move all the syscon register modifications from your wrapper driver
to your PHY driver, only clock and reset control are left in your wrapper driver.
This is generic enough to be done in the cdns3,usb driver itself so you don't need a
wrapper node.

Pawel, do you agree?
 
>>> +				reg = <0x0 0x10000>,
>>> +				      <0x10000 0x10000>,
>>> +				      <0x20000 0x10000>;
>>> +				reg-names = "otg", "xhci", "dev";
>>> +				interrupts = <100>, <108>, <110>;
>>> +				interrupt-names = "host", "peripheral", "otg";
>>> +				phys = <&usbphy0>;
>>> +				phy-names = "cdns3,usb2-phy";
>>
>> No need for *-names when there is only 1 entry. Names are local to the 
>> device and only to distinguish entries, so 'usb2' would be sufficient 
>> here.
>>
> The PHY name 'cdns3,usb2-phy'  is defined in cadence3 usb driver code.
> Cadence USB3 driver code using this name to get PHY instance.
> And all the PHY ops used in Cadence3 USB sub device. 
>>> +				maximum-speed = "super-speed";
>>> +			};
>>> +		};
>>> +
>>> +		usbphy0: phy@10200000 {
>>> +			compatible = "starfive,jh7110-usb-phy";
>>> +			reg = <0x0 0x10200000 0x0 0x10000>;
>>> +			clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
>>> +				 <&stgcrg JH7110_STGCLK_USB0_APP_125>;
>>> +			clock-names = "125m", "app_125";
>>> +			#phy-cells = <0>;
>>> +		};
>>> +
>>> +		pciephy0: phy@10210000 {
>>> +			compatible = "starfive,jh7110-pcie-phy";
>>> +			reg = <0x0 0x10210000 0x0 0x10000>;
>>> +			#phy-cells = <0>;
>>> +		};
>>> +
>>> +		pciephy1: phy@10220000 {
>>> +			compatible = "starfive,jh7110-pcie-phy";
>>> +			reg = <0x0 0x10220000 0x0 0x10000>;
>>> +			#phy-cells = <0>;
>>> +		};
>>> +
>>>  		stgcrg: clock-controller@10230000 {
>>>  			compatible = "starfive,jh7110-stgcrg";
>>>  			reg = <0x0 0x10230000 0x0 0x10000>;
>>> -- 
>>> 2.17.1
>>>

cheers,
-roger

Re: [PATCH v3 5/5] dts: usb: add StarFive JH7110 USB dts configuration.

[Minda Chen]

On 2023/3/22 16:00, Roger Quadros wrote:
> Hi Minda,
> 
> On 21/03/2023 14:35, Minda Chen wrote:
>> 
>> 
>> On 2023/3/20 23:34, Rob Herring wrote:
>>> On Wed, Mar 15, 2023 at 06:44:11PM +0800, Minda Chen wrote:
>>>> USB Glue layer and Cadence USB subnode configuration,
>>>> also includes USB and PCIe phy dts configuration.
>>>>
>>>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>>>> ---
>>>>  .../jh7110-starfive-visionfive-2.dtsi         |  7 +++
>>>>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 54 +++++++++++++++++++
>>>>  2 files changed, 61 insertions(+)
>>>>
>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>>>> index a132debb9b53..c64476aebc1a 100644
>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>>>> @@ -236,3 +236,10 @@
>>>>  	pinctrl-0 = <&uart0_pins>;
>>>>  	status = "okay";
>>>>  };
>>>> +
>>>> +&usb0 {
>>>> +	status = "okay";
>>>> +	usbdrd_cdns3: usb@0 {
>>>> +		dr_mode = "peripheral";
>>>> +	};
>>>> +};
>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>> index f70a4ed47eb4..17722fd1be62 100644
>>>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>> @@ -362,6 +362,60 @@
>>>>  			status = "disabled";
>>>>  		};
>>>>  
>>>> +		usb0: usb@10100000 {
>>>> +			compatible = "starfive,jh7110-usb";
>>>> +			clocks = <&stgcrg JH7110_STGCLK_USB0_LPM>,
>>>> +				 <&stgcrg JH7110_STGCLK_USB0_STB>,
>>>> +				 <&stgcrg JH7110_STGCLK_USB0_APB>,
>>>> +				 <&stgcrg JH7110_STGCLK_USB0_AXI>,
>>>> +				 <&stgcrg JH7110_STGCLK_USB0_UTMI_APB>;
>>>> +			clock-names = "lpm", "stb", "apb", "axi", "utmi_apb";
>>>> +			resets = <&stgcrg JH7110_STGRST_USB0_PWRUP>,
>>>> +				 <&stgcrg JH7110_STGRST_USB0_APB>,
>>>> +				 <&stgcrg JH7110_STGRST_USB0_AXI>,
>>>> +				 <&stgcrg JH7110_STGRST_USB0_UTMI_APB>;
>>>> +			starfive,stg-syscon = <&stg_syscon 0x4 0xc4 0x148 0x1f4>;
>>>> +			starfive,sys-syscon = <&sys_syscon 0x18>;
>>>> +			status = "disabled";
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <1>;
>>>> +			ranges = <0x0 0x0 0x10100000 0x100000>;
>>>> +
>>>> +			usbdrd_cdns3: usb@0 {
>>>> +				compatible = "cdns,usb3";
>>>
>>> This pattern of USB wrapper and then a "generic" IP node is discouraged 
>>> if it is just clocks, resets, power-domains, etc. IOW, unless there's an 
>>> actual wrapper h/w block with its own registers, then don't do this 
>>> split. Merge it all into a single node.
>>>
>> I am afraid it is difficult to merge in one single node. 
>> 
>> 1.If cadence3 usb device is still the sub device. All the dts setting are in
>> StarFive node. This can not work.
>> StarFive driver code Using platform_device_add generate cadenc3 usb platform device. 
>> Even IO memory space setting can be passed to cadence3 USB, PHY setting can not be passed.
>> For the PHY driver using dts now. But in this case, Cadence3 USB no dts configure.
>> 
>> 2. Just one USB Cadence platform device.
>> Maybe this can work. But Cadence USB driver code cdns3-plat.c required to changed.
>> 
>> Hi Peter Pawel and Roger
>>    There is a "platform_suspend" function pointer in "struct cdns3_platform_data"，
>>    Add "platform_init" and "platform_exit" for our JH7110 platform. Maybe it can work.
>>    Is it OK?   
> 
> Once you move all the syscon register modifications from your wrapper driver
> to your PHY driver, only clock and reset control are left in your wrapper driver.
> This is generic enough to be done in the cdns3,usb driver itself so you don't need a
> wrapper node.
> 
> Pawel, do you agree?
>  
move all the syscon codes to PHY driver is ok. I found dwc3/dwc3-of-simple.c is public codes
and contain just clock and reset control codes. I can change the residual codes to public codes.
But I found rockchip 3399 usb dts which is one of dwc3 simple platform still contain two nodes.
See Documentation/devicetree/bindings/usb/rockchip,rk3399-dwc3.yaml
>>>> +				reg = <0x0 0x10000>,
>>>> +				      <0x10000 0x10000>,
>>>> +				      <0x20000 0x10000>;
>>>> +				reg-names = "otg", "xhci", "dev";
>>>> +				interrupts = <100>, <108>, <110>;
>>>> +				interrupt-names = "host", "peripheral", "otg";
>>>> +				phys = <&usbphy0>;
>>>> +				phy-names = "cdns3,usb2-phy";
>>>
>>> No need for *-names when there is only 1 entry. Names are local to the 
>>> device and only to distinguish entries, so 'usb2' would be sufficient 
>>> here.
>>>
>> The PHY name 'cdns3,usb2-phy'  is defined in cadence3 usb driver code.
>> Cadence USB3 driver code using this name to get PHY instance.
>> And all the PHY ops used in Cadence3 USB sub device. 
>>>> +				maximum-speed = "super-speed";
>>>> +			};
>>>> +		};
>>>> +
>>>> +		usbphy0: phy@10200000 {
>>>> +			compatible = "starfive,jh7110-usb-phy";
>>>> +			reg = <0x0 0x10200000 0x0 0x10000>;
>>>> +			clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
>>>> +				 <&stgcrg JH7110_STGCLK_USB0_APP_125>;
>>>> +			clock-names = "125m", "app_125";
>>>> +			#phy-cells = <0>;
>>>> +		};
>>>> +
>>>> +		pciephy0: phy@10210000 {
>>>> +			compatible = "starfive,jh7110-pcie-phy";
>>>> +			reg = <0x0 0x10210000 0x0 0x10000>;
>>>> +			#phy-cells = <0>;
>>>> +		};
>>>> +
>>>> +		pciephy1: phy@10220000 {
>>>> +			compatible = "starfive,jh7110-pcie-phy";
>>>> +			reg = <0x0 0x10220000 0x0 0x10000>;
>>>> +			#phy-cells = <0>;
>>>> +		};
>>>> +
>>>>  		stgcrg: clock-controller@10230000 {
>>>>  			compatible = "starfive,jh7110-stgcrg";
>>>>  			reg = <0x0 0x10230000 0x0 0x10000>;
>>>> -- 
>>>> 2.17.1
>>>>
> 
> cheers,
> -roger