Brijesh-Thakkar opened a new pull request, #20919:
URL: https://github.com/apache/nuttx/pull/20919
**Title:** drivers/serial/uart_pci_16550: Fix probing of multi-port cards
*Refs #20870 (together with the defconfig PR for `citest_smp`/`jumbo`, which
fixes the other cause listed in the issue).*
## Summary
`pci_u16550_probe()` walks the ports of a PCI 16550 card. Two bugs in that
loop, both described in #20870:
1. **The probe aborts at the first unconfigured port.** When no `UARTn`
instance is configured for the current port it did `return -ENODEV`, so a
configuration that only uses a later port of a multi-port card (e.g. only
`UART1` = port 1 of a QEMU `pci-serial-2x`) never reached it. It now
`continue`s with the next port, and returns `-ENODEV` only if no port of
the device was registered, so a card none of whose ports is configured is
still rejected as before.
2. **The port address accumulates.** `base += type->portincr * port`
modifies
the BAR base on every iteration, so with `portincr = 8` ports 2 and 3 got
offsets 24 and 48 instead of 16 and 24. Port 2 therefore talked to
port 3's registers and port 3 to an unmapped range. Each port address is
now `BAR base + portincr * port`. The issue only mentioned this one from
reading the code; it is now tested (below), and it matters in practice
because fix 1 makes ports 2 and 3 reachable on their own.
Two commits, one per fix:
```
drivers/serial/uart_pci_16550: Don't abort probe at the first unconfigured
port
drivers/serial/uart_pci_16550: Compute port address from the BAR base
```
The other cause in the issue (the defconfigs select the single-port card but
leave `CONFIG_16550_PCI_UART1_PORT` at 1) is a separate PR touching only the
two defconfigs. The two PRs are independent.
## Impact
- **Users / boards:** only PCI 16550 users (`CONFIG_16550_PCI_UART`). A
configuration whose port 0 is configured behaves exactly as before; the
difference is for configurations where an earlier port is *not* configured,
and for cards with `portincr != 0` (QEMU `pci-serial-2x`/`4x`) from the
third port on. AX99100 (`portincr = 0`, one port per PCI function) is not
affected by the address change.
- **Build / docs / compatibility:** none; no Kconfig, no API change.
- Does not conflict with #20842 (`feat/16550-stage1-hooks`): `git merge-tree`
is clean, the merge builds, and the 4x test below passes on the merge too.
## Testing
**Host:** Ubuntu 26.04, Linux 7.0.0-38, x86_64, GCC 15.2.0, QEMU 10.2.1 +
KVM.
Builds in a throwaway clone: nuttx `b01e7e760c` (master) + these commits,
apps
`e0174bf71`. `tools/checkpatch.sh -m -g upstream/master..HEAD`: all checks
pass.
**Method.** Base config `qemu-intel64:citest_smp` plus `CONFIG_IRQCHAIN=y`
(see "Found while testing") and the UART options in the table. QEMU:
`-cpu host -enable-kvm -m 2G -smp 4 -kernel nuttx -nographic -serial
mon:stdio -device <card>`, every card port backed by a unix-socket chardev.
For each UART: `ls /dev` at the NSH prompt; TX = `echo MARK > /dev/ttySn` and
the marker must arrive on the expected host chardev and on no other; RX = a
host script sends `MARK` byte by byte to the chardev and `cat /dev/ttySn`
must print it. For the address check, a temporary `syslog()` of the port
address before `pci_u16550_initialize()` (not part of the commits); the BAR 0
base was 0xc040 (4x) and 0xc050 (2x).
| Card | UARTs configured | Driver | `/dev/ttyS*` | Result |
|---|---|---|---|---|
| `pci-serial-2x` | UART1 = `0003` port 1 | master | none | not registered |
| | | commit 1 | `ttyS1` | TX **PASS**, RX **PASS** (chardev 2), addr =
BAR+8 |
| | | both commits | `ttyS1` | TX **PASS**, RX **PASS** (chardev 2), addr =
BAR+8 |
| `pci-serial-4x` | UART1 = `0004` port 2, UART2 = `0004` port 3 | master |
none | not registered |
| | | commit 1 only | `ttyS1`, `ttyS2` | `ttyS1`: TX **FAIL** (output
appears on chardev **4**), RX FAIL; `ttyS2`: TX/RX FAIL; addrs BAR+24, BAR+48 |
| | | both commits | `ttyS1`, `ttyS2` | `ttyS1`: TX **PASS** RX **PASS**
(chardev 3); `ttyS2`: TX **PASS** RX **PASS** (chardev 4); no cross-talk; addrs
BAR+16, BAR+24 |
| `pci-serial-4x` | UART1 = `0003` (card is `0004`) | both commits | none,
no crash | `-ENODEV` kept |
| `pci-serial` (1x) | UART1 = `0002` port 0 | master / both commits |
`ttyS1` | identical: TX/RX PASS, addr BAR+0 |
| `pci-serial-4x` | UART1 = `0004` port 0 | master / both commits | `ttyS1`
| identical: TX/RX PASS, addr BAR+0 |
The "commit 1 only" rows show that the second commit is needed: fix 1 alone
registers ports 2/3 at the wrong addresses.
**Build only:** with this branch, `qemu-intel64:nxterm`, `nsh_pci`,
`nsh_pci_smp`, `knsh_romfs_pci` build with no new warnings (`nxterm` has the
same 8 warnings on master). `nsh_pci*` target an AX99100 (`125b:9100`) which
QEMU does not provide, so they cannot be booted there. `nxterm` (PCI
console,
wildcard ids) I could not boot: the kernel resets early, on unmodified master
too, with both `-kernel` and a GRUB ISO, so I have no runtime signal from it.
Merging with #20842's branch builds `citest_smp` and passes the 4x test.
**Found while testing (not changed here).** Opening `/dev/ttyS1` on a PCI
legacy-IRQ card asserts in `up_enable_irq()` (`g_irq_priv[irq].busy`) when
`CONFIG_IRQCHAIN` is not set, which is the case in `citest_smp`. It happens
with the single-port card and the unmodified driver, so it is independent of
this PR, and all my runtime tests above set `CONFIG_IRQCHAIN=y`.
Registration
(what #20870 is about) and `ls /dev` work without it. I can open a separate
issue for it if wanted.
**Not tested:** AX99100 or any real hardware; MMIO BARs (QEMU's cards use an
I/O BAR); `citest_smp` in CI (it has no PCI serial device).
## AI assistance
This change was developed with Claude Code (`claude-sonnet-5-5`). Both
commits carry an `Assisted-by:` field as required by CONTRIBUTING.md. I
reviewed the diff and certify the DCO with my `Signed-off-by`.
--
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]
