2023-02-01 05:12:34

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v9 0/8] Add Auxiliary driver support

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

2023-02-01 08:38:58

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH net-next v9 0/8] Add Auxiliary driver support

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

2023-02-01 16:56:26

by Ajit Khaparde

[permalink] [raw]
Subject: Re: [PATCH net-next v9 0/8] Add Auxiliary driver support

On Wed, Feb 1, 2023 at 12:38 AM Leon Romanovsky <[email protected]> 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


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2023-02-01 18:56:17

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v9 0/8] Add Auxiliary driver support

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.

2023-02-01 18:59:12

by Ajit Khaparde

[permalink] [raw]
Subject: Re: [PATCH net-next v9 0/8] Add Auxiliary driver support

On Wed, Feb 1, 2023 at 10:56 AM Jakub Kicinski <[email protected]> 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


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature