MT76x0: Correct s6_to_s8 function #631
Conversation
mt76x0/eeprom.h
Outdated
|
|
||
| if (ret & BIT(5)) | ||
| if (ret & BIT(6)) |
There was a problem hiding this comment.
This change doesn't seem right to me. The reference driver does this:
if (val & 0x20) val -= 64;
Subtracting BIT(6) if BIT(6) is set also doesn't make it have the correct range, because it means that the resulting number can never be negative.
There was a problem hiding this comment.
BIT(6) should always be 0. The reference driver masks the eeprom value with 0x3F.
There was a problem hiding this comment.
In https://github.com/zaccareal/openwrt-archer-c2/blob/master/package/mediatek/drivers/mt7610e/src/chips/mt76x0.c#L2933 I see some other logic. I thought that this logic is correct. Am I wrong? In that code I see substracting 0x40 if more or equal 0x40.
Now we subtracting BIT(6) if BIT(5) is set. In this case, if val=0x3f than we get FF. If we check BIT(6) we will doesn't subtract and will get 0x3f
There was a problem hiding this comment.
Archer c2 has 0x3f in eeprom and MT7610E-V10-FEM.bin has 0x3a.
There was a problem hiding this comment.
Even that code has InputTxpower & 0x3F
There was a problem hiding this comment.
I still think the original function is correct, as it matches my interpretation of the vendor code. Even if you revert to the BIT(5) check, including bit 6 in the mask is still wrong
You are right. Bit 6 in the mask is still wrong.
|
I still think the original function is correct, as it matches my interpretation of the vendor code. |
|
Does this fix work on archer c2 ? If so i would like it wrong but working :) |
|
No, unfortunately this fix doesn't work. I deeply investigated I saw that is cosmetic fix. It doesn't increase the real txpower. |
This fix #227 (comment) work for me on archer c2. But it doesn't suit for commit. |
MT76x0: Fix sign calculation for negative numbers in s6_to_s8 function