diff mbox series

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

Message ID 20240715-a38x-utmi-phy-v1-2-d57250f53cf2@solid-run.com (mailing list archive)
State New, archived
Headers show
Series phy: mvebu-cp110-utmi: add support for armada-380 utmi phys | expand

Commit Message

Josua Mayer July 15, 2024, 5:47 p.m. UTC
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(+)

Comments

Andrew Lunn July 15, 2024, 6:12 p.m. UTC | #1
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
Josua Mayer July 16, 2024, 8:16 a.m. UTC | #2
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
Josua Mayer July 16, 2024, 12:55 p.m. UTC | #3
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";
};
diff mbox series

Patch

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>;