diff mbox series

[2/2] arm/arm64: dts: Add MV88E6393X to CN9130-CRB device tree

Message ID 20211007230619.957016-3-chris.packham@alliedtelesis.co.nz (mailing list archive)
State New, archived
Headers show
Series arm/arm64: dts: Enable more network hardware | expand

Commit Message

Chris Packham Oct. 7, 2021, 11:06 p.m. UTC
The CN9130-CRB boards have a MV88E6393X switch connected to eth0.  Add
the necessary dts nodes and properties for this.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

This is taken from the Marvell SDK. I've re-ordered the port entries to
be in ascending order.

 arch/arm64/boot/dts/marvell/cn9130-crb.dtsi | 125 ++++++++++++++++++++
 1 file changed, 125 insertions(+)

Comments

Andrew Lunn Oct. 7, 2021, 11:51 p.m. UTC | #1
On Fri, Oct 08, 2021 at 12:06:19PM +1300, Chris Packham wrote:
> The CN9130-CRB boards have a MV88E6393X switch connected to eth0.  Add
> the necessary dts nodes and properties for this.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 
> This is taken from the Marvell SDK. I've re-ordered the port entries to
> be in ascending order.
> 
>  arch/arm64/boot/dts/marvell/cn9130-crb.dtsi | 125 ++++++++++++++++++++
>  1 file changed, 125 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi b/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
> index e7918f325646..171f7394948e 100644
> --- a/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
> +++ b/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
> @@ -185,6 +185,131 @@ &cp0_mdio {
>  	phy0: ethernet-phy@0 {
>  		reg = <0>;
>  	};
> +
> +	switch6: switch0@6 {
> +		/* Actual device is MV88E6393X */
> +		compatible = "marvell,mv88e6190";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <6>;

Is the interrupt output connected to a GPIO?

> +
> +		dsa,member = <0 0>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				label = "notused-port0";
> +				phy-mode = "10gbase-kr";
> +				status = "disabled";

What is meant by not used? Does it go to a header? Is it not wired at
all? You don't need to list a port if it is not actually used. So
maybe you just want to delete this port all together?

> +
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				label = "wan1";
> +				phy-handle = <&switch0phy1>;
> +			};
> +


> +
> +			port@8 {
> +				reg = <8>;
> +				label = "lan8";
> +				phy-handle = <&switch0phy8>;
> +			};
> +
> +			port@9 {
> +				reg = <9>;
> +				label = "wanp9";

Do these names correspond to some labeling? Ether the case or the silk
screen? wanp9 is an odd name. Is it connected to a header?

> +				phy-mode = "10gbase-kr";
> +				fixed-link {
> +					speed = <10000>;
> +					full-duplex;
> +				};
> +			};

  Andrew
Chris Packham Oct. 8, 2021, 12:09 a.m. UTC | #2
On 8/10/21 12:51 pm, Andrew Lunn wrote:

Some responses below.

Note I'll be on leave next week so I'll send v2 when I'm back the 
following week. That'll also allow some time for any other comments to 
come in.

> On Fri, Oct 08, 2021 at 12:06:19PM +1300, Chris Packham wrote:
>> The CN9130-CRB boards have a MV88E6393X switch connected to eth0.  Add
>> the necessary dts nodes and properties for this.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>
>> This is taken from the Marvell SDK. I've re-ordered the port entries to
>> be in ascending order.
>>
>>   arch/arm64/boot/dts/marvell/cn9130-crb.dtsi | 125 ++++++++++++++++++++
>>   1 file changed, 125 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi b/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
>> index e7918f325646..171f7394948e 100644
>> --- a/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
>> @@ -185,6 +185,131 @@ &cp0_mdio {
>>   	phy0: ethernet-phy@0 {
>>   		reg = <0>;
>>   	};
>> +
>> +	switch6: switch0@6 {
>> +		/* Actual device is MV88E6393X */
>> +		compatible = "marvell,mv88e6190";
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		reg = <6>;
> Is the interrupt output connected to a GPIO?
Yes it appears to be connected to CP_MPP28 although the comments in the 
schematic suggest this was added in Rev 1.30 of the design. I think that 
corresponds to the board I have but may not cover all the boards out 
there in the wild. I'll try adding it.
>
>> +
>> +		dsa,member = <0 0>;
>> +
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			port@0 {
>> +				reg = <0>;
>> +				label = "notused-port0";
>> +				phy-mode = "10gbase-kr";
>> +				status = "disabled";
> What is meant by not used? Does it go to a header? Is it not wired at
> all? You don't need to list a port if it is not actually used. So
> maybe you just want to delete this port all together?
>
It's completely disconnected so I'll remove the port.
>> +
>> +			};
>> +
>> +			port@1 {
>> +				reg = <1>;
>> +				label = "wan1";
>> +				phy-handle = <&switch0phy1>;
>> +			};
>> +
>
>> +
>> +			port@8 {
>> +				reg = <8>;
>> +				label = "lan8";
>> +				phy-handle = <&switch0phy8>;
>> +			};
>> +
>> +			port@9 {
>> +				reg = <9>;
>> +				label = "wanp9";
> Do these names correspond to some labeling? Ether the case or the silk
> screen?
The silkscreen just says P1-P8. I was tempted to rename "wan1" -> "lan1" 
to match the others. I could also change them all to "pN" or "portN" if 
preferred.
>   wanp9 is an odd name. Is it connected to a header?
P9 is connected to a SFP+ cage. I know there has been some work on the 
bindings for that which I haven't caught up with. Again I can rename 
this to "lan9", "p9" or "port9" as desired. But perhaps I'd be better 
off to not include the port and just leave a note here to say port9 
needs the sfp bindings (or I could get to grips with the bindings).
>
>> +				phy-mode = "10gbase-kr";
>> +				fixed-link {
>> +					speed = <10000>;
>> +					full-duplex;
>> +				};
>> +			};
>    Andrew
Andrew Lunn Oct. 10, 2021, 2:45 p.m. UTC | #3
> >> +			port@9 {
> >> +				reg = <9>;
> >> +				label = "wanp9";
> > Do these names correspond to some labeling? Ether the case or the silk
> > screen?
> The silkscreen just says P1-P8. I was tempted to rename "wan1" -> "lan1" 
> to match the others. I could also change them all to "pN" or "portN" if 
> preferred.

I normally say, use the labels from the case. That is what the user
sees. But if this RDK does not have case, then maybe lan1-lan8 would
be better, to match the silk screen. And then call port9 sfp1?

> P9 is connected to a SFP+ cage. I know there has been some work on the 
> bindings for that which I haven't caught up with.

Pretty simple. You need a node about the SFP itself:

sfp_eth3: sfp-eth3 {
        compatible = "sff,sfp";
        i2c-bus = <&sfp_1g_i2c>;
        los-gpios = <&cpm_gpio2 22 GPIO_ACTIVE_HIGH>;
        mod-def0-gpios = <&cpm_gpio2 21 GPIO_ACTIVE_LOW>;
        maximum-power-milliwatt = <1000>;
        pinctrl-names = "default";
        pinctrl-0 = <&cpm_sfp_1g_pins &cps_sfp_1g_pins>;
        tx-disable-gpios = <&cps_gpio1 24 GPIO_ACTIVE_HIGH>;
        tx-fault-gpios = <&cpm_gpio2 19 GPIO_ACTIVE_HIGH>;
};

If you don't have any of the GPIO, just don't list them.

And in the MAC you need to reference the sfp:

        sfp = <&sfp_eth3>;

Given that this is a marvell device, you might also need some comphy
configuration in the MAC node:

        phy-names = "comphy";
        phys = <&cps_comphy5 0>;

	Andrew
Andrew Lunn Oct. 10, 2021, 3:21 p.m. UTC | #4
> Given that this is a marvell device, you might also need some comphy
> configuration in the MAC node:
> 
>         phy-names = "comphy";
>         phys = <&cps_comphy5 0>;

Humm, actually, forget that, this is a switch, not a SoC.

      Andrew
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi b/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
index e7918f325646..171f7394948e 100644
--- a/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
+++ b/arch/arm64/boot/dts/marvell/cn9130-crb.dtsi
@@ -185,6 +185,131 @@  &cp0_mdio {
 	phy0: ethernet-phy@0 {
 		reg = <0>;
 	};
+
+	switch6: switch0@6 {
+		/* Actual device is MV88E6393X */
+		compatible = "marvell,mv88e6190";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <6>;
+
+		dsa,member = <0 0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				label = "notused-port0";
+				phy-mode = "10gbase-kr";
+				status = "disabled";
+
+			};
+
+			port@1 {
+				reg = <1>;
+				label = "wan1";
+				phy-handle = <&switch0phy1>;
+			};
+
+			port@2 {
+				reg = <2>;
+				label = "lan2";
+				phy-handle = <&switch0phy2>;
+			};
+
+			port@3 {
+				reg = <3>;
+				label = "lan3";
+				phy-handle = <&switch0phy3>;
+			};
+
+			port@4 {
+				reg = <4>;
+				label = "lan4";
+				phy-handle = <&switch0phy4>;
+			};
+
+			port@5 {
+				reg = <5>;
+				label = "lan5";
+				phy-handle = <&switch0phy5>;
+			};
+
+			port@6 {
+				reg = <6>;
+				label = "lan6";
+				phy-handle = <&switch0phy6>;
+			};
+
+			port@7 {
+				reg = <7>;
+				label = "lan7";
+				phy-handle = <&switch0phy7>;
+			};
+
+			port@8 {
+				reg = <8>;
+				label = "lan8";
+				phy-handle = <&switch0phy8>;
+			};
+
+			port@9 {
+				reg = <9>;
+				label = "wanp9";
+				phy-mode = "10gbase-kr";
+				fixed-link {
+					speed = <10000>;
+					full-duplex;
+				};
+			};
+
+			port@10 {
+				reg = <10>;
+				label = "cpu";
+				ethernet = <&cp0_eth0>;
+			};
+
+		};
+
+		mdio {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			switch0phy1: switch0phy1@1 {
+				reg = <0x1>;
+			};
+
+			switch0phy2: switch0phy2@2 {
+				reg = <0x2>;
+			};
+
+			switch0phy3: switch0phy3@3 {
+				reg = <0x3>;
+			};
+
+			switch0phy4: switch0phy4@4 {
+				reg = <0x4>;
+			};
+
+			switch0phy5: switch0phy5@5 {
+				reg = <0x5>;
+			};
+
+			switch0phy6: switch0phy6@6 {
+				reg = <0x6>;
+			};
+
+			switch0phy7: switch0phy7@7 {
+				reg = <0x7>;
+			};
+
+			switch0phy8: switch0phy8@8 {
+				reg = <0x8>;
+			};
+		};
+	};
 };
 
 &cp0_xmdio {