[PATCH] [RFC] staging: rtl8821ae: fix invalid bit mask on MSR_AP check

[PATCH] [RFC] staging: rtl8821ae: fix invalid bit mask on MSR_AP check

[Levente Kurusa]

Since MSR_AP is 0x3, ANDing it with 0xFC will never be true.
Add a NOT operation to 0xFC so that we will AND with the last
three bits which will result in a possibility that the condition
will succeed.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 Hi,

 This might not be the real solution since it is also possible that
 the particular condition never really happens. I seek comments
 so that I can post the proper patch.

 Thanks!
 
 drivers/staging/rtl8821ae/rtl8821ae/hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8821ae/rtl8821ae/hw.c b/drivers/staging/rtl8821ae/rtl8821ae/hw.c
index 5ed7a11..f204b31 100644
--- a/drivers/staging/rtl8821ae/rtl8821ae/hw.c
+++ b/drivers/staging/rtl8821ae/rtl8821ae/hw.c
@@ -1622,7 +1622,7 @@ static int _rtl8821ae_set_media_status(struct ieee80211_hw *hw,
 
 	rtl_write_byte(rtlpriv, (MSR), bt_msr);
 	rtlpriv->cfg->ops->led_control(hw, ledaction);
-	if ((bt_msr & 0xfc) == MSR_AP)
+	if ((bt_msr & ~0xfc) == MSR_AP)
 		rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x00);
 	else
 		rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x66);
-- 
1.7.9.5

Re: [PATCH] [RFC] staging: rtl8821ae: fix invalid bit mask on MSR_AP check

[Dan Carpenter]

On Fri, Feb 14, 2014 at 10:50:23PM +0100, Levente Kurusa wrote:
> Since MSR_AP is 0x3, ANDing it with 0xFC will never be true.
> Add a NOT operation to 0xFC so that we will AND with the last
> three bits which will result in a possibility that the condition
> will succeed.
> 
> Signed-off-by: Levente Kurusa <levex@linux.com>
> ---
>  Hi,
> 
>  This might not be the real solution since it is also possible that
>  the particular condition never really happens. I seek comments
>  so that I can post the proper patch.
> 

I can't test it either but I think your fix is correct.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

Re: [PATCH] [RFC] staging: rtl8821ae: fix invalid bit mask on MSR_AP check

[Levente Kurusa]

On 02/14/2014 11:00 PM, Dan Carpenter wrote:
> [...]
>>  Hi,
>>
>>  This might not be the real solution since it is also possible that
>>  the particular condition never really happens. I seek comments
>>  so that I can post the proper patch.
>>
> 
> I can't test it either but I think your fix is correct.
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> [...]

Thanks Dan, maybe you know some people who could test it?
RTLXXXXXX guys? Or maybe we can take the patch as is?
I cannot really think of any other solution other than removing
the other clause, but since that was written to the file,
there must have been some logic behind that. I am slightly
disappointed get_maintainer didn't really find anybody for
this patch...

Greg, can you take it as is or you would need the RFC tag removed?
(ie, repost)
Maybe you know somebody who could test it as well?

-- 
Regards,
Levente Kurusa

Re: [PATCH] [RFC] staging: rtl8821ae: fix invalid bit mask on MSR_AP check

[Dan Carpenter]

On Sat, Feb 15, 2014 at 08:53:34AM +0100, Levente Kurusa wrote:
> Thanks Dan, maybe you know some people who could test it?
> RTLXXXXXX guys? Or maybe we can take the patch as is?
> I cannot really think of any other solution other than removing
> the other clause, but since that was written to the file,
> there must have been some logic behind that. I am slightly
> disappointed get_maintainer didn't really find anybody for
> this patch...

This driver is going to be deleted in the next couple weeks and replaced
with a re-written from scratch driver.

Your fix is correct.  It's just that no one tested the AP code for this.
I would feel comfortable applying it as-is if no one is available to
test it.

regards,
dan carpenter

Re: [PATCH] [RFC] staging: rtl8821ae: fix invalid bit mask on MSR_AP check

[Levente Kurusa]

On 02/15/2014 11:36 AM, Dan Carpenter wrote:
> On Sat, Feb 15, 2014 at 08:53:34AM +0100, Levente Kurusa wrote:
>> Thanks Dan, maybe you know some people who could test it?
>> RTLXXXXXX guys? Or maybe we can take the patch as is?
>> I cannot really think of any other solution other than removing
>> the other clause, but since that was written to the file,
>> there must have been some logic behind that. I am slightly
>> disappointed get_maintainer didn't really find anybody for
>> this patch...
> 
> This driver is going to be deleted in the next couple weeks and replaced
> with a re-written from scratch driver.
> 

Oh, I did not know that. Since this fix isn't that big, I think it won't
get into the next -rc, and hence might not hit Linus' tree before
the new driver gets into staging. I am not sure if it is worth applying
if that is the case.

-- 
Regards,
Levente Kurusa