Re: [wireless-next:master 169/199] drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c:2469:3: error: implicit declaration of function 'atomic_set_mask'

Re: [wireless-next:master 169/199] drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c:2469:3: error: implicit declaration of function 'atomic_set_mask'

[Arend van Spriel]

On 02/28/14 23:44, Florian Fainelli wrote:
> 2014-02-28 14:33 GMT-08:00 Arend van Spriel<arend@broadcom.com>:
>> On 02/28/2014 11:26 PM, Florian Fainelli wrote:
>>> 2014-02-28 14:22 GMT-08:00 Arend van Spriel<arend@broadcom.com>:
>>>> On 02/28/2014 11:15 PM, kbuild test robot wrote:
>>>>> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next.git master
>>>>> head:   3e3831c4fdc53aabf3a56419ef6d96a841c52435
>>>>> commit: c98db0bec72ac7ef127119c1ed962d6f56802b12 [169/199] brcmfmac: Use atomic functions for intstatus update.
>>>>> config: make ARCH=ia64 allmodconfig
>>>>>
>>>>> All error/warnings:
>>>>>
>>>>>     drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c: In function 'brcmf_sdio_intr_rstatus':
>>>>>>> drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c:2469:3: error: implicit declaration of function 'atomic_set_mask' [-Werror=implicit-function-declaration]
>>>>>     cc1: some warnings being treated as errors
>>>>
>>>> Hi Fengguang,
>>>>
>>>> So atomic_set_mask() is not a generally available function. I thought
>>>> that if the arch does not provide it, the asm-generic implementation is
>>>> used.
>>>
>>> Though so as well.
>>>
>>>> Also it seems the function prototype is not the same for all
>>>> archs. :-(
>>>
>>> This does not sound like it is expected, and this might need fixing,
>>> do you want to tackle this?
>>
>> I got email m68k and ia64 failing. Some implementations are in assembly.
>> It has been (counting...) over 20 years since I did some baby steps in
>> m68k assembly and do not really feel a strong pull to that dark side.
>
> This is supposed to be a generic function that everyone should be able
> to use without knowing the underlying details. Not quite sure why
> those two architectures are different, maybe we should ask Geert and
> Tony about this?

I was about to when I decided to do some more digging. So taking a 
closer look at include/asm-generic/atomic.h revealed this:

/*
  * Generic C implementation of atomic counter operations. Usable on
  * UP systems only. Do not include in machine independent code.

And further down:

#ifdef CONFIG_SMP
/* Force people to define core atomics */
# if !defined(atomic_add_return) || !defined(atomic_sub_return) || \
      !defined(atomic_clear_mask) || !defined(atomic_set_mask)
#  error "SMP requires a little arch-specific magic"
# endif
#endif

So in these SMP era architecture specific implementations seem required. 
IA64 does not provide it (so CONFIG_SMP not supported?) and m68k has 
different prototype. Apparently, the comment /* Force people to define 
core atomics */ did not fly for all archs (yet) :-p.

Regards,
Arend

Re: [wireless-next:master 169/199] drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c:2469:3: error: implicit declaration of function 'atomic_set_mask'

[Geert Uytterhoeven]

On Mon, Mar 3, 2014 at 11:53 AM, Arend van Spriel <arend@broadcom.com> wrote:
> On 02/28/14 23:44, Florian Fainelli wrote:
>> 2014-02-28 14:33 GMT-08:00 Arend van Spriel<arend@broadcom.com>:
>>> On 02/28/2014 11:26 PM, Florian Fainelli wrote:
>>>> 2014-02-28 14:22 GMT-08:00 Arend van Spriel<arend@broadcom.com>:
>>>>> On 02/28/2014 11:15 PM, kbuild test robot wrote:
>>>>>> tree:
>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next.git
>>>>>> master
>>>>>> head:   3e3831c4fdc53aabf3a56419ef6d96a841c52435
>>>>>> commit: c98db0bec72ac7ef127119c1ed962d6f56802b12 [169/199] brcmfmac:
>>>>>> Use atomic functions for intstatus update.
>>>>>> config: make ARCH=ia64 allmodconfig
>>>>>>
>>>>>> All error/warnings:
>>>>>>
>>>>>>     drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c: In function
>>>>>> 'brcmf_sdio_intr_rstatus':
>>>>>>>>
>>>>>>>> drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c:2469:3: error:
>>>>>>>> implicit declaration of function 'atomic_set_mask'
>>>>>>>> [-Werror=implicit-function-declaration]
>>>>>>
>>>>>>     cc1: some warnings being treated as errors
>>>>>
>>>>>
>>>>> Hi Fengguang,
>>>>>
>>>>> So atomic_set_mask() is not a generally available function. I thought
>>>>> that if the arch does not provide it, the asm-generic implementation is
>>>>> used.
>>>>
>>>>
>>>> Though so as well.
>>>>
>>>>> Also it seems the function prototype is not the same for all
>>>>> archs. :-(
>>>>
>>>>
>>>> This does not sound like it is expected, and this might need fixing,
>>>> do you want to tackle this?
>>>
>>>
>>> I got email m68k and ia64 failing. Some implementations are in assembly.
>>> It has been (counting...) over 20 years since I did some baby steps in
>>> m68k assembly and do not really feel a strong pull to that dark side.
>>
>>
>> This is supposed to be a generic function that everyone should be able
>> to use without knowing the underlying details. Not quite sure why
>> those two architectures are different, maybe we should ask Geert and
>> Tony about this?
>
>
> I was about to when I decided to do some more digging. So taking a closer
> look at include/asm-generic/atomic.h revealed this:
>
> /*
>  * Generic C implementation of atomic counter operations. Usable on
>  * UP systems only. Do not include in machine independent code.
>
> And further down:
>
> #ifdef CONFIG_SMP
> /* Force people to define core atomics */
> # if !defined(atomic_add_return) || !defined(atomic_sub_return) || \
>      !defined(atomic_clear_mask) || !defined(atomic_set_mask)
> #  error "SMP requires a little arch-specific magic"
> # endif
> #endif
>
> So in these SMP era architecture specific implementations seem required.
> IA64 does not provide it (so CONFIG_SMP not supported?) and m68k has
> different prototype. Apparently, the comment /* Force people to define core
> atomics */ did not fly for all archs (yet) :-p.

It's not just m68k, but other UP-only architectures like frv and mn10300
(these probably got derived from m68k).

Furthermore, in addition to the above, m32r uses "unsigned long" instead of
"unsigned int" for the mask.
Even asm-generic uses different mask types for atomic_clear_mask() and
atomic_set_mask().

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds