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=-11.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,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 8C122C433ED for ; Fri, 16 Apr 2021 15:21:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6368861166 for ; Fri, 16 Apr 2021 15:21:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244855AbhDPPVk (ORCPT ); Fri, 16 Apr 2021 11:21:40 -0400 Received: from mail.kernel.org ([198.145.29.99]:45386 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240740AbhDPPUS (ORCPT ); Fri, 16 Apr 2021 11:20:18 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 126B76124B; Fri, 16 Apr 2021 15:19:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1618586393; bh=pSsgz6SFE9awjWhpCd9doKdNFULnoUddJE0eDHwlPEw=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=UxAqG9OtRb+QQxIjEDiDCQjDyuQCAA2d4ZFOE0N0q01hfecceetNP30Wn2cpoOVE9 WzIl3RlQROaX8eslzhr1HB02OC63p5qRAgAuwInnYgmiv5fbcBC9TWTmYo232GGxlq zl187zQg298hyuS7vBp9IbYu6DRnXQnCKamPIPA4bmh5K0mikLKCBT6to2dCP1x8ty zZx9ADxRCSv4iMzbDba1VKhTd/WRazgU+F/ual3PjLXwH8R3NcS540TNXA2d3csxLQ 52sEoFTMSZBaj5Io5xR7NND2H/FGQQX/xOraxhVAgnOADPCX7A2gJRiZJ6UQi8EIca qbHUW/1etaTZw== Received: by mail-qv1-f50.google.com with SMTP id dp18so8964626qvb.5; Fri, 16 Apr 2021 08:19:53 -0700 (PDT) X-Gm-Message-State: AOAM532l1zBWHnAq4t2Jj5dnVbng16I/pUevkfmsOrp4a9gX2OyAZlsE nsvKvN+kzewR/ABkKCT91eKm6uDkXmC8UEFLuQ== 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 Cc: Benjamin Herrenschmidt , QCA ath9k Development , Microchip Linux Driver Support , linux-arm-kernel , "linux-kernel@vger.kernel.org" , linuxppc-dev , netdev , "moderated list:ARM/Mediatek SoC support" , "open list:MEDIA DRIVERS FOR RENESAS - FCP" , "moderated list:ARM/STM32 ARCHITECTURE" , "open list:ARM/Amlogic Meson..." , linux-oxnas@groups.io, linux-omap , linux-wireless , devicetree@vger.kernel.org, linux-staging@lists.linux.dev, Andrew Lunn , Gregory Clement , Sebastian Hesselbarth , Russell King , Michael Ellerman , Paul Mackerras , Andreas Larsson , "David S . Miller" , Jakub Kicinski , Maxime Ripard , Chen-Yu Tsai , Jernej Skrabec , Joyce Ooi , Chris Snook , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" , Florian Fainelli , Nicolas Ferre , Claudiu Beznea , Sunil Goutham , Fugang Duan , Madalin Bucur , Pantelis Antoniou , Claudiu Manoil , Li Yang , Yisen Zhuang , Salil Mehta , Hauke Mehrtens , Thomas Petazzoni , Vadym Kochan , Taras Chornyi , Mirko Lindner , Stephen Hemminger , Felix Fietkau , John Crispin , Sean Wang , Mark Lee , Matthias Brugger , Bryan Whitehead , Vladimir Zapolskiy , Sergei Shtylyov , Byungho An , Kunihiko Hayashi , Giuseppe Cavallaro , Alexandre Torgue , Jose Abreu , Maxime Coquelin , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Kevin Hilman , Neil Armstrong , Jerome Brunet , Martin Blumenstingl , Vinod Koul , Nobuhiro Iwamatsu , Grygorii Strashko , Wingman Kwok , Murali Karicheri , Michal Simek , Radhey Shyam Pandey , Kalle Valo , Lorenzo Bianconi , Ryder Lee , Stanislaw Gruszka , Helmut Schaa , Heiner Kallweit , Frank Rowand , Greg Kroah-Hartman , =?UTF-8?B?SsOpcsO0bWUgUG91aWxsZXI=?= , Vivien Didelot , Vladimir Oltean Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org 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