[ath9k-devel] eeprom txGainType overridden if minor version < 19

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Marek Lindner]

Hi,

I was having some output power problems with a device using AR7240/AR9285. 
After digging into the ath9k driver I realized that it would override the 
power settings (txGainType) configured in the eeprom if the minor version does 
not meet its expectations. This is why it would not use the appropriate high 
power settings. The offending lines from eeprom_4k.c:

if (ver_minor >= AR5416_EEP_MINOR_VER_19)
return pBase->txGainType;
else
return AR5416_EEP_TXGAIN_ORIGINAL;

My minor version is 14 and consequently fell through the cracks. The commit 
introducing the check in question was 970bf9d40c03e48cc34ee2c1a70693a0e0fca3f6 
but it does not explain why the minor version has to be 19 or higher. Does 
anyone know the magic behind version 19 ? I could easily post a patch to 
remove that check if it causes no harm.

Regards,
Marek

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Adrian Chadd]

Well, what TX gain type is in your eeprom?


Adrian

On 2 August 2011 16:28, Marek Lindner <lindner_marek@yahoo.de> wrote:
>
> Hi,
>
> I was having some output power problems with a device using AR7240/AR9285.
> After digging into the ath9k driver I realized that it would override the
> power settings (txGainType) configured in the eeprom if the minor version does
> not meet its expectations. This is why it would not use the appropriate high
> power settings. The offending lines from eeprom_4k.c:
>
> if (ver_minor >= AR5416_EEP_MINOR_VER_19)
> return pBase->txGainType;
> else
> return AR5416_EEP_TXGAIN_ORIGINAL;
>
> My minor version is 14 and consequently fell through the cracks. The commit
> introducing the check in question was 970bf9d40c03e48cc34ee2c1a70693a0e0fca3f6
> but it does not explain why the minor version has to be 19 or higher. Does
> anyone know the magic behind version 19 ? I could easily post a patch to
> remove that check if it causes no harm.
>
> Regards,
> Marek
> _______________________________________________
> ath9k-devel mailing list
> ath9k-devel at lists.ath9k.org
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Marek Lindner]

On Tuesday, August 02, 2011 10:59:30 Adrian Chadd wrote:
> Well, what TX gain type is in your eeprom?

TX gain is 1 whereas the function returns 0 (AR5416_EEP_TXGAIN_ORIGINAL).


Regards,
Marek

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Adrian Chadd]

On 2 August 2011 17:03, Marek Lindner <lindner_marek@yahoo.de> wrote:
> On Tuesday, August 02, 2011 10:59:30 Adrian Chadd wrote:
>> Well, what TX gain type is in your eeprom?
>
> TX gain is 1 whereas the function returns 0 (AR5416_EEP_TXGAIN_ORIGINAL).

let's start at the beginning:

* Which NIC is this
* What's the datasheet say
* What does windows say/do ?



Adrian

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Marek Lindner]

On Tuesday, August 02, 2011 12:38:13 Adrian Chadd wrote:
> * Which NIC is this

As I said in my initial email: AR7240/AR9285. If you require more details let 
me know how to retrieve it.


> * What's the datasheet say

I don't have access to the Atheros AR9285 datasheet.


> * What does windows say/do ?

Windows does not run on this router.

Regards,
Marek

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Adrian Chadd]

Right, so it's an embedded board inside a wireless router? Which router? :)


Adrian

On 2 August 2011 18:48, Marek Lindner <lindner_marek@yahoo.de> wrote:
> On Tuesday, August 02, 2011 12:38:13 Adrian Chadd wrote:
>> * Which NIC is this
>
> As I said in my initial email: AR7240/AR9285. If you require more details let
> me know how to retrieve it.
>
>
>> * What's the datasheet say
>
> I don't have access to the Atheros AR9285 datasheet.
>
>
>> * What does windows say/do ?
>
> Windows does not run on this router.
>
> Regards,
> Marek
>

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Marek Lindner]

