Re: git: 76bfa33f2598 - main - uart: Go back to returning '0' when we've probed the device.
Date: Tue, 15 Oct 2024 15:32:40 UTC
On 10/15/24 04:01, Warner Losh wrote:
> The branch main has been updated by imp:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=76bfa33f259891a8b9108e20141bd18e2c8d312f
>
> commit 76bfa33f259891a8b9108e20141bd18e2c8d312f
> Author: Warner Losh <imp@FreeBSD.org>
> AuthorDate: 2024-10-15 10:59:29 +0000
> Commit: Warner Losh <imp@FreeBSD.org>
> CommitDate: 2024-10-15 10:59:29 +0000
>
> uart: Go back to returning '0' when we've probed the device.
>
> Two reasons for this: we know it's a uart after we call probe and it
> returns successfully. Second, uart passes data between probe and attach
> with softc. As it is now, we call probe twice, once in the bidding
> process and once after bidding id done. However, the probe process for
> uart isn't completely idempotent (we change state of the uart
> sometimes). The second call can result in odd behavior (though so far
> only in buggy version of other code I've not committed). The bigger
> problem is the softc: newbus creates it, we populate it, then frees it
> when we don't return 0 to claim the device. It then calls us again, we
> repopulate it, and this time it doesn't free it before calling attach.
> Returning 0 avoids both of these issues. The justification for doing it
> in the commit that changed it was 'while I'm here', so there doesn't
> seem to be a use case for it.
Hmm, I think it was changed in this commit which wasn't a "while I'm here":
commit b923a71020ae338f723d9aa01b5365c90aae2fec
Author: Marcel Moolenaar <marcel@FreeBSD.org>
Date: Fri Oct 28 06:30:39 2005 +0000
In uart_bus_probe() return BUS_PROBE_DEFAULT when the probe is
successful.
Notes:
svn path=/head/; revision=151792
diff --git a/sys/dev/uart/uart_core.c b/sys/dev/uart/uart_core.c
index 7b068187e565..b2ff203d0b2a 100644
--- a/sys/dev/uart/uart_core.c
+++ b/sys/dev/uart/uart_core.c
@@ -282,7 +282,7 @@ uart_bus_probe(device_t dev, int regshft, int rclk, int rid, int chan)
error = UART_PROBE(sc);
bus_release_resource(dev, sc->sc_rtype, sc->sc_rrid, sc->sc_rres);
- return (error);
+ return ((error) ? error : BUS_PROBE_DEFAULT);
}
int
It really is a bug to be depending on passing softc state from probe to
attach and it would be ideal to fix that, but going back to returning 0
is ok for now. Maybe instead of '0' though use BUS_PROBE_SPECIFIC?
--
John Baldwin