All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] input/misc: PCF8574 I2C keypad input device driver
@ 2010-03-09 15:01 Mike Frysinger
  2010-03-09 18:09 ` Dmitry Torokhov
  0 siblings, 1 reply; 7+ messages in thread
From: Mike Frysinger @ 2010-03-09 15:01 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov; +Cc: uclinux-dist-devel, Bryan Wu

From: Bryan Wu <cooloney@kernel.org>

Signed-off-by: Bryan Wu <cooloney@kernel.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/input/misc/Kconfig          |   10 ++
 drivers/input/misc/Makefile         |    1 +
 drivers/input/misc/pcf8574_keypad.c |  278 +++++++++++++++++++++++++++++++++++
 3 files changed, 289 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/misc/pcf8574_keypad.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 49a31bd..fa97c58 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -396,4 +396,14 @@ config INPUT_ADXL34X_SPI
 	  To compile this driver as a module, choose M here: the
 	  module will be called adxl34x-spi.
 
+config INPUT_PCF8574
+	tristate "PCF8574 Keypad input device"
+	depends on I2C && EXPERIMENTAL
+	help
+	  Say Y here if you want to support a keypad connetced via I2C
+	  with a PCF8574.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called pcf8574_keypad.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 775db74..8107c57 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_INPUT_AD714X)		+= ad714x.o
 obj-$(CONFIG_INPUT_AD714X_I2C)		+= ad714x-i2c.o
 obj-$(CONFIG_INPUT_AD714X_SPI)		+= ad714x-spi.o
 obj-$(CONFIG_INPUT_APANEL)		+= apanel.o
+obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
 obj-$(CONFIG_INPUT_ATI_REMOTE)		+= ati_remote.o
 obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
 obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
new file mode 100644
index 0000000..85bd87c
--- /dev/null
+++ b/drivers/input/misc/pcf8574_keypad.c
@@ -0,0 +1,278 @@
+/*
+ * Driver for a keypad w/16 buttons connected to a PCF8574 I2C I/O expander
+ *
+ * Copyright 2005-2008 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/workqueue.h>
+
+#define DRV_NAME "pcf8574_keypad"
+
+MODULE_AUTHOR("Michael Hennerich");
+MODULE_DESCRIPTION("Keypad input driver for 16 keys connected to PCF8574");
+MODULE_LICENSE("GPL");
+
+static unsigned char pcf8574_kp_btncode[] = {
+	[0] = KEY_RESERVED,
+	[1] = KEY_ENTER,
+	[2] = KEY_BACKSLASH,
+	[3] = KEY_0,
+	[4] = KEY_RIGHTBRACE,
+	[5] = KEY_C,
+	[6] = KEY_9,
+	[7] = KEY_8,
+	[8] = KEY_7,
+	[9] = KEY_B,
+	[10] = KEY_6,
+	[11] = KEY_5,
+	[12] = KEY_4,
+	[13] = KEY_A,
+	[14] = KEY_3,
+	[15] = KEY_2,
+	[16] = KEY_1
+};
+
+struct kp_data {
+	unsigned char *btncode;
+	struct input_dev *idev;
+	struct i2c_client *client;
+	char name[64];
+	char phys[32];
+	unsigned char laststate;
+	unsigned char statechanged;
+	unsigned long irq_handled;
+	unsigned long events_sended;
+	unsigned long events_processed;
+	struct work_struct pcf8574_kp_work;
+};
+
+static short read_state(struct kp_data *lp)
+{
+	unsigned char x, y, a, b;
+
+	if (lp->client) {
+		i2c_smbus_write_byte(lp->client, 240);
+		x = 0xF & (~(i2c_smbus_read_byte(lp->client) >> 4));
+
+		i2c_smbus_write_byte(lp->client, 15);
+		y = 0xF & (~i2c_smbus_read_byte(lp->client));
+
+		for (a = 0; x > 0; a++)
+			x = x >> 1;
+		for (b = 0; y > 0; b++)
+			y = y >> 1;
+
+		return ((a - 1) * 4) + b;
+	}
+
+	return -1;
+}
+
+static void check_and_notify(struct work_struct *work)
+{
+	struct kp_data *lp =
+	    container_of(work, struct kp_data, pcf8574_kp_work);
+	unsigned char nextstate = read_state(lp);
+
+	lp->statechanged = lp->laststate ^ nextstate;
+
+	if (lp->statechanged) {
+		input_report_key(lp->idev,
+				 nextstate > 17 ? lp->btncode[lp->laststate] :
+				 lp->btncode[nextstate],
+				 nextstate > 17 ? 0 : 1);
+
+		lp->events_sended++;
+	}
+	lp->laststate = nextstate;
+	input_sync(lp->idev);
+	enable_irq(lp->client->irq);
+}
+
+static irqreturn_t pcf8574_kp_irq_handler(int irq, void *dev_id)
+{
+	struct kp_data *lp = dev_id;
+
+	disable_irq_nosync(lp->client->irq);
+	schedule_work(&lp->pcf8574_kp_work);
+
+	return IRQ_HANDLED;
+}
+
+static int __devinit pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	int i, ret;
+	struct input_dev *idev;
+	struct kp_data *lp;
+
+	if (i2c_smbus_write_byte(client, 240) < 0) {
+		dev_err(&client->dev, "probe: write fail\n");
+		return -ENODEV;
+	}
+
+	lp = kzalloc(sizeof(struct kp_data), GFP_KERNEL);
+	if (!lp)
+		return -ENOMEM;
+	lp->client = client;
+
+	i2c_set_clientdata(client, lp);
+
+	if (client->irq > 0) {
+		ret = request_irq(client->irq, pcf8574_kp_irq_handler,
+			IRQF_TRIGGER_LOW, DRV_NAME, lp);
+		if (ret) {
+			dev_err(&client->dev, "IRQ %d is not free\n", client->irq);
+			goto fail_irq;
+		}
+	} else
+		dev_warn(&client->dev, "IRQ not configured!\n");
+
+
+	idev = input_allocate_device();
+	if (!idev) {
+		dev_err(&client->dev, "Can't allocate input device\n");
+		ret = -ENOMEM;
+		goto fail_allocate;
+	}
+
+	lp->idev = idev;
+	lp->btncode = pcf8574_kp_btncode;
+
+	idev->evbit[0] = 0;
+
+	idev->evbit[0] |= BIT_MASK(EV_KEY);
+	idev->keycode = lp->btncode;
+	idev->keycodesize = sizeof(pcf8574_kp_btncode);
+	idev->keycodemax = ARRAY_SIZE(pcf8574_kp_btncode);
+
+	for (i = 0; i < ARRAY_SIZE(pcf8574_kp_btncode); ++i)
+		__set_bit(lp->btncode[i] & KEY_MAX, idev->keybit);
+
+	__clear_bit(KEY_RESERVED, idev->keybit);
+
+	sprintf(lp->name, DRV_NAME);
+	sprintf(lp->phys, "kp_data/input0");
+
+	idev->name = lp->name;
+	idev->phys = lp->phys;
+	idev->id.bustype = BUS_I2C;
+	idev->id.vendor = 0x0001;
+	idev->id.product = 0x0001;
+	idev->id.version = 0x0100;
+
+	input_set_drvdata(idev, lp);
+
+	ret = input_register_device(lp->idev);
+	if (ret) {
+		dev_err(&client->dev, "input_register_device() failed\n");
+		goto fail_register;
+	}
+
+	lp->statechanged = 0x0;
+
+	lp->laststate = read_state(lp);
+
+	/* Set up our workqueue. */
+	INIT_WORK(&lp->pcf8574_kp_work, check_and_notify);
+
+	dev_info(&client->dev, "input: %s at %s IRQ %d\n", lp->name, lp->phys,
+		client->irq);
+
+	return 0;
+
+ fail_register:
+	input_set_drvdata(idev, NULL);
+	input_free_device(idev);
+ fail_allocate:
+	free_irq(client->irq, lp);
+ fail_irq:
+	i2c_set_clientdata(client, NULL);
+	kfree(lp);
+
+	return ret;
+}
+
+static int __devexit pcf8574_kp_remove(struct i2c_client *client)
+{
+	struct kp_data *lp = i2c_get_clientdata(client);
+
+	if (client->irq > 0)
+		free_irq(client->irq, lp);
+
+	input_set_drvdata(lp->idev, NULL);
+	input_unregister_device(lp->idev);
+	kfree(lp);
+
+	i2c_set_clientdata(client, NULL);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int pcf8574_kp_resume(struct i2c_client *client)
+{
+	struct kp_data *lp = i2c_get_clientdata(client);
+	int ret;
+
+	if (client->irq > 0) {
+		ret = request_irq(client->irq, pcf8574_kp_irq_handler,
+			IRQF_TRIGGER_LOW, DRV_NAME, lp);
+		if (ret) {
+			dev_err(&client->dev, "IRQ %d is not free\n", client->irq);
+			return -ENODEV;
+		}
+	}
+
+	return 0;
+}
+
+static int pcf8574_kp_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+	struct kp_data *lp = i2c_get_clientdata(client);
+
+	if (client->irq > 0)
+		free_irq(client->irq, lp);
+
+	return 0;
+}
+#else
+# define pcf8574_kp_resume  NULL
+# define pcf8574_kp_suspend NULL
+#endif
+
+static const struct i2c_device_id pcf8574_kp_id[] = {
+	{ DRV_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pcf8574_kp_id);
+
+static struct i2c_driver pcf8574_kp_driver = {
+	.driver = {
+		.name = DRV_NAME,
+	},
+	.probe    = pcf8574_kp_probe,
+	.remove   = __devexit_p(pcf8574_kp_remove),
+	.suspend  = pcf8574_kp_suspend,
+	.resume   = pcf8574_kp_resume,
+	.id_table = pcf8574_kp_id,
+};
+
+static int __init pcf8574_kp_init(void)
+{
+	return i2c_add_driver(&pcf8574_kp_driver);
+}
+
+static void __exit pcf8574_kp_exit(void)
+{
+	i2c_del_driver(&pcf8574_kp_driver);
+}
+
+module_init(pcf8574_kp_init);
+module_exit(pcf8574_kp_exit);
-- 
1.7.0.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] input/misc: PCF8574 I2C keypad input device driver
  2010-03-09 15:01 [PATCH] input/misc: PCF8574 I2C keypad input device driver Mike Frysinger
@ 2010-03-09 18:09 ` Dmitry Torokhov
  2010-03-09 21:01   ` [PATCH v2] " Mike Frysinger
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2010-03-09 18:09 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: linux-input, uclinux-dist-devel, Bryan Wu

Hi Mike,

On Tue, Mar 09, 2010 at 10:01:00AM -0500, Mike Frysinger wrote:
> From: Bryan Wu <cooloney@kernel.org>
> 
> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  drivers/input/misc/Kconfig          |   10 ++
>  drivers/input/misc/Makefile         |    1 +
>  drivers/input/misc/pcf8574_keypad.c |  278 +++++++++++++++++++++++++++++++++++
>  3 files changed, 289 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/misc/pcf8574_keypad.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 49a31bd..fa97c58 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -396,4 +396,14 @@ config INPUT_ADXL34X_SPI
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called adxl34x-spi.
>  
> +config INPUT_PCF8574
> +	tristate "PCF8574 Keypad input device"
> +	depends on I2C && EXPERIMENTAL
> +	help
> +	  Say Y here if you want to support a keypad connetced via I2C
> +	  with a PCF8574.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called pcf8574_keypad.
> +
>  endif
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 775db74..8107c57 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_INPUT_AD714X)		+= ad714x.o
>  obj-$(CONFIG_INPUT_AD714X_I2C)		+= ad714x-i2c.o
>  obj-$(CONFIG_INPUT_AD714X_SPI)		+= ad714x-spi.o
>  obj-$(CONFIG_INPUT_APANEL)		+= apanel.o
> +obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
>  obj-$(CONFIG_INPUT_ATI_REMOTE)		+= ati_remote.o
>  obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
>  obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
> diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
> new file mode 100644
> index 0000000..85bd87c
> --- /dev/null
> +++ b/drivers/input/misc/pcf8574_keypad.c
> @@ -0,0 +1,278 @@
> +/*
> + * Driver for a keypad w/16 buttons connected to a PCF8574 I2C I/O expander
> + *
> + * Copyright 2005-2008 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/workqueue.h>
> +
> +#define DRV_NAME "pcf8574_keypad"
> +
> +MODULE_AUTHOR("Michael Hennerich");
> +MODULE_DESCRIPTION("Keypad input driver for 16 keys connected to PCF8574");
> +MODULE_LICENSE("GPL");
> +
> +static unsigned char pcf8574_kp_btncode[] = {
> +	[0] = KEY_RESERVED,
> +	[1] = KEY_ENTER,
> +	[2] = KEY_BACKSLASH,
> +	[3] = KEY_0,
> +	[4] = KEY_RIGHTBRACE,
> +	[5] = KEY_C,
> +	[6] = KEY_9,
> +	[7] = KEY_8,
> +	[8] = KEY_7,
> +	[9] = KEY_B,
> +	[10] = KEY_6,
> +	[11] = KEY_5,
> +	[12] = KEY_4,
> +	[13] = KEY_A,
> +	[14] = KEY_3,
> +	[15] = KEY_2,
> +	[16] = KEY_1
> +};
> +
> +struct kp_data {
> +	unsigned char *btncode;

Make a copy of initial table so you change per-device keymap (if ever
there is more than one). Theinitial map should be 'const'.

> +	struct input_dev *idev;
> +	struct i2c_client *client;
> +	char name[64];
> +	char phys[32];
> +	unsigned char laststate;
> +	unsigned char statechanged;
> +	unsigned long irq_handled;

Unused.

> +	unsigned long events_sended;

"events_sent"

> +	unsigned long events_processed;

Unused.

> +	struct work_struct pcf8574_kp_work;
> +};
> +
> +static short read_state(struct kp_data *lp)
> +{
> +	unsigned char x, y, a, b;
> +
> +	if (lp->client) {
> +		i2c_smbus_write_byte(lp->client, 240);
> +		x = 0xF & (~(i2c_smbus_read_byte(lp->client) >> 4));
> +
> +		i2c_smbus_write_byte(lp->client, 15);
> +		y = 0xF & (~i2c_smbus_read_byte(lp->client));
> +
> +		for (a = 0; x > 0; a++)
> +			x = x >> 1;
> +		for (b = 0; y > 0; b++)
> +			y = y >> 1;
> +
> +		return ((a - 1) * 4) + b;
> +	}
> +
> +	return -1;
> +}
> +
> +static void check_and_notify(struct work_struct *work)
> +{
> +	struct kp_data *lp =
> +	    container_of(work, struct kp_data, pcf8574_kp_work);
> +	unsigned char nextstate = read_state(lp);
> +
> +	lp->statechanged = lp->laststate ^ nextstate;
> +
> +	if (lp->statechanged) {
> +		input_report_key(lp->idev,
> +				 nextstate > 17 ? lp->btncode[lp->laststate] :
> +				 lp->btncode[nextstate],
> +				 nextstate > 17 ? 0 : 1);
> +
> +		lp->events_sended++;
> +	}
> +	lp->laststate = nextstate;
> +	input_sync(lp->idev);
> +	enable_irq(lp->client->irq);
> +}
> +
> +static irqreturn_t pcf8574_kp_irq_handler(int irq, void *dev_id)
> +{
> +	struct kp_data *lp = dev_id;
> +
> +	disable_irq_nosync(lp->client->irq);
> +	schedule_work(&lp->pcf8574_kp_work);

This screams "threaded IRQ".

> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int __devinit pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> +	int i, ret;
> +	struct input_dev *idev;
> +	struct kp_data *lp;
> +
> +	if (i2c_smbus_write_byte(client, 240) < 0) {
> +		dev_err(&client->dev, "probe: write fail\n");
> +		return -ENODEV;
> +	}
> +
> +	lp = kzalloc(sizeof(struct kp_data), GFP_KERNEL);
> +	if (!lp)
> +		return -ENOMEM;
> +	lp->client = client;
> +
> +	i2c_set_clientdata(client, lp);

I prefer setting client data at the end (or you need to clear it in
error path).

> +
> +	if (client->irq > 0) {
> +		ret = request_irq(client->irq, pcf8574_kp_irq_handler,
> +			IRQF_TRIGGER_LOW, DRV_NAME, lp);
> +		if (ret) {
> +			dev_err(&client->dev, "IRQ %d is not free\n", client->irq);
> +			goto fail_irq;
> +		}
> +	} else
> +		dev_warn(&client->dev, "IRQ not configured!\n");

It won't work without so just error out. And earlier, before you even do
allocations.

Also, you need to have input device at least allocated before
reqistering IRQ, otherwise you may "boom".

> +
> +
> +	idev = input_allocate_device();
> +	if (!idev) {
> +		dev_err(&client->dev, "Can't allocate input device\n");
> +		ret = -ENOMEM;
> +		goto fail_allocate;
> +	}
> +
> +	lp->idev = idev;
> +	lp->btncode = pcf8574_kp_btncode;
> +
> +	idev->evbit[0] = 0;
> +
> +	idev->evbit[0] |= BIT_MASK(EV_KEY);
> +	idev->keycode = lp->btncode;
> +	idev->keycodesize = sizeof(pcf8574_kp_btncode);
> +	idev->keycodemax = ARRAY_SIZE(pcf8574_kp_btncode);
> +
> +	for (i = 0; i < ARRAY_SIZE(pcf8574_kp_btncode); ++i)
> +		__set_bit(lp->btncode[i] & KEY_MAX, idev->keybit);
> +
> +	__clear_bit(KEY_RESERVED, idev->keybit);
> +
> +	sprintf(lp->name, DRV_NAME);
> +	sprintf(lp->phys, "kp_data/input0");
> +
> +	idev->name = lp->name;
> +	idev->phys = lp->phys;
> +	idev->id.bustype = BUS_I2C;
> +	idev->id.vendor = 0x0001;
> +	idev->id.product = 0x0001;
> +	idev->id.version = 0x0100;
> +
> +	input_set_drvdata(idev, lp);
> +
> +	ret = input_register_device(lp->idev);
> +	if (ret) {
> +		dev_err(&client->dev, "input_register_device() failed\n");
> +		goto fail_register;
> +	}
> +
> +	lp->statechanged = 0x0;
> +
> +	lp->laststate = read_state(lp);
> +
> +	/* Set up our workqueue. */
> +	INIT_WORK(&lp->pcf8574_kp_work, check_and_notify);

Too late (and not needed at all if you use theraded IRQ).

> +
> +	dev_info(&client->dev, "input: %s at %s IRQ %d\n", lp->name, lp->phys,
> +		client->irq);

We are noisy enough, input core will also print about the device.

> +
> +	return 0;
> +
> + fail_register:
> +	input_set_drvdata(idev, NULL);
> +	input_free_device(idev);
> + fail_allocate:
> +	free_irq(client->irq, lp);
> + fail_irq:
> +	i2c_set_clientdata(client, NULL);
> +	kfree(lp);
> +
> +	return ret;
> +}
> +
> +static int __devexit pcf8574_kp_remove(struct i2c_client *client)
> +{
> +	struct kp_data *lp = i2c_get_clientdata(client);
> +
> +	if (client->irq > 0)
> +		free_irq(client->irq, lp);
> +
> +	input_set_drvdata(lp->idev, NULL);
> +	input_unregister_device(lp->idev);
> +	kfree(lp);
> +
> +	i2c_set_clientdata(client, NULL);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int pcf8574_kp_resume(struct i2c_client *client)
> +{
> +	struct kp_data *lp = i2c_get_clientdata(client);
> +	int ret;
> +
> +	if (client->irq > 0) {
> +		ret = request_irq(client->irq, pcf8574_kp_irq_handler,
> +			IRQF_TRIGGER_LOW, DRV_NAME, lp);
> +		if (ret) {
> +			dev_err(&client->dev, "IRQ %d is not free\n", client->irq);
> +			return -ENODEV;
> +		}
> +	}

I think you can do disable_irq() instead of release/request.

> +
> +	return 0;
> +}
> +
> +static int pcf8574_kp_suspend(struct i2c_client *client, pm_message_t mesg)
> +{
> +	struct kp_data *lp = i2c_get_clientdata(client);
> +
> +	if (client->irq > 0)
> +		free_irq(client->irq, lp);

Why irq check?


> +
> +	return 0;
> +}
> +#else
> +# define pcf8574_kp_resume  NULL
> +# define pcf8574_kp_suspend NULL
> +#endif
> +
> +static const struct i2c_device_id pcf8574_kp_id[] = {
> +	{ DRV_NAME, 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, pcf8574_kp_id);
> +
> +static struct i2c_driver pcf8574_kp_driver = {
> +	.driver = {
> +		.name = DRV_NAME,

	.owner = THIS_MODULE,

> +	},
> +	.probe    = pcf8574_kp_probe,
> +	.remove   = __devexit_p(pcf8574_kp_remove),
> +	.suspend  = pcf8574_kp_suspend,
> +	.resume   = pcf8574_kp_resume,
> +	.id_table = pcf8574_kp_id,
> +};
> +
> +static int __init pcf8574_kp_init(void)
> +{
> +	return i2c_add_driver(&pcf8574_kp_driver);
> +}
> +
> +static void __exit pcf8574_kp_exit(void)
> +{
> +	i2c_del_driver(&pcf8574_kp_driver);
> +}
> +
> +module_init(pcf8574_kp_init);
> +module_exit(pcf8574_kp_exit);
> -- 
> 1.7.0.2
> 

-- 
Dmitry

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] input/misc: PCF8574 I2C keypad input device driver
  2010-03-09 18:09 ` Dmitry Torokhov
