Re: [PATCH v7] Touchscreen driver for FT5x06 based EDT displays

[Simon Budig <simon.budig@kernelconcepts.de>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Henrik.

On 07/02/2012 11:31 AM, Henrik Rydberg wrote:
> Thank you for the thorough set of changes. Some minor comments 
> below.

Will try to address the new ones soonish  :)

>> +	wrbuf[0] = 0xf5; +	wrbuf[1] = 0x0e; +	for (pos = *off; pos < 
>> endpos; pos += colbytes) { +		wrbuf[2] = pos / colbytes;  /* 
>> column index */ +		error = 
>> edt_ft5x06_ts_readwrite(tsdata->client, +						sizeof(wrbuf), 
>> wrbuf, +						colbytes, rdbuf); +		if (error) +			goto out; + + 
>> start_off = pos % colbytes; +		error = copy_to_user(buf + pos - 
>> *off, rdbuf + start_off, +				     MIN(colbytes - start_off, 
>> endpos - pos));
> 
> Quite a few variables in play here... it looks correct, but a) 
> breaking out parts of this function would not hurt, and b) I bet 
> the column-by column copy-to-user algorithm could be slightly less
>  involved.

Hmm, I originally had it implemented as requiring offset == 0 as well
as buffer size big enough. That way it was as easy as the previous
sysfile implementation. I was unsure if this is acceptable though.

Not sure if dealing with the four possible cases independently would
improve things. I can only get the data in column-sized chunks and if
the buffer to fill is not properly aligned to these it will get a bit
messy.

I could allocate a temporary buffer for the whole raw frame, fill it
once (e.g. when offset == 0 or even on open()) and then just copy it
into the output buffer.

Maybe this is a more viable approach.

Bye,
        Simon

- -- 
       Simon Budig                        kernel concepts GmbH
       simon.budig@kernelconcepts.de      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/xcBYACgkQO2O/RXesiHCGhwCeICHFEGfBP0ejWHSDfsFbHRKm
WoEAn3LFFapVRYKokhxtbm56SLxmSIep
=vd6q
-----END PGP SIGNATURE-----