Skip to content

Commit

Permalink
net: dsa: seville: ignore mscc-miim read errors from Lynx PCS
Browse files Browse the repository at this point in the history
During the refactoring in the commit below, vsc9953_mdio_read() was
replaced with mscc_miim_read(), which has one extra step: it checks for
the MSCC_MIIM_DATA_ERROR bits before returning the result.

On T1040RDB, there are 8 QSGMII PCSes belonging to the switch, and they
are organized in 2 groups. First group responds to MDIO addresses 4-7
because QSGMIIACR1[MDEV_PORT] is 1, and the second group responds to
MDIO addresses 8-11 because QSGMIIBCR1[MDEV_PORT] is 2. I have double
checked that these values are correctly set in the SERDES, as well as
PCCR1[QSGMA_CFG] and PCCR1[QSGMB_CFG] are both 0b01.

mscc_miim_read: phyad 8 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 8 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 8 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 8 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 9 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 9 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 9 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 9 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 10 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 10 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 10 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 10 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 11 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 11 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 11 reg 0x1 MIIM_DATA 0x2d
mscc_miim_read: phyad 11 reg 0x5 MIIM_DATA 0x5801
mscc_miim_read: phyad 4 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 4 reg 0x5 MIIM_DATA 0x3da01, ERROR
mscc_miim_read: phyad 5 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 5 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 5 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 5 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 6 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 6 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 6 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 6 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 7 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 7 reg 0x5 MIIM_DATA 0x35801, ERROR
mscc_miim_read: phyad 7 reg 0x1 MIIM_DATA 0x3002d, ERROR
mscc_miim_read: phyad 7 reg 0x5 MIIM_DATA 0x35801, ERROR

As can be seen, the data in MIIM_DATA is still valid despite having the
MSCC_MIIM_DATA_ERROR bits set. The driver as introduced in commit
84705fc ("net: dsa: felix: introduce support for Seville VSC9953
switch") was ignoring these bits, perhaps deliberately (although
unbeknownst to me).

This is an old IP and the hardware team cannot seem to be able to help
me track down a plausible reason for these failures. I'll keep
investigating, but in the meantime, this is a direct regression which
must be restored to a working state.

The only thing I can do is keep ignoring the errors as before.

Fixes: b996584 ("net: dsa: ocelot: felix: utilize shared mscc-miim driver for indirect MDIO access")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
vladimiroltean authored and davem330 committed Feb 26, 2023
1 parent 3fa1056 commit 0322ef4
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 6 deletions.
4 changes: 2 additions & 2 deletions drivers/net/dsa/ocelot/seville_vsc9953.c
Original file line number Diff line number Diff line change
Expand Up @@ -893,8 +893,8 @@ static int vsc9953_mdio_bus_alloc(struct ocelot *ocelot)

rc = mscc_miim_setup(dev, &bus, "VSC9953 internal MDIO bus",
ocelot->targets[GCB],
ocelot->map[GCB][GCB_MIIM_MII_STATUS & REG_MASK]);

ocelot->map[GCB][GCB_MIIM_MII_STATUS & REG_MASK],
true);
if (rc) {
dev_err(dev, "failed to setup MDIO bus\n");
return rc;
Expand Down
9 changes: 6 additions & 3 deletions drivers/net/mdio/mdio-mscc-miim.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ struct mscc_miim_info {
struct mscc_miim_dev {
struct regmap *regs;
int mii_status_offset;
bool ignore_read_errors;
struct regmap *phy_regs;
const struct mscc_miim_info *info;
struct clk *clk;
Expand Down Expand Up @@ -135,7 +136,7 @@ static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum)
goto out;
}

if (val & MSCC_MIIM_DATA_ERROR) {
if (!miim->ignore_read_errors && !!(val & MSCC_MIIM_DATA_ERROR)) {
ret = -EIO;
goto out;
}
Expand Down Expand Up @@ -212,7 +213,8 @@ static const struct regmap_config mscc_miim_phy_regmap_config = {
};

int mscc_miim_setup(struct device *dev, struct mii_bus **pbus, const char *name,
struct regmap *mii_regmap, int status_offset)
struct regmap *mii_regmap, int status_offset,
bool ignore_read_errors)
{
struct mscc_miim_dev *miim;
struct mii_bus *bus;
Expand All @@ -234,6 +236,7 @@ int mscc_miim_setup(struct device *dev, struct mii_bus **pbus, const char *name,

miim->regs = mii_regmap;
miim->mii_status_offset = status_offset;
miim->ignore_read_errors = ignore_read_errors;

*pbus = bus;

Expand Down Expand Up @@ -285,7 +288,7 @@ static int mscc_miim_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(phy_regmap),
"Unable to create phy register regmap\n");

ret = mscc_miim_setup(dev, &bus, "mscc_miim", mii_regmap, 0);
ret = mscc_miim_setup(dev, &bus, "mscc_miim", mii_regmap, 0, false);
if (ret < 0) {
dev_err(dev, "Unable to setup the MDIO bus\n");
return ret;
Expand Down
2 changes: 1 addition & 1 deletion include/linux/mdio/mdio-mscc-miim.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@

int mscc_miim_setup(struct device *device, struct mii_bus **bus,
const char *name, struct regmap *mii_regmap,
int status_offset);
int status_offset, bool ignore_read_errors);

#endif

0 comments on commit 0322ef4

Please sign in to comment.