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

[Kevin Traynor]

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.yi...@amd.com>
Sent: Tuesday, September 19, 2023 7:05 PM
To: Wei Hu <w...@microsoft.com>; dev@dpdk.org; Long Li
<lon...@microsoft.com>
Cc: sta...@dpdk.org; Kevin Traynor <ktray...@redhat.com>; Luca Boccassi
<bl...@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.yi...@amd.com>
Sent: Tuesday, September 19, 2023 1:41 AM
To: Wei Hu <w...@microsoft.com>; dev@dpdk.org; Long Li
<lon...@microsoft.com>
Cc: sta...@dpdk.org; Kevin Traynor <ktray...@redhat.com>; Luca
Boccassi <bl...@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: sta...@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:
sta...@dpdk.org" in it.


Got it, lets get comment from the LTS maintainers.


Signed-off-by: Wei Hu <w...@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.