[PATCH] starfire: use BUILD_BUG_ON for netdrv_addr_t

[PATCH] starfire: use BUILD_BUG_ON for netdrv_addr_t

[Akinobu Mita]

Detect size mismatch for netdrv_addr_t at build time rather than
checking at module load time.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Ion Badulescu <ionut@badula.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
---
 drivers/net/starfire.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/net/starfire.c b/drivers/net/starfire.c
index 26c3634..c3bf288 100644
--- a/drivers/net/starfire.c
+++ b/drivers/net/starfire.c
@@ -2078,11 +2078,7 @@ static int __init starfire_init (void)
 	printk(KERN_INFO DRV_NAME ": polling (NAPI) enabled\n");
 #endif
 
-	/* we can do this test only at run-time... sigh */
-	if (sizeof(dma_addr_t) != sizeof(netdrv_addr_t)) {
-		printk("This driver has dma_addr_t issues, please send email to maintainer\n");
-		return -ENODEV;
-	}
+	BUILD_BUG_ON(sizeof(dma_addr_t) != sizeof(netdrv_addr_t));
 
 	return pci_register_driver(&starfire_driver);
 }
-- 
1.6.0.6

Re: [PATCH] starfire: use BUILD_BUG_ON for netdrv_addr_t

[David Miller]

From: Akinobu Mita <akinobu.mita@gmail.com>
Date: Sat, 28 Aug 2010 14:08:45 +0900

> Detect size mismatch for netdrv_addr_t at build time rather than
> checking at module load time.
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Ion Badulescu <ionut@badula.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: netdev@vger.kernel.org

Applied, but I suspect we will see some interesting build failures now
if that ugly and brittle ifdef test which constrols the netdrv_addr_t
define isn't %100 accurate.

Fujita-san, what this driver does is actually pretty reasonable.  It
has two kinds of descriptors, one supports 32-bit addresses and the
other supports 64-bit addresses.  It wants to CPP test which one to
use so that the driver is not burdoned with two duplicated sets of
routines.

Maybe we should provide a DMA_ADDR_T_SIZE or similar macro?  What do
you think?  Anything is better than what it uses now:

/*
 * This SUCKS.
 * We need a much better method to determine if dma_addr_t is 64-bit.
 */
#if (defined(__i386__) && defined(CONFIG_HIGHMEM64G)) || defined(__x86_64__) || defined (__ia64__) || defined(__alpha__) || defined(__mips64__) || (defined(__mips__) && defined(CONFIG_HIGHMEM) && defined(CONFIG_64BIT_PHYS_ADDR))

:-)

Re: [PATCH] starfire: use BUILD_BUG_ON for netdrv_addr_t

[FUJITA Tomonori]

CC'ed to linux-arch,

On Sat, 28 Aug 2010 15:41:25 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Akinobu Mita <akinobu.mita@gmail.com>
> Date: Sat, 28 Aug 2010 14:08:45 +0900
> 
> > Detect size mismatch for netdrv_addr_t at build time rather than
> > checking at module load time.
> > 
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > Cc: Ion Badulescu <ionut@badula.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: netdev@vger.kernel.org
> 
> Applied, but I suspect we will see some interesting build failures now
> if that ugly and brittle ifdef test which constrols the netdrv_addr_t
> define isn't %100 accurate.

Yeah, looks like you need to add another hack for powerpc?


> Fujita-san, what this driver does is actually pretty reasonable.  It
> has two kinds of descriptors, one supports 32-bit addresses and the
> other supports 64-bit addresses.  It wants to CPP test which one to
> use so that the driver is not burdoned with two duplicated sets of
> routines.
> 
> Maybe we should provide a DMA_ADDR_T_SIZE or similar macro?  What do
> you think?  Anything is better than what it uses now:

Introducing something like CONFIG_DMA_ADDR_T_64BIT works?

include/asm-generic/types.h has:

#ifndef dma_addr_t
#ifdef CONFIG_PHYS_ADDR_T_64BIT
typedef u64 dma_addr_t;
#else
typedef u32 dma_addr_t;
#endif /* CONFIG_PHYS_ADDR_T_64BIT */
#endif /* dma_addr_t */

But the above can't be perfect so about twenty architectures define
dma_addr_t. With CONFIG_DMA_ADDR_T_64BIT, we can clean up all the
mess nicely, I guess.


> /*
>  * This SUCKS.
>  * We need a much better method to determine if dma_addr_t is 64-bit.
>  */
> #if (defined(__i386__) && defined(CONFIG_HIGHMEM64G)) || defined(__x86_64__) || defined (__ia64__) || defined(__alpha__) || defined(__mips64__) || (defined(__mips__) && defined(CONFIG_HIGHMEM) && defined(CONFIG_64BIT_PHYS_ADDR))

Re: [PATCH] starfire: use BUILD_BUG_ON for netdrv_addr_t

[David Miller]

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date: Mon, 30 Aug 2010 07:59:38 +0900

> But the above can't be perfect so about twenty architectures define
> dma_addr_t. With CONFIG_DMA_ADDR_T_64BIT, we can clean up all the
> mess nicely, I guess.

Yes, that's what we need.

Re: [PATCH] starfire: use BUILD_BUG_ON for netdrv_addr_t

[FUJITA Tomonori]

On Sun, 29 Aug 2010 21:25:34 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Date: Mon, 30 Aug 2010 07:59:38 +0900
> 
> > But the above can't be perfect so about twenty architectures define
> > dma_addr_t. With CONFIG_DMA_ADDR_T_64BIT, we can clean up all the
> > mess nicely, I guess.
> 
> Yes, that's what we need.

Ok, I'll take care of this.