Re: [PATCH 12/30] net: wireless: cisco: airo: Fix a myriad of coding style issues

From: Kalle Valo
Date: Mon Aug 17 2020 - 09:27:57 EST


Lee Jones <lee.jones@xxxxxxxxxx> writes:

> On Fri, 14 Aug 2020, Kalle Valo wrote:
>
>> Lee Jones <lee.jones@xxxxxxxxxx> writes:
>>
>> > - Ensure spaces appear after {for, if, while, etc}
>> > - Ensure spaces to not appear after '('
>> > - Ensure spaces to not appear before ')'
>> > - Ensure spaces appear between ')' and '{'
>> > - Ensure spaces appear after ','
>> > - Ensure spaces do not appear before ','
>> > - Ensure spaces appear either side of '='
>> > - Ensure '{'s which open functions are on a new line
>> > - Remove trailing whitespace
>> >
>> > There are still a whole host of issues with this file, but this
>> > patch certainly breaks the back of them.
>> >
>> > Cc: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
>> > Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
>> > Cc: Jakub Kicinski <kuba@xxxxxxxxxx>
>> > Cc: Benjamin Reed <breed@xxxxxxxxxxxxxxxxxxxxx>
>> > Cc: Javier Achirica <achirica@xxxxxxxxxxxxxxxxxxxxx>
>> > Cc: Jean Tourrilhes <jt@xxxxxxxxxx>
>> > Cc: Fabrice Bellet <fabrice@xxxxxxxxxxx>
>> > Cc: linux-wireless@xxxxxxxxxxxxxxx
>> > Cc: netdev@xxxxxxxxxxxxxxx
>> > Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx>
>> > ---
>> > drivers/net/wireless/cisco/airo.c | 897 ++++++++++++++++--------------
>> > 1 file changed, 467 insertions(+), 430 deletions(-)
>>
>> This is a driver for ancient hardware, I'm not sure if it's worth trying
>> to fix any style issues. Is anyone even using it? Should we instead just
>> remove the driver?
>
> Sounds like a reasonable solution to me.
>
> I'm also happy to do it, if there are no objections.
>
> As it stands, it's polluting the code-base and the build-log, so
> something should be done.

I tried to find some comments about the driver and here's one successful
report from 2013:

https://martybugs.net/wireless/aironet4800.cgi

And here's one commit from 2015 where Ondrej (CCed) was also testing the
driver:

----------------------------------------------------------------------
commit dae0412d0caa4948da07fe4ad91352b5b61a70ec
Author: Ondrej Zary <linux@xxxxxxxxxxxxxxxxxxxx>
AuthorDate: Fri Oct 16 21:04:14 2015 +0200
Commit: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
CommitDate: Wed Oct 28 20:54:39 2015 +0200

airo: fix scan after SIOCSIWAP (airo_set_wap)

SIOCSIWAP (airo_set_wap) affects scan: only the AP specified by
SIOCSIWAP is present in scan results.

This makes NetworkManager work for the first time but then unable to
find any other APs.

Clear APList before starting scan and set it back after scan completes
to work-around the problem.

To avoid losing packets during scan, modify disable_MAC() to omit
netif_carrier_off() call when lock == 2.

Signed-off-by: Ondrej Zary <linux@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
----------------------------------------------------------------------

I was surprised to see that someone was using this driver in 2015, so
I'm not sure anymore what to do. Of course we could still just remove it
and later revert if someone steps up and claims the driver is still
usable. Hmm. Does anyone any users of this driver?

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