Re: git: 99e6980fcf5e - main - device_get_property: add a HANDLE case
Date: Tue, 11 Oct 2022 12:10:10 UTC
On Tue, 11 Oct 2022, Jessica Clarke wrote: > On 9 Oct 2022, at 22:53, Bjoern A. Zeeb <bz@FreeBSD.org> wrote: >> >> The branch main has been updated by bz: >> >> URL: https://cgit.FreeBSD.org/src/commit/?id=99e6980fcf5e12654c3e89b97b774de807d740a4 >> >> commit 99e6980fcf5e12654c3e89b97b774de807d740a4 >> Author: Bjoern A. Zeeb <bz@FreeBSD.org> >> AuthorDate: 2022-09-29 12:41:58 +0000 >> Commit: Bjoern A. Zeeb <bz@FreeBSD.org> >> CommitDate: 2022-10-09 21:51:25 +0000 >> >> device_get_property: add a HANDLE case >> >> This will resolve a reference and return the appropriate handle, a node >> on the simplebus or an ACPI_HANDLE for ACPI. For now we do not try to >> further abstract the return type. > > How’s this supposed to be used though? phandle_t is a uint32_t and > ACPI_HANDLE is a void *, which are not the same size (aside from > ILP32), let alone the same type. The existing interface lets you not > care about the bus because the type corresponds to what you ask for > (string or fixed-width integer), but this totally breaks that. This > really should be abstracting the type as well, presumably to a > uintptr_t. While that is all true, basic types are always easier to handle :) All known use cases so far have entirely different code paths in ACPI and FDT as they are "described" differently and there would be more work to be done to harmonize these cases. Making it a uintptr_t wouldn't solve that problem and that later code has to deal with the appropriate type still and call non-bus-bus-specific functions (e.g., acpi_get_device vs. OF_device_from_xref) with the appropriate type and I'd almost vote for extending this to return a device instead rather than a handle, but that may be highly dependent on the use case I am aware of. In earlier code I used the DEVICE_PROP_ANY with the bus-sized encoded handle. Now we get at least proper size checks on each bus, and avoid putting ACPI-internal code into drivers and avoid code duplication there and we have a way to track the use-cases. mw@ had initially asked for this to be broken out of another review and said he's working on more code (I haven't seen) which will hopefully benefit from this and I assume once we get there and some more bus-abstractions may be dealt with, we may be able to refine this. Hence the "For now ..." in the commit message. I'll highly likely defer the MFC for longer than specified for those reasons but I like to have the email to not forget about it. /bz -- Bjoern A. Zeeb r15:7