Re: [PATCH v2 1/1] net/mana: enable 32 bit build for mana driver

[Kevin Traynor <ktraynor@redhat.com>]

On 20/09/2023 11:03, Ferruh Yigit wrote:
> On 9/20/2023 4:06 AM, Wei Hu wrote:
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>>> Sent: Tuesday, September 19, 2023 7:05 PM
>>> To: Wei Hu <weh@microsoft.com>; dev@dpdk.org; Long Li
>>> <longli@microsoft.com>
>>> Cc: stable@dpdk.org; Kevin Traynor <ktraynor@redhat.com>; Luca Boccassi
>>> <bluca@debian.org>
>>> Subject: Re: [PATCH v2 1/1] net/mana: enable 32 bit build for mana driver
>>>
>>> On 9/19/2023 3:44 AM, Wei Hu wrote:
>>>>> -----Original Message-----
>>>>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>>>>> Sent: Tuesday, September 19, 2023 1:41 AM
>>>>> To: Wei Hu <weh@microsoft.com>; dev@dpdk.org; Long Li
>>>>> <longli@microsoft.com>
>>>>> Cc: stable@dpdk.org; Kevin Traynor <ktraynor@redhat.com>; Luca
>>>>> Boccassi <bluca@debian.org>
>>>>> Subject: Re: [PATCH v2 1/1] net/mana: enable 32 bit build for mana
>>>>> driver
>>>>>
>>>>> On 9/13/2023 1:23 PM, Wei Hu wrote:
>>>>>> Enable 32 bit build on x86 Linux. Fixed build warnings and errors
>>>>>> when building in 32 bit.
>>>>>>
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>
>>>>> This is not a fix, but adding 32bit support to the driver, so not
>>>>> sure about backporting. cc'ed Kevin & Luca for guidance.
>>>>>
>>>>> I will drop the tag while merging unles otherwise suggested by LTS
>>>>> maintainers.
>>>>
>>>> Hi Ferruh,
>>>>
>>>> We have a customer who needs it to be on 22.11.x. That is why I put "Cc:
>>> stable@dpdk.org" in it.
>>>>
>>>
>>> Got it, lets get comment from the LTS maintainers.
>>>
>>>>>
>>>>>> Signed-off-by: Wei Hu <weh@microsoft.com>
>>>>>>
>>>>>
>>>>> Hi Wei,
>>>>>
>>>>> Patch looks good, but it basically fixes the format specifiers that
>>>>> will cause build error for 32 bit.
>>>>>
>>>>> Is there any other change required to make driver functional for 32
>>>>> bit, or does it become functional with this change?
>>>>> And if it does can you please confirm explicityly that driver tested
>>>>> on
>>>>> 32 bit with this patch?
>>>>
>>>> Another patch, " net/mana: add 32 bit short doorbell", is required to
>>>> make mana fully functional  with 32 bit applications. This patch is to
>>>> fix some build time errors and warnings when build in 32 bit.  These
>>>> patches can be independently built and they are addressing two different
>>> issues. So, I put them into two independent patches.
>>>>
>>>> I have tested it on 32 bit applications. They can run with this patch.
>>>> Just mana would not work as the applications still use 64 bit long door bells,
>>> which do not work for 32 bit apps.
>>>>
>>>
>>> Thanks Wei, for clarification.
>>>
>>> As this patch enables the 32bit build of driver in meson, can you please add
>>> comment log that 32bit driver is not functional yet?
>>>
>>> Or maybe meson update can be moved to short doorbell patch, where driver
>>> becomes functional for 32 bit, this patch can mention in commit log that is
>>> preparation for 32 bit support, what do you think?
>>
>> Thanks Ferruh. I would like to add comment log that 32bit driver is not fully
>> functioning yet until short doorbell support is added. Meanwhile still keep
>> the meson update in within this patch to keep it complete. Hope this works
>> for you.
>>
> 
> I think it will do.
> Can you please send those two patches in a same patchset, that gives
> some context if we need to back trace it later?
> 

+1

Adding 22.11 maintainer Xueming. The main thing is that it would not 
cause a regression to 64 bit. That is something that would need to be 
tested by Microsoft.