Hi Markuss, Thanks for the quick review! I agree with most of your comments and will fix them in a v2 soon. I have a few doubts as discussed below. On 12/03/23 15:40, Markuss Broks wrote: > Why is the _ratelimited variant necessary? I assumed in case of the interrupt working, but i2c reads fail for some reason, it would spam a lot of error messages if the user touches the screen continuously, like a swipe up gesture or something. I referred to ad7879 touchscreen's irq handling code [1] and thought it's probably best to do this, to be on the safe side. I will remove this if it's not needed in v2. > Overall, I think it's better to cast the data type to a struct, which > would make this seem with less random. Sorry, I am not sure I got this right. Do you mean I create an array of struct called say "fts_point" that stores the x, y, type, etc. info of all the points, then report it separately. Like similar to something done by the auo-pixcir touchscreen driver [2]? If I didn't get this correctly, can you show me some code in mainline, that does it? It would be very helpful. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/ad7879.c?h=v6.3-rc1#n250 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/auo-pixcir-ts.c?h=v6.3-rc1#n162 > - Markuss Thanks, Joel