Hi Christoph,
thanks for taking the time to look it over.
comments below
Heiko
Am Sonntag, 4. Dezember 2011, 14:31:41 schrieb Christoph Fritz:
> Hi Heiko,
>
> you can find some annotations below.
>
> Thanks,
> -- chf
>
> On Sat, 2011-12-03 at 15:09 +0100, Heiko Stübner wrote:
> > Some displays from AUO have a so called in-cell touchscreen, meaning it
> > is built directly into the display unit.
> >
> > Touchdata is gathered through PIXCIR Tango-ICs and processed in an
> > Atmel ATmega168P with custom firmware. Communication between the host
> > system and ATmega is done via I2C.
> >
> > Devices using this touch solution include the Dell Streak5 and the family
> > of Qisda ebook readers.
> >
> > The driver reports single- and multi-touch events including touch area
> > values.
> >
> > Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx>
> > ---
> >
> > drivers/input/touchscreen/Kconfig | 14 +
> > drivers/input/touchscreen/Makefile | 1 +
> > drivers/input/touchscreen/auo-pixcir-ts.c | 657
> > +++++++++++++++++++++++++++++ include/linux/input/auo-pixcir-ts.h
> > | 57 +++
> > 4 files changed, 729 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/input/touchscreen/auo-pixcir-ts.c
> > create mode 100644 include/linux/input/auo-pixcir-ts.h
[...]
> > +
> > +static int auo_pixcir_int_config(struct auo_pixcir_ts *ts, int
> > int_setting) +{
> > + struct i2c_client *client = ts->client;
> > + struct auo_pixcir_ts_platdata *pdata = client->dev.platform_data;
> > + uint8_t value;
> > +
> > + value = i2c_smbus_read_byte_data(client, AUO_PIXCIR_REG_INT_SETTING);
> > + if (value < 0) {
>
> value is an unsigned int and can't be less than zero. An int value would
> be more appropriate.
you're right of course and I will fix the type
> > + dev_err(&client->dev, "unable to read reg %Xh, %d\n",
> > + AUO_PIXCIR_REG_INT_SETTING, value);
> > + return value;
> > + }
> > +
> > + value &= (~AUO_PIXCIR_INT_MODE_MASK);
> > + value |= int_setting;
> > + value |= AUO_PIXCIR_INT_POL_HIGH; /* always use high for interrupts
*/
> > +
> > + value = i2c_smbus_write_byte_data(client, AUO_PIXCIR_REG_INT_SETTING,
> > + value);
> > + if (value < 0) {
>
> the same here
here too
[...]
> > +static int __devinit auo_pixcir_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct auo_pixcir_ts *ts;
> > + struct auo_pixcir_ts_platdata *pdata = client->dev.platform_data;
> > + struct input_dev *input_dev;
> > + int ret;
> > +
> > + if (!pdata)
> > + return -EINVAL;
> > +
> > + ts = kzalloc(sizeof(struct auo_pixcir_ts), GFP_KERNEL);
> > + if (!ts)
> > + return -ENOMEM;
> > +
> > + ret = gpio_request(pdata->gpio_int, "auo_pixcir_ts_int");
> > + if (ret) {
> > + dev_err(&client->dev, "request of gpio %d failed, %d\n",
> > + pdata->gpio_int, ret);
> > + goto err_gpio_int;
> > + }
> > +
> > + ret = gpio_request(pdata->gpio_rst, "auo_pixcir_ts_rst");
> > + if (ret) {
> > + dev_err(&client->dev, "request of gpio %d failed, %d\n",
> > + pdata->gpio_int, ret);
> > + goto err_gpio_rst;
> > + }
> > +
> > + if (pdata->init_hw)
> > + pdata->init_hw();
> > +
> > + ts->client = client;
> > + ts->touch_ind_mode = 0;
> > + init_waitqueue_head(&ts->wait);
> > +
> > + snprintf(ts->phys, sizeof(ts->phys),
> > + "%s/input0", dev_name(&client->dev));
> > +
> > + input_dev = input_allocate_device();
> > + if (!input_dev) {
> > + dev_err(&client->dev, "could not allocate input device\n");
> > + goto err_input_alloc;
> > + }
> > +
> > + ts->input = input_dev;
> > +
> > + input_dev->name = "auo_pixcir_ts";
> > + input_dev->phys = ts->phys;
> > + input_dev->id.bustype = BUS_I2C;
> > + input_dev->dev.parent = &client->dev;
> > +
> > + input_dev->open = auo_pixcir_input_open;
> > + input_dev->close = auo_pixcir_input_close;
> > +
> > + __set_bit(EV_ABS, input_dev->evbit);
> > + __set_bit(EV_KEY, input_dev->evbit);
> > +
> > + __set_bit(BTN_TOUCH, input_dev->keybit);
> > +
> > + /* For single touch */
> > + input_set_abs_params(input_dev, ABS_X, 0, pdata->x_max, 0, 0);
> > + input_set_abs_params(input_dev, ABS_Y, 0, pdata->y_max, 0, 0);
> > +
> > + /* For multi touch */
> > + input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0,
> > + pdata->x_max, 0, 0);
> > + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
> > + pdata->y_max, 0, 0);
> > + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
> > + AUO_PIXCIR_MAX_AREA, 0, 0);
> > + input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR, 0,
> > + AUO_PIXCIR_MAX_AREA, 0, 0);
> > + input_set_abs_params(input_dev, ABS_MT_ORIENTATION, 0, 1, 0, 0);
> > +
> > + ret = i2c_smbus_read_byte_data(client, AUO_PIXCIR_REG_VERSION);
> > + if (ret < 0)
> > + goto err_fw_vers;
> > + dev_info(&client->dev, "firmware version 0x%X\n", ret);
> > +
> > + ret = auo_pixcir_int_config(ts, pdata->int_setting);
> > + if (ret)
> > + goto err_fw_vers;
> > +
> > + /* disable irqs on the device side before irq-request */
> > + auo_pixcir_int_enable(ts, 0);
> > + ts->stopped = true;
> > + ts->is_open = false;
> > +
> > + ret = request_threaded_irq(client->irq, NULL, auo_pixcir_interrupt,
> > + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > + input_dev->name, ts);
> > + if (ret) {
> > + dev_err(&client->dev, "irq %d requested failed\n", client->irq);
> > + goto err_fw_vers;
> > + }
> > +
> > + /* stop device and put it into deep sleep until it is opened */
> > + ret = auo_pixcir_stop(ts);
> > + if (ret < 0)
> > + goto err_input_register;
> > +
> > + ret = input_register_device(input_dev);
> > + if (ret) {
> > + dev_err(&client->dev, "could not register input device\n");
> > + goto err_input_register;
> > + }
> > +
> > + input_set_drvdata(ts->input, ts);
> > +
> > + i2c_set_clientdata(client, ts);
> > +
> > + return 0;
> > +
> > +err_input_register:
> > + free_irq(client->irq, ts);
> > +err_fw_vers:
> > + input_free_device(input_dev);
> > +err_input_alloc:
> > + if (pdata->exit_hw)
> > + pdata->exit_hw();
> > + gpio_free(pdata->gpio_rst);
> > +err_gpio_rst:
> > + gpio_free(pdata->gpio_int);
> > +err_gpio_int:
> > + kfree(ts);
> > +
> > + return ret;
> > +}
> > +
> > +static int __devexit auo_pixcir_remove(struct i2c_client *client)
> > +{
> > + struct auo_pixcir_ts *ts = (struct auo_pixcir_ts *)
> > + i2c_get_clientdata(client);
> > + struct auo_pixcir_ts_platdata *pdata = client->dev.platform_data;
> > +
> > + auo_pixcir_stop(ts);
> > +
> > + free_irq(client->irq, ts);
> > +
> > + input_unregister_device(ts->input);
> > + input_free_device(ts->input);
>
> to quote input.c:
>
> /**
> * input_free_device - free memory occupied by input_dev structure
> * @dev: input device to free
> *
> * This function should only be used if input_register_device()
> * was not called yet or if it failed. Once device was registered
> * use input_unregister_device() and memory will be freed once last
> * reference to the device is dropped.
> *
> * Device should be allocated by input_allocate_device().
> *
> * NOTE: If there are references to the input device then memory
> * will not be freed until last reference is dropped.
> */
thanks for this quote and I will remove the input_free_device call.
> the same applies to the error path in auo_pixcir_probe()
according to the docs your quoted this wouldn't be the same, as
input_free_device is called in _probe either before the input_register_device
call or if the call to it failed.
[...]
--
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
