| Message ID | 1282972125-9968-1-git-send-email-akinobu.mita@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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)) :-) -- 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
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)) -- 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: 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. -- 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 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. -- 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/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); }
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(-)