@ 2010-03-09 21:01   ` Mike Frysinger
  2010-03-09 21:21     ` Dmitry Torokhov
  0 siblings, 1 reply; 7+ messages in thread
From: Mike Frysinger @ 2010-03-09 21:01 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov; +Cc: uclinux-dist-devel, Bryan Wu

From: Bryan Wu <cooloney@kernel.org>

Signed-off-by: Bryan Wu <cooloney@kernel.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- process all feedback from Dmitry except for threaded irqs; that'll have to be done later

 drivers/input/misc/Kconfig          |   10 ++
 drivers/input/misc/Makefile         |    1 +
 drivers/input/misc/pcf8574_keypad.c |  251 +++++++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/misc/pcf8574_keypad.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 49a31bd..fa97c58 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -396,4 +396,14 @@ config INPUT_ADXL34X_SPI
 	  To compile this driver as a module, choose M here: the
 	  module will be called adxl34x-spi.
 
+config INPUT_PCF8574
+	tristate "PCF8574 Keypad input device"
+	depends on I2C && EXPERIMENTAL
+	help
+	  Say Y here if you want to support a keypad connetced via I2C
+	  with a PCF8574.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called pcf8574_keypad.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 775db74..8107c57 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_INPUT_AD714X)		+= ad714x.o
 obj-$(CONFIG_INPUT_AD714X_I2C)		+= ad714x-i2c.o
 obj-$(CONFIG_INPUT_AD714X_SPI)		+= ad714x-spi.o
 obj-$(CONFIG_INPUT_APANEL)		+= apanel.o
+obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
 obj-$(CONFIG_INPUT_ATI_REMOTE)		+= ati_remote.o
 obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
 obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
new file mode 100644
index 0000000..8b52a7e
--- /dev/null
+++ b/drivers/input/misc/pcf8574_keypad.c
@@ -0,0 +1,251 @@
+/*
+ * Driver for a keypad w/16 buttons connected to a PCF8574 I2C I/O expander
+ *
+ * Copyright 2005-2008 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/workqueue.h>
+
+#define DRV_NAME "pcf8574_keypad"
+
+static const unsigned char pcf8574_kp_btncode[] = {
+	[0] = KEY_RESERVED,
+	[1] = KEY_ENTER,
+	[2] = KEY_BACKSLASH,
+	[3] = KEY_0,
+	[4] = KEY_RIGHTBRACE,
+	[5] = KEY_C,
+	[6] = KEY_9,
+	[7] = KEY_8,
+	[8] = KEY_7,
+	[9] = KEY_B,
+	[10] = KEY_6,
+	[11] = KEY_5,
+	[12] = KEY_4,
+	[13] = KEY_A,
+	[14] = KEY_3,
+	[15] = KEY_2,
+	[16] = KEY_1
+};
+
+struct kp_data {
+	unsigned char btncode[17];
+	struct input_dev *idev;
+	struct i2c_client *client;
+	char name[64];
+	char phys[32];
+	unsigned char laststate;
+	unsigned char statechanged;
+	unsigned long events_sent;
+	struct work_struct pcf8574_kp_work;
+};
+
+static short read_state(struct kp_data *lp)
+{
+	unsigned char x, y, a, b;
+
+	if (lp->client) {
+		i2c_smbus_write_byte(lp->client, 240);
+		x = 0xF & (~(i2c_smbus_read_byte(lp->client) >> 4));
+
+		i2c_smbus_write_byte(lp->client, 15);
+		y = 0xF & (~i2c_smbus_read_byte(lp->client));
+
+		for (a = 0; x > 0; a++)
+			x = x >> 1;
+		for (b = 0; y > 0; b++)
+			y = y >> 1;
+
+		return ((a - 1) * 4) + b;
+	}
+
+	return -1;
+}
+
+static void check_and_notify(struct work_struct *work)
+{
+	struct kp_data *lp =
+	    container_of(work, struct kp_data, pcf8574_kp_work);
+	unsigned char nextstate = read_state(lp);
+
+	lp->statechanged = lp->laststate ^ nextstate;
+
+	if (lp->statechanged)
+		input_report_key(lp->idev,
+				 nextstate > 17 ? lp->btncode[lp->laststate] :
+				 lp->btncode[nextstate],
+				 nextstate > 17 ? 0 : 1);
+	lp->laststate = nextstate;
+
+	input_sync(lp->idev);
+	enable_irq(lp->client->irq);
+}
+
+static irqreturn_t pcf8574_kp_irq_handler(int irq, void *dev_id)
+{
+	struct kp_data *lp = dev_id;
+
+	disable_irq_nosync(lp->client->irq);
+	schedule_work(&lp->pcf8574_kp_work);
+
+	return IRQ_HANDLED;
+}
+
+static int __devinit pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	int i, ret;
+	struct input_dev *idev;
+	struct kp_data *lp;
+
+	if (i2c_smbus_write_byte(client, 240) < 0) {
+		dev_err(&client->dev, "probe: write fail\n");
+		return -ENODEV;
+	}
+
+	lp = kzalloc(sizeof(*lp), GFP_KERNEL);
+	if (!lp)
+		return -ENOMEM;
+	lp->client = client;
+
+	idev = input_allocate_device();
+	if (!idev) {
+		dev_err(&client->dev, "Can't allocate input device\n");
+		ret = -ENOMEM;
+		goto fail_allocate;
+	}
+
+	lp->idev = idev;
+	memcpy(lp->btncode, pcf8574_kp_btncode, sizeof(pcf8574_kp_btncode));
+
+	idev->evbit[0] = 0;
+
+	idev->evbit[0] |= BIT_MASK(EV_KEY);
+	idev->keycode = lp->btncode;
+	idev->keycodesize = sizeof(pcf8574_kp_btncode);
+	idev->keycodemax = ARRAY_SIZE(pcf8574_kp_btncode);
+
+	for (i = 0; i < ARRAY_SIZE(pcf8574_kp_btncode); ++i)
+		__set_bit(lp->btncode[i] & KEY_MAX, idev->keybit);
+
+	__clear_bit(KEY_RESERVED, idev->keybit);
+
+	sprintf(lp->name, DRV_NAME);
+	sprintf(lp->phys, "kp_data/input0");
+
+	idev->name = lp->name;
+	idev->phys = lp->phys;
+	idev->id.bustype = BUS_I2C;
+	idev->id.vendor = 0x0001;
+	idev->id.product = 0x0001;
+	idev->id.version = 0x0100;
+
+	input_set_drvdata(idev, lp);
+
+	ret = input_register_device(idev);
+	if (ret) {
+		dev_err(&client->dev, "input_register_device() failed\n");
+		goto fail_register;
+	}
+
+	lp->statechanged = 0x0;
+
+	lp->laststate = read_state(lp);
+
+	/* Set up our workqueue. */
+	INIT_WORK(&lp->pcf8574_kp_work, check_and_notify);
+
+	i2c_set_clientdata(client, lp);
+
+	ret = request_irq(client->irq, pcf8574_kp_irq_handler,
+		IRQF_TRIGGER_LOW, DRV_NAME, lp);
+	if (ret) {
+		dev_err(&client->dev, "IRQ %d is not free\n", client->irq);
+		goto fail_irq;
+	}
+
+	return 0;
+
+ fail_irq:
+	input_unregister_device(idev);
+ fail_register:
+	input_set_drvdata(idev, NULL);
+	input_free_device(idev);
+ fail_allocate:
+	kfree(lp);
+
+	return ret;
+}
+
+static int __devexit pcf8574_kp_remove(struct i2c_client *client)
+{
+	struct kp_data *lp = i2c_get_clientdata(client);
+
+	if (client->irq > 0)
+		free_irq(client->irq, lp);
+
+	input_set_drvdata(lp->idev, NULL);
+	input_unregister_device(lp->idev);
+	kfree(lp);
+
+	i2c_set_clientdata(client, NULL);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int pcf8574_kp_resume(struct i2c_client *client)
+{
+	enable_irq(client->irq);
+	return 0;
+}
+
+static int pcf8574_kp_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+	disable_irq(client->irq);
+	return 0;
+}
+#else
+# define pcf8574_kp_resume  NULL
+# define pcf8574_kp_suspend NULL
+#endif
+
+static const struct i2c_device_id pcf8574_kp_id[] = {
+	{ DRV_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pcf8574_kp_id);
+
+static struct i2c_driver pcf8574_kp_driver = {
+	.driver = {
+		.name  = DRV_NAME,
+		.owner = THIS_MODULE
+	},
+	.probe    = pcf8574_kp_probe,
+	.remove   = __devexit_p(pcf8574_kp_remove),
+	.suspend  = pcf8574_kp_suspend,
+	.resume   = pcf8574_kp_resume,
+	.id_table = pcf8574_kp_id,
+};
+
+static int __init pcf8574_kp_init(void)
+{
+	return i2c_add_driver(&pcf8574_kp_driver);
+}
+module_init(pcf8574_kp_init);
+
+static void __exit pcf8574_kp_exit(void)
+{
+	i2c_del_driver(&pcf8574_kp_driver);
+}
+module_exit(pcf8574_kp_exit);
+
+MODULE_AUTHOR("Michael Hennerich");
+MODULE_DESCRIPTION("Keypad input driver for 16 keys connected to PCF8574");
+MODULE_LICENSE("GPL");
-- 
1.7.0.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] input/misc: PCF8574 I2C keypad input device driver
  2010-03-09 21:01   ` [PATCH v2] " Mike Frysinger
@ 2010-03-09 21:21     ` Dmitry Torokhov
  2010-03-09 21:27       ` [Uclinux-dist-devel] " Mike Frysinger
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2010-03-09 21:21 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: linux-input, uclinux-dist-devel, Bryan Wu

On Tue, Mar 09, 2010 at 04:01:53PM -0500, Mike Frysinger wrote:
> From: Bryan Wu <cooloney@kernel.org>
> 
> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v2
> 	- process all feedback from Dmitry except for threaded irqs; that'll have to be done later

Could I ask why? As it stands the remove() is racy WRT to a work
running. But if you simply add cancel_work_sync() you'll risk leaving
unbalanced enable_irq()/disable_irq().

A few more comments I missed first time around:

> 
>  drivers/input/misc/Kconfig          |   10 ++
>  drivers/input/misc/Makefile         |    1 +
>  drivers/input/misc/pcf8574_keypad.c |  251 +++++++++++++++++++++++++++++++++++
>  3 files changed, 262 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/misc/pcf8574_keypad.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 49a31bd..fa97c58 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -396,4 +396,14 @@ config INPUT_ADXL34X_SPI
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called adxl34x-spi.
>  
> +config INPUT_PCF8574
> +	tristate "PCF8574 Keypad input device"
> +	depends on I2C && EXPERIMENTAL
> +	help
> +	  Say Y here if you want to support a keypad connetced via I2C
> +	  with a PCF8574.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called pcf8574_keypad.
> +
>  endif
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 775db74..8107c57 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_INPUT_AD714X)		+= ad714x.o
>  obj-$(CONFIG_INPUT_AD714X_I2C)		+= ad714x-i2c.o
>  obj-$(CONFIG_INPUT_AD714X_SPI)		+= ad714x-spi.o
>  obj-$(CONFIG_INPUT_APANEL)		+= apanel.o
> +obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
>  obj-$(CONFIG_INPUT_ATI_REMOTE)		+= ati_remote.o
>  obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
>  obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
> diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
> new file mode 100644
> index 0000000..8b52a7e
> --- /dev/null
> +++ b/drivers/input/misc/pcf8574_keypad.c
> @@ -0,0 +1,251 @@
> +/*
> + * Driver for a keypad w/16 buttons connected to a PCF8574 I2C I/O expander
> + *
> + * Copyright 2005-2008 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/workqueue.h>
> +
> +#define DRV_NAME "pcf8574_keypad"
> +
> +static const unsigned char pcf8574_kp_btncode[] = {
> +	[0] = KEY_RESERVED,
> +	[1] = KEY_ENTER,
> +	[2] = KEY_BACKSLASH,
> +	[3] = KEY_0,
> +	[4] = KEY_RIGHTBRACE,
> +	[5] = KEY_C,
> +	[6] = KEY_9,
> +	[7] = KEY_8,
> +	[8] = KEY_7,
> +	[9] = KEY_B,
> +	[10] = KEY_6,
> +	[11] = KEY_5,
> +	[12] = KEY_4,
> +	[13] = KEY_A,
> +	[14] = KEY_3,
> +	[15] = KEY_2,
> +	[16] = KEY_1
> +};
> +
> +struct kp_data {
> +	unsigned char btncode[17];

Unsigned short here. Also ARRAY_SIZE(pcf8574_kp_btncode).

> +	struct input_dev *idev;
> +	struct i2c_client *client;
> +	char name[64];
> +	char phys[32];
> +	unsigned char laststate;
> +	unsigned char statechanged;
> +	unsigned long events_sent;
> +	struct work_struct pcf8574_kp_work;
> +};
> +
> +static short read_state(struct kp_data *lp)
> +{
> +	unsigned char x, y, a, b;
> +
> +	if (lp->client) {

No need for this check - I do not see how we can end up with lp->client
being NULL here.

> +		i2c_smbus_write_byte(lp->client, 240);
> +		x = 0xF & (~(i2c_smbus_read_byte(lp->client) >> 4));
> +
> +		i2c_smbus_write_byte(lp->client, 15);
> +		y = 0xF & (~i2c_smbus_read_byte(lp->client));
> +
> +		for (a = 0; x > 0; a++)
> +			x = x >> 1;
> +		for (b = 0; y > 0; b++)
> +			y = y >> 1;
> +
> +		return ((a - 1) * 4) + b;
> +	}
> +
> +	return -1;
> +}
> +
> +static void check_and_notify(struct work_struct *work)
> +{
> +	struct kp_data *lp =
> +	    container_of(work, struct kp_data, pcf8574_kp_work);
> +	unsigned char nextstate = read_state(lp);
> +
> +	lp->statechanged = lp->laststate ^ nextstate;
> +
> +	if (lp->statechanged)

Hmm, why is it in lp? Local variable (or even lp->laststate ^ nextstate
expression) would work just fine.

> +		input_report_key(lp->idev,
> +				 nextstate > 17 ? lp->btncode[lp->laststate] :
> +				 lp->btncode[nextstate],
> +				 nextstate > 17 ? 0 : 1);
> +	lp->laststate = nextstate;
> +
> +	input_sync(lp->idev);
> +	enable_irq(lp->client->irq);
> +}
> +
> +static irqreturn_t pcf8574_kp_irq_handler(int irq, void *dev_id)
> +{
> +	struct kp_data *lp = dev_id;
> +
> +	disable_irq_nosync(lp->client->irq);
> +	schedule_work(&lp->pcf8574_kp_work);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int __devinit pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> +	int i, ret;
> +	struct input_dev *idev;
> +	struct kp_data *lp;
> +
> +	if (i2c_smbus_write_byte(client, 240) < 0) {
> +		dev_err(&client->dev, "probe: write fail\n");
> +		return -ENODEV;
> +	}
> +
> +	lp = kzalloc(sizeof(*lp), GFP_KERNEL);
> +	if (!lp)
> +		return -ENOMEM;
> +	lp->client = client;
> +
> +	idev = input_allocate_device();
> +	if (!idev) {
> +		dev_err(&client->dev, "Can't allocate input device\n");
> +		ret = -ENOMEM;
> +		goto fail_allocate;
> +	}
> +
> +	lp->idev = idev;
> +	memcpy(lp->btncode, pcf8574_kp_btncode, sizeof(pcf8574_kp_btncode));
> +
> +	idev->evbit[0] = 0;
> +
> +	idev->evbit[0] |= BIT_MASK(EV_KEY);
> +	idev->keycode = lp->btncode;
> +	idev->keycodesize = sizeof(pcf8574_kp_btncode);

Umm 17 bytes is a bit too much ;) sizeof(lp->btncode[0]); is more
appropriate.

> +	idev->keycodemax = ARRAY_SIZE(pcf8574_kp_btncode);
> +
> +	for (i = 0; i < ARRAY_SIZE(pcf8574_kp_btncode); ++i)
> +		__set_bit(lp->btncode[i] & KEY_MAX, idev->keybit);
> +
> +	__clear_bit(KEY_RESERVED, idev->keybit);
> +
> +	sprintf(lp->name, DRV_NAME);
> +	sprintf(lp->phys, "kp_data/input0");
> +
> +	idev->name = lp->name;
> +	idev->phys = lp->phys;
> +	idev->id.bustype = BUS_I2C;
> +	idev->id.vendor = 0x0001;
> +	idev->id.product = 0x0001;
> +	idev->id.version = 0x0100;
> +
> +	input_set_drvdata(idev, lp);
> +
> +	ret = input_register_device(idev);
> +	if (ret) {
> +		dev_err(&client->dev, "input_register_device() failed\n");
> +		goto fail_register;
> +	}
> +
> +	lp->statechanged = 0x0;
> +
> +	lp->laststate = read_state(lp);
> +
> +	/* Set up our workqueue. */
> +	INIT_WORK(&lp->pcf8574_kp_work, check_and_notify);
> +
> +	i2c_set_clientdata(client, lp);
> +
> +	ret = request_irq(client->irq, pcf8574_kp_irq_handler,
> +		IRQF_TRIGGER_LOW, DRV_NAME, lp);
> +	if (ret) {
> +		dev_err(&client->dev, "IRQ %d is not free\n", client->irq);
> +		goto fail_irq;
> +	}
> +
> +	return 0;
> +
> + fail_irq:
> +	input_unregister_device(idev);

You need "idev = NULL;" here.

> + fail_register:
> +	input_set_drvdata(idev, NULL);
> +	input_free_device(idev);
> + fail_allocate:
> +	kfree(lp);
> +
> +	return ret;
> +}
> +
> +static int __devexit pcf8574_kp_remove(struct i2c_client *client)
> +{
> +	struct kp_data *lp = i2c_get_clientdata(client);
> +
> +	if (client->irq > 0)

No need for this check.

> +		free_irq(client->irq, lp);
> +
> +	input_set_drvdata(lp->idev, NULL);
> +	input_unregister_device(lp->idev);
> +	kfree(lp);
> +
> +	i2c_set_clientdata(client, NULL);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int pcf8574_kp_resume(struct i2c_client *client)
> +{
> +	enable_irq(client->irq);
> +	return 0;
> +}
> +
> +static int pcf8574_kp_suspend(struct i2c_client *client, pm_message_t mesg)
> +{
> +	disable_irq(client->irq);
> +	return 0;
> +}
> +#else
> +# define pcf8574_kp_resume  NULL
> +# define pcf8574_kp_suspend NULL
> +#endif
> +
> +static const struct i2c_device_id pcf8574_kp_id[] = {
> +	{ DRV_NAME, 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, pcf8574_kp_id);
> +
> +static struct i2c_driver pcf8574_kp_driver = {
> +	.driver = {
> +		.name  = DRV_NAME,
> +		.owner = THIS_MODULE
> +	},
> +	.probe    = pcf8574_kp_probe,
> +	.remove   = __devexit_p(pcf8574_kp_remove),
> +	.suspend  = pcf8574_kp_suspend,
> +	.resume   = pcf8574_kp_resume,
> +	.id_table = pcf8574_kp_id,
> +};
> +
> +static int __init pcf8574_kp_init(void)
> +{
> +	return i2c_add_driver(&pcf8574_kp_driver);
> +}
> +module_init(pcf8574_kp_init);
> +
> +static void __exit pcf8574_kp_exit(void)
> +{
> +	i2c_del_driver(&pcf8574_kp_driver);
> +}
> +module_exit(pcf8574_kp_exit);
> +
> +MODULE_AUTHOR("Michael Hennerich");
> +MODULE_DESCRIPTION("Keypad input driver for 16 keys connected to PCF8574");
> +MODULE_LICENSE("GPL");
> -- 
> 1.7.0.2
> 

-- 
Dmitry

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Uclinux-dist-devel] [PATCH v2] input/misc: PCF8574 I2C keypad input device driver
  2010-03-09 21:21     ` Dmitry Torokhov
