diff mbox series

[v2] drivers: net: Replace acpi_bus_get_device()

Message ID 11918902.O9o76ZdvQC@kreacher
State New
Headers show
Series [v2] drivers: net: Replace acpi_bus_get_device() | expand

Commit Message

Rafael J. Wysocki Feb. 1, 2022, 7:58 p.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] drivers: net: Replace acpi_bus_get_device()

Replace acpi_bus_get_device() that is going to be dropped with
acpi_fetch_acpi_dev().

While at it, rearrange the local variable definitions in
bgx_acpi_register_phy() and mdio-xgene.c:acpi_register_phy() so as
to put them in the reverse xmas tree order.

No intentional functional impact.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2: Put local variable definitions in two functions the reverse xmas tree
       order (Andrew Lunn).

---
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c |    6 +++---
 drivers/net/fjes/fjes_main.c                      |   10 +++-------
 drivers/net/mdio/mdio-xgene.c                     |   10 ++++------
 3 files changed, 10 insertions(+), 16 deletions(-)

Comments

Jakub Kicinski Feb. 2, 2022, 4:14 a.m. UTC | #1 
On Tue, 01 Feb 2022 20:58:36 +0100 Rafael J. Wysocki wrote:
> -	struct bgx *bgx = context;
> +	struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
>  	struct device *dev = &bgx->pdev->dev;
> -	struct acpi_device *adev;
> +	struct bgx *bgx = context;

Compiler says you can't move bgx before dev.

Venturing deeper into the bikesheeding territory but I'd leave the
variable declarations be and move init of adev before the check.
Matter of preference but calling something that needs to be error 
checked in variable init breaks the usual
	
	ret = func(some, arguments);
	if (ret)
		goto explosions;

flow.

