mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
@ 2024-07-17  4:52 Vamsi Attunuru
  2024-07-17  5:16 ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Vamsi Attunuru @ 2024-07-17  4:52 UTC (permalink / raw)
  To: arnd, gregkh; +Cc: linux-kernel, nathan, quic_jjohnson, vattunuru

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@marvell.com>
---
 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
 
 #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
+
+static inline void dpi_reg_write(struct dpipf *dpi, u64 offset, u64 val)
+{
+	lo_hi_writeq(val, dpi->reg_base + offset);
+}
+
+static inline u64 dpi_reg_read(struct dpipf *dpi, u64 offset)
+{
+	return lo_hi_readq(dpi->reg_base + offset);
+}
+
+static inline void dpi_writeq(u64 val, void __iomem *addr)
+{
+	lo_hi_writeq(val, addr);
+}
+
+static inline u64 dpi_readq(const void __iomem *addr)
+{
+	return lo_hi_readq(addr);
+}
+
+#endif
+
 static void dpi_wqe_cs_offset(struct dpipf *dpi, u8 offset)
 {
 	u64 reg;
@@ -324,7 +363,7 @@ static void dpi_pfvf_mbox_work(struct work_struct *work)
 	memset(&msg, 0, sizeof(msg));
 
 	mutex_lock(&mbox->lock);
-	msg.word_l = readq(mbox->vf_pf_data_reg);
+	msg.word_l = dpi_readq(mbox->vf_pf_data_reg);
 	if (msg.word_l == (u64)-1)
 		goto exit;
 
@@ -333,13 +372,13 @@ static void dpi_pfvf_mbox_work(struct work_struct *work)
 		goto exit;
 
 	dpivf = &dpi->vf[vfid];
-	msg.word_h = readq(mbox->pf_vf_data_reg);
+	msg.word_h = dpi_readq(mbox->pf_vf_data_reg);
 
 	ret = queue_config(dpi, dpivf, &msg);
 	if (ret < 0)
-		writeq(DPI_MBOX_TYPE_RSP_NACK, mbox->pf_vf_data_reg);
+		dpi_writeq(DPI_MBOX_TYPE_RSP_NACK, mbox->pf_vf_data_reg);
 	else
-		writeq(DPI_MBOX_TYPE_RSP_ACK, mbox->pf_vf_data_reg);
+		dpi_writeq(DPI_MBOX_TYPE_RSP_ACK, mbox->pf_vf_data_reg);
 exit:
 	mutex_unlock(&mbox->lock);
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
  2024-07-17  4:52 [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM Vamsi Attunuru
@ 2024-07-17  5:16 ` Greg KH
  2024-07-17  5:35   ` [EXTERNAL] " Vamsi Krishna Attunuru
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2024-07-17  5:16 UTC (permalink / raw)
  To: Vamsi Attunuru; +Cc: arnd, linux-kernel, nathan, quic_jjohnson

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@marvell.com>
> ---
>  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?

>  
>  #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?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
  2024-07-17  5:16 ` Greg KH
@ 2024-07-17  5:35   ` Vamsi Krishna Attunuru
  2024-07-17  5:39     ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: Vamsi Krishna Attunuru @ 2024-07-17  5:35 UTC (permalink / raw)
  To: Greg KH; +Cc: arnd, linux-kernel, nathan, quic_jjohnson



>-----Original Message-----
>From: Greg KH <gregkh@linuxfoundation.org>
>Sent: Wednesday, July 17, 2024 10:47 AM
>To: Vamsi Krishna Attunuru <vattunuru@marvell.com>
>Cc: arnd@arndb.de; linux-kernel@vger.kernel.org; nathan@kernel.org;
>quic_jjohnson@quicinc.com
>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@marvell.com>
>> ---
>>  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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
  2024-07-17  5:35   ` [EXTERNAL] " Vamsi Krishna Attunuru
@ 2024-07-17  5:39     ` Arnd Bergmann
  2024-07-17 11:45       ` Vamsi Krishna Attunuru
  0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2024-07-17  5:39 UTC (permalink / raw)
  To: Vamsi Attunuru, Greg Kroah-Hartman
  Cc: linux-kernel, Nathan Chancellor, Jeff Johnson

On Wed, Jul 17, 2024, at 07:35, Vamsi Krishna Attunuru wrote:

>>>  #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.

You shouldn't need any #ifdef here, just call readq/writeq
unconditionally after including the header.

Have you been able to confirm whether the device works
correctly with the lo_hi ordering?

      Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
  2024-07-17  5:39     ` Arnd Bergmann
@ 2024-07-17 11:45       ` Vamsi Krishna Attunuru
  2024-07-17 11:49         ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: Vamsi Krishna Attunuru @ 2024-07-17 11:45 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman
  Cc: linux-kernel, Nathan Chancellor, Jeff Johnson



>-----Original Message-----
>From: Arnd Bergmann <arnd@arndb.de>
>Sent: Wednesday, July 17, 2024 11:10 AM
>To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; Greg Kroah-Hartman
><gregkh@linuxfoundation.org>
>Cc: linux-kernel@vger.kernel.org; Nathan Chancellor <nathan@kernel.org>;
>Jeff Johnson <quic_jjohnson@quicinc.com>
>Subject: Re: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve
>compilation issues on 32-bit ARM
>
>On Wed, Jul 17, 2024, at 07: 35, Vamsi Krishna Attunuru wrote: >>> #include
><uapi/misc/mrvl_cn10k_dpi. h> >>> >>> @@ -185,6 +188,8 @@ struct
>dpi_mbox_message { >>> uint64_t word_h; >>> }; >>>
>
>On Wed, Jul 17, 2024, at 07:35, Vamsi Krishna Attunuru wrote:
>
>>>>  #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.
>
>You shouldn't need any #ifdef here, just call readq/writeq unconditionally
>after including the header.
>
>Have you been able to confirm whether the device works correctly with the
>lo_hi ordering?
>

Neither of them worked in our case, HW folks also confirmed that only 64bit access work correctly.
I will just include the header that address the compilation errors with ARCH=arm, anyways nobody
will use this driver on 32-bit kernel.

Thanks Arnd. 

Vamsi

>      Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
  2024-07-17 11:45       ` Vamsi Krishna Attunuru
@ 2024-07-17 11:49         ` Arnd Bergmann
  2024-07-17 12:17           ` Vamsi Krishna Attunuru
  0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2024-07-17 11:49 UTC (permalink / raw)
  To: Vamsi Attunuru, Greg Kroah-Hartman
  Cc: linux-kernel, Nathan Chancellor, Jeff Johnson

On Wed, Jul 17, 2024, at 13:45, Vamsi Krishna Attunuru wrote:
>
> Neither of them worked in our case, HW folks also confirmed that only 
> 64bit access work correctly.
> I will just include the header that address the compilation errors with 
> ARCH=arm, anyways nobody
> will use this driver on 32-bit kernel.

Please just use a Kconfig dependency then. If the device
requires 64-bit register access, then the driver should not
use the fallback.

     Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
  2024-07-17 11:49         ` Arnd Bergmann
@ 2024-07-17 12:17           ` Vamsi Krishna Attunuru
  2024-07-17 12:52             ` Arnd Bergmann
  2024-07-17 13:41             ` Nathan Chancellor
  0 siblings, 2 replies; 10+ messages in thread
From: Vamsi Krishna Attunuru @ 2024-07-17 12:17 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman
  Cc: linux-kernel, Nathan Chancellor, Jeff Johnson



>-----Original Message-----
>From: Arnd Bergmann <arnd@arndb.de>
>Sent: Wednesday, July 17, 2024 5:20 PM
>To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; Greg Kroah-Hartman
><gregkh@linuxfoundation.org>
>Cc: linux-kernel@vger.kernel.org; Nathan Chancellor <nathan@kernel.org>;
>Jeff Johnson <quic_jjohnson@quicinc.com>
>Subject: Re: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve
>compilation issues on 32-bit ARM
>
>On Wed, Jul 17, 2024, at 13: 45, Vamsi Krishna Attunuru wrote: > > Neither of
>them worked in our case, HW folks also confirmed that only > 64bit access
>work correctly. > I will just include the header that address the compilation
>
>On Wed, Jul 17, 2024, at 13:45, Vamsi Krishna Attunuru wrote:
>>
>> Neither of them worked in our case, HW folks also confirmed that only
>> 64bit access work correctly.
>> I will just include the header that address the compilation errors
>> with ARCH=arm, anyways nobody will use this driver on 32-bit kernel.
>
>Please just use a Kconfig dependency then. If the device requires 64-bit
>register access, then the driver should not use the fallback.

Ack, since it needs to skip compilation on 32-bit platforms, can you please
confirm below change is suffice or not.

--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -588,7 +588,7 @@ config NSM
 config MARVELL_CN10K_DPI
        tristate "Octeon CN10K DPI driver"
        depends on PCI
-       depends on ARCH_THUNDER || COMPILE_TEST
+       depends on (ARCH_THUNDER || COMPILE_TEST) && 64BIT
        help

Regards
Vamsi
>
>     Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
  2024-07-17 12:17           ` Vamsi Krishna Attunuru
@ 2024-07-17 12:52             ` Arnd Bergmann
  2024-07-17 13:41             ` Nathan Chancellor
  1 sibling, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2024-07-17 12:52 UTC (permalink / raw)
  To: Vamsi Attunuru, Greg Kroah-Hartman
  Cc: linux-kernel, Nathan Chancellor, Jeff Johnson

On Wed, Jul 17, 2024, at 14:17, Vamsi Krishna Attunuru wrote:
>>
>>On Wed, Jul 17, 2024, at 13: 45, Vamsi Krishna Attunuru wrote: > > Neither of
>>them worked in our case, HW folks also confirmed that only > 64bit access
>>work correctly. > I will just include the header that address the compilation
>>
>>On Wed, Jul 17, 2024, at 13:45, Vamsi Krishna Attunuru wrote:
>>>
>>> Neither of them worked in our case, HW folks also confirmed that only
>>> 64bit access work correctly.
>>> I will just include the header that address the compilation errors
>>> with ARCH=arm, anyways nobody will use this driver on 32-bit kernel.
>>
>>Please just use a Kconfig dependency then. If the device requires 64-bit
>>register access, then the driver should not use the fallback.
>
> Ack, since it needs to skip compilation on 32-bit platforms, can you please
> confirm below change is suffice or not.
>
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -588,7 +588,7 @@ config NSM
>  config MARVELL_CN10K_DPI
>         tristate "Octeon CN10K DPI driver"
>         depends on PCI
> -       depends on ARCH_THUNDER || COMPILE_TEST
> +       depends on (ARCH_THUNDER || COMPILE_TEST) && 64BIT
>         help
>

Yes, this is correct, thanks!

I would probably put the 64BIT dependency in a separate
line, or next to the PCI one, but the result is the same,
so pick whichever makes most sense to you.

     Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
  2024-07-17 12:17           ` Vamsi Krishna Attunuru
  2024-07-17 12:52             ` Arnd Bergmann
@ 2024-07-17 13:41             ` Nathan Chancellor
  2024-07-17 13:59               ` Jeff Johnson
  1 sibling, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2024-07-17 13:41 UTC (permalink / raw)
  To: Vamsi Krishna Attunuru
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Jeff Johnson

On Wed, Jul 17, 2024 at 12:17:08PM +0000, Vamsi Krishna Attunuru wrote:
> 
> 
> >-----Original Message-----
> >From: Arnd Bergmann <arnd@arndb.de>
> >Sent: Wednesday, July 17, 2024 5:20 PM
> >To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; Greg Kroah-Hartman
> ><gregkh@linuxfoundation.org>
> >Cc: linux-kernel@vger.kernel.org; Nathan Chancellor <nathan@kernel.org>;
> >Jeff Johnson <quic_jjohnson@quicinc.com>
> >Subject: Re: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve
> >compilation issues on 32-bit ARM
> >
> >On Wed, Jul 17, 2024, at 13: 45, Vamsi Krishna Attunuru wrote: > > Neither of
> >them worked in our case, HW folks also confirmed that only > 64bit access
> >work correctly. > I will just include the header that address the compilation
> >
> >On Wed, Jul 17, 2024, at 13:45, Vamsi Krishna Attunuru wrote:
> >>
> >> Neither of them worked in our case, HW folks also confirmed that only
> >> 64bit access work correctly.
> >> I will just include the header that address the compilation errors
> >> with ARCH=arm, anyways nobody will use this driver on 32-bit kernel.
> >
> >Please just use a Kconfig dependency then. If the device requires 64-bit
> >register access, then the driver should not use the fallback.
> 
> Ack, since it needs to skip compilation on 32-bit platforms, can you please
> confirm below change is suffice or not.
> 
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -588,7 +588,7 @@ config NSM
>  config MARVELL_CN10K_DPI
>         tristate "Octeon CN10K DPI driver"
>         depends on PCI
> -       depends on ARCH_THUNDER || COMPILE_TEST
> +       depends on (ARCH_THUNDER || COMPILE_TEST) && 64BIT

I think it would be a little clearer written as

  depends on ARCH_THUNDER || (COMPILE_TEST && 64BIT)

because ARCH_THUNDER can only be defined when 64BIT is set. Regardless
though, that should resolve the issue.

Tested-by: Nathan Chancellor <nathan@kernel.org>

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM
  2024-07-17 13:41             ` Nathan Chancellor
@ 2024-07-17 13:59               ` Jeff Johnson
  0 siblings, 0 replies; 10+ messages in thread
From: Jeff Johnson @ 2024-07-17 13:59 UTC (permalink / raw)
  To: Nathan Chancellor, Vamsi Krishna Attunuru
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel

On 7/17/2024 6:41 AM, Nathan Chancellor wrote:
> On Wed, Jul 17, 2024 at 12:17:08PM +0000, Vamsi Krishna Attunuru wrote:
>>
>>
>>> -----Original Message-----
>>> From: Arnd Bergmann <arnd@arndb.de>
>>> Sent: Wednesday, July 17, 2024 5:20 PM
>>> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; Greg Kroah-Hartman
>>> <gregkh@linuxfoundation.org>
>>> Cc: linux-kernel@vger.kernel.org; Nathan Chancellor <nathan@kernel.org>;
>>> Jeff Johnson <quic_jjohnson@quicinc.com>
>>> Subject: Re: [EXTERNAL] Re: [PATCH] misc: mrvl-cn10k-dpi: resolve
>>> compilation issues on 32-bit ARM
>>>
>>> On Wed, Jul 17, 2024, at 13: 45, Vamsi Krishna Attunuru wrote: > > Neither of
>>> them worked in our case, HW folks also confirmed that only > 64bit access
>>> work correctly. > I will just include the header that address the compilation
>>>
>>> On Wed, Jul 17, 2024, at 13:45, Vamsi Krishna Attunuru wrote:
>>>>
>>>> Neither of them worked in our case, HW folks also confirmed that only
>>>> 64bit access work correctly.
>>>> I will just include the header that address the compilation errors
>>>> with ARCH=arm, anyways nobody will use this driver on 32-bit kernel.
>>>
>>> Please just use a Kconfig dependency then. If the device requires 64-bit
>>> register access, then the driver should not use the fallback.
>>
>> Ack, since it needs to skip compilation on 32-bit platforms, can you please
>> confirm below change is suffice or not.
>>
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -588,7 +588,7 @@ config NSM
>>  config MARVELL_CN10K_DPI
>>         tristate "Octeon CN10K DPI driver"
>>         depends on PCI
>> -       depends on ARCH_THUNDER || COMPILE_TEST
>> +       depends on (ARCH_THUNDER || COMPILE_TEST) && 64BIT
> 
> I think it would be a little clearer written as
> 
>   depends on ARCH_THUNDER || (COMPILE_TEST && 64BIT)
> 
> because ARCH_THUNDER can only be defined when 64BIT is set. Regardless
> though, that should resolve the issue.
> 
> Tested-by: Nathan Chancellor <nathan@kernel.org>
> 
> Cheers,
> Nathan

I took Nathan's suggestion locally,

Tested-by: Jeff Johnson <quic_jjohnson@quicinc.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-07-17 14:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-17  4:52 [PATCH] misc: mrvl-cn10k-dpi: resolve compilation issues on 32-bit ARM Vamsi Attunuru
2024-07-17  5:16 ` Greg KH
2024-07-17  5:35   ` [EXTERNAL] " Vamsi Krishna Attunuru
2024-07-17  5:39     ` Arnd Bergmann
2024-07-17 11:45       ` Vamsi Krishna Attunuru
2024-07-17 11:49         ` Arnd Bergmann
2024-07-17 12:17           ` Vamsi Krishna Attunuru
2024-07-17 12:52             ` Arnd Bergmann
2024-07-17 13:41             ` Nathan Chancellor
2024-07-17 13:59               ` Jeff Johnson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®