@ 2010-03-09 21:27       ` Mike Frysinger
  2010-03-09 22:30         ` Dmitry Torokhov
  0 siblings, 1 reply; 7+ messages in thread
From: Mike Frysinger @ 2010-03-09 21:27 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: uclinux-dist-devel, linux-input

On Tue, Mar 9, 2010 at 16:21, Dmitry Torokhov wrote:
> On Tue, Mar 09, 2010 at 04:01:53PM -0500, Mike Frysinger wrote:
>> From: Bryan Wu <cooloney@kernel.org>
>>
>> Signed-off-by: Bryan Wu <cooloney@kernel.org>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>> v2
>>       - process all feedback from Dmitry except for threaded irqs; that'll have to be done later
>
> Could I ask why? As it stands the remove() is racy WRT to a work
> running. But if you simply add cancel_work_sync() you'll risk leaving
> unbalanced enable_irq()/disable_irq().

ive never worked with threaded irq handlers before.  i'll have to read
about them before i can do the work.  i'm not suggesting you take the
driver now as is ... having it on a list in case someone else wants a
mostly-ok driver.
-mike
--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Uclinux-dist-devel] [PATCH v2] input/misc: PCF8574 I2C keypad input device driver
  2010-03-09 21:27       ` [Uclinux-dist-devel] " Mike Frysinger
@ 2010-03-09 22:30         ` Dmitry Torokhov
  2010-03-10  4:36           ` Dmitry Torokhov
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2010-03-09 22:30 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: uclinux-dist-devel, linux-input

On Tue, Mar 09, 2010 at 04:27:48PM -0500, Mike Frysinger wrote:
> On Tue, Mar 9, 2010 at 16:21, Dmitry Torokhov wrote:
> > On Tue, Mar 09, 2010 at 04:01:53PM -0500, Mike Frysinger wrote:
> >> From: Bryan Wu <cooloney@kernel.org>
> >>
> >> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> >> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> >> ---
> >> v2
> >>       - process all feedback from Dmitry except for threaded irqs; that'll have to be done later
> >
> > Could I ask why? As it stands the remove() is racy WRT to a work
> > running. But if you simply add cancel_work_sync() you'll risk leaving
> > unbalanced enable_irq()/disable_irq().
> 
> ive never worked with threaded irq handlers before.  i'll have to read
> about them before i can do the work.  i'm not suggesting you take the
> driver now as is ... having it on a list in case someone else wants a
> mostly-ok driver.

It is very simple, take a look at drivers/input/touchscreen/mcs5000_ts.c
Just make sure you request threaded irq with IRQF_ONESHOT.

-- 
Dmitry
--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Uclinux-dist-devel] [PATCH v2] input/misc: PCF8574 I2C keypad input device driver
  2010-03-09 22:30         ` Dmitry Torokhov
