[dimerr]

FH8652/FH8656/FH8658 share one chip ID, and so do FH8632/FH8632v2. mach-fh's fh_chipid tells the packages apart with a second register:

• FH865x: CHIP_INFO (PMU 0x33C), bits 3:2, under CONFIG_ARCH_FH865x;
• FH8632: SPC_IO_STATUS (PMU 0x054), bits 5:0, read by fh_get_chipid() for plat 0x46488302.

fh_detect_cpu() read IP_VER for both, with the vendor masks, so the package was never set and ipcinfo -c printed unknown.

Tested only in qemu-hisilicon's fh8652, fh8658 and fh8632 machines, whose PMU models take these registers from the vendor sources. ipcinfo -c now prints fh8652, fh8658 and fh8632; it printed unknown before. fh8626v100 was already right and is unchanged. Not run on an FH865x or FH8632 camera.

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

PR Summary by Qodo

Read FH865x package IDs from PMU CHIP_INFO

🐞 Bug fix 🕐 Less than 10 minutes

AI Description

• Identify FH8652, FH8656, and FH8658 using the vendor kernel’s PMU package register.
• Replace the FH865x IP_VER check that left package names unknown.

Diagram

graph TD
  A["PMU chip ID"] --> B{"FH865x ID?"} -->|yes| C["Read CHIP_INFO"] --> D["Mask package bits"] --> E["FH865x chip name"]

Loading

High-Level Assessment

Reading CHIP_INFO in the FH865x branch is the direct fix and matches the vendor kernel’s package source. The PR description also claims an FH8632 fix, but this diff leaves the FH8632 IP_VER branch unchanged; that claim should be corrected or addressed in a separate change.

Files changed (2) +5 / -2

Bug fix (2) +5 / -2

fh.cDetect FH865x packages from CHIP_INFO +4/-2

Detect FH865x packages from CHIP_INFO

• The FH865x chip-ID branch now reads PMU CHIP_INFO and uses bits 3:2 instead of IP_VER to select FH8652, FH8656, or FH8658. The FH8632 detection branch is unchanged.

src/hal/fh.c

fh.hDefine the CHIP_INFO PMU register +1/-0

Define the CHIP_INFO PMU register

• Adds the PMU register address at base offset 0x33c for FH865x package detection.

src/hal/fh.h

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

Code Review by Qodo

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

1. FH8632 packages remain unknown 📘 Rule violation ≡ Correctness

Description

fh_detect_cpu() still derives FH8632 and FH8632v2 from ipver, while the required package value
is in the PMU SPC_IO_STATUS register. When the chip ID is 0x46488302, the unchanged IP_VER
mask can leave chip_name unset, so both package names remain unidentified even though the new
register handling is present for FH865x only.

Code

src/hal/fh.c[R99-100]

+            /* mach-fh/fh_chipid.c reads the FH865x package from CHIP_INFO */
+            mem_reg(REG_PMU_CHIP_INFO, &chipinfo, OP_READ);

Evidence

The changed code adds a CHIP_INFO read only inside the FH865x case, while the PR branch still uses
ipver for the 0x46488302 FH8632 family case. This fails the HAL detection requirement for newly
handled hardware.

CLAUDE.md: Maintain HAL Detection and Probe Semantics: CLAUDE.md: Maintain HAL Detection and Probe Semantics
src/hal/fh.c[88-95]
src/hal/fh.c[99-100]

Agent prompt

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

## Issue description
FH8632 and FH8632v2 package detection still reads `IP_VER`, but the vendor implementation reads PMU `SPC_IO_STATUS` at offset `0x054` and masks bits 5:0. As a result, the `0x46488302` path can leave the package name unset.
## Fix Focus Areas
- src/hal/fh.c[88-95]
- src/hal/fh.h[8-12]
## Recommended Fix
Define the PMU `SPC_IO_STATUS` register address and read it in the `0x46488302` branch before applying the vendor’s `0x3f` mask, while retaining the existing FH865x `CHIP_INFO` handling separately.

ⓘ 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 turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

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

1. Fh8632 packages remain unknown 📘 Rule violation ≡ Correctness

fh_detect_cpu() still derives FH8632 and FH8632v2 from ipver, while the required package value
is in the PMU SPC_IO_STATUS register. When the chip ID is 0x46488302, the unchanged IP_VER
mask can leave chip_name unset, so both package names remain unidentified even though the new
register handling is present for FH865x only.

Agent Prompt

## Issue description
FH8632 and FH8632v2 package detection still reads `IP_VER`, but the vendor implementation reads PMU `SPC_IO_STATUS` at offset `0x054` and masks bits 5:0. As a result, the `0x46488302` path can leave the package name unset.

## Fix Focus Areas
- src/hal/fh.c[88-95]
- src/hal/fh.h[8-12]

## Recommended Fix
Define the PMU `SPC_IO_STATUS` register address and read it in the `0x46488302` branch before applying the vendor’s `0x3f` mask, while retaining the existing FH865x `CHIP_INFO` handling separately.

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

[dimerr]

Qodo's finding is right. The FH8632 half was in my working tree but not in the pushed commit, because the amend did not stage it. It is now in 8c3ea29: 0x46488302 reads SPC_IO_STATUS (PMU 0x054) & 0x3f. With that, qemu-hisilicon fh8632 prints ipcinfo -c → fh8632.

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

Qodo's finding is right. The FH8632 half was in my working tree but not in the pushed commit, because the amend did not stage it. It is now in 8c3ea29: 0x46488302 reads SPC_IO_STATUS (PMU 0x054) & 0x3f. With that, qemu-hisilicon fh8632 prints ipcinfo -c → fh8632.

Thanks for the clarification. The fix in 8c3ea29 addresses finding 1: the 0x46488302 path now reads PMU SPC_IO_STATUS at 0x054 and applies the 0x3f mask, allowing fh8632 detection. The published review predates that commit.