Re: [PATCH v3] Input: add ST1232 touchscreen controller driver.

[Henrik Rydberg <rydberg@euromail.se>]

> +

> +static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
> +{
> +	int i, ret, flg;


A "int count = 0;" makes more sense than flg.

> +	struct st1232_ts_data *ts = dev_id;
> +	struct st1232_ts_finger *finger = ts->finger;
> +
> +	ret = st1232_ts_read_data(ts);
> +	if (ret < 0)
> +		goto end;
> +
> +	/* multi touch protocol */
> +	for (i = 0, flg = 0; i < MAX_FINGERS; i++) {


flg initialization is hidden here, see above comment.

> +		if (!finger[i].is_valid)
> +			continue;
> +		else
> +			flg = 1;


dropping this...

> +
> +		input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR,
> +					finger[i].t);
> +		input_report_abs(ts->input_dev, ABS_MT_POSITION_X, finger[i].x);
> +		input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, finger[i].y);
> +		input_mt_sync(ts->input_dev);


and simply "count++;" here.

> +	}
> +
> +	/* SYN_MT_REPORT only if no contact */
> +	if (!flg)


(!count)

> +		input_mt_sync(ts->input_dev);
> +
> +	/* SYN_REPORT */
> +	input_sync(ts->input_dev);
> +
> +end:
> +	return IRQ_HANDLED;
> +}
> +
> +
> +	input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR,
> +				0, MAX_AREA, 0, 0);
> +	input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X,
> +				MIN_X, MAX_X, 0, 0);
> +	input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y,
> +				MIN_Y, MAX_Y, 0, 0);
> +
> +	ret = input_register_device(ts->input_dev);
> +	if (ret) {
> +		dev_err(&client->dev, "Unable to register %s input device\n",
> +			ts->input_dev->name);
> +		goto err_free_input_device;
> +	}
> +
> +	ret = request_threaded_irq(client->irq, NULL, st1232_ts_irq_handler,
> +					IRQF_ONESHOT, client->name, ts);
> +	if (ret) {
> +		dev_err(&client->dev, "Failed to register interrupt\n");
> +		goto err_free_input_device;
> +	}
> +
> +	device_init_wakeup(&client->dev, 1);
> +
> +	return 0;
> +
> +err_free_input_device:
> +	input_free_device(ts->input_dev);
> +err_free_mem:
> +	kfree(ts);
> +err:
> +	return ret;
> +}


Thank you,
Henrik