Re: [RFC PATCH] net: phy/mdio: enable mmd indirect access through phy_mii_ioctl()

[Andrew Lunn <andrew@lunn.ch>]

On Mon, Nov 01, 2021 at 08:28:59PM +0200, Grygorii Strashko wrote:
> This patch enables access to C22 PHY MMD address space through

I'm not sure the terminology is correct here. I think it actually
enables access to C45 address space, making use of C45 over C22.

> phy_mii_ioctl() SIOCGMIIREG/SIOCSMIIREG IOCTLs. It checks if R/W request is
> received with C45 flag enabled while MDIO bus doesn't support C45 and, in
> this case, tries to treat prtad as PHY MMD selector and use MMD API.
> 
> With this change it's possible to r/w PHY MMD registers with phytool, for
> example, before:
> 
>   phytool read eth0/0x1f:0/0x32
>   0xffea
> 
> after:
>   phytool read eth0/0x1f:0/0x32
>   0x00d1
> @@ -300,8 +300,19 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
>  			prtad = mii_data->phy_id;
>  			devad = mii_data->reg_num;
>  		}
> -		mii_data->val_out = mdiobus_read(phydev->mdio.bus, prtad,
> -						 devad);
> +		if (mdio_phy_id_is_c45(mii_data->phy_id) &&
> +		    phydev->mdio.bus->probe_capabilities <= MDIOBUS_C22) {
> +			phy_lock_mdio_bus(phydev);
> +
> +			mii_data->val_out = __mmd_phy_read(phydev->mdio.bus,
> +							   mdio_phy_id_devad(mii_data->phy_id),
> +							   prtad,
> +							   mii_data->reg_num);
> +
> +			phy_unlock_mdio_bus(phydev);
> +		} else {
> +			mii_data->val_out = mdiobus_read(phydev->mdio.bus, prtad, devad);
> +		}

The layering look wrong here. You are trying to perform MDIO bus
operation here, so it seems odd to perform __mmd_phy_read(). I wonder
if it would be cleaner to move C45 over C22 down into the mdiobus_
level API?

      Andrew