Hi Christoph, I'm sorry for my poor English grammar. I'll try to fix these typo, If still have error, please point me out. 2011/4/12 Christoph Fritz <chf.fritz@xxxxxxxxxxxxxx> > > Hi Zhang, > > this is just a superficial review of your driver. > > thanks, > Christoph > > On Tue, Apr 12, 2011 at 01:02:17AM +0800, Jiejing Zhang wrote: > > From: Zhang Jiejing <jiejing.zhang@xxxxxxxxxxxxx> > > > > This touchkey driver is based on Freescale mpr121 capacitive > > touch sensor controller. > > > > It can support up to 12 keys, it use i2c interface. > > typo -> It can supports up to 12 electrodes, and using i2c interface. > > > > > The product infomation(data sheet, application notes) can > > be found under this link: > > http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=MPR121 > > > > Signed-off-by: Zhang Jiejing <jiejing.zhang@xxxxxxxxxxxxx> > > --- > > drivers/input/keyboard/Kconfig | 12 ++ > > drivers/input/keyboard/Makefile | 1 + > > drivers/input/keyboard/mpr121.c | 301 +++++++++++++++++++++++++++++++++++++++ > > include/linux/i2c/mpr.h | 64 ++++++++ > > 4 files changed, 378 insertions(+), 0 deletions(-) > > create mode 100644 drivers/input/keyboard/mpr121.c > > create mode 100644 include/linux/i2c/mpr.h > > > > diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig > > index b16bed0..0666e1e 100644 > > --- a/drivers/input/keyboard/Kconfig > > +++ b/drivers/input/keyboard/Kconfig > > @@ -520,6 +520,18 @@ config KEYBOARD_XTKBD > > To compile this driver as a module, choose M here: the > > module will be called xtkbd. > > > > +config KEYBOARD_MPR121 > > + tristate "Freescale MPR121 Touchkey Driver" > > + depends on I2C > > + help > > + Say Y here if you have the touchkey Freescale mpr121 capacitive > > typo: two spaces will fix. > > > + touch sensor controller in your system. > > + > > + If unsure, say N. > > + > > + To compile this driver as a module, choose M here > > +endif > > + > > config KEYBOARD_W90P910 > > tristate "W90P910 Matrix Keypad support" > > depends on ARCH_W90X900 > > - add: "To compile this driver as a module, choose M here: the > module will be called mpr121." > - "endif" is wrong here, it breaks 'make config' > - maybe a more alphabetic approach in this list of drivers would > be nice yes. I will. > > > diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile > > index 878e6c2..b110ca8 100644 > > --- a/drivers/input/keyboard/Makefile > > +++ b/drivers/input/keyboard/Makefile > > @@ -47,4 +47,5 @@ obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o > > obj-$(CONFIG_KEYBOARD_TNETV107X) += tnetv107x-keypad.o > > obj-$(CONFIG_KEYBOARD_TWL4030) += twl4030_keypad.o > > obj-$(CONFIG_KEYBOARD_XTKBD) += xtkbd.o > > +obj-$(CONFIG_KEYBOARD_MPR121) += mpr121.o > > obj-$(CONFIG_KEYBOARD_W90P910) += w90p910_keypad.o > > for better examination, please mind alphabetic order will do. > > > diff --git a/drivers/input/keyboard/mpr121.c b/drivers/input/keyboard/mpr121.c > > new file mode 100644 > > index 0000000..ee149a8 > > --- /dev/null > > +++ b/drivers/input/keyboard/mpr121.c > > @@ -0,0 +1,301 @@ > > +/* > > + * Touchkey driver for Freescale MPR121 Controllor > > + * > > + * Copyright (C) 2011 Freescale Semiconductor, Inc. > > + * Author: Zhang Jiejing <jiejing.zhang@xxxxxxxxxxxxx> > > + * > > + * Based on mcs_touchkey.c > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/init.h> > > +#include <linux/i2c.h> > > +#include <linux/i2c/mpr.h> > > +#include <linux/interrupt.h> > > +#include <linux/input.h> > > +#include <linux/irq.h> > > do not use irq.h directly ok. > > > +#include <linux/slab.h> > > +#include <linux/delay.h> > > +#include <linux/bitops.h> > > + > > +struct mpr121_touchkey_data { > > + struct i2c_client *client; > > + struct input_dev *input_dev; > > + unsigned int key_val; > > + int statusbits; > > + int keycount; > > + u16 keycodes[MPR121_MAX_KEY_COUNT]; > > +}; > > + > > +struct mpr121_init_register { > > + int addr; > > + u8 val; > > +}; > > + > > +static struct mpr121_init_register init_reg_table[] = { > > + {MHD_RISING_ADDR, 0x1}, > > + {NHD_RISING_ADDR, 0x1}, > > + {MHD_FALLING_ADDR, 0x1}, > > + {NHD_FALLING_ADDR, 0x1}, > > + {NCL_FALLING_ADDR, 0xff}, > > + {FDL_FALLING_ADDR, 0x02}, > > + {FILTER_CONF_ADDR, 0x04}, > > + {AFE_CONF_ADDR, 0x0b}, > > + {AUTO_CONFIG_CTRL_ADDR, 0x0b}, > > +}; > > + > > +static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id) > > +{ > > + struct mpr121_touchkey_data *data = dev_id; > > + struct i2c_client *client = data->client; > > + struct input_dev *input = data->input_dev; > > + unsigned int key_num, pressed; > > + int reg; > > + > > + reg = i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_1_ADDR); > > + if (reg < 0) { > > + dev_err(&client->dev, "i2c read error [%d]\n", reg); > > + goto out; > > + } > > + > > + reg <<= 8; > > + reg |= i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_0_ADDR); > > + if (reg < 0) { > > + dev_err(&client->dev, "i2c read error [%d]\n", reg); > > + goto out; > > + } > > + > > + reg &= TOUCH_STATUS_MASK; > > + /* use old press bit to figure out which bit changed */ > > + key_num = ffs(reg ^ data->statusbits) - 1; > > + /* use the bit check the press status */ > > typo this comment will remove. > > > + pressed = (reg & (1 << (key_num))) >> key_num; > > + data->statusbits = reg; > > + data->key_val = data->keycodes[key_num]; > > + > > + input_event(input, EV_MSC, MSC_SCAN, data->key_val); > > + input_report_key(input, data->key_val, pressed); > > + input_sync(input); > > + > > + dev_dbg(&client->dev, "key %d %d %s\n", key_num, data->key_val, > > + pressed ? "pressed" : "released"); > > + > > +out: > > + return IRQ_HANDLED; > > +} > > + > > +static int mpr121_phys_init(struct mpr121_platform_data *pdata, > > + struct mpr121_touchkey_data *data, > > + struct i2c_client *client) > > +{ > > + struct mpr121_init_register *reg; > > + unsigned char usl, lsl, tl; > > + int i, t, vdd, ret; > > + > > + /* setup touch/release threshold for ele0-ele11 */ > > + for (i = 0; i <= MPR121_MAX_KEY_COUNT; i++) { > > + t = ELE0_TOUCH_THRESHOLD_ADDR + (i * 2); > > + ret = i2c_smbus_write_byte_data(client, t, TOUCH_THRESHOLD); > > + if (ret < 0) > > + goto err_i2c_write; > > + ret = i2c_smbus_write_byte_data(client, t + 1, > > + RELEASE_THRESHOLD); > > + if (ret < 0) > > + goto err_i2c_write; > > + } > > + /* setup init register */ > > + for (i = 0; i < ARRAY_SIZE(init_reg_table); i++) { > > + reg = &init_reg_table[i]; > > + ret = i2c_smbus_write_byte_data(client, reg->addr, reg->val); > > + if (ret < 0) > > + goto err_i2c_write; > > + } > > + /* setup auto-register by vdd,the formula please ref: AN3889 > > + * These register *must* set acrodding your VDD voltage supply > > + * to the chip*/ > > typo is this right? /* setup Auto Config Registers by VDD. * The formula was written in AN3889. * These registers *must* be set by VDD voltage. */ > > > + vdd = pdata->vdd_uv / 1000; > > + usl = ((vdd - 700) * 256) / vdd; > > + lsl = (usl * 65) / 100; > > + tl = (usl * 90) / 100; > > + ret = i2c_smbus_write_byte_data(client, AUTO_CONFIG_USL_ADDR, usl); > > + ret |= i2c_smbus_write_byte_data(client, AUTO_CONFIG_LSL_ADDR, lsl); > > + ret |= i2c_smbus_write_byte_data(client, AUTO_CONFIG_TL_ADDR, tl); > > + ret |= i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR, > > + data->keycount); > > + if (ret != 0) > > + goto err_i2c_write; > > + > > + dev_info(&client->dev, "mpr121: config as enable %x of electrode.\n", > > typo ? -> dev_dbg(&client->dev, "mpr121: setup with %x electrodes.\n", > > > + data->keycount); > > + > > + return 0; > > + > > +err_i2c_write: > > + dev_err(&client->dev, "i2c write error: %d\n", ret); > > + return ret; > > +} > > + > > +static int __devinit mpr_touchkey_probe(struct i2c_client *client, > > + const struct i2c_device_id *id) > > +{ > > + struct mpr121_platform_data *pdata; > > + struct mpr121_touchkey_data *data; > > + struct input_dev *input_dev; > > + int error; > > + int i; > > + > > + pdata = client->dev.platform_data; > > + if (!pdata) { > > + dev_err(&client->dev, "no platform data defined\n"); > > + return -EINVAL; > > + } > > + > > + if (!client->irq) { > > + dev_err(&client->dev, "The irq number should not be zero\n"); > > + return -EINVAL; > > + } > > + > > + data = kzalloc(sizeof(struct mpr121_touchkey_data), GFP_KERNEL); > > + input_dev = input_allocate_device(); > > + if (!data || !input_dev) { > > + dev_err(&client->dev, "Falied to allocate memory\n"); > > + error = -ENOMEM; > > + goto err_free_mem; > > + } > > + > > + data->client = client; > > + data->input_dev = input_dev; > > + data->keycount = pdata->keycount; > > + > > + if (data->keycount > MPR121_MAX_KEY_COUNT) { > > + dev_err(&client->dev, "Too many key defined\n"); > > typo -> dev_err(&client->dev, "too much keys defined\n"); > > > + error = -EINVAL; > > + goto err_free_mem; > > + } > > + > > + error = mpr121_phys_init(pdata, data, client); > > + if (error) { > > + dev_err(&client->dev, "Failed to init register\n"); > > + goto err_free_mem; > > + } > > + > > + i2c_set_clientdata(client, data); > > + > > + input_dev->name = "FSL MPR121 Touchkey"; > > + input_dev->id.bustype = BUS_I2C; > > + input_dev->dev.parent = &client->dev; > > + input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP); > > + input_dev->keycode = pdata->matrix; > > + input_dev->keycodesize = sizeof(pdata->matrix[0]); > > + input_dev->keycodemax = data->keycount; > > + > > + for (i = 0; i < input_dev->keycodemax; i++) { > > + __set_bit(pdata->matrix[i], input_dev->keybit); > > + data->keycodes[i] = pdata->matrix[i]; > > + } > > + > > + input_set_capability(input_dev, EV_MSC, MSC_SCAN); > > + input_set_drvdata(input_dev, data); > > + > > + error = request_threaded_irq(client->irq, NULL, > > + mpr_touchkey_interrupt, > > + IRQF_TRIGGER_FALLING, > > + client->dev.driver->name, data); > > + if (error) { > > + dev_err(&client->dev, "Failed to register interrupt\n"); > > + goto err_free_mem; > > + } > > + > > + error = input_register_device(input_dev); > > + if (error) > > + goto err_free_irq; > > + i2c_set_clientdata(client, data); > > + device_init_wakeup(&client->dev, pdata->wakeup); > > + dev_info(&client->dev, "Mpr121 touch keyboard init success.\n"); > > + return 0; > > + > > +err_free_irq: > > + free_irq(client->irq, data); > > +err_free_mem: > > + input_free_device(input_dev); > > + kfree(data); > > + return error; > > +} > > + > > +static int __devexit mpr_touchkey_remove(struct i2c_client *client) > > +{ > > + struct mpr121_touchkey_data *data = i2c_get_clientdata(client); > > + > > + free_irq(client->irq, data); > > + input_unregister_device(data->input_dev); > > + kfree(data); > > + > > + return 0; > > +} > > + > > +#ifdef CONFIG_PM_SLEEP > > +static int mpr_suspend(struct device *dev) > > +{ > > + struct i2c_client *client = to_i2c_client(dev); > > + > > + if (device_may_wakeup(&client->dev)) > > + enable_irq_wake(client->irq); > > + > > + i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR, 0x00); > > + > > + return 0; > > +} > > + > > +static int mpr_resume(struct device *dev) > > +{ > > + struct i2c_client *client = to_i2c_client(dev); > > + struct mpr121_touchkey_data *data = i2c_get_clientdata(client); > > + > > + if (device_may_wakeup(&client->dev)) > > + disable_irq_wake(client->irq); > > + > > + i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR, data->keycount); > > + > > + return 0; > > +} > > +#endif > > + > > +static const struct i2c_device_id mpr121_id[] = { > > + {"mpr121_touchkey", 0}, > > + { } > > +}; > > + > > +static SIMPLE_DEV_PM_OPS(mpr121_touchkey_pm_ops, mpr_suspend, mpr_resume); > > + > > +static struct i2c_driver mpr_touchkey_driver = { > > + .driver = { > > + .name = "mpr121", > > + .owner = THIS_MODULE, > > + .pm = &mpr121_touchkey_pm_ops, > > I would add a ifdef CONFIG_PM_SLEEP > > > + }, > > + .id_table = mpr121_id, > > + .probe = mpr_touchkey_probe, > > + .remove = __devexit_p(mpr_touchkey_remove), > > +}; > > + > > +static int __init mpr_touchkey_init(void) > > +{ > > + return i2c_add_driver(&mpr_touchkey_driver); > > +} > > + > > +static void __exit mpr_touchkey_exit(void) > > +{ > > + i2c_del_driver(&mpr_touchkey_driver); > > +} > > + > > +module_init(mpr_touchkey_init); > > +module_exit(mpr_touchkey_exit); > > + > > +MODULE_LICENSE("GPL"); > > +MODULE_AUTHOR("Zhang Jiejing <jiejing.zhang@xxxxxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Touch Key driver for FSL MPR121 Chip"); > > diff --git a/include/linux/i2c/mpr.h b/include/linux/i2c/mpr.h > > new file mode 100644 > > index 0000000..52d6e33 > > --- /dev/null > > +++ b/include/linux/i2c/mpr.h > > @@ -0,0 +1,64 @@ > > +/* mpr.h - Header file for Freescale mpr121 capacitive touch sensor > > no filename please will do. > > > + * controllor */ > > + > > +#ifndef MPR_H > > +#define MPR_H > > + > > +/* Register definitions */ > > +#define ELE_TOUCH_STATUS_0_ADDR 0x0 > > +#define ELE_TOUCH_STATUS_1_ADDR 0X1 > > +#define MHD_RISING_ADDR 0x2b > > +#define NHD_RISING_ADDR 0x2c > > +#define NCL_RISING_ADDR 0x2d > > +#define FDL_RISING_ADDR 0x2e > > +#define MHD_FALLING_ADDR 0x2f > > +#define NHD_FALLING_ADDR 0x30 > > +#define NCL_FALLING_ADDR 0x31 > > +#define FDL_FALLING_ADDR 0x32 > > +#define ELE0_TOUCH_THRESHOLD_ADDR 0x41 > > +#define ELE0_RELEASE_THRESHOLD_ADDR 0x42 > > +/* ELE0...ELE11's threshold will set in a loop */ > > typo -> /* ELE0...ELE11's threshold will be set in a loop */ > > > +#define AFE_CONF_ADDR 0x5c > > +#define FILTER_CONF_ADDR 0x5d > > + > > +/* ELECTRODE_CONF: this register is most important register, it > > + * control how many of electrode is enabled. If you set this register > > + * to 0x0, it make the sensor going to suspend mode. Other value(low > > + * bit is non-zero) will make the sensor into Run mode. This register > > + * should be write at last. > > typo -> * should be written in the end. > > > + */ > > +#define ELECTRODE_CONF_ADDR 0x5e > > +#define AUTO_CONFIG_CTRL_ADDR 0x7b > > +/* AUTO_CONFIG_USL: Upper Limit for auto baseline search, this > > + * register is related to VDD supplied on your board, the value of > > + * this register is calc by EQ: `((VDD-0.7)/VDD) * 256`. > > typo -> * this register is calculated by EQ: `((VDD-0.7)/VDD) * 256`. > > > + * AUTO_CONFIG_LSL: Low Limit of auto baseline search. This is 65% of > > + * USL AUTO_CONFIG_TL: The Traget Level of auto baseline search, This > > + * is 90% of USL */ > > typo -> * USL AUTO_CONFIG_TL: The traget level of auto baseline search, it is -> * 90% of USL */ > > > +#define AUTO_CONFIG_USL_ADDR 0x7d > > +#define AUTO_CONFIG_LSL_ADDR 0x7e > > +#define AUTO_CONFIG_TL_ADDR 0x7f > > + > > +/* Threshold of touch/release trigger */ > > +#define TOUCH_THRESHOLD 0x0f > > +#define RELEASE_THRESHOLD 0x0a > > +/* Mask Button bits of STATUS_0 & STATUS_1 register */ > > and or bit-and? don't need and or bit-and, it just means low 12 bits of status register is touch key's status. -> /* low 12 bits of status register is touch key's status */ > > > +#define TOUCH_STATUS_MASK 0xfff > > +/* MPR121 have 12 electrodes */ > > +#define MPR121_MAX_KEY_COUNT 12 > > + > > + > > +/** > > + * @keycount: how many key maped > > typo: "keys" ? -> * @keycount: how many keys mapped > > > + * @vdd_uv: voltage of vdd supply the chip in uV > > - typo > - uppercase for vdd -> * @vdd_uv: VDD voltage in uV > > > + * @matrix: maxtrix of keys > > + * @wakeup: can key wake up system. > > typo -> * @wakeup: can wake up system or not > > > + */ > > +struct mpr121_platform_data { > > + u16 keycount; > > + u16 *matrix; > > + int wakeup; > > + int vdd_uv; > > +}; > > + > > +#endif > > -- > > 1.7.1 > > > > -- > > 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.htmln Tue, Apr 12, 2011 at 01:02:17AM +0800, Jiejing Zhang wrote: :-( sorry again for my horrible grammar. Jiejing -- 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
