Re: [PATCH 10/14] brcmfmac: Use atomic functions for intstatus update.

[Arend van Spriel <arend@broadcom.com>]

On 02/26/2014 12:38 AM, Florian Fainelli wrote:
> Hi Arend,
> 
> 2014-02-25 11:30 GMT-08:00 Arend van Spriel <arend@broadcom.com>:
>> From: Hante Meuleman <meuleman@broadcom.com>
>>
>> The intstatus in sdio code can be updated from different
>> threads. To protect intstatus access, atomic functions are
>> used. One of them is set_bit, but this function is not
>> guaranteed atomic on all platforms. The loop was replaced
>> with local created OR function.
>>
>> Reviewed-by: Arend Van Spriel <arend@broadcom.com>
>> Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
>> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
>> Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
>> Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
>> Signed-off-by: Arend van Spriel <arend@broadcom.com>
>> ---
>>  drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |   29 ++++++++++----------
>>  1 file changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> index ac61419..90ff406 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> @@ -2444,12 +2444,21 @@ static inline void brcmf_sdio_clrintr(struct brcmf_sdio *bus)
>>         }
>>  }
>>
>> +static void atomic_orr(int val, atomic_t *v)
>> +{
>> +       int old_val;
>> +
>> +       old_val = atomic_read(v);
>> +       while (atomic_cmpxchg(v, old_val, val | old_val) != old_val)
>> +               old_val = atomic_read(v);
>> +}
> 
> Is not atomic_set_mask() doing the same thing?

You are right. Hante thought that one was not supported on all archs,
but asm-generic provides a fallback for those. I will revise this patch.

Thanks,
Arend