Download raw body.
chromebook keyboards
> Date: Tue, 8 Oct 2024 18:41:36 +0000
> From: Miod Vallat <miod@online.fr>
>
> [tl;dr: if your keyboard does not work, try this diff.]
Actually, if your keyboard does work, test this diff!
> There have been several reports, in the past, that some laptops had
> their keyboard not functioning correctly, often with repeated
> keystrokes.
>
> The #1 cause of such problems is legacy-free hardware which, for some
> reason, emulates an ISA keyboard controller but uses modern
> level-triggered interrupts, instead of traditional ISA edge-triggered
> interrupts.
>
> Our reaction so far had been to wear our grey beards and suspensers and
> tell you "here's a nickel kid, get yourself a better computer". (If you
> don't have the reference, use your favorite search engine and search for
> '"here's a nickel kid" dilbert' in the images section)
>
> Unfortunately, Chromebook designers (and maybe other laptops designers)
> disagree with this attitude and keep producing devices using
> level-triggered interrupts.
>
> The following diff introduces a pckbc@acpi attachment which will prevail
> upon the traditional pckbc@isa attachment, and will use the proper
> interrupt polarity settings, obtained from the ACPI mess^Winformation.
Note that I'm pretty sure I've seen where ACPI specified the interrupt
incorrectly (don't exactly remember whether it was the wrong polarity
or that it wrongly claimed to be level-triggered). But I suspect your
diff avoids that issue because you don't actually attach to PNP0303?
> Tested to work on bmercer@'s and robert@'s machines, where the keyboard
> did not work previously, and on a Lenovo laptop where pckbc@isa did work
> due to correct polarity, but is now recognized similarly to robert@'s
> machine and has pckbc@acpi attach now, with no regression in keyboard
> behaviour.
>
> If you have a laptop with non-functioning keyboard and this diff does
> not help, disassemble its DSTD (follow instructions in acpidump(8)) and
> check which HID name is used for the PNP0303 device; if it's neither
> "GOOG000A" nor "MSFT0001", try adding it to sys/dev/acpi/pckbc_acpi.c
> and report whether it helps (and also the PS2K part of your DSDT
> disassembly).
>
> Miod
>
> Index: share/man/man4/acpi.4
> ===================================================================
> RCS file: /OpenBSD/src/share/man/man4/acpi.4,v
> retrieving revision 1.75
> diff -u -p -r1.75 acpi.4
> --- share/man/man4/acpi.4 4 Aug 2024 16:31:02 -0000 1.75
> +++ share/man/man4/acpi.4 7 Oct 2024 19:31:17 -0000
> @@ -130,6 +130,8 @@ Intel OnChip System Fabric device
> Intelligent Platform Management Interface driver
> .It Xr pchgpio 4
> Intel PCH GPIO controller
> +.It Xr pckbc 4
> +Keyboard controller
> .It Xr pluart 4
> ARM PrimeCell PL011 UART
> .It Xr qcgpio 4
> Index: share/man/man4/pckbc.4
> ===================================================================
> RCS file: /OpenBSD/src/share/man/man4/pckbc.4,v
> retrieving revision 1.18
> diff -u -p -r1.18 pckbc.4
> --- share/man/man4/pckbc.4 26 Sep 2010 20:39:08 -0000 1.18
> +++ share/man/man4/pckbc.4 7 Oct 2024 19:31:17 -0000
> @@ -33,6 +33,7 @@
> .Nd PC (ISA) keyboard controller driver
> .Sh SYNOPSIS
> .Cd "pckbc* at isa? flags 0x00 " Pq "alpha, amd64, i386, loongson"
> +.Cd "pckbc* at acpi? " Pq "amd64"
> .Cd "pckbc* at ebus? " Pq "sparc64"
> .Cd "pckbd* at pckbc?"
> .Cd "pms* at pckbc?"
> @@ -40,19 +41,13 @@
> The
> .Nm
> driver handles resource allocation and device attachment for the
> -traditional PC/AT keyboard controller.
> -It provides two logical connections for child devices, the
> +traditional PC/AT keyboard controller, or emulations thereof.
> +It provides up to two logical connections for child devices, the
> .Dq keyboard
> slot for a keyboard and the
> .Dq auxiliary
> -slot for mice (the latter might be missing in older keyboard controllers).
> -.\" .Pp
> -.\" The optional
> -.\" .Dq slot
> -.\" locator argument can be used to force unusual connections of devices to
> -.\" logical slots.
> -.\" This feature is for experimentation only, it will not be
> -.\" useful in normal operation.
> +slot for mice (the latter might be missing in older keyboard controllers,
> +or recent emulations).
> .Pp
> To avoid attaching a phantom PS/2 keyboard device, the
> .Nm
> @@ -63,6 +58,7 @@ PS/2 keyboard.
> The keyboard can be forced to attach on these systems, by changing the
> device flags to 1.
> .Sh SEE ALSO
> +.Xr acpi 4 ,
> .Xr ebus 4 ,
> .Xr intro 4 ,
> .Xr isa 4 ,
> Index: sys/arch/amd64/conf/GENERIC
> ===================================================================
> RCS file: /OpenBSD/src/sys/arch/amd64/conf/GENERIC,v
> retrieving revision 1.526
> diff -u -p -r1.526 GENERIC
> --- sys/arch/amd64/conf/GENERIC 4 Sep 2024 07:45:08 -0000 1.526
> +++ sys/arch/amd64/conf/GENERIC 7 Oct 2024 19:31:17 -0000
> @@ -86,6 +86,7 @@ ipmi0 at acpi? disable
> ccpmic* at iic?
> tipmic* at iic?
> intelpmc* at acpi?
> +pckbc* at acpi?
>
> efi0 at bios0
> mpbios0 at bios0
> Index: sys/arch/amd64/conf/RAMDISK
> ===================================================================
> RCS file: /OpenBSD/src/sys/arch/amd64/conf/RAMDISK,v
> retrieving revision 1.87
> diff -u -p -r1.87 RAMDISK
> --- sys/arch/amd64/conf/RAMDISK 12 Aug 2024 18:43:41 -0000 1.87
> +++ sys/arch/amd64/conf/RAMDISK 7 Oct 2024 19:31:17 -0000
> @@ -37,6 +37,7 @@ acpimadt0 at acpi?
> com0 at acpi? addr 0x3f8
> com1 at acpi? addr 0x2f8
> com* at acpi?
> +pckbc* at acpi?
>
> mpbios0 at bios0
>
> Index: sys/arch/amd64/conf/RAMDISK_CD
> ===================================================================
> RCS file: /OpenBSD/src/sys/arch/amd64/conf/RAMDISK_CD,v
> retrieving revision 1.207
> diff -u -p -r1.207 RAMDISK_CD
> --- sys/arch/amd64/conf/RAMDISK_CD 14 Aug 2024 14:40:45 -0000 1.207
> +++ sys/arch/amd64/conf/RAMDISK_CD 7 Oct 2024 19:31:17 -0000
> @@ -56,6 +56,7 @@ com1 at acpi? addr 0x2f8
> com2 at acpi? addr 0x3e8
> com* at acpi?
> glkgpio* at acpi?
> +pckbc* at acpi?
>
> mpbios0 at bios0
>
> Index: sys/dev/acpi/acpi.c
> ===================================================================
> RCS file: /OpenBSD/src/sys/dev/acpi/acpi.c,v
> retrieving revision 1.439
> diff -u -p -r1.439 acpi.c
> --- sys/dev/acpi/acpi.c 4 Sep 2024 21:39:18 -0000 1.439
> +++ sys/dev/acpi/acpi.c 7 Oct 2024 19:31:17 -0000
> @@ -3036,7 +3036,6 @@ const char *acpi_skip_hids[] = {
>
> /* ISA devices for which we attach a driver later */
> const char *acpi_isa_hids[] = {
> - "PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
> "PNP0400", /* Standard LPT Parallel Port */
> "PNP0401", /* ECP Parallel Port */
> "PNP0700", /* PC-class Floppy Disk Controller */
> Index: sys/dev/acpi/files.acpi
> ===================================================================
> RCS file: /OpenBSD/src/sys/dev/acpi/files.acpi,v
> retrieving revision 1.71
> diff -u -p -r1.71 files.acpi
> --- sys/dev/acpi/files.acpi 4 Aug 2024 11:05:18 -0000 1.71
> +++ sys/dev/acpi/files.acpi 7 Oct 2024 19:31:17 -0000
> @@ -289,3 +289,7 @@ file dev/acpi/iosf_acpi.c iosf_acpi
> device intelpmc
> attach intelpmc at acpi
> file dev/acpi/intelpmc.c intelpmc
> +
> +# PS/2 Keyboard Controller
> +attach pckbc at acpi with pckbc_acpi
> +file dev/acpi/pckbc_acpi.c pckbc_acpi
> Index: sys/dev/acpi/pckbc_acpi.c
> ===================================================================
> RCS file: sys/dev/acpi/pckbc_acpi.c
> diff -N sys/dev/acpi/pckbc_acpi.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ sys/dev/acpi/pckbc_acpi.c 7 Oct 2024 19:31:17 -0000
> @@ -0,0 +1,189 @@
> +/* $OpenBSD$ */
> +/*
> + * Copyright (c) 2024, Miodrag Vallat.
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +/*
> + * Copyright (c) 1998
> + * Matthias Drochner. 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 the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, 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.
> + */
> +
> +#include <sys/param.h>
> +#include <sys/systm.h>
> +#include <sys/device.h>
> +#include <sys/malloc.h>
> +
> +#include <machine/bus.h>
> +
> +#include <dev/acpi/acpivar.h>
> +
> +#include <dev/ic/i8042reg.h>
> +#include <dev/ic/pckbcvar.h>
> +
> +int pckbc_acpi_match(struct device *, void *, void *);
> +void pckbc_acpi_attach(struct device *, struct device *, void *);
> +int pckbc_acpi_activate(struct device *, int);
> +
> +struct pckbc_acpi_softc {
> + struct pckbc_softc sc;
> + void *sc_ih;
> +};
> +
> +const struct cfattach pckbc_acpi_ca = {
> + .ca_devsize = sizeof(struct pckbc_acpi_softc),
> + .ca_match = pckbc_acpi_match,
> + .ca_attach = pckbc_acpi_attach,
> + .ca_activate = pckbc_acpi_activate
> +};
> +
> +const char *pckbc_hids[] = {
> + "GOOG000A", /* Chromebook built-in keyboard */
> + "MSFT0001", /* Built-in keyboard found on various systems */
> + NULL
> +};
> +
> +int
> +pckbc_acpi_match(struct device *parent, void *match, void *aux)
> +{
> + struct acpi_attach_args *aaa = aux;
> + struct cfdata *cf = match;
> +
> + if (aaa->aaa_naddr < 2)
> + return 0;
> + return acpi_matchhids(aaa, pckbc_hids, cf->cf_driver->cd_name);
> +}
> +
> +int
> +pckbc_acpi_activate(struct device *self, int act)
> +{
> + struct pckbc_acpi_softc *pasc = (struct pckbc_acpi_softc *)self;
> + struct pckbc_softc *sc = &pasc->sc;
> + int rv = 0;
> +
> + switch (act) {
> + case DVACT_SUSPEND:
> + rv = config_activate_children(self, act);
> + pckbc_stop(sc);
> + break;
> + case DVACT_RESUME:
> + pckbc_reset(sc);
> + rv = config_activate_children(self, act);
> + break;
> + default:
> + rv = config_activate_children(self, act);
> + break;
> + }
> + return rv;
> +}
> +
> +void
> +pckbc_acpi_attach(struct device *parent, struct device *self, void *aux)
> +{
> + struct pckbc_acpi_softc *pasc = (struct pckbc_acpi_softc *)self;
> + struct pckbc_softc *sc = &pasc->sc;
> + struct acpi_attach_args *aaa = aux;
> + struct pckbc_internal *t;
> + bus_space_handle_t ioh_d, ioh_c;
> + int rv;
> +
> + if (aaa->aaa_nirq < 1) {
> + printf(": no interrupt\n");
> + return;
> + }
> +
> + printf(" addr 0x%llx/0x%llx 0x%llx/0x%llx", aaa->aaa_addr[0],
> + aaa->aaa_size[0], aaa->aaa_addr[1], aaa->aaa_size[1]);
> + printf(" irq %d", aaa->aaa_irq[0]);
> + printf(": \"%s\"", aaa->aaa_dev);
> +
> + pasc->sc_ih = acpi_intr_establish(aaa->aaa_irq[0],
> + aaa->aaa_irq_flags[0], IPL_TTY, pckbcintr, sc, sc->sc_dv.dv_xname);
> + if (pasc->sc_ih == NULL) {
> + printf(": can't establish interrupt\n");
> + return;
> + }
> +
> + if (pckbc_is_console(aaa->aaa_bst[0], aaa->aaa_addr[0])) {
> + t = &pckbc_consdata;
> + pckbc_console_attached = 1;
> + /* t->t_cmdbyte was initialized by cnattach */
> + } else {
> + if ((rv = bus_space_map(aaa->aaa_bst[0], aaa->aaa_addr[0], 1, 0,
> + &ioh_d)) != 0) {
> + printf(": couldn't map data port (%d)\n", rv);
> + goto fail_mapd;
> + }
> + if ((rv = bus_space_map(aaa->aaa_bst[1], aaa->aaa_addr[1], 1, 0,
> + &ioh_c)) != 0) {
> + printf(": couldn't map command port (%d)\n", rv);
> + goto fail_mapc;
> + }
> +
> + t = malloc(sizeof(*t), M_DEVBUF, M_WAITOK | M_ZERO);
> + /*
> + * pckbc should theoretically be updated to use separate
> + * bus_space_tag_t for the data and command ports, since on
> + * this particular attachment they appear as separate I/O
> + * resources. But since these are I/O resources, all
> + * aaa_bst[] are identical, so we can avoid this change
> + * for the time being as long as the logic in
> + * acpi_parse_resources() does not change.
> + */
> + t->t_iot = aaa->aaa_bst[0];
> + t->t_ioh_d = ioh_d;
> + t->t_ioh_c = ioh_c;
> + t->t_cmdbyte = KC8_CPU; /* Enable ports */
> + }
> +
> + t->t_sc = sc;
> + sc->id = t;
> +
> + printf("\n");
> +
> + /*
> + * Make sure pckbc@isa will not try to attach.
> + */
> + {
> + extern int pckbc_acpi;
> + pckbc_acpi = 1;
> + }
We could avoid this by checking pcikbc_console_attached in the
pckbc@isa match function.
> +
> + pckbc_attach(sc, 0);
> + return;
> +
> + fail_mapc:
> + bus_space_unmap(aaa->aaa_bst[0], ioh_d, 1);
> + fail_mapd:
> + acpi_intr_disestablish(pasc->sc_ih);
> +}
> Index: sys/dev/ic/pckbc.c
> ===================================================================
> RCS file: /OpenBSD/src/sys/dev/ic/pckbc.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 pckbc.c
> --- sys/dev/ic/pckbc.c 26 Aug 2023 15:01:00 -0000 1.55
> +++ sys/dev/ic/pckbc.c 7 Oct 2024 19:31:17 -0000
> @@ -89,6 +89,10 @@ int pckbc_console_attached;
> int pckbc_console;
> static struct pckbc_slotdata pckbc_cons_slotdata;
>
> +#ifdef __HAVE_ACPI
> +int pckbc_acpi;
> +#endif
> +
> static int pckbc_wait_output(bus_space_tag_t, bus_space_handle_t);
>
> static int pckbc_get8042cmd(struct pckbc_internal *);
> Index: sys/dev/isa/pckbc_isa.c
> ===================================================================
> RCS file: /OpenBSD/src/sys/dev/isa/pckbc_isa.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 pckbc_isa.c
> --- sys/dev/isa/pckbc_isa.c 18 Aug 2015 06:54:00 -0000 1.19
> +++ sys/dev/isa/pckbc_isa.c 7 Oct 2024 19:31:17 -0000
> @@ -56,6 +56,15 @@ pckbc_isa_match(struct device *parent, v
> bus_space_handle_t ioh_d, ioh_c;
> int res;
>
> +#ifdef __HAVE_ACPI
> + /* Don't try anything if pckbc@acpi has claimed the keyboard. */
> + {
> + extern int pckbc_acpi;
> + if (pckbc_acpi)
> + return 0;
> + }
> +#endif
> +
> /* If values are hardwired to something that they can't be, punt. */
> if ((ia->ia_iobase != IOBASEUNK && ia->ia_iobase != IO_KBD) ||
> ia->ia_maddr != MADDRUNK ||
>
>
chromebook keyboards