Date: Mon, 26 Nov 2018 19:41:14 +0000 (UTC) From: Ben Widawsky <bwidawsk@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r340993 - head/sys/dev/acpica Message-ID: <201811261941.wAQJfEvL081489@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bwidawsk Date: Mon Nov 26 19:41:13 2018 New Revision: 340993 URL: https://svnweb.freebsd.org/changeset/base/340993 Log: acpi/ec: Fix regression caused by r340644 After r340644 there were two things wrong in cases where there is both an ECDT, and an EC device exposed via acpica. The first is a rather trivial situation where the device desc would say ECDT even when it was not implicitly created via ECDT (not really sure why the compiler doesn't seem to warn about this). The other more pervasive issue is that the code is designed to essentially not do anything for EC probe when its uid was already created an EC based on the ECDT's uid. The issue was that probe would still return 0 in this case, and so we'd end up with some weird duplication. Now to be honest, I'm not actually sure what exactly broke, but it was definitely not working as intended. To fix this, all that is really needed is to make sure we return ENXIO when we're probing the device already added for the ECDT entry. While here though, move the check for this earlier to avoid wasted cycles when we know after obtaining the uid that it's duplicative. There remains one questionable bit here which I don't want to touch - when doing probe for PNP0C09, if acquiring _UID for the device fails, 0 is assumed, which is a valid UID used by the implicit ECDT. Reported by: Charlie Li, et al. Reviewed by: jhb Differential Revision: https://reviews.freebsd.org/D18311 Modified: head/sys/dev/acpica/acpi_ec.c Modified: head/sys/dev/acpica/acpi_ec.c ============================================================================== --- head/sys/dev/acpica/acpi_ec.c Mon Nov 26 19:39:49 2018 (r340992) +++ head/sys/dev/acpica/acpi_ec.c Mon Nov 26 19:41:13 2018 (r340993) @@ -362,7 +362,8 @@ acpi_ec_probe(device_t dev) ret = 0; goto out; - } + } else + ecdt = 0; ret = ACPI_ID_PROBE(device_get_parent(dev), dev, ec_ids, NULL); if (ret > 0) @@ -382,6 +383,22 @@ acpi_ec_probe(device_t dev) if (ACPI_FAILURE(status)) params->uid = 0; + /* + * Check for a duplicate probe. This can happen when a probe via ECDT + * succeeded already. If this is a duplicate, disable this device. + * + * NB: It would seem device_disable would be sufficient to not get + * duplicated devices, and ENXIO isn't needed, however, device_probe() only + * checks DF_ENABLED at the start and so disabling it here is too late to + * prevent device_attach() from being called. + */ + peer = devclass_get_device(acpi_ec_devclass, params->uid); + if (peer != NULL && device_is_alive(peer)) { + device_disable(dev); + ret = ENXIO; + goto out; + } + status = acpi_GetInteger(h, "_GLK", ¶ms->glk); if (ACPI_FAILURE(status)) params->glk = 0; @@ -421,16 +438,6 @@ acpi_ec_probe(device_t dev) /* Store the values we got from the namespace for attach. */ acpi_set_private(dev, params); - - /* - * Check for a duplicate probe. This can happen when a probe via ECDT - * succeeded already. If this is a duplicate, disable this device. - */ - peer = devclass_get_device(acpi_ec_devclass, params->uid); - if (peer == NULL || !device_is_alive(peer)) - ret = 0; - else - device_disable(dev); if (buf.Pointer) AcpiOsFree(buf.Pointer);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201811261941.wAQJfEvL081489>