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

[PATCH RFC 0/2] 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>
---
Josua Mayer (2):
      phy: mvebu-cp110-utmi: add support for armada-380 utmi phys
      arm: dts: marvell: armada-38x: add description for usb phys

 arch/arm/boot/dts/marvell/armada-38x.dtsi  | 30 +++++++++
 drivers/phy/marvell/phy-mvebu-cp110-utmi.c | 97 +++++++++++++++++++++++-------
 2 files changed, 105 insertions(+), 22 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240715-a38x-utmi-phy-02e8059afe35

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

[PATCH RFC 1/2] 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)

Differentiation uses of_match_data with distinct compatible strings.

Add support for Armada 380 PHYs by introducing a per-port regs pointer,
and add extra logic mapping port id to device-mode mux register value.

This driver is not immediately usable on Armada 38x as it relies on
syscon framework for access to shared registers.
While all CP110 based designs declare a syscon node in device-tree,
Armada 38x has various drivers claiming parts of the respective area.

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

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
index 4922a5f3327d..29ee73b6d8b5 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
@@ -76,7 +76,11 @@
 #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 - PHY driver data
@@ -104,6 +108,7 @@ struct mvebu_cp110_utmi_port {
 	struct mvebu_cp110_utmi *priv;
 	u32 id;
 	enum usb_dr_mode dr_mode;
+	void __iomem *regs;
 };
 
 static void mvebu_cp110_utmi_port_setup(struct mvebu_cp110_utmi_port *port)
@@ -118,47 +123,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)
@@ -191,8 +196,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 = of_device_get_match_data(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 +220,38 @@ 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) {
+		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;
+		}
 		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));
+				   (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);
@@ -230,12 +264,12 @@ static int mvebu_cp110_utmi_phy_power_on(struct phy *phy)
 			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 +278,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 +287,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) {
@@ -274,7 +308,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,armada-380-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 +320,8 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
 	struct mvebu_cp110_utmi *utmi;
 	struct phy_provider *provider;
 	struct device_node *child;
+	const void *match;
+	enum mvebu_cp110_utmi_type type;
 	u32 usb_devices = 0;
 
 	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
@@ -293,6 +330,10 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
 
 	utmi->dev = dev;
 
+	match = of_device_get_match_data(dev);
+	if (match)
+		type = (enum mvebu_cp110_utmi_type)(uintptr_t)match;
+
 	/* Get system controller region */
 	utmi->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
 						       "marvell,system-controller");
@@ -326,6 +367,18 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
 			return -ENOMEM;
 		}
 
+		/* Get port memory region */
+		switch (type) {
+		case A380_UTMI:
+			port->regs = utmi->regs + port_id * 0x1000;
+			break;
+		case CP110_UTMI:
+			port->regs = utmi->regs + port_id * 0x2000;
+			break;
+		default:
+			return -EINVAL;
+		}
+
 		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

Re: [PATCH RFC 1/2] phy: mvebu-cp110-utmi: add support for armada-380 utmi phys

[Andrew Lunn]

