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,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 3E461C433B4 for ; Tue, 13 Apr 2021 00:56:44 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 9D9A661206 for ; Tue, 13 Apr 2021 00:56:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9D9A661206 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lunn.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=YVNU9VcL5OISXQkF6NLQ8jFJuSOQTX/w4QPqhZNzZi8=; b=LeCrv3nAKySnGt7IJOcp3qu7J SMvh2Q14r5FOIbM41/ZePPBiG/yDQC5EeeVi56q4iqsnjTeR2zCDZEigrxB7o/wgFjUMin/l8tDKL vo72gJTxtJSMWyJNE9HhiRpN4x8TTmzwLyJ8AYqeJMgoY2Je4xzPcCLmim50nRbqfTYkxEN0H2ol+ CzMUc3KOHtJO0dKpD3xxu/YvV/H8i8318y+6uKRoqBanqMEtTrP70YCbUFL2eRs0+mNailnPHhOqg WwWoUIammcr33MouxAdNOLOMZ1kGRs2OCofINXXBy8FCBjfg9H3pDuAAJWbrRZ2llbaxSgQwBi34Z K2yThNz5A==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lW7M3-007zDB-4M; Tue, 13 Apr 2021 00:56:35 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lW7Lx-007zCQ-P2; Tue, 13 Apr 2021 00:56:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=JlgNNW6opfbYsb2HY0jilFyMHcv1A4vZ2Vd6HwbRFUs=; b=4akmqH38kFHzDFi9+44DcjaCnX iGDRGIFwNIuqKdYe+sH+u+Ai683yrGxORuyDjkrFdoQyn3yakQnfiJWlEIBZotDBGn5GqnkgDlW+h MeNH/gOXZFZj134OHplaFnIjYIbfs1oRWMvxmPo/quDceB1pR4vWj2eDnuPl8+IZhLri5WVDemBkB 1m23k3hg+dhkk+B06T2lYJnBHhOEHwrR+AQ0U5/AzvNfSH+F6907Q/8ItV7U3NLOi/1IpBL92tahP v4hJzZhlI+4NE+x8V0/UHPbjx5+aYuqEZwoXZ+00biYrsEmzofzCb/xUyx/7pwypty80EGBa6CS/E V9RWLzuA==; Received: from vps0.lunn.ch ([185.16.172.187]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lW7Lv-006ezb-3X; Tue, 13 Apr 2021 00:56:28 +0000 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1lW7LN-00GOEl-SK; Tue, 13 Apr 2021 02:55:53 +0200 Date: Tue, 13 Apr 2021 02:55:53 +0200 From: Andrew Lunn To: Michael Walle Cc: ath9k-devel@qca.qualcomm.com, UNGLinuxDriver@microchip.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, netdev@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-renesas-soc@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-amlogic@lists.infradead.org, linux-oxnas@groups.io, linux-omap@vger.kernel.org, linux-wireless@vger.kernel.org, devicetree@vger.kernel.org, linux-staging@lists.linux.dev, Gregory Clement , Sebastian Hesselbarth , Russell King , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , Andreas Larsson , "David S . Miller" , Jakub Kicinski , Maxime Ripard , Chen-Yu Tsai , Jernej Skrabec , Joyce Ooi , Chris Snook , =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= , bcm-kernel-feedback-list@broadcom.com, 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 , Rob Herring , Frank Rowand , Greg Kroah-Hartman , =?iso-8859-1?B?Suly9G1l?= Pouiller , Vivien Didelot , Vladimir Oltean Subject: Re: [PATCH net-next v4 1/2] of: net: pass the dst buffer to of_get_mac_address() Message-ID: References: <20210412174718.17382-1-michael@walle.cc> <20210412174718.17382-2-michael@walle.cc> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210412174718.17382-2-michael@walle.cc> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210412_175627_175920_EDF2ADB5 X-CRM114-Status: GOOD ( 29.78 ) X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org On Mon, Apr 12, 2021 at 07:47:17PM +0200, Michael Walle wrote: > of_get_mac_address() returns a "const void*" pointer to a MAC address. > Lately, support to fetch the MAC address by an NVMEM provider was added. > But this will only work with platform devices. It will not work with > PCI devices (e.g. of an integrated root complex) and esp. not with DSA > ports. > > There is an of_* variant of the nvmem binding which works without > devices. The returned data of a nvmem_cell_read() has to be freed after > use. On the other hand the return of_get_mac_address() points to some > static data without a lifetime. The trick for now, was to allocate a > device resource managed buffer which is then returned. This will only > work if we have an actual device. > > Change it, so that the caller of of_get_mac_address() has to supply a > buffer where the MAC address is written to. Unfortunately, this will > touch all drivers which use the of_get_mac_address(). > > Usually the code looks like: > > const char *addr; > addr = of_get_mac_address(np); > if (!IS_ERR(addr)) > ether_addr_copy(ndev->dev_addr, addr); > > This can then be simply rewritten as: > > of_get_mac_address(np, ndev->dev_addr); > > Sometimes is_valid_ether_addr() is used to test the MAC address. > of_get_mac_address() already makes sure, it just returns a valid MAC > address. Thus we can just test its return code. But we have to be > careful if there are still other sources for the MAC address before the > of_get_mac_address(). In this case we have to keep the > is_valid_ether_addr() call. > > The following coccinelle patch was used to convert common cases to the > new style. Afterwards, I've manually gone over the drivers and fixed the > return code variable: either used a new one or if one was already > available use that. Mansour Moufid, thanks for that coccinelle patch! > > > @a@ > identifier x; > expression y, z; > @@ > - x = of_get_mac_address(y); > + x = of_get_mac_address(y, z); > <... > - ether_addr_copy(z, x); > ...> > > @@ > identifier a.x; > @@ > - if (<+... x ...+>) {} > > @@ > identifier a.x; > @@ > if (<+... x ...+>) { > ... > } > - else {} > > @@ > identifier a.x; > expression e; > @@ > - if (<+... x ...+>@e) > - {} > - else > + if (!(e)) > {...} > > @@ > expression x, y, z; > @@ > - x = of_get_mac_address(y, z); > + of_get_mac_address(y, z); > ... when != x > > > All drivers, except drivers/net/ethernet/aeroflex/greth.c, were > compile-time tested. > > Suggested-by: Andrew Lunn > Signed-off-by: Michael Walle I cannot say i looked at all the changes, but the ones i did exam seemed O.K. Reviewed-by: Andrew Lunn Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic 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,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 B8EA8C433ED for ; Tue, 13 Apr 2021 00:56:49 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 2A5AD61003 for ; Tue, 13 Apr 2021 00:56:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2A5AD61003 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lunn.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=gvTQWwgnghsZMzAr/P5EAuLQKoShQpruM6uPy9Wnk/4=; b=emnHLBBy2ZEIRMJWsbt4LA5vK /ea8QqXIqBwKMZvYTaQaU7YnliMozfjXRBOpq/d6u40CjHwEj1W27CDewevLXjbYKQZdBClDJMxWX mh2uZJze6TYvl482tQS2xJ+LRqUWWoQpzr1MGMJFj2gIoBZZCDcD+eHg8SohOW0TlNPwE723cAPpq LWM3iYuIFFrh1DGwxaMQi6hi3VaIU2dAKwNXFWCge80DbcDOGBjnL/P+svz0Td9OZAYVRRPTmDhrm /pybay2SqDUhLFKSFhGXPZ/4tjpSNsNuF31oBOJ9hR/xHCRgfCW9ApvnZeoWuX56GHwb4z2AhA3Y1 iSa7dItRg==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lW7M1-007zD3-Jh; Tue, 13 Apr 2021 00:56:33 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lW7Lx-007zCQ-P2; Tue, 13 Apr 2021 00:56:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=JlgNNW6opfbYsb2HY0jilFyMHcv1A4vZ2Vd6HwbRFUs=; b=4akmqH38kFHzDFi9+44DcjaCnX iGDRGIFwNIuqKdYe+sH+u+Ai683yrGxORuyDjkrFdoQyn3yakQnfiJWlEIBZotDBGn5GqnkgDlW+h MeNH/gOXZFZj134OHplaFnIjYIbfs1oRWMvxmPo/quDceB1pR4vWj2eDnuPl8+IZhLri5WVDemBkB 1m23k3hg+dhkk+B06T2lYJnBHhOEHwrR+AQ0U5/AzvNfSH+F6907Q/8ItV7U3NLOi/1IpBL92tahP v4hJzZhlI+4NE+x8V0/UHPbjx5+aYuqEZwoXZ+00biYrsEmzofzCb/xUyx/7pwypty80EGBa6CS/E V9RWLzuA==; Received: from vps0.lunn.ch ([185.16.172.187]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lW7Lv-006ezb-3X; Tue, 13 Apr 2021 00:56:28 +0000 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1lW7LN-00GOEl-SK; Tue, 13 Apr 2021 02:55:53 +0200 Date: Tue, 13 Apr 2021 02:55:53 +0200 From: Andrew Lunn To: Michael Walle Cc: ath9k-devel@qca.qualcomm.com, UNGLinuxDriver@microchip.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, netdev@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-renesas-soc@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-amlogic@lists.infradead.org, linux-oxnas@groups.io, linux-omap@vger.kernel.org, linux-wireless@vger.kernel.org, devicetree@vger.kernel.org, linux-staging@lists.linux.dev, Gregory Clement , Sebastian Hesselbarth , Russell King , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , Andreas Larsson , "David S . Miller" , Jakub Kicinski , Maxime Ripard , Chen-Yu Tsai , Jernej Skrabec , Joyce Ooi , Chris Snook , =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= , bcm-kernel-feedback-list@broadcom.com, 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 , Rob Herring , Frank Rowand , Greg Kroah-Hartman , =?iso-8859-1?B?Suly9G1l?= Pouiller , Vivien Didelot , Vladimir Oltean Subject: Re: [PATCH net-next v4 1/2] of: net: pass the dst buffer to of_get_mac_address() Message-ID: References: <20210412174718.17382-1-michael@walle.cc> <20210412174718.17382-2-michael@walle.cc> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210412174718.17382-2-michael@walle.cc> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210412_175627_175920_EDF2ADB5 X-CRM114-Status: GOOD ( 29.78 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Mon, Apr 12, 2021 at 07:47:17PM +0200, Michael Walle wrote: > of_get_mac_address() returns a "const void*" pointer to a MAC address. > Lately, support to fetch the MAC address by an NVMEM provider was added. > But this will only work with platform devices. It will not work with > PCI devices (e.g. of an integrated root complex) and esp. not with DSA > ports. > > There is an of_* variant of the nvmem binding which works without > devices. The returned data of a nvmem_cell_read() has to be freed after > use. On the other hand the return of_get_mac_address() points to some > static data without a lifetime. The trick for now, was to allocate a > device resource managed buffer which is then returned. This will only > work if we have an actual device. > > Change it, so that the caller of of_get_mac_address() has to supply a > buffer where the MAC address is written to. Unfortunately, this will > touch all drivers which use the of_get_mac_address(). > > Usually the code looks like: > > const char *addr; > addr = of_get_mac_address(np); > if (!IS_ERR(addr)) > ether_addr_copy(ndev->dev_addr, addr); > > This can then be simply rewritten as: > > of_get_mac_address(np, ndev->dev_addr); > > Sometimes is_valid_ether_addr() is used to test the MAC address. > of_get_mac_address() already makes sure, it just returns a valid MAC > address. Thus we can just test its return code. But we have to be > careful if there are still other sources for the MAC address before the > of_get_mac_address(). In this case we have to keep the > is_valid_ether_addr() call. > > The following coccinelle patch was used to convert common cases to the > new style. Afterwards, I've manually gone over the drivers and fixed the > return code variable: either used a new one or if one was already > available use that. Mansour Moufid, thanks for that coccinelle patch! > > > @a@ > identifier x; > expression y, z; > @@ > - x = of_get_mac_address(y); > + x = of_get_mac_address(y, z); > <... > - ether_addr_copy(z, x); > ...> > > @@ > identifier a.x; > @@ > - if (<+... x ...+>) {} > > @@ > identifier a.x; > @@ > if (<+... x ...+>) { > ... > } > - else {} > > @@ > identifier a.x; > expression e; > @@ > - if (<+... x ...+>@e) > - {} > - else > + if (!(e)) > {...} > > @@ > expression x, y, z; > @@ > - x = of_get_mac_address(y, z); > + of_get_mac_address(y, z); > ... when != x > > > All drivers, except drivers/net/ethernet/aeroflex/greth.c, were > compile-time tested. > > Suggested-by: Andrew Lunn > Signed-off-by: Michael Walle I cannot say i looked at all the changes, but the ones i did exam seemed O.K. Reviewed-by: Andrew Lunn Andrew _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek 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.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 5FB29C43460 for ; Tue, 13 Apr 2021 00:56:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3BB0C61003 for ; Tue, 13 Apr 2021 00:56:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238858AbhDMA4u (ORCPT ); Mon, 12 Apr 2021 20:56:50 -0400 Received: from vps0.lunn.ch ([185.16.172.187]:47030 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238450AbhDMA4u (ORCPT ); Mon, 12 Apr 2021 20:56:50 -0400 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1lW7LN-00GOEl-SK; Tue, 13 Apr 2021 02:55:53 +0200 Date: Tue, 13 Apr 2021 02:55:53 +0200 From: Andrew Lunn To: Michael Walle Cc: ath9k-devel@qca.qualcomm.com, UNGLinuxDriver@microchip.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, netdev@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-renesas-soc@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-amlogic@lists.infradead.org, linux-oxnas@groups.io, linux-omap@vger.kernel.org, linux-wireless@vger.kernel.org, devicetree@vger.kernel.org, linux-staging@lists.linux.dev, Gregory Clement , Sebastian Hesselbarth , Russell King , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , Andreas Larsson , "David S . Miller" , Jakub Kicinski , Maxime Ripard , Chen-Yu Tsai , Jernej Skrabec , Joyce Ooi , Chris Snook , =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= , bcm-kernel-feedback-list@broadcom.com, 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 , Rob Herring , Frank Rowand , Greg Kroah-Hartman , =?iso-8859-1?B?Suly9G1l?= Pouiller , Vivien Didelot , Vladimir Oltean Subject: Re: [PATCH net-next v4 1/2] of: net: pass the dst buffer to of_get_mac_address() Message-ID: References: <20210412174718.17382-1-michael@walle.cc> <20210412174718.17382-2-michael@walle.cc> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210412174718.17382-2-michael@walle.cc> Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org On Mon, Apr 12, 2021 at 07:47:17PM +0200, Michael Walle wrote: > of_get_mac_address() returns a "const void*" pointer to a MAC address. > Lately, support to fetch the MAC address by an NVMEM provider was added. > But this will only work with platform devices. It will not work with > PCI devices (e.g. of an integrated root complex) and esp. not with DSA > ports. > > There is an of_* variant of the nvmem binding which works without > devices. The returned data of a nvmem_cell_read() has to be freed after > use. On the other hand the return of_get_mac_address() points to some > static data without a lifetime. The trick for now, was to allocate a > device resource managed buffer which is then returned. This will only > work if we have an actual device. > > Change it, so that the caller of of_get_mac_address() has to supply a > buffer where the MAC address is written to. Unfortunately, this will > touch all drivers which use the of_get_mac_address(). > > Usually the code looks like: > > const char *addr; > addr = of_get_mac_address(np); > if (!IS_ERR(addr)) > ether_addr_copy(ndev->dev_addr, addr); > > This can then be simply rewritten as: > > of_get_mac_address(np, ndev->dev_addr); > > Sometimes is_valid_ether_addr() is used to test the MAC address. > of_get_mac_address() already makes sure, it just returns a valid MAC > address. Thus we can just test its return code. But we have to be > careful if there are still other sources for the MAC address before the > of_get_mac_address(). In this case we have to keep the > is_valid_ether_addr() call. > > The following coccinelle patch was used to convert common cases to the > new style. Afterwards, I've manually gone over the drivers and fixed the > return code variable: either used a new one or if one was already > available use that. Mansour Moufid, thanks for that coccinelle patch! > > > @a@ > identifier x; > expression y, z; > @@ > - x = of_get_mac_address(y); > + x = of_get_mac_address(y, z); > <... > - ether_addr_copy(z, x); > ...> > > @@ > identifier a.x; > @@ > - if (<+... x ...+>) {} > > @@ > identifier a.x; > @@ > if (<+... x ...+>) { > ... > } > - else {} > > @@ > identifier a.x; > expression e; > @@ > - if (<+... x ...+>@e) > - {} > - else > + if (!(e)) > {...} > > @@ > expression x, y, z; > @@ > - x = of_get_mac_address(y, z); > + of_get_mac_address(y, z); > ... when != x > > > All drivers, except drivers/net/ethernet/aeroflex/greth.c, were > compile-time tested. > > Suggested-by: Andrew Lunn > Signed-off-by: Michael Walle I cannot say i looked at all the changes, but the ones i did exam seemed O.K. Reviewed-by: Andrew Lunn Andrew 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.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 3CC6BC433ED for ; Tue, 13 Apr 2021 01:47:52 +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 1CB88600D1 for ; Tue, 13 Apr 2021 01:47:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1CB88600D1 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lunn.ch 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 4FK7ln2SZ1z3c1q for ; Tue, 13 Apr 2021 11:47:49 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lunn.ch (client-ip=185.16.172.187; helo=vps0.lunn.ch; envelope-from=andrew@lunn.ch; receiver=) Received: from vps0.lunn.ch (vps0.lunn.ch [185.16.172.187]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4FK6cR0V4Zz2xYn for ; Tue, 13 Apr 2021 10:56:22 +1000 (AEST) Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1lW7LN-00GOEl-SK; Tue, 13 Apr 2021 02:55:53 +0200 Date: Tue, 13 Apr 2021 02:55:53 +0200 From: Andrew Lunn To: Michael Walle Subject: Re: [PATCH net-next v4 1/2] of: net: pass the dst buffer to of_get_mac_address() Message-ID: References: <20210412174718.17382-1-michael@walle.cc> <20210412174718.17382-2-michael@walle.cc> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210412174718.17382-2-michael@walle.cc> X-Mailman-Approved-At: Tue, 13 Apr 2021 11:47:28 +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: Paul Mackerras , =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= , Nobuhiro Iwamatsu , linux-stm32@st-md-mailman.stormreply.com, Jerome Brunet , Neil Armstrong , Michal Simek , Jose Abreu , NXP Linux Team , Mark Lee , Hauke Mehrtens , Sascha Hauer , Lorenzo Bianconi , linux-omap@vger.kernel.org, Greg Kroah-Hartman , linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, Pengutronix Kernel Team , Vladimir Oltean , Claudiu Beznea , =?iso-8859-1?B?Suly9G1l?= Pouiller , 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 , linux-amlogic@lists.infradead.org, Kalle Valo , Mirko Lindner , Fugang Duan , Bryan Whitehead , ath9k-devel@qca.qualcomm.com, UNGLinuxDriver@microchip.com, 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 , bcm-kernel-feedback-list@broadcom.com, linux-arm-kernel@lists.infradead.org, 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 , linux-renesas-soc@vger.kernel.org, Ryder Lee , Russell King , Vadym Kochan , Jakub Kicinski , Vivien Didelot , Sunil Goutham , Sebastian Hesselbarth , devicetree@vger.kernel.org, Rob Herring , linux-mediatek@lists.infradead.org, Matthias Brugger , Jernej Skrabec , netdev@vger.kernel.org, Nicolas Ferre , Li Yang , Stephen Hemminger , Vinod Koul , Joyce Ooi , linuxppc-dev@lists.ozlabs.org, Felix Fietkau Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Mon, Apr 12, 2021 at 07:47:17PM +0200, Michael Walle wrote: > of_get_mac_address() returns a "const void*" pointer to a MAC address. > Lately, support to fetch the MAC address by an NVMEM provider was added. > But this will only work with platform devices. It will not work with > PCI devices (e.g. of an integrated root complex) and esp. not with DSA > ports. > > There is an of_* variant of the nvmem binding which works without > devices. The returned data of a nvmem_cell_read() has to be freed after > use. On the other hand the return of_get_mac_address() points to some > static data without a lifetime. The trick for now, was to allocate a > device resource managed buffer which is then returned. This will only > work if we have an actual device. > > Change it, so that the caller of of_get_mac_address() has to supply a > buffer where the MAC address is written to. Unfortunately, this will > touch all drivers which use the of_get_mac_address(). > > Usually the code looks like: > > const char *addr; > addr = of_get_mac_address(np); > if (!IS_ERR(addr)) > ether_addr_copy(ndev->dev_addr, addr); > > This can then be simply rewritten as: > > of_get_mac_address(np, ndev->dev_addr); > > Sometimes is_valid_ether_addr() is used to test the MAC address. > of_get_mac_address() already makes sure, it just returns a valid MAC > address. Thus we can just test its return code. But we have to be > careful if there are still other sources for the MAC address before the > of_get_mac_address(). In this case we have to keep the > is_valid_ether_addr() call. > > The following coccinelle patch was used to convert common cases to the > new style. Afterwards, I've manually gone over the drivers and fixed the > return code variable: either used a new one or if one was already > available use that. Mansour Moufid, thanks for that coccinelle patch! > > > @a@ > identifier x; > expression y, z; > @@ > - x = of_get_mac_address(y); > + x = of_get_mac_address(y, z); > <... > - ether_addr_copy(z, x); > ...> > > @@ > identifier a.x; > @@ > - if (<+... x ...+>) {} > > @@ > identifier a.x; > @@ > if (<+... x ...+>) { > ... > } > - else {} > > @@ > identifier a.x; > expression e; > @@ > - if (<+... x ...+>@e) > - {} > - else > + if (!(e)) > {...} > > @@ > expression x, y, z; > @@ > - x = of_get_mac_address(y, z); > + of_get_mac_address(y, z); > ... when != x > > > All drivers, except drivers/net/ethernet/aeroflex/greth.c, were > compile-time tested. > > Suggested-by: Andrew Lunn > Signed-off-by: Michael Walle I cannot say i looked at all the changes, but the ones i did exam seemed O.K. Reviewed-by: Andrew Lunn Andrew