wifi: b43legacy: clean up some inconsistent indentings

[PATCH net-next] wifi: b43legacy: clean up some inconsistent indentings

Posted by Yang Li 1 year, 12 months ago

drivers/net/wireless/broadcom/b43legacy/dma.c:178 priority_to_txring() warn: inconsistent indenting

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7783
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
---
 drivers/net/wireless/broadcom/b43legacy/dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
index 60e41de72f29..ea73c594d846 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -174,8 +174,8 @@ static struct b43legacy_dmaring *priority_to_txring(
 {
 	struct b43legacy_dmaring *ring;
 
-/*FIXME: For now we always run on TX-ring-1 */
-return dev->dma.tx_ring1;
+	/*FIXME: For now we always run on TX-ring-1 */
+	return dev->dma.tx_ring1;
 
 	/* 0 = highest priority */
 	switch (queue_priority) {
-- 
2.20.1.7.g153144c

Re: [PATCH net-next] wifi: b43legacy: clean up some inconsistent indentings

Posted by Michael Büsch 1 year, 12 months ago

On Wed, 20 Dec 2023 09:12:09 +0800
Yang Li <yang.lee@linux.alibaba.com> wrote:

> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7783

This link is not publicly accessible.

> a/drivers/net/wireless/broadcom/b43legacy/dma.c +++
> b/drivers/net/wireless/broadcom/b43legacy/dma.c @@ -174,8 +174,8 @@
> static struct b43legacy_dmaring *priority_to_txring( {
>  	struct b43legacy_dmaring *ring;
>  
> -/*FIXME: For now we always run on TX-ring-1 */
> -return dev->dma.tx_ring1;
> +	/*FIXME: For now we always run on TX-ring-1 */
> +	return dev->dma.tx_ring1;
>  
>  	/* 0 = highest priority */
>  	switch (queue_priority) {

Thanks for your patch.

But actually, I am kind of annoyed by the constant stream of whitespace
fixing and dead code removal and other trivial changes to this legacy
driver.

It does not improve the code to add two tabs to this _ancient_ code.

And I can already see the next patch coming that removes the dead code
after this FIXME return. And then the next patch will come to remove
this function altogether, and so on and so on.

This driver has a _lot_ of such code, because it is based on reverse
engineered knowledge with many many unknowns.

IMO this just creates additional maintenance work and pressure on our
maintainers for no good reason.

-- 
Michael Büsch
https://bues.ch/

wireless: cleanup patches

Posted by Kalle Valo 1 year, 11 months ago

Michael Büsch <m@bues.ch> writes:

> On Wed, 20 Dec 2023 09:12:09 +0800
> Yang Li <yang.lee@linux.alibaba.com> wrote:
>
>> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7783
>
> This link is not publicly accessible.
>
>> a/drivers/net/wireless/broadcom/b43legacy/dma.c +++
>> b/drivers/net/wireless/broadcom/b43legacy/dma.c @@ -174,8 +174,8 @@
>> static struct b43legacy_dmaring *priority_to_txring( {
>>  	struct b43legacy_dmaring *ring;
>>  
>> -/*FIXME: For now we always run on TX-ring-1 */
>> -return dev->dma.tx_ring1;
>> +	/*FIXME: For now we always run on TX-ring-1 */
>> +	return dev->dma.tx_ring1;
>>  
>>  	/* 0 = highest priority */
>>  	switch (queue_priority) {
>
> Thanks for your patch.
>
> But actually, I am kind of annoyed by the constant stream of whitespace
> fixing and dead code removal and other trivial changes to this legacy
> driver.
>
> It does not improve the code to add two tabs to this _ancient_ code.
>
> And I can already see the next patch coming that removes the dead code
> after this FIXME return. And then the next patch will come to remove
> this function altogether, and so on and so on.
>
> This driver has a _lot_ of such code, because it is based on reverse
> engineered knowledge with many many unknowns.
>
> IMO this just creates additional maintenance work and pressure on our
> maintainers for no good reason.

Yeah, the cleanup patches are a problem. Even more so that there can be
people who deliberately try to submit compromised code:

https://lore.kernel.org/lkml/202105051005.49BFABCE@keescook/

brtfs has a pretty good summary about their feelings towards cleanup
patches:

https://btrfs.readthedocs.io/en/latest/dev/Developer-s-FAQ.html#how-not-to-start

Johannes and me have been talking that we should write something similar
for wireless. Maybe we should start by adding that link to our
documentation :)

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

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

Re: [PATCH net-next] wifi: b43legacy: clean up some inconsistent indentings

Posted by Kalle Valo 1 year, 12 months ago

Yang Li <yang.lee@linux.alibaba.com> writes:

> drivers/net/wireless/broadcom/b43legacy/dma.c:178 priority_to_txring() warn: inconsistent indenting
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7783
> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>

wifi patches go to wireless-next, not net-next. No need to resend
because of this.

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

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