> -	if (acpi_bus_get_device(handle, &adev))
> +	if (!adev)
>  		goto out;
kernel test robot Feb. 2, 2022, 10:46 a.m. UTC | #2 
Hi "Rafael,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on net-next/master horms-ipvs/master linus/master v5.17-rc2 next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Rafael-J-Wysocki/drivers-net-Replace-acpi_bus_get_device/20220202-035902
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 881cc731df6af99a21622e9be25a23b81adcd10b
config: x86_64-allmodconfig (https://download.01.org/0day-ci/archive/20220202/202202021810.82z7OPTR-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1d2a29e30eb391a02f25f551e6f4242e32f5b01f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Rafael-J-Wysocki/drivers-net-Replace-acpi_bus_get_device/20220202-035902
        git checkout 1d2a29e30eb391a02f25f551e6f4242e32f5b01f
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/media/cec/platform/seco/ drivers/net/ethernet/cavium/thunder/ drivers/net/wireless/ath/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1409:24: error: use of undeclared identifier 'bgx'
           struct device *dev = &bgx->pdev->dev;
                                 ^
   1 error generated.


vim +/bgx +1409 drivers/net/ethernet/cavium/thunder/thunder_bgx.c

46b903a01c053d0 David Daney       2015-08-10  1403  
46b903a01c053d0 David Daney       2015-08-10  1404  /* Currently only sets the MAC address. */
46b903a01c053d0 David Daney       2015-08-10  1405  static acpi_status bgx_acpi_register_phy(acpi_handle handle,
46b903a01c053d0 David Daney       2015-08-10  1406  					 u32 lvl, void *context, void **rv)
46b903a01c053d0 David Daney       2015-08-10  1407  {
1d2a29e30eb391a Rafael J. Wysocki 2022-02-01  1408  	struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
1d82efaca87ecf5 Robert Richter    2016-02-11 @1409  	struct device *dev = &bgx->pdev->dev;
1d2a29e30eb391a Rafael J. Wysocki 2022-02-01  1410  	struct bgx *bgx = context;
46b903a01c053d0 David Daney       2015-08-10  1411  
1d2a29e30eb391a Rafael J. Wysocki 2022-02-01  1412  	if (!adev)
46b903a01c053d0 David Daney       2015-08-10  1413  		goto out;
46b903a01c053d0 David Daney       2015-08-10  1414  
7aa4865506a26c6 Vadim Lomovtsev   2017-01-12  1415  	acpi_get_mac_address(dev, adev, bgx->lmac[bgx->acpi_lmac_idx].mac);
46b903a01c053d0 David Daney       2015-08-10  1416  
7aa4865506a26c6 Vadim Lomovtsev   2017-01-12  1417  	SET_NETDEV_DEV(&bgx->lmac[bgx->acpi_lmac_idx].netdev, dev);
46b903a01c053d0 David Daney       2015-08-10  1418  
7aa4865506a26c6 Vadim Lomovtsev   2017-01-12  1419  	bgx->lmac[bgx->acpi_lmac_idx].lmacid = bgx->acpi_lmac_idx;
7aa4865506a26c6 Vadim Lomovtsev   2017-01-12  1420  	bgx->acpi_lmac_idx++; /* move to next LMAC */
46b903a01c053d0 David Daney       2015-08-10  1421  out:
46b903a01c053d0 David Daney       2015-08-10  1422  	return AE_OK;
46b903a01c053d0 David Daney       2015-08-10  1423  }
46b903a01c053d0 David Daney       2015-08-10  1424  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Rafael J. Wysocki Feb. 2, 2022, 1:54 p.m. UTC | #3 
On Wed, Feb 2, 2022 at 5:20 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 01 Feb 2022 20:58:36 +0100 Rafael J. Wysocki wrote:
> > -     struct bgx *bgx = context;
> > +     struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
> >       struct device *dev = &bgx->pdev->dev;
> > -     struct acpi_device *adev;
> > +     struct bgx *bgx = context;
>
> Compiler says you can't move bgx before dev.

Right, I've obviously missed that.

> Venturing deeper into the bikesheeding territory but I'd leave the
> variable declarations be and move init of adev before the check.
> Matter of preference but calling something that needs to be error
> checked in variable init breaks the usual
>
>         ret = func(some, arguments);
>         if (ret)
>                 goto explosions;
>
> flow.

It doesn't for me, but let me send a v3.

Thanks!

> > -     if (acpi_bus_get_device(handle, &adev))
> > +     if (!adev)
> >               goto out;
diff mbox series

Patch

Index: linux-pm/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
===================================================================
--- linux-pm.orig/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ linux-pm/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -1405,11 +1405,11 @@  static int acpi_get_mac_address(struct d
 static acpi_status bgx_acpi_register_phy(acpi_handle handle,
 					 u32 lvl, void *context, void **rv)
 {
-	struct bgx *bgx = context;
+	struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
 	struct device *dev = &bgx->pdev->dev;
-	struct acpi_device *adev;
+	struct bgx *bgx = context;
 
-	if (acpi_bus_get_device(handle, &adev))
+	if (!adev)
 		goto out;
 
 	acpi_get_mac_address(dev, adev, bgx->lmac[bgx->acpi_lmac_idx].mac);
Index: linux-pm/drivers/net/fjes/fjes_main.c
===================================================================
--- linux-pm.orig/drivers/net/fjes/fjes_main.c
+++ linux-pm/drivers/net/fjes/fjes_main.c
@@ -1512,15 +1512,11 @@  static acpi_status
 acpi_find_extended_socket_device(acpi_handle obj_handle, u32 level,
 				 void *context, void **return_value)
 {
-	struct acpi_device *device;
+	struct acpi_device *device = acpi_fetch_acpi_dev(obj_handle);
 	bool *found = context;
-	int result;
 
-	result = acpi_bus_get_device(obj_handle, &device);
-	if (result)
-		return AE_OK;
-
-	if (strcmp(acpi_device_hid(device), ACPI_MOTHERBOARD_RESOURCE_HID))
+	if (!device ||
+	    strcmp(acpi_device_hid(device), ACPI_MOTHERBOARD_RESOURCE_HID))
 		return AE_OK;
 
 	if (!is_extended_socket_device(device))
Index: linux-pm/drivers/net/mdio/mdio-xgene.c
===================================================================
--- linux-pm.orig/drivers/net/mdio/mdio-xgene.c
+++ linux-pm/drivers/net/mdio/mdio-xgene.c
@@ -279,16 +279,14 @@  EXPORT_SYMBOL(xgene_enet_phy_register);
 static acpi_status acpi_register_phy(acpi_handle handle, u32 lvl,
 				     void *context, void **ret)
 {
+	struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
 	struct mii_bus *mdio = context;
-	struct acpi_device *adev;
-	struct phy_device *phy_dev;
 	const union acpi_object *obj;
+	struct phy_device *phy_dev;
 	u32 phy_addr;
 
-	if (acpi_bus_get_device(handle, &adev))
-		return AE_OK;
-
-	if (acpi_dev_get_property(adev, "phy-channel", ACPI_TYPE_INTEGER, &obj))
+	if (!adev ||
+	    acpi_dev_get_property(adev, "phy-channel", ACPI_TYPE_INTEGER, &obj))
 		return AE_OK;
 	phy_addr = obj->integer.value;