[U-Boot] [PATCH 0/6] tegra: Enable keyboard with matrix keyboard driver

Tegra2 has a built-in a keyboard controller which we can use to scan a matrix keyboard. This series brings in a driver for this and adds support for the QUERTY keyboard on Seaboard as an example.
Anton Staff (3): tegra: fdt: Add keyboard controller definition tegra: fdt: Add keyboard definitions for Seaboard fdt: Add fdtdec functions to read byte array
Rakesh Iyer (1): tegra: Add tegra keyboard support
Simon Glass (2): tegra: Switch on console mux and use environment for console tegra: Enable keyboard for Seaboard
arch/arm/dts/tegra20.dtsi | 4 + board/nvidia/dts/tegra2-seaboard.dts | 71 ++++ drivers/input/Makefile | 1 + drivers/input/tegra-kbc.c | 626 ++++++++++++++++++++++++++++++++++ include/configs/seaboard.h | 9 + include/configs/tegra2-common.h | 9 +- include/fdtdec.h | 33 ++ include/tegra-kbc.h | 33 ++ lib/fdtdec.c | 25 ++ 9 files changed, 810 insertions(+), 1 deletions(-) create mode 100644 drivers/input/tegra-kbc.c create mode 100644 include/tegra-kbc.h

From: Anton Staff robotboy@chromium.org
The Tegra keyboard controller provides a simple interface to a matrix keyboard.
Signed-off-by: Simon Glass sjg@chromium.org --- arch/arm/dts/tegra20.dtsi | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi index d7ab843..5344b39 100644 --- a/arch/arm/dts/tegra20.dtsi +++ b/arch/arm/dts/tegra20.dtsi @@ -234,5 +234,9 @@ periph-id = <59>; // PERIPH_ID_USB3 };
+ kbc@7000e200 { + compatible = "nvidia,tegra20-kbc"; + reg = <0x7000e200 0x0078>; + }; };

From: Anton Staff robotboy@chromium.org
Seaboard uses a QUERTY keyboard. We add key codes for this to enable key scanning to work.
Signed-off-by: Simon Glass sjg@chromium.org --- board/nvidia/dts/tegra2-seaboard.dts | 71 ++++++++++++++++++++++++++++++++++ 1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts index 19c6503..79de16b 100644 --- a/board/nvidia/dts/tegra2-seaboard.dts +++ b/board/nvidia/dts/tegra2-seaboard.dts @@ -45,4 +45,75 @@ usb@c5008000 { status = "okay"; }; + + kbc@7000e200 { + status = "okay"; + keycode-plain = [00 00 'w' 's' 'a' 'z' 00 DE + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + '5' '4' 'r' 'e' 'f' 'd' 'x' 00 + '7' '6' 't' 'h' 'g' 'v' 'c' ' ' + '9' '8' 'u' 'y' 'j' 'n' 'b' 5C + '-' '0' 'o' 'i' 'l' 'k' ',' 'm' + 00 '=' ']' 0D 00 00 00 00 + 00 00 00 00 DF DF 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + '[' 'p' 27 ';' '/' '.' 00 00 + 00 00 08 '3' '2' 1E 00 00 + 00 7F 00 00 00 1D 1F 1C + 00 00 00 'q' 00 00 '1' 00 + 1B '`' 00 09 00 00 00 00]; + + keycode-shift = [00 00 'W' 'S' 'A' 'Z' 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + '%' '$' 'R' 'E' 'F' 'D' 'X' 00 + '&' '^' 'T' 'H' 'G' 'V' 'C' ' ' + '(' '*' 'U' 'Y' 'J' 'N' 'B' '|' + '_' ')' 'O' 'I' 'L' 'K' '<' 'M' + 00 '+' '}' 0D 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + '{' 'P' '"' ':' '?' '>' 00 00 + 00 00 08 '#' '@' 00 00 00 + 00 7F 00 00 00 00 00 00 + 00 00 00 'Q' 00 00 '!' 00 + 1B '~' 00 09 00 00 00 00]; + + keycode-fn = [00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + '7' 00 00 00 00 00 00 00 + '9' '8' '4' 00 '1' 00 00 00 + 00 '/' '6' '5' '3' '2' 00 '0' + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 27 00 '-' '+' '.' 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 '?' 00 00 00]; + + keycode-ctrl = [00 00 17 13 01 1a 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 12 05 06 04 18 00 + 00 00 14 08 07 16 03 00 + 00 00 15 19 0a 0e 02 00 + 00 00 0f 09 0c 0b 00 0d + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 10 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 00 00 00 00 00 + 00 00 00 11 00 00 00 00 + 00 00 00 00 00 00 00 00]; + }; };

On 12/02/2011 07:57 PM, Simon Glass wrote:
From: Anton Staff robotboy@chromium.org
Seaboard uses a QUERTY keyboard. We add key codes for this to enable key scanning to work.
+++ b/board/nvidia/dts/tegra2-seaboard.dts
...
- kbc@7000e200 {
status = "okay";
That's the default, so this isn't needed; instead, add status = "disabled" to any boards that don't use kbc.

From: Anton Staff robotboy@chromium.org
Sometimes we don't need a full cell for each value. This provides a simple function to read a byte array, both with and without copying it.
Signed-off-by: Simon Glass sjg@chromium.org --- include/fdtdec.h | 32 ++++++++++++++++++++++++++++++++ lib/fdtdec.c | 24 ++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h index 9871b81..f72d219 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -231,3 +231,35 @@ int fdtdec_decode_gpio(const void *blob, int node, const char *prop_name, * @return 0 if all ok or gpio was FDT_GPIO_NONE; -1 on error */ int fdtdec_setup_gpio(struct fdt_gpio_state *gpio); + +/** + * Look up a property in a node and return its contents in a byte + * array of given length. The property must have at least enough data for + * the array (count bytes). It may have more, but this will be ignored. + * + * @param blob FDT blob + * @param node node to examine + * @param prop_name name of property to find + * @param array array to fill with data + * @param count number of array elements + * @return 0 if ok, or -FDT_ERR_MISSING if the property is not found, + * or -FDT_ERR_BADLAYOUT if not enough data + */ +int fdtdec_get_byte_array(const void *blob, int node, const char *prop_name, + u8 *array, int count); + +/** + * Look up a property in a node and return a pointer to its contents as a + * byte array of given length. The property must have at least enough data + * for the array (count bytes). It may have more, but this will be ignored. + * The data is not copied. + * + * @param blob FDT blob + * @param node node to examine + * @param prop_name name of property to find + * @param count number of array elements + * @return pointer to byte array if found, or NULL if the property is not + * found or there is not enough data + */ +const u8 *fdtdec_locate_byte_array(const void *blob, int node, + const char *prop_name, int count); diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 509b81f..90db53c 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -315,3 +315,27 @@ int fdtdec_setup_gpio(struct fdt_gpio_state *gpio) return gpio_direction_input(gpio->gpio); } } + +int fdtdec_get_byte_array(const void *blob, int node, const char *prop_name, + u8 *array, int count) +{ + const u8 *cell; + int err; + + cell = get_prop_len(blob, node, prop_name, count, &err); + if (!err) + memcpy(array, cell, count); + return err; +} + +const u8 *fdtdec_locate_byte_array(const void *blob, int node, + const char *prop_name, int count) +{ + const u8 *cell; + int err; + + cell = get_prop_len(blob, node, prop_name, count, &err); + if (err) + return NULL; + return cell; +}

