[PATCH 07/30] net: wireless: broadcom: b43: main: Add braces around empty statements

[PATCH 07/30] net: wireless: broadcom: b43: main: Add braces around empty statements

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/net/wireless/broadcom/b43/main.c: In function ?b43_dummy_transmission?:
 drivers/net/wireless/broadcom/b43/main.c:785:3: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]
 drivers/net/wireless/broadcom/b43/main.c: In function ?b43_do_interrupt_thread?:
 drivers/net/wireless/broadcom/b43/main.c:2017:3: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]

Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Martin Langer <martin-langer@gmx.de>
Cc: Stefano Brivio <stefano.brivio@polimi.it>
Cc: Michael Buesch <m@bues.ch>
Cc: van Dyk <kugelfang@gentoo.org>
Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
Cc: Albert Herranz <albert_herranz@yahoo.es>
Cc: linux-wireless at vger.kernel.org
Cc: b43-dev at lists.infradead.org
Cc: netdev at vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/net/wireless/broadcom/b43/main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c
index a54dd4f7fa54a..2a29fa69f692c 100644
--- a/drivers/net/wireless/broadcom/b43/main.c
+++ b/drivers/net/wireless/broadcom/b43/main.c
@@ -781,8 +781,9 @@ void b43_dummy_transmission(struct b43_wldev *dev, bool ofdm, bool pa_on)
 	b43_write16(dev, B43_MMIO_XMTSEL, 0x0826);
 	b43_write16(dev, B43_MMIO_TXE0_CTL, 0x0000);
 
-	if (!pa_on && phy->type == B43_PHYTYPE_N)
+	if (!pa_on && phy->type == B43_PHYTYPE_N) {
 		; /*b43_nphy_pa_override(dev, false) */
+	}
 
 	switch (phy->type) {
 	case B43_PHYTYPE_N:
@@ -2013,8 +2014,9 @@ static void b43_do_interrupt_thread(struct b43_wldev *dev)
 		handle_irq_beacon(dev);
 	if (reason & B43_IRQ_PMQ)
 		handle_irq_pmq(dev);
-	if (reason & B43_IRQ_TXFIFO_FLUSH_OK)
+	if (reason & B43_IRQ_TXFIFO_FLUSH_OK) {
 		;/* TODO */
+	}
 	if (reason & B43_IRQ_NOISESAMPLE_OK)
 		handle_irq_noise(dev);
 
-- 
2.25.1

[PATCH 17/30] net: wireless: broadcom: b43legacy: main: Provide braces around empty 'if' body

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/net/wireless/broadcom/b43legacy/main.c: In function ?b43legacy_interrupt_tasklet?:
 drivers/net/wireless/broadcom/b43legacy/main.c:1344:3: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]

Cc: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Martin Langer <martin-langer@gmx.de>
Cc: Stefano Brivio <stefano.brivio@polimi.it>
Cc: Michael Buesch <m@bues.ch>
Cc: van Dyk <kugelfang@gentoo.org>
Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
Cc: linux-wireless at vger.kernel.org
Cc: b43-dev at lists.infradead.org
Cc: netdev at vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/net/wireless/broadcom/b43legacy/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/main.c b/drivers/net/wireless/broadcom/b43legacy/main.c
index 2eaf481f03f1d..044a5fa66ae79 100644
--- a/drivers/net/wireless/broadcom/b43legacy/main.c
+++ b/drivers/net/wireless/broadcom/b43legacy/main.c
@@ -1340,8 +1340,9 @@ static void b43legacy_interrupt_tasklet(unsigned long data)
 		handle_irq_beacon(dev);
 	if (reason & B43legacy_IRQ_PMQ)
 		handle_irq_pmq(dev);
-	if (reason & B43legacy_IRQ_TXFIFO_FLUSH_OK)
+	if (reason & B43legacy_IRQ_TXFIFO_FLUSH_OK) {
 		;/*TODO*/
+	}
 	if (reason & B43legacy_IRQ_NOISESAMPLE_OK)
 		handle_irq_noise(dev);
 
-- 
2.25.1

