[PATCH RFC v2 0/4] phy: mvebu-cp110-utmi: add support for armada-380 utmi phys

[PATCH RFC v2 0/4] phy: mvebu-cp110-utmi: add support for armada-380 utmi phys

[Josua Mayer]

Armada 380 has smilar USB-2.0 PHYs as CP-110 (Armada 8K).
    
Add support for Armada 380 to cp110 utmi phy driver.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
Changes in v2:
- add support for optional regs / make syscon use optional
- add device-tree changes for armada-388-clearfog
- attempted to fix warning reported by krobot (untested)
- tested on actual hardware
- drafted dt-bindings
- Link to v1: https://lore.kernel.org/r/20240715-a38x-utmi-phy-v1-0-d57250f53cf2@solid-run.com

---
Josua Mayer (4):
      dt-bindings: phy: cp110-utmi-phy: add compatible string for armada-38x
      arm: dts: marvell: armada-38x: add description for usb phys
      phy: mvebu-cp110-utmi: add support for armada-380 utmi phys
      arm: dts: marvell: armada-388-clearfog: add description for usb phys

 .../phy/marvell,armada-cp110-utmi-phy.yaml         |  15 +-
 .../boot/dts/marvell/armada-388-clearfog-base.dts  |   8 +
 arch/arm/boot/dts/marvell/armada-388-clearfog.dts  |   8 +
 arch/arm/boot/dts/marvell/armada-388-clearfog.dtsi |  30 ++-
 arch/arm/boot/dts/marvell/armada-38x.dtsi          |  29 +++
 drivers/phy/marvell/phy-mvebu-cp110-utmi.c         | 209 ++++++++++++++++-----
 6 files changed, 244 insertions(+), 55 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240715-a38x-utmi-phy-02e8059afe35

Sincerely,
-- 
Josua Mayer <josua@solid-run.com>

[PATCH RFC v2 1/4] dt-bindings: phy: cp110-utmi-phy: add compatible string for armada-38x

[Josua Mayer]

Armada 38x USB-2.0 PHYs are similar to Armada 8K (CP110) and can be
supported by the same driver with small differences.

Add new compatible string for armada-38x variant of utmi phy.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 .../bindings/phy/marvell,armada-cp110-utmi-phy.yaml       | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml
index 9ce7b4c6d208..b90bda10b779 100644
--- a/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.yaml
@@ -25,10 +25,21 @@ description:
 
 properties:
   compatible:
-    const: marvell,cp110-utmi-phy
+    enum:
+      - marvell,a38x-utmi-phy
+      - marvell,cp110-utmi-phy
 
   reg:
-    maxItems: 1
+    items:
+      - description: UTMI registers
+      - description: USB config register
+      - description: UTMI config registers
+
+  reg-names:
+    items:
+      - const: utmi
+      - const: usb-cfg
+      - const: utmi-cfg
 
   "#address-cells":
     const: 1

-- 
2.35.3

[PATCH RFC v2 2/4] arm: dts: marvell: armada-38x: add description for usb phys

[Josua Mayer]

Armada 38x has 3x USB-2.0 utmi phys. They are almost identical to the 2x
utmi phys on armada 8k.

Add descriptions for all 3 phy ports.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm/boot/dts/marvell/armada-38x.dtsi | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm/boot/dts/marvell/armada-38x.dtsi b/arch/arm/boot/dts/marvell/armada-38x.dtsi
index 446861b6b17b..701a1c0c19ad 100644
--- a/arch/arm/boot/dts/marvell/armada-38x.dtsi
+++ b/arch/arm/boot/dts/marvell/armada-38x.dtsi
@@ -392,6 +392,11 @@ comphy5: phy@5 {
 				};
 			};
 
