On Tue, 28 May 2019 at 12:58, Russell King <rmk+ker...@armlinux.org.uk> wrote:
>
> Allow userspace to generate Clause 45 MII access cycles via phylib.
> This is useful for tools such as mii-diag to be able to inspect Clause
> 45 PHYs.
>
> Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
> Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
> ---
> drivers/net/phy/phy.c | 33 ++++++++++++++++++++++++---------
> 1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 3745220c5c98..6d279c2ac1f8 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -386,6 +386,7 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq
> *ifr, int cmd)
> struct mii_ioctl_data *mii_data = if_mii(ifr);
> u16 val = mii_data->val_in;
> bool change_autoneg = false;
> + int prtad, devad;
>
> switch (cmd) {
> case SIOCGMIIPHY:
> @@ -393,14 +394,29 @@ int phy_mii_ioctl(struct phy_device *phydev, struct
> ifreq *ifr, int cmd)
> /* fall through */
>
> case SIOCGMIIREG:
> - mii_data->val_out = mdiobus_read(phydev->mdio.bus,
> - mii_data->phy_id,
> - mii_data->reg_num);
> + if (mdio_phy_id_is_c45(mii_data->phy_id)) {
> + prtad = mdio_phy_id_prtad(mii_data->phy_id);
> + devad = mdio_phy_id_devad(mii_data->phy_id);
> + devad = MII_ADDR_C45 | devad << 16 |
> mii_data->reg_num;
> + } else {
> + prtad = mii_data->phy_id;
> + devad = mii_data->reg_num;
> + }
> + mii_data->val_out = mdiobus_read(phydev->mdio.bus, prtad,
> + devad);
> return 0;
>
> case SIOCSMIIREG:
> - if (mii_data->phy_id == phydev->mdio.addr) {
> - switch (mii_data->reg_num) {
> + if (mdio_phy_id_is_c45(mii_data->phy_id)) {
> + prtad = mdio_phy_id_prtad(mii_data->phy_id);
> + devad = mdio_phy_id_devad(mii_data->phy_id);
> + devad = MII_ADDR_C45 | devad << 16 |
> mii_data->reg_num;
> + } else {
> + prtad = mii_data->phy_id;
> + devad = mii_data->reg_num;
> + }
> + if (prtad == phydev->mdio.addr) {
> + switch (devad) {
> case MII_BMCR:
> if ((val & (BMCR_RESET | BMCR_ANENABLE)) ==
> 0) {
> if (phydev->autoneg == AUTONEG_ENABLE)
> @@ -433,11 +449,10 @@ int phy_mii_ioctl(struct phy_device *phydev, struct
> ifreq *ifr, int cmd)
> }
> }
>
> - mdiobus_write(phydev->mdio.bus, mii_data->phy_id,
> - mii_data->reg_num, val);
> + mdiobus_write(phydev->mdio.bus, prtad, devad, val);
>
> - if (mii_data->phy_id == phydev->mdio.addr &&
> - mii_data->reg_num == MII_BMCR &&
> + if (prtad == phydev->mdio.addr &&
> + devad == MII_BMCR &&
> val & BMCR_RESET)
> return phy_init_hw(phydev);
>
> --
> 2.7.4
>
Hi Russell,
I find the SIOCGMIIREG/SIOCGMIIPHY ioctls useful for C45 just as much
as they are for C22, but I think the way they work is a big hack and
for that reason they're less than useful when you need them most.
These ioctls work by hijacking the MDIO bus driver of a PHY that is
attached to a net_device. Hence they can be used to access at most a
PHY that lies on the same MDIO bus as one you already have a
phy-handle to.
If you have a PHY issue that makes of_phy_connect fail and the
net_device to fail to probe, basically you're SOL because you lose
that one handle that userspace had to the MDIO bus.
Similarly if you're doing a bring-up and all PHY interfaces are fixed-link.
Maybe it would be better to rethink this and expose some sysfs nodes
for raw MDIO access in the bus drivers.
-Vladimir
