2015-02-26 16:09:09

by Scott Branden

[permalink] [raw]
Subject: [PATCH v2 0/2] Add support for Broadcom keypad controller

This series of patchsets contains the Broadcom keypad controller driver
and device tree binding documentation.

Changes from v1:
- updated dt documentation to remove to matrix-keymap.txt
- removed bcm_kp_remove as it is not necessary
- removed key-interrupt-trigger-type dt binding
- added bcm_kp_report_keys and call to process key registers

Scott Branden (2):
Input: bcm-keypad: add device tree bindings
Input: bcm-keypad: Add Broadcom keypad controller

.../devicetree/bindings/input/brcm,bcm-keypad.txt | 108 +++++
drivers/input/keyboard/Kconfig | 10 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/bcm-keypad.c | 464 +++++++++++++++++++++
4 files changed, 583 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
create mode 100644 drivers/input/keyboard/bcm-keypad.c

--
2.3.0


2015-02-26 16:09:11

by Scott Branden

[permalink] [raw]
Subject: [PATCH v2 1/2] Input: bcm-keypad: add device tree bindings

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 | 108 +++++++++++++++++++++
1 file changed, 108 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..e16bb2d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
@@ -0,0 +1,108 @@
+* 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.
+
+- 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.
+
+- linux,keypad-no-autorepeat: An optional property indicating to
+ not enable autorepeat feature.
+
+- linux,keymap: The keymap for keys as described in the binding document
+ devicetree/bindings/input/matrix-keymap.txt.
+
+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 */
+ col-debounce-filter-period = <5>;
+ row-output-enabled;
+ pull-up-enabled;
+
+ };
+};
--
2.3.0

2015-02-26 16:09:43

by Scott Branden

[permalink] [raw]
Subject: [PATCH v2 2/2] Input: bcm-keypad: Add Broadcom keypad controller

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 | 464 ++++++++++++++++++++++++++++++++++++
3 files changed, 475 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..f229ac0
--- /dev/null
+++ b/drivers/input/keyboard/bcm-keypad.c
@@ -0,0 +1,464 @@
+/*
+ * 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_BOTH 3
+
+#define KPSSR0_OFFSET 0x000000A0
+#define KPSSR1_OFFSET 0x000000A4
+#define KPSSRN_OFFSET(reg_n) (KPSSR0_OFFSET + (4 * reg_n))
+#define KPIMR0_OFFSET 0x000000B0
+#define KPIMR1_OFFSET 0x000000B4
+#define KPICR0_OFFSET 0x000000B8
+#define KPICR1_OFFSET 0x000000BC
+#define KPICRN_OFFSET(reg_n) (KPICR0_OFFSET + (4 * reg_n))
+#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_SSRN(bit_nr, reg_n) ((bit_nr >> 3) + (4 * reg_n))
+#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 void bcm_kp_report_keys(struct bcm_kp *kp, int reg_num, int pull_mode)
+{
+ unsigned long state, change;
+ int bit_nr;
+ int key_press;
+ int row, col;
+ unsigned int keycode;
+
+ /* Clear interrupts */
+ writel(0xFFFFFFFF, kp->base + KPICRN_OFFSET(reg_num));
+
+ state = readl(kp->base + KPSSRN_OFFSET(reg_num));
+ change = kp->last_state[reg_num] ^ state;
+ kp->last_state[reg_num] = 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_SSRN(bit_nr, reg_num);
+ col = BIT_TO_COL(bit_nr);
+ keycode = bcm_kp_get_keycode(kp, row, col);
+ input_report_key(kp->input_dev, keycode, key_press);
+ }
+}
+
+static irqreturn_t bcm_kp_isr_thread(int irq, void *dev_id)
+{
+ struct bcm_kp *kp = dev_id;
+ int pull_mode = (kp->kpcr >> KPCR_MODE_SHIFT) & KPCR_MODE_MASK;
+ int reg_num;
+
+ for (reg_num = 0; reg_num <= 1; reg_num++)
+ bcm_kp_report_keys(kp, reg_num, pull_mode);
+
+ 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);
+ }
+ }
+
+ /* Initialize the KPEMR Keypress Edge Mode Registers */
+ /* Trigger on both edges */
+ kp->kpemr = 0;
+ for (i = 0; i <= 30; i = i + 2)
+ kp->kpemr |= (KPEMR_EDGETYPE_BOTH << 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;
+ bool repeat;
+
+ 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);
+
+ repeat = !of_property_read_bool(pdev->dev.of_node,
+ "linux,input-no-autorepeat");
+ if (repeat)
+ __set_bit(EV_REP, input_dev->evbit);
+
+ 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;
+}
+
+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,
+ .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.3.0

2015-02-27 23:39:34

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Input: bcm-keypad: add device tree bindings

Hi Scott,

On Thu, Feb 26, 2015 at 08:08:48AM -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 | 108 +++++++++++++++++++++
> 1 file changed, 108 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..e16bb2d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
> @@ -0,0 +1,108 @@
> +* 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.
> +
> +- 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.
> +
> +- linux,keypad-no-autorepeat: An optional property indicating to
> + not enable autorepeat feature.

