[net-next,v9,0/8] Add Auxiliary driver support

Message ID	20230131052557.99119-1-ajit.khaparde@broadcom.com (mailing list archive)
Headers	show Return-Path: <linux-rdma-owner@vger.kernel.org> From: Ajit Khaparde <ajit.khaparde@broadcom.com> To: ajit.khaparde@broadcom.com Cc: andrew.gospodarek@broadcom.com, davem@davemloft.net, edumazet@google.com, jgg@ziepe.ca, kuba@kernel.org, leon@kernel.org, linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org, michael.chan@broadcom.com, netdev@vger.kernel.org, pabeni@redhat.com, selvin.xavier@broadcom.com, gregkh@linuxfoundation.org Subject: [PATCH net-next v9 0/8] Add Auxiliary driver support Date: Mon, 30 Jan 2023 21:25:49 -0800 Message-Id: <20230131052557.99119-1-ajit.khaparde@broadcom.com> MIME-Version: 1.0 Content-Type: multipart/signed; protocol="application/pkcs7-signature"; micalg=sha-256; boundary="00000000000094eb9a05f3889169" Precedence: bulk
Series	Add Auxiliary driver support \| expand [net-next,v9,0/8] Add Auxiliary driver support [net-next,v9,1/8] bnxt_en: Add auxiliary driver support [net-next,v9,2/8] RDMA/bnxt_re: Use auxiliary driver interface [net-next,v9,3/8] bnxt_en: Remove usage of ulp_id [net-next,v9,4/8] bnxt_en: Use direct API instead of indirection [net-next,v9,5/8] bnxt_en: Use auxiliary bus calls over proprietary calls [net-next,v9,6/8] bnxt_en: Remove struct bnxt access from RoCE driver [net-next,v9,7/8] RDMA/bnxt_re: Remove the sriov config callback [net-next,v9,8/8] bnxt_en: Remove runtime interrupt vector allocation

Ajit Khaparde Jan. 31, 2023, 5:25 a.m. UTC

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.

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.

v5->v6:
- Removed excessive checks for dev and other pointers
- Remove runtime interrupt vector allocation. bnxt_en preallocates
interrupt vectors for bnxt_re to use.

v6->v7:
- Removed incorrect usage of inline
- Updated Kconfig to select AUXILIARY BUS support
- Addressed various comments including removal of unnecessary forward
  declaration, using static functions where possible, unnecessary jump,
  cleanup logic, etc..
- Added Leon's Reviewed-by, to the commit log in the patches, from
  previous version.

v7->v8:
- Addressed various comments to remove unnecessary check for id, removed
  setting pointer to NULL after free, renamed private pointers to avoid
  confusing them with the auxiliary device names and refactored some
  code accordingly.
- Auxiliary device will be released through auxiliary_device_uninit();

v8->v9:
- Fixed the cleanup code to use the auxiliary driver release function to
  free allocated memory. This allows the cleanup to be performed once
  the device reference count drops to 0.
- Refactored the code to address the above.

Commit message uses Leon's Reviewed-by from earlier version.

Please apply. Thanks.

The following are changes since commit 90e8ca0abb05ada6c1e2710eaa21688dafca26f2
  Merge branch 'devlink-next'
and are available in the git repository at:
  https://github.com/ajitkhaparde1/net-next/tree/aux-bus-v9

Ajit Khaparde (7):
  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
  bnxt_en: Remove runtime interrupt vector allocation

Hongguang Gao (1):
  bnxt_en: Remove struct bnxt access from RoCE driver

 drivers/infiniband/hw/bnxt_re/bnxt_re.h       |  10 +-
 drivers/infiniband/hw/bnxt_re/main.c          | 635 +++++++-----------
 drivers/net/ethernet/broadcom/Kconfig         |   1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |   9 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   8 +
 .../net/ethernet/broadcom/bnxt/bnxt_sriov.c   |   7 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 478 ++++++-------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  51 +-
 8 files changed, 464 insertions(+), 735 deletions(-)

