linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miguel Aguilar <miguel.aguilar@ridgerun.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: nsnehaprabha@ti.com,
	davinci-linux-open-source@linux.davincidsp.com,
	linux-input@vger.kernel.org, todd.fischer@ridgerun.com,
	diego.dompe@ridgerun.com, clark.becker@ridgerun.com,
	santiago.nunez@ridgerun.com, David Brownell <david-b@pacbell.net>
Subject: Re: [PATCH 1/2] Input: DaVinci Keypad Driver
Date: Wed, 23 Sep 2009 11:07:06 -0600	[thread overview]
Message-ID: <4ABA55BA.7000009@ridgerun.com> (raw)
In-Reply-To: <20090923163546.GA13435@core.coreip.homeip.net>

Dmitry,

Dmitry Torokhov wrote:
> Hi Miguel,
> 
> [Adding David to CC for the __devexit/__exit discussion]
> 
> On Wed, Sep 23, 2009 at 08:52:40AM -0600, Miguel Aguilar wrote:
>> Dmitry,
>>
>> Dmitry Torokhov wrote:
>>> Hi Miguel,
>>>
>>> On Tue, Sep 22, 2009 at 03:27:30PM -0600, miguel.aguilar@ridgerun.com wrote:
>>>> From: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>>>>
>>>> Adds the driver for enabling keypad support for DaVinci platforms.
>>>>
>>>> DM365 is the only platform that uses this driver at the moment.
>>>>
>>>> This driver was tested on DM365 EVM rev C.
>>>>
>>>> Signed-off-by: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>>>> ---
>>>>  arch/arm/configs/davinci_all_defconfig      |    1 +
>>>>  arch/arm/mach-davinci/include/mach/keypad.h |   35 +++
>>>>  drivers/input/keyboard/Kconfig              |    7 +
>>>>  drivers/input/keyboard/Makefile             |    1 +
>>>>  drivers/input/keyboard/davinci_keypad.c     |  319 +++++++++++++++++++++++++++
>>>>  5 files changed, 363 insertions(+), 0 deletions(-)
>>>>  create mode 100644 arch/arm/mach-davinci/include/mach/keypad.h
>>>>  create mode 100644 drivers/input/keyboard/davinci_keypad.c
>>>>
>>>> diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
>>>> index ec63c15..e994c83 100644
>>>> --- a/arch/arm/configs/davinci_all_defconfig
>>>> +++ b/arch/arm/configs/davinci_all_defconfig
>>>> @@ -763,6 +763,7 @@ CONFIG_KEYBOARD_GPIO=y
>>>>  # CONFIG_KEYBOARD_STOWAWAY is not set
>>>>  # CONFIG_KEYBOARD_SUNKBD is not set
>>>>  CONFIG_KEYBOARD_XTKBD=m
>>>> +CONFIG_KEYBOARD_DAVINCI_DM365=m
>>>>  # CONFIG_INPUT_MOUSE is not set
>>>>  # CONFIG_INPUT_JOYSTICK is not set
>>>>  # CONFIG_INPUT_TABLET is not set
>>>
>>> If you want to merge the driver through input tree the defconfig chunk
>>> has to go elsewhere.
>> [MA] So, Where it should go?
> 
> In a separate patch submitted to the person maintaining defconfig for
> the arch in question once driver is in mainline.