+			syscon0: system-controller@18400 {
+				compatible = "syscon", "simple-mfd";
+				reg = <0x18420 0x30>;
+			};
+
 			coreclk: mvebu-sar@18600 {
 				compatible = "marvell,armada-380-core-clock";
 				reg = <0x18600 0x04>;
@@ -580,6 +585,30 @@ ahci0: sata@a8000 {
 				status = "disabled";
 			};
 
+			utmi: utmi@c0000 {
+				compatible = "marvell,a38x-utmi-phy";
+				reg = <0xc0000 0x6000>, <0x18420 4>, <0x18440 12>;
+				reg-names = "utmi", "usb-cfg", "utmi-cfg";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				status = "disabled";
+
+				utmi0: usb-phy@0 {
+					reg = <0>;
+					#phy-cells = <0>;
+				};
+
+				utmi1: usb-phy@1 {
+					reg = <1>;
+					#phy-cells = <0>;
+				};
+
+				utmi2: usb-phy@2 {
+					reg = <2>;
+					#phy-cells = <0>;
+				};
+			};
+
 			bm: bm@c8000 {
 				compatible = "marvell,armada-380-neta-bm";
 				reg = <0xc8000 0xac>;

-- 
2.35.3

[PATCH RFC v2 3/4] phy: mvebu-cp110-utmi: add support for armada-380 utmi phys

[Josua Mayer]

Armada 380 has smilar USB-2.0 PHYs as CP-110. The differences are:
- register base addresses
- gap between port registers
- number of ports: 388 has three, cp110 two
- device-mode mux has bit refers to different ports
- syscon register's base address (offsets identical)
- armada-8k uses syscon for various drivers, a38x not

Differentiation uses of_match_data with distinct compatible strings.

Add support for Armada 380 PHYs by partially restructuting the driver:
- Port register pointers are moved to the per-port private data.
- Add armada-38x-specific compatible string and store enum value in
  of_match_data for differentiation.
- Add support for optional regs usb-cfg and utmi-cfg replacing syscon.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/phy/marvell/phy-mvebu-cp110-utmi.c | 209 +++++++++++++++++++++++------
 1 file changed, 166 insertions(+), 43 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
index 4922a5f3327d..4341923e85bc 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
@@ -19,7 +19,7 @@
 #include <linux/usb/of.h>
 #include <linux/usb/otg.h>
 
-#define UTMI_PHY_PORTS				2
+#define UTMI_PHY_PORTS				3
 
 /* CP110 UTMI register macro definetions */
 #define SYSCON_USB_CFG_REG			0x420
@@ -76,32 +76,44 @@
 #define PLL_LOCK_DELAY_US			10000
 #define PLL_LOCK_TIMEOUT_US			1000000
 
-#define PORT_REGS(p)				((p)->priv->regs + (p)->id * 0x1000)
+enum mvebu_cp110_utmi_type {
+	/* 0 is reserved to avoid clashing with NULL */
+	A380_UTMI = 1,
+	CP110_UTMI = 2,
+};
+
+struct mvebu_cp110_utmi_port;
 
 /**
  * struct mvebu_cp110_utmi - PHY driver data
  *
- * @regs: PHY registers
+ * @regs_usb: USB configuration register
  * @syscon: Regmap with system controller registers
  * @dev: device driver handle
  * @ops: phy ops
+ * @ports: phy object for each port
  */
 struct mvebu_cp110_utmi {
-	void __iomem *regs;
+	void __iomem *regs_usb;
 	struct regmap *syscon;
 	struct device *dev;
 	const struct phy_ops *ops;
+	struct mvebu_cp110_utmi_port *ports[UTMI_PHY_PORTS];
 };
 
 /**
  * struct mvebu_cp110_utmi_port - PHY port data
  *
+ * @regs: PHY registers
+ * @regs_cfg: PHY config register
  * @priv: PHY driver data
  * @id: PHY port ID
  * @dr_mode: PHY connection: USB_DR_MODE_HOST or USB_DR_MODE_PERIPHERAL
  */
 struct mvebu_cp110_utmi_port {
 	struct mvebu_cp110_utmi *priv;
+	void __iomem *regs;
+	void __iomem *regs_cfg;
 	u32 id;
 	enum usb_dr_mode dr_mode;
 };
@@ -118,47 +130,47 @@ static void mvebu_cp110_utmi_port_setup(struct mvebu_cp110_utmi_port *port)
 	 * The crystal used for all platform boards is now 25MHz.
 	 * See the functional specification for details.
 	 */
-	reg = readl(PORT_REGS(port) + UTMI_PLL_CTRL_REG);
+	reg = readl(port->regs + UTMI_PLL_CTRL_REG);
 	reg &= ~(PLL_REFDIV_MASK | PLL_FBDIV_MASK | PLL_SEL_LPFR_MASK);
 	reg |= (PLL_REFDIV_VAL << PLL_REFDIV_OFFSET) |
 	       (PLL_FBDIV_VAL << PLL_FBDIV_OFFSET);
-	writel(reg, PORT_REGS(port) + UTMI_PLL_CTRL_REG);
+	writel(reg, port->regs + UTMI_PLL_CTRL_REG);
 
 	/* Impedance Calibration Threshold Setting */
-	reg = readl(PORT_REGS(port) + UTMI_CAL_CTRL_REG);
+	reg = readl(port->regs + UTMI_CAL_CTRL_REG);
 	reg &= ~IMPCAL_VTH_MASK;
 	reg |= IMPCAL_VTH_VAL << IMPCAL_VTH_OFFSET;
-	writel(reg, PORT_REGS(port) + UTMI_CAL_CTRL_REG);
+	writel(reg, port->regs + UTMI_CAL_CTRL_REG);
 
 	/* Set LS TX driver strength coarse control */
-	reg = readl(PORT_REGS(port) + UTMI_TX_CH_CTRL_REG);
+	reg = readl(port->regs + UTMI_TX_CH_CTRL_REG);
 	reg &= ~TX_AMP_MASK;
 	reg |= TX_AMP_VAL << TX_AMP_OFFSET;
-	writel(reg, PORT_REGS(port) + UTMI_TX_CH_CTRL_REG);
+	writel(reg, port->regs + UTMI_TX_CH_CTRL_REG);
 
 	/* Disable SQ and enable analog squelch detect */
-	reg = readl(PORT_REGS(port) + UTMI_RX_CH_CTRL0_REG);
+	reg = readl(port->regs + UTMI_RX_CH_CTRL0_REG);
 	reg &= ~SQ_DET_EN;
 	reg |= SQ_ANA_DTC_SEL;
-	writel(reg, PORT_REGS(port) + UTMI_RX_CH_CTRL0_REG);
+	writel(reg, port->regs + UTMI_RX_CH_CTRL0_REG);
 
 	/*
 	 * Set External squelch calibration number and
 	 * enable the External squelch calibration
 	 */
-	reg = readl(PORT_REGS(port) + UTMI_RX_CH_CTRL1_REG);
+	reg = readl(port->regs + UTMI_RX_CH_CTRL1_REG);
 	reg &= ~SQ_AMP_CAL_MASK;
 	reg |= (SQ_AMP_CAL_VAL << SQ_AMP_CAL_OFFSET) | SQ_AMP_CAL_EN;
-	writel(reg, PORT_REGS(port) + UTMI_RX_CH_CTRL1_REG);
+	writel(reg, port->regs + UTMI_RX_CH_CTRL1_REG);
 
 	/*
 	 * Set Control VDAT Reference Voltage - 0.325V and
 	 * Control VSRC Reference Voltage - 0.6V
 	 */
-	reg = readl(PORT_REGS(port) + UTMI_CHGDTC_CTRL_REG);
+	reg = readl(port->regs + UTMI_CHGDTC_CTRL_REG);
 	reg &= ~(VDAT_MASK | VSRC_MASK);
 	reg |= (VDAT_VAL << VDAT_OFFSET) | (VSRC_VAL << VSRC_OFFSET);
-	writel(reg, PORT_REGS(port) + UTMI_CHGDTC_CTRL_REG);
+	writel(reg, port->regs + UTMI_CHGDTC_CTRL_REG);
 }
 
 static int mvebu_cp110_utmi_phy_power_off(struct phy *phy)
@@ -166,22 +178,38 @@ static int mvebu_cp110_utmi_phy_power_off(struct phy *phy)
 	struct mvebu_cp110_utmi_port *port = phy_get_drvdata(phy);
 	struct mvebu_cp110_utmi *utmi = port->priv;
 	int i;
+	int reg;
 
 	/* Power down UTMI PHY port */
-	regmap_clear_bits(utmi->syscon, SYSCON_UTMI_CFG_REG(port->id),
-			  UTMI_PHY_CFG_PU_MASK);
+	if (!IS_ERR(port->regs_cfg)) {
+		reg = readl(port->regs_cfg);
+		reg &= ~(UTMI_PHY_CFG_PU_MASK);
+		writel(reg, port->regs_cfg);
+	} else
+		regmap_clear_bits(utmi->syscon, SYSCON_UTMI_CFG_REG(port->id),
+				  UTMI_PHY_CFG_PU_MASK);
 
 	for (i = 0; i < UTMI_PHY_PORTS; i++) {
-		int test = regmap_test_bits(utmi->syscon,
-					    SYSCON_UTMI_CFG_REG(i),
-					    UTMI_PHY_CFG_PU_MASK);
+		if (!utmi->ports[i])
+			continue;
+
+		if (!IS_ERR(utmi->ports[i]->regs_cfg))
+			reg = readl(utmi->ports[i]->regs_cfg);
+		else
+			regmap_read(utmi->syscon, SYSCON_UTMI_CFG_REG(i), &reg);
+		int test = reg & UTMI_PHY_CFG_PU_MASK;
 		/* skip PLL shutdown if there are active UTMI PHY ports */
 		if (test != 0)
 			return 0;
 	}
 
 	/* PLL Power down if all UTMI PHYs are down */
-	regmap_clear_bits(utmi->syscon, SYSCON_USB_CFG_REG, USB_CFG_PLL_MASK);
+	if (!IS_ERR(utmi->regs_usb)) {
+		reg = readl(utmi->regs_usb);
+		reg &= ~(USB_CFG_PLL_MASK);
+		writel(reg, utmi->regs_usb);
+	} else
+		regmap_clear_bits(utmi->syscon, SYSCON_USB_CFG_REG, USB_CFG_PLL_MASK);
 
 	return 0;
 }