On 12/02/2011 07:57 PM, Simon Glass wrote:
From: Anton Staff robotboy@chromium.org
Sometimes we don't need a full cell for each value. This provides a simple function to read a byte array, both with and without copying it.
+const u8 *fdtdec_locate_byte_array(const void *blob, int node,
const char *prop_name, int count)
+{
- const u8 *cell;
- int err;
- cell = get_prop_len(blob, node, prop_name, count, &err);
Isn't that called get_prop_check_min_len() now?
- if (err)
return NULL;
- return cell;
+}

Hi Stephen,
On Mon, Dec 5, 2011 at 3:54 PM, Stephen Warren swarren@nvidia.com wrote:
On 12/02/2011 07:57 PM, Simon Glass wrote:
From: Anton Staff robotboy@chromium.org
Sometimes we don't need a full cell for each value. This provides a simple function to read a byte array, both with and without copying it.
+const u8 *fdtdec_locate_byte_array(const void *blob, int node,
- const char *prop_name, int count)
+{
- const u8 *cell;
- int err;
- cell = get_prop_len(blob, node, prop_name, count, &err);
Isn't that called get_prop_check_min_len() now?
It depends on your definition of 'now'. I will of course rebase this series and test once the usb one is ready. At present it is probably based on the original v1 usb series. But there may be other changes too...
Regards, Simon
- if (err)
- return NULL;
- return cell;
+}
-- nvpublic

