[PATCH v3 8/8] board: gw_ventana: enable MV88E61XX DSA support

Tim Harvey tharvey at gateworks.com
Tue Jun 21 18:57:35 CEST 2022


On Mon, Jun 20, 2022 at 4:42 AM Vladimir Oltean <vladimir.oltean at 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 at gateworks.com>
> > ---
> > v3:
> >  - move mdio's mdio at 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 at 0 {
> > +                                     reg = <0>;
> > +                                     #address-cells = <1>;
> > +                                     #size-cells = <0>;
> > +
> > +                                     sw_phy0: ethernet-phy at 0 {
> > +                                             reg = <0x0>;
> > +                                     };
> > +
> > +                                     sw_phy1: ethernet-phy at 1 {
> > +                                             reg = <0x1>;
> > +                                     };
> > +
> > +                                     sw_phy2: ethernet-phy at 2 {
> > +                                             reg = <0x2>;
> > +                                     };
> > +
> > +                                     sw_phy3: ethernet-phy at 3 {
> > +                                             reg = <0x3>;
> > +                                     };
> > +                             };
> > +                     };
> > +
> >                       ports {
> >                               #address-cells = <1>;
> >                               #size-cells = <0>;
> > @@ -226,27 +253,41 @@
> >                               port at 0 {
> >                                       reg = <0>;
> >                                       label = "lan4";
> > +                                     phy-mode = "internal";
> > +                                     phy-handle = <&sw_phy0>;
> >                               };
> >
> >                               port at 1 {
> >                                       reg = <1>;
> >                                       label = "lan3";
> > +                                     phy-mode = "internal";
> > +                                     phy-handle = <&sw_phy1>;
> >                               };
> >
> >                               port at 2 {
> >                                       reg = <2>;
> >                                       label = "lan2";
> > +                                     phy-mode = "internal";
> > +                                     phy-handle = <&sw_phy2>;
> >                               };
> >
> >                               port at 3 {
> >                                       reg = <3>;
> >                                       label = "lan1";
> > +                                     phy-mode = "internal";
> > +                                     phy-handle = <&sw_phy3>;
> >                               };
> >
> >                               port at 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
> >


More information about the U-Boot mailing list