> @@ -191,8 +196,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 = of_device_get_match_data(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 +220,38 @@ 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) {
> +		switch (type) {

Just looking at this, i'm surprised there is not a warning about
type possibly being uninitialled. 

> @@ -285,6 +320,8 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>  	struct mvebu_cp110_utmi *utmi;
>  	struct phy_provider *provider;
>  	struct device_node *child;
> +	const void *match;
> +	enum mvebu_cp110_utmi_type type;
>  	u32 usb_devices = 0;
>  
>  	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
> @@ -293,6 +330,10 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>  
>  	utmi->dev = dev;
>  
> +	match = of_device_get_match_data(dev);
> +	if (match)
> +		type = (enum mvebu_cp110_utmi_type)(uintptr_t)match;
> +
>  	/* Get system controller region */
>  	utmi->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
>  						       "marvell,system-controller");
> @@ -326,6 +367,18 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>  			return -ENOMEM;
>  		}
>  
> +		/* Get port memory region */
> +		switch (type) {

Same here.

	Andrew

Re: [PATCH RFC 1/2] phy: mvebu-cp110-utmi: add support for armada-380 utmi phys

[Josua Mayer]

Am 15.07.24 um 20:05 schrieb Andrew Lunn:
>> @@ -191,8 +196,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 = of_device_get_match_data(dev);
Should be device_get_match_data?
>> +	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 +220,38 @@ 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) {
>> +		switch (type) {
> Just looking at this, i'm surprised there is not a warning about
> type possibly being uninitialled. 
Curious indeed. However I have not seen any compiler warnings
for uninitialized int (enum) recently.

I copied the pattern from drivers/gpu/drm/tiny/repaper.c,
there however is always an else case.

>
>> @@ -285,6 +320,8 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>>  	struct mvebu_cp110_utmi *utmi;
>>  	struct phy_provider *provider;
>>  	struct device_node *child;
>> +	const void *match;
>> +	enum mvebu_cp110_utmi_type type;
>>  	u32 usb_devices = 0;
>>  
>>  	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
>> @@ -293,6 +330,10 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>>  
>>  	utmi->dev = dev;
>>  
>> +	match = of_device_get_match_data(dev);
>> +	if (match)
>> +		type = (enum mvebu_cp110_utmi_type)(uintptr_t)match;
>> +
>>  	/* Get system controller region */
>>  	utmi->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
>>  						       "marvell,system-controller");
>> @@ -326,6 +367,18 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>>  			return -ENOMEM;
>>  		}
>>  
>> +		/* Get port memory region */
>> +		switch (type) {
> Same here.
>
> 	Andrew

[PATCH RFC 2/2] 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.

Also add a syscon node covering just the usb configuration registers.
Armada 8K have a syscon node covering configuration registers for
various functions including pinmux, woith dirvers using syscon framework
for register access.

Armada 388 has various drivers directly claiming some of those
configuration registers. Hence a similar syscon node would compete for
resources with these drivers.

This patch-set is marked RFC to figure out a solution. I have some
ideas:

1. Can syscon have holes, i.e. facilitate consumer drivers accessing
   certain offsets only?

2. Declare a tiny syscon (see this patch) covering just the area used by
   utmi phy driver: This impacts driver access offsets - can those be
   hard-coded - or is there a mechanism in device-tree?
   E.g. marvell,system-controller = <&syscon any-poffset-here>?

3. utmi phy driver access just three registers using syscon: all-ports
   power-up (probably enables clocks), device-mode mux, per-port power-up.

   Assign these registers individually to the phy device-node, and
   implement access in driver when syscon is not available.

   If this is preferred, which dt property should s[ecify their address?
   reg, ranges, ...?

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

diff --git a/arch/arm/boot/dts/marvell/armada-38x.dtsi b/arch/arm/boot/dts/marvell/armada-38x.dtsi
index 446861b6b17b..5cf9449162b1 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,31 @@ ahci0: sata@a8000 {
 				status = "disabled";
 			};
 
+			utmi: utmi@c0000 {
+				compatible = "marvell,armada-380-utmi-phy";
+				reg = <0xc0000 0x6000>;
+				ranges = <0x18420>, <0x00018440>, <0x00018444>, <0x00018448>;
+				marvell,system-controller = <&syscon0>;
+				#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

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

[Andrew Lunn]

On Mon, Jul 15, 2024 at 07:47:30PM +0200, 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.
> 
> Also add a syscon node covering just the usb configuration registers.
> Armada 8K have a syscon node covering configuration registers for
> various functions including pinmux, woith dirvers using syscon framework

woith -> with

> for register access.
> 
> Armada 388 has various drivers directly claiming some of those
> configuration registers. Hence a similar syscon node would compete for
> resources with these drivers.

Do these drivers make exclusive use of these registers? Or are
multiple drivers using the same registers and you need something to
mediate in order to have atomic access?

In the old days, a register space could be mapped into multiple
drivers, so long as all the drivers agreed to it. There are probably
orion5x, kirkwood era mvebu drivers doing this.
 
	Andrew

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

[Josua Mayer]

Am 15.07.24 um 20:12 schrieb Andrew Lunn:
> On Mon, Jul 15, 2024 at 07:47:30PM +0200, 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.
>>
>> Also add a syscon node covering just the usb configuration registers.
>> Armada 8K have a syscon node covering configuration registers for
>> various functions including pinmux, woith dirvers using syscon framework
> woith -> with
>
>> for register access.
>>
>> Armada 388 has various drivers directly claiming some of those
>> configuration registers. Hence a similar syscon node would compete for
>> resources with these drivers.
> Do these drivers make exclusive use of these registers? Or are
> multiple drivers using the same registers and you need something to
> mediate in order to have atomic access?

On Armada 38x these drivers exclusively use their own registers.

I suspect armada  8k was trying to avoid declaring lots of tiny ranges,
by using a syscon node.

The syscon registers accessed by cp110-utmi driver are exclusively
for usb and not accessed elsewhere.

>
> In the old days, a register space could be mapped into multiple
> drivers, so long as all the drivers agreed to it. There are probably
> orion5x, kirkwood era mvebu drivers doing this.
>  
> 	Andrew

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

[Josua Mayer]

Am 15.07.24 um 19:47 schrieb Josua Mayer:
> 3. utmi phy driver access just three registers using syscon: all-ports
>    power-up (probably enables clocks), device-mode mux, per-port power-up.
>
>    Assign these registers individually to the phy device-node, and
>    implement access in driver when syscon is not available.
>
>    If this is preferred, which dt property should s[ecify their address?
>    reg, ranges, ...?

I think I have my answer, with reg-names it seems manageable -
please see the example below:

            utmi: utmi@c0000 {
                compatible = "marvell,armada-380-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>;
                };
            };

If registers named "usb-cfg" and "utmi-cfg" are given, the driver can be extended
to optionally use those.

I have tested on armada-388-clearfog-pro,
and will send a v2 after tidying up my changes:

/* USB-2.0 Host, CON3 - nearest power */
&usb0 {
    phys = <&utmi0>;
    phy-names = "utmi";
    status = "okay";
};

/* USB-2.0 Host, CON2 - nearest CPU */
&usb3_0 {
    phys = <&utmi1>;
    phy-names = "utmi";
    status = "okay";
};

/* SRDS #3 - USB-2.0/3.0 Host, Type-A connector */
&usb3_1 {
    phys = <&utmi2>;
    phy-names = "utmi";
    status = "okay";
};

&utmi {
    status = "okay";
};