Christian Lamparter <chunkeey@xxxxxxxxx> writes: > On Tuesday, June 18, 2019 2:11:53 PM CEST Kalle Valo wrote: >> Christian Lamparter <chunkeey@xxxxxxxxx> writes: >> > On Monday, June 10, 2019 9:06:30 AM CEST Kalle Valo wrote: >> >> Christian Lamparter <chunkeey@xxxxxxxxx> writes: >> >> >> >> > This patch fixes a noisy warning triggered by -Wenum-compare >> >> > >> >> > |main.c:1390:31: warning: comparison between ‘enum nl80211_ac’ >> >> > | and ‘enum ar9170_txq’ [-Wenum-compare] >> >> > | BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ); >> >> > | ^ >> >> > | [...] >> >> > >> >> > This is a little bit unfortunate, since the number of queues >> >> > (hence NL80211_NUM_ACS) is a constant based on the IEEE 802.11 >> >> > (much like IEEE80211_NUM_ACS) and __AR9170_NUM_TXQ is more or >> >> > less defined by the AR9170 hardware. >> >> >> >> Is the warning enabled by default? TBH I'm not seeing how useful this >> >> warning is for kernel development. >> > >> > It is included in the "-Wall" (which is coming from "KBUILD_CFLAGS" >> > in the main Makefile). >> > >> > I tried debian's gcc starting from 4.6 to the lastest 8.3. They all >> > complain about it in various degrees. >> > >> > https://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Warning-Options.html#Warning-Options >> >> Ok, odd that I haven't noticed this warning. Maybe I have been just >> blind. > > Sorry. No, I messed up there. I made a patch back in the day during the spectre > mac80211 patches that I kept in my tree for the driver. But I now remember that > I never published those because of that exact "warning". > These lines are coming from there. > >> >> > --- a/drivers/net/wireless/ath/carl9170/main.c >> >> > +++ b/drivers/net/wireless/ath/carl9170/main.c >> >> > @@ -1387,7 +1387,7 @@ static int carl9170_op_conf_tx(struct ieee80211_hw *hw, >> >> > int ret; >> >> > >> >> > BUILD_BUG_ON(ARRAY_SIZE(ar9170_qmap) != __AR9170_NUM_TXQ); >> >> > - BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ); >> >> > + BUILD_BUG_ON((size_t)NL80211_NUM_ACS > (size_t)__AR9170_NUM_TXQ); >> >> >> >> IMHO this just makes the code worse. Does it make sense to workaround >> >> (stupid) compiler warnings like this? >> > >> > True. What's worse: This isn't really code. The BUILD_BUG_ON Macro is there >> > to guard but it's getting compiled away. I could also just drop it. >> >> Either way is fine for me. If the GCC (by default) emits a warning for >> this we need to silence that warning, so in that respect I guess we >> should apply this. Unless better solutions come up, of course :) > > Note: I dropped this patch from patchwork. So, there's nothing that > needs to be done now ;) Thanks, except I would prefer that I drop the patch myself :) In numerous cases I have been wondering there a patch has disappeared and only after a while I realise the submitter deleted the patch altogether (which also destroys the conversation from patchwork etc). IMHO that's really bad UX in patchwork, I should file a bug report about that. So in general it's enough to say "Kalle, please drop the patch" and I'll do the rest. > Well, except OT: Would you mind adding the QCA4019 boardfiles that > have accumulated over the past six-ish months? Yeah, those are in my queue. But I'll first want to implement a script so that I don't need to manually add all of them and I just haven't found the time for that. -- Kalle Valo
