Re: git: 99e6980fcf5e - main - device_get_property: add a HANDLE case

From: Bjoern A. Zeeb <bz_at_FreeBSD.org>
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