2020-02-19 15:13:54

by Vladimir Oltean

[permalink] [raw]
Subject: [PATCH v2 net-next/devicetree 4/5] arm64: dts: fsl: ls1028a: add node for Felix switch

From: Claudiu Manoil <[email protected]>

Add the switch device node, available on PF5, so that the switch port
sub-nodes (net devices) can be linked to corresponding board specific
phy nodes (external ports) or have their link mode defined (internal
ports).

The switch device features 6 ports, 4 with external links and 2
internally facing to the LS1028A SoC and connected via fixed links to 2
internal ENETC Ethernet controller ports.

Add the corresponding ENETC host port device nodes, mapped to PF2 and
PF6 PCIe functions. Since the switch only supports tagging on one CPU
port, only one port pair (swp4, eno2) is enabled by default and the
other, lower speed, port pair is disabled to prevent the PCI core from
probing them. If enabled, swp5 will be a fixed-link slave port.

DSA tagging can also be moved from the swp4-eno2 2.5G port pair to the
1G swp5-eno3 pair by changing the ethernet = <&enetc_port2> phandle to
<&enetc_port3> and moving it under port5, but in that case enetc_port2
should not be disabled, because it is the hardware owner of the Felix
PCS and disabling its memory would result in access faults in the Felix
DSA driver.

All ports are disabled by default, except one CPU port.

The switch's INTB interrupt line signals:
- PTP timestamp ready in timestamp FIFO
- TSN Frame Preemption

And don't forget to enable the 4MB BAR4 in the root complex ECAM space,
where the switch registers are mapped.

Signed-off-by: Claudiu Manoil <[email protected]>
Signed-off-by: Alex Marginean <[email protected]>
Signed-off-by: Yangbo Lu <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v2:
Adapted phy-mode = "gmii" to phy-mode = "internal".

.../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 84 ++++++++++++++++++-
1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index dfead691e509..a6b9c6d1eb5e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -700,7 +700,9 @@
/* PF1: VF0-1 BAR0 - non-prefetchable memory */
0x82000000 0x0 0x00000000 0x1 0xf8210000 0x0 0x020000
/* PF1: VF0-1 BAR2 - prefetchable memory */
- 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000>;
+ 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000
+ /* BAR4 (PF5) - non-prefetchable memory */
+ 0x82000000 0x0 0x00000000 0x1 0xfc000000 0x0 0x400000>;

enetc_port0: ethernet@0,0 {
compatible = "fsl,enetc";
@@ -710,6 +712,18 @@
compatible = "fsl,enetc";
reg = <0x000100 0 0 0 0>;
};
+
+ enetc_port2: ethernet@0,2 {
+ compatible = "fsl,enetc";
+ reg = <0x000200 0 0 0 0>;
+ phy-mode = "gmii";
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
enetc_mdio_pf3: mdio@0,3 {
compatible = "fsl,enetc-mdio";
reg = <0x000300 0 0 0 0>;
@@ -722,6 +736,74 @@
clocks = <&clockgen 4 0>;
little-endian;
};
+
+ ethernet-switch@0,5 {
+ reg = <0x000500 0 0 0 0>;
+ /* IEP INT_B */
+ interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* external ports */
+ mscc_felix_port0: port@0 {
+ reg = <0>;
+ status = "disabled";
+ };
+
+ mscc_felix_port1: port@1 {
+ reg = <1>;
+ status = "disabled";
+ };
+
+ mscc_felix_port2: port@2 {
+ reg = <2>;
+ status = "disabled";
+ };
+
+ mscc_felix_port3: port@3 {
+ reg = <3>;
+ status = "disabled";
+ };
+
+ /* Internal port with DSA tagging */
+ mscc_felix_port4: port@4 {
+ reg = <4>;
+ phy-mode = "internal";
+ ethernet = <&enetc_port2>;
+
+ fixed-link {
+ speed = <2500>;
+ full-duplex;
+ };
+ };
+
+ /* Internal port without DSA tagging */
+ mscc_felix_port5: port@5 {
+ reg = <5>;
+ phy-mode = "internal";
+ status = "disabled";
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ };
+ };
+
+ enetc_port3: ethernet@0,6 {
+ compatible = "fsl,enetc";
+ reg = <0x000600 0 0 0 0>;
+ status = "disabled";
+ phy-mode = "gmii";
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
};
};

