Re: [PATCH net-next v1 1/2] net: phy: realtek: add support for RTL8261

[Andrew Lunn]

> +static int rtl8261x_soft_reset(struct phy_device *phydev)
> +{
> +	int ret, val;
> +
> +	ret = phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, MDIO_CTRL1_RESET);
> +	if (ret < 0)
> +		return ret;
> +
> +	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PMAPMD,
> +					 MDIO_CTRL1, val,
> +					 !(val & MDIO_CTRL1_RESET),
> +					 5000, 100000, true);

We have a few copies of this function in various PHY drivers. Please
add a genphy_c45_soft_reset() helper.

> +}
> +
> +static int rtl8261x_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct rtl8261x_priv *priv;
> +	int sub_phy_id, ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, RTL8261X_EXT_ADDR_REG,
> +			    RTL_8261X_SUB_PHY_ID_ADDR);
> +	if (ret < 0)
> +		return ret;

blank line

> +	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, RTL8261X_EXT_DATA_REG);
> +	if (ret < 0)
> +		return ret;
> +
> +	sub_phy_id = (ret >> 8) & 0xff;
> +	priv->sub_phy_id = sub_phy_id;
> +	priv->is_generic = false;
> +
> +	switch (sub_phy_id) {
> +	case RTL8261C_CE_MODEL:
> +		priv->model = RTL8261_MODEL_C_CE;
> +		phydev_info(phydev, "RTL8261C/CE detected (sub_id 0x%02x)\n", sub_phy_id);
> +		break;
> +
> +	case RTL8261D_MODEL:
> +		priv->model = RTL8261_MODEL_D;
> +		phydev_info(phydev, "RTL8261D detected (sub_id 0x%02x)\n", sub_phy_id);
> +		break;
> +
> +	default:
> +		priv->model = RTL8261_MODEL_GENERIC;
> +		priv->is_generic = true;
> +		phydev_warn(phydev, "Unknown sub_id 0x%02x! Using GENERIC mode. Update driver for full support.\n",
> +			    sub_phy_id);
> +		break;

This will get ignored. It is better to return -ENODEV so the probe
fails.

	Andrew