Search Linux Wireless

Re: [PATCH 08/30] net: wireless: ath: carl9170: Mark 'ar9170_qmap' as __maybe_unused

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020-08-17 14:59, Kalle Valo wrote:
Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> writes:

On 14/08/2020 17.14, Christian Lamparter wrote:
On 2020-08-14 13:39, Lee Jones wrote:
'ar9170_qmap' is used in some source files which include carl9170.h,
but not all of them.  Mark it as __maybe_unused to show that this is
not only okay, it's expected.

Fixes the following W=1 kernel build warning(s)

Is this W=1 really a "must" requirement? I find it strange having
__maybe_unused in header files as this "suggests" that the
definition is redundant.

In this case it seems one could replace the table lookup with a

static inline u8 ar9170_qmap(u8 idx) { return 3 - idx; }

gcc doesn't warn about unused static inline functions (or one would have
a million warnings to deal with). Just my $0.02.

Yeah, this is much better.

Yes, this is much better than just adding __maybe_unused :).

To be on the safe side (and to get rid of a & in tx.c:666), the "3 - idx" should be something like "return (3 - idx) & CARL9170_TX_STATUS_QUEUE". I think its also possible to just use clamp_t (or min_t since the u8 has no negative values) or make use of a switch statement [analogues what was done in ath10k commit: 91493e8e10 "ath10k: fix recent bandwidth conversion bug" (just to be clear. Yes this ath10k commit has nothing to do with queues, but it is a nice, atomic switch-case static inline function example).]

And I think that static variables should not even be in the header
files. Doesn't it mean that there's a local copy of the variable
everytime the .h file is included? Sure, in this case the overhead is
small (4 bytes per include) but still it's wrong.
Seems to be "sort of". I compiled both, the current vanilla carl9170 and
with Lee Jones' patch on Debian's current gcc 10.2.0 (Debian 10.2.0-5)
and gnu ld 2.35.

The ar9170_qmap symbol is only present in the object file if ar9170_qmap is used by the source. In the final module file (carl9170.ko), there are two ar9170_qmap symboles in the module's .rodata section (one is coming from main.o code and the other from tx.o).

(The use of __maybe_unused didn't make any difference. Same overall section and file sizes).

Cheers,
Christian



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux