* Re: [PATCH 2/2] Input: goodix_berlin - Add support for Berlin-A series
2025-02-03 17:43 ` [PATCH 2/2] Input: goodix_berlin - Add support for Berlin-A series Jens Reidel
@ 2025-02-03 19:55 ` Luca Weiss
2025-02-04 8:25 ` Neil Armstrong
2025-02-04 14:07 ` Dmitry Torokhov
2 siblings, 0 replies; 7+ messages in thread
From: Luca Weiss @ 2025-02-03 19:55 UTC (permalink / raw)
To: Jens Reidel, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bastien Nocera, Hans de Goede, Neil Armstrong
Cc: linux-input, devicetree, linux-kernel, phone-devel, linux,
~postmarketos/upstreaming
On Mon Feb 3, 2025 at 6:43 PM CET, 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.
Thanks for sending this, works fine on Fairphone 5 on v6.14-rc1 kernel,
after also applying these general fixes:
https://lore.kernel.org/linux-input/20250103-goodix-berlin-fixes-v1-0-b014737b08b2@fairphone.com/
Tested-by: Luca Weiss <luca.weiss@fairphone.com>
Regards
Luca
>
> Signed-off-by: Jens Reidel <adrian@travitia.xyz>
> ---
> drivers/input/touchscreen/goodix_berlin.h | 9 ++++
> .../input/touchscreen/goodix_berlin_core.c | 27 +++++++++---
> drivers/input/touchscreen/goodix_berlin_i2c.c | 6 ++-
> drivers/input/touchscreen/goodix_berlin_spi.c | 44 +++++++++++++++----
> 4 files changed, 72 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
> index 38b6f9ddbdef..a5232e58c166 100644
> --- a/drivers/input/touchscreen/goodix_berlin.h
> +++ b/drivers/input/touchscreen/goodix_berlin.h
> @@ -12,6 +12,15 @@
>
> #include <linux/pm.h>
>
> +enum goodix_berlin_ic_type {
> + IC_TYPE_BERLIN_A,
> + IC_TYPE_BERLIN_D,
> +};
> +
> +struct goodix_berlin_ic_data {
> + enum goodix_berlin_ic_type ic_type;
> +};
> +
> 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..b892ab901d64 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/of.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/sizes.h>
> @@ -54,9 +55,11 @@
> #define GOODIX_BERLIN_DEV_CONFIRM_VAL 0xAA
> #define GOODIX_BERLIN_BOOTOPTION_ADDR 0x10000
> #define GOODIX_BERLIN_FW_VERSION_INFO_ADDR 0x10014
> +#define GOODIX_BERLIN_FW_VERSION_INFO_ADDR_A 0x1000C
>
> #define GOODIX_BERLIN_IC_INFO_MAX_LEN SZ_1K
> #define GOODIX_BERLIN_IC_INFO_ADDR 0x10070
> +#define GOODIX_BERLIN_IC_INFO_ADDR_A 0x10068
>
> #define GOODIX_BERLIN_CHECKSUM_SIZE sizeof(u16)
>
> @@ -297,9 +300,16 @@ 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 = of_device_get_match_data(cd->dev);
> + int fw_version_info_addr;
> int error;
>
> - error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_FW_VERSION_INFO_ADDR,
> + if (ic_data->ic_type == IC_TYPE_BERLIN_A)
> + fw_version_info_addr = GOODIX_BERLIN_FW_VERSION_INFO_ADDR_A;
> + else
> + fw_version_info_addr = GOODIX_BERLIN_FW_VERSION_INFO_ADDR;
> +
> + error = regmap_raw_read(cd->regmap, 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,16 +368,23 @@ 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 = of_device_get_match_data(cd->dev);
> u8 *afe_data __free(kfree) = NULL;
> __le16 length_raw;
> u16 length;
> + int ic_info_addr;
> int error;
>
> + if (ic_data->ic_type == IC_TYPE_BERLIN_A)
> + ic_info_addr = GOODIX_BERLIN_IC_INFO_ADDR_A;
> + else
> + ic_info_addr = GOODIX_BERLIN_IC_INFO_ADDR;
> +
> afe_data = kzalloc(GOODIX_BERLIN_IC_INFO_MAX_LEN, GFP_KERNEL);
> if (!afe_data)
> return -ENOMEM;
>
> - error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
> + error = regmap_raw_read(cd->regmap, ic_info_addr,
> &length_raw, sizeof(length_raw));
> if (error) {
> dev_err(cd->dev, "failed get ic info length, %d\n", error);
> @@ -380,7 +397,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_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..b3fd063b4242 100644
> --- a/drivers/input/touchscreen/goodix_berlin_i2c.c
> +++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
> @@ -53,8 +53,12 @@ static const struct i2c_device_id goodix_berlin_i2c_id[] = {
>
> MODULE_DEVICE_TABLE(i2c, goodix_berlin_i2c_id);
>
> +static const struct goodix_berlin_ic_data gt9916_data = {
> + .ic_type = IC_TYPE_BERLIN_D,
> +};
> +
> 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..f48f11a76b6d 100644
> --- a/drivers/input/touchscreen/goodix_berlin_spi.c
> +++ b/drivers/input/touchscreen/goodix_berlin_spi.c
> @@ -10,6 +10,7 @@
> #include <linux/unaligned.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/regmap.h>
> #include <linux/spi/spi.h>
> #include <linux/input.h>
> @@ -19,9 +20,13 @@
> #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_DUMMY_LEN_A 4
> #define GOODIX_BERLIN_SPI_READ_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> GOODIX_BERLIN_REGISTER_WIDTH + \
> GOODIX_BERLIN_SPI_READ_DUMMY_LEN)
> +#define GOODIX_BERLIN_SPI_READ_PREFIX_LEN_A (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> + GOODIX_BERLIN_REGISTER_WIDTH + \
> + GOODIX_BERLIN_SPI_READ_DUMMY_LEN_A)
> #define GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> GOODIX_BERLIN_REGISTER_WIDTH)
>
> @@ -33,32 +38,41 @@ 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 = of_device_get_match_data(&spi->dev);
> struct spi_transfer xfers;
> struct spi_message spi_msg;
> const u32 *reg = reg_buf; /* reg is stored as native u32 at start of buffer */
> + ssize_t read_prefix_len, read_dummy_len;
> int error;
>
> + if (ic_data->ic_type == IC_TYPE_BERLIN_A) {
> + read_prefix_len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN_A;
> + read_dummy_len = GOODIX_BERLIN_SPI_READ_DUMMY_LEN_A;
> + } else {
> + read_prefix_len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN;
> + read_dummy_len = GOODIX_BERLIN_SPI_READ_DUMMY_LEN;
> + }
> +
> if (reg_size != GOODIX_BERLIN_REGISTER_WIDTH)
> return -EINVAL;
>
> u8 *buf __free(kfree) =
> - kzalloc(GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size,
> - GFP_KERNEL);
> + kzalloc(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, read_dummy_len);
>
> xfers.tx_buf = buf;
> xfers.rx_buf = buf;
> - xfers.len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size;
> + xfers.len = read_prefix_len + val_size;
> xfers.cs_change = 0;
> spi_message_add_tail(&xfers, &spi_msg);
>
> @@ -68,7 +82,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 + read_prefix_len, val_size);
> return error;
> }
>
> @@ -123,6 +137,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 = of_device_get_match_data(&spi->dev);
> struct regmap_config regmap_config;
> struct regmap *regmap;
> size_t max_size;
> @@ -137,7 +152,10 @@ 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;
> + if (ic_data->ic_type == IC_TYPE_BERLIN_A)
> + regmap_config.max_raw_read = max_size - GOODIX_BERLIN_SPI_READ_PREFIX_LEN_A;
> + else
> + regmap_config.max_raw_read = max_size - GOODIX_BERLIN_SPI_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);
> @@ -153,13 +171,23 @@ static int goodix_berlin_spi_probe(struct spi_device *spi)
> }
>
> static const struct spi_device_id goodix_berlin_spi_ids[] = {
> + { "gt9897" },
> { "gt9916" },
> { },
> };
> MODULE_DEVICE_TABLE(spi, goodix_berlin_spi_ids);
>
> +static const struct goodix_berlin_ic_data gt9897_data = {
> + .ic_type = IC_TYPE_BERLIN_A,
> +};
> +
> +static const struct goodix_berlin_ic_data gt9916_data = {
> + .ic_type = IC_TYPE_BERLIN_D,
> +};
> +
> 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);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] Input: goodix_berlin - Add support for Berlin-A series
2025-02-03 17:43 ` [PATCH 2/2] Input: goodix_berlin - Add support for Berlin-A series Jens Reidel
2025-02-03 19:55 ` Luca Weiss
@ 2025-02-04 8:25 ` Neil Armstrong
2025-02-04 14:07 ` Dmitry Torokhov
2 siblings, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2025-02-04 8:25 UTC (permalink / raw)
To: Jens Reidel, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bastien Nocera, Hans de Goede
Cc: Luca Weiss, linux-input, devicetree, linux-kernel, phone-devel,
linux, ~postmarketos/upstreaming
Hi,
On 03/02/2025 18:43, 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@travitia.xyz>
> ---
> drivers/input/touchscreen/goodix_berlin.h | 9 ++++
> .../input/touchscreen/goodix_berlin_core.c | 27 +++++++++---
> drivers/input/touchscreen/goodix_berlin_i2c.c | 6 ++-
> drivers/input/touchscreen/goodix_berlin_spi.c | 44 +++++++++++++++----
> 4 files changed, 72 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
> index 38b6f9ddbdef..a5232e58c166 100644
> --- a/drivers/input/touchscreen/goodix_berlin.h
> +++ b/drivers/input/touchscreen/goodix_berlin.h
> @@ -12,6 +12,15 @@
>
> #include <linux/pm.h>
>
> +enum goodix_berlin_ic_type {
> + IC_TYPE_BERLIN_A,
> + IC_TYPE_BERLIN_D,
> +};
> +
> +struct goodix_berlin_ic_data {
> + enum goodix_berlin_ic_type ic_type;
> +};
> +
> 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..b892ab901d64 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/of.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/sizes.h>
> @@ -54,9 +55,11 @@
> #define GOODIX_BERLIN_DEV_CONFIRM_VAL 0xAA
> #define GOODIX_BERLIN_BOOTOPTION_ADDR 0x10000
> #define GOODIX_BERLIN_FW_VERSION_INFO_ADDR 0x10014
> +#define GOODIX_BERLIN_FW_VERSION_INFO_ADDR_A 0x1000C
>
> #define GOODIX_BERLIN_IC_INFO_MAX_LEN SZ_1K
> #define GOODIX_BERLIN_IC_INFO_ADDR 0x10070
> +#define GOODIX_BERLIN_IC_INFO_ADDR_A 0x10068
>
> #define GOODIX_BERLIN_CHECKSUM_SIZE sizeof(u16)
>
> @@ -297,9 +300,16 @@ 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 = of_device_get_match_data(cd->dev);
> + int fw_version_info_addr;
> int error;
>
> - error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_FW_VERSION_INFO_ADDR,
> + if (ic_data->ic_type == IC_TYPE_BERLIN_A)
> + fw_version_info_addr = GOODIX_BERLIN_FW_VERSION_INFO_ADDR_A;
> + else
> + fw_version_info_addr = GOODIX_BERLIN_FW_VERSION_INFO_ADDR;
> +
> + error = regmap_raw_read(cd->regmap, 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,16 +368,23 @@ 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 = of_device_get_match_data(cd->dev);
> u8 *afe_data __free(kfree) = NULL;
> __le16 length_raw;
> u16 length;
> + int ic_info_addr;
> int error;
>
> + if (ic_data->ic_type == IC_TYPE_BERLIN_A)
> + ic_info_addr = GOODIX_BERLIN_IC_INFO_ADDR_A;
> + else
> + ic_info_addr = GOODIX_BERLIN_IC_INFO_ADDR;
> +
> afe_data = kzalloc(GOODIX_BERLIN_IC_INFO_MAX_LEN, GFP_KERNEL);
> if (!afe_data)
> return -ENOMEM;
>
> - error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
> + error = regmap_raw_read(cd->regmap, ic_info_addr,
> &length_raw, sizeof(length_raw));
> if (error) {
> dev_err(cd->dev, "failed get ic info length, %d\n", error);
> @@ -380,7 +397,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_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..b3fd063b4242 100644
> --- a/drivers/input/touchscreen/goodix_berlin_i2c.c
> +++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
> @@ -53,8 +53,12 @@ static const struct i2c_device_id goodix_berlin_i2c_id[] = {
>
> MODULE_DEVICE_TABLE(i2c, goodix_berlin_i2c_id);
>
> +static const struct goodix_berlin_ic_data gt9916_data = {
> + .ic_type = IC_TYPE_BERLIN_D,
> +};
> +
> static const struct of_device_id goodix_berlin_i2c_of_match[] = {
> - { .compatible = "goodix,gt9916", },
> + { .compatible = "goodix,gt9916", .data = >9916_data },
Please also update the i2c_device_id table with the driver_data and use i2c_get_match_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..f48f11a76b6d 100644
> --- a/drivers/input/touchscreen/goodix_berlin_spi.c
> +++ b/drivers/input/touchscreen/goodix_berlin_spi.c
> @@ -10,6 +10,7 @@
> #include <linux/unaligned.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/regmap.h>
> #include <linux/spi/spi.h>
> #include <linux/input.h>
> @@ -19,9 +20,13 @@
> #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_DUMMY_LEN_A 4
> #define GOODIX_BERLIN_SPI_READ_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> GOODIX_BERLIN_REGISTER_WIDTH + \
> GOODIX_BERLIN_SPI_READ_DUMMY_LEN)
> +#define GOODIX_BERLIN_SPI_READ_PREFIX_LEN_A (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> + GOODIX_BERLIN_REGISTER_WIDTH + \
> + GOODIX_BERLIN_SPI_READ_DUMMY_LEN_A)
> #define GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
> GOODIX_BERLIN_REGISTER_WIDTH)
>
> @@ -33,32 +38,41 @@ 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 = of_device_get_match_data(&spi->dev);
> struct spi_transfer xfers;
> struct spi_message spi_msg;
> const u32 *reg = reg_buf; /* reg is stored as native u32 at start of buffer */
> + ssize_t read_prefix_len, read_dummy_len;
> int error;
>
> + if (ic_data->ic_type == IC_TYPE_BERLIN_A) {
> + read_prefix_len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN_A;
> + read_dummy_len = GOODIX_BERLIN_SPI_READ_DUMMY_LEN_A;
> + } else {
> + read_prefix_len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN;
> + read_dummy_len = GOODIX_BERLIN_SPI_READ_DUMMY_LEN;
> + }
> +
> if (reg_size != GOODIX_BERLIN_REGISTER_WIDTH)
> return -EINVAL;
>
> u8 *buf __free(kfree) =
> - kzalloc(GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size,
> - GFP_KERNEL);
> + kzalloc(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, read_dummy_len);
>
> xfers.tx_buf = buf;
> xfers.rx_buf = buf;
> - xfers.len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size;
> + xfers.len = read_prefix_len + val_size;
> xfers.cs_change = 0;
> spi_message_add_tail(&xfers, &spi_msg);
>
> @@ -68,7 +82,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 + read_prefix_len, val_size);
> return error;
> }
>
> @@ -123,6 +137,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 = of_device_get_match_data(&spi->dev);
> struct regmap_config regmap_config;
> struct regmap *regmap;
> size_t max_size;
> @@ -137,7 +152,10 @@ 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;
> + if (ic_data->ic_type == IC_TYPE_BERLIN_A)
> + regmap_config.max_raw_read = max_size - GOODIX_BERLIN_SPI_READ_PREFIX_LEN_A;
> + else
> + regmap_config.max_raw_read = max_size - GOODIX_BERLIN_SPI_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);
> @@ -153,13 +171,23 @@ static int goodix_berlin_spi_probe(struct spi_device *spi)
> }
>
> static const struct spi_device_id goodix_berlin_spi_ids[] = {
> + { "gt9897" },
> { "gt9916" },
You may also pass driver_data here aswell and use spi_get_device_match_data() instead.
> { },
> };
> MODULE_DEVICE_TABLE(spi, goodix_berlin_spi_ids);
>
> +static const struct goodix_berlin_ic_data gt9897_data = {
> + .ic_type = IC_TYPE_BERLIN_A,
> +};
> +
> +static const struct goodix_berlin_ic_data gt9916_data = {
> + .ic_type = IC_TYPE_BERLIN_D,
> +};
> +
> 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);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] Input: goodix_berlin - Add support for Berlin-A series
2025-02-03 17:43 ` [PATCH 2/2] Input: goodix_berlin - Add support for Berlin-A series Jens Reidel
2025-02-03 19:55 ` Luca Weiss
2025-02-04 8:25 ` Neil Armstrong
@ 2025-02-04 14:07 ` Dmitry Torokhov
2 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2025-02-04 14:07 UTC (permalink / raw)
To: Jens Reidel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Neil Armstrong, Luca Weiss, linux-input,
devicetree, linux-kernel, phone-devel, linux,
~postmarketos/upstreaming
Hi Jens,
On Mon, Feb 03, 2025 at 06:43:09PM +0100, Jens Reidel wrote:
>
> - error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_FW_VERSION_INFO_ADDR,
> + if (ic_data->ic_type == IC_TYPE_BERLIN_A)
> + fw_version_info_addr = GOODIX_BERLIN_FW_VERSION_INFO_ADDR_A;
> + else
> + fw_version_info_addr = GOODIX_BERLIN_FW_VERSION_INFO_ADDR;
> +
Instead of branching on IC type please have goodix_berlin_ic_data
structure describe differences in the chips (by adding
fw_version_info_addr, ic_info_addr and other fields) and then simply use
them.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 7+ messages in thread