From: Ferruh Yigit <ferruh.yigit@xilinx.com>
To: Chaoyong He <chaoyong.he@corigine.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: oss-drivers <oss-drivers@corigine.com>,
Niklas Soderlund <niklas.soderlund@corigine.com>
Subject: Re: [PATCH v7 03/12] net/nfp: move app specific init logic to own function
Date: Tue, 6 Sep 2022 10:47:21 +0100 [thread overview]
Message-ID: <ae329cbe-3c4e-1bf8-be41-f3548e285c35@xilinx.com> (raw)
In-Reply-To: <SJ0PR13MB5545EE0AF21C3199602E7F5F9E7E9@SJ0PR13MB5545.namprd13.prod.outlook.com>
On 9/6/2022 9:29 AM, Chaoyong He wrote:
> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
>> Sent: Monday, September 5, 2022 11:39 PM
>> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
>> Cc: oss-drivers <oss-drivers@corigine.com>; Niklas Soderlund
>> <niklas.soderlund@corigine.com>
>> Subject: Re: [PATCH v7 03/12] net/nfp: move app specific init logic to own
>> function
>>
>> On 8/12/2022 11:22 AM, Chaoyong He wrote:
>>> The NFP card can load different firmware applications.
>>> This commit move the init logic of corenic app of the secondary
>>> process into its own function.
>>>
>>> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>
>> <...>
>>
>>> + switch (app_id) {
>>> + case NFP_APP_CORE_NIC:
>>> + PMD_INIT_LOG(INFO, "Initializing coreNIC");
>>> + ret = nfp_secondary_init_app_nic(pci_dev, sym_tbl, cpp);
>>> + if (ret != 0) {
>>> + PMD_INIT_LOG(ERR, "Could not initialize coreNIC!");
>>> + goto sym_tbl_cleanup;
>>> }
>>
>> If you are planning to add more FW app support, what do you think to add
>> another abstraction for it? Something like
>>
>> struct fw_ops {
>> *init()
>> *secondary_init()
>> ...
>> }
>>
>> ...
>> ret = fw_ops[app_id].secondary_init(...);
>> ...
>>
>
> It does make sense if we can translate this switch statement into an array of function pointers.
> But there are some problems:
> 1. The `app_id` is returned by the firmware, so we can't simply regard it as an index of the array.
> There should import some relation map logic. And we can't find a suitable upper limit for the
> array, so maybe we will always update it as we add more and more firmware apps in the future.
> We also have to check the value of `fw_ops[app_id].secondary_init` before we invoke it.
> 2. Different firmware app may need different variables to initialize, which make it difficult to find a
> suitable function prototype when we declare the function pointer.
>
> So the final logics seems will more complicated and maybe we can keep use the logics now?
Got it, above mentioned issues looks valid, so it is OK to keep as it is.
next prev parent reply other threads:[~2022-09-06 9:47 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-12 10:22 [PATCH v7 00/12] preparation for the rte_flow offload of nfp PMD Chaoyong He
2022-08-12 10:22 ` [PATCH v7 01/12] net/nfp: move app specific attributes to own struct Chaoyong He
2022-09-05 15:38 ` Ferruh Yigit
2022-09-06 9:20 ` Chaoyong He
2022-09-06 9:35 ` Ferruh Yigit
2022-08-12 10:22 ` [PATCH v7 02/12] net/nfp: simplify initialization and remove dead code Chaoyong He
2022-08-12 10:22 ` [PATCH v7 03/12] net/nfp: move app specific init logic to own function Chaoyong He
2022-09-05 15:38 ` Ferruh Yigit
2022-09-06 8:29 ` Chaoyong He
2022-09-06 9:47 ` Ferruh Yigit [this message]
2022-08-12 10:22 ` [PATCH v7 04/12] net/nfp: add initial flower firmware support Chaoyong He
2022-09-05 15:39 ` Ferruh Yigit
2022-09-07 8:22 ` Chaoyong He
2022-09-07 13:24 ` Ferruh Yigit
2022-08-12 10:22 ` [PATCH v7 05/12] net/nfp: add flower PF setup and mempool init logic Chaoyong He
2022-09-05 15:42 ` Ferruh Yigit
2022-09-06 8:45 ` Chaoyong He
2022-09-06 10:18 ` Ferruh Yigit
2022-08-12 10:22 ` [PATCH v7 06/12] net/nfp: add flower PF related routines Chaoyong He
2022-08-12 10:22 ` [PATCH v7 07/12] net/nfp: add flower ctrl VNIC related logics Chaoyong He
2022-08-12 10:22 ` [PATCH v7 08/12] net/nfp: move common rxtx function for flower use Chaoyong He
2022-08-12 10:22 ` [PATCH v7 09/12] net/nfp: add flower ctrl VNIC rxtx logic Chaoyong He
2022-09-05 15:42 ` Ferruh Yigit
2022-08-12 10:22 ` [PATCH v7 10/12] net/nfp: add flower representor framework Chaoyong He
2022-08-12 10:22 ` [PATCH v7 11/12] net/nfp: move rxtx function to header file Chaoyong He
2022-08-12 10:22 ` [PATCH v7 12/12] net/nfp: add flower PF rxtx logic Chaoyong He
2022-09-01 22:10 ` [PATCH v7 00/12] preparation for the rte_flow offload of nfp PMD Niklas Söderlund
2022-09-05 15:40 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ae329cbe-3c4e-1bf8-be41-f3548e285c35@xilinx.com \
--to=ferruh.yigit@xilinx.com \
--cc=chaoyong.he@corigine.com \
--cc=dev@dpdk.org \
--cc=niklas.soderlund@corigine.com \
--cc=oss-drivers@corigine.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).