Date: Tue, 11 Oct 2022 14:23:01 +0100 From: Jessica Clarke <jrtc27@freebsd.org> To: "Bjoern A. Zeeb" <bz@FreeBSD.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 99e6980fcf5e - main - device_get_property: add a HANDLE case Message-ID: <7D99DFCA-2455-43E2-A9AB-2C3EDB8BC7AE@freebsd.org> In-Reply-To: <pp1651o-ps99-7610-8o78-7s6qn61rr53@SerrOFQ.bet> References: <202210092153.299LriCA024248@gitrepo.freebsd.org> <9C2F0D00-829F-4EF6-9F27-AC4242C60A3A@freebsd.org> <pp1651o-ps99-7610-8o78-7s6qn61rr53@SerrOFQ.bet>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11 Oct 2022, at 13:10, Bjoern A. Zeeb <bz@FreeBSD.org> wrote: >=20 > On Tue, 11 Oct 2022, Jessica Clarke wrote: >=20 >> On 9 Oct 2022, at 22:53, Bjoern A. Zeeb <bz@FreeBSD.org> wrote: >>>=20 >>> The branch main has been updated by bz: >>>=20 >>> URL: = https://cgit.FreeBSD.org/src/commit/?id=3D99e6980fcf5e12654c3e89b97b774de8= 07d740a4 >>>=20 >>> 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 >>>=20 >>> device_get_property: add a HANDLE case >>>=20 >>> 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. >>=20 >> How=E2=80=99s 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. >=20 > While that is all true, basic types are always easier to handle :) >=20 > 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. Not once you get in to the DSD world, where you have device tree = bindings inside ACPI and want to be able to use one code path for both. > 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. Using a device wouldn=E2=80=99t let you get at intermediary nodes that = have no device. You do want a way to turn the handle into a device in a bus-agnostic way, though. > 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. >=20 > 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. My concern is this is just not quite the right shape for the use cases I=E2=80=99m imagining, but tweaking it slightly (using uintptr_t, or integer-in-a-void *), would allow for that whilst being completely compatible with whatever you=E2=80=99re trying to achieve here. My = problem is I=E2=80=99ve been looking at trying to do more complete DSD things to = support PRP0001 bindings and this isn=E2=80=99t taking into account the = existence of that. Jess > 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. >=20 > /bz >=20 > --=20 > Bjoern A. Zeeb r15:7
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?7D99DFCA-2455-43E2-A9AB-2C3EDB8BC7AE>