[PATCH 2/2] input: apple: Add support for Apple SPI keyboard

Simon Glass sjg at chromium.org
Sat Jan 22 02:40:27 CET 2022


Hi Mark,

On Sun, 16 Jan 2022 at 10:06, Mark Kettenis <kettenis at openbsd.org> wrote:
>
> This driver adds support for the keyboard on Apple Silicon laptops.
> The controller for this keyboard sits on an SPI bus and uses an
> Apple-specific HID over SPI protocol. The packets sent by this
> controller for key presses and key releases are fairly simple and
> are decoded directly by the code in this driver and converted into
> standard Linux keycodes. The same controller handles the touchpad
> found on these laptops.  Packets for touchpad events are simply
> ignored.
>
> Signed-off-by: Mark Kettenis <kettenis at openbsd.org>
> ---
>  configs/apple_m1_defconfig    |   1 +
>  drivers/input/Kconfig         |   8 ++
>  drivers/input/Makefile        |   1 +
>  drivers/input/apple_spi_kbd.c | 255 ++++++++++++++++++++++++++++++++++
>  include/configs/apple.h       |   2 +-
>  5 files changed, 266 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/input/apple_spi_kbd.c

Reviewed-by: Simon Glass <sjg at chromium.org>
Tested on: Macbook Air M1
Tested-by: Simon Glass <sjg at chromium.org>

Various comments below

Doesn't this keyboard only existing on the laptop?

