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 <ccai...@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,
> ®lo, ®hi))
> - 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
