Skip to contentSkip to conversation

ath9k and ath9k_htc: rename variable "led_blink"

PATCH Superseded Hong Xu 4 participants 10 messages Jan 24, 2015 Qualcomm Atheros ATH9K wireless driver

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Hong Xu

author
#0 lore raw

PATCH 1/1 ath9k and ath9k_htc: rename variable "led_blink"

ath9k and ath9k_htc use the variable name "led_blink" to indicate whether the module parameter "blink" is on. This name is easy to conflict with other variables, thus they are renamed.

Signed-off-by Hong Xu <[email protected]>
---
 drivers/net/wireless/ath/ath9k/ath9k.h        | 2 +-
 drivers/net/wireless/ath/ath9k/gpio.c         | 2 +-
 drivers/net/wireless/ath/ath9k/htc.h          | 2 +-
 drivers/net/wireless/ath/ath9k/htc_drv_gpio.c | 2 +-
 drivers/net/wireless/ath/ath9k/htc_drv_init.c | 4 ++--
 drivers/net/wireless/ath/ath9k/init.c         | 4 ++--
 6 files changed, 8 insertions(+), 8 deletions(-)
drivers/net/wireless/ath/ath9k/ath9k.h +1 -1
index 1a9fe09..e84b576 100644
@@ -34,7 +34,7 @@ struct ath_vif;
 
 extern struct ieee80211_ops ath9k_ops;
 extern int ath9k_modparam_nohwcrypt;
-extern int led_blink;
+extern int ath9k_led_blink;
 extern bool is_ath9k_unloaded;
 extern int ath9k_use_chanctx;
drivers/net/wireless/ath/ath9k/gpio.c +1 -1
index 2fef7a4..da344b2 100644
@@ -49,7 +49,7 @@ void ath_init_leds(struct ath_softc *sc)
 	if (AR_SREV_9100(sc->sc_ah))
 		return;
 
-	if (!led_blink)
+	if (!ath9k_led_blink)
 		sc->led_cdev.default_trigger =
 			ieee80211_get_radio_led_name(sc->hw);
drivers/net/wireless/ath/ath9k/htc.h +1 -1
index c43fec5..300d367 100644
@@ -45,7 +45,7 @@
 extern struct ieee80211_ops ath9k_htc_ops;
 extern int htc_modparam_nohwcrypt;
 #ifdef CONFIG_MAC80211_LEDS
