[PATCH v2] Input: ili210x - Set the device name according to the device model

Subsystems: input (keyboard, mouse, joystick, touchscreen) drivers, the rest

STALE1861d

5 messages, 3 authors, 2021-09-08 · open the first message on its own page

[PATCH v2] Input: ili210x - Set the device name according to the device model

From: Rogerio Pimentel <hidden>
Date: 2021-09-03 16:55:17

Adding the device model into the device name is useful when
applications need to set different parameters according to the
touchscreen being used, e.g. X11 calibration points.

Signed-off-by: Rogerio Pimentel <redacted>
---

Changes since v1: Get the device ID from touchscreen controller
instead of driver's device list.

 drivers/input/touchscreen/ili210x.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 199cf3daec10..7a897a03ed70 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c

@@ -19,10 +19,14 @@
 #define ILI251X_DATA_SIZE1	31
 #define ILI251X_DATA_SIZE2	20
 
+#define ILI_NAME_LEN		27
+#define ILITEK_TS_NAME "Ilitek ILI%x%x Touchscreen"
+
 /* Touchscreen commands */
 #define REG_TOUCHDATA		0x10
 #define REG_PANEL_INFO		0x20
 #define REG_CALIBRATE		0xcc
+#define REG_TS_MODEL		0x61
 
 struct ili2xxx_chip {
 	int (*read_reg)(struct i2c_client *client, u8 reg,

@@ -384,6 +388,8 @@ static int ili210x_i2c_probe(struct i2c_client *client,
 	struct input_dev *input;
 	int error;
 	unsigned int max_xy;
+	unsigned char buf[2];
+	char *model_name;
 
 	dev_dbg(dev, "Probing for ILI210X I2C Touschreen driver");

@@ -430,7 +436,10 @@ static int ili210x_i2c_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, priv);
 
 	/* Setup input device */
-	input->name = "ILI210x Touchscreen";
+	input->name = ILITEK_TS_NAME;
+	model_name = (char *)input->name;
+	priv->chip->read_reg(priv->client, REG_TS_MODEL, buf, 2);
+	snprintf(model_name, ILI_NAME_LEN, input->name, buf[1], buf[0]);
 	input->id.bustype = BUS_I2C;
 
 	/* Multi touch */

-- 
2.17.1

Re: [PATCH v2] Input: ili210x - Set the device name according to the device model

From: Marek Vasut <marex@denx.de>
Date: 2021-09-03 17:05:31

On 9/3/21 6:54 PM, Rogerio Pimentel wrote:

quoted hunk

Adding the device model into the device name is useful when
applications need to set different parameters according to the
touchscreen being used, e.g. X11 calibration points.

Signed-off-by: Rogerio Pimentel <redacted>
---

Changes since v1: Get the device ID from touchscreen controller
instead of driver's device list.

  drivers/input/touchscreen/ili210x.c | 11 ++++++++++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 199cf3daec10..7a897a03ed70 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c

@@ -19,10 +19,14 @@
  #define ILI251X_DATA_SIZE1	31
  #define ILI251X_DATA_SIZE2	20
  
+#define ILI_NAME_LEN		27
+#define ILITEK_TS_NAME "Ilitek ILI%x%x Touchscreen"
+
  /* Touchscreen commands */
  #define REG_TOUCHDATA		0x10
  #define REG_PANEL_INFO		0x20
  #define REG_CALIBRATE		0xcc
+#define REG_TS_MODEL		0x61

Have a look at the following patch for the extra register names:
[PATCH v3 2/3] Input: ili210x - export ili251x version details via sysfs

quoted hunk

  struct ili2xxx_chip {
  	int (*read_reg)(struct i2c_client *client, u8 reg,

@@ -384,6 +388,8 @@ static int ili210x_i2c_probe(struct i2c_client *client,
  	struct input_dev *input;
  	int error;
  	unsigned int max_xy;
+	unsigned char buf[2];
+	char *model_name;
  
  	dev_dbg(dev, "Probing for ILI210X I2C Touschreen driver");
  
@@ -430,7 +436,10 @@ static int ili210x_i2c_probe(struct i2c_client *client,
  	i2c_set_clientdata(client, priv);
  
  	/* Setup input device */
-	input->name = "ILI210x Touchscreen";
+	input->name = ILITEK_TS_NAME;
+	model_name = (char *)input->name;
+	priv->chip->read_reg(priv->client, REG_TS_MODEL, buf, 2);

Please check return value of ^ that function.

+	snprintf(model_name, ILI_NAME_LEN, input->name, buf[1], buf[0]);
  	input->id.bustype = BUS_I2C;

This was still tested only on ili2511, right ?

Re: [PATCH v2] Input: ili210x - Set the device name according to the device model

From: Rogerio Pimentel <hidden>
Date: 2021-09-03 17:53:21

On Fri, Sep 3, 2021 at 2:05 PM Marek Vasut [off-list ref] wrote:

On 9/3/21 6:54 PM, Rogerio Pimentel wrote:

quoted

Adding the device model into the device name is useful when
applications need to set different parameters according to the
touchscreen being used, e.g. X11 calibration points.

Signed-off-by: Rogerio Pimentel <redacted>
---

Changes since v1: Get the device ID from touchscreen controller
instead of driver's device list.

  drivers/input/touchscreen/ili210x.c | 11 ++++++++++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 199cf3daec10..7a897a03ed70 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c

@@ -19,10 +19,14 @@
  #define ILI251X_DATA_SIZE1  31
  #define ILI251X_DATA_SIZE2  20

+#define ILI_NAME_LEN         27
+#define ILITEK_TS_NAME "Ilitek ILI%x%x Touchscreen"
+
  /* Touchscreen commands */
  #define REG_TOUCHDATA               0x10
  #define REG_PANEL_INFO              0x20
  #define REG_CALIBRATE               0xcc
+#define REG_TS_MODEL         0x61

Have a look at the following patch for the extra register names:
[PATCH v3 2/3] Input: ili210x - export ili251x version details via sysfs

Ok, I will.

quoted

  struct ili2xxx_chip {
      int (*read_reg)(struct i2c_client *client, u8 reg,

@@ -384,6 +388,8 @@ static int ili210x_i2c_probe(struct i2c_client *client,
      struct input_dev *input;
      int error;
      unsigned int max_xy;
+     unsigned char buf[2];
+     char *model_name;

      dev_dbg(dev, "Probing for ILI210X I2C Touschreen driver");
@@ -430,7 +436,10 @@ static int ili210x_i2c_probe(struct i2c_client *client,
      i2c_set_clientdata(client, priv);

      /* Setup input device */
-     input->name = "ILI210x Touchscreen";
+     input->name = ILITEK_TS_NAME;
+     model_name = (char *)input->name;
+     priv->chip->read_reg(priv->client, REG_TS_MODEL, buf, 2);

Please check return value of ^ that function.

Ok, I will make the changes.

quoted

+     snprintf(model_name, ILI_NAME_LEN, input->name, buf[1], buf[0]);
      input->id.bustype = BUS_I2C;

This was still tested only on ili2511, right ?

Yes, by now I have only tested ili2511.

Re: [PATCH v2] Input: ili210x - Set the device name according to the device model

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-09-04 06:17:12

Hi Rogerio,

On Fri, Sep 03, 2021 at 01:54:48PM -0300, Rogerio Pimentel wrote:

Adding the device model into the device name is useful when
applications need to set different parameters according to the
touchscreen being used, e.g. X11 calibration points.

Typically model would go into input->id.product and optionally
input->id.version.

quoted hunk

Signed-off-by: Rogerio Pimentel <redacted>
---

Changes since v1: Get the device ID from touchscreen controller
instead of driver's device list.

 drivers/input/touchscreen/ili210x.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 199cf3daec10..7a897a03ed70 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c

@@ -19,10 +19,14 @@
 #define ILI251X_DATA_SIZE1	31
 #define ILI251X_DATA_SIZE2	20
 
+#define ILI_NAME_LEN		27
+#define ILITEK_TS_NAME "Ilitek ILI%x%x Touchscreen"
+
 /* Touchscreen commands */
 #define REG_TOUCHDATA		0x10
 #define REG_PANEL_INFO		0x20
 #define REG_CALIBRATE		0xcc
+#define REG_TS_MODEL		0x61
 
 struct ili2xxx_chip {
 	int (*read_reg)(struct i2c_client *client, u8 reg,

@@ -384,6 +388,8 @@ static int ili210x_i2c_probe(struct i2c_client *client,
 	struct input_dev *input;
 	int error;
 	unsigned int max_xy;
+	unsigned char buf[2];
+	char *model_name;
 
 	dev_dbg(dev, "Probing for ILI210X I2C Touschreen driver");

@@ -430,7 +436,10 @@ static int ili210x_i2c_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, priv);
 
 	/* Setup input device */
-	input->name = "ILI210x Touchscreen";
+	input->name = ILITEK_TS_NAME;
+	model_name = (char *)input->name;

Umm, no. Smashing RO data is not nice.

+	priv->chip->read_reg(priv->client, REG_TS_MODEL, buf, 2);
+	snprintf(model_name, ILI_NAME_LEN, input->name, buf[1], buf[0]);
 	input->id.bustype = BUS_I2C;
 
 	/* Multi touch */
-- 
2.17.1

Thanks.

-- 
Dmitry

Re: [PATCH v2] Input: ili210x - Set the device name according to the device model

From: Rogerio Pimentel <hidden>
Date: 2021-09-08 15:18:45

On Sat, Sep 4, 2021 at 3:17 AM Dmitry Torokhov
[off-list ref] wrote:

Hi Rogerio,

On Fri, Sep 03, 2021 at 01:54:48PM -0300, Rogerio Pimentel wrote:

quoted

Adding the device model into the device name is useful when
applications need to set different parameters according to the
touchscreen being used, e.g. X11 calibration points.

Typically model would go into input->id.product and optionally
input->id.version.

quoted

Signed-off-by: Rogerio Pimentel <redacted>
---

Changes since v1: Get the device ID from touchscreen controller
instead of driver's device list.

 drivers/input/touchscreen/ili210x.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 199cf3daec10..7a897a03ed70 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c

@@ -19,10 +19,14 @@
 #define ILI251X_DATA_SIZE1   31
 #define ILI251X_DATA_SIZE2   20

+#define ILI_NAME_LEN         27
+#define ILITEK_TS_NAME "Ilitek ILI%x%x Touchscreen"
+
 /* Touchscreen commands */
 #define REG_TOUCHDATA                0x10
 #define REG_PANEL_INFO               0x20
 #define REG_CALIBRATE                0xcc
+#define REG_TS_MODEL         0x61

 struct ili2xxx_chip {
      int (*read_reg)(struct i2c_client *client, u8 reg,

@@ -384,6 +388,8 @@ static int ili210x_i2c_probe(struct i2c_client *client,
      struct input_dev *input;
      int error;
      unsigned int max_xy;
+     unsigned char buf[2];
+     char *model_name;

      dev_dbg(dev, "Probing for ILI210X I2C Touschreen driver");

@@ -430,7 +436,10 @@ static int ili210x_i2c_probe(struct i2c_client *client,
      i2c_set_clientdata(client, priv);

      /* Setup input device */
-     input->name = "ILI210x Touchscreen";
+     input->name = ILITEK_TS_NAME;
+     model_name = (char *)input->name;

Umm, no. Smashing RO data is not nice.

quoted

+     priv->chip->read_reg(priv->client, REG_TS_MODEL, buf, 2);
+     snprintf(model_name, ILI_NAME_LEN, input->name, buf[1], buf[0]);
      input->id.bustype = BUS_I2C;

      /* Multi touch */
--
2.17.1

Thanks.

I agree with the comments.
Please, discard this patch.

Rogerio

--
Dmitry

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help