> On Aug 15, 2021, at 9:56 AM, Taylor R Campbell
> <campbell+netbsd-tech-k...@mumble.net> wrote:
>
>> Date: Sun, 15 Aug 2021 09:10:48 -0700
>> From: Jason Thorpe <thor...@me.com>
>>
>>> On Aug 15, 2021, at 2:08 AM, Taylor R Campbell
>>> <campbell+netbsd-tech-k...@mumble.net> wrote:
>>>
>>> This is a little abstract to follow just from the types. Can you show
>>> examples of:
>>>
>>> - code scattered around to reflect device tree properties?
>>
>> Great example starts at: sys/arch/sparc64/sparc64/autoconf.c:1248
>> and continues to line 1319.
>
> Cool, thanks! Can you expand on what it would look like instead with
> the new API? Do you have any diffs to particulary compelling messy
> drivers or buses to show off how this makes things better?
Again, for Ethernet MAC addresses, it’s a little more complicated:
- Sun OpenBoot has a platform binding for them.
- OpenFirmware has a convention (but I don’t know if there was ever a formal
binding), and Sun OpenFirmware implementations, in addition to the general
OpenFirmware convention, also overlay their legacy OpenBoot platform binding on
top of it.
- FDT has a binding for this that is similar to the old OpenFirmware
convention, but that has a bunch of additional rules that we currently don’t
implement.
- ACPI has no formalized binding for this, as far as I can tell, but can
support the FDT properties via DSD.
THAT SAID, an Ethernet domain-specific “get my MAC address” function that has
both a platform binding back-end as well as a generic properties API to use
(along with a set of agreed-upon conventions for when a platform binding does
not exist) is pretty straight-forward.
> How do I know what properties are going to be available in a driver?
> Is it part of an internal kernel<->kernel interface like device
> properties, or just whatever is passed through the OF/FDT or ACPI
> firmware?
Well, right now it’s all very ad hoc, and I think we need to formalize the
conventions. I think the FDT bindings provide solid guidance for device
classes, and we may choose to augment them. But I view that issue as separate
from how the properties themselves are accessed.
> If the latter, what happens if I want to use the same driver on a
> system that might use OF/FDT or ACPI, where OF/FDT or ACPI provide the
> same information by different spellings like `mac-address' vs
> `ethernet-address' (hypothetical)? Or is this intended only for
> drivers where the binding to a particular type of firmware device tree
> is baked into the driver, so this kind of thing won't ever come up?
See above wrt. Ethernet MAC addresses. However, it’s also possible to make
some run-time decisions. For example, it’s possible to ask “what kind of
devhandle do I have?” (by calling devhandle_type()), and based on that, change
the name of the property you query if necessary.
>>> How do clients know they are following the typing rules? Does the
>>> compiler provide any way to detect errors, or can that be detected
>>> only at run-time depending on what OF/FDT/ACPI data the firmware
>>> provides?
>>
>> Code needs to check at run-time, which has been the case for a long
>> time already, and honestly I don't see any problem with that at all.
>
> Well, there's two possibilities and I wasn't sure which one was
> intended in your proposal:
>
> 1. This is an internal kernel<->kernel interface, where one part of
> the kernel (like sparc64/autoconf.c) provides properties and
> another part of the kernel consumes them.
>
> => In this case, it would be reasonable to expect a typed
> definition in a .h file that the provider.c and consumer.c files
> use so the compiler can detect typos and verify the types match
> at compile-time.
>
> 2. This is a driver<->firmware interface, where we have compile-time
> type-checking only on the driver side and there's nothing the
> compiler can do to detect typos or prevent disagreement with
> firmware that the driver might encounter.
>
> => In this case, the best we can hope for is that the firmware
> provides tagged data so that, say, device_getprop_uint64 can
> return failure if the firmware actually has a string and not a
> uint64.
>
> It sounds like the answer is (2) but I wanted to make sure I
> understood correctly.
It’s actually a combination of both. But the API I’ve proposed actually
enforces the types to the best of its ability: full-fidelity for properties
that happen to be in the dictionary, partial fidelity for ACPI properties (no
BOOL type in ACPI), but not really much at all for FDT / OFW / OBP (because
type information does not exist, but it does perform some sanity checking
around queries for NUMBER properties, and there is a convention for BOOL
properties … existence of property -> true).
> For device_properties, in contrast, the answer is (1), and by removing
> a non-type-checkable intermediary (the device_properties dictionary)
> we might improve the situation by reducing the number of places for
> undetectable errors to creep in.
I haven’t removed it. The API in fact checks in the dictionary first, and only
if it does not exist in the dictionary does it query the device tree. This
also has the effect of being able to override device tree property values if
there is a desire or need to do so. Perhaps I was’t clear about that in my
original message. Perhaps just looking at the diff will answer some of these
questions:
https://www.netbsd.org/~thorpej/device-getprop-diff-v1.txt
(This is just the new infrastructure - no call site conversions included,
although I have made several in my local tree.)
> Thanks, that sounds reasonable. Can you expand on what device handles
> are and how they are assigned, maybe with a couple of examples of
> machines that use some combination of PCI, OF/FDT, and ACPI? Might be
> nice to have for a man page EXAMPLES section later on!
Device handles (devhandle_t) are 2-pointer-long structure, intended to be
passed around by value, that contain a pointer to the handle’s backing
implementation (a struct devhandle_impl), as well as an opaque value whose
interpretation is implementation-specific.
The implementation defines the “type” of the handle, and there are currently 4
types defined (in addition to an INVALID type):
- DEVHANDLE_TYPE_ACPI
- DEVHANDLE_TYPE_OF (includes FDT)
- DEVHANDLE_TYPE_OPENBOOT
- DEVHANDLE_TYPE_PRIVATE
Handle implementations can be “sub-classed”, enabling machine-dependent code to
interpose MD behavior on top of generic platform binding code (this is used by
the sparc64 port in my thorpej-i2c-spi-conf2 branch to enumerate i2c devices
that don’t have nodes in the OFW device tree, while still allowing the code to
use the generic OFW/FDT i2c bindings code to enumerate devices that DO have
device tree nodes).
Implementations in addition to a pointer to any “super-class” they inherit from
also provide a function that looks up a “device call” routine by name. It’s
this mechanism that sub-classing uses to interpose… sub-classes look for the
calls they want to interpose, and defer to the super-class for any they don’t
care about.
PRIVATE handles are useful on systems that don’t have any native platform
device tree implementation, but who wish to participate in device tree agnostic
system services (I use this in the thorpej-i2c-spi-conf2 branch to enumerate
i2c devices on the “sandpoint” platform, for example).
Handles are created by implementation specific code like so (using ACPI as an
example):
devhandle_t
devhandle_from_acpi(ACPI_HANDLE const hdl)
{
devhandle_t handle = {
.impl = &acpi_devhandle_impl,
.pointer = hdl,
};
return handle;
}
An example of how they’re assigned, during ACPI-specific device enumeration,
from acpi_rescan_nodes():
ad->ad_device = config_found(sc->sc_dev, &aa, acpi_print,
CFARGS(.iattr = "acpinodebus",
.devhandle = devhandle_from_acpi(ad->ad_handle)));
For PCI, we do it a little differently, because we don’t use the platform
device tree to perform enumeration of PCI. Instead, however, I defined a
PCI-specific service the device tree back-ends can provide that returns a
devhandle for a pcitag_t (this is implemented for OFW / FDT and ACPI):
static devhandle_t
pci_bus_get_child_devhandle(struct pci_softc *sc, pcitag_t tag)
{
struct pci_bus_get_child_devhandle_args args = {
.pc = sc->sc_pc,
.tag = tag,
};
if (device_call(sc->sc_dev, "pci-bus-get-child-devhandle",
&args) != 0) {
/*
* The call is either not supported or the requested
* device was not found in the platform device tree.
* Return an invalid handle.
*/
devhandle_invalidate(&args.devhandle);
}
return args.devhandle;
}
…and in pci_probe_device():
.
.
.
devhandle_t devhandle = pci_bus_get_child_devhandle(sc, pa.pa_tag);
.
.
.
c->c_dev = config_found(sc->sc_dev, &pa, pciprint,
CFARGS(.submatch = config_stdsubmatch,
.locators = locs,
.devhandle = devhandle));
The resulting device_t now has a valid device handle that allows platform
device tree agnostic code to access platform device tree specific services,
such as enumerating child handles, enumerating child devices based on a
specific bus binding (e.g. OFW / FDT and ACPI i2c bindings), or fetching
properties.
(Yes, I really need to write a man page for it.)
-- thorpej