>
> diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
> index 1528217b17..433cc62334 100644
> --- a/configs/apple_m1_defconfig
> +++ b/configs/apple_m1_defconfig
> @@ -15,5 +15,6 @@ CONFIG_NVME_APPLE=y
>  CONFIG_USB_XHCI_HCD=y
>  CONFIG_USB_XHCI_DWC3=y
>  CONFIG_USB_KEYBOARD=y
> +CONFIG_APPLE_SPI_KEYB=y
>  CONFIG_VIDEO_SIMPLE=y
>  # CONFIG_GENERATE_SMBIOS_TABLE is not set
> diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
> index 0b753f37bf..2718b3674a 100644
> --- a/drivers/input/Kconfig
> +++ b/drivers/input/Kconfig
> @@ -47,6 +47,14 @@ config KEYBOARD
>           and is only used by novena. For new boards, use driver model
>           instead.
>
> +config APPLE_SPI_KEYB
> +       bool "Enable Apple SPI keyboard support"
> +       depends on DM_KEYBOARD && DM_SPI
> +       help
> +         This adds a driver for the keyboards found on various
> +         laptops based on Apple SoCs. These keyboards use an
> +         Apple-specific HID-over-SPI protocol.
> +
>  config CROS_EC_KEYB
>         bool "Enable Chrome OS EC keyboard support"
>         depends on INPUT
> diff --git a/drivers/input/Makefile b/drivers/input/Makefile
> index e440c921e4..b1133f772f 100644
> --- a/drivers/input/Makefile
> +++ b/drivers/input/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_$(SPL_TPL_)DM_KEYBOARD) += input.o keyboard-uclass.o
>
>  ifndef CONFIG_SPL_BUILD
>
> +obj-$(CONFIG_APPLE_SPI_KEYB) += apple_spi_kbd.o
>  obj-$(CONFIG_I8042_KEYB) += i8042.o
>  obj-$(CONFIG_TEGRA_KEYBOARD) += input.o tegra-kbc.o
>  obj-$(CONFIG_TWL4030_INPUT) += twl4030.o
> diff --git a/drivers/input/apple_spi_kbd.c b/drivers/input/apple_spi_kbd.c
> new file mode 100644
> index 0000000000..7e034d6008
> --- /dev/null
> +++ b/drivers/input/apple_spi_kbd.c
> @@ -0,0 +1,255 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2021 Mark Kettenis <kettenis at openbsd.org>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <keyboard.h>
> +#include <spi.h>
> +#include <stdio_dev.h>
> +#include <asm-generic/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/input.h>
> +
> +/*
> + * The Apple SPI keyboard controller implements a protocol that
> + * closely resembles HID Keyboard Boot protocol.  The key codes are
> + * mapped according to the HID Keyboard/Keypad Usage Table.
> + */
> +
> +/* Modifier key bits */
> +#define HID_MOD_LEFTCTRL       BIT(0)
> +#define HID_MOD_LEFTSHIFT      BIT(1)
> +#define HID_MOD_LEFTALT                BIT(2)
> +#define HID_MOD_LEFTGUI                BIT(3)
> +#define HID_MOD_RIGHTCTRL      BIT(4)
> +#define HID_MOD_RIGHTSHIFT     BIT(5)
> +#define HID_MOD_RIGHTALT       BIT(6)
> +#define HID_MOD_RIGHTGUI       BIT(7)
> +
> +static const u8 hid_kbd_keymap[] = {
> +       KEY_RESERVED, 0xff, 0xff, 0xff,
> +       KEY_A, KEY_B, KEY_C, KEY_D,
> +       KEY_E, KEY_F, KEY_G, KEY_H,
> +       KEY_I, KEY_J, KEY_K, KEY_L,
> +       KEY_M, KEY_N, KEY_O, KEY_P,
> +       KEY_Q, KEY_R, KEY_S, KEY_T,
> +       KEY_U, KEY_V, KEY_W, KEY_X,
> +       KEY_Y, KEY_Z, KEY_1, KEY_2,
> +       KEY_3, KEY_4, KEY_5, KEY_6,
> +       KEY_7, KEY_8, KEY_9, KEY_0,
> +       KEY_ENTER, KEY_ESC, KEY_BACKSPACE, KEY_TAB,
> +       KEY_SPACE, KEY_MINUS, KEY_EQUAL, KEY_LEFTBRACE,
> +       KEY_RIGHTBRACE, KEY_BACKSLASH, 0xff, KEY_SEMICOLON,
> +       KEY_APOSTROPHE, KEY_GRAVE, KEY_COMMA, KEY_DOT,
> +       KEY_SLASH, KEY_CAPSLOCK, KEY_F1, KEY_F2,
> +       KEY_F3, KEY_F4, KEY_F5, KEY_F6,
> +       KEY_F7, KEY_F8, KEY_F9, KEY_F10,
> +       KEY_F11, KEY_F12, KEY_SYSRQ, KEY_SCROLLLOCK,
> +       KEY_PAUSE, KEY_INSERT, KEY_HOME, KEY_PAGEUP,
> +       KEY_DELETE, KEY_END, KEY_PAGEDOWN, KEY_RIGHT,
> +       KEY_LEFT, KEY_DOWN, KEY_UP, KEY_NUMLOCK,
> +       KEY_KPSLASH, KEY_KPASTERISK, KEY_KPMINUS, KEY_KPPLUS,
> +       KEY_KPENTER, KEY_KP1, KEY_KP2, KEY_KP3,
> +       KEY_KP4, KEY_KP5, KEY_KP6, KEY_KP7,
> +       KEY_KP8, KEY_KP9, KEY_KP0, KEY_KPDOT,
> +       KEY_BACKSLASH, KEY_COMPOSE, KEY_POWER, KEY_KPEQUAL,
> +};
> +
> +/* Report ID used for keyboard input reports. */
> +#define KBD_REPORTID   0x01
> +
> +struct apple_spi_kbd_report {

Is there another kind of keyboard? Perhaps apple_kbd ?

> +       u8 reportid;
> +       u8 modifiers;
> +       u8 reserved;
> +       u8 keycode[6];
> +       u8 fn;
> +};
> +
> +struct apple_spi_kbd_priv {
> +       struct gpio_desc enable;
> +       struct apple_spi_kbd_report old;
> +       struct apple_spi_kbd_report new;
> +};

comment