@@ -191,8 +219,15 @@ static int mvebu_cp110_utmi_phy_power_on(struct phy *phy)
 	struct mvebu_cp110_utmi_port *port = phy_get_drvdata(phy);
 	struct mvebu_cp110_utmi *utmi = port->priv;
 	struct device *dev = &phy->dev;
+	const void *match;
+	enum mvebu_cp110_utmi_type type;
 	int ret;
 	u32 reg;
+	u32 sel;
+
+	match = device_get_match_data(utmi->dev);
+	if (match)
+		type = (enum mvebu_cp110_utmi_type)(uintptr_t)match;
 
 	/* It is necessary to power off UTMI before configuration */
 	ret = mvebu_cp110_utmi_phy_power_off(phy);
@@ -208,16 +243,45 @@ static int mvebu_cp110_utmi_phy_power_on(struct phy *phy)
 	 * to UTMI0 or to UTMI1 PHY port, but not to both.
 	 */
 	if (port->dr_mode == USB_DR_MODE_PERIPHERAL) {
-		regmap_update_bits(utmi->syscon, SYSCON_USB_CFG_REG,
-				   USB_CFG_DEVICE_EN_MASK | USB_CFG_DEVICE_MUX_MASK,
-				   USB_CFG_DEVICE_EN_MASK |
-				   (port->id << USB_CFG_DEVICE_MUX_OFFSET));
+		switch (type) {
+		case A380_UTMI:
+			/*
+			 * A380 muxes between ports 0/2:
+			 * - 0: Device mode on Port 2
+			 * - 1: Device mode on Port 0
+			 */
+			if (port->id == 1)
+				return -EINVAL;
+			sel = !!(port->id == 0);
+			break;
+		case CP110_UTMI:
+			/*
+			 * CP110 muxes between ports 0/1:
+			 * - 0: Device mode on Port 0
+			 * - 1: Device mode on Port 1
+			 */
+			sel = port->id;
+			break;
+		default:
+			return -EINVAL;
+		}
+		if (!IS_ERR(utmi->regs_usb)) {
+			reg = readl(utmi->regs_usb);
+			reg &= ~(USB_CFG_DEVICE_EN_MASK | USB_CFG_DEVICE_MUX_MASK);
+			reg |= USB_CFG_DEVICE_EN_MASK;
+			reg |= (sel << USB_CFG_DEVICE_MUX_OFFSET);
+			writel(reg, utmi->regs_usb);
+		} else
+			regmap_update_bits(utmi->syscon, SYSCON_USB_CFG_REG,
+					   USB_CFG_DEVICE_EN_MASK | USB_CFG_DEVICE_MUX_MASK,
+					   USB_CFG_DEVICE_EN_MASK |
+					   (sel << USB_CFG_DEVICE_MUX_OFFSET));
 	}
 
 	/* Set Test suspendm mode and enable Test UTMI select */
