* [PATCH 0/5] Pine64 PinePhone keyboard support
@ 2022-01-29 23:00 Samuel Holland
2022-01-29 23:00 ` [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard binding Samuel Holland
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Samuel Holland @ 2022-01-29 23:00 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang,
Ondrej Jirman, Samuel Holland
This series adds support for the official keyboard case for the Pine64
PinePhone and PinePhone Pro. This accessory contains a keyboard MCU and
an IP5209 power bank IC. The keyboard MCU firmware[0] is free software.
It exposes the keyboard scan matrix over I2C, and also provides commands
for SMBus access to the IP5209. In order to keep the IP5209 driver[1]
generic, this is modeled as an I2C bus child of the keyboard.
[0]: https://megous.com/git/pinephone-keyboard/about/
[1]: https://lore.kernel.org/lkml/20220129222424.45707-1-samuel@sholland.org/T/
Samuel Holland (5):
dt-bindings: input: Add the PinePhone keyboard binding
Input: pinephone-keyboard - Add PinePhone keyboard driver
Input: pinephone-keyboard - Build in the default keymap
Input: pinephone-keyboard - Support the proxied I2C bus
[DO NOT MERGE] arm64: dts: allwinner: pinephone: Add keyboard
.../input/pine64,pinephone-keyboard.yaml | 90 ++++
MAINTAINERS | 6 +
.../dts/allwinner/sun50i-a64-pinephone.dtsi | 18 +
drivers/input/keyboard/Kconfig | 10 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/pinephone-keyboard.c | 449 ++++++++++++++++++
6 files changed, 574 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml
create mode 100644 drivers/input/keyboard/pinephone-keyboard.c
--
2.33.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard binding 2022-01-29 23:00 [PATCH 0/5] Pine64 PinePhone keyboard support Samuel Holland @ 2022-01-29 23:00 ` Samuel Holland 2022-02-11 13:23 ` Rob Herring 2022-01-29 23:00 ` [PATCH 2/5] Input: pinephone-keyboard - Add PinePhone keyboard driver Samuel Holland ` (3 subsequent siblings) 4 siblings, 1 reply; 18+ messages in thread From: Samuel Holland @ 2022-01-29 23:00 UTC (permalink / raw) To: Dmitry Torokhov, linux-input Cc: linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman, Samuel Holland Add devicetree support for the PinePhone keyboard case, which provides a matrix keyboard interface and a proxied I2C bus. Signed-off-by: Samuel Holland <samuel@sholland.org> --- .../input/pine64,pinephone-keyboard.yaml | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml diff --git a/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml b/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml new file mode 100644 index 000000000000..00f084b263f0 --- /dev/null +++ b/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/input/pine64,pinephone-keyboard.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Pine64 PinePhone keyboard device tree bindings + +maintainers: + - Samuel Holland <samuel@sholland.org> + +description: + A keyboard accessory is available for the Pine64 PinePhone and PinePhone Pro. + It connects via I2C, providing a raw scan matrix, a flashing interface, and a + subordinate I2C bus for communication with a battery charger IC. + +allOf: + - $ref: /schemas/input/matrix-keymap.yaml# + +properties: + compatible: + const: pine64,pinephone-keyboard + + reg: + const: 0x15 + + interrupts: + maxItems: 1 + + linux,fn-keymap: + $ref: /schemas/input/matrix-keymap.yaml#/properties/linux,keymap + description: keymap used when the Fn key is pressed + + wakeup-source: true + + i2c-bus: + $ref: /schemas/i2c/i2c-controller.yaml# + +dependencies: + linux,fn-keymap: [ 'keypad,num-columns', 'keypad,num-rows' ] + linux,keymap: [ 'keypad,num-columns', 'keypad,num-rows' ] + +required: + - compatible + - reg + - interrupts + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/input/input.h> + #include <dt-bindings/interrupt-controller/irq.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + keyboard@15 { + compatible = "pine64,pinephone-keyboard"; + reg = <0x15>; + interrupt-parent = <&r_pio>; + interrupts = <0 12 IRQ_TYPE_EDGE_FALLING>; /* PL12 */ + keypad,num-rows = <6>; + keypad,num-columns = <12>; + linux,fn-keymap = <MATRIX_KEY(0, 0, KEY_FN_ESC) + MATRIX_KEY(0, 1, KEY_F1) + MATRIX_KEY(0, 2, KEY_F2) + /* ... */ + MATRIX_KEY(5, 2, KEY_FN) + MATRIX_KEY(5, 3, KEY_LEFTALT) + MATRIX_KEY(5, 5, KEY_RIGHTALT)>; + linux,keymap = <MATRIX_KEY(0, 0, KEY_ESC) + MATRIX_KEY(0, 1, KEY_1) + MATRIX_KEY(0, 2, KEY_2) + /* ... */ + MATRIX_KEY(5, 2, KEY_FN) + MATRIX_KEY(5, 3, KEY_LEFTALT) + MATRIX_KEY(5, 5, KEY_RIGHTALT)>; + + i2c-bus { + #address-cells = <1>; + #size-cells = <0>; + + charger@75 { + reg = <0x75>; + }; + }; + }; + }; -- 2.33.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard binding 2022-01-29 23:00 ` [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard binding Samuel Holland @ 2022-02-11 13:23 ` Rob Herring 2022-02-14 4:41 ` Samuel Holland 0 siblings, 1 reply; 18+ messages in thread From: Rob Herring @ 2022-02-11 13:23 UTC (permalink / raw) To: Samuel Holland Cc: Dmitry Torokhov, linux-input, linux-kernel, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman On Sat, Jan 29, 2022 at 05:00:38PM -0600, Samuel Holland wrote: > Add devicetree support for the PinePhone keyboard case, which provides a > matrix keyboard interface and a proxied I2C bus. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > .../input/pine64,pinephone-keyboard.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml > > diff --git a/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml b/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml > new file mode 100644 > index 000000000000..00f084b263f0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml > @@ -0,0 +1,90 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/pine64,pinephone-keyboard.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Pine64 PinePhone keyboard device tree bindings > + > +maintainers: > + - Samuel Holland <samuel@sholland.org> > + > +description: > + A keyboard accessory is available for the Pine64 PinePhone and PinePhone Pro. > + It connects via I2C, providing a raw scan matrix, a flashing interface, and a > + subordinate I2C bus for communication with a battery charger IC. > + > +allOf: > + - $ref: /schemas/input/matrix-keymap.yaml# > + > +properties: > + compatible: > + const: pine64,pinephone-keyboard > + > + reg: > + const: 0x15 > + > + interrupts: > + maxItems: 1 > + > + linux,fn-keymap: This should be handled in a common way. Not sure if there's anything existing for alternate key maps. Child nodes of alternate maps would scale better than new property name for every alternate map. Or you could make linux,keymap contain multiple maps (e.g. 2x XxY entries) Or if the map doesn't change, just put it in the driver. > + $ref: /schemas/input/matrix-keymap.yaml#/properties/linux,keymap Referencing individual properties should be avoided. > + description: keymap used when the Fn key is pressed > + > + wakeup-source: true > + > + i2c-bus: > + $ref: /schemas/i2c/i2c-controller.yaml# > + > +dependencies: > + linux,fn-keymap: [ 'keypad,num-columns', 'keypad,num-rows' ] > + linux,keymap: [ 'keypad,num-columns', 'keypad,num-rows' ] > + > +required: > + - compatible > + - reg > + - interrupts > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/input/input.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + keyboard@15 { > + compatible = "pine64,pinephone-keyboard"; > + reg = <0x15>; > + interrupt-parent = <&r_pio>; > + interrupts = <0 12 IRQ_TYPE_EDGE_FALLING>; /* PL12 */ > + keypad,num-rows = <6>; > + keypad,num-columns = <12>; > + linux,fn-keymap = <MATRIX_KEY(0, 0, KEY_FN_ESC) > + MATRIX_KEY(0, 1, KEY_F1) > + MATRIX_KEY(0, 2, KEY_F2) > + /* ... */ > + MATRIX_KEY(5, 2, KEY_FN) > + MATRIX_KEY(5, 3, KEY_LEFTALT) > + MATRIX_KEY(5, 5, KEY_RIGHTALT)>; > + linux,keymap = <MATRIX_KEY(0, 0, KEY_ESC) > + MATRIX_KEY(0, 1, KEY_1) > + MATRIX_KEY(0, 2, KEY_2) > + /* ... */ > + MATRIX_KEY(5, 2, KEY_FN) > + MATRIX_KEY(5, 3, KEY_LEFTALT) > + MATRIX_KEY(5, 5, KEY_RIGHTALT)>; > + > + i2c-bus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + charger@75 { > + reg = <0x75>; > + }; > + }; > + }; > + }; > -- > 2.33.1 > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard binding 2022-02-11 13:23 ` Rob Herring @ 2022-02-14 4:41 ` Samuel Holland 0 siblings, 0 replies; 18+ messages in thread From: Samuel Holland @ 2022-02-14 4:41 UTC (permalink / raw) To: Rob Herring, Dmitry Torokhov Cc: linux-input, linux-kernel, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman, Olof Johansson On 2/11/22 7:23 AM, Rob Herring wrote: > On Sat, Jan 29, 2022 at 05:00:38PM -0600, Samuel Holland wrote: >> Add devicetree support for the PinePhone keyboard case, which provides a >> matrix keyboard interface and a proxied I2C bus. >> >> Signed-off-by: Samuel Holland <samuel@sholland.org> >> --- >> >> .../input/pine64,pinephone-keyboard.yaml | 90 +++++++++++++++++++ >> 1 file changed, 90 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml >> >> diff --git a/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml b/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml >> new file mode 100644 >> index 000000000000..00f084b263f0 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml >> @@ -0,0 +1,90 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/input/pine64,pinephone-keyboard.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Pine64 PinePhone keyboard device tree bindings >> + >> +maintainers: >> + - Samuel Holland <samuel@sholland.org> >> + >> +description: >> + A keyboard accessory is available for the Pine64 PinePhone and PinePhone Pro. >> + It connects via I2C, providing a raw scan matrix, a flashing interface, and a >> + subordinate I2C bus for communication with a battery charger IC. >> + >> +allOf: >> + - $ref: /schemas/input/matrix-keymap.yaml# >> + >> +properties: >> + compatible: >> + const: pine64,pinephone-keyboard >> + >> + reg: >> + const: 0x15 >> + >> + interrupts: >> + maxItems: 1 >> + >> + linux,fn-keymap: > > This should be handled in a common way. Not sure if there's anything > existing for alternate key maps. Child nodes of alternate maps would > scale better than new property name for every alternate map. Or you > could make linux,keymap contain multiple maps (e.g. 2x XxY entries) matrix-keymap.yaml proposes doing exactly what I do here: Some users of this binding might choose to specify secondary keymaps for cases where there is a modifier key such as a Fn key. Proposed names for said properties are "linux,fn-keymap" or with another descriptive word for the modifier other from "Fn". The only other reference to linux,fn-keymap is in the nvidia,tegra20-kbc binding, even though that driver doesn't use this property. Instead, what the tegra-kbc.c code does is double the number of rows to include the Fn layer: if (kbc->keymap_data && kbc->use_fn_map) keymap_rows *= 2; (except that use_fn_map is set nowhere, so this is dead code). Using extra rows for the Fn layer seems a bit magic to me, but if it is documented, I suppose it is fine. So there is not much in the way of prior art. > Or if the map doesn't change, just put it in the driver. The keys are removable, so technically it can change, but maybe that's better handled by userspace than in the DTB? On the other hand, from Dmitry's comment, it sounds like he would prefer leaving the map out of the driver. Regards, Samuel >> + $ref: /schemas/input/matrix-keymap.yaml#/properties/linux,keymap > > Referencing individual properties should be avoided. > >> + description: keymap used when the Fn key is pressed >> + >> + wakeup-source: true >> + >> + i2c-bus: >> + $ref: /schemas/i2c/i2c-controller.yaml# >> + >> +dependencies: >> + linux,fn-keymap: [ 'keypad,num-columns', 'keypad,num-rows' ] >> + linux,keymap: [ 'keypad,num-columns', 'keypad,num-rows' ] >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + >> +unevaluatedProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/input/input.h> >> + #include <dt-bindings/interrupt-controller/irq.h> >> + >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + keyboard@15 { >> + compatible = "pine64,pinephone-keyboard"; >> + reg = <0x15>; >> + interrupt-parent = <&r_pio>; >> + interrupts = <0 12 IRQ_TYPE_EDGE_FALLING>; /* PL12 */ >> + keypad,num-rows = <6>; >> + keypad,num-columns = <12>; >> + linux,fn-keymap = <MATRIX_KEY(0, 0, KEY_FN_ESC) >> + MATRIX_KEY(0, 1, KEY_F1) >> + MATRIX_KEY(0, 2, KEY_F2) >> + /* ... */ >> + MATRIX_KEY(5, 2, KEY_FN) >> + MATRIX_KEY(5, 3, KEY_LEFTALT) >> + MATRIX_KEY(5, 5, KEY_RIGHTALT)>; >> + linux,keymap = <MATRIX_KEY(0, 0, KEY_ESC) >> + MATRIX_KEY(0, 1, KEY_1) >> + MATRIX_KEY(0, 2, KEY_2) >> + /* ... */ >> + MATRIX_KEY(5, 2, KEY_FN) >> + MATRIX_KEY(5, 3, KEY_LEFTALT) >> + MATRIX_KEY(5, 5, KEY_RIGHTALT)>; >> + >> + i2c-bus { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + charger@75 { >> + reg = <0x75>; >> + }; >> + }; >> + }; >> + }; >> -- >> 2.33.1 >> >> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/5] Input: pinephone-keyboard - Add PinePhone keyboard driver 2022-01-29 23:00 [PATCH 0/5] Pine64 PinePhone keyboard support Samuel Holland 2022-01-29 23:00 ` [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard binding Samuel Holland @ 2022-01-29 23:00 ` Samuel Holland 2022-02-02 8:07 ` Ondřej Jirman 2022-02-02 11:48 ` Ondřej Jirman 2022-01-29 23:00 ` [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap Samuel Holland ` (2 subsequent siblings) 4 siblings, 2 replies; 18+ messages in thread From: Samuel Holland @ 2022-01-29 23:00 UTC (permalink / raw) To: Dmitry Torokhov, linux-input Cc: linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman, Samuel Holland The official Pine64 PinePhone keyboard case contains a matrix keypad and a MCU which runs a libre firmware. Add support for its I2C interface. Signed-off-by: Samuel Holland <samuel@sholland.org> --- MAINTAINERS | 6 + drivers/input/keyboard/Kconfig | 10 + drivers/input/keyboard/Makefile | 1 + drivers/input/keyboard/pinephone-keyboard.c | 258 ++++++++++++++++++++ 4 files changed, 275 insertions(+) create mode 100644 drivers/input/keyboard/pinephone-keyboard.c diff --git a/MAINTAINERS b/MAINTAINERS index f41088418aae..85428a707e67 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15320,6 +15320,12 @@ S: Maintained F: Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.yaml F: drivers/iio/chemical/pms7003.c +PINE64 PINEPHONE KEYBOARD DRIVER +M: Samuel Holland <samuel@sholland.org> +S: Supported +F: Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml +F: drivers/input/keyboard/pinephone-keyboard.c + PLDMFW LIBRARY M: Jacob Keller <jacob.e.keller@intel.com> S: Maintained diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig index 0c607da9ee10..f9978da1c916 100644 --- a/drivers/input/keyboard/Kconfig +++ b/drivers/input/keyboard/Kconfig @@ -523,6 +523,16 @@ config KEYBOARD_OPENCORES To compile this driver as a module, choose M here; the module will be called opencores-kbd. +config KEYBOARD_PINEPHONE + tristate "Pine64 PinePhone Keyboard" + depends on I2C + select CRC8 + select INPUT_MATRIXKMAP + help + Say Y here to enable support for the keyboard in the Pine64 PinePhone + keyboard case. This driver supports the FLOSS firmware available at + https://megous.com/git/pinephone-keyboard/ + config KEYBOARD_PXA27x tristate "PXA27x/PXA3xx keypad support" depends on PXA27x || PXA3xx || ARCH_MMP diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile index e3c8648f834e..84da37e3a2f5 100644 --- a/drivers/input/keyboard/Makefile +++ b/drivers/input/keyboard/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_KEYBOARD_NSPIRE) += nspire-keypad.o obj-$(CONFIG_KEYBOARD_OMAP) += omap-keypad.o obj-$(CONFIG_KEYBOARD_OMAP4) += omap4-keypad.o obj-$(CONFIG_KEYBOARD_OPENCORES) += opencores-kbd.o +obj-$(CONFIG_KEYBOARD_PINEPHONE) += pinephone-keyboard.o obj-$(CONFIG_KEYBOARD_PMIC8XXX) += pmic8xxx-keypad.o obj-$(CONFIG_KEYBOARD_PXA27x) += pxa27x_keypad.o obj-$(CONFIG_KEYBOARD_PXA930_ROTARY) += pxa930_rotary.o diff --git a/drivers/input/keyboard/pinephone-keyboard.c b/drivers/input/keyboard/pinephone-keyboard.c new file mode 100644 index 000000000000..9a071753fd91 --- /dev/null +++ b/drivers/input/keyboard/pinephone-keyboard.c @@ -0,0 +1,258 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// Copyright (C) 2021-2022 Samuel Holland <samuel@sholland.org> + +#include <linux/crc8.h> +#include <linux/i2c.h> +#include <linux/input/matrix_keypad.h> +#include <linux/interrupt.h> +#include <linux/module.h> + +#define DRV_NAME "pinephone-keyboard" + +#define PPKB_CRC8_POLYNOMIAL 0x07 + +#define PPKB_DEVICE_ID_HI 0x00 +#define PPKB_DEVICE_ID_HI_VALUE 0x4b +#define PPKB_DEVICE_ID_LO 0x01 +#define PPKB_DEVICE_ID_LO_VALUE 0x42 +#define PPKB_FW_REVISION 0x02 +#define PPKB_FW_FEATURES 0x03 +#define PPKB_MATRIX_SIZE 0x06 +#define PPKB_SCAN_CRC 0x07 +#define PPKB_SCAN_DATA 0x08 +#define PPKB_SYS_CONFIG 0x20 +#define PPKB_SYS_CONFIG_DISABLE_SCAN BIT(0) + +struct pinephone_keyboard { + struct input_dev *input; + unsigned short *fn_keymap; + u8 crc_table[CRC8_TABLE_SIZE]; + u8 row_shift; + u8 rows; + u8 cols; + u8 fn_state; + u8 buf_swap; + u8 buf[]; +}; + +static int ppkb_set_scan(struct i2c_client *client, bool enable) +{ + struct device *dev = &client->dev; + int ret, val; + + ret = i2c_smbus_read_byte_data(client, PPKB_SYS_CONFIG); + if (ret < 0) { + dev_err(dev, "Failed to read config: %d\n", ret); + return ret; + } + + if (enable) + val = ret & ~PPKB_SYS_CONFIG_DISABLE_SCAN; + else + val = ret | PPKB_SYS_CONFIG_DISABLE_SCAN; + ret = i2c_smbus_write_byte_data(client, PPKB_SYS_CONFIG, val); + if (ret) { + dev_err(dev, "Failed to write config: %d\n", ret); + return ret; + } + + return 0; +} + +static void ppkb_update(struct i2c_client *client) +{ + struct pinephone_keyboard *ppkb = i2c_get_clientdata(client); + struct device *dev = &client->dev; + size_t buf_len = ppkb->cols + 1; + u8 *old_buf = ppkb->buf; + u8 *new_buf = ppkb->buf; + unsigned short *keymap; + int col, crc, ret, row; + + if (ppkb->buf_swap) + old_buf += buf_len; + else + new_buf += buf_len; + + ret = i2c_smbus_read_i2c_block_data(client, PPKB_SCAN_CRC, + buf_len, new_buf); + if (ret != buf_len) { + dev_err(dev, "Failed to read scan data: %d\n", ret); + return; + } + + crc = crc8(ppkb->crc_table, new_buf + 1, ppkb->cols, CRC8_INIT_VALUE); + if (crc != new_buf[0]) { + dev_err(dev, "Bad scan data (%02x != %02x)\n", crc, new_buf[0]); + return; + } + + ppkb->buf_swap = !ppkb->buf_swap; + + keymap = ppkb->fn_state ? ppkb->fn_keymap : ppkb->input->keycode; + for (col = 0; col < ppkb->cols; ++col) { + u8 old = *(++old_buf); + u8 new = *(++new_buf); + u8 changed = old ^ new; + + for (row = 0; row < ppkb->rows; ++row) { + int code = MATRIX_SCAN_CODE(row, col, ppkb->row_shift); + int value = new & BIT(row); + + if (!(changed & BIT(row))) + continue; + + dev_dbg(dev, "row %u col %u %sed\n", + row, col, value ? "press" : "releas"); + if (keymap[code] == KEY_FN) { + dev_dbg(dev, "FN is now %sed\n", + value ? "press" : "releas"); + keymap = value ? ppkb->fn_keymap + : ppkb->input->keycode; + ppkb->fn_state = value; + } + input_event(ppkb->input, EV_MSC, MSC_SCAN, code); + input_report_key(ppkb->input, keymap[code], value); + } + } + input_sync(ppkb->input); +} + +static int ppkb_open(struct input_dev *input) +{ + struct i2c_client *client = input_get_drvdata(input); + int ret; + + ret = ppkb_set_scan(client, true); + if (ret) + return ret; + + ppkb_update(client); + + return 0; +} + +static void ppkb_close(struct input_dev *input) +{ + struct i2c_client *client = input_get_drvdata(input); + + ppkb_set_scan(client, false); +} + +static irqreturn_t ppkb_irq_thread(int irq, void *data) +{ + struct i2c_client *client = data; + + ppkb_update(client); + + return IRQ_HANDLED; +} + +static int ppkb_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + unsigned int phys_rows, phys_cols; + unsigned int map_rows, map_cols; + struct pinephone_keyboard *ppkb; + u8 info[PPKB_MATRIX_SIZE + 1]; + int ret; + + ret = i2c_smbus_read_i2c_block_data(client, 0, sizeof(info), info); + if (ret != sizeof(info)) + return dev_err_probe(dev, ret, "Failed to read device ID\n"); + + if (info[PPKB_DEVICE_ID_HI] != PPKB_DEVICE_ID_HI_VALUE || + info[PPKB_DEVICE_ID_LO] != PPKB_DEVICE_ID_LO_VALUE) + return dev_err_probe(dev, -ENODEV, "Unexpected device ID\n"); + + dev_info(dev, "Found keyboard firmware version %d.%d features %#x\n", + info[PPKB_FW_REVISION] >> 4, + info[PPKB_FW_REVISION] & 0xf, + info[PPKB_FW_FEATURES]); + + /* Disable scan by default to save power. */ + ret = ppkb_set_scan(client, false); + if (ret) + return ret; + + ret = matrix_keypad_parse_properties(dev, &map_rows, &map_cols); + if (ret) + return ret; + + phys_rows = info[PPKB_MATRIX_SIZE] & 0xf; + phys_cols = info[PPKB_MATRIX_SIZE] >> 4; + if (map_rows != phys_rows || map_cols != phys_cols) + return dev_err_probe(dev, -EINVAL, + "Keyboard size is %ux%u, but keymap is %ux%u\n", + phys_rows, phys_cols, map_rows, map_cols); + + /* Allocate two buffers, and include space for reading the CRC. */ + ppkb = devm_kzalloc(dev, struct_size(ppkb, buf, 2 * (phys_cols + 1)), GFP_KERNEL); + if (!ppkb) + return -ENOMEM; + + i2c_set_clientdata(client, ppkb); + + crc8_populate_msb(ppkb->crc_table, PPKB_CRC8_POLYNOMIAL); + ppkb->row_shift = get_count_order(map_cols); + ppkb->rows = map_rows; + ppkb->cols = map_cols; + + ppkb->input = devm_input_allocate_device(dev); + if (!ppkb->input) + return -ENOMEM; + + input_set_drvdata(ppkb->input, client); + + ppkb->input->name = "PinePhone Keyboard"; + ppkb->input->phys = DRV_NAME "/input0"; + ppkb->input->id.bustype = BUS_I2C; + ppkb->input->open = ppkb_open; + ppkb->input->close = ppkb_close; + + __set_bit(EV_MSC, ppkb->input->evbit); + __set_bit(EV_REP, ppkb->input->evbit); + + ret = matrix_keypad_build_keymap(NULL, "linux,fn-keymap", + map_rows, map_cols, NULL, ppkb->input); + if (ret) + return dev_err_probe(dev, ret, "Failed to build FN keymap\n"); + + ppkb->fn_keymap = ppkb->input->keycode; + + ret = matrix_keypad_build_keymap(NULL, "linux,keymap", + map_rows, map_cols, NULL, ppkb->input); + if (ret) + return dev_err_probe(dev, ret, "Failed to build keymap\n"); + + ret = input_register_device(ppkb->input); + if (ret) + return dev_err_probe(dev, ret, "Failed to register input\n"); + + ret = devm_request_threaded_irq(dev, client->irq, NULL, ppkb_irq_thread, + IRQF_ONESHOT, client->name, client); + if (ret) + return dev_err_probe(dev, ret, "Failed to request IRQ\n"); + + return 0; +} + +static const struct of_device_id ppkb_of_match[] = { + { .compatible = "pine64,pinephone-keyboard" }, + { } +}; +MODULE_DEVICE_TABLE(of, ppkb_of_match); + +static struct i2c_driver ppkb_driver = { + .probe_new = ppkb_probe, + .driver = { + .name = DRV_NAME, + .of_match_table = ppkb_of_match, + }, +}; +module_i2c_driver(ppkb_driver); + +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>"); +MODULE_DESCRIPTION("Pine64 PinePhone keyboard driver"); +MODULE_LICENSE("GPL"); -- 2.33.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] Input: pinephone-keyboard - Add PinePhone keyboard driver 2022-01-29 23:00 ` [PATCH 2/5] Input: pinephone-keyboard - Add PinePhone keyboard driver Samuel Holland @ 2022-02-02 8:07 ` Ondřej Jirman 2022-02-02 11:48 ` Ondřej Jirman 1 sibling, 0 replies; 18+ messages in thread From: Ondřej Jirman @ 2022-02-02 8:07 UTC (permalink / raw) To: Samuel Holland Cc: Dmitry Torokhov, linux-input, linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang Hello Samuel, On Sat, Jan 29, 2022 at 05:00:39PM -0600, Samuel Holland wrote: > The official Pine64 PinePhone keyboard case contains a matrix keypad and > a MCU which runs a libre firmware. Add support for its I2C interface. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > [...] > + > + ppkb->buf_swap = !ppkb->buf_swap; > + > + keymap = ppkb->fn_state ? ppkb->fn_keymap : ppkb->input->keycode; > + for (col = 0; col < ppkb->cols; ++col) { > + u8 old = *(++old_buf); > + u8 new = *(++new_buf); > + u8 changed = old ^ new; > + > + for (row = 0; row < ppkb->rows; ++row) { > + int code = MATRIX_SCAN_CODE(row, col, ppkb->row_shift); > + int value = new & BIT(row); > + > + if (!(changed & BIT(row))) > + continue; > + > + dev_dbg(dev, "row %u col %u %sed\n", > + row, col, value ? "press" : "releas"); > + if (keymap[code] == KEY_FN) { > + dev_dbg(dev, "FN is now %sed\n", > + value ? "press" : "releas"); > + keymap = value ? ppkb->fn_keymap > + : ppkb->input->keycode; > + ppkb->fn_state = value; > + } > + input_event(ppkb->input, EV_MSC, MSC_SCAN, code); > + input_report_key(ppkb->input, keymap[code], value); I think there's a logic issue here with the Fn layer. Consider what happens when you press Fn press F1 and then release Fn and release F1. In that case input_report_key will report press of F1 (in fn layer) but release of '1' which is not in Fn layer, because Fn layer was de-activated before releasing the modified key. From the PoV of the user, this will probably lead to auto-repeat of F1 and spurious '1' release without preceding press event. So the userspace sees F1 as stuck and auto-repeats it. kind regards, o. > + } > + } > + input_sync(ppkb->input); > +} > + ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] Input: pinephone-keyboard - Add PinePhone keyboard driver 2022-01-29 23:00 ` [PATCH 2/5] Input: pinephone-keyboard - Add PinePhone keyboard driver Samuel Holland 2022-02-02 8:07 ` Ondřej Jirman @ 2022-02-02 11:48 ` Ondřej Jirman 1 sibling, 0 replies; 18+ messages in thread From: Ondřej Jirman @ 2022-02-02 11:48 UTC (permalink / raw) To: Samuel Holland Cc: Dmitry Torokhov, linux-input, linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang Hello again Samuel, On Sat, Jan 29, 2022 at 05:00:39PM -0600, Samuel Holland wrote: > The official Pine64 PinePhone keyboard case contains a matrix keypad and > a MCU which runs a libre firmware. Add support for its I2C interface. I had one bug report from a user that reminded me of something that's missing in the driver. Please add a regulator support for power supply for the keyboard. On Pinephone the keyboard needs USB-5V regulator to be enabled, because final version of the keyboard is supplied from the VBAT POGO pin and not internally from kb battery, as on the prototypes. If this regulator is not enabled the keyboard will be supplied from battery - voltage drop on a boost regulator diode, which causes very frequent brownouts of the keyboard MCU. Users don't notice, becuse MCU recovers quickly and only a key or two is missed every once in a while, but it should be fixed. And this problem probably only shows up when the phone battery is either quite discharged, or random load spikes push its voltage bellow what's necessary for the MCU to run, temporarily. kind regards, o. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap 2022-01-29 23:00 [PATCH 0/5] Pine64 PinePhone keyboard support Samuel Holland 2022-01-29 23:00 ` [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard binding Samuel Holland 2022-01-29 23:00 ` [PATCH 2/5] Input: pinephone-keyboard - Add PinePhone keyboard driver Samuel Holland @ 2022-01-29 23:00 ` Samuel Holland 2022-01-31 19:45 ` Dmitry Torokhov 2022-04-12 10:20 ` Jarrah 2022-01-29 23:00 ` [PATCH 4/5] Input: pinephone-keyboard - Support the proxied I2C bus Samuel Holland 2022-01-29 23:00 ` [PATCH 5/5] [DO NOT MERGE] arm64: dts: allwinner: pinephone: Add keyboard Samuel Holland 4 siblings, 2 replies; 18+ messages in thread From: Samuel Holland @ 2022-01-29 23:00 UTC (permalink / raw) To: Dmitry Torokhov, linux-input Cc: linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman, Samuel Holland The PinePhone keyboard comes with removable keys, but there is a default layout labeled from the factory. Use this keymap if none is provided in the devicetree. Suggested-by: Ondrej Jirman <x@xff.cz> Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/input/keyboard/pinephone-keyboard.c | 128 +++++++++++++++++++- 1 file changed, 123 insertions(+), 5 deletions(-) diff --git a/drivers/input/keyboard/pinephone-keyboard.c b/drivers/input/keyboard/pinephone-keyboard.c index 9a071753fd91..8065bc3e101a 100644 --- a/drivers/input/keyboard/pinephone-keyboard.c +++ b/drivers/input/keyboard/pinephone-keyboard.c @@ -24,6 +24,113 @@ #define PPKB_SYS_CONFIG 0x20 #define PPKB_SYS_CONFIG_DISABLE_SCAN BIT(0) +#define PPKB_DEFAULT_KEYMAP_ROWS 6 +#define PPKB_DEFAULT_KEYMAP_COLS 12 + +static const uint32_t ppkb_default_fn_keymap[] = { + KEY(0, 0, KEY_FN_ESC), + KEY(0, 1, KEY_F1), + KEY(0, 2, KEY_F2), + KEY(0, 3, KEY_F3), + KEY(0, 4, KEY_F4), + KEY(0, 5, KEY_F5), + KEY(0, 6, KEY_F6), + KEY(0, 7, KEY_F7), + KEY(0, 8, KEY_F8), + KEY(0, 9, KEY_F9), + KEY(0, 10, KEY_F10), + KEY(0, 11, KEY_DELETE), + + KEY(2, 0, KEY_SYSRQ), + KEY(2, 10, KEY_INSERT), + + KEY(3, 0, KEY_LEFTSHIFT), + KEY(3, 8, KEY_HOME), + KEY(3, 9, KEY_UP), + KEY(3, 10, KEY_END), + + KEY(4, 1, KEY_LEFTCTRL), + KEY(4, 6, KEY_LEFT), + KEY(4, 8, KEY_RIGHT), + KEY(4, 9, KEY_DOWN), + + KEY(5, 2, KEY_FN), + KEY(5, 3, KEY_LEFTALT), + KEY(5, 5, KEY_RIGHTALT), +}; + +static const struct matrix_keymap_data ppkb_default_fn_keymap_data = { + .keymap = ppkb_default_fn_keymap, + .keymap_size = ARRAY_SIZE(ppkb_default_fn_keymap), +}; + +static const uint32_t ppkb_default_keymap[] = { + KEY(0, 0, KEY_ESC), + KEY(0, 1, KEY_1), + KEY(0, 2, KEY_2), + KEY(0, 3, KEY_3), + KEY(0, 4, KEY_4), + KEY(0, 5, KEY_5), + KEY(0, 6, KEY_6), + KEY(0, 7, KEY_7), + KEY(0, 8, KEY_8), + KEY(0, 9, KEY_9), + KEY(0, 10, KEY_0), + KEY(0, 11, KEY_BACKSPACE), + + KEY(1, 0, KEY_TAB), + KEY(1, 1, KEY_Q), + KEY(1, 2, KEY_W), + KEY(1, 3, KEY_E), + KEY(1, 4, KEY_R), + KEY(1, 5, KEY_T), + KEY(1, 6, KEY_Y), + KEY(1, 7, KEY_U), + KEY(1, 8, KEY_I), + KEY(1, 9, KEY_O), + KEY(1, 10, KEY_P), + KEY(1, 11, KEY_ENTER), + + KEY(2, 0, KEY_LEFTMETA), + KEY(2, 1, KEY_A), + KEY(2, 2, KEY_S), + KEY(2, 3, KEY_D), + KEY(2, 4, KEY_F), + KEY(2, 5, KEY_G), + KEY(2, 6, KEY_H), + KEY(2, 7, KEY_J), + KEY(2, 8, KEY_K), + KEY(2, 9, KEY_L), + KEY(2, 10, KEY_SEMICOLON), + + KEY(3, 0, KEY_LEFTSHIFT), + KEY(3, 1, KEY_Z), + KEY(3, 2, KEY_X), + KEY(3, 3, KEY_C), + KEY(3, 4, KEY_V), + KEY(3, 5, KEY_B), + KEY(3, 6, KEY_N), + KEY(3, 7, KEY_M), + KEY(3, 8, KEY_COMMA), + KEY(3, 9, KEY_DOT), + KEY(3, 10, KEY_SLASH), + + KEY(4, 1, KEY_LEFTCTRL), + KEY(4, 4, KEY_SPACE), + KEY(4, 6, KEY_APOSTROPHE), + KEY(4, 8, KEY_RIGHTBRACE), + KEY(4, 9, KEY_LEFTBRACE), + + KEY(5, 2, KEY_FN), + KEY(5, 3, KEY_LEFTALT), + KEY(5, 5, KEY_RIGHTALT), +}; + +static const struct matrix_keymap_data ppkb_default_keymap_data = { + .keymap = ppkb_default_keymap, + .keymap_size = ARRAY_SIZE(ppkb_default_keymap), +}; + struct pinephone_keyboard { struct input_dev *input; unsigned short *fn_keymap; @@ -151,6 +258,8 @@ static irqreturn_t ppkb_irq_thread(int irq, void *data) static int ppkb_probe(struct i2c_client *client) { + const struct matrix_keymap_data *fn_keymap_data = &ppkb_default_fn_keymap_data; + const struct matrix_keymap_data *keymap_data = &ppkb_default_keymap_data; struct device *dev = &client->dev; unsigned int phys_rows, phys_cols; unsigned int map_rows, map_cols; @@ -176,9 +285,18 @@ static int ppkb_probe(struct i2c_client *client) if (ret) return ret; - ret = matrix_keypad_parse_properties(dev, &map_rows, &map_cols); - if (ret) - return ret; + /* Allow the devicetree to override the default keymaps. */ + if (of_property_read_bool(dev->of_node, "linux,fn-keymap") || + of_property_read_bool(dev->of_node, "linux,keymap")) { + ret = matrix_keypad_parse_properties(dev, &map_rows, &map_cols); + if (ret) + return ret; + + fn_keymap_data = keymap_data = NULL; + } else { + map_rows = PPKB_DEFAULT_KEYMAP_ROWS; + map_cols = PPKB_DEFAULT_KEYMAP_COLS; + } phys_rows = info[PPKB_MATRIX_SIZE] & 0xf; phys_cols = info[PPKB_MATRIX_SIZE] >> 4; @@ -214,14 +332,14 @@ static int ppkb_probe(struct i2c_client *client) __set_bit(EV_MSC, ppkb->input->evbit); __set_bit(EV_REP, ppkb->input->evbit); - ret = matrix_keypad_build_keymap(NULL, "linux,fn-keymap", + ret = matrix_keypad_build_keymap(fn_keymap_data, "linux,fn-keymap", map_rows, map_cols, NULL, ppkb->input); if (ret) return dev_err_probe(dev, ret, "Failed to build FN keymap\n"); ppkb->fn_keymap = ppkb->input->keycode; - ret = matrix_keypad_build_keymap(NULL, "linux,keymap", + ret = matrix_keypad_build_keymap(keymap_data, "linux,keymap", map_rows, map_cols, NULL, ppkb->input); if (ret) return dev_err_probe(dev, ret, "Failed to build keymap\n"); -- 2.33.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap 2022-01-29 23:00 ` [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap Samuel Holland @ 2022-01-31 19:45 ` Dmitry Torokhov 2022-02-02 4:58 ` Samuel Holland 2022-04-12 10:20 ` Jarrah 1 sibling, 1 reply; 18+ messages in thread From: Dmitry Torokhov @ 2022-01-31 19:45 UTC (permalink / raw) To: Samuel Holland Cc: linux-input, linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman Hi Samuel, On Sat, Jan 29, 2022 at 05:00:40PM -0600, Samuel Holland wrote: > The PinePhone keyboard comes with removable keys, but there is a default > layout labeled from the factory. Use this keymap if none is provided in > the devicetree. Why can't we require to have it in device tree? Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap 2022-01-31 19:45 ` Dmitry Torokhov @ 2022-02-02 4:58 ` Samuel Holland 2022-05-30 7:05 ` Pavel Machek 0 siblings, 1 reply; 18+ messages in thread From: Samuel Holland @ 2022-02-02 4:58 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman Hi, On 1/31/22 1:45 PM, Dmitry Torokhov wrote: > Hi Samuel, > > On Sat, Jan 29, 2022 at 05:00:40PM -0600, Samuel Holland wrote: >> The PinePhone keyboard comes with removable keys, but there is a default >> layout labeled from the factory. Use this keymap if none is provided in >> the devicetree. > > Why can't we require to have it in device tree? We can. I am okay with dropping this patch and making the properties required if that is preferred. The keyboard is supported on at least four device trees (three revisions of PinePhone, plus the PinePhone Pro), so moving the default keymap to the driver avoids duplicating that block of data in each device tree/overlay. Regards, Samuel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap 2022-02-02 4:58 ` Samuel Holland @ 2022-05-30 7:05 ` Pavel Machek 0 siblings, 0 replies; 18+ messages in thread From: Pavel Machek @ 2022-05-30 7:05 UTC (permalink / raw) To: Samuel Holland Cc: Dmitry Torokhov, linux-input, linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman Hi! > > On Sat, Jan 29, 2022 at 05:00:40PM -0600, Samuel Holland wrote: > >> The PinePhone keyboard comes with removable keys, but there is a default > >> layout labeled from the factory. Use this keymap if none is provided in > >> the devicetree. > > > > Why can't we require to have it in device tree? > > We can. I am okay with dropping this patch and making the properties required if > that is preferred. > > The keyboard is supported on at least four device trees (three revisions of > PinePhone, plus the PinePhone Pro), so moving the default keymap to the driver > avoids duplicating that block of data in each device tree/overlay. #include is supported on dts, so there's no need to duplicate the keymaps, etc. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap 2022-01-29 23:00 ` [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap Samuel Holland 2022-01-31 19:45 ` Dmitry Torokhov @ 2022-04-12 10:20 ` Jarrah 2022-04-12 11:34 ` Ondřej Jirman 1 sibling, 1 reply; 18+ messages in thread From: Jarrah @ 2022-04-12 10:20 UTC (permalink / raw) To: Samuel Holland, Dmitry Torokhov, linux-input Cc: linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman On 1/30/22 10:00, Samuel Holland wrote: > + > +static const uint32_t ppkb_default_fn_keymap[] = { > + KEY(0, 0, KEY_FN_ESC), > + KEY(0, 1, KEY_F1), > + KEY(0, 2, KEY_F2), > + KEY(0, 3, KEY_F3), > + KEY(0, 4, KEY_F4), > + KEY(0, 5, KEY_F5), > + KEY(0, 6, KEY_F6), > + KEY(0, 7, KEY_F7), > + KEY(0, 8, KEY_F8), > + KEY(0, 9, KEY_F9), > + KEY(0, 10, KEY_F10), > + KEY(0, 11, KEY_DELETE), > + > + KEY(2, 0, KEY_SYSRQ), > + KEY(2, 10, KEY_INSERT), > + The driver currently being patched into most distros supporting the keyboard exposes the symbols printed on the keyboard rather than the F* keys on the function layer. While I agree than exposing function keys on the Fn layer is more logical, in practice running this patch for a day I've found it's far more useful to have quick access to the standard set of symbols (such as | and -) than to have the function keys. Would it be possible to either set the default back to symbols or expose another layer (potentially under the "pine" key)? An alternative solution proposed on the Mobian issue for this was to add a module option, allowing these to be switched at runtime rather than compile time. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap 2022-04-12 10:20 ` Jarrah @ 2022-04-12 11:34 ` Ondřej Jirman 0 siblings, 0 replies; 18+ messages in thread From: Ondřej Jirman @ 2022-04-12 11:34 UTC (permalink / raw) To: Jarrah Cc: Samuel Holland, Dmitry Torokhov, linux-input, linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang On Tue, Apr 12, 2022 at 08:20:24PM +1000, Jarrah wrote: > > On 1/30/22 10:00, Samuel Holland wrote: > > + > > +static const uint32_t ppkb_default_fn_keymap[] = { > > + KEY(0, 0, KEY_FN_ESC), > > + KEY(0, 1, KEY_F1), > > + KEY(0, 2, KEY_F2), > > + KEY(0, 3, KEY_F3), > > + KEY(0, 4, KEY_F4), > > + KEY(0, 5, KEY_F5), > > + KEY(0, 6, KEY_F6), > > + KEY(0, 7, KEY_F7), > > + KEY(0, 8, KEY_F8), > > + KEY(0, 9, KEY_F9), > > + KEY(0, 10, KEY_F10), > > + KEY(0, 11, KEY_DELETE), > > + > > + KEY(2, 0, KEY_SYSRQ), > > + KEY(2, 10, KEY_INSERT), > > + > > > The driver currently being patched into most distros supporting the keyboard > exposes the symbols printed on the keyboard rather than the F* keys on the > function layer. While I agree than exposing function keys on the Fn layer is > more logical, in practice running this patch for a day I've found it's far > more useful to have quick access to the standard set of symbols (such as | > and -) than to have the function keys. > > Would it be possible to either set the default back to symbols or expose > another layer (potentially under the "pine" key)? An alternative solution > proposed on the Mobian issue for this was to add a module option, allowing > these to be switched at runtime rather than compile time. You will not get access to all the symbols anyway, this way. You should solve this via xkb and kernel keymaps (man keymaps(5)). Normal users should not be modifying basic keymap in DT or the driver anyway. kind regards, o. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/5] Input: pinephone-keyboard - Support the proxied I2C bus 2022-01-29 23:00 [PATCH 0/5] Pine64 PinePhone keyboard support Samuel Holland ` (2 preceding siblings ...) 2022-01-29 23:00 ` [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap Samuel Holland @ 2022-01-29 23:00 ` Samuel Holland 2022-01-30 2:05 ` Ondřej Jirman 2022-01-29 23:00 ` [PATCH 5/5] [DO NOT MERGE] arm64: dts: allwinner: pinephone: Add keyboard Samuel Holland 4 siblings, 1 reply; 18+ messages in thread From: Samuel Holland @ 2022-01-29 23:00 UTC (permalink / raw) To: Dmitry Torokhov, linux-input Cc: linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman, Samuel Holland The PinePhone keyboard case contains a battery managed by an integrated power bank IC. The power bank IC communicates over I2C, and the keyboard MCU firmware provides an interface to read and write its registers. Let's use this interface to implement a SMBus adapter, so we can reuse the driver for the power bank IC. Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/input/keyboard/pinephone-keyboard.c | 73 +++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/drivers/input/keyboard/pinephone-keyboard.c b/drivers/input/keyboard/pinephone-keyboard.c index 8065bc3e101a..7d2e16e588a0 100644 --- a/drivers/input/keyboard/pinephone-keyboard.c +++ b/drivers/input/keyboard/pinephone-keyboard.c @@ -3,6 +3,7 @@ // Copyright (C) 2021-2022 Samuel Holland <samuel@sholland.org> #include <linux/crc8.h> +#include <linux/delay.h> #include <linux/i2c.h> #include <linux/input/matrix_keypad.h> #include <linux/interrupt.h> @@ -23,6 +24,11 @@ #define PPKB_SCAN_DATA 0x08 #define PPKB_SYS_CONFIG 0x20 #define PPKB_SYS_CONFIG_DISABLE_SCAN BIT(0) +#define PPKB_SYS_SMBUS_COMMAND 0x21 +#define PPKB_SYS_SMBUS_DATA 0x22 +#define PPKB_SYS_COMMAND 0x23 +#define PPKB_SYS_COMMAND_SMBUS_READ 0x91 +#define PPKB_SYS_COMMAND_SMBUS_WRITE 0xa1 #define PPKB_DEFAULT_KEYMAP_ROWS 6 #define PPKB_DEFAULT_KEYMAP_COLS 12 @@ -132,6 +138,7 @@ static const struct matrix_keymap_data ppkb_default_keymap_data = { }; struct pinephone_keyboard { + struct i2c_adapter adapter; struct input_dev *input; unsigned short *fn_keymap; u8 crc_table[CRC8_TABLE_SIZE]; @@ -143,6 +150,57 @@ struct pinephone_keyboard { u8 buf[]; }; +static int ppkb_adap_smbus_xfer(struct i2c_adapter *adap, u16 addr, + unsigned short flags, char read_write, + u8 command, int size, + union i2c_smbus_data *data) +{ + struct i2c_client *client = adap->algo_data; + u8 buf[3]; + int ret; + + buf[0] = command; + buf[1] = data->byte; + buf[2] = read_write == I2C_SMBUS_READ ? PPKB_SYS_COMMAND_SMBUS_READ + : PPKB_SYS_COMMAND_SMBUS_WRITE; + + ret = i2c_smbus_write_i2c_block_data(client, PPKB_SYS_SMBUS_COMMAND, + sizeof(buf), buf); + if (ret) + return ret; + + /* Read back the command status until it passes or fails. */ + do { + usleep_range(300, 500); + ret = i2c_smbus_read_byte_data(client, PPKB_SYS_COMMAND); + } while (ret == buf[2]); + if (ret < 0) + return ret; + /* Commands return 0x00 on success and 0xff on failure. */ + if (ret) + return -EIO; + + if (read_write == I2C_SMBUS_READ) { + ret = i2c_smbus_read_byte_data(client, PPKB_SYS_SMBUS_DATA); + if (ret < 0) + return ret; + + data->byte = ret; + } + + return 0; +} + +static u32 ppkg_adap_functionality(struct i2c_adapter *adap) +{ + return I2C_FUNC_SMBUS_BYTE_DATA; +} + +static const struct i2c_algorithm ppkb_adap_algo = { + .smbus_xfer = ppkb_adap_smbus_xfer, + .functionality = ppkg_adap_functionality, +}; + static int ppkb_set_scan(struct i2c_client *client, bool enable) { struct device *dev = &client->dev; @@ -265,6 +323,7 @@ static int ppkb_probe(struct i2c_client *client) unsigned int map_rows, map_cols; struct pinephone_keyboard *ppkb; u8 info[PPKB_MATRIX_SIZE + 1]; + struct device_node *i2c_bus; int ret; ret = i2c_smbus_read_i2c_block_data(client, 0, sizeof(info), info); @@ -312,6 +371,20 @@ static int ppkb_probe(struct i2c_client *client) i2c_set_clientdata(client, ppkb); + i2c_bus = of_get_child_by_name(dev->of_node, "i2c-bus"); + if (i2c_bus) { + ppkb->adapter.owner = THIS_MODULE; + ppkb->adapter.algo = &ppkb_adap_algo; + ppkb->adapter.algo_data = client; + ppkb->adapter.dev.parent = dev; + ppkb->adapter.dev.of_node = i2c_bus; + strscpy(ppkb->adapter.name, DRV_NAME, sizeof(ppkb->adapter.name)); + + ret = devm_i2c_add_adapter(dev, &ppkb->adapter); + if (ret) + return dev_err_probe(dev, ret, "Failed to add I2C adapter\n"); + } + crc8_populate_msb(ppkb->crc_table, PPKB_CRC8_POLYNOMIAL); ppkb->row_shift = get_count_order(map_cols); ppkb->rows = map_rows; -- 2.33.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] Input: pinephone-keyboard - Support the proxied I2C bus 2022-01-29 23:00 ` [PATCH 4/5] Input: pinephone-keyboard - Support the proxied I2C bus Samuel Holland @ 2022-01-30 2:05 ` Ondřej Jirman 2022-01-30 2:43 ` Samuel Holland 0 siblings, 1 reply; 18+ messages in thread From: Ondřej Jirman @ 2022-01-30 2:05 UTC (permalink / raw) To: Samuel Holland Cc: Dmitry Torokhov, linux-input, linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang Hello Samuel, On Sat, Jan 29, 2022 at 05:00:41PM -0600, Samuel Holland wrote: > The PinePhone keyboard case contains a battery managed by an integrated > power bank IC. The power bank IC communicates over I2C, and the keyboard > MCU firmware provides an interface to read and write its registers. > Let's use this interface to implement a SMBus adapter, so we can reuse > the driver for the power bank IC. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > drivers/input/keyboard/pinephone-keyboard.c | 73 +++++++++++++++++++++ > 1 file changed, 73 insertions(+) > > diff --git a/drivers/input/keyboard/pinephone-keyboard.c b/drivers/input/keyboard/pinephone-keyboard.c > index 8065bc3e101a..7d2e16e588a0 100644 > --- a/drivers/input/keyboard/pinephone-keyboard.c > +++ b/drivers/input/keyboard/pinephone-keyboard.c > @@ -3,6 +3,7 @@ > // Copyright (C) 2021-2022 Samuel Holland <samuel@sholland.org> > > #include <linux/crc8.h> > +#include <linux/delay.h> > #include <linux/i2c.h> > #include <linux/input/matrix_keypad.h> > #include <linux/interrupt.h> > @@ -23,6 +24,11 @@ > #define PPKB_SCAN_DATA 0x08 > #define PPKB_SYS_CONFIG 0x20 > #define PPKB_SYS_CONFIG_DISABLE_SCAN BIT(0) > +#define PPKB_SYS_SMBUS_COMMAND 0x21 > +#define PPKB_SYS_SMBUS_DATA 0x22 > +#define PPKB_SYS_COMMAND 0x23 > +#define PPKB_SYS_COMMAND_SMBUS_READ 0x91 > +#define PPKB_SYS_COMMAND_SMBUS_WRITE 0xa1 > > #define PPKB_DEFAULT_KEYMAP_ROWS 6 > #define PPKB_DEFAULT_KEYMAP_COLS 12 > @@ -132,6 +138,7 @@ static const struct matrix_keymap_data ppkb_default_keymap_data = { > }; > > struct pinephone_keyboard { > + struct i2c_adapter adapter; > struct input_dev *input; > unsigned short *fn_keymap; > u8 crc_table[CRC8_TABLE_SIZE]; > @@ -143,6 +150,57 @@ struct pinephone_keyboard { > u8 buf[]; > }; > > +static int ppkb_adap_smbus_xfer(struct i2c_adapter *adap, u16 addr, > + unsigned short flags, char read_write, > + u8 command, int size, > + union i2c_smbus_data *data) > +{ > + struct i2c_client *client = adap->algo_data; > + u8 buf[3]; > + int ret; > + > + buf[0] = command; > + buf[1] = data->byte; > + buf[2] = read_write == I2C_SMBUS_READ ? PPKB_SYS_COMMAND_SMBUS_READ > + : PPKB_SYS_COMMAND_SMBUS_WRITE; > + > + ret = i2c_smbus_write_i2c_block_data(client, PPKB_SYS_SMBUS_COMMAND, > + sizeof(buf), buf); > + if (ret) > + return ret; [1] > + /* Read back the command status until it passes or fails. */ > + do { > + usleep_range(300, 500); > + ret = i2c_smbus_read_byte_data(client, PPKB_SYS_COMMAND); > + } while (ret == buf[2]); > + if (ret < 0) > + return ret; > + /* Commands return 0x00 on success and 0xff on failure. */ > + if (ret) > + return -EIO; > + > + if (read_write == I2C_SMBUS_READ) { > + ret = i2c_smbus_read_byte_data(client, PPKB_SYS_SMBUS_DATA); > + if (ret < 0) > + return ret; Please use a single read transfer to get both command result and data. There will be less risk that some userspace app will issue another command in between command status being read as 0 and data byte being read. Otherwise if you use this in some read/modify/write operation, you may write unexpected value to PMIC. I2C register layout is designed to make this as optimal as possible in a single I2C transaction, so you only need 3 bytes to start command and 2 bytes to read the result and data, both in a single xfer. There's very high likelihood the command will complete in those 300 - 500 us anyway, because the timing is predictable. If this delay is set right, it's almost guaranteed, only two xfers will be necessary to run the command and get the result+ status. And if possible, it would be best if the bus was somehow made busy for other users, until the whole comand/result sequence completes, to eliminate the possibility of another command being issued by other bus users around [1]. Thank you and kind regards, o. > + data->byte = ret; > + } > + > + return 0; > +} > + > +static u32 ppkg_adap_functionality(struct i2c_adapter *adap) > +{ > + return I2C_FUNC_SMBUS_BYTE_DATA; > +} > + > +static const struct i2c_algorithm ppkb_adap_algo = { > + .smbus_xfer = ppkb_adap_smbus_xfer, > + .functionality = ppkg_adap_functionality, > +}; > + > static int ppkb_set_scan(struct i2c_client *client, bool enable) > { > struct device *dev = &client->dev; > @@ -265,6 +323,7 @@ static int ppkb_probe(struct i2c_client *client) > unsigned int map_rows, map_cols; > struct pinephone_keyboard *ppkb; > u8 info[PPKB_MATRIX_SIZE + 1]; > + struct device_node *i2c_bus; > int ret; > > ret = i2c_smbus_read_i2c_block_data(client, 0, sizeof(info), info); > @@ -312,6 +371,20 @@ static int ppkb_probe(struct i2c_client *client) > > i2c_set_clientdata(client, ppkb); > > + i2c_bus = of_get_child_by_name(dev->of_node, "i2c-bus"); > + if (i2c_bus) { > + ppkb->adapter.owner = THIS_MODULE; > + ppkb->adapter.algo = &ppkb_adap_algo; > + ppkb->adapter.algo_data = client; > + ppkb->adapter.dev.parent = dev; > + ppkb->adapter.dev.of_node = i2c_bus; > + strscpy(ppkb->adapter.name, DRV_NAME, sizeof(ppkb->adapter.name)); > + > + ret = devm_i2c_add_adapter(dev, &ppkb->adapter); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to add I2C adapter\n"); > + } > + > crc8_populate_msb(ppkb->crc_table, PPKB_CRC8_POLYNOMIAL); > ppkb->row_shift = get_count_order(map_cols); > ppkb->rows = map_rows; > -- > 2.33.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] Input: pinephone-keyboard - Support the proxied I2C bus 2022-01-30 2:05 ` Ondřej Jirman @ 2022-01-30 2:43 ` Samuel Holland 2022-01-30 3:00 ` Ondřej Jirman 0 siblings, 1 reply; 18+ messages in thread From: Samuel Holland @ 2022-01-30 2:43 UTC (permalink / raw) To: Ondřej Jirman, Dmitry Torokhov, linux-input, linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang On 1/29/22 8:05 PM, Ondřej Jirman wrote: > Hello Samuel, > > On Sat, Jan 29, 2022 at 05:00:41PM -0600, Samuel Holland wrote: >> The PinePhone keyboard case contains a battery managed by an integrated >> power bank IC. The power bank IC communicates over I2C, and the keyboard >> MCU firmware provides an interface to read and write its registers. >> Let's use this interface to implement a SMBus adapter, so we can reuse >> the driver for the power bank IC. >> >> Signed-off-by: Samuel Holland <samuel@sholland.org> >> --- >> >> drivers/input/keyboard/pinephone-keyboard.c | 73 +++++++++++++++++++++ >> 1 file changed, 73 insertions(+) >> >> diff --git a/drivers/input/keyboard/pinephone-keyboard.c b/drivers/input/keyboard/pinephone-keyboard.c >> index 8065bc3e101a..7d2e16e588a0 100644 >> --- a/drivers/input/keyboard/pinephone-keyboard.c >> +++ b/drivers/input/keyboard/pinephone-keyboard.c >> @@ -3,6 +3,7 @@ >> // Copyright (C) 2021-2022 Samuel Holland <samuel@sholland.org> >> >> #include <linux/crc8.h> >> +#include <linux/delay.h> >> #include <linux/i2c.h> >> #include <linux/input/matrix_keypad.h> >> #include <linux/interrupt.h> >> @@ -23,6 +24,11 @@ >> #define PPKB_SCAN_DATA 0x08 >> #define PPKB_SYS_CONFIG 0x20 >> #define PPKB_SYS_CONFIG_DISABLE_SCAN BIT(0) >> +#define PPKB_SYS_SMBUS_COMMAND 0x21 >> +#define PPKB_SYS_SMBUS_DATA 0x22 >> +#define PPKB_SYS_COMMAND 0x23 >> +#define PPKB_SYS_COMMAND_SMBUS_READ 0x91 >> +#define PPKB_SYS_COMMAND_SMBUS_WRITE 0xa1 >> >> #define PPKB_DEFAULT_KEYMAP_ROWS 6 >> #define PPKB_DEFAULT_KEYMAP_COLS 12 >> @@ -132,6 +138,7 @@ static const struct matrix_keymap_data ppkb_default_keymap_data = { >> }; >> >> struct pinephone_keyboard { >> + struct i2c_adapter adapter; >> struct input_dev *input; >> unsigned short *fn_keymap; >> u8 crc_table[CRC8_TABLE_SIZE]; >> @@ -143,6 +150,57 @@ struct pinephone_keyboard { >> u8 buf[]; >> }; >> >> +static int ppkb_adap_smbus_xfer(struct i2c_adapter *adap, u16 addr, >> + unsigned short flags, char read_write, >> + u8 command, int size, >> + union i2c_smbus_data *data) >> +{ >> + struct i2c_client *client = adap->algo_data; >> + u8 buf[3]; >> + int ret; >> + >> + buf[0] = command; >> + buf[1] = data->byte; >> + buf[2] = read_write == I2C_SMBUS_READ ? PPKB_SYS_COMMAND_SMBUS_READ >> + : PPKB_SYS_COMMAND_SMBUS_WRITE; >> + >> + ret = i2c_smbus_write_i2c_block_data(client, PPKB_SYS_SMBUS_COMMAND, >> + sizeof(buf), buf); >> + if (ret) >> + return ret; > > [1] > >> + /* Read back the command status until it passes or fails. */ >> + do { >> + usleep_range(300, 500); >> + ret = i2c_smbus_read_byte_data(client, PPKB_SYS_COMMAND); >> + } while (ret == buf[2]); >> + if (ret < 0) >> + return ret; >> + /* Commands return 0x00 on success and 0xff on failure. */ >> + if (ret) >> + return -EIO; >> + >> + if (read_write == I2C_SMBUS_READ) { >> + ret = i2c_smbus_read_byte_data(client, PPKB_SYS_SMBUS_DATA); >> + if (ret < 0) >> + return ret; > > Please use a single read transfer to get both command result and data. > There will be less risk that some userspace app will issue another command > in between command status being read as 0 and data byte being read. > > Otherwise if you use this in some read/modify/write operation, you > may write unexpected value to PMIC. I2C register layout is designed > to make this as optimal as possible in a single I2C transaction, so > you only need 3 bytes to start command and 2 bytes to read the result > and data, both in a single xfer. There's very high likelihood the command > will complete in those 300 - 500 us anyway, because the timing is > predictable. If this delay is set right, it's almost guaranteed, > only two xfers will be necessary to run the command and get the result+ > status. I did this originally, but it causes a different race condition: since the data is read first, the command can complete between when the data is read and when the result is read. If this happens, the command will be seen as complete, but the data will be garbage. This caused occasional read errors for the charger's power supply properties, because I2C reads sometimes returned nonsensical values for those bytes. > And if possible, it would be best if the bus was somehow made busy for > other users, until the whole comand/result sequence completes, to eliminate > the possibility of another command being issued by other bus users > around [1]. Yes, I can add a call to i2c_lock_bus() here. Regards, Samuel > Thank you and kind regards, > o. > >> + data->byte = ret; >> + } >> + >> + return 0; >> +} >> + >> +static u32 ppkg_adap_functionality(struct i2c_adapter *adap) >> +{ >> + return I2C_FUNC_SMBUS_BYTE_DATA; >> +} >> + >> +static const struct i2c_algorithm ppkb_adap_algo = { >> + .smbus_xfer = ppkb_adap_smbus_xfer, >> + .functionality = ppkg_adap_functionality, >> +}; >> + >> static int ppkb_set_scan(struct i2c_client *client, bool enable) >> { >> struct device *dev = &client->dev; >> @@ -265,6 +323,7 @@ static int ppkb_probe(struct i2c_client *client) >> unsigned int map_rows, map_cols; >> struct pinephone_keyboard *ppkb; >> u8 info[PPKB_MATRIX_SIZE + 1]; >> + struct device_node *i2c_bus; >> int ret; >> >> ret = i2c_smbus_read_i2c_block_data(client, 0, sizeof(info), info); >> @@ -312,6 +371,20 @@ static int ppkb_probe(struct i2c_client *client) >> >> i2c_set_clientdata(client, ppkb); >> >> + i2c_bus = of_get_child_by_name(dev->of_node, "i2c-bus"); >> + if (i2c_bus) { >> + ppkb->adapter.owner = THIS_MODULE; >> + ppkb->adapter.algo = &ppkb_adap_algo; >> + ppkb->adapter.algo_data = client; >> + ppkb->adapter.dev.parent = dev; >> + ppkb->adapter.dev.of_node = i2c_bus; >> + strscpy(ppkb->adapter.name, DRV_NAME, sizeof(ppkb->adapter.name)); >> + >> + ret = devm_i2c_add_adapter(dev, &ppkb->adapter); >> + if (ret) >> + return dev_err_probe(dev, ret, "Failed to add I2C adapter\n"); >> + } >> + >> crc8_populate_msb(ppkb->crc_table, PPKB_CRC8_POLYNOMIAL); >> ppkb->row_shift = get_count_order(map_cols); >> ppkb->rows = map_rows; >> -- >> 2.33.1 >> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] Input: pinephone-keyboard - Support the proxied I2C bus 2022-01-30 2:43 ` Samuel Holland @ 2022-01-30 3:00 ` Ondřej Jirman 0 siblings, 0 replies; 18+ messages in thread From: Ondřej Jirman @ 2022-01-30 3:00 UTC (permalink / raw) To: Samuel Holland Cc: Dmitry Torokhov, linux-input, linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang On Sat, Jan 29, 2022 at 08:43:30PM -0600, Samuel Holland wrote: > On 1/29/22 8:05 PM, Ondřej Jirman wrote: > > > > Please use a single read transfer to get both command result and data. > > There will be less risk that some userspace app will issue another command > > in between command status being read as 0 and data byte being read. > > > > Otherwise if you use this in some read/modify/write operation, you > > may write unexpected value to PMIC. I2C register layout is designed > > to make this as optimal as possible in a single I2C transaction, so > > you only need 3 bytes to start command and 2 bytes to read the result > > and data, both in a single xfer. There's very high likelihood the command > > will complete in those 300 - 500 us anyway, because the timing is > > predictable. If this delay is set right, it's almost guaranteed, > > only two xfers will be necessary to run the command and get the result+ > > status. > > I did this originally, but it causes a different race condition: since the data > is read first, the command can complete between when the data is read and when > the result is read. If this happens, the command will be seen as complete, but > the data will be garbage. > > This caused occasional read errors for the charger's power supply properties, > because I2C reads sometimes returned nonsensical values for those bytes. Oh, well. :) I guess the firmware would need to wait for any ongoing I2C tranfer to finish before setting the command status to 0, for this to work. Another lesson learned. :( > > And if possible, it would be best if the bus was somehow made busy for > > other users, until the whole comand/result sequence completes, to eliminate > > the possibility of another command being issued by other bus users > > around [1]. > > Yes, I can add a call to i2c_lock_bus() here. Perfect. thank you, o. > Regards, > Samuel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 5/5] [DO NOT MERGE] arm64: dts: allwinner: pinephone: Add keyboard 2022-01-29 23:00 [PATCH 0/5] Pine64 PinePhone keyboard support Samuel Holland ` (3 preceding siblings ...) 2022-01-29 23:00 ` [PATCH 4/5] Input: pinephone-keyboard - Support the proxied I2C bus Samuel Holland @ 2022-01-29 23:00 ` Samuel Holland 4 siblings, 0 replies; 18+ messages in thread From: Samuel Holland @ 2022-01-29 23:00 UTC (permalink / raw) To: Dmitry Torokhov, linux-input Cc: linux-kernel, Rob Herring, devicetree, linux-i2c, Wolfram Sang, Ondrej Jirman, Samuel Holland The official PinePhone keyboard accessory connects to the phone's POGO pins for I2C and interrupts. It has an Injoinic IP5209 power bank IC connected to the keyboard's internal I2C bus. Signed-off-by: Samuel Holland <samuel@sholland.org> --- .../dts/allwinner/sun50i-a64-pinephone.dtsi | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi index 87847116ab6d..2fa1bdf8aa63 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi @@ -208,6 +208,24 @@ accelerometer@68 { /* Connected to pogo pins (external spring based pinheader for user addons) */ &i2c2 { status = "okay"; + + keyboard@15 { + compatible = "pine64,pinephone-keyboard"; + reg = <0x15>; + interrupt-parent = <&r_pio>; + interrupts = <0 12 IRQ_TYPE_EDGE_FALLING>; /* PL12 */ + wakeup-source; + + i2c-bus { + #address-cells = <1>; + #size-cells = <0>; + + charger@75 { + compatible = "injoinic,ip5209"; + reg = <0x75>; + }; + }; + }; }; &lradc { -- 2.33.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-05-30 7:06 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-29 23:00 [PATCH 0/5] Pine64 PinePhone keyboard support Samuel Holland 2022-01-29 23:00 ` [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard binding Samuel Holland 2022-02-11 13:23 ` Rob Herring 2022-02-14 4:41 ` Samuel Holland 2022-01-29 23:00 ` [PATCH 2/5] Input: pinephone-keyboard - Add PinePhone keyboard driver Samuel Holland 2022-02-02 8:07 ` Ondřej Jirman 2022-02-02 11:48 ` Ondřej Jirman 2022-01-29 23:00 ` [PATCH 3/5] Input: pinephone-keyboard - Build in the default keymap Samuel Holland 2022-01-31 19:45 ` Dmitry Torokhov 2022-02-02 4:58 ` Samuel Holland 2022-05-30 7:05 ` Pavel Machek 2022-04-12 10:20 ` Jarrah 2022-04-12 11:34 ` Ondřej Jirman 2022-01-29 23:00 ` [PATCH 4/5] Input: pinephone-keyboard - Support the proxied I2C bus Samuel Holland 2022-01-30 2:05 ` Ondřej Jirman 2022-01-30 2:43 ` Samuel Holland 2022-01-30 3:00 ` Ondřej Jirman 2022-01-29 23:00 ` [PATCH 5/5] [DO NOT MERGE] arm64: dts: allwinner: pinephone: Add keyboard Samuel Holland
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).