@ 2010-03-10  4:36           ` Dmitry Torokhov
  0 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2010-03-10  4:36 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: uclinux-dist-devel, linux-input

On Tue, Mar 09, 2010 at 02:30:39PM -0800, Dmitry Torokhov wrote:
> On Tue, Mar 09, 2010 at 04:27:48PM -0500, Mike Frysinger wrote:
> > On Tue, Mar 9, 2010 at 16:21, Dmitry Torokhov wrote:
> > > On Tue, Mar 09, 2010 at 04:01:53PM -0500, Mike Frysinger wrote:
> > >> From: Bryan Wu <cooloney@kernel.org>
> > >>
> > >> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> > >> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > >> ---
> > >> v2
> > >>       - process all feedback from Dmitry except for threaded irqs; that'll have to be done later
> > >
> > > Could I ask why? As it stands the remove() is racy WRT to a work
> > > running. But if you simply add cancel_work_sync() you'll risk leaving
> > > unbalanced enable_irq()/disable_irq().
> > 
> > ive never worked with threaded irq handlers before.  i'll have to read
> > about them before i can do the work.  i'm not suggesting you take the
> > driver now as is ... having it on a list in case someone else wants a
> > mostly-ok driver.
> 
> It is very simple, take a look at drivers/input/touchscreen/mcs5000_ts.c
> Just make sure you request threaded irq with IRQF_ONESHOT.
> 

Does the following still work by any chance?

-- 
Dmitry

Input: add PCF8574 I2C keypad input device driver

From: Bryan Wu <cooloney@kernel.org>

Signed-off-by: Bryan Wu <cooloney@kernel.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/misc/Kconfig          |   10 ++
 drivers/input/misc/Makefile         |    1 
 drivers/input/misc/pcf8574_keypad.c |  227 +++++++++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/misc/pcf8574_keypad.c


diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b2cd5a1..b8a96ef 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -354,4 +354,14 @@ config INPUT_PCAP
 	  To compile this driver as a module, choose M here: the
 	  module will be called pcap_keys.
 
+config INPUT_PCF8574
+	tristate "PCF8574 Keypad input device"
+	depends on I2C && EXPERIMENTAL
+	help
+	  Say Y here if you want to support a keypad connetced via I2C
+	  with a PCF8574.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called pcf8574_keypad.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index f620a09..5e05174 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -6,6 +6,7 @@
 
 obj-$(CONFIG_INPUT_88PM860X_ONKEY)	+= 88pm860x_onkey.o
 obj-$(CONFIG_INPUT_APANEL)		+= apanel.o
+obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
 obj-$(CONFIG_INPUT_ATI_REMOTE)		+= ati_remote.o
 obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
 obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
new file mode 100644
index 0000000..91e900b
--- /dev/null
+++ b/drivers/input/misc/pcf8574_keypad.c
@@ -0,0 +1,227 @@
+/*
+ * Driver for a keypad w/16 buttons connected to a PCF8574 I2C I/O expander
+ *
+ * Copyright 2005-2008 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/workqueue.h>
+
+#define DRV_NAME "pcf8574_keypad"
+
+static const unsigned char pcf8574_kp_btncode[] = {
+	[0] = KEY_RESERVED,
+	[1] = KEY_ENTER,
+	[2] = KEY_BACKSLASH,
+	[3] = KEY_0,
+	[4] = KEY_RIGHTBRACE,
+	[5] = KEY_C,
+	[6] = KEY_9,
+	[7] = KEY_8,
+	[8] = KEY_7,
+	[9] = KEY_B,
+	[10] = KEY_6,
+	[11] = KEY_5,
+	[12] = KEY_4,
+	[13] = KEY_A,
+	[14] = KEY_3,
+	[15] = KEY_2,
+	[16] = KEY_1
+};
+
+struct kp_data {
+	unsigned short btncode[ARRAY_SIZE(pcf8574_kp_btncode)];
+	struct input_dev *idev;
+	struct i2c_client *client;
+	char name[64];
+	char phys[32];
+	unsigned char laststate;
+};
+
+static short read_state(struct kp_data *lp)
+{
+	unsigned char x, y, a, b;
+
+	i2c_smbus_write_byte(lp->client, 240);
+	x = 0xF & (~(i2c_smbus_read_byte(lp->client) >> 4));
+
+	i2c_smbus_write_byte(lp->client, 15);
+	y = 0xF & (~i2c_smbus_read_byte(lp->client));
+
+	for (a = 0; x > 0; a++)
+		x = x >> 1;
+	for (b = 0; y > 0; b++)
+		y = y >> 1;
+
+	return ((a - 1) * 4) + b;
+}
+
+static irqreturn_t pcf8574_kp_irq_handler(int irq, void *dev_id)
+{
+	struct kp_data *lp = dev_id;
+	unsigned char nextstate = read_state(lp);
+
+	if (lp->laststate != nextstate) {
+		int key_down = nextstate <= ARRAY_SIZE(lp->btncode);
+		unsigned short keycode = key_down ?
+			lp->btncode[nextstate] : lp->btncode[lp->laststate];
+
+		input_report_key(lp->idev, keycode, key_down);
+		input_sync(lp->idev);
+
+		lp->laststate = nextstate;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int __devinit pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	int i, ret;
+	struct input_dev *idev;
+	struct kp_data *lp;
+
+	if (i2c_smbus_write_byte(client, 240) < 0) {
+		dev_err(&client->dev, "probe: write fail\n");
+		return -ENODEV;
+	}
+
+	lp = kzalloc(sizeof(*lp), GFP_KERNEL);
+	if (!lp)
+		return -ENOMEM;
+
+	idev = input_allocate_device();
+	if (!idev) {
+		dev_err(&client->dev, "Can't allocate input device\n");
+		ret = -ENOMEM;
+		goto fail_allocate;
+	}
+
+	lp->idev = idev;
+	lp->client = client;
+
+	idev->evbit[0] = BIT_MASK(EV_KEY);
+	idev->keycode = lp->btncode;
+	idev->keycodesize = sizeof(pcf8574_kp_btncode);
+	idev->keycodemax = ARRAY_SIZE(pcf8574_kp_btncode);
+
+	for (i = 0; i < ARRAY_SIZE(pcf8574_kp_btncode); i++) {
+		lp->btncode[i] = pcf8574_kp_btncode[i];
+		__set_bit(lp->btncode[i] & KEY_MAX, idev->keybit);
+	}
+	__clear_bit(KEY_RESERVED, idev->keybit);
+
+	sprintf(lp->name, DRV_NAME);
+	sprintf(lp->phys, "kp_data/input0");
+
+	idev->name = lp->name;
+	idev->phys = lp->phys;
+	idev->id.bustype = BUS_I2C;
+	idev->id.vendor = 0x0001;
+	idev->id.product = 0x0001;
+	idev->id.version = 0x0100;
+
+	input_set_drvdata(idev, lp);
+
+	ret = input_register_device(idev);
+	if (ret) {
+		dev_err(&client->dev, "input_register_device() failed\n");
+		goto fail_register;
+	}
+
+	lp->laststate = read_state(lp);
+
+	ret = request_threaded_irq(client->irq, NULL, pcf8574_kp_irq_handler,
+				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+				   DRV_NAME, lp);
+	if (ret) {
+		dev_err(&client->dev, "IRQ %d is not free\n", client->irq);
+		goto fail_irq;
+	}
+
+	i2c_set_clientdata(client, lp);
+	return 0;
+
+ fail_irq:
+	input_unregister_device(idev);
+ fail_register:
+	input_set_drvdata(idev, NULL);
+	input_free_device(idev);
+ fail_allocate:
+	kfree(lp);
+
+	return ret;
+}
+
+static int __devexit pcf8574_kp_remove(struct i2c_client *client)
+{
+	struct kp_data *lp = i2c_get_clientdata(client);
+
+	free_irq(client->irq, lp);
+
+	input_unregister_device(lp->idev);
+	kfree(lp);
+
+	i2c_set_clientdata(client, NULL);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int pcf8574_kp_resume(struct i2c_client *client)
+{
+	enable_irq(client->irq);
+
+	return 0;
+}
+
+static int pcf8574_kp_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+	disable_irq(client->irq);
+
+	return 0;
+}
+#else
+# define pcf8574_kp_resume  NULL
+# define pcf8574_kp_suspend NULL
+#endif
+
+static const struct i2c_device_id pcf8574_kp_id[] = {
+	{ DRV_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pcf8574_kp_id);
+
+static struct i2c_driver pcf8574_kp_driver = {
+	.driver = {
+		.name  = DRV_NAME,
+		.owner = THIS_MODULE,
+	},
+	.probe    = pcf8574_kp_probe,
+	.remove   = __devexit_p(pcf8574_kp_remove),
+	.suspend  = pcf8574_kp_suspend,
+	.resume   = pcf8574_kp_resume,
+	.id_table = pcf8574_kp_id,
+};
+
+static int __init pcf8574_kp_init(void)
+{
+	return i2c_add_driver(&pcf8574_kp_driver);
+}
+module_init(pcf8574_kp_init);
+
+static void __exit pcf8574_kp_exit(void)
+{
+	i2c_del_driver(&pcf8574_kp_driver);
+}
+module_exit(pcf8574_kp_exit);
+
+MODULE_AUTHOR("Michael Hennerich");
+MODULE_DESCRIPTION("Keypad input driver for 16 keys connected to PCF8574");
+MODULE_LICENSE("GPL");
--
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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-03-10  4:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-09 15:01 [PATCH] input/misc: PCF8574 I2C keypad input device driver Mike Frysinger
2010-03-09 18:09 ` Dmitry Torokhov
2010-03-09 21:01   ` [PATCH v2] " Mike Frysinger
2010-03-09 21:21     ` Dmitry Torokhov
2010-03-09 21:27       ` [Uclinux-dist-devel] " Mike Frysinger
2010-03-09 22:30         ` Dmitry Torokhov
2010-03-10  4:36           ` Dmitry Torokhov

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.