[PATCH 26/30] net: wireless: broadcom: b43: phy_common: Demote non-conformant kerneldoc header

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/net/wireless/broadcom/b43/phy_common.c:467: warning: Function parameter or member 'work' not described in 'b43_phy_txpower_adjust_work'

Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: "Alexander A. Klimov" <grandmaster@al2klimov.de>
Cc: Martin Langer <martin-langer@gmx.de>
Cc: Stefano Brivio <stefano.brivio@polimi.it>
Cc: Michael Buesch <m@bues.ch>
Cc: van Dyk <kugelfang@gentoo.org>
Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
Cc: linux-wireless at vger.kernel.org
Cc: b43-dev at lists.infradead.org
Cc: netdev at vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/net/wireless/broadcom/b43/phy_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_common.c b/drivers/net/wireless/broadcom/b43/phy_common.c
index 1de4de094d61b..285490f6f0a1d 100644
--- a/drivers/net/wireless/broadcom/b43/phy_common.c
+++ b/drivers/net/wireless/broadcom/b43/phy_common.c
@@ -458,7 +458,7 @@ void b43_software_rfkill(struct b43_wldev *dev, bool blocked)
 	b43_mac_enable(dev);
 }
 
-/**
+/*
  * b43_phy_txpower_adjust_work - TX power workqueue.
  *
  * Workqueue for updating the TX power parameters in hardware.
-- 
2.25.1

[PATCH 30/30] net: wireless: broadcom: b43: phy_n: Add empty braces around empty statements

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/net/wireless/broadcom/b43/phy_n.c: In function ?b43_nphy_workarounds_rev3plus?:
 drivers/net/wireless/broadcom/b43/phy_n.c:3346:3: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]
 drivers/net/wireless/broadcom/b43/phy_n.c: In function ?b43_nphy_spur_workaround?:
 drivers/net/wireless/broadcom/b43/phy_n.c:4608:4: warning: suggest braces around empty body in an ?else? statement [-Wempty-body]
 drivers/net/wireless/broadcom/b43/phy_n.c:4641:4: warning: suggest braces around empty body in an ?else? statement [-Wempty-body]
 drivers/net/wireless/broadcom/b43/phy_n.c: In function ?b43_phy_initn?:
 drivers/net/wireless/broadcom/b43/phy_n.c:6170:3: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]
 drivers/net/wireless/broadcom/b43/phy_n.c:6215:5: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]

Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: "Alexander A. Klimov" <grandmaster@al2klimov.de>
Cc: Michael Buesch <m@bues.ch>
Cc: linux-wireless at vger.kernel.org
Cc: b43-dev at lists.infradead.org
Cc: netdev at vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/net/wireless/broadcom/b43/phy_n.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index ca2018da97538..9e4d61e64adf5 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -3342,8 +3342,9 @@ static void b43_nphy_workarounds_rev3plus(struct b43_wldev *dev)
 	b43_phy_write(dev, B43_NPHY_ED_CRS20UDEASSERTTHRESH0, 0x0381);
 	b43_phy_write(dev, B43_NPHY_ED_CRS20UDEASSERTTHRESH1, 0x0381);
 
-	if (dev->phy.rev >= 6 && sprom->boardflags2_lo & B43_BFL2_SINGLEANT_CCK)
+	if (dev->phy.rev >= 6 && sprom->boardflags2_lo & B43_BFL2_SINGLEANT_CCK) {
 		; /* TODO: 0x0080000000000000 HF */
+	}
 }
 
 static void b43_nphy_workarounds_rev1_2(struct b43_wldev *dev)
@@ -4602,10 +4603,11 @@ static void b43_nphy_spur_workaround(struct b43_wldev *dev)
 
 	if (nphy->gband_spurwar_en) {
 		/* TODO: N PHY Adjust Analog Pfbw (7) */
-		if (channel == 11 && b43_is_40mhz(dev))
+		if (channel == 11 && b43_is_40mhz(dev)) {
 			; /* TODO: N PHY Adjust Min Noise Var(2, tone, noise)*/
-		else
+		} else {
 			; /* TODO: N PHY Adjust Min Noise Var(0, NULL, NULL)*/
+		}
 		/* TODO: N PHY Adjust CRS Min Power (0x1E) */
 	}
 
