Re: [PATCH net-next v3 4/4] net: dsa: initial support for MT7628 embedded switch

[Andrew Lunn <andrew@lunn.ch>]

>  source "drivers/net/dsa/lantiq/Kconfig"
>  
> +config NET_DSA_MT7628
> +	tristate "MediaTek MT7628 Embedded Ethernet switch support"
> +	select NET_DSA_TAG_MT7628
> +	select MEDIATEK_FE_SOC_PHY
> +	help
> +	  This enables support for the built-in Ethernet switch found
> +	  in the MT7628 SoC.
> +
>  config NET_DSA_MT7530
>  	tristate "MediaTek MT7530 and MT7531 Ethernet switch support"

NET_DSA_MT7628 is > NET_DSA_MT7530 so should come second.

> +static int mt7628_mii_read(struct mii_bus *bus, int port, int regnum)
> +{
> +	struct mt7628_esw *esw = bus->priv;
> +	int ret;
> +	u32 val;
> +
> +	ret = regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,
> +				       !(val & MT7628_ESW_PCR1_RD_DONE), 10,
> +				       5000);
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_write(esw->regmap, MT7628_ESW_REG_PCR0,
> +			   FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_REG,
> +				      regnum) |
> +			   FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_ADDR,
> +				      port) | MT7628_ESW_PCR0_RD_PHY_CMD);
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,
> +				       (val & MT7628_ESW_PCR1_RD_DONE), 10,
> +				       5000);
> +out:
> +	if (ret) {
> +		dev_err(&bus->dev, "read failed. MDIO timeout?\n");
> +		return ret;
> +	}
> +	return FIELD_GET(MT7628_ESW_PCR1_RD_DATA, val);

This works, but it is an unusual structure. Normally the label is
after the normal exist path, and only deals with errors.

> +}
> +
> +static int mt7628_mii_write(struct mii_bus *bus, int port, int regnum, u16 dat)
> +{
> +	ret = regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,
> +				       (val & MT7628_ESW_PCR1_WT_DONE), 10,
> +				       5000);
> +out:
> +	if (ret) {
> +		dev_err(&bus->dev, "write failed. MDIO timeout?\n");
> +		return ret;
> +	}
> +	return 0;
> +}

Same here.

Otherwise this looks O.K.

	  Andrew