Date: Fri, 17 Apr 2026 11:28:05 +0200 From: Roger Pau =?utf-8?B?TW9ubsOp?= <royger@freebsd.org> To: Mark Johnston <markj@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org, imp@freebsd.org Subject: Re: git: 1491fe8f864a - main - uart/pci: support 16550A PCI serial devices Message-ID: <aeH9JbtoFly6xZMD@macbook.local> In-Reply-To: <aeFDv6l3-f6EbU6i@nuc> References: <69c640e1.1fc2b.4542bc24@gitrepo.freebsd.org> <aeFDv6l3-f6EbU6i@nuc>
index | next in thread | previous in thread | raw e-mail
On Thu, Apr 16, 2026 at 04:17:03PM -0400, Mark Johnston wrote: > On Fri, Mar 27, 2026 at 08:33:37AM +0000, Roger Pau Monné wrote: > > The branch main has been updated by royger: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=1491fe8f864af5af37e83f1d12459905fb6097fd > > > > commit 1491fe8f864af5af37e83f1d12459905fb6097fd > > Author: Roger Pau Monné <royger@FreeBSD.org> > > AuthorDate: 2026-03-26 10:01:57 +0000 > > Commit: Roger Pau Monné <royger@FreeBSD.org> > > CommitDate: 2026-03-27 08:26:32 +0000 > > > > uart/pci: support 16550A PCI serial devices > > > > Expand the current check to also attach the ns8250 driver to devices > > reporting as 16550A. This has been tested to work on a real device. > > > > From an inspection of the code in the ns8250 driver it seems like it should > > support up to 16950A devices, but I don't have hardware to ensure that, > > hence be conservative with the change. > > I have a system which hangs in uart_bus_probe() after this change. It > has a 2-port UART PCI card installed: > > puc0@pci0:6:0:0: class=0x070002 rev=0x00 hdr=0x00 vendor=0x1415 device=0xc158 subvendor=0x1415 subdevice=0xc158 > vendor = 'Oxford Semiconductor Ltd' > device = 'OXPCIe952 Dual Native 950 UART' > class = simple comms > subclass = UART > > Normally puc(4) attaches to it and creates two child uart devices: > > puc0 > Interrupt request lines: > 0x11 > pcib7 memory window: > 0x84000000-0x841fffff > 0x84200000-0x843fffff > 0x84400000-0x84403fff > uart2 > puc0 I/O memory mapping: > 0x84401000-0x844011ff > puc0 port numbers: > 0x1 > uart3 > puc0 I/O memory mapping: > 0x84401200-0x844013ff > puc0 port numbers: > 0x2 > > After this change, though, uart(4) tries to probe the device directly > and hangs. Presumably we still want to exclude at least multi-port > devices here? Hm, I see, sorry, I didn't think changing the UART version compatibility would cause this. What about using BUS_PROBE_SPECIFIC vs BUS_PROBE_GENERIC? If the PCI device matches a device in the pci_ns8250_ids array we return SPECIFIC, otherwise if it's a generic PCI serial device we return GENERIC, thus allowing the puc driver to take over it. Could you please give a try to the patch below. Thanks, Roger. --- diff --git a/sys/dev/uart/uart_bus_pci.c b/sys/dev/uart/uart_bus_pci.c index b0d285e3c603..f8da4eaf8d25 100644 --- a/sys/dev/uart/uart_bus_pci.c +++ b/sys/dev/uart/uart_bus_pci.c @@ -280,33 +280,43 @@ uart_pci_probe(device_t dev) { struct uart_softc *sc; const struct pci_id *id; - struct pci_id cid = { - .regshft = 0, - .rclk = 0, - .rid = 0x10 | PCI_NO_MSI, - .desc = "Generic SimpleComm PCI device", - }; - int result; sc = device_get_softc(dev); id = uart_pci_match(dev, pci_ns8250_ids); if (id != NULL) { sc->sc_class = &uart_ns8250_class; - goto match; + return BUS_PROBE_SPECIFIC; } if (pci_get_class(dev) == PCIC_SIMPLECOMM && pci_get_subclass(dev) == PCIS_SIMPLECOMM_UART && pci_get_progif(dev) <= PCIP_SIMPLECOMM_UART_16550A) { - /* XXX rclk what to do */ - id = &cid; sc->sc_class = &uart_ns8250_class; - goto match; + return BUS_PROBE_GENERIC; } /* Add checks for non-ns8250 IDs here. */ return (ENXIO); +} + +static int +uart_pci_attach(device_t dev) +{ + const struct pci_id cid = { + .regshft = 0, + .rclk = 0, + .rid = 0x10 | PCI_NO_MSI, + .desc = "Generic SimpleComm PCI device", + }; + struct uart_softc *sc; + const struct pci_id *id = uart_pci_match(dev, pci_ns8250_ids); + int count, result; + + if (id == NULL) + /* No specific PCI ID match, must be a generic device. */ + id = &cid; + + sc = device_get_softc(dev); - match: result = uart_bus_probe(dev, id->regshft, 0, id->rclk, id->rid & PCI_RID_MASK, 0, 0); /* Bail out on error. */ @@ -322,26 +332,13 @@ uart_pci_probe(device_t dev) /* Set/override the device description. */ if (id->desc) device_set_desc(dev, id->desc); - return (result); -} - -static int -uart_pci_attach(device_t dev) -{ - struct uart_softc *sc; - const struct pci_id *id; - int count; - - sc = device_get_softc(dev); /* - * Use MSI in preference to legacy IRQ if available. However, experience - * suggests this is only reliable when one MSI vector is advertised. + * Use MSI in preference to legacy IRQ if available. However, + * experience suggests this is only reliable when one MSI vector is + * advertised. */ - id = uart_pci_match(dev, pci_ns8250_ids); - /* Always disable MSI for generic devices. */ - if (id != NULL && (id->rid & PCI_NO_MSI) == 0 && - pci_msi_count(dev) == 1) { + if ((id->rid & PCI_NO_MSI) == 0 && pci_msi_count(dev) == 1) { count = 1; if (pci_alloc_msi(dev, &count) == 0) { sc->sc_irid = 1;home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?aeH9JbtoFly6xZMD>