[compot-dev]

A Macronix SPI NOR has one status register. The page and the app showed what 0x35 and 0x15 read on it (ff, 07 on the MX25L12805D) as SR2 and SR3, with the bits of Winbond, so a WPS lamp was lit on a chip that has no such bit.

• ms_chip tells sr_macronix; the web state has sr_macronix, the app gets bit 24 of sr.

• The page and the app show SR1 alone with SRWD, QE, BP3, BP2, BP1, BP0, WEL, WIP, BP3 and SRWD as protection bits, and a note of their own. Other chips do not change.

• -u, erases and writes clear CMP and SRP1 with the two-byte WRSR too, for the chips that do not know WRSR2; QE stays.

• Version 1.21.2.

• -i prints SR1 alone for these chips, and -t reads their protection right: BP3 counts, there is no TB and no CMP.

Tests: websim checks the state and the page, stub the protection line of -t. Hardware: two chips with the ID of the MX25L12805D, on CH341A and CH347T: the web mode, -i, and with SR1 written by hand, -t says PROTECTED for BP3 alone (0x20), -u clears it, keeps QE (0x40), and -uu clears QE. The Android app on a TV box with the CH341A: Detect and Registers show one register with the bits of Macronix. The two-byte WRSR for CMP and SRP1 is checked in norsim only: no such chip at hand.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Handle Macronix SR1 correctly and clear SR2 protection without WRSR2

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Show Macronix’s sole status register and its bit names across the CLI, web page, and Android app.
• Interpret Macronix BP3 correctly during self-test instead of treating unsupported registers as
 protection.
• Clear CMP and SRP1 with two-byte WRSR when WRSR2 is unsupported, preserving QE.

Diagram

