Download raw body.
aplhidev: enable on amd64, add keyboard backlight support
On Thu, Nov 13, 2025 at 08:15:16PM -0600, joshua stein wrote:
> This moves aplhidev out of arch/arm64 and into dev/spi, adds
> attachment via ACPI through ispi, enables it on amd64, and adds
> support for adjusting keyboard backlight through 'wsconsctl
> keyboard.backlight'.
>
> Tested on a 2017 MacBook 12". Hopefully it didn't break anything on
> Apple M* arm64 laptops and I'm curious if the keyboard backlight
> support also works there.
I haven't tested your diff, but at least on Apple M1 there is are
seperate pwm(4)/pwmled(4) drivers that handle the keyboard backlight
which this may conflict with.
./sys/dev/fdt/pwmleds.c
>
> Individual commits are here but this is just one consolidated diff:
>
> https://github.com/jcs/openbsd-src/commit/5cc8811f104532139877139b6852ae627963a602
> https://github.com/jcs/openbsd-src/commit/3a57a3b8440efc45d4e75a36e843328f8e5ff015
> https://github.com/jcs/openbsd-src/commit/852de258b6625c33641bdf36303699ba157f3d55
>
> I also tested it in RAMDISK_CD so the keyboard works during
> installation but that's not included here.
>
>
> diff --git sys/arch/amd64/conf/GENERIC sys/arch/amd64/conf/GENERIC
> index 37f1ac6d7c0..4046a147d47 100644
> --- sys/arch/amd64/conf/GENERIC
> +++ sys/arch/amd64/conf/GENERIC
> @@ -208,6 +208,11 @@ gpio* at skgpio?
>
> ispi* at acpi? # Intel LPSS SPI controller
> ispi* at pci?
> +aplhidev* at spi? # Apple SPI HID
> +aplkbd* at aplhidev?
> +wskbd* at aplkbd? mux 1
> +aplms* at aplhidev?
> +wsmouse* at aplms? mux 0
>
> #option PCMCIAVERBOSE
>
> diff --git sys/arch/amd64/conf/files.amd64 sys/arch/amd64/conf/files.amd64
> index 0c1e224f5c2..25476378a97 100644
> --- sys/arch/amd64/conf/files.amd64
> +++ sys/arch/amd64/conf/files.amd64
> @@ -280,6 +280,17 @@ include "dev/onewire/files.onewire"
> #
> attach ipmi at mainbus
>
> +#
> +# SPI
> +#
> +device aplhidev {}
> +attach aplhidev at spi
> +device aplkbd: hid, hidkbd, wskbddev
> +attach aplkbd at aplhidev
> +device aplms: hid, hidms, wsmousedev
> +attach aplms at aplhidev
> +file dev/spi/aplhidev.c aplhidev | aplkbd | aplms needs-flag
> +
> #
> # device major numbers
> #
> diff --git sys/arch/arm64/conf/files.arm64 sys/arch/arm64/conf/files.arm64
> index 34d9f5d35f5..5cf73280050 100644
> --- sys/arch/arm64/conf/files.arm64
> +++ sys/arch/arm64/conf/files.arm64
> @@ -186,7 +186,7 @@ device aplkbd: hid, hidkbd, wskbddev
> attach aplkbd at aplhidev
> device aplms: hid, hidms, wsmousedev
> attach aplms at aplhidev
> -file arch/arm64/dev/aplhidev.c aplhidev | aplkbd | aplms needs-flag
> +file dev/spi/aplhidev.c aplhidev | aplkbd | aplms needs-flag
>
> device aplmbox
> attach aplmbox at fdt
> diff --git sys/dev/acpi/ispi_acpi.c sys/dev/acpi/ispi_acpi.c
> index 6a226a30482..8e0b820be58 100644
> --- sys/dev/acpi/ispi_acpi.c
> +++ sys/dev/acpi/ispi_acpi.c
> @@ -92,6 +92,7 @@ int
> ispi_acpi_found_hid(struct aml_node *node, void *arg)
> {
> struct ispi_softc *sc = (struct ispi_softc *)arg;
> + struct spi_attach_args sa;
> int64_t sta;
> char cdev[32], dev[32];
>
> @@ -110,7 +111,17 @@ ispi_acpi_found_hid(struct aml_node *node, void *arg)
>
> acpi_attach_deps(acpi_softc, node->parent);
>
> - /* TODO */
> + if (strcmp(cdev, "apple-spi-topcase") == 0) {
> + memset(&sa, 0, sizeof(sa));
> + sa.sa_tag = &sc->sc_spi_tag;
> + sa.sa_name = cdev;
> + sa.sa_cookie = node->parent;
> +
> + if (config_found(&sc->sc_dev, &sa, ispi_spi_print)) {
> + node->attached = 1;
> + return 1;
> + }
> + }
>
> return 0;
> }
> diff --git sys/arch/arm64/dev/aplhidev.c sys/dev/spi/aplhidev.c
> similarity index 74%
> rename from sys/arch/arm64/dev/aplhidev.c
> rename to sys/dev/spi/aplhidev.c
> index 97d00a8878a..f8f0f35d88b 100644
> --- sys/arch/arm64/dev/aplhidev.c
> +++ sys/dev/spi/aplhidev.c
> @@ -25,13 +25,23 @@
>
> #include <lib/libkern/crc16.h>
>
> -#include <machine/fdt.h>
> -
> #include <dev/spi/spivar.h>
>
> +#ifdef __HAVE_FDT
> +#include <machine/fdt.h>
> #include <dev/ofw/openfirm.h>
> #include <dev/ofw/ofw_gpio.h>
> #include <dev/ofw/ofw_pinctrl.h>
> +#endif
> +
> +#include "acpi.h"
> +#if NACPI > 0
> +#include <dev/acpi/acpireg.h>
> +#include <dev/acpi/acpivar.h>
> +#include <dev/acpi/acpidev.h>
> +#include <dev/acpi/amltypes.h>
> +#include <dev/acpi/dsdt.h>
> +#endif
>
> #include <dev/usb/usbdevs.h>
>
> @@ -46,6 +56,14 @@
>
> #include "aplhidev.h"
>
> +/* #define APLHIDEV_DEBUG 1 */
> +
> +#ifdef APLHIDEV_DEBUG
> +#define DPRINTF(x) printf x
> +#else
> +#define DPRINTF(x)
> +#endif
> +
> #define APLHIDEV_READ_PACKET 0x20
> #define APLHIDEV_WRITE_PACKET 0x40
>
> @@ -63,6 +81,12 @@
> #define APLHIDEV_MODE_HID 0x00
> #define APLHIDEV_MODE_RAW 0x01
> #define APLHIDEV_GET_DIMENSIONS 0xd932
> +#define APLHIDEV_SET_BACKLIGHT 0xb051
> +#define APLHIDEV_BACKLIGHT_MAGIC 0x01b0
> +#define APLHIDEV_BACKLIGHT_MIN 32
> +#define APLHIDEV_BACKLIGHT_MAX 255
> +#define APLHIDEV_BACKLIGHT_ON 0x01f4
> +#define APLHIDEV_BACKLIGHT_OFF 0x0001
>
> struct aplhidev_dim {
> uint32_t width;
> @@ -131,9 +155,19 @@ struct aplhidev_set_mode {
> uint16_t crc;
> };
>
> +struct aplhidev_set_backlight {
> + struct aplhidev_msghdr hdr;
> + uint16_t magic;
> + uint16_t level;
> + uint16_t on_off;
> + uint16_t crc;
> +};
> +
> struct aplhidev_softc {
> struct device sc_dev;
> - int sc_node;
> +#if NACPI > 0
> + struct aml_node *sc_dev_node;
> +#endif
>
> spi_tag_t sc_spi_tag;
> struct spi_config sc_spi_conf;
> @@ -166,6 +200,13 @@ struct aplhidev_softc {
>
> int aplhidev_match(struct device *, void *, void *);
> void aplhidev_attach(struct device *, struct device *, void *);
> +#ifdef __HAVE_FDT
> +void aplhidev_fdt_init(struct aplhidev_softc *, void *);
> +#elif NACPI > 0
> +int aplhidev_acpi_init(struct aplhidev_softc *);
> +int aplhidev_enable_spi(struct aplhidev_softc *);
> +int aplhidev_gpe_intr(struct aml_node *, int, void *);
> +#endif
>
> const struct cfattach aplhidev_ca = {
> sizeof(struct aplhidev_softc), aplhidev_match, aplhidev_attach
> @@ -178,9 +219,16 @@ struct cfdriver aplhidev_cd = {
> void aplhidev_get_info(struct aplhidev_softc *);
> void aplhidev_get_descriptor(struct aplhidev_softc *, uint8_t);
> void aplhidev_set_leds(struct aplhidev_softc *, uint8_t);
> +void aplhidev_set_backlight(struct aplhidev_softc *, uint8_t);
> void aplhidev_set_mode(struct aplhidev_softc *, uint8_t);
> void aplhidev_get_dimensions(struct aplhidev_softc *);
>
> +void aplkbd_set_backlight(void *);
> +extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> +extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> +int aplkbd_wskbd_get_backlight(struct wskbd_backlight *);
> +int aplkbd_wskbd_set_backlight(struct wskbd_backlight *);
> +
> int aplhidev_intr(void *);
> void aplkbd_intr(struct device *, uint8_t *, size_t);
> void aplms_intr(struct device *, uint8_t *, size_t);
> @@ -189,10 +237,26 @@ int
> aplhidev_match(struct device *parent, void *match, void *aux)
> {
> struct spi_attach_args *sa = aux;
> +#if NACPI > 0
> + struct aml_node *node = (struct aml_node *)sa->sa_cookie;
> + uint64_t val;
> +#endif
>
> +#if defined(__HAVE_FDT)
> if (strcmp(sa->sa_name, "apple,spi-hid-transport") == 0)
> return 1;
>
> +#elif NACPI > 0
> + if (strcmp(sa->sa_name, "apple-spi-topcase") == 0) {
> + /* don't attach if USB interface is present */
> + if (aml_evalinteger(acpi_softc, node, "UIST", 0, NULL,
> + &val) == 0 && val)
> + return 0;
> +
> + return 1;
> + }
> +#endif
> +
> return 0;
> }
>
> @@ -206,34 +270,15 @@ aplhidev_attach(struct device *parent, struct device *self, void *aux)
> int retry;
>
> sc->sc_spi_tag = sa->sa_tag;
> - sc->sc_node = *(int *)sa->sa_cookie;
>
> - sc->sc_gpiolen = OF_getproplen(sc->sc_node, "spien-gpios");
> - if (sc->sc_gpiolen > 0) {
> - sc->sc_gpio = malloc(sc->sc_gpiolen, M_TEMP, M_WAITOK);
> - OF_getpropintarray(sc->sc_node, "spien-gpios",
> - sc->sc_gpio, sc->sc_gpiolen);
> - gpio_controller_config_pin(sc->sc_gpio, GPIO_CONFIG_OUTPUT);
> +#ifdef __HAVE_FDT
> + aplhidev_fdt_init(sc, sa->sa_cookie);
>
> - /* Reset */
> - gpio_controller_set_pin(sc->sc_gpio, 1);
> - delay(5000);
> - gpio_controller_set_pin(sc->sc_gpio, 0);
> - delay(5000);
> -
> - /* Enable. */
> - gpio_controller_set_pin(sc->sc_gpio, 1);
> - delay(50000);
> - }
> -
> - sc->sc_spi_conf.sc_bpw = 8;
> - sc->sc_spi_conf.sc_freq = OF_getpropint(sc->sc_node,
> - "spi-max-frequency", 0);
> - sc->sc_spi_conf.sc_cs = OF_getpropint(sc->sc_node, "reg", 0);
> - sc->sc_spi_conf.sc_cs_delay = 100;
> -
> - fdt_intr_establish(sc->sc_node, IPL_TTY,
> - aplhidev_intr, sc, sc->sc_dev.dv_xname);
> +#elif NACPI > 0
> + sc->sc_dev_node = (struct aml_node *)sa->sa_cookie;
> + if (aplhidev_acpi_init(sc) != 0)
> + return;
> +#endif
>
> aplhidev_get_info(sc);
> for (retry = 10; retry > 0; retry--) {
> @@ -303,6 +348,179 @@ aplhidev_attach(struct device *parent, struct device *self, void *aux)
> }
> }
>
> +#ifdef __HAVE_FDT
> +void
> +aplhidev_fdt_init(struct aplhidev_softc *sc, void *cookie)
> +{
> + int node = *(int *)cookie;
> +
> + sc->sc_gpiolen = OF_getproplen(inode, "spien-gpios");
> + if (sc->sc_gpiolen > 0) {
> + sc->sc_gpio = malloc(sc->sc_gpiolen, M_TEMP, M_WAITOK);
> + OF_getpropintarray(node, "spien-gpios",
> + sc->sc_gpio, sc->sc_gpiolen);
> + gpio_controller_config_pin(sc->sc_gpio, GPIO_CONFIG_OUTPUT);
> +
> + /* Reset */
> + gpio_controller_set_pin(sc->sc_gpio, 1);
> + delay(5000);
> + gpio_controller_set_pin(sc->sc_gpio, 0);
> + delay(5000);
> +
> + /* Enable. */
> + gpio_controller_set_pin(sc->sc_gpio, 1);
> + delay(50000);
> + }
> +
> + sc->sc_spi_conf.sc_bpw = 8;
> + sc->sc_spi_conf.sc_freq = OF_getpropint(node, "spi-max-frequency", 0);
> + sc->sc_spi_conf.sc_cs = OF_getpropint(node, "reg", 0);
> + sc->sc_spi_conf.sc_cs_delay = 100;
> +
> + fdt_intr_establish(sc->sc_node, IPL_TTY,
> + aplhidev_intr, sc, sc->sc_dev.dv_xname);
> +}
> +#endif
> +
> +#if NACPI > 0
> +int
> +aplhidev_acpi_init(struct aplhidev_softc *sc)
> +{
> + /* a0b5b7c6-1318-441c-b0c9-fe695eaf949b */
> + static uint8_t guid[] = {
> + 0xC6, 0xB7, 0xB5, 0xA0, 0x18, 0x13, 0x1C, 0x44,
> + 0xB0, 0xC9, 0xFE, 0x69, 0x5E, 0xAF, 0x94, 0x9B,
> + };
> + struct aml_value cmd[4], res;
> + struct aml_node *node;
> + uint64_t val;
> + int i;
> +
> + /*
> + * On newer Apple hardware where we claim an OSI of Darwin, _CRS
> + * doesn't return a useful SpiSerialBusV2 object but instead returns
> + * parameters from a _DSM method when called with a particular UUID
> + * which macOS does.
> + */
> + if (!aml_searchname(sc->sc_dev_node, "_DSM")) {
> + printf("%s: couldn't find _DSM at %s\n", sc->sc_dev.dv_xname,
> + aml_nodename(sc->sc_dev_node));
> + return 1;
> + }
> +
> + bzero(&cmd, sizeof(cmd));
> + cmd[0].type = AML_OBJTYPE_BUFFER;
> + cmd[0].v_buffer = (uint8_t *)&guid;
> + cmd[0].length = sizeof(guid);
> + cmd[1].type = AML_OBJTYPE_INTEGER;
> + cmd[1].v_integer = 1;
> + cmd[1].length = 1;
> + cmd[2].type = AML_OBJTYPE_INTEGER;
> + cmd[2].v_integer = 1;
> + cmd[2].length = 1;
> + cmd[3].type = AML_OBJTYPE_BUFFER;
> + cmd[3].length = 0;
> +
> + if (aml_evalname(acpi_softc, sc->sc_dev_node, "_DSM", 4, cmd, &res)) {
> + printf("%s: eval of _DSM at %s failed\n",
> + sc->sc_dev.dv_xname, aml_nodename(sc->sc_dev_node));
> + return 1;
> + }
> +
> + if (res.type != AML_OBJTYPE_PACKAGE) {
> + printf("%s: bad _DSM result at %s: %d\n",
> + sc->sc_dev.dv_xname, aml_nodename(sc->sc_dev_node),
> + res.type);
> + aml_freevalue(&res);
> + return 1;
> + }
> +
> + if (res.length % 2 != 0) {
> + printf("%s: _DSM length %d not even\n", sc->sc_dev.dv_xname,
> + res.length);
> + aml_freevalue(&res);
> + return 1;
> + }
> +
> + for (i = 0; i < res.length; i += 2) {
> + char *k;
> +
> + if (res.v_package[i]->type != AML_OBJTYPE_STRING ||
> + res.v_package[i + 1]->type != AML_OBJTYPE_BUFFER) {
> + printf("%s: expected string+buffer, got %d+%d\n",
> + sc->sc_dev.dv_xname, res.v_package[i]->type,
> + res.v_package[i + 1]->type);
> + aml_freevalue(&res);
> + return 1;
> + }
> +
> + k = res.v_package[i]->v_string;
> + val = aml_val2int(res.v_package[i + 1]);
> +
> + DPRINTF(("%s: %s = %lld\n", sc->sc_dev.dv_xname, k, val));
> +
> + if (strcmp(k, "spiSclkPeriod") == 0) {
> + sc->sc_spi_conf.sc_freq = 1000000000 / val;
> + } else if (strcmp(k, "spiWordSize") == 0) {
> + sc->sc_spi_conf.sc_bpw = val;
> + } else if (strcmp(k, "spiSPO") == 0) {
> + if (val)
> + sc->sc_spi_conf.sc_flags |= SPI_CONFIG_CPOL;
> + } else if (strcmp(k, "spiSPH") == 0) {
> + if (val)
> + sc->sc_spi_conf.sc_flags |= SPI_CONFIG_CPHA;
> + } else if (strcmp(k, "spiCSDelay") == 0) {
> + sc->sc_spi_conf.sc_cs_delay = val;
> + } else {
> + DPRINTF(("%s: unused _DSM key %s\n",
> + sc->sc_dev.dv_xname, k));
> + }
> + }
> + aml_freevalue(&res);
> +
> + if (aplhidev_enable_spi(sc) != 0)
> + return 1;
> +
> + node = aml_searchname(sc->sc_dev_node, "_GPE");
> + if (node)
> + aml_register_notify(sc->sc_dev_node, NULL, aplhidev_gpe_intr,
> + sc, ACPIDEV_NOPOLL);
> +
> + return 0;
> +}
> +
> +int
> +aplhidev_enable_spi(struct aplhidev_softc *sc)
> +{
> + struct aml_value arg;
> + uint64_t val;
> +
> + /* if SPI is not enabled, enable it */
> + if (aml_evalinteger(acpi_softc, sc->sc_dev_node, "SIST", 0, NULL,
> + &val) == 0 && !val) {
> + DPRINTF(("%s: SIST is %lld\n", sc->sc_dev.dv_xname, val));
> +
> + bzero(&arg, sizeof(arg));
> + arg.type = AML_OBJTYPE_INTEGER;
> + arg.v_integer = 1;
> + arg.length = 1;
> +
> + if (aml_evalname(acpi_softc, sc->sc_dev_node, "SIEN", 1, &arg,
> + NULL) != 0) {
> + DPRINTF(("%s: couldn't enable SPI mode\n", __func__));
> + return 1;
> + }
> +
> + DELAY(500);
> + } else {
> + DPRINTF(("%s: SIST is already %lld\n", sc->sc_dev.dv_xname,
> + val));
> + }
> +
> + return 0;
> +}
> +#endif
> +
> void
> aplhidev_get_info(struct aplhidev_softc *sc)
> {
> @@ -409,6 +627,45 @@ aplhidev_set_leds(struct aplhidev_softc *sc, uint8_t leds)
> spi_release_bus(sc->sc_spi_tag, 0);
> }
>
> +void
> +aplhidev_set_backlight(struct aplhidev_softc *sc, uint8_t level)
> +{
> + struct aplhidev_spi_packet packet;
> + struct aplhidev_set_backlight *msg;
> + struct aplhidev_spi_status status;
> +
> + memset(&packet, 0, sizeof(packet));
> + packet.flags = APLHIDEV_WRITE_PACKET;
> + packet.device = APLHIDEV_KBD_DEVICE;
> + packet.len = sizeof(*msg);
> +
> + msg = (void *)&packet.data[0];
> + msg->hdr.type = APLHIDEV_SET_BACKLIGHT;
> + msg->hdr.device = APLHIDEV_KBD_DEVICE;
> + msg->hdr.msgid = sc->sc_msgid++;
> + msg->hdr.cmdlen = sizeof(*msg) - sizeof(struct aplhidev_msghdr) - 2;
> + msg->hdr.rsplen = msg->hdr.cmdlen;
> + msg->magic = htole16(APLHIDEV_BACKLIGHT_MAGIC);
> + if (level <= APLHIDEV_BACKLIGHT_MIN) {
> + msg->level = 0;
> + msg->on_off = htole16(APLHIDEV_BACKLIGHT_OFF);
> + } else {
> + msg->level = htole16(level);
> + msg->on_off = htole16(APLHIDEV_BACKLIGHT_ON);
> + }
> + msg->crc = crc16(0, (void *)msg, sizeof(*msg) - 2);
> +
> + packet.crc = crc16(0, (void *)&packet, sizeof(packet) - 2);
> +
> + spi_acquire_bus(sc->sc_spi_tag, 0);
> + spi_config(sc->sc_spi_tag, &sc->sc_spi_conf);
> + spi_transfer(sc->sc_spi_tag, (char *)&packet, NULL, sizeof(packet),
> + SPI_KEEP_CS);
> + delay(100);
> + spi_read(sc->sc_spi_tag, (char *)&status, sizeof(status));
> + spi_release_bus(sc->sc_spi_tag, 0);
> +}
> +
> void
> aplhidev_set_mode(struct aplhidev_softc *sc, uint8_t mode)
> {
> @@ -477,6 +734,16 @@ aplhidev_get_dimensions(struct aplhidev_softc *sc)
> delay(1000);
> }
>
> +#if NACPI > 0
> +int
> +aplhidev_gpe_intr(struct aml_node *node, int gpe, void *arg)
> +{
> + struct aplhidev_softc *sc = arg;
> +
> + return aplhidev_intr(sc);
> +}
> +#endif
> +
> int
> aplhidev_intr(void *arg)
> {
> @@ -566,6 +833,7 @@ struct aplkbd_softc {
> struct aplhidev_softc *sc_hidev;
> struct hidkbd sc_kbd;
> int sc_spl;
> + int sc_backlight;
> };
>
> void aplkbd_cngetc(void *, u_int *, int *);
> @@ -641,7 +909,12 @@ aplkbd_attach(struct device *parent, struct device *self, void *aux)
> aplkbd_enable(sc, 1);
> }
>
> +
> hidkbd_attach_wskbd(kbd, KB_US | KB_DEFAULT, &aplkbd_accessops);
> +
> + sc->sc_backlight = APLHIDEV_BACKLIGHT_MIN;
> + wskbd_get_backlight = aplkbd_wskbd_get_backlight;
> + wskbd_set_backlight = aplkbd_wskbd_set_backlight;
> }
>
> void
> @@ -693,6 +966,43 @@ aplkbd_set_leds(void *v, int leds)
> aplhidev_set_leds(sc->sc_hidev, res);
> }
>
> +int
> +aplkbd_wskbd_get_backlight(struct wskbd_backlight *kbl)
> +{
> + struct aplkbd_softc *sc = aplkbd_cd.cd_devs[0];
> +
> + if (sc == NULL)
> + return 0;
> +
> + kbl->min = APLHIDEV_BACKLIGHT_MIN;
> + kbl->max = APLHIDEV_BACKLIGHT_MAX;
> + kbl->curval = sc->sc_backlight;
> +
> + return 0;
> +}
> +
> +int
> +aplkbd_wskbd_set_backlight(struct wskbd_backlight *kbl)
> +{
> + struct aplkbd_softc *sc = aplkbd_cd.cd_devs[0];
> + int val;
> +
> + if (sc == NULL)
> + return -1;
> +
> + val = kbl->curval;
> + if (val < APLHIDEV_BACKLIGHT_MIN)
> + val = APLHIDEV_BACKLIGHT_MIN;
> + if (val > APLHIDEV_BACKLIGHT_MAX)
> + val = APLHIDEV_BACKLIGHT_MAX;
> +
> + sc->sc_backlight = val;
> + aplhidev_set_backlight((struct aplhidev_softc *)sc->sc_dev.dv_parent,
> + sc->sc_backlight);
> +
> + return 0;
> +}
> +
> /* Console interface. */
> void
> aplkbd_cngetc(void *v, u_int *type, int *data)
>
>
aplhidev: enable on amd64, add keyboard backlight support