[PATCH 2/3] arm: kirkwood: Dreamplug: Use Ethernet PHY name and address from device tree

[Tony Dinh]

Hi Stefan,

On Sat, Jul 31, 2021 at 12:41 AM Stefan Roese <sr at denx.de> wrote:
>
> On 26.07.21 08:01, Tony Dinh wrote:
> > In DM Ethernet, the old "egiga0" and 'egiga1" names are no longer valid,
> > so replace these with Ethernet PHY names from device tree. Also, read
> > Ethernet PHY address for each port from device tree.
> >
> > Signed-off-by: Tony Dinh <mibodhi at gmail.com>
> > ---
> >
> >   board/Marvell/dreamplug/dreamplug.c | 62 ++++++++++++++++++++++-------
> >   1 file changed, 48 insertions(+), 14 deletions(-)
> >
> > diff --git a/board/Marvell/dreamplug/dreamplug.c b/board/Marvell/dreamplug/dreamplug.c
> > index e1c64b5224..d5b6b22ddf 100644
> > --- a/board/Marvell/dreamplug/dreamplug.c
> > +++ b/board/Marvell/dreamplug/dreamplug.c
> > @@ -1,5 +1,6 @@
> >   // SPDX-License-Identifier: GPL-2.0+
> >   /*
> > + * Copyright (C) 2021  Tony Dinh <mibodhi at gmail.com>
> >    * (C) Copyright 2011
> >    * Jason Cooper <u-boot at lakedaemon.net>
> >    *
> > @@ -97,42 +98,75 @@ int board_init(void)
> >       return 0;
> >   }
> >
> > +static int fdt_get_phy_addr(const char *path)
> > +{
> > +     const void *fdt = gd->fdt_blob;
> > +     const u32 *reg;
> > +     const u32 *val;
> > +     int node, phandle, addr;
> > +
> > +     /* Find the node by its full path */
> > +     node = fdt_path_offset(fdt, path);
> > +     if (node >= 0) {
> > +             /* Look up phy-handle */
> > +             val = fdt_getprop(fdt, node, "phy-handle", NULL);
> > +             if (val) {
> > +                     phandle = fdt32_to_cpu(*val);
> > +                     if (!phandle)
> > +                             return -1;
> > +                     /* Follow it to its node */
> > +                     node = fdt_node_offset_by_phandle(fdt, phandle);
> > +                     if (node) {
> > +                             /* Look up reg */
> > +                             reg = fdt_getprop(fdt, node, "reg", NULL);
> > +                             if (reg) {
> > +                                     addr = fdt32_to_cpu(*reg);
> > +                                     return addr;
> > +                             }
> > +                     }
> > +             }
> > +     }
> > +     return -1;
> > +}
>
> You've added this exact some function now for the 3rd time IIRC. Could
> please please consolidate this function into one of the fdt / dt common
> functions instead? Perhaps there is already something similar for
> reading PHY addresses?
>
> Please do this in a follow-up patch, after I've pulled this one.

Yes, I realized this function needs consolidation too, and mentioned
it in the patch for the Sheevaplug. Currently, it is hard to find a
home for this. ./common/fdt_support.c is one level of abstraction
below this fdt_get_phy_addr(). So it is not appropriate for this
function to be in that file.

I've looked at various Armada 37x/38x SoC DTS, the binding is a bit
different, i.e. the "phy-handle" is not defined as consistently as in
Kirkwood DTSs, so this function would not work for Marvell SoCs, in
general.

How about if we put this function in a new area in
arch/arm/mach-kirkwood/? such as arch/arm/mach-kirkwood/fdt/ ?

I'll send in a separate patch for this, which will apply to the
Kirkwood boards which have already been converted to DM Ethernet.

Thanks,
Tony

> Other that this:
>
> Reviewed-by: Stefan Roese <sr at denx.de>
>
> Thanks,
> Stefan
>
>
> > +
> >   #ifdef CONFIG_RESET_PHY_R
> > -void mv_phy_88e1116_init(char *name)
> > +void mv_phy_88e1116_init(const char *name, const char *path)
> >   {
> >       u16 reg;
> > -     u16 devadr;
> > +     int phyaddr;
> >
> >       if (miiphy_set_current_dev(name))
> >               return;
> >
> > -     /* command to read PHY dev address */
> > -     if (miiphy_read(name, 0xEE, 0xEE, (u16 *) &devadr)) {
> > -             printf("Err..%s could not read PHY dev address\n",
> > -                     __func__);
> > +     phyaddr = fdt_get_phy_addr(path);
> > +     if (phyaddr < 0)
> >               return;
> > -     }
> >
> >       /*
> >        * Enable RGMII delay on Tx and Rx for CPU port
> >        * Ref: sec 4.7.2 of chip datasheet
> >        */
> > -     miiphy_write(name, devadr, MV88E1116_PGADR_REG, 2);
> > -     miiphy_read(name, devadr, MV88E1116_MAC_CTRL2_REG, &reg);
> > +     miiphy_write(name, phyaddr, MV88E1116_PGADR_REG, 2);
> > +     miiphy_read(name, phyaddr, MV88E1116_MAC_CTRL2_REG, &reg);
> >       reg |= (MV88E1116_RGMII_RXTM_CTRL | MV88E1116_RGMII_TXTM_CTRL);
> > -     miiphy_write(name, devadr, MV88E1116_MAC_CTRL2_REG, reg);
> > -     miiphy_write(name, devadr, MV88E1116_PGADR_REG, 0);
> > +     miiphy_write(name, phyaddr, MV88E1116_MAC_CTRL2_REG, reg);
> > +     miiphy_write(name, phyaddr, MV88E1116_PGADR_REG, 0);
> >
> >       /* reset the phy */
> > -     miiphy_reset(name, devadr);
> > +     miiphy_reset(name, phyaddr);
> >
> >       printf("88E1116 Initialized on %s\n", name);
> >   }
> >
> >   void reset_phy(void)
> >   {
> > +     char *eth0_name = "ethernet-controller at 72000";
> > +     char *eth0_path = "/ocp at f1000000/ethernet-controller at 72000/ethernet0-port at 0";
> > +     char *eth1_name = "ethernet-controller at 76000";
> > +     char *eth1_path = "/ocp at f1000000/ethernet-controller at 72000/ethernet1-port at 0";
> > +
> >       /* configure and initialize both PHY's */
> > -     mv_phy_88e1116_init("egiga0");
> > -     mv_phy_88e1116_init("egiga1");
> > +     mv_phy_88e1116_init(eth0_name, eth0_path);
> > +     mv_phy_88e1116_init(eth1_name, eth1_path);
> >   }
> >   #endif /* CONFIG_RESET_PHY_R */
> >
>
>
> Viele Grüße,
> Stefan
>
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de