Message ID | 20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp3454646vqr; Tue, 6 Jun 2023 07:57:30 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ77xgJm5zspz3IdPMtpZfO1jR6kKNra1iHZC4SDEmS3NxCqhcdN1/xcROJ+hVds0eDjBNhT X-Received: by 2002:a05:622a:256:b0:3f4:e020:63dd with SMTP id c22-20020a05622a025600b003f4e02063ddmr2853635qtx.7.1686063450003; Tue, 06 Jun 2023 07:57:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686063449; cv=none; d=google.com; s=arc-20160816; b=JiNz+9Kb4jcbCiVrvzllXmX8r1IXNyXXtteItySWwJxhaFCzmED+tb0O1gIKEZUjxA xI7bX0keNqp02iURdG2x6Sc+xyJjDTDPHu4e9AXh2kKDpD57odLYk+QUbyeLmBq9fO+N 1X33afEkbhaIs0x37HueVAD/KxDonB+aaf4krq8UVgzRp76B5KWNg8mnDBqFi7HEepm5 DqvizOwuaPfSEOTdaub6e0ONT6GwcA+/nEO/y7MAfmt6Yn5azsIVjkJ77F/bRGFf6Pbn wlwI1PKLtiwSqvRUBmAUhoozp1Y3wnWfk1TOWuXLbny9vh+n6hJPLmA2CwC/I8+VK+4L nY4Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:content-transfer-encoding:mime-version :message-id:date:subject:from:dkim-signature; bh=f3FWgbLaTsMXyxlnHqLG8/53dcD+IHDf7oHMhEF25Eo=; b=Jnf4NTIVO2eEl+gbzjaElsur61IyTZTEiNpIzDTG3jG0UlavPnZ0h7nELb0uVpV3YF f0iZQjUuwKYHsZPG5FgNJ3owEU6L6/W4z+Ud0AuGcaYVxP5G7ViD+YiCXfU82+4Z4cCn 1cRanGfYfcQlmR21YBfq/kSKetTlKsWQby0/bAG0dbeTfxX50FgJOEspx+g0twfXr8CT EapQpFYCSJq2jVAJ6D9LOxzw3KY/MCRYyam606U5vyXQQHSNlTDDU264kNM84qZDCx7o L18Fw3ilI9rh4UsWtQIVqr2Mv0nloUne8vFaLzFiDqrH0Aq6S+5kMOC4RF+mbzmbR/YW LXLA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=XZzIrRVu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id v11-20020a05622a130b00b003ef49fe5460si6368508qtk.765.2023.06.06.07.57.15; Tue, 06 Jun 2023 07:57:29 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=XZzIrRVu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237889AbjFFOcP (ORCPT <rfc822;xxoosimple@gmail.com> + 99 others); Tue, 6 Jun 2023 10:32:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36714 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232596AbjFFOcN (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 6 Jun 2023 10:32:13 -0400 Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C25A810F4 for <linux-kernel@vger.kernel.org>; Tue, 6 Jun 2023 07:32:03 -0700 (PDT) Received: by mail-wm1-x329.google.com with SMTP id 5b1f17b1804b1-3f6d7abe9a4so53991415e9.2 for <linux-kernel@vger.kernel.org>; Tue, 06 Jun 2023 07:32:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1686061922; x=1688653922; h=cc:to:content-transfer-encoding:mime-version:message-id:date :subject:from:from:to:cc:subject:date:message-id:reply-to; bh=f3FWgbLaTsMXyxlnHqLG8/53dcD+IHDf7oHMhEF25Eo=; b=XZzIrRVuLoimzz/NSHevNJnbsurxaneyPqFlR86WSk4yp9MEhtiAEydMWldCXWiZuN x3Tcf/1eGdlJXm0dU+WN42Kedj0XQEJnF21iyQ1rFDHd11xl+MdQotjeTFRR1GnOuhWf ez0CokClYUCEgJvUTBtkLfpmy0p7vaOKwFdhNE7ki4eM6F1uYFTsRSAmy1zyB9D0QPjp uBqDJR8VMBwOlTktEXsLAkK0oWSAabxqFRpo3PTXZPRFiCgqRZcbkwAp5ZSIwOFTikfJ WZCHlBbdYs78gBm5ly5M+6AaLxhP1feeEbU+X35HKRsN18jFUQ5vszbB7iU8AHros0qi +I3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686061922; x=1688653922; h=cc:to:content-transfer-encoding:mime-version:message-id:date :subject:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=f3FWgbLaTsMXyxlnHqLG8/53dcD+IHDf7oHMhEF25Eo=; b=V+GN7KBJgImSYnvEVMsnw/HPq4ZsJ5n7kaX6sNbECeZ5uRDj7gUhbJ0xsL3WYRQbDm 0zVNDtbM92Szy0VkkfcOSQf+3drFLJOp5WFVC9xbknwiSWCZZCb+tDpV+sczUAnvjMbw wJlRhCe340CE43E/m/AUUAc4WbdWm54oZqfd5v5TwMt5NycnpxPgcj5itgUy1bv7JzTn dKH97w7EBz/YYXOUB4A3b5lM3DM3oQuzRFNK+XPPD3N8e7eVObVoP8fW3AiP+xNP2i0c 7muYCz4TCY/V/DY7nxrbDAxmsmahG8/a5UPiujn4Y+QZddsHp68fwiA3ILWj3rkdM5AX I95g== X-Gm-Message-State: AC+VfDzsChyY+7vyyT9kYrR/efm5aBKqrf25cNwpluXMpkNLJGGHT3Q3 3XUi4ctJkNe/4y+NKj08VXREVcQMXiIvwgmcB55eyGqI X-Received: by 2002:a1c:f206:0:b0:3f6:f4b:38bd with SMTP id s6-20020a1cf206000000b003f60f4b38bdmr2346928wmc.8.1686061922174; Tue, 06 Jun 2023 07:32:02 -0700 (PDT) Received: from arrakeen.starnux.net ([2a01:e0a:982:cbb0:52eb:f6ff:feb3:451a]) by smtp.gmail.com with ESMTPSA id fc14-20020a05600c524e00b003f61177faffsm3883600wmb.0.2023.06.06.07.32.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 Jun 2023 07:32:01 -0700 (PDT) From: Neil Armstrong <neil.armstrong@linaro.org> Subject: [PATCH RFC 0/4] input: touchscreen: add initial support for Goodix Berlin touchscreen IC Date: Tue, 06 Jun 2023 16:31:55 +0200 Message-Id: <20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-B4-Tracking: v=1; b=H4sIAFtDf2QC/x2NQQrCMBAAv1L27EKsUqtXwQd4FQ+bdNsuxCRsU imU/t3gceYws0FmFc5wazZQ/kqWGCocDw24mcLEKENlaE17Mp3psMQkDqcYB1nRsnoJuKRclOm DEqQIebR0vXDPrh/PDmrKUma0SsHNNRYW76tMyqOs//cLno87vPf9B/IuzqOQAAAA To: Dmitry Torokhov <dmitry.torokhov@gmail.com>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, Bastien Nocera <hadess@hadess.net>, Hans de Goede <hdegoede@redhat.com>, Henrik Rydberg <rydberg@bitmath.org> Cc: linux-input@vger.kernel.org, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Neil Armstrong <neil.armstrong@linaro.org> X-Mailer: b4 0.12.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=1884; i=neil.armstrong@linaro.org; h=from:subject:message-id; bh=HpTYvEajINY4Y1VMwHDbzKbegLEdvB2suIe1KsahLI8=; b=owEBbQKS/ZANAwAKAXfc29rIyEnRAcsmYgBkf0NfJoMrxdtP6eAK0YQ+0WEhVaRSveQFFXUN5K88 a5CFuFqJAjMEAAEKAB0WIQQ9U8YmyFYF/h30LIt33NvayMhJ0QUCZH9DXwAKCRB33NvayMhJ0avPD/ 9WqTj1r1nreA4R7Q5h2H1dPOCO/bsyB1ANjAkK0TGBAn/I37s9vggSviGHMFGzQr0Ko0jjErdVNpZS 2+TYTfAqPLYfXCeTbTcxFfAoYlzubyiZQKOFinIYyJ0D9NiAohh+V9TmpHCYdpwfC0xUGZURDE36RJ bl/PbaHZBneGgyT9VKsli69Oi6HiKXdteoQPG5EUVxzKWh4D8SsG93YMdONDHRYcpNBP7nIFTNuZxc 1wnrFdqq61MlfKzWzBmKXMhGzeGQhAxr7c7KBOsbYzFqmQ+dZTEHweoAN5U7b4wZrF88/3TWDrAU8n cBxx0l5GBAvpJaDFwkK90Wxcz2jWvWW85b16DWtrKrkpsji8DCHQCHTCtPAAqBkQ1h2HemIukG7edI YKrkj3LHoDZLkWq/t2kPH46azM960ZIX8DnfjZ5ZTA8PdrWBRYAk4iyoPXvyRmUQwHELfSwmCZ8jv6 gZ7NTor8sE82/9A/6RC7L/Z9Ledjv0XjCK3Tl53Mx2JYrdvSQWG0G8eV2SBVSmzeqcPxC0vSXrm9Gt ecderG6nppwMUI735pLaCH3/aR0ydsU4+3uYk2UuRIPtHVZXKX4dJAGngec+xCMrrCkvsMEUYQlmDR HmIyWH9MeuHz9xVd/KD6G+IXEOK/JJRI+IkBMLlNWd4JEmuWSmthDqPxpMbA== X-Developer-Key: i=neil.armstrong@linaro.org; a=openpgp; fpr=89EC3D058446217450F22848169AB7B1A4CFF8AE X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1767965667937954310?= X-GMAIL-MSGID: =?utf-8?q?1767965667937954310?= |
Series |
input: touchscreen: add initial support for Goodix Berlin touchscreen IC
|
|
Message
Neil Armstrong
June 6, 2023, 2:31 p.m. UTC
These touchscreen ICs support SPI, I2C and I3C interface, up to
10 finger touch, stylus and gestures events.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
Support for advanced features like:
- Firmware & config update
- Stylus events
- Gestures events
- Previous revisions support (BerlinA or BerlinB)
is not included in current version.
The current support will work with currently flashed firmware
and config, and bail out if firmware or config aren't flashed yet.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Neil Armstrong (4):
dt-bindings: input: document Goodix Berlin Touchscreen IC
input: touchscreen: add core support for Goodix Berlin Touchscreen IC
input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC
input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC
.../bindings/input/touchscreen/goodix-berlin.yaml | 81 ++
drivers/input/touchscreen/Kconfig | 33 +
drivers/input/touchscreen/Makefile | 3 +
drivers/input/touchscreen/goodix_berlin.h | 228 +++++
drivers/input/touchscreen/goodix_berlin_core.c | 935 +++++++++++++++++++++
drivers/input/touchscreen/goodix_berlin_i2c.c | 76 ++
drivers/input/touchscreen/goodix_berlin_spi.c | 183 ++++
7 files changed, 1539 insertions(+)
---
base-commit: 6db29e14f4fb7bce9eb5290288e71b05c2b0d118
change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c
Best regards,
Comments
Hi Neil, On 6/6/23 16:31, Neil Armstrong wrote: > These touchscreen ICs support SPI, I2C and I3C interface, up to > 10 finger touch, stylus and gestures events. > > This initial driver is derived from the Goodix goodix_ts_berlin > available at [1] and [2] and only supports the GT9916 IC > present on the Qualcomm SM8550 MTP & QRD touch panel. > > The current implementation only supports BerlinD, aka GT9916. > > Support for advanced features like: > - Firmware & config update > - Stylus events > - Gestures events > - Previous revisions support (BerlinA or BerlinB) > is not included in current version. > > The current support will work with currently flashed firmware > and config, and bail out if firmware or config aren't flashed yet. What I'm missing here / in the commit msg of "input: touchscreen: add core support for Goodix Berlin Touchscreen IC" is an explanation why this is a new driver instead of adding support to the existing goodix.c code. I assume you have good reasons for this, but it would be good if you can write the reasons for this down. Regards, Hans > > [1] https://github.com/goodix/goodix_ts_berlin > [2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > Neil Armstrong (4): > dt-bindings: input: document Goodix Berlin Touchscreen IC > input: touchscreen: add core support for Goodix Berlin Touchscreen IC > input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC > input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC > > .../bindings/input/touchscreen/goodix-berlin.yaml | 81 ++ > drivers/input/touchscreen/Kconfig | 33 + > drivers/input/touchscreen/Makefile | 3 + > drivers/input/touchscreen/goodix_berlin.h | 228 +++++ > drivers/input/touchscreen/goodix_berlin_core.c | 935 +++++++++++++++++++++ > drivers/input/touchscreen/goodix_berlin_i2c.c | 76 ++ > drivers/input/touchscreen/goodix_berlin_spi.c | 183 ++++ > 7 files changed, 1539 insertions(+) > --- > base-commit: 6db29e14f4fb7bce9eb5290288e71b05c2b0d118 > change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c > > Best regards,
Hi, On 06/06/2023 17:31, Hans de Goede wrote: > Hi Neil, > > On 6/6/23 16:31, Neil Armstrong wrote: >> These touchscreen ICs support SPI, I2C and I3C interface, up to >> 10 finger touch, stylus and gestures events. >> >> This initial driver is derived from the Goodix goodix_ts_berlin >> available at [1] and [2] and only supports the GT9916 IC >> present on the Qualcomm SM8550 MTP & QRD touch panel. >> >> The current implementation only supports BerlinD, aka GT9916. >> >> Support for advanced features like: >> - Firmware & config update >> - Stylus events >> - Gestures events >> - Previous revisions support (BerlinA or BerlinB) >> is not included in current version. >> >> The current support will work with currently flashed firmware >> and config, and bail out if firmware or config aren't flashed yet. > > What I'm missing here / in the commit msg of > "input: touchscreen: add core support for Goodix Berlin Touchscreen IC" > > is an explanation why this is a new driver instead of adding > support to the existing goodix.c code. > > I assume you have good reasons for this, but it would be good > if you can write the reasons for this down. Sure, should I write it down here and/or update the commit message in a new revision ? Anyway, here's the reasons: - globally the event handling "looks like" the current goodix.c, but again the offsets are again different and none of the register address are the same, and unlike the current support all registers are provided by the "ic_info" structure - while with the current code it *could* be possible to merge it, with a lot of changes, the firmware management looks really different, and it would be really hard to merge. But I may be wrong, and may be misleaded by the goodix driver structure (even if it went through a really heavy cleaning process). Globally it seems they tried to match the "event handling" process of the previous generations, but the firmware interface is completely different. Neil > > Regards, > > Hans > > > >> >> [1] https://github.com/goodix/goodix_ts_berlin >> [2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> Neil Armstrong (4): >> dt-bindings: input: document Goodix Berlin Touchscreen IC >> input: touchscreen: add core support for Goodix Berlin Touchscreen IC >> input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC >> input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC >> >> .../bindings/input/touchscreen/goodix-berlin.yaml | 81 ++ >> drivers/input/touchscreen/Kconfig | 33 + >> drivers/input/touchscreen/Makefile | 3 + >> drivers/input/touchscreen/goodix_berlin.h | 228 +++++ >> drivers/input/touchscreen/goodix_berlin_core.c | 935 +++++++++++++++++++++ >> drivers/input/touchscreen/goodix_berlin_i2c.c | 76 ++ >> drivers/input/touchscreen/goodix_berlin_spi.c | 183 ++++ >> 7 files changed, 1539 insertions(+) >> --- >> base-commit: 6db29e14f4fb7bce9eb5290288e71b05c2b0d118 >> change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c >> >> Best regards, >
On Tue, Jun 06, 2023 at 08:12:04PM +0200, Neil Armstrong wrote: > Hi, > > On 06/06/2023 17:31, Hans de Goede wrote: > > Hi Neil, > > > > On 6/6/23 16:31, Neil Armstrong wrote: > > > These touchscreen ICs support SPI, I2C and I3C interface, up to > > > 10 finger touch, stylus and gestures events. > > > > > > This initial driver is derived from the Goodix goodix_ts_berlin > > > available at [1] and [2] and only supports the GT9916 IC > > > present on the Qualcomm SM8550 MTP & QRD touch panel. > > > > > > The current implementation only supports BerlinD, aka GT9916. > > > > > > Support for advanced features like: > > > - Firmware & config update > > > - Stylus events > > > - Gestures events > > > - Previous revisions support (BerlinA or BerlinB) > > > is not included in current version. > > > > > > The current support will work with currently flashed firmware > > > and config, and bail out if firmware or config aren't flashed yet. > > > > What I'm missing here / in the commit msg of > > "input: touchscreen: add core support for Goodix Berlin Touchscreen IC" > > > > is an explanation why this is a new driver instead of adding > > support to the existing goodix.c code. > > > > I assume you have good reasons for this, but it would be good > > if you can write the reasons for this down. > > Sure, should I write it down here and/or update the commit message in a new revision ? > > Anyway, here's the reasons: > - globally the event handling "looks like" the current goodix.c, but again the offsets > are again different and none of the register address are the same, and unlike the current > support all registers are provided by the "ic_info" structure > - while with the current code it *could* be possible to merge it, with a lot of changes, > the firmware management looks really different, and it would be really hard to merge. > > But I may be wrong, and may be misleaded by the goodix driver structure (even if it > went through a really heavy cleaning process). > > Globally it seems they tried to match the "event handling" process of the previous > generations, but the firmware interface is completely different. It is not unprecedented for drivers to share event processing and implement several ways/generations of firmware update mechanisms. Thanks.
Hi Dmitry, On 06/06/2023 20:44, Dmitry Torokhov wrote: > On Tue, Jun 06, 2023 at 08:12:04PM +0200, Neil Armstrong wrote: >> Hi, >> >> On 06/06/2023 17:31, Hans de Goede wrote: >>> Hi Neil, >>> >>> On 6/6/23 16:31, Neil Armstrong wrote: >>>> These touchscreen ICs support SPI, I2C and I3C interface, up to >>>> 10 finger touch, stylus and gestures events. >>>> >>>> This initial driver is derived from the Goodix goodix_ts_berlin >>>> available at [1] and [2] and only supports the GT9916 IC >>>> present on the Qualcomm SM8550 MTP & QRD touch panel. >>>> >>>> The current implementation only supports BerlinD, aka GT9916. >>>> >>>> Support for advanced features like: >>>> - Firmware & config update >>>> - Stylus events >>>> - Gestures events >>>> - Previous revisions support (BerlinA or BerlinB) >>>> is not included in current version. >>>> >>>> The current support will work with currently flashed firmware >>>> and config, and bail out if firmware or config aren't flashed yet. >>> >>> What I'm missing here / in the commit msg of >>> "input: touchscreen: add core support for Goodix Berlin Touchscreen IC" >>> >>> is an explanation why this is a new driver instead of adding >>> support to the existing goodix.c code. >>> >>> I assume you have good reasons for this, but it would be good >>> if you can write the reasons for this down. >> >> Sure, should I write it down here and/or update the commit message in a new revision ? >> >> Anyway, here's the reasons: >> - globally the event handling "looks like" the current goodix.c, but again the offsets >> are again different and none of the register address are the same, and unlike the current >> support all registers are provided by the "ic_info" structure >> - while with the current code it *could* be possible to merge it, with a lot of changes, >> the firmware management looks really different, and it would be really hard to merge. >> >> But I may be wrong, and may be misleaded by the goodix driver structure (even if it >> went through a really heavy cleaning process). >> >> Globally it seems they tried to match the "event handling" process of the previous >> generations, but the firmware interface is completely different. > > It is not unprecedented for drivers to share event processing and > implement several ways/generations of firmware update mechanisms. Thanks for your reply, I'm perfectly aware of that, this is why I posted this as RFC. If the event handling is vaguely similar, I'm not sure it's worth refactoring the current driver since I do not have the old and current IC datasheet nor HW to check for current support non-regression. What I'm sure is that not a single register address, flag or struct is even close to the current upstream defined ones. Neil > > Thanks. >
Hi, On 6/6/23 20:12, Neil Armstrong wrote: > Hi, > > On 06/06/2023 17:31, Hans de Goede wrote: >> Hi Neil, >> >> On 6/6/23 16:31, Neil Armstrong wrote: >>> These touchscreen ICs support SPI, I2C and I3C interface, up to >>> 10 finger touch, stylus and gestures events. >>> >>> This initial driver is derived from the Goodix goodix_ts_berlin >>> available at [1] and [2] and only supports the GT9916 IC >>> present on the Qualcomm SM8550 MTP & QRD touch panel. >>> >>> The current implementation only supports BerlinD, aka GT9916. >>> >>> Support for advanced features like: >>> - Firmware & config update >>> - Stylus events >>> - Gestures events >>> - Previous revisions support (BerlinA or BerlinB) >>> is not included in current version. >>> >>> The current support will work with currently flashed firmware >>> and config, and bail out if firmware or config aren't flashed yet. >> >> What I'm missing here / in the commit msg of >> "input: touchscreen: add core support for Goodix Berlin Touchscreen IC" >> >> is an explanation why this is a new driver instead of adding >> support to the existing goodix.c code. >> >> I assume you have good reasons for this, but it would be good >> if you can write the reasons for this down. > > Sure, should I write it down here and/or update the commit message in a new revision ? Yes please add this to the commit msg for the next version. > Anyway, here's the reasons: > - globally the event handling "looks like" the current goodix.c, but again the offsets > are again different and none of the register address are the same, and unlike the current > support all registers are provided by the "ic_info" structure > - while with the current code it *could* be possible to merge it, with a lot of changes, > the firmware management looks really different, and it would be really hard to merge. > > But I may be wrong, and may be misleaded by the goodix driver structure (even if it > went through a really heavy cleaning process). No doing a new separate driver sounds about right to me. The current goodix driver already has a lot of different code-paths. So since there does not seem to be a whole lot of code sharing potential adding yet more special case handling / paths is not desirable IMHO. Regards, Hans >>> [1] https://github.com/goodix/goodix_ts_berlin >>> [2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers >>> >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>> --- >>> Neil Armstrong (4): >>> dt-bindings: input: document Goodix Berlin Touchscreen IC >>> input: touchscreen: add core support for Goodix Berlin Touchscreen IC >>> input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC >>> input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC >>> >>> .../bindings/input/touchscreen/goodix-berlin.yaml | 81 ++ >>> drivers/input/touchscreen/Kconfig | 33 + >>> drivers/input/touchscreen/Makefile | 3 + >>> drivers/input/touchscreen/goodix_berlin.h | 228 +++++ >>> drivers/input/touchscreen/goodix_berlin_core.c | 935 +++++++++++++++++++++ >>> drivers/input/touchscreen/goodix_berlin_i2c.c | 76 ++ >>> drivers/input/touchscreen/goodix_berlin_spi.c | 183 ++++ >>> 7 files changed, 1539 insertions(+) >>> --- >>> base-commit: 6db29e14f4fb7bce9eb5290288e71b05c2b0d118 >>> change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c >>> >>> Best regards, >> >
On Tue, Jun 06, 2023 at 08:55:35PM +0200, Neil Armstrong wrote: > Hi Dmitry, > > On 06/06/2023 20:44, Dmitry Torokhov wrote: > > On Tue, Jun 06, 2023 at 08:12:04PM +0200, Neil Armstrong wrote: > > > Hi, > > > > > > On 06/06/2023 17:31, Hans de Goede wrote: > > > > Hi Neil, > > > > > > > > On 6/6/23 16:31, Neil Armstrong wrote: > > > > > These touchscreen ICs support SPI, I2C and I3C interface, up to > > > > > 10 finger touch, stylus and gestures events. > > > > > > > > > > This initial driver is derived from the Goodix goodix_ts_berlin > > > > > available at [1] and [2] and only supports the GT9916 IC > > > > > present on the Qualcomm SM8550 MTP & QRD touch panel. > > > > > > > > > > The current implementation only supports BerlinD, aka GT9916. > > > > > > > > > > Support for advanced features like: > > > > > - Firmware & config update > > > > > - Stylus events > > > > > - Gestures events > > > > > - Previous revisions support (BerlinA or BerlinB) > > > > > is not included in current version. > > > > > > > > > > The current support will work with currently flashed firmware > > > > > and config, and bail out if firmware or config aren't flashed yet. > > > > > > > > What I'm missing here / in the commit msg of > > > > "input: touchscreen: add core support for Goodix Berlin Touchscreen IC" > > > > > > > > is an explanation why this is a new driver instead of adding > > > > support to the existing goodix.c code. > > > > > > > > I assume you have good reasons for this, but it would be good > > > > if you can write the reasons for this down. > > > > > > Sure, should I write it down here and/or update the commit message in a new revision ? > > > > > > Anyway, here's the reasons: > > > - globally the event handling "looks like" the current goodix.c, but again the offsets > > > are again different and none of the register address are the same, and unlike the current > > > support all registers are provided by the "ic_info" structure > > > - while with the current code it *could* be possible to merge it, with a lot of changes, > > > the firmware management looks really different, and it would be really hard to merge. > > > > > > But I may be wrong, and may be misleaded by the goodix driver structure (even if it > > > went through a really heavy cleaning process). > > > > > > Globally it seems they tried to match the "event handling" process of the previous > > > generations, but the firmware interface is completely different. > > > > It is not unprecedented for drivers to share event processing and > > implement several ways/generations of firmware update mechanisms. > > Thanks for your reply, I'm perfectly aware of that, this is why I posted > this as RFC. > > If the event handling is vaguely similar, I'm not sure it's worth refactoring the > current driver since I do not have the old and current IC datasheet nor > HW to check for current support non-regression. > > What I'm sure is that not a single register address, flag or struct is even close > to the current upstream defined ones. OK, it looks like Hans' preference is also to have a separate driver, so let's keep them separate. Thanks.
Hi! > > Sure, should I write it down here and/or update the commit message in a new revision ? > > Yes please add this to the commit msg for the next version. Actually, putting this into comment in the driver itself might be good. BR, Pavel