Unfortunately we've accumulated a lot of bindings for turning autorepeat
on or off, I would prefer if we standardize on what gpio-keys is doing
and use "autorepeat" (no "linux,") for new drivers.

Thanks.

--
Dmitry

2015-02-27 23:45:09

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] Input: bcm-keypad: Add Broadcom keypad controller

Hi Scott,

Mostly style nitpicks here...

On Thu, Feb 26, 2015 at 08:08:49AM -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 | 464 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 475 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..f229ac0
> --- /dev/null
> +++ b/drivers/input/keyboard/bcm-keypad.c
> @@ -0,0 +1,464 @@
> +/*
> + * 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_BOTH 3
> +
> +#define KPSSR0_OFFSET 0x000000A0
> +#define KPSSR1_OFFSET 0x000000A4
> +#define KPSSRN_OFFSET(reg_n) (KPSSR0_OFFSET + (4 * reg_n))
> +#define KPIMR0_OFFSET 0x000000B0
> +#define KPIMR1_OFFSET 0x000000B4
> +#define KPICR0_OFFSET 0x000000B8
> +#define KPICR1_OFFSET 0x000000BC
> +#define KPICRN_OFFSET(reg_n) (KPICR0_OFFSET + (4 * reg_n))
> +#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_SSRN(bit_nr, reg_n) ((bit_nr >> 3) + (4 * reg_n))
> +#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 void bcm_kp_report_keys(struct bcm_kp *kp, int reg_num, int pull_mode)
> +{
> + unsigned long state, change;
> + int bit_nr;
> + int key_press;
> + int row, col;
> + unsigned int keycode;
> +
> + /* Clear interrupts */
> + writel(0xFFFFFFFF, kp->base + KPICRN_OFFSET(reg_num));
> +
> + state = readl(kp->base + KPSSRN_OFFSET(reg_num));
> + change = kp->last_state[reg_num] ^ state;
> + kp->last_state[reg_num] = 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_SSRN(bit_nr, reg_num);
> + col = BIT_TO_COL(bit_nr);
> + keycode = bcm_kp_get_keycode(kp, row, col);
> + input_report_key(kp->input_dev, keycode, key_press);
> + }
> +}
> +
> +static irqreturn_t bcm_kp_isr_thread(int irq, void *dev_id)
> +{
> + struct bcm_kp *kp = dev_id;
> + int pull_mode = (kp->kpcr >> KPCR_MODE_SHIFT) & KPCR_MODE_MASK;
> + int reg_num;
> +
> + for (reg_num = 0; reg_num <= 1; reg_num++)
> + bcm_kp_report_keys(kp, reg_num, pull_mode);
> +
> + 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 */
> + {

Woudl prefer not having nested unnamed blocks in functions. If it
deserves a separate block it deserves its own function.

> + 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>
> + */

Multi-line comments are in form

/*
* Multi
* line
* comment
*/

> +
> + 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);
> + }
> + }
> +
> + /* Initialize the KPEMR Keypress Edge Mode Registers */
> + /* Trigger on both edges */
> + kp->kpemr = 0;
> + for (i = 0; i <= 30; i = i + 2)
> + kp->kpemr |= (KPEMR_EDGETYPE_BOTH << 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;
> + bool repeat;
> +
> + 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);
> +
> + repeat = !of_property_read_bool(pdev->dev.of_node,
> + "linux,input-no-autorepeat");
> + if (repeat)
> + __set_bit(EV_REP, input_dev->evbit);
> +
> + 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");

If one branch has curly braces all other have to have them too.

> + 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;
> +}
> +
> +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,
> + .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.3.0
>

Thanks!

--
Dmitry

2015-02-28 14:16:24

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] Input: bcm-keypad: Add Broadcom keypad controller

On Thu, 2015-02-26 at 08:08 -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 | 464 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 475 insertions(+)
> create mode 100644 drivers/input/keyboard/bcm-keypad.c

License nit follows.

> diff --git a/drivers/input/keyboard/bcm-keypad.c b/drivers/input/keyboard/bcm-keypad.c
> new file mode 100644
> index 0000000..f229ac0
> --- /dev/null
> +++ b/drivers/input/keyboard/bcm-keypad.c
> @@ -0,0 +1,464 @@
> +/*
> + * 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 states that the license is GPL v2 only.

> + *
> + * 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.
> + */

[...]

> +MODULE_LICENSE("GPL");

So you probably want
MODULE_LICENSE("GPL v2");


Paul Bolle

2015-02-28 15:36:10

by Scott Branden

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] Input: bcm-keypad: Add Broadcom keypad controller


Yes, will correct. Thanks.

On 15-02-28 06:16 AM, Paul Bolle wrote:
> On Thu, 2015-02-26 at 08:08 -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 | 464 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 475 insertions(+)
>> create mode 100644 drivers/input/keyboard/bcm-keypad.c
>
> License nit follows.
>
>> diff --git a/drivers/input/keyboard/bcm-keypad.c b/drivers/input/keyboard/bcm-keypad.c
>> new file mode 100644
>> index 0000000..f229ac0
>> --- /dev/null
>> +++ b/drivers/input/keyboard/bcm-keypad.c
>> @@ -0,0 +1,464 @@
>> +/*
>> + * 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 states that the license is GPL v2 only.
>
>> + *
>> + * 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.
>> + */
>
> [...]
>
>> +MODULE_LICENSE("GPL");
>
> So you probably want
> MODULE_LICENSE("GPL v2");
>
>
> Paul Bolle
>

