Hi Christian, On Thu, Jan 30, 2014 at 10:29:45AM +0100, Christian Gmeiner wrote: > This driver is quite simple and only supports the Touch > Reporting Protocol.
Pretty clean and neat, just a few comments. > > Signed-off-by: Christian Gmeiner <[email protected]> > --- > drivers/input/touchscreen/Kconfig | 12 ++ > drivers/input/touchscreen/Makefile | 1 + > drivers/input/touchscreen/ar1021_i2c.c | 201 > ++++++++++++++++++++++++++++++++ > 3 files changed, 214 insertions(+) > create mode 100644 drivers/input/touchscreen/ar1021_i2c.c > > diff --git a/drivers/input/touchscreen/Kconfig > b/drivers/input/touchscreen/Kconfig > index 07e9e82..15cc9a1 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -86,6 +86,18 @@ config TOUCHSCREEN_AD7879_SPI > To compile this driver as a module, choose M here: the > module will be called ad7879-spi. > > +config TOUCHSCREEN_AR1021_I2C > + tristate "Microchip AR1021 i2c touchscreen" > + depends on I2C && OF > + help > + Say Y here if you have the Microchip AR1021 touchscreen controller > + chip in your system. > + > + If unsure, say N. > + > + To compile this driver as a module, choose M here: the > + module will be called microchip_ar1021_i2c. s/microchip_ar1021_i2c/ar1021_i2c > + > config TOUCHSCREEN_ATMEL_MXT > tristate "Atmel mXT I2C Touchscreen" > depends on I2C > diff --git a/drivers/input/touchscreen/Makefile > b/drivers/input/touchscreen/Makefile > index 62801f2..efaa328 100644 > --- a/drivers/input/touchscreen/Makefile > +++ b/drivers/input/touchscreen/Makefile > @@ -12,6 +12,7 @@ obj-$(CONFIG_TOUCHSCREEN_AD7879) += ad7879.o > obj-$(CONFIG_TOUCHSCREEN_AD7879_I2C) += ad7879-i2c.o > obj-$(CONFIG_TOUCHSCREEN_AD7879_SPI) += ad7879-spi.o > obj-$(CONFIG_TOUCHSCREEN_ADS7846) += ads7846.o > +obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o > obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o > obj-$(CONFIG_TOUCHSCREEN_ATMEL_TSADCC) += atmel_tsadcc.o > obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o > diff --git a/drivers/input/touchscreen/ar1021_i2c.c > b/drivers/input/touchscreen/ar1021_i2c.c > new file mode 100644 > index 0000000..c77ce05 > --- /dev/null > +++ b/drivers/input/touchscreen/ar1021_i2c.c > @@ -0,0 +1,201 @@ > +/* > + * Microchip AR1021 driver for I2C > + * > + * Author: Christian Gmeiner <[email protected]> > + * > + * License: GPL as published by the FSF. > + */ > + > +#include <linux/module.h> > +#include <linux/input.h> > +#include <linux/of.h> > +#include <linux/i2c.h> > +#include <linux/slab.h> > +#include <linux/irq.h> > +#include <linux/interrupt.h> > +#include <linux/gpio.h> > +#include <asm/unaligned.h> > + > +#define AR1021_TOCUH_PKG_SIZE 5 > + > +struct ar1021_i2c { > + struct i2c_client *client; > + struct input_dev *input; > + u8 data[AR1021_TOCUH_PKG_SIZE]; > +}; > + > +static irqreturn_t ar1021_i2c_irq(int irq, void *dev_id) > +{ > + struct ar1021_i2c *ar1021 = dev_id; > + struct input_dev *input = ar1021->input; > + u8 *data = ar1021->data; > + unsigned int x, y, button; > + int error; > + > + error = i2c_master_recv(ar1021->client, > + ar1021->data, sizeof(ar1021->data)); > + if (error < 0) > + goto out; > + > + button = !(data[0] & BIT(0)); > + x = ((data[2] & 0x1f) << 7) | (data[1] & 0x7f); > + y = ((data[4] & 0x1f) << 7) | (data[3] & 0x7f); > + > + input_report_key(input, BTN_TOUCH, button); > + input_report_abs(input, ABS_X, x); > + input_report_abs(input, ABS_Y, y); > + input_sync(input); > + > +out: > + return IRQ_HANDLED; > +} > + > +static int ar1021_i2c_open(struct input_dev *dev) > +{ > + struct ar1021_i2c *wac_i2c = input_get_drvdata(dev); > + struct i2c_client *client = wac_i2c->client; > + > + enable_irq(client->irq); > + > + return 0; > +} > + > +static void ar1021_i2c_close(struct input_dev *dev) > +{ > + struct ar1021_i2c *wac_i2c = input_get_drvdata(dev); > + struct i2c_client *client = wac_i2c->client; > + > + disable_irq(client->irq); > +} > + > +static int ar1021_i2c_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct ar1021_i2c *ar1021; > + struct input_dev *input; > + int error; > + > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { > + dev_err(&client->dev, "i2c_check_functionality error\n"); > + return -EIO; > + } > + > + ar1021 = devm_kzalloc(client->dev, sizeof(*ar1021), GFP_KERNEL); > + input = input_allocate_device(); Use devm_input_allocate_device() and later devm_request_threaded_irq() as well. > + if (!ar1021 || !input) { > + error = -ENOMEM; > + goto err_free_mem; > + } > + > + ar1021->client = client; > + ar1021->input = input; > + > + input->name = "ar1021 I2C Touchscreen"; > + input->id.bustype = BUS_I2C; > + input->dev.parent = &client->dev; > + input->open = ar1021_i2c_open; > + input->close = ar1021_i2c_close; > + > + input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS); > + > + __set_bit(BTN_TOOL_PEN, input->keybit); > + > + input_set_abs_params(input, ABS_X, 0, 4095, 0, 0); > + input_set_abs_params(input, ABS_Y, 0, 4095, 0, 0); > + > + input_set_drvdata(input, ar1021); > + > + error = request_threaded_irq(client->irq, NULL, ar1021_i2c_irq, > + IRQF_TRIGGER_LOW | IRQF_ONESHOT, > + "ar1021_i2c", ar1021); > + if (error) { > + dev_err(&client->dev, > + "Failed to enable IRQ, error: %d\n", error); > + goto err_free_mem; > + } > + > + /* Disable the IRQ, we'll enable it in wac_i2c_open() */ No, not in wac_i2c_open ;) It looks like you might want to update copyright notice to mentioned what driver you used as a base... > + disable_irq(client->irq); > + > + error = input_register_device(ar1021->input); > + if (error) { > + dev_err(&client->dev, > + "Failed to register input device, error: %d\n", error); > + goto err_free_irq; > + } > + > + i2c_set_clientdata(client, ar1021); > + return 0; > + > +err_free_irq: > + free_irq(client->irq, ar1021); > +err_free_mem: > + input_free_device(input); > + > + return error; > +} > + > +static int ar1021_i2c_remove(struct i2c_client *client) > +{ > + struct ar1021_i2c *ar1021 = i2c_get_clientdata(client); > + > + free_irq(client->irq, ar1021); > + input_unregister_device(ar1021->input); > + > + return 0; > +} If you use devm throughout you won't need ar1021_i2c_remove method at all. > + > +#ifdef CONFIG_PM_SLEEP > +static int ar1021_i2c_suspend(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + > + disable_irq(client->irq); > + > + return 0; > +} > + > +static int ar1021_i2c_resume(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + > + enable_irq(client->irq); You do not want to enable IRQ if there are no users (nobody opened device). > + > + return 0; > +} > +#endif > + > +static SIMPLE_DEV_PM_OPS(ar1021_i2c_pm, ar1021_i2c_suspend, > ar1021_i2c_resume); > + > +static const struct i2c_device_id ar1021_i2c_id[] = { > + { "MICROCHIP_AR1021_I2C", 0 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(i2c, ar1021_i2c_id); > + > +#ifdef CONFIG_OF > +static struct of_device_id ar1021_i2c_of_match[] = { > + { .compatible = "mc,ar1021-i2c", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, ar1021_i2c_of_match); > +#endif > + > +static struct i2c_driver ar1021_i2c_driver = { > + .driver = { > + .name = "ar1021_i2c", > + .owner = THIS_MODULE, > + .pm = &ar1021_i2c_pm, > + .of_match_table = of_match_ptr(ar1021_i2c_of_match), > + }, > + > + .probe = ar1021_i2c_probe, > + .remove = ar1021_i2c_remove, > + .id_table = ar1021_i2c_id, > +}; > +module_i2c_driver(ar1021_i2c_driver); > + > +MODULE_AUTHOR("Christian Gmeiner <[email protected]>"); > +MODULE_DESCRIPTION("Microchip AR1021 I2C Driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:ar1021_i2c"); Why platform if it is I2C driver? This MODULE_ALIAS is not needed at all. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
