* [PATCH v5] add mtk matrix keypad driver for keypad on MTK SoC
@ 2020-04-23 1:19 ` Fengping yu
0 siblings, 0 replies; 24+ messages in thread
From: Fengping yu @ 2020-04-23 1:19 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko, Marco Felsch, Yingjoe Chen
Cc: linux-kernel, linux-mediatek, wsd_upstream, linux-input,
linux-arm-kernel
Change since v4:
- remove extra space and redundant lines
- remove disable_irq_nosync and enable_irq in irq handler
- put defconfig as a single patch
- unified device properties interface for ACPI and device trees
- fixed other issue according reviewer comments
fengping.yu (3):
dt-bindings: add matrix keypad documentation
arm64: configs: defconfig: enable mtk keypad config
drivers: input: keyboard: add mtk keypad driver
.../devicetree/bindings/input/mtk-kpd.txt | 61 +++++
arch/arm64/configs/defconfig | 1 +
drivers/input/keyboard/Kconfig | 9 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 242 ++++++++++++++++++
5 files changed, 314 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
create mode 100644 drivers/input/keyboard/mtk-kpd.c
--
2.18.0
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5] add mtk matrix keypad driver for keypad on MTK SoC
@ 2020-04-23 1:19 ` Fengping yu
0 siblings, 0 replies; 24+ messages in thread
From: Fengping yu @ 2020-04-23 1:19 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko, Marco Felsch, Yingjoe Chen
Cc: linux-arm-kernel, linux-mediatek, linux-kernel, wsd_upstream,
linux-input
Change since v4:
- remove extra space and redundant lines
- remove disable_irq_nosync and enable_irq in irq handler
- put defconfig as a single patch
- unified device properties interface for ACPI and device trees
- fixed other issue according reviewer comments
fengping.yu (3):
dt-bindings: add matrix keypad documentation
arm64: configs: defconfig: enable mtk keypad config
drivers: input: keyboard: add mtk keypad driver
.../devicetree/bindings/input/mtk-kpd.txt | 61 +++++
arch/arm64/configs/defconfig | 1 +
drivers/input/keyboard/Kconfig | 9 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 242 ++++++++++++++++++
5 files changed, 314 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
create mode 100644 drivers/input/keyboard/mtk-kpd.c
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5] add mtk matrix keypad driver for keypad on MTK SoC
@ 2020-04-23 1:19 ` Fengping yu
0 siblings, 0 replies; 24+ messages in thread
From: Fengping yu @ 2020-04-23 1:19 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko, Marco Felsch, Yingjoe Chen
Cc: linux-arm-kernel, linux-mediatek, linux-kernel, wsd_upstream,
linux-input
Change since v4:
- remove extra space and redundant lines
- remove disable_irq_nosync and enable_irq in irq handler
- put defconfig as a single patch
- unified device properties interface for ACPI and device trees
- fixed other issue according reviewer comments
fengping.yu (3):
dt-bindings: add matrix keypad documentation
arm64: configs: defconfig: enable mtk keypad config
drivers: input: keyboard: add mtk keypad driver
.../devicetree/bindings/input/mtk-kpd.txt | 61 +++++
arch/arm64/configs/defconfig | 1 +
drivers/input/keyboard/Kconfig | 9 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 242 ++++++++++++++++++
5 files changed, 314 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
create mode 100644 drivers/input/keyboard/mtk-kpd.c
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 1/3] dt-bindings: add matrix keypad documentation
2020-04-23 1:19 ` Fengping yu
@ 2020-04-23 1:19 ` Fengping yu
-1 siblings, 0 replies; 24+ messages in thread
From: Fengping yu @ 2020-04-23 1:19 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko, Marco Felsch, Yingjoe Chen
Cc: wsd_upstream, linux-kernel, linux-mediatek, linux-input,
fengping.yu, linux-arm-kernel
From: "fengping.yu" <fengping.yu@mediatek.com>
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
.../devicetree/bindings/input/mtk-kpd.txt | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
diff --git a/Documentation/devicetree/bindings/input/mtk-kpd.txt b/Documentation/devicetree/bindings/input/mtk-kpd.txt
new file mode 100644
index 000000000000..8b154a5e2f7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/mtk-kpd.txt
@@ -0,0 +1,61 @@
+* Mediatek's Keypad Controller device tree binding
+
+Mediatek's 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.
+
+Required properties:
+- compatible should contain:
+ * "mediatek,kp" for common keypad
+ * "mediatek,mt6779-keypad" for specific keypad chip
+
+- reg: The base address of the Keypad register bank.
+
+- interrupts: A single interrupt specifier.
+
+- mediatek,debounce-us: Debounce interval in microseconds, maximum value
+ is 256000 microseconds.
+
+- keypad,num-rows: Number of row lines connected to the keypad controller, it is
+ not equal to PCB rows number, instead you should add required value for each IC.
+
+- keypad,num-columns: Number of column lines connected to the keypad controller,
+ it is not equal to PCB columns number, instead you should add required value
+ for each IC.
+
+- linux,keymap: The keymap for keys as described in the binding document
+ devicetree/bindings/input/matrix-keymap.txt.
+
+- pinctrl: Should specify pin control groups used for this controller.
+ See ../pinctrl/pinctrl-bindings.txt for details.
+
+- clocks: Must contain one entry, for the module clock.
+ See ../clocks/clock-bindings.txt for details.
+
+- clock-names: Names of the clocks listed in clocks property in the same order.
+
+Optional Properties:
+- wakeup-source: use any event on keypad as wakeup event.
+
+Example:
+
+ keypad: kp@10010000 {
+ compatible = "mediatek,kp";
+ reg = <0 0x10010000 0 0x1000>;
+ wakeup-source;
+ interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_FALLING>;
+ clocks = <&clk26m>;
+ clock-names = "kpd";
+ };
+
+ &keypad {
+ mediatek,debounce-us = <32000>;
+ keypad,num-rows = <8>;
+ keypad,num-columns = <9>;
+ linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_VOLUMEDOWN) >;
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&kpd_gpios_def_cfg>;
+ };
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 1/3] dt-bindings: add matrix keypad documentation
@ 2020-04-23 1:19 ` Fengping yu
0 siblings, 0 replies; 24+ messages in thread
From: Fengping yu @ 2020-04-23 1:19 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko, Marco Felsch, Yingjoe Chen
Cc: wsd_upstream, linux-kernel, linux-mediatek, linux-input,
fengping.yu, linux-arm-kernel
From: "fengping.yu" <fengping.yu@mediatek.com>
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
.../devicetree/bindings/input/mtk-kpd.txt | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
diff --git a/Documentation/devicetree/bindings/input/mtk-kpd.txt b/Documentation/devicetree/bindings/input/mtk-kpd.txt
new file mode 100644
index 000000000000..8b154a5e2f7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/mtk-kpd.txt
@@ -0,0 +1,61 @@
+* Mediatek's Keypad Controller device tree binding
+
+Mediatek's 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.
+
+Required properties:
+- compatible should contain:
+ * "mediatek,kp" for common keypad
+ * "mediatek,mt6779-keypad" for specific keypad chip
+
+- reg: The base address of the Keypad register bank.
+
+- interrupts: A single interrupt specifier.
+
+- mediatek,debounce-us: Debounce interval in microseconds, maximum value
+ is 256000 microseconds.
+
+- keypad,num-rows: Number of row lines connected to the keypad controller, it is
+ not equal to PCB rows number, instead you should add required value for each IC.
+
+- keypad,num-columns: Number of column lines connected to the keypad controller,
+ it is not equal to PCB columns number, instead you should add required value
+ for each IC.
+
+- linux,keymap: The keymap for keys as described in the binding document
+ devicetree/bindings/input/matrix-keymap.txt.
+
+- pinctrl: Should specify pin control groups used for this controller.
+ See ../pinctrl/pinctrl-bindings.txt for details.
+
+- clocks: Must contain one entry, for the module clock.
+ See ../clocks/clock-bindings.txt for details.
+
+- clock-names: Names of the clocks listed in clocks property in the same order.
+
+Optional Properties:
+- wakeup-source: use any event on keypad as wakeup event.
+
+Example:
+
+ keypad: kp@10010000 {
+ compatible = "mediatek,kp";
+ reg = <0 0x10010000 0 0x1000>;
+ wakeup-source;
+ interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_FALLING>;
+ clocks = <&clk26m>;
+ clock-names = "kpd";
+ };
+
+ &keypad {
+ mediatek,debounce-us = <32000>;
+ keypad,num-rows = <8>;
+ keypad,num-columns = <9>;
+ linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_VOLUMEDOWN) >;
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&kpd_gpios_def_cfg>;
+ };
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 2/3] arm64: configs: defconfig: enable mtk keypad config
2020-04-23 1:19 ` Fengping yu
@ 2020-04-23 1:20 ` Fengping yu
-1 siblings, 0 replies; 24+ messages in thread
From: Fengping yu @ 2020-04-23 1:20 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko, Marco Felsch, Yingjoe Chen
Cc: wsd_upstream, linux-kernel, linux-mediatek, linux-input,
fengping.yu, linux-arm-kernel
From: "fengping.yu" <fengping.yu@mediatek.com>
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0f212889c931..7863352521e5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -316,6 +316,7 @@ CONFIG_KEYBOARD_GPIO=y
CONFIG_KEYBOARD_SNVS_PWRKEY=m
CONFIG_KEYBOARD_IMX_SC_KEY=m
CONFIG_KEYBOARD_CROS_EC=y
+CONFIG_KEYBOARD_MTK_KPD=y
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_ATMEL_MXT=m
CONFIG_INPUT_MISC=y
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 2/3] arm64: configs: defconfig: enable mtk keypad config
@ 2020-04-23 1:20 ` Fengping yu
0 siblings, 0 replies; 24+ messages in thread
From: Fengping yu @ 2020-04-23 1:20 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko, Marco Felsch, Yingjoe Chen
Cc: wsd_upstream, linux-kernel, linux-mediatek, linux-input,
fengping.yu, linux-arm-kernel
From: "fengping.yu" <fengping.yu@mediatek.com>
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0f212889c931..7863352521e5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -316,6 +316,7 @@ CONFIG_KEYBOARD_GPIO=y
CONFIG_KEYBOARD_SNVS_PWRKEY=m
CONFIG_KEYBOARD_IMX_SC_KEY=m
CONFIG_KEYBOARD_CROS_EC=y
+CONFIG_KEYBOARD_MTK_KPD=y
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_ATMEL_MXT=m
CONFIG_INPUT_MISC=y
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 3/3] drivers: input: keyboard: add mtk keypad driver
2020-04-23 1:19 ` Fengping yu
@ 2020-04-23 1:20 ` Fengping yu
-1 siblings, 0 replies; 24+ messages in thread
From: Fengping yu @ 2020-04-23 1:20 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko, Marco Felsch, Yingjoe Chen
Cc: wsd_upstream, linux-kernel, linux-mediatek, linux-input,
fengping.yu, linux-arm-kernel
From: "fengping.yu" <fengping.yu@mediatek.com>
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
drivers/input/keyboard/Kconfig | 9 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 242 +++++++++++++++++++++++++++++++
3 files changed, 252 insertions(+)
create mode 100644 drivers/input/keyboard/mtk-kpd.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 4706ff09f0e8..4a387d8683b1 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -772,6 +772,15 @@ config KEYBOARD_BCM
To compile this driver as a module, choose M here: the
module will be called bcm-keypad.
+config KEYBOARD_MTK_KPD
+ tristate "MediaTek Keypad Support"
+ help
+ Say Y here if you want to use the keypad.
+ If unuse, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called mtk-kpd.
+
config KEYBOARD_MTK_PMIC
tristate "MediaTek PMIC keys support"
depends on MFD_MT6397
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index f5b17524adf2..63324d3e36c5 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
+obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
new file mode 100644
index 000000000000..7f8f091b2734
--- /dev/null
+++ b/drivers/input/keyboard/mtk-kpd.c
@@ -0,0 +1,242 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 MediaTek Inc.
+ * Author Terry Chang <terry.chang@mediatek.com>
+ */
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+
+#define KPD_NAME "mtk-kpd"
+
+#define KP_MEM 0x0004
+#define KP_DEBOUNCE 0x0018
+
+#define KPD_DEBOUNCE_MASK GENMASK(13, 0)
+#define KPD_DEBOUNCE_MAX 256000
+#define KPD_NUM_MEMS 5
+#define KPD_NUM_BITS 136 /* 4 * 32 + 8 MEM5 only use 8 BITS */
+#define BITS_TO_U32(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(u32))
+
+struct mtk_keypad {
+ struct input_dev *input_dev;
+ struct clk *clk;
+ void __iomem *base;
+ unsigned int irqnr;
+ bool wakeup;
+ u32 key_debounce;
+ u32 n_rows;
+ u32 n_cols;
+ DECLARE_BITMAP(keymap_state, KPD_NUM_BITS);
+};
+
+static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
+{
+ /* use _nosync to avoid deadlock */
+ struct mtk_keypad *keypad = dev_id;
+ unsigned short *keycode = keypad->input_dev->keycode;
+ DECLARE_BITMAP(new_state, KPD_NUM_BITS);
+ DECLARE_BITMAP(change, KPD_NUM_BITS);
+ int bit_nr;
+ int pressed;
+ unsigned short code;
+
+ memcpy_fromio(new_state, keypad->base + KP_MEM, KPD_NUM_MEMS);
+
+ bitmap_xor(change, new_state, keypad->keymap_state, KPD_NUM_BITS);
+
+ for_each_set_bit(bit_nr, change, KPD_NUM_BITS) {
+ pressed = test_bit(bit_nr, new_state) == 0U;
+ dev_dbg(&keypad->input_dev->dev, "%s",
+ pressed ? "pressed" : "released");
+
+ /* per 32bit register only use low 16bit as keypad mem register */
+ code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
+
+ input_report_key(keypad->input_dev, code, pressed);
+ input_sync(keypad->input_dev);
+
+ dev_dbg(&keypad->input_dev->dev,
+ "report Linux keycode = %d\n", code);
+ }
+
+ bitmap_copy(keypad->keymap_state, new_state, KPD_NUM_BITS);
+
+ return IRQ_HANDLED;
+}
+
+static int kpd_get_dts_info(struct mtk_keypad *keypad)
+{
+ int ret;
+ struct device *dev = keypad->input_dev->dev.parent;
+ struct device_node *node = dev->of_node;
+
+ ret = matrix_keypad_parse_properties(dev, &keypad->n_rows,
+ &keypad->n_cols);
+
+ if (ret) {
+ dev_err(dev, "Failed to parse keypad params\n");
+ return ret;
+ }
+
+ ret = device_property_read_u32(dev, "mediatek,debounce-us",
+ &keypad->key_debounce);
+ if (ret) {
+ dev_err(dev, "Failed to read mediatek debounce time\n");
+ return ret;
+ }
+
+ if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
+ dev_err(dev, "Debounce time exceeds the maximum allowed time 256ms\n");
+ return -EINVAL;
+ }
+
+ keypad->wakeup = device_property_read_bool(node, "wakeup-source");
+
+ dev_dbg(dev, "n_row=%d n_col=%d debounce=%d\n",
+ keypad->n_rows, keypad->n_cols,
+ keypad->key_debounce);
+
+ return 0;
+}
+
+static int kpd_pdrv_probe(struct platform_device *pdev)
+{
+ struct mtk_keypad *keypad;
+ struct pinctrl *keypad_pinctrl;
+ struct pinctrl_state *kpd_default;
+ int ret;
+
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
+ if (!keypad)
+ return -ENOMEM;
+
+ bitmap_fill(keypad->keymap_state, KPD_NUM_BITS);
+
+ keypad->input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!keypad->input_dev) {
+ dev_err(&pdev->dev, "Failed to allocate input dev\n");
+ return -ENOMEM;
+ }
+
+ keypad->input_dev->name = KPD_NAME;
+ keypad->input_dev->id.bustype = BUS_HOST;
+
+ ret = kpd_get_dts_info(keypad);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to get dts info\n");
+ return ret;
+ }
+
+ ret = matrix_keypad_build_keymap(NULL, NULL,
+ keypad->n_rows,
+ keypad->n_cols,
+ NULL,
+ keypad->input_dev);
+
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to build keymap\n");
+ return ret;
+ }
+
+ input_set_drvdata(keypad->input_dev, keypad);
+
+ keypad->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(keypad->base)) {
+ dev_err(&pdev->dev, "Failed to get resource and iomap keypad\n");
+ return PTR_ERR(keypad->base);
+ }
+
+ if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
+ dev_err(&pdev->dev, "Invalid debounce time value.\n");
+ return -EINVAL;
+ }
+
+ writew(keypad->key_debounce * 32 / 1000 & KPD_DEBOUNCE_MASK,
+ keypad->base + KP_DEBOUNCE);
+
+ keypad->clk = devm_clk_get(&pdev->dev, "kpd");
+ if (IS_ERR(keypad->clk)) {
+ return PTR_ERR(keypad->clk);
+ }
+
+ ret = clk_prepare_enable(keypad->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
+ return ret;
+ }
+
+ keypad_pinctrl = devm_pinctrl_get(&pdev->dev);
+ if (IS_ERR(keypad_pinctrl)) {
+ ret = PTR_ERR(keypad_pinctrl);
+ goto disable_kpd_clk;
+ }
+
+ kpd_default = pinctrl_lookup_state(keypad_pinctrl, "default");
+ if (IS_ERR(kpd_default)) {
+ dev_err(&pdev->dev, "No default pinctrl state\n");
+ ret = PTR_ERR(kpd_default);
+ goto disable_kpd_clk;
+ }
+
+ pinctrl_select_state(keypad_pinctrl, kpd_default);
+
+ keypad->irqnr = platform_get_irq(pdev, 0);
+ if (keypad->irqnr < 0) {
+ dev_err(&pdev->dev, "Failed to get irq\n");
+ ret = -keypad->irqnr;
+ goto disable_kpd_clk;
+ }
+
+ ret = devm_request_irq(&pdev->dev, keypad->irqnr,
+ kpd_irq_handler, 0,
+ KPD_NAME, keypad);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
+ keypad->irqnr, ret);
+ goto disable_kpd_clk;
+ }
+
+ ret = input_register_device(keypad->input_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register device\n");
+ goto disable_kpd_clk;
+ }
+
+ device_init_wakeup(&pdev->dev, keypad->wakeup);
+
+ platform_set_drvdata(pdev, keypad);
+
+ return 0;
+
+disable_kpd_clk:
+ clk_disable_unprepare(keypad->clk);
+ return ret;
+}
+
+static const struct of_device_id kpd_of_match[] = {
+ {.compatible = "mediatek,kp"},
+ {}
+};
+
+static struct platform_driver kpd_pdrv = {
+ .probe = kpd_pdrv_probe,
+ .driver = {
+ .name = KPD_NAME,
+ .of_match_table = kpd_of_match,
+ },
+};
+
+module_platform_driver(kpd_pdrv);
+
+MODULE_AUTHOR("Mediatek Corporation");
+MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
+MODULE_LICENSE("GPL");
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 3/3] drivers: input: keyboard: add mtk keypad driver
@ 2020-04-23 1:20 ` Fengping yu
0 siblings, 0 replies; 24+ messages in thread
From: Fengping yu @ 2020-04-23 1:20 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko, Marco Felsch, Yingjoe Chen
Cc: wsd_upstream, linux-kernel, linux-mediatek, linux-input,
fengping.yu, linux-arm-kernel
From: "fengping.yu" <fengping.yu@mediatek.com>
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
drivers/input/keyboard/Kconfig | 9 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 242 +++++++++++++++++++++++++++++++
3 files changed, 252 insertions(+)
create mode 100644 drivers/input/keyboard/mtk-kpd.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 4706ff09f0e8..4a387d8683b1 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -772,6 +772,15 @@ config KEYBOARD_BCM
To compile this driver as a module, choose M here: the
module will be called bcm-keypad.
+config KEYBOARD_MTK_KPD
+ tristate "MediaTek Keypad Support"
+ help
+ Say Y here if you want to use the keypad.
+ If unuse, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called mtk-kpd.
+
config KEYBOARD_MTK_PMIC
tristate "MediaTek PMIC keys support"
depends on MFD_MT6397
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index f5b17524adf2..63324d3e36c5 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
+obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
new file mode 100644
index 000000000000..7f8f091b2734
--- /dev/null
+++ b/drivers/input/keyboard/mtk-kpd.c
@@ -0,0 +1,242 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 MediaTek Inc.
+ * Author Terry Chang <terry.chang@mediatek.com>
+ */
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+
+#define KPD_NAME "mtk-kpd"
+
+#define KP_MEM 0x0004
+#define KP_DEBOUNCE 0x0018
+
+#define KPD_DEBOUNCE_MASK GENMASK(13, 0)
+#define KPD_DEBOUNCE_MAX 256000
+#define KPD_NUM_MEMS 5
+#define KPD_NUM_BITS 136 /* 4 * 32 + 8 MEM5 only use 8 BITS */
+#define BITS_TO_U32(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(u32))
+
+struct mtk_keypad {
+ struct input_dev *input_dev;
+ struct clk *clk;
+ void __iomem *base;
+ unsigned int irqnr;
+ bool wakeup;
+ u32 key_debounce;
+ u32 n_rows;
+ u32 n_cols;
+ DECLARE_BITMAP(keymap_state, KPD_NUM_BITS);
+};
+
+static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
+{
+ /* use _nosync to avoid deadlock */
+ struct mtk_keypad *keypad = dev_id;
+ unsigned short *keycode = keypad->input_dev->keycode;
+ DECLARE_BITMAP(new_state, KPD_NUM_BITS);
+ DECLARE_BITMAP(change, KPD_NUM_BITS);
+ int bit_nr;
+ int pressed;
+ unsigned short code;
+
+ memcpy_fromio(new_state, keypad->base + KP_MEM, KPD_NUM_MEMS);
+
+ bitmap_xor(change, new_state, keypad->keymap_state, KPD_NUM_BITS);
+
+ for_each_set_bit(bit_nr, change, KPD_NUM_BITS) {
+ pressed = test_bit(bit_nr, new_state) == 0U;
+ dev_dbg(&keypad->input_dev->dev, "%s",
+ pressed ? "pressed" : "released");
+
+ /* per 32bit register only use low 16bit as keypad mem register */
+ code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
+
+ input_report_key(keypad->input_dev, code, pressed);
+ input_sync(keypad->input_dev);
+
+ dev_dbg(&keypad->input_dev->dev,
+ "report Linux keycode = %d\n", code);
+ }
+
+ bitmap_copy(keypad->keymap_state, new_state, KPD_NUM_BITS);
+
+ return IRQ_HANDLED;
+}
+
+static int kpd_get_dts_info(struct mtk_keypad *keypad)
+{
+ int ret;
+ struct device *dev = keypad->input_dev->dev.parent;
+ struct device_node *node = dev->of_node;
+
+ ret = matrix_keypad_parse_properties(dev, &keypad->n_rows,
+ &keypad->n_cols);
+
+ if (ret) {
+ dev_err(dev, "Failed to parse keypad params\n");
+ return ret;
+ }
+
+ ret = device_property_read_u32(dev, "mediatek,debounce-us",
+ &keypad->key_debounce);
+ if (ret) {
+ dev_err(dev, "Failed to read mediatek debounce time\n");
+ return ret;
+ }
+
+ if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
+ dev_err(dev, "Debounce time exceeds the maximum allowed time 256ms\n");
+ return -EINVAL;
+ }
+
+ keypad->wakeup = device_property_read_bool(node, "wakeup-source");
+
+ dev_dbg(dev, "n_row=%d n_col=%d debounce=%d\n",
+ keypad->n_rows, keypad->n_cols,
+ keypad->key_debounce);
+
+ return 0;
+}
+
+static int kpd_pdrv_probe(struct platform_device *pdev)
+{
+ struct mtk_keypad *keypad;
+ struct pinctrl *keypad_pinctrl;
+ struct pinctrl_state *kpd_default;
+ int ret;
+
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
+ if (!keypad)
+ return -ENOMEM;
+
+ bitmap_fill(keypad->keymap_state, KPD_NUM_BITS);
+
+ keypad->input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!keypad->input_dev) {
+ dev_err(&pdev->dev, "Failed to allocate input dev\n");
+ return -ENOMEM;
+ }
+
+ keypad->input_dev->name = KPD_NAME;
+ keypad->input_dev->id.bustype = BUS_HOST;
+
+ ret = kpd_get_dts_info(keypad);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to get dts info\n");
+ return ret;
+ }
+
+ ret = matrix_keypad_build_keymap(NULL, NULL,
+ keypad->n_rows,
+ keypad->n_cols,
+ NULL,
+ keypad->input_dev);
+
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to build keymap\n");
+ return ret;
+ }
+
+ input_set_drvdata(keypad->input_dev, keypad);
+
+ keypad->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(keypad->base)) {
+ dev_err(&pdev->dev, "Failed to get resource and iomap keypad\n");
+ return PTR_ERR(keypad->base);
+ }
+
+ if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
+ dev_err(&pdev->dev, "Invalid debounce time value.\n");
+ return -EINVAL;
+ }
+
+ writew(keypad->key_debounce * 32 / 1000 & KPD_DEBOUNCE_MASK,
+ keypad->base + KP_DEBOUNCE);
+
+ keypad->clk = devm_clk_get(&pdev->dev, "kpd");
+ if (IS_ERR(keypad->clk)) {
+ return PTR_ERR(keypad->clk);
+ }
+
+ ret = clk_prepare_enable(keypad->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
+ return ret;
+ }
+
+ keypad_pinctrl = devm_pinctrl_get(&pdev->dev);
+ if (IS_ERR(keypad_pinctrl)) {
+ ret = PTR_ERR(keypad_pinctrl);
+ goto disable_kpd_clk;
+ }
+
+ kpd_default = pinctrl_lookup_state(keypad_pinctrl, "default");
+ if (IS_ERR(kpd_default)) {
+ dev_err(&pdev->dev, "No default pinctrl state\n");
+ ret = PTR_ERR(kpd_default);
+ goto disable_kpd_clk;
+ }
+
+ pinctrl_select_state(keypad_pinctrl, kpd_default);
+
+ keypad->irqnr = platform_get_irq(pdev, 0);
+ if (keypad->irqnr < 0) {
+ dev_err(&pdev->dev, "Failed to get irq\n");
+ ret = -keypad->irqnr;
+ goto disable_kpd_clk;
+ }
+
+ ret = devm_request_irq(&pdev->dev, keypad->irqnr,
+ kpd_irq_handler, 0,
+ KPD_NAME, keypad);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
+ keypad->irqnr, ret);
+ goto disable_kpd_clk;
+ }
+
+ ret = input_register_device(keypad->input_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register device\n");
+ goto disable_kpd_clk;
+ }
+
+ device_init_wakeup(&pdev->dev, keypad->wakeup);
+
+ platform_set_drvdata(pdev, keypad);
+
+ return 0;
+
+disable_kpd_clk:
+ clk_disable_unprepare(keypad->clk);
+ return ret;
+}
+
+static const struct of_device_id kpd_of_match[] = {
+ {.compatible = "mediatek,kp"},
+ {}
+};
+
+static struct platform_driver kpd_pdrv = {
+ .probe = kpd_pdrv_probe,
+ .driver = {
+ .name = KPD_NAME,
+ .of_match_table = kpd_of_match,
+ },
+};
+
+module_platform_driver(kpd_pdrv);
+
+MODULE_AUTHOR("Mediatek Corporation");
+MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
+MODULE_LICENSE("GPL");
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/3] dt-bindings: add matrix keypad documentation
2020-04-23 1:19 ` Fengping yu
(?)
@ 2020-04-23 5:26 ` Marco Felsch
-1 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 5:26 UTC (permalink / raw)
To: Fengping yu
Cc: Dmitry Torokhov, Andy Shevchenko, Yingjoe Chen, linux-kernel,
linux-mediatek, wsd_upstream, linux-input, linux-arm-kernel
Hi Fengping,
On 20-04-23 09:19, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
> ---
> .../devicetree/bindings/input/mtk-kpd.txt | 61 +++++++++++++++++++
Pls don't add new binding docs as .txt instead you should use the new
yaml schema.
Regards,
Marco
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
>
> diff --git a/Documentation/devicetree/bindings/input/mtk-kpd.txt b/Documentation/devicetree/bindings/input/mtk-kpd.txt
> new file mode 100644
> index 000000000000..8b154a5e2f7d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/mtk-kpd.txt
> @@ -0,0 +1,61 @@
> +* Mediatek's Keypad Controller device tree binding
> +
> +Mediatek's 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.
> +
> +Required properties:
> +- compatible should contain:
> + * "mediatek,kp" for common keypad
> + * "mediatek,mt6779-keypad" for specific keypad chip
> +
> +- reg: The base address of the Keypad register bank.
> +
> +- interrupts: A single interrupt specifier.
> +
> +- mediatek,debounce-us: Debounce interval in microseconds, maximum value
> + is 256000 microseconds.
> +
> +- keypad,num-rows: Number of row lines connected to the keypad controller, it is
> + not equal to PCB rows number, instead you should add required value for each IC.
> +
> +- keypad,num-columns: Number of column lines connected to the keypad controller,
> + it is not equal to PCB columns number, instead you should add required value
> + for each IC.
> +
> +- linux,keymap: The keymap for keys as described in the binding document
> + devicetree/bindings/input/matrix-keymap.txt.
> +
> +- pinctrl: Should specify pin control groups used for this controller.
> + See ../pinctrl/pinctrl-bindings.txt for details.
> +
> +- clocks: Must contain one entry, for the module clock.
> + See ../clocks/clock-bindings.txt for details.
> +
> +- clock-names: Names of the clocks listed in clocks property in the same order.
> +
> +Optional Properties:
> +- wakeup-source: use any event on keypad as wakeup event.
> +
> +Example:
> +
> + keypad: kp@10010000 {
> + compatible = "mediatek,kp";
> + reg = <0 0x10010000 0 0x1000>;
> + wakeup-source;
> + interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_FALLING>;
> + clocks = <&clk26m>;
> + clock-names = "kpd";
> + };
> +
> + &keypad {
> + mediatek,debounce-us = <32000>;
> + keypad,num-rows = <8>;
> + keypad,num-columns = <9>;
> + linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_VOLUMEDOWN) >;
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&kpd_gpios_def_cfg>;
> + };
> --
> 2.18.0
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/3] dt-bindings: add matrix keypad documentation
@ 2020-04-23 5:26 ` Marco Felsch
0 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 5:26 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, linux-kernel, linux-mediatek,
linux-input, Yingjoe Chen, Andy Shevchenko, linux-arm-kernel
Hi Fengping,
On 20-04-23 09:19, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
> ---
> .../devicetree/bindings/input/mtk-kpd.txt | 61 +++++++++++++++++++
Pls don't add new binding docs as .txt instead you should use the new
yaml schema.
Regards,
Marco
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
>
> diff --git a/Documentation/devicetree/bindings/input/mtk-kpd.txt b/Documentation/devicetree/bindings/input/mtk-kpd.txt
> new file mode 100644
> index 000000000000..8b154a5e2f7d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/mtk-kpd.txt
> @@ -0,0 +1,61 @@
> +* Mediatek's Keypad Controller device tree binding
> +
> +Mediatek's 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.
> +
> +Required properties:
> +- compatible should contain:
> + * "mediatek,kp" for common keypad
> + * "mediatek,mt6779-keypad" for specific keypad chip
> +
> +- reg: The base address of the Keypad register bank.
> +
> +- interrupts: A single interrupt specifier.
> +
> +- mediatek,debounce-us: Debounce interval in microseconds, maximum value
> + is 256000 microseconds.
> +
> +- keypad,num-rows: Number of row lines connected to the keypad controller, it is
> + not equal to PCB rows number, instead you should add required value for each IC.
> +
> +- keypad,num-columns: Number of column lines connected to the keypad controller,
> + it is not equal to PCB columns number, instead you should add required value
> + for each IC.
> +
> +- linux,keymap: The keymap for keys as described in the binding document
> + devicetree/bindings/input/matrix-keymap.txt.
> +
> +- pinctrl: Should specify pin control groups used for this controller.
> + See ../pinctrl/pinctrl-bindings.txt for details.
> +
> +- clocks: Must contain one entry, for the module clock.
> + See ../clocks/clock-bindings.txt for details.
> +
> +- clock-names: Names of the clocks listed in clocks property in the same order.
> +
> +Optional Properties:
> +- wakeup-source: use any event on keypad as wakeup event.
> +
> +Example:
> +
> + keypad: kp@10010000 {
> + compatible = "mediatek,kp";
> + reg = <0 0x10010000 0 0x1000>;
> + wakeup-source;
> + interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_FALLING>;
> + clocks = <&clk26m>;
> + clock-names = "kpd";
> + };
> +
> + &keypad {
> + mediatek,debounce-us = <32000>;
> + keypad,num-rows = <8>;
> + keypad,num-columns = <9>;
> + linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_VOLUMEDOWN) >;
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&kpd_gpios_def_cfg>;
> + };
> --
> 2.18.0
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/3] dt-bindings: add matrix keypad documentation
@ 2020-04-23 5:26 ` Marco Felsch
0 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 5:26 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, linux-kernel, linux-mediatek,
linux-input, Yingjoe Chen, Andy Shevchenko, linux-arm-kernel
Hi Fengping,
On 20-04-23 09:19, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
> ---
> .../devicetree/bindings/input/mtk-kpd.txt | 61 +++++++++++++++++++
Pls don't add new binding docs as .txt instead you should use the new
yaml schema.
Regards,
Marco
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
>
> diff --git a/Documentation/devicetree/bindings/input/mtk-kpd.txt b/Documentation/devicetree/bindings/input/mtk-kpd.txt
> new file mode 100644
> index 000000000000..8b154a5e2f7d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/mtk-kpd.txt
> @@ -0,0 +1,61 @@
> +* Mediatek's Keypad Controller device tree binding
> +
> +Mediatek's 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.
> +
> +Required properties:
> +- compatible should contain:
> + * "mediatek,kp" for common keypad
> + * "mediatek,mt6779-keypad" for specific keypad chip
> +
> +- reg: The base address of the Keypad register bank.
> +
> +- interrupts: A single interrupt specifier.
> +
> +- mediatek,debounce-us: Debounce interval in microseconds, maximum value
> + is 256000 microseconds.
> +
> +- keypad,num-rows: Number of row lines connected to the keypad controller, it is
> + not equal to PCB rows number, instead you should add required value for each IC.
> +
> +- keypad,num-columns: Number of column lines connected to the keypad controller,
> + it is not equal to PCB columns number, instead you should add required value
> + for each IC.
> +
> +- linux,keymap: The keymap for keys as described in the binding document
> + devicetree/bindings/input/matrix-keymap.txt.
> +
> +- pinctrl: Should specify pin control groups used for this controller.
> + See ../pinctrl/pinctrl-bindings.txt for details.
> +
> +- clocks: Must contain one entry, for the module clock.
> + See ../clocks/clock-bindings.txt for details.
> +
> +- clock-names: Names of the clocks listed in clocks property in the same order.
> +
> +Optional Properties:
> +- wakeup-source: use any event on keypad as wakeup event.
> +
> +Example:
> +
> + keypad: kp@10010000 {
> + compatible = "mediatek,kp";
> + reg = <0 0x10010000 0 0x1000>;
> + wakeup-source;
> + interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_FALLING>;
> + clocks = <&clk26m>;
> + clock-names = "kpd";
> + };
> +
> + &keypad {
> + mediatek,debounce-us = <32000>;
> + keypad,num-rows = <8>;
> + keypad,num-columns = <9>;
> + linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_VOLUMEDOWN) >;
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&kpd_gpios_def_cfg>;
> + };
> --
> 2.18.0
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 2/3] arm64: configs: defconfig: enable mtk keypad config
2020-04-23 1:20 ` Fengping yu
(?)
@ 2020-04-23 5:28 ` Marco Felsch
-1 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 5:28 UTC (permalink / raw)
To: Fengping yu
Cc: Dmitry Torokhov, Andy Shevchenko, Yingjoe Chen, linux-kernel,
linux-mediatek, wsd_upstream, linux-input, linux-arm-kernel
On 20-04-23 09:20, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
Nit, this should be the last patch in this series. Also you need to add
a few comments wihtin the commit message. Empty commit messages are not
allowed.
Regards,
Marco
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 0f212889c931..7863352521e5 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -316,6 +316,7 @@ CONFIG_KEYBOARD_GPIO=y
> CONFIG_KEYBOARD_SNVS_PWRKEY=m
> CONFIG_KEYBOARD_IMX_SC_KEY=m
> CONFIG_KEYBOARD_CROS_EC=y
> +CONFIG_KEYBOARD_MTK_KPD=y
> CONFIG_INPUT_TOUCHSCREEN=y
> CONFIG_TOUCHSCREEN_ATMEL_MXT=m
> CONFIG_INPUT_MISC=y
> --
> 2.18.0
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 2/3] arm64: configs: defconfig: enable mtk keypad config
@ 2020-04-23 5:28 ` Marco Felsch
0 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 5:28 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, linux-kernel, linux-mediatek,
linux-input, Yingjoe Chen, Andy Shevchenko, linux-arm-kernel
On 20-04-23 09:20, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
Nit, this should be the last patch in this series. Also you need to add
a few comments wihtin the commit message. Empty commit messages are not
allowed.
Regards,
Marco
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 0f212889c931..7863352521e5 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -316,6 +316,7 @@ CONFIG_KEYBOARD_GPIO=y
> CONFIG_KEYBOARD_SNVS_PWRKEY=m
> CONFIG_KEYBOARD_IMX_SC_KEY=m
> CONFIG_KEYBOARD_CROS_EC=y
> +CONFIG_KEYBOARD_MTK_KPD=y
> CONFIG_INPUT_TOUCHSCREEN=y
> CONFIG_TOUCHSCREEN_ATMEL_MXT=m
> CONFIG_INPUT_MISC=y
> --
> 2.18.0
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 2/3] arm64: configs: defconfig: enable mtk keypad config
@ 2020-04-23 5:28 ` Marco Felsch
0 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 5:28 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, linux-kernel, linux-mediatek,
linux-input, Yingjoe Chen, Andy Shevchenko, linux-arm-kernel
On 20-04-23 09:20, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
Nit, this should be the last patch in this series. Also you need to add
a few comments wihtin the commit message. Empty commit messages are not
allowed.
Regards,
Marco
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 0f212889c931..7863352521e5 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -316,6 +316,7 @@ CONFIG_KEYBOARD_GPIO=y
> CONFIG_KEYBOARD_SNVS_PWRKEY=m
> CONFIG_KEYBOARD_IMX_SC_KEY=m
> CONFIG_KEYBOARD_CROS_EC=y
> +CONFIG_KEYBOARD_MTK_KPD=y
> CONFIG_INPUT_TOUCHSCREEN=y
> CONFIG_TOUCHSCREEN_ATMEL_MXT=m
> CONFIG_INPUT_MISC=y
> --
> 2.18.0
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5] add mtk matrix keypad driver for keypad on MTK SoC
2020-04-23 1:19 ` Fengping yu
(?)
@ 2020-04-23 5:29 ` Marco Felsch
-1 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 5:29 UTC (permalink / raw)
To: Fengping yu
Cc: Dmitry Torokhov, Andy Shevchenko, Yingjoe Chen, linux-kernel,
linux-mediatek, wsd_upstream, linux-input, linux-arm-kernel
Hi Fengping,
On 20-04-23 09:19, Fengping yu wrote:
>
> Change since v4:
> - remove extra space and redundant lines
> - remove disable_irq_nosync and enable_irq in irq handler
> - put defconfig as a single patch
> - unified device properties interface for ACPI and device trees
This is a good changelog for reviewers.
> - fixed other issue according reviewer comments
This one is a bit useless..
Regards,
Marco
> fengping.yu (3):
> dt-bindings: add matrix keypad documentation
> arm64: configs: defconfig: enable mtk keypad config
> drivers: input: keyboard: add mtk keypad driver
>
> .../devicetree/bindings/input/mtk-kpd.txt | 61 +++++
> arch/arm64/configs/defconfig | 1 +
> drivers/input/keyboard/Kconfig | 9 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/mtk-kpd.c | 242 ++++++++++++++++++
> 5 files changed, 314 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
> create mode 100644 drivers/input/keyboard/mtk-kpd.c
>
> --
> 2.18.0
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5] add mtk matrix keypad driver for keypad on MTK SoC
@ 2020-04-23 5:29 ` Marco Felsch
0 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 5:29 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, linux-kernel, linux-mediatek,
linux-input, Yingjoe Chen, Andy Shevchenko, linux-arm-kernel
Hi Fengping,
On 20-04-23 09:19, Fengping yu wrote:
>
> Change since v4:
> - remove extra space and redundant lines
> - remove disable_irq_nosync and enable_irq in irq handler
> - put defconfig as a single patch
> - unified device properties interface for ACPI and device trees
This is a good changelog for reviewers.
> - fixed other issue according reviewer comments
This one is a bit useless..
Regards,
Marco
> fengping.yu (3):
> dt-bindings: add matrix keypad documentation
> arm64: configs: defconfig: enable mtk keypad config
> drivers: input: keyboard: add mtk keypad driver
>
> .../devicetree/bindings/input/mtk-kpd.txt | 61 +++++
> arch/arm64/configs/defconfig | 1 +
> drivers/input/keyboard/Kconfig | 9 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/mtk-kpd.c | 242 ++++++++++++++++++
> 5 files changed, 314 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
> create mode 100644 drivers/input/keyboard/mtk-kpd.c
>
> --
> 2.18.0
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5] add mtk matrix keypad driver for keypad on MTK SoC
@ 2020-04-23 5:29 ` Marco Felsch
0 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 5:29 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, linux-kernel, linux-mediatek,
linux-input, Yingjoe Chen, Andy Shevchenko, linux-arm-kernel
Hi Fengping,
On 20-04-23 09:19, Fengping yu wrote:
>
> Change since v4:
> - remove extra space and redundant lines
> - remove disable_irq_nosync and enable_irq in irq handler
> - put defconfig as a single patch
> - unified device properties interface for ACPI and device trees
This is a good changelog for reviewers.
> - fixed other issue according reviewer comments
This one is a bit useless..
Regards,
Marco
> fengping.yu (3):
> dt-bindings: add matrix keypad documentation
> arm64: configs: defconfig: enable mtk keypad config
> drivers: input: keyboard: add mtk keypad driver
>
> .../devicetree/bindings/input/mtk-kpd.txt | 61 +++++
> arch/arm64/configs/defconfig | 1 +
> drivers/input/keyboard/Kconfig | 9 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/mtk-kpd.c | 242 ++++++++++++++++++
> 5 files changed, 314 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
> create mode 100644 drivers/input/keyboard/mtk-kpd.c
>
> --
> 2.18.0
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 3/3] drivers: input: keyboard: add mtk keypad driver
2020-04-23 1:20 ` Fengping yu
(?)
@ 2020-04-23 6:47 ` Marco Felsch
-1 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 6:47 UTC (permalink / raw)
To: Fengping yu
Cc: Dmitry Torokhov, Andy Shevchenko, Yingjoe Chen, linux-kernel,
linux-mediatek, wsd_upstream, linux-input, linux-arm-kernel
Hi Fengping,
On 20-04-23 09:20, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
Missing commit message?
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
> ---
> drivers/input/keyboard/Kconfig | 9 ++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/mtk-kpd.c | 242 +++++++++++++++++++++++++++++++
> 3 files changed, 252 insertions(+)
> create mode 100644 drivers/input/keyboard/mtk-kpd.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 4706ff09f0e8..4a387d8683b1 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -772,6 +772,15 @@ config KEYBOARD_BCM
> To compile this driver as a module, choose M here: the
> module will be called bcm-keypad.
>
> +config KEYBOARD_MTK_KPD
> + tristate "MediaTek Keypad Support"
I think here are some missing deps.
> + help
> + Say Y here if you want to use the keypad.
I would write:
"Say Y here if you want to use the keypad on MediaTek SoCs"
> + If unuse, say N.
s/unuse/unsure/
> +
> + To compile this driver as a module, choose M here: the
> + module will be called mtk-kpd.
> +
> config KEYBOARD_MTK_PMIC
> tristate "MediaTek PMIC keys support"
> depends on MFD_MT6397
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index f5b17524adf2..63324d3e36c5 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -42,6 +42,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
> obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
> obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
> obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
> +obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
> obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
> obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
> obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
> diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
> new file mode 100644
> index 000000000000..7f8f091b2734
> --- /dev/null
> +++ b/drivers/input/keyboard/mtk-kpd.c
> @@ -0,0 +1,242 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 MediaTek Inc.
> + * Author Terry Chang <terry.chang@mediatek.com>
> + */
> +#include <linux/clk.h>
> +#include <linux/gpio.h>
> +#include <linux/init.h>
> +#include <linux/input/matrix_keypad.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
Pls, check if all includes are needed.
> +
> +#define KPD_NAME "mtk-kpd"
> +
> +#define KP_MEM 0x0004
> +#define KP_DEBOUNCE 0x0018
> +
> +#define KPD_DEBOUNCE_MASK GENMASK(13, 0)
> +#define KPD_DEBOUNCE_MAX 256000
> +#define KPD_NUM_MEMS 5
> +#define KPD_NUM_BITS 136 /* 4 * 32 + 8 MEM5 only use 8 BITS */
> +#define BITS_TO_U32(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(u32))
> +
> +struct mtk_keypad {
> + struct input_dev *input_dev;
> + struct clk *clk;
> + void __iomem *base;
> + unsigned int irqnr;
You don't need the irqnr here.
> + bool wakeup;
> + u32 key_debounce;
> + u32 n_rows;
> + u32 n_cols;
> + DECLARE_BITMAP(keymap_state, KPD_NUM_BITS);
> +};
> +
> +static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
> +{
> + /* use _nosync to avoid deadlock */
What did you mean by this comment?
> + struct mtk_keypad *keypad = dev_id;
> + unsigned short *keycode = keypad->input_dev->keycode;
> + DECLARE_BITMAP(new_state, KPD_NUM_BITS);
> + DECLARE_BITMAP(change, KPD_NUM_BITS);
> + int bit_nr;
> + int pressed;
> + unsigned short code;
> +
> + memcpy_fromio(new_state, keypad->base + KP_MEM, KPD_NUM_MEMS);
As I written below, pls make use of the regmap() framework.
> + bitmap_xor(change, new_state, keypad->keymap_state, KPD_NUM_BITS);
> +
> + for_each_set_bit(bit_nr, change, KPD_NUM_BITS) {
> + pressed = test_bit(bit_nr, new_state) == 0U;
I would write !test_bit() and add a comment /* pressed == 0 */
> + dev_dbg(&keypad->input_dev->dev, "%s",
> + pressed ? "pressed" : "released");
> +
> + /* per 32bit register only use low 16bit as keypad mem register */
Pls, align the comment.
> + code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
> +
> + input_report_key(keypad->input_dev, code, pressed);
> + input_sync(keypad->input_dev);
> +
> + dev_dbg(&keypad->input_dev->dev,
> + "report Linux keycode = %d\n", code);
> + }
> +
> + bitmap_copy(keypad->keymap_state, new_state, KPD_NUM_BITS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int kpd_get_dts_info(struct mtk_keypad *keypad)
> +{
> + int ret;
> + struct device *dev = keypad->input_dev->dev.parent;
> + struct device_node *node = dev->of_node;
> +
> + ret = matrix_keypad_parse_properties(dev, &keypad->n_rows,
> + &keypad->n_cols);
> +
Unnecessary newline.
> + if (ret) {
> + dev_err(dev, "Failed to parse keypad params\n");
> + return ret;
> + }
> +
> + ret = device_property_read_u32(dev, "mediatek,debounce-us",
> + &keypad->key_debounce);
> + if (ret) {
> + dev_err(dev, "Failed to read mediatek debounce time\n");
> + return ret;
> + }
> +
> + if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
> + dev_err(dev, "Debounce time exceeds the maximum allowed time 256ms\n");
> + return -EINVAL;
> + }
> +
> + keypad->wakeup = device_property_read_bool(node, "wakeup-source");
Did you test this? It should be:
device_property_read_bool(dev, "wakeup-source");
> +
> + dev_dbg(dev, "n_row=%d n_col=%d debounce=%d\n",
> + keypad->n_rows, keypad->n_cols,
> + keypad->key_debounce);
> +
> + return 0;
> +}
> +
> +static int kpd_pdrv_probe(struct platform_device *pdev)
> +{
> + struct mtk_keypad *keypad;
> + struct pinctrl *keypad_pinctrl;
> + struct pinctrl_state *kpd_default;
> + int ret;
> +
> + keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> + if (!keypad)
> + return -ENOMEM;
> +
> + bitmap_fill(keypad->keymap_state, KPD_NUM_BITS);
> +
> + keypad->input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!keypad->input_dev) {
> + dev_err(&pdev->dev, "Failed to allocate input dev\n");
> + return -ENOMEM;
> + }
> +
> + keypad->input_dev->name = KPD_NAME;
> + keypad->input_dev->id.bustype = BUS_HOST;
> +
> + ret = kpd_get_dts_info(keypad);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to get dts info\n");
> + return ret;
> + }
> +
> + ret = matrix_keypad_build_keymap(NULL, NULL,
> + keypad->n_rows,
> + keypad->n_cols,
> + NULL,
> + keypad->input_dev);
> +
Unnecessary Newline.
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to build keymap\n");
> + return ret;
> + }
> +
> + input_set_drvdata(keypad->input_dev, keypad);
> +
> + keypad->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(keypad->base)) {
> + dev_err(&pdev->dev, "Failed to get resource and iomap keypad\n");
> + return PTR_ERR(keypad->base);
> + }
> +
> + if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
> + dev_err(&pdev->dev, "Invalid debounce time value.\n");
> + return -EINVAL;
> + }
This check is already done withn the kpd_get_dts_info(). I would say to
drop the kpd_get_dts_info() function and probe it all within the probe()
function. It is not that much data you need and it removes the wakupsrc
var fromt the driver state. Furthermore devm_platform_ioremap_resource()
and devm_clk_get() parses the DT too. I would parse the deps before you
setup and fill the input_dev to fail early.
> + writew(keypad->key_debounce * 32 / 1000 & KPD_DEBOUNCE_MASK,
> + keypad->base + KP_DEBOUNCE);
Also I would avoid this completely and instead use the
regmap() interface which has mmio support.
> + keypad->clk = devm_clk_get(&pdev->dev, "kpd");
> + if (IS_ERR(keypad->clk)) {
> + return PTR_ERR(keypad->clk);
> + }
> +
> + ret = clk_prepare_enable(keypad->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
> + return ret;
> + }
Nit you can add a devm_add_action_or_reset() here to avoid the goto:
error handling.
> + keypad_pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (IS_ERR(keypad_pinctrl)) {
> + ret = PTR_ERR(keypad_pinctrl);
> + goto disable_kpd_clk;
> + }
> +
> + kpd_default = pinctrl_lookup_state(keypad_pinctrl, "default");
> + if (IS_ERR(kpd_default)) {
> + dev_err(&pdev->dev, "No default pinctrl state\n");
> + ret = PTR_ERR(kpd_default);
> + goto disable_kpd_clk;
> + }
> +
> + pinctrl_select_state(keypad_pinctrl, kpd_default);
> +
> + keypad->irqnr = platform_get_irq(pdev, 0);
As writting above, the irqnr can be a local var.
> + if (keypad->irqnr < 0) {
> + dev_err(&pdev->dev, "Failed to get irq\n");
> + ret = -keypad->irqnr;
> + goto disable_kpd_clk;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, keypad->irqnr,
> + kpd_irq_handler, 0,
> + KPD_NAME, keypad);
Why do you don't use the threaded irq [devm_request_threaded_irq()]?
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
> + keypad->irqnr, ret);
> + goto disable_kpd_clk;
> + }
> +
> + ret = input_register_device(keypad->input_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register device\n");
> + goto disable_kpd_clk;
> + }
> +
> + device_init_wakeup(&pdev->dev, keypad->wakeup);
> +
> + platform_set_drvdata(pdev, keypad);
Where do you need the drvdata? It seems useless.
> + return 0;
> +
> +disable_kpd_clk:
> + clk_disable_unprepare(keypad->clk);
> + return ret;
> +}
> +
> +static const struct of_device_id kpd_of_match[] = {
> + {.compatible = "mediatek,kp"},
> + {}
Nit { }
or
{ /*sentinel*/ }
> +};
MODULE_DEVICE_TABLE(of, kpd_of_match);
> +
> +static struct platform_driver kpd_pdrv = {
> + .probe = kpd_pdrv_probe,
> + .driver = {
> + .name = KPD_NAME,
> + .of_match_table = kpd_of_match,
> + },
> +};
> +
Unnecessary newline.
Regards,
Marco
> +module_platform_driver(kpd_pdrv);
> +
> +MODULE_AUTHOR("Mediatek Corporation");
> +MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 3/3] drivers: input: keyboard: add mtk keypad driver
@ 2020-04-23 6:47 ` Marco Felsch
0 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 6:47 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, linux-kernel, linux-mediatek,
linux-input, Yingjoe Chen, Andy Shevchenko, linux-arm-kernel
Hi Fengping,
On 20-04-23 09:20, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
Missing commit message?
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
> ---
> drivers/input/keyboard/Kconfig | 9 ++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/mtk-kpd.c | 242 +++++++++++++++++++++++++++++++
> 3 files changed, 252 insertions(+)
> create mode 100644 drivers/input/keyboard/mtk-kpd.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 4706ff09f0e8..4a387d8683b1 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -772,6 +772,15 @@ config KEYBOARD_BCM
> To compile this driver as a module, choose M here: the
> module will be called bcm-keypad.
>
> +config KEYBOARD_MTK_KPD
> + tristate "MediaTek Keypad Support"
I think here are some missing deps.
> + help
> + Say Y here if you want to use the keypad.
I would write:
"Say Y here if you want to use the keypad on MediaTek SoCs"
> + If unuse, say N.
s/unuse/unsure/
> +
> + To compile this driver as a module, choose M here: the
> + module will be called mtk-kpd.
> +
> config KEYBOARD_MTK_PMIC
> tristate "MediaTek PMIC keys support"
> depends on MFD_MT6397
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index f5b17524adf2..63324d3e36c5 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -42,6 +42,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
> obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
> obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
> obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
> +obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
> obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
> obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
> obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
> diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
> new file mode 100644
> index 000000000000..7f8f091b2734
> --- /dev/null
> +++ b/drivers/input/keyboard/mtk-kpd.c
> @@ -0,0 +1,242 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 MediaTek Inc.
> + * Author Terry Chang <terry.chang@mediatek.com>
> + */
> +#include <linux/clk.h>
> +#include <linux/gpio.h>
> +#include <linux/init.h>
> +#include <linux/input/matrix_keypad.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
Pls, check if all includes are needed.
> +
> +#define KPD_NAME "mtk-kpd"
> +
> +#define KP_MEM 0x0004
> +#define KP_DEBOUNCE 0x0018
> +
> +#define KPD_DEBOUNCE_MASK GENMASK(13, 0)
> +#define KPD_DEBOUNCE_MAX 256000
> +#define KPD_NUM_MEMS 5
> +#define KPD_NUM_BITS 136 /* 4 * 32 + 8 MEM5 only use 8 BITS */
> +#define BITS_TO_U32(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(u32))
> +
> +struct mtk_keypad {
> + struct input_dev *input_dev;
> + struct clk *clk;
> + void __iomem *base;
> + unsigned int irqnr;
You don't need the irqnr here.
> + bool wakeup;
> + u32 key_debounce;
> + u32 n_rows;
> + u32 n_cols;
> + DECLARE_BITMAP(keymap_state, KPD_NUM_BITS);
> +};
> +
> +static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
> +{
> + /* use _nosync to avoid deadlock */
What did you mean by this comment?
> + struct mtk_keypad *keypad = dev_id;
> + unsigned short *keycode = keypad->input_dev->keycode;
> + DECLARE_BITMAP(new_state, KPD_NUM_BITS);
> + DECLARE_BITMAP(change, KPD_NUM_BITS);
> + int bit_nr;
> + int pressed;
> + unsigned short code;
> +
> + memcpy_fromio(new_state, keypad->base + KP_MEM, KPD_NUM_MEMS);
As I written below, pls make use of the regmap() framework.
> + bitmap_xor(change, new_state, keypad->keymap_state, KPD_NUM_BITS);
> +
> + for_each_set_bit(bit_nr, change, KPD_NUM_BITS) {
> + pressed = test_bit(bit_nr, new_state) == 0U;
I would write !test_bit() and add a comment /* pressed == 0 */
> + dev_dbg(&keypad->input_dev->dev, "%s",
> + pressed ? "pressed" : "released");
> +
> + /* per 32bit register only use low 16bit as keypad mem register */
Pls, align the comment.
> + code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
> +
> + input_report_key(keypad->input_dev, code, pressed);
> + input_sync(keypad->input_dev);
> +
> + dev_dbg(&keypad->input_dev->dev,
> + "report Linux keycode = %d\n", code);
> + }
> +
> + bitmap_copy(keypad->keymap_state, new_state, KPD_NUM_BITS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int kpd_get_dts_info(struct mtk_keypad *keypad)
> +{
> + int ret;
> + struct device *dev = keypad->input_dev->dev.parent;
> + struct device_node *node = dev->of_node;
> +
> + ret = matrix_keypad_parse_properties(dev, &keypad->n_rows,
> + &keypad->n_cols);
> +
Unnecessary newline.
> + if (ret) {
> + dev_err(dev, "Failed to parse keypad params\n");
> + return ret;
> + }
> +
> + ret = device_property_read_u32(dev, "mediatek,debounce-us",
> + &keypad->key_debounce);
> + if (ret) {
> + dev_err(dev, "Failed to read mediatek debounce time\n");
> + return ret;
> + }
> +
> + if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
> + dev_err(dev, "Debounce time exceeds the maximum allowed time 256ms\n");
> + return -EINVAL;
> + }
> +
> + keypad->wakeup = device_property_read_bool(node, "wakeup-source");
Did you test this? It should be:
device_property_read_bool(dev, "wakeup-source");
> +
> + dev_dbg(dev, "n_row=%d n_col=%d debounce=%d\n",
> + keypad->n_rows, keypad->n_cols,
> + keypad->key_debounce);
> +
> + return 0;
> +}
> +
> +static int kpd_pdrv_probe(struct platform_device *pdev)
> +{
> + struct mtk_keypad *keypad;
> + struct pinctrl *keypad_pinctrl;
> + struct pinctrl_state *kpd_default;
> + int ret;
> +
> + keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> + if (!keypad)
> + return -ENOMEM;
> +
> + bitmap_fill(keypad->keymap_state, KPD_NUM_BITS);
> +
> + keypad->input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!keypad->input_dev) {
> + dev_err(&pdev->dev, "Failed to allocate input dev\n");
> + return -ENOMEM;
> + }
> +
> + keypad->input_dev->name = KPD_NAME;
> + keypad->input_dev->id.bustype = BUS_HOST;
> +
> + ret = kpd_get_dts_info(keypad);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to get dts info\n");
> + return ret;
> + }
> +
> + ret = matrix_keypad_build_keymap(NULL, NULL,
> + keypad->n_rows,
> + keypad->n_cols,
> + NULL,
> + keypad->input_dev);
> +
Unnecessary Newline.
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to build keymap\n");
> + return ret;
> + }
> +
> + input_set_drvdata(keypad->input_dev, keypad);
> +
> + keypad->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(keypad->base)) {
> + dev_err(&pdev->dev, "Failed to get resource and iomap keypad\n");
> + return PTR_ERR(keypad->base);
> + }
> +
> + if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
> + dev_err(&pdev->dev, "Invalid debounce time value.\n");
> + return -EINVAL;
> + }
This check is already done withn the kpd_get_dts_info(). I would say to
drop the kpd_get_dts_info() function and probe it all within the probe()
function. It is not that much data you need and it removes the wakupsrc
var fromt the driver state. Furthermore devm_platform_ioremap_resource()
and devm_clk_get() parses the DT too. I would parse the deps before you
setup and fill the input_dev to fail early.
> + writew(keypad->key_debounce * 32 / 1000 & KPD_DEBOUNCE_MASK,
> + keypad->base + KP_DEBOUNCE);
Also I would avoid this completely and instead use the
regmap() interface which has mmio support.
> + keypad->clk = devm_clk_get(&pdev->dev, "kpd");
> + if (IS_ERR(keypad->clk)) {
> + return PTR_ERR(keypad->clk);
> + }
> +
> + ret = clk_prepare_enable(keypad->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
> + return ret;
> + }
Nit you can add a devm_add_action_or_reset() here to avoid the goto:
error handling.
> + keypad_pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (IS_ERR(keypad_pinctrl)) {
> + ret = PTR_ERR(keypad_pinctrl);
> + goto disable_kpd_clk;
> + }
> +
> + kpd_default = pinctrl_lookup_state(keypad_pinctrl, "default");
> + if (IS_ERR(kpd_default)) {
> + dev_err(&pdev->dev, "No default pinctrl state\n");
> + ret = PTR_ERR(kpd_default);
> + goto disable_kpd_clk;
> + }
> +
> + pinctrl_select_state(keypad_pinctrl, kpd_default);
> +
> + keypad->irqnr = platform_get_irq(pdev, 0);
As writting above, the irqnr can be a local var.
> + if (keypad->irqnr < 0) {
> + dev_err(&pdev->dev, "Failed to get irq\n");
> + ret = -keypad->irqnr;
> + goto disable_kpd_clk;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, keypad->irqnr,
> + kpd_irq_handler, 0,
> + KPD_NAME, keypad);
Why do you don't use the threaded irq [devm_request_threaded_irq()]?
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
> + keypad->irqnr, ret);
> + goto disable_kpd_clk;
> + }
> +
> + ret = input_register_device(keypad->input_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register device\n");
> + goto disable_kpd_clk;
> + }
> +
> + device_init_wakeup(&pdev->dev, keypad->wakeup);
> +
> + platform_set_drvdata(pdev, keypad);
Where do you need the drvdata? It seems useless.
> + return 0;
> +
> +disable_kpd_clk:
> + clk_disable_unprepare(keypad->clk);
> + return ret;
> +}
> +
> +static const struct of_device_id kpd_of_match[] = {
> + {.compatible = "mediatek,kp"},
> + {}
Nit { }
or
{ /*sentinel*/ }
> +};
MODULE_DEVICE_TABLE(of, kpd_of_match);
> +
> +static struct platform_driver kpd_pdrv = {
> + .probe = kpd_pdrv_probe,
> + .driver = {
> + .name = KPD_NAME,
> + .of_match_table = kpd_of_match,
> + },
> +};
> +
Unnecessary newline.
Regards,
Marco
> +module_platform_driver(kpd_pdrv);
> +
> +MODULE_AUTHOR("Mediatek Corporation");
> +MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
> +MODULE_LICENSE("GPL");
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 3/3] drivers: input: keyboard: add mtk keypad driver
@ 2020-04-23 6:47 ` Marco Felsch
0 siblings, 0 replies; 24+ messages in thread
From: Marco Felsch @ 2020-04-23 6:47 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, linux-kernel, linux-mediatek,
linux-input, Yingjoe Chen, Andy Shevchenko, linux-arm-kernel
Hi Fengping,
On 20-04-23 09:20, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
Missing commit message?
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
> ---
> drivers/input/keyboard/Kconfig | 9 ++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/mtk-kpd.c | 242 +++++++++++++++++++++++++++++++
> 3 files changed, 252 insertions(+)
> create mode 100644 drivers/input/keyboard/mtk-kpd.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 4706ff09f0e8..4a387d8683b1 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -772,6 +772,15 @@ config KEYBOARD_BCM
> To compile this driver as a module, choose M here: the
> module will be called bcm-keypad.
>
> +config KEYBOARD_MTK_KPD
> + tristate "MediaTek Keypad Support"
I think here are some missing deps.
> + help
> + Say Y here if you want to use the keypad.
I would write:
"Say Y here if you want to use the keypad on MediaTek SoCs"
> + If unuse, say N.
s/unuse/unsure/
> +
> + To compile this driver as a module, choose M here: the
> + module will be called mtk-kpd.
> +
> config KEYBOARD_MTK_PMIC
> tristate "MediaTek PMIC keys support"
> depends on MFD_MT6397
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index f5b17524adf2..63324d3e36c5 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -42,6 +42,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
> obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
> obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
> obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
> +obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
> obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
> obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
> obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
> diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
> new file mode 100644
> index 000000000000..7f8f091b2734
> --- /dev/null
> +++ b/drivers/input/keyboard/mtk-kpd.c
> @@ -0,0 +1,242 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 MediaTek Inc.
> + * Author Terry Chang <terry.chang@mediatek.com>
> + */
> +#include <linux/clk.h>
> +#include <linux/gpio.h>
> +#include <linux/init.h>
> +#include <linux/input/matrix_keypad.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
Pls, check if all includes are needed.
> +
> +#define KPD_NAME "mtk-kpd"
> +
> +#define KP_MEM 0x0004
> +#define KP_DEBOUNCE 0x0018
> +
> +#define KPD_DEBOUNCE_MASK GENMASK(13, 0)
> +#define KPD_DEBOUNCE_MAX 256000
> +#define KPD_NUM_MEMS 5
> +#define KPD_NUM_BITS 136 /* 4 * 32 + 8 MEM5 only use 8 BITS */
> +#define BITS_TO_U32(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(u32))
> +
> +struct mtk_keypad {
> + struct input_dev *input_dev;
> + struct clk *clk;
> + void __iomem *base;
> + unsigned int irqnr;
You don't need the irqnr here.
> + bool wakeup;
> + u32 key_debounce;
> + u32 n_rows;
> + u32 n_cols;
> + DECLARE_BITMAP(keymap_state, KPD_NUM_BITS);
> +};
> +
> +static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
> +{
> + /* use _nosync to avoid deadlock */
What did you mean by this comment?
> + struct mtk_keypad *keypad = dev_id;
> + unsigned short *keycode = keypad->input_dev->keycode;
> + DECLARE_BITMAP(new_state, KPD_NUM_BITS);
> + DECLARE_BITMAP(change, KPD_NUM_BITS);
> + int bit_nr;
> + int pressed;
> + unsigned short code;
> +
> + memcpy_fromio(new_state, keypad->base + KP_MEM, KPD_NUM_MEMS);
As I written below, pls make use of the regmap() framework.
> + bitmap_xor(change, new_state, keypad->keymap_state, KPD_NUM_BITS);
> +
> + for_each_set_bit(bit_nr, change, KPD_NUM_BITS) {
> + pressed = test_bit(bit_nr, new_state) == 0U;
I would write !test_bit() and add a comment /* pressed == 0 */
> + dev_dbg(&keypad->input_dev->dev, "%s",
> + pressed ? "pressed" : "released");
> +
> + /* per 32bit register only use low 16bit as keypad mem register */
Pls, align the comment.
> + code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
> +
> + input_report_key(keypad->input_dev, code, pressed);
> + input_sync(keypad->input_dev);
> +
> + dev_dbg(&keypad->input_dev->dev,
> + "report Linux keycode = %d\n", code);
> + }
> +
> + bitmap_copy(keypad->keymap_state, new_state, KPD_NUM_BITS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int kpd_get_dts_info(struct mtk_keypad *keypad)
> +{
> + int ret;
> + struct device *dev = keypad->input_dev->dev.parent;
> + struct device_node *node = dev->of_node;
> +
> + ret = matrix_keypad_parse_properties(dev, &keypad->n_rows,
> + &keypad->n_cols);
> +
Unnecessary newline.
> + if (ret) {
> + dev_err(dev, "Failed to parse keypad params\n");
> + return ret;
> + }
> +
> + ret = device_property_read_u32(dev, "mediatek,debounce-us",
> + &keypad->key_debounce);
> + if (ret) {
> + dev_err(dev, "Failed to read mediatek debounce time\n");
> + return ret;
> + }
> +
> + if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
> + dev_err(dev, "Debounce time exceeds the maximum allowed time 256ms\n");
> + return -EINVAL;
> + }
> +
> + keypad->wakeup = device_property_read_bool(node, "wakeup-source");
Did you test this? It should be:
device_property_read_bool(dev, "wakeup-source");
> +
> + dev_dbg(dev, "n_row=%d n_col=%d debounce=%d\n",
> + keypad->n_rows, keypad->n_cols,
> + keypad->key_debounce);
> +
> + return 0;
> +}
> +
> +static int kpd_pdrv_probe(struct platform_device *pdev)
> +{
> + struct mtk_keypad *keypad;
> + struct pinctrl *keypad_pinctrl;
> + struct pinctrl_state *kpd_default;
> + int ret;
> +
> + keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> + if (!keypad)
> + return -ENOMEM;
> +
> + bitmap_fill(keypad->keymap_state, KPD_NUM_BITS);
> +
> + keypad->input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!keypad->input_dev) {
> + dev_err(&pdev->dev, "Failed to allocate input dev\n");
> + return -ENOMEM;
> + }
> +
> + keypad->input_dev->name = KPD_NAME;
> + keypad->input_dev->id.bustype = BUS_HOST;
> +
> + ret = kpd_get_dts_info(keypad);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to get dts info\n");
> + return ret;
> + }
> +
> + ret = matrix_keypad_build_keymap(NULL, NULL,
> + keypad->n_rows,
> + keypad->n_cols,
> + NULL,
> + keypad->input_dev);
> +
Unnecessary Newline.
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to build keymap\n");
> + return ret;
> + }
> +
> + input_set_drvdata(keypad->input_dev, keypad);
> +
> + keypad->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(keypad->base)) {
> + dev_err(&pdev->dev, "Failed to get resource and iomap keypad\n");
> + return PTR_ERR(keypad->base);
> + }
> +
> + if (keypad->key_debounce > KPD_DEBOUNCE_MAX) {
> + dev_err(&pdev->dev, "Invalid debounce time value.\n");
> + return -EINVAL;
> + }
This check is already done withn the kpd_get_dts_info(). I would say to
drop the kpd_get_dts_info() function and probe it all within the probe()
function. It is not that much data you need and it removes the wakupsrc
var fromt the driver state. Furthermore devm_platform_ioremap_resource()
and devm_clk_get() parses the DT too. I would parse the deps before you
setup and fill the input_dev to fail early.
> + writew(keypad->key_debounce * 32 / 1000 & KPD_DEBOUNCE_MASK,
> + keypad->base + KP_DEBOUNCE);
Also I would avoid this completely and instead use the
regmap() interface which has mmio support.
> + keypad->clk = devm_clk_get(&pdev->dev, "kpd");
> + if (IS_ERR(keypad->clk)) {
> + return PTR_ERR(keypad->clk);
> + }
> +
> + ret = clk_prepare_enable(keypad->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
> + return ret;
> + }
Nit you can add a devm_add_action_or_reset() here to avoid the goto:
error handling.
> + keypad_pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (IS_ERR(keypad_pinctrl)) {
> + ret = PTR_ERR(keypad_pinctrl);
> + goto disable_kpd_clk;
> + }
> +
> + kpd_default = pinctrl_lookup_state(keypad_pinctrl, "default");
> + if (IS_ERR(kpd_default)) {
> + dev_err(&pdev->dev, "No default pinctrl state\n");
> + ret = PTR_ERR(kpd_default);
> + goto disable_kpd_clk;
> + }
> +
> + pinctrl_select_state(keypad_pinctrl, kpd_default);
> +
> + keypad->irqnr = platform_get_irq(pdev, 0);
As writting above, the irqnr can be a local var.
> + if (keypad->irqnr < 0) {
> + dev_err(&pdev->dev, "Failed to get irq\n");
> + ret = -keypad->irqnr;
> + goto disable_kpd_clk;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, keypad->irqnr,
> + kpd_irq_handler, 0,
> + KPD_NAME, keypad);
Why do you don't use the threaded irq [devm_request_threaded_irq()]?
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
> + keypad->irqnr, ret);
> + goto disable_kpd_clk;
> + }
> +
> + ret = input_register_device(keypad->input_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register device\n");
> + goto disable_kpd_clk;
> + }
> +
> + device_init_wakeup(&pdev->dev, keypad->wakeup);
> +
> + platform_set_drvdata(pdev, keypad);
Where do you need the drvdata? It seems useless.
> + return 0;
> +
> +disable_kpd_clk:
> + clk_disable_unprepare(keypad->clk);
> + return ret;
> +}
> +
> +static const struct of_device_id kpd_of_match[] = {
> + {.compatible = "mediatek,kp"},
> + {}
Nit { }
or
{ /*sentinel*/ }
> +};
MODULE_DEVICE_TABLE(of, kpd_of_match);
> +
> +static struct platform_driver kpd_pdrv = {
> + .probe = kpd_pdrv_probe,
> + .driver = {
> + .name = KPD_NAME,
> + .of_match_table = kpd_of_match,
> + },
> +};
> +
Unnecessary newline.
Regards,
Marco
> +module_platform_driver(kpd_pdrv);
> +
> +MODULE_AUTHOR("Mediatek Corporation");
> +MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
> +MODULE_LICENSE("GPL");
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 3/3] drivers: input: keyboard: add mtk keypad driver
2020-04-23 1:20 ` Fengping yu
(?)
@ 2020-04-23 10:41 ` Andy Shevchenko
-1 siblings, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2020-04-23 10:41 UTC (permalink / raw)
To: Fengping yu
Cc: Dmitry Torokhov, Marco Felsch, Yingjoe Chen, linux-kernel,
linux-mediatek, wsd_upstream, linux-input, linux-arm-kernel
On Thu, Apr 23, 2020 at 09:20:02AM +0800, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
This misses the commit message. It's a show stopper for such patches.
Read this [1].
[1]: https://chris.beams.io/posts/git-commit/
...
> +#define KPD_DEBOUNCE_MASK GENMASK(13, 0)
> +#define KPD_DEBOUNCE_MAX 256000
Is there any unit in which debounce time is being measured? Add it as a suffix
to the definition, if it's possible.
...
> +#define BITS_TO_U32(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(u32))
This already defined in bits.h.
...
> + keypad->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(keypad->base)) {
> + dev_err(&pdev->dev, "Failed to get resource and iomap keypad\n");
This is duplicate noisy message, please remove.
> + return PTR_ERR(keypad->base);
> + }
...
> + writew(keypad->key_debounce * 32 / 1000 & KPD_DEBOUNCE_MASK,
Perhaps one pair of parentheses is needed to make logic clear.
(Yes, I remember I commented on this in earlier versions where it was many
parentheses around above calculations, but you have to use common sense as well)
> + keypad->base + KP_DEBOUNCE);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 3/3] drivers: input: keyboard: add mtk keypad driver
@ 2020-04-23 10:41 ` Andy Shevchenko
0 siblings, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2020-04-23 10:41 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, Marco Felsch, linux-kernel,
linux-mediatek, linux-input, Yingjoe Chen, linux-arm-kernel
On Thu, Apr 23, 2020 at 09:20:02AM +0800, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
This misses the commit message. It's a show stopper for such patches.
Read this [1].
[1]: https://chris.beams.io/posts/git-commit/
...
> +#define KPD_DEBOUNCE_MASK GENMASK(13, 0)
> +#define KPD_DEBOUNCE_MAX 256000
Is there any unit in which debounce time is being measured? Add it as a suffix
to the definition, if it's possible.
...
> +#define BITS_TO_U32(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(u32))
This already defined in bits.h.
...
> + keypad->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(keypad->base)) {
> + dev_err(&pdev->dev, "Failed to get resource and iomap keypad\n");
This is duplicate noisy message, please remove.
> + return PTR_ERR(keypad->base);
> + }
...
> + writew(keypad->key_debounce * 32 / 1000 & KPD_DEBOUNCE_MASK,
Perhaps one pair of parentheses is needed to make logic clear.
(Yes, I remember I commented on this in earlier versions where it was many
parentheses around above calculations, but you have to use common sense as well)
> + keypad->base + KP_DEBOUNCE);
--
With Best Regards,
Andy Shevchenko
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 3/3] drivers: input: keyboard: add mtk keypad driver
@ 2020-04-23 10:41 ` Andy Shevchenko
0 siblings, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2020-04-23 10:41 UTC (permalink / raw)
To: Fengping yu
Cc: wsd_upstream, Dmitry Torokhov, Marco Felsch, linux-kernel,
linux-mediatek, linux-input, Yingjoe Chen, linux-arm-kernel
On Thu, Apr 23, 2020 at 09:20:02AM +0800, Fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
This misses the commit message. It's a show stopper for such patches.
Read this [1].
[1]: https://chris.beams.io/posts/git-commit/
...
> +#define KPD_DEBOUNCE_MASK GENMASK(13, 0)
> +#define KPD_DEBOUNCE_MAX 256000
Is there any unit in which debounce time is being measured? Add it as a suffix
to the definition, if it's possible.
...
> +#define BITS_TO_U32(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(u32))
This already defined in bits.h.
...
> + keypad->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(keypad->base)) {
> + dev_err(&pdev->dev, "Failed to get resource and iomap keypad\n");
This is duplicate noisy message, please remove.
> + return PTR_ERR(keypad->base);
> + }
...
> + writew(keypad->key_debounce * 32 / 1000 & KPD_DEBOUNCE_MASK,
Perhaps one pair of parentheses is needed to make logic clear.
(Yes, I remember I commented on this in earlier versions where it was many
parentheses around above calculations, but you have to use common sense as well)
> + keypad->base + KP_DEBOUNCE);
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2020-04-23 10:41 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-23 1:19 [PATCH v5] add mtk matrix keypad driver for keypad on MTK SoC Fengping yu
2020-04-23 1:19 ` Fengping yu
2020-04-23 1:19 ` Fengping yu
2020-04-23 1:19 ` [PATCH v5 1/3] dt-bindings: add matrix keypad documentation Fengping yu
2020-04-23 1:19 ` Fengping yu
2020-04-23 5:26 ` Marco Felsch
2020-04-23 5:26 ` Marco Felsch
2020-04-23 5:26 ` Marco Felsch
2020-04-23 1:20 ` [PATCH v5 2/3] arm64: configs: defconfig: enable mtk keypad config Fengping yu
2020-04-23 1:20 ` Fengping yu
2020-04-23 5:28 ` Marco Felsch
2020-04-23 5:28 ` Marco Felsch
2020-04-23 5:28 ` Marco Felsch
2020-04-23 1:20 ` [PATCH v5 3/3] drivers: input: keyboard: add mtk keypad driver Fengping yu
2020-04-23 1:20 ` Fengping yu
2020-04-23 6:47 ` Marco Felsch
2020-04-23 6:47 ` Marco Felsch
2020-04-23 6:47 ` Marco Felsch
2020-04-23 10:41 ` Andy Shevchenko
2020-04-23 10:41 ` Andy Shevchenko
2020-04-23 10:41 ` Andy Shevchenko
2020-04-23 5:29 ` [PATCH v5] add mtk matrix keypad driver for keypad on MTK SoC Marco Felsch
2020-04-23 5:29 ` Marco Felsch
2020-04-23 5:29 ` Marco Felsch
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.