From: Marcus Glocker Subject: Re: samsabi(4): Keyboard backlight support for the SGB4E To: Mark Kettenis Cc: tech@openbsd.org Date: Mon, 25 May 2026 22:07:02 +0200 On Mon, May 25, 2026 at 09:39:09PM +0200, Mark Kettenis wrote: > > Date: Mon, 25 May 2026 21:10:36 +0200 > > From: Marcus Glocker > > Content-Type: text/plain; charset=us-ascii > > Content-Disposition: inline > > > > This diff adds initial keyboard backlight support for the Samsung Galaxy > > Book4 Edge. From the man page: > > > > "The samsabi driver provides support for the Samsung Advanced BIOS > > Interface (SABI), a vendor command interface of the embedded controller > > found in Samsung laptops based on the Qualcomm Snapdragon X1E80100, such > > as the Galaxy Book4 Edge. The controller, identified as "SAM060B" in > > ACPI, is an ENE KB9058 micro controller running Samsung firmware. It > > exposes the SABI command protocol on the I2C bus at a separate address > > from the "Mbox" channel used by sambat(4) for battery status." > > > > Further devices can be controlled through this interface in the future, > > for example fan control. > > > > Ok? > > A few remarks: > > * You "invented" the devicetree bindings for this thing isn't it? > That's fine, but if this ever ends up as an "official" binding and > changes get made, we should follow. Not a big issue since this > isn't essential hardware. That's right. And yes, fully agree, if this DTS should ever land in Linux, we would need to make the adaptions in our drivers. But I think that should be feasible. > * The locking looks very suspicious. Why do you think you need this > lock? As far as I can tell, all this code runs with the kernel > lock. I was not sure about the kernel lock, but you're right. I removed the lock. > * Why do you use I2C_F_POLL? As far as I can tell all this code runs > in process context, so there is no reason we cannot sleep. You're also right. I moved I2C_F_POLL to 0. Attached the updated diff. Index: sys/dev/i2c/samsabi.c =================================================================== RCS file: sys/dev/i2c/samsabi.c diff -N sys/dev/i2c/samsabi.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ sys/dev/i2c/samsabi.c 25 May 2026 20:03:18 -0000 @@ -0,0 +1,281 @@ +/* $OpenBSD$ */ + +/* + * Copyright (c) 2026 Marcus Glocker + * + * 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. + */ + +/* + * A driver to control the Samsung Advanced BIOS Interface (SABI), a Samsung + * proprietary firmware/EC command interface on address 0x62. + * + * For now the driver supports: + * + * - Keyboard backlight control + */ + +#include +#include +#include +#include + +#include +#include + +/* Debugging */ +/* #define SAMSABI_DEBUG */ +#ifdef SAMSABI_DEBUG +#define DPRINTF(x) printf x +#else +#define DPRINTF(x) +#endif + +/* Keyboard control */ +#define SAMSABI_KBD_CMD_SET 0x10 +#define SAMSABI_KBD_CMD_GET 0x11 +#define SAMSABI_KBD_MIN_BACKLIGHT 0 +#define SAMSABI_KBD_MAX_BACKLIGHT 3 + +struct samsabi_softc { + struct device sc_dev; + i2c_tag_t sc_tag; + int sc_addr; + + unsigned int sc_backlight; + struct timeout sc_to_keepalive; +}; + +int samsabi_match(struct device *, void *, void *); +void samsabi_attach(struct device *, struct device *, void *); +int samsabi_activate(struct device *, int); + +int samsabi_cmd(struct samsabi_softc *, const uint8_t *, size_t, + uint8_t *, size_t); +int samsabi_kbd_get(struct samsabi_softc *, uint8_t *); +int samsabi_kbd_set(struct samsabi_softc *, uint8_t); +int samsabi_kbd_probe(struct samsabi_softc *); +void samsabi_kbd_keepalive(void *); +int samsabi_get_backlight(struct wskbd_backlight *); +int samsabi_set_backlight(struct wskbd_backlight *); +extern int (*wskbd_get_backlight)(struct wskbd_backlight *); +extern int (*wskbd_set_backlight)(struct wskbd_backlight *); + +const struct cfattach samsabi_ca = { + sizeof(struct samsabi_softc), samsabi_match, samsabi_attach, NULL, + samsabi_activate +}; + +struct cfdriver samsabi_cd = { + NULL, "samsabi", DV_DULL +}; + +int +samsabi_match(struct device *parent, void *match, void *aux) +{ + struct i2c_attach_args *ia = aux; + + if (strcmp(ia->ia_name, "samsung,galaxybook-sabi") == 0) + return 1; + + return 0; +} + +void +samsabi_attach(struct device *parent, struct device *self, void *aux) +{ + struct samsabi_softc *sc = (struct samsabi_softc *)self; + struct i2c_attach_args *ia = aux; + int error; + + sc->sc_tag = ia->ia_tag; + sc->sc_addr = ia->ia_addr; + + error = samsabi_kbd_probe(sc); + if (error) { + printf(": keyboard backlight probe error=%d\n", error); + return; + } + + wskbd_get_backlight = samsabi_get_backlight; + wskbd_set_backlight = samsabi_set_backlight; + + timeout_set_proc(&sc->sc_to_keepalive, samsabi_kbd_keepalive, sc); + + printf(": %s\n", ia->ia_name); +} + +int +samsabi_activate(struct device *self, int act) +{ + struct samsabi_softc *sc = (struct samsabi_softc *)self; + + switch (act) { + case DVACT_QUIESCE: + timeout_del(&sc->sc_to_keepalive); + samsabi_kbd_set(sc, 0); + break; + case DVACT_WAKEUP: + samsabi_kbd_set(sc, sc->sc_backlight); + if (sc->sc_backlight != 0) + timeout_add_sec(&sc->sc_to_keepalive, 1); + break; + } + + return 0; +} + +int +samsabi_cmd(struct samsabi_softc *sc, const uint8_t *cmd, size_t cmdlen, + uint8_t *rsp, size_t rsplen) +{ + int error; + + iic_acquire_bus(sc->sc_tag, 0); + + if (rsplen > 0) { + /* write command and read response */ + error = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, + sc->sc_addr, cmd, cmdlen, rsp, rsplen, 0); + } else { + /* write command */ + error = iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP, + sc->sc_addr, cmd, cmdlen, NULL, 0, 0); + } + + iic_release_bus(sc->sc_tag, 0); + + return error; +} + +int +samsabi_kbd_get(struct samsabi_softc *sc, uint8_t *level) +{ + uint8_t cmd = SAMSABI_KBD_CMD_GET; + /* + * GET command response: + * rsp[3] = Backlight brightness level (0-3) + */ + uint8_t rsp[8]; + int error; + + memset(rsp, 0, sizeof(rsp)); + + error = samsabi_cmd(sc, &cmd, 1, rsp, sizeof(rsp)); + if (error) + return error; + + DPRINTF(("%s: rsp=%02x %02x %02x %02x %02x %02x %02x %02x\n", + __func__, rsp[0], rsp[1], rsp[2], rsp[3], rsp[4], rsp[5], rsp[6], + rsp[7])); + + *level = rsp[3]; + + return 0; +} + +int +samsabi_kbd_set(struct samsabi_softc *sc, uint8_t level) +{ + /* + * SET command: + * cmd[0] = 0x10 + * cmd[2] = Backlight brightness level: + * 0 = off, 1 = low, 2 = mid, 3 = high + */ + uint8_t cmd[3] = { SAMSABI_KBD_CMD_SET, 0, level }; + int error; + + error = samsabi_cmd(sc, cmd, sizeof(cmd), NULL, 0); + if (error) + return error; + + return 0; +} + +int +samsabi_kbd_probe(struct samsabi_softc *sc) +{ + int error; + uint8_t level; + + /* Check if we can get the level value */ + error = samsabi_kbd_get(sc, &level); + if (error) + return error; + + /* Make the keyboard light up once */ + error = samsabi_kbd_set(sc, SAMSABI_KBD_MAX_BACKLIGHT); + if (error) + return error; + + return 0; +} + +void +samsabi_kbd_keepalive(void *arg) +{ + struct samsabi_softc *sc = arg; + + DPRINTF(("%s: level=%d\n", __func__, sc->sc_backlight)); + + (void)samsabi_kbd_set(sc, sc->sc_backlight); + + timeout_add_sec(&sc->sc_to_keepalive, 1); +} + +int +samsabi_get_backlight(struct wskbd_backlight *kbl) +{ + struct samsabi_softc *sc = samsabi_cd.cd_devs[0]; + + KASSERT(sc != NULL); + + kbl->min = SAMSABI_KBD_MIN_BACKLIGHT; + kbl->max = SAMSABI_KBD_MAX_BACKLIGHT; + kbl->curval = sc->sc_backlight; + + return 0; +} + +int +samsabi_set_backlight(struct wskbd_backlight *kbl) +{ + struct samsabi_softc *sc = samsabi_cd.cd_devs[0]; + int error; + uint8_t level; + + KASSERT(sc != NULL); + + DPRINTF(("%s: curval=%d\n", __func__, kbl->curval)); + + /* + * XXX Only full brightness can be held lit: the EC fades lower + * levels faster than the keepalive timeout can refresh them, so + * treat any non-zero request as full brightness for now. + */ + level = kbl->curval ? SAMSABI_KBD_MAX_BACKLIGHT : 0; + + error = samsabi_kbd_set(sc, level); + if (error) + return error; + + sc->sc_backlight = level; + + if (sc->sc_backlight == 0) + timeout_del(&sc->sc_to_keepalive); + else + timeout_add_sec(&sc->sc_to_keepalive, 1); + + return 0; +} Index: sys/dev/i2c/files.i2c =================================================================== RCS file: /cvs/src/sys/dev/i2c/files.i2c,v diff -u -p -u -p -r1.75 files.i2c --- sys/dev/i2c/files.i2c 19 May 2026 04:40:45 -0000 1.75 +++ sys/dev/i2c/files.i2c 25 May 2026 20:03:18 -0000 @@ -279,6 +279,11 @@ device sambat attach sambat at i2c file dev/i2c/sambat.c sambat +# Samsung Advanced BIOS Interface (SABI) +device samsabi +attach samsabi at i2c +file dev/i2c/samsabi.c samsabi + # Consumer Control Keyboards device icc: hid, hidcc, wskbddev attach icc at ihidbus Index: sys/arch/arm64/conf/GENERIC =================================================================== RCS file: /cvs/src/sys/arch/arm64/conf/GENERIC,v diff -u -p -u -p -r1.316 GENERIC --- sys/arch/arm64/conf/GENERIC 19 May 2026 04:49:33 -0000 1.316 +++ sys/arch/arm64/conf/GENERIC 25 May 2026 20:03:19 -0000 @@ -628,6 +628,7 @@ tcpci* at iic? # USB Type-C controlle tipd* at iic? # TPS6598x Type-C controller pijuice* at iic? # PiJuice HAT sambat* at iic? # Samsung SAM060B battery monitor +samsabi* at iic? # Samsung Advance BIOS Interface # GPIO "pin bus" drivers gpioiic* at gpio? # I2C bus bit-banging Index: share/man/man4/samsabi.4 =================================================================== RCS file: share/man/man4/samsabi.4 diff -N share/man/man4/samsabi.4 --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ share/man/man4/samsabi.4 25 May 2026 20:03:19 -0000 @@ -0,0 +1,72 @@ +.\" $OpenBSD$ +.\" +.\" Copyright (c) 2026 Marcus Glocker +.\" +.\" 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. +.\" +.Dd $Mdocdate: May 25 2026 $ +.Dt SAMSABI 4 +.Os +.Sh NAME +.Nm samsabi +.Nd Samsung laptop keyboard backlight +.Sh SYNOPSIS +.Cd "samsabi* at iic?" +.Sh DESCRIPTION +The +.Nm +driver provides support for the Samsung Advanced BIOS Interface +.Pq SABI , +a vendor command interface of the embedded controller found in Samsung +laptops based on the Qualcomm Snapdragon X1E80100, such as the Galaxy +Book4 Edge. +The controller, identified as +.Dq SAM060B +in ACPI, is an ENE KB9058 micro controller running Samsung firmware. +It exposes the SABI command protocol on the I2C bus at a separate +address from the +.Dq Mbox +channel used by +.Xr sambat 4 +for battery status. +.Pp +The +.Nm +driver currently uses this interface to control the keyboard backlight. +The backlight level can be retrieved and modified with the +.Xr wsconsctl 8 +variable +.Va keyboard.backlight . +.Sh SEE ALSO +.Xr iic 4 , +.Xr intro 4 , +.Xr sambat 4 , +.Xr wskbd 4 , +.Xr wsconsctl 8 +.Sh HISTORY +The +.Nm +driver first appeared in +.Ox 8.0 . +.Sh AUTHORS +.An -nosplit +The +.Nm +driver was written by +.An Marcus Glocker Aq Mt mglocker@openbsd.org . +.Sh BUGS +The embedded controller does not hold intermediate keyboard backlight +levels: it fades the backlight off shortly after a level is set, and +only full brightness can be refreshed often enough to be kept lit. +As a result the driver treats any non-zero brightness request as full +brightness.