Re: [PATCH 3/4] input: Add new sun4i-keypad driver

2015-09-21 Thread Dmitry Torokhov
Hi Yassin,

On Wed, Sep 16, 2015 at 12:05:56AM +1000, yassinjaf...@gmail.com wrote:
> From: Yassin Jaffer 
> 
> Allwinnner SUN4i Keypad controller is used to interface a SoC
> with a matrix-typekeypad 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 patch adds a driver support to this.
> The keypad controller driver does not give proper information
> if more that two keys are selected.
> 
> Signed-off-by: Yassin Jaffer 
> ---
>  drivers/input/keyboard/Kconfig|  11 ++
>  drivers/input/keyboard/Makefile   |   1 +
>  drivers/input/keyboard/sun4i-keypad.c | 361 
> ++
>  3 files changed, 373 insertions(+)
>  create mode 100644 drivers/input/keyboard/sun4i-keypad.c
> 
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 2e80107..4f2f3f8 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -590,6 +590,17 @@ config KEYBOARD_SUN4I_LRADC
> To compile this driver as a module, choose M here: the
> module will be called sun4i-lradc-keys.
>  
> +config KEYBOARD_SUN4I_KEYPAD
> + tristate "Allwinner sun4i keypad support"
> + depends on ARCH_SUNXI
> + select INPUT_MATRIXKMAP
> + help
> +   This selects support for the Allwinner keypad
> +   on Allwinner sunxi SoCs.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called sun4i-keypad.
> +
>  config KEYBOARD_DAVINCI
>   tristate "TI DaVinci Key Scan"
>   depends on ARCH_DAVINCI_DM365
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 1d416dd..d9f54b4 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_KEYBOARD_STMPE)+= 
> stmpe-keypad.o
>  obj-$(CONFIG_KEYBOARD_STOWAWAY)  += stowaway.o
>  obj-$(CONFIG_KEYBOARD_ST_KEYSCAN)+= st-keyscan.o
>  obj-$(CONFIG_KEYBOARD_SUN4I_LRADC)   += sun4i-lradc-keys.o
> +obj-$(CONFIG_KEYBOARD_SUN4I_KEYPAD)  += sun4i-keypad.o
>  obj-$(CONFIG_KEYBOARD_SUNKBD)+= sunkbd.o
>  obj-$(CONFIG_KEYBOARD_TC3589X)   += tc3589x-keypad.o
>  obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> diff --git a/drivers/input/keyboard/sun4i-keypad.c 
> b/drivers/input/keyboard/sun4i-keypad.c
> new file mode 100644
> index 000..995f9665
> --- /dev/null
> +++ b/drivers/input/keyboard/sun4i-keypad.c
> @@ -0,0 +1,361 @@
> +/*
> + * Allwinner sun4i keypad Controller driver
> + *
> + * Copyright (C) 2015 Yassin Jaffer 
> + *
> + * Parts of this software are based on (derived from):
> + * Copyright (C) 2013-2015 lim...@allwinnertech.com,
> + *qys
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * Keypad Controller registers
> + */
> +#define KP_CTL   0x00  /* Keypad Control register */
> +#define KP_TIMING0x04  /* Keypad Timing register */
> +#define KP_INT_CFG   0x08  /* Keypad interrupt Config register */
> +#define KP_INT_STA   0x0c  /* Keypad interrupt Status register */
> +
> +#define KP_IN_OFFSET 0x10 /* Keypad Input Data register 0 */
> +#define KP_INX_OFFSET(reg_n) (KP_IN_OFFSET + 4 * (reg_n))
> +
> +/* KP_CTL bits */
> +#define ENABLE(x)((x) << 0)
> +#define COLMASK(x)   ((~x & 0xff) << 8)
> +#define ROWMASK(x)   ((~x & 0xff) << 16)
> +
> +/* KP_TIMING bits */
> +#define SCAN_CYCLE(x)((x) << 0)
> +#define DBC_CYCLE(x) ((x) << 16)
> +
> +/* KP_INT_CFG bits */
> +#define KP_IRQ_FEDGE BIT(0)
> +#define  KP_IRQ_REDGEBIT(1)
> +
> +/* KP_INT_STA bits */
> +#define KP_STA_FEDGE BIT(0)
> +#define  KP_STA_REDGEBIT(1)
> +
> +#define KP_MAX_ROWS  8
> +#define KP_MAX_COLS  8
> +#define N_ROWS_REG   2
> +#define KP_ROW_SHIFT 3
> +#define KP_BIT_SHIFT32
> +
> +#define MAX_MATRIX_KEY_NUM   (KP_MAX_ROWS * KP_MAX_COLS)
> +
> +#define 

Re: [PATCH 3/4] input: Add new sun4i-keypad driver

2015-09-18 Thread Maxime Ripard
Hi Yassin,

