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.
[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,
