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
