mt76x02: fix sign_extend() sign inversion#5
Merged
HiGarfield merged 5 commits intomasterfrom Mar 25, 2026
Merged
Conversation
The mt76x02_sign_extend() function uses sign-magnitude encoding where bit[size-1] is the sign bit and the lower bits are the magnitude. The return statement had the sign condition inverted: 'sign ? val : -val' returned a positive value when the sign bit was set (indicating a negative number) and vice versa. Fix this by swapping the return expressions to 'sign ? -val : val', so that a set sign bit correctly yields a negative result. This bug affected RSSI offsets, LNA gain calibration, TX power delta values, and other calibration parameters read from EEPROM.
The rate power table iteration loops used sizeof(r->all) to determine the iteration count. This works by coincidence because the element type is s8 (1 byte), making sizeof equal to the element count. Use ARRAY_SIZE(r->all) instead, which explicitly expresses the intent of iterating over all elements and avoids a silent dependency on the element size.
Three coding style/correctness issues are fixed: 1. mt76x02_mmio.c: dma_is_busy() used 4-space indentation instead of tabs for the return statement, inconsistent with the rest of the file. 2. mt76x02_mmio.c: The DMA busy wait loop used '--retry != 0' which only performs 4 iterations despite retry being initialized to 5. Use 'retry-- > 0' to perform the intended 5 iterations. 3. mt76x02_txrx.c: Missing space between the '=' operator and 'dev->' in three assignment statements.
The mt76x02_eeprom_copy() function returned -1 on error instead of a proper Linux error code. Use -EINVAL to follow the standard kernel convention for invalid argument errors, as this error indicates the requested EEPROM field range exceeds the available EEPROM size.
The do-while loop in mt7603_wtbl_set_smps() had the opening brace on a separate line from the 'do' keyword, and the mt76_poll() call had excessive indentation for its continuation argument. Fix both issues to match the Linux kernel coding style.
Copilot created this pull request from a session on behalf of
HiGarfield
March 25, 2026 23:44
View session
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Fixes incorrect sign-magnitude decoding in mt76x02_sign_extend(), which inverted the sign of multiple EEPROM-sourced calibration values used across mt76x02-based devices, and includes a few small correctness/style cleanups in nearby code.
Changes:
- Correct
mt76x02_sign_extend()to return negative values when the sign bit is set. - Replace
sizeof(r->all)loops withARRAY_SIZE(r->all)for element-count correctness. - Minor cleanups: consistent spacing/indentation, more specific error code (
-EINVAL) for out-of-bounds EEPROM copies, and a small retry-loop readability tweak.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| mt76x02_eeprom.h | Fixes sign inversion in mt76x02_sign_extend() used for EEPROM calibration decoding. |
| mt76x02_phy.c | Uses ARRAY_SIZE() instead of sizeof() for iterating the rate_power array elements. |
| mt76x02_eeprom.c | Returns -EINVAL on invalid EEPROM copy bounds instead of -1. |
| mt76x02_mmio.c | Style cleanup and clearer retry loop condition during watchdog reset. |
| mt76x02_txrx.c | Whitespace/style fixes in rate-power indexing. |
| mt7603/mac.c | Whitespace/style fixes for SMPS WTBL update loop. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mt76x02_sign_extend()had the sign-magnitude decode backwards — when the sign bit was set (negative), it returned a positive value, and vice versa. This corrupted every EEPROM-sourced calibration value that flows through it.Affected call chains (all fixed by the one-line change):
lna_gain—mt76x2_read_rx_gain()→rssi -= lna_gain. Confirmed correct by cross-checking mt76x0, which stores the same EEPROM byte as a rawu8→s8cast and uses the identical consumer; mt76x2 must produce the same sign.temp_offset—mt76x0_set_temp_offset()→(35 * (adc_raw - temp_offset)) / 10 + 25. With the inverted sign a nominal offset of0x32(50) yielded ~375 °C instead of 25 °C at the calibration point.freq_offsetcorrection —mt76x0_set_freq_offset()→freq_offset -= sign_extend(val, 8). Inverted sign shifted the frequency calibration in the wrong direction.high_gain[0/1]—mt76x2_set_rx_gain_group()→ AGC registergain ± offset. 4-bit sign-magnitude; inversion flipped every AGC adjustment.rssi_offsetandchain.delta— viasign_extend_optional(val, 7). Enable flag is bit 7; sign is bit 6; magnitude is bits 5:0. Theoptionalwrapper correctly ignores the enable bit before delegating tosign_extend.