Received-SPF: pass (google.com: domain of linux-kernel+bounces-186321-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <7d84912f-7bc4-4376-9f13-31fae16013f4@gmail.com>
Date: Wed, 22 May 2024 09:00:37 -0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen
 controller
To: Krzysztof Kozlowski <krzk@kernel.org>, Hans de Goede
 <hdegoede@redhat.com>, Dmitry Torokhov <dmitry.torokhov@gmail.com>,
 Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>,
 Conor Dooley <conor+dt@kernel.org>
Cc: linux-input@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240521-nvt-ts-devicetree-regulator-support-v1-0-8d766c639dca@gmail.com>
 <20240521-nvt-ts-devicetree-regulator-support-v1-1-8d766c639dca@gmail.com>
 <6f22e42d-8a06-4c24-93bd-25b6ac141cea@kernel.org>
Content-Language: en-US
From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
In-Reply-To: <6f22e42d-8a06-4c24-93bd-25b6ac141cea@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi Krzysztof Kozlowski,

On 5/21/24 11:48, Krzysztof Kozlowski wrote:
> On 21/05/2024 14:09, Joel Selvaraj via B4 Relay wrote:
>> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>>
>> Document the Novatek NVT touchscreen driver which is used in devices like
> 
> driver? or device?

touchscreen "controller" would be correct I think. I will fix it in v2.

>> the Xiaomi Poco F1 [1]. Also, include the devictree binding file in the
>> MAINTAINERS file.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9
>>
>> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> ---
>>   .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 ++++++++++++++++++++++
>>   MAINTAINERS                                        |  1 +
>>   2 files changed, 63 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> new file mode 100644
>> index 0000000000000..7839c6a028e4a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/touchscreen/novatek,nvt-ts.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Novatek NVT Touchscreen Controller
>> +
>> +maintainers:
>> +  - Hans de Goede <hdegoede@redhat.com>
>> +
>> +allOf:
>> +  - $ref: touchscreen.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - novatek,nvt-ts
> 
> That's too generic. Looking at your driver change, it is not even needed.
> 
>> +      - novatek,nt36672a-ts
> 
> Eh, we have already panel. Why there is a need for touchscreen binding
> (binding, not driver)?

I am not sure I understand this correctly. Help me a bit here. For 
context, in mainline there is an existing driver for the novatek nvt 
touchscreen controller. The driver did not have devicetree support. It 
only had a i2c_device_id "NVT-ts". I don't know what is the variant of 
that Novatek touchscreen controller. To use the driver in Xiaomi Poco 
F1, I introduced a devicetree compatible for it "novatek,nvt-ts". The 
However, the Novatek touchscreen controller present in Xiaomi Poco F1 is 
"NT36672A" which has a different chip id than the one in existing 
driver. So I created a separate compatible for this touchscreen 
controller variant "novatek,nt36672a-ts". I used compatible data to 
differentiate the two variants. Since there are two variants, I am 
mentioning both here.

Between, the chip_id and wake_type are the only values that changes 
between these two variants. And these are only checked during the probe 
and is not used anywhere else in the code. If we remove this sanity 
check during probing, then there is no need for two variants and we can 
just keep the generic "novatek,nvt-ts".

Kindly let me know what is the correct thing to do here? How this should 
be handled? I will be happy to address it in v2.

>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +
>> +  vcc-supply: true
>> +  iovcc-supply: true
>> +
>> +unevaluatedProperties: false
> 
> This goes after required:

Will fix in v2.

>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
> 
> 
> Best regards,
> Krzysztof

Regards,
Joel Selvaraj