From: Martin Kepplinger <[email protected]>
The st1232 driver gains support for the ST1633 controller too; update
the bindings doc accordingly.
Signed-off-by: Martin Kepplinger <[email protected]>
---
.../bindings/input/touchscreen/sitronix-st1232.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
index 64ad48b824a2..e73e826e0f2a 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
@@ -1,7 +1,9 @@
-* Sitronix st1232 touchscreen controller
+* Sitronix st1232 or st1633 touchscreen controller
Required properties:
-- compatible: must be "sitronix,st1232"
+- compatible: must contain one of
+ * "sitronix,st1232"
+ * "sitronix,st1633"
- reg: I2C address of the chip
- interrupts: interrupt to which the chip is connected
--
2.20.1
From: Martin Kepplinger <[email protected]>
This adds myself as an author of the st1232 driver module as Tony's
email address doesn't seem to work anymore.
Signed-off-by: Martin Kepplinger <[email protected]>
---
drivers/input/touchscreen/st1232.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index 19a665d48dad..906b233970aa 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -346,5 +346,6 @@ static struct i2c_driver st1232_ts_driver = {
module_i2c_driver(st1232_ts_driver);
MODULE_AUTHOR("Tony SIM <[email protected]>");
+MODULE_AUTHOR("Martin Kepplinger <[email protected]>");
MODULE_DESCRIPTION("SITRONIX ST1232 Touchscreen Controller Driver");
MODULE_LICENSE("GPL v2");
--
2.20.1
From: Martin Kepplinger <[email protected]>
Add support for the Sitronix ST1633 touchscreen controller to the st1232
driver. A protocol spec can be found here:
http://www.ampdisplay.com/documents/pdf/AM-320480B6TZQW-TC0H.pdf
Signed-off-by: Martin Kepplinger <[email protected]>
---
drivers/input/touchscreen/Kconfig | 6 +-
drivers/input/touchscreen/st1232.c | 122 +++++++++++++++++++++--------
2 files changed, 94 insertions(+), 34 deletions(-)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 068dbbc610fc..7c597a49c265 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1168,11 +1168,11 @@ config TOUCHSCREEN_SIS_I2C
module will be called sis_i2c.
config TOUCHSCREEN_ST1232
- tristate "Sitronix ST1232 touchscreen controllers"
+ tristate "Sitronix ST1232 or ST1633 touchscreen controllers"
depends on I2C
help
- Say Y here if you want to support Sitronix ST1232
- touchscreen controller.
+ Say Y here if you want to support the Sitronix ST1232
+ or ST1633 touchscreen controller.
If unsure, say N.
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index 11ff32c68025..19a665d48dad 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -23,13 +23,15 @@
#include <linux/types.h>
#define ST1232_TS_NAME "st1232-ts"
+#define ST1633_TS_NAME "st1633-ts"
+
+enum {
+ st1232,
+ st1633,
+};
#define MIN_X 0x00
#define MIN_Y 0x00
-#define MAX_X 0x31f /* (800 - 1) */
-#define MAX_Y 0x1df /* (480 - 1) */
-#define MAX_AREA 0xff
-#define MAX_FINGERS 2
struct st1232_ts_finger {
u16 x;
@@ -38,12 +40,24 @@ struct st1232_ts_finger {
bool is_valid;
};
+struct st_chip_info {
+ bool have_z;
+ u16 max_x;
+ u16 max_y;
+ u16 max_area;
+ u16 max_fingers;
+ u8 start_reg;
+};
+
struct st1232_ts_data {
struct i2c_client *client;
struct input_dev *input_dev;
- struct st1232_ts_finger finger[MAX_FINGERS];
struct dev_pm_qos_request low_latency_req;
int reset_gpio;
+ const struct st_chip_info *chip_info;
+ int read_buf_len;
+ u8 *read_buf;
+ struct st1232_ts_finger *finger;
};
static int st1232_ts_read_data(struct st1232_ts_data *ts)
@@ -52,40 +66,35 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts)
struct i2c_client *client = ts->client;
struct i2c_msg msg[2];
int error;
- u8 start_reg;
- u8 buf[10];
+ int i, y;
+ u8 start_reg = ts->chip_info->start_reg;
+ u8 *buf = ts->read_buf;
- /* read touchscreen data from ST1232 */
+ /* read touchscreen data */
msg[0].addr = client->addr;
msg[0].flags = 0;
msg[0].len = 1;
msg[0].buf = &start_reg;
- start_reg = 0x10;
msg[1].addr = ts->client->addr;
msg[1].flags = I2C_M_RD;
- msg[1].len = sizeof(buf);
+ msg[1].len = ts->read_buf_len;
msg[1].buf = buf;
error = i2c_transfer(client->adapter, msg, 2);
if (error < 0)
return error;
- /* get "valid" bits */
- finger[0].is_valid = buf[2] >> 7;
- finger[1].is_valid = buf[5] >> 7;
+ for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) {
+ finger[i].is_valid = buf[i + y] >> 7;
+ if (finger[i].is_valid) {
+ finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1];
+ finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2];
- /* get xy coordinate */
- if (finger[0].is_valid) {
- finger[0].x = ((buf[2] & 0x0070) << 4) | buf[3];
- finger[0].y = ((buf[2] & 0x0007) << 8) | buf[4];
- finger[0].t = buf[8];
- }
-
- if (finger[1].is_valid) {
- finger[1].x = ((buf[5] & 0x0070) << 4) | buf[6];
- finger[1].y = ((buf[5] & 0x0007) << 8) | buf[7];
- finger[1].t = buf[9];
+ /* st1232 includes a z-axis / touch strength */
+ if (ts->chip_info->have_z)
+ finger[i].t = buf[i + 6];
+ }
}
return 0;
@@ -104,11 +113,14 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
goto end;
/* multi touch protocol */
- for (i = 0; i < MAX_FINGERS; i++) {
+ for (i = 0; i < ts->chip_info->max_fingers; i++) {
if (!finger[i].is_valid)
continue;
- input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, finger[i].t);
+ if (ts->chip_info->have_z)
+ input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
+ finger[i].t);
+
input_report_abs(input_dev, ABS_MT_POSITION_X, finger[i].x);
input_report_abs(input_dev, ABS_MT_POSITION_Y, finger[i].y);
input_mt_sync(input_dev);
@@ -142,12 +154,40 @@ static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
gpio_direction_output(ts->reset_gpio, poweron);
}
+static const struct st_chip_info st1232_chip_info = {
+ .have_z = true,
+ .max_x = 0x31f, /* 800 - 1 */
+ .max_y = 0x1df, /* 480 -1 */
+ .max_area = 0xff,
+ .max_fingers = 2,
+ .start_reg = 0x12,
+};
+
+static const struct st_chip_info st1633_chip_info = {
+ .have_z = false,
+ .max_x = 0x13f, /* 320 - 1 */
+ .max_y = 0x1df, /* 480 -1 */
+ .max_area = 0x00,
+ .max_fingers = 5,
+ .start_reg = 0x12,
+};
+
static int st1232_ts_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct st1232_ts_data *ts;
+ struct st1232_ts_finger *finger;
struct input_dev *input_dev;
int error;
+ const struct st_chip_info *match = NULL;
+
+ match = device_get_match_data(&client->dev);
+ if (!match && id)
+ match = (const void *)id->driver_data;
+ if (!match) {
+ dev_err(&client->dev, "unknown device model\n");
+ return -ENODEV;
+ }
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
dev_err(&client->dev, "need I2C_FUNC_I2C\n");
@@ -163,6 +203,19 @@ static int st1232_ts_probe(struct i2c_client *client,
if (!ts)
return -ENOMEM;
+ ts->chip_info = match;
+ ts->finger = devm_kzalloc(&client->dev,
+ sizeof(*finger) * ts->chip_info->max_fingers,
+ GFP_KERNEL);
+ if (!ts->finger)
+ return -ENOMEM;
+
+ /* allocate a buffer according to the number of registers to read */
+ ts->read_buf_len = ts->chip_info->max_fingers * 4;
+ ts->read_buf = devm_kzalloc(&client->dev, ts->read_buf_len, GFP_KERNEL);
+ if (!ts->read_buf)
+ return -ENOMEM;
+
input_dev = devm_input_allocate_device(&client->dev);
if (!input_dev)
return -ENOMEM;
@@ -192,9 +245,14 @@ static int st1232_ts_probe(struct i2c_client *client,
__set_bit(EV_KEY, input_dev->evbit);
__set_bit(EV_ABS, input_dev->evbit);
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_POSITION_X, MIN_X, MAX_X, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_POSITION_Y, MIN_Y, MAX_Y, 0, 0);
+ if (ts->chip_info->have_z)
+ input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
+ ts->chip_info->max_area, 0, 0);
+
+ input_set_abs_params(input_dev, ABS_MT_POSITION_X,
+ MIN_X, ts->chip_info->max_x, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
+ MIN_Y, ts->chip_info->max_y, 0, 0);
error = devm_request_threaded_irq(&client->dev, client->irq,
NULL, st1232_ts_irq_handler,
@@ -261,13 +319,15 @@ static SIMPLE_DEV_PM_OPS(st1232_ts_pm_ops,
st1232_ts_suspend, st1232_ts_resume);
static const struct i2c_device_id st1232_ts_id[] = {
- { ST1232_TS_NAME, 0 },
+ { ST1232_TS_NAME, (unsigned long)&st1232_chip_info },
+ { ST1633_TS_NAME, (unsigned long)&st1633_chip_info },
{ }
};
MODULE_DEVICE_TABLE(i2c, st1232_ts_id);
static const struct of_device_id st1232_ts_dt_ids[] = {
- { .compatible = "sitronix,st1232", },
+ { .compatible = "sitronix,st1232", .data = &st1232_chip_info },
+ { .compatible = "sitronix,st1633", .data = &st1633_chip_info },
{ }
};
MODULE_DEVICE_TABLE(of, st1232_ts_dt_ids);
--
2.20.1
________________________________________
Von: Martin Kepplinger [[email protected]]
Gesendet: Montag, 28. J?nner 2019 09:44
An: [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Kepplinger Martin
Betreff: [PATCH 2/3] Input: st1232 - add support for st1633
From: Martin Kepplinger <[email protected]>
Add support for the Sitronix ST1633 touchscreen controller to the st1232
driver. A protocol spec can be found here:
http://www.ampdisplay.com/documents/pdf/AM-320480B6TZQW-TC0H.pdf
Signed-off-by: Martin Kepplinger <[email protected]>
---
sorry, this is v3 of this series.
anyways, the of_device.h header isn't directly need now neither. thanks Dmitry!
martin
revision history
----------------
v3: implement Dmitry's suggestion for i2c probe from v2 review
v2: use device_get_match_data(), invent an internal "have_z" bool property
v1: initial idea for the change
drivers/input/touchscreen/Kconfig | 6 +-
drivers/input/touchscreen/st1232.c | 122 +++++++++++++++++++++--------
2 files changed, 94 insertions(+), 34 deletions(-)
On Mon, Jan 28, 2019 at 09:44:49AM +0100, Martin Kepplinger wrote:
> From: Martin Kepplinger <[email protected]>
>
> This adds myself as an author of the st1232 driver module as Tony's
> email address doesn't seem to work anymore.
>
> Signed-off-by: Martin Kepplinger <[email protected]>
Applied, thank you.
> ---
> drivers/input/touchscreen/st1232.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
> index 19a665d48dad..906b233970aa 100644
> --- a/drivers/input/touchscreen/st1232.c
> +++ b/drivers/input/touchscreen/st1232.c
> @@ -346,5 +346,6 @@ static struct i2c_driver st1232_ts_driver = {
> module_i2c_driver(st1232_ts_driver);
>
> MODULE_AUTHOR("Tony SIM <[email protected]>");
> +MODULE_AUTHOR("Martin Kepplinger <[email protected]>");
> MODULE_DESCRIPTION("SITRONIX ST1232 Touchscreen Controller Driver");
> MODULE_LICENSE("GPL v2");
> --
> 2.20.1
>
--
Dmitry
Hi Martin,
On Mon, Jan 28, 2019 at 09:44:48AM +0100, Martin Kepplinger wrote:
> From: Martin Kepplinger <[email protected]>
>
> Add support for the Sitronix ST1633 touchscreen controller to the st1232
> driver. A protocol spec can be found here:
> http://www.ampdisplay.com/documents/pdf/AM-320480B6TZQW-TC0H.pdf
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> ---
> drivers/input/touchscreen/Kconfig | 6 +-
> drivers/input/touchscreen/st1232.c | 122 +++++++++++++++++++++--------
> 2 files changed, 94 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 068dbbc610fc..7c597a49c265 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1168,11 +1168,11 @@ config TOUCHSCREEN_SIS_I2C
> module will be called sis_i2c.
>
> config TOUCHSCREEN_ST1232
> - tristate "Sitronix ST1232 touchscreen controllers"
> + tristate "Sitronix ST1232 or ST1633 touchscreen controllers"
> depends on I2C
> help
> - Say Y here if you want to support Sitronix ST1232
> - touchscreen controller.
> + Say Y here if you want to support the Sitronix ST1232
> + or ST1633 touchscreen controller.
>
> If unsure, say N.
>
> diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
> index 11ff32c68025..19a665d48dad 100644
> --- a/drivers/input/touchscreen/st1232.c
> +++ b/drivers/input/touchscreen/st1232.c
> @@ -23,13 +23,15 @@
> #include <linux/types.h>
>
> #define ST1232_TS_NAME "st1232-ts"
> +#define ST1633_TS_NAME "st1633-ts"
> +
> +enum {
> + st1232,
> + st1633,
> +};
This enum seems no longer needed, I dropped it.
>
> #define MIN_X 0x00
> #define MIN_Y 0x00
Given we no longer have constant MAX_X/Y I simply used 0 in
input_set_abs_params() and dropped MIN-X/Y.
> -#define MAX_X 0x31f /* (800 - 1) */
> -#define MAX_Y 0x1df /* (480 - 1) */
> -#define MAX_AREA 0xff
> -#define MAX_FINGERS 2
>
> struct st1232_ts_finger {
> u16 x;
> @@ -38,12 +40,24 @@ struct st1232_ts_finger {
> bool is_valid;
> };
>
> +struct st_chip_info {
> + bool have_z;
> + u16 max_x;
> + u16 max_y;
> + u16 max_area;
> + u16 max_fingers;
> + u8 start_reg;
> +};
> +
> struct st1232_ts_data {
> struct i2c_client *client;
> struct input_dev *input_dev;
> - struct st1232_ts_finger finger[MAX_FINGERS];
> struct dev_pm_qos_request low_latency_req;
> int reset_gpio;
Could you please create an additional patch converting this to gpiod?
Instead of doing of_get_gpio()/gpio_is_valid()/devm_gpio_request() smply
do
ts->reset_gpio = devm_gpiod_get_optional();
if (IS_ERR(ts->reset_gpio)) {
...
}
and later
if (ts->reset_gpio)
...
Looking at the code it looks like reset is as usual active low, so you
will need to invert the logic as gpiod takes care of convertion logical
value to proper level (active low or high).
> + const struct st_chip_info *chip_info;
> + int read_buf_len;
> + u8 *read_buf;
> + struct st1232_ts_finger *finger;
> };
>
> static int st1232_ts_read_data(struct st1232_ts_data *ts)
> @@ -52,40 +66,35 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts)
> struct i2c_client *client = ts->client;
> struct i2c_msg msg[2];
> int error;
> - u8 start_reg;
> - u8 buf[10];
> + int i, y;
> + u8 start_reg = ts->chip_info->start_reg;
> + u8 *buf = ts->read_buf;
>
> - /* read touchscreen data from ST1232 */
> + /* read touchscreen data */
> msg[0].addr = client->addr;
> msg[0].flags = 0;
> msg[0].len = 1;
> msg[0].buf = &start_reg;
> - start_reg = 0x10;
>
> msg[1].addr = ts->client->addr;
> msg[1].flags = I2C_M_RD;
> - msg[1].len = sizeof(buf);
> + msg[1].len = ts->read_buf_len;
> msg[1].buf = buf;
>
> error = i2c_transfer(client->adapter, msg, 2);
> if (error < 0)
> return error;
>
> - /* get "valid" bits */
> - finger[0].is_valid = buf[2] >> 7;
> - finger[1].is_valid = buf[5] >> 7;
> + for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) {
> + finger[i].is_valid = buf[i + y] >> 7;
> + if (finger[i].is_valid) {
> + finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1];
> + finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2];
>
> - /* get xy coordinate */
> - if (finger[0].is_valid) {
> - finger[0].x = ((buf[2] & 0x0070) << 4) | buf[3];
> - finger[0].y = ((buf[2] & 0x0007) << 8) | buf[4];
> - finger[0].t = buf[8];
> - }
> -
> - if (finger[1].is_valid) {
> - finger[1].x = ((buf[5] & 0x0070) << 4) | buf[6];
> - finger[1].y = ((buf[5] & 0x0007) << 8) | buf[7];
> - finger[1].t = buf[9];
> + /* st1232 includes a z-axis / touch strength */
> + if (ts->chip_info->have_z)
> + finger[i].t = buf[i + 6];
> + }
> }
>
> return 0;
> @@ -104,11 +113,14 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
> goto end;
>
> /* multi touch protocol */
> - for (i = 0; i < MAX_FINGERS; i++) {
> + for (i = 0; i < ts->chip_info->max_fingers; i++) {
> if (!finger[i].is_valid)
> continue;
>
> - input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, finger[i].t);
> + if (ts->chip_info->have_z)
> + input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> + finger[i].t);
> +
> input_report_abs(input_dev, ABS_MT_POSITION_X, finger[i].x);
> input_report_abs(input_dev, ABS_MT_POSITION_Y, finger[i].y);
> input_mt_sync(input_dev);
> @@ -142,12 +154,40 @@ static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
> gpio_direction_output(ts->reset_gpio, poweron);
> }
>
> +static const struct st_chip_info st1232_chip_info = {
> + .have_z = true,
> + .max_x = 0x31f, /* 800 - 1 */
> + .max_y = 0x1df, /* 480 -1 */
> + .max_area = 0xff,
> + .max_fingers = 2,
> + .start_reg = 0x12,
> +};
> +
> +static const struct st_chip_info st1633_chip_info = {
> + .have_z = false,
> + .max_x = 0x13f, /* 320 - 1 */
> + .max_y = 0x1df, /* 480 -1 */
> + .max_area = 0x00,
> + .max_fingers = 5,
> + .start_reg = 0x12,
> +};
> +
> static int st1232_ts_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> struct st1232_ts_data *ts;
> + struct st1232_ts_finger *finger;
> struct input_dev *input_dev;
> int error;
> + const struct st_chip_info *match = NULL;
There is no need to initialize with NULL given that we do unconditional
assignment below. I removed initialization.
> +
> + match = device_get_match_data(&client->dev);
> + if (!match && id)
> + match = (const void *)id->driver_data;
> + if (!match) {
> + dev_err(&client->dev, "unknown device model\n");
> + return -ENODEV;
> + }
>
> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> dev_err(&client->dev, "need I2C_FUNC_I2C\n");
> @@ -163,6 +203,19 @@ static int st1232_ts_probe(struct i2c_client *client,
> if (!ts)
> return -ENOMEM;
>
> + ts->chip_info = match;
> + ts->finger = devm_kzalloc(&client->dev,
> + sizeof(*finger) * ts->chip_info->max_fingers,
> + GFP_KERNEL);
I replaced it with devm_kcalloc(&client->dev,
ts->chip_info->max_fingers, sizeof(*finger),
GFP_KERNEL);
and applied.
> + if (!ts->finger)
> + return -ENOMEM;
> +
> + /* allocate a buffer according to the number of registers to read */
> + ts->read_buf_len = ts->chip_info->max_fingers * 4;
> + ts->read_buf = devm_kzalloc(&client->dev, ts->read_buf_len, GFP_KERNEL);
> + if (!ts->read_buf)
> + return -ENOMEM;
> +
> input_dev = devm_input_allocate_device(&client->dev);
> if (!input_dev)
> return -ENOMEM;
> @@ -192,9 +245,14 @@ static int st1232_ts_probe(struct i2c_client *client,
> __set_bit(EV_KEY, input_dev->evbit);
> __set_bit(EV_ABS, input_dev->evbit);
>
> - input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
> - input_set_abs_params(input_dev, ABS_MT_POSITION_X, MIN_X, MAX_X, 0, 0);
> - input_set_abs_params(input_dev, ABS_MT_POSITION_Y, MIN_Y, MAX_Y, 0, 0);
> + if (ts->chip_info->have_z)
> + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
> + ts->chip_info->max_area, 0, 0);
> +
> + input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> + MIN_X, ts->chip_info->max_x, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> + MIN_Y, ts->chip_info->max_y, 0, 0);
>
> error = devm_request_threaded_irq(&client->dev, client->irq,
> NULL, st1232_ts_irq_handler,
> @@ -261,13 +319,15 @@ static SIMPLE_DEV_PM_OPS(st1232_ts_pm_ops,
> st1232_ts_suspend, st1232_ts_resume);
>
> static const struct i2c_device_id st1232_ts_id[] = {
> - { ST1232_TS_NAME, 0 },
> + { ST1232_TS_NAME, (unsigned long)&st1232_chip_info },
> + { ST1633_TS_NAME, (unsigned long)&st1633_chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, st1232_ts_id);
>
> static const struct of_device_id st1232_ts_dt_ids[] = {
> - { .compatible = "sitronix,st1232", },
> + { .compatible = "sitronix,st1232", .data = &st1232_chip_info },
> + { .compatible = "sitronix,st1633", .data = &st1633_chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(of, st1232_ts_dt_ids);
> --
> 2.20.1
>
Thanks.
--
Dmitry
Am 28.01.2019 20:03 schrieb Dmitry Torokhov:
> Hi Martin,
>
> On Mon, Jan 28, 2019 at 09:44:48AM +0100, Martin Kepplinger wrote:
>> From: Martin Kepplinger <[email protected]>
>>
>> Add support for the Sitronix ST1633 touchscreen controller to the
>> st1232
>> driver. A protocol spec can be found here:
>> http://www.ampdisplay.com/documents/pdf/AM-320480B6TZQW-TC0H.pdf
>>
>> Signed-off-by: Martin Kepplinger <[email protected]>
>> ---
>> drivers/input/touchscreen/Kconfig | 6 +-
>> drivers/input/touchscreen/st1232.c | 122
>> +++++++++++++++++++++--------
>> 2 files changed, 94 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/Kconfig
>> b/drivers/input/touchscreen/Kconfig
>> index 068dbbc610fc..7c597a49c265 100644
>> --- a/drivers/input/touchscreen/Kconfig
>> +++ b/drivers/input/touchscreen/Kconfig
>> @@ -1168,11 +1168,11 @@ config TOUCHSCREEN_SIS_I2C
>> module will be called sis_i2c.
>>
>> config TOUCHSCREEN_ST1232
>> - tristate "Sitronix ST1232 touchscreen controllers"
>> + tristate "Sitronix ST1232 or ST1633 touchscreen controllers"
>> depends on I2C
>> help
>> - Say Y here if you want to support Sitronix ST1232
>> - touchscreen controller.
>> + Say Y here if you want to support the Sitronix ST1232
>> + or ST1633 touchscreen controller.
>>
>> If unsure, say N.
>>
>> diff --git a/drivers/input/touchscreen/st1232.c
>> b/drivers/input/touchscreen/st1232.c
>> index 11ff32c68025..19a665d48dad 100644
>> --- a/drivers/input/touchscreen/st1232.c
>> +++ b/drivers/input/touchscreen/st1232.c
>> @@ -23,13 +23,15 @@
>> #include <linux/types.h>
>>
>> #define ST1232_TS_NAME "st1232-ts"
>> +#define ST1633_TS_NAME "st1633-ts"
>> +
>> +enum {
>> + st1232,
>> + st1633,
>> +};
>
> This enum seems no longer needed, I dropped it.
>
>>
>> #define MIN_X 0x00
>> #define MIN_Y 0x00
>
> Given we no longer have constant MAX_X/Y I simply used 0 in
> input_set_abs_params() and dropped MIN-X/Y.
>
>> -#define MAX_X 0x31f /* (800 - 1) */
>> -#define MAX_Y 0x1df /* (480 - 1) */
>> -#define MAX_AREA 0xff
>> -#define MAX_FINGERS 2
>>
>> struct st1232_ts_finger {
>> u16 x;
>> @@ -38,12 +40,24 @@ struct st1232_ts_finger {
>> bool is_valid;
>> };
>>
>> +struct st_chip_info {
>> + bool have_z;
>> + u16 max_x;
>> + u16 max_y;
>> + u16 max_area;
>> + u16 max_fingers;
>> + u8 start_reg;
>> +};
>> +
>> struct st1232_ts_data {
>> struct i2c_client *client;
>> struct input_dev *input_dev;
>> - struct st1232_ts_finger finger[MAX_FINGERS];
>> struct dev_pm_qos_request low_latency_req;
>> int reset_gpio;
>
> Could you please create an additional patch converting this to gpiod?
> Instead of doing of_get_gpio()/gpio_is_valid()/devm_gpio_request()
> smply
> do
>
> ts->reset_gpio = devm_gpiod_get_optional();
> if (IS_ERR(ts->reset_gpio)) {
> ...
> }
>
> and later
>
> if (ts->reset_gpio)
> ...
>
> Looking at the code it looks like reset is as usual active low, so you
> will need to invert the logic as gpiod takes care of convertion logical
> value to proper level (active low or high).
I'll test your applied changes and get back to this tomorrow.
thanks.
>
>> + const struct st_chip_info *chip_info;
>> + int read_buf_len;
>> + u8 *read_buf;
>> + struct st1232_ts_finger *finger;
>> };
>>
>> static int st1232_ts_read_data(struct st1232_ts_data *ts)
>> @@ -52,40 +66,35 @@ static int st1232_ts_read_data(struct
>> st1232_ts_data *ts)
>> struct i2c_client *client = ts->client;
>> struct i2c_msg msg[2];
>> int error;
>> - u8 start_reg;
>> - u8 buf[10];
>> + int i, y;
>> + u8 start_reg = ts->chip_info->start_reg;
>> + u8 *buf = ts->read_buf;
>>
>> - /* read touchscreen data from ST1232 */
>> + /* read touchscreen data */
>> msg[0].addr = client->addr;
>> msg[0].flags = 0;
>> msg[0].len = 1;
>> msg[0].buf = &start_reg;
>> - start_reg = 0x10;
>>
>> msg[1].addr = ts->client->addr;
>> msg[1].flags = I2C_M_RD;
>> - msg[1].len = sizeof(buf);
>> + msg[1].len = ts->read_buf_len;
>> msg[1].buf = buf;
>>
>> error = i2c_transfer(client->adapter, msg, 2);
>> if (error < 0)
>> return error;
>>
>> - /* get "valid" bits */
>> - finger[0].is_valid = buf[2] >> 7;
>> - finger[1].is_valid = buf[5] >> 7;
>> + for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) {
>> + finger[i].is_valid = buf[i + y] >> 7;
>> + if (finger[i].is_valid) {
>> + finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1];
>> + finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2];
>>
>> - /* get xy coordinate */
>> - if (finger[0].is_valid) {
>> - finger[0].x = ((buf[2] & 0x0070) << 4) | buf[3];
>> - finger[0].y = ((buf[2] & 0x0007) << 8) | buf[4];
>> - finger[0].t = buf[8];
>> - }
>> -
>> - if (finger[1].is_valid) {
>> - finger[1].x = ((buf[5] & 0x0070) << 4) | buf[6];
>> - finger[1].y = ((buf[5] & 0x0007) << 8) | buf[7];
>> - finger[1].t = buf[9];
>> + /* st1232 includes a z-axis / touch strength */
>> + if (ts->chip_info->have_z)
>> + finger[i].t = buf[i + 6];
>> + }
>> }
>>
>> return 0;
>> @@ -104,11 +113,14 @@ static irqreturn_t st1232_ts_irq_handler(int
>> irq, void *dev_id)
>> goto end;
>>
>> /* multi touch protocol */
>> - for (i = 0; i < MAX_FINGERS; i++) {
>> + for (i = 0; i < ts->chip_info->max_fingers; i++) {
>> if (!finger[i].is_valid)
>> continue;
>>
>> - input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, finger[i].t);
>> + if (ts->chip_info->have_z)
>> + input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
>> + finger[i].t);
>> +
>> input_report_abs(input_dev, ABS_MT_POSITION_X, finger[i].x);
>> input_report_abs(input_dev, ABS_MT_POSITION_Y, finger[i].y);
>> input_mt_sync(input_dev);
>> @@ -142,12 +154,40 @@ static void st1232_ts_power(struct
>> st1232_ts_data *ts, bool poweron)
>> gpio_direction_output(ts->reset_gpio, poweron);
>> }
>>
>> +static const struct st_chip_info st1232_chip_info = {
>> + .have_z = true,
>> + .max_x = 0x31f, /* 800 - 1 */
>> + .max_y = 0x1df, /* 480 -1 */
>> + .max_area = 0xff,
>> + .max_fingers = 2,
>> + .start_reg = 0x12,
>> +};
>> +
>> +static const struct st_chip_info st1633_chip_info = {
>> + .have_z = false,
>> + .max_x = 0x13f, /* 320 - 1 */
>> + .max_y = 0x1df, /* 480 -1 */
>> + .max_area = 0x00,
>> + .max_fingers = 5,
>> + .start_reg = 0x12,
>> +};
>> +
>> static int st1232_ts_probe(struct i2c_client *client,
>> const struct i2c_device_id *id)
>> {
>> struct st1232_ts_data *ts;
>> + struct st1232_ts_finger *finger;
>> struct input_dev *input_dev;
>> int error;
>> + const struct st_chip_info *match = NULL;
>
> There is no need to initialize with NULL given that we do unconditional
> assignment below. I removed initialization.
>
>> +
>> + match = device_get_match_data(&client->dev);
>> + if (!match && id)
>> + match = (const void *)id->driver_data;
>> + if (!match) {
>> + dev_err(&client->dev, "unknown device model\n");
>> + return -ENODEV;
>> + }
>>
>> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> dev_err(&client->dev, "need I2C_FUNC_I2C\n");
>> @@ -163,6 +203,19 @@ static int st1232_ts_probe(struct i2c_client
>> *client,
>> if (!ts)
>> return -ENOMEM;
>>
>> + ts->chip_info = match;
>> + ts->finger = devm_kzalloc(&client->dev,
>> + sizeof(*finger) * ts->chip_info->max_fingers,
>> + GFP_KERNEL);
>
> I replaced it with devm_kcalloc(&client->dev,
> ts->chip_info->max_fingers, sizeof(*finger),
> GFP_KERNEL);
>
> and applied.
>
>> + if (!ts->finger)
>> + return -ENOMEM;
>> +
>> + /* allocate a buffer according to the number of registers to read */
>> + ts->read_buf_len = ts->chip_info->max_fingers * 4;
>> + ts->read_buf = devm_kzalloc(&client->dev, ts->read_buf_len,
>> GFP_KERNEL);
>> + if (!ts->read_buf)
>> + return -ENOMEM;
>> +
>> input_dev = devm_input_allocate_device(&client->dev);
>> if (!input_dev)
>> return -ENOMEM;
>> @@ -192,9 +245,14 @@ static int st1232_ts_probe(struct i2c_client
>> *client,
>> __set_bit(EV_KEY, input_dev->evbit);
>> __set_bit(EV_ABS, input_dev->evbit);
>>
>> - input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0,
>> 0);
>> - input_set_abs_params(input_dev, ABS_MT_POSITION_X, MIN_X, MAX_X, 0,
>> 0);
>> - input_set_abs_params(input_dev, ABS_MT_POSITION_Y, MIN_Y, MAX_Y, 0,
>> 0);
>> + if (ts->chip_info->have_z)
>> + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
>> + ts->chip_info->max_area, 0, 0);
>> +
>> + input_set_abs_params(input_dev, ABS_MT_POSITION_X,
>> + MIN_X, ts->chip_info->max_x, 0, 0);
>> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
>> + MIN_Y, ts->chip_info->max_y, 0, 0);
>>
>> error = devm_request_threaded_irq(&client->dev, client->irq,
>> NULL, st1232_ts_irq_handler,
>> @@ -261,13 +319,15 @@ static SIMPLE_DEV_PM_OPS(st1232_ts_pm_ops,
>> st1232_ts_suspend, st1232_ts_resume);
>>
>> static const struct i2c_device_id st1232_ts_id[] = {
>> - { ST1232_TS_NAME, 0 },
>> + { ST1232_TS_NAME, (unsigned long)&st1232_chip_info },
>> + { ST1633_TS_NAME, (unsigned long)&st1633_chip_info },
>> { }
>> };
>> MODULE_DEVICE_TABLE(i2c, st1232_ts_id);
>>
>> static const struct of_device_id st1232_ts_dt_ids[] = {
>> - { .compatible = "sitronix,st1232", },
>> + { .compatible = "sitronix,st1232", .data = &st1232_chip_info },
>> + { .compatible = "sitronix,st1633", .data = &st1633_chip_info },
>> { }
>> };
>> MODULE_DEVICE_TABLE(of, st1232_ts_dt_ids);
>> --
>> 2.20.1
>>
>
> Thanks.
Hi Martin,
Matthias Fend
R&D Electronics
Wolfvision GmbH
Oberes Ried 14 | 6833 Klaus | Austria
Tel: +43 5523 52250 | Mail: [email protected]
Webpage: http://www.wolfvision.com | http://www.wolfvision.com/green
Firmenbuch / Commercial Register: FN283521v Feldkirch/Austria
> -----Urspr?ngliche Nachricht-----
> Von: [email protected] <linux-input-
> [email protected]> Im Auftrag von Martin Kepplinger
> Gesendet: Montag, 28. J?nner 2019 20:10
> An: Dmitry Torokhov <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Martin Kepplinger <[email protected]>
> Betreff: Re: [PATCH 2/3] Input: st1232 - add support for st1633
>
> Am 28.01.2019 20:03 schrieb Dmitry Torokhov:
> > Hi Martin,
> >
> > On Mon, Jan 28, 2019 at 09:44:48AM +0100, Martin Kepplinger wrote:
> >> From: Martin Kepplinger <[email protected]>
> >>
> >> Add support for the Sitronix ST1633 touchscreen controller to the
> >> st1232
> >> driver. A protocol spec can be found here:
> >>
> https://emea01.safelinks.protection.outlook.com/?url=www.ampdisplay.co
> m%2Fdocuments%2Fpdf%2FAM-320480B6TZQW-
> TC0H.pdf&data=01%7C01%7CMatthias.Fend%40wolfvision.net%7C4f9d
> 56475f674b1e3b4008d685543b35%7Ce94ec9da9183471e83b351baa8eb804f%
> 7C0&sdata=YTw33BPKKpdN%2BBFrufVk8e4YqXOzR6VQKuzRMOjcwWk
> %3D&reserved=0
> >>
> >> Signed-off-by: Martin Kepplinger <[email protected]>
> >> ---
> >> drivers/input/touchscreen/Kconfig | 6 +-
> >> drivers/input/touchscreen/st1232.c | 122
> >> +++++++++++++++++++++--------
> >> 2 files changed, 94 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/drivers/input/touchscreen/Kconfig
> >> b/drivers/input/touchscreen/Kconfig
> >> index 068dbbc610fc..7c597a49c265 100644
> >> --- a/drivers/input/touchscreen/Kconfig
> >> +++ b/drivers/input/touchscreen/Kconfig
> >> @@ -1168,11 +1168,11 @@ config TOUCHSCREEN_SIS_I2C
> >> module will be called sis_i2c.
> >>
> >> config TOUCHSCREEN_ST1232
> >> - tristate "Sitronix ST1232 touchscreen controllers"
> >> + tristate "Sitronix ST1232 or ST1633 touchscreen controllers"
> >> depends on I2C
> >> help
> >> - Say Y here if you want to support Sitronix ST1232
> >> - touchscreen controller.
> >> + Say Y here if you want to support the Sitronix ST1232
> >> + or ST1633 touchscreen controller.
> >>
> >> If unsure, say N.
> >>
> >> diff --git a/drivers/input/touchscreen/st1232.c
> >> b/drivers/input/touchscreen/st1232.c
> >> index 11ff32c68025..19a665d48dad 100644
> >> --- a/drivers/input/touchscreen/st1232.c
> >> +++ b/drivers/input/touchscreen/st1232.c
> >> @@ -23,13 +23,15 @@
> >> #include <linux/types.h>
> >>
> >> #define ST1232_TS_NAME "st1232-ts"
> >> +#define ST1633_TS_NAME "st1633-ts"
> >> +
> >> +enum {
> >> + st1232,
> >> + st1633,
> >> +};
> >
> > This enum seems no longer needed, I dropped it.
> >
> >>
> >> #define MIN_X 0x00
> >> #define MIN_Y 0x00
> >
> > Given we no longer have constant MAX_X/Y I simply used 0 in
> > input_set_abs_params() and dropped MIN-X/Y.
> >
> >> -#define MAX_X 0x31f /* (800 - 1) */
> >> -#define MAX_Y 0x1df /* (480 - 1) */
> >> -#define MAX_AREA 0xff
> >> -#define MAX_FINGERS 2
> >>
> >> struct st1232_ts_finger {
> >> u16 x;
> >> @@ -38,12 +40,24 @@ struct st1232_ts_finger {
> >> bool is_valid;
> >> };
> >>
> >> +struct st_chip_info {
> >> + bool have_z;
> >> + u16 max_x;
> >> + u16 max_y;
> >> + u16 max_area;
> >> + u16 max_fingers;
> >> + u8 start_reg;
> >> +};
> >> +
> >> struct st1232_ts_data {
> >> struct i2c_client *client;
> >> struct input_dev *input_dev;
> >> - struct st1232_ts_finger finger[MAX_FINGERS];
> >> struct dev_pm_qos_request low_latency_req;
> >> int reset_gpio;
> >
> > Could you please create an additional patch converting this to gpiod?
> > Instead of doing of_get_gpio()/gpio_is_valid()/devm_gpio_request()
> > smply
> > do
> >
> > ts->reset_gpio = devm_gpiod_get_optional();
> > if (IS_ERR(ts->reset_gpio)) {
> > ...
> > }
> >
> > and later
> >
> > if (ts->reset_gpio)
> > ...
> >
> > Looking at the code it looks like reset is as usual active low, so you
> > will need to invert the logic as gpiod takes care of convertion logical
> > value to proper level (active low or high).
>
> I'll test your applied changes and get back to this tomorrow.
>
> thanks.
>
> >
> >> + const struct st_chip_info *chip_info;
> >> + int read_buf_len;
> >> + u8 *read_buf;
> >> + struct st1232_ts_finger *finger;
> >> };
> >>
> >> static int st1232_ts_read_data(struct st1232_ts_data *ts)
> >> @@ -52,40 +66,35 @@ static int st1232_ts_read_data(struct
> >> st1232_ts_data *ts)
> >> struct i2c_client *client = ts->client;
> >> struct i2c_msg msg[2];
> >> int error;
> >> - u8 start_reg;
> >> - u8 buf[10];
> >> + int i, y;
> >> + u8 start_reg = ts->chip_info->start_reg;
> >> + u8 *buf = ts->read_buf;
> >>
> >> - /* read touchscreen data from ST1232 */
> >> + /* read touchscreen data */
> >> msg[0].addr = client->addr;
> >> msg[0].flags = 0;
> >> msg[0].len = 1;
> >> msg[0].buf = &start_reg;
> >> - start_reg = 0x10;
> >>
> >> msg[1].addr = ts->client->addr;
> >> msg[1].flags = I2C_M_RD;
> >> - msg[1].len = sizeof(buf);
> >> + msg[1].len = ts->read_buf_len;
> >> msg[1].buf = buf;
> >>
> >> error = i2c_transfer(client->adapter, msg, 2);
> >> if (error < 0)
> >> return error;
> >>
> >> - /* get "valid" bits */
> >> - finger[0].is_valid = buf[2] >> 7;
> >> - finger[1].is_valid = buf[5] >> 7;
> >> + for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) {
> >> + finger[i].is_valid = buf[i + y] >> 7;
> >> + if (finger[i].is_valid) {
> >> + finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1];
> >> + finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2];
> >>
> >> - /* get xy coordinate */
> >> - if (finger[0].is_valid) {
> >> - finger[0].x = ((buf[2] & 0x0070) << 4) | buf[3];
> >> - finger[0].y = ((buf[2] & 0x0007) << 8) | buf[4];
> >> - finger[0].t = buf[8];
> >> - }
> >> -
> >> - if (finger[1].is_valid) {
> >> - finger[1].x = ((buf[5] & 0x0070) << 4) | buf[6];
> >> - finger[1].y = ((buf[5] & 0x0007) << 8) | buf[7];
> >> - finger[1].t = buf[9];
> >> + /* st1232 includes a z-axis / touch strength */
> >> + if (ts->chip_info->have_z)
> >> + finger[i].t = buf[i + 6];
> >> + }
> >> }
> >>
> >> return 0;
> >> @@ -104,11 +113,14 @@ static irqreturn_t st1232_ts_irq_handler(int
> >> irq, void *dev_id)
> >> goto end;
> >>
> >> /* multi touch protocol */
> >> - for (i = 0; i < MAX_FINGERS; i++) {
> >> + for (i = 0; i < ts->chip_info->max_fingers; i++) {
> >> if (!finger[i].is_valid)
> >> continue;
> >>
> >> - input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> finger[i].t);
> >> + if (ts->chip_info->have_z)
> >> + input_report_abs(input_dev,
> ABS_MT_TOUCH_MAJOR,
> >> + finger[i].t);
> >> +
> >> input_report_abs(input_dev, ABS_MT_POSITION_X,
> finger[i].x);
> >> input_report_abs(input_dev, ABS_MT_POSITION_Y,
> finger[i].y);
> >> input_mt_sync(input_dev);
> >> @@ -142,12 +154,40 @@ static void st1232_ts_power(struct
> >> st1232_ts_data *ts, bool poweron)
> >> gpio_direction_output(ts->reset_gpio, poweron);
> >> }
> >>
> >> +static const struct st_chip_info st1232_chip_info = {
> >> + .have_z = true,
> >> + .max_x = 0x31f, /* 800 - 1 */
> >> + .max_y = 0x1df, /* 480 -1 */
> >> + .max_area = 0xff,
> >> + .max_fingers = 2,
> >> + .start_reg = 0x12,
> >> +};
> >> +
> >> +static const struct st_chip_info st1633_chip_info = {
> >> + .have_z = false,
> >> + .max_x = 0x13f, /* 320 - 1 */
> >> + .max_y = 0x1df, /* 480 -1 */
I guess these values are the dimensions of the referenced TFT display and not any limits of the touch controller. I wonder which values are correct here?
Maybe it would also be easier to just use the decimal representation and drop the comment, what do you think?
Thanks,
~Matthias
> >> + .max_area = 0x00,
> >> + .max_fingers = 5,
> >> + .start_reg = 0x12,
> >> +};
> >> +
> >> static int st1232_ts_probe(struct i2c_client *client,
> >> const struct i2c_device_id *id)
> >> {
> >> struct st1232_ts_data *ts;
> >> + struct st1232_ts_finger *finger;
> >> struct input_dev *input_dev;
> >> int error;
> >> + const struct st_chip_info *match = NULL;
> >
> > There is no need to initialize with NULL given that we do unconditional
> > assignment below. I removed initialization.
> >
> >> +
> >> + match = device_get_match_data(&client->dev);
> >> + if (!match && id)
> >> + match = (const void *)id->driver_data;
> >> + if (!match) {
> >> + dev_err(&client->dev, "unknown device model\n");
> >> + return -ENODEV;
> >> + }
> >>
> >> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> >> dev_err(&client->dev, "need I2C_FUNC_I2C\n");
> >> @@ -163,6 +203,19 @@ static int st1232_ts_probe(struct i2c_client
> >> *client,
> >> if (!ts)
> >> return -ENOMEM;
> >>
> >> + ts->chip_info = match;
> >> + ts->finger = devm_kzalloc(&client->dev,
> >> + sizeof(*finger) * ts->chip_info-
> >max_fingers,
> >> + GFP_KERNEL);
> >
> > I replaced it with devm_kcalloc(&client->dev,
> > ts->chip_info->max_fingers, sizeof(*finger),
> > GFP_KERNEL);
> >
> > and applied.
> >
> >> + if (!ts->finger)
> >> + return -ENOMEM;
> >> +
> >> + /* allocate a buffer according to the number of registers to read */
> >> + ts->read_buf_len = ts->chip_info->max_fingers * 4;
> >> + ts->read_buf = devm_kzalloc(&client->dev, ts->read_buf_len,
> >> GFP_KERNEL);
> >> + if (!ts->read_buf)
> >> + return -ENOMEM;
> >> +
> >> input_dev = devm_input_allocate_device(&client->dev);
> >> if (!input_dev)
> >> return -ENOMEM;
> >> @@ -192,9 +245,14 @@ static int st1232_ts_probe(struct i2c_client
> >> *client,
> >> __set_bit(EV_KEY, input_dev->evbit);
> >> __set_bit(EV_ABS, input_dev->evbit);
> >>
> >> - input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
> MAX_AREA, 0,
> >> 0);
> >> - input_set_abs_params(input_dev, ABS_MT_POSITION_X, MIN_X,
> MAX_X, 0,
> >> 0);
> >> - input_set_abs_params(input_dev, ABS_MT_POSITION_Y, MIN_Y,
> MAX_Y, 0,
> >> 0);
> >> + if (ts->chip_info->have_z)
> >> + input_set_abs_params(input_dev,
> ABS_MT_TOUCH_MAJOR, 0,
> >> + ts->chip_info->max_area, 0, 0);
> >> +
> >> + input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> >> + MIN_X, ts->chip_info->max_x, 0, 0);
> >> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> >> + MIN_Y, ts->chip_info->max_y, 0, 0);
> >>
> >> error = devm_request_threaded_irq(&client->dev, client->irq,
> >> NULL, st1232_ts_irq_handler,
> >> @@ -261,13 +319,15 @@ static
> SIMPLE_DEV_PM_OPS(st1232_ts_pm_ops,
> >> st1232_ts_suspend, st1232_ts_resume);
> >>
> >> static const struct i2c_device_id st1232_ts_id[] = {
> >> - { ST1232_TS_NAME, 0 },
> >> + { ST1232_TS_NAME, (unsigned long)&st1232_chip_info },
> >> + { ST1633_TS_NAME, (unsigned long)&st1633_chip_info },
> >> { }
> >> };
> >> MODULE_DEVICE_TABLE(i2c, st1232_ts_id);
> >>
> >> static const struct of_device_id st1232_ts_dt_ids[] = {
> >> - { .compatible = "sitronix,st1232", },
> >> + { .compatible = "sitronix,st1232", .data = &st1232_chip_info },
> >> + { .compatible = "sitronix,st1633", .data = &st1633_chip_info },
> >> { }
> >> };
> >> MODULE_DEVICE_TABLE(of, st1232_ts_dt_ids);
> >> --
> >> 2.20.1
> >>
> >
> > Thanks.
On Mon, 28 Jan 2019 09:44:47 +0100, Martin Kepplinger wrote:
> From: Martin Kepplinger <[email protected]>
>
> The st1232 driver gains support for the ST1633 controller too; update
> the bindings doc accordingly.
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> ---
> .../bindings/input/touchscreen/sitronix-st1232.txt | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
Reviewed-by: Rob Herring <[email protected]>