* [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
@ 2009-09-17 20:06 miguel.aguilar
2009-09-18 5:35 ` Nori, Sekhar
0 siblings, 1 reply; 10+ messages in thread
From: miguel.aguilar @ 2009-09-17 20:06 UTC (permalink / raw)
To: nsnehaprabha, davinci-linux-open-source, linux-input
Cc: todd.fischer, diego.dompe, clark.becker, santiago.nunez, Miguel Aguilar
From: Miguel Aguilar <miguel.aguilar@ridgerun.com>
Adds the driver for enabling keypad support on DM365 platform.
This driver was tested on DM365 EVM rev c.
Signed-off-by: Miguel Aguilar <miguel.aguilar@ridgerun.com>
---
arch/arm/mach-davinci/include/mach/keypad.h | 35 +++
drivers/input/keyboard/Kconfig | 6 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/davinci_dm365_keypad.c | 331 +++++++++++++++++++++++++
4 files changed, 373 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-davinci/include/mach/keypad.h
create mode 100644 drivers/input/keyboard/davinci_dm365_keypad.c
diff --git a/arch/arm/mach-davinci/include/mach/keypad.h b/arch/arm/mach-davinci/include/mach/keypad.h
new file mode 100644
index 0000000..922d20e
--- /dev/null
+++ b/arch/arm/mach-davinci/include/mach/keypad.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2009 Texas Instruments, Inc
+ *
+ * Author: Miguel Aguilar <miguel.aguilar@ridgerun.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef DAVINCI_KEYPAD_H
+#define DAVINCI_KEYPAD_H
+
+#include <linux/io.h>
+
+struct davinci_kp_platform_data {
+ int *keymap;
+ u32 keymapsize;
+ u32 rep:1;
+ u32 strobe;
+ u32 interval;
+};
+
+#endif
+
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a6b989a..08ed7d4 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -361,4 +361,10 @@ config KEYBOARD_XTKBD
To compile this driver as a module, choose M here: the
module will be called xtkbd.
+config KEYBOARD_DAVINCI_DM365
+ tristate "TI DaVinci DM365 Keypad"
+ depends on ARCH_DAVINCI_DM365
+ help
+ Supports the keypad module on the DM365
+
endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index b5b5eae..1921bac 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
obj-$(CONFIG_KEYBOARD_TOSA) += tosakbd.o
obj-$(CONFIG_KEYBOARD_XTKBD) += xtkbd.o
+obj-$(CONFIG_KEYBOARD_DAVINCI_DM365) += davinci_dm365_keypad.o
diff --git a/drivers/input/keyboard/davinci_dm365_keypad.c b/drivers/input/keyboard/davinci_dm365_keypad.c
new file mode 100644
index 0000000..5db8eed
--- /dev/null
+++ b/drivers/input/keyboard/davinci_dm365_keypad.c
@@ -0,0 +1,331 @@
+/*
+ * DaVinci DM365 Keypad Driver
+ *
+ * Copyright (C) 2009 Texas Instruments, Inc
+ *
+ * Author: Miguel Aguilar <miguel.aguilar@ridgerun.com>
+ *
+ * Intial Code: Sandeep Paulraj <s-paulraj@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/types.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/errno.h>
+
+#include <asm/irq.h>
+
+#include <mach/hardware.h>
+#include <mach/irqs.h>
+#include <mach/keypad.h>
+
+/* Keypad registers */
+#define DM365_KEYPAD_KEYCTRL 0x0000
+#define DM365_KEYPAD_INTENA 0x0004
+#define DM365_KEYPAD_INTFLAG 0x0008
+#define DM365_KEYPAD_INTCLR 0x000c
+#define DM365_KEYPAD_STRBWIDTH 0x0010
+#define DM365_KEYPAD_INTERVAL 0x0014
+#define DM365_KEYPAD_CONTTIME 0x0018
+#define DM365_KEYPAD_CURRENTST 0x001c
+#define DM365_KEYPAD_PREVSTATE 0x0020
+#define DM365_KEYPAD_EMUCTRL 0x0024
+#define DM365_KEYPAD_IODFTCTRL 0x002c
+
+/* Key Control Register (KEYCTRL) */
+#define DM365_KEYPAD_KEYEN 0x00000001
+#define DM365_KEYPAD_PREVMODE 0x00000002
+#define DM365_KEYPAD_CHATOFF 0x00000004
+#define DM365_KEYPAD_AUTODET 0x00000008
+#define DM365_KEYPAD_SCANMODE 0x00000010
+#define DM365_KEYPAD_OUTTYPE 0x00000020
+#define DM365_KEYPAD_4X4 0x00000040
+
+/* Masks for the interrupts */
+#define DM365_KEYPAD_INT_CONT 0x00000008
+#define DM365_KEYPAD_INT_OFF 0x00000004
+#define DM365_KEYPAD_INT_ON 0x00000002
+#define DM365_KEYPAD_INT_CHANGE 0x00000001
+#define DM365_KEYPAD_INT_ALL 0x0000000f
+
+struct davinci_kp {
+ struct input_dev *input;
+ struct davinci_kp_platform_data *pdata;
+ int irq;
+ void __iomem *base;
+ resource_size_t pbase;
+ size_t base_size;
+};
+
+static void dm365_kp_write(struct davinci_kp *dm365_kp, u32 val, u32 addr)
+{
+ u32 base = (u32)dm365_kp->base;
+
+ __raw_writel(val,(u32 *)(base + addr));
+}
+
+static u32 dm365_kp_read(struct davinci_kp *dm365_kp, u32 addr)
+{
+ u32 base = (u32)dm365_kp->base;
+
+ return __raw_readl((u32 *)(base + addr));
+}
+
+/* Initializing the kp Module */
+static void dm365_kp_initialize(struct davinci_kp *dm365_kp)
+{
+ u32 strobe = dm365_kp->pdata->strobe;
+ u32 interval = dm365_kp->pdata->interval;
+
+ /* Enable all interrupts */
+ dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTENA);
+
+ /* Clear interrupts if any */
+ dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
+
+ /* Setup the scan period = strobe + interval */
+ dm365_kp_write(dm365_kp, strobe, DM365_KEYPAD_STRBWIDTH);
+ dm365_kp_write(dm365_kp, interval, DM365_KEYPAD_INTERVAL);
+ dm365_kp_write(dm365_kp, 0x01, DM365_KEYPAD_CONTTIME);
+
+ /* Enable Keyscan module and enable */
+ dm365_kp_write(dm365_kp, DM365_KEYPAD_AUTODET | DM365_KEYPAD_KEYEN,
+ DM365_KEYPAD_KEYCTRL);
+}
+
+static irqreturn_t dm365_kp_interrupt(int irq, void *dev_id)
+{
+ int i;
+ u32 prev_status, new_status, changed;
+ int keycode = KEY_UNKNOWN;
+ struct davinci_kp *dm365_kp = dev_id;
+ int *keymap = dm365_kp->pdata->keymap;
+ u32 keymapsize = dm365_kp->pdata->keymapsize;
+ struct device *dev = &dm365_kp->input->dev;
+
+ /* Disable interrupt */
+ dm365_kp_write(dm365_kp, 0x0, DM365_KEYPAD_INTENA);
+
+ /* Reading previous and new status of the keypad */
+ prev_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_PREVSTATE);
+ new_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_CURRENTST);
+
+ changed = prev_status ^ new_status;
+
+ for (i = 0; i < keymapsize; i++) {
+ if ((changed >> i) & 0x1) {
+ keycode = keymap[i];
+ if((new_status >> i) & 0x1) {
+ /* Report release */
+ input_report_key(dm365_kp->input, keycode, 0);
+ input_sync(dm365_kp->input);
+ dev_dbg(dev, "dm365_keypad: key %d released\n",
+ keycode);
+ } else {
+ /* Report press */
+ input_report_key(dm365_kp->input, keycode, 1);
+ input_sync(dm365_kp->input);
+ dev_dbg(dev, "dm365_keypad: key %d pressed\n",
+ keycode);
+ }
+ }
+ }
+
+ /* Clearing interrupt */
+ dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
+
+ /* Enable interrupts */
+ dm365_kp_write(dm365_kp, 0x1, DM365_KEYPAD_INTENA);
+
+ return IRQ_HANDLED;
+}
+
+/*
+ * Registers keypad device with input sub system and configures
+ * DM365 keypad registers
+ */
+static int dm365_kp_probe(struct platform_device *pdev)
+{
+ struct davinci_kp *dm365_kp;
+ struct input_dev *key_dev;
+ struct resource *res, *mem;
+ int ret, i;
+ struct device * dev = &pdev->dev;
+ struct davinci_kp_platform_data *pdata = pdev->dev.platform_data;
+
+ dev_info(dev, "DaVinci DM365 Keypad Driver\n");
+
+ if (!pdata->keymap) {
+ dev_dbg(dev, "%s: No keymap from pdata\n", pdev->name);
+ return -EINVAL;
+ }
+
+ dm365_kp = kzalloc(sizeof *dm365_kp, GFP_KERNEL);
+ if(!dm365_kp) {
+ dev_dbg(dev, "%s: Could not allocate memory for private data\n",
+ pdev->name);
+ return -ENOMEM;
+ }
+
+ key_dev = input_allocate_device();
+ if (!key_dev) {
+ dev_dbg(dev, "%s: Could not allocate input device\n",
+ pdev->name);
+ ret = -ENOMEM;
+ goto fail1;
+ }
+
+ platform_set_drvdata(pdev, dm365_kp);
+
+ dm365_kp->input = key_dev;
+
+ dm365_kp->irq = platform_get_irq(pdev, 0);
+ if (dm365_kp->irq <= 0) {
+ dev_err(dev, "%s: No DM365 Keypad irq\n", pdev->name);
+ ret = dm365_kp->irq;
+ goto fail2;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(dev, "%s: No MEM resource\n", pdev->name);
+ ret = -ENODEV;
+ goto fail2;
+ }
+
+ dm365_kp->pbase = res->start;
+ dm365_kp->base_size = resource_size(res);
+
+ mem = request_mem_region(dm365_kp->pbase, dm365_kp->base_size, pdev->name);
+ if (!mem) {
+ dev_err(dev,"%s: KEYSCAN registers at %08x are not free\n",
+ pdev->name, dm365_kp->pbase);
+ ret = -EBUSY;
+ goto fail2;
+ }
+
+ dm365_kp->base = ioremap(dm365_kp->pbase, dm365_kp->base_size);
+ if (!dm365_kp->base) {
+ dev_err(dev,"%s: Can't ioremap MEM resource.\n", pdev->name);
+ ret = -ENOMEM;
+ goto fail3;
+ }
+
+ /* Enable auto repeat feature of Linux input subsystem */
+ if (pdata->rep)
+ __set_bit(EV_REP, key_dev->evbit);
+
+ /* Setup input device */
+ __set_bit(EV_KEY, key_dev->evbit);
+
+ /* Setup the keymap */
+ dm365_kp->pdata = pdata;
+
+ for (i = 0; i < dm365_kp->pdata->keymapsize; i++)
+ __set_bit(dm365_kp->pdata->keymap[i], key_dev->keybit);
+
+ key_dev->name = "dm365_keypad";
+ key_dev->phys = "dm365_keypad/input0";
+ key_dev->dev.parent = &pdev->dev;
+ key_dev->id.bustype = BUS_HOST;
+ key_dev->id.vendor = 0x0001;
+ key_dev->id.product = 0x0365;
+ key_dev->id.version = 0x0001;
+ key_dev->keycode = dm365_kp->pdata->keymap;
+ key_dev->keycodesize = sizeof(unsigned int);
+ key_dev->keycodemax = dm365_kp->pdata->keymapsize;
+
+ ret = input_register_device(dm365_kp->input);
+ if (ret < 0) {
+ dev_err(dev, "%s: Unable to register DaVinci DM365 keypad device\n",
+ pdev->name);
+ goto fail4;
+ }
+
+ ret = request_irq(dm365_kp->irq, dm365_kp_interrupt, IRQF_DISABLED,
+ "dm365_keypad", dm365_kp);
+ if (ret < 0) {
+ dev_err(dev, "%s: Unable to register DaVinci DM365 keypad Interrupt\n",
+ pdev->name);
+ goto fail5;
+ }
+
+ dm365_kp_initialize(dm365_kp);
+
+ return 0;
+fail5:
+ input_unregister_device(dm365_kp->input);
+ key_dev = NULL;
+fail4:
+ iounmap(dm365_kp->base);
+fail3:
+ release_mem_region(dm365_kp->pbase, dm365_kp->base_size);
+fail2:
+ input_free_device(key_dev);
+fail1:
+ kfree(dm365_kp);
+
+ return ret;
+}
+
+static int __exit dm365_kp_remove(struct platform_device *pdev)
+{
+ struct davinci_kp *dm365_kp = platform_get_drvdata(pdev);
+
+ free_irq(dm365_kp->irq, dm365_kp);
+
+ iounmap(dm365_kp->base);
+ release_mem_region(dm365_kp->pbase, dm365_kp->base_size);
+
+ platform_set_drvdata(pdev, NULL);
+
+ input_unregister_device(dm365_kp->input);
+
+ kfree(dm365_kp);
+
+ return 0;
+}
+
+static struct platform_driver dm365_kp_driver = {
+ .driver = {
+ .name = "dm365_keypad",
+ .owner = THIS_MODULE,
+ },
+ .remove = __exit_p(dm365_kp_remove),
+};
+
+static int __init dm365_kp_init(void)
+{
+
+
+ return platform_driver_probe(&dm365_kp_driver, dm365_kp_probe);
+}
+module_init(dm365_kp_init);
+
+static void __exit dm365_kp_exit(void)
+{
+ platform_driver_unregister(&dm365_kp_driver);
+}
+module_exit(dm365_kp_exit);
+
+MODULE_AUTHOR("Miguel Aguilar");
+MODULE_DESCRIPTION("Texas Instruments DaVinci DM365 EVM Keypad Driver");
+MODULE_LICENSE("GPL");
--
1.6.0.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
2009-09-17 20:06 [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support miguel.aguilar
@ 2009-09-18 5:35 ` Nori, Sekhar
2009-09-18 14:32 ` Miguel Aguilar
0 siblings, 1 reply; 10+ messages in thread
From: Nori, Sekhar @ 2009-09-18 5:35 UTC (permalink / raw)
To: miguel.aguilar, Narnakaje, Snehaprabha,
davinci-linux-open-source@linux.davincidsp.com
Cc: santiago.nunez, todd.fischer, clark.becker
On Fri, Sep 18, 2009 at 01:36:36, miguel.aguilar@ridgerun.com wrote:
> From: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>
> Adds the driver for enabling keypad support on DM365 platform.
>
> This driver was tested on DM365 EVM rev c.
>
> Signed-off-by: Miguel Aguilar <miguel.aguilar@ridgerun.com>
> ---
[...]
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index a6b989a..08ed7d4 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -361,4 +361,10 @@ config KEYBOARD_XTKBD
> To compile this driver as a module, choose M here: the
> module will be called xtkbd.
>
> +config KEYBOARD_DAVINCI_DM365
How about calling it just KEYBOARD_DAVINCI in honor of this
being the first DaVinci keypad driver in drivers/input/keyboard?
Also, in the hope that this will get reused on a future DaVinci.
Unless, there is something that makes it really DM365 specific..
> + tristate "TI DaVinci DM365 Keypad"
> + depends on ARCH_DAVINCI_DM365
> + help
> + Supports the keypad module on the DM365
> +
> endif
[...]
> diff --git a/drivers/input/keyboard/davinci_dm365_keypad.c b/drivers/input/keyboard/davinci_dm365_keypad.c
> new file mode 100644
> index 0000000..5db8eed
> --- /dev/null
> +++ b/drivers/input/keyboard/davinci_dm365_keypad.c
> @@ -0,0 +1,331 @@
> +/*
> + * DaVinci DM365 Keypad Driver
> + *
> + * Copyright (C) 2009 Texas Instruments, Inc
> + *
> + * Author: Miguel Aguilar <miguel.aguilar@ridgerun.com>
> + *
> + * Intial Code: Sandeep Paulraj <s-paulraj@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/types.h>
> +#include <linux/input.h>
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/errno.h>
> +
> +#include <asm/irq.h>
> +
> +#include <mach/hardware.h>
> +#include <mach/irqs.h>
> +#include <mach/keypad.h>
> +
> +/* Keypad registers */
> +#define DM365_KEYPAD_KEYCTRL 0x0000
There are many places in the driver (functions, macros) which
probably can just be called DAVINCI_ instead of DM365_
> +#define DM365_KEYPAD_INTENA 0x0004
> +#define DM365_KEYPAD_INTFLAG 0x0008
> +#define DM365_KEYPAD_INTCLR 0x000c
> +#define DM365_KEYPAD_STRBWIDTH 0x0010
> +#define DM365_KEYPAD_INTERVAL 0x0014
> +#define DM365_KEYPAD_CONTTIME 0x0018
> +#define DM365_KEYPAD_CURRENTST 0x001c
> +#define DM365_KEYPAD_PREVSTATE 0x0020
> +#define DM365_KEYPAD_EMUCTRL 0x0024
> +#define DM365_KEYPAD_IODFTCTRL 0x002c
> +
> +/* Key Control Register (KEYCTRL) */
> +#define DM365_KEYPAD_KEYEN 0x00000001
> +#define DM365_KEYPAD_PREVMODE 0x00000002
> +#define DM365_KEYPAD_CHATOFF 0x00000004
> +#define DM365_KEYPAD_AUTODET 0x00000008
> +#define DM365_KEYPAD_SCANMODE 0x00000010
> +#define DM365_KEYPAD_OUTTYPE 0x00000020
> +#define DM365_KEYPAD_4X4 0x00000040
> +
> +/* Masks for the interrupts */
> +#define DM365_KEYPAD_INT_CONT 0x00000008
> +#define DM365_KEYPAD_INT_OFF 0x00000004
> +#define DM365_KEYPAD_INT_ON 0x00000002
> +#define DM365_KEYPAD_INT_CHANGE 0x00000001
> +#define DM365_KEYPAD_INT_ALL 0x0000000f
> +
> +struct davinci_kp {
> + struct input_dev *input;
> + struct davinci_kp_platform_data *pdata;
> + int irq;
> + void __iomem *base;
> + resource_size_t pbase;
> + size_t base_size;
> +};
> +
> +static void dm365_kp_write(struct davinci_kp *dm365_kp, u32 val, u32 addr)
> +{
> + u32 base = (u32)dm365_kp->base;
> +
> + __raw_writel(val,(u32 *)(base + addr));
> +}
> +
> +static u32 dm365_kp_read(struct davinci_kp *dm365_kp, u32 addr)
> +{
> + u32 base = (u32)dm365_kp->base;
> +
> + return __raw_readl((u32 *)(base + addr));
> +}
> +
> +/* Initializing the kp Module */
> +static void dm365_kp_initialize(struct davinci_kp *dm365_kp)
> +{
> + u32 strobe = dm365_kp->pdata->strobe;
> + u32 interval = dm365_kp->pdata->interval;
> +
> + /* Enable all interrupts */
> + dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTENA);
> +
> + /* Clear interrupts if any */
> + dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
> +
> + /* Setup the scan period = strobe + interval */
> + dm365_kp_write(dm365_kp, strobe, DM365_KEYPAD_STRBWIDTH);
> + dm365_kp_write(dm365_kp, interval, DM365_KEYPAD_INTERVAL);
> + dm365_kp_write(dm365_kp, 0x01, DM365_KEYPAD_CONTTIME);
> +
> + /* Enable Keyscan module and enable */
> + dm365_kp_write(dm365_kp, DM365_KEYPAD_AUTODET | DM365_KEYPAD_KEYEN,
> + DM365_KEYPAD_KEYCTRL);
> +}
> +
> +static irqreturn_t dm365_kp_interrupt(int irq, void *dev_id)
> +{
> + int i;
> + u32 prev_status, new_status, changed;
> + int keycode = KEY_UNKNOWN;
> + struct davinci_kp *dm365_kp = dev_id;
> + int *keymap = dm365_kp->pdata->keymap;
> + u32 keymapsize = dm365_kp->pdata->keymapsize;
> + struct device *dev = &dm365_kp->input->dev;
> +
> + /* Disable interrupt */
> + dm365_kp_write(dm365_kp, 0x0, DM365_KEYPAD_INTENA);
> +
> + /* Reading previous and new status of the keypad */
> + prev_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_PREVSTATE);
> + new_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_CURRENTST);
> +
> + changed = prev_status ^ new_status;
> +
> + for (i = 0; i < keymapsize; i++) {
> + if ((changed >> i) & 0x1) {
Using ffs(changed) will probably lead to a faster ISR, especially
because only one key would have changed in most cases.
> + keycode = keymap[i];
> + if((new_status >> i) & 0x1) {
> + /* Report release */
> + input_report_key(dm365_kp->input, keycode, 0);
> + input_sync(dm365_kp->input);
> + dev_dbg(dev, "dm365_keypad: key %d released\n",
> + keycode);
> + } else {
> + /* Report press */
> + input_report_key(dm365_kp->input, keycode, 1);
> + input_sync(dm365_kp->input);
> + dev_dbg(dev, "dm365_keypad: key %d pressed\n",
> + keycode);
> + }
> + }
> + }
> +
> + /* Clearing interrupt */
> + dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
> +
> + /* Enable interrupts */
> + dm365_kp_write(dm365_kp, 0x1, DM365_KEYPAD_INTENA);
> +
> + return IRQ_HANDLED;
> +}
> +
> +/*
> + * Registers keypad device with input sub system and configures
> + * DM365 keypad registers
> + */
> +static int dm365_kp_probe(struct platform_device *pdev)
This should be __init function.
> +{
> + struct davinci_kp *dm365_kp;
> + struct input_dev *key_dev;
> + struct resource *res, *mem;
> + int ret, i;
> + struct device * dev = &pdev->dev;
> + struct davinci_kp_platform_data *pdata = pdev->dev.platform_data;
> +
> + dev_info(dev, "DaVinci DM365 Keypad Driver\n");
> +
> + if (!pdata->keymap) {
> + dev_dbg(dev, "%s: No keymap from pdata\n", pdev->name);
> + return -EINVAL;
> + }
> +
> + dm365_kp = kzalloc(sizeof *dm365_kp, GFP_KERNEL);
> + if(!dm365_kp) {
> + dev_dbg(dev, "%s: Could not allocate memory for private data\n",
> + pdev->name);
> + return -ENOMEM;
> + }
> +
> + key_dev = input_allocate_device();
> + if (!key_dev) {
> + dev_dbg(dev, "%s: Could not allocate input device\n",
> + pdev->name);
> + ret = -ENOMEM;
> + goto fail1;
> + }
> +
> + platform_set_drvdata(pdev, dm365_kp);
> +
> + dm365_kp->input = key_dev;
> +
> + dm365_kp->irq = platform_get_irq(pdev, 0);
> + if (dm365_kp->irq <= 0) {
> + dev_err(dev, "%s: No DM365 Keypad irq\n", pdev->name);
> + ret = dm365_kp->irq;
Probe will return success when dm365_kp->irq is 0, but you actually
want a failure.
> + goto fail2;
> + }
> +
[...]
> + key_dev->name = "dm365_keypad";
> + key_dev->phys = "dm365_keypad/input0";
...
> + key_dev->id.vendor = 0x0001;
> + key_dev->id.product = 0x0365;
These initializations seem to make the driver's use on future SoCs
a little uncomfortable..
> + key_dev->id.version = 0x0001;
> + key_dev->keycode = dm365_kp->pdata->keymap;
> + key_dev->keycodesize = sizeof(unsigned int);
> + key_dev->keycodemax = dm365_kp->pdata->keymapsize;
> +
> + ret = input_register_device(dm365_kp->input);
> + if (ret < 0) {
> + dev_err(dev, "%s: Unable to register DaVinci DM365 keypad device\n",
Can avoid hard coding "DM365" in error messages too...
> + pdev->name);
> + goto fail4;
> + }
> +
> + ret = request_irq(dm365_kp->irq, dm365_kp_interrupt, IRQF_DISABLED,
> + "dm365_keypad", dm365_kp);
> + if (ret < 0) {
> + dev_err(dev, "%s: Unable to register DaVinci DM365 keypad Interrupt\n",
> + pdev->name);
> + goto fail5;
> + }
> +
> + dm365_kp_initialize(dm365_kp);
> +
> + return 0;
> +fail5:
> + input_unregister_device(dm365_kp->input);
> + key_dev = NULL;
> +fail4:
> + iounmap(dm365_kp->base);
> +fail3:
> + release_mem_region(dm365_kp->pbase, dm365_kp->base_size);
> +fail2:
> + input_free_device(key_dev);
> +fail1:
> + kfree(dm365_kp);
> +
> + return ret;
> +}
> +
> +static int __exit dm365_kp_remove(struct platform_device *pdev)
> +{
> + struct davinci_kp *dm365_kp = platform_get_drvdata(pdev);
> +
> + free_irq(dm365_kp->irq, dm365_kp);
> +
> + iounmap(dm365_kp->base);
> + release_mem_region(dm365_kp->pbase, dm365_kp->base_size);
> +
> + platform_set_drvdata(pdev, NULL);
> +
> + input_unregister_device(dm365_kp->input);
It would be nice to see the remove happen in reverse order of probe,
ie. unregister device first and unmap memory later.
> +
> + kfree(dm365_kp);
> +
> + return 0;
> +}
> +
> +static struct platform_driver dm365_kp_driver = {
> + .driver = {
> + .name = "dm365_keypad",
> + .owner = THIS_MODULE,
> + },
> + .remove = __exit_p(dm365_kp_remove),
> +};
> +
> +static int __init dm365_kp_init(void)
> +{
> +
> +
Extra lines here.
> + return platform_driver_probe(&dm365_kp_driver, dm365_kp_probe);
> +}
> +module_init(dm365_kp_init);
> +
> +static void __exit dm365_kp_exit(void)
> +{
> + platform_driver_unregister(&dm365_kp_driver);
> +}
> +module_exit(dm365_kp_exit);
> +
> +MODULE_AUTHOR("Miguel Aguilar");
> +MODULE_DESCRIPTION("Texas Instruments DaVinci DM365 EVM Keypad Driver");
The driver is definitely not specific to the EVM.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
2009-09-18 5:35 ` Nori, Sekhar
@ 2009-09-18 14:32 ` Miguel Aguilar
2009-09-22 7:05 ` Nori, Sekhar
0 siblings, 1 reply; 10+ messages in thread
From: Miguel Aguilar @ 2009-09-18 14:32 UTC (permalink / raw)
To: Nori, Sekhar
Cc: Narnakaje, Snehaprabha, davinci-linux-open-source, linux-input,
santiago.nunez, todd.fischer, clark.becker
Sekhar,
See the comments below.
Nori, Sekhar wrote:
> On Fri, Sep 18, 2009 at 01:36:36, miguel.aguilar@ridgerun.com wrote:
>> From: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>>
>> Adds the driver for enabling keypad support on DM365 platform.
>>
>> This driver was tested on DM365 EVM rev c.
>>
>> Signed-off-by: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>> ---
>
> [...]
>
>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>> index a6b989a..08ed7d4 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -361,4 +361,10 @@ config KEYBOARD_XTKBD
>> To compile this driver as a module, choose M here: the
>> module will be called xtkbd.
>>
>> +config KEYBOARD_DAVINCI_DM365
>
> How about calling it just KEYBOARD_DAVINCI in honor of this
> being the first DaVinci keypad driver in drivers/input/keyboard?
> Also, in the hope that this will get reused on a future DaVinci.
>
> Unless, there is something that makes it really DM365 specific..
[MA] It is better keep it specific since that symbols refers to a driver which
is DM365 specific at the moment.
>
>> + tristate "TI DaVinci DM365 Keypad"
>> + depends on ARCH_DAVINCI_DM365
>> + help
>> + Supports the keypad module on the DM365
>> +
>> endif
>
> [...]
>
>> diff --git a/drivers/input/keyboard/davinci_dm365_keypad.c b/drivers/input/keyboard/davinci_dm365_keypad.c
>> new file mode 100644
>> index 0000000..5db8eed
>> --- /dev/null
>> +++ b/drivers/input/keyboard/davinci_dm365_keypad.c
>> @@ -0,0 +1,331 @@
>> +/*
>> + * DaVinci DM365 Keypad Driver
>> + *
>> + * Copyright (C) 2009 Texas Instruments, Inc
>> + *
>> + * Author: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>> + *
>> + * Intial Code: Sandeep Paulraj <s-paulraj@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> + */
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/types.h>
>> +#include <linux/input.h>
>> +#include <linux/kernel.h>
>> +#include <linux/delay.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/errno.h>
>> +
>> +#include <asm/irq.h>
>> +
>> +#include <mach/hardware.h>
>> +#include <mach/irqs.h>
>> +#include <mach/keypad.h>
>> +
>> +/* Keypad registers */
>> +#define DM365_KEYPAD_KEYCTRL 0x0000
>
> There are many places in the driver (functions, macros) which
> probably can just be called DAVINCI_ instead of DM365_
[MA]Again, these are DM365 specific definitions, If your proposal is to make
this driver generic to Davinci instead of DM365 specific, I should get rid of
all dm365 labels, messages, function names, etc in this driver, even the
driver's name should be davinci_keypad instead of dm365_keypad, so in that sense
not only the macros and the config symbol should be changed; but for doing that
we are assuming that future chips of the Davinci family will include the same
keypad module, so they would be compatible among them to use the same driver.
>
>> +#define DM365_KEYPAD_INTENA 0x0004
>> +#define DM365_KEYPAD_INTFLAG 0x0008
>> +#define DM365_KEYPAD_INTCLR 0x000c
>> +#define DM365_KEYPAD_STRBWIDTH 0x0010
>> +#define DM365_KEYPAD_INTERVAL 0x0014
>> +#define DM365_KEYPAD_CONTTIME 0x0018
>> +#define DM365_KEYPAD_CURRENTST 0x001c
>> +#define DM365_KEYPAD_PREVSTATE 0x0020
>> +#define DM365_KEYPAD_EMUCTRL 0x0024
>> +#define DM365_KEYPAD_IODFTCTRL 0x002c
>> +
>> +/* Key Control Register (KEYCTRL) */
>> +#define DM365_KEYPAD_KEYEN 0x00000001
>> +#define DM365_KEYPAD_PREVMODE 0x00000002
>> +#define DM365_KEYPAD_CHATOFF 0x00000004
>> +#define DM365_KEYPAD_AUTODET 0x00000008
>> +#define DM365_KEYPAD_SCANMODE 0x00000010
>> +#define DM365_KEYPAD_OUTTYPE 0x00000020
>> +#define DM365_KEYPAD_4X4 0x00000040
>> +
>> +/* Masks for the interrupts */
>> +#define DM365_KEYPAD_INT_CONT 0x00000008
>> +#define DM365_KEYPAD_INT_OFF 0x00000004
>> +#define DM365_KEYPAD_INT_ON 0x00000002
>> +#define DM365_KEYPAD_INT_CHANGE 0x00000001
>> +#define DM365_KEYPAD_INT_ALL 0x0000000f
>> +
>> +struct davinci_kp {
>> + struct input_dev *input;
>> + struct davinci_kp_platform_data *pdata;
>> + int irq;
>> + void __iomem *base;
>> + resource_size_t pbase;
>> + size_t base_size;
>> +};
>> +
>> +static void dm365_kp_write(struct davinci_kp *dm365_kp, u32 val, u32 addr)
>> +{
>> + u32 base = (u32)dm365_kp->base;
>> +
>> + __raw_writel(val,(u32 *)(base + addr));
>> +}
>> +
>> +static u32 dm365_kp_read(struct davinci_kp *dm365_kp, u32 addr)
>> +{
>> + u32 base = (u32)dm365_kp->base;
>> +
>> + return __raw_readl((u32 *)(base + addr));
>> +}
>> +
>> +/* Initializing the kp Module */
>> +static void dm365_kp_initialize(struct davinci_kp *dm365_kp)
>> +{
>> + u32 strobe = dm365_kp->pdata->strobe;
>> + u32 interval = dm365_kp->pdata->interval;
>> +
>> + /* Enable all interrupts */
>> + dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTENA);
>> +
>> + /* Clear interrupts if any */
>> + dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
>> +
>> + /* Setup the scan period = strobe + interval */
>> + dm365_kp_write(dm365_kp, strobe, DM365_KEYPAD_STRBWIDTH);
>> + dm365_kp_write(dm365_kp, interval, DM365_KEYPAD_INTERVAL);
>> + dm365_kp_write(dm365_kp, 0x01, DM365_KEYPAD_CONTTIME);
>> +
>> + /* Enable Keyscan module and enable */
>> + dm365_kp_write(dm365_kp, DM365_KEYPAD_AUTODET | DM365_KEYPAD_KEYEN,
>> + DM365_KEYPAD_KEYCTRL);
>> +}
>> +
>> +static irqreturn_t dm365_kp_interrupt(int irq, void *dev_id)
>> +{
>> + int i;
>> + u32 prev_status, new_status, changed;
>> + int keycode = KEY_UNKNOWN;
>> + struct davinci_kp *dm365_kp = dev_id;
>> + int *keymap = dm365_kp->pdata->keymap;
>> + u32 keymapsize = dm365_kp->pdata->keymapsize;
>> + struct device *dev = &dm365_kp->input->dev;
>> +
>> + /* Disable interrupt */
>> + dm365_kp_write(dm365_kp, 0x0, DM365_KEYPAD_INTENA);
>> +
>> + /* Reading previous and new status of the keypad */
>> + prev_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_PREVSTATE);
>> + new_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_CURRENTST);
>> +
>> + changed = prev_status ^ new_status;
>> +
>> + for (i = 0; i < keymapsize; i++) {
>> + if ((changed >> i) & 0x1) {
>
> Using ffs(changed) will probably lead to a faster ISR, especially
> because only one key would have changed in most cases.
[MA] OK, I will try ffs.
>
>> + keycode = keymap[i];
>> + if((new_status >> i) & 0x1) {
>> + /* Report release */
>> + input_report_key(dm365_kp->input, keycode, 0);
>> + input_sync(dm365_kp->input);
>> + dev_dbg(dev, "dm365_keypad: key %d released\n",
>> + keycode);
>> + } else {
>> + /* Report press */
>> + input_report_key(dm365_kp->input, keycode, 1);
>> + input_sync(dm365_kp->input);
>> + dev_dbg(dev, "dm365_keypad: key %d pressed\n",
>> + keycode);
>> + }
>> + }
>> + }
>> +
>> + /* Clearing interrupt */
>> + dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
>> +
>> + /* Enable interrupts */
>> + dm365_kp_write(dm365_kp, 0x1, DM365_KEYPAD_INTENA);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +/*
>> + * Registers keypad device with input sub system and configures
>> + * DM365 keypad registers
>> + */
>> +static int dm365_kp_probe(struct platform_device *pdev)
>
> This should be __init function.
[MA] OK
>
>> +{
>> + struct davinci_kp *dm365_kp;
>> + struct input_dev *key_dev;
>> + struct resource *res, *mem;
>> + int ret, i;
>> + struct device * dev = &pdev->dev;
>> + struct davinci_kp_platform_data *pdata = pdev->dev.platform_data;
>> +
>> + dev_info(dev, "DaVinci DM365 Keypad Driver\n");
>> +
>> + if (!pdata->keymap) {
>> + dev_dbg(dev, "%s: No keymap from pdata\n", pdev->name);
>> + return -EINVAL;
>> + }
>> +
>> + dm365_kp = kzalloc(sizeof *dm365_kp, GFP_KERNEL);
>> + if(!dm365_kp) {
>> + dev_dbg(dev, "%s: Could not allocate memory for private data\n",
>> + pdev->name);
>> + return -ENOMEM;
>> + }
>> +
>> + key_dev = input_allocate_device();
>> + if (!key_dev) {
>> + dev_dbg(dev, "%s: Could not allocate input device\n",
>> + pdev->name);
>> + ret = -ENOMEM;
>> + goto fail1;
>> + }
>> +
>> + platform_set_drvdata(pdev, dm365_kp);
>> +
>> + dm365_kp->input = key_dev;
>> +
>> + dm365_kp->irq = platform_get_irq(pdev, 0);
>> + if (dm365_kp->irq <= 0) {
>> + dev_err(dev, "%s: No DM365 Keypad irq\n", pdev->name);
>> + ret = dm365_kp->irq;
>
> Probe will return success when dm365_kp->irq is 0, but you actually
> want a failure.
[MA] What is the proper error code for this case?.
>
>> + goto fail2;
>> + }
>> +
>
> [...]
>
>> + key_dev->name = "dm365_keypad";
>> + key_dev->phys = "dm365_keypad/input0";
> ...
>> + key_dev->id.vendor = 0x0001;
>> + key_dev->id.product = 0x0365;
>
> These initializations seem to make the driver's use on future SoCs
> a little uncomfortable..
>
>> + key_dev->id.version = 0x0001;
>> + key_dev->keycode = dm365_kp->pdata->keymap;
>> + key_dev->keycodesize = sizeof(unsigned int);
>> + key_dev->keycodemax = dm365_kp->pdata->keymapsize;
>> +
>> + ret = input_register_device(dm365_kp->input);
>> + if (ret < 0) {
>> + dev_err(dev, "%s: Unable to register DaVinci DM365 keypad device\n",
>
> Can avoid hard coding "DM365" in error messages too...
[MA] Same issue about DM365 specific driver.
>
>> + pdev->name);
>> + goto fail4;
>> + }
>> +
>> + ret = request_irq(dm365_kp->irq, dm365_kp_interrupt, IRQF_DISABLED,
>> + "dm365_keypad", dm365_kp);
>> + if (ret < 0) {
>> + dev_err(dev, "%s: Unable to register DaVinci DM365 keypad Interrupt\n",
>> + pdev->name);
>> + goto fail5;
>> + }
>> +
>> + dm365_kp_initialize(dm365_kp);
>> +
>> + return 0;
>> +fail5:
>> + input_unregister_device(dm365_kp->input);
>> + key_dev = NULL;
>> +fail4:
>> + iounmap(dm365_kp->base);
>> +fail3:
>> + release_mem_region(dm365_kp->pbase, dm365_kp->base_size);
>> +fail2:
>> + input_free_device(key_dev);
>> +fail1:
>> + kfree(dm365_kp);
>> +
>> + return ret;
>> +}
>> +
>> +static int __exit dm365_kp_remove(struct platform_device *pdev)
>> +{
>> + struct davinci_kp *dm365_kp = platform_get_drvdata(pdev);
>> +
>> + free_irq(dm365_kp->irq, dm365_kp);
>> +
>> + iounmap(dm365_kp->base);
>> + release_mem_region(dm365_kp->pbase, dm365_kp->base_size);
>> +
>> + platform_set_drvdata(pdev, NULL);
>> +
>> + input_unregister_device(dm365_kp->input);
>
> It would be nice to see the remove happen in reverse order of probe,
> ie. unregister device first and unmap memory later.
[MA] Yes, it was supposed to be in the reverse order however it seem that
unregister device is not after free irq and before iounmap.
>
>> +
>> + kfree(dm365_kp);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver dm365_kp_driver = {
>> + .driver = {
>> + .name = "dm365_keypad",
>> + .owner = THIS_MODULE,
>> + },
>> + .remove = __exit_p(dm365_kp_remove),
>> +};
>> +
>> +static int __init dm365_kp_init(void)
>> +{
>> +
>> +
>
> Extra lines here.
[MA] Typo!
>
>> + return platform_driver_probe(&dm365_kp_driver, dm365_kp_probe);
>> +}
>> +module_init(dm365_kp_init);
>> +
>> +static void __exit dm365_kp_exit(void)
>> +{
>> + platform_driver_unregister(&dm365_kp_driver);
>> +}
>> +module_exit(dm365_kp_exit);
>> +
>> +MODULE_AUTHOR("Miguel Aguilar");
>> +MODULE_DESCRIPTION("Texas Instruments DaVinci DM365 EVM Keypad Driver");
>
> The driver is definitely not specific to the EVM.
[MA] Typo!
>
> Thanks,
> Sekhar
Thanks,
Miguel Aguilar
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
2009-09-18 14:32 ` Miguel Aguilar
@ 2009-09-22 7:05 ` Nori, Sekhar
2009-09-22 14:01 ` Miguel Aguilar
0 siblings, 1 reply; 10+ messages in thread
From: Nori, Sekhar @ 2009-09-22 7:05 UTC (permalink / raw)
To: Miguel Aguilar
Cc: Narnakaje, Snehaprabha, davinci-linux-open-source, linux-input,
santiago.nunez, todd.fischer, clark.becker
On Fri, Sep 18, 2009 at 20:02:32, Miguel Aguilar wrote:
> Sekhar,
>
> See the comments below.
>
> Nori, Sekhar wrote:
> > On Fri, Sep 18, 2009 at 01:36:36, miguel.aguilar@ridgerun.com wrote:
[...]
> >> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> >> index a6b989a..08ed7d4 100644
> >> --- a/drivers/input/keyboard/Kconfig
> >> +++ b/drivers/input/keyboard/Kconfig
> >> @@ -361,4 +361,10 @@ config KEYBOARD_XTKBD
> >> To compile this driver as a module, choose M here: the
> >> module will be called xtkbd.
> >>
> >> +config KEYBOARD_DAVINCI_DM365
> >
> > How about calling it just KEYBOARD_DAVINCI in honor of this
> > being the first DaVinci keypad driver in drivers/input/keyboard?
> > Also, in the hope that this will get reused on a future DaVinci.
> >
> > Unless, there is something that makes it really DM365 specific..
> [MA] It is better keep it specific since that symbols refers to a driver which
> is DM365 specific at the moment.
Okay. There is no device which reuses the keyscan module at
present, but there has been a lot of IP reuse in the DaVinci
family before (emac, mmc/sd, i2c, spi etc.).
> >
> >> + tristate "TI DaVinci DM365 Keypad"
> >> + depends on ARCH_DAVINCI_DM365
> >> + help
> >> + Supports the keypad module on the DM365
> >> +
> >> endif
> >
> > [...]
> >
> >> diff --git a/drivers/input/keyboard/davinci_dm365_keypad.c b/drivers/input/keyboard/davinci_dm365_keypad.c
> >> new file mode 100644
> >> index 0000000..5db8eed
> >> --- /dev/null
> >> +++ b/drivers/input/keyboard/davinci_dm365_keypad.c
> >> @@ -0,0 +1,331 @@
> >> +/*
> >> + * DaVinci DM365 Keypad Driver
> >> + *
> >> + * Copyright (C) 2009 Texas Instruments, Inc
> >> + *
> >> + * Author: Miguel Aguilar <miguel.aguilar@ridgerun.com>
> >> + *
> >> + * Intial Code: Sandeep Paulraj <s-paulraj@ti.com>
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published by
> >> + * the Free Software Foundation; either version 2 of the License, or
> >> + * (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> >> + */
> >> +#include <linux/module.h>
> >> +#include <linux/init.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/types.h>
> >> +#include <linux/input.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/delay.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/errno.h>
> >> +
> >> +#include <asm/irq.h>
> >> +
> >> +#include <mach/hardware.h>
> >> +#include <mach/irqs.h>
> >> +#include <mach/keypad.h>
> >> +
> >> +/* Keypad registers */
> >> +#define DM365_KEYPAD_KEYCTRL 0x0000
> >
> > There are many places in the driver (functions, macros) which
> > probably can just be called DAVINCI_ instead of DM365_
> [MA]Again, these are DM365 specific definitions, If your proposal is to make
> this driver generic to Davinci instead of DM365 specific, I should get rid of
> all dm365 labels, messages, function names, etc in this driver, even the
> driver's name should be davinci_keypad instead of dm365_keypad, so in that sense
> not only the macros and the config symbol should be changed; but for doing that
> we are assuming that future chips of the Davinci family will include the same
> keypad module, so they would be compatible among them to use the same driver.
Yes, typically attempt is made to reuse the IP as-is.
Slight changes in the IPs re-used have previously been
taken care use a "version" field coming from the platform
data.
The product number population below seems to be a bigger
sore than the variable/macro/Kconfig naming.
> >> +static irqreturn_t dm365_kp_interrupt(int irq, void *dev_id)
> >> +{
> >> + int i;
> >> + u32 prev_status, new_status, changed;
> >> + int keycode = KEY_UNKNOWN;
> >> + struct davinci_kp *dm365_kp = dev_id;
> >> + int *keymap = dm365_kp->pdata->keymap;
> >> + u32 keymapsize = dm365_kp->pdata->keymapsize;
> >> + struct device *dev = &dm365_kp->input->dev;
> >> +
> >> + /* Disable interrupt */
> >> + dm365_kp_write(dm365_kp, 0x0, DM365_KEYPAD_INTENA);
> >> +
> >> + /* Reading previous and new status of the keypad */
> >> + prev_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_PREVSTATE);
> >> + new_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_CURRENTST);
> >> +
> >> + changed = prev_status ^ new_status;
> >> +
> >> + for (i = 0; i < keymapsize; i++) {
> >> + if ((changed >> i) & 0x1) {
> >
> > Using ffs(changed) will probably lead to a faster ISR, especially
> > because only one key would have changed in most cases.
> [MA] OK, I will try ffs.
> >
> >> + keycode = keymap[i];
> >> + if((new_status >> i) & 0x1) {
> >> + /* Report release */
> >> + input_report_key(dm365_kp->input, keycode, 0);
> >> + input_sync(dm365_kp->input);
> >> + dev_dbg(dev, "dm365_keypad: key %d released\n",
> >> + keycode);
> >> + } else {
> >> + /* Report press */
> >> + input_report_key(dm365_kp->input, keycode, 1);
> >> + input_sync(dm365_kp->input);
> >> + dev_dbg(dev, "dm365_keypad: key %d pressed\n",
> >> + keycode);
> >> + }
> >> + }
> >> + }
> >> +
> >> + /* Clearing interrupt */
> >> + dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
> >> +
> >> + /* Enable interrupts */
> >> + dm365_kp_write(dm365_kp, 0x1, DM365_KEYPAD_INTENA);
> >> +
> >> + return IRQ_HANDLED;
I missed this before, if the loop above turns up nothing,
then the IRQ was not handled. IRQ_NONE will be a better return
in that case.
[...]
> >> + dm365_kp->irq = platform_get_irq(pdev, 0);
> >> + if (dm365_kp->irq <= 0) {
> >> + dev_err(dev, "%s: No DM365 Keypad irq\n", pdev->name);
> >> + ret = dm365_kp->irq;
> >
> > Probe will return success when dm365_kp->irq is 0, but you actually
> > want a failure.
> [MA] What is the proper error code for this case?.
platform_get_irq() returns an error which should be propagated. The
question is: Why is 0 not a valid IRQ number for keyscan module?
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
2009-09-22 7:05 ` Nori, Sekhar
@ 2009-09-22 14:01 ` Miguel Aguilar
2009-09-22 17:37 ` Kevin Hilman
0 siblings, 1 reply; 10+ messages in thread
From: Miguel Aguilar @ 2009-09-22 14:01 UTC (permalink / raw)
To: Nori, Sekhar
Cc: Narnakaje, Snehaprabha, davinci-linux-open-source, linux-input,
santiago.nunez, todd.fischer, clark.becker
Sekhar,
If you and Sneha agree I will write driver in terms of davinci instead of dm365.
Thanks,
Miguel Aguilar
Nori, Sekhar wrote:
> On Fri, Sep 18, 2009 at 20:02:32, Miguel Aguilar wrote:
>> Sekhar,
>>
>> See the comments below.
>>
>> Nori, Sekhar wrote:
>>> On Fri, Sep 18, 2009 at 01:36:36, miguel.aguilar@ridgerun.com wrote:
>
> [...]
>
>>>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>>>> index a6b989a..08ed7d4 100644
>>>> --- a/drivers/input/keyboard/Kconfig
>>>> +++ b/drivers/input/keyboard/Kconfig
>>>> @@ -361,4 +361,10 @@ config KEYBOARD_XTKBD
>>>> To compile this driver as a module, choose M here: the
>>>> module will be called xtkbd.
>>>>
>>>> +config KEYBOARD_DAVINCI_DM365
>>> How about calling it just KEYBOARD_DAVINCI in honor of this
>>> being the first DaVinci keypad driver in drivers/input/keyboard?
>>> Also, in the hope that this will get reused on a future DaVinci.
>>>
>>> Unless, there is something that makes it really DM365 specific..
>> [MA] It is better keep it specific since that symbols refers to a driver which
>> is DM365 specific at the moment.
>
> Okay. There is no device which reuses the keyscan module at
> present, but there has been a lot of IP reuse in the DaVinci
> family before (emac, mmc/sd, i2c, spi etc.).
>
>>>> + tristate "TI DaVinci DM365 Keypad"
>>>> + depends on ARCH_DAVINCI_DM365
>>>> + help
>>>> + Supports the keypad module on the DM365
>>>> +
>>>> endif
>>> [...]
>>>
>>>> diff --git a/drivers/input/keyboard/davinci_dm365_keypad.c b/drivers/input/keyboard/davinci_dm365_keypad.c
>>>> new file mode 100644
>>>> index 0000000..5db8eed
>>>> --- /dev/null
>>>> +++ b/drivers/input/keyboard/davinci_dm365_keypad.c
>>>> @@ -0,0 +1,331 @@
>>>> +/*
>>>> + * DaVinci DM365 Keypad Driver
>>>> + *
>>>> + * Copyright (C) 2009 Texas Instruments, Inc
>>>> + *
>>>> + * Author: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>>>> + *
>>>> + * Intial Code: Sandeep Paulraj <s-paulraj@ti.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License as published by
>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>> + * (at your option) any later version.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU General Public License
>>>> + * along with this program; if not, write to the Free Software
>>>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>>>> + */
>>>> +#include <linux/module.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/interrupt.h>
>>>> +#include <linux/types.h>
>>>> +#include <linux/input.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/delay.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/errno.h>
>>>> +
>>>> +#include <asm/irq.h>
>>>> +
>>>> +#include <mach/hardware.h>
>>>> +#include <mach/irqs.h>
>>>> +#include <mach/keypad.h>
>>>> +
>>>> +/* Keypad registers */
>>>> +#define DM365_KEYPAD_KEYCTRL 0x0000
>>> There are many places in the driver (functions, macros) which
>>> probably can just be called DAVINCI_ instead of DM365_
>> [MA]Again, these are DM365 specific definitions, If your proposal is to make
>> this driver generic to Davinci instead of DM365 specific, I should get rid of
>> all dm365 labels, messages, function names, etc in this driver, even the
>> driver's name should be davinci_keypad instead of dm365_keypad, so in that sense
>> not only the macros and the config symbol should be changed; but for doing that
>> we are assuming that future chips of the Davinci family will include the same
>> keypad module, so they would be compatible among them to use the same driver.
>
> Yes, typically attempt is made to reuse the IP as-is.
> Slight changes in the IPs re-used have previously been
> taken care use a "version" field coming from the platform
> data.
>
> The product number population below seems to be a bigger
> sore than the variable/macro/Kconfig naming.
>
>>>> +static irqreturn_t dm365_kp_interrupt(int irq, void *dev_id)
>>>> +{
>>>> + int i;
>>>> + u32 prev_status, new_status, changed;
>>>> + int keycode = KEY_UNKNOWN;
>>>> + struct davinci_kp *dm365_kp = dev_id;
>>>> + int *keymap = dm365_kp->pdata->keymap;
>>>> + u32 keymapsize = dm365_kp->pdata->keymapsize;
>>>> + struct device *dev = &dm365_kp->input->dev;
>>>> +
>>>> + /* Disable interrupt */
>>>> + dm365_kp_write(dm365_kp, 0x0, DM365_KEYPAD_INTENA);
>>>> +
>>>> + /* Reading previous and new status of the keypad */
>>>> + prev_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_PREVSTATE);
>>>> + new_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_CURRENTST);
>>>> +
>>>> + changed = prev_status ^ new_status;
>>>> +
>>>> + for (i = 0; i < keymapsize; i++) {
>>>> + if ((changed >> i) & 0x1) {
>>> Using ffs(changed) will probably lead to a faster ISR, especially
>>> because only one key would have changed in most cases.
>> [MA] OK, I will try ffs.
>>>> + keycode = keymap[i];
>>>> + if((new_status >> i) & 0x1) {
>>>> + /* Report release */
>>>> + input_report_key(dm365_kp->input, keycode, 0);
>>>> + input_sync(dm365_kp->input);
>>>> + dev_dbg(dev, "dm365_keypad: key %d released\n",
>>>> + keycode);
>>>> + } else {
>>>> + /* Report press */
>>>> + input_report_key(dm365_kp->input, keycode, 1);
>>>> + input_sync(dm365_kp->input);
>>>> + dev_dbg(dev, "dm365_keypad: key %d pressed\n",
>>>> + keycode);
>>>> + }
>>>> + }
>>>> + }
>>>> +
>>>> + /* Clearing interrupt */
>>>> + dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
>>>> +
>>>> + /* Enable interrupts */
>>>> + dm365_kp_write(dm365_kp, 0x1, DM365_KEYPAD_INTENA);
>>>> +
>>>> + return IRQ_HANDLED;
>
> I missed this before, if the loop above turns up nothing,
> then the IRQ was not handled. IRQ_NONE will be a better return
> in that case.
>
> [...]
>
>>>> + dm365_kp->irq = platform_get_irq(pdev, 0);
>>>> + if (dm365_kp->irq <= 0) {
>>>> + dev_err(dev, "%s: No DM365 Keypad irq\n", pdev->name);
>>>> + ret = dm365_kp->irq;
>>> Probe will return success when dm365_kp->irq is 0, but you actually
>>> want a failure.
>> [MA] What is the proper error code for this case?.
>
> platform_get_irq() returns an error which should be propagated. The
> question is: Why is 0 not a valid IRQ number for keyscan module?
>
> Thanks,
> Sekhar
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
2009-09-22 14:01 ` Miguel Aguilar
@ 2009-09-22 17:37 ` Kevin Hilman
2009-09-23 3:46 ` Dmitry Torokhov
0 siblings, 1 reply; 10+ messages in thread
From: Kevin Hilman @ 2009-09-22 17:37 UTC (permalink / raw)
To: Miguel Aguilar
Cc: Nori, Sekhar, davinci-linux-open-source, clark.becker,
linux-input, santiago.nunez, todd.fischer
Miguel Aguilar <miguel.aguilar@ridgerun.com> writes:
> Sekhar,
>
> If you and Sneha agree I will write driver in terms of davinci instead of dm365.
>
I add a vote the davinci* naming instead of dm365*.
Kevin
> Thanks,
>
> Miguel Aguilar
>
> Nori, Sekhar wrote:
>> On Fri, Sep 18, 2009 at 20:02:32, Miguel Aguilar wrote:
>>> Sekhar,
>>>
>>> See the comments below.
>>>
>>> Nori, Sekhar wrote:
>>>> On Fri, Sep 18, 2009 at 01:36:36, miguel.aguilar@ridgerun.com wrote:
>>
>> [...]
>>
>>>>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>>>>> index a6b989a..08ed7d4 100644
>>>>> --- a/drivers/input/keyboard/Kconfig
>>>>> +++ b/drivers/input/keyboard/Kconfig
>>>>> @@ -361,4 +361,10 @@ config KEYBOARD_XTKBD
>>>>> To compile this driver as a module, choose M here: the
>>>>> module will be called xtkbd.
>>>>>
>>>>> +config KEYBOARD_DAVINCI_DM365
>>>> How about calling it just KEYBOARD_DAVINCI in honor of this
>>>> being the first DaVinci keypad driver in drivers/input/keyboard?
>>>> Also, in the hope that this will get reused on a future DaVinci.
>>>>
>>>> Unless, there is something that makes it really DM365 specific..
>>> [MA] It is better keep it specific since that symbols refers to a driver which
>>> is DM365 specific at the moment.
>>
>> Okay. There is no device which reuses the keyscan module at
>> present, but there has been a lot of IP reuse in the DaVinci
>> family before (emac, mmc/sd, i2c, spi etc.).
>>
>>>>> + tristate "TI DaVinci DM365 Keypad"
>>>>> + depends on ARCH_DAVINCI_DM365
>>>>> + help
>>>>> + Supports the keypad module on the DM365
>>>>> +
>>>>> endif
>>>> [...]
>>>>
>>>>> diff --git a/drivers/input/keyboard/davinci_dm365_keypad.c b/drivers/input/keyboard/davinci_dm365_keypad.c
>>>>> new file mode 100644
>>>>> index 0000000..5db8eed
>>>>> --- /dev/null
>>>>> +++ b/drivers/input/keyboard/davinci_dm365_keypad.c
>>>>> @@ -0,0 +1,331 @@
>>>>> +/*
>>>>> + * DaVinci DM365 Keypad Driver
>>>>> + *
>>>>> + * Copyright (C) 2009 Texas Instruments, Inc
>>>>> + *
>>>>> + * Author: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>>>>> + *
>>>>> + * Intial Code: Sandeep Paulraj <s-paulraj@ti.com>
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or modify
>>>>> + * it under the terms of the GNU General Public License as published by
>>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>>> + * (at your option) any later version.
>>>>> + *
>>>>> + * This program is distributed in the hope that it will be useful,
>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>>> + * GNU General Public License for more details.
>>>>> + *
>>>>> + * You should have received a copy of the GNU General Public License
>>>>> + * along with this program; if not, write to the Free Software
>>>>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>>>>> + */
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/init.h>
>>>>> +#include <linux/interrupt.h>
>>>>> +#include <linux/types.h>
>>>>> +#include <linux/input.h>
>>>>> +#include <linux/kernel.h>
>>>>> +#include <linux/delay.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <linux/errno.h>
>>>>> +
>>>>> +#include <asm/irq.h>
>>>>> +
>>>>> +#include <mach/hardware.h>
>>>>> +#include <mach/irqs.h>
>>>>> +#include <mach/keypad.h>
>>>>> +
>>>>> +/* Keypad registers */
>>>>> +#define DM365_KEYPAD_KEYCTRL 0x0000
>>>> There are many places in the driver (functions, macros) which
>>>> probably can just be called DAVINCI_ instead of DM365_
>>> [MA]Again, these are DM365 specific definitions, If your proposal is to make
>>> this driver generic to Davinci instead of DM365 specific, I should get rid of
>>> all dm365 labels, messages, function names, etc in this driver, even the
>>> driver's name should be davinci_keypad instead of dm365_keypad, so in that sense
>>> not only the macros and the config symbol should be changed; but for doing that
>>> we are assuming that future chips of the Davinci family will include the same
>>> keypad module, so they would be compatible among them to use the same driver.
>>
>> Yes, typically attempt is made to reuse the IP as-is.
>> Slight changes in the IPs re-used have previously been
>> taken care use a "version" field coming from the platform
>> data.
>>
>> The product number population below seems to be a bigger
>> sore than the variable/macro/Kconfig naming.
>>
>>>>> +static irqreturn_t dm365_kp_interrupt(int irq, void *dev_id)
>>>>> +{
>>>>> + int i;
>>>>> + u32 prev_status, new_status, changed;
>>>>> + int keycode = KEY_UNKNOWN;
>>>>> + struct davinci_kp *dm365_kp = dev_id;
>>>>> + int *keymap = dm365_kp->pdata->keymap;
>>>>> + u32 keymapsize = dm365_kp->pdata->keymapsize;
>>>>> + struct device *dev = &dm365_kp->input->dev;
>>>>> +
>>>>> + /* Disable interrupt */
>>>>> + dm365_kp_write(dm365_kp, 0x0, DM365_KEYPAD_INTENA);
>>>>> +
>>>>> + /* Reading previous and new status of the keypad */
>>>>> + prev_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_PREVSTATE);
>>>>> + new_status = dm365_kp_read(dm365_kp, DM365_KEYPAD_CURRENTST);
>>>>> +
>>>>> + changed = prev_status ^ new_status;
>>>>> +
>>>>> + for (i = 0; i < keymapsize; i++) {
>>>>> + if ((changed >> i) & 0x1) {
>>>> Using ffs(changed) will probably lead to a faster ISR, especially
>>>> because only one key would have changed in most cases.
>>> [MA] OK, I will try ffs.
>>>>> + keycode = keymap[i];
>>>>> + if((new_status >> i) & 0x1) {
>>>>> + /* Report release */
>>>>> + input_report_key(dm365_kp->input, keycode, 0);
>>>>> + input_sync(dm365_kp->input);
>>>>> + dev_dbg(dev, "dm365_keypad: key %d released\n",
>>>>> + keycode);
>>>>> + } else {
>>>>> + /* Report press */
>>>>> + input_report_key(dm365_kp->input, keycode, 1);
>>>>> + input_sync(dm365_kp->input);
>>>>> + dev_dbg(dev, "dm365_keypad: key %d pressed\n",
>>>>> + keycode);
>>>>> + }
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + /* Clearing interrupt */
>>>>> + dm365_kp_write(dm365_kp, DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
>>>>> +
>>>>> + /* Enable interrupts */
>>>>> + dm365_kp_write(dm365_kp, 0x1, DM365_KEYPAD_INTENA);
>>>>> +
>>>>> + return IRQ_HANDLED;
>>
>> I missed this before, if the loop above turns up nothing,
>> then the IRQ was not handled. IRQ_NONE will be a better return
>> in that case.
>
>>
>> [...]
>>
>>>>> + dm365_kp->irq = platform_get_irq(pdev, 0);
>>>>> + if (dm365_kp->irq <= 0) {
>>>>> + dev_err(dev, "%s: No DM365 Keypad irq\n", pdev->name);
>>>>> + ret = dm365_kp->irq;
>>>> Probe will return success when dm365_kp->irq is 0, but you actually
>>>> want a failure.
>>> [MA] What is the proper error code for this case?.
>>
>> platform_get_irq() returns an error which should be propagated. The
>> question is: Why is 0 not a valid IRQ number for keyscan module?
>>
>> Thanks,
>> Sekhar
>>
>
>
>
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
2009-09-22 17:37 ` Kevin Hilman
@ 2009-09-23 3:46 ` Dmitry Torokhov
2009-09-23 6:20 ` Nori, Sekhar
0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2009-09-23 3:46 UTC (permalink / raw)
To: Kevin Hilman
Cc: Miguel Aguilar, Nori, Sekhar, davinci-linux-open-source,
clark.becker, linux-input, santiago.nunez, todd.fischer
On Tue, Sep 22, 2009 at 10:37:35AM -0700, Kevin Hilman wrote:
> Miguel Aguilar <miguel.aguilar@ridgerun.com> writes:
>
> > Sekhar,
> >
> > If you and Sneha agree I will write driver in terms of davinci instead of dm365.
> >
>
> I add a vote the davinci* naming instead of dm365*.
>
Can you guarantee that this will be the only keypad implementation for
davinci? If not then I'd say using base model number and updating
Kconfig text is fine.
--
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
2009-09-23 3:46 ` Dmitry Torokhov
@ 2009-09-23 6:20 ` Nori, Sekhar
2009-09-23 14:58 ` Miguel Aguilar
0 siblings, 1 reply; 10+ messages in thread
From: Nori, Sekhar @ 2009-09-23 6:20 UTC (permalink / raw)
To: Dmitry Torokhov, Kevin Hilman
Cc: Miguel Aguilar, davinci-linux-open-source, clark.becker,
linux-input, santiago.nunez, todd.fischer
On Wed, Sep 23, 2009 at 09:16:27, Dmitry Torokhov wrote:
> On Tue, Sep 22, 2009 at 10:37:35AM -0700, Kevin Hilman wrote:
> > Miguel Aguilar <miguel.aguilar@ridgerun.com> writes:
> >
> > > Sekhar,
> > >
> > > If you and Sneha agree I will write driver in terms of davinci instead of dm365.
> > >
> >
> > I add a vote the davinci* naming instead of dm365*.
> >
>
> Can you guarantee that this will be the only keypad implementation for
> davinci?
No. That cannot be guaranteed. Surely there will be attempt to reuse
this before inventing new. A new implementation will be a probably be
a new driver (named after what the new implementation is called).
So, I guess it is better to call this driver "davinci-keyscan" after
what the IP has been called (http://focus.ti.com/lit/ug/sprufi8/sprufi8.pdf).
> If not then I'd say using base model number and updating
> Kconfig text is fine.
But hope the other way is okay too. Miguel has already submitted a new
patch using davinci naming.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
2009-09-23 6:20 ` Nori, Sekhar
@ 2009-09-23 14:58 ` Miguel Aguilar
2009-09-23 15:13 ` Nori, Sekhar
0 siblings, 1 reply; 10+ messages in thread
From: Miguel Aguilar @ 2009-09-23 14:58 UTC (permalink / raw)
To: Nori, Sekhar
Cc: Dmitry Torokhov, Kevin Hilman, davinci-linux-open-source,
clark.becker, linux-input, santiago.nunez, todd.fischer
Sekhar,
Nori, Sekhar wrote:
> On Wed, Sep 23, 2009 at 09:16:27, Dmitry Torokhov wrote:
>> On Tue, Sep 22, 2009 at 10:37:35AM -0700, Kevin Hilman wrote:
>>> Miguel Aguilar <miguel.aguilar@ridgerun.com> writes:
>>>
>>>> Sekhar,
>>>>
>>>> If you and Sneha agree I will write driver in terms of davinci instead of dm365.
>>>>
>>> I add a vote the davinci* naming instead of dm365*.
>>>
>> Can you guarantee that this will be the only keypad implementation for
>> davinci?
>
> No. That cannot be guaranteed. Surely there will be attempt to reuse
> this before inventing new. A new implementation will be a probably be
> a new driver (named after what the new implementation is called).
>
> So, I guess it is better to call this driver "davinci-keyscan" after
[MA] Do you think that it is better naming this driver as Key Scan instead of
Keypad?
> what the IP has been called (http://focus.ti.com/lit/ug/sprufi8/sprufi8.pdf).
>
>> If not then I'd say using base model number and updating
>> Kconfig text is fine.
>
> But hope the other way is okay too. Miguel has already submitted a new
> patch using davinci naming.
>
> Thanks,
> Sekhar
>
Thanks,
Miguel Aguilar
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support
2009-09-23 14:58 ` Miguel Aguilar
@ 2009-09-23 15:13 ` Nori, Sekhar
0 siblings, 0 replies; 10+ messages in thread
From: Nori, Sekhar @ 2009-09-23 15:13 UTC (permalink / raw)
To: Miguel Aguilar
Cc: Dmitry Torokhov, Kevin Hilman, davinci-linux-open-source,
clark.becker, linux-input, santiago.nunez, todd.fischer
On Wed, Sep 23, 2009 at 20:28:19, Miguel Aguilar wrote:
> Sekhar,
>
> Nori, Sekhar wrote:
> > On Wed, Sep 23, 2009 at 09:16:27, Dmitry Torokhov wrote:
> >> On Tue, Sep 22, 2009 at 10:37:35AM -0700, Kevin Hilman wrote:
> >>> Miguel Aguilar <miguel.aguilar@ridgerun.com> writes:
> >>>
> >>>> Sekhar,
> >>>>
> >>>> If you and Sneha agree I will write driver in terms of davinci instead of dm365.
> >>>>
> >>> I add a vote the davinci* naming instead of dm365*.
> >>>
> >> Can you guarantee that this will be the only keypad implementation for
> >> davinci?
> >
> > No. That cannot be guaranteed. Surely there will be attempt to reuse
> > this before inventing new. A new implementation will be a probably be
> > a new driver (named after what the new implementation is called).
> >
> > So, I guess it is better to call this driver "davinci-keyscan" after
>
> [MA] Do you think that it is better naming this driver as Key Scan instead of
> Keypad?
Yes.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-09-23 15:13 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-17 20:06 [PATCH v3 1/2] Input: Add DaVinci DM365 Keypad support miguel.aguilar
2009-09-18 5:35 ` Nori, Sekhar
2009-09-18 14:32 ` Miguel Aguilar
2009-09-22 7:05 ` Nori, Sekhar
2009-09-22 14:01 ` Miguel Aguilar
2009-09-22 17:37 ` Kevin Hilman
2009-09-23 3:46 ` Dmitry Torokhov
2009-09-23 6:20 ` Nori, Sekhar
2009-09-23 14:58 ` Miguel Aguilar
2009-09-23 15:13 ` Nori, Sekhar
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.