From owner-svn-src-all@freebsd.org Mon Nov 26 19:41:14 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id AC5DD11424E7; Mon, 26 Nov 2018 19:41:14 +0000 (UTC) (envelope-from bwidawsk@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 7631385C58; Mon, 26 Nov 2018 19:41:14 +0000 (UTC) (envelope-from bwidawsk@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 58C13105F9; Mon, 26 Nov 2018 19:41:14 +0000 (UTC) (envelope-from bwidawsk@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id wAQJfEPp081490; Mon, 26 Nov 2018 19:41:14 GMT (envelope-from bwidawsk@FreeBSD.org) Received: (from bwidawsk@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id wAQJfEvL081489; Mon, 26 Nov 2018 19:41:14 GMT (envelope-from bwidawsk@FreeBSD.org) Message-Id: <201811261941.wAQJfEvL081489@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: bwidawsk set sender to bwidawsk@FreeBSD.org using -f From: Ben Widawsky Date: Mon, 26 Nov 2018 19:41:14 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r340993 - head/sys/dev/acpica X-SVN-Group: head X-SVN-Commit-Author: bwidawsk X-SVN-Commit-Paths: head/sys/dev/acpica X-SVN-Commit-Revision: 340993 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 7631385C58 X-Spamd-Result: default: False [1.67 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_SPAM_SHORT(0.40)[0.404,0]; NEURAL_SPAM_MEDIUM(0.61)[0.607,0]; NEURAL_SPAM_LONG(0.66)[0.661,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-Rspamd-Server: mx1.freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Nov 2018 19:41:15 -0000 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);