2012-03-12 05:57:42

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: [RFC] ath9k_hw: Fix chip revision checks

From: Mohammed Shafi Shajakhan <[email protected]>

not sure if these checks are previously avoided may be those revision of
chipsets are obselete ?

Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
---
drivers/net/wireless/ath/ath9k/reg.h | 20 +++++++++++++++-----
1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/reg.h b/drivers/net/wireless/ath/ath9k/reg.h
index 80b1856..55e3513 100644
--- a/drivers/net/wireless/ath/ath9k/reg.h
+++ b/drivers/net/wireless/ath/ath9k/reg.h
@@ -821,20 +821,28 @@
((_ah)->hw_version.macRev == AR_SREV_REVISION_9160_11))
#define AR_SREV_9280(_ah) \
(((_ah)->hw_version.macVersion == AR_SREV_VERSION_9280))
-#define AR_SREV_9280_20_OR_LATER(_ah) \
- (((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9280))
#define AR_SREV_9280_20(_ah) \
(((_ah)->hw_version.macVersion == AR_SREV_VERSION_9280))
+#define AR_SREV_9280_10_OR_LATER(_ah) \
+ (((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9280))
+#define AR_SREV_9280_20_OR_LATER(_ah) \
+ (((_ah)->hw_version.macVersion > AR_SREV_VERSION_9280) || \
+ (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9280) && \
+ ((_ah)->hw_version.macRev >= AR_SREV_REVISION_9280_20)))

#define AR_SREV_9285(_ah) \
(((_ah)->hw_version.macVersion == AR_SREV_VERSION_9285))
#define AR_SREV_9285_12_OR_LATER(_ah) \
- (((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9285))
+ (((_ah)->hw_version.macVersion > AR_SREV_VERSION_9285) || \
+ (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9285) && \
+ ((_ah)->hw_version.macRev >= AR_SREV_REVISION_9285_12)))

#define AR_SREV_9287(_ah) \
(((_ah)->hw_version.macVersion == AR_SREV_VERSION_9287))
#define AR_SREV_9287_11_OR_LATER(_ah) \
- (((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9287))
+ (((_ah)->hw_version.macVersion > AR_SREV_VERSION_9287) || \
+ (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9287) && \
+ ((_ah)->hw_version.macRev >= AR_SREV_REVISION_9285_11)))
#define AR_SREV_9287_11(_ah) \
(((_ah)->hw_version.macVersion == AR_SREV_VERSION_9287) && \
((_ah)->hw_version.macRev == AR_SREV_REVISION_9287_11))
@@ -862,7 +870,9 @@
#define AR_SREV_9300(_ah) \
(((_ah)->hw_version.macVersion == AR_SREV_VERSION_9300))
#define AR_SREV_9300_20_OR_LATER(_ah) \
- ((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9300)
+ (((_ah)->hw_version.macVersion > AR_SREV_VERSION_9300) || \
+ (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9300) && \
+ ((_ah)->hw_version.macRev >= AR_SREV_REVISION_9300_20)))

#define AR_SREV_9330(_ah) \
(((_ah)->hw_version.macVersion == AR_SREV_VERSION_9330))
--
1.7.0.4



2012-03-12 09:41:13

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [ath9k-devel] [RFC] ath9k_hw: Fix chip revision checks


> while adding some new code while AR_9280_SREV_20 later was not in
> uniform with the other chipset macros. i should have just checked with
> git annotate reg.h could have found your commit.

Felix had said this could be better done by using AR_SREV_9280_OR_LATER
macro with the corresponding changes to be effect in the hardware code.
thanks.

--
thanks,
shafi

2012-03-13 04:36:08

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [ath9k-devel] [RFC] ath9k_hw: Fix chip revision checks

Hi Luis,

On Tuesday 13 March 2012 01:46 AM, Luis R. Rodriguez wrote:
> On Mon, Mar 12, 2012 at 02:46:04PM +0530, Mohammed Shafi Shajakhan wrote:
>> Hi Felix,
>>
>> On Monday 12 March 2012 02:32 PM, Felix Fietkau wrote:
>>> On 2012-03-12 6:57 AM, Mohammed Shafi Shajakhan wrote:
>>>> From: Mohammed Shafi Shajakhan<[email protected]>
>>>>
>>>> not sure if these checks are previously avoided may be those revision of
>>>> chipsets are obselete ?
>>> NACK. The extra checks that this patch adds have been intentionally
>>> removed, since all earlier versions were never sold and thus do not need
>>> to be considered. This simplifies the generated binary code.
>>
>> IMHO i don't think this patch does anything wrong to deserve a NACK!
>> sometimes these optimizations make it tad difficult if we want to
>> quickly check with the HAL code.
>
> "HAL" code from internal codebases need to change, not the other
> way around. You have your priorities wrong. I support the NACK.
>

we have checks like this

case 1.#define AR_SREV_9280_20_OR_LATER(_ah) \
(((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9280))

case 2. #define AR_SREV_9485_OR_LATER(_ah) \
(((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9485))


case 3. #define AR_SREV_9287_13_OR_LATER(_ah) \
(((_ah)->hw_version.macVersion > AR_SREV_VERSION_9287) || \
(((_ah)->hw_version.macVersion == AR_SREV_VERSION_9287) && \
((_ah)->hw_version.macRev >= AR_SREV_REVISION_9287_13)))

it made be bit confused and i was just adding some hardware related
changes, let me do accept i missed to see why the check for AR9280 is
like that. i just thought of making the changes in sync with the other
macros, also thats why sent an RFC too

Felix suggested a better solution would be

#define AR_SREV_9280_OR_LATER(_ah) \
(((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9280))

instead of the older one (or) what my patch does

#define AR_SREV_9280_20_OR_LATER(_ah) \
(((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9280))

and make corresponding changes in the hardware code.

thanks!


--
thanks,
shafi

2012-03-12 09:02:17

by Felix Fietkau

[permalink] [raw]
Subject: Re: [ath9k-devel] [RFC] ath9k_hw: Fix chip revision checks

On 2012-03-12 6:57 AM, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <[email protected]>
>
> not sure if these checks are previously avoided may be those revision of
> chipsets are obselete ?
NACK. The extra checks that this patch adds have been intentionally
removed, since all earlier versions were never sold and thus do not need
to be considered. This simplifies the generated binary code.

- Felix

2012-03-12 09:16:10

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [ath9k-devel] [RFC] ath9k_hw: Fix chip revision checks

Hi Felix,

On Monday 12 March 2012 02:32 PM, Felix Fietkau wrote:
> On 2012-03-12 6:57 AM, Mohammed Shafi Shajakhan wrote:
>> From: Mohammed Shafi Shajakhan<[email protected]>
>>
>> not sure if these checks are previously avoided may be those revision of
>> chipsets are obselete ?
> NACK. The extra checks that this patch adds have been intentionally
> removed, since all earlier versions were never sold and thus do not need
> to be considered. This simplifies the generated binary code.

IMHO i don't think this patch does anything wrong to deserve a NACK!
sometimes these optimizations make it tad difficult if we want to
quickly check with the HAL code.

true i just found from the commit

ath9k_hw: simplify revision checks for AR9280

Since AR9280 v1.0 was never sold (and the initvals removed), v1.0
specific
revision checks can be removed and the 'v2.0 or later' check can be
simplified to a check for AR9280 or later.

while adding some new code while AR_9280_SREV_20 later was not in
uniform with the other chipset macros. i should have just checked with
git annotate reg.h could have found your commit.

thanks for the review!

>
> - Felix


--
thanks,
shafi

2012-03-12 09:44:47

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [RFC] ath9k_hw: Fix chip revision checks

On Monday 12 March 2012 11:27 AM, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan<[email protected]>
>
> not sure if these checks are previously avoided may be those revision of
> chipsets are obselete ?
>
> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/reg.h | 20 +++++++++++++++-----
> 1 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/reg.h b/drivers/net/wireless/ath/ath9k/reg.h
> index 80b1856..55e3513 100644
> --- a/drivers/net/wireless/ath/ath9k/reg.h
> +++ b/drivers/net/wireless/ath/ath9k/reg.h
> @@ -821,20 +821,28 @@
> ((_ah)->hw_version.macRev == AR_SREV_REVISION_9160_11))
> #define AR_SREV_9280(_ah) \
> (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9280))
> -#define AR_SREV_9280_20_OR_LATER(_ah) \
> - (((_ah)->hw_version.macVersion>= AR_SREV_VERSION_9280))
> #define AR_SREV_9280_20(_ah) \
> (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9280))
> +#define AR_SREV_9280_10_OR_LATER(_ah) \
> + (((_ah)->hw_version.macVersion>= AR_SREV_VERSION_9280))
> +#define AR_SREV_9280_20_OR_LATER(_ah) \
> + (((_ah)->hw_version.macVersion> AR_SREV_VERSION_9280) || \
> + (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9280)&& \
> + ((_ah)->hw_version.macRev>= AR_SREV_REVISION_9280_20)))
>
> #define AR_SREV_9285(_ah) \
> (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9285))
> #define AR_SREV_9285_12_OR_LATER(_ah) \
> - (((_ah)->hw_version.macVersion>= AR_SREV_VERSION_9285))
> + (((_ah)->hw_version.macVersion> AR_SREV_VERSION_9285) || \
> + (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9285)&& \
> + ((_ah)->hw_version.macRev>= AR_SREV_REVISION_9285_12)))
>
> #define AR_SREV_9287(_ah) \
> (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9287))
> #define AR_SREV_9287_11_OR_LATER(_ah) \
> - (((_ah)->hw_version.macVersion>= AR_SREV_VERSION_9287))
> + (((_ah)->hw_version.macVersion> AR_SREV_VERSION_9287) || \
> + (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9287)&& \
> + ((_ah)->hw_version.macRev>= AR_SREV_REVISION_9285_11)))