On Fri, Sep 18, 2015 at 10:19:55AM +1000, Yassin Jaffer wrote:
> Hi Maxime
> 
> I appreciate your time and efforts .
> 
> Do you need that rate to be enforced, or is it some leftover from the
> > allwinner BSP?
> 
> 
> I've found that clock rate works fine with the default denounce and scan
> cycle.

It was not really my point. My point was do you *need* it to operate.

And so your properties were in clock cycles?

Please use a unit indenpendant of the clock rate, like seconds or Hz
(or any multiple of them).

> By the way do you have any dev board which expose the keypad pins?

I don't think I have any.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH 3/4] input: Add new sun4i-keypad driver

2015-09-17 Thread Maxime Ripard
Hi,

On Wed, Sep 16, 2015 at 12:05:56AM +1000, yassinjaf...@gmail.com wrote:
> From: Yassin Jaffer 
> 
> Allwinnner SUN4i Keypad controller is used to interface a SoC
> with a matrix-typekeypad 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 patch adds a driver support to this.
> The keypad controller driver does not give proper information
> if more that two keys are selected.
> 
> Signed-off-by: Yassin Jaffer 
> ---
>  drivers/input/keyboard/Kconfig|  11 ++
>  drivers/input/keyboard/Makefile   |   1 +
>  drivers/input/keyboard/sun4i-keypad.c | 361 
> ++
>  3 files changed, 373 insertions(+)
>  create mode 100644 drivers/input/keyboard/sun4i-keypad.c
> 
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 2e80107..4f2f3f8 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -590,6 +590,17 @@ config KEYBOARD_SUN4I_LRADC
> To compile this driver as a module, choose M here: the
> module will be called sun4i-lradc-keys.
>  
> +config KEYBOARD_SUN4I_KEYPAD
> + tristate "Allwinner sun4i keypad support"
> + depends on ARCH_SUNXI

Is this IP found on all the know SoCs, or just a subset of them?

You probably want to add || COMPILE_TEST in that depends on too.

> + select INPUT_MATRIXKMAP
> + help
> +   This selects support for the Allwinner keypad
> +   on Allwinner sunxi SoCs.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called sun4i-keypad.
> +
>  config KEYBOARD_DAVINCI
>   tristate "TI DaVinci Key Scan"
>   depends on ARCH_DAVINCI_DM365
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 1d416dd..d9f54b4 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_KEYBOARD_STMPE)+= 
> stmpe-keypad.o
>  obj-$(CONFIG_KEYBOARD_STOWAWAY)  += stowaway.o
>  obj-$(CONFIG_KEYBOARD_ST_KEYSCAN)+= st-keyscan.o
>  obj-$(CONFIG_KEYBOARD_SUN4I_LRADC)   += sun4i-lradc-keys.o
> +obj-$(CONFIG_KEYBOARD_SUN4I_KEYPAD)  += sun4i-keypad.o
>  obj-$(CONFIG_KEYBOARD_SUNKBD)+= sunkbd.o
>  obj-$(CONFIG_KEYBOARD_TC3589X)   += tc3589x-keypad.o
>  obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> diff --git a/drivers/input/keyboard/sun4i-keypad.c 
> b/drivers/input/keyboard/sun4i-keypad.c
> new file mode 100644
> index 000..995f9665
> --- /dev/null
> +++ b/drivers/input/keyboard/sun4i-keypad.c
> @@ -0,0 +1,361 @@
> +/*
> + * Allwinner sun4i keypad Controller driver
> + *
> + * Copyright (C) 2015 Yassin Jaffer 
> + *
> + * Parts of this software are based on (derived from):
> + * Copyright (C) 2013-2015 lim...@allwinnertech.com,
> + *qys
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * Keypad Controller registers
> + */
> +#define KP_CTL   0x00  /* Keypad Control register */
> +#define KP_TIMING0x04  /* Keypad Timing register */
> +#define KP_INT_CFG   0x08  /* Keypad interrupt Config register */
> +#define KP_INT_STA   0x0c  /* Keypad interrupt Status register */
> +
> +#define KP_IN_OFFSET 0x10 /* Keypad Input Data register 0 */
> +#define KP_INX_OFFSET(reg_n) (KP_IN_OFFSET + 4 * (reg_n))
> +
> +/* KP_CTL bits */
> +#define ENABLE(x)((x) << 0)
> +#define COLMASK(x)   ((~x & 0xff) << 8)
> +#define ROWMASK(x)   ((~x & 0xff) << 16)

Having the name of the register in that define would be great, to spot
more easily issues (writing a value to another register, for example).

Something like KP_CTL_ENABLE(x) in this case.

> +/* KP_TIMING bits */
> +#define SCAN_CYCLE(x)((x) << 0)
> +#define DBC_CYCLE(x) ((x) << 16)
> +
> +/* KP_INT_CFG bits */
> +#define KP_IRQ_FEDGE BIT(0)
> +#define  KP_IRQ_REDGEBIT(1)
> +
> +/* KP_INT_STA bits */
> +#define KP_STA_FEDGE BIT(0)

