Re: General device properties API

[Jason Thorpe <thorpej%me.com@localhost>]

> On Aug 15, 2021, at 9:56 AM, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
> 
>> Date: Sun, 15 Aug 2021 09:10:48 -0700
>> From: Jason Thorpe <thorpej%me.com@localhost>
>> 
>>> On Aug 15, 2021, at 2:08 AM, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> 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