diff mbox series

[U-Boot,v5,3/3] cmd: mdio: Switch to generic helpers when accessing the registers

Message ID 20190208172508.23601-4-ccaione@baylibre.com
State Accepted
Commit e55047ec51a662c12ed53ff543ec7cdf158b2137
Delegated to: Joe Hershberger
Headers show
Series Add MMD PHY helpers | expand

Commit Message

Carlo Caione Feb. 8, 2019, 5:25 p.m. UTC
Switch to use the generic helpers to access the MMD registers so that we
can used the same command also for C45 PHYs, C22 PHYs with direct and
indirect access and PHYs implementing a custom way to access the
registers.

Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 cmd/mdio.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

Comments

Vladimir Oltean Feb. 12, 2019, 12:20 p.m. UTC | #1
On 08.02.2019 19:26, Carlo Caione wrote:
> Switch to use the generic helpers to access the MMD registers so that we
> can used the same command also for C45 PHYs, C22 PHYs with direct and
> indirect access and PHYs implementing a custom way to access the
> registers.
> 
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
> ---
>   cmd/mdio.c | 27 ++++++++++++++++-----------
>   1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/cmd/mdio.c b/cmd/mdio.c
> index 184868063a..efe8c9ef09 100644
> --- a/cmd/mdio.c
> +++ b/cmd/mdio.c
> @@ -39,21 +39,24 @@ static int extract_range(char *input, int *plo, int *phi)
>   	return 0;
>   }
>   
> -static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
> +static int mdio_write_ranges(struct mii_dev *bus,
>   			     int addrlo,
>   			     int addrhi, int devadlo, int devadhi,
>   			     int reglo, int reghi, unsigned short data,
>   			     int extended)
>   {
> +	struct phy_device *phydev;
>   	int addr, devad, reg;
>   	int err = 0;
>   
>   	for (addr = addrlo; addr <= addrhi; addr++) {
> +		phydev = bus->phymap[addr];
> +
>   		for (devad = devadlo; devad <= devadhi; devad++) {
>   			for (reg = reglo; reg <= reghi; reg++) {
>   				if (!extended)
> -					err = bus->write(bus, addr, devad,
> -							 reg, data);
> +					err = phy_write_mmd(phydev, devad,
> +							    reg, data);
>   				else
>   					err = phydev->drv->writeext(phydev,
>   							addr, devad, reg, data);
> @@ -68,15 +71,17 @@ err_out:
>   	return err;
>   }
>   
> -static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
> +static int mdio_read_ranges(struct mii_dev *bus,
>   			    int addrlo,
>   			    int addrhi, int devadlo, int devadhi,
>   			    int reglo, int reghi, int extended)
>   {
>   	int addr, devad, reg;
> +	struct phy_device *phydev;
>   
>   	printf("Reading from bus %s\n", bus->name);
>   	for (addr = addrlo; addr <= addrhi; addr++) {
> +		phydev = bus->phymap[addr];
>   		printf("PHY at address %x:\n", addr);
>   
>   		for (devad = devadlo; devad <= devadhi; devad++) {
> @@ -84,7 +89,7 @@ static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
>   				int val;
>   
>   				if (!extended)
> -					val = bus->read(bus, addr, devad, reg);
> +					val = phy_read_mmd(phydev, devad, reg);
>   				else
>   					val = phydev->drv->readext(phydev, addr,
>   						devad, reg);
> @@ -222,14 +227,14 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   				bus = phydev->bus;
>   				extended = 1;
>   			} else {
> -				return -1;
> +				return CMD_RET_FAILURE;
>   			}
>   
>   			if (!phydev->drv ||
>   			    (!phydev->drv->writeext && (op[0] == 'w')) ||
>   			    (!phydev->drv->readext && (op[0] == 'r'))) {
>   				puts("PHY does not have extended functions\n");
> -				return -1;
> +				return CMD_RET_FAILURE;
>   			}
>   		}
>   	}
> @@ -242,13 +247,13 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   		if (pos > 1)
>   			if (extract_reg_range(argv[pos--], &devadlo, &devadhi,
>   					      &reglo, &reghi))
> -				return -1;
> +				return CMD_RET_FAILURE;
>   
>   	default:
>   		if (pos > 1)
>   			if (extract_phy_range(&argv[2], pos - 1, &bus,
>   					      &phydev, &addrlo, &addrhi))
> -				return -1;
> +				return CMD_RET_FAILURE;
>   
>   		break;
>   	}
> @@ -264,12 +269,12 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   
>   	switch (op[0]) {
>   	case 'w':
> -		mdio_write_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
> +		mdio_write_ranges(bus, addrlo, addrhi, devadlo, devadhi,
>   				  reglo, reghi, data, extended);
>   		break;
>   
>   	case 'r':
> -		mdio_read_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
> +		mdio_read_ranges(bus, addrlo, addrhi, devadlo, devadhi,
>   				 reglo, reghi, extended);
>   		break;
>   	}
> 

Hi Carlo,

I tested your patch on the NXP LS1088A-RDB board which is equipped with 
2 VSC8514 PHYs and 1 AQR105 PHY. The VSC8514 is a clause 22 QSGMII 
10/100/1000 Base-T PHY, but its 802.1az (EEE) registers are accessible 
through indirect clause 45 (clause 22 registers 13 and 14). The Aquantia 
AQR105 is a 10G Base-T PHY with native clause 45 addressing.

I started off by making sure that what should work (clause 45 
addressing) still works, so I tested the AQR105 without the patchset.

=> mdio list
FSL_MDIO0:
c - Vitesse VSC8514 <--> DPMAC3@qsgmii
d - Vitesse VSC8514 <--> DPMAC4@qsgmii
e - Vitesse VSC8514 <--> DPMAC5@qsgmii
f - Vitesse VSC8514 <--> DPMAC6@qsgmii
1c - Vitesse VSC8514 <--> DPMAC7@qsgmii
1d - Vitesse VSC8514 <--> DPMAC8@qsgmii
1e - Vitesse VSC8514 <--> DPMAC9@qsgmii
1f - Vitesse VSC8514 <--> DPMAC10@qsgmii
FSL_MDIO1:
0 - Generic PHY <--> DPMAC2@xgmii

# AQR105: PMA Standard Device Identifier 1 & 2 registers
=> mdio read DPMAC2@xgmii 1.2-3
Reading from bus FSL_MDIO1
PHY at address 0:
1.2 - 0x3a1
1.3 - 0xb4a3

Then I applied the patchset and went on to test both PHYs (since now 
mdio read MMD is supposed to work on both, not just the AQR).

# VSC8514: EEE Capability register
=> mdio read DPMAC3@qsgmii 3.14
Reading from bus FSL_MDIO0
PHY at address c:
3.20 - 0x6

So indirect addressing works, I will no longer refer to this and instead 
focus on the native C45 PHYs.

=> mdio read DPMAC2@xgmii 1.2-3
Reading from bus FSL_MDIO1
PHY at address 0:
1.2 - 0xffff
1.3 - 0xffff

Oops, the AQR105 no longer works.
But notice that U-boot probes it as "Generic PHY", so this is a board 
defconfig problem. So I went on to enable CONFIG_PHYLIB_10G=y and repeat 
the test.

=> mdio list
FSL_MDIO0:
c - Vitesse VSC8514 <--> DPMAC3@qsgmii
d - Vitesse VSC8514 <--> DPMAC4@qsgmii
e - Vitesse VSC8514 <--> DPMAC5@qsgmii
f - Vitesse VSC8514 <--> DPMAC6@qsgmii
1c - Vitesse VSC8514 <--> DPMAC7@qsgmii
1d - Vitesse VSC8514 <--> DPMAC8@qsgmii
1e - Vitesse VSC8514 <--> DPMAC9@qsgmii
1f - Vitesse VSC8514 <--> DPMAC10@qsgmii
FSL_MDIO1:
0 - Generic 10G PHY <--> DPMAC2@xgmii

Notice that the AQR105 is now probed as "Generic 10G PHY".
But:
# AQR105: PMA Standard Device Identifier 1 & 2 registers
=> mdio read DPMAC2@xgmii 1.2-3
Reading from bus FSL_MDIO1
PHY at address 0:
1.2 - 0xffff
1.3 - 0xffff

So it still doesn't work.
This is because in drivers/net/phy/generic_10g.c, the 
gen10g_driver.features is 0 and not PHY_10G_FEATURES as it properly 
should (comments?)

Then I went on to enable the proper driver for the AQR105, which is 
CONFIG_PHY_AQUANTIA.

=> mdio list
FSL_MDIO0:
c - Vitesse VSC8514 <--> DPMAC3@qsgmii
d - Vitesse VSC8514 <--> DPMAC4@qsgmii
e - Vitesse VSC8514 <--> DPMAC5@qsgmii
f - Vitesse VSC8514 <--> DPMAC6@qsgmii
1c - Vitesse VSC8514 <--> DPMAC7@qsgmii
1d - Vitesse VSC8514 <--> DPMAC8@qsgmii
1e - Vitesse VSC8514 <--> DPMAC9@qsgmii
1f - Vitesse VSC8514 <--> DPMAC10@qsgmii
FSL_MDIO1:
0 - Aquantia AQR105 <--> DPMAC2@xgmii

# AQR105: PMA Standard Device Identifier 1 & 2 registers
=> mdio read DPMAC2@xgmii 1.2-3
Reading from bus FSL_MDIO1
PHY at address 0:
1.2 - 0x3a1
1.3 - 0xb4a3

So it finally works now with your patchset.

Posting this just to raise awareness that:
* There are boards with 10G PHYs that didn't use to define 
CONFIG_PHYLIB_10G. Their MDIO drivers were working because they were 
only inferring the clause 45 aspect from the fact that a non-zero MMD 
access was being performed. This no longer works so they have to enable 
the 10g phylib support now.
* The generic 10G PHY driver doesn't define PHY_10G_FEATURES and needs 
to be fixed.
* The PHY vendors define their MMD register addresses in decimal. The 
U-boot mdio command has no way of inputting the address in decimal. For 
the VSC8514, to access its EEE registers I have to specify 3.14 (hex) in 
order for it to access 3.20 (decimal). This behavior does not originate 
from this patchset, but I think it's worth pointing it out now since it 
touches this area.

None of the above is a deal breaker IMO and I think that this patchset 
does work as intended (with the mention that more patches are needed).

Thanks,
-Vladimir
Vladimir Oltean Feb. 15, 2019, 10:46 p.m. UTC | #2
On 2/12/19 2:20 PM, Vladimir Oltean wrote:
> On 08.02.2019 19:26, Carlo Caione wrote:
>> Switch to use the generic helpers to access the MMD registers so that we
>> can used the same command also for C45 PHYs, C22 PHYs with direct and
>> indirect access and PHYs implementing a custom way to access the
>> registers.
>>
>> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
>> ---
>>    cmd/mdio.c | 27 ++++++++++++++++-----------
>>    1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/cmd/mdio.c b/cmd/mdio.c
>> index 184868063a..efe8c9ef09 100644
>> --- a/cmd/mdio.c
>> +++ b/cmd/mdio.c
>> @@ -39,21 +39,24 @@ static int extract_range(char *input, int *plo, int *phi)
>>    	return 0;
>>    }
>>    
>> -static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
>> +static int mdio_write_ranges(struct mii_dev *bus,
>>    			     int addrlo,
>>    			     int addrhi, int devadlo, int devadhi,
>>    			     int reglo, int reghi, unsigned short data,
>>    			     int extended)
>>    {
>> +	struct phy_device *phydev;
>>    	int addr, devad, reg;
>>    	int err = 0;
>>    
>>    	for (addr = addrlo; addr <= addrhi; addr++) {
>> +		phydev = bus->phymap[addr];
>> +
>>    		for (devad = devadlo; devad <= devadhi; devad++) {
>>    			for (reg = reglo; reg <= reghi; reg++) {
>>    				if (!extended)
>> -					err = bus->write(bus, addr, devad,
>> -							 reg, data);
>> +					err = phy_write_mmd(phydev, devad,
>> +							    reg, data);
>>    				else
>>    					err = phydev->drv->writeext(phydev,
>>    							addr, devad, reg, data);
>> @@ -68,15 +71,17 @@ err_out:
>>    	return err;
>>    }
>>    
>> -static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
>> +static int mdio_read_ranges(struct mii_dev *bus,
>>    			    int addrlo,
>>    			    int addrhi, int devadlo, int devadhi,
>>    			    int reglo, int reghi, int extended)
>>    {
>>    	int addr, devad, reg;
>> +	struct phy_device *phydev;
>>    
>>    	printf("Reading from bus %s\n", bus->name);
>>    	for (addr = addrlo; addr <= addrhi; addr++) {
>> +		phydev = bus->phymap[addr];
>>    		printf("PHY at address %x:\n", addr);
>>    
>>    		for (devad = devadlo; devad <= devadhi; devad++) {
>> @@ -84,7 +89,7 @@ static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
>>    				int val;
>>    
>>    				if (!extended)
>> -					val = bus->read(bus, addr, devad, reg);
>> +					val = phy_read_mmd(phydev, devad, reg);
>>    				else
>>    					val = phydev->drv->readext(phydev, addr,
>>    						devad, reg);
>> @@ -222,14 +227,14 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>    				bus = phydev->bus;
>>    				extended = 1;
>>    			} else {
>> -				return -1;
>> +				return CMD_RET_FAILURE;
>>    			}
>>    
>>    			if (!phydev->drv ||
>>    			    (!phydev->drv->writeext && (op[0] == 'w')) ||
>>    			    (!phydev->drv->readext && (op[0] == 'r'))) {
>>    				puts("PHY does not have extended functions\n");
>> -				return -1;
>> +				return CMD_RET_FAILURE;
>>    			}
>>    		}
>>    	}
>> @@ -242,13 +247,13 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>    		if (pos > 1)
>>    			if (extract_reg_range(argv[pos--], &devadlo, &devadhi,
>>    					      &reglo, &reghi))
>> -				return -1;
>> +				return CMD_RET_FAILURE;
>>    
>>    	default:
>>    		if (pos > 1)
>>    			if (extract_phy_range(&argv[2], pos - 1, &bus,
>>    					      &phydev, &addrlo, &addrhi))
>> -				return -1;
>> +				return CMD_RET_FAILURE;
>>    
>>    		break;
>>    	}
>> @@ -264,12 +269,12 @@ static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>    
>>    	switch (op[0]) {
>>    	case 'w':
>> -		mdio_write_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
>> +		mdio_write_ranges(bus, addrlo, addrhi, devadlo, devadhi,
>>    				  reglo, reghi, data, extended);
>>    		break;
>>    
>>    	case 'r':
>> -		mdio_read_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
>> +		mdio_read_ranges(bus, addrlo, addrhi, devadlo, devadhi,
>>    				 reglo, reghi, extended);
>>    		break;
>>    	}
>>
> 
> Hi Carlo,
> 
> I tested your patch on the NXP LS1088A-RDB board which is equipped with
> 2 VSC8514 PHYs and 1 AQR105 PHY. The VSC8514 is a clause 22 QSGMII
> 10/100/1000 Base-T PHY, but its 802.1az (EEE) registers are accessible
> through indirect clause 45 (clause 22 registers 13 and 14). The Aquantia
> AQR105 is a 10G Base-T PHY with native clause 45 addressing.
> 
> I started off by making sure that what should work (clause 45
> addressing) still works, so I tested the AQR105 without the patchset.
> 
> => mdio list
> FSL_MDIO0:
> c - Vitesse VSC8514 <--> DPMAC3@qsgmii
> d - Vitesse VSC8514 <--> DPMAC4@qsgmii
> e - Vitesse VSC8514 <--> DPMAC5@qsgmii
> f - Vitesse VSC8514 <--> DPMAC6@qsgmii
> 1c - Vitesse VSC8514 <--> DPMAC7@qsgmii
> 1d - Vitesse VSC8514 <--> DPMAC8@qsgmii
> 1e - Vitesse VSC8514 <--> DPMAC9@qsgmii
> 1f - Vitesse VSC8514 <--> DPMAC10@qsgmii
> FSL_MDIO1:
> 0 - Generic PHY <--> DPMAC2@xgmii
> 
> # AQR105: PMA Standard Device Identifier 1 & 2 registers
> => mdio read DPMAC2@xgmii 1.2-3
> Reading from bus FSL_MDIO1
> PHY at address 0:
> 1.2 - 0x3a1
> 1.3 - 0xb4a3
> 
> Then I applied the patchset and went on to test both PHYs (since now
> mdio read MMD is supposed to work on both, not just the AQR).
> 
> # VSC8514: EEE Capability register
> => mdio read DPMAC3@qsgmii 3.14
> Reading from bus FSL_MDIO0
> PHY at address c:
> 3.20 - 0x6
> 
> So indirect addressing works, I will no longer refer to this and instead
> focus on the native C45 PHYs.
> 
> => mdio read DPMAC2@xgmii 1.2-3
> Reading from bus FSL_MDIO1
> PHY at address 0:
> 1.2 - 0xffff
> 1.3 - 0xffff
> 
> Oops, the AQR105 no longer works.
> But notice that U-boot probes it as "Generic PHY", so this is a board
> defconfig problem. So I went on to enable CONFIG_PHYLIB_10G=y and repeat
> the test.
> 
> => mdio list
> FSL_MDIO0:
> c - Vitesse VSC8514 <--> DPMAC3@qsgmii
> d - Vitesse VSC8514 <--> DPMAC4@qsgmii
> e - Vitesse VSC8514 <--> DPMAC5@qsgmii
> f - Vitesse VSC8514 <--> DPMAC6@qsgmii
> 1c - Vitesse VSC8514 <--> DPMAC7@qsgmii
> 1d - Vitesse VSC8514 <--> DPMAC8@qsgmii
> 1e - Vitesse VSC8514 <--> DPMAC9@qsgmii
> 1f - Vitesse VSC8514 <--> DPMAC10@qsgmii
> FSL_MDIO1:
> 0 - Generic 10G PHY <--> DPMAC2@xgmii
> 
> Notice that the AQR105 is now probed as "Generic 10G PHY".
> But:
> # AQR105: PMA Standard Device Identifier 1 & 2 registers
> => mdio read DPMAC2@xgmii 1.2-3
> Reading from bus FSL_MDIO1
> PHY at address 0:
> 1.2 - 0xffff
> 1.3 - 0xffff
> 
> So it still doesn't work.
> This is because in drivers/net/phy/generic_10g.c, the
> gen10g_driver.features is 0 and not PHY_10G_FEATURES as it properly
> should (comments?)
> 
> Then I went on to enable the proper driver for the AQR105, which is
> CONFIG_PHY_AQUANTIA.
> 
> => mdio list
> FSL_MDIO0:
> c - Vitesse VSC8514 <--> DPMAC3@qsgmii
> d - Vitesse VSC8514 <--> DPMAC4@qsgmii
> e - Vitesse VSC8514 <--> DPMAC5@qsgmii
> f - Vitesse VSC8514 <--> DPMAC6@qsgmii
> 1c - Vitesse VSC8514 <--> DPMAC7@qsgmii
> 1d - Vitesse VSC8514 <--> DPMAC8@qsgmii
> 1e - Vitesse VSC8514 <--> DPMAC9@qsgmii
> 1f - Vitesse VSC8514 <--> DPMAC10@qsgmii
> FSL_MDIO1:
> 0 - Aquantia AQR105 <--> DPMAC2@xgmii
> 
> # AQR105: PMA Standard Device Identifier 1 & 2 registers
> => mdio read DPMAC2@xgmii 1.2-3
> Reading from bus FSL_MDIO1
> PHY at address 0:
> 1.2 - 0x3a1
> 1.3 - 0xb4a3
> 
> So it finally works now with your patchset.
> 
> Posting this just to raise awareness that:
> * There are boards with 10G PHYs that didn't use to define
> CONFIG_PHYLIB_10G. Their MDIO drivers were working because they were
> only inferring the clause 45 aspect from the fact that a non-zero MMD
> access was being performed. This no longer works so they have to enable
> the 10g phylib support now.
> * The generic 10G PHY driver doesn't define PHY_10G_FEATURES and needs
> to be fixed.
> * The PHY vendors define their MMD register addresses in decimal. The
> U-boot mdio command has no way of inputting the address in decimal. For
> the VSC8514, to access its EEE registers I have to specify 3.14 (hex) in
> order for it to access 3.20 (decimal). This behavior does not originate
> from this patchset, but I think it's worth pointing it out now since it
> touches this area.
> 
> None of the above is a deal breaker IMO and I think that this patchset
> does work as intended (with the mention that more patches are needed).
> 
> Thanks,
> -Vladimir
> 

Carlo, Joe,

One additional piece of information that for some reason escaped me 
initially is that Pankaj Bansal already added support for struct 
phy_device property is_c45 as part of this patch: 
http://patchwork.ozlabs.org/patch/998770/
Maybe this patchset should go in the direction of using that.

-Vladimir
Carlo Caione Feb. 16, 2019, 10:57 a.m. UTC | #3
On 15/02/2019 22:46, Vladimir Oltean wrote:
> On 2/12/19 2:20 PM, Vladimir Oltean wrote:

/cut
> 
> Carlo, Joe,
> 
> One additional piece of information that for some reason escaped me
> initially is that Pankaj Bansal already added support for struct
> phy_device property is_c45 as part of this patch:
> http://patchwork.ozlabs.org/patch/998770/
> Maybe this patchset should go in the direction of using that.

Oh, that's nice.

Joe,
can you review this patchset before I send a new one?

Thanks,

--
Carlo Caione
Joe Hershberger Feb. 19, 2019, 6:06 p.m. UTC | #4
On Fri, Feb 8, 2019 at 11:31 AM Carlo Caione <ccaione@baylibre.com> wrote:
>
> Switch to use the generic helpers to access the MMD registers so that we
> can used the same command also for C45 PHYs, C22 PHYs with direct and
> indirect access and PHYs implementing a custom way to access the
> registers.
>
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Joe Hershberger March 5, 2019, 6:04 p.m. UTC | #5
Hi Carlo,

https://patchwork.ozlabs.org/patch/1038821/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe
diff mbox series

Patch

diff --git a/cmd/mdio.c b/cmd/mdio.c
index 184868063a..efe8c9ef09 100644
--- a/cmd/mdio.c
+++ b/cmd/mdio.c
@@ -39,21 +39,24 @@  static int extract_range(char *input, int *plo, int *phi)
 	return 0;
 }
 
-static int mdio_write_ranges(struct phy_device *phydev, struct mii_dev *bus,
+static int mdio_write_ranges(struct mii_dev *bus,
 			     int addrlo,
 			     int addrhi, int devadlo, int devadhi,
 			     int reglo, int reghi, unsigned short data,
 			     int extended)
 {
+	struct phy_device *phydev;
 	int addr, devad, reg;
 	int err = 0;
 
 	for (addr = addrlo; addr <= addrhi; addr++) {
+		phydev = bus->phymap[addr];
+
 		for (devad = devadlo; devad <= devadhi; devad++) {
 			for (reg = reglo; reg <= reghi; reg++) {
 				if (!extended)
-					err = bus->write(bus, addr, devad,
-							 reg, data);
+					err = phy_write_mmd(phydev, devad,
+							    reg, data);
 				else
 					err = phydev->drv->writeext(phydev,
 							addr, devad, reg, data);
@@ -68,15 +71,17 @@  err_out:
 	return err;
 }
 
-static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
+static int mdio_read_ranges(struct mii_dev *bus,
 			    int addrlo,
 			    int addrhi, int devadlo, int devadhi,
 			    int reglo, int reghi, int extended)
 {
 	int addr, devad, reg;
+	struct phy_device *phydev;
 
 	printf("Reading from bus %s\n", bus->name);
 	for (addr = addrlo; addr <= addrhi; addr++) {
+		phydev = bus->phymap[addr];
 		printf("PHY at address %x:\n", addr);
 
 		for (devad = devadlo; devad <= devadhi; devad++) {
@@ -84,7 +89,7 @@  static int mdio_read_ranges(struct phy_device *phydev, struct mii_dev *bus,
 				int val;
 
 				if (!extended)
-					val = bus->read(bus, addr, devad, reg);
+					val = phy_read_mmd(phydev, devad, reg);
 				else
 					val = phydev->drv->readext(phydev, addr,
 						devad, reg);
@@ -222,14 +227,14 @@  static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 				bus = phydev->bus;
 				extended = 1;
 			} else {
-				return -1;
+				return CMD_RET_FAILURE;
 			}
 
 			if (!phydev->drv ||
 			    (!phydev->drv->writeext && (op[0] == 'w')) ||
 			    (!phydev->drv->readext && (op[0] == 'r'))) {
 				puts("PHY does not have extended functions\n");
-				return -1;
+				return CMD_RET_FAILURE;
 			}
 		}
 	}
@@ -242,13 +247,13 @@  static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		if (pos > 1)
 			if (extract_reg_range(argv[pos--], &devadlo, &devadhi,
 					      &reglo, &reghi))
-				return -1;
+				return CMD_RET_FAILURE;
 
 	default:
 		if (pos > 1)
 			if (extract_phy_range(&argv[2], pos - 1, &bus,
 					      &phydev, &addrlo, &addrhi))
-				return -1;
+				return CMD_RET_FAILURE;
 
 		break;
 	}
@@ -264,12 +269,12 @@  static int do_mdio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	switch (op[0]) {
 	case 'w':
-		mdio_write_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
+		mdio_write_ranges(bus, addrlo, addrhi, devadlo, devadhi,
 				  reglo, reghi, data, extended);
 		break;
 
 	case 'r':
-		mdio_read_ranges(phydev, bus, addrlo, addrhi, devadlo, devadhi,
+		mdio_read_ranges(bus, addrlo, addrhi, devadlo, devadhi,
 				 reglo, reghi, extended);
 		break;
 	}