* [PATCH] ath_hw: Use common REG_WRITE parameter order
@ 2012-10-04 16:05 Sven Eckelmann
2012-10-04 17:29 ` Luis R. Rodriguez
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Sven Eckelmann @ 2012-10-04 16:05 UTC (permalink / raw)
To: linux-wireless; +Cc: linville, mcgrof, Sven Eckelmann
All defines for REG_WRITE in Atheros wireless drivers use the order "ah",
"register" and "value". hw.c is the only file using the order "ah", "value" and
"register". This inconsistent definition can easily lead to implementation
errors.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
drivers/net/wireless/ath/hw.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/ath/hw.c b/drivers/net/wireless/ath/hw.c
index 19befb3..39e8a59 100644
--- a/drivers/net/wireless/ath/hw.c
+++ b/drivers/net/wireless/ath/hw.c
@@ -20,8 +20,8 @@
#include "ath.h"
#include "reg.h"
-#define REG_READ (common->ops->read)
-#define REG_WRITE (common->ops->write)
+#define REG_READ (common->ops->read)
+#define REG_WRITE(_ah, _reg, _val) (common->ops->write)(_ah, _val, _reg)
/**
* ath_hw_set_bssid_mask - filter out bssids we listen
@@ -119,8 +119,8 @@ void ath_hw_setbssidmask(struct ath_common *common)
{
void *ah = common->ah;
- REG_WRITE(ah, get_unaligned_le32(common->bssidmask), AR_BSSMSKL);
- REG_WRITE(ah, get_unaligned_le16(common->bssidmask + 4), AR_BSSMSKU);
+ REG_WRITE(ah, AR_BSSMSKL, get_unaligned_le32(common->bssidmask));
+ REG_WRITE(ah, AR_BSSMSKU, get_unaligned_le16(common->bssidmask + 4));
}
EXPORT_SYMBOL(ath_hw_setbssidmask);
@@ -139,7 +139,7 @@ void ath_hw_cycle_counters_update(struct ath_common *common)
void *ah = common->ah;
/* freeze */
- REG_WRITE(ah, AR_MIBC_FMC, AR_MIBC);
+ REG_WRITE(ah, AR_MIBC, AR_MIBC_FMC);
/* read */
cycles = REG_READ(ah, AR_CCCNT);
@@ -148,13 +148,13 @@ void ath_hw_cycle_counters_update(struct ath_common *common)
tx = REG_READ(ah, AR_TFCNT);
/* clear */
- REG_WRITE(ah, 0, AR_CCCNT);
- REG_WRITE(ah, 0, AR_RFCNT);
- REG_WRITE(ah, 0, AR_RCCNT);
- REG_WRITE(ah, 0, AR_TFCNT);
+ REG_WRITE(ah, AR_CCCNT, 0);
+ REG_WRITE(ah, AR_RFCNT, 0);
+ REG_WRITE(ah, AR_RCCNT, 0);
+ REG_WRITE(ah, AR_TFCNT, 0);
/* unfreeze */
- REG_WRITE(ah, 0, AR_MIBC);
+ REG_WRITE(ah, AR_MIBC, 0);
/* update all cycle counters here */
common->cc_ani.cycles += cycles;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] ath_hw: Use common REG_WRITE parameter order
2012-10-04 16:05 [PATCH] ath_hw: Use common REG_WRITE parameter order Sven Eckelmann
@ 2012-10-04 17:29 ` Luis R. Rodriguez
2012-10-04 17:42 ` Sven Eckelmann
2012-10-04 17:43 ` [PATCHv2] " Sven Eckelmann
2012-10-05 5:15 ` [PATCH] " Mohammed Shafi
2 siblings, 1 reply; 8+ messages in thread
From: Luis R. Rodriguez @ 2012-10-04 17:29 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: linux-wireless, linville
On Thu, Oct 4, 2012 at 9:05 AM, Sven Eckelmann <sven@narfation.org> wrote:
> All defines for REG_WRITE in Atheros wireless drivers use the order "ah",
> "register" and "value". hw.c is the only file using the order "ah", "value" and
> "register". This inconsistent definition can easily lead to implementation
> errors.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
Good catch! But can you resend specifying in your commit log that this
change is a no-op? Also it may help the reader for you to git grep and
show the other REG_WRITE() definitions on the other c files within the
commit log.
Luis
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ath_hw: Use common REG_WRITE parameter order
2012-10-04 17:29 ` Luis R. Rodriguez
@ 2012-10-04 17:42 ` Sven Eckelmann
2012-10-04 17:46 ` Luis R. Rodriguez
0 siblings, 1 reply; 8+ messages in thread
From: Sven Eckelmann @ 2012-10-04 17:42 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linux-wireless, linville
[-- Attachment #1: Type: text/plain, Size: 771 bytes --]
On Thursday 04 October 2012 10:29:59 Luis R. Rodriguez wrote:
> On Thu, Oct 4, 2012 at 9:05 AM, Sven Eckelmann <sven@narfation.org> wrote:
> > All defines for REG_WRITE in Atheros wireless drivers use the order "ah",
> > "register" and "value". hw.c is the only file using the order "ah",
> > "value" and "register". This inconsistent definition can easily lead to
> > implementation errors.
> >
> > Signed-off-by: Sven Eckelmann <sven@narfation.org>
>
> Good catch! But can you resend specifying in your commit log that this
> change is a no-op?
I really don't know what you want from me here.
> Also it may help the reader for you to git grep and
> show the other REG_WRITE() definitions on the other c files within the
> commit log.
Ehrm, ok.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ath_hw: Use common REG_WRITE parameter order
2012-10-04 17:42 ` Sven Eckelmann
@ 2012-10-04 17:46 ` Luis R. Rodriguez
0 siblings, 0 replies; 8+ messages in thread
From: Luis R. Rodriguez @ 2012-10-04 17:46 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: linux-wireless, linville
On Thu, Oct 4, 2012 at 10:42 AM, Sven Eckelmann <sven@narfation.org> wrote:
> On Thursday 04 October 2012 10:29:59 Luis R. Rodriguez wrote:
>> On Thu, Oct 4, 2012 at 9:05 AM, Sven Eckelmann <sven@narfation.org> wrote:
>> > All defines for REG_WRITE in Atheros wireless drivers use the order "ah",
>> > "register" and "value". hw.c is the only file using the order "ah",
>> > "value" and "register". This inconsistent definition can easily lead to
>> > implementation errors.
>> >
>> > Signed-off-by: Sven Eckelmann <sven@narfation.org>
>>
>> Good catch! But can you resend specifying in your commit log that this
>> change is a no-op?
>
> I really don't know what you want from me here.
Just specify in the commit log that this change causes no functional changes.
Luis
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCHv2] ath_hw: Use common REG_WRITE parameter order
2012-10-04 16:05 [PATCH] ath_hw: Use common REG_WRITE parameter order Sven Eckelmann
2012-10-04 17:29 ` Luis R. Rodriguez
@ 2012-10-04 17:43 ` Sven Eckelmann
2012-10-04 20:35 ` Luis R. Rodriguez
2012-10-05 16:29 ` Arend van Spriel
2012-10-05 5:15 ` [PATCH] " Mohammed Shafi
2 siblings, 2 replies; 8+ messages in thread
From: Sven Eckelmann @ 2012-10-04 17:43 UTC (permalink / raw)
To: linux-wireless; +Cc: linville, mcgrof, Sven Eckelmann, Simon Wunderlich
All defines for REG_WRITE in Atheros wireless drivers use the order "ah",
"register" and "value". hw.c is the only file using the order "ah", "value" and
"register".
drivers/net/wireless/ath/ath9k/hw.h:#define REG_WRITE(_ah, _reg, _val) \
drivers/net/wireless/ath/key.c:#define REG_WRITE(_ah, _reg, _val) (common->ops->write)(_ah, _val, _reg)
This inconsistent definition can easily lead to implementation errors. The
modification doesn't change the behavior of the driver or the generated code.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
Changed commit message
drivers/net/wireless/ath/hw.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/ath/hw.c b/drivers/net/wireless/ath/hw.c
index 19befb3..39e8a59 100644
--- a/drivers/net/wireless/ath/hw.c
+++ b/drivers/net/wireless/ath/hw.c
@@ -20,8 +20,8 @@
#include "ath.h"
#include "reg.h"
-#define REG_READ (common->ops->read)
-#define REG_WRITE (common->ops->write)
+#define REG_READ (common->ops->read)
+#define REG_WRITE(_ah, _reg, _val) (common->ops->write)(_ah, _val, _reg)
/**
* ath_hw_set_bssid_mask - filter out bssids we listen
@@ -119,8 +119,8 @@ void ath_hw_setbssidmask(struct ath_common *common)
{
void *ah = common->ah;
- REG_WRITE(ah, get_unaligned_le32(common->bssidmask), AR_BSSMSKL);
- REG_WRITE(ah, get_unaligned_le16(common->bssidmask + 4), AR_BSSMSKU);
+ REG_WRITE(ah, AR_BSSMSKL, get_unaligned_le32(common->bssidmask));
+ REG_WRITE(ah, AR_BSSMSKU, get_unaligned_le16(common->bssidmask + 4));
}
EXPORT_SYMBOL(ath_hw_setbssidmask);
@@ -139,7 +139,7 @@ void ath_hw_cycle_counters_update(struct ath_common *common)
void *ah = common->ah;
/* freeze */
- REG_WRITE(ah, AR_MIBC_FMC, AR_MIBC);
+ REG_WRITE(ah, AR_MIBC, AR_MIBC_FMC);
/* read */
cycles = REG_READ(ah, AR_CCCNT);
@@ -148,13 +148,13 @@ void ath_hw_cycle_counters_update(struct ath_common *common)
tx = REG_READ(ah, AR_TFCNT);
/* clear */
- REG_WRITE(ah, 0, AR_CCCNT);
- REG_WRITE(ah, 0, AR_RFCNT);
- REG_WRITE(ah, 0, AR_RCCNT);
- REG_WRITE(ah, 0, AR_TFCNT);
+ REG_WRITE(ah, AR_CCCNT, 0);
+ REG_WRITE(ah, AR_RFCNT, 0);
+ REG_WRITE(ah, AR_RCCNT, 0);
+ REG_WRITE(ah, AR_TFCNT, 0);
/* unfreeze */
- REG_WRITE(ah, 0, AR_MIBC);
+ REG_WRITE(ah, AR_MIBC, 0);
/* update all cycle counters here */
common->cc_ani.cycles += cycles;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCHv2] ath_hw: Use common REG_WRITE parameter order
2012-10-04 17:43 ` [PATCHv2] " Sven Eckelmann
@ 2012-10-04 20:35 ` Luis R. Rodriguez
2012-10-05 16:29 ` Arend van Spriel
1 sibling, 0 replies; 8+ messages in thread
From: Luis R. Rodriguez @ 2012-10-04 20:35 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: linux-wireless, linville, rodrigue, Simon Wunderlich
On Thu, Oct 04, 2012 at 07:43:15PM +0200, Sven Eckelmann wrote:
> All defines for REG_WRITE in Atheros wireless drivers use the order "ah",
> "register" and "value". hw.c is the only file using the order "ah", "value" and
> "register".
>
> drivers/net/wireless/ath/ath9k/hw.h:#define REG_WRITE(_ah, _reg, _val) \
> drivers/net/wireless/ath/key.c:#define REG_WRITE(_ah, _reg, _val) (common->ops->write)(_ah, _val, _reg)
>
> This inconsistent definition can easily lead to implementation errors. The
> modification doesn't change the behavior of the driver or the generated code.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Acked-by: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>
Luis
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv2] ath_hw: Use common REG_WRITE parameter order
2012-10-04 17:43 ` [PATCHv2] " Sven Eckelmann
2012-10-04 20:35 ` Luis R. Rodriguez
@ 2012-10-05 16:29 ` Arend van Spriel
1 sibling, 0 replies; 8+ messages in thread
From: Arend van Spriel @ 2012-10-05 16:29 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: linux-wireless, linville, mcgrof, Simon Wunderlich
On 10/04/2012 07:43 PM, Sven Eckelmann wrote:
> All defines for REG_WRITE in Atheros wireless drivers use the order "ah",
> "register" and "value". hw.c is the only file using the order "ah", "value" and
> "register".
>
> drivers/net/wireless/ath/ath9k/hw.h:#define REG_WRITE(_ah, _reg, _val) \
Just to bad you did not catch the next line in hw.h.
Gr. AvS
> drivers/net/wireless/ath/key.c:#define REG_WRITE(_ah, _reg, _val) (common->ops->write)(_ah, _val, _reg)
>
> This inconsistent definition can easily lead to implementation errors. The
> modification doesn't change the behavior of the driver or the generated code.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> ---
> Changed commit message
>
> drivers/net/wireless/ath/hw.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ath_hw: Use common REG_WRITE parameter order
2012-10-04 16:05 [PATCH] ath_hw: Use common REG_WRITE parameter order Sven Eckelmann
2012-10-04 17:29 ` Luis R. Rodriguez
2012-10-04 17:43 ` [PATCHv2] " Sven Eckelmann
@ 2012-10-05 5:15 ` Mohammed Shafi
2 siblings, 0 replies; 8+ messages in thread
From: Mohammed Shafi @ 2012-10-05 5:15 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: linux-wireless, linville, mcgrof
On Thu, Oct 4, 2012 at 9:35 PM, Sven Eckelmann <sven@narfation.org> wrote:
> All defines for REG_WRITE in Atheros wireless drivers use the order "ah",
> "register" and "value". hw.c is the only file using the order "ah", "value" and
> "register". This inconsistent definition can easily lead to implementation
> errors.
oh yes, once Raj and myself were reviewing cycle counters stuff and
was intially confused
with the slight variance in the REG_WRITE definition in ath9k/hw.h,
and ath/hw.c.
thanks to you now, as it was gone and make our life easier.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> drivers/net/wireless/ath/hw.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/hw.c b/drivers/net/wireless/ath/hw.c
> index 19befb3..39e8a59 100644
> --- a/drivers/net/wireless/ath/hw.c
> +++ b/drivers/net/wireless/ath/hw.c
> @@ -20,8 +20,8 @@
> #include "ath.h"
> #include "reg.h"
>
> -#define REG_READ (common->ops->read)
> -#define REG_WRITE (common->ops->write)
> +#define REG_READ (common->ops->read)
> +#define REG_WRITE(_ah, _reg, _val) (common->ops->write)(_ah, _val, _reg)
>
> /**
> * ath_hw_set_bssid_mask - filter out bssids we listen
> @@ -119,8 +119,8 @@ void ath_hw_setbssidmask(struct ath_common *common)
> {
> void *ah = common->ah;
>
> - REG_WRITE(ah, get_unaligned_le32(common->bssidmask), AR_BSSMSKL);
> - REG_WRITE(ah, get_unaligned_le16(common->bssidmask + 4), AR_BSSMSKU);
> + REG_WRITE(ah, AR_BSSMSKL, get_unaligned_le32(common->bssidmask));
> + REG_WRITE(ah, AR_BSSMSKU, get_unaligned_le16(common->bssidmask + 4));
> }
> EXPORT_SYMBOL(ath_hw_setbssidmask);
>
> @@ -139,7 +139,7 @@ void ath_hw_cycle_counters_update(struct ath_common *common)
> void *ah = common->ah;
>
> /* freeze */
> - REG_WRITE(ah, AR_MIBC_FMC, AR_MIBC);
> + REG_WRITE(ah, AR_MIBC, AR_MIBC_FMC);
>
> /* read */
> cycles = REG_READ(ah, AR_CCCNT);
> @@ -148,13 +148,13 @@ void ath_hw_cycle_counters_update(struct ath_common *common)
> tx = REG_READ(ah, AR_TFCNT);
>
> /* clear */
> - REG_WRITE(ah, 0, AR_CCCNT);
> - REG_WRITE(ah, 0, AR_RFCNT);
> - REG_WRITE(ah, 0, AR_RCCNT);
> - REG_WRITE(ah, 0, AR_TFCNT);
> + REG_WRITE(ah, AR_CCCNT, 0);
> + REG_WRITE(ah, AR_RFCNT, 0);
> + REG_WRITE(ah, AR_RCCNT, 0);
> + REG_WRITE(ah, AR_TFCNT, 0);
>
> /* unfreeze */
> - REG_WRITE(ah, 0, AR_MIBC);
> + REG_WRITE(ah, AR_MIBC, 0);
>
> /* update all cycle counters here */
> common->cc_ani.cycles += cycles;
> --
> 1.7.10.4
>
> --
> 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
--
thanks,
shafi
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-10-05 16:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-04 16:05 [PATCH] ath_hw: Use common REG_WRITE parameter order Sven Eckelmann
2012-10-04 17:29 ` Luis R. Rodriguez
2012-10-04 17:42 ` Sven Eckelmann
2012-10-04 17:46 ` Luis R. Rodriguez
2012-10-04 17:43 ` [PATCHv2] " Sven Eckelmann
2012-10-04 20:35 ` Luis R. Rodriguez
2012-10-05 16:29 ` Arend van Spriel
2012-10-05 5:15 ` [PATCH] " Mohammed Shafi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).