 @@ -1,14 +1,14 @@
-/*	$NetBSD: acpi_ec.c,v 1.86 2021/12/31 17:22:25 riastradh Exp $	*/
+/*	$NetBSD: acpi_ec.c,v 1.86.4.1 2023/07/30 12:01:53 martin Exp $	*/
 /*-
  * Copyright (c) 2007 Joerg Sonnenberger <joerg@NetBSD.org>.
  * All rights reserved.
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
+ *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in
 @@ -24,52 +24,54 @@
  * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
  * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
  * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
  * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
 /*
  * The ACPI Embedded Controller (EC) driver serves two different purposes:
  * - read and write access from ASL, e.g. to read battery state
  * - notification of ASL of System Control Interrupts.
+ *
- * Access to the EC is serialised by sc_access_mtx and optionally the
+ * Lock order:
- * ACPI global mutex.  Both locks are held until the request is fulfilled.
+ *	sc_access_mtx (serializes EC transactions -- read, write, or SCI)
- * All access to the softc has to hold sc_mtx to serialise against the GPE
+ *	-> ACPI global lock (excludes other ACPI access during EC transaction)
- * handler and the callout.  sc_mtx is also used for wakeup conditions.
+ *	-> sc_mtx (serializes state machine transitions and waits)
+ *
- * SCIs are processed in a kernel thread. Handling gets a bit complicated
+ * SCIs are processed in a kernel thread.
  * by the lock order (sc_mtx must be acquired after sc_access_mtx and the
  * ACPI global mutex).
+ *
  * Read and write requests spin around for a short time as many requests
  * can be handled instantly by the EC.  During normal processing interrupt
  * mode is used exclusively.  At boot and resume time interrupts are not
  * working and the handlers just busy loop.
+ *
  * A callout is scheduled to compensate for missing interrupts on some
  * hardware.  If the EC doesn't process a request for 5s, it is most likely
  * in a wedged state.  No method to reset the EC is currently known.
+ *
  * Special care has to be taken to not poll the EC in a busy loop without
  * delay.  This can prevent processing of Power Button events. At least some
  * Lenovo Thinkpads seem to be implement the Power Button Override in the EC
  * and the only option to recover on those models is to cut off all power.
  */
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.86 2021/12/31 17:22:25 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.86.4.1 2023/07/30 12:01:53 martin Exp $");
 #ifdef _KERNEL_OPT
 #include "opt_acpi_ec.h"
 #endif
 #include <sys/param.h>
 #include <sys/callout.h>
 #include <sys/condvar.h>
 #include <sys/device.h>
 #include <sys/kernel.h>
 #include <sys/kthread.h>
 #include <sys/mutex.h>
 #include <sys/systm.h>
 #include <dev/acpi/acpireg.h>
 #include <dev/acpi/acpivar.h>
 #include <dev/acpi/acpi_ecvar.h>
 @@ -91,92 +93,159 @@ ACPI_MODULE_NAME            ("acpi_ec")
 #define	EC_COMMAND_WRITE	0x81
 #define	EC_COMMAND_BURST_EN	0x82
 #define	EC_COMMAND_BURST_DIS	0x83
 #define	EC_COMMAND_QUERY	0x84
 /* From ACPI 3.0b, chapter 12.2.1 */
 #define	EC_STATUS_OBF		0x01
 #define	EC_STATUS_IBF		0x02
 #define	EC_STATUS_CMD		0x08
 #define	EC_STATUS_BURST		0x10
 #define	EC_STATUS_SCI		0x20
 #define	EC_STATUS_SMI		0x40
 #define	EC_STATUS_FMT							      \
 	"\x10\10IGN7\7SMI\6SCI\5BURST\4CMD\3IGN2\2IBF\1OBF"
 static const struct device_compatible_entry compat_data[] = {
 	{ .compat = "PNP0C09" },
 	DEVICE_COMPAT_EOL
 };
 #define	EC_STATE_ENUM(F)						      \
 	F(EC_STATE_QUERY, "QUERY")					      \
 	F(EC_STATE_QUERY_VAL, "QUERY_VAL")				      \
 	F(EC_STATE_READ, "READ")					      \
 	F(EC_STATE_READ_ADDR, "READ_ADDR")				      \
 	F(EC_STATE_READ_VAL, "READ_VAL")				      \
 	F(EC_STATE_WRITE, "WRITE")					      \
 	F(EC_STATE_WRITE_ADDR, "WRITE_ADDR")				      \
 	F(EC_STATE_WRITE_VAL, "WRITE_VAL")				      \
 	F(EC_STATE_FREE, "FREE")					      \
 enum ec_state_t {
-	EC_STATE_QUERY,
+#define	F(N, S)	N,
-	EC_STATE_QUERY_VAL,
+	EC_STATE_ENUM(F)
-	EC_STATE_READ,
+#undef F
-	EC_STATE_READ_ADDR,
+};
 	EC_STATE_READ_VAL,
