Re: [PATCH] Add support for the egalax serial touchscreen driver

[Boszormenyi Zoltan <zboszor@pr.hu>]

Hi,

2015-12-15 20:06 keltezéssel, Stephen Kitt írta:
> Hi,
>
> Thanks for submitting this. I have just a couple of questions...
>
> On Tue, 15 Dec 2015 12:24:32 +0100, Böszörményi Zoltán
> <zboszormenyi@sicom.com> wrote:
>> +static int egalax_init(int fd, unsigned long *id, unsigned long *extra) {
>> +	unsigned char packet_alive_query[3] = { 0x0a, 0x01, 'A' };
>> +	unsigned char packet_fw_ver[3] = { 0x0a, 0x01, 'D' };
>> +	unsigned char packet_ctrl_type[3] = { 0x0a, 0x01, 'E' };
>> +	unsigned char response[128];
>> +
>> +	if (check_egalax_response(fd, packet_alive_query, sizeof(packet_alive_query), response))
>> +		return -1;
>> +
>> +	if (check_egalax_response(fd, packet_fw_ver, sizeof(packet_fw_ver), response))
>> +		return -1;
>> +
>> +	response[(unsigned char)response[1] + 2] = '\0';
>> +	printf("EETI eGalaxTouch firmware: %s\n", &response[3]);
> inputattach is generally silent when everything goes well. I can see how this
> kind of info would be useful though; would you mind simply commenting the
> printf() lines out, and I'll add a verbose mode later on?

Sure, I'll put it under #ifdef 0.

>
>> +
>> +	if (check_egalax_response(fd, packet_ctrl_type, sizeof(packet_ctrl_type), response))
>> +		return -1;
>> +
>> +	response[(unsigned char)response[1] + 2] = '\0';
>> +	printf("EETI eGalaxTouch controller type: %s\n", &response[3]);
> As above.
>
>> +#ifdef SERIO_HAMPSHIRE
>> +{ "--hampshire",   "-ham",         "Hampshire touchscreen",
>> +	B9600, CS8,
>> +	SERIO_HAMPSHIRE,	0x00,   0x00,   0,  NULL },
>> +#endif
> Is this intentional? If so, could you mention it in the commit message?

Yes, it is intentional. I noticed that the SERIO_HAMPSHIRE support
is also missing from inputattach. I don't currently know if the
Hampshire touchscreen needs an init function or not, so it is a
placeholder at the moment. Also, the SERIO_HAMPSHIRE value is
smaller than SERIO_PS2MULT which is unconditionally enabled,
so maybe it doesn't need the #ifdef SERIO_HAMPSHIRE ... #endif
cover at all. I will mention it in the commit message for the v2 patch,
which I will send tomorrow.

Best regards,
Zoltán Böszörményi

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html