-	reg = readl(PORT_REGS(port) + UTMI_CTRL_STATUS0_REG);
+	reg = readl(port->regs + UTMI_CTRL_STATUS0_REG);
 	reg |= SUSPENDM | TEST_SEL;
-	writel(reg, PORT_REGS(port) + UTMI_CTRL_STATUS0_REG);
+	writel(reg, port->regs + UTMI_CTRL_STATUS0_REG);
 
 	/* Wait for UTMI power down */
 	mdelay(1);
@@ -226,16 +290,21 @@ static int mvebu_cp110_utmi_phy_power_on(struct phy *phy)
 	mvebu_cp110_utmi_port_setup(port);
 
 	/* Power UP UTMI PHY */
-	regmap_set_bits(utmi->syscon, SYSCON_UTMI_CFG_REG(port->id),
-			UTMI_PHY_CFG_PU_MASK);
+	if (!IS_ERR(port->regs_cfg)) {
+		reg = readl(port->regs_cfg);
+		reg |= UTMI_PHY_CFG_PU_MASK;
+		writel(reg, port->regs_cfg);
+	} else
+		regmap_set_bits(utmi->syscon, SYSCON_UTMI_CFG_REG(port->id),
+				UTMI_PHY_CFG_PU_MASK);
 
 	/* Disable Test UTMI select */