@@ -4635,10 +4637,11 @@ static void b43_nphy_spur_workaround(struct b43_wldev *dev)
 			noise[0] = 0;
 		}
 
-		if (!tone[0] && !noise[0])
+		if (!tone[0] && !noise[0]) {
 			; /* TODO: N PHY Adjust Min Noise Var(1, tone, noise)*/
-		else
+		} else {
 			; /* TODO: N PHY Adjust Min Noise Var(0, NULL, NULL)*/
+		}
 	}
 
 	if (nphy->hang_avoid)
@@ -6166,8 +6169,9 @@ static int b43_phy_initn(struct b43_wldev *dev)
 
 	if (nphy->phyrxchain != 3)
 		b43_nphy_set_rx_core_state(dev, nphy->phyrxchain);
-	if (nphy->mphase_cal_phase_id > 0)
+	if (nphy->mphase_cal_phase_id > 0) {
 		;/* TODO PHY Periodic Calibration Multi-Phase Restart */
+	}
 
 	do_rssi_cal = false;
 	if (phy->rev >= 3) {
@@ -6211,8 +6215,9 @@ static int b43_phy_initn(struct b43_wldev *dev)
 				if (!b43_nphy_cal_tx_iq_lo(dev, target, true, false))
 					if (b43_nphy_cal_rx_iq(dev, target, 2, 0) == 0)
 						b43_nphy_save_cal(dev);
-			} else if (nphy->mphase_cal_phase_id == 0)
+			} else if (nphy->mphase_cal_phase_id == 0) {
 				;/* N PHY Periodic Calibration with arg 3 */
+			}
 		} else {
 			b43_nphy_restore_cal(dev);
 		}
-- 
2.25.1

[PATCH 07/30] net: wireless: broadcom: b43: main: Add braces around empty statements

[Kalle Valo]

Lee Jones <lee.jones@linaro.org> writes:

> Fixes the following W=1 kernel build warning(s):
>
>  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_dummy_transmission?:
>  drivers/net/wireless/broadcom/b43/main.c:785:3: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]
>  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_do_interrupt_thread?:
>  drivers/net/wireless/broadcom/b43/main.c:2017:3: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]
>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Martin Langer <martin-langer@gmx.de>
> Cc: Stefano Brivio <stefano.brivio@polimi.it>
> Cc: Michael Buesch <m@bues.ch>
> Cc: van Dyk <kugelfang@gentoo.org>
> Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
> Cc: Albert Herranz <albert_herranz@yahoo.es>
> Cc: linux-wireless at vger.kernel.org
> Cc: b43-dev at lists.infradead.org
> Cc: netdev at vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/net/wireless/broadcom/b43/main.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Please don't copy the full directory structure to the title. I'll change
the title to more simple version:

b43: add braces around empty statements

I'll do similar changes to other wireless-drivers patches.

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

[PATCH 26/30] net: wireless: broadcom: b43: phy_common: Demote non-conformant kerneldoc header

[Lee Jones]

On Fri, 14 Aug 2020, Artur Dubrovsky wrote:

> remove me from mailing list

I guess you have to remove yourself:

http://lists.infradead.org/mailman/listinfo/b43-dev

> ??, 14 ???. 2020 ?. ? 14:43, Lee Jones <lee.jones@linaro.org>:
> 
> > Fixes the following W=1 kernel build warning(s):
> >
> >  drivers/net/wireless/broadcom/b43/phy_common.c:467: warning: Function
> > parameter or member 'work' not described in 'b43_phy_txpower_adjust_work'
> >
> > Cc: Kalle Valo <kvalo@codeaurora.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: "Alexander A. Klimov" <grandmaster@al2klimov.de>
> > Cc: Martin Langer <martin-langer@gmx.de>
> > Cc: Stefano Brivio <stefano.brivio@polimi.it>
> > Cc: Michael Buesch <m@bues.ch>
> > Cc: van Dyk <kugelfang@gentoo.org>
> > Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
> > Cc: linux-wireless at vger.kernel.org
> > Cc: b43-dev at lists.infradead.org
> > Cc: netdev at vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/net/wireless/broadcom/b43/phy_common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)