From: Rakesh Iyer riyer@nvidia.com
Add support for internal matrix keyboard controller for Nvidia Tegra platforms. This driver uses the fdt decode function to obtain its key codes.
Support for the Ctrl modifier is provided. The left and right ctrl keys are dealt with in the same way.
When the user presses Ctrl+D, the top of the keyboard controller FIFO is usually filled with several "Ctrl" only keycodes. The modifier keys handling code would remove the keycode and return 0. So, the "tstc" will indicate that no key is pressed.
The correct behaviour for "tstc" is to check all the entries of the FIFO until we find a significant one or return 0 if there is none.
This driver adds a small FIFO that is used to store conversions of keycodes to escape sequences that U-Boot expects to see from its input device drivers. The fifo also replaces the testc/getc last keypress management code.
Key detection before the driver is loaded is supported. This key will be picked up with the keyboard driver is initialized.
Signed-off-by: Simon Glass sjg@chromium.org --- drivers/input/Makefile | 1 + drivers/input/tegra-kbc.c | 626 +++++++++++++++++++++++++++++++++++++++++++++ include/fdtdec.h | 1 + include/tegra-kbc.h | 33 +++ lib/fdtdec.c | 1 + 5 files changed, 662 insertions(+), 0 deletions(-) create mode 100644 drivers/input/tegra-kbc.c create mode 100644 include/tegra-kbc.h
diff --git a/drivers/input/Makefile b/drivers/input/Makefile index 1f4dad3..4684e55 100644 --- a/drivers/input/Makefile +++ b/drivers/input/Makefile @@ -26,6 +26,7 @@ include $(TOPDIR)/config.mk LIB := $(obj)libinput.o
COBJS-$(CONFIG_I8042_KBD) += i8042.o +COBJS-$(CONFIG_TEGRA2_KEYBOARD) += tegra-kbc.o ifdef CONFIG_PS2KBD COBJS-y += keyboard.o pc_keyb.o COBJS-$(CONFIG_PS2MULT) += ps2mult.o ps2ser.o diff --git a/drivers/input/tegra-kbc.c b/drivers/input/tegra-kbc.c new file mode 100644 index 0000000..7a45440 --- /dev/null +++ b/drivers/input/tegra-kbc.c @@ -0,0 +1,626 @@ +/* + * (C) Copyright 2011 + * NVIDIA Corporation <www.nvidia.com> + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ +#define DEBUG +#include <common.h> +#include <malloc.h> +#include <stdio_dev.h> +#include <asm/io.h> +#include <asm/arch/clock.h> +#include <asm/arch/pinmux.h> +#include <asm/arch/timer.h> +#include <tegra-kbc.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR; + +#define DEVNAME "tegra-kbc" + +enum { + KBC_MAX_ROW = 16, + KBC_MAX_COL = 8, + KBC_KEY_COUNT = KBC_MAX_ROW * KBC_MAX_COL, + KBC_MAX_GPIO = 24, + KBC_MAX_KPENT = 8, /* size of keypress entry queue */ +}; + +enum KEYS { + KEY_FIRST_MODIFIER = 220, + KEY_RIGHT_CTRL = KEY_FIRST_MODIFIER, + KEY_LEFT_CTRL, + KEY_FN, + KEY_SHIFT, +}; + +/* KBC row scan time and delay for beginning the row scan (in usecs) */ +#define KBC_ROW_SCAN_TIME 16 +#define KBC_ROW_SCAN_DLY 5 + +/* uses a 32KHz clock so a cycle = 1/32Khz */ +#define KBC_CYCLE_IN_USEC DIV_ROUND_UP(1000, 32) + +#define KBC_FIFO_TH_CNT_SHIFT(cnt) (cnt << 14) +#define KBC_DEBOUNCE_CNT_SHIFT(cnt) (cnt << 4) +#define KBC_CONTROL_FIFO_CNT_INT_EN (1 << 3) +#define KBC_CONTROL_KBC_EN (1 << 0) +#define KBC_INT_FIFO_CNT_INT_STATUS (1 << 2) +#define KBC_KPENT_VALID (1 << 7) + +#define KBC_RPT_DLY 20 +#define KBC_RPT_RATE 4 + +/* + * Use a simple FIFO to convert some keys into escape sequences and to handle + * testc vs getc. The FIFO length must be a power of two. Currently, four + * characters is the smallest power of two that is large enough to contain all + * generated escape sequences. + */ +#define KBC_FIFO_LENGTH (1 << 2) + +static struct keyb { + /* + * Information about keycode mappings. The plain_keycode array must + * exist but the others may be NULL in which case they are not + * decoded. + */ + const u8 *plain_keycode; /* key code for each row / column */ + const u8 *shift_keycode; /* ...when shift held down */ + const u8 *fn_keycode; /* ...when Fn held down */ + const u8 *ctrl_keycode; /* ...when ctrl held down */ + + struct kbc_tegra *kbc; /* tegra keyboard controller */ + int prev_fifo[KBC_MAX_KPENT]; /* keys left over from last scan */ + int prev_cnt; /* number of keys left over */ + unsigned char prev_key; /* previously decoded key */ + unsigned int rpt_dly; /* delay until next key repeat */ + unsigned char repeat_key; /* the key being held down */ + unsigned char inited; /* 1 if keyboard has been inited */ + int fifo[KBC_FIFO_LENGTH]; /* key fifo, with decoded keys */ + int fifo_read; /* Key fifo read pointer */ + int fifo_write; /* Key fifo write pointer */ + unsigned int repoll_time; /* next time to poll keyboard (us) */ + + /* + * After init we must wait a short time before polling the keyboard. + * This gives the tegra keyboard controller time to react after reset + * and lets us grab keys pressed during reset. + */ + unsigned int init_dly; /* Delay before we can read keyboard */ + unsigned long start_time; /* Time that we inited (in us) */ +} config; + + +static int tegra_kbc_keycode(struct keyb *config, int r, int c, + int modifier) +{ + int entry = r * KBC_MAX_COL + c; + + if (modifier == KEY_FN && config->fn_keycode) + return config->fn_keycode[entry]; + + /* Put ctrl keys ahead of shift */ + else if ((modifier == KEY_LEFT_CTRL || modifier == KEY_RIGHT_CTRL) + && config->ctrl_keycode) + return config->ctrl_keycode[entry]; + else if (modifier == KEY_SHIFT && config->shift_keycode) + return config->shift_keycode[entry]; + else + return config->plain_keycode[entry]; +} + +/* determines if current keypress configuration can cause key ghosting */ +static int is_ghost_key_config(int *rows_val, int *cols_val, int valid) +{ + int i, j, key_in_same_col = 0, key_in_same_row = 0; + + /* + * Matrix keyboard designs are prone to keyboard ghosting. + * Ghosting occurs if there are 3 keys such that - + * any 2 of the 3 keys share a row, and any 2 of them share a column. + */ + for (i = 0; i < valid; i++) { + /* + * Find 2 keys such that one key is in the same row + * and the other is in the same column as the i-th key. + */ + for (j = i + 1; j < valid; j++) { + if (cols_val[j] == cols_val[i]) + key_in_same_col = 1; + if (rows_val[j] == rows_val[i]) + key_in_same_row = 1; + } + } + + if (key_in_same_col && key_in_same_row) + return 1; + else + return 0; +} + +/* reads the keyboard fifo for current keypresses. */ +static int tegra_kbc_find_keys(struct keyb *config, int *fifo) +{ + int rows_val[KBC_MAX_KPENT], cols_val[KBC_MAX_KPENT]; + u32 kp_ent_val[(KBC_MAX_KPENT + 3) / 4]; + u32 *kp_ents = kp_ent_val; + u32 kp_ent = 0; + int i, j, k, valid = 0; + int modifier = 0; + + for (i = 0; i < ARRAY_SIZE(kp_ent_val); i++) + kp_ent_val[i] = readl(&config->kbc->kp_ent[i]); + + valid = 0; + for (i = 0; i < KBC_MAX_KPENT; i++) { + if (!(i&3)) + kp_ent = *kp_ents++; + + if (kp_ent & KBC_KPENT_VALID) { + cols_val[valid] = kp_ent & 0x7; + rows_val[valid++] = (kp_ent >> 3) & 0xf; + } + kp_ent >>= 8; + } + for (i = 0; i < valid; i++) { + k = tegra_kbc_keycode(config, rows_val[i], cols_val[i], 0); + if (KEY_IS_MODIFIER(k)) { + modifier = k; + break; + } + } + + /* For a ghost key config, ignore the keypresses for this iteration. */ + if ((valid >= 3) && is_ghost_key_config(rows_val, cols_val, valid)) + return KBC_MAX_KPENT + 1; + + j = 0; + for (i = 0; i < valid; i++) { + k = tegra_kbc_keycode(config, rows_val[i], cols_val[i], + modifier); + /* Key modifiers (Fn and Shift) will not be part of the fifo */ + if (k && (k != -1)) + fifo[j++] = k; + } + + return j; +} + +static void process_keys(struct keyb *config, int fifo[], int cnt) +{ + int prev_key_in_fifo = 0; + char key = config->prev_key; + int i, j; + + /* + * If multiple keys are pressed such that it results in * 2 or more + * unmodified keys, we will determine the newest key and report + * that to the upper layer. + */ + for (i = 0; i < cnt; i++) { + for (j = 0; j < config->prev_cnt; j++) { + if (fifo[i] == config->prev_fifo[j]) + break; + } + /* Found a new key. */ + if (j == config->prev_cnt) { + key = fifo[i]; + break; + } + if (config->prev_key == fifo[i]) + prev_key_in_fifo = 1; + } + /* + * Keys were released and FIFO does not contain * the previous + * reported key. So report a null key. + */ + if (i == cnt && !prev_key_in_fifo) + key = 0; + + for (i = 0; i < cnt; i++) + config->prev_fifo[i] = fifo[i]; + config->prev_cnt = cnt; + config->prev_key = key; +} + +/* processes all the keypresses in fifo and returns a single key */ +static int tegra_kbc_get_single_char(struct keyb *config, u32 fifo_cnt) +{ + int fifo[KBC_MAX_KPENT] = {0}; + int cnt; + + if (fifo_cnt) { + do { + cnt = tegra_kbc_find_keys(config, fifo); + /* If this is a ghost key combo, report no change */ + if (cnt <= KBC_MAX_KPENT) + process_keys(config, fifo, cnt); + } while (!config->prev_key && --fifo_cnt); + } else { + config->prev_cnt = 0; + config->prev_key = 0; + } + + udelay((fifo_cnt == 1) ? config->repoll_time : 1000); + + return config->prev_key; +} + +/* manages keyboard hardware registers on keypresses and returns a key.*/ +static unsigned char tegra_kbc_get_char(struct keyb *config) +{ + struct kbc_tegra *kbc = config->kbc; + u32 val, ctl; + char key = 0; + + /* + * Until all keys are released, defer further processing to + * the polling loop. + */ + ctl = readl(&kbc->control); + ctl &= ~KBC_CONTROL_FIFO_CNT_INT_EN; + writel(ctl, &kbc->control); + + /* + * Quickly bail out & reenable interrupts if the interrupt source + * wasn't fifo count threshold + */ + val = readl(&kbc->interrupt); + writel(val, &kbc->interrupt); + + if (val & KBC_INT_FIFO_CNT_INT_STATUS) + key = tegra_kbc_get_single_char(config, (val >> 4) & 0xf); + + ctl |= KBC_CONTROL_FIFO_CNT_INT_EN; + writel(ctl, &kbc->control); + + return key; +} + +/* handles key repeat delay and key repeat rate.*/ +static int kbd_fetch_char(struct keyb *config, int test) +{ + unsigned char key; + + do { + key = tegra_kbc_get_char(config); + if (!key) { + config->repeat_key = 0; + if (test) + break; + else + continue; + } + + /* This logic takes care of the repeat rate */ + if ((key != config->repeat_key) || !(config->rpt_dly--)) + break; + } while (1); + + if (key == config->repeat_key) { + config->rpt_dly = KBC_RPT_RATE; + } else { + config->rpt_dly = KBC_RPT_DLY; + config->repeat_key = key; + } + + return key; +} + +/* + * Return true if there are no characters in the FIFO. + */ +static int kbd_fifo_empty(struct keyb *config) +{ + return config->fifo_read == config->fifo_write; +} + +/* + * Return number of characters of free space in the FIFO. + */ +static int kbd_fifo_free_space(struct keyb *config) +{ + return KBC_FIFO_LENGTH - (config->fifo_write - config->fifo_read); +} + +/* + * Insert a character into the FIFO. Calling this function when the FIFO is + * full will overwrite the oldest character in the FIFO. + */ +static void kbd_fifo_insert(struct keyb *config, int key) +{ + int index = config->fifo_write & (KBC_FIFO_LENGTH - 1); + + assert(kbd_fifo_free_space(config) > 0); + + config->fifo[index] = key; + + config->fifo_write++; +} + +/* + * Remove a character from the FIFO, it is an error to call this function when + * the FIFO is empty. + */ +static int kbd_fifo_remove(struct keyb *config) +{ + int index = config->fifo_read & (KBC_FIFO_LENGTH - 1); + int key = config->fifo[index]; + + assert(!kbd_fifo_empty(config)); + + config->fifo_read++; + + return key; +} + +/* + * Given a keycode, convert it to the sequence of characters that U-Boot expects + * to recieve from its input devices and insert the characters into the FIFO. + * If the keycode is 0, ignore it. Currently this function will only ever add + * at most three characters to the FIFO, so it is always safe to call when the + * FIFO is empty. + */ +static void kbd_fifo_refill(struct keyb *config, int key) +{ + switch (key) { + /* + * We need to deal with a zero keycode value here because the + * testc call can call us with zero if no key is pressed. + */ + case 0x00: + break; + + /* + * Generate escape sequences for arrow keys. Additional escape + * sequences can be added to the switch statements, but it may + * be better in the future t0 use the top bit of the keycode to + * indicate a key that generates an escape sequence. Then the + * outer switch could turn into an "if (key & 0x80)". + */ + case 0x1C: + case 0x1D: + case 0x1E: + case 0x1F: + kbd_fifo_insert(config, 0x1B); /* Escape */ + kbd_fifo_insert(config, '['); + + switch (key) { + case 0x1C: + kbd_fifo_insert(config, 'D'); + break; + case 0x1D: + kbd_fifo_insert(config, 'C'); + break; + case 0x1E: + kbd_fifo_insert(config, 'A'); + break; + case 0x1F: + kbd_fifo_insert(config, 'B'); + break; + } + break; + + /* + * All other keycodes can be treated as characters as is and + * are inserted into the FIFO directly. + */ + default: + kbd_fifo_insert(config, key); + break; + } +} + +static void kbd_wait_for_fifo_init(struct keyb *config) +{ + /* + * In order to detect keys pressed on boot, wait for the hardware to + * complete scanning the keys. This includes time to transition from + * Wkup mode to Continous polling mode and the repoll time. We can + * deduct the time thats already elapsed. + */ + if (!config->inited) { + unsigned long elapsed_time; + long delay; + + elapsed_time = timer_get_us() - config->start_time; + delay = config->init_dly + config->repoll_time - elapsed_time; + if (delay > 0) + udelay(delay); + + config->inited = 1; + } +} + +static int kbd_testc(void) +{ + kbd_wait_for_fifo_init(&config); + + if (kbd_fifo_empty(&config)) + kbd_fifo_refill(&config, kbd_fetch_char(&config, 1)); + + return !kbd_fifo_empty(&config); +} + +static int kbd_getc(void) +{ + if (kbd_fifo_empty(&config)) + kbd_fifo_refill(&config, kbd_fetch_char(&config, 0)); + + return kbd_fifo_remove(&config); +} + +/* configures keyboard GPIO registers to use the rows and columns */ +static void config_kbc(struct kbc_tegra *kbc) +{ + int i; + + for (i = 0; i < KBC_MAX_GPIO; i++) { + u32 row_cfg, col_cfg; + u32 r_shift = 5 * (i % 6); + u32 c_shift = 4 * (i % 8); + u32 r_mask = 0x1f << r_shift; + u32 c_mask = 0xf << c_shift; + u32 r_offs = i / 6; + u32 c_offs = i / 8; + + row_cfg = readl(&kbc->row_cfg[r_offs]); + col_cfg = readl(&kbc->col_cfg[c_offs]); + + row_cfg &= ~r_mask; + col_cfg &= ~c_mask; + + if (i < KBC_MAX_ROW) + row_cfg |= ((i << 1) | 1) << r_shift; + else + col_cfg |= (((i - KBC_MAX_ROW) << 1) | 1) << c_shift; + + writel(row_cfg, &kbc->row_cfg[r_offs]); + writel(col_cfg, &kbc->col_cfg[c_offs]); + } +} + +static int tegra_kbc_open(void) +{ + unsigned int scan_time_rows, debounce_cnt, rpt_cnt; + struct kbc_tegra *kbc = config.kbc; + u32 val = 0; + + config_kbc(kbc); + + /* + * The time delay between two consecutive reads of the FIFO is + * the sum of the repeat time and the time taken for scanning + * the rows. There is an additional delay before the row scanning + * starts. The repoll delay is computed in microseconds. + */ + rpt_cnt = 5 * DIV_ROUND_UP(1000, KBC_CYCLE_IN_USEC); + debounce_cnt = 2; + scan_time_rows = (KBC_ROW_SCAN_TIME + debounce_cnt) * KBC_MAX_ROW; + config.repoll_time = KBC_ROW_SCAN_DLY + scan_time_rows + rpt_cnt; + config.repoll_time = config.repoll_time * KBC_CYCLE_IN_USEC; + + writel(rpt_cnt, &kbc->rpt_dly); + + val = KBC_DEBOUNCE_CNT_SHIFT(debounce_cnt); + val |= KBC_FIFO_TH_CNT_SHIFT(1); /* set fifo interrupt threshold to 1 */ + val |= KBC_CONTROL_FIFO_CNT_INT_EN; /* interrupt on FIFO threshold */ + val |= KBC_CONTROL_KBC_EN; /* enable */ + + writel(val, &kbc->control); + + config.init_dly = readl(&kbc->init_dly) * KBC_CYCLE_IN_USEC; + config.start_time = timer_get_us(); + + return 0; +} + +void config_kbc_pinmux(void) +{ + enum pmux_pingrp pingrp[] = {PINGRP_KBCA, PINGRP_KBCB, PINGRP_KBCC, + PINGRP_KBCD, PINGRP_KBCE, PINGRP_KBCF}; + int i; + + for (i = 0; i < (sizeof(pingrp)/sizeof(pingrp[0])); i++) { + pinmux_tristate_disable(pingrp[i]); + pinmux_set_func(pingrp[i], PMUX_FUNC_KBC); + pinmux_set_pullupdown(pingrp[i], PMUX_PULL_UP); + } +} + +static int fdt_decode_kbc(const void *blob, int node, struct keyb *config) +{ + config->kbc = (struct kbc_tegra *)fdtdec_get_addr(blob, node, "reg"); + config->plain_keycode = fdtdec_locate_byte_array(blob, node, + "keycode-plain", KBC_KEY_COUNT); + config->shift_keycode = fdtdec_locate_byte_array(blob, node, + "keycode-shift", KBC_KEY_COUNT); + config->fn_keycode = fdtdec_locate_byte_array(blob, node, + "keycode-fn", KBC_KEY_COUNT); + config->ctrl_keycode = fdtdec_locate_byte_array(blob, node, + "keycode-ctrl", KBC_KEY_COUNT); + if (!config->plain_keycode) { + debug("%s: cannot find keycode-plain map\n", __func__); + return -1; + } + return 0; +} + +int drv_keyboard_init(void) +{ + int error; + struct stdio_dev kbddev; + char *stdinname; + +#ifdef CONFIG_OF_CONTROL + int node; + + node = fdtdec_next_compatible(gd->fdt_blob, 0, + COMPAT_NVIDIA_TEGRA20_KBC); + if (node < 0) { + debug("%s: cannot locate keyboard node\n", __func__); + return node; + } + if (fdt_decode_kbc(gd->fdt_blob, node, &config)) + return -1; +#else +#error "Tegra keyboard driver requires FDT definitions" +#endif + config.rpt_dly = KBC_RPT_DLY; + + config_kbc_pinmux(); + + /* + * All of the Tegra board use the same clock configuration for now. + * This can be moved to the board specific configuration if that + * changes. + */ + clock_enable(PERIPH_ID_KBC); + + stdinname = getenv("stdin"); + memset(&kbddev, 0, sizeof(kbddev)); + strcpy(kbddev.name, DEVNAME); + kbddev.flags = DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM; + kbddev.putc = NULL; + kbddev.puts = NULL; + kbddev.getc = kbd_getc; + kbddev.tstc = kbd_testc; + kbddev.start = tegra_kbc_open; + + error = stdio_register(&kbddev); + if (!error) { + /* check if this is the standard input device */ + if (strcmp(stdinname, DEVNAME) == 0) { + /* reassign the console */ + if (OVERWRITE_CONSOLE) + return 1; + + error = console_assign(stdin, DEVNAME); + if (!error) + return 0; + else + return error; + } + return 1; + } + + return error; +} diff --git a/include/fdtdec.h b/include/fdtdec.h index f72d219..9a61611 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -58,6 +58,7 @@ struct fdt_memory { enum fdt_compat_id { COMPAT_UNKNOWN, COMPAT_NVIDIA_TEGRA20_USB, /* Tegra2 USB port */ + COMPAT_NVIDIA_TEGRA20_KBC, /* Tegra2 Keyboard */
COMPAT_COUNT, }; diff --git a/include/tegra-kbc.h b/include/tegra-kbc.h new file mode 100644 index 0000000..7803f10 --- /dev/null +++ b/include/tegra-kbc.h @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2011 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef __include_tegra_kbc_h__ +#define __include_tegra_kbc_h__ + +#include <common.h> + +#define KEY_IS_MODIFIER(key) ((key) >= KEY_FIRST_MODIFIER) + +struct kbc_tegra { + u32 control; + u32 interrupt; + u32 row_cfg[4]; + u32 col_cfg[3]; + u32 timeout_cnt; + u32 init_dly; + u32 rpt_dly; + u32 kp_ent[2]; + u32 row_mask[16]; +}; + +#ifdef CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE +extern int overwrite_console(void); +#define OVERWRITE_CONSOLE overwrite_console() +#else +#define OVERWRITE_CONSOLE 0 +#endif /* CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE */ + +#endif /* __include_tegra_kbc_h__ */ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 90db53c..6db814a 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -38,6 +38,7 @@ DECLARE_GLOBAL_DATA_PTR; static const char * const compat_names[COMPAT_COUNT] = { COMPAT(UNKNOWN, "<none>"), COMPAT(NVIDIA_TEGRA20_USB, "nvidia,tegra20-ehci"), + COMPAT(NVIDIA_TEGRA20_KBC, "nvidia,tegra20-kbc"), };
/**

On 12/02/2011 07:57 PM, Simon Glass wrote:
Add support for internal matrix keyboard controller for Nvidia Tegra platforms. This driver uses the fdt decode function to obtain its key codes.
+static int fdt_decode_kbc(const void *blob, int node, struct keyb *config) +{
config->kbc = (struct kbc_tegra *)fdtdec_get_addr(blob, node, "reg");
config->plain_keycode = fdtdec_locate_byte_array(blob, node,
"keycode-plain", KBC_KEY_COUNT);
config->shift_keycode = fdtdec_locate_byte_array(blob, node,
"keycode-shift", KBC_KEY_COUNT);
config->fn_keycode = fdtdec_locate_byte_array(blob, node,
"keycode-fn", KBC_KEY_COUNT);
config->ctrl_keycode = fdtdec_locate_byte_array(blob, node,
"keycode-ctrl", KBC_KEY_COUNT);
if (!config->plain_keycode) {
debug("%s: cannot find keycode-plain map\n", __func__);
return -1;
}
return 0;
+}
Simon,
Sorry to keep pushing back on everything so much, but I don't believe the structure of this binding is correct.
From a purely HW perspective, the only thing that exists is a single
mapping from (row, col) to keycode. I believe that's all the KBC driver's binding should define.
I'll amend that slightly: keyboards commonly implement the Fn key internally in HW, since it causes keys to emit completely different raw codes, so I can see this driver wanting both keycode-plain and keycode-fn.
However, keycode-shift and keycode-ctrl are purely SW concepts. As such, they shouldn't be part of the KBC's own mapping table. Witness how the kernel's Tegra KBC driver only contains the plain and fn tables (albeit merged into a single array for ease of use), and the input core is what interpets e.g. KEY_LEFTSHIFT as a modifier.
So specifically what I'd like to see changed in this binding is:
a) We need binding documentation for the Tegra KBC binding, in the same style as found in the kernel's Documentation/devicetree/bindings.
b) The Tegra KBC binding should include only the keycode-plain and keycode-fn properties; nothing to do with shift/ctrl/alt/.... (Well, also any standardized properties such as compatible, reg, interrupts).
c) We need binding documentation for the data that is/could-be common across multiple keyboards: i.e. what does each key code value represent; which is KEY_A, KEY_LEFTSHIFT, etc.? These values should be common across (1) all keyboards (2) some standardized meaning for DT that can be used by U-Boot, the Linux kernel, etc. Perhaps there is already such a standard?
d) Shift/Ctrl/Alt/... modifier mapping tables should be specified by a separate binding that can be common to any/all keyboards (probably the same document as (b) above). The input to this table should be the raw codes from keycode-plain/keycode-fn. The output would be the values sent to whatever consumes keyboard input. The naming of these properties should make it obvious that it's something not specific to Tegra's KBC, and SW oriented. Perhaps something semantically similar to loadkeys' or xkbcomp's input, although I haven't looked at those in detail.
Linux probably wouldn't use (d), since it already has its own SW-oriented methods of setting up such mapping tables. Although perhaps in the future the default mappings could be set up from these properties.
U-Boot would need (d), since AIUI, there is no handling of such a higher layer of mapping tables there right now.
Initially, the code to parse and implement the mappings in (d) could be part of the Tegra KBC driver, if there's nowhere else to put it. I simply want to ensure that the structure of the mapping table bindings doesn't require them to be specific to Tegra KBC, or perpetually implemented by the Tegra KBC driver even when/if U-Boot does acquire a higher layer that deals with this kind of thing. That's because they're SW concepts not part of the HW/HW-binding.

Hi Stephen,
On Wed, Dec 7, 2011 at 2:02 PM, Stephen Warren swarren@nvidia.com wrote:
On 12/02/2011 07:57 PM, Simon Glass wrote:
Add support for internal matrix keyboard controller for Nvidia Tegra platforms. This driver uses the fdt decode function to obtain its key codes.
+static int fdt_decode_kbc(const void *blob, int node, struct keyb *config) +{
- config->kbc = (struct kbc_tegra *)fdtdec_get_addr(blob, node, "reg");
- config->plain_keycode = fdtdec_locate_byte_array(blob, node,
- "keycode-plain", KBC_KEY_COUNT);
- config->shift_keycode = fdtdec_locate_byte_array(blob, node,
- "keycode-shift", KBC_KEY_COUNT);
- config->fn_keycode = fdtdec_locate_byte_array(blob, node,
- "keycode-fn", KBC_KEY_COUNT);
- config->ctrl_keycode = fdtdec_locate_byte_array(blob, node,
- "keycode-ctrl", KBC_KEY_COUNT);
- if (!config->plain_keycode) {
- debug("%s: cannot find keycode-plain map\n", __func__);
- return -1;
- }
- return 0;
+}
Simon,
Sorry to keep pushing back on everything so much, but I don't believe the structure of this binding is correct.
If I didn't want your feedback I would not have copied you on the patches :-) It is very useful to know how things are being done in the kernel and we need to try to keep U-Boot and the kernel as close as possible on the fdt side (although perhaps not in any other way - U-Boot is a boot loader not an OS). I'm starting to form the view that U-Boot will need a few more tweaks on top of the kernel file in some cases, but let's see. Thanks very much for taking the time to provide this very useful feedback. If nothing else these discussions might tease out issues to address later.
From a purely HW perspective, the only thing that exists is a single mapping from (row, col) to keycode. I believe that's all the KBC driver's binding should define.
I'll amend that slightly: keyboards commonly implement the Fn key internally in HW, since it causes keys to emit completely different raw codes, so I can see this driver wanting both keycode-plain and keycode-fn.
However, keycode-shift and keycode-ctrl are purely SW concepts. As such, they shouldn't be part of the KBC's own mapping table. Witness how the kernel's Tegra KBC driver only contains the plain and fn tables (albeit merged into a single array for ease of use), and the input core is what interpets e.g. KEY_LEFTSHIFT as a modifier.
Yes, certainly for ctrl I agree that we can simply calculate it. Although interestingly the other two keyboard drivers in U-Boot use the same approach as here, so maybe there is a fish hook somewhere (I have not written this code before).
So specifically what I'd like to see changed in this binding is:
a) We need binding documentation for the Tegra KBC binding, in the same style as found in the kernel's Documentation/devicetree/bindings.
OK will do - should I just put that in the cover letter? There is no such file in U-Boot.
b) The Tegra KBC binding should include only the keycode-plain and keycode-fn properties; nothing to do with shift/ctrl/alt/.... (Well, also any standardized properties such as compatible, reg, interrupts).
OK. I'm not sure how I specify what happens when you press shift...
c) We need binding documentation for the data that is/could-be common across multiple keyboards: i.e. what does each key code value represent; which is KEY_A, KEY_LEFTSHIFT, etc.? These values should be common across (1) all keyboards (2) some standardized meaning for DT that can be used by U-Boot, the Linux kernel, etc. Perhaps there is already such a standard?
I'm not aware of it.
d) Shift/Ctrl/Alt/... modifier mapping tables should be specified by a separate binding that can be common to any/all keyboards (probably the same document as (b) above). The input to this table should be the raw codes from keycode-plain/keycode-fn. The output would be the values sent to whatever consumes keyboard input. The naming of these properties should make it obvious that it's something not specific to Tegra's KBC, and SW oriented. Perhaps something semantically similar to loadkeys' or xkbcomp's input, although I haven't looked at those in detail.
While I agree this would be nice, it involves adding a layer of software into U-Boot which doesn't currently exist (converting key codes + modifies into ASCII codes).
Linux probably wouldn't use (d), since it already has its own SW-oriented methods of setting up such mapping tables. Although perhaps in the future the default mappings could be set up from these properties.
Well, and X already has its own, etc.
U-Boot would need (d), since AIUI, there is no handling of such a higher layer of mapping tables there right now.
Initially, the code to parse and implement the mappings in (d) could be part of the Tegra KBC driver, if there's nowhere else to put it. I simply want to ensure that the structure of the mapping table bindings doesn't require them to be specific to Tegra KBC, or perpetually implemented by the Tegra KBC driver even when/if U-Boot does acquire a higher layer that deals with this kind of thing. That's because they're SW concepts not part of the HW/HW-binding.
Do you think for the purposes of this series I should modify it to remove support for the shift key and make the ctrl key use calculated values? We can then address shift later. That at least puts the hardware driver up there.
However, even if I do that, the question of what do do about shift needs to be resolved. At present we have a 128-byte table and a few lines of code which allows use to support any keyboard type. Whether we have @ or " above the 2 key, we can support it. If there is a funny blue hotkey which means 'boot from USB' when you press it and 'boot from SD' when you press it with shift, we can support it with a mapping >= ascii 128. It provides maximum flexibility.
The problem is when we have lots of keyboards we may want to do the translation across all of them to reduce duplicated code in U-Boot.
From what I can see there will be three keyboard implementations in U-Boot:
- drivers/input/keyboard.c has a translation table - drivers/input/i8042.c has a translation table also, and supports German and English - this driver which has a translation table in the fdt, which seems like a step forward
We can certainly have a discussion about shift key translation and how it should work. The same discussion probably applies to un-shifted keys though, since they may well be printed differently in other languages. At least with the scheme in this series you can support any keyboard you like and have complete control over how it behaves.
I think asking for a new input layer in U-Boot. But this does not exist at present. Don't you think this is taking things a little far? I am trying to upstream a hardware driver :-)
My reading of the Tegra keyboard driver in the kernel is that it *could* use the keycode-plain and keycode-fn properties as given in this series. They would replace the tegra_kbc_default_keymap[] array. However, code would need to be added to create that array for use by the upper layers of linux keyboard input, since presumably it will be a while before the kernel's drivers/input/keyboard/matrix_keypad.c supports an fdt-based mapping. It certainly will not use a shift or ctrl mapping, since these are handled by upper layers of Linux input.
So after all this musing I see two options:
1. Modify this series to remove 'shift' support and make 'ctrl' use a calculated value (i.e. unlike the other two existing U-Boot drivers). We lose access to a number of symbols but I could hard-code a mapping in for the keyboard on Seaboard, say.
2. a. Go with what we have, put a 'u-boot,' prefix on the keycode-shift property and don't expect the kernel to ever use it. b. Start talking on the U-Boot list about the need for a middle input translation layer and a generic header file which defines key codes in a standardized way. Then write this layer, get it accepted and refactor the 3 keyboard drivers to use it.
Thoughts?
Regards, Simon
-- nvpublic

On 12/08/2011 02:56 PM, Simon Glass wrote:
Hi Stephen,
On Wed, Dec 7, 2011 at 2:02 PM, Stephen Warren swarren@nvidia.com wrote:
On 12/02/2011 07:57 PM, Simon Glass wrote:
Add support for internal matrix keyboard controller for Nvidia Tegra platforms. This driver uses the fdt decode function to obtain its key codes.
...
From a purely HW perspective, the only thing that exists is a single mapping from (row, col) to keycode. I believe that's all the KBC driver's binding should define.
I'll amend that slightly: keyboards commonly implement the Fn key internally in HW, since it causes keys to emit completely different raw codes, so I can see this driver wanting both keycode-plain and keycode-fn.
However, keycode-shift and keycode-ctrl are purely SW concepts. As such, they shouldn't be part of the KBC's own mapping table. Witness how the kernel's Tegra KBC driver only contains the plain and fn tables (albeit merged into a single array for ease of use), and the input core is what interpets e.g. KEY_LEFTSHIFT as a modifier.
Yes, certainly for ctrl I agree that we can simply calculate it. Although interestingly the other two keyboard drivers in U-Boot use the same approach as here, so maybe there is a fish hook somewhere (I have not written this code before).
So specifically what I'd like to see changed in this binding is:
a) We need binding documentation for the Tegra KBC binding, in the same style as found in the kernel's Documentation/devicetree/bindings.
OK will do - should I just put that in the cover letter? There is no such file in U-Boot.
Right now, the canonical location for binding documentation appears to be the kernel's Documentation/devicetree/bindings/ directory. Perhaps we should add the documentation there first?
At least, we should post the binding document in the standard kernel style to the linux-input and devicetree-discuss mailing lists even if it doesn't get immediately checked in.
I recall from the kernel summit that Grant Likely was thinking of creating an outside-the-kernel repository for either/both of binding documentation and .dts/.dtsi. I'm not sure if any progress has been made there yet.
b) The Tegra KBC binding should include only the keycode-plain and keycode-fn properties; nothing to do with shift/ctrl/alt/.... (Well, also any standardized properties such as compatible, reg, interrupts).
OK. I'm not sure how I specify what happens when you press shift...
c) We need binding documentation for the data that is/could-be common across multiple keyboards: i.e. what does each key code value represent; which is KEY_A, KEY_LEFTSHIFT, etc.? These values should be common across (1) all keyboards (2) some standardized meaning for DT that can be used by U-Boot, the Linux kernel, etc. Perhaps there is already such a standard?
I'm not aware of it.
I looked around for any kind of existing keyboard mapping binding, and I couldn't find anything.
I did find a Samsung matrix KBC binding in the kernel (Documentation/devicetree/bindings/input/samsung-keypad.txt) which is conceptually at the same level as having a "plain" table, although no "fn" table in their case. I prefer the proposed Tegra binding's table approach rather than having a separate node per key myself.
d) Shift/Ctrl/Alt/... modifier mapping tables should be specified by a separate binding that can be common to any/all keyboards (probably the same document as (b) above). The input to this table should be the raw codes from keycode-plain/keycode-fn. The output would be the values sent to whatever consumes keyboard input. The naming of these properties should make it obvious that it's something not specific to Tegra's KBC, and SW oriented. Perhaps something semantically similar to loadkeys' or xkbcomp's input, although I haven't looked at those in detail.
While I agree this would be nice, it involves adding a layer of software into U-Boot which doesn't currently exist (converting key codes + modifies into ASCII codes).
It doesn't have to be a separate lyaer in U-Boot; the binding just has to be defined in a way that doesn't tie it to the Tegra KBC driver, so it can be re-used.
In other words:
Tegra KBC's binding defines:
nvidia,keycode-plain nvidia,keycode-fn
(these should output "shift" and "ctrl" keycodes for later processing)
Generic key mapping binding defines:
modifier-mapping-shift modifier-mapping-ctrl
... which take as input the values generated by keycode-plain/keycode-fn and output the rewritten keycodes.
(the difference here is that the input to those tables is the output from the nvidia,keycode-foo tables, rather than the raw HW keycodes)
As far as implementation goes, all of that can be handled inside the Tegra KBC driver for now, so no new layer is required.
I think we should run this plan, and both binding definitions past the linux-input mailing list; people there will be far more aware of any previous work in this area, whether this plan makes sense, etc.
...
I think asking for a new input layer in U-Boot. But this does not exist at present. Don't you think this is taking things a little far? I am trying to upstream a hardware driver :-)
Yes, I'd like a clear documentation/naming separation between the HW-specific plain/fn keycode array and the higher-level shift/ctrl mappings. I certainly am not requesting that you create a separate input layer in U-Boot to handle those mappings; it can all be done in the Tegra KBC driver for now.
My reading of the Tegra keyboard driver in the kernel is that it *could* use the keycode-plain and keycode-fn properties as given in this series. They would replace the tegra_kbc_default_keymap[] array.
Yes, that seems quite reasonable.
However, code would need to be added to create that array for use by the upper layers of linux keyboard input, since presumably it will be a while before the kernel's drivers/input/keyboard/matrix_keypad.c supports an fdt-based mapping. It certainly will not use a shift or ctrl mapping, since these are handled by upper layers of Linux input.
So after all this musing I see two options:
- Modify this series to remove 'shift' support and make 'ctrl' use a
calculated value (i.e. unlike the other two existing U-Boot drivers). We lose access to a number of symbols but I could hard-code a mapping in for the keyboard on Seaboard, say.
- a. Go with what we have, put a 'u-boot,' prefix on the
keycode-shift property and don't expect the kernel to ever use it. b. Start talking on the U-Boot list about the need for a middle input translation layer and a generic header file which defines key codes in a standardized way. Then write this layer, get it accepted and refactor the 3 keyboard drivers to use it.
I'd like to ask for comments in linux-input in case they have any other ideas, e.g. whether a set of separately documented modifier mapping properties makes sense. If not, I suppose the following set of properties would suffice:
nvidia,keycode-plain nvidia,keycode-fn u-boot,keycode-shift u-boot,keycode-ctrl
(although the last two seem to want both nvidia, and u-boot, prefixes)

Hi Stephen,
On Mon, Dec 12, 2011 at 10:00 AM, Stephen Warren swarren@nvidia.com wrote:
On 12/08/2011 02:56 PM, Simon Glass wrote:
Hi Stephen,
On Wed, Dec 7, 2011 at 2:02 PM, Stephen Warren swarren@nvidia.com wrote:
On 12/02/2011 07:57 PM, Simon Glass wrote:
Add support for internal matrix keyboard controller for Nvidia Tegra platforms. This driver uses the fdt decode function to obtain its key codes.
...
From a purely HW perspective, the only thing that exists is a single mapping from (row, col) to keycode. I believe that's all the KBC driver's binding should define.
I'll amend that slightly: keyboards commonly implement the Fn key internally in HW, since it causes keys to emit completely different raw codes, so I can see this driver wanting both keycode-plain and keycode-fn.
However, keycode-shift and keycode-ctrl are purely SW concepts. As such, they shouldn't be part of the KBC's own mapping table. Witness how the kernel's Tegra KBC driver only contains the plain and fn tables (albeit merged into a single array for ease of use), and the input core is what interpets e.g. KEY_LEFTSHIFT as a modifier.
Yes, certainly for ctrl I agree that we can simply calculate it. Although interestingly the other two keyboard drivers in U-Boot use the same approach as here, so maybe there is a fish hook somewhere (I have not written this code before).
So specifically what I'd like to see changed in this binding is:
a) We need binding documentation for the Tegra KBC binding, in the same style as found in the kernel's Documentation/devicetree/bindings.
OK will do - should I just put that in the cover letter? There is no such file in U-Boot.
Right now, the canonical location for binding documentation appears to be the kernel's Documentation/devicetree/bindings/ directory. Perhaps we should add the documentation there first?
At least, we should post the binding document in the standard kernel style to the linux-input and devicetree-discuss mailing lists even if it doesn't get immediately checked in.
I recall from the kernel summit that Grant Likely was thinking of creating an outside-the-kernel repository for either/both of binding documentation and .dts/.dtsi. I'm not sure if any progress has been made there yet.
Not enough that it is done, but I believe it is happening.
b) The Tegra KBC binding should include only the keycode-plain and keycode-fn properties; nothing to do with shift/ctrl/alt/.... (Well, also any standardized properties such as compatible, reg, interrupts).
OK. I'm not sure how I specify what happens when you press shift...
c) We need binding documentation for the data that is/could-be common across multiple keyboards: i.e. what does each key code value represent; which is KEY_A, KEY_LEFTSHIFT, etc.? These values should be common across (1) all keyboards (2) some standardized meaning for DT that can be used by U-Boot, the Linux kernel, etc. Perhaps there is already such a standard?
I'm not aware of it.
I looked around for any kind of existing keyboard mapping binding, and I couldn't find anything.
I did find a Samsung matrix KBC binding in the kernel (Documentation/devicetree/bindings/input/samsung-keypad.txt) which is conceptually at the same level as having a "plain" table, although no "fn" table in their case. I prefer the proposed Tegra binding's table approach rather than having a separate node per key myself.
d) Shift/Ctrl/Alt/... modifier mapping tables should be specified by a separate binding that can be common to any/all keyboards (probably the same document as (b) above). The input to this table should be the raw codes from keycode-plain/keycode-fn. The output would be the values sent to whatever consumes keyboard input. The naming of these properties should make it obvious that it's something not specific to Tegra's KBC, and SW oriented. Perhaps something semantically similar to loadkeys' or xkbcomp's input, although I haven't looked at those in detail.
While I agree this would be nice, it involves adding a layer of software into U-Boot which doesn't currently exist (converting key codes + modifies into ASCII codes).
It doesn't have to be a separate lyaer in U-Boot; the binding just has to be defined in a way that doesn't tie it to the Tegra KBC driver, so it can be re-used.
In other words:
Tegra KBC's binding defines:
nvidia,keycode-plain nvidia,keycode-fn
(these should output "shift" and "ctrl" keycodes for later processing)
Generic key mapping binding defines:
modifier-mapping-shift modifier-mapping-ctrl
... which take as input the values generated by keycode-plain/keycode-fn and output the rewritten keycodes.
(the difference here is that the input to those tables is the output from the nvidia,keycode-foo tables, rather than the raw HW keycodes)
I'm not sure this passes the 'simple' test. But OK. If we limit it to characters <128 then we can avoid doubling the size of each mapping.
As far as implementation goes, all of that can be handled inside the Tegra KBC driver for now, so no new layer is required.
I think we should run this plan, and both binding definitions past the linux-input mailing list; people there will be far more aware of any previous work in this area, whether this plan makes sense, etc.
...
I think asking for a new input layer in U-Boot. But this does not exist at present. Don't you think this is taking things a little far? I am trying to upstream a hardware driver :-)
Yes, I'd like a clear documentation/naming separation between the HW-specific plain/fn keycode array and the higher-level shift/ctrl mappings. I certainly am not requesting that you create a separate input layer in U-Boot to handle those mappings; it can all be done in the Tegra KBC driver for now.
I would prefer to just leave it as it is and deal with the input layer when there is demand for it. But I'm OK with your plan.
My reading of the Tegra keyboard driver in the kernel is that it *could* use the keycode-plain and keycode-fn properties as given in this series. They would replace the tegra_kbc_default_keymap[] array.
Yes, that seems quite reasonable.
However, code would need to be added to create that array for use by the upper layers of linux keyboard input, since presumably it will be a while before the kernel's drivers/input/keyboard/matrix_keypad.c supports an fdt-based mapping. It certainly will not use a shift or ctrl mapping, since these are handled by upper layers of Linux input.
So after all this musing I see two options:
- Modify this series to remove 'shift' support and make 'ctrl' use a
calculated value (i.e. unlike the other two existing U-Boot drivers). We lose access to a number of symbols but I could hard-code a mapping in for the keyboard on Seaboard, say.
- a. Go with what we have, put a 'u-boot,' prefix on the
keycode-shift property and don't expect the kernel to ever use it. b. Start talking on the U-Boot list about the need for a middle input translation layer and a generic header file which defines key codes in a standardized way. Then write this layer, get it accepted and refactor the 3 keyboard drivers to use it.
I'd like to ask for comments in linux-input in case they have any other ideas, e.g. whether a set of separately documented modifier mapping properties makes sense. If not, I suppose the following set of properties would suffice:
nvidia,keycode-plain nvidia,keycode-fn u-boot,keycode-shift u-boot,keycode-ctrl
(although the last two seem to want both nvidia, and u-boot, prefixes)
OK. Since this seems to be a kernel issue and Nvidia-specific can I ask if you can please send this email? I will wait for confirmation that this is OK before going further.
Regards, Simon
-- nvpublic

On 12/12/2011 11:10 AM, Simon Glass wrote:
On Mon, Dec 12, 2011 at 10:00 AM, Stephen Warren swarren@nvidia.com wrote:
On 12/08/2011 02:56 PM, Simon Glass wrote:
...
I'd like to ask for comments in linux-input in case they have any other ideas, e.g. whether a set of separately documented modifier mapping properties makes sense. If not, I suppose the following set of properties would suffice:
nvidia,keycode-plain nvidia,keycode-fn u-boot,keycode-shift u-boot,keycode-ctrl
(although the last two seem to want both nvidia, and u-boot, prefixes)
OK. Since this seems to be a kernel issue and Nvidia-specific can I ask if you can please send this email? I will wait for confirmation that this is OK before going further.
OK, I'll send the email.
I would like to point out though that this is neither a kernel issue nor NVIDIA-specific: It's a device tree issue dealing with how to best represent certain HW features, and it will apply equally to any HW that has a keyboard with modifier keys; essentially all of them.

Hi Stephen,
On Mon, Dec 12, 2011 at 10:31 AM, Stephen Warren swarren@nvidia.com wrote:
On 12/12/2011 11:10 AM, Simon Glass wrote:
On Mon, Dec 12, 2011 at 10:00 AM, Stephen Warren swarren@nvidia.com wrote:
On 12/08/2011 02:56 PM, Simon Glass wrote:
...
I'd like to ask for comments in linux-input in case they have any other ideas, e.g. whether a set of separately documented modifier mapping properties makes sense. If not, I suppose the following set of properties would suffice:
nvidia,keycode-plain nvidia,keycode-fn u-boot,keycode-shift u-boot,keycode-ctrl
(although the last two seem to want both nvidia, and u-boot, prefixes)
OK. Since this seems to be a kernel issue and Nvidia-specific can I ask if you can please send this email? I will wait for confirmation that this is OK before going further.
OK, I'll send the email.
Thanks.
I would like to point out though that this is neither a kernel issue nor NVIDIA-specific: It's a device tree issue dealing with how to best represent certain HW features, and it will apply equally to any HW that has a keyboard with modifier keys; essentially all of them.
Sorry I don't quite understand what you mean by that, given your comments that we should ask on linux-input. While it could apply to the two existing keyboard drivers, they each do their own thing at present.
Regards, Simon
-- nvpublic

All tegra boards will use these options by default.
Signed-off-by: Simon Glass sjg@chromium.org --- include/configs/tegra2-common.h | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-common.h index 6549d00..054695c 100644 --- a/include/configs/tegra2-common.h +++ b/include/configs/tegra2-common.h @@ -116,11 +116,18 @@
#define CONFIG_SYS_NO_FLASH
-/* Environment information */ +/* Environment information, boards can override if required */ +#define CONFIG_CONSOLE_MUX +#define CONFIG_SYS_CONSOLE_IS_IN_ENV +#define TEGRA2_DEVICE_SETTINGS "stdin=serial\0" \ + "stdout=serial\0" \ + "stderr=serial\0" + #define CONFIG_EXTRA_ENV_SETTINGS \ "console=ttyS0,115200n8\0" \ "mem=" TEGRA2_SYSMEM "\0" \ "smpflag=smp\0" \ + TEGRA2_DEVICE_SETTINGS
#define CONFIG_LOADADDR 0x408000 /* def. location for kernel */ #define CONFIG_BOOTDELAY 2 /* -1 to disable auto boot */

This enables the standard keyboard on Seaboard. Signed-off-by: Simon Glass sjg@chromium.org --- include/configs/seaboard.h | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h index a105c88..4d2852e 100644 --- a/include/configs/seaboard.h +++ b/include/configs/seaboard.h @@ -87,4 +87,13 @@ #define CONFIG_USB_STORAGE #define CONFIG_CMD_USB
+/* Enable keyboard */ +#define CONFIG_TEGRA2_KEYBOARD +#define CONFIG_KEYBOARD + +#undef TEGRA2_DEVICE_SETTINGS +#define TEGRA2_DEVICE_SETTINGS "stdin=serial,tegra-kbc\0" \ + "stdout=serial\0" \ + "stderr=serial\0" + #endif /* __CONFIG_H */
participants (2)
-
Simon Glass
-
Stephen Warren