-	reg = readl(PORT_REGS(port) + UTMI_CTRL_STATUS0_REG);
+	reg = readl(port->regs + UTMI_CTRL_STATUS0_REG);
 	reg &= ~TEST_SEL;
-	writel(reg, PORT_REGS(port) + UTMI_CTRL_STATUS0_REG);
+	writel(reg, port->regs + UTMI_CTRL_STATUS0_REG);
 
 	/* Wait for impedance calibration */
-	ret = readl_poll_timeout(PORT_REGS(port) + UTMI_CAL_CTRL_REG, reg,
+	ret = readl_poll_timeout(port->regs + UTMI_CAL_CTRL_REG, reg,
 				 reg & IMPCAL_DONE,
 				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
 	if (ret) {
@@ -244,7 +313,7 @@ static int mvebu_cp110_utmi_phy_power_on(struct phy *phy)
 	}
 
 	/* Wait for PLL calibration */
-	ret = readl_poll_timeout(PORT_REGS(port) + UTMI_CAL_CTRL_REG, reg,
+	ret = readl_poll_timeout(port->regs + UTMI_CAL_CTRL_REG, reg,
 				 reg & PLLCAL_DONE,
 				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
 	if (ret) {
@@ -253,7 +322,7 @@ static int mvebu_cp110_utmi_phy_power_on(struct phy *phy)
 	}
 
 	/* Wait for PLL ready */
-	ret = readl_poll_timeout(PORT_REGS(port) + UTMI_PLL_CTRL_REG, reg,
+	ret = readl_poll_timeout(port->regs + UTMI_PLL_CTRL_REG, reg,
 				 reg & PLL_RDY,
 				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
 	if (ret) {
@@ -262,7 +331,12 @@ static int mvebu_cp110_utmi_phy_power_on(struct phy *phy)
 	}
 
 	/* PLL Power up */
-	regmap_set_bits(utmi->syscon, SYSCON_USB_CFG_REG, USB_CFG_PLL_MASK);
+	if (!IS_ERR(utmi->regs_usb)) {
+		reg = readl(utmi->regs_usb);
+		reg |= USB_CFG_PLL_MASK;
+		writel(reg, utmi->regs_usb);
+	} else
+		regmap_set_bits(utmi->syscon, SYSCON_USB_CFG_REG, USB_CFG_PLL_MASK);
 
 	return 0;
 }
@@ -274,7 +348,8 @@ static const struct phy_ops mvebu_cp110_utmi_phy_ops = {
 };
 
 static const struct of_device_id mvebu_cp110_utmi_of_match[] = {
-	{ .compatible = "marvell,cp110-utmi-phy" },
+	{ .compatible = "marvell,a38x-utmi-phy", .data = (void *)A380_UTMI },
+	{ .compatible = "marvell,cp110-utmi-phy", .data = (void *)CP110_UTMI },
 	{},
 };
 MODULE_DEVICE_TABLE(of, mvebu_cp110_utmi_of_match);
@@ -285,6 +360,10 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
 	struct mvebu_cp110_utmi *utmi;
 	struct phy_provider *provider;
 	struct device_node *child;
+	void __iomem *regs_utmi;
+	void __iomem *regs_utmi_cfg;
+	const void *match;
+	enum mvebu_cp110_utmi_type type;
 	u32 usb_devices = 0;
 
 	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
@@ -293,18 +372,44 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
 
 	utmi->dev = dev;
 
+	match = device_get_match_data(dev);
+	if (match)
+		type = (enum mvebu_cp110_utmi_type)(uintptr_t)match;
+
+	/* Get UTMI memory region */
+	regs_utmi = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(regs_utmi)) {
+		dev_err(dev, "Failed to map utmi regs\n");
+		return PTR_ERR(regs_utmi);
+	}
+
+	/* Get usb config region */
+	utmi->regs_usb = devm_platform_ioremap_resource_byname(pdev, "usb-cfg");
+	if (IS_ERR(utmi->regs_usb) && PTR_ERR(utmi->regs_usb) != -EINVAL) {
+		dev_err(dev, "Failed to map usb config regs\n");
+		return PTR_ERR(utmi->regs_usb);
+	}
+
+	/* Get utmi config region */
+	regs_utmi_cfg = devm_platform_ioremap_resource_byname(pdev, "utmi-cfg");
+	if (IS_ERR(regs_utmi_cfg) && PTR_ERR(regs_utmi_cfg) != -EINVAL) {
+		dev_err(dev, "Failed to map usb config regs\n");
+		return PTR_ERR(regs_utmi_cfg);
+	}
+
 	/* Get system controller region */
 	utmi->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
 						       "marvell,system-controller");
-	if (IS_ERR(utmi->syscon)) {
-		dev_err(dev, "Missing UTMI system controller\n");
+	if (IS_ERR(utmi->syscon) && PTR_ERR(utmi->syscon) != -ENODEV) {
+		dev_err(dev, "Failed to get system controller\n");
 		return PTR_ERR(utmi->syscon);
 	}
 
-	/* Get UTMI memory region */
-	utmi->regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(utmi->regs))
-		return PTR_ERR(utmi->regs);
+	if (IS_ERR(utmi->syscon) &&
+	    (IS_ERR(utmi->regs_usb) || IS_ERR(regs_utmi_cfg))) {
+		dev_err(dev, "Missing utmi system controller or config regs");
+		return -EINVAL;
+	}
 
 	for_each_available_child_of_node(dev->of_node, child) {
 		struct mvebu_cp110_utmi_port *port;
@@ -326,6 +431,24 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
 			return -ENOMEM;
 		}
 
+		utmi->ports[port_id] = port;
+
+		/* Get port memory region */
+		switch (type) {
+		case A380_UTMI:
+			port->regs = regs_utmi + port_id * 0x1000;
+			break;
+		case CP110_UTMI:
+			port->regs = regs_utmi + port_id * 0x2000;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		/* assign utmi cfg reg */
+		if (!IS_ERR(regs_utmi_cfg))
+			port->regs_cfg = regs_utmi_cfg + port_id * 4;
+
 		port->dr_mode = of_usb_get_dr_mode_by_phy(child, -1);
 		if ((port->dr_mode != USB_DR_MODE_HOST) &&
 		    (port->dr_mode != USB_DR_MODE_PERIPHERAL)) {

-- 
2.35.3

[PATCH RFC v2 4/4] arm: dts: marvell: armada-388-clearfog: add description for usb phys

[Josua Mayer]

Clearfog Base and Pro use three sets of USB-2.0 signals going to various
connectors. Only two were enabled so far without links to their phys.
Further there were some minor issues with usb port descriptions.

Firstly add references for each phy to the corresponding controller
nodes. At the same time redeclarations of controller nodes are replaced
with references to labels in armada-38x.dtsi.

Secondly enable USB-2.0 controller routed to Clearfog Base M.2 /
Clearfog Pro mPCIe connectors.

FInally add missing dr_mode properties to stop utmi phy driver from
complaining.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 .../boot/dts/marvell/armada-388-clearfog-base.dts  |  8 ++++++
 arch/arm/boot/dts/marvell/armada-388-clearfog.dts  |  8 ++++++
 arch/arm/boot/dts/marvell/armada-388-clearfog.dtsi | 30 ++++++++++++++--------
 3 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/marvell/armada-388-clearfog-base.dts b/arch/arm/boot/dts/marvell/armada-388-clearfog-base.dts
index f7daa3bc707e..09ba024e432e 100644
--- a/arch/arm/boot/dts/marvell/armada-388-clearfog-base.dts
+++ b/arch/arm/boot/dts/marvell/armada-388-clearfog-base.dts
@@ -66,3 +66,11 @@ rear_button_pins: rear-button-pins {
 		marvell,function = "gpio";
 	};
 };
+
+/* SRDS #4 - USB-2.0/3.0 Host, M.2 */
+&usb3_0 {
+	phys = <&utmi1>;
+	phy-names = "utmi";
+	dr_mode = "host";
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/marvell/armada-388-clearfog.dts b/arch/arm/boot/dts/marvell/armada-388-clearfog.dts
index 09bf2e6d4ed0..4f5bb5867f20 100644
--- a/arch/arm/boot/dts/marvell/armada-388-clearfog.dts
+++ b/arch/arm/boot/dts/marvell/armada-388-clearfog.dts
@@ -182,3 +182,11 @@ &spi1 {
 	 */
 	pinctrl-0 = <&spi1_pins &clearfog_spi1_cs_pins &mikro_spi_pins>;
 };
+
+/* USB-2.0 Host, CON2 - nearest CPU */
+&usb3_0 {
+	phys = <&utmi1>;
+	phy-names = "utmi";
+	dr_mode = "host";
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/marvell/armada-388-clearfog.dtsi b/arch/arm/boot/dts/marvell/armada-388-clearfog.dtsi
index f8a06ae4a3c9..0497fe13f56d 100644
--- a/arch/arm/boot/dts/marvell/armada-388-clearfog.dtsi
+++ b/arch/arm/boot/dts/marvell/armada-388-clearfog.dtsi
@@ -51,16 +51,6 @@ sdhci@d8000 {
 				vmmc-supply = <&reg_3p3v>;
 				wp-inverted;
 			};
-
-			usb@58000 {
-				/* CON3, nearest  power. */
-				status = "okay";
-			};
-
-			usb3@f8000 {
-				/* CON7 */
-				status = "okay";
-			};
 		};
 
 		pcie {
@@ -243,3 +233,23 @@ &uart1 {
 	pinctrl-names = "default";
 	status = "okay";
 };
+
+/* USB-2.0 Host, CON3 - nearest power */
+&usb0 {
+	phys = <&utmi0>;
+	phy-names = "utmi";
+	dr_mode = "host";
+	status = "okay";
+};
+
+/* SRDS #3 - USB-2.0/3.0 Host, Type-A connector */
+&usb3_1 {
+	phys = <&utmi2>;
+	phy-names = "utmi";
+	dr_mode = "host";
+	status = "okay";
+};
+
+&utmi {
+	status = "okay";
+};

-- 
2.35.3

Re: [PATCH RFC v2 1/4] dt-bindings: phy: cp110-utmi-phy: add compatible string for armada-38x

[Rob Herring (Arm)]

On Tue, 16 Jul 2024 22:52:37 +0200, Josua Mayer wrote:
> Armada 38x USB-2.0 PHYs are similar to Armada 8K (CP110) and can be
> supported by the same driver with small differences.
> 
> Add new compatible string for armada-38x variant of utmi phy.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  .../bindings/phy/marvell,armada-cp110-utmi-phy.yaml       | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-phy.example.dtb: utmi@580000: reg: [[5767168, 8192]] is too short
	from schema $id: http://devicetree.org/schemas/phy/marvell,armada-cp110-utmi-phy.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240716-a38x-utmi-phy-v2-1-dae3a9c6ca3e@solid-run.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH RFC v2 2/4] arm: dts: marvell: armada-38x: add description for usb phys

[Krzysztof Kozlowski]

On 16/07/2024 22:52, Josua Mayer wrote:
> Armada 38x has 3x USB-2.0 utmi phys. They are almost identical to the 2x
> utmi phys on armada 8k.
> 
> Add descriptions for all 3 phy ports.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  arch/arm/boot/dts/marvell/armada-38x.dtsi | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/marvell/armada-38x.dtsi b/arch/arm/boot/dts/marvell/armada-38x.dtsi
> index 446861b6b17b..701a1c0c19ad 100644
> --- a/arch/arm/boot/dts/marvell/armada-38x.dtsi
> +++ b/arch/arm/boot/dts/marvell/armada-38x.dtsi
> @@ -392,6 +392,11 @@ comphy5: phy@5 {
>  				};
>  			};
>  
> +			syscon0: system-controller@18400 {
> +				compatible = "syscon", "simple-mfd";

That's not a valid pair. They cannot be alone.



Best regards,
Krzysztof

Re: [PATCH RFC v2 2/4] arm: dts: marvell: armada-38x: add description for usb phys

[Josua Mayer]

Am 17.07.24 um 14:04 schrieb Krzysztof Kozlowski:
> On 16/07/2024 22:52, Josua Mayer wrote:
>> Armada 38x has 3x USB-2.0 utmi phys. They are almost identical to the 2x
>> utmi phys on armada 8k.
>>
>> Add descriptions for all 3 phy ports.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
>>  arch/arm/boot/dts/marvell/armada-38x.dtsi | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/marvell/armada-38x.dtsi b/arch/arm/boot/dts/marvell/armada-38x.dtsi
>> index 446861b6b17b..701a1c0c19ad 100644
>> --- a/arch/arm/boot/dts/marvell/armada-38x.dtsi
>> +++ b/arch/arm/boot/dts/marvell/armada-38x.dtsi
>> @@ -392,6 +392,11 @@ comphy5: phy@5 {
>>  				};
>>  			};
>>  
>> +			syscon0: system-controller@18400 {
>> +				compatible = "syscon", "simple-mfd";
> That's not a valid pair. They cannot be alone.
Curious! I have seen it in armada-cp11x.dtsi.

However this is my mistake,
especially since with v2 the syscon node is not actually used.

sincerely
Josua Mayer

Re: [PATCH RFC v2 2/4] arm: dts: marvell: armada-38x: add description for usb phys

[Krzysztof Kozlowski]

On 20/07/2024 15:19, Josua Mayer wrote:
> Am 17.07.24 um 14:04 schrieb Krzysztof Kozlowski:
>> On 16/07/2024 22:52, Josua Mayer wrote:
>>> Armada 38x has 3x USB-2.0 utmi phys. They are almost identical to the 2x
>>> utmi phys on armada 8k.
>>>
>>> Add descriptions for all 3 phy ports.
>>>
>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>> ---
>>>  arch/arm/boot/dts/marvell/armada-38x.dtsi | 29 +++++++++++++++++++++++++++++
>>>  1 file changed, 29 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/marvell/armada-38x.dtsi b/arch/arm/boot/dts/marvell/armada-38x.dtsi
>>> index 446861b6b17b..701a1c0c19ad 100644
>>> --- a/arch/arm/boot/dts/marvell/armada-38x.dtsi
>>> +++ b/arch/arm/boot/dts/marvell/armada-38x.dtsi
>>> @@ -392,6 +392,11 @@ comphy5: phy@5 {
>>>  				};
>>>  			};
>>>  
>>> +			syscon0: system-controller@18400 {
>>> +				compatible = "syscon", "simple-mfd";
>> That's not a valid pair. They cannot be alone.
> Curious! I have seen it in armada-cp11x.dtsi.

Old code, I don't think anyone is working on armada and other Marvell
chips, so by copying old code you will copy bugs or wrong designs. I can
only suggest to work on recent platform where such oddities are fixed...

Best regards,
Krzysztof

Re: [PATCH RFC v2 2/4] arm: dts: marvell: armada-38x: add description for usb phys

[Andrew Lunn]

On Sat, Jul 20, 2024 at 08:23:09PM +0200, Krzysztof Kozlowski wrote:
> On 20/07/2024 15:19, Josua Mayer wrote:
> > Am 17.07.24 um 14:04 schrieb Krzysztof Kozlowski:
> >> On 16/07/2024 22:52, Josua Mayer wrote:
> >>> Armada 38x has 3x USB-2.0 utmi phys. They are almost identical to the 2x
> >>> utmi phys on armada 8k.
> >>>
> >>> Add descriptions for all 3 phy ports.
> >>>
> >>> Signed-off-by: Josua Mayer <josua@solid-run.com>
> >>> ---
> >>>  arch/arm/boot/dts/marvell/armada-38x.dtsi | 29 +++++++++++++++++++++++++++++
> >>>  1 file changed, 29 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/marvell/armada-38x.dtsi b/arch/arm/boot/dts/marvell/armada-38x.dtsi
> >>> index 446861b6b17b..701a1c0c19ad 100644
> >>> --- a/arch/arm/boot/dts/marvell/armada-38x.dtsi
> >>> +++ b/arch/arm/boot/dts/marvell/armada-38x.dtsi
> >>> @@ -392,6 +392,11 @@ comphy5: phy@5 {
> >>>  				};
> >>>  			};
> >>>  
> >>> +			syscon0: system-controller@18400 {
> >>> +				compatible = "syscon", "simple-mfd";
> >> That's not a valid pair. They cannot be alone.
> > Curious! I have seen it in armada-cp11x.dtsi.
> 
> Old code, I don't think anyone is working on armada and other Marvell
> chips, so by copying old code you will copy bugs or wrong designs. I can
> only suggest to work on recent platform where such oddities are fixed...

As you say, these are old platform. I'm not sure they are actually
buggy or wrong design, it is more that current day best practice is
different to the best practice back then. This was from before the
time linting tools and .yaml existed for DT.

The kirkwood-b3.dts i wrote in 2013 still works, my NAS box gets its
kernel updated about once a year, and a new disk about every 8
years. These systems can keep running for a long time. We have to
accept there will be legacy DT.

       Andrew