| Message ID | 20221207175310.23656-1-ajit.khaparde@broadcom.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add Auxiliary driver support | expand |
On Wed, Dec 07, 2022 at 09:53:03AM -0800, Ajit Khaparde wrote: > Add auxiliary device driver for Broadcom devices. > The bnxt_en driver will register and initialize an aux device > if RDMA is enabled in the underlying device. > The bnxt_re driver will then probe and initialize the > RoCE interfaces with the infiniband stack. > > We got rid of the bnxt_en_ops which the bnxt_re driver used to > communicate with bnxt_en. > Similarly We have tried to clean up most of the bnxt_ulp_ops. > In most of the cases we used the functions and entry points provided > by the auxiliary bus driver framework. > And now these are the minimal functions needed to support the functionality. > > We will try to work on getting rid of the remaining if we find any > other viable option in future. > > v1->v2: > - Incorporated review comments including usage of ulp_id & > complex function indirections. > - Used function calls provided by the auxiliary bus interface > instead of proprietary calls. > - Refactor code to remove ROCE driver's access to bnxt structure. I still see wrong usage of auxiliary driver model, especially for RDMA device. That model mimics general driver model, where you should separate between device creation and configuration. I would expect that your bnxt_en create pre-configured devices with right amount of MSI-X, limits, capabilities e.t.c and RDMA driver will simply bind to it. It means that calls like bnxt_re_request_msix() should go too. All PCI-related logic needs to be in netdev. In addition, I saw IS_ERR_OR_NULL(..) and "if(dev)" checks in various uninit functions and it can be one of two: wrong unwind flow or wrong use of driver model. In right implementation, your driver will be called only on valid device and uninit won't be called for not-initialized device. Also I spotted .ulp_async_notifier, which is not used and bnxt_re_sriov_config() is prune to races due to separation between driver bind and device creation. You should configure SR-IOV in device creation stage. Thanks > > v2->v3: > - Addressed review comments including cleanup of some unnecessary wrappers > - Fixed warnings seen during cross compilation > > v3->v4: > - Cleaned up bnxt_ulp.c and bnxt_ulp.h further > - Removed some more dead code > - Sending the patchset as a standalone series > > v4->v5: > - Removed the SRIOV config callback which bnxt_en driver was calling into > bnxt_re driver. > - Removed excessive checks for rdev and other pointers. > > Please apply. Thanks. > > Ajit Khaparde (6): > bnxt_en: Add auxiliary driver support > RDMA/bnxt_re: Use auxiliary driver interface > bnxt_en: Remove usage of ulp_id > bnxt_en: Use direct API instead of indirection > bnxt_en: Use auxiliary bus calls over proprietary calls > RDMA/bnxt_re: Remove the sriov config callback > > Hongguang Gao (1): > bnxt_en: Remove struct bnxt access from RoCE driver > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 9 +- > drivers/infiniband/hw/bnxt_re/main.c | 591 +++++++----------- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 10 +- > drivers/net/ethernet/broadcom/bnxt/bnxt.h | 8 + > .../net/ethernet/broadcom/bnxt/bnxt_sriov.c | 7 +- > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 413 ++++++------ > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 53 +- > 7 files changed, 490 insertions(+), 601 deletions(-) > > -- > 2.37.1 (Apple Git-137.1) >