[dpdk-dev] config: compile avp driver only on intel 64 bit

  The AVP devices are only supported on Intel 64-bit architectures so
adjusting the defconfig attributes accordingly.

Fixes: c5ab4707ba97 ("net/avp: support driver registration")

Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
---
 config/common_linuxapp                        | 1 -
 config/defconfig_i686-native-linuxapp-gcc     | 5 -----
 config/defconfig_i686-native-linuxapp-icc     | 5 -----
 config/defconfig_x86_64-native-linuxapp-clang | 5 +++++
 config/defconfig_x86_64-native-linuxapp-gcc   | 6 ++++++
 config/defconfig_x86_64-native-linuxapp-icc   | 5 +++++
 config/defconfig_x86_x32-native-linuxapp-gcc  | 5 -----
 7 files changed, 16 insertions(+), 16 deletions(-)
  

On 4/4/2017 5:36 PM, Allain Legacy wrote:
> The AVP devices are only supported on Intel 64-bit architectures so
> adjusting the defconfig attributes accordingly.
> 
> Fixes: c5ab4707ba97 ("net/avp: support driver registration")
> 
> Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
> ---
>  config/common_linuxapp                        | 1 -
>  config/defconfig_i686-native-linuxapp-gcc     | 5 -----
>  config/defconfig_i686-native-linuxapp-icc     | 5 -----
>  config/defconfig_x86_64-native-linuxapp-clang | 5 +++++

Right now compiler config files only have compiler and architecture
configs, although it is OK to update them, to be consistent with what
other PMDs did, what do you think:

Keep AVP config enabled in "common_linuxapp"

Disable AVP in following configs:
defconfig_arm64-armv8a-linuxapp-gcc
defconfig_arm-armv7a-linuxapp-gcc
defconfig_ppc_64-power8-linuxapp-gcc

>  config/defconfig_x86_64-native-linuxapp-gcc   | 6 ++++++
>  config/defconfig_x86_64-native-linuxapp-icc   | 5 +++++
>  config/defconfig_x86_x32-native-linuxapp-gcc  | 5 -----
>  7 files changed, 16 insertions(+), 16 deletions(-)
> 
<...>
  

> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Tuesday, April 04, 2017 1:12 PM
<...>
> 
> Right now compiler config files only have compiler and architecture configs,
> although it is OK to update them, to be consistent with what other PMDs did,
> what do you think:
> 
> Keep AVP config enabled in "common_linuxapp"
> 
> Disable AVP in following configs:
> defconfig_arm64-armv8a-linuxapp-gcc
> defconfig_arm-armv7a-linuxapp-gcc
> defconfig_ppc_64-power8-linuxapp-gcc

If that is your preference I can update the patch but either way a compiler config file is going to be changed.   I did it the current way so that if a new architecture came along that they would not need to know to set AVP=n in their file. 

Do you still want me to align to your suggestion?
  

On 4/4/2017 6:17 PM, Legacy, Allain wrote:
>> -----Original Message-----
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Tuesday, April 04, 2017 1:12 PM
> <...>
>>
>> Right now compiler config files only have compiler and architecture configs,
>> although it is OK to update them, to be consistent with what other PMDs did,
>> what do you think:
>>
>> Keep AVP config enabled in "common_linuxapp"
>>
>> Disable AVP in following configs:
>> defconfig_arm64-armv8a-linuxapp-gcc
>> defconfig_arm-armv7a-linuxapp-gcc
>> defconfig_ppc_64-power8-linuxapp-gcc
> 
> If that is your preference I can update the patch but either way a compiler config file is going to be changed.   
> I did it the current way so that if a new architecture came along that they would not need to know to set AVP=n in their file.

Indeed, this makes sense.

> 
> Do you still want me to align to your suggestion?

I still believe good to be consistent with current usage. They all can
be fixed later, for example when a new architecture added, in one go.

Thanks,
ferruh
  

