Re: [PATCH net-next v17 2/2] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

[Andrew Lunn]

> +struct yt8824_shared_priv {
> +	unsigned int interface_mode;

phy_interface_t

> +	if (!of_property_read_string(np, "phy-mode",
> +				     &interface_mode_name)) {
> +		if (!strcasecmp(interface_mode_name,
> +				phy_modes(PHY_INTERFACE_MODE_USXGMII))) {
> +			priv->interface_mode = PHY_INTERFACE_MODE_USXGMII;
> +		} else if (!strcasecmp
> +				(interface_mode_name,
> +				 phy_modes(PHY_INTERFACE_MODE_INTERNAL))) {
> +			priv->interface_mode = PHY_INTERFACE_MODE_INTERNAL;
> +		} else {
> +			return -EINVAL;
> +		}
> +	} else {
> +		phydev_warn(phydev, "%s, phy-mode missing in DTS.\n",
> +			    __func__);
> +	}

Please don't reinvent the wheel. Look around to find an existing
wheel.

> +/**
> + * genphy_c45_template_testmode - configure template testmode registers
> + * @phydev: target phy_device struct
> + * @test_mode: testmode includes Normal to Test mode 7
> + *
> + * Description: Set template testmode include Normal to Test mode 7
> + *
> + * Return: 0 on success, or a negative error code on failure (e.g. register
> + *	read/write error).
> + */
> +int genphy_c45_template_testmode(struct phy_device *phydev, int test_mode)
> +{
> +	int ctrl = 0;
> +
> +	switch (test_mode) {
> +	case 0:
> +		ctrl = MDIO_PMA_10GBT_TESTMODE_NORMAL;
> +		break;
> +
> +	case 1:
> +		ctrl = MDIO_PMA_10GBT_TESTMODE_1;
> +		break;
> +
> +	case 2:
> +		ctrl = MDIO_PMA_10GBT_TESTMODE_2;
> +		break;
> +
> +	case 3:
> +		ctrl = MDIO_PMA_10GBT_TESTMODE_3;
> +		break;
> +
> +	case 4:
> +		ctrl = MDIO_PMA_10GBT_TESTMODE_4;
> +		break;
> +
> +	case 5:
> +		ctrl = MDIO_PMA_10GBT_TESTMODE_5;
> +		break;
> +
> +	case 6:
> +		ctrl = MDIO_PMA_10GBT_TESTMODE_6;
> +		break;
> +
> +	case 7:
> +		ctrl = MDIO_PMA_10GBT_TESTMODE_7;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +	return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_10GBT_TESTMODE,
> +			      MDIO_PMA_10GBT_TESTMODE_MASK, ctrl);
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_template_testmode);

It would be normal to put this in a patch of its own. We just need to
see a user of it within the same patchset.


    Andrew

---
pw-bot: cr