diff mbox series

[v2,5/6] bnxt_en: Use auxiliary bus calls over proprietary calls

Message ID 20221025173110.33192-6-ajit.khaparde@broadcom.com (mailing list archive)
State Superseded
Headers show
Series [v2,1/6] bnxt_en: Add auxiliary driver support | expand

Commit Message

Ajit Khaparde Oct. 25, 2022, 5:31 p.m. UTC 
Wherever possible use the function ops provided by auxiliary bus
instead of using proprietary ops.

Defined bnxt_re_suspend and bnxt_re_resume calls which can be
invoked by the bnxt_en driver instead of the ULP stop/start calls.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c          | 102 +++++++++++-------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  40 ++++---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |   2 -
 3 files changed, 87 insertions(+), 57 deletions(-)

Comments

kernel test robot Oct. 25, 2022, 9:18 p.m. UTC | #1 
Hi Ajit,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.1-rc2 next-20221025]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ajit-Khaparde/bnxt_en-Add-auxiliary-driver-support/20221026-023141
patch link:    https://lore.kernel.org/r/20221025173110.33192-6-ajit.khaparde%40broadcom.com
patch subject: [PATCH v2 5/6] bnxt_en: Use auxiliary bus calls over proprietary calls
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 12.1.0
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/intel-lab-lkp/linux/commit/f04080c956abfeabf32961081d0401a800163d99
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Ajit-Khaparde/bnxt_en-Add-auxiliary-driver-support/20221026-023141
        git checkout f04080c956abfeabf32961081d0401a800163d99
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/infiniband/hw/bnxt_re/

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

All warnings (new ones prefixed by >>):

>> drivers/infiniband/hw/bnxt_re/main.c:1585:5: warning: no previous prototype for 'bnxt_re_suspend' [-Wmissing-prototypes]
    1585 | int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
         |     ^~~~~~~~~~~~~~~
>> drivers/infiniband/hw/bnxt_re/main.c:1623:5: warning: no previous prototype for 'bnxt_re_resume' [-Wmissing-prototypes]
    1623 | int bnxt_re_resume(struct auxiliary_device *adev)
         |     ^~~~~~~~~~~~~~


vim +/bnxt_re_suspend +1585 drivers/infiniband/hw/bnxt_re/main.c

  1584	
> 1585	int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
  1586	{
  1587		struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
  1588		struct bnxt *bp;
  1589	
  1590		if (!rdev)
  1591			return 0;
  1592	
  1593		mutex_lock(&bnxt_re_mutex);
  1594		/* L2 driver may invoke this callback during device error/crash or device
  1595		 * reset. Current RoCE driver doesn't recover the device in case of
  1596		 * error. Handle the error by dispatching fatal events to all qps
  1597		 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
  1598		 * L2 driver want to modify the MSIx table.
  1599		 */
  1600		bp = netdev_priv(rdev->netdev);
  1601	
  1602		ibdev_info(&rdev->ibdev, "Handle device suspend call");
  1603		/* Check the current device state from L2 structure and move the
  1604		 * device to detached state if FW_FATAL_COND is set.
  1605		 * This prevents more commands to HW during clean-up,
  1606		 * in case the device is already in error.
  1607		 */
  1608		if (test_bit(BNXT_STATE_FW_FATAL_COND, &bp->state))
  1609			set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
  1610	
  1611		bnxt_re_dev_stop(rdev);
  1612		bnxt_re_stop_irq(rdev);
  1613		/* Move the device states to detached and  avoid sending any more
  1614		 * commands to HW
  1615		 */
  1616		set_bit(BNXT_RE_FLAG_ERR_DEVICE_DETACHED, &rdev->flags);
  1617		set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
  1618		mutex_unlock(&bnxt_re_mutex);
  1619	
  1620		return 0;
  1621	}
  1622	