On Tuesday, August 02, 2011 13:13:27 Adrian Chadd wrote:
> Right, so it's an embedded board inside a wireless router? Which router? :)

The OM2P but how is this going to help ? I have the feeling we are not getting 
closer to answer my question: Why does ath9k_hw_4k_get_eeprom() requires the 
minor version 19 or above before even considering to read the eeprom value ?
Can I propose a patch that removes that check ? I already have that patch 
working for me - just wanted to be sure it does no harm.


Regards,
Marek

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Mohammed Shafi]

On Tue, Aug 2, 2011 at 8:07 PM, Marek Lindner <lindner_marek@yahoo.de> wrote:
> On Tuesday, August 02, 2011 13:13:27 Adrian Chadd wrote:
>> Right, so it's an embedded board inside a wireless router? Which router? :)
>
> The OM2P but how is this going to help ? I have the feeling we are not getting
> closer to answer my question: Why does ath9k_hw_4k_get_eeprom() requires the
> minor version 19 or above before even considering to read the eeprom value ?
> Can I propose a patch that removes that check ? I already have that patch
> working for me - just wanted to be sure it does no harm.

hi,

yes, looks like the check should not be there for common, but I don't
know for what reason the developer made it as a common check. let me
check it out.  please wait. does any one has any comments.  can you
please share what is the issue you are issuing


>
>
> Regards,
> Marek
> _______________________________________________
> ath9k-devel mailing list
> ath9k-devel at lists.ath9k.org
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>



-- 
shafi

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Mohammed Shafi]

On Tue, Aug 2, 2011 at 8:36 PM, Mohammed Shafi <shafi.wireless@gmail.com> wrote:
> On Tue, Aug 2, 2011 at 8:07 PM, Marek Lindner <lindner_marek@yahoo.de> wrote:
>> On Tuesday, August 02, 2011 13:13:27 Adrian Chadd wrote:
>>> Right, so it's an embedded board inside a wireless router? Which router? :)
>>
>> The OM2P but how is this going to help ? I have the feeling we are not getting
>> closer to answer my question: Why does ath9k_hw_4k_get_eeprom() requires the
>> minor version 19 or above before even considering to read the eeprom value ?
>> Can I propose a patch that removes that check ? I already have that patch
>> working for me - just wanted to be sure it does no harm.
>
> hi,
>
> yes, looks like the check should not be there for common, but I don't
> know for what reason the developer made it as a common check. let me
> check it out. ?please wait. does any one has any comments. ?can you
> please share what is the issue you are issuing


adding the people who made those changes, I am also unable to
understand. verified with the internal code base, the check was not
there.

>
>
>>
>>
>> Regards,
>> Marek
>> _______________________________________________
>> ath9k-devel mailing list
>> ath9k-devel at lists.ath9k.org
>> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>>
>
>
>
> --
> shafi
>



-- 
shafi

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Gabor Juhos]

2011.08.02. 17:37 keltez?ssel, Mohammed Shafi ?rta:
> On Tue, Aug 2, 2011 at 8:36 PM, Mohammed Shafi <shafi.wireless@gmail.com> wrote:
>> On Tue, Aug 2, 2011 at 8:07 PM, Marek Lindner <lindner_marek@yahoo.de> wrote:
>>> On Tuesday, August 02, 2011 13:13:27 Adrian Chadd wrote:
>>>> Right, so it's an embedded board inside a wireless router? Which router? :)
>>>
>>> The OM2P but how is this going to help ? I have the feeling we are not getting
>>> closer to answer my question: Why does ath9k_hw_4k_get_eeprom() requires the
>>> minor version 19 or above before even considering to read the eeprom value ?
>>> Can I propose a patch that removes that check ? I already have that patch
>>> working for me - just wanted to be sure it does no harm.
>>
>> hi,
>>
>> yes, looks like the check should not be there for common, but I don't
>> know for what reason the developer made it as a common check. let me
>> check it out.  please wait. does any one has any comments.  can you
>> please share what is the issue you are issuing
> 
> 
> adding the people who made those changes, I am also unable to
> understand. verified with the internal code base, the check was not
> there.

