mbox series

[v2,0/3] Add support for Goodix GT917S touch controller

Message ID 20200227160802.7043-1-icenowy@aosc.io
Headers show
Series Add support for Goodix GT917S touch controller | expand

Message

Icenowy Zheng Feb. 27, 2020, 4:07 p.m. UTC
This patchset introduces support for Goodix GT917S touch controller.

The major difference with other touch controllers from Goodix is that
the ID string is no longer number-only (it contains a 'S'), so an
additional patch is introduced for migrating the ID to a string.

Icenowy Zheng (3):
  dt-bindings: input: touchscreen: add compatible string for Goodix
    GT917S
  Input: goodix - use string-based chip ID
  Input: goodix - Add support for Goodix GT917S

 .../bindings/input/touchscreen/goodix.yaml    |  1 +
 drivers/input/touchscreen/goodix.c            | 63 +++++++++++--------
 2 files changed, 38 insertions(+), 26 deletions(-)

Comments

Bastien Nocera Feb. 27, 2020, 5:50 p.m. UTC | #1
On Fri, 2020-02-28 at 00:08 +0800, Icenowy Zheng wrote:
> For Goodix GT917S chip, the chip ID string is "917S", which contains
> not
> only numbers now.
> 
> Use string-based chip ID in the driver to support this chip and
> further
> chips with alphanumber ID.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> Changes in v2:
> - Used a table to save ID and chip data info.
> - Use strscpy().
> 
>  drivers/input/touchscreen/goodix.c | 61 +++++++++++++++++-----------
> --
>  1 file changed, 35 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index 0403102e807e..dde85b894ca4 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -37,6 +37,11 @@ struct goodix_chip_data {
>  	int (*check_config)(struct goodix_ts_data *, const struct
> firmware *);
>  };
>  
> +struct goodix_chip_id {
> +	const char *id;
> +	const struct goodix_chip_data *data;
> +};
> +
>  struct goodix_ts_data {
>  	struct i2c_client *client;
>  	struct input_dev *input_dev;
> @@ -48,7 +53,7 @@ struct goodix_ts_data {
>  	struct regulator *vddio;
>  	struct gpio_desc *gpiod_int;
>  	struct gpio_desc *gpiod_rst;
> -	u16 id;
> +	char id[5];

Could you please change this "5" into a constant (#define) to make sure
that...
<snip>
> +	strscpy(ts->id, id_str, 5);

...we copy the same length here. Otherwise you can be certain it's
going to cause problems in the future.
Bastien Nocera Feb. 27, 2020, 5:51 p.m. UTC | #2
On Fri, 2020-02-28 at 00:07 +0800, Icenowy Zheng wrote:
> This patchset introduces support for Goodix GT917S touch controller.
> 
> The major difference with other touch controllers from Goodix is that
> the ID string is no longer number-only (it contains a 'S'), so an
> additional patch is introduced for migrating the ID to a string.
> 
> Icenowy Zheng (3):
>   dt-bindings: input: touchscreen: add compatible string for Goodix
>     GT917S
>   Input: goodix - use string-based chip ID

I'd put the dt-bindings patch after this change, but I don't think it
matters much in this case.

>   Input: goodix - Add support for Goodix GT917S

I didn't receive that last patch, whether on my address or the linux-
input@ list.

Cheers
Icenowy Zheng Feb. 28, 2020, 12:44 a.m. UTC | #3
于 2020年2月28日 GMT+08:00 上午1:51:54, Bastien Nocera <hadess@hadess.net> 写到:
>On Fri, 2020-02-28 at 00:07 +0800, Icenowy Zheng wrote:
>> This patchset introduces support for Goodix GT917S touch controller.
>> 
>> The major difference with other touch controllers from Goodix is that
>> the ID string is no longer number-only (it contains a 'S'), so an
>> additional patch is introduced for migrating the ID to a string.
>> 
>> Icenowy Zheng (3):
>>   dt-bindings: input: touchscreen: add compatible string for Goodix
>>     GT917S
>>   Input: goodix - use string-based chip ID
>
>I'd put the dt-bindings patch after this change, but I don't think it
>matters much in this case.
>
>>   Input: goodix - Add support for Goodix GT917S
>
>I didn't receive that last patch, whether on my address or the linux-
>input@ list.

My ISP works poorly recently and it's never sent.

I will sent v3 now including it.

>
>Cheers