> -----Original Message-----
> From: Sean Anderson <sean.anderson@xxxxxxxx>
> Sent: Thursday, December 15, 2022 18:33
> To: Camelia Alexandra Groza <camelia.groza@xxxxxxx>
> Cc: David S . Miller <davem@xxxxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; Michael Ellerman <mpe@xxxxxxxxxxxxxx>;
> linux-kernel@xxxxxxxxxxxxxxx; Rob Herring <robh+dt@xxxxxxxxxx>;
> Christophe Leroy <christophe.leroy@xxxxxxxxxx>; linuxppc-
> dev@xxxxxxxxxxxxxxxx; Nicholas Piggin <npiggin@xxxxxxxxx>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>
> Subject: Re: [PATCH net-next] powerpc: dts: t208x: Disable 10G on MAC1 and
> MAC2
>
> On 12/15/22 11:12, Camelia Alexandra Groza wrote:
> >> -----Original Message-----
> >> From: Sean Anderson <sean.anderson@xxxxxxxx>
> >> Sent: Thursday, November 3, 2022 23:29
> >> To: David S . Miller <davem@xxxxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx
> >> Cc: devicetree@xxxxxxxxxxxxxxx; Michael Ellerman
> <mpe@xxxxxxxxxxxxxx>;
> >> linux-kernel@xxxxxxxxxxxxxxx; Rob Herring <robh+dt@xxxxxxxxxx>;
> >> Christophe Leroy <christophe.leroy@xxxxxxxxxx>; linuxppc-
> >> dev@xxxxxxxxxxxxxxxx; Nicholas Piggin <npiggin@xxxxxxxxx>; Krzysztof
> >> Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>; Sean Anderson
> >> <sean.anderson@xxxxxxxx>; Camelia Alexandra Groza
> >> <camelia.groza@xxxxxxx>
> >> Subject: [PATCH net-next] powerpc: dts: t208x: Disable 10G on MAC1 and
> >> MAC2
> >>
> >> There aren't enough resources to run these ports at 10G speeds. Just
> >> keep the pcs changes, and revert the rest. This is not really correct,
> >> since the hardware could support 10g in some other configuration...
> >>
> >> Fixes: 36926a7d70c2 ("powerpc: dts: t208x: Mark MAC1 and MAC2 as 10G")
> >> Reported-by: Camelia Alexandra Groza <camelia.groza@xxxxxxx>
> >> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx>
> >> ---
> >>
> >
> > Hi Sean,
> >
> > I know I'm late, but there are a couple of issues with this patch. Do you
> intend
> > on sending a v2 or should I pick it up?
> >
> >> .../boot/dts/fsl/qoriq-fman3-0-10g-2.dtsi | 45 -------------------
> >> .../boot/dts/fsl/qoriq-fman3-0-10g-3.dtsi | 45 -------------------
> >> arch/powerpc/boot/dts/fsl/t2081si-post.dtsi | 6 ++-
> >> 3 files changed, 4 insertions(+), 92 deletions(-)
> >> delete mode 100644 arch/powerpc/boot/dts/fsl/qoriq-fman3-0-10g-
> 2.dtsi
> >> delete mode 100644 arch/powerpc/boot/dts/fsl/qoriq-fman3-0-10g-
> 3.dtsi
> >>
> >> diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman3-0-10g-2.dtsi
> >> b/arch/powerpc/boot/dts/fsl/qoriq-fman3-0-10g-2.dtsi
> >> deleted file mode 100644
> >> index 6b3609574b0f..000000000000
> >> --- a/arch/powerpc/boot/dts/fsl/qoriq-fman3-0-10g-2.dtsi
> >> +++ /dev/null
> >> @@ -1,45 +0,0 @@
> >> -// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-or-later
> >> -/*
> >> - * QorIQ FMan v3 10g port #2 device tree stub [ controller @ offset
> >> 0x400000 ]
> >> - *
> >> - * Copyright 2022 Sean Anderson <sean.anderson@xxxxxxxx>
> >> - * Copyright 2012 - 2015 Freescale Semiconductor Inc.
> >> - */
> >> -
> >> -fman@400000 {
> >> - fman0_rx_0x08: port@88000 {
> >> - cell-index = <0x8>;
> >> - compatible = "fsl,fman-v3-port-rx";
> >> - reg = <0x88000 0x1000>;
> >> - fsl,fman-10g-port;
> >> - };
> >> -
> >> - fman0_tx_0x28: port@a8000 {
> >> - cell-index = <0x28>;
> >> - compatible = "fsl,fman-v3-port-tx";
> >> - reg = <0xa8000 0x1000>;
> >> - fsl,fman-10g-port;
> >> - };
> >> -
> >> - ethernet@e0000 {
> >> - cell-index = <0>;
> >> - compatible = "fsl,fman-memac";
> >> - reg = <0xe0000 0x1000>;
> >> - fsl,fman-ports = <&fman0_rx_0x08 &fman0_tx_0x28>;
> >> - ptp-timer = <&ptp_timer0>;
> >> - pcsphy-handle = <&pcsphy0>, <&pcsphy0>;
> >> - pcs-handle-names = "sgmii", "xfi";
> >> - };
> >> -
> >> - mdio@e1000 {
> >> - #address-cells = <1>;
> >> - #size-cells = <0>;
> >> - compatible = "fsl,fman-memac-mdio", "fsl,fman-xmdio";
> >> - reg = <0xe1000 0x1000>;
> >> - fsl,erratum-a011043; /* must ignore read errors */
> >> -
> >> - pcsphy0: ethernet-phy@0 {
> >> - reg = <0x0>;
> >> - };
> >> - };
> >> -};
> >> diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman3-0-10g-3.dtsi
> >> b/arch/powerpc/boot/dts/fsl/qoriq-fman3-0-10g-3.dtsi
> >> deleted file mode 100644
> >> index 28ed1a85a436..000000000000
> >> --- a/arch/powerpc/boot/dts/fsl/qoriq-fman3-0-10g-3.dtsi
> >> +++ /dev/null
> >> @@ -1,45 +0,0 @@
> >> -// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-or-later
> >> -/*
> >> - * QorIQ FMan v3 10g port #3 device tree stub [ controller @ offset
> >> 0x400000 ]
> >> - *
> >> - * Copyright 2022 Sean Anderson <sean.anderson@xxxxxxxx>
> >> - * Copyright 2012 - 2015 Freescale Semiconductor Inc.
> >> - */
> >> -
> >> -fman@400000 {
> >> - fman0_rx_0x09: port@89000 {
> >> - cell-index = <0x9>;
> >> - compatible = "fsl,fman-v3-port-rx";
> >> - reg = <0x89000 0x1000>;
> >> - fsl,fman-10g-port;
> >> - };
> >> -
> >> - fman0_tx_0x29: port@a9000 {
> >> - cell-index = <0x29>;
> >> - compatible = "fsl,fman-v3-port-tx";
> >> - reg = <0xa9000 0x1000>;
> >> - fsl,fman-10g-port;
> >> - };
> >> -
> >> - ethernet@e2000 {
> >> - cell-index = <1>;
> >> - compatible = "fsl,fman-memac";
> >> - reg = <0xe2000 0x1000>;
> >> - fsl,fman-ports = <&fman0_rx_0x09 &fman0_tx_0x29>;
> >> - ptp-timer = <&ptp_timer0>;
> >> - pcsphy-handle = <&pcsphy1>, <&pcsphy1>;
> >> - pcs-handle-names = "sgmii", "xfi";
> >> - };
> >> -
> >> - mdio@e3000 {
> >> - #address-cells = <1>;
> >> - #size-cells = <0>;
> >> - compatible = "fsl,fman-memac-mdio", "fsl,fman-xmdio";
> >> - reg = <0xe3000 0x1000>;
> >> - fsl,erratum-a011043; /* must ignore read errors */
> >> -
> >> - pcsphy1: ethernet-phy@0 {
> >> - reg = <0x0>;
> >> - };
> >> - };
> >> -};
> >> diff --git a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
> >> b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
> >> index 74e17e134387..fed3879fa0aa 100644
> >> --- a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
> >> +++ b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
> >> @@ -609,8 +609,8 @@ usb1: usb@211000 {
> >> /include/ "qoriq-bman1.dtsi"
> >>
> >> /include/ "qoriq-fman3-0.dtsi"
> >> -/include/ "qoriq-fman3-0-10g-2.dtsi"
> >> -/include/ "qoriq-fman3-0-10g-3.dtsi"
> >> +/include/ "qoriq-fman3-0-1g-2.dtsi"
> >> +/include/ "qoriq-fman3-0-1g-3.dtsi"
> >
> > These two should be qoriq-fman3-0-1g-0.dtsi and qoriq-fman3-0-1g-1.dtsi.
> > You are including 1g-2.dtsi and 1g-3.dtsi twice.
>
> So they should.
>
> >> /include/ "qoriq-fman3-0-1g-2.dtsi"
> >> /include/ "qoriq-fman3-0-1g-3.dtsi"
> >> /include/ "qoriq-fman3-0-1g-4.dtsi"
> >> @@ -619,9 +619,11 @@ usb1: usb@211000 {
> >> /include/ "qoriq-fman3-0-10g-1.dtsi"
> >> fman@400000 {
> >> enet0: ethernet@e0000 {
> >> + pcs-handle-names = "sgmii", "xfi";
> >> };
> >>
> >> enet1: ethernet@e2000 {
> >> + pcs-handle-names = "sgmii", "xfi";
> >
> > The second pcsphy for this port is still qsgmiia_pcs1 as described in
> > qoriq-fman3-0-1g-1.dtsi. It should also be overwritten, not only the name
> > property:
> > pcsphy-handle = <&pcsphy1>, <&pcsphy1>;
>
> This is the sort of reason I wanted to just delete the 10g property.
>
> --Sean
I was against adding the 10g property in qoriq-fman3-0-10g-2/3.dtsi and
then deleting it in from t2081si-post.dtsi. Since there aren't other users for
these two new dtsi, it felt unneeded adding it in the first place.
I was thinking of just removing the property from qoriq-fman3-0-10g-2/3.dtsi
from the start.
I am reconsidering now. I see the value in maintaining the layout across all
qoriq-fman3-0-10g-x.dtsi files. And given the resource allocation issue is
relevant only for this one SoC, not all generic ports, it makes sense to edit the
SoC dtsi only.
In short, we can go with your original proposal. Sorry for the noise.
Camelia
> >> };
> >>
> >> enet2: ethernet@e4000 {
> >> --
> >> 2.35.1.1320.gc452695387.dirty
> >
