> On Aug 21, 2021, at 2:09 AM, Taylor R Campbell 
> <campbell+netbsd-tech-k...@mumble.net> 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

Reply via email to