[v2,5/6] bnxt_en: Use auxiliary bus calls over proprietary calls
Commit Message
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
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
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
@@ -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,
};
@@ -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)
@@ -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 *);