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

[Stefan Roese]

Hi Tony,

(added Joe & Ramon as network custodians)

On 31.07.21 11:55, Tony Dinh wrote:
> 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.

Joe, Ramon, do you know of any DT / FDT helper function already
available to read the PHY address from the device-tree? Or if not,
where would you like to have this new function being placed in the
U-Boot source tree?

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

Not sure here (sorry for not looking deeper into the docs), but what is
the difference between the "phy-handle" and "phy" property here in the
ethernet DT nodes? Do we perhaps need to handle both properties?

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

I'm not a fan of using a kirkwood specific place here. This sounds like
a common (platform independent) issue and should be able to be used
by all boards.

Thanks,
Stefan

> 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


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