Message ID | 1231951391.11301.131.camel@lb-tlvb-eliezer |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
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.
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
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
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
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 --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",
Signed-off-by: Eilon Greenstein <eilong@broadcom.com> --- drivers/net/bnx2x_main.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)