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 <[email protected]>
---
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
On Thu, Oct 4, 2012 at 9:05 AM, Sven Eckelmann <[email protected]> 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 <[email protected]>
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
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 <[email protected]>
> Signed-off-by: Simon Wunderlich <[email protected]>
> ---
> Changed commit message
>
> drivers/net/wireless/ath/hw.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
On Thursday 04 October 2012 10:29:59 Luis R. Rodriguez wrote:
> On Thu, Oct 4, 2012 at 9:05 AM, Sven Eckelmann <[email protected]> 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 <[email protected]>
>
> 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
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 <[email protected]>
Signed-off-by: Simon Wunderlich <[email protected]>
---
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
On Thu, Oct 4, 2012 at 9:35 PM, Sven Eckelmann <[email protected]> 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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
thanks,
shafi
On Thu, Oct 4, 2012 at 10:42 AM, Sven Eckelmann <[email protected]> wrote:
> On Thursday 04 October 2012 10:29:59 Luis R. Rodriguez wrote:
>> On Thu, Oct 4, 2012 at 9:05 AM, Sven Eckelmann <[email protected]> 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 <[email protected]>
>>
>> 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
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 <[email protected]>
> Signed-off-by: Simon Wunderlich <[email protected]>
Acked-by: Luis R. Rodriguez <[email protected]>
Luis