Hi Shen,

On Wed, Mar 09, 2011 at 03:18:57PM +0800, voice wrote:

+
+/* AT42QT1070 support up to 7 keys */
+static unsigned char qt1070_key2code[] = {

This should be const.

+	KEY_0, KEY_1, KEY_2, KEY_3,
+	KEY_4, KEY_5, KEY_6,
+};

+
+struct qt1070_data {
+	struct i2c_client *client;
+	struct input_dev *input;
+	unsigned char keycodes[ARRAY_SIZE(qt1070_key2code)];

This should be unsigned short since we have key values above 255.

+
+static int qt1070_read_key(struct qt1070_data *data)
+{
+	struct i2c_client *client = data->client;
+	struct input_dev *input = data->input;
+	u8 new_key, old_key, keyval;
+	int ret, i, mask;
+
+	/* Read the detected status register, using to clear interrupt */
+	ret = qt1070_read(client, DET_STATUS);
+
+	/* Read which key changed */
+	ret = qt1070_read(client, KEY_STATUS);
+	old_key = data->key;
+	data->key = new_key = ret;
+
+	mask = 0x01;
+	for (i = 0; i < ARRAY_SIZE(qt1070_key2code); i++) {
+		keyval = new_key & mask;
+		if ((old_key & mask) != keyval)
+			input_report_key(input, data->keycodes[i], keyval);
+		mask <<= 1;
+	}
+	input_sync(input);
+
+	return 0;
+}
+
+static irqreturn_t qt1070_interrupt(int irq, void *dev_id)
+{
+	struct qt1070_data *data = dev_id;
+	int ret;
+
+	ret = qt1070_read_key(data);

I'd fold qt1070_read_key into qt1070_interrupt.

+
+	if (client->irq) {

The driver is completely unusable without irq, I'd recommend simply
refusing to bind if irq is not supplied.

Otherwise looks very good.

Thanks.

-- 
Dmitry