Den 2023-02-09 kl. 08:29, skrev Siddharth Vadapalli:
Hello Ulf,

On 07/02/23 13:55, Ulf Samuelsson wrote:
cpsw_mdio_get_alive reads the wrong register.
See page 2316 in SPRUH73Q AM335x TRM

Signed-off-by: Ulf Samuelsson <u...@emagii.com>
Cc: Joe Hershberger <joe.hershber...@ni.com>
Cc: Ramon Fried <rfried....@gmail.com>
---
  drivers/net/ti/cpsw_mdio.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ti/cpsw_mdio.c b/drivers/net/ti/cpsw_mdio.c
index a5ba73b739..ac791faa81 100644
--- a/drivers/net/ti/cpsw_mdio.c
+++ b/drivers/net/ti/cpsw_mdio.c
@@ -51,7 +51,7 @@ struct cpsw_mdio_regs {
  #define USERACCESS_PHY_REG_SHIFT      (21)
  #define USERACCESS_PHY_ADDR_SHIFT     (16)
  #define USERACCESS_DATA               GENMASK(15, 0)
-       } user[0];
+       } user[2];
  };
#define CPSW_MDIO_DIV_DEF 0xff
@@ -366,8 +366,8 @@ u32 cpsw_mdio_get_alive(struct mii_dev *bus)
        struct cpsw_mdio *mdio = bus->priv;
        u32 val;
- val = readl(&mdio->regs->control);
-       return val & GENMASK(15, 0);
+       val = readl(&mdio->regs->alive);
+       return val & GENMASK(7, 0);

Referring to the TRM for AM335x at [0], on page 2401, the MDIOALIVE register
reports the presence of Ethernet PHYs up to ADDR 31. Also, the cpsw driver:
drivers/net/ti/cpsw.c which uses the cpsw_mdio_get_alive() function, expects the
return value to be a 16 bit value. Therefore, I believe that the GENMASK should
retain the previous value of "GENMASK(15, 0)", while only the register being
read needs to be changed from "control" to "alive". Please let me know if I
misunderstood something regarding your implementation.

We had an unusual H/W where the PHY (actually a switch) was connected to the second Ethernet port. All boards I have seen have the PHY connected to the first port or have both ports connected.

IIRC correctly (this patch is > 2 years old),
we had bad data in the bits (15:8) and this is why only the 8 bits.
Only bits [1:0] are used anyway.



[0] https://www.ti.com/lit/ug/spruh73q/spruh73q.pdf

Regards,
Siddharth.

  }
struct mii_dev *cpsw_mdio_init(const char *name, phys_addr_t mdio_base,

Best Regards
Ulf Samuelsson

Reply via email to