2015-02-28 15:41:24

by Scott Branden

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Input: bcm-keypad: add device tree bindings

Hi Dmitry,

On 15-02-27 03:39 PM, Dmitry Torokhov wrote:
> Hi Scott,
>
> On Thu, Feb 26, 2015 at 08:08:48AM -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 | 108 +++++++++++++++++++++
>> 1 file changed, 108 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..e16bb2d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/brcm,bcm-keypad.txt
>> @@ -0,0 +1,108 @@
>> +* 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.
>> +
>> +- 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.
>> +
>> +- linux,keypad-no-autorepeat: An optional property indicating to
>> + not enable autorepeat feature.
>
> Unfortunately we've accumulated a lot of bindings for turning autorepeat
> on or off, I would prefer if we standardize on what gpio-keys is doing
> and use "autorepeat" (no "linux,") for new drivers.
OK. I can change it.
>
> Thanks.
>

2015-02-28 15:52:59

by Scott Branden

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] Input: bcm-keypad: Add Broadcom keypad controller

On 15-02-27 03:44 PM, Dmitry Torokhov wrote:
> Hi Scott,
>
> Mostly style nitpicks here...
>
> On Thu, Feb 26, 2015 at 08:08:49AM -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 | 464 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 475 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..f229ac0
>> --- /dev/null
>> +++ b/drivers/input/keyboard/bcm-keypad.c
>> @@ -0,0 +1,464 @@
>> +/*
>> + * 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_BOTH 3
>> +
>> +#define KPSSR0_OFFSET 0x000000A0
>> +#define KPSSR1_OFFSET 0x000000A4
>> +#define KPSSRN_OFFSET(reg_n) (KPSSR0_OFFSET + (4 * reg_n))
>> +#define KPIMR0_OFFSET 0x000000B0
>> +#define KPIMR1_OFFSET 0x000000B4
>> +#define KPICR0_OFFSET 0x000000B8
>> +#define KPICR1_OFFSET 0x000000BC
>> +#define KPICRN_OFFSET(reg_n) (KPICR0_OFFSET + (4 * reg_n))
>> +#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_SSRN(bit_nr, reg_n) ((bit_nr >> 3) + (4 * reg_n))
>> +#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 void bcm_kp_report_keys(struct bcm_kp *kp, int reg_num, int pull_mode)
>> +{
>> + unsigned long state, change;
>> + int bit_nr;
>> + int key_press;
>> + int row, col;
>> + unsigned int keycode;
>> +
>> + /* Clear interrupts */
>> + writel(0xFFFFFFFF, kp->base + KPICRN_OFFSET(reg_num));
>> +
>> + state = readl(kp->base + KPSSRN_OFFSET(reg_num));
>> + change = kp->last_state[reg_num] ^ state;
>> + kp->last_state[reg_num] = 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_SSRN(bit_nr, reg_num);
>> + col = BIT_TO_COL(bit_nr);
>> + keycode = bcm_kp_get_keycode(kp, row, col);
>> + input_report_key(kp->input_dev, keycode, key_press);
>> + }
>> +}
>> +
>> +static irqreturn_t bcm_kp_isr_thread(int irq, void *dev_id)
>> +{
>> + struct bcm_kp *kp = dev_id;
>> + int pull_mode = (kp->kpcr >> KPCR_MODE_SHIFT) & KPCR_MODE_MASK;
>> + int reg_num;
>> +
>> + for (reg_num = 0; reg_num <= 1; reg_num++)
>> + bcm_kp_report_keys(kp, reg_num, pull_mode);
>> +
>> + 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 */
>> + {
>
> Woudl prefer not having nested unnamed blocks in functions. If it
> deserves a separate block it deserves its own function.
removed {
>
>> + 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>
>> + */
>
> Multi-line comments are in form
>
> /*
> * Multi
> * line
> * comment
> */
>
fixed

>> +
>> + 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);
>> + }
>> + }
>> +
>> + /* Initialize the KPEMR Keypress Edge Mode Registers */
>> + /* Trigger on both edges */
>> + kp->kpemr = 0;
>> + for (i = 0; i <= 30; i = i + 2)
>> + kp->kpemr |= (KPEMR_EDGETYPE_BOTH << 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;
>> + bool repeat;
>> +
>> + 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);
>> +
>> + repeat = !of_property_read_bool(pdev->dev.of_node,
>> + "linux,input-no-autorepeat");
>> + if (repeat)
>> + __set_bit(EV_REP, input_dev->evbit);
>> +
>> + 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");
>
> If one branch has curly braces all other have to have them too.
>
ok. would be nice for checkpatch to deal with such K&R concerns.
>> + 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;
>> +}
>> +
>> +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,
>> + .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.3.0
>>
>
> Thanks!
>