-	EC_STATE_WRITE,
+#ifdef ACPIEC_DEBUG
-	EC_STATE_WRITE_ADDR,
+static const char *const acpiec_state_names[] = {
-	EC_STATE_WRITE_VAL,
+#define F(N, S)	[N] = S,
-	EC_STATE_FREE
+	EC_STATE_ENUM(F)
 #undef F
 };
 #endif
 struct acpiec_softc {
 	device_t sc_dev;
 	ACPI_HANDLE sc_ech;
 	ACPI_HANDLE sc_gpeh;
 	uint8_t sc_gpebit;
 	bus_space_tag_t sc_data_st;
 	bus_space_handle_t sc_data_sh;
 	bus_space_tag_t sc_csr_st;
 	bus_space_handle_t sc_csr_sh;
 	bool sc_need_global_lock;
 	uint32_t sc_global_lock;
 	kmutex_t sc_mtx, sc_access_mtx;
 	kcondvar_t sc_cv, sc_cv_sci;
 	enum ec_state_t sc_state;
 	bool sc_got_sci;
 	callout_t sc_pseudo_intr;
 	uint8_t sc_cur_addr, sc_cur_val;
 };
 #ifdef ACPIEC_DEBUG
 #define	ACPIEC_DEBUG_ENUM(F)						      \
 	F(ACPIEC_DEBUG_REG, "REG")					      \
 	F(ACPIEC_DEBUG_RW, "RW")					      \
 	F(ACPIEC_DEBUG_QUERY, "QUERY")					      \
 	F(ACPIEC_DEBUG_TRANSITION, "TRANSITION")			      \
 	F(ACPIEC_DEBUG_INTR, "INTR")					      \
 enum {
 #define	F(N, S)	N,
 	ACPIEC_DEBUG_ENUM(F)
 #undef F
 };
 static const char *const acpiec_debug_names[] = {
 #define	F(N, S)	[N] = S,
 	ACPIEC_DEBUG_ENUM(F)
 #undef F
 };
 int acpiec_debug = ACPIEC_DEBUG;
 #define	DPRINTF(n, sc, fmt, ...) do					      \
 {									      \
 	if (acpiec_debug & __BIT(n)) {					      \
 		char dprintbuf[16];					      \
 		const char *state;					      \
+									      \
 		/* paranoia */						      \
 		if ((sc)->sc_state < __arraycount(acpiec_state_names)) {      \
 			state = acpiec_state_names[(sc)->sc_state];	      \
 		} else {						      \
 			snprintf(dprintbuf, sizeof(dprintbuf), "0x%x",	      \
 			    (sc)->sc_state);				      \
 			state = dprintbuf;				      \
 		}							      \
+									      \
 		device_printf((sc)->sc_dev, "(%s) [%s] "fmt,		      \
 		    acpiec_debug_names[n], state, ##__VA_ARGS__);	      \
 	}								      \
 } while (0)
 #else
 #define	DPRINTF(n, sc, fmt, ...)	__nothing
 #endif
 static int acpiecdt_match(device_t, cfdata_t, void *);
 static void acpiecdt_attach(device_t, device_t, void *);
 static int acpiec_match(device_t, cfdata_t, void *);
 static void acpiec_attach(device_t, device_t, void *);
 static void acpiec_common_attach(device_t, device_t, ACPI_HANDLE,
     bus_space_tag_t, bus_addr_t, bus_space_tag_t, bus_addr_t,
     ACPI_HANDLE, uint8_t);
 static bool acpiec_suspend(device_t, const pmf_qual_t *);
 static bool acpiec_resume(device_t, const pmf_qual_t *);
 static bool acpiec_shutdown(device_t, int);
 static bool acpiec_parse_gpe_package(device_t, ACPI_HANDLE,
     ACPI_HANDLE *, uint8_t *);
 static void acpiec_callout(void *);
 static void acpiec_gpe_query(void *);
 static uint32_t acpiec_gpe_handler(ACPI_HANDLE, uint32_t, void *);
 static ACPI_STATUS acpiec_space_setup(ACPI_HANDLE, uint32_t, void *, void **);
 static ACPI_STATUS acpiec_space_handler(uint32_t, ACPI_PHYSICAL_ADDRESS,
     uint32_t, ACPI_INTEGER *, void *, void *);
-static void acpiec_gpe_state_machine(device_t);
+static void acpiec_gpe_state_machine(struct acpiec_softc *);
 CFATTACH_DECL_NEW(acpiec, sizeof(struct acpiec_softc),
     acpiec_match, acpiec_attach, NULL, NULL);
 CFATTACH_DECL_NEW(acpiecdt, sizeof(struct acpiec_softc),
     acpiecdt_match, acpiecdt_attach, NULL, NULL);
 static device_t ec_singleton = NULL;
 static bool acpiec_cold = false;
 static bool
 acpiecdt_find(device_t parent, ACPI_HANDLE *ec_handle,
     bus_addr_t *cmd_reg, bus_addr_t *data_reg, uint8_t *gpebit)
 @@ -303,26 +372,28 @@ fail0:	if (!pmf_device_register(self, NU
 		aprint_error_dev(self, "couldn't establish power handler\n");
+}
 static void
 acpiec_common_attach(device_t parent, device_t self,
     ACPI_HANDLE ec_handle, bus_space_tag_t cmdt, bus_addr_t cmd_reg,
     bus_space_tag_t datat, bus_addr_t data_reg,
     ACPI_HANDLE gpe_handle, uint8_t gpebit)
+{
 	struct acpiec_softc *sc = device_private(self);
 	ACPI_STATUS rv;
 	ACPI_INTEGER val;
 	sc->sc_dev = self;
 	sc->sc_csr_st = cmdt;
 	sc->sc_data_st = datat;
 	sc->sc_ech = ec_handle;
 	sc->sc_gpeh = gpe_handle;
 	sc->sc_gpebit = gpebit;
 	sc->sc_state = EC_STATE_FREE;
 	mutex_init(&sc->sc_mtx, MUTEX_DRIVER, IPL_TTY);
 	mutex_init(&sc->sc_access_mtx, MUTEX_DEFAULT, IPL_NONE);
 	cv_init(&sc->sc_cv, "eccv");
 	cv_init(&sc->sc_cv_sci, "ecsci");
 @@ -341,54 +412,54 @@ acpiec_common_attach(device_t parent, de
 	if (rv == AE_OK) {
 		sc->sc_need_global_lock = val != 0;
 	} else if (rv != AE_NOT_FOUND) {
 		aprint_error_dev(self, "unable to evaluate _GLK: %s\n",
 		    AcpiFormatException(rv));
 		goto post_csr_map;
 	} else {
 		sc->sc_need_global_lock = false;
+	}
 	if (sc->sc_need_global_lock)
 		aprint_normal_dev(self, "using global ACPI lock\n");
 	callout_init(&sc->sc_pseudo_intr, CALLOUT_MPSAFE);
-	callout_setfunc(&sc->sc_pseudo_intr, acpiec_callout, self);
+	callout_setfunc(&sc->sc_pseudo_intr, acpiec_callout, sc);
 	rv = AcpiInstallAddressSpaceHandler(sc->sc_ech, ACPI_ADR_SPACE_EC,
-	    acpiec_space_handler, acpiec_space_setup, self);
+	    acpiec_space_handler, acpiec_space_setup, sc);
 	if (rv != AE_OK) {
 		aprint_error_dev(self,
 		    "unable to install address space handler: %s\n",
 		    AcpiFormatException(rv));
 		goto post_csr_map;
+	}
 	rv = AcpiInstallGpeHandler(sc->sc_gpeh, sc->sc_gpebit,
-	    ACPI_GPE_EDGE_TRIGGERED, acpiec_gpe_handler, self);
+	    ACPI_GPE_EDGE_TRIGGERED, acpiec_gpe_handler, sc);
 	if (rv != AE_OK) {
 		aprint_error_dev(self, "unable to install GPE handler: %s\n",
 		    AcpiFormatException(rv));
 		goto post_csr_map;
+	}
 	rv = AcpiEnableGpe(sc->sc_gpeh, sc->sc_gpebit);
 	if (rv != AE_OK) {
 		aprint_error_dev(self, "unable to enable GPE: %s\n",
 		    AcpiFormatException(rv));
 		goto post_csr_map;
+	}
 	if (kthread_create(PRI_NONE, KTHREAD_MPSAFE, NULL, acpiec_gpe_query,
-		           self, NULL, "acpiec sci thread")) {
+		sc, NULL, "acpiec sci thread")) {
 		aprint_error_dev(self, "unable to create query kthread\n");
 		goto post_csr_map;
+	}
 	ec_singleton = self;
 	if (!pmf_device_register1(self, acpiec_suspend, acpiec_resume,
 	    acpiec_shutdown))
 		aprint_error_dev(self, "couldn't establish power handler\n");
 	return;
 post_csr_map:
 @@ -396,46 +467,68 @@ post_csr_map:
 	    acpiec_gpe_handler);
 	(void)AcpiRemoveAddressSpaceHandler(sc->sc_ech,
 	    ACPI_ADR_SPACE_EC, acpiec_space_handler);
 	bus_space_unmap(sc->sc_csr_st, sc->sc_csr_sh, 1);
 post_data_map:
 	bus_space_unmap(sc->sc_data_st, sc->sc_data_sh, 1);
 	if (!pmf_device_register(self, NULL, NULL))
 		aprint_error_dev(self, "couldn't establish power handler\n");
