[dimerr]

Refs OpenIPC/defib#148 and OpenIPC/u-boot#105.

Problem

A Hi3516CV610 with a KH25L12833FM2I-10GY (ID c2 20 18) panics at boot, and a K662C6S shows the same failure:

bsp-sfc bsp_spi_nor.0: spi nor register fail!
bsp-sfc: probe of bsp_spi_nor.0 failed with error -5
VFS: Cannot open root device "mtdblock3" or unknown-block(0,0): error -6

The ID already matches mx25l12835f, so this is not a missing table entry.

Cause

SPI_NOR_QUAD_READ on that entry makes spi_nor_init() set QE through spi_nor_sr1_bit6_quad_enable(). OpenIPC U-Boot before u-boot#105 hands over a Macronix part with WEL set. Linux reads SR 0x02 and writes 0x42; WRSR clears WEL, the read-back gives 0x40, and the comparison fails with -EIO.

Change

Quad gives more problems than benefits on these cameras, so bsp-sfc no longer offers it, at probe or at resume. spi_nor_quad_enable() then returns early and SR is not written, so cameras that still have the old U-Boot boot too.

Reads on this part drop to 1-2-2 (0xBB) at 84 MHz, which matches the datasheet's default dummy cycles.

Testing

Build-tested only: drivers/mtd/spi-nor with the hi3516cv6xx config from OpenIPC/firmware. Please check on hardware:

• A Macronix board boots, and a whole-partition read gives a stable md5sum in dual mode.
• A Winbond or GigaDevice board still boots. With no quad in hwcaps, spi_nor_dtrclear_pinreset() now reaches spi_nor_sr3_to_reset() there. It writes SR3 with no WREN first, so it should do nothing with WEL clear.

CLOSE_SPI_8PIN_4IO no longer has any effect on this branch.

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

PR Summary by Qodo

hi3516cv6xx: Disable quad SPI in bsp-sfc

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Remove quad capabilities at probe and resume to avoid Macronix QE failures after old U-Boot
 handoff.
• Retain dual-read support; non-quad capability masks also activate the existing reset-pin path.

Diagram

graph TD
  Probe(["Probe"]) --> Caps["Capability mask"] --> Setup["SPI NOR setup"] --> Dual["Dual read"]
  Resume(["Resume"]) --> Caps
  Setup --> QESkip["Skip quad QE"]
  Caps --> Reset["Reset pin path"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Correct Macronix QE status comparison

• ➕ Could preserve quad throughput while tolerating WEL clearing after WRSR.
• ➖ Changes shared flash initialization behavior and would need broader hardware validation.
• ➖ Does not address the stated preference to avoid quad on these cameras.

Recommendation: Disabling quad in both BSP capability masks is the narrower choice for these cameras and avoids depending on a U-Boot fix. Validate Macronix dual reads and Winbond/GigaDevice boot on hardware before merging, since testing so far is build-only.

Files changed (2) +0 / -14

Bug fix (2) +0 / -14

bsp-sfc.cStop advertising quad modes during bsp-sfc probe +0/-7

Stop advertising quad modes during bsp-sfc probe

• Removes the conditional quad-read and quad-page-program capability additions before spi_nor_scan(). On the BSP configuration, setup can select a non-quad protocol, avoiding the Macronix QE write implicated in the probe failure.

drivers/mtd/spi-nor/controllers/bsp-sfc.c

core.cKeep quad modes disabled on BSP SPI NOR resume +0/-7

Keep quad modes disabled on BSP SPI NOR resume

• Removes the same conditional quad capability additions from bsp_spi_nor_resume(). The non-quad mask also allows the existing reset-pin handling path to run for Winbond and GigaDevice parts.

drivers/mtd/spi-nor/core.c

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

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. Flash reset pins stay disabled 🐞 Bug ≡ Correctness

Description

spi_nor_sr3_to_reset() sends the status-register write without first issuing Write Enable, then
checks the previous read result instead of the write result. Removing quad from the BSP capability
mask makes Winbond and GigaDevice flashes reach this path during scan; when WEL is clear, the
attempted reset-pin configuration does not take effect, but initialization reports success.

Code

drivers/mtd/spi-nor/controllers/bsp-sfc.c[L623-624]

-	hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4 |
-		       SNOR_HWCAPS_READ_1_4_4 |

Evidence

The deleted capability bits cause spi_nor_dtrclear_pinreset() to take its no-quad branch. That
branch routes Winbond and GigaDevice parts to an SR3 write that lacks the Write Enable command used
by the other status-register write path, and its error check cannot detect a failed write.

drivers/mtd/spi-nor/controllers/bsp-sfc.c[562-573]
drivers/mtd/spi-nor/core.c[2408-2413]
drivers/mtd/spi-nor/core.c[2463-2470]
drivers/mtd/spi-nor/core.c[2377-2405]
drivers/mtd/spi-nor/core.c[2428-2445]

Agent prompt

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

## Issue description
Removing quad capabilities activates a Winbond/GigaDevice reset-pin setup path that attempts to write SR3 without Write Enable and reports success without checking the write.
## Fix Focus Areas
- drivers/mtd/spi-nor/controllers/bsp-sfc.c[622-627]
- drivers/mtd/spi-nor/core.c[2377-2405]
- drivers/mtd/spi-nor/core.c[2463-2470]
## Recommended Fix
In `spi_nor_sr3_to_reset()`, issue Write Enable before WRSR3, assign and check the write result, and verify that the requested SR3 bit was set before reporting success. Ensure this handles both probe and resume, which share the helper.

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

---

Context sources

✅ Tickets: 🎫 hi3516cv610:20s — bootrom upload fails after DDR training ("Failed to send U-Boot"), stock boot prints "DDR size" but burn never does

Review mode: Auto: ⚖️ Balanced: Small removal, but touches boot-critical SPI NOR probe and resume paths on hardware.

Tip of the day

💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