[...]

-- 
Lee Jones [???]
Senior Technical Lead - Developer Services
Linaro.org ? Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 07/30] net: wireless: broadcom: b43: main: Add braces around empty statements

[Lee Jones]

On Fri, 14 Aug 2020, Kalle Valo wrote:

> Lee Jones <lee.jones@linaro.org> writes:
> 
> > Fixes the following W=1 kernel build warning(s):
> >
> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_dummy_transmission?:
> >  drivers/net/wireless/broadcom/b43/main.c:785:3: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]
> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_do_interrupt_thread?:
> >  drivers/net/wireless/broadcom/b43/main.c:2017:3: warning: suggest braces around empty body in an ?if? statement [-Wempty-body]
> >
> > Cc: Kalle Valo <kvalo@codeaurora.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Martin Langer <martin-langer@gmx.de>
> > Cc: Stefano Brivio <stefano.brivio@polimi.it>
> > Cc: Michael Buesch <m@bues.ch>
> > Cc: van Dyk <kugelfang@gentoo.org>
> > Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
> > Cc: Albert Herranz <albert_herranz@yahoo.es>
> > Cc: linux-wireless at vger.kernel.org
> > Cc: b43-dev at lists.infradead.org
> > Cc: netdev at vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/net/wireless/broadcom/b43/main.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Please don't copy the full directory structure to the title. I'll change
> the title to more simple version:
> 
> b43: add braces around empty statements

This seems to go the other way.

"net: wireless: b43" seems sensible.

> I'll do similar changes to other wireless-drivers patches.

Thanks.

Does that mean it's been applied, or is this future tense?

-- 
Lee Jones [???]
Senior Technical Lead - Developer Services
Linaro.org ? Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 07/30] net: wireless: broadcom: b43: main: Add braces around empty statements

[Kalle Valo]

Lee Jones <lee.jones@linaro.org> writes:

> On Fri, 14 Aug 2020, Kalle Valo wrote:
>
>> Lee Jones <lee.jones@linaro.org> writes:
>> 
>> > Fixes the following W=1 kernel build warning(s):
>> >
>> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_dummy_transmission?:
>> >  drivers/net/wireless/broadcom/b43/main.c:785:3: warning: suggest
>> > braces around empty body in an ?if? statement [-Wempty-body]
>> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_do_interrupt_thread?:
>> >  drivers/net/wireless/broadcom/b43/main.c:2017:3: warning: suggest
>> > braces around empty body in an ?if? statement [-Wempty-body]
>> >
>> > Cc: Kalle Valo <kvalo@codeaurora.org>
>> > Cc: "David S. Miller" <davem@davemloft.net>
>> > Cc: Jakub Kicinski <kuba@kernel.org>
>> > Cc: Martin Langer <martin-langer@gmx.de>
>> > Cc: Stefano Brivio <stefano.brivio@polimi.it>
>> > Cc: Michael Buesch <m@bues.ch>
>> > Cc: van Dyk <kugelfang@gentoo.org>
>> > Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
>> > Cc: Albert Herranz <albert_herranz@yahoo.es>
>> > Cc: linux-wireless at vger.kernel.org
>> > Cc: b43-dev at lists.infradead.org
>> > Cc: netdev at vger.kernel.org
>> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> > ---
>> >  drivers/net/wireless/broadcom/b43/main.c | 6 ++++--
>> >  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> Please don't copy the full directory structure to the title. I'll change
>> the title to more simple version:
>> 
>> b43: add braces around empty statements
>
> This seems to go the other way.
>
> "net: wireless: b43" seems sensible.

Sorry, not understanding what you mean here.

>> I'll do similar changes to other wireless-drivers patches.
>
> Thanks.
>
> Does that mean it's been applied, or is this future tense?

It's not applied yet, there will be an automatic "applied" email once I
have done that.

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

[PATCH 26/30] net: wireless: broadcom: b43: phy_common: Demote non-conformant kerneldoc header

[Rafał Miłecki]

On Fri, 14 Aug 2020 at 13:41, Lee Jones <lee.jones@linaro.org> wrote:
> Fixes the following W=1 kernel build warning(s):
>
>  drivers/net/wireless/broadcom/b43/phy_common.c:467: warning: Function parameter or member 'work' not described in 'b43_phy_txpower_adjust_work'

Why you can't document @work instead? Should be quite a better solution.

[PATCH 07/30] net: wireless: broadcom: b43: main: Add braces around empty statements

[Lee Jones]

On Fri, 14 Aug 2020, Kalle Valo wrote:

> Lee Jones <lee.jones@linaro.org> writes:
> 
> > On Fri, 14 Aug 2020, Kalle Valo wrote:
> >
> >> Lee Jones <lee.jones@linaro.org> writes:
> >> 
> >> > Fixes the following W=1 kernel build warning(s):
> >> >
> >> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_dummy_transmission?:
> >> >  drivers/net/wireless/broadcom/b43/main.c:785:3: warning: suggest
> >> > braces around empty body in an ?if? statement [-Wempty-body]
> >> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_do_interrupt_thread?:
> >> >  drivers/net/wireless/broadcom/b43/main.c:2017:3: warning: suggest
> >> > braces around empty body in an ?if? statement [-Wempty-body]
> >> >
> >> > Cc: Kalle Valo <kvalo@codeaurora.org>
> >> > Cc: "David S. Miller" <davem@davemloft.net>
> >> > Cc: Jakub Kicinski <kuba@kernel.org>
> >> > Cc: Martin Langer <martin-langer@gmx.de>
> >> > Cc: Stefano Brivio <stefano.brivio@polimi.it>
> >> > Cc: Michael Buesch <m@bues.ch>
> >> > Cc: van Dyk <kugelfang@gentoo.org>
> >> > Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
> >> > Cc: Albert Herranz <albert_herranz@yahoo.es>
> >> > Cc: linux-wireless at vger.kernel.org
> >> > Cc: b43-dev at lists.infradead.org
> >> > Cc: netdev at vger.kernel.org
> >> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >> > ---
> >> >  drivers/net/wireless/broadcom/b43/main.c | 6 ++++--
> >> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >> 
> >> Please don't copy the full directory structure to the title. I'll change
> >> the title to more simple version:
> >> 
> >> b43: add braces around empty statements
> >
> > This seems to go the other way.
> >
> > "net: wireless: b43" seems sensible.
> 
> Sorry, not understanding what you mean here.

So I agree that:

  "net: wireless: broadcom: b43: main"

... seems unnecessarily long and verbose.  However, IMHO:

  "b43:"

... is too short and not forthcoming enough.  Obviously this fine when
something like `git log -- net/wireless`, as you already know what the
patch pertains to, however when someone who is not in the know (like I
would be) does `git log` and sees a "b43:" patch, they would have no
idea which subsystem this patch is adapting.  Even:

  "wireless: b43:"

... would be worlds better.

A Git log which omitted all subsystem tags would be of limited use.

> >> I'll do similar changes to other wireless-drivers patches.
> >
> > Thanks.
> >
> > Does that mean it's been applied, or is this future tense?
> 
> It's not applied yet, there will be an automatic "applied" email once I
> have done that.

I see.  Thanks for the clarification.

-- 
Lee Jones [???]
Senior Technical Lead - Developer Services
Linaro.org ? Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 26/30] net: wireless: broadcom: b43: phy_common: Demote non-conformant kerneldoc header

[Lee Jones]

On Mon, 17 Aug 2020, Rafa? Mi?ecki wrote:

> On Fri, 14 Aug 2020 at 13:41, Lee Jones <lee.jones@linaro.org> wrote:
> > Fixes the following W=1 kernel build warning(s):
> >
> >  drivers/net/wireless/broadcom/b43/phy_common.c:467: warning: Function parameter or member 'work' not described in 'b43_phy_txpower_adjust_work'
> 
> Why you can't document @work instead? Should be quite a better solution.

It some circumstances it would, but not this one.

 1. This 'kernel-doc' function is not referenced from any
    documentation location i.e. it is presently unused and there is no
    reason for it to be kernel-doc in the first place.
 2. This patch stops `scripts/find-unused-docs.sh` complaining about
    phy_common.c
 3. 'b43_phy_txpower_adjust_work' is the only function that has been
     documented as kernel-doc - why is that?  Seems like a mistake.
 
