| Message ID | 20230520050649.2494497-3-yangcong5@huaqin.corp-partner.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] HID: i2c-hid: elan: Add ili9882t timing | expand |
Hi, On Fri, May 19, 2023 at 10:07 PM Cong Yang <yangcong5@huaqin.corp-partner.google.com> wrote: > > Add an ilitek touch screen chip ili9882t. > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > --- > .../devicetree/bindings/input/elan,ekth6915.yaml | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) I'm curious about the DT maintainers opinion here. Should this be a new bindings file, or should it be together in the elan file. If nothing else, I think the secondary voltage rail name is wrong. I took a quick peek at a datasheet I found and I don't even see a 3.3V rail going to the ili9882t. That makes it weird to reuse "vcc33-supply" for a second supply... -Doug
On 22/05/2023 17:33, Doug Anderson wrote: > Hi, > > On Fri, May 19, 2023 at 10:07 PM Cong Yang > <yangcong5@huaqin.corp-partner.google.com> wrote: >> >> Add an ilitek touch screen chip ili9882t. >> >> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> >> --- >> .../devicetree/bindings/input/elan,ekth6915.yaml | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) > > I'm curious about the DT maintainers opinion here. Should this be a > new bindings file, or should it be together in the elan file. If > nothing else, I think the secondary voltage rail name is wrong. I took > a quick peek at a datasheet I found and I don't even see a 3.3V rail > going to the ili9882t. That makes it weird to reuse "vcc33-supply" for > a second supply... It's easier if they are CCed... Best regards, Krzysztof
Hi, On Tue, May 30, 2023 at 4:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 22/05/2023 17:33, Doug Anderson wrote: > > Hi, > > > > On Fri, May 19, 2023 at 10:07 PM Cong Yang > > <yangcong5@huaqin.corp-partner.google.com> wrote: > >> > >> Add an ilitek touch screen chip ili9882t. > >> > >> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> > >> --- > >> .../devicetree/bindings/input/elan,ekth6915.yaml | 9 +++++++-- > >> 1 file changed, 7 insertions(+), 2 deletions(-) > > > > I'm curious about the DT maintainers opinion here. Should this be a > > new bindings file, or should it be together in the elan file. If > > nothing else, I think the secondary voltage rail name is wrong. I took > > a quick peek at a datasheet I found and I don't even see a 3.3V rail > > going to the ili9882t. That makes it weird to reuse "vcc33-supply" for > > a second supply... > > It's easier if they are CCed... Crud. I just assumed and didn't check the CC list. Cong: can you resend and make sure you're CCing the people that get_maintainers points at. One way to find that would be: ./scripts/get_maintainer.pl -f Documentation/devicetree/bindings/input/elan,ekth6915.yaml
Hi, On Wed, May 31, 2023 at 8:06 PM cong yang <yangcong5@huaqin.corp-partner.google.com> wrote: > > Thanks, I'll keep an eye on that next time. This patch can be discarded,.After adding this series https://lore.kernel.org/r/20230523193017.4109557-1-dianders@chromium.org， Thanks! I'll see if I can give that series a spin soon and then see how we can make progress to getting it landed. > using ekth6915 also can meet my needs. Even if using ekth6915 can meet your needs, it's still better to actually add the right compatible string. Putting in the device tree that you have an "elan6915" and that you're providing "vcc33" isn't the best when you actually have a different touchscreen and are providing a very different voltage. Adding the proper bindings is definitely preferred. > On Wed, May 31, 2023 at 12:58 AM Doug Anderson <dianders@chromium.org> wrote: >> >> Hi, >> >> On Tue, May 30, 2023 at 4:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >> > >> > On 22/05/2023 17:33, Doug Anderson wrote: >> > > Hi, >> > > >> > > On Fri, May 19, 2023 at 10:07 PM Cong Yang >> > > <yangcong5@huaqin.corp-partner.google.com> wrote: >> > >> >> > >> Add an ilitek touch screen chip ili9882t. >> > >> >> > >> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> >> > >> --- >> > >> .../devicetree/bindings/input/elan,ekth6915.yaml | 9 +++++++-- >> > >> 1 file changed, 7 insertions(+), 2 deletions(-) >> > > >> > > I'm curious about the DT maintainers opinion here. Should this be a >> > > new bindings file, or should it be together in the elan file. If >> > > nothing else, I think the secondary voltage rail name is wrong. I took >> > > a quick peek at a datasheet I found and I don't even see a 3.3V rail >> > > going to the ili9882t. That makes it weird to reuse "vcc33-supply" for >> > > a second supply... >> > >> > It's easier if they are CCed... >> >> Crud. I just assumed and didn't check the CC list. Cong: can you >> resend and make sure you're CCing the people that get_maintainers >> points at. One way to find that would be: >> >> ./scripts/get_maintainer.pl -f >> Documentation/devicetree/bindings/input/elan,ekth6915.yaml
diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml index 05e6f2df604c..29eb63b2360b 100644 --- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml +++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml @@ -15,11 +15,16 @@ description: properties: compatible: - items: + oneOf: - const: elan,ekth6915 + - items: + - const: elan,ekth6915 + - const: ilitek,ili9882t reg: - const: 0x10 + enum: + - 0x10 + - 0x41 interrupts: maxItems: 1
Add an ilitek touch screen chip ili9882t. Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> --- .../devicetree/bindings/input/elan,ekth6915.yaml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)