2023-10-03 13:36:00

by Karel Balej

[permalink] [raw]
Subject: [PATCH v2 0/5] input/touchscreen: imagis: add support for IST3032C

From: Karel Balej <[email protected]>

This patch series generalizes the Imagis touchscreen driver to support
other Imagis chips, namely IST3038B, which use a slightly different
protocol.

It also adds necessary information to the driver so that the IST3032C
touchscreen can be used with it. The motivation for this is the
samsung,coreprimevelte smartphone with which this series has been
tested. However, the support for this device is not yet in-tree, the
effort is happening at [1]. In particular, the driver for the regulator
needed by the touchscreen on this device has not been rewritten for
mainline yet.

[1] https://lore.kernel.org/all/[email protected]/
---
Changes in v2:
- Do not rename the driver.
- Do not hardcode voltage required by the IST3032C.
- Use Markuss' series which generalizes the driver.
- Separate bindings into separate patch.
- v1: https://lore.kernel.org/all/[email protected]/
---

Karel Balej (2):
dt-bindings: input/touchscreen: imagis: add compatible for IST3032C
input/touchscreen: imagis: add support for IST3032C

Markuss Broks (3):
input/touchscreen: imagis: Correct the maximum touch area value
dt-bindings: input/touchscreen: Add compatible for IST3038B
input/touchscreen: imagis: Add support for Imagis IST3038B

.../input/touchscreen/imagis,ist3038c.yaml | 2 +
drivers/input/touchscreen/imagis.c | 70 +++++++++++++++----
2 files changed, 60 insertions(+), 12 deletions(-)

--
2.42.0


2023-10-03 13:36:25

by Karel Balej

[permalink] [raw]
Subject: [PATCH v2 1/5] input/touchscreen: imagis: Correct the maximum touch area value

From: Karel Balej <[email protected]>

From: Markuss Broks <[email protected]>

As specified in downstream IST3038B driver and proved by testing,
the correct maximum reported value of touch area is 16.

Signed-off-by: Markuss Broks <[email protected]>
Signed-off-by: Karel Balej <[email protected]>
---
drivers/input/touchscreen/imagis.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 07111ca24455..e67fd3011027 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -210,7 +210,7 @@ static int imagis_init_input_dev(struct imagis_ts *ts)

input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 16, 0, 0);

