[U-Boot] [PATCH 2/8] net: tsec: Fix offset of MDIO registers for DM_ETH

[Vladimir Oltean]

Hi Joe,

On Mon, 15 Jul 2019 at 21:00, Joe Hershberger <joe.hershberger at ni.com> wrote:
>
> On Sun, Jun 23, 2019 at 12:51 PM Vladimir Oltean <olteanv at gmail.com> wrote:
> >
> > By convention, the eTSEC MDIO controller nodes are defined in DT at
> > 0x2d24000 and 0x2d50000, but actually U-boot does not touch the
> > interrupt portion of the register map (MDIO_IEVENTM, MDIO_IMASKM,
> > MDIO_EMAPM).
> >
> > That leaves only the MDIO bus registers (MDIO_MIIMCFG, MDIO_MIIMCOM,
> > MDIO_MIIMADD, MDIO_MIIMADD, MDIO_MIIMCON, MDIO_MIIMSTAT) which start at
> > the 0x520 offset.
> >
> > So shift the DT-defined register map by the offset of MDIO_MIIMCFG when
> > mapping the MDIO bus registers.
> >
> > Signed-off-by: Vladimir Oltean <olteanv at gmail.com>
>
> Acked-by: Joe Hershberger <joe.hershberger at ni.com>
>
> > ---
> >  drivers/net/tsec.c | 13 +++++++------
> >  include/tsec.h     |  4 +++-
> >  2 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> > index 53eb5470f4c8..576398676af7 100644
> > --- a/drivers/net/tsec.c
> > +++ b/drivers/net/tsec.c
> > @@ -801,6 +801,7 @@ int tsec_probe(struct udevice *dev)
> >         u32 tbiaddr = CONFIG_SYS_TBIPA_VALUE;
> >         ofnode parent;
> >         const char *phy_mode;
> > +       fdt_addr_t reg;
> >         int ret;
> >
> >         pdata->iobase = (phys_addr_t)dev_read_addr(dev);
> > @@ -817,15 +818,15 @@ int tsec_probe(struct udevice *dev)
> >         }
> >
> >         parent = ofnode_get_parent(phandle_args.node);
> > -       if (ofnode_valid(parent)) {
> > -               int reg = ofnode_get_addr_index(parent, 0);
> > -
> > -               priv->phyregs_sgmii = (struct tsec_mii_mng *)reg;
> > -       } else {
> > -               debug("No parent node for PHY?\n");
> > +       if (!ofnode_valid(parent)) {
> > +               printf("No parent node for PHY?\n");
> >                 return -ENOENT;
> >         }
> >
> > +       reg = ofnode_get_addr_index(parent, 0);
> > +       priv->phyregs_sgmii = (struct tsec_mii_mng *)
> > +                       (reg + TSEC_MDIO_REGS_OFFSET);
>
> I'm surprised not to see a .dts change in this patch as well or some
> other consumer of this phyregs_sgmii member.
>

This surprises me as well, to be honest.
Actually Bin Meng's patchset to convert the TSEC driver to DM never
got completely merged. I suppose the LS1021A-TWR conversion was sort
of mechanical and probably not tested on hardware, otherwise I can't
explain. There is no DT patch because this portion of the code never
worked. The DM version of the TSEC driver had no (upstream?) users.

>
> > +
> >         ret = dev_read_phandle_with_args(dev, "tbi-handle", NULL, 0, 0,
> >                                          &phandle_args);
> >         if (ret == 0)
> > diff --git a/include/tsec.h b/include/tsec.h
> > index e90095121bdd..b17fa957df5b 100644
> > --- a/include/tsec.h
> > +++ b/include/tsec.h
> > @@ -17,6 +17,8 @@
> >  #include <config.h>
> >  #include <phy.h>
> >
> > +#define TSEC_MDIO_REGS_OFFSET  0x520
> > +
> >  #ifndef CONFIG_DM_ETH
> >
> >  #ifdef CONFIG_ARCH_LS1021A
> > @@ -27,7 +29,7 @@
> >  #define TSEC_MDIO_OFFSET       0x01000
> >  #endif
> >
> > -#define CONFIG_SYS_MDIO_BASE_ADDR (MDIO_BASE_ADDR + 0x520)
> > +#define CONFIG_SYS_MDIO_BASE_ADDR (MDIO_BASE_ADDR + TSEC_MDIO_REGS_OFFSET)
> >
> >  #define TSEC_GET_REGS(num, offset) \
> >         (struct tsec __iomem *)\
> > --
> > 2.17.1
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot

Thanks,
-Vladimir