-- 
Lee Jones [???]
Senior Technical Lead - Developer Services
Linaro.org ? Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 07/30] net: wireless: broadcom: b43: main: Add braces around empty statements

[Kalle Valo]

Lee Jones <lee.jones@linaro.org> writes:

> On Fri, 14 Aug 2020, Kalle Valo wrote:
>
>> Lee Jones <lee.jones@linaro.org> writes:
>> 
>> > On Fri, 14 Aug 2020, Kalle Valo wrote:
>> >
>> >> Lee Jones <lee.jones@linaro.org> writes:
>> >> 
>> >> > Fixes the following W=1 kernel build warning(s):
>> >> >
>> >> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_dummy_transmission?:
>> >> >  drivers/net/wireless/broadcom/b43/main.c:785:3: warning: suggest
>> >> > braces around empty body in an ?if? statement [-Wempty-body]
>> >> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_do_interrupt_thread?:
>> >> >  drivers/net/wireless/broadcom/b43/main.c:2017:3: warning: suggest
>> >> > braces around empty body in an ?if? statement [-Wempty-body]
>> >> >
>> >> > Cc: Kalle Valo <kvalo@codeaurora.org>
>> >> > Cc: "David S. Miller" <davem@davemloft.net>
>> >> > Cc: Jakub Kicinski <kuba@kernel.org>
>> >> > Cc: Martin Langer <martin-langer@gmx.de>
>> >> > Cc: Stefano Brivio <stefano.brivio@polimi.it>
>> >> > Cc: Michael Buesch <m@bues.ch>
>> >> > Cc: van Dyk <kugelfang@gentoo.org>
>> >> > Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
>> >> > Cc: Albert Herranz <albert_herranz@yahoo.es>
>> >> > Cc: linux-wireless at vger.kernel.org
>> >> > Cc: b43-dev at lists.infradead.org
>> >> > Cc: netdev at vger.kernel.org
>> >> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> >> > ---
>> >> >  drivers/net/wireless/broadcom/b43/main.c | 6 ++++--
>> >> >  1 file changed, 4 insertions(+), 2 deletions(-)
>> >> 
>> >> Please don't copy the full directory structure to the title. I'll change
>> >> the title to more simple version:
>> >> 
>> >> b43: add braces around empty statements
>> >
>> > This seems to go the other way.
>> >
>> > "net: wireless: b43" seems sensible.
>> 
>> Sorry, not understanding what you mean here.
>
> So I agree that:
>
>   "net: wireless: broadcom: b43: main"
>
> ... seems unnecessarily long and verbose.  However, IMHO:
>
>   "b43:"
>
> ... is too short and not forthcoming enough.  Obviously this fine when
> something like `git log -- net/wireless`, as you already know what the
> patch pertains to, however when someone who is not in the know (like I
> would be) does `git log` and sees a "b43:" patch, they would have no
> idea which subsystem this patch is adapting.  Even:
>
>   "wireless: b43:"
>
> ... would be worlds better.
>
> A Git log which omitted all subsystem tags would be of limited use.

There are good reasons why the style is like it is. If I would start
adding "wireless:" tags to the title it would clutter 'git log
--oneline' and gitk output, which I use all the time. And I'm not
interested making my work harder, there would need to be really strong
reasons why I would even recondiser changing it.

BTW, this is also documented in our wiki:

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

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

[PATCH 07/30] net: wireless: broadcom: b43: main: Add braces around empty statements

[Lee Jones]

On Thu, 27 Aug 2020, Kalle Valo wrote:

