On 13/03/2023 01:21, Joel Selvaraj wrote: > 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. Downstream DTS is never a guideline on design of upstream bindings. They violate DT binding rules so many times so much, that I don't treat it as argument. The property does not look board but device specific, so you should infer it from compatible. Best regards, Krzysztof
