From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A5166C432BE for ; Sat, 31 Jul 2021 11:50:18 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4DEAB60F94 for ; Sat, 31 Jul 2021 11:50:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4DEAB60F94 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id D65C182DC1; Sat, 31 Jul 2021 13:50:14 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1627732215; bh=X/A9zLDgvuh9W3PzY6CeJokeQ2Ahkf9yJDpvpqHLxy4=; h=Subject:From:To:Cc:References:Date:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=OfTm9QgRwSZlqHIoLJgI3MAvw9a0AV+Hsl7zTdXs0NlaEIpPLwec7SHsrgjBFySN3 +geo/e1QTTU3BfJCWA/ah3/GMCQe7O8ZRF1C+JsKfA1Bzgnzi7g/0P2JNYFPHAdZK2 J0gHj/3S1ggocpL316QJy7EaZVexJvw7Tci2xC7Bjg5gKn53LMrdeQBC8Ptz5MnryM 5womzNFeCL+Sk51MUFjoORwFBIo0zMr/a1XQpvDnO7JD+lGWgEW/AphxuMb1auwPz9 ikxU7WIh9Sx6ka3lW+BRgLVIi7wTYE1j1oVVs2W77fabzCYpI7zy2tufiHaEFxV8c+ juWLxwKm4JrzA== Received: by phobos.denx.de (Postfix, from userid 109) id 626B282DC1; Sat, 31 Jul 2021 13:50:13 +0200 (CEST) Received: from mout-u-107.mailbox.org (mout-u-107.mailbox.org [IPv6:2001:67c:2050:1::465:107]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id BCDC982C9B for ; Sat, 31 Jul 2021 13:50:09 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=sr@denx.de Received: from smtp2.mailbox.org (smtp2.mailbox.org [IPv6:2001:67c:2050:105:465:1:2:0]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-u-107.mailbox.org (Postfix) with ESMTPS id 4GcMyT398VzQjgn; Sat, 31 Jul 2021 13:50:09 +0200 (CEST) Received: from smtp2.mailbox.org ([80.241.60.241]) by spamfilter03.heinlein-hosting.de (spamfilter03.heinlein-hosting.de [80.241.56.117]) (amavisd-new, port 10030) with ESMTP id cGmob32-0Zmc; Sat, 31 Jul 2021 13:50:05 +0200 (CEST) Subject: Re: [PATCH 2/3] arm: kirkwood: Dreamplug: Use Ethernet PHY name and address from device tree From: Stefan Roese To: Tony Dinh Cc: U-Boot Mailing List , Jason Cooper , Chris Packham , Tom Rini , Simon Glass , Ramon Fried , Joe Hershberger References: <20210726060121.7253-1-mibodhi@gmail.com> <20210726060121.7253-3-mibodhi@gmail.com> <64fd303b-ca30-b2b3-92ef-d31e99299fed@denx.de> <0d146635-817f-182f-d04d-c8c890279c6b@denx.de> Message-ID: Date: Sat, 31 Jul 2021 13:50:05 +0200 MIME-Version: 1.0 In-Reply-To: <0d146635-817f-182f-d04d-c8c890279c6b@denx.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 51AD51839 X-Rspamd-UID: 5c18da X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On 31.07.21 12:27, Stefan Roese wrote: > 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 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 >>>> --- >>>> >>>>    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 >>>>     * (C) Copyright 2011 >>>>     * Jason Cooper >>>>     * >>>> @@ -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? It just now came to my mind that the "normal" functions for base address reading like dev_read_addr(), fdt_get_base_address() and friends can most likely be used for PHY MDIO address reading as well. Thanks, Stefan > 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 >>> >>> 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, ®); >>>> +     miiphy_write(name, phyaddr, MV88E1116_PGADR_REG, 2); >>>> +     miiphy_read(name, phyaddr, MV88E1116_MAC_CTRL2_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@72000"; >>>> +     char *eth0_path = >>>> "/ocp@f1000000/ethernet-controller@72000/ethernet0-port@0"; >>>> +     char *eth1_name = "ethernet-controller@76000"; >>>> +     char *eth1_path = >>>> "/ocp@f1000000/ethernet-controller@72000/ethernet1-port@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@denx.de > > > Viele Grüße, > Stefan > 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@denx.de