ssb: Make ssb_wait_bit multi-bit safe

ssb_wait_bit was designed for only one-bit bitmasks. People start using it for multi-bit bitmasks. Make the "set" case is safe for this. The "unset" case is already safe.

2011/2/15 Michael B?sch <[email protected]>:

Quoted 3 lines

ssb_wait_bit was designed for only one-bit bitmasks. People start using it for multi-bit bitmasks. Make the "set" case is safe for this. The "unset" case is already safe.

Thanks :)

Quoted 5 lines

2011/2/15 Michael B?sch <[email protected]>:

Quoted 3 lines

ssb_wait_bit was designed for only one-bit bitmasks. People start using it for multi-bit bitmasks. Make the "set" case is safe for this. The "unset" case is already safe.

Quoted 1 line

Thanks :)

This should not change the behavior of your patch, though, as you only add an "unset" case, which was already correct.

So this doesn't fix an actual bug but rather hardens the code for future use.

ssb_wait_bit was designed for only one-bit bitmasks. People start using it for multi-bit bitmasks. Make the "set" case is safe for this. The "unset" case is already safe.

This does not change behavior of the current code.

Signed-off-by Michael Buesch <[email protected]>

---

Index: wireless-testing/drivers/ssb/main.c
===================================================================

drivers/ssb/main.c 2011-02-15 00:17:35.727816704 +0100 renamed from drivers/ssb/main.c ? ?2010-12-30 15:49:39.235946210 +0100 +1 -1

@@ -1192,10 +1192,10 @@ void ssb_device_enable(struct ssb_device
 ?}
 ?EXPORT_SYMBOL(ssb_device_enable);
 
-/* Wait for a bit in a register to get set or unset.
+/* Wait for bitmask in a register to get set or cleared.

?* timeout is in units of ten-microseconds */

?{
? ? ? ?int i;
? ? ? ?u32 val;

drivers/ssb/main.c 2011-02-15 00:17:35.727816704 +0100 renamed from drivers/ssb/main.c ? ?2010-12-30 15:49:39.235946210 +0100 +3 -3

@@ -1203,7 +1203,7 @@ static int ssb_wait_bit(struct ssb_devic
 ? ? ? ?for (i = 0; i < timeout; i++) {
 ? ? ? ? ? ? ? ?val = ssb_read32(dev, reg);
 ? ? ? ? ? ? ? ?if (set) {
- ? ? ? ? ? ? ? ? ? ? ? if (val & bitmask)
+ ? ? ? ? ? ? ? ? ? ? ? if ((val & bitmask) == bitmask)
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?return 0;
 ? ? ? ? ? ? ? ?} else {
 ? ? ? ? ? ? ? ? ? ? ? ?if (!(val & bitmask))

@@ -1227,8 +1227,8 @@ void ssb_device_disable(struct ssb_devic
 
 ? ? ? ?reject = ssb_tmslow_reject_bitmask(dev);
 ? ? ? ?ssb_write32(dev, SSB_TMSLOW, reject | SSB_TMSLOW_CLOCK);
- ? ? ? ssb_wait_bit(dev, SSB_TMSLOW, reject, 1000, 1);
- ? ? ? ssb_wait_bit(dev, SSB_TMSHIGH, SSB_TMSHIGH_BUSY, 1000, 0);
+ ? ? ? ssb_wait_bits(dev, SSB_TMSLOW, reject, 1000, 1);
+ ? ? ? ssb_wait_bits(dev, SSB_TMSHIGH, SSB_TMSHIGH_BUSY, 1000, 0);
 ? ? ? ?ssb_write32(dev, SSB_TMSLOW,

? ? ? ? ? ? ? ? ? ?SSB_TMSLOW_FGC | SSB_TMSLOW_CLOCK | ? ? ? ? ? ? ? ? ? ?reject | SSB_TMSLOW_RESET |

--
Greetings Michael.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

drivers/ssb/main.c void ssb_device_enable(struct ssb_device · line 1198

Quoted 4 lines

drivers/ssb/main.c +2 -2

-static int ssb_wait_bit(struct ssb_device *dev, u16 reg, u32 bitmask,
-                       int timeout, int set)
+static int ssb_wait_bits(struct ssb_device *dev, u16 reg, u32 bitmask,
+                        int timeout, int set)

While you are at it, you may want to change this to "bool set".