[PATCH v3 8/8] board: gw_ventana: enable MV88E61XX DSA support
Vladimir Oltean
vladimir.oltean at nxp.com
Fri Jun 24 12:25:31 CEST 2022
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 {
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 at 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 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 &&
> + ((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
>
More information about the U-Boot
mailing list