Re: [PATCH v2 1/2] drivers/i40e: fix X722 macro absence result in compile

From: "Guo, Jia" <jia.guo@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"Zhang, Helin" <helin.zhang@intel.com>,
	"Wu, Jingjing" <jingjing.wu@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [PATCH v2 1/2] drivers/i40e: fix X722 macro absence result in compile
Date: Mon, 17 Oct 2016 15:44:18 +0800	[thread overview]
Message-ID: <b9708e79-5000-cc35-bd89-33573c61978b@intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0C1FF9@irsmsx105.ger.corp.intel.com>

hi, Konstantin
Thanks your constructive suggestion. I don't think your question is 
silly and we also think about the code style simply and effective, but 
may be i would interpret the reason why we do that.

1) Sure, user definitely can choose to define the macro or not when 
building dpdk i40e PMD, but i don't think it is
necessary to invoke a ret_config option to let up layer user freedom use 
it,  because only the older version i40e driver does not support X722, 
the newer version i40e driver will always support X722, so the macro 
will be default hard code in the makefile. and we will use mac.type to 
distinguish the difference register configure in run time. So we may 
consider the macro just like a flag that highlight the difference of the 
shared code between X710 and X722, that would benify the X710/X722 pmd 
development but hardly no use to exposure to the up layer user.

2)  i think the answer also could find from above. But i think if we 
develop go to a certain stage in the future, mute the macro or use 
script to remove them like the way from hw driver, for support all 
device types maybe not a bad idea, right?

On 10/16/2016 9:31 PM, Ananyev, Konstantin wrote:
> Hi Jeff,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
>> Sent: Sunday, October 16, 2016 2:40 AM
>> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
>> Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>
>> Subject: [dpdk-dev] [PATCH v2 1/2] drivers/i40e: fix X722 macro absence result in compile
>>
>> Since some register only be supported by X722 but may not be supported
>> by other NICs, so add X722 macro to distinguish that to avoid compile error
>> when the X722 macro is undefined.
>
> Two probably silly questions:
> 1) So who will setup X722_SUPPORT macro?
> Is that a user responsibility when he is building dpdk i40e PMD?
> If so, why it is not a rte_config option?
> 2) Why this all has to be build  time decision?
> Why nor run-time?
> Why i40e driver can't support all devices (including x722)
> and invoke different config functions (write different registers)
> based on device type/id information?
> As it does for other device types/ids?
>
> Konstantin