This series of patchsets contains the Broadcom keypad controller driver
and device tree binding documentation.
Scott Branden (2):
Input: bcm-keypad: add device tree bindings
Input: bcm-keypad: Add Broadcom keypad controller
.../devicetree/bindings/input/brcm,bcm-keypad.txt | 118 +++++
drivers/input/keyboard/Kconfig | 10 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/bcm-keypad.c | 489 +++++++++++++++++++++
4 files changed, 618 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
create mode 100644 drivers/input/keyboard/bcm-keypad.c
--
2.2.2
Documents the Broadcom keypad controller device tree bindings.
Reviewed-by: Ray Jui <[email protected]>
Signed-off-by: Scott Branden <[email protected]>
---
.../devicetree/bindings/input/brcm,bcm-keypad.txt | 118 +++++++++++++++++++++
1 file changed, 118 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
diff --git a/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
new file mode 100644
index 0000000..645829d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
@@ -0,0 +1,118 @@
+* Broadcom Keypad Controller device tree bindings
+
+Broadcom Keypad controller is used to interface a SoC with a matrix-type
+keypad device. The keypad controller supports multiple row and column lines.
+A key can be placed at each intersection of a unique row and a unique column.
+The keypad controller can sense a key-press and key-release and report the
+event using a interrupt to the cpu.
+
+This binding is based on the matrix-keymap binding with the following
+changes:
+
+keypad,num-rows and keypad,num-columns are required.
+
+Required SoC Specific Properties:
+- compatible: should be "brcm,bcm-keypad"
+
+- reg: physical base address of the controller and length of memory mapped
+ region.
+
+- interrupts: The interrupt number to the cpu.
+
+Board Specific Properties:
+- keypad,num-rows: Number of row lines connected to the keypad
+ controller.
+
+- keypad,num-columns: Number of column lines connected to the
+ keypad controller.
+
+- key-interrupt-trigger-type: The type of interrupt trigger asociated with the Keypad matrix.
+
+ KEYPAD_INTERRUPT_NO_TRIGGER = 0
+ KEYPAD_INTERRUPT_RISING_EDGE = 1
+ KEYPAD_INTERRUPT_FALLING_EDGE = 2
+ KEYPAD_INTERRUPT_BOTH_EDGES = 3
+
+- col-debounce-filter-period: The debounce period for the Column filter.
+
+ KEYPAD_DEBOUNCE_1_ms = 0
+ KEYPAD_DEBOUNCE_2_ms = 1
+ KEYPAD_DEBOUNCE_4_ms = 2
+ KEYPAD_DEBOUNCE_8_ms = 3
+ KEYPAD_DEBOUNCE_16_ms = 4
+ KEYPAD_DEBOUNCE_32_ms = 5
+ KEYPAD_DEBOUNCE_64_ms = 6
+ KEYPAD_DEBOUNCE_128_ms = 7
+
+- status-debounce-filter-period: The debounce period for the Status filter.
+
+ KEYPAD_DEBOUNCE_1_ms = 0
+ KEYPAD_DEBOUNCE_2_ms = 1
+ KEYPAD_DEBOUNCE_4_ms = 2
+ KEYPAD_DEBOUNCE_8_ms = 3
+ KEYPAD_DEBOUNCE_16_ms = 4
+ KEYPAD_DEBOUNCE_32_ms = 5
+ KEYPAD_DEBOUNCE_64_ms = 6
+ KEYPAD_DEBOUNCE_128_ms = 7
+
+- row-output-enabled: An optional property indicating whether the row or
+ column is being used as output. If specified the row is being used
+ as the output. Else defaults to column.
+
+- pull-up-enabled: An optional property indicating the Keypad scan mode.
+ If specified implies the keypad scan pull-up has been enabled.
+
+- Keys represented as child nodes: Each key connected to the keypad
+ controller is represented as a child node to the keypad controller
+ device node and should include the following properties.
+ - row: the row number to which the key is connected.
+ - column: the column number to which the key is connected.
+ - code: the key-code to be reported when the key is pressed
+ and released.
+
+Example:
+#include "dt-bindings/input/input.h"
+
+/ {
+ keypad: keypad@180ac000 {
+ /* Required SoC specific properties */
+ compatible = "brcm,bcm-keypad";
+
+ /* Required Board specific properties */
+ keypad,num-rows = <5>;
+ keypad,num-columns = <5>;
+ status = "okay";
+
+ linux,keymap = <MATRIX_KEY(0x00, 0x02, KEY_F) /* key_forward */
+ MATRIX_KEY(0x00, 0x03, KEY_HOME) /* key_home */
+ MATRIX_KEY(0x00, 0x04, KEY_M) /* key_message */
+ MATRIX_KEY(0x01, 0x00, KEY_A) /* key_contacts */
+ MATRIX_KEY(0x01, 0x01, KEY_1) /* key_1 */
+ MATRIX_KEY(0x01, 0x02, KEY_2) /* key_2 */
+ MATRIX_KEY(0x01, 0x03, KEY_3) /* key_3 */
+ MATRIX_KEY(0x01, 0x04, KEY_S) /* key_speaker */
+ MATRIX_KEY(0x02, 0x00, KEY_P) /* key_phone */
+ MATRIX_KEY(0x02, 0x01, KEY_4) /* key_4 */
+ MATRIX_KEY(0x02, 0x02, KEY_5) /* key_5 */
+ MATRIX_KEY(0x02, 0x03, KEY_6) /* key_6 */
+ MATRIX_KEY(0x02, 0x04, KEY_VOLUMEUP) /* key_vol_up */
+ MATRIX_KEY(0x03, 0x00, KEY_C) /* key_call_log */
+ MATRIX_KEY(0x03, 0x01, KEY_7) /* key_7 */
+ MATRIX_KEY(0x03, 0x02, KEY_8) /* key_8 */
+ MATRIX_KEY(0x03, 0x03, KEY_9) /* key_9 */
+ MATRIX_KEY(0x03, 0x04, KEY_VOLUMEDOWN) /* key_vol_down */
+ MATRIX_KEY(0x04, 0x00, KEY_H) /* key_headset */
+ MATRIX_KEY(0x04, 0x01, KEY_KPASTERISK) /* key_* */
+ MATRIX_KEY(0x04, 0x02, KEY_0) /* key_0 */
+ MATRIX_KEY(0x04, 0x03, KEY_GRAVE) /* key_# */
+ MATRIX_KEY(0x04, 0x04, KEY_MUTE) /* key_mute */
+ >;
+
+ /* Optional board specific properties */
+ key-interrupt-trigger-type = <3>;
+ col-debounce-filter-period = <5>;
+ row-output-enabled;
+ pull-up-enabled;
+
+ };
+};
--
2.2.2
Add driver for Broadcom's keypad controller.
Broadcom Keypad controller is used to interface a SoC with a matrix-type
keypad device. The keypad controller supports multiple row and column lines.
A key can be placed at each intersection of a unique row and a unique column.
The keypad controller can sense a key-press and key-release and report the
event using a interrupt to the cpu.
Reviewed-by: Ray Jui <[email protected]>
Signed-off-by: Scott Branden <[email protected]>
---
drivers/input/keyboard/Kconfig | 10 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/bcm-keypad.c | 489 ++++++++++++++++++++++++++++++++++++
3 files changed, 500 insertions(+)
create mode 100644 drivers/input/keyboard/bcm-keypad.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a5d9b3f..3a0c0f2 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -676,4 +676,14 @@ config KEYBOARD_CAP11XX
To compile this driver as a module, choose M here: the
module will be called cap11xx.
+config KEYBOARD_BCM
+ tristate "Broadcom keypad driver"
+ select INPUT_MATRIXKMAP
+ default ARCH_BCM_CYGNUS
+ help
+ Say Y here if you want to use Broadcom keypad.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bcm-keypad.
+
endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index febafa5..3cff8f6 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_KEYBOARD_ADP5589) += adp5589-keys.o
obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
+obj-$(CONFIG_KEYBOARD_BCM) += bcm-keypad.o
obj-$(CONFIG_KEYBOARD_BFIN) += bf54x-keys.o
obj-$(CONFIG_KEYBOARD_CAP11XX) += cap11xx.o
obj-$(CONFIG_KEYBOARD_CLPS711X) += clps711x-keypad.o
diff --git a/drivers/input/keyboard/bcm-keypad.c b/drivers/input/keyboard/bcm-keypad.c
new file mode 100644
index 0000000..b2c4bb7
--- /dev/null
+++ b/drivers/input/keyboard/bcm-keypad.c
@@ -0,0 +1,489 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * 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 version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/bitops.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/sizes.h>
+#include <linux/stddef.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+#include <linux/clk.h>
+#include <linux/types.h>
+#include <linux/string.h>
+#include <asm/memory.h>
+#include <linux/io.h>
+#include <linux/input/matrix_keypad.h>
+
+#define DEFAULT_CLK_HZ 31250
+/* Repeat period (ms) */
+#define KEY_REPEAT_PERIOD 100
+/* First time press dly (ms) */
+#define KEY_REPEAT_DELAY 400
+#define MAX_ROWS 8
+#define MAX_COLS 8
+
+/* Register/field definitions */
+#define KPCR_OFFSET 0x00000080
+#define KPCR_MODE 0x00000002
+#define KPCR_MODE_SHIFT 1
+#define KPCR_MODE_MASK 1
+#define KPCR_ENABLE 0x00000001
+#define KPCR_STATUSFILTERENABLE 0x00008000
+#define KPCR_STATUSFILTERTYPE_SHIFT 12
+#define KPCR_COLFILTERENABLE 0x00000800
+#define KPCR_COLFILTERTYPE_SHIFT 8
+#define KPCR_ROWWIDTH_SHIFT 20
+#define KPCR_COLUMNWIDTH_SHIFT 16
+
+#define KPIOR_OFFSET 0x00000084
+#define KPIOR_ROWOCONTRL_SHIFT 24
+#define KPIOR_ROWOCONTRL_MASK 0xFF000000
+#define KPIOR_COLUMNOCONTRL_SHIFT 16
+#define KPIOR_COLUMNOCONTRL_MASK 0x00FF0000
+#define KPIOR_COLUMN_IO_DATA_SHIFT 0
+
+#define KPEMR0_OFFSET 0x00000090
+#define KPEMR1_OFFSET 0x00000094
+#define KPEMR2_OFFSET 0x00000098
+#define KPEMR3_OFFSET 0x0000009C
+#define KPEMR_EDGETYPE_MAX 3
+
+#define KPSSR0_OFFSET 0x000000A0
+#define KPSSR1_OFFSET 0x000000A4
+#define KPIMR0_OFFSET 0x000000B0
+#define KPIMR1_OFFSET 0x000000B4
+#define KPICR0_OFFSET 0x000000B8
+#define KPICR1_OFFSET 0x000000BC
+#define KPISR0_OFFSET 0x000000C0
+#define KPISR1_OFFSET 0x000000C4
+
+#define KPCR_STATUSFILTERTYPE_MAX 7
+#define KPCR_COLFILTERTYPE_MAX 7
+
+/* Macros to determine the row/column from a bit that is set in SSR0/1. */
+#define BIT_TO_ROW_SSR0(bit_nr) (bit_nr >> 3)
+#define BIT_TO_ROW_SSR1(bit_nr) ((bit_nr >> 3) + 4)
+#define BIT_TO_COL(bit_nr) (bit_nr % 8)
+
+/* Structure representing various run-time entities */
+struct bcm_kp {
+ void __iomem *base;
+ int irq;
+ struct clk *clk;
+ struct input_dev *input_dev;
+ unsigned long last_state[2];
+ unsigned int n_rows;
+ unsigned int n_cols;
+ u32 kpcr;
+ u32 kpior;
+ u32 kpemr;
+ u32 imr0_val;
+ u32 imr1_val;
+};
+
+/*
+ * Returns the keycode from the input device keymap given the row and
+ * column.
+ */
+static inline int bcm_kp_get_keycode(struct bcm_kp *kp, int row, int col)
+{
+ unsigned int row_shift = get_count_order(kp->n_cols);
+ unsigned short *keymap = kp->input_dev->keycode;
+
+ return keymap[MATRIX_SCAN_CODE(row, col, row_shift)];
+}
+
+static irqreturn_t bcm_kp_isr_thread(int irq, void *dev_id)
+{
+ struct bcm_kp *kp = dev_id;
+ unsigned long state, change;
+ int bit_nr;
+ int pull_mode = (kp->kpcr >> KPCR_MODE_SHIFT) & KPCR_MODE_MASK;
+ int key_press;
+ int row, col;
+ unsigned int keycode;
+
+ /* Clear interrupts */
+ writel(0xFFFFFFFF, kp->base + KPICR0_OFFSET);
+ writel(0xFFFFFFFF, kp->base + KPICR1_OFFSET);
+
+ state = readl(kp->base + KPSSR0_OFFSET);
+ change = kp->last_state[0] ^ state;
+ kp->last_state[0] = state;
+
+ for_each_set_bit(bit_nr, &change, BITS_PER_LONG) {
+ key_press = state & BIT(bit_nr);
+ /* The meaning of SSR register depends on pull mode. */
+ key_press = pull_mode ? !key_press : key_press;
+ row = BIT_TO_ROW_SSR0(bit_nr);
+ col = BIT_TO_COL(bit_nr);
+ keycode = bcm_kp_get_keycode(kp, row, col);
+ input_report_key(kp->input_dev, keycode, key_press);
+ }
+
+ state = readl(kp->base + KPSSR1_OFFSET);
+ change = kp->last_state[1] ^ state;
+ kp->last_state[1] = state;
+
+ for_each_set_bit(bit_nr, &change, BITS_PER_LONG) {
+ key_press = state & BIT(bit_nr);
+ /* The meaning of SSR register depends on pull mode. */
+ key_press = pull_mode ? !key_press : key_press;
+ row = BIT_TO_ROW_SSR1(bit_nr);
+ col = BIT_TO_COL(bit_nr);
+ keycode = bcm_kp_get_keycode(kp, row, col);
+ input_report_key(kp->input_dev, keycode, key_press);
+ }
+
+ input_sync(kp->input_dev);
+
+ return IRQ_HANDLED;
+}
+
+static int bcm_kp_start(struct bcm_kp *kp)
+{
+ int error;
+
+ error = clk_prepare_enable(kp->clk);
+ if (error)
+ return error;
+
+ writel(kp->kpior, kp->base + KPIOR_OFFSET);
+
+ writel(kp->imr0_val, kp->base + KPIMR0_OFFSET);
+ writel(kp->imr1_val, kp->base + KPIMR1_OFFSET);
+
+ writel(kp->kpemr, kp->base + KPEMR0_OFFSET);
+ writel(kp->kpemr, kp->base + KPEMR1_OFFSET);
+ writel(kp->kpemr, kp->base + KPEMR2_OFFSET);
+ writel(kp->kpemr, kp->base + KPEMR3_OFFSET);
+
+ writel(0xFFFFFFFF, kp->base + KPICR0_OFFSET);
+ writel(0xFFFFFFFF, kp->base + KPICR1_OFFSET);
+
+ kp->last_state[0] = readl(kp->base + KPSSR0_OFFSET);
+ kp->last_state[0] = readl(kp->base + KPSSR1_OFFSET);
+
+ writel(kp->kpcr | KPCR_ENABLE, kp->base + KPCR_OFFSET);
+
+ return 0;
+}
+
+static void bcm_kp_stop(const struct bcm_kp *kp)
+{
+ u32 val;
+
+ val = readl(kp->base + KPCR_OFFSET);
+ val &= ~KPCR_ENABLE;
+ writel(0, kp->base + KPCR_OFFSET);
+ writel(0, kp->base + KPIMR0_OFFSET);
+ writel(0, kp->base + KPIMR1_OFFSET);
+ writel(0xFFFFFFFF, kp->base + KPICR0_OFFSET);
+ writel(0xFFFFFFFF, kp->base + KPICR1_OFFSET);
+
+ clk_disable_unprepare(kp->clk);
+}
+
+static int bcm_kp_open(struct input_dev *dev)
+{
+ struct bcm_kp *kp = input_get_drvdata(dev);
+
+ return bcm_kp_start(kp);
+}
+
+static void bcm_kp_close(struct input_dev *dev)
+{
+ struct bcm_kp *kp = input_get_drvdata(dev);
+
+ bcm_kp_stop(kp);
+}
+
+static int bcm_kp_matrix_key_parse_dt(struct bcm_kp *kp)
+{
+ struct device *dev = kp->input_dev->dev.parent;
+ struct device_node *np = dev->of_node;
+ int error;
+ unsigned int dt_val;
+ unsigned int i;
+
+ /* Initialize the KPCR Keypad Configuration Register */
+ kp->kpcr = KPCR_STATUSFILTERENABLE | KPCR_COLFILTERENABLE;
+
+ error = matrix_keypad_parse_of_params(dev, &kp->n_rows, &kp->n_cols);
+ if (error) {
+ dev_err(dev, "failed to parse kp params\n");
+ return error;
+ }
+ /* Set row width for the ASIC block. */
+ kp->kpcr |= (kp->n_rows - 1) << KPCR_ROWWIDTH_SHIFT;
+
+ /* Set column width for the ASIC block. */
+ kp->kpcr |= (kp->n_cols - 1) << KPCR_COLUMNWIDTH_SHIFT;
+
+ /* Configure the IMR registers */
+ {
+ unsigned int num_rows, col_mask, rows_set;
+
+ /* IMR registers contain interrupt enable bits for 8x8 matrix
+ * IMR0 register format: <row3> <row2> <row1> <row0>
+ * IMR1 register format: <row7> <row6> <row5> <row4>
+ */
+
+ col_mask = (1 << (kp->n_cols)) - 1;
+ num_rows = kp->n_rows;
+
+ /* Set column bits in rows 0 to 3 in IMR0 */
+ kp->imr0_val = col_mask;
+
+ rows_set = 1;
+ while ((--num_rows) && (rows_set++ < 4))
+ kp->imr0_val |= (kp->imr0_val << MAX_COLS);
+
+ /* Set column bits in rows 4 to 7 in IMR1 */
+ kp->imr1_val = 0;
+ if (num_rows) {
+ kp->imr1_val = col_mask;
+ while (--num_rows)
+ kp->imr1_val |= (kp->imr1_val << MAX_COLS);
+ }
+ }
+
+ /*
+ * Obtain the interrupt trigger type specified and verify against the
+ * possible values specified in the DT binding.
+ */
+ of_property_read_u32(np, "key-interrupt-trigger-type",
+ &dt_val);
+ if ((dt_val == 0) || (dt_val > KPEMR_EDGETYPE_MAX)) {
+ dev_err(dev, "Invalid interrupt trigger type %d\n", dt_val);
+ return -EINVAL;
+ }
+ /* Initialize the KPEMR Keypress Edge Mode Registers */
+ kp->kpemr = 0;
+ for (i = 0; i <= 30; i = i + 2)
+ kp->kpemr |= (dt_val << i);
+
+ /*
+ * Obtain the Status filter debounce value and verify against the
+ * possible values specified in the DT binding.
+ */
+ of_property_read_u32(np, "status-debounce-filter-period",
+ &dt_val);
+ if (dt_val > KPCR_STATUSFILTERTYPE_MAX) {
+ dev_err(dev, "Invalid Status filter debounce value %d\n",
+ dt_val);
+ return -EINVAL;
+ }
+ kp->kpcr |= dt_val << KPCR_STATUSFILTERTYPE_SHIFT;
+
+
+ /*
+ * Obtain the Column filter debounce value and verify against the
+ * possible values specified in the DT binding.
+ */
+ of_property_read_u32(np, "col-debounce-filter-period",
+ &dt_val);
+
+ if (dt_val > KPCR_COLFILTERTYPE_MAX) {
+ dev_err(dev, "Invalid Column filter debounce value %d\n",
+ dt_val);
+ return -EINVAL;
+ }
+ kp->kpcr |= dt_val << KPCR_COLFILTERTYPE_SHIFT;
+
+ /*
+ * Determine between the row and column,
+ * which should be configured as output.
+ */
+ if (of_property_read_bool(np, "row-output-enabled")) {
+ /*
+ * Set RowOContrl or ColumnOContrl in KPIOR
+ * to the number of pins to drive as outputs
+ */
+ kp->kpior = (((1 << kp->n_rows) - 1) <<
+ KPIOR_ROWOCONTRL_SHIFT);
+ } else {
+ kp->kpior = (((1 << kp->n_cols) - 1) <<
+ KPIOR_COLUMNOCONTRL_SHIFT);
+ }
+
+ /*
+ * Determine if the scan pull up needs to be enabled
+ */
+ if (of_property_read_bool(np, "pull-up-enabled"))
+ kp->kpcr |= KPCR_MODE;
+
+ dev_dbg(dev, "n_rows=%d n_col=%d kpcr=%x kpior=%x kpemr=%x\n",
+ kp->n_rows, kp->n_cols,
+ kp->kpcr, kp->kpior, kp->kpemr);
+
+ return 0;
+}
+
+
+static int bcm_kp_probe(struct platform_device *pdev)
+{
+ struct bcm_kp *kp;
+ struct input_dev *input_dev;
+ struct resource *res;
+ int error;
+
+ kp = devm_kzalloc(&pdev->dev, sizeof(*kp),
+ GFP_KERNEL);
+ if (!kp)
+ return -ENOMEM;
+
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev) {
+ dev_err(&pdev->dev, "failed to allocate the input device\n");
+ return -ENOMEM;
+ }
+ __set_bit(EV_KEY, input_dev->evbit);
+ __set_bit(EV_REP, input_dev->evbit);
+ input_dev->rep[REP_PERIOD] = KEY_REPEAT_PERIOD;
+ input_dev->rep[REP_DELAY] = KEY_REPEAT_DELAY;
+ input_dev->name = pdev->name;
+ input_dev->phys = "keypad/input0";
+ input_dev->dev.parent = &pdev->dev;
+ input_dev->open = bcm_kp_open;
+ input_dev->close = bcm_kp_close;
+
+ input_dev->id.bustype = BUS_HOST;
+ input_dev->id.vendor = 0x0001;
+ input_dev->id.product = 0x0001;
+ input_dev->id.version = 0x0100;
+
+ input_set_drvdata(input_dev, kp);
+
+ kp->input_dev = input_dev;
+
+ platform_set_drvdata(pdev, kp);
+
+ error = bcm_kp_matrix_key_parse_dt(kp);
+ if (error)
+ return error;
+
+ error = matrix_keypad_build_keymap(NULL, NULL,
+ kp->n_rows,
+ kp->n_cols,
+ NULL, input_dev);
+ if (error) {
+ dev_err(&pdev->dev, "failed to build keymap\n");
+ return error;
+ }
+
+ /* Get the KEYPAD base address */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "Missing keypad base address resource\n");
+ return -ENODEV;
+ }
+
+ kp->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(kp->base))
+ return PTR_ERR(kp->base);
+
+ /* Enable clock */
+
+ kp->clk = devm_clk_get(&pdev->dev, "peri_clk");
+ if (IS_ERR(kp->clk))
+ dev_info(&pdev->dev,
+ "No clock specified. Assuming it's enabled\n");
+ else {
+ unsigned int desired_rate;
+ long actual_rate;
+
+ error = of_property_read_u32(pdev->dev.of_node,
+ "clock-frequency", &desired_rate);
+ if (error < 0)
+ desired_rate = DEFAULT_CLK_HZ;
+
+ actual_rate = clk_round_rate(kp->clk, desired_rate);
+ if (actual_rate <= 0)
+ return -EINVAL;
+
+ error = clk_set_rate(kp->clk, actual_rate);
+ if (error)
+ return -EINVAL;
+
+ error = clk_prepare_enable(kp->clk);
+ if (error)
+ return -EINVAL;
+ }
+
+ /* Put the kp into a known sane state */
+ bcm_kp_stop(kp);
+
+ kp->irq = platform_get_irq(pdev, 0);
+ if (kp->irq < 0) {
+ dev_err(&pdev->dev, "no IRQ specified\n");
+ return -EINVAL;
+ }
+
+ error = devm_request_threaded_irq(&pdev->dev, kp->irq, NULL,
+ bcm_kp_isr_thread,
+ IRQF_ONESHOT, pdev->name, kp);
+ if (error) {
+ dev_err(&pdev->dev, "failed to request IRQ\n");
+ return error;
+ }
+
+ error = input_register_device(input_dev);
+ if (error) {
+ dev_err(&pdev->dev, "failed to register input device\n");
+ return error;
+ }
+
+ return 0;
+}
+
+/* Called to perform module cleanup when the module is unloaded. */
+static int bcm_kp_remove(struct platform_device *pdev)
+{
+ struct bcm_kp *kp = platform_get_drvdata(pdev);
+
+ pr_info("bcm_kp_remove\n");
+
+ /* Unregister everything */
+ input_unregister_device(kp->input_dev);
+ clk_disable_unprepare(kp->clk);
+
+ return 0;
+}
+
+static const struct of_device_id bcm_kp_of_match[] = {
+ { .compatible = "brcm,bcm-keypad" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, bcm_kp_of_match);
+
+static struct platform_driver bcm_kp_device_driver = {
+ .probe = bcm_kp_probe,
+ .remove = bcm_kp_remove,
+ .driver = {
+ .name = "bcm-keypad",
+ .of_match_table = of_match_ptr(bcm_kp_of_match),
+ }
+};
+
+module_platform_driver(bcm_kp_device_driver);
+
+MODULE_AUTHOR("Broadcom Corporation");
+MODULE_DESCRIPTION("BCM Keypad Driver");
+MODULE_LICENSE("GPL");
--
2.2.2
Hi Scott,
On Mon, Feb 09, 2015 at 04:07:40PM -0800, Scott Branden wrote:
> Documents the Broadcom keypad controller device tree bindings.
>
> Reviewed-by: Ray Jui <[email protected]>
> Signed-off-by: Scott Branden <[email protected]>
> ---
> .../devicetree/bindings/input/brcm,bcm-keypad.txt | 118 +++++++++++++++++++++
> 1 file changed, 118 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
>
> diff --git a/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
> new file mode 100644
> index 0000000..645829d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
> @@ -0,0 +1,118 @@
> +* Broadcom Keypad Controller device tree bindings
> +
> +Broadcom Keypad controller is used to interface a SoC with a matrix-type
> +keypad device. The keypad controller supports multiple row and column lines.
> +A key can be placed at each intersection of a unique row and a unique column.
> +The keypad controller can sense a key-press and key-release and report the
> +event using a interrupt to the cpu.
> +
> +This binding is based on the matrix-keymap binding with the following
> +changes:
> +
> +keypad,num-rows and keypad,num-columns are required.
> +
> +Required SoC Specific Properties:
> +- compatible: should be "brcm,bcm-keypad"
> +
> +- reg: physical base address of the controller and length of memory mapped
> + region.
> +
> +- interrupts: The interrupt number to the cpu.
> +
> +Board Specific Properties:
> +- keypad,num-rows: Number of row lines connected to the keypad
> + controller.
> +
> +- keypad,num-columns: Number of column lines connected to the
> + keypad controller.
> +
> +- key-interrupt-trigger-type: The type of interrupt trigger asociated with the Keypad matrix.
> +
> + KEYPAD_INTERRUPT_NO_TRIGGER = 0
> + KEYPAD_INTERRUPT_RISING_EDGE = 1
> + KEYPAD_INTERRUPT_FALLING_EDGE = 2
> + KEYPAD_INTERRUPT_BOTH_EDGES = 3
Can we get this data from the interrupt spec?
> +
> +- col-debounce-filter-period: The debounce period for the Column filter.
> +
> + KEYPAD_DEBOUNCE_1_ms = 0
> + KEYPAD_DEBOUNCE_2_ms = 1
> + KEYPAD_DEBOUNCE_4_ms = 2
> + KEYPAD_DEBOUNCE_8_ms = 3
> + KEYPAD_DEBOUNCE_16_ms = 4
> + KEYPAD_DEBOUNCE_32_ms = 5
> + KEYPAD_DEBOUNCE_64_ms = 6
> + KEYPAD_DEBOUNCE_128_ms = 7
> +
> +- status-debounce-filter-period: The debounce period for the Status filter.
> +
> + KEYPAD_DEBOUNCE_1_ms = 0
> + KEYPAD_DEBOUNCE_2_ms = 1
> + KEYPAD_DEBOUNCE_4_ms = 2
> + KEYPAD_DEBOUNCE_8_ms = 3
> + KEYPAD_DEBOUNCE_16_ms = 4
> + KEYPAD_DEBOUNCE_32_ms = 5
> + KEYPAD_DEBOUNCE_64_ms = 6
> + KEYPAD_DEBOUNCE_128_ms = 7
I could swear device-specific properties should be in form of
<vendor-prefix>,<property-name> to ensure it won't clash with changes on
subsystem level later on. Device-tree folks, what say you?
> +
> +- row-output-enabled: An optional property indicating whether the row or
> + column is being used as output. If specified the row is being used
> + as the output. Else defaults to column.
> +
> +- pull-up-enabled: An optional property indicating the Keypad scan mode.
> + If specified implies the keypad scan pull-up has been enabled.
> +
> +- Keys represented as child nodes: Each key connected to the keypad
> + controller is represented as a child node to the keypad controller
> + device node and should include the following properties.
> + - row: the row number to which the key is connected.
> + - column: the column number to which the key is connected.
> + - code: the key-code to be reported when the key is pressed
> + and released.
That does not seem to be right, linux,keymap is a list, not a subtree.
I'd simply refer to
Documentation/devicetree/bindings/input/matrix-keymap.txt for details.
> +
> +Example:
> +#include "dt-bindings/input/input.h"
> +
> +/ {
> + keypad: keypad@180ac000 {
> + /* Required SoC specific properties */
> + compatible = "brcm,bcm-keypad";
> +
> + /* Required Board specific properties */
> + keypad,num-rows = <5>;
> + keypad,num-columns = <5>;
> + status = "okay";
> +
> + linux,keymap = <MATRIX_KEY(0x00, 0x02, KEY_F) /* key_forward */
> + MATRIX_KEY(0x00, 0x03, KEY_HOME) /* key_home */
> + MATRIX_KEY(0x00, 0x04, KEY_M) /* key_message */
> + MATRIX_KEY(0x01, 0x00, KEY_A) /* key_contacts */
> + MATRIX_KEY(0x01, 0x01, KEY_1) /* key_1 */
> + MATRIX_KEY(0x01, 0x02, KEY_2) /* key_2 */
> + MATRIX_KEY(0x01, 0x03, KEY_3) /* key_3 */
> + MATRIX_KEY(0x01, 0x04, KEY_S) /* key_speaker */
> + MATRIX_KEY(0x02, 0x00, KEY_P) /* key_phone */
> + MATRIX_KEY(0x02, 0x01, KEY_4) /* key_4 */
> + MATRIX_KEY(0x02, 0x02, KEY_5) /* key_5 */
> + MATRIX_KEY(0x02, 0x03, KEY_6) /* key_6 */
> + MATRIX_KEY(0x02, 0x04, KEY_VOLUMEUP) /* key_vol_up */
> + MATRIX_KEY(0x03, 0x00, KEY_C) /* key_call_log */
> + MATRIX_KEY(0x03, 0x01, KEY_7) /* key_7 */
> + MATRIX_KEY(0x03, 0x02, KEY_8) /* key_8 */
> + MATRIX_KEY(0x03, 0x03, KEY_9) /* key_9 */
> + MATRIX_KEY(0x03, 0x04, KEY_VOLUMEDOWN) /* key_vol_down */
> + MATRIX_KEY(0x04, 0x00, KEY_H) /* key_headset */
> + MATRIX_KEY(0x04, 0x01, KEY_KPASTERISK) /* key_* */
> + MATRIX_KEY(0x04, 0x02, KEY_0) /* key_0 */
> + MATRIX_KEY(0x04, 0x03, KEY_GRAVE) /* key_# */
> + MATRIX_KEY(0x04, 0x04, KEY_MUTE) /* key_mute */
> + >;
> +
> + /* Optional board specific properties */
> + key-interrupt-trigger-type = <3>;
> + col-debounce-filter-period = <5>;
> + row-output-enabled;
> + pull-up-enabled;
> +
> + };
> +};
> --
> 2.2.2
>
Thanks.
--
Dmitry
Hi Scott,
On Mon, Feb 09, 2015 at 04:07:41PM -0800, Scott Branden wrote:
> Add driver for Broadcom's keypad controller.
>
> Broadcom Keypad controller is used to interface a SoC with a matrix-type
> keypad device. The keypad controller supports multiple row and column lines.
> A key can be placed at each intersection of a unique row and a unique column.
> The keypad controller can sense a key-press and key-release and report the
> event using a interrupt to the cpu.
>
> Reviewed-by: Ray Jui <[email protected]>
> Signed-off-by: Scott Branden <[email protected]>
> ---
> drivers/input/keyboard/Kconfig | 10 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/bcm-keypad.c | 489 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 500 insertions(+)
> create mode 100644 drivers/input/keyboard/bcm-keypad.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index a5d9b3f..3a0c0f2 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -676,4 +676,14 @@ config KEYBOARD_CAP11XX
> To compile this driver as a module, choose M here: the
> module will be called cap11xx.
>
> +config KEYBOARD_BCM
> + tristate "Broadcom keypad driver"
> + select INPUT_MATRIXKMAP
> + default ARCH_BCM_CYGNUS
> + help
> + Say Y here if you want to use Broadcom keypad.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called bcm-keypad.
> +
> endif
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index febafa5..3cff8f6 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_KEYBOARD_ADP5589) += adp5589-keys.o
> obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
> obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
> obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
> +obj-$(CONFIG_KEYBOARD_BCM) += bcm-keypad.o
> obj-$(CONFIG_KEYBOARD_BFIN) += bf54x-keys.o
> obj-$(CONFIG_KEYBOARD_CAP11XX) += cap11xx.o
> obj-$(CONFIG_KEYBOARD_CLPS711X) += clps711x-keypad.o
> diff --git a/drivers/input/keyboard/bcm-keypad.c b/drivers/input/keyboard/bcm-keypad.c
> new file mode 100644
> index 0000000..b2c4bb7
> --- /dev/null
> +++ b/drivers/input/keyboard/bcm-keypad.c
> @@ -0,0 +1,489 @@
> +/*
> + * Copyright (C) 2014 Broadcom Corporation
> + *
> + * 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 version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/bitops.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +#include <linux/sizes.h>
> +#include <linux/stddef.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/slab.h>
> +#include <linux/clk.h>
> +#include <linux/types.h>
> +#include <linux/string.h>
> +#include <asm/memory.h>
> +#include <linux/io.h>
> +#include <linux/input/matrix_keypad.h>
> +
> +#define DEFAULT_CLK_HZ 31250
> +/* Repeat period (ms) */
> +#define KEY_REPEAT_PERIOD 100
> +/* First time press dly (ms) */
> +#define KEY_REPEAT_DELAY 400
> +#define MAX_ROWS 8
> +#define MAX_COLS 8
> +
> +/* Register/field definitions */
> +#define KPCR_OFFSET 0x00000080
> +#define KPCR_MODE 0x00000002
> +#define KPCR_MODE_SHIFT 1
> +#define KPCR_MODE_MASK 1
> +#define KPCR_ENABLE 0x00000001
> +#define KPCR_STATUSFILTERENABLE 0x00008000
> +#define KPCR_STATUSFILTERTYPE_SHIFT 12
> +#define KPCR_COLFILTERENABLE 0x00000800
> +#define KPCR_COLFILTERTYPE_SHIFT 8
> +#define KPCR_ROWWIDTH_SHIFT 20
> +#define KPCR_COLUMNWIDTH_SHIFT 16
> +
> +#define KPIOR_OFFSET 0x00000084
> +#define KPIOR_ROWOCONTRL_SHIFT 24
> +#define KPIOR_ROWOCONTRL_MASK 0xFF000000
> +#define KPIOR_COLUMNOCONTRL_SHIFT 16
> +#define KPIOR_COLUMNOCONTRL_MASK 0x00FF0000
> +#define KPIOR_COLUMN_IO_DATA_SHIFT 0
> +
> +#define KPEMR0_OFFSET 0x00000090
> +#define KPEMR1_OFFSET 0x00000094
> +#define KPEMR2_OFFSET 0x00000098
> +#define KPEMR3_OFFSET 0x0000009C
> +#define KPEMR_EDGETYPE_MAX 3
> +
> +#define KPSSR0_OFFSET 0x000000A0
> +#define KPSSR1_OFFSET 0x000000A4
> +#define KPIMR0_OFFSET 0x000000B0
> +#define KPIMR1_OFFSET 0x000000B4
> +#define KPICR0_OFFSET 0x000000B8
> +#define KPICR1_OFFSET 0x000000BC
> +#define KPISR0_OFFSET 0x000000C0
> +#define KPISR1_OFFSET 0x000000C4
> +
> +#define KPCR_STATUSFILTERTYPE_MAX 7
> +#define KPCR_COLFILTERTYPE_MAX 7
> +
> +/* Macros to determine the row/column from a bit that is set in SSR0/1. */
> +#define BIT_TO_ROW_SSR0(bit_nr) (bit_nr >> 3)
> +#define BIT_TO_ROW_SSR1(bit_nr) ((bit_nr >> 3) + 4)
> +#define BIT_TO_COL(bit_nr) (bit_nr % 8)
> +
> +/* Structure representing various run-time entities */
> +struct bcm_kp {
> + void __iomem *base;
> + int irq;
> + struct clk *clk;
> + struct input_dev *input_dev;
> + unsigned long last_state[2];
> + unsigned int n_rows;
> + unsigned int n_cols;
> + u32 kpcr;
> + u32 kpior;
> + u32 kpemr;
> + u32 imr0_val;
> + u32 imr1_val;
> +};
> +
> +/*
> + * Returns the keycode from the input device keymap given the row and
> + * column.
> + */
> +static inline int bcm_kp_get_keycode(struct bcm_kp *kp, int row, int col)
> +{
> + unsigned int row_shift = get_count_order(kp->n_cols);
> + unsigned short *keymap = kp->input_dev->keycode;
> +
> + return keymap[MATRIX_SCAN_CODE(row, col, row_shift)];
> +}
> +
> +static irqreturn_t bcm_kp_isr_thread(int irq, void *dev_id)
> +{
> + struct bcm_kp *kp = dev_id;
> + unsigned long state, change;
> + int bit_nr;
> + int pull_mode = (kp->kpcr >> KPCR_MODE_SHIFT) & KPCR_MODE_MASK;
> + int key_press;
> + int row, col;
> + unsigned int keycode;
> +
> + /* Clear interrupts */
> + writel(0xFFFFFFFF, kp->base + KPICR0_OFFSET);
> + writel(0xFFFFFFFF, kp->base + KPICR1_OFFSET);
> +
> + state = readl(kp->base + KPSSR0_OFFSET);
> + change = kp->last_state[0] ^ state;
> + kp->last_state[0] = state;
> +
> + for_each_set_bit(bit_nr, &change, BITS_PER_LONG) {
> + key_press = state & BIT(bit_nr);
> + /* The meaning of SSR register depends on pull mode. */
> + key_press = pull_mode ? !key_press : key_press;
> + row = BIT_TO_ROW_SSR0(bit_nr);
> + col = BIT_TO_COL(bit_nr);
> + keycode = bcm_kp_get_keycode(kp, row, col);
> + input_report_key(kp->input_dev, keycode, key_press);
> + }
> +
> + state = readl(kp->base + KPSSR1_OFFSET);
> + change = kp->last_state[1] ^ state;
> + kp->last_state[1] = state;
> +
> + for_each_set_bit(bit_nr, &change, BITS_PER_LONG) {
> + key_press = state & BIT(bit_nr);
> + /* The meaning of SSR register depends on pull mode. */
> + key_press = pull_mode ? !key_press : key_press;
> + row = BIT_TO_ROW_SSR1(bit_nr);
> + col = BIT_TO_COL(bit_nr);
> + keycode = bcm_kp_get_keycode(kp, row, col);
> + input_report_key(kp->input_dev, keycode, key_press);
> + }
Please split it into a separate function that processes one bank and
call it twice instead of doing cut-and-paste of almost the same code. Or
even better combine it all into one block (i.e have changes[2], use
bitmap_xor to figure difference, etc...).
> +
> + input_sync(kp->input_dev);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int bcm_kp_start(struct bcm_kp *kp)
> +{
> + int error;
> +
> + error = clk_prepare_enable(kp->clk);
> + if (error)
> + return error;
> +
> + writel(kp->kpior, kp->base + KPIOR_OFFSET);
> +
> + writel(kp->imr0_val, kp->base + KPIMR0_OFFSET);
> + writel(kp->imr1_val, kp->base + KPIMR1_OFFSET);
> +
> + writel(kp->kpemr, kp->base + KPEMR0_OFFSET);
> + writel(kp->kpemr, kp->base + KPEMR1_OFFSET);
> + writel(kp->kpemr, kp->base + KPEMR2_OFFSET);
> + writel(kp->kpemr, kp->base + KPEMR3_OFFSET);
> +
> + writel(0xFFFFFFFF, kp->base + KPICR0_OFFSET);
> + writel(0xFFFFFFFF, kp->base + KPICR1_OFFSET);
> +
> + kp->last_state[0] = readl(kp->base + KPSSR0_OFFSET);
> + kp->last_state[0] = readl(kp->base + KPSSR1_OFFSET);
> +
> + writel(kp->kpcr | KPCR_ENABLE, kp->base + KPCR_OFFSET);
> +
> + return 0;
> +}
> +
> +static void bcm_kp_stop(const struct bcm_kp *kp)
> +{
> + u32 val;
> +
> + val = readl(kp->base + KPCR_OFFSET);
> + val &= ~KPCR_ENABLE;
> + writel(0, kp->base + KPCR_OFFSET);
> + writel(0, kp->base + KPIMR0_OFFSET);
> + writel(0, kp->base + KPIMR1_OFFSET);
> + writel(0xFFFFFFFF, kp->base + KPICR0_OFFSET);
> + writel(0xFFFFFFFF, kp->base + KPICR1_OFFSET);
> +
> + clk_disable_unprepare(kp->clk);
> +}
> +
> +static int bcm_kp_open(struct input_dev *dev)
> +{
> + struct bcm_kp *kp = input_get_drvdata(dev);
> +
> + return bcm_kp_start(kp);
> +}
> +
> +static void bcm_kp_close(struct input_dev *dev)
> +{
> + struct bcm_kp *kp = input_get_drvdata(dev);
> +
> + bcm_kp_stop(kp);
> +}
> +
> +static int bcm_kp_matrix_key_parse_dt(struct bcm_kp *kp)
> +{
> + struct device *dev = kp->input_dev->dev.parent;
> + struct device_node *np = dev->of_node;
> + int error;
> + unsigned int dt_val;
> + unsigned int i;
> +
> + /* Initialize the KPCR Keypad Configuration Register */
> + kp->kpcr = KPCR_STATUSFILTERENABLE | KPCR_COLFILTERENABLE;
> +
> + error = matrix_keypad_parse_of_params(dev, &kp->n_rows, &kp->n_cols);
> + if (error) {
> + dev_err(dev, "failed to parse kp params\n");
> + return error;
> + }
> + /* Set row width for the ASIC block. */
> + kp->kpcr |= (kp->n_rows - 1) << KPCR_ROWWIDTH_SHIFT;
> +
> + /* Set column width for the ASIC block. */
> + kp->kpcr |= (kp->n_cols - 1) << KPCR_COLUMNWIDTH_SHIFT;
> +
> + /* Configure the IMR registers */
> + {
> + unsigned int num_rows, col_mask, rows_set;
> +
> + /* IMR registers contain interrupt enable bits for 8x8 matrix
> + * IMR0 register format: <row3> <row2> <row1> <row0>
> + * IMR1 register format: <row7> <row6> <row5> <row4>
> + */
> +
> + col_mask = (1 << (kp->n_cols)) - 1;
> + num_rows = kp->n_rows;
> +
> + /* Set column bits in rows 0 to 3 in IMR0 */
> + kp->imr0_val = col_mask;
> +
> + rows_set = 1;
> + while ((--num_rows) && (rows_set++ < 4))
> + kp->imr0_val |= (kp->imr0_val << MAX_COLS);
> +
> + /* Set column bits in rows 4 to 7 in IMR1 */
> + kp->imr1_val = 0;
> + if (num_rows) {
> + kp->imr1_val = col_mask;
> + while (--num_rows)
> + kp->imr1_val |= (kp->imr1_val << MAX_COLS);
> + }
> + }
> +
> + /*
> + * Obtain the interrupt trigger type specified and verify against the
> + * possible values specified in the DT binding.
> + */
> + of_property_read_u32(np, "key-interrupt-trigger-type",
> + &dt_val);
> + if ((dt_val == 0) || (dt_val > KPEMR_EDGETYPE_MAX)) {
> + dev_err(dev, "Invalid interrupt trigger type %d\n", dt_val);
> + return -EINVAL;
> + }
> + /* Initialize the KPEMR Keypress Edge Mode Registers */
> + kp->kpemr = 0;
> + for (i = 0; i <= 30; i = i + 2)
> + kp->kpemr |= (dt_val << i);
> +
> + /*
> + * Obtain the Status filter debounce value and verify against the
> + * possible values specified in the DT binding.
> + */
> + of_property_read_u32(np, "status-debounce-filter-period",
> + &dt_val);
> + if (dt_val > KPCR_STATUSFILTERTYPE_MAX) {
> + dev_err(dev, "Invalid Status filter debounce value %d\n",
> + dt_val);
> + return -EINVAL;
> + }
> + kp->kpcr |= dt_val << KPCR_STATUSFILTERTYPE_SHIFT;
> +
> +
> + /*
> + * Obtain the Column filter debounce value and verify against the
> + * possible values specified in the DT binding.
> + */
> + of_property_read_u32(np, "col-debounce-filter-period",
> + &dt_val);
> +
> + if (dt_val > KPCR_COLFILTERTYPE_MAX) {
> + dev_err(dev, "Invalid Column filter debounce value %d\n",
> + dt_val);
> + return -EINVAL;
> + }
> + kp->kpcr |= dt_val << KPCR_COLFILTERTYPE_SHIFT;
> +
> + /*
> + * Determine between the row and column,
> + * which should be configured as output.
> + */
> + if (of_property_read_bool(np, "row-output-enabled")) {
> + /*
> + * Set RowOContrl or ColumnOContrl in KPIOR
> + * to the number of pins to drive as outputs
> + */
> + kp->kpior = (((1 << kp->n_rows) - 1) <<
> + KPIOR_ROWOCONTRL_SHIFT);
> + } else {
> + kp->kpior = (((1 << kp->n_cols) - 1) <<
> + KPIOR_COLUMNOCONTRL_SHIFT);
> + }
> +
> + /*
> + * Determine if the scan pull up needs to be enabled
> + */
> + if (of_property_read_bool(np, "pull-up-enabled"))
> + kp->kpcr |= KPCR_MODE;
> +
> + dev_dbg(dev, "n_rows=%d n_col=%d kpcr=%x kpior=%x kpemr=%x\n",
> + kp->n_rows, kp->n_cols,
> + kp->kpcr, kp->kpior, kp->kpemr);
> +
> + return 0;
> +}
> +
> +
> +static int bcm_kp_probe(struct platform_device *pdev)
> +{
> + struct bcm_kp *kp;
> + struct input_dev *input_dev;
> + struct resource *res;
> + int error;
> +
> + kp = devm_kzalloc(&pdev->dev, sizeof(*kp),
> + GFP_KERNEL);
> + if (!kp)
> + return -ENOMEM;
> +
> + input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!input_dev) {
> + dev_err(&pdev->dev, "failed to allocate the input device\n");
> + return -ENOMEM;
> + }
> + __set_bit(EV_KEY, input_dev->evbit);
> + __set_bit(EV_REP, input_dev->evbit);
> + input_dev->rep[REP_PERIOD] = KEY_REPEAT_PERIOD;
> + input_dev->rep[REP_DELAY] = KEY_REPEAT_DELAY;
That does not do what you think: if you set up custom delay and period
you are supposed to supply your own timer as well. Either adjust values
after registering input device, or leave defaults.
Also it might be a good idea to have "linux,autorepeat" DT property to
control it.
> + input_dev->name = pdev->name;
> + input_dev->phys = "keypad/input0";
> + input_dev->dev.parent = &pdev->dev;
> + input_dev->open = bcm_kp_open;
> + input_dev->close = bcm_kp_close;
> +
> + input_dev->id.bustype = BUS_HOST;
> + input_dev->id.vendor = 0x0001;
> + input_dev->id.product = 0x0001;
> + input_dev->id.version = 0x0100;
> +
> + input_set_drvdata(input_dev, kp);
> +
> + kp->input_dev = input_dev;
> +
> + platform_set_drvdata(pdev, kp);
> +
> + error = bcm_kp_matrix_key_parse_dt(kp);
> + if (error)
> + return error;
> +
> + error = matrix_keypad_build_keymap(NULL, NULL,
> + kp->n_rows,
> + kp->n_cols,
> + NULL, input_dev);
> + if (error) {
> + dev_err(&pdev->dev, "failed to build keymap\n");
> + return error;
> + }
> +
> + /* Get the KEYPAD base address */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "Missing keypad base address resource\n");
> + return -ENODEV;
> + }
> +
> + kp->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(kp->base))
> + return PTR_ERR(kp->base);
> +
> + /* Enable clock */
> +
> + kp->clk = devm_clk_get(&pdev->dev, "peri_clk");
> + if (IS_ERR(kp->clk))
> + dev_info(&pdev->dev,
> + "No clock specified. Assuming it's enabled\n");
> + else {
> + unsigned int desired_rate;
> + long actual_rate;
> +
> + error = of_property_read_u32(pdev->dev.of_node,
> + "clock-frequency", &desired_rate);
> + if (error < 0)
> + desired_rate = DEFAULT_CLK_HZ;
> +
> + actual_rate = clk_round_rate(kp->clk, desired_rate);
> + if (actual_rate <= 0)
> + return -EINVAL;
> +
> + error = clk_set_rate(kp->clk, actual_rate);
> + if (error)
> + return -EINVAL;
> +
> + error = clk_prepare_enable(kp->clk);
> + if (error)
> + return -EINVAL;
> + }
> +
> + /* Put the kp into a known sane state */
> + bcm_kp_stop(kp);
> +
> + kp->irq = platform_get_irq(pdev, 0);
> + if (kp->irq < 0) {
> + dev_err(&pdev->dev, "no IRQ specified\n");
> + return -EINVAL;
> + }
> +
> + error = devm_request_threaded_irq(&pdev->dev, kp->irq, NULL,
> + bcm_kp_isr_thread,
> + IRQF_ONESHOT, pdev->name, kp);
> + if (error) {
> + dev_err(&pdev->dev, "failed to request IRQ\n");
> + return error;
> + }
> +
> + error = input_register_device(input_dev);
> + if (error) {
> + dev_err(&pdev->dev, "failed to register input device\n");
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +/* Called to perform module cleanup when the module is unloaded. */
> +static int bcm_kp_remove(struct platform_device *pdev)
> +{
> + struct bcm_kp *kp = platform_get_drvdata(pdev);
> +
> + pr_info("bcm_kp_remove\n");
> +
> + /* Unregister everything */
> + input_unregister_device(kp->input_dev);
> + clk_disable_unprepare(kp->clk);
You already calling clk_disable_unprepare() as part of bcm_kp_close()
(in stop()), so you do not need it here. input_unregister_device() will
also be called automatically since you are using managed input device,
so please remove bcm_kp_remove altogether.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id bcm_kp_of_match[] = {
> + { .compatible = "brcm,bcm-keypad" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, bcm_kp_of_match);
> +
> +static struct platform_driver bcm_kp_device_driver = {
> + .probe = bcm_kp_probe,
> + .remove = bcm_kp_remove,
> + .driver = {
> + .name = "bcm-keypad",
> + .of_match_table = of_match_ptr(bcm_kp_of_match),
> + }
> +};
> +
> +module_platform_driver(bcm_kp_device_driver);
> +
> +MODULE_AUTHOR("Broadcom Corporation");
> +MODULE_DESCRIPTION("BCM Keypad Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.2.2
>
Thanks.
--
Dmitry
Hi Dmitry,
Comments inline. I still have an issue with vendor prefix as there are
not documented guidelines I can find in this area?
On 15-02-09 04:51 PM, Dmitry Torokhov wrote:
> Hi Scott,
>
> On Mon, Feb 09, 2015 at 04:07:40PM -0800, Scott Branden wrote:
>> Documents the Broadcom keypad controller device tree bindings.
>>
>> Reviewed-by: Ray Jui <[email protected]>
>> Signed-off-by: Scott Branden <[email protected]>
>> ---
>> .../devicetree/bindings/input/brcm,bcm-keypad.txt | 118 +++++++++++++++++++++
>> 1 file changed, 118 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
>>
>> diff --git a/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
>> new file mode 100644
>> index 0000000..645829d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
>> @@ -0,0 +1,118 @@
>> +* Broadcom Keypad Controller device tree bindings
>> +
>> +Broadcom Keypad controller is used to interface a SoC with a matrix-type
>> +keypad device. The keypad controller supports multiple row and column lines.
>> +A key can be placed at each intersection of a unique row and a unique column.
>> +The keypad controller can sense a key-press and key-release and report the
>> +event using a interrupt to the cpu.
>> +
>> +This binding is based on the matrix-keymap binding with the following
>> +changes:
>> +
>> +keypad,num-rows and keypad,num-columns are required.
>> +
>> +Required SoC Specific Properties:
>> +- compatible: should be "brcm,bcm-keypad"
>> +
>> +- reg: physical base address of the controller and length of memory mapped
>> + region.
>> +
>> +- interrupts: The interrupt number to the cpu.
>> +
>> +Board Specific Properties:
>> +- keypad,num-rows: Number of row lines connected to the keypad
>> + controller.
>> +
>> +- keypad,num-columns: Number of column lines connected to the
>> + keypad controller.
>> +
>> +- key-interrupt-trigger-type: The type of interrupt trigger asociated with the Keypad matrix.
>> +
>> + KEYPAD_INTERRUPT_NO_TRIGGER = 0
>> + KEYPAD_INTERRUPT_RISING_EDGE = 1
>> + KEYPAD_INTERRUPT_FALLING_EDGE = 2
>> + KEYPAD_INTERRUPT_BOTH_EDGES = 3
>
> Can we get this data from the interrupt spec?
I don't understand your question. Could you elaborate?
But, looking at this closer this determines when the hardware should
generate interrupts. I think we would always need to set it to both
edges and this binding option can probably we removed?
>
>> +
>> +- col-debounce-filter-period: The debounce period for the Column filter.
>> +
>> + KEYPAD_DEBOUNCE_1_ms = 0
>> + KEYPAD_DEBOUNCE_2_ms = 1
>> + KEYPAD_DEBOUNCE_4_ms = 2
>> + KEYPAD_DEBOUNCE_8_ms = 3
>
>> + KEYPAD_DEBOUNCE_16_ms = 4
>> + KEYPAD_DEBOUNCE_32_ms = 5
>> + KEYPAD_DEBOUNCE_64_ms = 6
>> + KEYPAD_DEBOUNCE_128_ms = 7
>> +
>> +- status-debounce-filter-period: The debounce period for the Status filter.
>> +
>> + KEYPAD_DEBOUNCE_1_ms = 0
>> + KEYPAD_DEBOUNCE_2_ms = 1
>> + KEYPAD_DEBOUNCE_4_ms = 2
>> + KEYPAD_DEBOUNCE_8_ms = 3
>> + KEYPAD_DEBOUNCE_16_ms = 4
>> + KEYPAD_DEBOUNCE_32_ms = 5
>> + KEYPAD_DEBOUNCE_64_ms = 6
>> + KEYPAD_DEBOUNCE_128_ms = 7
>
> I could swear device-specific properties should be in form of
> <vendor-prefix>,<property-name> to ensure it won't clash with changes on
> subsystem level later on. Device-tree folks, what say you?
I see examples with and without vendor-prefix.
qcom,pm8xxx-keypad.txt does not have prefixes
st-keyscan.txt does have a prefix
I can't find any documented guidelines for this.
Clash changes should not happen because as new standard properties are
added the drivers should be adjusted to use the new dt-bindings?
>
>> +
>> +- row-output-enabled: An optional property indicating whether the row or
>> + column is being used as output. If specified the row is being used
>> + as the output. Else defaults to column.
>> +
>> +- pull-up-enabled: An optional property indicating the Keypad scan mode.
>> + If specified implies the keypad scan pull-up has been enabled.
>> +
>> +- Keys represented as child nodes: Each key connected to the keypad
>> + controller is represented as a child node to the keypad controller
>> + device node and should include the following properties.
>> + - row: the row number to which the key is connected.
>> + - column: the column number to which the key is connected.
>> + - code: the key-code to be reported when the key is pressed
>> + and released.
>
> That does not seem to be right, linux,keymap is a list, not a subtree.
> I'd simply refer to
> Documentation/devicetree/bindings/input/matrix-keymap.txt for details.
Yes, we moved to matrix-keymap and forgot to update this documentation.
Will correct - thanks.
>
>> +
>> +Example:
>> +#include "dt-bindings/input/input.h"
>> +
>> +/ {
>> + keypad: keypad@180ac000 {
>> + /* Required SoC specific properties */
>> + compatible = "brcm,bcm-keypad";
>> +
>> + /* Required Board specific properties */
>> + keypad,num-rows = <5>;
>> + keypad,num-columns = <5>;
>> + status = "okay";
>> +
>> + linux,keymap = <MATRIX_KEY(0x00, 0x02, KEY_F) /* key_forward */
>> + MATRIX_KEY(0x00, 0x03, KEY_HOME) /* key_home */
>> + MATRIX_KEY(0x00, 0x04, KEY_M) /* key_message */
>> + MATRIX_KEY(0x01, 0x00, KEY_A) /* key_contacts */
>> + MATRIX_KEY(0x01, 0x01, KEY_1) /* key_1 */
>> + MATRIX_KEY(0x01, 0x02, KEY_2) /* key_2 */
>> + MATRIX_KEY(0x01, 0x03, KEY_3) /* key_3 */
>> + MATRIX_KEY(0x01, 0x04, KEY_S) /* key_speaker */
>> + MATRIX_KEY(0x02, 0x00, KEY_P) /* key_phone */
>> + MATRIX_KEY(0x02, 0x01, KEY_4) /* key_4 */
>> + MATRIX_KEY(0x02, 0x02, KEY_5) /* key_5 */
>> + MATRIX_KEY(0x02, 0x03, KEY_6) /* key_6 */
>> + MATRIX_KEY(0x02, 0x04, KEY_VOLUMEUP) /* key_vol_up */
>> + MATRIX_KEY(0x03, 0x00, KEY_C) /* key_call_log */
>> + MATRIX_KEY(0x03, 0x01, KEY_7) /* key_7 */
>> + MATRIX_KEY(0x03, 0x02, KEY_8) /* key_8 */
>> + MATRIX_KEY(0x03, 0x03, KEY_9) /* key_9 */
>> + MATRIX_KEY(0x03, 0x04, KEY_VOLUMEDOWN) /* key_vol_down */
>> + MATRIX_KEY(0x04, 0x00, KEY_H) /* key_headset */
>> + MATRIX_KEY(0x04, 0x01, KEY_KPASTERISK) /* key_* */
>> + MATRIX_KEY(0x04, 0x02, KEY_0) /* key_0 */
>> + MATRIX_KEY(0x04, 0x03, KEY_GRAVE) /* key_# */
>> + MATRIX_KEY(0x04, 0x04, KEY_MUTE) /* key_mute */
>> + >;
>> +
>> + /* Optional board specific properties */
>> + key-interrupt-trigger-type = <3>;
>> + col-debounce-filter-period = <5>;
>> + row-output-enabled;
>> + pull-up-enabled;
>> +
>> + };
>> +};
>> --
>> 2.2.2
>>
>
> Thanks.
>
Hi Dmitry,
Good review. Thanks. Commented inline.
On 15-02-09 05:02 PM, Dmitry Torokhov wrote:
> Hi Scott,
>
> On Mon, Feb 09, 2015 at 04:07:41PM -0800, Scott Branden wrote:
>> Add driver for Broadcom's keypad controller.
>>
>> Broadcom Keypad controller is used to interface a SoC with a matrix-type
>> keypad device. The keypad controller supports multiple row and column lines.
>> A key can be placed at each intersection of a unique row and a unique column.
>> The keypad controller can sense a key-press and key-release and report the
>> event using a interrupt to the cpu.
>>
>> Reviewed-by: Ray Jui <[email protected]>
>> Signed-off-by: Scott Branden <[email protected]>
>> ---
>> drivers/input/keyboard/Kconfig | 10 +
>> drivers/input/keyboard/Makefile | 1 +
>> drivers/input/keyboard/bcm-keypad.c | 489 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 500 insertions(+)
>> create mode 100644 drivers/input/keyboard/bcm-keypad.c
>>
>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>> index a5d9b3f..3a0c0f2 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -676,4 +676,14 @@ config KEYBOARD_CAP11XX
>> To compile this driver as a module, choose M here: the
>> module will be called cap11xx.
>>
>> +config KEYBOARD_BCM
>> + tristate "Broadcom keypad driver"
>> + select INPUT_MATRIXKMAP
>> + default ARCH_BCM_CYGNUS
>> + help
>> + Say Y here if you want to use Broadcom keypad.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called bcm-keypad.
>> +
>> endif
>> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
>> index febafa5..3cff8f6 100644
>> --- a/drivers/input/keyboard/Makefile
>> +++ b/drivers/input/keyboard/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_KEYBOARD_ADP5589) += adp5589-keys.o
>> obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
>> obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
>> obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
>> +obj-$(CONFIG_KEYBOARD_BCM) += bcm-keypad.o
>> obj-$(CONFIG_KEYBOARD_BFIN) += bf54x-keys.o
>> obj-$(CONFIG_KEYBOARD_CAP11XX) += cap11xx.o
>> obj-$(CONFIG_KEYBOARD_CLPS711X) += clps711x-keypad.o
>> diff --git a/drivers/input/keyboard/bcm-keypad.c b/drivers/input/keyboard/bcm-keypad.c
>> new file mode 100644
>> index 0000000..b2c4bb7
>> --- /dev/null
>> +++ b/drivers/input/keyboard/bcm-keypad.c
>> @@ -0,0 +1,489 @@
>> +/*
>> + * Copyright (C) 2014 Broadcom Corporation
>> + *
>> + * 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 version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/mm.h>
>> +#include <linux/module.h>
>> +#include <linux/input.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/bitops.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/delay.h>
>> +#include <linux/sizes.h>
>> +#include <linux/stddef.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/slab.h>
>> +#include <linux/clk.h>
>> +#include <linux/types.h>
>> +#include <linux/string.h>
>> +#include <asm/memory.h>
>> +#include <linux/io.h>
>> +#include <linux/input/matrix_keypad.h>
>> +
>> +#define DEFAULT_CLK_HZ 31250
>> +/* Repeat period (ms) */
>> +#define KEY_REPEAT_PERIOD 100
>> +/* First time press dly (ms) */
>> +#define KEY_REPEAT_DELAY 400
>> +#define MAX_ROWS 8
>> +#define MAX_COLS 8
>> +
>> +/* Register/field definitions */
>> +#define KPCR_OFFSET 0x00000080
>> +#define KPCR_MODE 0x00000002
>> +#define KPCR_MODE_SHIFT 1
>> +#define KPCR_MODE_MASK 1
>> +#define KPCR_ENABLE 0x00000001
>> +#define KPCR_STATUSFILTERENABLE 0x00008000
>> +#define KPCR_STATUSFILTERTYPE_SHIFT 12
>> +#define KPCR_COLFILTERENABLE 0x00000800
>> +#define KPCR_COLFILTERTYPE_SHIFT 8
>> +#define KPCR_ROWWIDTH_SHIFT 20
>> +#define KPCR_COLUMNWIDTH_SHIFT 16
>> +
>> +#define KPIOR_OFFSET 0x00000084
>> +#define KPIOR_ROWOCONTRL_SHIFT 24
>> +#define KPIOR_ROWOCONTRL_MASK 0xFF000000
>> +#define KPIOR_COLUMNOCONTRL_SHIFT 16
>> +#define KPIOR_COLUMNOCONTRL_MASK 0x00FF0000
>> +#define KPIOR_COLUMN_IO_DATA_SHIFT 0
>> +
>> +#define KPEMR0_OFFSET 0x00000090
>> +#define KPEMR1_OFFSET 0x00000094
>> +#define KPEMR2_OFFSET 0x00000098
>> +#define KPEMR3_OFFSET 0x0000009C
>> +#define KPEMR_EDGETYPE_MAX 3
>> +
>> +#define KPSSR0_OFFSET 0x000000A0
>> +#define KPSSR1_OFFSET 0x000000A4
>> +#define KPIMR0_OFFSET 0x000000B0
>> +#define KPIMR1_OFFSET 0x000000B4
>> +#define KPICR0_OFFSET 0x000000B8
>> +#define KPICR1_OFFSET 0x000000BC
>> +#define KPISR0_OFFSET 0x000000C0
>> +#define KPISR1_OFFSET 0x000000C4
>> +
>> +#define KPCR_STATUSFILTERTYPE_MAX 7
>> +#define KPCR_COLFILTERTYPE_MAX 7
>> +
>> +/* Macros to determine the row/column from a bit that is set in SSR0/1. */
>> +#define BIT_TO_ROW_SSR0(bit_nr) (bit_nr >> 3)
>> +#define BIT_TO_ROW_SSR1(bit_nr) ((bit_nr >> 3) + 4)
>> +#define BIT_TO_COL(bit_nr) (bit_nr % 8)
>> +
>> +/* Structure representing various run-time entities */
>> +struct bcm_kp {
>> + void __iomem *base;
>> + int irq;
>> + struct clk *clk;
>> + struct input_dev *input_dev;
>> + unsigned long last_state[2];
>> + unsigned int n_rows;
>> + unsigned int n_cols;
>> + u32 kpcr;
>> + u32 kpior;
>> + u32 kpemr;
>> + u32 imr0_val;
>> + u32 imr1_val;
>> +};
>> +
>> +/*
>> + * Returns the keycode from the input device keymap given the row and
>> + * column.
>> + */
>> +static inline int bcm_kp_get_keycode(struct bcm_kp *kp, int row, int col)
>> +{
>> + unsigned int row_shift = get_count_order(kp->n_cols);
>> + unsigned short *keymap = kp->input_dev->keycode;
>> +
>> + return keymap[MATRIX_SCAN_CODE(row, col, row_shift)];
>> +}
>> +
>> +static irqreturn_t bcm_kp_isr_thread(int irq, void *dev_id)
>> +{
>> + struct bcm_kp *kp = dev_id;
>> + unsigned long state, change;
>> + int bit_nr;
>> + int pull_mode = (kp->kpcr >> KPCR_MODE_SHIFT) & KPCR_MODE_MASK;
>> + int key_press;
>> + int row, col;
>> + unsigned int keycode;
>> +
>> + /* Clear interrupts */
>> + writel(0xFFFFFFFF, kp->base + KPICR0_OFFSET);
>> + writel(0xFFFFFFFF, kp->base + KPICR1_OFFSET);
>> +
>> + state = readl(kp->base + KPSSR0_OFFSET);
>> + change = kp->last_state[0] ^ state;
>> + kp->last_state[0] = state;
>> +
>> + for_each_set_bit(bit_nr, &change, BITS_PER_LONG) {
>> + key_press = state & BIT(bit_nr);
>> + /* The meaning of SSR register depends on pull mode. */
>> + key_press = pull_mode ? !key_press : key_press;
>> + row = BIT_TO_ROW_SSR0(bit_nr);
>> + col = BIT_TO_COL(bit_nr);
>> + keycode = bcm_kp_get_keycode(kp, row, col);
>> + input_report_key(kp->input_dev, keycode, key_press);
>> + }
>> +
>> + state = readl(kp->base + KPSSR1_OFFSET);
>> + change = kp->last_state[1] ^ state;
>> + kp->last_state[1] = state;
>> +
>> + for_each_set_bit(bit_nr, &change, BITS_PER_LONG) {
>> + key_press = state & BIT(bit_nr);
>> + /* The meaning of SSR register depends on pull mode. */
>> + key_press = pull_mode ? !key_press : key_press;
>> + row = BIT_TO_ROW_SSR1(bit_nr);
>> + col = BIT_TO_COL(bit_nr);
>> + keycode = bcm_kp_get_keycode(kp, row, col);
>> + input_report_key(kp->input_dev, keycode, key_press);
>> + }
>
> Please split it into a separate function that processes one bank and
> call it twice instead of doing cut-and-paste of almost the same code. Or
> even better combine it all into one block (i.e have changes[2], use
> bitmap_xor to figure difference, etc...).
>
OK. It was a few lines of code called twice but I'll change it.
>> +
>> + input_sync(kp->input_dev);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int bcm_kp_start(struct bcm_kp *kp)
>> +{
>> + int error;
>> +
>> + error = clk_prepare_enable(kp->clk);
>> + if (error)
>> + return error;
>> +
>> + writel(kp->kpior, kp->base + KPIOR_OFFSET);
>> +
>> + writel(kp->imr0_val, kp->base + KPIMR0_OFFSET);
>> + writel(kp->imr1_val, kp->base + KPIMR1_OFFSET);
>> +
>> + writel(kp->kpemr, kp->base + KPEMR0_OFFSET);
>> + writel(kp->kpemr, kp->base + KPEMR1_OFFSET);
>> + writel(kp->kpemr, kp->base + KPEMR2_OFFSET);
>> + writel(kp->kpemr, kp->base + KPEMR3_OFFSET);
>> +
>> + writel(0xFFFFFFFF, kp->base + KPICR0_OFFSET);
>> + writel(0xFFFFFFFF, kp->base + KPICR1_OFFSET);
>> +
>> + kp->last_state[0] = readl(kp->base + KPSSR0_OFFSET);
>> + kp->last_state[0] = readl(kp->base + KPSSR1_OFFSET);
>> +
>> + writel(kp->kpcr | KPCR_ENABLE, kp->base + KPCR_OFFSET);
>> +
>> + return 0;
>> +}
>> +
>> +static void bcm_kp_stop(const struct bcm_kp *kp)
>> +{
>> + u32 val;
>> +
>> + val = readl(kp->base + KPCR_OFFSET);
>> + val &= ~KPCR_ENABLE;
>> + writel(0, kp->base + KPCR_OFFSET);
>> + writel(0, kp->base + KPIMR0_OFFSET);
>> + writel(0, kp->base + KPIMR1_OFFSET);
>> + writel(0xFFFFFFFF, kp->base + KPICR0_OFFSET);
>> + writel(0xFFFFFFFF, kp->base + KPICR1_OFFSET);
>> +
>> + clk_disable_unprepare(kp->clk);
>> +}
>> +
>> +static int bcm_kp_open(struct input_dev *dev)
>> +{
>> + struct bcm_kp *kp = input_get_drvdata(dev);
>> +
>> + return bcm_kp_start(kp);
>> +}
>> +
>> +static void bcm_kp_close(struct input_dev *dev)
>> +{
>> + struct bcm_kp *kp = input_get_drvdata(dev);
>> +
>> + bcm_kp_stop(kp);
>> +}
>> +
>> +static int bcm_kp_matrix_key_parse_dt(struct bcm_kp *kp)
>> +{
>> + struct device *dev = kp->input_dev->dev.parent;
>> + struct device_node *np = dev->of_node;
>> + int error;
>> + unsigned int dt_val;
>> + unsigned int i;
>> +
>> + /* Initialize the KPCR Keypad Configuration Register */
>> + kp->kpcr = KPCR_STATUSFILTERENABLE | KPCR_COLFILTERENABLE;
>> +
>> + error = matrix_keypad_parse_of_params(dev, &kp->n_rows, &kp->n_cols);
>> + if (error) {
>> + dev_err(dev, "failed to parse kp params\n");
>> + return error;
>> + }
>> + /* Set row width for the ASIC block. */
>> + kp->kpcr |= (kp->n_rows - 1) << KPCR_ROWWIDTH_SHIFT;
>> +
>> + /* Set column width for the ASIC block. */
>> + kp->kpcr |= (kp->n_cols - 1) << KPCR_COLUMNWIDTH_SHIFT;
>> +
>> + /* Configure the IMR registers */
>> + {
>> + unsigned int num_rows, col_mask, rows_set;
>> +
>> + /* IMR registers contain interrupt enable bits for 8x8 matrix
>> + * IMR0 register format: <row3> <row2> <row1> <row0>
>> + * IMR1 register format: <row7> <row6> <row5> <row4>
>> + */
>> +
>> + col_mask = (1 << (kp->n_cols)) - 1;
>> + num_rows = kp->n_rows;
>> +
>> + /* Set column bits in rows 0 to 3 in IMR0 */
>> + kp->imr0_val = col_mask;
>> +
>> + rows_set = 1;
>> + while ((--num_rows) && (rows_set++ < 4))
>> + kp->imr0_val |= (kp->imr0_val << MAX_COLS);
>> +
>> + /* Set column bits in rows 4 to 7 in IMR1 */
>> + kp->imr1_val = 0;
>> + if (num_rows) {
>> + kp->imr1_val = col_mask;
>> + while (--num_rows)
>> + kp->imr1_val |= (kp->imr1_val << MAX_COLS);
>> + }
>> + }
>> +
>> + /*
>> + * Obtain the interrupt trigger type specified and verify against the
>> + * possible values specified in the DT binding.
>> + */
>> + of_property_read_u32(np, "key-interrupt-trigger-type",
>> + &dt_val);
>> + if ((dt_val == 0) || (dt_val > KPEMR_EDGETYPE_MAX)) {
>> + dev_err(dev, "Invalid interrupt trigger type %d\n", dt_val);
>> + return -EINVAL;
>> + }
>> + /* Initialize the KPEMR Keypress Edge Mode Registers */
>> + kp->kpemr = 0;
>> + for (i = 0; i <= 30; i = i + 2)
>> + kp->kpemr |= (dt_val << i);
>> +
>> + /*
>> + * Obtain the Status filter debounce value and verify against the
>> + * possible values specified in the DT binding.
>> + */
>> + of_property_read_u32(np, "status-debounce-filter-period",
>> + &dt_val);
>> + if (dt_val > KPCR_STATUSFILTERTYPE_MAX) {
>> + dev_err(dev, "Invalid Status filter debounce value %d\n",
>> + dt_val);
>> + return -EINVAL;
>> + }
>> + kp->kpcr |= dt_val << KPCR_STATUSFILTERTYPE_SHIFT;
>> +
>> +
>> + /*
>> + * Obtain the Column filter debounce value and verify against the
>> + * possible values specified in the DT binding.
>> + */
>> + of_property_read_u32(np, "col-debounce-filter-period",
>> + &dt_val);
>> +
>> + if (dt_val > KPCR_COLFILTERTYPE_MAX) {
>> + dev_err(dev, "Invalid Column filter debounce value %d\n",
>> + dt_val);
>> + return -EINVAL;
>> + }
>> + kp->kpcr |= dt_val << KPCR_COLFILTERTYPE_SHIFT;
>> +
>> + /*
>> + * Determine between the row and column,
>> + * which should be configured as output.
>> + */
>> + if (of_property_read_bool(np, "row-output-enabled")) {
>> + /*
>> + * Set RowOContrl or ColumnOContrl in KPIOR
>> + * to the number of pins to drive as outputs
>> + */
>> + kp->kpior = (((1 << kp->n_rows) - 1) <<
>> + KPIOR_ROWOCONTRL_SHIFT);
>> + } else {
>> + kp->kpior = (((1 << kp->n_cols) - 1) <<
>> + KPIOR_COLUMNOCONTRL_SHIFT);
>> + }
>> +
>> + /*
>> + * Determine if the scan pull up needs to be enabled
>> + */
>> + if (of_property_read_bool(np, "pull-up-enabled"))
>> + kp->kpcr |= KPCR_MODE;
>> +
>> + dev_dbg(dev, "n_rows=%d n_col=%d kpcr=%x kpior=%x kpemr=%x\n",
>> + kp->n_rows, kp->n_cols,
>> + kp->kpcr, kp->kpior, kp->kpemr);
>> +
>> + return 0;
>> +}
>> +
>> +
>> +static int bcm_kp_probe(struct platform_device *pdev)
>> +{
>> + struct bcm_kp *kp;
>> + struct input_dev *input_dev;
>> + struct resource *res;
>> + int error;
>> +
>> + kp = devm_kzalloc(&pdev->dev, sizeof(*kp),
>> + GFP_KERNEL);
>> + if (!kp)
>> + return -ENOMEM;
>> +
>> + input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!input_dev) {
>> + dev_err(&pdev->dev, "failed to allocate the input device\n");
>> + return -ENOMEM;
>> + }
>> + __set_bit(EV_KEY, input_dev->evbit);
>> + __set_bit(EV_REP, input_dev->evbit);
>> + input_dev->rep[REP_PERIOD] = KEY_REPEAT_PERIOD;
>> + input_dev->rep[REP_DELAY] = KEY_REPEAT_DELAY;
>
> That does not do what you think: if you set up custom delay and period
> you are supposed to supply your own timer as well. Either adjust values
> after registering input device, or leave defaults.
>
> Also it might be a good idea to have "linux,autorepeat" DT property to
> control it.
>
I'll look into this in more detail to understand.
>> + input_dev->name = pdev->name;
>> + input_dev->phys = "keypad/input0";
>> + input_dev->dev.parent = &pdev->dev;
>> + input_dev->open = bcm_kp_open;
>> + input_dev->close = bcm_kp_close;
>> +
>> + input_dev->id.bustype = BUS_HOST;
>> + input_dev->id.vendor = 0x0001;
>> + input_dev->id.product = 0x0001;
>> + input_dev->id.version = 0x0100;
>> +
>> + input_set_drvdata(input_dev, kp);
>> +
>> + kp->input_dev = input_dev;
>> +
>> + platform_set_drvdata(pdev, kp);
>> +
>> + error = bcm_kp_matrix_key_parse_dt(kp);
>> + if (error)
>> + return error;
>> +
>> + error = matrix_keypad_build_keymap(NULL, NULL,
>> + kp->n_rows,
>> + kp->n_cols,
>> + NULL, input_dev);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to build keymap\n");
>> + return error;
>> + }
>> +
>> + /* Get the KEYPAD base address */
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res) {
>> + dev_err(&pdev->dev, "Missing keypad base address resource\n");
>> + return -ENODEV;
>> + }
>> +
>> + kp->base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(kp->base))
>> + return PTR_ERR(kp->base);
>> +
>> + /* Enable clock */
>> +
>> + kp->clk = devm_clk_get(&pdev->dev, "peri_clk");
>> + if (IS_ERR(kp->clk))
>> + dev_info(&pdev->dev,
>> + "No clock specified. Assuming it's enabled\n");
>> + else {
>> + unsigned int desired_rate;
>> + long actual_rate;
>> +
>> + error = of_property_read_u32(pdev->dev.of_node,
>> + "clock-frequency", &desired_rate);
>> + if (error < 0)
>> + desired_rate = DEFAULT_CLK_HZ;
>> +
>> + actual_rate = clk_round_rate(kp->clk, desired_rate);
>> + if (actual_rate <= 0)
>> + return -EINVAL;
>> +
>> + error = clk_set_rate(kp->clk, actual_rate);
>> + if (error)
>> + return -EINVAL;
>> +
>> + error = clk_prepare_enable(kp->clk);
>> + if (error)
>> + return -EINVAL;
>> + }
>> +
>> + /* Put the kp into a known sane state */
>> + bcm_kp_stop(kp);
>> +
>> + kp->irq = platform_get_irq(pdev, 0);
>> + if (kp->irq < 0) {
>> + dev_err(&pdev->dev, "no IRQ specified\n");
>> + return -EINVAL;
>> + }
>> +
>> + error = devm_request_threaded_irq(&pdev->dev, kp->irq, NULL,
>> + bcm_kp_isr_thread,
>> + IRQF_ONESHOT, pdev->name, kp);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to request IRQ\n");
>> + return error;
>> + }
>> +
>> + error = input_register_device(input_dev);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to register input device\n");
>> + return error;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/* Called to perform module cleanup when the module is unloaded. */
>> +static int bcm_kp_remove(struct platform_device *pdev)
>> +{
>> + struct bcm_kp *kp = platform_get_drvdata(pdev);
>> +
>> + pr_info("bcm_kp_remove\n");
>> +
>> + /* Unregister everything */
>> + input_unregister_device(kp->input_dev);
>> + clk_disable_unprepare(kp->clk);
>
> You already calling clk_disable_unprepare() as part of bcm_kp_close()
> (in stop()), so you do not need it here. input_unregister_device() will
> also be called automatically since you are using managed input device,
> so please remove bcm_kp_remove altogether.
>
OK, will remove. That's an easy thing to do.
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id bcm_kp_of_match[] = {
>> + { .compatible = "brcm,bcm-keypad" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, bcm_kp_of_match);
>> +
>> +static struct platform_driver bcm_kp_device_driver = {
>> + .probe = bcm_kp_probe,
>> + .remove = bcm_kp_remove,
>> + .driver = {
>> + .name = "bcm-keypad",
>> + .of_match_table = of_match_ptr(bcm_kp_of_match),
>> + }
>> +};
>> +
>> +module_platform_driver(bcm_kp_device_driver);
>> +
>> +MODULE_AUTHOR("Broadcom Corporation");
>> +MODULE_DESCRIPTION("BCM Keypad Driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.2.2
>>
>
> Thanks.
>
Hi Scott,
On Sat, Feb 14, 2015 at 08:49:15AM -0800, Scott Branden wrote:
> Hi Dmitry,
>
> Comments inline. I still have an issue with vendor prefix as there
> are not documented guidelines I can find in this area?
I looked and I could not locate it written down either. I think Grant is
in Santa Rosa this week, I'll ask him to clarify.
>
>
> On 15-02-09 04:51 PM, Dmitry Torokhov wrote:
> >Hi Scott,
> >
> >On Mon, Feb 09, 2015 at 04:07:40PM -0800, Scott Branden wrote:
> >>Documents the Broadcom keypad controller device tree bindings.
> >>
> >>Reviewed-by: Ray Jui <[email protected]>
> >>Signed-off-by: Scott Branden <[email protected]>
> >>---
> >> .../devicetree/bindings/input/brcm,bcm-keypad.txt | 118 +++++++++++++++++++++
> >> 1 file changed, 118 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
> >>
> >>diff --git a/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
> >>new file mode 100644
> >>index 0000000..645829d
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
> >>@@ -0,0 +1,118 @@
> >>+* Broadcom Keypad Controller device tree bindings
> >>+
> >>+Broadcom Keypad controller is used to interface a SoC with a matrix-type
> >>+keypad device. The keypad controller supports multiple row and column lines.
> >>+A key can be placed at each intersection of a unique row and a unique column.
> >>+The keypad controller can sense a key-press and key-release and report the
> >>+event using a interrupt to the cpu.
> >>+
> >>+This binding is based on the matrix-keymap binding with the following
> >>+changes:
> >>+
> >>+keypad,num-rows and keypad,num-columns are required.
> >>+
> >>+Required SoC Specific Properties:
> >>+- compatible: should be "brcm,bcm-keypad"
> >>+
> >>+- reg: physical base address of the controller and length of memory mapped
> >>+ region.
> >>+
> >>+- interrupts: The interrupt number to the cpu.
> >>+
> >>+Board Specific Properties:
> >>+- keypad,num-rows: Number of row lines connected to the keypad
> >>+ controller.
> >>+
> >>+- keypad,num-columns: Number of column lines connected to the
> >>+ keypad controller.
> >>+
> >>+- key-interrupt-trigger-type: The type of interrupt trigger asociated with the Keypad matrix.
> >>+
> >>+ KEYPAD_INTERRUPT_NO_TRIGGER = 0
> >>+ KEYPAD_INTERRUPT_RISING_EDGE = 1
> >>+ KEYPAD_INTERRUPT_FALLING_EDGE = 2
> >>+ KEYPAD_INTERRUPT_BOTH_EDGES = 3
> >
> >Can we get this data from the interrupt spec?
> I don't understand your question. Could you elaborate?
> But, looking at this closer this determines when the hardware should
> generate interrupts. I think we would always need to set it to both
> edges and this binding option can probably we removed?
What I meant that interrupt binding allows specifying the trigger and
you have separate binding for trigger here. It would be nice to have
just one (the standard interrupt binding).
> >
> >>+
> >>+- col-debounce-filter-period: The debounce period for the Column filter.
> >>+
> >>+ KEYPAD_DEBOUNCE_1_ms = 0
> >>+ KEYPAD_DEBOUNCE_2_ms = 1
> >>+ KEYPAD_DEBOUNCE_4_ms = 2
> >>+ KEYPAD_DEBOUNCE_8_ms = 3
> >
> >>+ KEYPAD_DEBOUNCE_16_ms = 4
> >>+ KEYPAD_DEBOUNCE_32_ms = 5
> >>+ KEYPAD_DEBOUNCE_64_ms = 6
> >>+ KEYPAD_DEBOUNCE_128_ms = 7
> >>+
> >>+- status-debounce-filter-period: The debounce period for the Status filter.
> >>+
> >>+ KEYPAD_DEBOUNCE_1_ms = 0
> >>+ KEYPAD_DEBOUNCE_2_ms = 1
> >>+ KEYPAD_DEBOUNCE_4_ms = 2
> >>+ KEYPAD_DEBOUNCE_8_ms = 3
> >>+ KEYPAD_DEBOUNCE_16_ms = 4
> >>+ KEYPAD_DEBOUNCE_32_ms = 5
> >>+ KEYPAD_DEBOUNCE_64_ms = 6
> >>+ KEYPAD_DEBOUNCE_128_ms = 7
> >
> >I could swear device-specific properties should be in form of
> ><vendor-prefix>,<property-name> to ensure it won't clash with changes on
> >subsystem level later on. Device-tree folks, what say you?
> I see examples with and without vendor-prefix.
> qcom,pm8xxx-keypad.txt does not have prefixes
> st-keyscan.txt does have a prefix
>
> I can't find any documented guidelines for this.
As I mentioned I'll try to get clarification on this.
> Clash changes should not happen because as new standard properties
> are added the drivers should be adjusted to use the new dt-bindings?
This is a misconception: device tree bindings are supposed to form ABI
(and the goal to eventually separate them from the kernel) and so we
need to support old bindings in new kernels.
Thanks.
--
Dmitry
Hi Scott,
On Sun, Feb 15, 2015 at 09:17:51PM -0800, Dmitry Torokhov wrote:
> On Sat, Feb 14, 2015 at 08:49:15AM -0800, Scott Branden wrote:
> > On 15-02-09 04:51 PM, Dmitry Torokhov wrote:
> > >On Mon, Feb 09, 2015 at 04:07:40PM -0800, Scott Branden wrote:
> > >>+
> > >>+- col-debounce-filter-period: The debounce period for the Column filter.
> > >>+
> > >>+ KEYPAD_DEBOUNCE_1_ms = 0
> > >>+ KEYPAD_DEBOUNCE_2_ms = 1
> > >>+ KEYPAD_DEBOUNCE_4_ms = 2
> > >>+ KEYPAD_DEBOUNCE_8_ms = 3
> > >
> > >>+ KEYPAD_DEBOUNCE_16_ms = 4
> > >>+ KEYPAD_DEBOUNCE_32_ms = 5
> > >>+ KEYPAD_DEBOUNCE_64_ms = 6
> > >>+ KEYPAD_DEBOUNCE_128_ms = 7
> > >>+
> > >>+- status-debounce-filter-period: The debounce period for the Status filter.
> > >>+
> > >>+ KEYPAD_DEBOUNCE_1_ms = 0
> > >>+ KEYPAD_DEBOUNCE_2_ms = 1
> > >>+ KEYPAD_DEBOUNCE_4_ms = 2
> > >>+ KEYPAD_DEBOUNCE_8_ms = 3
> > >>+ KEYPAD_DEBOUNCE_16_ms = 4
> > >>+ KEYPAD_DEBOUNCE_32_ms = 5
> > >>+ KEYPAD_DEBOUNCE_64_ms = 6
> > >>+ KEYPAD_DEBOUNCE_128_ms = 7
> > >
> > >I could swear device-specific properties should be in form of
> > ><vendor-prefix>,<property-name> to ensure it won't clash with changes on
> > >subsystem level later on. Device-tree folks, what say you?
> > I see examples with and without vendor-prefix.
> > qcom,pm8xxx-keypad.txt does not have prefixes
> > st-keyscan.txt does have a prefix
> >
> > I can't find any documented guidelines for this.
>
> As I mentioned I'll try to get clarification on this.
I have chatted with a couple of people on this and it is acceptable to
omit vendor prefix in bindings when we are using a specific driver like
we have here (i.e. when driver's compatible string already includes
vendor prefix). Vendor prefixes on properties are required when we
augment a generic driver's binding.
So the above 2 entries are fine as is.
Thanks.
--
Dmitry
Hi Dmitry,
Thanks for the update of no need to change any of dt-binding prefixes.
I just sent out a v2 patch addressing all of your other comments.
On 15-02-23 09:49 AM, Dmitry Torokhov wrote:
> Hi Scott,
>
> On Sun, Feb 15, 2015 at 09:17:51PM -0800, Dmitry Torokhov wrote:
>> On Sat, Feb 14, 2015 at 08:49:15AM -0800, Scott Branden wrote:
>>> On 15-02-09 04:51 PM, Dmitry Torokhov wrote:
>>>> On Mon, Feb 09, 2015 at 04:07:40PM -0800, Scott Branden wrote:
>>>>> +
>>>>> +- col-debounce-filter-period: The debounce period for the Column filter.
>>>>> +
>>>>> + KEYPAD_DEBOUNCE_1_ms = 0
>>>>> + KEYPAD_DEBOUNCE_2_ms = 1
>>>>> + KEYPAD_DEBOUNCE_4_ms = 2
>>>>> + KEYPAD_DEBOUNCE_8_ms = 3
>>>>
>>>>> + KEYPAD_DEBOUNCE_16_ms = 4
>>>>> + KEYPAD_DEBOUNCE_32_ms = 5
>>>>> + KEYPAD_DEBOUNCE_64_ms = 6
>>>>> + KEYPAD_DEBOUNCE_128_ms = 7
>>>>> +
>>>>> +- status-debounce-filter-period: The debounce period for the Status filter.
>>>>> +
>>>>> + KEYPAD_DEBOUNCE_1_ms = 0
>>>>> + KEYPAD_DEBOUNCE_2_ms = 1
>>>>> + KEYPAD_DEBOUNCE_4_ms = 2
>>>>> + KEYPAD_DEBOUNCE_8_ms = 3
>>>>> + KEYPAD_DEBOUNCE_16_ms = 4
>>>>> + KEYPAD_DEBOUNCE_32_ms = 5
>>>>> + KEYPAD_DEBOUNCE_64_ms = 6
>>>>> + KEYPAD_DEBOUNCE_128_ms = 7
>>>>
>>>> I could swear device-specific properties should be in form of
>>>> <vendor-prefix>,<property-name> to ensure it won't clash with changes on
>>>> subsystem level later on. Device-tree folks, what say you?
>>> I see examples with and without vendor-prefix.
>>> qcom,pm8xxx-keypad.txt does not have prefixes
>>> st-keyscan.txt does have a prefix
>>>
>>> I can't find any documented guidelines for this.
>>
>> As I mentioned I'll try to get clarification on this.
>
> I have chatted with a couple of people on this and it is acceptable to
> omit vendor prefix in bindings when we are using a specific driver like
> we have here (i.e. when driver's compatible string already includes
> vendor prefix). Vendor prefixes on properties are required when we
> augment a generic driver's binding.
>
> So the above 2 entries are fine as is.
>
> Thanks.
>