Hi Allan
> +static int vsc85xx_downshift_get(struct phy_device *phydev, u8 *count)
> +{
> + int rc;
> + u16 reg_val;
> +
> + mutex_lock(&phydev->lock);
> + rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
> + if (rc != 0)
> + goto out_unlock;
> +
> + reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
> + reg_val &= DOWNSHIFT_CNTL_MASK;
> + if (!(reg_val & DOWNSHIFT_EN))
> + *count = 0;
DOWNSHIFT_DEV_DISABLE
> + else
> + *count = ((reg_val & ~DOWNSHIFT_EN) >> DOWNSHIFT_CNTL_POS) + 2;
>
> + rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
> +
> +out_unlock:
> + mutex_unlock(&phydev->lock);
> +
> + return rc;
> +}
> +
> +static int vsc85xx_downshift_set(struct phy_device *phydev, u8 count)
> +{
> + int rc;
> + u16 reg_val;
> +
> + if (count == DOWNSHIFT_DEV_DEFAULT_COUNT) {
> + /* Default downshift count 3 (i.e. Bit3:2 = 0b01) */
> + count = ((1 << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
> + } else if (count > DOWNSHIFT_COUNT_MAX || count == 1) {
> + phydev_err(phydev, "Invalid downshift count\n");
Maybe include a hint what is valid?
> + return -EINVAL;
ERANGE? I don't know error codes too well, so this needs verifying.
> + } else if (count) {
> + /* Downshift count is either 2,3,4 or 5 */
> + count = (((count - 2) << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
Ah, now i see why + 2. But this means it never does what you ask it to
do. It would be better to round up < 2 to 2, and leave all the others
as is.
> +static int vsc85xx_get_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna, void *data)
> +{
> + switch (tuna->id) {
> + case ETHTOOL_PHY_DOWNSHIFT:
> + return vsc85xx_downshift_get(phydev, (u8 *)data);
> + default:
> + phydev_err(phydev, "Unsupported PHY tunable id\n");
> + return -EINVAL;
This is not really a error you should complain about. There could be
many tunables, and some your hardware cannot support. So return
-ENOSUPP and no phydev_err().
> + }
> +}
> +
> +static int vsc85xx_set_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna,
> + const void *data)
> +{
> + switch (tuna->id) {
> + case ETHTOOL_PHY_DOWNSHIFT:
> + return vsc85xx_downshift_set(phydev, *(u8 *)data);
> + default:
> + phydev_err(phydev, "Unsupported PHY tunable id\n");
> + return -EINVAL;
Same as above.
Andrew
