Hi Wayne, On Tue, Jul 27, 2010 at 09:49:05AM +0800, Wayne Lin wrote: > From: Mandeep Singh Baines <msb@xxxxxxxxxxxx> > > BUG=NONE > TEST=Built and ran on st1.5 Surely there can be a better changelog. > > Signed-off-by: Horace Fu <horace.fu@xxxxxxxxxxxx> > Signed-off-by: Mandeep Singh Baines <msb@xxxxxxxxxxxx> > Where is your signed-off-by? > Review URL: http://codereview.chromium.org/724001 > --- > drivers/input/mouse/Kconfig | 9 + > drivers/input/mouse/Makefile | 1 + > drivers/input/mouse/qci_touchpad.c | 308 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 318 insertions(+), 0 deletions(-) > create mode 100644 drivers/input/mouse/qci_touchpad.c > > diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig > index 3feeb3a..35aab2c 100644 > --- a/drivers/input/mouse/Kconfig > +++ b/drivers/input/mouse/Kconfig > @@ -321,4 +321,13 @@ config MOUSE_SYNAPTICS_I2C > To compile this driver as a module, choose M here: the > module will be called synaptics_i2c. > > +config MOUSE_QCITP > + tristate "Quanta Computer Inc. Touchpad" > + depends on I2C && ARCH_MSM_SCORPION > + default n > + help > + Say Y here if you want to use the Quanta touchpad driver for ST 1.5 > + platform. > + To compile this driver as a module... > + > endif > diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile > index 570c84a..57d5135 100644 > --- a/drivers/input/mouse/Makefile > +++ b/drivers/input/mouse/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_MOUSE_RISCPC) += rpcmouse.o > obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o > obj-$(CONFIG_MOUSE_SYNAPTICS_I2C) += synaptics_i2c.o > obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o > +obj-$(CONFIG_MOUSE_QCITP) += qci_touchpad.o > Please keep makefile (and Kconfig) sorted alphabetically. > psmouse-objs := psmouse-base.o synaptics.o > > diff --git a/drivers/input/mouse/qci_touchpad.c b/drivers/input/mouse/qci_touchpad.c > new file mode 100644 > index 0000000..1cfa0ee > --- /dev/null > +++ b/drivers/input/mouse/qci_touchpad.c > @@ -0,0 +1,308 @@ > +/* Quanta I2C Touchpad Driver > + * > + * Copyright (C) 2009 Quanta Computer Inc. > + * Author: Hsin Wu <hsin.wu@xxxxxxxxxxxx> > + * Author: Austin Lai <austin.lai@xxxxxxxxxxxx> > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * 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. > + * > + */ > + > + /* > + * > + * The Driver with I/O communications via the I2C Interface for ON2 of AP BU. > + * And it is only working on the nuvoTon WPCE775x Embedded Controller. > + * > + */ Bad formatting. > + > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/jiffies.h> > +#include <linux/i2c.h> > +#include <linux/mutex.h> > +#include <linux/interrupt.h> > +#include <linux/input.h> > +#include <linux/keyboard.h> > +#include <linux/gpio.h> > +#include <linux/delay.h> > + > +#define TOUCHPAD_ID_NAME "qci-i2cpad" > +#define TOUCHPAD_NAME "PS2 Touchpad" > +#define TOUCHPAD_DEVICE "/i2c/input1" > +#define TOUCHPAD_CMD_ENABLE 0xF4 > + > +static int __devinit qcitp_probe(struct i2c_client *client, > + const struct i2c_device_id *id); > +static int __devexit qcitp_remove(struct i2c_client *kbd); > + Instead fo forward declarators just move your driver structure declaration lower. > +/* General structure to hold the driver data */ > +struct i2ctpad_drv_data { > + struct i2c_client *ti2c_client; > + struct work_struct work; > + struct input_dev *qcitp_dev; > + struct kobject *tp_kobj; > + unsigned int qcitp_gpio; > + unsigned int qcitp_irq; > + char ecdata[8]; > +}; > + > +static int tp_sense_val = 10; > +static ssize_t tp_sensitive_show(struct kobject *kobj, > + struct kobj_attribute *attr, char * buf) > +{ > + return sprintf(buf, "%d\n", tp_sense_val); > +} > + > +static ssize_t tp_sensitive_store(struct kobject *kobj, > + struct kobj_attribute *attr, const char* buf, size_t n) > +{ > + unsigned int val = 0; > + sscanf(buf, "%d", &val); > + > + if (val >= 1 && val <= 10) > + tp_sense_val = val; > + else > + return -ENOSYS; > + > + return sizeof(buf); > +} > + > +static struct kobj_attribute tp_sensitivity = __ATTR(tp_sensitivity , > + 0644 , > + tp_sensitive_show , > + tp_sensitive_store); > + > +static struct attribute *g_tp[] = { > + &tp_sensitivity.attr, > + NULL, > +}; > + > +static struct attribute_group attr_group = { > + .attrs = g_tp, > +}; > + > +/*----------------------------------------------------------------------------- > + * Driver functions > + *---------------------------------------------------------------------------*/ > + > +#ifdef CONFIG_PM > +static int qcitp_suspend(struct device *dev) > +{ > + return 0; > +} > + > +static int qcitp_resume(struct device *dev) > +{ > + return 0; > +} No need for placeholders. > +#endif > + > +static const struct i2c_device_id qcitp_idtable[] = { > + { TOUCHPAD_ID_NAME, 0 }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(i2c, qcitp_idtable); > +#ifdef CONFIG_PM > +static struct dev_pm_ops qcitp_pm_ops = { > + .suspend = qcitp_suspend, > + .resume = qcitp_resume, > +}; > +#endif > +static struct i2c_driver i2ctp_driver = { > + .driver = { > + .owner = THIS_MODULE, > + .name = TOUCHPAD_ID_NAME, > +#ifdef CONFIG_PM > + .pm = &qcitp_pm_ops, > +#endif > + }, > + .probe = qcitp_probe, > + .remove = __devexit_p(qcitp_remove), > + .id_table = qcitp_idtable, > +}; > + > +static void qcitp_fetch_data(struct i2c_client *tpad_client, > + char *ec_data) > +{ > + struct i2c_msg tp_msg; > + int ret; > + tp_msg.addr = tpad_client->addr; > + tp_msg.flags = I2C_M_RD; > + tp_msg.len = 3; > + tp_msg.buf = (char *)&ec_data[0]; > + ret = i2c_transfer(tpad_client->adapter, &tp_msg, 1); > +} > + > +static void qcitp_report_key(struct input_dev *tpad_dev, char *ec_data) > +{ > + int dx = 0; > + int dy = 0; > + > + if (ec_data[1]) > + dx = (int) ec_data[1] - > + (int) ((ec_data[0] << 4) & 0x100); > + > + if (ec_data[2]) > + dy = (int) ((ec_data[0] << 3) & 0x100) - > + (int) ec_data[2]; > + > + dx = (dx * tp_sense_val)/10; > + dy = (dy * tp_sense_val)/10; Why do you do scaling in kernel? Userspace is better suited for such tasks. > + > + input_report_key(tpad_dev, BTN_LEFT, ec_data[0] & 0x01); > + input_report_key(tpad_dev, BTN_RIGHT, ec_data[0] & 0x02); > + input_report_key(tpad_dev, BTN_MIDDLE, ec_data[0] & 0x04); > + input_report_rel(tpad_dev, REL_X, dx); > + input_report_rel(tpad_dev, REL_Y, dy); > + input_sync(tpad_dev); > +} > + > +static void qcitp_work_handler(struct work_struct *_work) > +{ > + struct i2ctpad_drv_data *itpad_drv_data = > + container_of(_work, struct i2ctpad_drv_data, work); > + > + struct i2c_client *itpad_client = itpad_drv_data->ti2c_client; > + struct input_dev *itpad_dev = itpad_drv_data->qcitp_dev; > + > + qcitp_fetch_data(itpad_client, itpad_drv_data->ecdata); > + qcitp_report_key(itpad_dev, itpad_drv_data->ecdata); > +} > + > +static irqreturn_t qcitp_interrupt(int irq, void *dev_id) > +{ > + struct i2ctpad_drv_data *itpad_drv_data = dev_id; > + schedule_work(&itpad_drv_data->work); Replace with threaded IRQ. Or make sure you shut off work when unbinding the device. > + return IRQ_HANDLED; > +} > + > +static int __devinit qcitp_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + int err = -ENOMEM; > + struct i2ctpad_drv_data *context = 0; > + > + context = kzalloc(sizeof(struct i2ctpad_drv_data), GFP_KERNEL); > + if (!context) > + return err; > + i2c_set_clientdata(client, context); > + context->ti2c_client = client; > + context->qcitp_gpio = client->irq; > + > + /* Enable mouse */ > + i2c_smbus_write_byte(client, TOUCHPAD_CMD_ENABLE); > + mdelay(100); This looks like a candidate for qcitp_open() method. > + i2c_smbus_read_byte(client); > + /*allocate and register input device*/ > + context->qcitp_dev = input_allocate_device(); > + if (!context->qcitp_dev) { > + pr_err("[TouchPad] allocting memory fail\n"); > + err = -ENOMEM; > + goto allocate_fail; > + } > + context->qcitp_dev->name = TOUCHPAD_NAME; > + context->qcitp_dev->phys = TOUCHPAD_DEVICE; > + context->qcitp_dev->id.bustype = BUS_I2C; > + context->qcitp_dev->id.vendor = 0x1050; > + context->qcitp_dev->id.product = 0x1; > + context->qcitp_dev->id.version = 0x1; > + context->qcitp_dev->evbit[0] = BIT_MASK(EV_KEY) | > + BIT_MASK(EV_REL); > + context->qcitp_dev->relbit[0] = BIT_MASK(REL_X) | > + BIT_MASK(REL_Y); > + context->qcitp_dev->keybit[BIT_WORD(BTN_LEFT)] = BIT_MASK(BTN_LEFT) | > + BIT_MASK(BTN_MIDDLE) | > + BIT_MASK(BTN_RIGHT); > + > + input_set_drvdata(context->qcitp_dev, context); > + err = input_register_device(context->qcitp_dev); > + if (err) { > + pr_err("[TouchPad] register device fail\n"); > + goto register_fail; > + } > + > + /*request intterrupt*/ > + INIT_WORK(&context->work, qcitp_work_handler); > + > + err = gpio_request(context->qcitp_gpio, "qci-pad"); > + if (err) { > + pr_err("[TouchPad]err gpio request\n"); > + goto gpio_request_fail; > + } > + > + context->qcitp_irq = gpio_to_irq(context->qcitp_gpio); > + err = request_irq(context->qcitp_irq, > + qcitp_interrupt, > + IRQF_TRIGGER_FALLING, > + TOUCHPAD_ID_NAME, > + context); > + if (err) { > + pr_err("[TouchPad] unable to get IRQ\n"); > + goto request_irq_fail; > + } > + /*create touchpad kobject*/ > + context->tp_kobj = kobject_create_and_add("touchpad", NULL); Hmm, attributes normally attached to device object, not standalone kobject. > + > + err = sysfs_create_group(context->tp_kobj, &attr_group); > + if (err) > + pr_warning("[TouchPad] sysfs create fail\n"); > + > + tp_sense_val = 10; > + > + return 0; > + > +request_irq_fail: > + gpio_free(context->qcitp_gpio); > + > +gpio_request_fail: > + input_unregister_device(context->qcitp_dev); > + > +register_fail: > + input_free_device(context->qcitp_dev); You may not call input_free_device() after calling input_unregister_device(). The easiest way is to do: context->qcitp_dev = NULL; after calling input_unregister_device(). Or rearrange the code so that input_register_device() is called last. > + > +allocate_fail: > + i2c_set_clientdata(client, NULL); > + kfree(context); > + return err; > +} > + > +static int __devexit qcitp_remove(struct i2c_client *dev) > +{ > + struct i2ctpad_drv_data *context = i2c_get_clientdata(dev); > + > + free_irq(context->qcitp_irq, context); You need to have cancel_work_sync() here. Is there an opposite of i2c_smbus_write_byte(client, TOUCHPAD_CMD_ENABLE); so that you can shut the device off? If so it should go in close(). > + gpio_free(context->qcitp_gpio); > + input_free_device(context->qcitp_dev); > + input_unregister_device(context->qcitp_dev); Nice attempt of working around the rule that you may not call input_free_device() after input_unregister_device() ;) But still wrong. > + kfree(context); > + > + return 0; > +} > + > +static int __init qcitp_init(void) > +{ > + return i2c_add_driver(&i2ctp_driver); > +} > + > + > +static void __exit qcitp_exit(void) > +{ > + i2c_del_driver(&i2ctp_driver); > +} > + > +module_init(qcitp_init); > +module_exit(qcitp_exit); > + > +MODULE_AUTHOR("Quanta Computer Inc."); > +MODULE_DESCRIPTION("Quanta Embedded Controller I2C Touch Pad Driver"); > +MODULE_LICENSE("GPL v2"); Thanks. -- Dmitry -- 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
