[RFC] ath9k_hw: Fix chip revision checks

[RFC] ath9k_hw: Fix chip revision checks

[Mohammed Shafi Shajakhan]

From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>

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

Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
---
 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

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

[Felix Fietkau]

On 2012-03-12 6:57 AM, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
> 
> 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

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

[Mohammed Shafi Shajakhan]

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<mohammed@qca.qualcomm.com>
>>
>> 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

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

[Mohammed Shafi Shajakhan]

> 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

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

[Luis R. Rodriguez]

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<mohammed@qca.qualcomm.com>
> >>
> >>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

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

[Mohammed Shafi Shajakhan]

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<mohammed@qca.qualcomm.com>
>>>>
>>>> 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

Re: [RFC] ath9k_hw: Fix chip revision checks

[Mohammed Shafi Shajakhan]

On Monday 12 March 2012 11:27 AM, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan<mohammed@qca.qualcomm.com>
>
> not sure if these checks are previously avoided may be those revision of
> chipsets are obselete ?
>
> Signed-off-by: Mohammed Shafi Shajakhan<mohammed@qca.qualcomm.com>
> ---
>   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