--
2.17.1


2020-02-22 11:39:05

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v2 net-next/devicetree 4/5] arm64: dts: fsl: ls1028a: add node for Felix switch

Hi,

> Add the switch device node, available on PF5, so that the switch port
> sub-nodes (net devices) can be linked to corresponding board specific
> phy nodes (external ports) or have their link mode defined (internal
> ports).
>
> The switch device features 6 ports, 4 with external links and 2
> internally facing to the LS1028A SoC and connected via fixed links to 2
> internal ENETC Ethernet controller ports.
>
> Add the corresponding ENETC host port device nodes, mapped to PF2 and
> PF6 PCIe functions. Since the switch only supports tagging on one CPU
> port, only one port pair (swp4, eno2) is enabled by default and the
> other, lower speed, port pair is disabled to prevent the PCI core from
> probing them. If enabled, swp5 will be a fixed-link slave port.
>
> DSA tagging can also be moved from the swp4-eno2 2.5G port pair to the
> 1G swp5-eno3 pair by changing the ethernet = <&enetc_port2> phandle to
> <&enetc_port3> and moving it under port5, but in that case enetc_port2
> should not be disabled, because it is the hardware owner of the Felix
> PCS and disabling its memory would result in access faults in the Felix
> DSA driver.
>
> All ports are disabled by default, except one CPU port.
>
> The switch's INTB interrupt line signals:
> - PTP timestamp ready in timestamp FIFO
> - TSN Frame Preemption
>
> And don't forget to enable the 4MB BAR4 in the root complex ECAM space,
> where the switch registers are mapped.
>
> Signed-off-by: Claudiu Manoil <[email protected]>
> Signed-off-by: Alex Marginean <[email protected]>
> Signed-off-by: Yangbo Lu <[email protected]>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> Changes in v2:
> Adapted phy-mode = "gmii" to phy-mode = "internal".
>
> .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 84 ++++++++++++++++++-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index dfead691e509..a6b9c6d1eb5e 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -700,7 +700,9 @@
> /* PF1: VF0-1 BAR0 - non-prefetchable memory */
> 0x82000000 0x0 0x00000000 0x1 0xf8210000 0x0 0x020000
> /* PF1: VF0-1 BAR2 - prefetchable memory */
> - 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000>;
> + 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000
> + /* BAR4 (PF5) - non-prefetchable memory */
> + 0x82000000 0x0 0x00000000 0x1 0xfc000000 0x0 0x400000>;
>
> enetc_port0: ethernet@0,0 {
> compatible = "fsl,enetc";
> @@ -710,6 +712,18 @@
> compatible = "fsl,enetc";
> reg = <0x000100 0 0 0 0>;
> };
> +
> + enetc_port2: ethernet@0,2 {
> + compatible = "fsl,enetc";
> + reg = <0x000200 0 0 0 0>;
> + phy-mode = "gmii";
Can we disable this port by default in this dtsi? As mentioned in the other
mail, I'd prefer to have all ports disabled because it doesn't make sense
to have this port while having all the external ports disabled.

> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> enetc_mdio_pf3: mdio@0,3 {
> compatible = "fsl,enetc-mdio";
> reg = <0x000300 0 0 0 0>;
> @@ -722,6 +736,74 @@
> clocks = <&clockgen 4 0>;
> little-endian;
> };
> +
> + ethernet-switch@0,5 {
> + reg = <0x000500 0 0 0 0>;
> + /* IEP INT_B */
> + interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* external ports */
> + mscc_felix_port0: port@0 {
> + reg = <0>;
> + status = "disabled";
> + };
> +
> + mscc_felix_port1: port@1 {
> + reg = <1>;
> + status = "disabled";
> + };
> +
> + mscc_felix_port2: port@2 {
> + reg = <2>;
> + status = "disabled";
> + };
> +
> + mscc_felix_port3: port@3 {
> + reg = <3>;
> + status = "disabled";
> + };
> +
> + /* Internal port with DSA tagging */
> + mscc_felix_port4: port@4 {
> + reg = <4>;
> + phy-mode = "internal";
> + ethernet = <&enetc_port2>;
Likewise, I'd prefer to have this disabled.

> +
> + fixed-link {
> + speed = <2500>;
> + full-duplex;
> + };
> + };
> +
> + /* Internal port without DSA tagging */
> + mscc_felix_port5: port@5 {
> + reg = <5>;
> + phy-mode = "internal";
> + status = "disabled";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> +
> + enetc_port3: ethernet@0,6 {
> + compatible = "fsl,enetc";
> + reg = <0x000600 0 0 0 0>;
> + status = "disabled";
> + phy-mode = "gmii";
shouldn't the status be after the phy-mode property?

-michael

> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> };
> };
>
> --
> 2.17.1

2020-02-22 12:26:11

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next/devicetree 4/5] arm64: dts: fsl: ls1028a: add node for Felix switch

Hi Michael,

On Sat, 22 Feb 2020 at 13:38, Michael Walle <[email protected]> wrote:
>
> Hi,
>

> > +
> > + enetc_port2: ethernet@0,2 {
> > + compatible = "fsl,enetc";
> > + reg = <0x000200 0 0 0 0>;
> > + phy-mode = "gmii";
> Can we disable this port by default in this dtsi? As mentioned in the other
> mail, I'd prefer to have all ports disabled because it doesn't make sense
> to have this port while having all the external ports disabled.
>

Ok. What would you want to happen with the "ethernet" property? Do you
want the board dts to set that too?

> > + /* Internal port with DSA tagging */
> > + mscc_felix_port4: port@4 {
> > + reg = <4>;
> > + phy-mode = "internal";
> > + ethernet = <&enetc_port2>;
> Likewise, I'd prefer to have this disabled.
>

Ok.

> > + enetc_port3: ethernet@0,6 {
> > + compatible = "fsl,enetc";
> > + reg = <0x000600 0 0 0 0>;
> > + status = "disabled";
> > + phy-mode = "gmii";
> shouldn't the status be after the phy-mode property?

Why?

>
> -michael
>

Regards,
-Vladimir

2020-02-22 13:20:18

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v2 net-next/devicetree 4/5] arm64: dts: fsl: ls1028a: add node for Felix switch

Hi Vladimir,

Am 2020-02-22 13:25, schrieb Vladimir Oltean:
> Hi Michael,
>
> On Sat, 22 Feb 2020 at 13:38, Michael Walle <[email protected]> wrote:
>>
>> Hi,
>>
>
>> > +
>> > + enetc_port2: ethernet@0,2 {
>> > + compatible = "fsl,enetc";
>> > + reg = <0x000200 0 0 0 0>;
>> > + phy-mode = "gmii";
>> Can we disable this port by default in this dtsi? As mentioned in the
>> other
>> mail, I'd prefer to have all ports disabled because it doesn't make
>> sense
>> to have this port while having all the external ports disabled.
>>
>
> Ok. What would you want to happen with the "ethernet" property? Do you
> want the board dts to set that too?

That's something I've also thought about. And now that you've mention
this, I think it makes more sense to have that in the board too. Because
if you have the freedom to use either eno2/swp4 or eno3/swp5, then if I
choose the second one I'd have to delete the ethernet property from the
first, correct? I actually thought about adding the ethernet property
to both; but (1) I don't know if that is even possible (given that one
is always disabled) and (2) if one want to use the second port as an
additional link to the switch you'd have to remove the ethernet property
on that port. correct?


>> > + /* Internal port with DSA tagging */
>> > + mscc_felix_port4: port@4 {
>> > + reg = <4>;
>> > + phy-mode = "internal";
>> > + ethernet = <&enetc_port2>;
>> Likewise, I'd prefer to have this disabled.
>>
>
> Ok.
>
>> > + enetc_port3: ethernet@0,6 {
>> > + compatible = "fsl,enetc";
>> > + reg = <0x000600 0 0 0 0>;
>> > + status = "disabled";
>> > + phy-mode = "gmii";
>> shouldn't the status be after the phy-mode property?
>
> Why?

I thought that would be a rule. I just had a quick look on some other
device
trees before and they all has the status property as the last property
(before
any subnodes). I might be mistaken. If so, you could do it for
consistency
reasons ;) all status property in the ls1028a.dtsi are the last ones.

-michael

2020-02-24 06:38:41

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v2 net-next/devicetree 4/5] arm64: dts: fsl: ls1028a: add node for Felix switch

On Wed, Feb 19, 2020 at 05:12:58PM +0200, Vladimir Oltean wrote:
> From: Claudiu Manoil <[email protected]>
>
> Add the switch device node, available on PF5, so that the switch port
> sub-nodes (net devices) can be linked to corresponding board specific
> phy nodes (external ports) or have their link mode defined (internal
> ports).
>
> The switch device features 6 ports, 4 with external links and 2
> internally facing to the LS1028A SoC and connected via fixed links to 2
> internal ENETC Ethernet controller ports.
>
> Add the corresponding ENETC host port device nodes, mapped to PF2 and
> PF6 PCIe functions. Since the switch only supports tagging on one CPU
> port, only one port pair (swp4, eno2) is enabled by default and the
> other, lower speed, port pair is disabled to prevent the PCI core from
> probing them. If enabled, swp5 will be a fixed-link slave port.
>
> DSA tagging can also be moved from the swp4-eno2 2.5G port pair to the
> 1G swp5-eno3 pair by changing the ethernet = <&enetc_port2> phandle to
> <&enetc_port3> and moving it under port5, but in that case enetc_port2
> should not be disabled, because it is the hardware owner of the Felix
> PCS and disabling its memory would result in access faults in the Felix
> DSA driver.
>
> All ports are disabled by default, except one CPU port.
>
> The switch's INTB interrupt line signals:
> - PTP timestamp ready in timestamp FIFO
> - TSN Frame Preemption
>
> And don't forget to enable the 4MB BAR4 in the root complex ECAM space,
> where the switch registers are mapped.
>
> Signed-off-by: Claudiu Manoil <[email protected]>
> Signed-off-by: Alex Marginean <[email protected]>
> Signed-off-by: Yangbo Lu <[email protected]>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> Changes in v2:
> Adapted phy-mode = "gmii" to phy-mode = "internal".
>
> .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 84 ++++++++++++++++++-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index dfead691e509..a6b9c6d1eb5e 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -700,7 +700,9 @@
> /* PF1: VF0-1 BAR0 - non-prefetchable memory */
> 0x82000000 0x0 0x00000000 0x1 0xf8210000 0x0 0x020000
> /* PF1: VF0-1 BAR2 - prefetchable memory */
> - 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000>;
> + 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000
> + /* BAR4 (PF5) - non-prefetchable memory */
> + 0x82000000 0x0 0x00000000 0x1 0xfc000000 0x0 0x400000>;
>
> enetc_port0: ethernet@0,0 {
> compatible = "fsl,enetc";
> @@ -710,6 +712,18 @@
> compatible = "fsl,enetc";
> reg = <0x000100 0 0 0 0>;
> };
> +
> + enetc_port2: ethernet@0,2 {
> + compatible = "fsl,enetc";
> + reg = <0x000200 0 0 0 0>;
> + phy-mode = "gmii";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> enetc_mdio_pf3: mdio@0,3 {
> compatible = "fsl,enetc-mdio";
> reg = <0x000300 0 0 0 0>;
> @@ -722,6 +736,74 @@
> clocks = <&clockgen 4 0>;
> little-endian;
> };
> +
> + ethernet-switch@0,5 {
> + reg = <0x000500 0 0 0 0>;
> + /* IEP INT_B */
> + interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* external ports */
> + mscc_felix_port0: port@0 {
> + reg = <0>;
> + status = "disabled";
> + };
> +
> + mscc_felix_port1: port@1 {
> + reg = <1>;
> + status = "disabled";
> + };
> +
> + mscc_felix_port2: port@2 {
> + reg = <2>;
> + status = "disabled";
> + };
> +
> + mscc_felix_port3: port@3 {
> + reg = <3>;
> + status = "disabled";
> + };
> +
> + /* Internal port with DSA tagging */
> + mscc_felix_port4: port@4 {
> + reg = <4>;
> + phy-mode = "internal";
> + ethernet = <&enetc_port2>;
> +
> + fixed-link {
> + speed = <2500>;
> + full-duplex;
> + };
> + };
> +
> + /* Internal port without DSA tagging */
> + mscc_felix_port5: port@5 {
> + reg = <5>;
> + phy-mode = "internal";
> + status = "disabled";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> +
> + enetc_port3: ethernet@0,6 {
> + compatible = "fsl,enetc";
> + reg = <0x000600 0 0 0 0>;
> + status = "disabled";

Please have 'status' at bottom of the property list.

Shawn

> + phy-mode = "gmii";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> };
> };
>
> --
> 2.17.1
>