-extern int led_blink;
+extern int ath9k_htc_led_blink;
 #endif
 
 enum htc_phymode {
drivers/net/wireless/ath/ath9k/htc_drv_gpio.c +1 -1
index 998b558..2aabcbd 100644
@@ -279,7 +279,7 @@ void ath9k_init_leds(struct ath9k_htc_priv *priv)
 	else
 		priv->ah->led_pin = ATH_LED_PIN_DEF;
 
-	if (!led_blink)
+	if (!ath9k_htc_led_blink)
 		priv->led_cdev.default_trigger =
 			ieee80211_get_radio_led_name(priv->hw);
drivers/net/wireless/ath/ath9k/htc_drv_init.c +2 -2
index 9470127..fd22940 100644
@@ -39,8 +39,8 @@ module_param_named(ps_enable, ath9k_ps_enable, int, 0444);
 MODULE_PARM_DESC(ps_enable, "Enable WLAN PowerSave");
 
 #ifdef CONFIG_MAC80211_LEDS
-int led_blink = 1;
-module_param_named(blink, led_blink, int, 0444);
+int ath9k_htc_led_blink = 1;
+module_param_named(blink, ath9k_htc_led_blink, int, 0444);
 MODULE_PARM_DESC(blink, "Enable LED blink on activity");
 
 static const struct ieee80211_tpt_blink ath9k_htc_tpt_blink[] = {
drivers/net/wireless/ath/ath9k/init.c +2 -2
index d1c3934..2aef14e 100644
@@ -45,8 +45,8 @@ int ath9k_modparam_nohwcrypt;
 module_param_named(nohwcrypt, ath9k_modparam_nohwcrypt, int, 0444);
 MODULE_PARM_DESC(nohwcrypt, "Disable hardware encryption");
 
-int led_blink;
-module_param_named(blink, led_blink, int, 0444);
+int ath9k_led_blink;
+module_param_named(blink, ath9k_led_blink, int, 0444);
 MODULE_PARM_DESC(blink, "Enable LED blink on activity");
 
 static int ath9k_btcoex_enable;
Signature · 1 line
2.2.2
#1 lore raw

PATCH 1/1 ath9k and ath9k_htc: rename variable "led_blink"

Quoted 3 lines · Hong Xu

ath9k and ath9k_htc use the variable name "led_blink" to indicate whether the module parameter "blink" is on. This name is easy to conflict with other variables, thus they are renamed.

Please state clearly that this fixes a compiler error found by kbuild. Also it's very much recommended to add the actual compiler error to the commit log:

drivers/net/wireless/ath/ath9k/ath9k_htc.o:(.data+0x47c): multiple definition of `led_blink' drivers/net/wireless/ath/ath9k/ath9k.o:(.bss+0x20): first defined here

Signature · 1 line
Kalle Valo
#2 lore raw

PATCH 1/1 ath9k and ath9k_htc: rename variable "led_blink"

Quoted 6 lines · Kalle Valo
Quoted 3 lines · Hong Xu

ath9k and ath9k_htc use the variable name "led_blink" to indicate whether the module parameter "blink" is on. This name is easy to conflict with other variables, thus they are renamed.

Quoted 7 lines

Please state clearly that this fixes a compiler error found by kbuild. Also it's very much recommended to add the actual compiler error to the commit log:

drivers/net/wireless/ath/ath9k/ath9k_htc.o:(.data+0x47c): multiple definition of `led_blink' drivers/net/wireless/ath/ath9k/ath9k.o:(.bss+0x20): first defined here

Some more things...

If you post a vN (referring to your v3) of a patch, please add a history of changes.

2nd, use the Fixes tag [2] in this case with a reference to the culprit commit [1].

Fixes 3a939a671225 ("ath9k_htc: Add a module parameter to disable blink")

Last but not least, honour credits like Reported-by/Tested-by (plus some infos like broken in Linux-next $VERSION).

- Sedat -

[1] http://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=3a939a671225909c19b09bfcb6e4761109e913d9
[2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n159

#3 lore raw

PATCH 1/1 ath9k and ath9k_htc: rename variable "led_blink"

Am 24.01.2015 um 11:33 schrieb Sedat Dilek:

Quoted 11 lines · Kalle Valo
Quoted 10 lines
Quoted 3 lines · Hong Xu

ath9k and ath9k_htc use the variable name "led_blink" to indicate whether the module parameter "blink" is on. This name is easy to conflict with other variables, thus they are renamed.

Please state clearly that this fixes a compiler error found by kbuild. Also it's very much recommended to add the actual compiler error to the commit log:

Quoted 8 lines
Quoted 3 lines

drivers/net/wireless/ath/ath9k/ath9k_htc.o:(.data+0x47c): multiple definition of `led_blink' drivers/net/wireless/ath/ath9k/ath9k.o:(.bss+0x20): first defined here

Some more things...

If you post a vN (referring to your v3) of a patch, please add a history of changes.

What kind of history do you wont to have here? Kernel log should not be overloaded with patch preparation history.

Quoted 17 lines

2nd, use the Fixes tag [2] in this case with a reference to the culprit commit [1].

Fixes 3a939a671225 ("ath9k_htc: Add a module parameter to disable blink")

Last but not least, honour credits like Reported-by/Tested-by (plus some infos like broken in Linux-next $VERSION).

- Sedat -

[1] http://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/commit/?id=3a939a671225909c19b09bfcb6e4761109e913d9
[2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n159
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 10 lines
Regards,
Oleksij

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: OpenPGP digital signature
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20150124/7ab76d0c/attachment.pgp
#4 lore raw

PATCH 1/1 ath9k and ath9k_htc: rename variable "led_blink"

Quoted 16 lines · Oleksij Rempel

Am 24.01.2015 um 11:33 schrieb Sedat Dilek:

Quoted 15 lines · Kalle Valo
Quoted 13 lines
Quoted 3 lines · Hong Xu

ath9k and ath9k_htc use the variable name "led_blink" to indicate whether the module parameter "blink" is on. This name is easy to conflict with other variables, thus they are renamed.

Please state clearly that this fixes a compiler error found by kbuild. Also it's very much recommended to add the actual compiler error to the commit log:

drivers/net/wireless/ath/ath9k/ath9k_htc.o:(.data+0x47c): multiple definition of `led_blink' drivers/net/wireless/ath/ath9k/ath9k.o:(.bss+0x20): first defined here

Quoted 8 lines
Quoted 4 lines

Some more things...

If you post a vN (referring to your v3) of a patch, please add a history of changes.

What kind of history do you wont to have here? Kernel log should not be overloaded with patch preparation history.

Not here.
Referring to "[PATCH v3] ath9k_htc: Add a module parameter to disable blink".

And no, the history of changes are 1. important 2. not in the "commit-body", so you will not see them when the patch is pushed to the appropriate Git tree.

- Sedat -

#5 lore raw
In reply to Sedat Dilek

(I'll drop ath9k-devel as someone from there seems to be spamming with vacation responses)

Quoted 4 lines · Sedat Dilek

2nd, use the Fixes tag [2] in this case with a reference to the culprit commit [1].

Fixes 3a939a671225 ("ath9k_htc: Add a module parameter to disable blink")

Good point. I just found a handy tip from SubmittingPatches, just add this to ~/.gitconfig:

	[core]
		abbrev = 12
	[pretty]
		fixes = Fixes: %h (\"%s\")

And then you get the fixes line automatically formatted for whatever commit you want:

$ git log --format=fixes | head -5
Fixes: fa1270678ce4 ("ath10k: Enable the MCS8 and MCS9 at 2.4G band")
Fixes: 296f98ed02dd ("ath10k: fix pmf for wmi-tlv on qca6174")
Fixes: 3443aaf6f11d ("ath10k: remove sw encryption for pmf")
Fixes: d541c931dc93 ("ath10k: disable irqs after fw crash")
Fixes: 9f282daeedc9 ("ath10k: disable uapsd autotrigger")

Yet another reason why git is so awesome.

Signature · 1 line
Kalle Valo
#6 lore raw
Quoted 21 lines · Kalle Valo

(I'll drop ath9k-devel as someone from there seems to be spamming with vacation responses)

Quoted 4 lines · Sedat Dilek

2nd, use the Fixes tag [2] in this case with a reference to the culprit commit [1].

Fixes 3a939a671225 ("ath9k_htc: Add a module parameter to disable blink")

Good point. I just found a handy tip from SubmittingPatches, just add this to ~/.gitconfig:

        [core]
                abbrev = 12
        [pretty]
                fixes = Fixes: %h (\"%s\")

And then you get the fixes line automatically formatted for whatever commit you want:

Quoted 9 lines
$ git log --format=fixes | head -5
Fixes: fa1270678ce4 ("ath10k: Enable the MCS8 and MCS9 at 2.4G band")
Fixes: 296f98ed02dd ("ath10k: fix pmf for wmi-tlv on qca6174")
Fixes: 3443aaf6f11d ("ath10k: remove sw encryption for pmf")
Fixes: d541c931dc93 ("ath10k: disable irqs after fw crash")
Fixes: 9f282daeedc9 ("ath10k: disable uapsd autotrigger")

Yet another reason why git is so awesome.

/o\.
Thanks, I added a core and pretty tag to my ~/.gitconfig, espacially the 12-digit thingie was a long standing todo :-).

Working with tig I have to check the tipps of Peter Hutterer in his blog for my local ~/.tigrc again.
This got lost when changing to Ubuntu/precise.

- Sedat -

#7 lore raw

[...]

Quoted 4 lines

Working with tig I have to check the tipps of Peter Hutterer in his blog for my local ~/.tigrc again.
This got lost when changing to Ubuntu/precise.

For tigrc... [2] has [1] included.

[3] and [4] as a reference to tig ("...is an ncurses-based text-mode interface for git.).

- Sedat -

[1] http://who-t.blogspot.de/2009/04/git-format-patch-for-single-commit.html
[2] http://who-t.blogspot.de/2014/03/using-git-next-level.html
[3] http://jonas.nitro.dk/tig/
[4] http://jonas.nitro.dk/tig/manual.html

author
#8 lore raw

PATCH 1/1 ath9k and ath9k_htc: rename variable "led_blink"

In reply to Sedat Dilek
Quoted 20 lines · Sedat Dilek
Quoted 6 lines · Kalle Valo
Quoted 3 lines · Hong Xu

ath9k and ath9k_htc use the variable name "led_blink" to indicate whether the module parameter "blink" is on. This name is easy to conflict with other variables, thus they are renamed.

Quoted 7 lines

Please state clearly that this fixes a compiler error found by kbuild. Also it's very much recommended to add the actual compiler error to the commit log:

drivers/net/wireless/ath/ath9k/ath9k_htc.o:(.data+0x47c): multiple definition of `led_blink' drivers/net/wireless/ath/ath9k/ath9k.o:(.bss+0x20): first defined here

Some more things...

If you post a vN (referring to your v3) of a patch, please add a history of changes.

Quoted 8 lines

2nd, use the Fixes tag [2] in this case with a reference to the culprit commit [1].

Fixes 3a939a671225 ("ath9k_htc: Add a module parameter to disable blink")

Last but not least, honour credits like Reported-by/Tested-by (plus some infos like broken in Linux-next $VERSION).

Thanks for your comments, I should have been more careful with the guideline. I guess we shouldn't credit kbuild for reporting in this case?

Hong
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20150124/8ac921fd/attachment.pgp

#9 lore raw

PATCH 1/1 ath9k and ath9k_htc: rename variable "led_blink"

Quoted 26 lines · Hong Xu
Quoted 23 lines · Sedat Dilek
Quoted 6 lines · Kalle Valo
Quoted 3 lines · Hong Xu

ath9k and ath9k_htc use the variable name "led_blink" to indicate whether the module parameter "blink" is on. This name is easy to conflict with other variables, thus they are renamed.

Quoted 7 lines

Please state clearly that this fixes a compiler error found by kbuild. Also it's very much recommended to add the actual compiler error to the commit log:

drivers/net/wireless/ath/ath9k/ath9k_htc.o:(.data+0x47c): multiple definition of `led_blink' drivers/net/wireless/ath/ath9k/ath9k.o:(.bss+0x20): first defined here

Some more things...

If you post a vN (referring to your v3) of a patch, please add a history of changes.

2nd, use the Fixes tag [2] in this case with a reference to the culprit commit [1].

Quoted 8 lines
Quoted 5 lines
Fixes 3a939a671225 ("ath9k_htc: Add a module parameter to disable blink")

Last but not least, honour credits like Reported-by/Tested-by (plus some infos like broken in Linux-next $VERSION).

Thanks for your comments, I should have been more careful with the guideline. I guess we shouldn't credit kbuild for reporting in this case?

Learning by doing :-).

Yesterday, I looked trough some Git trees offline and I saw a commit with kbuild-test-bot as "author", so... :-).
Not sure, if this is good, but I would prefer to put a note in the commit-body (some call it changelog, I refer to terms in email context)... "...found by kbuild test bot" with appropriate lines of warning.
( See example in [1]. )

- Sedat -

[1] http://www.spinics.net/lists/linux-usb/msg110760.html