Thanks! Now everything seems to work. Minor tweak - maybe need extra
newline after "acpiec0 at acpi0"
acpiprt9 at acpi0: bus -1 (RP08)
acpiprt10 at acpi0: bus -1 (PEG0)
acpiprt11 at acpi0: bus -1 (PEG1)
acpiprt12 at acpi0: bus -1 (PEG2)
acpiec0 at acpi0acpiec0: Not running on HW-Reduced ACPI type 8
acpiec0: invalid resource #3 type 134
acpicpu0 at acpi0: C3(200@506 mwait.1@0x60), C2(200@148 mwait.1@0x33), C1(1000@1
mwait.1), PSS
acpicpu1 at acpi0: C3(200@506 mwait.1@0x60), C2(200@148 mwait.1@0x33), C1(1000@1
mwait.1), PSS
On Tue, Oct 18, 2016 at 11:31 AM, Paul Irofti <p...@irofti.net> wrote:
> On Tue, Oct 18, 2016 at 10:22:06AM -0700, Ilya Kaliman wrote:
>> Hi!
>>
>> The patch seems to work partially - acpiec now gets initialized. The
>> suspend-on-lid-close does not work anymore, though (it works with my
>> initial patch).
>
> Right, that's because I had an off-by-one in the switch case. I wonder
> how my laptop managed to function with that bug...
>
> Please try again with this diff.
>
>
> 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 18 Oct 2016 18:30:24 -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 18 Oct 2016 18:30:25 -0000
> @@ -407,13 +407,53 @@ acpiec_getregister(const u_int8_t *buf,
> }
>
> int
> +acpiec_parse_resources(union acpi_resource *crs, void *arg)
> +{
> + struct acpiec_softc *sc = arg;
> + int type = AML_CRSTYPE(crs);
> +
> + static int argno = 0;
> +
> + 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);
> + }
> +
> + 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;
> @@ -423,11 +463,17 @@ acpiec_getcrs(struct acpiec_softc *sc, s
> /* 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;
> -
> - dtype = ecdt->ec_data.address_space_id;
> - ec_data = ecdt->ec_data.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;
> +
> + 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);
> @@ -456,60 +502,24 @@ 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)) {
> + if (bus_space_map(sc->sc_data_bt, sc->sc_ec_data, 1, 0,
> &sc->sc_data_bh)) {
> 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);
