[PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[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.

Signed-off-by: Hong Xu <hong@topbug.net>
---
 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(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 1a9fe09..e84b576 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -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;
 
diff --git a/drivers/net/wireless/ath/ath9k/gpio.c b/drivers/net/wireless/ath/ath9k/gpio.c
index 2fef7a4..da344b2 100644
--- a/drivers/net/wireless/ath/ath9k/gpio.c
+++ b/drivers/net/wireless/ath/ath9k/gpio.c
@@ -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);
 
diff --git a/drivers/net/wireless/ath/ath9k/htc.h b/drivers/net/wireless/ath/ath9k/htc.h
index c43fec5..300d367 100644
--- a/drivers/net/wireless/ath/ath9k/htc.h
+++ b/drivers/net/wireless/ath/ath9k/htc.h
@@ -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 {
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c b/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c
index 998b558..2aabcbd 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c
@@ -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);
 
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index 9470127..fd22940 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -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[] = {
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index d1c3934..2aef14e 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -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;
-- 
2.2.2

Re: [PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[Kalle Valo]

Hong Xu <hong@topbug.net> writes:

> 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

-- 
Kalle Valo

Re: [PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[Sedat Dilek]

On Sat, Jan 24, 2015 at 11:23 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
> Hong Xu <hong@topbug.net> writes:
>
>> 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
>

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

Re: [PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[Oleksij Rempel]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

Am 24.01.2015 um 11:33 schrieb Sedat Dilek:
> On Sat, Jan 24, 2015 at 11:23 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Hong Xu <hong@topbug.net> writes:
>>
>>> 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
>>
> 
> 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.

> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Regards,
Oleksij


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: [PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[Kalle Valo]

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

Sedat Dilek <sedat.dilek@gmail.com> writes:

> 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.

-- 
Kalle Valo

Re: [PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[Sedat Dilek]

On Sat, Jan 24, 2015 at 11:38 AM, Oleksij Rempel <linux@rempel-privat.de> wrote:
> Am 24.01.2015 um 11:33 schrieb Sedat Dilek:
>> On Sat, Jan 24, 2015 at 11:23 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Hong Xu <hong@topbug.net> writes:
>>>
>>>> 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
>>>
>>
>> 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 -

Re: [PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[Sedat Dilek]

On Sat, Jan 24, 2015 at 11:39 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
> (I'll drop ath9k-devel as someone from there seems to be spamming with
> vacation responses)
>
> Sedat Dilek <sedat.dilek@gmail.com> writes:
>
>> 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.
>

/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 -

Re: [PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[Sedat Dilek]

[...]
> 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

Re: [PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[Hong Xu]

[-- Attachment #1: Type: text/plain, Size: 1258 bytes --]


Sedat Dilek <sedat.dilek@gmail.com> writes:

> On Sat, Jan 24, 2015 at 11:23 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Hong Xu <hong@topbug.net> writes:
>>
>>> 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
>>
>
> 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).
>
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] ath9k and ath9k_htc: rename variable "led_blink"

[Sedat Dilek]

On Sat, Jan 24, 2015 at 12:10 PM, Hong Xu <hong@topbug.net> wrote:
>
> Sedat Dilek <sedat.dilek@gmail.com> writes:
>
>> On Sat, Jan 24, 2015 at 11:23 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Hong Xu <hong@topbug.net> writes:
>>>
>>>> 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
>>>
>>
>> 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).
>>
> 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