> 1623	int bnxt_re_resume(struct auxiliary_device *adev)
  1624	{
  1625		struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
  1626	
  1627		if (!rdev)
  1628			return 0;
  1629	
  1630		mutex_lock(&bnxt_re_mutex);
  1631		/* L2 driver may invoke this callback during device recovery, resume.
  1632		 * reset. Current RoCE driver doesn't recover the device in case of
  1633		 * error. Handle the error by dispatching fatal events to all qps
  1634		 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
  1635		 * L2 driver want to modify the MSIx table.
  1636		 */
  1637	
  1638		ibdev_info(&rdev->ibdev, "Handle device resume call");
  1639		mutex_unlock(&bnxt_re_mutex);
  1640	
  1641		return 0;
  1642	}
  1643
kernel test robot Oct. 26, 2022, 3:21 a.m. UTC | #2 
Hi Ajit,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.1-rc2 next-20221025]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ajit-Khaparde/bnxt_en-Add-auxiliary-driver-support/20221026-023141
patch link:    https://lore.kernel.org/r/20221025173110.33192-6-ajit.khaparde%40broadcom.com
patch subject: [PATCH v2 5/6] bnxt_en: Use auxiliary bus calls over proprietary calls
config: x86_64-randconfig-a003-20221024 (attached as .config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
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/intel-lab-lkp/linux/commit/f04080c956abfeabf32961081d0401a800163d99
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Ajit-Khaparde/bnxt_en-Add-auxiliary-driver-support/20221026-023141
        git checkout f04080c956abfeabf32961081d0401a800163d99
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/infiniband/hw/bnxt_re/

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

All warnings (new ones prefixed by >>):

>> drivers/infiniband/hw/bnxt_re/main.c:1585:5: warning: no previous prototype for function 'bnxt_re_suspend' [-Wmissing-prototypes]
   int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
       ^
   drivers/infiniband/hw/bnxt_re/main.c:1585:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
   ^
   static 
>> drivers/infiniband/hw/bnxt_re/main.c:1623:5: warning: no previous prototype for function 'bnxt_re_resume' [-Wmissing-prototypes]
   int bnxt_re_resume(struct auxiliary_device *adev)
       ^
   drivers/infiniband/hw/bnxt_re/main.c:1623:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int bnxt_re_resume(struct auxiliary_device *adev)
   ^
   static 
   2 warnings generated.


vim +/bnxt_re_suspend +1585 drivers/infiniband/hw/bnxt_re/main.c

  1584	
> 1585	int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
  1586	{
  1587		struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
  1588		struct bnxt *bp;
  1589	
  1590		if (!rdev)
  1591			return 0;
  1592	
  1593		mutex_lock(&bnxt_re_mutex);
  1594		/* L2 driver may invoke this callback during device error/crash or device
  1595		 * reset. Current RoCE driver doesn't recover the device in case of
  1596		 * error. Handle the error by dispatching fatal events to all qps
  1597		 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
  1598		 * L2 driver want to modify the MSIx table.
  1599		 */
  1600		bp = netdev_priv(rdev->netdev);
  1601	
  1602		ibdev_info(&rdev->ibdev, "Handle device suspend call");
  1603		/* Check the current device state from L2 structure and move the
  1604		 * device to detached state if FW_FATAL_COND is set.
  1605		 * This prevents more commands to HW during clean-up,
  1606		 * in case the device is already in error.
  1607		 */
  1608		if (test_bit(BNXT_STATE_FW_FATAL_COND, &bp->state))
  1609			set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
  1610	
  1611		bnxt_re_dev_stop(rdev);
  1612		bnxt_re_stop_irq(rdev);
  1613		/* Move the device states to detached and  avoid sending any more
  1614		 * commands to HW
  1615		 */
  1616		set_bit(BNXT_RE_FLAG_ERR_DEVICE_DETACHED, &rdev->flags);
  1617		set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
  1618		mutex_unlock(&bnxt_re_mutex);
  1619	
  1620		return 0;
  1621	}
  1622	
> 1623	int bnxt_re_resume(struct auxiliary_device *adev)
  1624	{
  1625		struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
  1626	
  1627		if (!rdev)
  1628			return 0;
  1629	
  1630		mutex_lock(&bnxt_re_mutex);
  1631		/* L2 driver may invoke this callback during device recovery, resume.
  1632		 * reset. Current RoCE driver doesn't recover the device in case of
  1633		 * error. Handle the error by dispatching fatal events to all qps
  1634		 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
  1635		 * L2 driver want to modify the MSIx table.
  1636		 */
  1637	
  1638		ibdev_info(&rdev->ibdev, "Handle device resume call");
  1639		mutex_unlock(&bnxt_re_mutex);
  1640	
  1641		return 0;
  1642	}
  1643
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 67872e9bfd63..1266cfe7ea88 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -226,45 +226,6 @@  static void bnxt_re_set_resource_limits(struct bnxt_re_dev *rdev)
 		bnxt_re_limit_vf_res(&rdev->qplib_ctx, num_vfs);
 }
 
