Re: General device properties API

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

> On Aug 21, 2021, at 2:09 AM, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
> 
> I read the man page, and...I'm more confused now than I was before.

This isn’t exactly a difficult concept to grasp.  I’m honestly baffled at your confusion.

> 1. Who defines properties?  Who is the naming authority that a driver
>   author or auditor consults to determine what the meaning of a
>   property is?  Is it NetBSD, or is it vendor-supplied (and, perhaps,
>   -documented) firmware?  If you set a property, how do you avoid
>   colliding with existing meaning for the name?  How do you audit a
>   driver to determine whether it's making sensible use of the
>   properties?

Right now in NetBSD it’s the wild f***ing west.  It’s my personal opinion that we should just adopt the Device Tree specification's bindings / conventions for everything.

Note that that spec has a naming scheme that allows for vendor-specific and OS-specific properties.

> 2. Why is there now a third copy of essentially the same dictionary
>   lookup API, for devhandle_t?  When would a driver choose device_*
>   or devhandle_* and why?

If you have a kernel device object, use device_getprop_*().  If a kernel device object is not available (because maybe you’re looking at a node from the underlying device tree that doesn’t [yet] have a kernel device object associated with it), then use devhandle_getprop_*().

Again, this isn’t a difficult concept.  Perhaps I need to add additional clarifying language in the man page.

> I'm seeing a lot of new mechanism here but what I really want to see
> is how this helps writing, maintaining, and auditing device drivers.

It’s not super easy to demonstrate with the way our system currently works.  But, for example, there are things for which FDT has a set of bindings for a thing (like i2c HID, for example) and ACPI also has bindings for that thing.  We end up with #ifdefs or other ad hoc run-time checks.  As a first order approximation, every place you see “#ifdef FDT” in a driver is a failure to have some sort of unifying API that works across platform configuration schemes.

But it goes beyond that.  The subject of Ethernet MAC addresses keeps coming up because it’s something that the device property dictionary is used for quite a bit.  But just looking for a “mac-address” property isn’t enough.  The Device Tree spec has a set of rules for Ethernet device as to how the properties are consulted, and in which order, and failing that you then check for data in “nvmem” (which we don’t even yet support, although I have some patches I wrote maybe a year ago floating around that add it that I need to dust off); just in case it’s not clear, we don’t actually this this right at all, currently.  But for Sun systems, even one with PCI network interfaces, there are a slightly different set of rules in how you are supposed to consult system-wide properties in addition to device-local properties.  For ACPI, the trend appears to be heading towards “use Device Tree specification properties encapsulated in _DSD”.  One of my goals here is that there can be a unifying API that allows Ethernet drivers to easily get it right (named something like ether_device_get_mac_address() or whatever) that itself under the covers uses a combination of device_getprop_*() and device_call() (to access a device tree [FDT, ACPI, whatever] or a machine-dependent method, if the platform has one).

> P.S.  I want to get a clearer idea of the higher-level purpose first
> before going into implementation details, but I also want to put up
> front that I'm very uncomfortable with the untyped runtime string
> method name dispatch mechanism of device_call -- and I'd like to see
> that addressed before we start committing to users of it that make it
> harder to change.

As for the way device_call() works, propose an alternative with the following attributes:

==> Allows multiple implementations of a method to exist and be used within the same system (e.g. ACPI and FDT/OFW co-existing).

==> Does not require referencing a symbol that would cause a link failure if a method is not available on some platform scheme.

==> Allows a kind of sub-classing model that allows generic implementations to be overridden or augmented on a per-device handle basis.

==> Allows for platforms that don’t have a platform device tree to still provide those methods so as to participate in the higher-level mechanisms that utilize them.

==> Does not require modifying general-purpose code in order to implement special-purpose methods (i.e. adding a method for PCI should only require modifying code in sys/dev/pci + the OFW / ACPI / whatever code that’s providing the service to PCI).

==> Does not needlessly make it more difficult for NetBSD to have a stable kernel ABI in the future.

…and I’ll consider it.

-- thorpej