+}
 static bool
 acpiec_suspend(device_t dv, const pmf_qual_t *qual)
+{
 	struct acpiec_softc *sc = device_private(dv);
 	/*
 	 * XXX This looks bad because acpiec_cold is global and
 	 * sc->sc_mtx doesn't look like it's global, but we can have
 	 * only one acpiec(4) device anyway.  Maybe acpiec_cold should
 	 * live in the softc to make this look less bad?
+	 *
 	 * XXX Should this block read/write/query transactions until
 	 * resume?
+	 *
 	 * XXX Should this interrupt existing transactions to make them
 	 * fail promptly or restart on resume?
 	 */
 	mutex_enter(&sc->sc_mtx);
 	acpiec_cold = true;
 	mutex_exit(&sc->sc_mtx);
 	return true;
+}
 static bool
 acpiec_resume(device_t dv, const pmf_qual_t *qual)
+{
 	struct acpiec_softc *sc = device_private(dv);
 	mutex_enter(&sc->sc_mtx);
 	acpiec_cold = false;
 	mutex_exit(&sc->sc_mtx);
 	return true;
+}
 static bool
 acpiec_shutdown(device_t dv, int how)
+{
 	struct acpiec_softc *sc = device_private(dv);
 	mutex_enter(&sc->sc_mtx);
 	acpiec_cold = true;
 	mutex_exit(&sc->sc_mtx);
 	return true;
+}
 static bool
 acpiec_parse_gpe_package(device_t self, ACPI_HANDLE ec_handle,
     ACPI_HANDLE *gpe_handle, uint8_t *gpebit)