-/* for handling bnxt_en callbacks later */
-static void bnxt_re_stop(void *p)
-{
-	struct bnxt_re_dev *rdev = p;
-	struct bnxt *bp;
-
-	if (!rdev)
-		return;
-
-	/* L2 driver invokes this callback during device error/crash or device
-	 * reset. Current RoCE driver doesn't recover the device in case of
-	 * error. Handle the error by dispatching fatal events to all qps
-	 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
-	 * L2 driver want to modify the MSIx table.
-	 */
-	bp = netdev_priv(rdev->netdev);
-
-	ibdev_info(&rdev->ibdev, "Handle device stop call from L2 driver");
-	/* Check the current device state from L2 structure and move the
-	 * device to detached state if FW_FATAL_COND is set.
-	 * This prevents more commands to HW during clean-up,
-	 * in case the device is already in error.
-	 */
-	if (test_bit(BNXT_STATE_FW_FATAL_COND, &bp->state))
-		set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
-
-	bnxt_re_dev_stop(rdev);
-	bnxt_re_stop_irq(rdev);
-	/* Move the device states to detached and  avoid sending any more
-	 * commands to HW
-	 */
-	set_bit(BNXT_RE_FLAG_ERR_DEVICE_DETACHED, &rdev->flags);
-	set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
-}
-
-static void bnxt_re_start(void *p)
-{
-}
-
 static void bnxt_re_sriov_config(void *p, int num_vfs)
 {
 	struct bnxt_re_dev *rdev = p;
@@ -344,8 +305,6 @@  static void bnxt_re_start_irq(void *handle, struct bnxt_msix_entry *ent)
 }
 
 static struct bnxt_ulp_ops bnxt_re_ulp_ops = {
-	.ulp_stop = bnxt_re_stop,
-	.ulp_start = bnxt_re_start,
 	.ulp_sriov_config = bnxt_re_sriov_config,
 	.ulp_irq_stop = bnxt_re_stop_irq,
 	.ulp_irq_restart = bnxt_re_start_irq
@@ -1623,6 +1582,65 @@  static int bnxt_re_probe(struct auxiliary_device *adev,
 	return rc;
 }
 
+int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
+{
+	struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
+	struct bnxt *bp;
+
+	if (!rdev)
+		return 0;
+
+	mutex_lock(&bnxt_re_mutex);
+	/* L2 driver may invoke this callback during device error/crash or device
+	 * reset. Current RoCE driver doesn't recover the device in case of
+	 * error. Handle the error by dispatching fatal events to all qps
+	 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
+	 * L2 driver want to modify the MSIx table.
+	 */
+	bp = netdev_priv(rdev->netdev);
+
+	ibdev_info(&rdev->ibdev, "Handle device suspend call");
+	/* Check the current device state from L2 structure and move the
+	 * device to detached state if FW_FATAL_COND is set.
+	 * This prevents more commands to HW during clean-up,
+	 * in case the device is already in error.
+	 */
+	if (test_bit(BNXT_STATE_FW_FATAL_COND, &bp->state))
+		set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
+
+	bnxt_re_dev_stop(rdev);
+	bnxt_re_stop_irq(rdev);
+	/* Move the device states to detached and  avoid sending any more
+	 * commands to HW
+	 */
+	set_bit(BNXT_RE_FLAG_ERR_DEVICE_DETACHED, &rdev->flags);
+	set_bit(ERR_DEVICE_DETACHED, &rdev->rcfw.cmdq.flags);
+	mutex_unlock(&bnxt_re_mutex);
+
+	return 0;
+}
+
+int bnxt_re_resume(struct auxiliary_device *adev)
+{
+	struct bnxt_re_dev *rdev = auxiliary_get_drvdata(adev);
+
+	if (!rdev)
+		return 0;
+
+	mutex_lock(&bnxt_re_mutex);
+	/* L2 driver may invoke this callback during device recovery, resume.
+	 * reset. Current RoCE driver doesn't recover the device in case of
+	 * error. Handle the error by dispatching fatal events to all qps
+	 * ie. by calling bnxt_re_dev_stop and release the MSIx vectors as
+	 * L2 driver want to modify the MSIx table.
+	 */
+
+	ibdev_info(&rdev->ibdev, "Handle device resume call");
+	mutex_unlock(&bnxt_re_mutex);
+
+	return 0;
+}
+
 static const struct auxiliary_device_id bnxt_re_id_table[] = {
 	{ .name = BNXT_ADEV_NAME ".rdma", },
 	{},
@@ -1635,6 +1653,8 @@  static struct auxiliary_driver bnxt_re_driver = {
 	.probe = bnxt_re_probe,
 	.remove = bnxt_re_remove,
 	.shutdown = bnxt_re_shutdown,
+	.suspend = bnxt_re_suspend,
+	.resume = bnxt_re_resume,
 	.id_table = bnxt_re_id_table,
 };
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index dbb1fa086013..ba0c281c74ba 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -270,26 +270,31 @@  static void bnxt_ulp_put(struct bnxt_ulp *ulp)
 
 void bnxt_ulp_stop(struct bnxt *bp)
 {
+	struct bnxt_aux_dev *bnxt_aux = bp->aux_dev;
 	struct bnxt_en_dev *edev = bp->edev;
-	struct bnxt_ulp_ops *ops;
-	struct bnxt_ulp *ulp;
 
 	if (!edev)
 		return;
 
 	edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
-	ulp = edev->ulp_tbl;
-	ops = rtnl_dereference(ulp->ulp_ops);
-	if (!ops || !ops->ulp_stop)
-		return;
-	ops->ulp_stop(ulp->handle);
+	if (bnxt_aux) {
+		struct auxiliary_device *adev;
+
+		adev = &bnxt_aux->aux_dev;
+		if (adev->dev.driver) {
+			struct auxiliary_driver *adrv;
+			pm_message_t pm = {};
+
+			adrv = to_auxiliary_drv(adev->dev.driver);
+			adrv->suspend(adev, pm);
+		}
+	}
 }
 
 void bnxt_ulp_start(struct bnxt *bp, int err)
 {
+	struct bnxt_aux_dev *bnxt_aux = bp->aux_dev;
 	struct bnxt_en_dev *edev = bp->edev;
-	struct bnxt_ulp_ops *ops;
-	struct bnxt_ulp *ulp;
 
 	if (!edev)
 		return;
@@ -299,11 +304,18 @@  void bnxt_ulp_start(struct bnxt *bp, int err)
 	if (err)
 		return;
 
-	ulp = edev->ulp_tbl;
-	ops = rtnl_dereference(ulp->ulp_ops);
-	if (!ops || !ops->ulp_start)
-		return;
-	ops->ulp_start(ulp->handle);
+	if (bnxt_aux) {
+		struct auxiliary_device *adev;
+
+		adev = &bnxt_aux->aux_dev;
+		if (adev->dev.driver) {
+			struct auxiliary_driver *adrv;
+
+			adrv = to_auxiliary_drv(adev->dev.driver);
+			adrv->resume(adev);
+		}
+	}
+
 }
 
 void bnxt_ulp_sriov_cfg(struct bnxt *bp, int num_vfs)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index 42e324635e6b..ba253ba5aba0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -29,8 +29,6 @@  struct bnxt_msix_entry {
 struct bnxt_ulp_ops {
 	/* async_notifier() cannot sleep (in BH context) */
 	void (*ulp_async_notifier)(void *, struct hwrm_async_event_cmpl *);
-	void (*ulp_stop)(void *);
-	void (*ulp_start)(void *);
 	void (*ulp_sriov_config)(void *, int);
 	void (*ulp_shutdown)(void *);
 	void (*ulp_irq_stop)(void *);