[PATCH 2/2] input: apple: Add support for Apple SPI keyboard
Mark Kettenis
mark.kettenis at xs4all.nl
Sat Jan 22 17:26:51 CET 2022
> From: Simon Glass <sjg at chromium.org>
> Date: Fri, 21 Jan 2022 18:40:27 -0700
Hi Simon,
> 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?
It does (and the Kconfig entry says so). The driver is enabled in
apple_m1_defconfig since these machines have tons of memory and it
doesn't increase the build time in a meaningful way. So we can use a
single defconfig for all M1 machines.
> > 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 ?
There are Apple USB keyboards that may need quirks.
> > + 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?
Yes; the message is embedded in a packet. Comments added.
>
> > + 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
Sure.
> > + 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?
We think so. The hardware differences between the different models
are really small. The Linux folks may still refine their device tree
bindings, at which point we can update this code.
> > +
> > + 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?
Actually, I can delete that last delay. If we poll the keyboard
before the controller is ready nothing bad will happen.
> > +
> > + 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