oops, also i made a mistake here, it should be AR_SREV_REVISION_9287_11


> #define AR_SREV_9287_11(_ah) \
> (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9287)&& \
> ((_ah)->hw_version.macRev == AR_SREV_REVISION_9287_11))
> @@ -862,7 +870,9 @@
> #define AR_SREV_9300(_ah) \
> (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9300))
> #define AR_SREV_9300_20_OR_LATER(_ah) \
> - ((_ah)->hw_version.macVersion>= AR_SREV_VERSION_9300)
> + (((_ah)->hw_version.macVersion> AR_SREV_VERSION_9300) || \
> + (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9300)&& \
> + ((_ah)->hw_version.macRev>= AR_SREV_REVISION_9300_20)))
>
> #define AR_SREV_9330(_ah) \
> (((_ah)->hw_version.macVersion == AR_SREV_VERSION_9330))


--
thanks,
shafi

2012-03-12 20:16:57

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath9k-devel] [RFC] ath9k_hw: Fix chip revision checks

On Mon, Mar 12, 2012 at 02:46:04PM +0530, Mohammed Shafi Shajakhan wrote:
> Hi Felix,
>
> On Monday 12 March 2012 02:32 PM, Felix Fietkau wrote:
> >On 2012-03-12 6:57 AM, Mohammed Shafi Shajakhan wrote:
> >>From: Mohammed Shafi Shajakhan<[email protected]>
> >>
> >>not sure if these checks are previously avoided may be those revision of
> >>chipsets are obselete ?
> >NACK. The extra checks that this patch adds have been intentionally
> >removed, since all earlier versions were never sold and thus do not need
> >to be considered. This simplifies the generated binary code.
>
> IMHO i don't think this patch does anything wrong to deserve a NACK!
> sometimes these optimizations make it tad difficult if we want to
> quickly check with the HAL code.

"HAL" code from internal codebases need to change, not the other
way around. You have your priorities wrong. I support the NACK.

Luis