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=-8.8 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 0384DC433B4 for ; Sat, 17 Apr 2021 01:01:14 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6F0DD610FC for ; Sat, 17 Apr 2021 01:01:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6F0DD610FC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4FMZX74mPhz3cC0 for ; Sat, 17 Apr 2021 11:01:11 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=Uf7m8Z8p; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=kernel.org (client-ip=198.145.29.99; helo=mail.kernel.org; envelope-from=robh+dt@kernel.org; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=Uf7m8Z8p; dkim-atps=neutral Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4FMKdS0C63z301B for ; Sat, 17 Apr 2021 01:19:56 +1000 (AEST) Received: by mail.kernel.org (Postfix) with ESMTPSA id E56786101E for ; Fri, 16 Apr 2021 15:19:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1618586392; bh=pSsgz6SFE9awjWhpCd9doKdNFULnoUddJE0eDHwlPEw=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=Uf7m8Z8ptnraoZ51IQdKXyFPpb17ObjRbvh/tgHJQQ5o+ciQAAcW4tbcnun59sBJX Ov85d36wXd57lDyw06gGeyqcFbb5FjxK1gFC7Qrxt0siWMpBPxslLI1Ib7jRn6v++F cAmJS6suNmYyLsln2n1OJnlvyXwMidQQNjPAbPQs7MfkhM0fJUKSB+nyMr8L4qzQud HTwRuAwiftEcwCLZDIRxIKUVRJ/SoYxXJ/sWW/PMvfAdr8QdP7WvJBtbdVkTfMRnMB pphfcPjJEo4hhDegHATnmlmSQaHF3XNmwJ/bD1K3yRsXX8ChKcHmEOFBr928TWvaEy TDNKkp8iMeOHg== Received: by mail-qv1-f46.google.com with SMTP id i9so13411166qvo.3 for ; Fri, 16 Apr 2021 08:19:52 -0700 (PDT) X-Gm-Message-State: AOAM533q/H23BgRSfOOpCoTjCY0ihbfM/jp1XeXaxc3CjIgC+xxXwMm7 e7HTeRTreazbkDZ9hSod7ZuJz16zGGSN0W2W+A== X-Google-Smtp-Source: ABdhPJwFV5j0PpR0OOvbKTHrtQGM0/UHlnjLPz7IjIHFauq3IAwfsuj1+lSbzphYfDXiggRtdMYLzMTYnQvOIyCHQj8= X-Received: by 2002:ad4:5a07:: with SMTP id ei7mr8951384qvb.50.1618586391992; Fri, 16 Apr 2021 08:19:51 -0700 (PDT) MIME-Version: 1.0 References: <20210412174718.17382-1-michael@walle.cc> <20210412174718.17382-3-michael@walle.cc> <730d603b12e590c56770309b4df2bd668f7afbe3.camel@kernel.crashing.org> <8157eba9317609294da80472622deb28@walle.cc> In-Reply-To: <8157eba9317609294da80472622deb28@walle.cc> From: Rob Herring Date: Fri, 16 Apr 2021 10:19:40 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH net-next v4 2/2] of: net: fix of_get_mac_addr_nvmem() for non-platform devices To: Michael Walle Content-Type: text/plain; charset="UTF-8" X-Mailman-Approved-At: Sat, 17 Apr 2021 11:00:18 +1000 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Lunn , Paul Mackerras , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Nobuhiro Iwamatsu , "moderated list:ARM/STM32 ARCHITECTURE" , Jerome Brunet , Neil Armstrong , Michal Simek , Jose Abreu , NXP Linux Team , Mark Lee , Hauke Mehrtens , Sascha Hauer , Lorenzo Bianconi , linux-omap , Greg Kroah-Hartman , linux-wireless , "linux-kernel@vger.kernel.org" , Pengutronix Kernel Team , Vladimir Oltean , Claudiu Beznea , =?UTF-8?B?SsOpcsO0bWUgUG91aWxsZXI=?= , Kunihiko Hayashi , Chris Snook , Frank Rowand , Gregory Clement , Madalin Bucur , Martin Blumenstingl , Murali Karicheri , Yisen Zhuang , Alexandre Torgue , Wingman Kwok , Sean Wang , Maxime Ripard , Claudiu Manoil , "open list:ARM/Amlogic Meson..." , Kalle Valo , Mirko Lindner , Fugang Duan , Bryan Whitehead , QCA ath9k Development , Microchip Linux Driver Support , Taras Chornyi , Maxime Coquelin , Kevin Hilman , Heiner Kallweit , Andreas Larsson , Giuseppe Cavallaro , Fabio Estevam , Stanislaw Gruszka , Florian Fainelli , linux-staging@lists.linux.dev, Chen-Yu Tsai , "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" , linux-arm-kernel , Grygorii Strashko , Byungho An , Radhey Shyam Pandey , Vladimir Zapolskiy , John Crispin , Salil Mehta , Sergei Shtylyov , linux-oxnas@groups.io, Shawn Guo , "David S . Miller" , Helmut Schaa , Thomas Petazzoni , "open list:MEDIA DRIVERS FOR RENESAS - FCP" , Ryder Lee , Russell King , Vadym Kochan , Jakub Kicinski , Vivien Didelot , Sunil Goutham , Sebastian Hesselbarth , devicetree@vger.kernel.org, "moderated list:ARM/Mediatek SoC support" , Matthias Brugger , Jernej Skrabec , netdev , Nicolas Ferre , Li Yang , Stephen Hemminger , Vinod Koul , Joyce Ooi , linuxppc-dev , Felix Fietkau Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Fri, Apr 16, 2021 at 2:30 AM Michael Walle wrote: > > Am 2021-04-16 05:24, schrieb Benjamin Herrenschmidt: > > On Mon, 2021-04-12 at 19:47 +0200, Michael Walle wrote: > >> > >> /** > >> * of_get_phy_mode - Get phy mode for given device_node > >> @@ -59,15 +60,39 @@ static int of_get_mac_addr(struct device_node *np, > >> const char *name, u8 *addr) > >> static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr) > >> { > >> struct platform_device *pdev = of_find_device_by_node(np); > >> + struct nvmem_cell *cell; > >> + const void *mac; > >> + size_t len; > >> int ret; > >> > >> - if (!pdev) > >> - return -ENODEV; > >> + /* Try lookup by device first, there might be a > >> nvmem_cell_lookup > >> + * associated with a given device. > >> + */ > >> + if (pdev) { > >> + ret = nvmem_get_mac_address(&pdev->dev, addr); > >> + put_device(&pdev->dev); > >> + return ret; > >> + } > >> + > > > > This smells like the wrong band aid :) > > > > Any struct device can contain an OF node pointer these days. > > But not all nodes might have an associated device, see DSA for example. I believe what Ben is saying and what I said earlier is going from dev -> OF node is right and OF node -> dev is wrong. If you only have an OF node, then use an of_* function. > And as the name suggests of_get_mac_address() operates on a node. So > if a driver calls of_get_mac_address() it should work on the node. What > is wrong IMHO, is that the ethernet drivers where the corresponding > board > has a nvmem_cell_lookup registered is calling of_get_mac_address(node). > It should rather call eth_get_mac_address(dev) in the first place. > > One would need to figure out if there is an actual device (with an > assiciated of_node), then call eth_get_mac_address(dev) and if there > isn't a device call of_get_mac_address(node). Yes, I think we're all in agreement. > But I don't know if that is easy to figure out. Well, one could start > with just the device where nvmem_cell_lookup is used. Then we could > drop the workaround above. Start with the ones just passing dev.of_node directly: $ git grep 'of_get_mac_address(.*of_node)' drivers/net/ethernet/aeroflex/greth.c: addr = of_get_mac_address(ofdev->dev.of_node); drivers/net/ethernet/altera/altera_tse_main.c: macaddr = of_get_mac_address(pdev->dev.of_node); drivers/net/ethernet/arc/emac_main.c: mac_addr = of_get_mac_address(dev->of_node); drivers/net/ethernet/broadcom/bgmac-bcma.c: mac = of_get_mac_address(bgmac->dev->of_node); drivers/net/ethernet/cavium/octeon/octeon_mgmt.c: mac = of_get_mac_address(pdev->dev.of_node); drivers/net/ethernet/ethoc.c: mac = of_get_mac_address(pdev->dev.of_node); drivers/net/ethernet/ezchip/nps_enet.c: mac_addr = of_get_mac_address(dev->of_node); drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c: mac_addr = of_get_mac_address(ofdev->dev.of_node); drivers/net/ethernet/marvell/pxa168_eth.c: mac_addr = of_get_mac_address(pdev->dev.of_node); drivers/net/ethernet/marvell/sky2.c: iap = of_get_mac_address(hw->pdev->dev.of_node); drivers/net/ethernet/mediatek/mtk_eth_soc.c: mac_addr = of_get_mac_address(mac->of_node); drivers/net/ethernet/microchip/lan743x_main.c: mac_addr = of_get_mac_address(pdev->dev.of_node); drivers/net/ethernet/qualcomm/qca_spi.c: mac = of_get_mac_address(spi->dev.of_node); drivers/net/ethernet/qualcomm/qca_uart.c: mac = of_get_mac_address(serdev->dev.of_node); drivers/net/ethernet/wiznet/w5100-spi.c: const void *mac = of_get_mac_address(spi->dev.of_node); drivers/net/ethernet/xilinx/xilinx_axienet_main.c: mac_addr = of_get_mac_address(pdev->dev.of_node); drivers/net/ethernet/xilinx/xilinx_emaclite.c: mac_address = of_get_mac_address(ofdev->dev.of_node); drivers/net/wireless/ralink/rt2x00/rt2x00dev.c: mac_addr = of_get_mac_address(rt2x00dev->dev->of_node); drivers/staging/octeon/ethernet.c: mac = of_get_mac_address(priv->of_node); drivers/staging/wfx/main.c: macaddr = of_get_mac_address(wdev->dev->of_node); net/ethernet/eth.c: addr = of_get_mac_address(dev->of_node); Then this will find most of the rest: git grep -W 'of_get_mac_address([a-z]*)'| grep -E '(node|np)' Rob