Hi ,
Some comments / doubts.

On Friday 23 March 2012 11:10 AM, Tatsunosuke Tobita wrote:

May be use dev_err . Feel free to ignore if you feel so. Just a suggestion

+		return -1;

Why mask the error?

+	}
+
+	cmd[0] = CMD_THROW0; cmd[1] = CMD_THROW1;
+	ret = i2c_master_send(wac_i2c->client, cmd, 2);
+	if (ret < 0) {
+		dev_printk(KERN_ERR, &wac_i2c->client->dev,
+			   "Sending 2nd Query failed \n");
+		return -1;

same

+	}
+
+	ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	if (ret < 0) {
+		dev_printk(KERN_ERR, &wac_i2c->client->dev,
+			   "Receiving data failed 1\n");
+		return -1;

same

+	}
+
+	wac_i2c->wac_feature.x_max = get_unaligned_le16(&data[3]);
+	wac_i2c->wac_feature.y_max = get_unaligned_le16(&data[5]);
+	wac_i2c->wac_feature.pressure_max = get_unaligned_le16(&data[11]);
+	wac_i2c->wac_feature.fw_version = get_unaligned_le16(&data[13]);
+
+	dev_dbg(&wac_i2c->client->dev,
+		   "x_max:%d, y_max:%d, pressure:%d, fw:%d\n",
+		   wac_i2c->wac_feature.x_max, wac_i2c->wac_feature.y_max,
+		   wac_i2c->wac_feature.pressure_max, wac_i2c->wac_feature.fw_version);
+
+	return 0;
+}
+
+static irqreturn_t wacom_i2c_irq(int irqno, void *param)
+{
+	struct wacom_i2c *wac_i2c = param;
+       int ret;
+	unsigned int x, y, pressure;
+	unsigned char tsw, f1, f2, ers;
+	u8 data[COM_COORD_NUM];
+
+	do {
+		ret = i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	} while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
+
+	if (ret > 0) {
+		tsw = data[3]&0x01;
+		ers = data[3]&0x04;
+		f1 = data[3]&0x02;
+		f2 = data[3]&0x10;
+		x = le16_to_cpup((__le16 *)&data[4]);
+		y = le16_to_cpup((__le16 *)&data[6]);
+		pressure = le16_to_cpup((__le16 *)&data[8]);
+
+		input_report_abs(wac_i2c->input, ABS_X, x);
+		input_report_abs(wac_i2c->input, ABS_Y, y);
+		input_report_key(wac_i2c->input, BTN_TOOL_PEN, tsw);
+		input_report_key(wac_i2c->input, BTN_TOOL_RUBBER, ers);
+		input_report_abs(wac_i2c->input, ABS_PRESSURE, pressure);
+		input_report_key(wac_i2c->input, BTN_TOUCH, (tsw || ers));
+		input_report_key(wac_i2c->input, BTN_STYLUS, f1);
+		input_report_key(wac_i2c->input, BTN_STYLUS2, f2);
+
+		input_sync(wac_i2c->input);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int wacom_i2c_input_open(struct input_dev *dev)
+{
+	struct wacom_i2c *wac_i2c = input_get_drvdata(dev);
+	int ret;
+	ret = wacom_send_query(wac_i2c);
+	if (ret < 0)
+		return -EIO;
+
+	wac_i2c->input->id.version = wac_i2c->wac_feature.fw_version;
+	input_set_abs_params(wac_i2c->input, ABS_X, 0,
+			     wac_i2c->wac_feature.x_max, 0, 0);
+	input_set_abs_params(wac_i2c->input, ABS_Y, 0,
+			     wac_i2c->wac_feature.y_max, 0, 0);
+	input_set_abs_params(wac_i2c->input, ABS_PRESSURE, 0,
+			     wac_i2c->wac_feature.pressure_max, 0, 0);
+
+	return 0;
+}
+
+static void wacom_i2c_input_close(struct input_dev *dev)
+{
+}

May be this could be cleaned up?

+
+static int __devinit wacom_i2c_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	struct wacom_i2c *wac_i2c;
+	int err;
+	u8 data[COM_COORD_NUM];
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		err = -EIO;
+		goto fail1;
+	}
+
+	wac_i2c = kzalloc(sizeof(*wac_i2c), GFP_KERNEL);
+	if (wac_i2c == NULL) {
+		err = -ENOMEM;
+		goto fail2;
+	}
+
+	wac_i2c->input = input_allocate_device();
+	if (wac_i2c->input == NULL) {
+		err = -ENOMEM;
+		goto fail2;

We are leaking wac_i2c here ?

+	}
+
+	wac_i2c->client = client;
+	i2c_set_clientdata(client, wac_i2c);
+	mutex_init(&wac_i2c->lock);
+
+	__set_bit(BTN_TOOL_PEN, wac_i2c->input->keybit);
+	__set_bit(BTN_TOOL_RUBBER, wac_i2c->input->keybit);
+	__set_bit(BTN_STYLUS, wac_i2c->input->keybit);
+	__set_bit(BTN_STYLUS2, wac_i2c->input->keybit);
+	__set_bit(BTN_TOUCH, wac_i2c->input->keybit);
+
+	wac_i2c->input->name = "Wacom I2C Digitizer";
+	wac_i2c->input->id.bustype = BUS_I2C;
+	wac_i2c->input->id.vendor = 0x56a;
+	wac_i2c->input->dev.parent = &client->dev;
+	wac_i2c->input->open = wacom_i2c_input_open;
+	wac_i2c->input->close = wacom_i2c_input_close;
+	wac_i2c->input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	input_set_drvdata(wac_i2c->input, wac_i2c);
+
+	if (input_register_device(wac_i2c->input)) {
+		dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to register input device \n");
+		err = -ENODEV;
+		goto fail3;
+	}
+
+
+	err = request_threaded_irq(wac_i2c->client->irq, NULL,
+				   wacom_i2c_irq, IRQF_TRIGGER_FALLING, "WACOM_I2C_IRQ", wac_i2c);
+	if (err) {
+		dev_printk(KERN_ERR, &wac_i2c->client->dev, "Failed to enable IRQ for EMR \n");
+		goto fail3;
+	}
+
+	/*Clear the buffer in the firmware*/
+	do {
+		i2c_master_recv(wac_i2c->client, data, QUERY_SIZE);
+	} while (gpio_get_value(irq_to_gpio(wac_i2c->client->irq)) == 0);
+
+	return 0;
+
+ fail3:
+	input_free_device(wac_i2c->input);
+ fail2:
+	dev_err(&wac_i2c->client->dev, "Freeing device \n");
+ fail1:
+	return err;
+}
+
+static int __devexit wacom_i2c_remove(struct i2c_client *client)
+{
+	struct wacom_i2c *wac_i2c = i2c_get_clientdata(client);
+
+	free_irq(wac_i2c->client->irq, wac_i2c);
+	input_unregister_device(wac_i2c->input);
+	kfree(wac_i2c);
+
+	return 0;
+}
+
+static int wacom_i2c_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	disable_irq(client->irq);

Why is this disable needed?

nit : 2012?