> Lee Jones <lee.jones@linaro.org> writes:
> 
> > On Fri, 14 Aug 2020, Kalle Valo wrote:
> >
> >> Lee Jones <lee.jones@linaro.org> writes:
> >> 
> >> > On Fri, 14 Aug 2020, Kalle Valo wrote:
> >> >
> >> >> Lee Jones <lee.jones@linaro.org> writes:
> >> >> 
> >> >> > Fixes the following W=1 kernel build warning(s):
> >> >> >
> >> >> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_dummy_transmission?:
> >> >> >  drivers/net/wireless/broadcom/b43/main.c:785:3: warning: suggest
> >> >> > braces around empty body in an ?if? statement [-Wempty-body]
> >> >> >  drivers/net/wireless/broadcom/b43/main.c: In function ?b43_do_interrupt_thread?:
> >> >> >  drivers/net/wireless/broadcom/b43/main.c:2017:3: warning: suggest
> >> >> > braces around empty body in an ?if? statement [-Wempty-body]
> >> >> >
> >> >> > Cc: Kalle Valo <kvalo@codeaurora.org>
> >> >> > Cc: "David S. Miller" <davem@davemloft.net>
> >> >> > Cc: Jakub Kicinski <kuba@kernel.org>
> >> >> > Cc: Martin Langer <martin-langer@gmx.de>
> >> >> > Cc: Stefano Brivio <stefano.brivio@polimi.it>
> >> >> > Cc: Michael Buesch <m@bues.ch>
> >> >> > Cc: van Dyk <kugelfang@gentoo.org>
> >> >> > Cc: Andreas Jaggi <andreas.jaggi@waterwave.ch>
> >> >> > Cc: Albert Herranz <albert_herranz@yahoo.es>
> >> >> > Cc: linux-wireless at vger.kernel.org
> >> >> > Cc: b43-dev at lists.infradead.org
> >> >> > Cc: netdev at vger.kernel.org
> >> >> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >> >> > ---
> >> >> >  drivers/net/wireless/broadcom/b43/main.c | 6 ++++--
> >> >> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >> >> 
> >> >> Please don't copy the full directory structure to the title. I'll change
> >> >> the title to more simple version:
> >> >> 
> >> >> b43: add braces around empty statements
> >> >
> >> > This seems to go the other way.
> >> >
> >> > "net: wireless: b43" seems sensible.
> >> 
> >> Sorry, not understanding what you mean here.
> >
> > So I agree that:
> >
> >   "net: wireless: broadcom: b43: main"
> >
> > ... seems unnecessarily long and verbose.  However, IMHO:
> >
> >   "b43:"
> >
> > ... is too short and not forthcoming enough.  Obviously this fine when
> > something like `git log -- net/wireless`, as you already know what the
> > patch pertains to, however when someone who is not in the know (like I
> > would be) does `git log` and sees a "b43:" patch, they would have no
> > idea which subsystem this patch is adapting.  Even:
> >
> >   "wireless: b43:"
> >
> > ... would be worlds better.
> >
> > A Git log which omitted all subsystem tags would be of limited use.
> 
> There are good reasons why the style is like it is. If I would start
> adding "wireless:" tags to the title it would clutter 'git log
> --oneline' and gitk output, which I use all the time. And I'm not
> interested making my work harder, there would need to be really strong
> reasons why I would even recondiser changing it.
> 
> BTW, this is also documented in our wiki:
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_title_is_wrong

Documented or otherwise, it's still odd.

Yes, it's okay for *you* being the Maintainer of Wireless, but by
keeping your own workspace clutter free you obfuscate the `git log`
for everyone else.