+{
 	ACPI_BUFFER buf;
 	ACPI_OBJECT *p, *c;
 	ACPI_STATUS rv;
 	rv = acpi_eval_struct(ec_handle, "_GPE", &buf);
 	if (rv != AE_OK) {
 @@ -481,197 +574,244 @@ acpiec_parse_gpe_package(device_t self,
 		aprint_error_dev(self,
 		    "_GPE package needs integer as 2nd field\n");
 		ACPI_FREE(p);
 		return false;
+	}
 	*gpebit = c->Integer.Value;
 	ACPI_FREE(p);
 	return true;
+}
 static uint8_t
 acpiec_read_data(struct acpiec_softc *sc)
+{
-	return bus_space_read_1(sc->sc_data_st, sc->sc_data_sh, 0);
+	uint8_t x;
 	KASSERT(mutex_owned(&sc->sc_mtx));
 	x = bus_space_read_1(sc->sc_data_st, sc->sc_data_sh, 0);
 	DPRINTF(ACPIEC_DEBUG_REG, sc, "read data=0x%"PRIx8"\n", x);
 	return x;
+}
 static void
 acpiec_write_data(struct acpiec_softc *sc, uint8_t val)
+{
 	KASSERT(mutex_owned(&sc->sc_mtx));
 	DPRINTF(ACPIEC_DEBUG_REG, sc, "write data=0x%"PRIx8"\n", val);
 	bus_space_write_1(sc->sc_data_st, sc->sc_data_sh, 0, val);
+}
 static uint8_t
 acpiec_read_status(struct acpiec_softc *sc)
+{
-	return bus_space_read_1(sc->sc_csr_st, sc->sc_csr_sh, 0);
+	uint8_t x;
 	KASSERT(mutex_owned(&sc->sc_mtx));
 	x = bus_space_read_1(sc->sc_csr_st, sc->sc_csr_sh, 0);
 	DPRINTF(ACPIEC_DEBUG_REG, sc, "read status=0x%"PRIx8"\n", x);
 	return x;
+}
 static void
 acpiec_write_command(struct acpiec_softc *sc, uint8_t cmd)
+{
 	KASSERT(mutex_owned(&sc->sc_mtx));
 	DPRINTF(ACPIEC_DEBUG_REG, sc, "write command=0x%"PRIx8"\n", cmd);
 	bus_space_write_1(sc->sc_csr_st, sc->sc_csr_sh, 0, cmd);
+}
 static ACPI_STATUS
 acpiec_space_setup(ACPI_HANDLE region, uint32_t func, void *arg,
     void **region_arg)
+{
 	if (func == ACPI_REGION_DEACTIVATE)
 		*region_arg = NULL;
 	else
 		*region_arg = arg;
 	return AE_OK;
+}
 static void
-acpiec_lock(device_t dv)
+acpiec_lock(struct acpiec_softc *sc)
+{
 	struct acpiec_softc *sc = device_private(dv);
 	ACPI_STATUS rv;
 	mutex_enter(&sc->sc_access_mtx);
 	if (sc->sc_need_global_lock) {
 		rv = AcpiAcquireGlobalLock(EC_LOCK_TIMEOUT,
 		    &sc->sc_global_lock);
 		if (rv != AE_OK) {
-			aprint_error_dev(dv,
+			aprint_error_dev(sc->sc_dev,
 			    "failed to acquire global lock: %s\n",
 			    AcpiFormatException(rv));
 			return;
+		}
+	}
+}
 static void
-acpiec_unlock(device_t dv)
+acpiec_unlock(struct acpiec_softc *sc)
+{
 	struct acpiec_softc *sc = device_private(dv);
 	ACPI_STATUS rv;
 	if (sc->sc_need_global_lock) {
 		rv = AcpiReleaseGlobalLock(sc->sc_global_lock);
 		if (rv != AE_OK) {
-			aprint_error_dev(dv,
+			aprint_error_dev(sc->sc_dev,
 			    "failed to release global lock: %s\n",
 			    AcpiFormatException(rv));
+		}
+	}
 	mutex_exit(&sc->sc_access_mtx);
+}
 static ACPI_STATUS
-acpiec_read(device_t dv, uint8_t addr, uint8_t *val)
+acpiec_wait_timeout(struct acpiec_softc *sc)
+{
-	struct acpiec_softc *sc = device_private(dv);
+	device_t dv = sc->sc_dev;
-	int i, timeo = 1000 * EC_CMD_TIMEOUT;
+	int i;
 	acpiec_lock(dv);
 	mutex_enter(&sc->sc_mtx);
 	sc->sc_cur_addr = addr;
 	sc->sc_state = EC_STATE_READ;
 	for (i = 0; i < EC_POLL_TIMEOUT; ++i) {
-		acpiec_gpe_state_machine(dv);
+		acpiec_gpe_state_machine(sc);
 		if (sc->sc_state == EC_STATE_FREE)
-			goto done;
+			return AE_OK;
 		delay(1);
+	}
 	DPRINTF(ACPIEC_DEBUG_RW, sc, "SCI polling timeout\n");
 	if (cold || acpiec_cold) {
 		int timeo = 1000 * EC_CMD_TIMEOUT;
 		while (sc->sc_state != EC_STATE_FREE && timeo-- > 0) {
 			delay(1000);
-			acpiec_gpe_state_machine(dv);
+			acpiec_gpe_state_machine(sc);
+		}
 		if (sc->sc_state != EC_STATE_FREE) {
 			mutex_exit(&sc->sc_mtx);
 			acpiec_unlock(dv);
 			aprint_error_dev(dv, "command timed out, state %d\n",
 			    sc->sc_state);
 			return AE_ERROR;
+		}
-	} else if (cv_timedwait(&sc->sc_cv, &sc->sc_mtx, EC_CMD_TIMEOUT * hz)) {
+	} else {
-		mutex_exit(&sc->sc_mtx);
+		const unsigned deadline = getticks() + EC_CMD_TIMEOUT*hz;
-		acpiec_unlock(dv);
+		unsigned delta;
 		aprint_error_dev(dv,
-		    "command takes over %d sec...\n", EC_CMD_TIMEOUT);
+		while (sc->sc_state != EC_STATE_FREE &&
-		return AE_ERROR;
+		    (delta = deadline - getticks()) < INT_MAX)
 			(void)cv_timedwait(&sc->sc_cv, &sc->sc_mtx, delta);
 		if (sc->sc_state != EC_STATE_FREE) {
 			aprint_error_dev(dv,
 			    "command takes over %d sec...\n",
 			    EC_CMD_TIMEOUT);
 			return AE_ERROR;
+		}
+	}
-done:
+	return AE_OK;
+}
 static ACPI_STATUS
 acpiec_read(struct acpiec_softc *sc, uint8_t addr, uint8_t *val)
+{
 	ACPI_STATUS rv;
 	acpiec_lock(sc);
 	mutex_enter(&sc->sc_mtx);
 	DPRINTF(ACPIEC_DEBUG_RW, sc,
 	    "pid %ld %s, lid %ld%s%s: read addr 0x%"PRIx8"\n",
 	    (long)curproc->p_pid, curproc->p_comm,
 	    (long)curlwp->l_lid, curlwp->l_name ? " " : "",
 	    curlwp->l_name ? curlwp->l_name : "",
 	    addr);
 	KASSERT(sc->sc_state == EC_STATE_FREE);
 	sc->sc_cur_addr = addr;
 	sc->sc_state = EC_STATE_READ;
 	rv = acpiec_wait_timeout(sc);
 	if (ACPI_FAILURE(rv))
 		goto out;
 	DPRINTF(ACPIEC_DEBUG_RW, sc,
 	    "pid %ld %s, lid %ld%s%s: read addr 0x%"PRIx8": 0x%"PRIx8"\n",
 	    (long)curproc->p_pid, curproc->p_comm,
 	    (long)curlwp->l_lid, curlwp->l_name ? " " : "",
 	    curlwp->l_name ? curlwp->l_name : "",
 	    addr, sc->sc_cur_val);
 	*val = sc->sc_cur_val;
-	mutex_exit(&sc->sc_mtx);
+out:	mutex_exit(&sc->sc_mtx);
-	acpiec_unlock(dv);
+	acpiec_unlock(sc);
-	return AE_OK;
+	return rv;
+}
 static ACPI_STATUS
-acpiec_write(device_t dv, uint8_t addr, uint8_t val)
+acpiec_write(struct acpiec_softc *sc, uint8_t addr, uint8_t val)
+{
-	struct acpiec_softc *sc = device_private(dv);
+	ACPI_STATUS rv;
 	int i, timeo = 1000 * EC_CMD_TIMEOUT;
-	acpiec_lock(dv);
+	acpiec_lock(sc);
 	mutex_enter(&sc->sc_mtx);
 	DPRINTF(ACPIEC_DEBUG_RW, sc,
 	    "pid %ld %s, lid %ld%s%s write addr 0x%"PRIx8": 0x%"PRIx8"\n",
 	    (long)curproc->p_pid, curproc->p_comm,
 	    (long)curlwp->l_lid, curlwp->l_name ? " " : "",
 	    curlwp->l_name ? curlwp->l_name : "",
 	    addr, val);
 	KASSERT(sc->sc_state == EC_STATE_FREE);
 	sc->sc_cur_addr = addr;
 	sc->sc_cur_val = val;
 	sc->sc_state = EC_STATE_WRITE;
-	for (i = 0; i < EC_POLL_TIMEOUT; ++i) {
+	rv = acpiec_wait_timeout(sc);
-		acpiec_gpe_state_machine(dv);
+	if (ACPI_FAILURE(rv))
-		if (sc->sc_state == EC_STATE_FREE)
+		goto out;
 			goto done;
 		delay(1);
-	if (cold || acpiec_cold) {
+	DPRINTF(ACPIEC_DEBUG_RW, sc,
-		while (sc->sc_state != EC_STATE_FREE && timeo-- > 0) {
+	    "pid %ld %s, lid %ld%s%s: write addr 0x%"PRIx8": 0x%"PRIx8
-			delay(1000);
+	    " done\n",
-			acpiec_gpe_state_machine(dv);
+	    (long)curproc->p_pid, curproc->p_comm,
 	    (long)curlwp->l_lid, curlwp->l_name ? " " : "",
-		if (sc->sc_state != EC_STATE_FREE) {
+	    curlwp->l_name ? curlwp->l_name : "",
-			mutex_exit(&sc->sc_mtx);
+	    addr, val);
 			acpiec_unlock(dv);
 			aprint_error_dev(dv, "command timed out, state %d\n",
 			    sc->sc_state);
 			return AE_ERROR;
 	} else if (cv_timedwait(&sc->sc_cv, &sc->sc_mtx, EC_CMD_TIMEOUT * hz)) {
 		mutex_exit(&sc->sc_mtx);
 		acpiec_unlock(dv);
 		aprint_error_dev(dv,
 		    "command takes over %d sec...\n", EC_CMD_TIMEOUT);
 		return AE_ERROR;
-done:
+out:	mutex_exit(&sc->sc_mtx);
-	mutex_exit(&sc->sc_mtx);
+	acpiec_unlock(sc);
-	acpiec_unlock(dv);
+	return rv;
 	return AE_OK;
+}
 /*
  * acpiec_space_handler(func, paddr, bitwidth, value, arg, region_arg)
+ *
  *	Transfer bitwidth/8 bytes of data between paddr and *value:
  *	from paddr to *value when func is ACPI_READ, and the other way
- *	when func is ACPI_WRITE.  arg is the acpiec(4) or acpiecdt(4)
+ *	when func is ACPI_WRITE.  arg is the acpiec_softc pointer.
- *	device.  region_arg is ignored (XXX why? determined by
+ *	region_arg is ignored (XXX why? determined by
  *	acpiec_space_setup but never used by anything that I can see).
+ *
  *	The caller always provides storage at *value large enough for
  *	an ACPI_INTEGER object, i.e., a 64-bit integer.  However,
  *	bitwidth may be larger; in this case the caller provides larger
  *	storage at *value, e.g. 128 bits as documented in
  *	<https://gnats.netbsd.org/55206>.
+ *
  *	On reads, this fully initializes one ACPI_INTEGER's worth of
  *	data at *value, even if bitwidth < 64.  The integer is
  *	interpreted in host byte order; in other words, bytes of data
  *	are transferred in order between paddr and (uint8_t *)value.
  *	The transfer is not atomic; it may go byte-by-byte.
 @@ -681,247 +821,298 @@ done:
  *	in the low-order bits of the result.  A big-endian system could
  *	read a 64-bit integer in big-endian (and it did for a while!),
  *	but what should it do for larger reads?  Unclear!
+ *
  *	XXX It's not clear whether the object at *value is always
  *	_aligned_ adequately for an ACPI_INTEGER object.  Currently it
  *	always is as long as malloc, used by AcpiOsAllocate, returns
  *	64-bit-aligned data.
  */
 static ACPI_STATUS
 acpiec_space_handler(uint32_t func, ACPI_PHYSICAL_ADDRESS paddr,
     uint32_t width, ACPI_INTEGER *value, void *arg, void *region_arg)
+{
-	device_t dv;
+	struct acpiec_softc *sc = arg;
 	ACPI_STATUS rv;
 	uint8_t addr, *buf;
 	unsigned int i;
 	if (paddr > 0xff || width % 8 != 0 ||
 	    value == NULL || arg == NULL || paddr + width / 8 > 0x100)
 		return AE_BAD_PARAMETER;
 	addr = paddr;
 	dv = arg;
 	buf = (uint8_t *)value;
 	rv = AE_OK;
 	switch (func) {
 	case ACPI_READ:
 		for (i = 0; i < width; i += 8, ++addr, ++buf) {
-			rv = acpiec_read(dv, addr, buf);
+			rv = acpiec_read(sc, addr, buf);
 			if (rv != AE_OK)
 				break;
+		}
 		/*
 		 * Make sure to fully initialize at least an
 		 * ACPI_INTEGER-sized object.
 		 */
 		for (; i < sizeof(*value)*8; i += 8, ++buf)
 			*buf = 0;
 		break;
 	case ACPI_WRITE:
 		for (i = 0; i < width; i += 8, ++addr, ++buf) {
-			rv = acpiec_write(dv, addr, *buf);
+			rv = acpiec_write(sc, addr, *buf);
 			if (rv != AE_OK)
 				break;
+		}
 		break;
 	default:
-		aprint_error("%s: invalid Address Space function called: %x\n",
+		aprint_error_dev(sc->sc_dev,
-		    device_xname(dv), (unsigned int)func);
+		    "invalid Address Space function called: %x\n",
 		    (unsigned int)func);
 		return AE_BAD_PARAMETER;
+	}
 	return rv;
+}
 static void
 acpiec_wait(struct acpiec_softc *sc)
+{
 	int i;
 	/*
 	 * First, attempt to get the query by polling.
 	 */
 	for (i = 0; i < EC_POLL_TIMEOUT; ++i) {
 		acpiec_gpe_state_machine(sc);
 		if (sc->sc_state == EC_STATE_FREE)
 			return;
 		delay(1);
+	}
 	/*
 	 * Polling timed out.  Try waiting for interrupts -- either GPE
 	 * interrupts, or periodic callouts in case GPE interrupts are
 	 * broken.
 	 */
 	DPRINTF(ACPIEC_DEBUG_QUERY, sc, "SCI polling timeout\n");
 	while (sc->sc_state != EC_STATE_FREE)
 		cv_wait(&sc->sc_cv, &sc->sc_mtx);
+}
 static void
 acpiec_gpe_query(void *arg)
+{
-	device_t dv = arg;
+	struct acpiec_softc *sc = arg;
 	struct acpiec_softc *sc = device_private(dv);
 	uint8_t reg;
 	char qxx[5];
 	ACPI_STATUS rv;
 	int i;
 loop:
 	/*
 	 * Wait until the EC sends an SCI requesting a query.
 	 */
 	mutex_enter(&sc->sc_mtx);
 	while (!sc->sc_got_sci)
 	if (sc->sc_got_sci == false)
 		cv_wait(&sc->sc_cv_sci, &sc->sc_mtx);
 	DPRINTF(ACPIEC_DEBUG_QUERY, sc, "SCI query requested\n");
 	mutex_exit(&sc->sc_mtx);
-	acpiec_lock(dv);
+	/*
 	 * EC wants to submit a query to us.  Exclude concurrent reads
 	 * and writes while we handle it.
 	 */
 	acpiec_lock(sc);
 	mutex_enter(&sc->sc_mtx);
 	DPRINTF(ACPIEC_DEBUG_QUERY, sc, "SCI query\n");
 	KASSERT(sc->sc_state == EC_STATE_FREE);
 	/* The Query command can always be issued, so be defensive here. */
 	KASSERT(sc->sc_got_sci);
 	sc->sc_got_sci = false;
 	sc->sc_state = EC_STATE_QUERY;
-	for (i = 0; i < EC_POLL_TIMEOUT; ++i) {
+	acpiec_wait(sc);
 		acpiec_gpe_state_machine(dv);
 		if (sc->sc_state == EC_STATE_FREE)
 			goto done;
 		delay(1);
 	cv_wait(&sc->sc_cv, &sc->sc_mtx);
 done:
 	reg = sc->sc_cur_val;
 	DPRINTF(ACPIEC_DEBUG_QUERY, sc, "SCI query: 0x%"PRIx8"\n", reg);
 	mutex_exit(&sc->sc_mtx);
-	acpiec_unlock(dv);
+	acpiec_unlock(sc);
 	if (reg == 0)
 		goto loop; /* Spurious query result */
 	/*
 	 * Evaluate _Qxx to respond to the controller.
 	 */
 	snprintf(qxx, sizeof(qxx), "_Q%02X", (unsigned int)reg);
 	rv = AcpiEvaluateObject(sc->sc_ech, qxx, NULL, NULL);
 	if (rv != AE_OK && rv != AE_NOT_FOUND) {
-		aprint_error_dev(dv, "GPE query method %s failed: %s",
+		aprint_error_dev(sc->sc_dev, "GPE query method %s failed: %s",
 		    qxx, AcpiFormatException(rv));
+	}
 	goto loop;
+}
 static void
-acpiec_gpe_state_machine(device_t dv)
+acpiec_gpe_state_machine(struct acpiec_softc *sc)
+{
 	struct acpiec_softc *sc = device_private(dv);
 	uint8_t reg;
 	KASSERT(mutex_owned(&sc->sc_mtx));
 	reg = acpiec_read_status(sc);
-	if (reg & EC_STATUS_SCI)
+#ifdef ACPIEC_DEBUG
-		sc->sc_got_sci = true;
+	if (acpiec_debug & __BIT(ACPIEC_DEBUG_TRANSITION)) {
 		char buf[128];
 		snprintb(buf, sizeof(buf), EC_STATUS_FMT, reg);
 		DPRINTF(ACPIEC_DEBUG_TRANSITION, sc, "%s\n", buf);
+	}
 #endif
 	switch (sc->sc_state) {
 	case EC_STATE_QUERY:
 		if ((reg & EC_STATUS_IBF) != 0)
 			break; /* Nothing of interest here. */
 		acpiec_write_command(sc, EC_COMMAND_QUERY);
 		sc->sc_state = EC_STATE_QUERY_VAL;
 		break;
 	case EC_STATE_QUERY_VAL:
 		if ((reg & EC_STATUS_OBF) == 0)
 			break; /* Nothing of interest here. */
 		sc->sc_cur_val = acpiec_read_data(sc);
 		sc->sc_state = EC_STATE_FREE;
 		cv_signal(&sc->sc_cv);
 		break;
 	case EC_STATE_READ:
 		if ((reg & EC_STATUS_IBF) != 0)
 			break; /* Nothing of interest here. */
 		acpiec_write_command(sc, EC_COMMAND_READ);
 		sc->sc_state = EC_STATE_READ_ADDR;
 		break;
 	case EC_STATE_READ_ADDR:
 		if ((reg & EC_STATUS_IBF) != 0)
 			break; /* Nothing of interest here. */
 		acpiec_write_data(sc, sc->sc_cur_addr);
 		sc->sc_state = EC_STATE_READ_VAL;
 		break;
 	case EC_STATE_READ_VAL:
 		if ((reg & EC_STATUS_OBF) == 0)
 			break; /* Nothing of interest here. */
 		sc->sc_cur_val = acpiec_read_data(sc);
 		sc->sc_state = EC_STATE_FREE;
 		cv_signal(&sc->sc_cv);
 		break;
 	case EC_STATE_WRITE:
 		if ((reg & EC_STATUS_IBF) != 0)
 			break; /* Nothing of interest here. */
 		acpiec_write_command(sc, EC_COMMAND_WRITE);
 		sc->sc_state = EC_STATE_WRITE_ADDR;
 		break;
 	case EC_STATE_WRITE_ADDR:
 		if ((reg & EC_STATUS_IBF) != 0)
 			break; /* Nothing of interest here. */
 		acpiec_write_data(sc, sc->sc_cur_addr);
 		sc->sc_state = EC_STATE_WRITE_VAL;
 		break;
 	case EC_STATE_WRITE_VAL:
 		if ((reg & EC_STATUS_IBF) != 0)
 			break; /* Nothing of interest here. */
 		sc->sc_state = EC_STATE_FREE;
 		cv_signal(&sc->sc_cv);
 		acpiec_write_data(sc, sc->sc_cur_val);
 		sc->sc_state = EC_STATE_FREE;
 		break;
 	case EC_STATE_FREE:
 		if (sc->sc_got_sci)
 			cv_signal(&sc->sc_cv_sci);
 		break;
 	default:
 		panic("invalid state");
+	}
-	if (sc->sc_state != EC_STATE_FREE)
+	/*
 	 * If we are not in a transaction, wake anyone waiting to start
 	 * one.  If an SCI was requested, notify the SCI thread that it
 	 * needs to handle the SCI.
 	 */
 	if (sc->sc_state == EC_STATE_FREE) {
 		cv_signal(&sc->sc_cv);
 		if (reg & EC_STATUS_SCI) {
 			DPRINTF(ACPIEC_DEBUG_TRANSITION, sc,
 			    "wake SCI thread\n");
 			sc->sc_got_sci = true;
 			cv_signal(&sc->sc_cv_sci);
+		}
+	}
 	/*
 	 * In case GPE interrupts are broken, poll once per tick for EC
 	 * status updates while a transaction is still pending.
 	 */
 	if (sc->sc_state != EC_STATE_FREE) {
 		DPRINTF(ACPIEC_DEBUG_INTR, sc, "schedule callout\n");
 		callout_schedule(&sc->sc_pseudo_intr, 1);
+	}
 	DPRINTF(ACPIEC_DEBUG_TRANSITION, sc, "return\n");
+}
 static void
 acpiec_callout(void *arg)
+{
-	device_t dv = arg;
+	struct acpiec_softc *sc = arg;
 	struct acpiec_softc *sc = device_private(dv);
 	mutex_enter(&sc->sc_mtx);
-	acpiec_gpe_state_machine(dv);
+	DPRINTF(ACPIEC_DEBUG_INTR, sc, "callout\n");
 	acpiec_gpe_state_machine(sc);
 	mutex_exit(&sc->sc_mtx);
+}
 static uint32_t
 acpiec_gpe_handler(ACPI_HANDLE hdl, uint32_t gpebit, void *arg)
+{
-	device_t dv = arg;
+	struct acpiec_softc *sc = arg;
 	struct acpiec_softc *sc = device_private(dv);
 	mutex_enter(&sc->sc_mtx);
-	acpiec_gpe_state_machine(dv);
+	DPRINTF(ACPIEC_DEBUG_INTR, sc, "GPE\n");
 	acpiec_gpe_state_machine(sc);
 	mutex_exit(&sc->sc_mtx);
 	return ACPI_INTERRUPT_HANDLED | ACPI_REENABLE_GPE;
+}
 ACPI_STATUS
 acpiec_bus_read(device_t dv, u_int addr, ACPI_INTEGER *val, int width)
+{
-	return acpiec_space_handler(ACPI_READ, addr, width * 8, val, dv, NULL);
+	struct acpiec_softc *sc = device_private(dv);
 	return acpiec_space_handler(ACPI_READ, addr, width * 8, val, sc, NULL);
+}
 ACPI_STATUS
 acpiec_bus_write(device_t dv, u_int addr, ACPI_INTEGER val, int width)
+{
-	return acpiec_space_handler(ACPI_WRITE, addr, width * 8, &val, dv,
+	struct acpiec_softc *sc = device_private(dv);
 	return acpiec_space_handler(ACPI_WRITE, addr, width * 8, &val, sc,
 	    NULL);
+}
 ACPI_HANDLE
 acpiec_get_handle(device_t dv)
+{
 	struct acpiec_softc *sc = device_private(dv);
 	return sc->sc_ech;
+}

 @@ -1,33 +1,33 @@
-# $NetBSD: ALL,v 1.174 2022/09/24 11:05:17 riastradh Exp $
+# $NetBSD: ALL,v 1.174.4.1 2023/07/30 12:01:54 martin Exp $
 # From NetBSD: GENERIC,v 1.787 2006/10/01 18:37:54 bouyer Exp
+#
 # ALL machine description file
+#
 # This machine description includes all devices and options and it is
 # used to compile-test the source and does not necessarily produce a
 # bootable or useful kernel.
+#
 # For further information on hardware support for this architecture, see
 # the intro(4) man page.  For further information about kernel options
 # for this architecture, see the options(4) man page.  For an explanation
 # of each device driver in this file see the section 4 man page for the
 # device.
 include 	"arch/amd64/conf/std.amd64"
 options 	INCLUDE_CONFIG_FILE	# embed config file in kernel binary
-#ident		"ALL-$Revision: 1.174 $"
+#ident		"ALL-$Revision: 1.174.4.1 $"
 maxusers	64		# estimated number of users
 makeoptions	USE_SSP=yes
 # CPU-related options.
 options 	USER_LDT	# User-settable LDT, used by Wine
 options 	X86EMU		# 386 Real Mode emulator
 #options 	PAE		# PAE mode (36 bits physical addressing)
 makeoptions	SPECTRE_V2_GCC_MITIGATION=1	# GCC Spectre variant 2
 						# migitation
 options 	PCPU_IDT	# Per CPU IDT
 @@ -356,26 +356,27 @@ options 	PCI_ADDR_FIXUP		# fixup PCI I/O
 options 	ACPI_ACTIVATE_DEV	# If set, activate inactive devices
 options 	VGA_POST		# in-kernel support for VGA POST
 acpi0		at mainbus0
 # ACPI devices
 apm*		at acpi?		# ACPI apm emulation
 acpiacad*	at acpi?		# ACPI AC Adapter
 acpibat*	at acpi?		# ACPI Battery
 acpibut*	at acpi?		# ACPI Button
 acpidalb*	at acpi?		# ACPI Direct Application Launch Button
 acpiec* 	at acpi?		# ACPI Embedded Controller (late)
 acpiecdt*	at acpi?		# ACPI Embedded Controller (early)
 options 	ACPIEC_DEBUG=-1
 acpifan*	at acpi?		# ACPI Fan
 acpilid*	at acpi?		# ACPI Lid Switch
 acpipmtr*	at acpi?		# ACPI Power Meter (experimental)
 # XXX Do not enable this in a real kernel unless you also disable any
 # XXX "native" i2c controller.  Otherwise you'll have two accessors to
 # XXX the same bus, and bad things (tm) will happen!
 acpismbus*	at acpi?		# ACPI SMBus CMI (experimental)
 acpitz* 	at acpi?		# ACPI Thermal Zone
 acpivga*	at acpi?		# ACPI Display Adapter
 acpiout*	at acpivga?		# ACPI Display Output Device
 acpiwdrt*	at acpi?		# ACPI Watchdog Resource Table
 acpiwmi*	at acpi?		# ACPI WMI Mapper

 @@ -1,33 +1,33 @@
-# $NetBSD: ALL,v 1.503 2022/09/24 11:05:17 riastradh Exp $
+# $NetBSD: ALL,v 1.503.4.1 2023/07/30 12:01:54 martin Exp $
 # From NetBSD: GENERIC,v 1.787 2006/10/01 18:37:54 bouyer Exp
+#
 # ALL machine description file
+#
 # This machine description includes all devices and options and it is
 # used to compile-test the source and does not necessarily produce a
 # bootable or useful kernel.
+#
 # For further information on hardware support for this architecture, see
 # the intro(4) man page.  For further information about kernel options
 # for this architecture, see the options(4) man page.  For an explanation
 # of each device driver in this file see the section 4 man page for the
 # device.
 include 	"arch/i386/conf/std.i386"
 options 	INCLUDE_CONFIG_FILE	# embed config file in kernel binary
-#ident		"ALL-$Revision: 1.503 $"
+#ident		"ALL-$Revision: 1.503.4.1 $"
 maxusers	64		# estimated number of users
 makeoptions	USE_SSP=yes
 # CPU-related options.
 options 	USER_LDT	# user-settable LDT; used by WINE
 options 	X86EMU		# 386 Real Mode emulator
 options 	PAE		# PAE mode (36 bits physical addressing)
 makeoptions	SPECTRE_V2_GCC_MITIGATION=1	# GCC Spectre variant 2
 						# migitation
 options 	PCPU_IDT	# Per CPU IDT
 @@ -343,26 +343,27 @@ options 	ACPI_SCANPCI	# find PCI roots u
 acpi0		at mainbus0
 options 	ACPI_ACTIVATE_DEV	# If set, activate inactive devices
 # ACPI devices
 apm*		at acpi?		# ACPI apm emulation
 acpiacad*	at acpi?		# ACPI AC Adapter
 acpibat*	at acpi?		# ACPI Battery
 acpibut*	at acpi?		# ACPI Button
 acpidalb*	at acpi?		# ACPI Direct Application Launch Button
 acpiec* 	at acpi?		# ACPI Embedded Controller (late)
 acpiecdt*	at acpi?		# ACPI Embedded Controller (early)
 options 	ACPIEC_DEBUG=-1
 acpifan*	at acpi?		# ACPI Fan
 acpilid*	at acpi?		# ACPI Lid Switch
 acpipmtr*	at acpi?		# ACPI Power Meter (experimental)
 # XXX Do not enable this in a real kernel unless you also disable any
 # XXX "native" i2c controller.  Otherwise you'll have two accessors to
 # XXX the same bus, and bad things (tm) will happen!
 acpismbus*	at acpi?		# ACPI SMBus CMI (experimental)
 acpitz* 	at acpi?		# ACPI Thermal Zone
 acpivga*	at acpi?		# ACPI Display Adapter
 acpiout*	at acpivga?		# ACPI Display Output Device
 acpiwdrt*	at acpi?		# ACPI Watchdog Resource Table
 acpiwmi*	at acpi?		# ACPI WMI Mapper

 @@ -1,14 +1,14 @@
-#	$NetBSD: files.acpi,v 1.126 2022/05/26 02:16:44 mrg Exp $
+#	$NetBSD: files.acpi,v 1.126.4.1 2023/07/30 12:01:53 martin Exp $
 defflag	opt_acpi.h	ACPIVERBOSE ACPI_DEBUG ACPI_ACTIVATE_DEV
 			ACPI_DSDT_OVERRIDE ACPI_SCANPCI ACPI_BREAKPOINT
 			ACPI_REDUCED_HW ACPI__DIS_IS_BROKEN
 defparam opt_acpi.h	ACPI_DSDT_FILE := "\"/dev/null\""
 defparam opt_acpi.h	ACPI_BLACKLIST_YEAR = 2000
 define	acpiapmbus { }
 define	acpinodebus { }
 define	acpiecdtbus { }
 define	acpihpetbus { }
 define	acpiwdrtbus { }
 define	acpisdtbus { }
 @@ -35,26 +35,27 @@ file	dev/acpi/acpi_usb.c		acpi
 file	dev/acpi/acpi_util.c		acpi
 file	dev/acpi/acpi_wakedev.c		acpi
 file	dev/acpi/acpi_verbose.c		acpi & acpiverbose
 # ACPI/apm emulation.
 attach  apm at acpiapmbus with acpiapm: sysmon_envsys
 file	dev/acpi/acpi_apm.c		acpiapm
 # ACPI Embedded Controller
 device	acpiec
 attach	acpiec at acpinodebus
 device	acpiecdt
 attach	acpiecdt at acpiecdtbus
 defparam opt_acpi_ec.h			ACPIEC_DEBUG
 file	dev/acpi/acpi_ec.c		acpiec|acpiecdt
 # ACPI Lid Switch
 device	acpilid: sysmon_power
 attach	acpilid at acpinodebus
 file	dev/acpi/acpi_lid.c		acpilid
 # ACPI Button
 device	acpibut: sysmon_power
 attach	acpibut at acpinodebus
 file	dev/acpi/acpi_button.c		acpibut
 # ACPI AC Adapter