Re: [PATCH 1/5] dt-bindings: input: touchscreen: add bindings for focaltech,fts

[Krzysztof Kozlowski]

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