I can't find another subsystem that does this:

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/scsi | cat
0c1a65a347227 scsi: pm8001: pm8001_hwi: Remove unused variable 'value'
beccf4070cabd scsi: pm8001: pm8001_sas: Fix strncpy() warning
50e619cb0966b scsi: arcmsr: arcmsr_hba: Make room for the trailing NULL, even if it is over-written
9c6c4a4606ecf scsi: megaraid: megaraid_sas_base: Provide prototypes for non-static functions
32417d7844ab0 scsi: esas2r: Remove unnecessary casts

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/i2c | cat
ab70935d37bbd i2c: Remove 'default n' from busses/Kconfig
0204081128d58 i2c: iproc: Fix shifting 31 bits
914a7b3563b8f i2c: rcar: in slave mode, clear NACK earlier
e4682b8a688bc i2c: acpi: Remove dead code, i.e. i2c_acpi_match_device()
e3cb82c6d6f6c i2c: core: Don't fail PRP0001 enumeration when no ID table exist

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/spi | cat
b0e37c5157332 spi: spi-fsl-espi: Remove use of %p
2ea370a9173f4 spi: spi-cadence-quadspi: Populate get_name() interface
20c05a0550636 spi: spi-fsl-dspi: delete EOQ transfer mode
df561f6688fef treewide: Use fallthrough pseudo-keyword
c76964e810a55 spi: imx: Remove unneeded probe message

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/nfc/ | cat
f97c04c316d8f NFC: st95hf: Fix memleak in st95hf_in_send_cmd
df561f6688fef treewide: Use fallthrough pseudo-keyword
f8c931f3be8dd nfc: st21nfca: Remove unnecessary cast
0eddbef6489cf nfc: st-nci: Remove unnecessary cast
1e8fd3a97f2d8 nfc: s3fwrn5: add missing release on skb in s3fwrn5_recv_frame

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/misc | cat
e19e862938acf misc: ocxl: config: Rename function attribute description
ca99b8bdf84ef misc: c2port: core: Make copying name from userspace more secure
99363d1c26c82 eeprom: at24: Tidy at24_read()
df561f6688fef treewide: Use fallthrough pseudo-keyword
5aba368893c0d habanalabs: correctly report inbound pci region cfg error

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/iio | cat
fc2404e94d3fb iio: industrialio-trigger: Use 'gnu_printf' format notation
e4130d150831b iio: imu: adis16400: Provide description for missing struct attribute 'avail_scan_mask'
4a6b899005ef5 iio: adc: mcp320x: Change ordering of compiler attribute macro
96d7124f00e62 iio: gyro: adxrs450: Change ordering of compiler attribute macro
21ef78342f557 iio: resolver: ad2s1200: Change ordering of compiler attribute macro

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/firmware/ | cat
9bbb6d7de490a efi/fake_mem: arrange for a resource entry per efi_fake_mem instance
f75fa0a51b8b5 efi: Rename arm-init to efi-init common for all arch
92efdc54a2c04 RISC-V: Add EFI stub support.
8a8a3237a78cb efi/libstub: Handle unterminated cmdline
a37ca6a2af9df efi/libstub: Handle NULL cmdline

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/remoteproc/ | cat
df561f6688fef treewide: Use fallthrough pseudo-keyword
62b8f9e99329c remoteproc: core: Register the character device interface
4476770881d7a remoteproc: Add remoteproc character device interface
2f3ee5e481ce8 remoteproc: kill IPA notify code
87218f96c21a9 remoteproc: k3-dsp: Add support for C71x DSPs

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/dma | cat
78a2f92e4c4a3 dmaengine: axi-dmac: add support for reading bus attributes from registers
3061a65c1b3db dmaengine: axi-dmac: wrap channel parameter adjust into function
06b6e88c7ecf4 dmaengine: axi-dmac: wrap entire dt parse in a function
08b36dba23e5b dmaengine: axi-dmac: move clock enable earlier
a88fdece44d40 dmaengine: axi-dmac: move active_descs list init after device-tree init

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/clk | cat
0b8056106c02b clk: imx: vf610: Add CRC clock
7d6b5e4f24457 clk: imx: Explicitly include bits.h
e0d0d4d86c766 clk: imx8qxp: Support building i.MX8QXP clock driver as module
9a976cd278eaf clk: imx8m: Support module build
f1f018dc030ed clk: imx: Add clock configuration for ARMv7 platforms

lee at dell:~/projects/linux/kernel [tb-fix-w1-warnings]$ git log --oneline --follow --no-merges -5 drivers/crypto | cat
3033fd177bcc3 crypto: stm32 - Add missing header inclusions
df561f6688fef treewide: Use fallthrough pseudo-keyword
1b77be463929e crypto/chcr: Moving chelsio's inline ipsec functionality to /drivers/net
44fd1c1fd8219 chelsio/chtls: separate chelsio tls driver from crypto driver
3d29e98d1d755 crypto: hisilicon/qm - fix the process of register algorithms to crypto

-- 
Lee Jones [???]
Senior Technical Lead - Developer Services
Linaro.org ? Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog