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

Vladimir Oltean vladimir.oltean at nxp.com
Wed Oct 5 01:58:47 CEST 2022


On Tue, Oct 04, 2022 at 09:49:18AM -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.

Not really correct. Better said, U-Boot requires a more restrictive
subset of what DT bindings Linux supports, for sake of simplicity of the
code. It is equally valid to link ports to their internal PHY by
phy-handle in the Linux mv88e6xxx driver.

>  - 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
> 
> Note that the dt changes were not placed in a u-boot.dtsi as it is
> intended that these changes get merged with the Linux dt's.
> 
> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> Reviewed-by: Fabio Estevam <festevam at denx.de>
> ---
> v5:
>  - fix typo in defconfig s/MV88E61XX/MV88E6XXX
>  - added Fabio's rb tag
>  - added note to commit log
> v4:
>  - use standard Linux internal MDIO dt structure
>  - use PHY_FIXED_ID define
> v3:
>  - move mdio's mdio at 0 subnode
> v2: no changes
> ---
>  arch/arm/dts/imx6qdl-gw5904.dtsi        | 31 +++++++++++++++
>  board/gateworks/gw_ventana/gw_ventana.c | 50 +++++++++----------------
>  configs/gwventana_gw5904_defconfig      |  7 ++--
>  drivers/net/mv88e6xxx.c                 | 29 ++++++--------
>  4 files changed, 64 insertions(+), 53 deletions(-)
> 
> diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi b/arch/arm/dts/imx6qdl-gw5904.dtsi
> index 612b6e068e28..487dd7648274 100644
> --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> @@ -212,6 +212,27 @@
>  			compatible = "marvell,mv88e6085";
>  			reg = <0>;
>  
> +			mdio {
> +				#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>;
> @@ -219,27 +240,37 @@
>  				port at 0 {
>  					reg = <0>;
>  					label = "lan4";
> +					phy-handle = <&sw_phy0>;
>  				};
>  
>  				port at 1 {
>  					reg = <1>;
>  					label = "lan3";
> +					phy-handle = <&sw_phy1>;
>  				};
>  
>  				port at 2 {
>  					reg = <2>;
>  					label = "lan2";
> +					phy-handle = <&sw_phy2>;
>  				};
>  
>  				port at 3 {
>  					reg = <3>;
>  					label = "lan1";
> +					phy-handle = <&sw_phy3>;
>  				};
>  
>  				port at 5 {
>  					reg = <5>;
>  					label = "cpu";

This is not used, please remove it.

>  					ethernet = <&fec>;
> +					phy-mode = "rgmii-id";
> +
> +					fixed-link {
> +						speed = <1000>;
> +						full-duplex;
> +					};
>  				};
>  			};
>  		};

Binding looks good, thanks.

> diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c
> index 99f52b9953e2..1c3da2c82707 100644
> --- a/board/gateworks/gw_ventana/gw_ventana.c
> +++ b/board/gateworks/gw_ventana/gw_ventana.c
> @@ -83,44 +83,30 @@ int board_phy_config(struct phy_device *phydev)
>  		break;
>  	}
>  
> +	/* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */
> +	if (phydev->phy_id == PHY_FIXED_ID &&
> +	    (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);
> +	}
> +

Ugh. I'll have to get over this.

BTW, does this run before or after the switch driver getting probed?
Don't you lose the config once you reset the switch? How do you ensure
ordering?

>  	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 fb5870fa5803..50115b21e263 100644
> --- a/configs/gwventana_gw5904_defconfig
> +++ b/configs/gwventana_gw5904_defconfig
> @@ -109,13 +109,12 @@ 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_PHY_FIXED=y
>  CONFIG_DM_MDIO=y
> +CONFIG_DM_DSA=y
>  CONFIG_E1000=y
>  CONFIG_FEC_MXC=y
> +CONFIG_MV88E6XXX=y
>  CONFIG_MII=y
>  CONFIG_PCI=y
>  CONFIG_PCIE_IMX=y
> diff --git a/drivers/net/mv88e6xxx.c b/drivers/net/mv88e6xxx.c
> index e59e2464c641..09ce6dd1142d 100644
> --- a/drivers/net/mv88e6xxx.c
> +++ b/drivers/net/mv88e6xxx.c
> @@ -728,31 +728,26 @@ static const struct dsa_ops mv88e6xxx_dsa_ops = {
>  static int mv88e6xxx_probe_mdio(struct udevice *dev)
>  {
>  	struct udevice *mdev;
> -	ofnode node, mdios;
>  	const char *name;
> +	ofnode node;
>  	int ret;
>  
> -	/* bind phy ports of mdios child node to mv88e6xxx_mdio device */
> -	mdios = dev_read_subnode(dev, "mdios");
> -	if (!ofnode_valid(mdios))
> +	/* bind phy ports of mdio child node to mv88e6xxx_mdio device */
> +	node = dev_read_subnode(dev, "mdio");
> +	if (!ofnode_valid(node))
>  		return 0;
>  
> -	ofnode_for_each_subnode(node, mdios) {
> -		name = ofnode_get_name(node);
> -		ret = device_bind_driver_to_node(dev,
> -						 "mv88e6xxx_mdio",
> -						 name, node, NULL);
> -		if (ret) {
> -			dev_err(dev, "failed to bind %s: %d\n", name, ret);
> -			continue;
> -		}
> -
> +	name = ofnode_get_name(node);
> +	ret = device_bind_driver_to_node(dev,
> +					 "mv88e6xxx_mdio",
> +					 name, node, NULL);
> +	if (ret) {
> +		dev_err(dev, "failed to bind %s: %d\n", name, ret);
> +	} else {
>  		/* need to probe it as there is no compatible to do so */
>  		ret = uclass_get_device_by_ofnode(UCLASS_MDIO, node, &mdev);
> -		if (ret) {
> +		if (ret)
>  			dev_err(dev, "failed to probe %s: %d\n", name, ret);
> -			continue;
> -		}
>  	}
>  
>  	return ret;

Please move this hunk into patch 7/8.

> -- 
> 2.25.1
>


More information about the U-Boot mailing list