On Tue, Oct 18, 2016 at 09:01:59PM -0700, Ilya Kaliman wrote: > Thanks! Now everything seems to work. Minor tweak - maybe need extra > newline after "acpiec0 at acpi0"
New diff that fixes the printfs, removes dead code (suggested by guenther@) and takes care of some small style(9) nits. OK? Index: acpidev.h =================================================================== RCS file: /cvs/src/sys/dev/acpi/acpidev.h,v retrieving revision 1.38 diff -u -p -u -p -r1.38 acpidev.h --- acpidev.h 12 Aug 2015 05:59:54 -0000 1.38 +++ acpidev.h 21 Oct 2016 07:35:22 -0000 @@ -323,10 +323,12 @@ struct acpiec_softc { int sc_ecbusy; /* command/status register */ + bus_size_t sc_ec_sc; bus_space_tag_t sc_cmd_bt; bus_space_handle_t sc_cmd_bh; /* data register */ + bus_size_t sc_ec_data; bus_space_tag_t sc_data_bt; bus_space_handle_t sc_data_bh; Index: acpiec.c =================================================================== RCS file: /cvs/src/sys/dev/acpi/acpiec.c,v retrieving revision 1.54 diff -u -p -u -p -r1.54 acpiec.c --- acpiec.c 23 Aug 2016 18:26:21 -0000 1.54 +++ acpiec.c 21 Oct 2016 07:35:22 -0000 @@ -48,7 +48,7 @@ void acpiec_write(struct acpiec_softc * int acpiec_getcrs(struct acpiec_softc *, struct acpi_attach_args *); -int acpiec_getregister(const u_int8_t *, int, int *, bus_size_t *); +int acpiec_parse_resources(union acpi_resource *, void *); void acpiec_wait(struct acpiec_softc *, u_int8_t, u_int8_t); void acpiec_sci_event(struct acpiec_softc *); @@ -285,15 +285,16 @@ acpiec_attach(struct device *parent, str return; } + printf("\n"); if (acpiec_getcrs(sc, aa)) { - printf(": Failed to read resource settings\n"); + printf("%s: Failed to read resource settings\n", DEVNAME(sc)); return; } sc->sc_acpi->sc_ec = sc; if (acpiec_reg(sc)) { - printf(": Failed to register address space\n"); + printf("%s: Failed to register address space\n", DEVNAME(sc)); return; } @@ -305,15 +306,13 @@ acpiec_attach(struct device *parent, str acpi_set_gpehandler(sc->sc_acpi, sc->sc_gpe, acpiec_gpehandler, sc, 1); #endif - + if (aml_evalname(sc->sc_acpi, sc->sc_devnode, "_GLK", 0, NULL, &res)) sc->sc_glk = 0; else if (res.type != AML_OBJTYPE_INTEGER) sc->sc_glk = 0; else sc->sc_glk = res.v_integer ? 1 : 0; - - printf("\n"); } void @@ -366,68 +365,75 @@ acpiec_gpehandler(struct acpi_softc *acp return (0); } -/* parse the resource buffer to get a 'register' value */ int -acpiec_getregister(const u_int8_t *buf, int size, int *type, bus_size_t *addr) +acpiec_parse_resources(union acpi_resource *crs, void *arg) { - int len, hlen; + struct acpiec_softc *sc = arg; + int type = AML_CRSTYPE(crs); -#define RES_TYPE_MASK 0x80 -#define RES_LENGTH_MASK 0x07 -#define RES_TYPE_IOPORT 0x47 -#define RES_TYPE_ENDTAG 0x79 + static int argno = 0; - if (size <= 0) - return (0); - - if (*buf & RES_TYPE_MASK) { - /* large resource */ - if (size < 3) - return (1); - len = (int)buf[1] + 256 * (int)buf[2]; - hlen = 3; - } else { - /* small resource */ - len = buf[0] & RES_LENGTH_MASK; - hlen = 1; + switch (argno) { + case 0: + if (type != SR_IOPORT) { + printf("%s: Unexpected resource #%d type %d\n", + DEVNAME(sc), argno, type); + break; + } + sc->sc_data_bt = sc->sc_acpi->sc_iot; + sc->sc_ec_data = crs->sr_ioport._max; + break; + case 1: + if (type != SR_IOPORT) { + printf("%s: Unexpected resource #%d type %d\n", + DEVNAME(sc), argno, type); + break; + } + sc->sc_cmd_bt = sc->sc_acpi->sc_iot; + sc->sc_ec_sc = crs->sr_ioport._max; + break; + case 2: + if (!sc->sc_acpi->sc_hw_reduced) { + printf("%s: Not running on HW-Reduced ACPI type %d\n", + DEVNAME(sc), type); + break; + } + /* XXX: handle SCI GPIO */ + break; + default: + printf("%s: invalid resource #%d type %d\n", + DEVNAME(sc), argno, type); } - /* XXX todo: decode other types */ - if (*buf != RES_TYPE_IOPORT) - return (0); - - if (size < hlen + len) - return (0); - - /* XXX validate? */ - *type = GAS_SYSTEM_IOSPACE; - *addr = (int)buf[2] + 256 * (int)buf[3]; - - return (hlen + len); + argno++; + return 0; } int acpiec_getcrs(struct acpiec_softc *sc, struct acpi_attach_args *aa) { struct aml_value res; - bus_size_t ec_sc, ec_data; - int dtype, ctype; - char *buf; - int size, ret; int64_t gpe; struct acpi_ecdt *ecdt = aa->aaa_table; extern struct aml_node aml_root; + int rc; /* Check if this is ECDT initialization */ if (ecdt) { /* Get GPE, Data and Control segments */ sc->sc_gpe = ecdt->gpe_bit; - ctype = ecdt->ec_control.address_space_id; - ec_sc = ecdt->ec_control.address; + if (ecdt->ec_control.address_space_id == GAS_SYSTEM_IOSPACE) + sc->sc_cmd_bt = sc->sc_acpi->sc_iot; + else + sc->sc_cmd_bt = sc->sc_acpi->sc_memt; + sc->sc_ec_sc = ecdt->ec_control.address; - dtype = ecdt->ec_data.address_space_id; - ec_data = ecdt->ec_data.address; + if (ecdt->ec_data.address_space_id == GAS_SYSTEM_IOSPACE) + sc->sc_data_bt = sc->sc_acpi->sc_iot; + else + sc->sc_data_bt = sc->sc_acpi->sc_memt; + sc->sc_ec_data = ecdt->ec_data.address; /* Get devnode from header */ sc->sc_devnode = aml_searchname(&aml_root, ecdt->ec_id); @@ -435,7 +441,9 @@ acpiec_getcrs(struct acpiec_softc *sc, s goto ecdtdone; } - if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "_GPE", 0, NULL, &gpe)) { + rc = aml_evalinteger(sc->sc_acpi, sc->sc_devnode, + "_GPE", 0, NULL, &gpe); + if (rc) { dnprintf(10, "%s: no _GPE\n", DEVNAME(sc)); return (1); } @@ -456,60 +464,26 @@ acpiec_getcrs(struct acpiec_softc *sc, s return (1); } - size = res.length; - buf = res.v_buffer; - - ret = acpiec_getregister(buf, size, &dtype, &ec_data); - if (ret <= 0) { - dnprintf(10, "%s: failed to read DATA from _CRS\n", - DEVNAME(sc)); - aml_freevalue(&res); - return (1); - } - - buf += ret; - size -= ret; - - ret = acpiec_getregister(buf, size, &ctype, &ec_sc); - if (ret <= 0) { - dnprintf(10, "%s: failed to read S/C from _CRS\n", - DEVNAME(sc)); - aml_freevalue(&res); - return (1); - } - - buf += ret; - size -= ret; - - if (size != 2 || *buf != RES_TYPE_ENDTAG) { - dnprintf(10, "%s: no _CRS end tag\n", DEVNAME(sc)); - aml_freevalue(&res); + aml_parse_resource(&res, acpiec_parse_resources, sc); + aml_freevalue(&res); + if (sc->sc_ec_data == 0 || sc->sc_ec_sc == 0) { + printf("%s: failed to read from _CRS\n", DEVNAME(sc)); return (1); } - aml_freevalue(&res); - /* XXX: todo - validate _CRS checksum? */ ecdtdone: dnprintf(10, "%s: Data: 0x%lx, S/C: 0x%lx\n", - DEVNAME(sc), ec_data, ec_sc); + DEVNAME(sc), sc->sc_ec_data, sc->sc_ec_sc); - if (ctype == GAS_SYSTEM_IOSPACE) - sc->sc_cmd_bt = aa->aaa_iot; - else - sc->sc_cmd_bt = aa->aaa_memt; - - if (bus_space_map(sc->sc_cmd_bt, ec_sc, 1, 0, &sc->sc_cmd_bh)) { + if (bus_space_map(sc->sc_cmd_bt, sc->sc_ec_sc, 1, 0, &sc->sc_cmd_bh)) { dnprintf(10, "%s: failed to map S/C reg.\n", DEVNAME(sc)); return (1); } - if (dtype == GAS_SYSTEM_IOSPACE) - sc->sc_data_bt = aa->aaa_iot; - else - sc->sc_data_bt = aa->aaa_memt; - - if (bus_space_map(sc->sc_data_bt, ec_data, 1, 0, &sc->sc_data_bh)) { + rc = bus_space_map(sc->sc_data_bt, sc->sc_ec_data, 1, 0, + &sc->sc_data_bh); + if (rc) { dnprintf(10, "%s: failed to map DATA reg.\n", DEVNAME(sc)); bus_space_unmap(sc->sc_cmd_bt, sc->sc_cmd_bh, 1); return (1);