* [PATCH v2 0/2] Add ili9882t timing @ 2023-06-05 6:05 Cong Yang 2023-06-05 6:05 ` [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip Cong Yang 2023-06-05 6:05 ` [PATCH v2 2/2] HID: i2c-hid: elan: Add ili9882t timing Cong Yang 0 siblings, 2 replies; 12+ messages in thread From: Cong Yang @ 2023-06-05 6:05 UTC (permalink / raw) To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi Cc: linux-input, devicetree, linux-kernel, Cong Yang Add ili9882t dt-bindings and timing Changes in v2: - PATCH 1/2: fix ran make dt_binding_check warnings/errors. - PATCH 1/2: remove oneOf,just enum. - Link to v1: https://lore.kernel.org/all/20230602140948.2138668-1-yangcong5@huaqin.corp-partner.google.com/ Cong Yang (2): dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip HID: i2c-hid: elan: Add ili9882t timing .../bindings/input/elan,ekth6915.yaml | 23 ++++++++++++++++--- drivers/hid/i2c-hid/i2c-hid-of-elan.c | 20 ++++++++++++---- 2 files changed, 36 insertions(+), 7 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-05 6:05 [PATCH v2 0/2] Add ili9882t timing Cong Yang @ 2023-06-05 6:05 ` Cong Yang 2023-06-05 10:20 ` Conor Dooley 2023-06-05 10:34 ` Krzysztof Kozlowski 2023-06-05 6:05 ` [PATCH v2 2/2] HID: i2c-hid: elan: Add ili9882t timing Cong Yang 1 sibling, 2 replies; 12+ messages in thread From: Cong Yang @ 2023-06-05 6:05 UTC (permalink / raw) To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi Cc: linux-input, devicetree, linux-kernel, Cong Yang Add an ilitek touch screen chip ili9882t. Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> --- .../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. 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. required: - compatible @@ -41,6 +46,18 @@ required: - interrupts - vcc33-supply +if: + properties: + compatible: + contains: + const: ilitek,ili9882t +then: + required: + - compatible + - reg + - interrupts + - vccio-supply + additionalProperties: false examples: -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-05 6:05 ` [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip Cong Yang @ 2023-06-05 10:20 ` Conor Dooley 2023-06-06 2:06 ` cong yang 2023-06-05 10:34 ` Krzysztof Kozlowski 1 sibling, 1 reply; 12+ messages in thread From: Conor Dooley @ 2023-06-05 10:20 UTC (permalink / raw) To: Cong Yang Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi, linux-input, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2208 bytes --] Hey Cong Yang, On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote: > Add an ilitek touch screen chip ili9882t. Could you add a comment here mentioning the relationship between these chips? On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote: > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > --- > .../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 Is 0x10 only valid for the elan,ekth6915 & 0x41 for the ilitek one? If so, please add some enforcement of the values based on the compatible. > > 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. > > 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. There's no need for these sort of comments, you can rely on the required sections to describe these relationships. Cheers, Conor. > > required: > - compatible > @@ -41,6 +46,18 @@ required: > - interrupts > - vcc33-supply > > +if: > + properties: > + compatible: > + contains: > + const: ilitek,ili9882t > +then: > + required: > + - compatible > + - reg > + - interrupts > + - vccio-supply > + > additionalProperties: false > > examples: > -- > 2.25.1 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-05 10:20 ` Conor Dooley @ 2023-06-06 2:06 ` cong yang 2023-06-06 18:47 ` Dmitry Torokhov 2023-06-09 22:03 ` Rob Herring 0 siblings, 2 replies; 12+ messages in thread From: cong yang @ 2023-06-06 2:06 UTC (permalink / raw) To: Conor Dooley Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi, linux-input, devicetree, linux-kernel Hi,Conor, On Mon, Jun 5, 2023 at 6:20 PM Conor Dooley <conor@kernel.org> wrote: > > Hey Cong Yang, > > On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote: > > Add an ilitek touch screen chip ili9882t. > > Could you add a comment here mentioning the relationship between these > chips? Okay, I will add in V3 version. > On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote: > > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > > --- > > .../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 > > Is 0x10 only valid for the elan,ekth6915 & 0x41 for the ilitek one? > If so, please add some enforcement of the values based on the > compatible. I don't think 0x10 is the only address for ekth6915,(nor is 0x41 the only address for ili9882t). It depends on the hardware design. > > > > > 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. > > > > 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. > > There's no need for these sort of comments, you can rely on the required > sections to describe these relationships. Got it ,thanks. > > Cheers, > Conor. > > > > > required: > > - compatible > > @@ -41,6 +46,18 @@ required: > > - interrupts > > - vcc33-supply > > > > +if: > > + properties: > > + compatible: > > + contains: > > + const: ilitek,ili9882t > > +then: > > + required: > > + - compatible > > + - reg > > + - interrupts > > + - vccio-supply > > + > > additionalProperties: false > > > > examples: > > -- > > 2.25.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-06 2:06 ` cong yang @ 2023-06-06 18:47 ` Dmitry Torokhov 2023-06-09 22:03 ` Rob Herring 1 sibling, 0 replies; 12+ messages in thread From: Dmitry Torokhov @ 2023-06-06 18:47 UTC (permalink / raw) To: cong yang Cc: Conor Dooley, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos, benjamin.tissoires, dianders, hsinyi, linux-input, devicetree, linux-kernel On Tue, Jun 06, 2023 at 10:06:05AM +0800, cong yang wrote: > Hi,Conor, > > On Mon, Jun 5, 2023 at 6:20 PM Conor Dooley <conor@kernel.org> wrote: > > > > Hey Cong Yang, > > > > On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote: > > > Add an ilitek touch screen chip ili9882t. > > > > Could you add a comment here mentioning the relationship between these > > chips? > > Okay, I will add in V3 version. > > > On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote: > > > > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > > > --- > > > .../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 > > > > Is 0x10 only valid for the elan,ekth6915 & 0x41 for the ilitek one? > > If so, please add some enforcement of the values based on the > > compatible. > > I don't think 0x10 is the only address for ekth6915,(nor is 0x41 the > only address for ili9882t). It depends on the hardware design. Only a handful of controllers allow switching between addresses, and if they do they typically have a "main" and an "alternate" one, and not something completely customizable. I do not believe Elan offers ways to program different address, do they? Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-06 2:06 ` cong yang 2023-06-06 18:47 ` Dmitry Torokhov @ 2023-06-09 22:03 ` Rob Herring 1 sibling, 0 replies; 12+ messages in thread From: Rob Herring @ 2023-06-09 22:03 UTC (permalink / raw) To: cong yang Cc: Conor Dooley, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi, linux-input, devicetree, linux-kernel On Tue, Jun 06, 2023 at 10:06:05AM +0800, cong yang wrote: > Hi,Conor, > > On Mon, Jun 5, 2023 at 6:20 PM Conor Dooley <conor@kernel.org> wrote: > > > > Hey Cong Yang, > > > > On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote: > > > Add an ilitek touch screen chip ili9882t. > > > > Could you add a comment here mentioning the relationship between these > > chips? > > Okay, I will add in V3 version. > > > On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote: > > > > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > > > --- > > > .../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 > > > > Is 0x10 only valid for the elan,ekth6915 & 0x41 for the ilitek one? > > If so, please add some enforcement of the values based on the > > compatible. > > I don't think 0x10 is the only address for ekth6915,(nor is 0x41 the > only address for ili9882t). It depends on the hardware design. I'd just drop the values as we don't typically enforce 'reg' values. Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-05 6:05 ` [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip Cong Yang 2023-06-05 10:20 ` Conor Dooley @ 2023-06-05 10:34 ` Krzysztof Kozlowski 2023-06-06 2:18 ` cong yang 2023-06-06 18:51 ` Dmitry Torokhov 1 sibling, 2 replies; 12+ messages in thread From: Krzysztof Kozlowski @ 2023-06-05 10:34 UTC (permalink / raw) To: Cong Yang, robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi Cc: linux-input, devicetree, linux-kernel On 05/06/2023 08:05, Cong Yang wrote: > Add an ilitek touch screen chip ili9882t. > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > --- > .../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. > > 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. > > required: > - compatible > @@ -41,6 +46,18 @@ required: > - interrupts > - vcc33-supply > > +if: Keep it in allOf. Will save you one indentation later. > + properties: > + compatible: > + contains: > + const: ilitek,ili9882t > +then: > + required: > + - compatible > + - reg > + - interrupts Don't duplicate. > + - vccio-supply Best regards, Krzysztof ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-05 10:34 ` Krzysztof Kozlowski @ 2023-06-06 2:18 ` cong yang 2023-06-06 6:21 ` Krzysztof Kozlowski 2023-06-06 18:51 ` Dmitry Torokhov 1 sibling, 1 reply; 12+ messages in thread From: cong yang @ 2023-06-06 2:18 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi, linux-input, devicetree, linux-kernel Hi,Krzysztof On Mon, Jun 5, 2023 at 6:34 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 05/06/2023 08:05, Cong Yang wrote: > > Add an ilitek touch screen chip ili9882t. > > > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > > --- > > .../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 ? > > > > > 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. > > > > required: > > - compatible > > @@ -41,6 +46,18 @@ required: > > - interrupts > > - vcc33-supply > > > > +if: > > Keep it in allOf. Will save you one indentation later. Got it.thanks. > > > + properties: > > + compatible: > > + contains: > > + const: ilitek,ili9882t > > +then: > > + required: > > + - compatible > > + - reg > > + - interrupts > > Don't duplicate. Got it.thanks. > > > + - vccio-supply > > > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-06 2:18 ` cong yang @ 2023-06-06 6:21 ` Krzysztof Kozlowski 0 siblings, 0 replies; 12+ messages in thread From: Krzysztof Kozlowski @ 2023-06-06 6:21 UTC (permalink / raw) To: cong yang Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi, linux-input, devicetree, linux-kernel On 06/06/2023 04:18, cong yang wrote: > Hi,Krzysztof > > On Mon, Jun 5, 2023 at 6:34 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 05/06/2023 08:05, Cong Yang wrote: >>> Add an ilitek touch screen chip ili9882t. >>> >>> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> >>> --- >>> .../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 > ? https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ Best regards, Krzysztof ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-05 10:34 ` Krzysztof Kozlowski 2023-06-06 2:18 ` cong yang @ 2023-06-06 18:51 ` Dmitry Torokhov 2023-06-07 8:55 ` cong yang 1 sibling, 1 reply; 12+ messages in thread From: Dmitry Torokhov @ 2023-06-06 18:51 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Cong Yang, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos, benjamin.tissoires, dianders, hsinyi, linux-input, devicetree, linux-kernel On Mon, Jun 05, 2023 at 12:34:54PM +0200, Krzysztof Kozlowski wrote: > On 05/06/2023 08:05, Cong Yang wrote: > > Add an ilitek touch screen chip ili9882t. > > > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > > --- > > .../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 do not think it makes sense to merge Ilitek and Elan into a single binding. The only thing that they have in common is that we are trying to reuse drivers/hid/i2c-hid/i2c-hid-of-elan.c to handle reset timings (which is also questionable IMO). Maybe if we had a single unified binding for HID-over-I2C touchscreens then combining would make more sense, at least to me... Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip 2023-06-06 18:51 ` Dmitry Torokhov @ 2023-06-07 8:55 ` cong yang 0 siblings, 0 replies; 12+ messages in thread From: cong yang @ 2023-06-07 8:55 UTC (permalink / raw) To: Dmitry Torokhov Cc: Krzysztof Kozlowski, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos, benjamin.tissoires, dianders, hsinyi, linux-input, devicetree, linux-kernel Hi,Dmitry On Wed, Jun 7, 2023 at 2:51 AM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > On Mon, Jun 05, 2023 at 12:34:54PM +0200, Krzysztof Kozlowski wrote: > > On 05/06/2023 08:05, Cong Yang wrote: > > > Add an ilitek touch screen chip ili9882t. > > > > > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > > > --- > > > .../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 do not think it makes sense to merge Ilitek and Elan into a single > binding. The only thing that they have in common is that we are trying > to reuse drivers/hid/i2c-hid/i2c-hid-of-elan.c to handle reset timings > (which is also questionable IMO). > > Maybe if we had a single unified binding for HID-over-I2C touchscreens > then combining would make more sense, at least to me... > Okay, thanks for the suggestion, I will add an ilitek binding. > Thanks. > > -- > Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] HID: i2c-hid: elan: Add ili9882t timing 2023-06-05 6:05 [PATCH v2 0/2] Add ili9882t timing Cong Yang 2023-06-05 6:05 ` [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip Cong Yang @ 2023-06-05 6:05 ` Cong Yang 1 sibling, 0 replies; 12+ messages in thread From: Cong Yang @ 2023-06-05 6:05 UTC (permalink / raw) To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi Cc: linux-input, devicetree, linux-kernel, Cong Yang The ili9882t is a TDDI IC (Touch with Display Driver). The datasheet specifies there should be 60ms between touch SDA sleep and panel RESX. Doug's series[1] allows panels and touchscreens to power on/off together, so we can add the 65 ms delay in i2c_hid_core_suspend before panel_unprepare. [1]: https: //lore.kernel.org/all/20230523193017.4109557-1-dianders@chromium.org/ Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> --- drivers/hid/i2c-hid/i2c-hid-of-elan.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c index 76ddc8be1cbb..411d7ea2725d 100644 --- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c +++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c @@ -18,7 +18,8 @@ #include "i2c-hid.h" struct elan_i2c_hid_chip_data { - unsigned int post_gpio_reset_delay_ms; + unsigned int post_gpio_reset_on_delay_ms; + unsigned int post_gpio_reset_off_delay_ms; unsigned int post_power_delay_ms; u16 hid_descriptor_address; }; @@ -52,8 +53,8 @@ static int elan_i2c_hid_power_up(struct i2chid_ops *ops) msleep(ihid_elan->chip_data->post_power_delay_ms); gpiod_set_value_cansleep(ihid_elan->reset_gpio, 0); - if (ihid_elan->chip_data->post_gpio_reset_delay_ms) - msleep(ihid_elan->chip_data->post_gpio_reset_delay_ms); + if (ihid_elan->chip_data->post_gpio_reset_on_delay_ms) + msleep(ihid_elan->chip_data->post_gpio_reset_on_delay_ms); return 0; } @@ -64,6 +65,9 @@ static void elan_i2c_hid_power_down(struct i2chid_ops *ops) container_of(ops, struct i2c_hid_of_elan, ops); gpiod_set_value_cansleep(ihid_elan->reset_gpio, 1); + if (ihid_elan->chip_data->post_gpio_reset_off_delay_ms) + msleep(ihid_elan->chip_data->post_gpio_reset_off_delay_ms); + regulator_disable(ihid_elan->vccio); regulator_disable(ihid_elan->vcc33); } @@ -101,12 +105,20 @@ static int i2c_hid_of_elan_probe(struct i2c_client *client) static const struct elan_i2c_hid_chip_data elan_ekth6915_chip_data = { .post_power_delay_ms = 1, - .post_gpio_reset_delay_ms = 300, + .post_gpio_reset_on_delay_ms = 300, + .hid_descriptor_address = 0x0001, +}; + +static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = { + .post_power_delay_ms = 1, + .post_gpio_reset_on_delay_ms = 200, + .post_gpio_reset_off_delay_ms = 65, .hid_descriptor_address = 0x0001, }; static const struct of_device_id elan_i2c_hid_of_match[] = { { .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data }, + { .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data }, { } }; MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match); -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-06-09 22:03 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-05 6:05 [PATCH v2 0/2] Add ili9882t timing Cong Yang 2023-06-05 6:05 ` [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip Cong Yang 2023-06-05 10:20 ` Conor Dooley 2023-06-06 2:06 ` cong yang 2023-06-06 18:47 ` Dmitry Torokhov 2023-06-09 22:03 ` Rob Herring 2023-06-05 10:34 ` Krzysztof Kozlowski 2023-06-06 2:18 ` cong yang 2023-06-06 6:21 ` Krzysztof Kozlowski 2023-06-06 18:51 ` Dmitry Torokhov 2023-06-07 8:55 ` cong yang 2023-06-05 6:05 ` [PATCH v2 2/2] HID: i2c-hid: elan: Add ili9882t timing Cong Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).