RE: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM

From: Vamsi Krishna Attunuru
Date: Wed Jul 17 2024 - 01:35:39 EST




>-----Original Message-----
>From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
>Sent: Wednesday, July 17, 2024 10:47 AM
>To: Vamsi Krishna Attunuru <vattunuru@xxxxxxxxxxx>
>Cc: arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; nathan@xxxxxxxxxx;
>quic_jjohnson@xxxxxxxxxxx
>Subject: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation
>issues on 32-bit ARM
>
>On Tue, Jul 16, 2024 at 09: 52: 25PM -0700, Vamsi Attunuru wrote: > Upon
>adding CONFIG_ARCH_THUNDER & CONFIG_COMPILE_TEST dependency, >
>compilation errors arise on 32-bit ARM with writeq() & readq() calls > which
>are used for
>On Tue, Jul 16, 2024 at 09:52:25PM -0700, Vamsi Attunuru wrote:
>> Upon adding CONFIG_ARCH_THUNDER & CONFIG_COMPILE_TEST
>dependency,
>> compilation errors arise on 32-bit ARM with writeq() & readq() calls
>> which are used for accessing 64-bit values.
>>
>> Patch utilizes CONFIG_64BIT checks to define appropriate calls for
>> accessing 64-bit values.
>>
>> Fixes: a5e43e2d202d ("misc: Kconfig: add a new dependency for
>> MARVELL_CN10K_DPI")
>> Signed-off-by: Vamsi Attunuru <vattunuru@xxxxxxxxxxx>
>> ---
>> drivers/misc/mrvl_cn10k_dpi.c | 47
>> ++++++++++++++++++++++++++++++++---
>> 1 file changed, 43 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/misc/mrvl_cn10k_dpi.c
>> b/drivers/misc/mrvl_cn10k_dpi.c index 7d5433121ff6..8d24dd6b421b
>> 100644
>> --- a/drivers/misc/mrvl_cn10k_dpi.c
>> +++ b/drivers/misc/mrvl_cn10k_dpi.c
>> @@ -13,6 +13,9 @@
>> #include <linux/pci.h>
>> #include <linux/irq.h>
>> #include <linux/interrupt.h>
>> +#ifndef CONFIG_64BIT
>> +#include <linux/io-64-nonatomic-lo-hi.h> #endif
>
>Are you sure the #ifndef is needed for this include file?

Check may not be needed, will discard the check.
>
>>
>> #include <uapi/misc/mrvl_cn10k_dpi.h>
>>
>> @@ -185,6 +188,8 @@ struct dpi_mbox_message {
>> uint64_t word_h;
>> };
>>
>> +#ifdef CONFIG_64BIT
>> +
>> static inline void dpi_reg_write(struct dpipf *dpi, u64 offset, u64
>> val) {
>> writeq(val, dpi->reg_base + offset); @@ -195,6 +200,40 @@ static
>> inline u64 dpi_reg_read(struct dpipf *dpi, u64 offset)
>> return readq(dpi->reg_base + offset); }
>>
>> +static inline void dpi_writeq(u64 val, void __iomem *addr) {
>> + writeq(val, addr);
>> +}
>> +
>> +static inline u64 dpi_readq(const void __iomem *addr) {
>> + return readq(addr);
>> +}
>> +
>> +#else
>
>Normally we do not like #ifdef in .c files, are you sure this is the correct way to
>handle this?

Ok, came across the similar usage in some other drivers and presumed it's fine with small routines. I will move the #ifdef inside the routines than.

Thank you, Greg, for the prompt feedback.
>
>thanks,
>
>greg k-h