On Tue, Apr 04, 2017 at 06:12:12PM +0100, Ferruh Yigit wrote:
> On 4/4/2017 5:36 PM, Allain Legacy wrote:
> > The AVP devices are only supported on Intel 64-bit architectures so
> > adjusting the defconfig attributes accordingly.
> > 
> > Fixes: c5ab4707ba97 ("net/avp: support driver registration")
> > 
> > Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
> > ---
> >  config/common_linuxapp                        | 1 -
> >  config/defconfig_i686-native-linuxapp-gcc     | 5 -----
> >  config/defconfig_i686-native-linuxapp-icc     | 5 -----
> >  config/defconfig_x86_64-native-linuxapp-clang | 5 +++++
> 
> Right now compiler config files only have compiler and architecture
> configs, although it is OK to update them, to be consistent with what
> other PMDs did, what do you think:
> 
> Keep AVP config enabled in "common_linuxapp"
> 
> Disable AVP in following configs:
> defconfig_arm64-armv8a-linuxapp-gcc
> defconfig_arm-armv7a-linuxapp-gcc
> defconfig_ppc_64-power8-linuxapp-gcc
> 

I actually disagree with that, and I think the approach proposed is
better, given that it's only one target type that is supported.

However, I think we should have an AVP config option in common base, set
to "n", so that we at least there have an entry to indicate it exists.

/Bruce
  

2017-04-05 09:23, Bruce Richardson:
> On Tue, Apr 04, 2017 at 06:12:12PM +0100, Ferruh Yigit wrote:
> > On 4/4/2017 5:36 PM, Allain Legacy wrote:
> > > The AVP devices are only supported on Intel 64-bit architectures so
> > > adjusting the defconfig attributes accordingly.
> > > 
> > > Fixes: c5ab4707ba97 ("net/avp: support driver registration")
> > > 
> > > Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
> > > ---
> > >  config/common_linuxapp                        | 1 -
> > >  config/defconfig_i686-native-linuxapp-gcc     | 5 -----
> > >  config/defconfig_i686-native-linuxapp-icc     | 5 -----
> > >  config/defconfig_x86_64-native-linuxapp-clang | 5 +++++
> > 
> > Right now compiler config files only have compiler and architecture
> > configs, although it is OK to update them, to be consistent with what
> > other PMDs did, what do you think:
> > 
> > Keep AVP config enabled in "common_linuxapp"
> > 
> > Disable AVP in following configs:
> > defconfig_arm64-armv8a-linuxapp-gcc
> > defconfig_arm-armv7a-linuxapp-gcc
> > defconfig_ppc_64-power8-linuxapp-gcc
> > 
> 
> I actually disagree with that, and I think the approach proposed is
> better, given that it's only one target type that is supported.
> 
> However, I think we should have an AVP config option in common base, set
> to "n", so that we at least there have an entry to indicate it exists.

Yes, seems reasonnable
  

On 4/5/2017 9:23 AM, Bruce Richardson wrote:
> On Tue, Apr 04, 2017 at 06:12:12PM +0100, Ferruh Yigit wrote:
>> On 4/4/2017 5:36 PM, Allain Legacy wrote:
>>> The AVP devices are only supported on Intel 64-bit architectures so
>>> adjusting the defconfig attributes accordingly.
>>>
>>> Fixes: c5ab4707ba97 ("net/avp: support driver registration")
>>>
>>> Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
>>> ---
>>>  config/common_linuxapp                        | 1 -
>>>  config/defconfig_i686-native-linuxapp-gcc     | 5 -----
>>>  config/defconfig_i686-native-linuxapp-icc     | 5 -----
>>>  config/defconfig_x86_64-native-linuxapp-clang | 5 +++++
>>
>> Right now compiler config files only have compiler and architecture
>> configs, although it is OK to update them, to be consistent with what
>> other PMDs did, what do you think:
>>
>> Keep AVP config enabled in "common_linuxapp"
>>
>> Disable AVP in following configs:
>> defconfig_arm64-armv8a-linuxapp-gcc
>> defconfig_arm-armv7a-linuxapp-gcc
>> defconfig_ppc_64-power8-linuxapp-gcc
>>
> 
> I actually disagree with that, and I think the approach proposed is
> better, given that it's only one target type that is supported.

Agree that it is better, but other PMDs are doing what I suggested, so I
am for AVP follow the same, and fix them all later when required.

> 
> However, I think we should have an AVP config option in common base, set
> to "n", so that we at least there have an entry to indicate it exists.
> 
> /Bruce
>