I intentionally ignored your advice earlier because when I tried to implement it, it seemed impossible to ever support more than a single EC that way.

Perhaps I mangled what you meant though. In my interpretation, you meant to add a check in the probe to find a fixedclass EC device, if found, skip the probe. If not found, continue on with probe. This solution I think always works when there is a single EC. If you ever want to support multiple ECs, it no longer works.

Perhaps that case never will exist, or we should only bother when it does? I'm fine with changing it if you prefer, I just wanted to make my intention clear.

Please advise.

The change as described won't prevent multiple ECs, though the current code already tries to avoid duplicate ECs. I'd be surprised if there was a system with more than one EC. If we encounter one we can revise the current code that tries to handle duplicates.

My suggestion was to use the check to avoid rejecting ECDT devices for which the ACPI_ID_PROBE check would fail. That is, if we had 'device_has_fixed_devclass()' then the entire change in acpi_ec.c would be to make the first few lines in acpi_ec_probe() this:

/* Check that this is a device and that EC is not disabled. */
if (acpi_get_type(dev) != ACPI_TYPE_DEVICE || acpi_disabled("ec") ||
    (!device_has_fixed_devclass(dev) && ACPI_ID_PROBE(device_get_parent(dev), dev, ec_ids) != 0))
    return (ENXIO);

Those lines are a bit long though so I might structure it as something like:

if (acpi_get_type(dev) != ACPI_TYPE_DEVICE || acpi_disabled("ec"))
    return (ENXIO);

if (device_has_fixed_devclass(dev)) {
    ecdt = 1;
    params = acpi_get_private(dev);
    if (params != NULL)
        ret = 0;
    else
        ret = ENXIO;
} else {
    ret = ACPI_ID_PROBE(device_get_parent(dev), dev, ec_ids);
    if (ret > 0)
        return (ret);

    params = malloc(...);
    /* Existing non-ECDT code */
}

This also ensures that the invariant about parameters only existing for ECDT devices still holds.

Won't this fail if another ACPI device has a fixed class type, and private data? It seems to just be kicking the can down the road unless I don't understand quite right.

No, because if it has a fixed device class then it can only be probed by drivers whose name matches the fixed devclass. Thus, in this case, only drivers that have the "acpi_ec" name (of which there is only this driver). We do have some device classes (names) that have multiple drivers (e.g. "pci" and "pcib"), but "acpi_ec" only has a single driver, so this test is sufficient.

You're right. I will respin to add the accessor and use that flag.

On looking a bit closer, won't we pretty much always get a FIXEDCLASS device? The only case we do not is when there is no ECDT, or what am I missing here?

Yes, if there is no ECDT but the EC is still enumerated as a Device() with a EC _HID. That is the 'params == NULL' case.

Okay, just seemed a bit weird that the most common case is probably there is an ECDT and EC Device(), which we're calling "FIXEDCLASS"... I'm fine with that, just making sure I understood first.

Yes, it is odd. :( I think there were older systems that only used ECDT or only used Device(). I guess we have something in place to not create two device_t objects for the EC if you have both, but I'd have to look some more to see how that works.

Actually, my ivy bridge desktop doesn't have an ECDT at all, only a Device(). Checking the 6.2 spec it seems that ECDT is optional, and it wasn't present in ACPI 1.0. I actually don't know what we do on a system with the ECDT present currently. It seems like we might create two acpi_ec devices. I wonder if the second one (via Device()) fails to attach because it's resources are already allocated?