diff mbox

[10/34] bnx2x: Compilation issue on IA64

Message ID 1231951391.11301.131.camel@lb-tlvb-eliezer
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Eilon Greenstein Jan. 14, 2009, 4:43 p.m. UTC
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/bnx2x_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Ben Hutchings Jan. 14, 2009, 5:07 p.m. UTC | #1
On Wed, 2009-01-14 at 18:43 +0200, Eilon Greenstein wrote:
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> ---
>  drivers/net/bnx2x_main.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c
> index 4993707..ba56d1c 100644
> --- a/drivers/net/bnx2x_main.c
> +++ b/drivers/net/bnx2x_main.c
> @@ -1199,7 +1199,7 @@ static void bnx2x_tpa_start(struct bnx2x_fastpath *fp, u16 queue,
>  
>  #ifdef BNX2X_STOP_ON_ERROR
>  	fp->tpa_queue_used |= (1 << queue);
> -#ifdef __powerpc64__
> +#if (defined __powerpc64__) || (defined _ASM_IA64_TYPES_H)
>  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%lx\n",
>  #else
>  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%llx\n",

Or you could cast the value to unsigned long long and remove this
fragile #ifdef.

Maybe <linux/types.h> should define format strings like C99's PRIx64..

Ben.
Eilon Greenstein Jan. 14, 2009, 5:44 p.m. UTC | #2
On Wed, 2009-01-14 at 09:07 -0800, Ben Hutchings wrote:
> On Wed, 2009-01-14 at 18:43 +0200, Eilon Greenstein wrote:
> > Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
[...]  
> >  #ifdef BNX2X_STOP_ON_ERROR
> >  	fp->tpa_queue_used |= (1 << queue);
> > -#ifdef __powerpc64__
> > +#if (defined __powerpc64__) || (defined _ASM_IA64_TYPES_H)
> >  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%lx\n",
> >  #else
> >  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%llx\n",
> 
> Or you could cast the value to unsigned long long and remove this
> fragile #ifdef.
> 
You are right - it is a better solution. The issue is that this uglier
solution is already in, and I hate to re-send this patch just for this.
Can we have this one applied and I will re visit it in net-next?

Eilon


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Jan. 14, 2009, 9:10 p.m. UTC | #3
From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Wed, 14 Jan 2009 19:44:33 +0200

> On Wed, 2009-01-14 at 09:07 -0800, Ben Hutchings wrote:
> > On Wed, 2009-01-14 at 18:43 +0200, Eilon Greenstein wrote:
> > > Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> [...]  
> > >  #ifdef BNX2X_STOP_ON_ERROR
> > >  	fp->tpa_queue_used |= (1 << queue);
> > > -#ifdef __powerpc64__
> > > +#if (defined __powerpc64__) || (defined _ASM_IA64_TYPES_H)
> > >  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%lx\n",
> > >  #else
> > >  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%llx\n",
> > 
> > Or you could cast the value to unsigned long long and remove this
> > fragile #ifdef.
> > 
> You are right - it is a better solution. The issue is that this uglier
> solution is already in, and I hate to re-send this patch just for this.
> Can we have this one applied and I will re visit it in net-next?

There is no reason to crap up any driver with arch ifdefs, absolutely
none, for this typing issue.

The whole point is that we're trying to get all of the arch's to use
the same type for u64 in the kernel, so that these warnings can be
cleared away very easily.

Live with the warnings meanwhile until this is all sorted out.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eilon Greenstein Jan. 23, 2009, 8:42 a.m. UTC | #4
On Wed, 2009-01-14 at 13:10 -0800, David Miller wrote:
> From: "Eilon Greenstein" <eilong@broadcom.com>
> Date: Wed, 14 Jan 2009 19:44:33 +0200
> 
> > On Wed, 2009-01-14 at 09:07 -0800, Ben Hutchings wrote:
> > > On Wed, 2009-01-14 at 18:43 +0200, Eilon Greenstein wrote:
> > > > Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> > [...]  
> > > >  #ifdef BNX2X_STOP_ON_ERROR
> > > >  	fp->tpa_queue_used |= (1 << queue);
> > > > -#ifdef __powerpc64__
> > > > +#if (defined __powerpc64__) || (defined _ASM_IA64_TYPES_H)
> > > >  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%lx\n",
> > > >  #else
> > > >  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%llx\n",
> > > 
> > > Or you could cast the value to unsigned long long and remove this
> > > fragile #ifdef.
> > > 
> > You are right - it is a better solution. The issue is that this uglier
> > solution is already in, and I hate to re-send this patch just for this.
> > Can we have this one applied and I will re visit it in net-next?
> 
> There is no reason to crap up any driver with arch ifdefs, absolutely
> none, for this typing issue.
> 
> The whole point is that we're trying to get all of the arch's to use
> the same type for u64 in the kernel, so that these warnings can be
> cleared away very easily.
> 
> Live with the warnings meanwhile until this is all sorted out.

I can live with that, but I was wondering if this change is something
that you can consider:
-#ifdef __powerpc64__
+#ifdef _ASM_GENERIC_INT_L64_H

My problem is that in the mean time, the driver has this __powerpc64__
ifdef in it.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Jan. 26, 2009, 2:16 a.m. UTC | #5
From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Fri, 23 Jan 2009 10:42:27 +0200

> On Wed, 2009-01-14 at 13:10 -0800, David Miller wrote:
> > From: "Eilon Greenstein" <eilong@broadcom.com>
> > Date: Wed, 14 Jan 2009 19:44:33 +0200
> > 
> > > On Wed, 2009-01-14 at 09:07 -0800, Ben Hutchings wrote:
> > > > On Wed, 2009-01-14 at 18:43 +0200, Eilon Greenstein wrote:
> > > > > Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> > > [...]  
> > > > >  #ifdef BNX2X_STOP_ON_ERROR
> > > > >  	fp->tpa_queue_used |= (1 << queue);
> > > > > -#ifdef __powerpc64__
> > > > > +#if (defined __powerpc64__) || (defined _ASM_IA64_TYPES_H)
> > > > >  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%lx\n",
> > > > >  #else
> > > > >  	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%llx\n",
> > > > 
> > > > Or you could cast the value to unsigned long long and remove this
> > > > fragile #ifdef.
> > > > 
> > > You are right - it is a better solution. The issue is that this uglier
> > > solution is already in, and I hate to re-send this patch just for this.
> > > Can we have this one applied and I will re visit it in net-next?
> > 
> > There is no reason to crap up any driver with arch ifdefs, absolutely
> > none, for this typing issue.
> > 
> > The whole point is that we're trying to get all of the arch's to use
> > the same type for u64 in the kernel, so that these warnings can be
> > cleared away very easily.
> > 
> > Live with the warnings meanwhile until this is all sorted out.
> 
> I can live with that, but I was wondering if this change is something
> that you can consider:
> -#ifdef __powerpc64__
> +#ifdef _ASM_GENERIC_INT_L64_H
> 
> My problem is that in the mean time, the driver has this __powerpc64__
> ifdef in it.

Just use %ll unconditionally and cast the argument to
long long.  Then you don't need any ifdefs at all.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c
index 4993707..ba56d1c 100644
--- a/drivers/net/bnx2x_main.c
+++ b/drivers/net/bnx2x_main.c
@@ -1199,7 +1199,7 @@  static void bnx2x_tpa_start(struct bnx2x_fastpath *fp, u16 queue,
 
 #ifdef BNX2X_STOP_ON_ERROR
 	fp->tpa_queue_used |= (1 << queue);
-#ifdef __powerpc64__
+#if (defined __powerpc64__) || (defined _ASM_IA64_TYPES_H)
 	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%lx\n",
 #else
 	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%llx\n",