casaroli opened a new pull request, #19535:
URL: https://github.com/apache/nuttx/pull/19535
## Summary
`arch/arm/src/rp23xx/hardware/rp23xx_powman.h` defines a register address
macro
for each POWMAN register, then a block of register bit definitions. Three of
the
bit definitions reuse the name of a register address macro, so each of those
register addresses is silently redefined as a bit mask:
| Macro | Intended (register address) | Redefined to |
|---|---|---|
| `RP23XX_POWMAN_BADPASSWD` | `0x40100000` | `(1 << 0)` |
| `RP23XX_POWMAN_BOD_CTRL` | `0x40100018` | `(1 << 12)` |
| `RP23XX_POWMAN_DBG_PWRCFG` | `0x401000a4` | `(1 << 0)` |
The header currently has no in-tree consumer, which is why this has gone
unnoticed; it appears as three `"..." redefined` warnings as soon as any
driver
includes it. Code that included the header and used one of these three
registers by name would dereference `0x1`, `0x1000` and `0x1` rather than the
register.
## Changes
**`BOD_CTRL` and `DBG_PWRCFG` were plain duplicates.** Per the RP2350
datasheet
`BOD_CTRL` bit 12 is `ISOLATE` and `DBG_PWRCFG` bit 0 is `IGNORE`, and
`RP23XX_POWMAN_BOD_CTRL_ISOLATE` / `RP23XX_POWMAN_DBG_PWRCFG_IGNORE` were
already defined with those exact values on the immediately following lines.
The
bare names are therefore removed, with no loss of information.
While removing the `BOD_CTRL` line, the blank line separating the
`VREG_LP_EXIT` group from the `BOD_CTRL` group is restored — its absence is
what allowed the duplicate to hide inside the preceding group.
**`BADPASSWD` is a single-field register** whose field carries no separate
name
(the SDK exposes it as the register-level `POWMAN_BADPASSWD_BITS`). Its bit
definition is renamed to `RP23XX_POWMAN_BADPASSWD_MASK`, matching the `_MASK`
spelling this header already uses for a field extent, and written in hex like
its neighbours.
**The `rp23xx-rv` copy** of the header is byte-identical to the arm one apart
from the include guard, and carries the same three clashes, so it receives
the
same change. The two files remain in sync.
## Impact
No functional change: none of the three names has a single user anywhere in
the
tree, so nothing can change behaviour. This is a latent-bug and warning fix.
## Testing
- Verified with `grep` that none of the three macro names is referenced
anywhere in the tree outside the header itself, so the rename and the two
removals cannot break any caller.
- Confirmed against the Pico SDK register headers
(`rp2350/hardware_regs/.../powman.h`) that `BOD_CTRL` bit 12 is `ISOLATE`,
`DBG_PWRCFG` bit 0 is `IGNORE`, and that `BADPASSWD` is a one-bit register
with no separately named field.
- Mechanically verified that no duplicate `#define` name remains in either
copy
of the header.
- **Reproduced and fixed the warnings against a real consumer.** Built
`raspberrypi-pico-2:nsh` with a POWMAN-using driver (the RTC work in
#19526,
which includes this header):
- before: the three `redefined` warnings above;
- after: builds clean, zero warnings, and the driver still links
(`up_rtc_initialize` / `up_rtc_time` / `up_rtc_settime`).
- `tools/checkpatch.sh -g` clean on the commit.
## Note for reviewers
The same register-address-vs-bit-name clash exists in two further rp23xx
headers, which I have deliberately left out of this PR to keep it reviewable:
- `rp23xx_busctrl.h`: `RP23XX_BUSCTRL_BUS_PRIORITY_ACK`,
`RP23XX_BUSCTRL_PERFCTR_EN`
- `rp23xx_pads_qspi.h`: `RP23XX_PADS_QSPI_VOLTAGE_SELECT`
Happy to fix those here as well, or in a follow-up, whichever reviewers
prefer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
