svn commit: r340993 - head/sys/dev/acpica

[Ben Widawsky]

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", &params->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);