* [PATCH] ssb: Make ssb_wait_bit multi-bit safe
@ 2011-02-14 23:21 Michael Büsch
2011-02-14 23:44 ` Rafał Miłecki
2011-02-15 0:03 ` Gábor Stefanik
0 siblings, 2 replies; 6+ messages in thread
From: Michael Büsch @ 2011-02-14 23:21 UTC (permalink / raw)
To: John Linville; +Cc: Rafał Miłecki, b43-dev, linux-wireless
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 <mb@bu3sch.de>
---
Index: wireless-testing/drivers/ssb/main.c
===================================================================
--- wireless-testing.orig/drivers/ssb/main.c 2010-12-30 15:49:39.235946210 +0100
+++ wireless-testing/drivers/ssb/main.c 2011-02-15 00:17:35.727816704 +0100
@@ -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 */
-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)
{
int i;
u32 val;
@@ -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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ssb: Make ssb_wait_bit multi-bit safe
2011-02-14 23:21 [PATCH] ssb: Make ssb_wait_bit multi-bit safe Michael Büsch
@ 2011-02-14 23:44 ` Rafał Miłecki
2011-02-14 23:59 ` Michael Büsch
2011-02-15 0:03 ` Gábor Stefanik
1 sibling, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2011-02-14 23:44 UTC (permalink / raw)
To: Michael Büsch; +Cc: John Linville, b43-dev, linux-wireless
2011/2/15 Michael Büsch <mb@bu3sch.de>:
> 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 :)
--
Rafał
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ssb: Make ssb_wait_bit multi-bit safe
2011-02-14 23:44 ` Rafał Miłecki
@ 2011-02-14 23:59 ` Michael Büsch
2011-02-15 0:04 ` Rafał Miłecki
0 siblings, 1 reply; 6+ messages in thread
From: Michael Büsch @ 2011-02-14 23:59 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: John Linville, b43-dev, linux-wireless
On Tue, 2011-02-15 at 00:44 +0100, Rafał Miłecki wrote:
> 2011/2/15 Michael Büsch <mb@bu3sch.de>:
> > 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 :)
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.
--
Greetings Michael.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ssb: Make ssb_wait_bit multi-bit safe
2011-02-14 23:59 ` Michael Büsch
@ 2011-02-15 0:04 ` Rafał Miłecki
0 siblings, 0 replies; 6+ messages in thread
From: Rafał Miłecki @ 2011-02-15 0:04 UTC (permalink / raw)
To: Michael Büsch; +Cc: John Linville, b43-dev, linux-wireless
W dniu 15 lutego 2011 00:59 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
> On Tue, 2011-02-15 at 00:44 +0100, Rafał Miłecki wrote:
>> 2011/2/15 Michael Büsch <mb@bu3sch.de>:
>> > 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 :)
>
> 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.
Yes, I can see that :)
--
Rafał
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ssb: Make ssb_wait_bit multi-bit safe
2011-02-14 23:21 [PATCH] ssb: Make ssb_wait_bit multi-bit safe Michael Büsch
2011-02-14 23:44 ` Rafał Miłecki
@ 2011-02-15 0:03 ` Gábor Stefanik
2011-02-15 0:20 ` Michael Büsch
1 sibling, 1 reply; 6+ messages in thread
From: Gábor Stefanik @ 2011-02-15 0:03 UTC (permalink / raw)
To: Michael Büsch
Cc: John Linville, Rafał Miłecki, b43-dev, linux-wireless
On Tue, Feb 15, 2011 at 12:21 AM, Michael Büsch <mb@bu3sch.de> wrote:
> 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 <mb@bu3sch.de>
>
> ---
>
> Index: wireless-testing/drivers/ssb/main.c
> ===================================================================
> --- wireless-testing.orig/drivers/ssb/main.c 2010-12-30 15:49:39.235946210 +0100
> +++ wireless-testing/drivers/ssb/main.c 2011-02-15 00:17:35.727816704 +0100
> @@ -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 */
> -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".
> {
> int i;
> u32 val;
> @@ -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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ssb: Make ssb_wait_bit multi-bit safe
2011-02-15 0:03 ` Gábor Stefanik
@ 2011-02-15 0:20 ` Michael Büsch
0 siblings, 0 replies; 6+ messages in thread
From: Michael Büsch @ 2011-02-15 0:20 UTC (permalink / raw)
To: Gábor Stefanik
Cc: John Linville, Rafał Miłecki, b43-dev, linux-wireless
On Tue, 2011-02-15 at 01:03 +0100, Gábor Stefanik wrote:
> > -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".
Please feel free to submit a separate patch on top of this.
--
Greetings Michael.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-02-15 0:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-14 23:21 [PATCH] ssb: Make ssb_wait_bit multi-bit safe Michael Büsch
2011-02-14 23:44 ` Rafał Miłecki
2011-02-14 23:59 ` Michael Büsch
2011-02-15 0:04 ` Rafał Miłecki
2011-02-15 0:03 ` Gábor Stefanik
2011-02-15 0:20 ` Michael Büsch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).