Message-ID: <87zgpb83uz.fsf@codeaurora.org>
Date:   Wed, 08 Dec 2021 18:21:08 +0200
From:   Kalle Valo <kvalo@...nel.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
        Luciano Coelho <luca@...lho.fi>,
        Loic Poulain <loic.poulain@...aro.org>
Subject: Re: pull-request: wireless-drivers-next-2021-12-07

Jakub Kicinski <kuba@...nel.org> writes:

> On Wed, 08 Dec 2021 10:00:15 +0200 Kalle Valo wrote:
>> Jakub Kicinski <kuba@...nel.org> writes:
>> 
>> > On Tue,  7 Dec 2021 14:42:11 +0000 (UTC) Kalle Valo wrote:  
>> >> here's a pull request to net-next tree, more info below. Please let me know if
>> >> there are any problems.  
>> >
>> > Pulled, thanks! Could you chase the appropriate people so that the new
>> > W=1 C=1 warnings get resolved before the merge window's here?
>> >
>> > https://patchwork.kernel.org/project/netdevbpf/patch/20211207144211.A9949C341C1@smtp.kernel.org/
>> 
>> Just so that I understand right, you are referring to this patchwork
>> test:
>> 
>>   Errors and warnings before: 111 this patch: 115
>> 
>>   https://patchwork.hopto.org/static/nipa/591659/12662005/build_32bit/
>> 
>> And you want the four new warnings to be fixed? That can be quite time
>> consuming, to be honest I would rather revert the commits than using a
>> lot of my time trying to get people fix the warnings. Is there an easy
>> way to find what are the new warnings?
>
> Yeah, scroll down, there is a diff of the old warnings vs new ones, and
> a summary of which files have changed their warning count:
>
> +      2 ../drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +      3 ../drivers/net/wireless/intel/iwlwifi/mei/main.c
> -      1 ../drivers/net/wireless/intel/iwlwifi/mvm/ops.c
> +      2 ../drivers/net/wireless/intel/iwlwifi/mvm/ops.c
> -      2 ../drivers/net/wireless/microchip/wilc1000/wlan.c

Ah, that makes it easier.

> So presumably these are the warnings that were added:
>
> drivers/net/wireless/intel/iwlwifi/mei/main.c:193: warning: cannot
> understand function prototype: 'struct '
> drivers/net/wireless/intel/iwlwifi/mei/main.c:1784: warning: Function
> parameter or member 'cldev' not described in 'iwl_mei_probe'
> drivers/net/wireless/intel/iwlwifi/mei/main.c:1784: warning: Function
> parameter or member 'id' not described in 'iwl_mei_probe'

Luca, please take a look and send a patch. I'll then apply it directly
to wireless-drivers-next.

> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:3911:28:
> warning: incorrect type in assignment (different base types)
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:3911:28:
> expected restricted __le32 [assigned] [usertype] period_msec
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:3911:28:
> got restricted __le16 [usertype]
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:3913:30:
> warning: incorrect type in assignment (different base types)
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:3913:30:
> expected unsigned char [assigned] [usertype] keep_alive_id
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:3913:30:
> got restricted __le16 [usertype]

Loic, your patch should fix these, right?

https://patchwork.kernel.org/project/linux-wireless/patch/1638953708-29192-1-git-send-email-loic.poulain@linaro.org/

> drivers/net/wireless/intel/iwlwifi/mvm/ops.c:684:12: warning: context
> imbalance in 'iwl_mvm_start_get_nvm' - wrong count at exit

Luca, please also take a look at this.

>> But in the big picture are you saying the net trees now have a rule that
>> no new W=1 and C=1 warnings are allowed? I do test ath10k and ath11k
>> drivers for W=1 and C=1 warnings, but all other drivers are on their own
>> in this regard. At the moment I have no tooling in place to check all
>> wireless drivers.
>
> For the code we merge directly we try to make sure there are no new
> warnings. I realize it's quite a bit of work for larger trees unless 
> you have the infra so not a hard requirement (for you).

Yeah, at the moment I really would not be able to catch W=1 or sparse
warnings :/ And fixing them afterwards is just too slow. But if we would
be able to fix all the warnings in drivers/net/wireless then it would be
easy for me to enable W=1 and C=1 in my own build tests.

> FWIW the build bot we wrote is available on GH:
>
> https://github.com/kuba-moo/nipa
>
> But it currently hard codes tree matching logic for bpf and netdev,
> so would probably take a few hours to adopt it.

Thanks, it would good to have a similar system for wireless trees.
Anyone want to help? :)

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Powered by blists - more mailing lists