Hi Daniel, Looks pretty good, a couple of things below. On the next iteration please CC Jean Delvare for the I2C bits. On Fri, May 15, 2009 at 11:41:11AM +0200, Daniel Mack wrote: > + > + mutex_lock(&priv->mutex); > + > + while (!gpio_get_value(irq_to_gpio(priv->irq)) && --to) > + i2c_master_recv(priv->client, buf, sizeof(buf)); > + > + if (!to) { > + dev_err(&priv->client->dev, > + "unable to clear IRQ - line stuck?\n"); > + mutex_lock(&priv->mutex); Tsk, tsk... Please just use standard "goto out;" schema and unlock the mutex there. > + return; > + } > + > + /* drop non-report packets */ > + if (!(buf[0] & 0x80)) { > + mutex_unlock(&priv->mutex); > + return; > + } > + > + pressed = buf[0] & REPORT_BIT_PRESSED; > + res = REPORT_RES_BITS(buf[0] & (REPORT_BIT_AD0 | REPORT_BIT_AD1)); > + x = buf[2] | (buf[1] << 8); > + y = buf[4] | (buf[3] << 8); > + > + /* fix the range to 11 bits */ > + x >>= res - EETI_TS_BITDEPTH; > + y >>= res - EETI_TS_BITDEPTH; > + > + if (flip_x) > + x = EETI_MAXVAL - x; > + > + if (flip_y) > + y = EETI_MAXVAL - y; > + > + if ((buf[0] & REPORT_BIT_HAS_PRESSURE) && !pressed) > + input_report_abs(priv->input, ABS_PRESSURE, buf[5]); Please report the pressure always (if you have it of course), not only at the beginning of the touch. This will allow userspace control sensitivity. > + > + input_report_abs(priv->input, ABS_X, x); > + input_report_abs(priv->input, ABS_Y, y); > + input_report_key(priv->input, BTN_TOUCH, !!pressed); > + input_sync(priv->input); > + mutex_unlock(&priv->mutex); > +} > + > +static irqreturn_t eeti_ts_isr(int irq, void *dev_id) > +{ > + struct eeti_ts_priv *priv = dev_id; > + > + /* postpone I2C transactions as we are atomic */ > + schedule_delayed_work(&priv->work, HZ / 100); Why delayed work? I'd think you want to schedule it immediately. > + > + return IRQ_HANDLED; > +} > + > +static int eeti_ts_open(struct input_dev *dev) > +{ > + struct eeti_ts_priv *priv = input_get_drvdata(dev); > + > + enable_irq(priv->irq); > + > + /* Read the events once to arm the IRQ */ > + eeti_ts_read(&priv->work.work); > + > + return 0; > +} > + > +static void eeti_ts_close(struct input_dev *dev) > +{ > + struct eeti_ts_priv *priv = input_get_drvdata(dev); > + > + disable_irq(priv->irq); > + cancel_delayed_work_sync(&priv->work); > +} > + > +static int eeti_ts_probe(struct i2c_client *client, > + const struct i2c_device_id *idp) __devinit? > +{ > + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); > + struct eeti_ts_priv *priv; > + struct input_dev *input; > + int err = -ENOMEM; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE)) > + return -EIO; > + > + /* In contrast to what's described in the datasheet, there seems > + * to be no way of probing the presence of that device using I2C > + * commands. So we need to blindly believe it is there, and wait > + * for interrupts to occur. */ > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) { > + dev_err(&client->dev, "failed to allocate driver data\n"); > + goto err0; > + } > + > + mutex_init(&priv->mutex); > + input = input_allocate_device(); > + > + if (!input) { > + dev_err(&client->dev, "Failed to allocate input device.\n"); > + goto err1; > + } > + > + input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS); > + input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH); > + > + input_set_abs_params(input, ABS_X, 0, EETI_MAXVAL, 0, 0); > + input_set_abs_params(input, ABS_Y, 0, EETI_MAXVAL, 0, 0); > + input_set_abs_params(input, ABS_PRESSURE, 0, 0xff, 0, 0); > + > + input->name = client->name; > + input->id.bustype = BUS_I2C; > + input->dev.parent = &client->dev; > + input->open = eeti_ts_open; > + input->close = eeti_ts_close; > + > + input_set_drvdata(input, priv); > + > + priv->client = client; > + priv->input = input; > + priv->irq = client->irq; > + dev_set_drvdata(&client->dev, priv); > + INIT_DELAYED_WORK(&priv->work, eeti_ts_read); > + > + err = input_register_device(input); > + if (err) > + goto err1; > + > + err = request_irq(priv->irq, eeti_ts_isr, IRQF_TRIGGER_FALLING, > + client->name, priv); > + if (err) { > + dev_err(&client->dev, "Unable to request touchscreen IRQ.\n"); > + goto err2; > + } > + > + /* Disable the irq for now. It will be enabled once the input device > + * is opened. */ > + disable_irq(priv->irq); > + > + device_init_wakeup(&client->dev, 0); > + return 0; > + > +err2: > + input_unregister_device(input); > + input = NULL; /* so we dont try to free it below */ > +err1: > + input_free_device(input); > + kfree(priv); > +err0: > + dev_set_drvdata(&client->dev, NULL); > + return err; > +} > + > +static int eeti_ts_remove(struct i2c_client *client) __devexit? > +{ > + struct eeti_ts_priv *priv = dev_get_drvdata(&client->dev); > + > + free_irq(priv->irq, priv); > + input_unregister_device(priv->input); > + dev_set_drvdata(&client->dev, NULL); > + kfree(priv); > + > + return 0; > +} > + > +#ifdef CONFIG_PM > +static int eeti_ts_suspend(struct i2c_client *client, pm_message_t mesg) > +{ > + struct eeti_ts_priv *priv = dev_get_drvdata(&client->dev); > + > + if (device_may_wakeup(&client->dev)) > + enable_irq_wake(priv->irq); > + > + return 0; > +} > + > +static int eeti_ts_resume(struct i2c_client *client) > +{ > + struct eeti_ts_priv *priv = dev_get_drvdata(&client->dev); > + > + if (device_may_wakeup(&client->dev)) > + disable_irq_wake(priv->irq); > + > + return 0; > +} > +#else > +#define eeti_ts_suspend NULL > +#define eeti_ts_resume NULL > +#endif > + > +static const struct i2c_device_id eeti_ts_id[] = { > + { "eeti_ts", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, eeti_ts_id); > + > +static struct i2c_driver eeti_ts_driver = { > + .driver = { > + .name = "eeti_ts", > + }, > + .probe = eeti_ts_probe, > + .remove = eeti_ts_remove, __devexit_p? 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