From nobody Fri Apr 17 09:28:05 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4fxqLT1bmYz6ZQcq; Fri, 17 Apr 2026 09:28:17 +0000 (UTC) (envelope-from royger@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R13" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4fxqLS5X5Yz3qx6; Fri, 17 Apr 2026 09:28:16 +0000 (UTC) (envelope-from royger@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1776418096; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3Ig0+wsjGXB2SyFzYYosvGXFfusb9md+hjUPfEBTMfI=; b=VsqM0YW+lwkjrW2Vaa4nxjCuWFowsUdmp2AfE6YZbccg8b08mBz5qOpx8rreXHdvKChCCm ZpV1Zccz4CgFOdMQ14CsCBIs3mpdinyM2FBRCFIAldDQYOgCG0HVz0by2ScwOoUcbdQddJ ulVJu/6gahArVUve3/zkrjKTso14Pq28W5i0j/Tb9pgqwoK/BR7S82jln2jhqka8aOclf7 mxRSh3Nk8hn3XyUc+JMyiFjs7K4xMy8vTTpfb+6CxbsmCoT8NVYIvh/5eC26+5keatQZaA u9TJV4HaS8GCV7J/7u0z0FqzlLgVNZ65Yqq1A6YrhfuAeGclIvI31R0FlYxsdg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1776418096; a=rsa-sha256; cv=none; b=DUo4Gl0DhsaYfD0sSufCOEX8y4XxvuwtEhPwVzDXSWJYnL91AuEFxSBLhYkAgEELjMmbz/ xgxw7JOX+UL3a62POKrcW37BODK5RoscGG+dQ6LFJfKotvma9sn5WNxBGHUWIuroHq638V KPtKsi0qh7O5hHHHvr6ikuRiNUO2dfcm+d8RCkOQPFXfE5mggIs6Me6hwjp9bQ1OP8wOEk 3KkIALoZbPekAzsQsCPPVfYEuIku6L5Rqp2szmuFea8KAZ+UBgxXuzaNwfsZcRAMHHng1K VoLHvsQaWFeljODgZfAajqZEBKI1vyPEVQmG6Dkh9fP9I5Ffe5qtcwcv+ZogZA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1776418096; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3Ig0+wsjGXB2SyFzYYosvGXFfusb9md+hjUPfEBTMfI=; b=aj/Ko1nbML58tqrS7ejFkKg9CpghLnECxbZ6NcSEYTVm5bxRINBO7s0qzV+Cyvcw+Ne4t5 6BFjsMM3Bs6y2ppCE8O8CsL9ki6Gp3rZ9fb+pzGb/IBXe/X2pjJPSBCR1xFzH7alBN/Gqu wLvE5zXpXF99Jrerhg0wA50fS01b9RREP7IVrZ3aiinWBGvXfFA4QzkOR6NS9PKrsmfNDq SF5W80dm8PT8Ftcd0vPnpSNpucV/kCRRYxceZLqSdqWMSZ9BImdH6LWxFRwEf+QHQbPiR4 BonlV7UolmIYKZOOkIncq5Sr/Gg+6A2I4YLNQTS/kTE4dzrGiiEXI7SI2qbGUw== Received: from localhost (112.pool92-178-7.dynamic.orange.es [92.178.7.112]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (prime256v1) server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: royger) by smtp.freebsd.org (Postfix) with ESMTPSA id 4fxqLS2MqZznnC; Fri, 17 Apr 2026 09:28:16 +0000 (UTC) (envelope-from royger@freebsd.org) Date: Fri, 17 Apr 2026 11:28:05 +0200 From: Roger Pau =?utf-8?B?TW9ubsOp?= To: Mark Johnston 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: References: <69c640e1.1fc2b.4542bc24@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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é > > AuthorDate: 2026-03-26 10:01:57 +0000 > > Commit: Roger Pau Monné > > 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;