[ofa-general] bitops take an unsigned long *

[Andrew Morton]

On Fri, 09 May 2008 22:37:36 -0700 Roland Dreier <rdreier at cisco.com> wrote:

>  > Most architectures could (and should) take an unsigned long * arg for their
>  > bitops.  x86 doesn't do this and it needs fixing.  I fixed it.  Infiniband
>  > is being a problem.
>
> ...
> 
>  > drivers/infiniband/hw/ipath/ipath_sdma.c:691: warning: passing argument 2 of 'constant_test_bit' from incompatible pointer type
>  > drivers/infiniband/hw/ipath/ipath_sdma.c:691: warning: passing argument 2 of 'variable_test_bit' from incompatible pointer type
> 
> So all of these are ipath warnings, seemingly all because
> ipath_devdata.ipath_sdma_status is a u64.  The stupid fix is to change
> this declaration to unsigned long as below, but this sets a trap if the
> driver is ever fixed so that it doesn't depend on 64BIT, because of
> 
>     /* bit positions for sdma_status */
>     #define IPATH_SDMA_ABORTING  0
>     #define IPATH_SDMA_DISARMED  1
>     #define IPATH_SDMA_DISABLED  2
>     #define IPATH_SDMA_LAYERBUF  3
>     #define IPATH_SDMA_RUNNING  62
>     #define IPATH_SDMA_SHUTDOWN 63
> 
> I don't see that this status is shared with hardware, and I don't see
> why the RUNNING and SHUTDOWN bits need to be 62 and 63... converting to
> unsigned long and moving those to bits 4 and 5 seems like it might be a
> clean fix.
> 
> The other option is to convert to a bitmap and using the bitmap
> operations, which ends up being a bigger patch.
> 
> But since I don't really understand this part of the driver, some
> guidance would be helpful...
> 

Another option might be

-	u64			ipath_sdma_status;
+	unsigned long		ipath_sdma_status[64/BITS_PER_LONG];

Because the bitops are OK for use against an _array_ of unsigned longs, not
just a single unsigned long.

Or, if you want to preserve that u64:

	union {
		u64 ipath_sdma_status;
		unsigned long ipath_sdma_status_bits[64/BITS_PER_LONG];
	};

and do the bitops on ipath_sdma_status_bits.

Or just remove all the set_bit/clear_bit/etc and use plain old |, &, etc.


It all needs a bit of thought if you're supporting big-endian machines,
however.