Hi Krzysztof, Thanks for the review! I agree with most of your comments and will fix them in v2. I have a few doubts as discussed below. On 12/03/23 15:47, Krzysztof Kozlowski wrote: > I have doubts you will cover here all possible FTS controllers, so > filename should be more specific, e.g. choose the oldest device compatible. The driver is kind of widely used and can actually support 49 touch panel variants as per the downstream code [1]. With some slight modifications, the other touch panels can be supported too. However, in real world, we have only tested the driver against the two panel we have access to (FT8719 - Poco F1 Phone and FT5452 - Shiftmq6 Phone). Although its very generic and widely used, I agree we don't know that will be the case forever. So I am ok with changing it to more specific one. But I don't think the panel chip number denote which is older and which newer. Shall I just go with focaltech,fts5452, as that's the lowest number panel that we have tested so far and is supported? Or do I just keep it generic as it can potentially support a lot of variants? >> + focaltech,max-touch-number: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: max number of fingers supported > > Why this is not implied from compatible? IOW, why this differs between > boards? Without proper datasheet it is kind of hard to say if this is the maximum supported touch points by hardware or just a vendor specified one. Because, downstream has it as devicetree property and we only know what's set in that from each vendor tree. The FT8719 used in Poco F1 specifies 10 touch points in downstrean devicetree. But, if I specify it as 2, it will still work fine. The FT5452 used in shiftmq6 specifies 5 touch points in downstream devicetree, but we won't know if that is the maximum possible, unless we try to increase it upto 10 and confirm. So, yeah without the datasheet, we will be just kind of assuming that is is the maximum possible number of touch points by the hardware. I am not sure if we wanna hard code that in the driver. Is it okay if we let this configurable? Boards/Phones can use the max touch number their vendor driver points too or if they have a datasheet, they can specify maximum supported one too. Kindly let me know your thoughts on this. [1] https://github.com/LineageOS/android_kernel_xiaomi_sdm845/blob/f1977d9edd01cff3fc9a12e09cd6a5a8052fc8ca/drivers/input/touchscreen/focaltech_touch/focaltech_config.h#L37 > Best regards, > Krzysztof Thanks, Joel