graph TD
  chip[("SPI NOR")] --> driver["NOR driver"] --> state["Chip status"] --> web["Web registers"]
  state --> app["Android registers"]
  state --> selftest["CLI self-test"]
  driver --> clear["Unprotect logic"] -->|"WRSR2 ineffective"| fallback["Two-byte WRSR"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Per-chip status-register capabilities

• ➕ Could describe register count, bit layout, and write support without vendor-wide assumptions.
• ➖ Requires broader chip-table and consumer changes for a targeted compatibility fix.

2. Explicit Android status metadata

• ➕ Avoids encoding the Macronix indicator in an otherwise register-valued integer.
• ➖ Changes the JNI result signature and Java model for one additional flag.

Recommendation: The scoped Macronix indicator and read-back-triggered WRSR fallback are appropriate for this fix: they preserve existing behavior for other chips and avoid a broad register-model redesign. Per-chip capabilities would be preferable if further vendor-specific layouts or write protocols are added.

Files changed (19) +109 / -34

Bug fix (13) +76 / -33

microsnander-jni.cPass the Macronix indicator to Android +1/-1

Pass the Macronix indicator to Android

• Packs the chip’s Macronix status flag into bit 24 of the status value returned through JNI.

android/app/src/main/cpp/microsnander-jni.c

MainActivity.javaRender Macronix SR1 and its protection bits +10/-4

Render Macronix SR1 and its protection bits

• Shows only SR1 for Macronix chips, using their bit names and a chip-specific note. Other chips retain the three-register display.

android/app/src/main/java/org/openipc/microsnander/MainActivity.java

activity_main.xmlMake the register note selectable +1/-0

Make the register note selectable

• Adds an ID to the existing note so the activity can switch its text by chip type.

android/app/src/main/res/layout/activity_main.xml

strings.xmlAdd Macronix-specific register text +2/-0

Add Macronix-specific register text

• Adds a single-register heading and a note explaining Macronix protection bits and QE preservation.

android/app/src/main/res/values/strings.xml

main.cSet the CLI self-test flag and bump the version +2/-1

Set the CLI self-test flag and bump the version

• Passes Macronix status semantics into the CLI self-test and changes the version to 1.21.2.

src/main.c

ops.cCarry Macronix status through chip operations +2/-0

Carry Macronix status through chip operations

• Sets the flag when registers are read and propagates it into the operations-layer self-test.

src/ops.c

ops.hAdd Macronix status metadata to chips +1/-0

Add Macronix status metadata to chips

• Extends the chip result with a flag indicating that only SR1 has Macronix status-register semantics.

src/ops.h

selftest.cInterpret Macronix BP3 during self-test +8/-0

Interpret Macronix BP3 during self-test

• Checks the four Macronix BP bits in SR1 and reports SRWD when applicable, without interpreting TB or CMP from other register reads.

src/selftest.c

selftest.hDescribe Macronix status semantics to self-test +1/-0

Describe Macronix status semantics to self-test

• Adds the flag that selects Macronix SR1 protection interpretation.

src/selftest.h

snorcmd_api.hExpose the Macronix status query +1/-0

Expose the Macronix status query

• Declares the driver query used to identify chips whose only status register is SR1.

src/snorcmd_api.h

spi_nor_flash.cCorrect NOR register reporting and WRSR fallback +35/-21

Correct NOR register reporting and WRSR fallback

• Prints only SR1 during Macronix detection. If an SR2 protection bit survives WRSR2, retries with two-byte WRSR and verifies the result while retaining QE.

src/spi_nor_flash.c

web.cPublish Macronix status in web state +4/-2

Publish Macronix status in web state

• Retains the chip flag alongside cached register values and exposes it as 'sr_macronix' in the state response.

src/web.c

web_page.htmlShow Macronix SR1 without false SR2 or SR3 lamps +8/-4

Show Macronix SR1 without false SR2 or SR3 lamps

• Selects the single-register layout, Macronix bit names, and a chip-specific note when web state marks the chip as Macronix.

src/web_page.html

Tests (4) +22 / -0

norsim.cExercise SR2 protection-bit write fallback +8/-0

Exercise SR2 protection-bit write fallback

• Tests two-byte WRSR for CMP and SRP1, QE preservation, and the existing WRSR2 path on a chip that supports it.

tests/norsim.c

run.shTest Macronix self-test protection reporting +3/-0

Test Macronix self-test protection reporting

• Checks that BP3 alone reports protection and that QE and unsupported-register reads do not.

tests/run.sh

stub.cSimulate Macronix detection in CLI tests +1/-0

Simulate Macronix detection in CLI tests

• Adds an environment-controlled Macronix status query for the stub driver.

tests/stub.c

websim.cCheck the web Macronix state flag +10/-0

Check the web Macronix state flag

• Simulates both chip types, asserts their 'sr_macronix' state values, and checks that the page includes Macronix bit names.

tests/websim.c

Documentation (2) +11 / -1

CHANGELOG.mdDocument the 1.21.2 register fixes +10/-0

Document the 1.21.2 register fixes

• Records the Macronix display and protection corrections and the two-byte WRSR fallback.

CHANGELOG.md

Native.javaDocument the packed Android status flag +1/-1

Document the packed Android status flag

• Clarifies that bit 24 marks a Macronix chip and that only SR1 counts for it.

android/app/src/main/java/org/openipc/microsnander/Native.java

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Failed Macronix self-tests appear to pass 🐞 Bug ≡ Correctness

Description

The new negative assertion in tests/run.sh checks only whether grep finds PROTECTED,
discarding the exit status of $B -t. If the self-test fails for another reason without printing
that word, the assertion prints ok and leaves the test suite's failure flag unset.

Code

tests/run.sh[76]

+MS_MACRONIX=1 MS_SR=40ff07 $B -t | grep -q 'PROTECTED' && { echo "FAIL -t Macronix QE taken for protection"; fail=1; } || echo "ok     -t Macronix: QE is not a protection, ff and 07 are not read as registers"

Evidence

The added pipeline tests grep's status rather than the self-test's status. The self-test has
failure paths, including unreadable blocks and failed erases, that do not require a PROTECTED
message.

tests/run.sh[74-76]
src/selftest.c[116-123]
src/selftest.c[169-181]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Macronix negative assertion passes when the self-test fails without printing `PROTECTED`.
## Fix Focus Areas
- tests/run.sh[76-76]
## Recommended Fix
Capture the command's output and exit status separately. Require a successful exit and the absence of `PROTECTED` before printing `ok`; otherwise set `fail=1`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[compot-dev]

Qodo finding "Failed Macronix self-tests appear to pass" is left for the next change: it is in tests/run.sh only, the check should look at the exit code of -t too. The behaviour of the program does not depend on it.