I had no access to the internal code base when I have added that code, so i was
unable to verify that the version check is required or not for the 4K EEPROMs.
To be on the safe side, I have used the same check which was present in the
generic eeprom code.

-Gabor

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Mohammed Shafi]

>
> I had no access to the internal code base when I have added that code, so i was
> unable to verify that the version check is required or not for the 4K EEPROMs.
> To be on the safe side, I have used the same check which was present in the
> generic eeprom code.

Gabor, thanks for your reply.


-- 
shafi

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Adrian Chadd]

Hm, I just checked the reference code and it doesn't at all look like
that check is actually needed; it's only needed for Merlin.

So, good catch!

I'll fix FreeBSD; who wants to fix ath9k? :)


Adrian

On 3 August 2011 12:59, Mohammed Shafi <shafi.wireless@gmail.com> wrote:
>>
>> I had no access to the internal code base when I have added that code, so i was
>> unable to verify that the version check is required or not for the 4K EEPROMs.
>> To be on the safe side, I have used the same check which was present in the
>> generic eeprom code.
>
> Gabor, thanks for your reply.
>
>
> --
> shafi
> _______________________________________________
> ath9k-devel mailing list
> ath9k-devel at lists.ath9k.org
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Marek Lindner]

On Wednesday, August 03, 2011 08:48:12 Adrian Chadd wrote:
> Hm, I just checked the reference code and it doesn't at all look like
> that check is actually needed; it's only needed for Merlin.
> 
> So, good catch!

Thanks for the info!


> I'll fix FreeBSD; who wants to fix ath9k? :)

I could post the patch I am already using. It goes to linux-wireless, right ?

Regards,
Marek

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Mohammed Shafi]

On Wed, Aug 3, 2011 at 12:33 PM, Marek Lindner <lindner_marek@yahoo.de> wrote:
> On Wednesday, August 03, 2011 08:48:12 Adrian Chadd wrote:
>> Hm, I just checked the reference code and it doesn't at all look like
>> that check is actually needed; it's only needed for Merlin.
>>
>> So, good catch!
>
> Thanks for the info!
>
>
>> I'll fix FreeBSD; who wants to fix ath9k? :)
>
> I could post the patch I am already using. It goes to linux-wireless, right ?

also please do mention, what it fixes.


>
> Regards,
> Marek
>



-- 
shafi

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Adrian Chadd]

The point is to try and get as much information about the physical
hardware as possible.

For example (and conjecture because I don't know the real reasons,
this is just based on what I've seen going on in the past!) the reason
the EEPROM is there may be because reference designs before some date
(call it X) had EEPROM versions up to 18, and they only had one set of
output amplifiers for TX gain. Someone then updated the reference
board design and then bumped the EEPROM version to support different
arrangements of output PAs and such.

Hopefully someone inside Atheros can verify. In the meantime, unless
you can hook up a spectrum analyser and actually verify that the NIC
is outputting a clean signal (and isn't distorting, or outputting
harmonics in adjacent channels), I'd suggest playing it safe.

Lastly, I'd suggest poking the hardware board vendor and asking them
what's going on. I can't find a datasheet on the OM2P at all and you
haven't mentioned what the expected versus measured TX power is. :-)

Adrian

On 2 August 2011 22:37, Marek Lindner <lindner_marek@yahoo.de> wrote:
> On Tuesday, August 02, 2011 13:13:27 Adrian Chadd wrote:
>> Right, so it's an embedded board inside a wireless router? Which router? :)
>
> The OM2P but how is this going to help ? I have the feeling we are not getting
> closer to answer my question: Why does ath9k_hw_4k_get_eeprom() requires the
> minor version 19 or above before even considering to read the eeprom value ?
> Can I propose a patch that removes that check ? I already have that patch
> working for me - just wanted to be sure it does no harm.
>
>
> Regards,
> Marek
>

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Mohammed Shafi]

