| Message ID | 20090507141114M.fujita.tomonori@lab.ntt.co.jp |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
FUJITA Tomonori wrote: > We replaced all DMA_nBIT_MASK macros with DMA_BIT_MASK(n) but why do > we still keep DMA_nBIT_MASK macros in include/linux/dma-mapping.h? > > As long as these macros exist, people use them. The current git has > two users and linux-next have other users. > > Is it better to remove DMA_nBIT_MASK macros completely now? > CC:ingo I have no objections,actually I used to remove all these defines in my first commit of these patch series,but got suggestions that keep these defines one more circle.Maybe it's time to remove these defines now or to remove at the end of this circle? Reviewed-by:yanghy@cn.fujitsu.com > = > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > Subject: [PATCH] remove DMA_nBIT_MASK macros > > This removes DMA_nBIT_MASK macros in dma-mapping.h so that we will not > have any new users. > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > --- > arch/arm/mach-davinci/board-dm644x-evm.c | 4 ++-- > drivers/net/igbvf/netdev.c | 13 +++++++------ > include/linux/dma-mapping.h | 19 ------------------- > 3 files changed, 9 insertions(+), 27 deletions(-) > > diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c > index c039674..b2e7f9c 100644 > --- a/arch/arm/mach-davinci/board-dm644x-evm.c > +++ b/arch/arm/mach-davinci/board-dm644x-evm.c > @@ -211,7 +211,7 @@ static struct resource ide_resources[] = { > }, > }; > > -static u64 ide_dma_mask = DMA_32BIT_MASK; > +static u64 ide_dma_mask = DMA_BIT_MASK(32); > > static struct platform_device ide_dev = { > .name = "palm_bk3710", > @@ -220,7 +220,7 @@ static struct platform_device ide_dev = { > .num_resources = ARRAY_SIZE(ide_resources), > .dev = { > .dma_mask = &ide_dma_mask, > - .coherent_dma_mask = DMA_32BIT_MASK, > + .coherent_dma_mask = DMA_BIT_MASK(32), > }, > }; > > diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c > index b774666..c084d3b 100644 > --- a/drivers/net/igbvf/netdev.c > +++ b/drivers/net/igbvf/netdev.c > @@ -1279,7 +1279,7 @@ static void igbvf_configure_tx(struct igbvf_adapter *adapter) > /* Setup the HW Tx Head and Tail descriptor pointers */ > ew32(TDLEN(0), tx_ring->count * sizeof(union e1000_adv_tx_desc)); > tdba = tx_ring->dma; > - ew32(TDBAL(0), (tdba & DMA_32BIT_MASK)); > + ew32(TDBAL(0), (tdba & DMA_BIT_MASK(32))); > ew32(TDBAH(0), (tdba >> 32)); > ew32(TDH(0), 0); > ew32(TDT(0), 0); > @@ -1365,7 +1365,7 @@ static void igbvf_configure_rx(struct igbvf_adapter *adapter) > * the Base and Length of the Rx Descriptor Ring > */ > rdba = rx_ring->dma; > - ew32(RDBAL(0), (rdba & DMA_32BIT_MASK)); > + ew32(RDBAL(0), (rdba & DMA_BIT_MASK(32))); > ew32(RDBAH(0), (rdba >> 32)); > ew32(RDLEN(0), rx_ring->count * sizeof(union e1000_adv_rx_desc)); > rx_ring->head = E1000_RDH(0); > @@ -2637,15 +2637,16 @@ static int __devinit igbvf_probe(struct pci_dev *pdev, > return err; > > pci_using_dac = 0; > - err = pci_set_dma_mask(pdev, DMA_64BIT_MASK); > + err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); > if (!err) { > - err = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK); > + err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)); > if (!err) > pci_using_dac = 1; > } else { > - err = pci_set_dma_mask(pdev, DMA_32BIT_MASK); > + err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32)); > if (err) { > - err = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK); > + err = pci_set_consistent_dma_mask(pdev, > + DMA_BIT_MASK(32)); > if (err) { > dev_err(&pdev->dev, "No usable DMA " > "configuration, aborting\n"); > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h > index 8083b6a..f56e607 100644 > --- a/include/linux/dma-mapping.h > +++ b/include/linux/dma-mapping.h > @@ -63,25 +63,6 @@ struct dma_map_ops { > Is it better to add a comment here to tell people that the old macro was deleted? > #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) > > -/* > - * NOTE: do not use the below macros in new code and do not add new definitions > - * here. > - * > - * Instead, just open-code DMA_BIT_MASK(n) within your driver > - */ > -#define DMA_64BIT_MASK DMA_BIT_MASK(64) > -#define DMA_48BIT_MASK DMA_BIT_MASK(48) > -#define DMA_47BIT_MASK DMA_BIT_MASK(47) > -#define DMA_40BIT_MASK DMA_BIT_MASK(40) > -#define DMA_39BIT_MASK DMA_BIT_MASK(39) > -#define DMA_35BIT_MASK DMA_BIT_MASK(35) > -#define DMA_32BIT_MASK DMA_BIT_MASK(32) > -#define DMA_31BIT_MASK DMA_BIT_MASK(31) > -#define DMA_30BIT_MASK DMA_BIT_MASK(30) > -#define DMA_29BIT_MASK DMA_BIT_MASK(29) > -#define DMA_28BIT_MASK DMA_BIT_MASK(28) > -#define DMA_24BIT_MASK DMA_BIT_MASK(24) > - > #define DMA_MASK_NONE 0x0ULL > > static inline int valid_dma_direction(int dma_direction)
On Thu, 7 May 2009 14:14:16 +0900 FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote: > We replaced all DMA_nBIT_MASK macros with DMA_BIT_MASK(n) but why do > we still keep DMA_nBIT_MASK macros in include/linux/dma-mapping.h? > > As long as these macros exist, people use them. The current git has > two users and linux-next have other users. > > Is it better to remove DMA_nBIT_MASK macros completely now? Yes, the plan is to remove them. Doing so will break lots and lots of out-of-tree drivers, causing people some grief. Is there any way in which we can cause their use to cause __deprecated warnings for a couple of months, to give people a chance to migrate? -- 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
Andrew Morton wrote: > On Thu, 7 May 2009 14:14:16 +0900 FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote: > >> We replaced all DMA_nBIT_MASK macros with DMA_BIT_MASK(n) but why do >> we still keep DMA_nBIT_MASK macros in include/linux/dma-mapping.h? >> >> As long as these macros exist, people use them. The current git has >> two users and linux-next have other users. >> >> Is it better to remove DMA_nBIT_MASK macros completely now? > > Yes, the plan is to remove them. > > Doing so will break lots and lots of out-of-tree drivers, causing > people some grief. Is there any way in which we can cause their use to > cause __deprecated warnings for a couple of months, to give people a > chance to migrate? > > Shall we use something like below to warn people? #define DMA_64BIT_MASK DMA_BIT_MASK(64);(__deprecated warnings:use DMA_BIT_MASK(64) instead) ?
On 05/07/2009 10:29 AM, Yang Hongyang wrote: > Shall we use something like below to warn people? > #define DMA_64BIT_MASK DMA_BIT_MASK(64);(__deprecated warnings:use DMA_BIT_MASK(64) instead) ? Something like an irq flags deprecation in the past: http://lwn.net/Articles/229673/ ? I don't know how many in-initializer (struct x x = { DMA_64BIT_MASK } ) users are out there though. -- 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
Jiri Slaby wrote: > On 05/07/2009 10:29 AM, Yang Hongyang wrote: >> Shall we use something like below to warn people? >> #define DMA_64BIT_MASK DMA_BIT_MASK(64);(__deprecated warnings:use DMA_BIT_MASK(64) instead) ? > > Something like an irq flags deprecation in the past: > http://lwn.net/Articles/229673/ > ? good catch,but this need to be modified (add one line is enough)to cause their use to cause __deprecated warnings. > > I don't know how many in-initializer (struct x x = { DMA_64BIT_MASK } ) > users are out there though. > >
On 05/07/2009 10:59 AM, Yang Hongyang wrote: > Jiri Slaby wrote: >> Something like an irq flags deprecation in the past: >> http://lwn.net/Articles/229673/ >> ? > > good catch,but this need to be modified (add one line is enough)to cause their use to > cause __deprecated warnings. I'm lost, that inline does it, right? -- 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
* Yang Hongyang <yanghy@cn.fujitsu.com> wrote: > FUJITA Tomonori wrote: > > We replaced all DMA_nBIT_MASK macros with DMA_BIT_MASK(n) but why do > > we still keep DMA_nBIT_MASK macros in include/linux/dma-mapping.h? > > > > As long as these macros exist, people use them. The current git has > > two users and linux-next have other users. > > > > Is it better to remove DMA_nBIT_MASK macros completely now? Can you see a way to emit build warnings? If yes then that might be a better solution instead of breaking in-the-pipeline code. We missed the upstream window of removing the facilities altogether, we could certainly do that in the next merge window though. > CC:ingo > > I have no objections,actually I used to remove all these defines in my > first commit of these patch series,but got suggestions that keep these > defines one more circle.Maybe it's time to remove these defines now or > to remove at the end of this circle? > > Reviewed-by:yanghy@cn.fujitsu.com Thanks! Ingo -- 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 Thu, 7 May 2009 13:18:46 +0200 Ingo Molnar <mingo@elte.hu> wrote: > > * Yang Hongyang <yanghy@cn.fujitsu.com> wrote: > > > FUJITA Tomonori wrote: > > > We replaced all DMA_nBIT_MASK macros with DMA_BIT_MASK(n) but why do > > > we still keep DMA_nBIT_MASK macros in include/linux/dma-mapping.h? > > > > > > As long as these macros exist, people use them. The current git has > > > two users and linux-next have other users. > > > > > > Is it better to remove DMA_nBIT_MASK macros completely now? > > Can you see a way to emit build warnings? If yes then that might be > a better solution instead of breaking in-the-pipeline code. Unfortunately, no. Since 2.6.24, include/linux/dma-mapping.h has the warning: /* * NOTE: do not use the below macros in new code and do not add new definitions * here. * * Instead, just open-code DMA_BIT_MASK(n) within your driver */ IMO, we give people enough time for migration. > We missed the upstream window of removing the facilities altogether, > we could certainly do that in the next merge window though. Can you apply this patch to the tip? I want to have this patch in linux-next to let new users of DMA-nBIT_MASK in linux-next know that they use wrong macros. -- 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 Thu, 07 May 2009 10:46:09 +0200 Jiri Slaby <jirislaby@gmail.com> wrote: > On 05/07/2009 10:29 AM, Yang Hongyang wrote: > > Shall we use something like below to warn people? > > #define DMA_64BIT_MASK DMA_BIT_MASK(64);(__deprecated warnings:use DMA_BIT_MASK(64) instead) ? > > Something like an irq flags deprecation in the past: > http://lwn.net/Articles/229673/ > ? > > I don't know how many in-initializer (struct x x = { DMA_64BIT_MASK } ) > users are out there though. Unfortunately, there are lots. Converting these macros to functions would breaks drivers too. -- 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/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c index c039674..b2e7f9c 100644 --- a/arch/arm/mach-davinci/board-dm644x-evm.c +++ b/arch/arm/mach-davinci/board-dm644x-evm.c @@ -211,7 +211,7 @@ static struct resource ide_resources[] = { }, }; -static u64 ide_dma_mask = DMA_32BIT_MASK; +static u64 ide_dma_mask = DMA_BIT_MASK(32); static struct platform_device ide_dev = { .name = "palm_bk3710", @@ -220,7 +220,7 @@ static struct platform_device ide_dev = { .num_resources = ARRAY_SIZE(ide_resources), .dev = { .dma_mask = &ide_dma_mask, - .coherent_dma_mask = DMA_32BIT_MASK, + .coherent_dma_mask = DMA_BIT_MASK(32), }, }; diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c index b774666..c084d3b 100644 --- a/drivers/net/igbvf/netdev.c +++ b/drivers/net/igbvf/netdev.c @@ -1279,7 +1279,7 @@ static void igbvf_configure_tx(struct igbvf_adapter *adapter) /* Setup the HW Tx Head and Tail descriptor pointers */ ew32(TDLEN(0), tx_ring->count * sizeof(union e1000_adv_tx_desc)); tdba = tx_ring->dma; - ew32(TDBAL(0), (tdba & DMA_32BIT_MASK)); + ew32(TDBAL(0), (tdba & DMA_BIT_MASK(32))); ew32(TDBAH(0), (tdba >> 32)); ew32(TDH(0), 0); ew32(TDT(0), 0); @@ -1365,7 +1365,7 @@ static void igbvf_configure_rx(struct igbvf_adapter *adapter) * the Base and Length of the Rx Descriptor Ring */ rdba = rx_ring->dma; - ew32(RDBAL(0), (rdba & DMA_32BIT_MASK)); + ew32(RDBAL(0), (rdba & DMA_BIT_MASK(32))); ew32(RDBAH(0), (rdba >> 32)); ew32(RDLEN(0), rx_ring->count * sizeof(union e1000_adv_rx_desc)); rx_ring->head = E1000_RDH(0); @@ -2637,15 +2637,16 @@ static int __devinit igbvf_probe(struct pci_dev *pdev, return err; pci_using_dac = 0; - err = pci_set_dma_mask(pdev, DMA_64BIT_MASK); + err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); if (!err) { - err = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK); + err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)); if (!err) pci_using_dac = 1; } else { - err = pci_set_dma_mask(pdev, DMA_32BIT_MASK); + err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32)); if (err) { - err = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK); + err = pci_set_consistent_dma_mask(pdev, + DMA_BIT_MASK(32)); if (err) { dev_err(&pdev->dev, "No usable DMA " "configuration, aborting\n"); diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index 8083b6a..f56e607 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -63,25 +63,6 @@ struct dma_map_ops { #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) -/* - * NOTE: do not use the below macros in new code and do not add new definitions - * here. - * - * Instead, just open-code DMA_BIT_MASK(n) within your driver - */ -#define DMA_64BIT_MASK DMA_BIT_MASK(64) -#define DMA_48BIT_MASK DMA_BIT_MASK(48) -#define DMA_47BIT_MASK DMA_BIT_MASK(47) -#define DMA_40BIT_MASK DMA_BIT_MASK(40) -#define DMA_39BIT_MASK DMA_BIT_MASK(39) -#define DMA_35BIT_MASK DMA_BIT_MASK(35) -#define DMA_32BIT_MASK DMA_BIT_MASK(32) -#define DMA_31BIT_MASK DMA_BIT_MASK(31) -#define DMA_30BIT_MASK DMA_BIT_MASK(30) -#define DMA_29BIT_MASK DMA_BIT_MASK(29) -#define DMA_28BIT_MASK DMA_BIT_MASK(28) -#define DMA_24BIT_MASK DMA_BIT_MASK(24) - #define DMA_MASK_NONE 0x0ULL static inline int valid_dma_direction(int dma_direction)