[PATCH 3/4] input: Add new sun4i-keypad driver

2015-09-15 Thread yassinjaffer
From: Yassin Jaffer 

Allwinnner SUN4i Keypad controller is used to interface a SoC
with a matrix-typekeypad 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 patch adds a driver support to this.
The keypad controller driver does not give proper information
if more that two keys are selected.

Signed-off-by: Yassin Jaffer 
---
 drivers/input/keyboard/Kconfig|  11 ++
 drivers/input/keyboard/Makefile   |   1 +
 drivers/input/keyboard/sun4i-keypad.c | 361 ++
 3 files changed, 373 insertions(+)
 create mode 100644 drivers/input/keyboard/sun4i-keypad.c

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 2e80107..4f2f3f8 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -590,6 +590,17 @@ config KEYBOARD_SUN4I_LRADC
  To compile this driver as a module, choose M here: the
  module will be called sun4i-lradc-keys.
 
+config KEYBOARD_SUN4I_KEYPAD
+   tristate "Allwinner sun4i keypad support"
+   depends on ARCH_SUNXI
+   select INPUT_MATRIXKMAP
+   help
+ This selects support for the Allwinner keypad
+ on Allwinner sunxi SoCs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called sun4i-keypad.
+
 config KEYBOARD_DAVINCI
tristate "TI DaVinci Key Scan"
depends on ARCH_DAVINCI_DM365
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 1d416dd..d9f54b4 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_KEYBOARD_STMPE)  += stmpe-keypad.o
 obj-$(CONFIG_KEYBOARD_STOWAWAY)+= stowaway.o
 obj-$(CONFIG_KEYBOARD_ST_KEYSCAN)  += st-keyscan.o
 obj-$(CONFIG_KEYBOARD_SUN4I_LRADC) += sun4i-lradc-keys.o
+obj-$(CONFIG_KEYBOARD_SUN4I_KEYPAD)+= sun4i-keypad.o
 obj-$(CONFIG_KEYBOARD_SUNKBD)  += sunkbd.o
 obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
 obj-$(CONFIG_KEYBOARD_TEGRA)   += tegra-kbc.o
diff --git a/drivers/input/keyboard/sun4i-keypad.c 
b/drivers/input/keyboard/sun4i-keypad.c
new file mode 100644
index 000..995f9665
--- /dev/null
+++ b/drivers/input/keyboard/sun4i-keypad.c
@@ -0,0 +1,361 @@
+/*
+ * Allwinner sun4i keypad Controller driver
+ *
+ * Copyright (C) 2015 Yassin Jaffer 
+ *
+ * Parts of this software are based on (derived from):
+ * Copyright (C) 2013-2015 lim...@allwinnertech.com,
+ *  qys
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Keypad Controller registers
+ */
+#define KP_CTL 0x00  /* Keypad Control register */
+#define KP_TIMING  0x04  /* Keypad Timing register */
+#define KP_INT_CFG 0x08  /* Keypad interrupt Config register */
+#define KP_INT_STA 0x0c  /* Keypad interrupt Status register */
+
+#define KP_IN_OFFSET   0x10 /* Keypad Input Data register 0 */
+#define KP_INX_OFFSET(reg_n)   (KP_IN_OFFSET + 4 * (reg_n))
+
+/* KP_CTL bits */
+#define ENABLE(x)  ((x) << 0)
+#define COLMASK(x) ((~x & 0xff) << 8)
+#define ROWMASK(x) ((~x & 0xff) << 16)
+
+/* KP_TIMING bits */
+#define SCAN_CYCLE(x)  ((x) << 0)
+#define DBC_CYCLE(x)   ((x) << 16)
+
+/* KP_INT_CFG bits */
+#define KP_IRQ_FEDGE   BIT(0)
+#defineKP_IRQ_REDGEBIT(1)
+
+/* KP_INT_STA bits */
+#define KP_STA_FEDGE   BIT(0)
+#defineKP_STA_REDGEBIT(1)
+
+#define KP_MAX_ROWS8
+#define KP_MAX_COLS8
+#define N_ROWS_REG 2
+#define KP_ROW_SHIFT   3
+#define KP_BIT_SHIFT32
+
+#define MAX_MATRIX_KEY_NUM (KP_MAX_ROWS * KP_MAX_COLS)
+
+#define KP_BASE_CLK100
+#define MIN_CYCLE  0x10
+#define MIN_SCAN_CYCLE 0x100
+#define MIN_DBC_CYCLE  0x200
+
+/*
+ * keypad Controller structure: stores sunxi keypad controller information
+ *
+ * @dev:   parent device
+ * @input: pointer to input device