From: Kalle Valo <kvalo@kernel.org>
To: "Michael Büsch" <m@bues.ch>
Cc: Yang Li <yang.lee@linux.alibaba.com>,
Larry.Finger@lwfinger.net, linux-wireless@vger.kernel.org,
b43-dev@lists.infradead.org, linux-kernel@vger.kernel.org,
Abaci Robot <abaci@linux.alibaba.com>
Subject: wireless: cleanup patches
Date: Sat, 13 Jan 2024 18:46:32 +0200 [thread overview]
Message-ID: <87mst989lj.fsf_-_@kernel.org> (raw)
In-Reply-To: <20231220102307.7dd1f187@barney> ("Michael Büsch"'s message of "Wed, 20 Dec 2023 10:23:07 +0100")
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
_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev
WARNING: multiple messages have this Message-ID (diff)
From: Kalle Valo <kvalo@kernel.org>
To: "Michael Büsch" <m@bues.ch>
Cc: Yang Li <yang.lee@linux.alibaba.com>,
Larry.Finger@lwfinger.net, linux-wireless@vger.kernel.org,
b43-dev@lists.infradead.org, linux-kernel@vger.kernel.org,
Abaci Robot <abaci@linux.alibaba.com>
Subject: wireless: cleanup patches
Date: Sat, 13 Jan 2024 18:46:32 +0200 [thread overview]
Message-ID: <87mst989lj.fsf_-_@kernel.org> (raw)
In-Reply-To: <20231220102307.7dd1f187@barney> ("Michael Büsch"'s message of "Wed, 20 Dec 2023 10:23:07 +0100")
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
next prev parent reply other threads:[~2024-01-13 16:47 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-20 1:12 [PATCH net-next] wifi: b43legacy: clean up some inconsistent indentings Yang Li
2023-12-20 1:12 ` Yang Li
2023-12-20 5:14 ` Kalle Valo
2023-12-20 5:14 ` Kalle Valo
2023-12-20 9:23 ` Michael Büsch
2023-12-20 9:23 ` Michael Büsch
2024-01-13 16:46 ` Kalle Valo [this message]
2024-01-13 16:46 ` wireless: cleanup patches Kalle Valo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87mst989lj.fsf_-_@kernel.org \
--to=kvalo@kernel.org \
--cc=Larry.Finger@lwfinger.net \
--cc=abaci@linux.alibaba.com \
--cc=b43-dev@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=m@bues.ch \
--cc=yang.lee@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.