Skip to content

SPI NOR: -uu clears QE as well; CI builds on pushes to main only - #56

Merged
compot-dev merged 2 commits into
mainfrom
unprotect-qe
Nov 4, 2026
Merged

compot-dev merged 2 commits into
mainfrom
unprotect-qe

Conversation

@compot-dev

@compot-dev compot-dev commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

-u keeps QE (quad enable), which protects nothing but turns WP# and HOLD# into data lines. -uu clears the protection as -u does, and then QE.

  • QE is bit 1 of status register 2, written with WRSR2, or with the two-byte WRSR for the chips that do not know WRSR2. On Macronix chips it is bit 6 of status register 1.
  • A QE that stays set is told and the run fails; a chip without the bit has nothing to clear. A protection that stays stops -uu before QE.
  • -uuu is an error. SPI NOR on the command line only: not in the web mode or the Android app. Not with SPI NAND or the EZP programmers.
  • READMEs ×3, version 1.21.0. If the SigmaStar PR (SigmaStar ISP: the flash of a board through the I2C of its SoC #48) goes first, this one takes the next version.

Tests: norsim covers the kinds of chip above, stub the options.

Hardware: CH341A + W25Q64 (ef 40 17, SR 00 02 60): -uu reports that QE stays by both ways of writing it, so on this part it is fixed at the factory; registers and data unchanged. Clearing QE on a real chip is not checked yet, only in the simulator.

CI: build.yml builds on pushes to main only, as a push to a branch with a pull request started it twice. Pull requests and workflow_dispatch still build any branch.

-u keeps QE (quad enable), which protects nothing. -uu clears the protection
and then QE: bit 1 of status register 2, with WRSR2 or the two-byte WRSR for
the chips that do not know it, bit 6 of status register 1 on Macronix chips.
A QE that stays set is told. Command line only.

Version 1.21.0.

[skip ci]
A push to the branch of a pull request started the Build workflow twice, on
push and on pull_request, and every run takes GitHub's minutes. Pull requests
and workflow_dispatch still build any branch, and main still publishes.
@compot-dev compot-dev changed the title SPI NOR: -uu clears QE as well SPI NOR: -uu clears QE as well; CI builds on pushes to main only Nov 4, 2026
@compot-dev
compot-dev marked this pull request as ready for review November 4, 2026 11:48
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add SPI NOR -uu QE clearing and limit CI push builds to main

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Add SPI NOR -uu to clear protection and QE while preserving -u behavior.
• Cover chip variants and CLI errors with simulator and stub tests.
• Document boot risks, release version 1.21.0, and avoid duplicate branch-push builds.
Diagram

graph TD
  CLI["CLI -uu"] --> Dispatch["NOR dispatch"] --> Unprotect["Clear protection"] --> Location{"QE location"}
  Location -->|SR1| Macronix["Macronix WRSR"] --> Result["Readback result"]
  Location -->|SR2| WRSR2["WRSR2 attempt"] --> Result
  WRSR2 -->|still set| Fallback["Two-byte WRSR"] --> Result
  Location -->|no QE| Result
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use a per-chip QE capability table
  • ➕ Selects the known QE location and supported write command without a fallback attempt.
  • ➖ Requires reliable capability data for every supported chip and ongoing maintenance.

Recommendation: Keep the current readback-and-fallback approach for broad chip support without expanding the chip database. Its main remaining validation need is a successful QE clear on real hardware, particularly for the two-byte fallback.

Files changed (13) +184 / -12

Enhancement (5) +100 / -4
flashcmd_api.cBind the QE-clearing callback for SPI NOR +1/-0

Bind the QE-clearing callback for SPI NOR

• Assigns 'snor_clear_qe' alongside the existing NOR unprotect callback during flash initialization.

src/flashcmd_api.c

flashcmd_api.hExpose QE clearing through flash command dispatch +1/-0

Expose QE clearing through flash command dispatch

• Adds a 'flash_clear_qe' callback to 'struct flash_cmd' for the '-uu' operation.

src/flashcmd_api.h

main.cParse and dispatch the new -uu option +12/-4

Parse and dispatch the new -uu option

• Counts '-u' occurrences, rejects more than two or conflicting operations, and dispatches '-uu' to QE clearing. Updates help text and the version to 1.21.0.

src/main.c

snorcmd_api.hDeclare the SPI NOR QE-clearing operation +1/-0

Declare the SPI NOR QE-clearing operation

• Exports 'snor_clear_qe' and documents that it unprotects first and uses the existing unprotect result convention.

src/snorcmd_api.h

spi_nor_flash.cClear and verify chip-specific QE bits +85/-0

Clear and verify chip-specific QE bits

• Adds 'snor_clear_qe', which stops if unprotection fails, handles Macronix SR1 and other chips' SR2, and falls back to two-byte WRSR when QE remains set. Readback reports a QE bit that cannot be cleared.

src/spi_nor_flash.c

Tests (3) +54 / -5
norsim.cSimulate QE clearing across NOR variants +44/-4

Simulate QE clearing across NOR variants

• Models two-byte status-register writes and chips without WRSR2. Tests successful clears, fixed QE, absent registers, Macronix QE, and protection that blocks QE handling.

tests/norsim.c

run.shTest CLI acceptance and rejection of -uu +8/-0

Test CLI acceptance and rejection of -uu

• Covers combined and separate '-u' options, QE failure, unsupported NAND, '-uuu', conflicting operations, and distinct '-u' versus '-uu' dispatch.

tests/run.sh

stub.cStub QE-clearing dispatch for CLI tests +2/-1

Stub QE-clearing dispatch for CLI tests

• Adds a NOR-only QE callback that identifies its invocation and can simulate failure for option-handling tests.

tests/stub.c

Documentation (4) +28 / -3
CHANGELOG.mdRecord the 1.21.0 QE-clearing release +8/-0

Record the 1.21.0 QE-clearing release

• Documents '-uu', chip-specific QE behavior, fixed QE bits, and the risk to boards that boot in quad mode.

CHANGELOG.md

README.mdExplain -u versus -uu in English +7/-1

Explain -u versus -uu in English

• Updates the option reference and explains QE, command-line scope, fixed-bit behavior, and the potential boot impact.

README.md

README.ru.mdExplain -u versus -uu in the Russian README +7/-1

Explain -u versus -uu in the Russian README

• Adds option guidance and describes QE clearing, chip limitations, and quad-mode boot risk.

README.ru.md

README.zh-CN.mdExplain -u versus -uu in the Chinese README +6/-1

Explain -u versus -uu in the Chinese README

• Adds option guidance and describes QE clearing, chip limitations, and quad-mode boot risk.

README.zh-CN.md

Other (1) +2 / -0
build.ymlRestrict push-triggered builds to main +2/-0

Restrict push-triggered builds to main

• Limits the push trigger to 'main' so branches with pull requests do not start duplicate builds. Pull-request and manual triggers remain available.

.github/workflows/build.yml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Protected older chips cannot clear QE 🐞 Bug ≡ Correctness
Description
snor_clear_qe() returns when unprotect(1) cannot clear status-register-2 protection through
WRSR2, before its two-byte WRSR fallback runs. On a chip that accepts two-byte WRSR but not
WRSR2, a set CMP or SRP1 bit leaves protection in place and stops -uu, even though the supported
command can clear those bits along with QE.
Code

src/spi_nor_flash.c[R330-333]

+	int ret = unprotect(1);
+
+	if (ret)
+		return ret;
Evidence
The existing unprotect path writes SR2 protection solely through WRSR2 and returns if the bits
remain. The new QE fallback is later than that return, while the simulator demonstrates a chip mode
in which WRSR2 is ignored and two-byte WRSR writes SR2; its new test starts that mode with
protection already clear.

src/spi_nor_flash.c[240-258]
src/spi_nor_flash.c[287-305]
src/spi_nor_flash.c[330-360]
tests/norsim.c[98-113]
tests/norsim.c[304-307]

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

## Issue description
On chips without WRSR2, SR2 protection makes `unprotect(1)` return before `-uu` can try the supported two-byte WRSR command.
## Fix Focus Areas
- src/spi_nor_flash.c[240-258]
- src/spi_nor_flash.c[287-305]
- src/spi_nor_flash.c[330-360]
- tests/norsim.c[291-321]
## Recommended Fix
When an SR2 protection write does not take, attempt the supported two-byte WRSR path using freshly read SR1 and SR2 values, clearing the protection bits before proceeding to QE. Read both registers back and retain the existing failure behavior if protection still remains. Add a no-WRSR2 simulator case starting with SR2 protection and QE set.

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


Grey Divider

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 ↗

Grey Divider

Qodo Logo

@compot-dev

Copy link
Copy Markdown
Collaborator Author

Qodo finding "Protected older chips cannot clear QE" is left for now: on a chip without WRSR2, a set CMP or SRP1 stops -uu with an error, with nothing changed and nothing reported as done. -u has the same limit for those bits, so the two-byte WRSR belongs in the unprotect itself, as a separate change.

@compot-dev
compot-dev merged commit beeefff into main Nov 4, 2026
7 checks passed
@compot-dev
compot-dev deleted the unprotect-qe branch November 4, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant