Re: [PATCH v5] input: Add MELFAS mms114 touchscreen driver

["Henrik Rydberg" <rydberg@xxxxxxxxxxx>]

Hi Joonyoung,

> This is a initial driver for new touchscreen chip mms114 of MELFAS.
> It uses I2C interface and supports 10 multi touch.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> ---
> This v5 patch was updated from Henrik review mainly.

Looking neat now, thanks for making the changes. One comment and one question:

> +#define MMS114_PACKET_NUM		8

I would have dropped this in favor of sizeof(touch[0]) instead.

> +static irqreturn_t mms114_interrupt(int irq, void *dev_id)
> +{
> +	struct mms114_data *data = dev_id;
> +	struct mms114_touch touch[MMS114_MAX_TOUCH];
> +	int packet_size;
> +	int touch_size;
> +	int index;
> +	int error;
> +
> +	if (!data->enabled)
> +		goto out;
> +
> +	packet_size = mms114_read_reg(data, MMS114_PACKET_SIZE);
> +	if (packet_size <= 0)
> +		goto out;
> +
> +	touch_size = packet_size / MMS114_PACKET_NUM;

Since MMS114_PACKET_NUM changed, this calculation is no longer the
same. Will you still get the correct number of touches for all
firmware versions?

> +
> +	error = __mms114_read_reg(data, MMS114_INFOMATION, packet_size,
> +			(u8 *)touch);
> +	if (error < 0)
> +		goto out;
> +
> +	for (index = 0; index < touch_size; index++)
> +		mms114_proc_mt(data, touch + index);
> +
> +	input_mt_report_pointer_emulation(data->input_dev, true);
> +	input_sync(data->input_dev);
> +
> +out:
> +	return IRQ_HANDLED;
> +}

Thanks,
Henrik
--
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