linux-kernel - Re: [PATCH v4 8/9] Input: atmel_mxt_ts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 20 Jun 2016 18:22:43 +0200
From:	Hans Verkuil <hverkuil@...all.nl>
To:	Nick Dyer <nick.dyer@...ev.co.uk>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-media@...r.kernel.org,
	Benjamin Tissoires <benjamin.tissoires@...hat.com>,
	Benson Leung <bleung@...omium.org>,
	Alan Bowens <Alan.Bowens@...el.com>,
	Javier Martinez Canillas <javier@....samsung.com>,
	Chris Healy <cphealy@...il.com>,
	Henrik Rydberg <rydberg@...math.org>,
	Andrew Duggan <aduggan@...aptics.com>,
	James Chen <james.chen@....com.tw>,
	Dudley Du <dudl@...ress.com>,
	Andrew de los Reyes <adlr@...omium.org>,
	sheckylin@...omium.org, Peter Hutterer <peter.hutterer@...-t.net>,
	Florian Echtler <floe@...terbrot.org>, mchehab@....samsung.com
Subject: Re: [PATCH v4 8/9] Input: atmel_mxt_ts - add support for reference
 data

On 06/20/2016 06:18 PM, Nick Dyer wrote:
> On 20/06/2016 17:09, Hans Verkuil wrote:
>> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>>> @@ -2325,11 +2344,20 @@ static int mxt_vidioc_querycap(struct file *file, void *priv,
>>>  static int mxt_vidioc_enum_input(struct file *file, void *priv,
>>>  				   struct v4l2_input *i)
>>>  {
>>> -	if (i->index > 0)
>>> +	if (i->index >= MXT_V4L_INPUT_MAX)
>>>  		return -EINVAL;
>>>  
>>>  	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
>>> -	strlcpy(i->name, "Mutual References", sizeof(i->name));
>>> +
>>> +	switch (i->index) {
>>> +	case MXT_V4L_INPUT_REFS:
>>> +		strlcpy(i->name, "Mutual References", sizeof(i->name));
>>> +		break;
>>> +	case MXT_V4L_INPUT_DELTAS:
>>> +		strlcpy(i->name, "Mutual Deltas", sizeof(i->name));
>>
>> I don't think this name is very clear. I have no idea how to interpret "Mutual"
>> in this context.
> 
> "Mutual" is a touch domain specific term, it means the delta value is for
> the capacitance between the horizontal and vertical lines at a particular
> "node" on the touchscreen matrix, see
> https://en.wikipedia.org/wiki/Touchscreen#Mutual_capacitance
> 
> I'll put in a comment.

As I mentioned in an earlier review, we need a v4l-touch interface description anyway.
I think it might be useful to describe some of these touch-specific terms there.
That way that could be a useful reference for end-users.

Nobody reads comments, but people do read the spec (well, I do).

Regards,

	Hans

> 
>>
>>> +		break;
>>> +	}
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> @@ -2337,12 +2365,16 @@ static int mxt_set_input(struct mxt_data *data, unsigned int i)
>>>  {
>>>  	struct v4l2_pix_format *f = &data->dbg.format;
>>>  
>>> -	if (i > 0)
>>> +	if (i >= MXT_V4L_INPUT_MAX)
>>>  		return -EINVAL;
>>>  
>>> +	if (i == MXT_V4L_INPUT_DELTAS)
>>> +		f->pixelformat = V4L2_PIX_FMT_YS16;
>>> +	else
>>> +		f->pixelformat = V4L2_PIX_FMT_Y16;
>>
>> You probably need a V4L2_TOUCH_FMT_U16 or something for this as well. It certainly
>> needs to be documented.
> 
> OK, will change this.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>