touchscreen_parse_properties(input_dev, true, &ts->prop);
if (!ts->prop.max_x || !ts->prop.max_y) {
--
2.42.0

2023-10-03 13:36:37

by Karel Balej

[permalink] [raw]
Subject: [PATCH v2 2/5] dt-bindings: input/touchscreen: Add compatible for IST3038B

From: Karel Balej <[email protected]>

From: Markuss Broks <[email protected]>

Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
add the compatible for it to the IST3038C bindings.

Signed-off-by: Markuss Broks <[email protected]>
Signed-off-by: Karel Balej <[email protected]>
---
.../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
index 0d6b033fd5fb..b5372c4eae56 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
@@ -18,6 +18,7 @@ properties:

compatible:
enum:
+ - imagis,ist3038b
- imagis,ist3038c

reg:
--
2.42.0

2023-10-03 13:36:43

by Karel Balej

[permalink] [raw]
Subject: [PATCH v2 3/5] input/touchscreen: imagis: Add support for Imagis IST3038B

From: Karel Balej <[email protected]>

From: Markuss Broks <[email protected]>

Imagis IST3038B is another variant of Imagis IST3038 IC, which has
a different register interface from IST3038C (possibly firmware defined).
This should also work for IST3044B (though untested), however other
variants using this interface/protocol(IST3026, IST3032, IST3026B,
IST3032B) have a different format for coordinates, and they'd need
additional effort to be supported by this driver.

Signed-off-by: Markuss Broks <[email protected]>
Signed-off-by: Karel Balej <[email protected]>
---
drivers/input/touchscreen/imagis.c | 58 ++++++++++++++++++++++++------
1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index e67fd3011027..84a02672ac47 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -13,7 +13,7 @@

#define IST3038C_HIB_ACCESS (0x800B << 16)
#define IST3038C_DIRECT_ACCESS BIT(31)
-#define IST3038C_REG_CHIPID 0x40001000
+#define IST3038C_REG_CHIPID (0x40001000 | IST3038C_DIRECT_ACCESS)
#define IST3038C_REG_HIB_BASE 0x30000100
#define IST3038C_REG_TOUCH_STATUS (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS)
#define IST3038C_REG_TOUCH_COORD (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS | 0x8)
@@ -31,8 +31,21 @@
#define IST3038C_FINGER_COUNT_SHIFT 12
#define IST3038C_FINGER_STATUS_MASK GENMASK(9, 0)

+#define IST3038B_REG_STATUS 0x20
+#define IST3038B_REG_CHIPID 0x30
+#define IST3038B_WHOAMI 0x30380b
+
+struct imagis_properties {
+ unsigned int interrupt_msg_cmd;
+ unsigned int touch_coord_cmd;
+ unsigned int whoami_cmd;
+ unsigned int whoami_val;
+ bool protocol_b;
+};
+
struct imagis_ts {
struct i2c_client *client;
+ const struct imagis_properties *tdata;
struct input_dev *input_dev;
struct touchscreen_properties prop;
struct regulator_bulk_data supplies[2];
@@ -84,8 +97,7 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
int i;
int error;

- error = imagis_i2c_read_reg(ts, IST3038C_REG_INTR_MESSAGE,
- &intr_message);
+ error = imagis_i2c_read_reg(ts, ts->tdata->interrupt_msg_cmd, &intr_message);
if (error) {
dev_err(&ts->client->dev,
"failed to read the interrupt message: %d\n", error);
@@ -104,9 +116,13 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
finger_pressed = intr_message & IST3038C_FINGER_STATUS_MASK;

for (i = 0; i < finger_count; i++) {
- error = imagis_i2c_read_reg(ts,
- IST3038C_REG_TOUCH_COORD + (i * 4),
- &finger_status);
+ if (ts->tdata->protocol_b)
+ error = imagis_i2c_read_reg(ts,
+ ts->tdata->touch_coord_cmd, &finger_status);
+ else
+ error = imagis_i2c_read_reg(ts,
+ ts->tdata->touch_coord_cmd + (i * 4),
+ &finger_status);
if (error) {
dev_err(&ts->client->dev,
"failed to read coordinates for finger %d: %d\n",
@@ -261,6 +277,12 @@ static int imagis_probe(struct i2c_client *i2c)

ts->client = i2c;

+ ts->tdata = device_get_match_data(dev);
+ if (!ts->tdata) {
+ dev_err(dev, "missing chip data\n");
+ return -EINVAL;
+ }
+
error = imagis_init_regulators(ts);
if (error) {
dev_err(dev, "regulator init error: %d\n", error);
@@ -279,15 +301,13 @@ static int imagis_probe(struct i2c_client *i2c)
return error;
}

- error = imagis_i2c_read_reg(ts,
- IST3038C_REG_CHIPID | IST3038C_DIRECT_ACCESS,
- &chip_id);
+ error = imagis_i2c_read_reg(ts, ts->tdata->whoami_cmd, &chip_id);
if (error) {
dev_err(dev, "chip ID read failure: %d\n", error);
return error;
}

- if (chip_id != IST3038C_WHOAMI) {
+ if (chip_id != ts->tdata->whoami_val) {
dev_err(dev, "unknown chip ID: 0x%x\n", chip_id);
return -EINVAL;
}
@@ -343,9 +363,25 @@ static int imagis_resume(struct device *dev)

static DEFINE_SIMPLE_DEV_PM_OPS(imagis_pm_ops, imagis_suspend, imagis_resume);

+static const struct imagis_properties imagis_3038b_data = {
+ .interrupt_msg_cmd = IST3038B_REG_STATUS,
+ .touch_coord_cmd = IST3038B_REG_STATUS,
+ .whoami_cmd = IST3038B_REG_CHIPID,
+ .whoami_val = IST3038B_WHOAMI,
+ .protocol_b = true,
+};
+
+static const struct imagis_properties imagis_3038c_data = {
+ .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
+ .touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
+ .whoami_cmd = IST3038C_REG_CHIPID,
+ .whoami_val = IST3038C_WHOAMI,
+};
+
#ifdef CONFIG_OF
static const struct of_device_id imagis_of_match[] = {
- { .compatible = "imagis,ist3038c", },
+ { .compatible = "imagis,ist3038b", .data = &imagis_3038b_data },
+ { .compatible = "imagis,ist3038c", .data = &imagis_3038c_data },
{ },
};
MODULE_DEVICE_TABLE(of, imagis_of_match);
--
2.42.0

2023-10-03 13:36:57

by Karel Balej

[permalink] [raw]
Subject: [PATCH v2 4/5] dt-bindings: input/touchscreen: imagis: add compatible for IST3032C

From: Karel Balej <[email protected]>

From: Karel Balej <[email protected]>

Document possible usage of the Imagis driver with the IST3032C
touchscreen.

Signed-off-by: Karel Balej <[email protected]>
---
.../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
index b5372c4eae56..2af71cbcc97d 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
@@ -18,6 +18,7 @@ properties:

compatible:
enum:
+ - imagis,ist3032c
- imagis,ist3038b
- imagis,ist3038c

--
2.42.0

2023-10-03 13:37:02

by Karel Balej

[permalink] [raw]
Subject: [PATCH v2 5/5] input/touchscreen: imagis: add support for IST3032C

From: Karel Balej <[email protected]>

From: Karel Balej <[email protected]>

IST3032C is a touchscreen chip used for instance in the
samsung,coreprimevelte smartphone, with which this was tested. Add the
chip specific information to the driver.

Signed-off-by: Karel Balej <[email protected]>
---
drivers/input/touchscreen/imagis.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 84a02672ac47..41f28e6e9cb1 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -35,6 +35,8 @@
#define IST3038B_REG_CHIPID 0x30
#define IST3038B_WHOAMI 0x30380b

+#define IST3032C_WHOAMI 0x32c
+
struct imagis_properties {
unsigned int interrupt_msg_cmd;
unsigned int touch_coord_cmd;
@@ -363,6 +365,13 @@ static int imagis_resume(struct device *dev)

static DEFINE_SIMPLE_DEV_PM_OPS(imagis_pm_ops, imagis_suspend, imagis_resume);

+static const struct imagis_properties imagis_3032c_data = {
+ .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
+ .touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
+ .whoami_cmd = IST3038C_REG_CHIPID,
+ .whoami_val = IST3032C_WHOAMI,
+};
+
static const struct imagis_properties imagis_3038b_data = {
.interrupt_msg_cmd = IST3038B_REG_STATUS,
.touch_coord_cmd = IST3038B_REG_STATUS,
@@ -380,6 +389,7 @@ static const struct imagis_properties imagis_3038c_data = {

#ifdef CONFIG_OF
static const struct of_device_id imagis_of_match[] = {
+ { .compatible = "imagis,ist3032c", .data = &imagis_3032c_data },
{ .compatible = "imagis,ist3038b", .data = &imagis_3038b_data },
{ .compatible = "imagis,ist3038c", .data = &imagis_3038c_data },
{ },
--
2.42.0

2023-10-03 13:46:18

by Karel Balej

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] input/touchscreen: imagis: add support for IST3032C

Hello,

> From: Karel Balej <[email protected]>

I am very sorry, I wanted to use a different email address for sending
than for commiting so that the message would also reach people whose
email providers have more strict requirements on sender authentication
(such as Google), but it seems that I have made a mistake and confused
git altogether.

I will fix it in a possible v3 after I receive some feedback or I will
resend it before it gets applied and I will make sure to properly test
the setup then.

My apologies,
K. B.

2023-10-04 02:50:49

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] input/touchscreen: imagis: Add support for Imagis IST3038B

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus robh/for-next linus/master v6.6-rc4 next-20231003]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/karelb-gimli-ms-mff-cuni-cz/input-touchscreen-imagis-Correct-the-maximum-touch-area-value/20231003-213739
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20231003133440.4696-4-karelb%40gimli.ms.mff.cuni.cz
patch subject: [PATCH v2 3/5] input/touchscreen: imagis: Add support for Imagis IST3038B
config: x86_64-randconfig-004-20231004 (https://download.01.org/0day-ci/archive/20231004/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231004/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/input/touchscreen/imagis.c:374:39: warning: 'imagis_3038c_data' defined but not used [-Wunused-const-variable=]
374 | static const struct imagis_properties imagis_3038c_data = {
| ^~~~~~~~~~~~~~~~~
>> drivers/input/touchscreen/imagis.c:366:39: warning: 'imagis_3038b_data' defined but not used [-Wunused-const-variable=]
366 | static const struct imagis_properties imagis_3038b_data = {
| ^~~~~~~~~~~~~~~~~


vim +/imagis_3038c_data +374 drivers/input/touchscreen/imagis.c

365
> 366 static const struct imagis_properties imagis_3038b_data = {
367 .interrupt_msg_cmd = IST3038B_REG_STATUS,
368 .touch_coord_cmd = IST3038B_REG_STATUS,
369 .whoami_cmd = IST3038B_REG_CHIPID,
370 .whoami_val = IST3038B_WHOAMI,
371 .protocol_b = true,
372 };
373
> 374 static const struct imagis_properties imagis_3038c_data = {
375 .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
376 .touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
377 .whoami_cmd = IST3038C_REG_CHIPID,
378 .whoami_val = IST3038C_WHOAMI,
379 };
380

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-10-04 06:42:46

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] input/touchscreen: imagis: add support for IST3032C

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus robh/for-next linus/master v6.6-rc4 next-20231003]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/karelb-gimli-ms-mff-cuni-cz/input-touchscreen-imagis-Correct-the-maximum-touch-area-value/20231003-213739
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20231003133440.4696-6-karelb%40gimli.ms.mff.cuni.cz
patch subject: [PATCH v2 5/5] input/touchscreen: imagis: add support for IST3032C
config: x86_64-randconfig-004-20231004 (https://download.01.org/0day-ci/archive/20231004/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231004/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

drivers/input/touchscreen/imagis.c:383:39: warning: 'imagis_3038c_data' defined but not used [-Wunused-const-variable=]
383 | static const struct imagis_properties imagis_3038c_data = {
| ^~~~~~~~~~~~~~~~~
drivers/input/touchscreen/imagis.c:375:39: warning: 'imagis_3038b_data' defined but not used [-Wunused-const-variable=]
375 | static const struct imagis_properties imagis_3038b_data = {
| ^~~~~~~~~~~~~~~~~
>> drivers/input/touchscreen/imagis.c:368:39: warning: 'imagis_3032c_data' defined but not used [-Wunused-const-variable=]
368 | static const struct imagis_properties imagis_3032c_data = {
| ^~~~~~~~~~~~~~~~~


vim +/imagis_3032c_data +368 drivers/input/touchscreen/imagis.c

367
> 368 static const struct imagis_properties imagis_3032c_data = {
369 .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
370 .touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
371 .whoami_cmd = IST3038C_REG_CHIPID,
372 .whoami_val = IST3032C_WHOAMI,
373 };
374

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-10-04 08:10:04

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: input/touchscreen: Add compatible for IST3038B

On 03/10/2023 15:34, [email protected] wrote:
> From: Karel Balej <[email protected]>
>
> From: Markuss Broks <[email protected]>

This does not look right. Please apply it to your tree and see the
result. You cannot have two From fields,

>
> Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
> add the compatible for it to the IST3038C bindings.
>
> Signed-off-by: Markuss Broks <[email protected]>
> Signed-off-by: Karel Balej <[email protected]>
Best regards,
Krzysztof

2023-10-04 08:33:07

by Karel Balej

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: input/touchscreen: Add compatible for IST3038B

Hello, Krzysztof,

On Wed Oct 4, 2023 at 10:09 AM CEST, Krzysztof Kozlowski wrote:
> On 03/10/2023 15:34, [email protected] wrote:
> > From: Karel Balej <[email protected]>
> >
> > From: Markuss Broks <[email protected]>
>
> This does not look right. Please apply it to your tree and see the
> result. You cannot have two From fields,

I am aware of this problem and I have explained it in a follow-up
message to the cover-letter, which should have reached you too - was it
not so?

The patches themselves should be fine though, so I plan to wait for some
feedback and fix this either in v3 or in a resend before application.

Best regards,
K. B.