[MA] Ok I see.
> 
>>>> 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..b6b9517 100644
>>>> --- a/drivers/input/keyboard/Kconfig
>>>> +++ b/drivers/input/keyboard/Kconfig
>>>> @@ -361,4 +361,11 @@ config KEYBOARD_XTKBD
>>>>  	  To compile this driver as a module, choose M here: the
>>>>  	  module will be called xtkbd.
>>>>  +config KEYBOARD_DAVINCI
>>>> +	tristate "TI DaVinci Keypad"
>>>> +	depends on ARCH_DAVINCI_DM365
>>>> +	help
>>>> +	  Say Y to enable keypad module support for the TI DaVinci
>>>> +	  platforms (DM365)
>>>> +
>>> "To compile this driver as a module..."
>> [MA] Ok.
>>>>  endif
>>>> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
>>>> index b5b5eae..0b0274e 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)		+= davinci_keypad.o
>>>> diff --git a/drivers/input/keyboard/davinci_keypad.c b/drivers/input/keyboard/davinci_keypad.c
>>>> new file mode 100644
>>>> index 0000000..6f0e793
>>>> --- /dev/null
>>>> +++ b/drivers/input/keyboard/davinci_keypad.c
>>>> @@ -0,0 +1,319 @@
>>>> +/*
>>>> + * DaVinci Keypad Driver for TI platforms
>>>> + *
>>>> + * 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 DAVINCI_KEYPAD_KEYCTRL		0x0000
>>>> +#define DAVINCI_KEYPAD_INTENA		0x0004
>>>> +#define DAVINCI_KEYPAD_INTFLAG		0x0008
>>>> +#define DAVINCI_KEYPAD_INTCLR		0x000c
>>>> +#define DAVINCI_KEYPAD_STRBWIDTH	0x0010
>>>> +#define DAVINCI_KEYPAD_INTERVAL		0x0014
>>>> +#define DAVINCI_KEYPAD_CONTTIME		0x0018
>>>> +#define DAVINCI_KEYPAD_CURRENTST	0x001c
>>>> +#define DAVINCI_KEYPAD_PREVSTATE	0x0020
>>>> +#define DAVINCI_KEYPAD_EMUCTRL		0x0024
>>>> +#define DAVINCI_KEYPAD_IODFTCTRL	0x002c
>>>> +
>>>> +/* Key Control Register (KEYCTRL) */
>>>> +#define DAVINCI_KEYPAD_KEYEN		0x00000001
>>>> +#define DAVINCI_KEYPAD_PREVMODE		0x00000002
>>>> +#define DAVINCI_KEYPAD_CHATOFF		0x00000004
>>>> +#define DAVINCI_KEYPAD_AUTODET		0x00000008
>>>> +#define DAVINCI_KEYPAD_SCANMODE		0x00000010
>>>> +#define DAVINCI_KEYPAD_OUTTYPE		0x00000020
>>>> +#define DAVINCI_KEYPAD_4X4		0x00000040
>>>> +
>>>> +/* Masks for the interrupts */
>>>> +#define DAVINCI_KEYPAD_INT_CONT		0x00000008
>>>> +#define DAVINCI_KEYPAD_INT_OFF		0x00000004
>>>> +#define DAVINCI_KEYPAD_INT_ON		0x00000002
>>>> +#define DAVINCI_KEYPAD_INT_CHANGE	0x00000001
>>>> +#define DAVINCI_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 davinci_kp_write(struct davinci_kp *davinci_kp, u32 val, u32 addr)
>>>> +{
>>>> +	u32 base = (u32)davinci_kp->base;
>>>> +
>>>> +	__raw_writel(val,(u32 *)(base + addr));
>>>> +}
>>>> +
>>>> +static u32 davinci_kp_read(struct davinci_kp *davinci_kp, u32 addr)
>>>> +{
>>>> +	u32 base = (u32)davinci_kp->base;
>>>> +
>>>> +	return __raw_readl((u32 *)(base + addr));
>>>> +}
>>>> +
>>>> +/* Initializing the kp Module */
>>>> +static void davinci_kp_initialize(struct davinci_kp *davinci_kp)
>>>> +{
>>>> +	u32 strobe = davinci_kp->pdata->strobe;
>>>> +	u32 interval = davinci_kp->pdata->interval;
>>>> +
>>>> +	/* Enable all interrupts */
>>>> +	davinci_kp_write(davinci_kp, DAVINCI_KEYPAD_INT_ALL, DAVINCI_KEYPAD_INTENA);
>>>> +
>>>> +	/* Clear interrupts if any */
>>>> +	davinci_kp_write(davinci_kp, DAVINCI_KEYPAD_INT_ALL, DAVINCI_KEYPAD_INTCLR);
>>>> +
>>>> +	/* Setup the scan period = strobe + interval */
>>>> +	davinci_kp_write(davinci_kp, strobe, DAVINCI_KEYPAD_STRBWIDTH);
>>>> +	davinci_kp_write(davinci_kp, interval, DAVINCI_KEYPAD_INTERVAL);
>>>> +	davinci_kp_write(davinci_kp, 0x01, DAVINCI_KEYPAD_CONTTIME);
>>>> +
>>>> +	/* Enable Keyscan module and enable */
>>>> +	davinci_kp_write(davinci_kp, DAVINCI_KEYPAD_AUTODET | DAVINCI_KEYPAD_KEYEN,
>>>> +			DAVINCI_KEYPAD_KEYCTRL);
>>>> +}
>>>> +
>>>> +static irqreturn_t davinci_kp_interrupt(int irq, void *dev_id)
>>>> +{
>>>> +	struct davinci_kp *davinci_kp = dev_id;
>>>> +	struct device *dev = &davinci_kp->input->dev;
>>>> +	int *keymap = davinci_kp->pdata->keymap;
>>>> +		u32 prev_status, new_status, changed, position;
>>>> +	int keycode = KEY_UNKNOWN;
>>>> +	int ret = IRQ_NONE;
>>>> +
>>>> +	/* Disable interrupt */
>>>> +	davinci_kp_write(davinci_kp, 0x0, DAVINCI_KEYPAD_INTENA);
>>>> +
>>>> +	/* Reading previous and new status of the keypad */
>>>> +	prev_status = davinci_kp_read(davinci_kp, DAVINCI_KEYPAD_PREVSTATE);
>>>> +	new_status = davinci_kp_read(davinci_kp, DAVINCI_KEYPAD_CURRENTST);
>>>> +
>>>> +	changed = prev_status ^ new_status;
>>>> +	position = ffs(changed) - 1;
>>>> +
>>>> +	if (changed) {
>>> Can there be several buttons that change status at once?
>> [MA] It is not suppose to change several buttons at once.
> 
> "Not supposed to happen" vs. "it can not happen, even in theory"?
[MA]According with the TI documentation the scan process and the way of how the 
IRQ is handle, expects one key at time.
> 
>>>> +		keycode = keymap[position];
>>>> +		if((new_status >> position) & 0x1) {
>>> bool release = (new_status >> position) & 0x1;
>>> input_report_key(davinci_kp->input, keycode, !release);
>>> dev_dbg(dev, "davinci_keypad: key %d %s\n",
>>> 	keycode, release ? "released" : "pressed");
>>>
>> [MA] Ok I'll try that.
>>> is shorter.
>>>
>>>> +			/* Report release */
>>>> +			dev_dbg(dev, "davinci_keypad: key %d released\n",
>>>> +				    keycode);
>>>> +			input_report_key(davinci_kp->input, keycode, 0);
>>>> +		} else {
>>>> +			/* Report press */
>>>> +			dev_dbg(dev, "davinci_keypad: key %d pressed\n",
>>>> +				    keycode);
>>>> +			input_report_key(davinci_kp->input, keycode, 1);
>>>> +		}
>>>> +		input_sync(davinci_kp->input);
>>>> +		ret = IRQ_HANDLED;
>>>> +	}
>>>> +
>>>> +	/* Clearing interrupt */
>>>> +	davinci_kp_write(davinci_kp, DAVINCI_KEYPAD_INT_ALL, DAVINCI_KEYPAD_INTCLR);
>>> You return IRQ_HANDLED only if keypad state changed but clear interrupt
>>> regardless. This is suspicious.
>> [MA] Ok. I'll clear the irq status only if IRQ_HANDLED.
> 
> It really depends... What if key was pressed but is released before
> interrupt is handled? Do you want to see "spurious IRQ" warnings?
> 
So, what is the proper way to clear the interrupt in this particular case?
>>>> +
>>>> +	/* Enable interrupts */
>>>> +	davinci_kp_write(davinci_kp, 0x1, DAVINCI_KEYPAD_INTENA);
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +static int __init davinci_kp_probe(struct platform_device *pdev)
>>>> +{
>>>> +	struct davinci_kp *davinci_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 Keypad Driver\n");
>>>> +
>>>> +	if (!pdata->keymap) {
>>>> +		dev_dbg(dev, "no keymap from pdata\n");
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	davinci_kp = kzalloc(sizeof *davinci_kp, GFP_KERNEL);
>>>> +	if(!davinci_kp) {
>>>> +		dev_dbg(dev, "could not allocate memory for private data\n");
>>>> +		return -ENOMEM;
>>>> +	}
>>>> +
>>>> +	key_dev = input_allocate_device();
>>>> +	if (!key_dev) {
>>>> +		dev_dbg(dev, "could not allocate input device\n");
>>>> +		ret = -ENOMEM;
>>>> +		goto fail1;
>>>> +	}
>>>> +
>>>> +	platform_set_drvdata(pdev, davinci_kp);
>>>> +
>>>> +	davinci_kp->input = key_dev;
>>>> +
>>>> +	davinci_kp->irq = platform_get_irq(pdev, 0);
>>>> +	if (davinci_kp->irq < 0) {
>>>> +		dev_err(dev, "no keypad irq\n");
>>>> +		ret = davinci_kp->irq;
>>>> +		goto fail2;
>>>> +	}
>>>> +
>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +	if (!res) {
>>>> +		dev_err(dev, "no mem resource\n");
>>>> +		ret = -ENODEV;
>>> -EINVAL I'd say.
>> [MA] platform_get_resource should fail with -ENODEV.
> 
> If you fail to get platform resource then th eplatform code is set up
> incorrectly, therefore I'd still argue for -EINVAL. It is not as if
> device may not be there because then the platform device would not be
> present at all.
[MA] It is weird since most of the drivers use -ENODEV. I addressed the Sergei 
comments in this sence:

"
 > What are the proper error codes when platform_get_resource,

    -ENODEV.

 > request_mem_region

    -EBUSY.

 > and ioremap functions fail?.

    -ENOMEM.
"

> 
>>>> +		goto fail2;
>>>> +	}
>>>> +
>>>> +	davinci_kp->pbase = res->start;
>>>> +	davinci_kp->base_size = resource_size(res);
>>>> +
>>>> +	mem = request_mem_region(davinci_kp->pbase, davinci_kp->base_size, pdev->name);
>>>> +	if (!mem) {
>>>> +		dev_err(dev, "KEYSCAN registers at %08x are not free\n",
>>>> +			davinci_kp->pbase);
>>>> +		ret = -EBUSY;
>>>> +		goto fail2;
>>>> +	}
>>>> +
>>>> +	davinci_kp->base = ioremap(davinci_kp->pbase, davinci_kp->base_size);
>>>> +	if (!davinci_kp->base) {
>>>> +		dev_err(dev, "can't ioremap MEM resource.\n");
>>>> +		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 */
>>>> +	davinci_kp->pdata = pdata;
>>>> +
>>>> +	for (i = 0; i < davinci_kp->pdata->keymapsize; i++)
>>>> +		__set_bit(davinci_kp->pdata->keymap[i], key_dev->keybit);
>>>> +
>>>> +	key_dev->name = "davinci_keypad";
>>>> +	key_dev->phys = "davinci_keypad/input0";
>>>> +	key_dev->dev.parent = &pdev->dev;
>>>> +	key_dev->id.bustype = BUS_HOST;
>>>> +	key_dev->id.vendor = 0x0001;
>>>> +	key_dev->id.product = 0x0001;
>>>> +	key_dev->id.version = 0x0001;
>>>> +	key_dev->keycode = davinci_kp->pdata->keymap;
>>> Please kopy keymap into the davinci_kp stucture and use it so that
>>> platform data is never changed and can be declared const.
>> Do you mean something like this?
>>
>> struct davinci_kp {
>> 	...
>> 	const int	*keymap;
>> 	...
>> };
>>
> 
> More like:
> 
> struct davinci_kp {
> 	...
> 	unsgned char keymap[];
> 	...
> };
> 
[MA] Ok.
> and then you copy keymap from platform data into device-private
> structure so it can be safely changed via EVIOCSKEYCODE without
> affecting platform data. Also, if you reload the module, the keymap with
> be restored to its original state. The platform data may also be marked
> as const.
[MA] you mean add const in the plaform data of the davinci_kp structure.
> 
> If max size of keymap is not known then you may need to allocate it
> dynamically.
> 
>>>> +	key_dev->keycodesize = sizeof(unsigned int);
>>> sizeof(davinci_kp->keymap[0]) is safer. Plus make it unsigned short.
>> [MA] Ok.
>>>> +	key_dev->keycodemax = davinci_kp->pdata->keymapsize;
>>>> +
>>>> +	ret = input_register_device(davinci_kp->input);
>>>> +	if (ret < 0) {
>>>> +		dev_err(dev, "unable to register DaVinci keypad device\n");
>>>> +		goto fail4;
>>>> +	}
>>>> +
>>>> +	ret = request_irq(davinci_kp->irq, davinci_kp_interrupt, IRQF_DISABLED,
>>>> +                         "davinci_keypad", davinci_kp);
>>>> +	if (ret < 0) {
>>>> +		dev_err(dev, "unable to register DaVinci keypad Interrupt\n");
>>>> +		goto fail5;
>>>> +	}
>>>> +
>>>> +	davinci_kp_initialize(davinci_kp);
>>>> +
>>>> +	return 0;
>>>> +fail5:
>>>> +	input_unregister_device(davinci_kp->input);
>>>> +	key_dev = NULL;
>>>> +fail4:
>>>> +	iounmap(davinci_kp->base);
>>>> +fail3:
>>>> +	release_mem_region(davinci_kp->pbase, davinci_kp->base_size);
>>>> +fail2:
>>>> +	input_free_device(key_dev);
>>>> +fail1:
>>>> +	kfree(davinci_kp);
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +static int __exit davinci_kp_remove(struct platform_device *pdev)
>>> __devexit?
>> [MA] According to comments from David Brownell to the first version of 
>> this patch the __exit should be used.
>>
>> " - Use platform_driver_probe() and __exit/__exit_p();
>>    there's no point in keeping that code around in
>>    typical configs, it'd just waste memory. "
> 
> I am afraid David is wrong here. Even when we register driver with
> platform_driver_probe() we still have "unbind" attribute in sysfs which
> may be used to unbind the device from driver. If code is __exit then
> such attempts will cause oops.
[MA] Let's wait here for David response.
> 
>>>> +{
>>>> +	struct davinci_kp *davinci_kp = platform_get_drvdata(pdev);
>>>> +
>>>> +	free_irq(davinci_kp->irq, davinci_kp);
>>>> +
>>>> +	input_unregister_device(davinci_kp->input);
>>>> +
>>>> +	iounmap(davinci_kp->base);
>>>> +	release_mem_region(davinci_kp->pbase, davinci_kp->base_size);
>>>> +
>>>> +	platform_set_drvdata(pdev, NULL);
>>>> +
>>>> +	kfree(davinci_kp);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static struct platform_driver davinci_kp_driver = {
>>>> +	.driver = {
>>>> +			.name = "davinci_keypad",
>>>> +			.owner = THIS_MODULE,
>>>> +		},
>>>> +	.remove = __exit_p(davinci_kp_remove),
>>> __devexit_p(). I think you can still unbind the device even if you use
>>> platform_driver_probe.
>> [MA] Same.
> 
> Right, see above.
> 
>>>> +};
>>>> +
>>>> +static int __init davinci_kp_init(void)
>>>> +{
>>>> +	return platform_driver_probe(&davinci_kp_driver, davinci_kp_probe);
>>>> +}
>>>> +module_init(davinci_kp_init);
>>>> +
>>>> +static void __exit davinci_kp_exit(void)
>>>> +{
>>>> +	platform_driver_unregister(&davinci_kp_driver);
>>>> +}
>>>> +module_exit(davinci_kp_exit);
>>>> +
>>>> +MODULE_AUTHOR("Miguel Aguilar");
>>>> +MODULE_DESCRIPTION("Texas Instruments DaVinci Keypad Driver");
>>>> +MODULE_LICENSE("GPL");
>>>> -- 
>>>> 1.6.0.4
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 

Thanks,

Miguel Aguilar

  reply	other threads:[~2009-09-23 17:07 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-22 21:27 [PATCH 1/2] Input: DaVinci Keypad Driver miguel.aguilar
2009-09-23  3:46 ` Dmitry Torokhov
2009-09-23 14:52   ` Miguel Aguilar
2009-09-23 16:35     ` Dmitry Torokhov
2009-09-23 17:07       ` Miguel Aguilar [this message]
2009-09-23 17:25         ` Miguel Aguilar
2009-09-23 17:41           ` Dmitry Torokhov
2009-09-23 18:15             ` Miguel Aguilar
2009-09-23 18:19               ` Dmitry Torokhov
2009-09-23 17:51       ` David Brownell
2009-09-23 18:07         ` Dmitry Torokhov
2009-09-23 19:29           ` David Brownell
2009-09-23 19:51             ` Dmitry Torokhov
2009-09-23 23:05               ` David Brownell
2009-09-24  5:40                 ` Dmitry Torokhov
2009-09-24 14:59   ` Miguel Aguilar
2009-09-24 16:21     ` Dmitry Torokhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ABA55BA.7000009@ridgerun.com \
    --to=miguel.aguilar@ridgerun.com \
    --cc=clark.becker@ridgerun.com \
    --cc=david-b@pacbell.net \
    --cc=davinci-linux-open-source@linux.davincidsp.com \
    --cc=diego.dompe@ridgerun.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=nsnehaprabha@ti.com \
    --cc=santiago.nunez@ridgerun.com \
    --cc=todd.fischer@ridgerun.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).