Hi,Krzysztof

On Mon, Jun 5, 2023 at 6:34 PM Krzysztof Kozlowski
[off-list ref] wrote:

On 05/06/2023 08:05, Cong Yang wrote:

quoted

Add an ilitek touch screen chip ili9882t.

Signed-off-by: Cong Yang <redacted>
---
 .../bindings/input/elan,ekth6915.yaml         | 23 ++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
index 05e6f2df604c..f0e7ffdce605 100644
--- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
+++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml

@@ -15,11 +15,14 @@ description:

 properties:
   compatible:
-    items:
-      - const: elan,ekth6915
+    enum:
+      - elan,ekth6915
+      - ilitek,ili9882t

   reg:
-    const: 0x10
+    enum:
+      - 0x10
+      - 0x41

   interrupts:
     maxItems: 1

@@ -29,11 +32,13 @@ properties:

   vcc33-supply:
     description: The 3.3V supply to the touchscreen.
+                 If using ili9882t then this supply will not be needed.

What does it mean "will not be needed"? Describe the hardware, not your
drivers.

I don't think you tested your DTS. Submit DTS users, because I do not
believe you are testing your patches. You already got such comment and I
don't see much of improvements here.

I ran make dt_binding_check in the codebase root directory before
sending the V2 Patch, and there were no errors or warnings (the V1
version run reported some errors). Is there some other way to test DTS
?

quoted

   vccio-supply:
     description:
       The IO supply to the touchscreen. Need not be specified if this is the
       same as the 3.3V supply.
+      If using ili9882t, the IO supply is required.

Don't repeat constraints in free form text.

Got it.thanks.

quoted

 required:
   - compatible

@@ -41,6 +46,18 @@ required:
   - interrupts
   - vcc33-supply

+if:

Keep it in allOf. Will save you one indentation later.

Got it.thanks.

quoted

+  properties:
+    compatible:
+      contains:
+        const: ilitek,ili9882t
+then:
+  required:
+    - compatible
+    - reg
+    - interrupts

Don't duplicate.

Got it.thanks.

quoted

+    - vccio-supply

Best regards,
Krzysztof