> +
> +/* Keyboard device. */
> +#define KBD_DEVICE     0x01
> +
> +struct apple_spi_kbd_packet {
> +       u8 flags;
> +#define PACKET_READ    0x20
> +       u8 device;
> +       u16 offset;
> +       u16 remaining;
> +       u16 len;
> +       u8 data[246];
> +       u16 crc;
> +};
> +
> +struct apple_spi_kbd_msg {

what are these two? Need a comment. One is a message and one is a packet?

> +       u8 type;
> +#define MSG_REPORT     0x10
> +       u8 device;
> +       u8 unknown;
> +       u8 msgid;
> +       u16 rsplen;
> +       u16 cmdlen;
> +       u8 data[0];
> +};
> +
> +static void apple_spi_kbd_service_modifiers(struct input_config *input)
> +{
> +       struct apple_spi_kbd_priv *priv = dev_get_priv(input->dev);
> +       u8 new = priv->new.modifiers;
> +       u8 old = priv->old.modifiers;
> +
> +       if ((new ^ old) & HID_MOD_LEFTCTRL)
> +               input_add_keycode(input, KEY_LEFTCTRL,
> +                                 old & HID_MOD_LEFTCTRL);
> +       if ((new ^ old) & HID_MOD_RIGHTCTRL)
> +               input_add_keycode(input, KEY_RIGHTCTRL,
> +                                 old & HID_MOD_RIGHTCTRL);
> +       if ((new ^ old) & HID_MOD_LEFTSHIFT)
> +               input_add_keycode(input, KEY_LEFTSHIFT,
> +                                 old & HID_MOD_LEFTSHIFT);
> +       if ((new ^ old) & HID_MOD_RIGHTSHIFT)
> +               input_add_keycode(input, KEY_RIGHTSHIFT,
> +                                 old & HID_MOD_RIGHTSHIFT);
> +       if ((new ^ old) & HID_MOD_LEFTALT)
> +               input_add_keycode(input, KEY_LEFTALT,
> +                                 old & HID_MOD_LEFTALT);
> +       if ((new ^ old) & HID_MOD_RIGHTALT)
> +               input_add_keycode(input, KEY_RIGHTALT,
> +                                 old & HID_MOD_RIGHTALT);
> +       if ((new ^ old) & HID_MOD_LEFTGUI)
> +               input_add_keycode(input, KEY_LEFTMETA,
> +                                 old & HID_MOD_LEFTGUI);
> +       if ((new ^ old) & HID_MOD_RIGHTGUI)
> +               input_add_keycode(input, KEY_RIGHTMETA,
> +                                 old & HID_MOD_RIGHTGUI);
> +}
> +
> +static void apple_spi_kbd_service_key(struct input_config *input, int i,
> +                                     int released)
> +{
> +       struct apple_spi_kbd_priv *priv = dev_get_priv(input->dev);
> +       u8 *new;
> +       u8 *old;
> +
> +       if (released) {
> +               new = priv->new.keycode;
> +               old = priv->old.keycode;
> +       } else {
> +               new = priv->old.keycode;
> +               old = priv->new.keycode;
> +       }
> +
> +       if (memscan(new, old[i], sizeof(priv->new.keycode)) ==
> +           new + sizeof(priv->new.keycode) &&
> +           old[i] < ARRAY_SIZE(hid_kbd_keymap))
> +               input_add_keycode(input, hid_kbd_keymap[old[i]], released);
> +}
> +
> +static int apple_spi_kbd_check(struct input_config *input)
> +{
> +       struct udevice *dev = input->dev;
> +       struct apple_spi_kbd_priv *priv = dev_get_priv(dev);
> +       struct apple_spi_kbd_packet packet;
> +       struct apple_spi_kbd_msg *msg;
> +       struct apple_spi_kbd_report *report;
> +       int i, ret;
> +
> +       memset(&packet, 0, sizeof(packet));
> +
> +       ret = dm_spi_claim_bus(dev);
> +       if (ret < 0)
> +               return ret;
> +
> +       /*
> +        * The keyboard controller needs delays after asserting CS#
> +        * and before deasserting CS#.
> +        */
> +       dm_spi_xfer(dev, 0, NULL, NULL, SPI_XFER_BEGIN);

Check error returns in these

> +       udelay(100);
> +       dm_spi_xfer(dev, sizeof(packet) * 8, NULL, &packet, 0);
> +       udelay(100);
> +       dm_spi_xfer(dev, 0, NULL, NULL, SPI_XFER_END);
> +
> +       dm_spi_release_bus(dev);
> +
> +       /*
> +        * The keyboard controller needs a delay between subsequent
> +        * SPI transfers.
> +        */
> +       udelay(250);

Is this the same delay for all models?

> +
> +       msg = (struct apple_spi_kbd_msg *)packet.data;
> +       report = (struct apple_spi_kbd_report *)msg->data;
> +       if (packet.flags == PACKET_READ && packet.device == KBD_DEVICE &&
> +           msg->type == MSG_REPORT && msg->device == KBD_DEVICE &&
> +           msg->cmdlen == sizeof(struct apple_spi_kbd_report) &&
> +           report->reportid == KBD_REPORTID) {
> +               memcpy(&priv->new, report,
> +                      sizeof(struct apple_spi_kbd_report));
> +               apple_spi_kbd_service_modifiers(input);
> +               for (i = 0; i < sizeof(priv->new.keycode); i++) {
> +                       apple_spi_kbd_service_key(input, i, 1);
> +                       apple_spi_kbd_service_key(input, i, 0);
> +               }
> +               memcpy(&priv->old, &priv->new,
> +                      sizeof(struct apple_spi_kbd_report));
> +               return 1;
> +       }
> +
> +       return 0;
> +}
> +
> +static int apple_spi_kbd_probe(struct udevice *dev)
> +{
> +       struct apple_spi_kbd_priv *priv = dev_get_priv(dev);
> +       struct keyboard_priv *uc_priv = dev_get_uclass_priv(dev);
> +       struct stdio_dev *sdev = &uc_priv->sdev;
> +       struct input_config *input = &uc_priv->input;
> +       int ret;
> +
> +       ret = gpio_request_by_name(dev, "spien-gpios", 0, &priv->enable,
> +                                  GPIOD_IS_OUT);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* Reset the keyboard controller. */
> +       dm_gpio_set_value(&priv->enable, 1);
> +       udelay(5000);
> +       dm_gpio_set_value(&priv->enable, 0);
> +       udelay(5000);
> +
> +       /* Enable the keyboard controller. */
> +       dm_gpio_set_value(&priv->enable, 1);
> +       udelay(50000);

That is a long time. Could we timestamp this instead and then check
when the keyboard is read that enough time has elapsed?

> +
> +       input->dev = dev;
> +       input->read_keys = apple_spi_kbd_check;
> +       input_add_tables(input, false);
> +       strcpy(sdev->name, "spikbd");
> +
> +       return input_stdio_register(sdev);
> +}
> +
> +static const struct keyboard_ops apple_spi_kbd_ops = {
> +};
> +
> +static const struct udevice_id apple_spi_kbd_of_match[] = {
> +       { .compatible = "apple,spi-hid-transport" },
> +       { /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(apple_spi_kbd) = {
> +       .name = "apple_spi_kbd",
> +       .id = UCLASS_KEYBOARD,
> +       .of_match = apple_spi_kbd_of_match,
> +       .probe = apple_spi_kbd_probe,
> +       .priv_auto = sizeof(struct apple_spi_kbd_priv),
> +       .ops = &apple_spi_kbd_ops,
> +};
> diff --git a/include/configs/apple.h b/include/configs/apple.h
> index 47faad8150..f12e9bdef5 100644
> --- a/include/configs/apple.h
> +++ b/include/configs/apple.h
> @@ -5,7 +5,7 @@
>
>  /* Environment */
>  #define ENV_DEVICE_SETTINGS \
> -       "stdin=serial,usbkbd\0" \
> +       "stdin=serial,usbkbd,spikbd\0" \
>         "stdout=serial,vidconsole\0" \
>         "stderr=serial,vidconsole\0"
>
> --
> 2.34.1
>

Regards,
Simon


More information about the U-Boot mailing list