[PATCH v8 01/12] net/nfp: move app specific attributes to own struct

Chaoyong He chaoyong.he at corigine.com
Fri Sep 9 07:43:43 CEST 2022


> > On 9/8/2022 9:44 AM, Chaoyong He wrote:
> > > The NFP card can load different firmware applications. Currently
> > > only the CoreNIC application is supported. This commit makes needed
> > > infrastructure changes in order to support other firmware
> > > applications too.
> > >
> > > Clearer separation is made between the PF device and any application
> > > specific concepts. The PF struct is now generic regardless of the
> > > application loaded. A new struct is also made for the CoreNIC
> > > application. Future additions to support other applications should
> > > also add an applications specific struct.
> > >
> >
> > What do you think to replace 'application' usage in the commit log
> > with 'application firmware'?
> >
> > <...>
> >
> > > diff --git a/drivers/net/nfp/nfp_ethdev.c
> > > b/drivers/net/nfp/nfp_ethdev.c index e9d01f4..bd9cf67 100644
> > > --- a/drivers/net/nfp/nfp_ethdev.c
> > > +++ b/drivers/net/nfp/nfp_ethdev.c
> > > @@ -39,15 +39,15 @@
> > >   #include "nfp_cpp_bridge.h"
> > >
> > >   static int
> > > -nfp_net_pf_read_mac(struct nfp_pf_dev *pf_dev, int port)
> > > +nfp_net_pf_read_mac(struct nfp_app_fw_nic *app_hw_nic, int port)
> >
> > Is this intentional that struct name is 'nfp_app_fw_nic' but variable
> > name is 'app_hw_nic'? Why is app_fw vs app_hw difference?
> >
> Sorry, I'm not quite sure I catch your doubt.
> Do you mean I should just use `app_hw` as variable name if the function only
> process one type of the application firmware?
> 
Oh, sorry, I understand now. 
I misspelled 'app_fw' to 'app_hw' in some place, I'll revise and check it.
Thanks!

> > <...>
> >
> > > @@ -890,27 +937,12 @@
> > >   	}
> > >
> > >   	/* Populate the newly created PF device */
> > > +	pf_dev->app_fw_id = app_hw_id;
> >
> > ditto.
Our PMD driver can support two different application firmwares now.
We get the application firmware's type from the firmware in the probe
function, and store it in the structure of pf device. Then we can invoke
different initialization logics according to the application firmware's type.

We have a `structure nfp_pf_dev`, which is used to store the common
information.
We defined a structure for each type of application firmware to keep the
specific information.
The `structure nfp_pf_dev` has a `void *app_fw_priv` filed, which can point
to different structure based on the type of application firmware.

So, what you mean is we need not store it in the structure of pf device as no
other logics out of this function will use it?



More information about the dev mailing list