These patches remove redundant DMA_..BIT_MASK definitions across two

drivers. The computation of the majority of the bitmasks is done by the

compiler. The initial split of the patch touching each a different file got

removed due to possible git bisect breakage.
Andrew, can you please apply this patch for it touches drivers maintained by

different people and i there might be responsibility issues, imho.
Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>

Cc: Jeremy Fitzhardinge <jeremy@goop.org>

Cc: Muli Ben-Yehuda <muli@il.ibm.com>
 drivers/net/netxen/netxen_nic_main.c |    3 ---

 drivers/scsi/gdth.c                  |    5 -----

 include/linux/dma-mapping.h          |   23 +++++++++++++----------

 3 files changed, 13 insertions(+), 18 deletions(-)
--

Index: b/include/linux/dma-mapping.h

===================================================================

--- a/include/linux/dma-mapping.h	2007-09-18 21:12:30.000000000 +0200

+++ b/include/linux/dma-mapping.h	2007-09-18 21:13:17.000000000 +0200

@@ -13,16 +13,19 @@

 	DMA_NONE = 3,

 };
-#define DMA_64BIT_MASK	0xffffffffffffffffULL

-#define DMA_48BIT_MASK	0x0000ffffffffffffULL

-#define DMA_40BIT_MASK	0x000000ffffffffffULL

-#define DMA_39BIT_MASK	0x0000007fffffffffULL

-#define DMA_32BIT_MASK	0x00000000ffffffffULL

-#define DMA_31BIT_MASK	0x000000007fffffffULL

-#define DMA_30BIT_MASK	0x000000003fffffffULL

-#define DMA_29BIT_MASK	0x000000001fffffffULL

-#define DMA_28BIT_MASK	0x000000000fffffffULL

-#define DMA_24BIT_MASK	0x0000000000ffffffULL

+#define DMA_BIT_MASK(n)	((1ULL<<(n))-1)

+

+#define DMA_64BIT_MASK	(~0ULL)

+#define DMA_48BIT_MASK	DMA_BIT_MASK(48)

+#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_M...
[ message continues ]

On Tue, 18 Sep 2007 21:46:47 +0200
Now that you've done this, those DMA_xxBIT_MASK macros are pointless and

stupid and we should aim to get rid of them.
From: Andrew Morton <akpm@linux-foundation.org>
Now that we have DMA_BIT_MASK(), these macros are pointless.
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

---
 include/linux/dma-mapping.h |    6 ++++++

 1 file changed, 6 insertions(+)
diff -puN include/linux/dma-mapping.h~a include/linux/dma-mapping.h

--- a/include/linux/dma-mapping.h~a

+++ a/include/linux/dma-mapping.h

@@ -15,6 +15,12 @@ enum dma_data_direction {
 #define DMA_BIT_MASK(n)	((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	(~0ULL)

 #define DMA_48BIT_MASK	DMA_BIT_MASK(48)

 #define DMA_47BIT_MASK	DMA_BIT_MASK(47)

_
-

Except, unfortunately, DMA_64BIT_MASK.  I guess we could special case

it, assuming this works in all the contexts the macro is used in (ie,

compile-time constant?):
#define DMA_BIT_MASK(n)	(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
    J

-

On Fri, 05 Oct 2007 13:43:54 -0700
doh.  Thanks.
--- a/include/linux/dma-mapping.h~stop-using-dma_xxbit_mask-fix

+++ a/include/linux/dma-mapping.h

@@ -13,7 +13,7 @@ enum dma_data_direction {

 	DMA_NONE = 3,

 };
-#define DMA_BIT_MASK(n)	((1ULL<<(n))-1)

+#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

@@ -21,7 +21,7 @@ enum dma_data_direction {

  *

  * Instead, just open-code DMA_BIT_MASK(n) within your driver

  */

-#define DMA_64BIT_MASK	(~0ULL)

+#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)

_
it's irksome that there doesn't seem to be a neater way of doing

this, until they give us unsigned long long longs.

-

Hi and sorry for the late reply. IMHO, this solution is the most concise and still

clear enough a macro to understand what it does after a quick scan (unlike

uglies like IO_COND() in lib/iomap.c¹, :)) And, as a next step, we probably

should do
perl -pi -e 's/DMA_(..)BIT_MASK/DMA_BIT_MASK($1)/g' *
after removing the #define DMA_..BIT_MASK defines in /include/linux/dma-mapping.h

and the other two headers in the original patch after the x86 merge.

Current git (9f34073b4e54ad58541e0e2b4a87f4f6c1460e21) contains about 394

instances of usage of those macros, including the #definitions.
¹ this is not a flame!

--

Regards/Gruß,

    Boris.

-

or you could take advantage of the macros in kernel.h and write that

as:
+#define DMA_BIT_MASK(n)      (((n) == 64) ? ULLONG_MAX : ((1ULL<<(n))-1))
rday

--

========================================================================

Robert P. J. Day

Linux Consulting, Training and Annoying Kernel Pedantry

Waterloo, Ontario, CANADA
http://crashcourse.ca

========================================================================

-

#define DMA_BIT_MASK(n)         ((u64)-1 >> (64 - (n)))
Andreas.
--

Andreas Schwab, SuSE Labs, schwab@suse.de

SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany

PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5

"And now for something completely different."

-

Yeah, that's cleaner.
    J

-

On Fri, 05 Oct 2007 14:28:45 -0700
Well yes, but DMA_BIT_MASK(0) invokes undefined behaviour, generates a

compiler warning and evaluates to 0xffffffffffffffff (with my setup).
That won't be a problem in practice, but it is strictly wrong and doesn't set

a good exmaple for the children ;)
-

It's interesting that it doesn't seem to be possible to define this

without invoking some undefined behaviour.  But a device that supports 0

bits of DMA address probably isn't terribly concerned about this - it's

certainly better than making 64 bit masks warty.
    J

-

But that's a more indirect way of expressing "I want all 1's".
    J

-

... and ULLONG_MAX _is_ (~0ULL).
--

Regards/Gruß,

    Boris.

-

Thanks, Borislav.
Reviewed-by: Satyam Sharma <satyam@infradead.org>

-