Hi Sandeep,
CCing linux-input. You have to submit only keypad driver to
linux-input again, once all the dependencies are mainlined first.
On Sun, Jun 21, 2009 at 8:36 PM, <s-paulraj@xxxxxx> wrote:
> From: Sandeep Paulraj <s-paulraj@xxxxxx>
>
> Patch adds support for the Keypad on the DM365 EVM.
> At present the driver comes up fine and the device gets
> registered.
>
This is anyway expected. Do you want to keep this in commit text?
> For the keypad to work, we need to enable with the help of the
> CPLD on the DM365 EVM.
> David Brownell has a patch ready(Thanks for sending me the patch David)
>
> The CPLD patch adds on top of my DM365 patches and also
> re arranges a lot of my code that i submitted.
All dependencies plus platform data header files should be first
mainlined through davinci GIT tree and then you need to submit this
keypad driver only to linux-input so that it go mainline through
Dmitry's tree.
> create mode 100644 arch/arm/mach-davinci/include/mach/dm365_keypad.h
> create mode 100755 drivers/input/keyboard/dm365evm_keypad.c
File mode problem. Looks like you are editing from different $OS.
>
> +static int dm365evm_keymap[] = {
> + KEY_DM365_KEY2,
> + KEY_DM365_LEFT,
> + KEY_DM365_EXIT,
> + KEY_DM365_DOWN,
> + KEY_DM365_ENTER,
> + KEY_DM365_UP,
> + KEY_DM365_KEY1,
> + KEY_DM365_RIGHT,
> + KEY_DM365_MENU,
> + KEY_DM365_REC,
> + KEY_DM365_REW,
> + KEY_DM365_SKIPMINUS,
> + KEY_DM365_STOP,
> + KEY_DM365_FF,
> + KEY_DM365_SKIPPLUL,
> + KEY_DM365_PLAYPAUSE,
> + 0
> +};
As David suggested, it is better to convert it into the keymap format
like other input driver do. You can grep for gpio matrix driver on
linux-input archives.
> +
> +static struct dm365_kp_platform_data dm365evm_kp_data = {
> + .keymap = dm365evm_keymap,
> + .keymapsize = ARRAY_SIZE(dm365evm_keymap),
> + .rep = 1,
> +};
You might want to add more platform data here, like "scan time",
"debounce_time" if your keypad controller has those configurable
parameters.
> static struct davinci_uart_config uart_config __initdata = {
> diff --git a/arch/arm/mach-davinci/include/mach/dm365_keypad.h b/arch/arm/mach-davinci/include/mach/dm365_keypad.h
> new file mode 100644
> index 0000000..3d0b348
> --- /dev/null
> +++ b/arch/arm/mach-davinci/include/mach/dm365_keypad.h
> @@ -0,0 +1,81 @@
> +/*
> + *
> + * Copyright (C) 2009 Texas Instruments, Inc.
> + *
> + * 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 __DM365_KEYPAD_H__
> +#define __DM365_KEYPAD_H__
> +
> +struct dm365_kp_platform_data {
> + int *keymap;
> + unsigned int keymapsize;
> + unsigned int rep:1;
> +};
> +
> +/* Keypad registers */
> +#define DM365_KEYSCAN_BASE 0x01C69400
> +#define KEYPAD_BASE DM365_KEYSCAN_BASE
> +#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
> +
This is a wrong file for registers. You should keep them in ".c" file
of driver itself.
> +/*Masks for the various keys on the DM365*/
> +
> +#define KEY_DM365_KEY2 0
> +#define KEY_DM365_LEFT 1
> +#define KEY_DM365_EXIT 2
> +#define KEY_DM365_DOWN 3
> +#define KEY_DM365_ENTER 4
> +#define KEY_DM365_UP 5
> +#define KEY_DM365_KEY1 6
> +#define KEY_DM365_RIGHT 7
> +#define KEY_DM365_MENU 8
> +#define KEY_DM365_REC 9
> +#define KEY_DM365_REW 10
> +#define KEY_DM365_SKIPMINUS 11
> +#define KEY_DM365_STOP 12
> +#define KEY_DM365_FF 13
> +#define KEY_DM365_SKIPPLUL 14
> +#define KEY_DM365_PLAYPAUSE 15
> +
> +#endif
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 5f09663..aee1790 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -340,4 +340,10 @@ config KEYBOARD_DM355EVM
> Supports the pushbuttons and IR remote used with
> the DM355 EVM board.
>
> +config KEYBOARD_DM365EVM
> + tristate "TI DaVinci DM365 EVM Keypad"
> + depends on ARCH_DAVINCI_DM365
> + help
> + Supports only the keypad Module on the DM365 EVM
> +
> endif
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index a698caa..77d547d 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_KEYBOARD_MAPLE) += maple_keyb.o
> obj-$(CONFIG_KEYBOARD_BFIN) += bf54x-keys.o
> obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
> obj-$(CONFIG_KEYBOARD_DM355EVM) += dm355evm_keys.o
> +obj-$(CONFIG_KEYBOARD_DM365EVM) += dm365evm_keypad.o
> diff --git a/drivers/input/keyboard/dm365evm_keypad.c b/drivers/input/keyboard/dm365evm_keypad.c
> new file mode 100755
> index 0000000..99b12e2
> --- /dev/null
> +++ b/drivers/input/keyboard/dm365evm_keypad.c
> @@ -0,0 +1,279 @@
> +/*
> + *
> + * Copyright (C) 2009 Texas Instruments, Inc
> + *
> + * 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 <linux/mutex.h>
> +#include <linux/io.h>
> +#include <asm/irq.h>
> +#include <mach/dm365_keypad.h>
> +#include <mach/hardware.h>
> +#include <mach/irqs.h>
> +#include <mach/mux.h>
> +
> +#define dm365_keypad_write(val, addr) __raw_writel(val, \
> + (unsigned int *)((u32)dm365_kp_base + (u32)(addr)))
> +#define dm365_keypad_read(addr) __raw_readl(\
> + (unsigned int *)((u32)dm365_kp_base + (u32)(addr)))
> +
Please convert them into functions, so that you can have benefit of
type checking.
> +struct davinci_kp {
> + struct input_dev *input;
> + int irq;
> +};
> +
> +static void __iomem *dm365_kp_base;
> +static resource_size_t dm365_kp_pbase;
> +static size_t dm365_kp_base_size;
> +static int *keymap;
Someof this should go into your driver structure "davinci_kp" instead.
> +
> +void dm365_keypad_initialize(void)
> +{
> + /*Initializing the Keypad Module */
> +
> + /* Enable all interrupts */
> + dm365_keypad_write(DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTENA);
> +
> + /* Clear interrupts if any */
> + dm365_keypad_write(DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
> +
> + /* Setup the scan period */
> + dm365_keypad_write(0x05, DM365_KEYPAD_STRBWIDTH);
> + dm365_keypad_write(0x02, DM365_KEYPAD_INTERVAL);
> + dm365_keypad_write(0x01, DM365_KEYPAD_CONTTIME);
As said above "scan" period should come from platform data instead.
> +
> + /* Enable Keyscan module and enable */
> + dm365_keypad_write(DM365_KEYPAD_AUTODET | DM365_KEYPAD_KEYEN,
> + DM365_KEYPAD_KEYCTRL);
> +}
> +
> +static irqreturn_t dm365_kp_interrupt(int irq, void *dev_id)
> +{
> + int i;
> + unsigned int status, temp, temp1;
> + int keycode = KEY_UNKNOWN;
> + struct davinci_kp *dm365_kp = dev_id;
> +
> + /* Reading the status of the Keypad */
> + status = dm365_keypad_read(DM365_KEYPAD_PREVSTATE);
> +
> + temp = ~status;
> +
> + for (i = 0; i < 16; i++) {
What is "16"?
> + temp1 = temp >> i;
> + if (temp1 & 0x1) {
> + keycode = keymap[i];
> +
> + /* report press + release */
> + input_report_key(dm365_kp->input, keycode, 1);
> + input_sync(dm365_kp->input);
> + input_report_key(dm365_kp->input, keycode, 0);
> + input_sync(dm365_kp->input);
Why you want to send "release" just after "press"? This code needs some comment.
> + }
> + }
> +
> + /* clearing interrupts */
> + dm365_keypad_write(DM365_KEYPAD_INT_ALL, DM365_KEYPAD_INTCLR);
> +
> + 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;
> + unsigned int val;
> + struct dm365_kp_platform_data *pdata = pdev->dev.platform_data;
> +
> + /* Enabling pins for Keypad */
> + davinci_cfg_reg(DM365_KEYPAD);
> +
> + /* TODO
> + * Enable the Keypad Module by writing appropriate value
> + * to CPLD Register 3
> + * Will wait for David Brownell's CPLD patch
> + */
> +
> + if (!pdata->keymap) {
> + printk(KERN_ERR "No keymap from pdata\n");
> + return -EINVAL;
> + }
> +
> + dm365_kp = kzalloc(sizeof *dm365_kp, GFP_KERNEL);
> + key_dev = input_allocate_device();
> +
> + if (!dm365_kp || !key_dev) {
> + printk(KERN_ERR "Could not allocate input device\n");
> + return -EINVAL;
> + }
> +
> + platform_set_drvdata(pdev, dm365_kp);
> +
> + dm365_kp->input = key_dev;
> +
> + dm365_kp->irq = platform_get_irq(pdev, 0);
> + if (dm365_kp->irq <= 0) {
> + pr_debug("%s: No DM365 Keypad irq\n", pdev->name);
> + goto fail1;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + dm365_kp_pbase = res->start;
> + dm365_kp_base_size = res->end - res->start + 1;
Please use resource_size.
> +
> + if (res)
> + mem = request_mem_region(res->start,
> + dm365_kp_base_size, pdev->name);
> + else
> + mem = NULL;
> +
> + if (!mem) {
> + pr_debug("%s: KEYSCAN registers at %08x are not free\n",
> + pdev->name, DM365_KEYSCAN_BASE);
> + goto fail1;
> + }
> +
> + dm365_kp_base = ioremap(res->start, dm365_kp_base_size);
> + if (dm365_kp_base == NULL) {
> + pr_debug("%s: Can't ioremap MEM resource.\n", pdev->name);
> + goto fail2;
> + }
> +
> + /* Enable auto repeat feature of Linux input subsystem */
> + if (pdata->rep)
> + set_bit(EV_REP, key_dev->evbit);
Please use non-atomic version of this. __set_bit.
> +
> + /* setup input device */
> + set_bit(EV_KEY, key_dev->evbit);
__set_bit.
> +
> + /* Setup the keymap */
> + keymap = pdata->keymap;
> +
> + for (i = 0; i < 16; i++)
> + set_bit(keymap[i], key_dev->keybit);
__set_bit.
> +
> + 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 = keymap;
> + key_dev->keycodesize = sizeof(unsigned int);
> + key_dev->keycodemax = pdata->keymapsize;
Instead you should define max_keymap_size, which is supported for
controller itself.
> +
> + ret = input_register_device(dm365_kp->input);
> + if (ret < 0) {
> + printk(KERN_ERR
> + "Unable to register DaVinci DM365 keypad device\n");
> + goto fail3;
> + }
> +
> + ret = request_irq(dm365_kp->irq, dm365_kp_interrupt, IRQF_DISABLED,
> + "dm365_keypad", dm365_kp);
> + if (ret < 0) {
> + printk(KERN_ERR
> + "Unable to register DaVinci DM365 keypad Interrupt\n");
> + goto fail4;
> + }
> +
> + dm365_keypad_initialize();
> +
> + return 0;
> +fail4:
> + input_unregister_device(dm365_kp->input);
> +fail3:
> + iounmap(dm365_kp_base);
> +fail2:
> + release_mem_region(dm365_kp_pbase, dm365_kp_base_size);
> +fail1:
> + kfree(dm365_kp);
> + input_free_device(key_dev);
input_free_device after input_unregister_device is not required,
before input_dev is refcounted. In above code put dm365_kp->input =
NULL after input_unregister_device.
> +
> + /* TODO
> + * Re enabling other modules by doing a CPLD write
> + * Will wait for David Brownell's patch
> + */
> +
> + return -EINVAL;
> +}
> +
> +static int __devexit dm365_kp_remove(struct platform_device *pdev)
> +{
> + struct davinci_kp *dm365_kp = platform_get_drvdata(pdev);
> +
> + input_unregister_device(dm365_kp->input);
> + free_irq(dm365_kp->irq, dm365_kp);
> + kfree(dm365_kp);
> +
> + iounmap(dm365_kp_base);
> + release_mem_region(dm365_kp_pbase, dm365_kp_base_size);
> +
> + platform_set_drvdata(pdev, NULL);
> +
> + /* TODO
> + * Re enabling other modules by doing a CPLD write
> + * Will wait for David Brownell's patch
> + */
> +
> + return 0;
> +}
> +
> +static struct platform_driver dm365_kp_driver = {
> + .probe = dm365_kp_probe,
> + .remove = __devexit_p(dm365_kp_remove),
> + .driver = {
> + .name = "dm365_keypad",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init dm365_kp_init(void)
> +{
> + printk(KERN_INFO "DaVinci DM365 Keypad Driver\n");
> +
No need of banner.
> + return platform_driver_register(&dm365_kp_driver);
> +}
> +
> +static void __exit dm365_kp_exit(void)
> +{
> + platform_driver_unregister(&dm365_kp_driver);
> +}
> +
> +module_init(dm365_kp_init);
> +module_exit(dm365_kp_exit);
> +
> +MODULE_AUTHOR("Sandeep Paulraj");
> +MODULE_DESCRIPTION("Texas Instruments DaVinci DM365 EVM Keypad Driver");
> +MODULE_LICENSE("GPL");
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
