| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6ad69aad-417c-479c-b02f-121faefa722b@linaro.org>
Date: Mon, 24 Feb 2025 09:28:54 +0100
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Jens Reidel <adrian@...vitia.xyz>,
Dmitry Torokhov <dmitry.torokhov@...il.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Bastien Nocera <hadess@...ess.net>,
Hans de Goede <hdegoede@...hat.com>
Cc: Luca Weiss <luca.weiss@...rphone.com>, linux-input@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
phone-devel@...r.kernel.org, linux@...nlining.org,
~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH v2 2/2] Input: goodix_berlin - Add support for Berlin-A
series
Hi,
On 14/02/2025 06:29, Jens Reidel wrote:
> The current implementation of the goodix_berlin driver lacks support for
> revisions A and B of the Berlin IC. This change adds support for the
> gt9897 IC, which is a Berlin-A revision part.
>
> The differences between revision D and A are rather minor, a handful of
> address changes and a slightly larger read buffer. They were taken from
> the driver published by Goodix, which does a few more things that don't
> appear to be necessary for the touchscreen to work properly.
>
> Signed-off-by: Jens Reidel <adrian@...vitia.xyz>
> Tested-by: Luca Weiss <luca.weiss@...rphone.com>
> ---
> drivers/input/touchscreen/goodix_berlin.h | 13 ++++++
> .../input/touchscreen/goodix_berlin_core.c | 15 ++++---
> drivers/input/touchscreen/goodix_berlin_i2c.c | 9 +++-
> drivers/input/touchscreen/goodix_berlin_spi.c | 45 ++++++++++++++-----
> 4 files changed, 61 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
> index 38b6f9ddbdef..b186a7fb3586 100644
> --- a/drivers/input/touchscreen/goodix_berlin.h
> +++ b/drivers/input/touchscreen/goodix_berlin.h
> @@ -12,6 +12,19 @@
>
> #include <linux/pm.h>
>
> +#define GOODIX_BERLIN_FW_VERSION_INFO_ADDR_A 0x1000C
> +#define GOODIX_BERLIN_FW_VERSION_INFO_ADDR_D 0x10014
> +
> +#define GOODIX_BERLIN_IC_INFO_ADDR_A 0x10068
> +#define GOODIX_BERLIN_IC_INFO_ADDR_D 0x10070
> +
> +struct goodix_berlin_ic_data {
> + int fw_version_info_addr;
> + int ic_info_addr;
> + ssize_t read_dummy_len;
> + ssize_t read_prefix_len;
> +};
> +
> struct device;
> struct input_id;
> struct regmap;
> diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
> index 3fc03cf0ca23..f9fbde63ab52 100644
> --- a/drivers/input/touchscreen/goodix_berlin_core.c
> +++ b/drivers/input/touchscreen/goodix_berlin_core.c
> @@ -12,7 +12,7 @@
> * to the previous generations.
> *
> * Currently the driver only handles Multitouch events with already
> - * programmed firmware and "config" for "Revision D" Berlin IC.
> + * programmed firmware and "config" for "Revision A/D" Berlin IC.
> *
> * Support is missing for:
> * - ESD Management
> @@ -20,7 +20,7 @@
> * - "Config" update/flashing
> * - Stylus Events
> * - Gesture Events
> - * - Support for older revisions (A & B)
> + * - Support for revision B
> */
>
> #include <linux/bitfield.h>
> @@ -28,6 +28,7 @@
> #include <linux/input.h>
> #include <linux/input/mt.h>
> #include <linux/input/touchscreen.h>
> +#include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/sizes.h>
> @@ -53,10 +54,8 @@
>
> #define GOODIX_BERLIN_DEV_CONFIRM_VAL 0xAA
> #define GOODIX_BERLIN_BOOTOPTION_ADDR 0x10000
> -#define GOODIX_BERLIN_FW_VERSION_INFO_ADDR 0x10014
>
> #define GOODIX_BERLIN_IC_INFO_MAX_LEN SZ_1K
> -#define GOODIX_BERLIN_IC_INFO_ADDR 0x10070
>
> #define GOODIX_BERLIN_CHECKSUM_SIZE sizeof(u16)
>
> @@ -297,9 +296,10 @@ static void goodix_berlin_power_off(struct goodix_berlin_core *cd)
>
> static int goodix_berlin_read_version(struct goodix_berlin_core *cd)
> {
> + const struct goodix_berlin_ic_data *ic_data = device_get_match_data(cd->dev);
This won't fully work, device_get_match_data() will only get the of_device_id match
data, and not the spi_device_id or i2c_device_id ones.
Instead add a const goodix_berlin_ic_data pointer in struct goodix_berlin_core and
store the match data retrieved by spi_get_device_match_data() or i2c_get_match_data()
from the i2c/spi probe and drop any usage of device_get_match_data().
Neil
> int error;
>
> - error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_FW_VERSION_INFO_ADDR,
> + error = regmap_raw_read(cd->regmap, ic_data->fw_version_info_addr,
> &cd->fw_version, sizeof(cd->fw_version));
> if (error) {
> dev_err(cd->dev, "error reading fw version, %d\n", error);
> @@ -358,6 +358,7 @@ static int goodix_berlin_parse_ic_info(struct goodix_berlin_core *cd,
>
> static int goodix_berlin_get_ic_info(struct goodix_berlin_core *cd)
> {
> + const struct goodix_berlin_ic_data *ic_data = device_get_match_data(cd->dev);
> u8 *afe_data __free(kfree) = NULL;
> __le16 length_raw;
> u16 length;
> @@ -367,7 +368,7 @@ static int goodix_berlin_get_ic_info(struct goodix_berlin_core *cd)
> if (!afe_data)
> return -ENOMEM;
>
> - error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
> + error = regmap_raw_read(cd->regmap, ic_data->ic_info_addr,
> &length_raw, sizeof(length_raw));
> if (error) {
> dev_err(cd->dev, "failed get ic info length, %d\n", error);
> @@ -380,7 +381,7 @@ static int goodix_berlin_get_ic_info(struct goodix_berlin_core *cd)
> return -EINVAL;
> }
>
> - error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
> + error = regmap_raw_read(cd->regmap, ic_data->ic_info_addr,
> afe_data, length);
> if (error) {
> dev_err(cd->dev, "failed get ic info data, %d\n", error);
> diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
> index ad7a60d94338..7db234c74db8 100644
> --- a/drivers/input/touchscreen/goodix_berlin_i2c.c
> +++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
> @@ -46,15 +46,20 @@ static int goodix_berlin_i2c_probe(struct i2c_client *client)
> return 0;
> }
>
> +static const struct goodix_berlin_ic_data gt9916_data = {
> + .fw_version_info_addr = GOODIX_BERLIN_FW_VERSION_INFO_ADDR_D,
> + .ic_info_addr = GOODIX_BERLIN_IC_INFO_ADDR_D,
> +};
> +
> static const struct i2c_device_id goodix_berlin_i2c_id[] = {
> - { "gt9916" },
> + { .name = "gt9916", .driver_data = (long)>9916_data },
> { }
> };
>
> MODULE_DEVICE_TABLE(i2c, goodix_berlin_i2c_id);
>
> static const struct of_device_id goodix_berlin_i2c_of_match[] = {
> - { .compatible = "goodix,gt9916", },
> + { .compatible = "goodix,gt9916", .data = >9916_data },
> { }
> };
> MODULE_DEVICE_TABLE(of, goodix_berlin_i2c_of_match);
> diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c
> index 0662e87b8692..a5e77e6585e8 100644
> --- a/drivers/input/touchscreen/goodix_berlin_spi.c
> +++ b/drivers/input/touchscreen/goodix_berlin_spi.c
> @@ -18,10 +18,14 @@
>
> #define GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN 1
> #define GOODIX_BERLIN_REGISTER_WIDTH 4
> -#define GOODIX_BERLIN_SPI_READ_DUMMY_LEN 3
> -#define GOODIX_BERLIN_SPI_READ_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> +#define GOODIX_BERLIN_SPI_READ_DUMMY_LEN_A 4
> +#define GOODIX_BERLIN_SPI_READ_DUMMY_LEN_D 3
> +#define GOODIX_BERLIN_SPI_READ_PREFIX_LEN_A (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> GOODIX_BERLIN_REGISTER_WIDTH + \
> - GOODIX_BERLIN_SPI_READ_DUMMY_LEN)
> + GOODIX_BERLIN_SPI_READ_DUMMY_LEN_A)
> +#define GOODIX_BERLIN_SPI_READ_PREFIX_LEN_D (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> + GOODIX_BERLIN_REGISTER_WIDTH + \
> + GOODIX_BERLIN_SPI_READ_DUMMY_LEN_D)
> #define GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> GOODIX_BERLIN_REGISTER_WIDTH)
>
> @@ -33,6 +37,7 @@ static int goodix_berlin_spi_read(void *context, const void *reg_buf,
> size_t val_size)
> {
> struct spi_device *spi = context;
> + const struct goodix_berlin_ic_data *ic_data = spi_get_device_match_data(spi);
> struct spi_transfer xfers;
> struct spi_message spi_msg;
> const u32 *reg = reg_buf; /* reg is stored as native u32 at start of buffer */
> @@ -42,23 +47,22 @@ static int goodix_berlin_spi_read(void *context, const void *reg_buf,
> return -EINVAL;
>
> u8 *buf __free(kfree) =
> - kzalloc(GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size,
> - GFP_KERNEL);
> + kzalloc(ic_data->read_prefix_len + val_size, GFP_KERNEL);
> if (!buf)
> return -ENOMEM;
>
> spi_message_init(&spi_msg);
> memset(&xfers, 0, sizeof(xfers));
>
> - /* buffer format: 0xF1 + addr(4bytes) + dummy(3bytes) + data */
> + /* buffer format: 0xF1 + addr(4bytes) + dummy(3/4bytes) + data */
> buf[0] = GOODIX_BERLIN_SPI_READ_FLAG;
> put_unaligned_be32(*reg, buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN);
> memset(buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + GOODIX_BERLIN_REGISTER_WIDTH,
> - 0xff, GOODIX_BERLIN_SPI_READ_DUMMY_LEN);
> + 0xff, ic_data->read_dummy_len);
>
> xfers.tx_buf = buf;
> xfers.rx_buf = buf;
> - xfers.len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size;
> + xfers.len = ic_data->read_prefix_len + val_size;
> xfers.cs_change = 0;
> spi_message_add_tail(&xfers, &spi_msg);
>
> @@ -68,7 +72,7 @@ static int goodix_berlin_spi_read(void *context, const void *reg_buf,
> return error;
> }
>
> - memcpy(val_buf, buf + GOODIX_BERLIN_SPI_READ_PREFIX_LEN, val_size);
> + memcpy(val_buf, buf + ic_data->read_prefix_len, val_size);
> return error;
> }
>
> @@ -123,6 +127,7 @@ static const struct input_id goodix_berlin_spi_input_id = {
>
> static int goodix_berlin_spi_probe(struct spi_device *spi)
> {
> + const struct goodix_berlin_ic_data *ic_data = spi_get_device_match_data(spi);
> struct regmap_config regmap_config;
> struct regmap *regmap;
> size_t max_size;
> @@ -137,7 +142,7 @@ static int goodix_berlin_spi_probe(struct spi_device *spi)
> max_size = spi_max_transfer_size(spi);
>
> regmap_config = goodix_berlin_spi_regmap_conf;
> - regmap_config.max_raw_read = max_size - GOODIX_BERLIN_SPI_READ_PREFIX_LEN;
> + regmap_config.max_raw_read = max_size - ic_data->read_prefix_len;
> regmap_config.max_raw_write = max_size - GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN;
>
> regmap = devm_regmap_init(&spi->dev, NULL, spi, ®map_config);
> @@ -152,14 +157,30 @@ static int goodix_berlin_spi_probe(struct spi_device *spi)
> return 0;
> }
>
> +static const struct goodix_berlin_ic_data gt9897_data = {
> + .fw_version_info_addr = GOODIX_BERLIN_FW_VERSION_INFO_ADDR_A,
> + .ic_info_addr = GOODIX_BERLIN_IC_INFO_ADDR_A,
> + .read_dummy_len = GOODIX_BERLIN_SPI_READ_DUMMY_LEN_A,
> + .read_prefix_len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN_A,
> +};
> +
> +static const struct goodix_berlin_ic_data gt9916_data = {
> + .fw_version_info_addr = GOODIX_BERLIN_FW_VERSION_INFO_ADDR_D,
> + .ic_info_addr = GOODIX_BERLIN_IC_INFO_ADDR_D,
> + .read_dummy_len = GOODIX_BERLIN_SPI_READ_DUMMY_LEN_D,
> + .read_prefix_len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN_D,
> +};
> +
> static const struct spi_device_id goodix_berlin_spi_ids[] = {
> - { "gt9916" },
> + { .name = "gt9897", .driver_data = (long)>9897_data },
> + { .name = "gt9916", .driver_data = (long)>9916_data },
> { },
> };
> MODULE_DEVICE_TABLE(spi, goodix_berlin_spi_ids);
>
> static const struct of_device_id goodix_berlin_spi_of_match[] = {
> - { .compatible = "goodix,gt9916", },
> + { .compatible = "goodix,gt9897", .data = >9897_data },
> + { .compatible = "goodix,gt9916", .data = >9916_data },
> { }
> };
> MODULE_DEVICE_TABLE(of, goodix_berlin_spi_of_match);
Powered by blists - more mailing lists