On Tue, Aug 2, 2011 at 1:58 PM, Marek Lindner <lindner_marek@yahoo.de> wrote:
>
> Hi,
>
> I was having some output power problems with a device using AR7240/AR9285.
> After digging into the ath9k driver I realized that it would override the
> power settings (txGainType) configured in the eeprom if the minor version does
> not meet its expectations. This is why it would not use the appropriate high
> power settings. The offending lines from eeprom_4k.c:
>
> if (ver_minor >= AR5416_EEP_MINOR_VER_19)
> return pBase->txGainType;
> else
> return AR5416_EEP_TXGAIN_ORIGINAL;
>
> My minor version is 14 and consequently fell through the cracks. The commit
> introducing the check in question was 970bf9d40c03e48cc34ee2c1a70693a0e0fca3f6
> but it does not explain why the minor version has to be 19 or higher. Does
> anyone know the magic behind version 19 ? I could easily post a patch to
> remove that check if it causes no harm.

lets be careful while changing this, we must also note things like,

        /* PA CAL is not needed for high power solution */
        if (ah->eep_ops->get_eeprom(ah, EEP_TXGAIN_TYPE) ==
            AR5416_EEP_TXGAIN_HIGH_POWER)
                return;


>
> Regards,
> Marek
> _______________________________________________
> ath9k-devel mailing list
> ath9k-devel at lists.ath9k.org
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>



-- 
shafi

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Marek Lindner]

On Tuesday, August 02, 2011 12:09:59 Mohammed Shafi wrote:
> lets be careful while changing this, we must also note things like,
> 
>         /* PA CAL is not needed for high power solution */
>         if (ah->eep_ops->get_eeprom(ah, EEP_TXGAIN_TYPE) ==
>             AR5416_EEP_TXGAIN_HIGH_POWER)
>                 return;

I have that seen too and disabled that check in my code but I intended to make 
further tests before bringing this up.

Let me emphasize again that the correct eeprom value is returned once your 
minor version is 19 or above. Only the smaller versions are overridden by the 
driver. Therefore I don't see a direct relation to the code you posted. Can 
you clarify ?

Regards,
Marek

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Mohammed Shafi]

On Tue, Aug 2, 2011 at 3:45 PM, Marek Lindner <lindner_marek@yahoo.de> wrote:
> On Tuesday, August 02, 2011 12:09:59 Mohammed Shafi wrote:
>> lets be careful while changing this, we must also note things like,
>>
>> ? ? ? ? /* PA CAL is not needed for high power solution */
>> ? ? ? ? if (ah->eep_ops->get_eeprom(ah, EEP_TXGAIN_TYPE) ==
>> ? ? ? ? ? ? AR5416_EEP_TXGAIN_HIGH_POWER)
>> ? ? ? ? ? ? ? ? return;
>
> I have that seen too and disabled that check in my code but I intended to make
> further tests before bringing this up.
>
> Let me emphasize again that the correct eeprom value is returned once your
> minor version is 19 or above. Only the smaller versions are overridden by the
> driver. Therefore I don't see a direct relation to the code you posted. Can
> you clarify ?

if the ver < 19 then we are sure that the code will never return from
the above check ? am i missing something ?


>
> Regards,
> Marek
>



-- 
shafi

[ath9k-devel] eeprom txGainType overridden if minor version < 19

[Marek Lindner]

On Tuesday, August 02, 2011 12:36:27 Mohammed Shafi wrote:
> if the ver < 19 then we are sure that the code will never return from
> the above check ? am i missing something ?

Never return ?
If ver < 19 it will always return 0 otherwise it will return whatever the 
eeprom value is (in my case 1).

Regards,
Marek