| Message ID | 20220523182549.30242-9-tharvey@gateworks.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | Add MV88E61xx DSA driver and use on gwventana | expand |
On Mon, May 23, 2022 at 11:26 AM Tim Harvey <tharvey@gateworks.com> wrote: > > Add MV88E61XX DSA support: > - update dt: U-Boot dsa driver requires different device-tree syntax > than the linux driver in order to link the dsa ports to the mdio bus. > - update defconfig > - replace mv88e61xx_hw_reset weak override with board_phy_config support > for mv88e61xx configuration that is outside the scope of the DSA driver > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > --- > v3: > - move mdio's mdio@0 subnode > v2: no changes > --- > arch/arm/dts/imx6qdl-gw5904.dtsi | 41 ++++++++++++++++++++ > board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++---------------- > configs/gwventana_gw5904_defconfig | 7 ++-- > 3 files changed, 62 insertions(+), 36 deletions(-) > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi > index 286c7a9924c2..1b2f70d1ccb2 100644 > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi > @@ -219,6 +219,33 @@ > compatible = "marvell,mv88e6085"; > reg = <0>; > > + mdios { > + #address-cells = <1>; > + #size-cells = <0>; > + > + mdio@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + sw_phy0: ethernet-phy@0 { > + reg = <0x0>; > + }; > + > + sw_phy1: ethernet-phy@1 { > + reg = <0x1>; > + }; > + > + sw_phy2: ethernet-phy@2 { > + reg = <0x2>; > + }; > + > + sw_phy3: ethernet-phy@3 { > + reg = <0x3>; > + }; > + }; > + }; > + Vladimir, I'm not sure if you've had a chance to look at this latest patch revision yet. I believe above is what you were describing as the correct way to do this (without modifying the Linux driver and improving bindings) Best Regards, Tim > ports { > #address-cells = <1>; > #size-cells = <0>; > @@ -226,27 +253,41 @@ > port@0 { > reg = <0>; > label = "lan4"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy0>; > }; > > port@1 { > reg = <1>; > label = "lan3"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy1>; > }; > > port@2 { > reg = <2>; > label = "lan2"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy2>; > }; > > port@3 { > reg = <3>; > label = "lan1"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy3>; > }; > > port@5 { > reg = <5>; > label = "cpu"; > ethernet = <&fec>; > + phy-mode = "rgmii-id"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > }; > }; > }; > diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c > index c06630a66b66..bef3f7ef0d2b 100644 > --- a/board/gateworks/gw_ventana/gw_ventana.c > +++ b/board/gateworks/gw_ventana/gw_ventana.c > @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev) > phy_write(phydev, MDIO_DEVAD_NONE, 14, val); > } > > + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */ > + else if (phydev->phy_id == 0xa5a55a5a && > + ((board_type == GW5904) || (board_type == GW5909))) { > + struct mii_dev *bus = miiphy_get_dev_by_name("mdio"); > + > + puts("MV88E61XX "); > + /* GPIO[0] output CLK125 for RGMII_REFCLK */ > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe); > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7); > + > + /* Port 0-3 LED configuration: Table 80/82 */ > + /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > + bus->write(bus, 0x10, 0, 0x16, 0x8088); > + bus->write(bus, 0x11, 0, 0x16, 0x8088); > + bus->write(bus, 0x12, 0, 0x16, 0x8088); > + bus->write(bus, 0x13, 0, 0x16, 0x8088); > + } > + > if (phydev->drv->config) > phydev->drv->config(phydev); > > return 0; > } > > -#ifdef CONFIG_MV88E61XX_SWITCH > -int mv88e61xx_hw_reset(struct phy_device *phydev) > -{ > - struct mii_dev *bus = phydev->bus; > - > - /* GPIO[0] output, CLK125 */ > - debug("enabling RGMII_REFCLK\n"); > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > - 0x1a /*MV_SCRATCH_MISC*/, > - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > - 0x1a /*MV_SCRATCH_MISC*/, > - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); > - > - /* RGMII delay - Physical Control register bit[15:14] */ > - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); > - /* forced 1000mbps full-duplex link */ > - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); > - phydev->autoneg = AUTONEG_DISABLE; > - phydev->speed = SPEED_1000; > - phydev->duplex = DUPLEX_FULL; > - > - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > - bus->write(bus, 0x10, 0, 0x16, 0x8088); > - bus->write(bus, 0x11, 0, 0x16, 0x8088); > - bus->write(bus, 0x12, 0, 0x16, 0x8088); > - bus->write(bus, 0x13, 0, 0x16, 0x8088); > - > - return 0; > -} > -#endif // CONFIG_MV88E61XX_SWITCH > - > #if defined(CONFIG_VIDEO_IPUV3) > static void enable_hdmi(struct display_info_t const *dev) > { > diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig > index d25a8324b1df..9809bc691508 100644 > --- a/configs/gwventana_gw5904_defconfig > +++ b/configs/gwventana_gw5904_defconfig > @@ -103,12 +103,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y > CONFIG_FSL_USDHC=y > CONFIG_MTD=y > CONFIG_PHYLIB=y > -CONFIG_MV88E61XX_SWITCH=y > -CONFIG_MV88E61XX_CPU_PORT=5 > -CONFIG_MV88E61XX_PHY_PORTS=0xf > -CONFIG_MV88E61XX_FIXED_PORTS=0x0 > +CONFIG_MV88E61XX=y > +CONFIG_PHY_FIXED=y > CONFIG_DM_ETH=y > CONFIG_DM_MDIO=y > +CONFIG_DM_DSA=y > CONFIG_E1000=y > CONFIG_FEC_MXC=y > CONFIG_MII=y > -- > 2.17.1 >
Hi Tim, On Tue, Jun 14, 2022 at 10:00:57AM -0700, Tim Harvey wrote: > Vladimir, > > I'm not sure if you've had a chance to look at this latest patch > revision yet. I believe above is what you were describing as the > correct way to do this (without modifying the Linux driver and > improving bindings) > > Best Regards, > > Tim I'm currently on vacation but I'm slowly working through my inbox. This is certainly one of the reasonable ways of doing things, but it will require modifying the mv88e6xxx Linux driver to support an "mdios" subnode (something which nobody should object against, I believe). I'll follow up with more comments when I get to these patches.
On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote: > Add MV88E61XX DSA support: > - update dt: U-Boot dsa driver requires different device-tree syntax > than the linux driver in order to link the dsa ports to the mdio bus. > - update defconfig > - replace mv88e61xx_hw_reset weak override with board_phy_config support > for mv88e61xx configuration that is outside the scope of the DSA driver > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > --- > v3: > - move mdio's mdio@0 subnode > v2: no changes > --- > arch/arm/dts/imx6qdl-gw5904.dtsi | 41 ++++++++++++++++++++ > board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++---------------- > configs/gwventana_gw5904_defconfig | 7 ++-- > 3 files changed, 62 insertions(+), 36 deletions(-) > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi > index 286c7a9924c2..1b2f70d1ccb2 100644 > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi > @@ -219,6 +219,33 @@ > compatible = "marvell,mv88e6085"; > reg = <0>; > > + mdios { > + #address-cells = <1>; > + #size-cells = <0>; > + > + mdio@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + sw_phy0: ethernet-phy@0 { > + reg = <0x0>; > + }; > + > + sw_phy1: ethernet-phy@1 { > + reg = <0x1>; > + }; > + > + sw_phy2: ethernet-phy@2 { > + reg = <0x2>; > + }; > + > + sw_phy3: ethernet-phy@3 { > + reg = <0x3>; > + }; > + }; > + }; > + > ports { > #address-cells = <1>; > #size-cells = <0>; > @@ -226,27 +253,41 @@ > port@0 { > reg = <0>; > label = "lan4"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy0>; > }; > > port@1 { > reg = <1>; > label = "lan3"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy1>; > }; > > port@2 { > reg = <2>; > label = "lan2"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy2>; > }; > > port@3 { > reg = <3>; > label = "lan1"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy3>; > }; > > port@5 { > reg = <5>; > label = "cpu"; > ethernet = <&fec>; > + phy-mode = "rgmii-id"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > }; > }; > }; > diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c > index c06630a66b66..bef3f7ef0d2b 100644 > --- a/board/gateworks/gw_ventana/gw_ventana.c > +++ b/board/gateworks/gw_ventana/gw_ventana.c > @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev) > phy_write(phydev, MDIO_DEVAD_NONE, 14, val); > } > > + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */ > + else if (phydev->phy_id == 0xa5a55a5a && PHY_FIXED_ID, but see below. > + ((board_type == GW5904) || (board_type == GW5909))) { > + struct mii_dev *bus = miiphy_get_dev_by_name("mdio"); > + > + puts("MV88E61XX "); > + /* GPIO[0] output CLK125 for RGMII_REFCLK */ > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe); > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7); > + > + /* Port 0-3 LED configuration: Table 80/82 */ > + /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > + bus->write(bus, 0x10, 0, 0x16, 0x8088); > + bus->write(bus, 0x11, 0, 0x16, 0x8088); > + bus->write(bus, 0x12, 0, 0x16, 0x8088); > + bus->write(bus, 0x13, 0, 0x16, 0x8088); > + } > + There's nothing too board specific about this configuration, why do you feel it does not belong to the DSA driver? Some macros hiding away magic register addresses and values would be good either way. > if (phydev->drv->config) > phydev->drv->config(phydev); > > return 0; > } > > -#ifdef CONFIG_MV88E61XX_SWITCH > -int mv88e61xx_hw_reset(struct phy_device *phydev) > -{ > - struct mii_dev *bus = phydev->bus; > - > - /* GPIO[0] output, CLK125 */ > - debug("enabling RGMII_REFCLK\n"); > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > - 0x1a /*MV_SCRATCH_MISC*/, > - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > - 0x1a /*MV_SCRATCH_MISC*/, > - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); > - > - /* RGMII delay - Physical Control register bit[15:14] */ > - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); > - /* forced 1000mbps full-duplex link */ > - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); > - phydev->autoneg = AUTONEG_DISABLE; > - phydev->speed = SPEED_1000; > - phydev->duplex = DUPLEX_FULL; > - > - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > - bus->write(bus, 0x10, 0, 0x16, 0x8088); > - bus->write(bus, 0x11, 0, 0x16, 0x8088); > - bus->write(bus, 0x12, 0, 0x16, 0x8088); > - bus->write(bus, 0x13, 0, 0x16, 0x8088); > - > - return 0; > -} > -#endif // CONFIG_MV88E61XX_SWITCH > - > #if defined(CONFIG_VIDEO_IPUV3) > static void enable_hdmi(struct display_info_t const *dev) > { > diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig > index d25a8324b1df..9809bc691508 100644 > --- a/configs/gwventana_gw5904_defconfig > +++ b/configs/gwventana_gw5904_defconfig > @@ -103,12 +103,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y > CONFIG_FSL_USDHC=y > CONFIG_MTD=y > CONFIG_PHYLIB=y > -CONFIG_MV88E61XX_SWITCH=y > -CONFIG_MV88E61XX_CPU_PORT=5 > -CONFIG_MV88E61XX_PHY_PORTS=0xf > -CONFIG_MV88E61XX_FIXED_PORTS=0x0 > +CONFIG_MV88E61XX=y > +CONFIG_PHY_FIXED=y > CONFIG_DM_ETH=y > CONFIG_DM_MDIO=y > +CONFIG_DM_DSA=y > CONFIG_E1000=y > CONFIG_FEC_MXC=y > CONFIG_MII=y > -- > 2.17.1 >
On Mon, Jun 20, 2022 at 4:42 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote: > > Add MV88E61XX DSA support: > > - update dt: U-Boot dsa driver requires different device-tree syntax > > than the linux driver in order to link the dsa ports to the mdio bus. > > - update defconfig > > - replace mv88e61xx_hw_reset weak override with board_phy_config support > > for mv88e61xx configuration that is outside the scope of the DSA driver > > > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > > --- > > v3: > > - move mdio's mdio@0 subnode > > v2: no changes > > --- > > arch/arm/dts/imx6qdl-gw5904.dtsi | 41 ++++++++++++++++++++ > > board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++---------------- > > configs/gwventana_gw5904_defconfig | 7 ++-- > > 3 files changed, 62 insertions(+), 36 deletions(-) > > > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi > > index 286c7a9924c2..1b2f70d1ccb2 100644 > > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi > > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi > > @@ -219,6 +219,33 @@ > > compatible = "marvell,mv88e6085"; > > reg = <0>; > > > > + mdios { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + mdio@0 { > > + reg = <0>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + sw_phy0: ethernet-phy@0 { > > + reg = <0x0>; > > + }; > > + > > + sw_phy1: ethernet-phy@1 { > > + reg = <0x1>; > > + }; > > + > > + sw_phy2: ethernet-phy@2 { > > + reg = <0x2>; > > + }; > > + > > + sw_phy3: ethernet-phy@3 { > > + reg = <0x3>; > > + }; > > + }; > > + }; > > + > > ports { > > #address-cells = <1>; > > #size-cells = <0>; > > @@ -226,27 +253,41 @@ > > port@0 { > > reg = <0>; > > label = "lan4"; > > + phy-mode = "internal"; > > + phy-handle = <&sw_phy0>; > > }; > > > > port@1 { > > reg = <1>; > > label = "lan3"; > > + phy-mode = "internal"; > > + phy-handle = <&sw_phy1>; > > }; > > > > port@2 { > > reg = <2>; > > label = "lan2"; > > + phy-mode = "internal"; > > + phy-handle = <&sw_phy2>; > > }; > > > > port@3 { > > reg = <3>; > > label = "lan1"; > > + phy-mode = "internal"; > > + phy-handle = <&sw_phy3>; > > }; > > > > port@5 { > > reg = <5>; > > label = "cpu"; > > ethernet = <&fec>; > > + phy-mode = "rgmii-id"; > > + > > + fixed-link { > > + speed = <1000>; > > + full-duplex; > > + }; > > }; > > }; > > }; > > diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c > > index c06630a66b66..bef3f7ef0d2b 100644 > > --- a/board/gateworks/gw_ventana/gw_ventana.c > > +++ b/board/gateworks/gw_ventana/gw_ventana.c > > @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev) > > phy_write(phydev, MDIO_DEVAD_NONE, 14, val); > > } > > > > + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */ > > + else if (phydev->phy_id == 0xa5a55a5a && > > PHY_FIXED_ID, but see below. > > > + ((board_type == GW5904) || (board_type == GW5909))) { > > + struct mii_dev *bus = miiphy_get_dev_by_name("mdio"); > > + > > + puts("MV88E61XX "); > > + /* GPIO[0] output CLK125 for RGMII_REFCLK */ > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe); > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7); > > + > > + /* Port 0-3 LED configuration: Table 80/82 */ > > + /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > > + bus->write(bus, 0x10, 0, 0x16, 0x8088); > > + bus->write(bus, 0x11, 0, 0x16, 0x8088); > > + bus->write(bus, 0x12, 0, 0x16, 0x8088); > > + bus->write(bus, 0x13, 0, 0x16, 0x8088); > > + } > > + > > There's nothing too board specific about this configuration, why do you > feel it does not belong to the DSA driver? > > Some macros hiding away magic register addresses and values would be > good either way. > I don't much care for MAC/PHY drivers configuring GPIO's and LED's due to the lack of consistent dt bindings for such a thing and I wasn't planning on trying to enhance the capabilities of the mv88e6xxx driver. There are 7 functions each of the 15 GPIO's can be set to: 0 - GPIO 1 - PTP_TRIG - Precise Timing Protocol Trigger Generate Output 2 - PTP_EVREQ - Precise Timing Protocol Event Request Input 3 - PTP_EXTCLK - Precise Timing Protocol ExternalClock Input 4 - SE_RCLK0 - SyncE Receive Clock 0 Output 5 - SE_RCLK1 - SyncE Receive Clock 1 Output 6 - Reserved 7 - CLK125 - Free running 125 MHz Clock Output There are two LED's per port each of which can be set to 16 different functions also and these functions take a lot of words to describe thus probably wouldn't lend well to #define names. Are there dt bindings that you would suggest I look at for per-port LED config on a switch, or GPIO config on a switch? If I add dt-bindings I'll have to update the kernel driver as well which again, was not my goal here. Maybe moving these into the mv88e6xxx driver per dt bindings could be a 'todo'. This patch isn't making what is already in the board file more obscure, it is just updating it to work with the new dsa driver. The following was what this patch replaced: -#ifdef CONFIG_MV88E61XX_SWITCH -int mv88e61xx_hw_reset(struct phy_device *phydev) -{ - struct mii_dev *bus = phydev->bus; - - /* GPIO[0] output, CLK125 */ - debug("enabling RGMII_REFCLK\n"); - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, - 0x1a /*MV_SCRATCH_MISC*/, - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, - 0x1a /*MV_SCRATCH_MISC*/, - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); - - /* RGMII delay - Physical Control register bit[15:14] */ - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); - /* forced 1000mbps full-duplex link */ - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); - phydev->autoneg = AUTONEG_DISABLE; - phydev->speed = SPEED_1000; - phydev->duplex = DUPLEX_FULL; - - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ - bus->write(bus, 0x10, 0, 0x16, 0x8088); - bus->write(bus, 0x11, 0, 0x16, 0x8088); - bus->write(bus, 0x12, 0, 0x16, 0x8088); - bus->write(bus, 0x13, 0, 0x16, 0x8088); - - return 0; -} -#endif // CONFIG_MV88E61XX_SWITCH Best Regards, Tim > > if (phydev->drv->config) > > phydev->drv->config(phydev); > > > > return 0; > > } > > > > -#ifdef CONFIG_MV88E61XX_SWITCH > > -int mv88e61xx_hw_reset(struct phy_device *phydev) > > -{ > > - struct mii_dev *bus = phydev->bus; > > - > > - /* GPIO[0] output, CLK125 */ > > - debug("enabling RGMII_REFCLK\n"); > > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > > - 0x1a /*MV_SCRATCH_MISC*/, > > - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); > > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > > - 0x1a /*MV_SCRATCH_MISC*/, > > - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); > > - > > - /* RGMII delay - Physical Control register bit[15:14] */ > > - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); > > - /* forced 1000mbps full-duplex link */ > > - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); > > - phydev->autoneg = AUTONEG_DISABLE; > > - phydev->speed = SPEED_1000; > > - phydev->duplex = DUPLEX_FULL; > > - > > - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > > - bus->write(bus, 0x10, 0, 0x16, 0x8088); > > - bus->write(bus, 0x11, 0, 0x16, 0x8088); > > - bus->write(bus, 0x12, 0, 0x16, 0x8088); > > - bus->write(bus, 0x13, 0, 0x16, 0x8088); > > - > > - return 0; > > -} > > -#endif // CONFIG_MV88E61XX_SWITCH > > - > > #if defined(CONFIG_VIDEO_IPUV3) > > static void enable_hdmi(struct display_info_t const *dev) > > { > > diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig > > index d25a8324b1df..9809bc691508 100644 > > --- a/configs/gwventana_gw5904_defconfig > > +++ b/configs/gwventana_gw5904_defconfig > > @@ -103,12 +103,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y > > CONFIG_FSL_USDHC=y > > CONFIG_MTD=y > > CONFIG_PHYLIB=y > > -CONFIG_MV88E61XX_SWITCH=y > > -CONFIG_MV88E61XX_CPU_PORT=5 > > -CONFIG_MV88E61XX_PHY_PORTS=0xf > > -CONFIG_MV88E61XX_FIXED_PORTS=0x0 > > +CONFIG_MV88E61XX=y > > +CONFIG_PHY_FIXED=y > > CONFIG_DM_ETH=y > > CONFIG_DM_MDIO=y > > +CONFIG_DM_DSA=y > > CONFIG_E1000=y > > CONFIG_FEC_MXC=y > > CONFIG_MII=y > > -- > > 2.17.1 > >
On Tue, Jun 21, 2022 at 09:57:35AM -0700, Tim Harvey wrote: > > > diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c > > > index c06630a66b66..bef3f7ef0d2b 100644 > > > --- a/board/gateworks/gw_ventana/gw_ventana.c > > > +++ b/board/gateworks/gw_ventana/gw_ventana.c > > > @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev) > > > phy_write(phydev, MDIO_DEVAD_NONE, 14, val); > > > } > > > > > > + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */ > > > + else if (phydev->phy_id == 0xa5a55a5a && > > > + ((board_type == GW5904) || (board_type == GW5909))) { > > > + struct mii_dev *bus = miiphy_get_dev_by_name("mdio"); > > > + > > > + puts("MV88E61XX "); > > > + /* GPIO[0] output CLK125 for RGMII_REFCLK */ > > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe); > > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7); > > > + > > > + /* Port 0-3 LED configuration: Table 80/82 */ > > > + /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > > > + bus->write(bus, 0x10, 0, 0x16, 0x8088); > > > + bus->write(bus, 0x11, 0, 0x16, 0x8088); > > > + bus->write(bus, 0x12, 0, 0x16, 0x8088); > > > + bus->write(bus, 0x13, 0, 0x16, 0x8088); > > > + } > > > + > > > > There's nothing too board specific about this configuration, why do you > > feel it does not belong to the DSA driver? > > > > Some macros hiding away magic register addresses and values would be > > good either way. > > > > I don't much care for MAC/PHY drivers configuring GPIO's and LED's due > to the lack of consistent dt bindings for such a thing and I wasn't > planning on trying to enhance the capabilities of the mv88e6xxx > driver. > > There are 7 functions each of the 15 GPIO's can be set to: > 0 - GPIO > 1 - PTP_TRIG - Precise Timing Protocol Trigger Generate Output > 2 - PTP_EVREQ - Precise Timing Protocol Event Request Input > 3 - PTP_EXTCLK - Precise Timing Protocol ExternalClock Input > 4 - SE_RCLK0 - SyncE Receive Clock 0 Output > 5 - SE_RCLK1 - SyncE Receive Clock 1 Output > 6 - Reserved > 7 - CLK125 - Free running 125 MHz Clock Output > > There are two LED's per port each of which can be set to 16 different > functions also and these functions take a lot of words to describe > thus probably wouldn't lend well to #define names. > > Are there dt bindings that you would suggest I look at for per-port > LED config on a switch, or GPIO config on a switch? If I add > dt-bindings I'll have to update the kernel driver as well which again, > was not my goal here. Maybe moving these into the mv88e6xxx driver per > dt bindings could be a 'todo'. > > This patch isn't making what is already in the board file more > obscure, it is just updating it to work with the new dsa driver. The > following was what this patch replaced: > -#ifdef CONFIG_MV88E61XX_SWITCH > -int mv88e61xx_hw_reset(struct phy_device *phydev) > -{ > - struct mii_dev *bus = phydev->bus; > - > - /* GPIO[0] output, CLK125 */ > - debug("enabling RGMII_REFCLK\n"); > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > - 0x1a /*MV_SCRATCH_MISC*/, > - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > - 0x1a /*MV_SCRATCH_MISC*/, > - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); > - > - /* RGMII delay - Physical Control register bit[15:14] */ > - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); > - /* forced 1000mbps full-duplex link */ > - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); > - phydev->autoneg = AUTONEG_DISABLE; > - phydev->speed = SPEED_1000; > - phydev->duplex = DUPLEX_FULL; > - > - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > - bus->write(bus, 0x10, 0, 0x16, 0x8088); > - bus->write(bus, 0x11, 0, 0x16, 0x8088); > - bus->write(bus, 0x12, 0, 0x16, 0x8088); > - bus->write(bus, 0x13, 0, 0x16, 0x8088); > - > - return 0; > -} > -#endif // CONFIG_MV88E61XX_SWITCH > > Best Regards, > > Tim Ok, I was thinking PHY LED configuration could be hardcoded in the driver itself (no DT bindings) and nobody would probably even notice. For pin configuration as RGMII 125 MHz clock or GPIO or otherwise, there would probably need to be a "pinctrl" DT subnode with a specific pinctrl driver attached. It's best if the development for that would be done in concert with the Linux community, perhaps even in Linux first. In any case, from my side it's ok if you leave a pinctrl sub-driver as TODO.
On Thu, Jun 23, 2022 at 5:42 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > On Tue, Jun 21, 2022 at 09:57:35AM -0700, Tim Harvey wrote: > > > > diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c > > > > index c06630a66b66..bef3f7ef0d2b 100644 > > > > --- a/board/gateworks/gw_ventana/gw_ventana.c > > > > +++ b/board/gateworks/gw_ventana/gw_ventana.c > > > > @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev) > > > > phy_write(phydev, MDIO_DEVAD_NONE, 14, val); > > > > } > > > > > > > > + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */ > > > > + else if (phydev->phy_id == 0xa5a55a5a && > > > > + ((board_type == GW5904) || (board_type == GW5909))) { > > > > + struct mii_dev *bus = miiphy_get_dev_by_name("mdio"); > > > > + > > > > + puts("MV88E61XX "); > > > > + /* GPIO[0] output CLK125 for RGMII_REFCLK */ > > > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe); > > > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7); > > > > + > > > > + /* Port 0-3 LED configuration: Table 80/82 */ > > > > + /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > > > > + bus->write(bus, 0x10, 0, 0x16, 0x8088); > > > > + bus->write(bus, 0x11, 0, 0x16, 0x8088); > > > > + bus->write(bus, 0x12, 0, 0x16, 0x8088); > > > > + bus->write(bus, 0x13, 0, 0x16, 0x8088); > > > > + } > > > > + > > > > > > There's nothing too board specific about this configuration, why do you > > > feel it does not belong to the DSA driver? > > > > > > Some macros hiding away magic register addresses and values would be > > > good either way. > > > > > > > I don't much care for MAC/PHY drivers configuring GPIO's and LED's due > > to the lack of consistent dt bindings for such a thing and I wasn't > > planning on trying to enhance the capabilities of the mv88e6xxx > > driver. > > > > There are 7 functions each of the 15 GPIO's can be set to: > > 0 - GPIO > > 1 - PTP_TRIG - Precise Timing Protocol Trigger Generate Output > > 2 - PTP_EVREQ - Precise Timing Protocol Event Request Input > > 3 - PTP_EXTCLK - Precise Timing Protocol ExternalClock Input > > 4 - SE_RCLK0 - SyncE Receive Clock 0 Output > > 5 - SE_RCLK1 - SyncE Receive Clock 1 Output > > 6 - Reserved > > 7 - CLK125 - Free running 125 MHz Clock Output > > > > There are two LED's per port each of which can be set to 16 different > > functions also and these functions take a lot of words to describe > > thus probably wouldn't lend well to #define names. > > > > Are there dt bindings that you would suggest I look at for per-port > > LED config on a switch, or GPIO config on a switch? If I add > > dt-bindings I'll have to update the kernel driver as well which again, > > was not my goal here. Maybe moving these into the mv88e6xxx driver per > > dt bindings could be a 'todo'. > > > > This patch isn't making what is already in the board file more > > obscure, it is just updating it to work with the new dsa driver. The > > following was what this patch replaced: > > -#ifdef CONFIG_MV88E61XX_SWITCH > > -int mv88e61xx_hw_reset(struct phy_device *phydev) > > -{ > > - struct mii_dev *bus = phydev->bus; > > - > > - /* GPIO[0] output, CLK125 */ > > - debug("enabling RGMII_REFCLK\n"); > > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > > - 0x1a /*MV_SCRATCH_MISC*/, > > - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); > > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > > - 0x1a /*MV_SCRATCH_MISC*/, > > - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); > > - > > - /* RGMII delay - Physical Control register bit[15:14] */ > > - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); > > - /* forced 1000mbps full-duplex link */ > > - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); > > - phydev->autoneg = AUTONEG_DISABLE; > > - phydev->speed = SPEED_1000; > > - phydev->duplex = DUPLEX_FULL; > > - > > - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > > - bus->write(bus, 0x10, 0, 0x16, 0x8088); > > - bus->write(bus, 0x11, 0, 0x16, 0x8088); > > - bus->write(bus, 0x12, 0, 0x16, 0x8088); > > - bus->write(bus, 0x13, 0, 0x16, 0x8088); > > - > > - return 0; > > -} > > -#endif // CONFIG_MV88E61XX_SWITCH > > > > Best Regards, > > > > Tim > > Ok, I was thinking PHY LED configuration could be hardcoded in the > driver itself (no DT bindings) and nobody would probably even notice. No, it is always a bad idea to hard code a specific configuration in a driver. Most modern PHY's have 4 LEDs with 16 configurations and board vendors vary on what 2 LED's they use and how. I have seen this done in PHY drivers and it makes no sense to inflict your LED policy on other users without doing it in a configurable dt way and avoiding changes if a configuration is not found for backwards compatibility. > For pin configuration as RGMII 125 MHz clock or GPIO or otherwise, > there would probably need to be a "pinctrl" DT subnode with a specific > pinctrl driver attached. It's best if the development for that would be > done in concert with the Linux community, perhaps even in Linux first. > > In any case, from my side it's ok if you leave a pinctrl sub-driver as TODO. Can I have your Reviewed-By on this patch or do you want me to update this with a 'TODO' comment? Best Regards, Tim
On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote: > Add MV88E61XX DSA support: > - update dt: U-Boot dsa driver requires different device-tree syntax > than the linux driver in order to link the dsa ports to the mdio bus. > - update defconfig > - replace mv88e61xx_hw_reset weak override with board_phy_config support > for mv88e61xx configuration that is outside the scope of the DSA driver > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > --- > v3: > - move mdio's mdio@0 subnode > v2: no changes > --- > arch/arm/dts/imx6qdl-gw5904.dtsi | 41 ++++++++++++++++++++ > board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++---------------- > configs/gwventana_gw5904_defconfig | 7 ++-- > 3 files changed, 62 insertions(+), 36 deletions(-) > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi > index 286c7a9924c2..1b2f70d1ccb2 100644 > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi > @@ -219,6 +219,33 @@ > compatible = "marvell,mv88e6085"; > reg = <0>; > > + mdios { > + #address-cells = <1>; > + #size-cells = <0>; > + > + mdio@0 { If you are going to follow this new model with a dedicated "mdios" subnode, I've consulted with Andrew Lunn and Florian Fainelli and there shouldn't be a problem to later make Linux accept this alternate binding format. But in that case, please match this OF node by a dedicated compatible string, like "marvell,mv88e6xxx-mdio-internal", so that there will be a way to differentiate this from the existing "marvell,mv88e6xxx-mdio-external" when support for that is added in U-Boot. Alternatively, to repeat myself, you can always follow the de-facto bindings for Linux mv88e6xxx which have: switch0: switch0@0 { compatible = "marvell,mv88e6190"; ports { #address-cells = <1>; #size-cells = <0>; ... }; mdio { // internal #address-cells = <1>; #size-cells = <0>; ... }; mdio1 { compatible = "marvell,mv88e6xxx-mdio-external"; #address-cells = <1>; #size-cells = <0>; ... }; }; and the associated parsing code: static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip, struct device_node *np) { struct device_node *child; int err; /* Always register one mdio bus for the internal/default mdio * bus. This maybe represented in the device tree, but is * optional. */ child = of_get_child_by_name(np, "mdio"); err = mv88e6xxx_mdio_register(chip, child, false); of_node_put(child); if (err) return err; /* Walk the device tree, and see if there are any other nodes * which say they are compatible with the external mdio * bus. */ for_each_available_child_of_node(np, child) { if (of_device_is_compatible( child, "marvell,mv88e6xxx-mdio-external")) { err = mv88e6xxx_mdio_register(chip, child, true); if (err) { mv88e6xxx_mdios_unregister(chip); of_node_put(child); return err; } } } return 0; } Personally I believe that if you have limited amount of time to spend on U-Boot DM_DSA and DT bindings in general, you should just follow the format already accepted by Linux ("mdio" node is for internal MDIO, doesn't have compatible string, is placed directly under "switch" node", while external MDIO is matched by compatible string and its node can have any name). What we should try to accomplish is that the DT blob that U-Boot creates for itself here to be coherent enough that Linux is able to understand and use it, in case we decide to pass it to the operating system. With your approach you'd have work to do in Linux as well to accept the "mdios" subnode and parse controllers by compatible string inside, and I'm not exactly sure you're willing to do that. > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + sw_phy0: ethernet-phy@0 { > + reg = <0x0>; > + }; > + > + sw_phy1: ethernet-phy@1 { > + reg = <0x1>; > + }; > + > + sw_phy2: ethernet-phy@2 { > + reg = <0x2>; > + }; > + > + sw_phy3: ethernet-phy@3 { > + reg = <0x3>; > + }; > + }; > + }; > + > ports { > #address-cells = <1>; > #size-cells = <0>; > @@ -226,27 +253,41 @@ > port@0 { > reg = <0>; > label = "lan4"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy0>; > }; > > port@1 { > reg = <1>; > label = "lan3"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy1>; > }; > > port@2 { > reg = <2>; > label = "lan2"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy2>; > }; > > port@3 { > reg = <3>; > label = "lan1"; > + phy-mode = "internal"; > + phy-handle = <&sw_phy3>; > }; > > port@5 { > reg = <5>; > label = "cpu"; > ethernet = <&fec>; > + phy-mode = "rgmii-id"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > }; > }; > }; > diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c > index c06630a66b66..bef3f7ef0d2b 100644 > --- a/board/gateworks/gw_ventana/gw_ventana.c > +++ b/board/gateworks/gw_ventana/gw_ventana.c > @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev) > phy_write(phydev, MDIO_DEVAD_NONE, 14, val); > } > > + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */ > + else if (phydev->phy_id == 0xa5a55a5a && > + ((board_type == GW5904) || (board_type == GW5909))) { > + struct mii_dev *bus = miiphy_get_dev_by_name("mdio"); > + > + puts("MV88E61XX "); > + /* GPIO[0] output CLK125 for RGMII_REFCLK */ > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe); > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7); > + > + /* Port 0-3 LED configuration: Table 80/82 */ > + /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > + bus->write(bus, 0x10, 0, 0x16, 0x8088); > + bus->write(bus, 0x11, 0, 0x16, 0x8088); > + bus->write(bus, 0x12, 0, 0x16, 0x8088); > + bus->write(bus, 0x13, 0, 0x16, 0x8088); > + } > + > if (phydev->drv->config) > phydev->drv->config(phydev); > > return 0; > } > > -#ifdef CONFIG_MV88E61XX_SWITCH > -int mv88e61xx_hw_reset(struct phy_device *phydev) > -{ > - struct mii_dev *bus = phydev->bus; > - > - /* GPIO[0] output, CLK125 */ > - debug("enabling RGMII_REFCLK\n"); > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > - 0x1a /*MV_SCRATCH_MISC*/, > - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > - 0x1a /*MV_SCRATCH_MISC*/, > - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); > - > - /* RGMII delay - Physical Control register bit[15:14] */ > - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); > - /* forced 1000mbps full-duplex link */ > - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); > - phydev->autoneg = AUTONEG_DISABLE; > - phydev->speed = SPEED_1000; > - phydev->duplex = DUPLEX_FULL; > - > - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > - bus->write(bus, 0x10, 0, 0x16, 0x8088); > - bus->write(bus, 0x11, 0, 0x16, 0x8088); > - bus->write(bus, 0x12, 0, 0x16, 0x8088); > - bus->write(bus, 0x13, 0, 0x16, 0x8088); > - > - return 0; > -} > -#endif // CONFIG_MV88E61XX_SWITCH > - > #if defined(CONFIG_VIDEO_IPUV3) > static void enable_hdmi(struct display_info_t const *dev) > { > diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig > index d25a8324b1df..9809bc691508 100644 > --- a/configs/gwventana_gw5904_defconfig > +++ b/configs/gwventana_gw5904_defconfig > @@ -103,12 +103,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y > CONFIG_FSL_USDHC=y > CONFIG_MTD=y > CONFIG_PHYLIB=y > -CONFIG_MV88E61XX_SWITCH=y > -CONFIG_MV88E61XX_CPU_PORT=5 > -CONFIG_MV88E61XX_PHY_PORTS=0xf > -CONFIG_MV88E61XX_FIXED_PORTS=0x0 > +CONFIG_MV88E61XX=y > +CONFIG_PHY_FIXED=y > CONFIG_DM_ETH=y > CONFIG_DM_MDIO=y > +CONFIG_DM_DSA=y > CONFIG_E1000=y > CONFIG_FEC_MXC=y > CONFIG_MII=y > -- > 2.17.1 >
On Fri, Jun 24, 2022 at 3:25 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote: > > Add MV88E61XX DSA support: > > - update dt: U-Boot dsa driver requires different device-tree syntax > > than the linux driver in order to link the dsa ports to the mdio bus. > > - update defconfig > > - replace mv88e61xx_hw_reset weak override with board_phy_config support > > for mv88e61xx configuration that is outside the scope of the DSA driver > > > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > > --- > > v3: > > - move mdio's mdio@0 subnode > > v2: no changes > > --- > > arch/arm/dts/imx6qdl-gw5904.dtsi | 41 ++++++++++++++++++++ > > board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++---------------- > > configs/gwventana_gw5904_defconfig | 7 ++-- > > 3 files changed, 62 insertions(+), 36 deletions(-) > > > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi > > index 286c7a9924c2..1b2f70d1ccb2 100644 > > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi > > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi > > @@ -219,6 +219,33 @@ > > compatible = "marvell,mv88e6085"; > > reg = <0>; > > > > + mdios { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + mdio@0 { > > If you are going to follow this new model with a dedicated "mdios" subnode, > I've consulted with Andrew Lunn and Florian Fainelli and there shouldn't > be a problem to later make Linux accept this alternate binding format. > But in that case, please match this OF node by a dedicated compatible > string, like "marvell,mv88e6xxx-mdio-internal", so that there will be a > way to differentiate this from the existing "marvell,mv88e6xxx-mdio-external" > when support for that is added in U-Boot. > > Alternatively, to repeat myself, you can always follow the de-facto > bindings for Linux mv88e6xxx which have: > > switch0: switch0@0 { > compatible = "marvell,mv88e6190"; > > ports { > #address-cells = <1>; > #size-cells = <0>; > > ... > }; > > mdio { // internal > #address-cells = <1>; > #size-cells = <0>; > > ... > }; > > mdio1 { > compatible = "marvell,mv88e6xxx-mdio-external"; > #address-cells = <1>; > #size-cells = <0>; > > ... > }; > }; > Documentation/devicetree/bindings/net/dsa/marvell.txt shows en example with just one child node under the internal mdio node: mdio { #address-cells = <1>; #size-cells = <0>; switch1phy0: switch1phy0@0 { reg = <0>; interrupt-parent = <&switch0>; interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; }; }; Am I to assume I can add additional nodes there for the other ports such as the following? mdio { #address-cells = <1>; #size-cells = <0>; switch1phy0: switch1phy0@0 { reg = <0>; }; switch1phy1: switch1phy1@1 { reg = <1>; }; switch1phy2: switch1phy2@2 { reg = <2>; }; switch1phy3: switch1phy3@3 { reg = <3>; }; ... }; Best Regards, Tim > and the associated parsing code: > > static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip, > struct device_node *np) > { > struct device_node *child; > int err; > > /* Always register one mdio bus for the internal/default mdio > * bus. This maybe represented in the device tree, but is > * optional. > */ > child = of_get_child_by_name(np, "mdio"); > err = mv88e6xxx_mdio_register(chip, child, false); > of_node_put(child); > if (err) > return err; > > /* Walk the device tree, and see if there are any other nodes > * which say they are compatible with the external mdio > * bus. > */ > for_each_available_child_of_node(np, child) { > if (of_device_is_compatible( > child, "marvell,mv88e6xxx-mdio-external")) { > err = mv88e6xxx_mdio_register(chip, child, true); > if (err) { > mv88e6xxx_mdios_unregister(chip); > of_node_put(child); > return err; > } > } > } > > return 0; > } > > Personally I believe that if you have limited amount of time to spend on > U-Boot DM_DSA and DT bindings in general, you should just follow the > format already accepted by Linux ("mdio" node is for internal MDIO, > doesn't have compatible string, is placed directly under "switch" node", > while external MDIO is matched by compatible string and its node can > have any name). > > What we should try to accomplish is that the DT blob that U-Boot creates > for itself here to be coherent enough that Linux is able to understand > and use it, in case we decide to pass it to the operating system. With > your approach you'd have work to do in Linux as well to accept the > "mdios" subnode and parse controllers by compatible string inside, and > I'm not exactly sure you're willing to do that. > > > + reg = <0>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + sw_phy0: ethernet-phy@0 { > > + reg = <0x0>; > > + }; > > + > > + sw_phy1: ethernet-phy@1 { > > + reg = <0x1>; > > + }; > > + > > + sw_phy2: ethernet-phy@2 { > > + reg = <0x2>; > > + }; > > + > > + sw_phy3: ethernet-phy@3 { > > + reg = <0x3>; > > + }; > > + }; > > + }; > > + > > ports { > > #address-cells = <1>; > > #size-cells = <0>; > > @@ -226,27 +253,41 @@ > > port@0 { > > reg = <0>; > > label = "lan4"; > > + phy-mode = "internal"; > > + phy-handle = <&sw_phy0>; > > }; > > > > port@1 { > > reg = <1>; > > label = "lan3"; > > + phy-mode = "internal"; > > + phy-handle = <&sw_phy1>; > > }; > > > > port@2 { > > reg = <2>; > > label = "lan2"; > > + phy-mode = "internal"; > > + phy-handle = <&sw_phy2>; > > }; > > > > port@3 { > > reg = <3>; > > label = "lan1"; > > + phy-mode = "internal"; > > + phy-handle = <&sw_phy3>; > > }; > > > > port@5 { > > reg = <5>; > > label = "cpu"; > > ethernet = <&fec>; > > + phy-mode = "rgmii-id"; > > + > > + fixed-link { > > + speed = <1000>; > > + full-duplex; > > + }; > > }; > > }; > > }; > > diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c > > index c06630a66b66..bef3f7ef0d2b 100644 > > --- a/board/gateworks/gw_ventana/gw_ventana.c > > +++ b/board/gateworks/gw_ventana/gw_ventana.c > > @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev) > > phy_write(phydev, MDIO_DEVAD_NONE, 14, val); > > } > > > > + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */ > > + else if (phydev->phy_id == 0xa5a55a5a && > > + ((board_type == GW5904) || (board_type == GW5909))) { > > + struct mii_dev *bus = miiphy_get_dev_by_name("mdio"); > > + > > + puts("MV88E61XX "); > > + /* GPIO[0] output CLK125 for RGMII_REFCLK */ > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe); > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7); > > + > > + /* Port 0-3 LED configuration: Table 80/82 */ > > + /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > > + bus->write(bus, 0x10, 0, 0x16, 0x8088); > > + bus->write(bus, 0x11, 0, 0x16, 0x8088); > > + bus->write(bus, 0x12, 0, 0x16, 0x8088); > > + bus->write(bus, 0x13, 0, 0x16, 0x8088); > > + } > > + > > if (phydev->drv->config) > > phydev->drv->config(phydev); > > > > return 0; > > } > > > > -#ifdef CONFIG_MV88E61XX_SWITCH > > -int mv88e61xx_hw_reset(struct phy_device *phydev) > > -{ > > - struct mii_dev *bus = phydev->bus; > > - > > - /* GPIO[0] output, CLK125 */ > > - debug("enabling RGMII_REFCLK\n"); > > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > > - 0x1a /*MV_SCRATCH_MISC*/, > > - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); > > - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, > > - 0x1a /*MV_SCRATCH_MISC*/, > > - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); > > - > > - /* RGMII delay - Physical Control register bit[15:14] */ > > - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); > > - /* forced 1000mbps full-duplex link */ > > - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); > > - phydev->autoneg = AUTONEG_DISABLE; > > - phydev->speed = SPEED_1000; > > - phydev->duplex = DUPLEX_FULL; > > - > > - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ > > - bus->write(bus, 0x10, 0, 0x16, 0x8088); > > - bus->write(bus, 0x11, 0, 0x16, 0x8088); > > - bus->write(bus, 0x12, 0, 0x16, 0x8088); > > - bus->write(bus, 0x13, 0, 0x16, 0x8088); > > - > > - return 0; > > -} > > -#endif // CONFIG_MV88E61XX_SWITCH > > - > > #if defined(CONFIG_VIDEO_IPUV3) > > static void enable_hdmi(struct display_info_t const *dev) > > { > > diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig > > index d25a8324b1df..9809bc691508 100644 > > --- a/configs/gwventana_gw5904_defconfig > > +++ b/configs/gwventana_gw5904_defconfig > > @@ -103,12 +103,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y > > CONFIG_FSL_USDHC=y > > CONFIG_MTD=y > > CONFIG_PHYLIB=y > > -CONFIG_MV88E61XX_SWITCH=y > > -CONFIG_MV88E61XX_CPU_PORT=5 > > -CONFIG_MV88E61XX_PHY_PORTS=0xf > > -CONFIG_MV88E61XX_FIXED_PORTS=0x0 > > +CONFIG_MV88E61XX=y > > +CONFIG_PHY_FIXED=y > > CONFIG_DM_ETH=y > > CONFIG_DM_MDIO=y > > +CONFIG_DM_DSA=y > > CONFIG_E1000=y > > CONFIG_FEC_MXC=y > > CONFIG_MII=y > > -- > > 2.17.1 > >
On Fri, Jun 24, 2022 at 04:16:34PM -0700, Tim Harvey wrote: > On Fri, Jun 24, 2022 at 3:25 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > > > On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote: > > > Add MV88E61XX DSA support: > > > - update dt: U-Boot dsa driver requires different device-tree syntax > > > than the linux driver in order to link the dsa ports to the mdio bus. > > > - update defconfig > > > - replace mv88e61xx_hw_reset weak override with board_phy_config support > > > for mv88e61xx configuration that is outside the scope of the DSA driver > > > > > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > > > --- > > > v3: > > > - move mdio's mdio@0 subnode > > > v2: no changes > > > --- > > > arch/arm/dts/imx6qdl-gw5904.dtsi | 41 ++++++++++++++++++++ > > > board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++---------------- > > > configs/gwventana_gw5904_defconfig | 7 ++-- > > > 3 files changed, 62 insertions(+), 36 deletions(-) > > > > > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi > > > index 286c7a9924c2..1b2f70d1ccb2 100644 > > > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi > > > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi > > > @@ -219,6 +219,33 @@ > > > compatible = "marvell,mv88e6085"; > > > reg = <0>; > > > > > > + mdios { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + mdio@0 { > > > > If you are going to follow this new model with a dedicated "mdios" subnode, > > I've consulted with Andrew Lunn and Florian Fainelli and there shouldn't > > be a problem to later make Linux accept this alternate binding format. > > But in that case, please match this OF node by a dedicated compatible > > string, like "marvell,mv88e6xxx-mdio-internal", so that there will be a > > way to differentiate this from the existing "marvell,mv88e6xxx-mdio-external" > > when support for that is added in U-Boot. > > > > Alternatively, to repeat myself, you can always follow the de-facto > > bindings for Linux mv88e6xxx which have: > > > > switch0: switch0@0 { > > compatible = "marvell,mv88e6190"; > > > > ports { > > #address-cells = <1>; > > #size-cells = <0>; > > > > ... > > }; > > > > mdio { // internal > > #address-cells = <1>; > > #size-cells = <0>; > > > > ... > > }; > > > > mdio1 { > > compatible = "marvell,mv88e6xxx-mdio-external"; > > #address-cells = <1>; > > #size-cells = <0>; > > > > ... > > }; > > }; > > > > Documentation/devicetree/bindings/net/dsa/marvell.txt shows en example > with just one child node under the internal mdio node: > > mdio { > #address-cells = <1>; > #size-cells = <0>; > switch1phy0: switch1phy0@0 { > reg = <0>; > interrupt-parent = <&switch0>; > interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; > }; > }; > > Am I to assume I can add additional nodes there for the other ports > such as the following? > > mdio { > #address-cells = <1>; > #size-cells = <0>; > > switch1phy0: switch1phy0@0 { > reg = <0>; > }; > > switch1phy1: switch1phy1@1 { > reg = <1>; > }; > > switch1phy2: switch1phy2@2 { > reg = <2>; > }; > > switch1phy3: switch1phy3@3 { > reg = <3>; > }; > > ... > }; Sure, but name those PHY nodes "ethernet-phy@N" rather than "switchMphyN", as Documentation/devicetree/bindings/net/ethernet-phy.yaml requires. Many mistakes were made in writing mv88e6xxx device trees, we don't need to follow each and every one of them, only the important ones.
On Fri, Jun 24, 2022 at 5:13 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > On Fri, Jun 24, 2022 at 04:16:34PM -0700, Tim Harvey wrote: > > On Fri, Jun 24, 2022 at 3:25 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > > > > > On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote: > > > > Add MV88E61XX DSA support: > > > > - update dt: U-Boot dsa driver requires different device-tree syntax > > > > than the linux driver in order to link the dsa ports to the mdio bus. > > > > - update defconfig > > > > - replace mv88e61xx_hw_reset weak override with board_phy_config support > > > > for mv88e61xx configuration that is outside the scope of the DSA driver > > > > > > > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > > > > --- > > > > v3: > > > > - move mdio's mdio@0 subnode > > > > v2: no changes > > > > --- > > > > arch/arm/dts/imx6qdl-gw5904.dtsi | 41 ++++++++++++++++++++ > > > > board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++---------------- > > > > configs/gwventana_gw5904_defconfig | 7 ++-- > > > > 3 files changed, 62 insertions(+), 36 deletions(-) > > > > > > > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi > > > > index 286c7a9924c2..1b2f70d1ccb2 100644 > > > > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi > > > > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi > > > > @@ -219,6 +219,33 @@ > > > > compatible = "marvell,mv88e6085"; > > > > reg = <0>; > > > > > > > > + mdios { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + mdio@0 { > > > > > > If you are going to follow this new model with a dedicated "mdios" subnode, > > > I've consulted with Andrew Lunn and Florian Fainelli and there shouldn't > > > be a problem to later make Linux accept this alternate binding format. > > > But in that case, please match this OF node by a dedicated compatible > > > string, like "marvell,mv88e6xxx-mdio-internal", so that there will be a > > > way to differentiate this from the existing "marvell,mv88e6xxx-mdio-external" > > > when support for that is added in U-Boot. > > > > > > Alternatively, to repeat myself, you can always follow the de-facto > > > bindings for Linux mv88e6xxx which have: > > > > > > switch0: switch0@0 { > > > compatible = "marvell,mv88e6190"; > > > > > > ports { > > > #address-cells = <1>; > > > #size-cells = <0>; > > > > > > ... > > > }; > > > > > > mdio { // internal > > > #address-cells = <1>; > > > #size-cells = <0>; > > > > > > ... > > > }; > > > > > > mdio1 { > > > compatible = "marvell,mv88e6xxx-mdio-external"; > > > #address-cells = <1>; > > > #size-cells = <0>; > > > > > > ... > > > }; > > > }; > > > > > > > Documentation/devicetree/bindings/net/dsa/marvell.txt shows en example > > with just one child node under the internal mdio node: > > > > mdio { > > #address-cells = <1>; > > #size-cells = <0>; > > switch1phy0: switch1phy0@0 { > > reg = <0>; > > interrupt-parent = <&switch0>; > > interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; > > }; > > }; > > > > Am I to assume I can add additional nodes there for the other ports > > such as the following? > > > > mdio { > > #address-cells = <1>; > > #size-cells = <0>; > > > > switch1phy0: switch1phy0@0 { > > reg = <0>; > > }; > > > > switch1phy1: switch1phy1@1 { > > reg = <1>; > > }; > > > > switch1phy2: switch1phy2@2 { > > reg = <2>; > > }; > > > > switch1phy3: switch1phy3@3 { > > reg = <3>; > > }; > > > > ... > > }; > > Sure, but name those PHY nodes "ethernet-phy@N" rather than "switchMphyN", > as Documentation/devicetree/bindings/net/ethernet-phy.yaml requires. > Many mistakes were made in writing mv88e6xxx device trees, we don't need > to follow each and every one of them, only the important ones. Ok, I'll attempt to go that route. I'm out of the office next week so expect a v4 sometime after that. Best Regards, Tim
diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi index 286c7a9924c2..1b2f70d1ccb2 100644 --- a/arch/arm/dts/imx6qdl-gw5904.dtsi +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi @@ -219,6 +219,33 @@ compatible = "marvell,mv88e6085"; reg = <0>; + mdios { + #address-cells = <1>; + #size-cells = <0>; + + mdio@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + + sw_phy0: ethernet-phy@0 { + reg = <0x0>; + }; + + sw_phy1: ethernet-phy@1 { + reg = <0x1>; + }; + + sw_phy2: ethernet-phy@2 { + reg = <0x2>; + }; + + sw_phy3: ethernet-phy@3 { + reg = <0x3>; + }; + }; + }; + ports { #address-cells = <1>; #size-cells = <0>; @@ -226,27 +253,41 @@ port@0 { reg = <0>; label = "lan4"; + phy-mode = "internal"; + phy-handle = <&sw_phy0>; }; port@1 { reg = <1>; label = "lan3"; + phy-mode = "internal"; + phy-handle = <&sw_phy1>; }; port@2 { reg = <2>; label = "lan2"; + phy-mode = "internal"; + phy-handle = <&sw_phy2>; }; port@3 { reg = <3>; label = "lan1"; + phy-mode = "internal"; + phy-handle = <&sw_phy3>; }; port@5 { reg = <5>; label = "cpu"; ethernet = <&fec>; + phy-mode = "rgmii-id"; + + fixed-link { + speed = <1000>; + full-duplex; + }; }; }; }; diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c index c06630a66b66..bef3f7ef0d2b 100644 --- a/board/gateworks/gw_ventana/gw_ventana.c +++ b/board/gateworks/gw_ventana/gw_ventana.c @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev) phy_write(phydev, MDIO_DEVAD_NONE, 14, val); } + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */ + else if (phydev->phy_id == 0xa5a55a5a && + ((board_type == GW5904) || (board_type == GW5909))) { + struct mii_dev *bus = miiphy_get_dev_by_name("mdio"); + + puts("MV88E61XX "); + /* GPIO[0] output CLK125 for RGMII_REFCLK */ + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) | 0xfe); + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7); + + /* Port 0-3 LED configuration: Table 80/82 */ + /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ + bus->write(bus, 0x10, 0, 0x16, 0x8088); + bus->write(bus, 0x11, 0, 0x16, 0x8088); + bus->write(bus, 0x12, 0, 0x16, 0x8088); + bus->write(bus, 0x13, 0, 0x16, 0x8088); + } + if (phydev->drv->config) phydev->drv->config(phydev); return 0; } -#ifdef CONFIG_MV88E61XX_SWITCH -int mv88e61xx_hw_reset(struct phy_device *phydev) -{ - struct mii_dev *bus = phydev->bus; - - /* GPIO[0] output, CLK125 */ - debug("enabling RGMII_REFCLK\n"); - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, - 0x1a /*MV_SCRATCH_MISC*/, - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe); - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0, - 0x1a /*MV_SCRATCH_MISC*/, - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7); - - /* RGMII delay - Physical Control register bit[15:14] */ - debug("setting port%d RGMII rx/tx delay\n", CONFIG_MV88E61XX_CPU_PORT); - /* forced 1000mbps full-duplex link */ - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe); - phydev->autoneg = AUTONEG_DISABLE; - phydev->speed = SPEED_1000; - phydev->duplex = DUPLEX_FULL; - - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */ - bus->write(bus, 0x10, 0, 0x16, 0x8088); - bus->write(bus, 0x11, 0, 0x16, 0x8088); - bus->write(bus, 0x12, 0, 0x16, 0x8088); - bus->write(bus, 0x13, 0, 0x16, 0x8088); - - return 0; -} -#endif // CONFIG_MV88E61XX_SWITCH - #if defined(CONFIG_VIDEO_IPUV3) static void enable_hdmi(struct display_info_t const *dev) { diff --git a/configs/gwventana_gw5904_defconfig b/configs/gwventana_gw5904_defconfig index d25a8324b1df..9809bc691508 100644 --- a/configs/gwventana_gw5904_defconfig +++ b/configs/gwventana_gw5904_defconfig @@ -103,12 +103,11 @@ CONFIG_SUPPORT_EMMC_BOOT=y CONFIG_FSL_USDHC=y CONFIG_MTD=y CONFIG_PHYLIB=y -CONFIG_MV88E61XX_SWITCH=y -CONFIG_MV88E61XX_CPU_PORT=5 -CONFIG_MV88E61XX_PHY_PORTS=0xf -CONFIG_MV88E61XX_FIXED_PORTS=0x0 +CONFIG_MV88E61XX=y +CONFIG_PHY_FIXED=y CONFIG_DM_ETH=y CONFIG_DM_MDIO=y +CONFIG_DM_DSA=y CONFIG_E1000=y CONFIG_FEC_MXC=y CONFIG_MII=y
Add MV88E61XX DSA support: - update dt: U-Boot dsa driver requires different device-tree syntax than the linux driver in order to link the dsa ports to the mdio bus. - update defconfig - replace mv88e61xx_hw_reset weak override with board_phy_config support for mv88e61xx configuration that is outside the scope of the DSA driver Signed-off-by: Tim Harvey <tharvey@gateworks.com> --- v3: - move mdio's mdio@0 subnode v2: no changes --- arch/arm/dts/imx6qdl-gw5904.dtsi | 41 ++++++++++++++++++++ board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++---------------- configs/gwventana_gw5904_defconfig | 7 ++-- 3 files changed, 62 insertions(+), 36 deletions(-)