2023-03-15 10:44:27

by Minda Chen

[permalink] [raw]
Subject: [PATCH v3 0/5] Add JH7110 USB and USB PHY driver support

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/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/

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/[email protected]/
v2: https://patchwork.kernel.org/project/linux-usb/cover/[email protected]/

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



2023-03-15 10:44:31

by Minda Chen

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

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 <[email protected]>
---
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 <[email protected]>
+M: Minda Chen <[email protected]>
+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 <[email protected]>
M: Josh Poimboeuf <[email protected]>
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 <[email protected]>
+ */
+
+#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 <[email protected]>");
+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 <[email protected]>
+ */
+
+#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 <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.17.1


2023-03-15 10:44:33

by Minda Chen

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

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

Signed-off-by: Minda Chen <[email protected]>
---
.../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 <[email protected]>
+
+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


2023-03-15 10:44:37

by Minda Chen

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

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

Signed-off-by: Minda Chen <[email protected]>
---
.../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


2023-03-15 10:44:40

by Minda Chen

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

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 <[email protected]>
---
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 <[email protected]>
+M: Minda Chen <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
+F: drivers/usb/cdns3/cdns3-starfive.c
+
STATIC BRANCH/CALL
M: Peter Zijlstra <[email protected]>
M: Josh Poimboeuf <[email protected]>
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 <[email protected]>
+ * Author: Mason Huo <[email protected]>
+ * Author: Minda Chen <[email protected]>
+ */
+
+#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 <[email protected]>");
+MODULE_AUTHOR("Mason Huo <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
--
2.17.1


2023-03-15 13:36:17

by Dongliang Mu

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

.

On Wed, Mar 15, 2023 at 6:48 PM Minda Chen <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>
> +M: Minda Chen <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> +F: drivers/usb/cdns3/cdns3-starfive.c
> +
> STATIC BRANCH/CALL
> M: Peter Zijlstra <[email protected]>
> M: Josh Poimboeuf <[email protected]>
> 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 <[email protected]>
> + * Author: Mason Huo <[email protected]>
> + * Author: Minda Chen <[email protected]>
> + */
> +
> +#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 <[email protected]>");
> +MODULE_AUTHOR("Mason Huo <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
> --
> 2.17.1
>

2023-03-16 02:43:32

by Peter Chen

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

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 <[email protected]>
> ---
> .../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

2023-03-16 02:44:13

by Peter Chen

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

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 <[email protected]>

Reviewed-by: Peter Chen <[email protected]>

> ---
> .../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 <[email protected]>
> +
> +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

2023-03-16 02:47:12

by Peter Chen

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> +M: Minda Chen <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> +F: drivers/usb/cdns3/cdns3-starfive.c
> +
> STATIC BRANCH/CALL
> M: Peter Zijlstra <[email protected]>
> M: Josh Poimboeuf <[email protected]>
> 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 <[email protected]>
> + * Author: Mason Huo <[email protected]>
> + * Author: Minda Chen <[email protected]>
> + */
> +
> +#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 <[email protected]>");
> +MODULE_AUTHOR("Mason Huo <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
> --
> 2.17.1
>

--

Thanks,
Peter Chen

2023-03-16 03:02:36

by Minda Chen

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



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 <[email protected]>
>> ---
>> .../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
>>
>

2023-03-16 11:17:05

by Minda Chen

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



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 <[email protected]>
>> ---
>> 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 <[email protected]>
>> +M: Minda Chen <[email protected]>
>> +S: Maintained
>> +F: Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> +F: drivers/usb/cdns3/cdns3-starfive.c
>> +
>> STATIC BRANCH/CALL
>> M: Peter Zijlstra <[email protected]>
>> M: Josh Poimboeuf <[email protected]>
>> 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 <[email protected]>
>> + * Author: Mason Huo <[email protected]>
>> + * Author: Minda Chen <[email protected]>
>> + */
>> +
>> +#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 <[email protected]>");
>> +MODULE_AUTHOR("Mason Huo <[email protected]>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
>> --
>> 2.17.1
>>
>

2023-03-16 11:18:34

by Minda Chen

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



On 2023/3/15 21:32, Dongliang Mu wrote:
> .
>
> On Wed, Mar 15, 2023 at 6:48 PM Minda Chen <[email protected]> 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 <[email protected]>
>> ---
>> 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 <[email protected]>
>> +M: Minda Chen <[email protected]>
>> +S: Maintained
>> +F: Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> +F: drivers/usb/cdns3/cdns3-starfive.c
>> +
>> STATIC BRANCH/CALL
>> M: Peter Zijlstra <[email protected]>
>> M: Josh Poimboeuf <[email protected]>
>> 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 <[email protected]>
>> + * Author: Mason Huo <[email protected]>
>> + * Author: Minda Chen <[email protected]>
>> + */
>> +
>> +#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 <[email protected]>");
>> +MODULE_AUTHOR("Mason Huo <[email protected]>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
>> --
>> 2.17.1
>>

2023-03-17 08:45:37

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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


2023-03-17 08:46:09

by Krzysztof Kozlowski

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

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 <[email protected]>

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


2023-03-17 10:30:59

by Minda Chen

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



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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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
>

2023-03-17 10:59:35

by Minda Chen

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



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 <[email protected]>
>
> 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
>

2023-03-20 08:56:08

by Vinod Koul

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> +M: Minda Chen <[email protected]>
> +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 <[email protected]>
> M: Josh Poimboeuf <[email protected]>
> 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 <[email protected]>
> + */
> +
> +#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 <[email protected]>");
> +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 <[email protected]>
> + */
> +
> +#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 <[email protected]>");
> +MODULE_LICENSE("GPL");
> --
> 2.17.1

--
~Vinod

2023-03-20 11:06:07

by Minda Chen

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



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 <[email protected]>
>> ---
>> 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 <[email protected]>
>> +M: Minda Chen <[email protected]>
>> +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 <[email protected]>
>> M: Josh Poimboeuf <[email protected]>
>> 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 <[email protected]>
>> + */
>> +
>> +#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 <[email protected]>");
>> +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 <[email protected]>
>> + */
>> +
>> +#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 <[email protected]>");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.17.1
>

2023-03-20 15:34:29

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> +M: Minda Chen <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
> +F: drivers/usb/cdns3/cdns3-starfive.c
> +
> STATIC BRANCH/CALL
> M: Peter Zijlstra <[email protected]>
> M: Josh Poimboeuf <[email protected]>
> 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 <[email protected]>
> + * Author: Mason Huo <[email protected]>
> + * Author: Minda Chen <[email protected]>
> + */
> +
> +#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 <[email protected]>");
> +MODULE_AUTHOR("Mason Huo <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
> --
> 2.17.1
>

2023-03-20 15:44:15

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> .../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
>

2023-03-21 11:50:54

by Minda Chen

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



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 <[email protected]>
>> ---
>> 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 <[email protected]>
>> +M: Minda Chen <[email protected]>
>> +S: Maintained
>> +F: Documentation/devicetree/bindings/usb/starfive,jh7110-usb.yaml
>> +F: drivers/usb/cdns3/cdns3-starfive.c
>> +
>> STATIC BRANCH/CALL
>> M: Peter Zijlstra <[email protected]>
>> M: Josh Poimboeuf <[email protected]>
>> 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 <[email protected]>
>> + * Author: Mason Huo <[email protected]>
>> + * Author: Minda Chen <[email protected]>
>> + */
>> +
>> +#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 <[email protected]>");
>> +MODULE_AUTHOR("Mason Huo <[email protected]>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Cadence USB3 StarFive Glue Layer");
>> --
>> 2.17.1
>>

2023-03-21 12:36:12

by Minda Chen

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



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 <[email protected]>
>> ---
>> .../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
>>

2023-03-22 08:05:59

by Roger Quadros

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

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 <[email protected]>
>>> ---
>>> .../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

2023-03-22 10:52:41

by Minda Chen

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



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 <[email protected]>
>>>> ---
>>>> .../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

2023-03-23 09:30:02

by Philipp Zabel

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

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 <[email protected]>
> ---
>  .../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 <[email protected]>
> +
> +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

2023-03-23 09:39:16

by Philipp Zabel

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

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 <[email protected]>
> ---
[...]
> 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

2023-03-27 11:07:35

by Minda Chen

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



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 <[email protected]>
>> ---
> [...]
>> 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

2023-03-27 11:08:40

by Minda Chen

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



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 <[email protected]>
>> ---
>>  .../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 <[email protected]>
>> +
>> +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