Jakub Kicinski Feb. 1, 2023, 5:12 a.m. UTC | #1

On Mon, 30 Jan 2023 21:25:49 -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.

Better :)

Leon, looks good to you as well?

Note to DaveM/self - this needs to be pulled rather than applied:

> The following are changes since commit 90e8ca0abb05ada6c1e2710eaa21688dafca26f2
>  Merge branch 'devlink-next'
> and are available in the git repository at:
>  https://github.com/ajitkhaparde1/net-next/tree/aux-bus-v9

Leon Romanovsky Feb. 1, 2023, 8:38 a.m. UTC | #2

On Tue, Jan 31, 2023 at 09:12:28PM -0800, Jakub Kicinski wrote:
> On Mon, 30 Jan 2023 21:25:49 -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.
> 
> Better :)
> 
> Leon, looks good to you as well?

Good enough, we beat this horse to death already.

BTW, it still has useless NULL assignments and variable initializations.

+       aux_priv->edev->en_ops = NULL;
+       kfree(aux_priv->edev);

Thanks

> 
> Note to DaveM/self - this needs to be pulled rather than applied:
> 
> > The following are changes since commit 90e8ca0abb05ada6c1e2710eaa21688dafca26f2
> >  Merge branch 'devlink-next'
> > and are available in the git repository at:
> >  https://github.com/ajitkhaparde1/net-next/tree/aux-bus-v9

Ajit Khaparde Feb. 1, 2023, 4:56 p.m. UTC | #3

On Wed, Feb 1, 2023 at 12:38 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Tue, Jan 31, 2023 at 09:12:28PM -0800, Jakub Kicinski wrote:
> > On Mon, 30 Jan 2023 21:25:49 -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.
> >
> > Better :)
> >
> > Leon, looks good to you as well?
>
> Good enough, we beat this horse to death already.
>
> BTW, it still has useless NULL assignments and variable initializations.
I can spin another patchset to remove these.
Or I can include it in the next patchset which is in the pipeline.
Please let me know.

Thanks

>
> +       aux_priv->edev->en_ops = NULL;
> +       kfree(aux_priv->edev);
>
> Thanks
>
> >
> > Note to DaveM/self - this needs to be pulled rather than applied:
> >
> > > The following are changes since commit 90e8ca0abb05ada6c1e2710eaa21688dafca26f2
> > >  Merge branch 'devlink-next'
> > > and are available in the git repository at:
> > >  https://github.com/ajitkhaparde1/net-next/tree/aux-bus-v9

Jakub Kicinski Feb. 1, 2023, 6:56 p.m. UTC | #4

On Mon, 30 Jan 2023 21:25:49 -0800 Ajit Khaparde wrote:
> The following are changes since commit 90e8ca0abb05ada6c1e2710eaa21688dafca26f2
>   Merge branch 'devlink-next'
> and are available in the git repository at:
>   https://github.com/ajitkhaparde1/net-next/tree/aux-bus-v9

This is not a valid pull URL, try running git pull on it yourself.
Also the tree must be based on commits in Linus's tree if you want
both us and RDMA to pull it.

Ajit Khaparde Feb. 1, 2023, 6:58 p.m. UTC | #5

On Wed, Feb 1, 2023 at 10:56 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 30 Jan 2023 21:25:49 -0800 Ajit Khaparde wrote:
> > The following are changes since commit 90e8ca0abb05ada6c1e2710eaa21688dafca26f2
> >   Merge branch 'devlink-next'
> > and are available in the git repository at:
> >   https://github.com/ajitkhaparde1/net-next/tree/aux-bus-v9
>
> This is not a valid pull URL, try running git pull on it yourself.
Let me do that.

> Also the tree must be based on commits in Linus's tree if you want
> both us and RDMA to pull it.

OK. I will rebase against Linus's tree and send it again.
While I am at it, I will address the last comment from Leon as well.

Thanks

[net-next,v9,0/8] Add Auxiliary driver support

Message

Comments