LinuxLists.cc - [PATCH 0/3] Add support for Imagis IST3038 and clarify the usage of protocol

2024-06-12 03:21:25

Subject: [PATCH 0/3] Add support for Imagis IST3038 and clarify the usage of protocol_b

2024-06-12 03:22:13

Subject: [PATCH 3/3] dt-bindings: input/touchscreen: imagis: Document ist3038

Imagis IST3038 is a variant of Imagis touchscreen IC. Document it in
imagis,ist3038c bindings.

Signed-off-by: Raymond Hackley <[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 77ba280b3bdc..2aea21bfe6ac 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
@@ -15,6 +15,7 @@ properties:

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

2024-06-12 03:24:12

by Raymond Hackley

[permalink] [raw]

Subject: [PATCH 1/3] input/touchscreen: imagis: Clarify the usage of protocol_b

protocol_b is a property, which tells Imagis panel to use a different
format for coordinates.

IST30XXC series is known for using protocol B, while the other series
aren't. Note this could be confusing, unlike the model name implies.

Adjust the usage of protocol_b to avoid confusion.

Signed-off-by: Raymond Hackley <[email protected]>
---
drivers/input/touchscreen/imagis.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 074dd6c342ec..886bcfc8497a 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -120,12 +120,12 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)

for (i = 0; i < finger_count; i++) {
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);
+ else
+ error = imagis_i2c_read_reg(ts,
+ ts->tdata->touch_coord_cmd, &finger_status);
if (error) {
dev_err(&ts->client->dev,
"failed to read coordinates for finger %d: %d\n",
@@ -394,6 +394,7 @@ static const struct imagis_properties imagis_3032c_data = {
.whoami_cmd = IST3038C_REG_CHIPID,
.whoami_val = IST3032C_WHOAMI,
.touch_keys_supported = true,
+ .protocol_b = true,
};

static const struct imagis_properties imagis_3038b_data = {
@@ -401,7 +402,6 @@ static const struct imagis_properties imagis_3038b_data = {
.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 = {
@@ -409,6 +409,7 @@ static const struct imagis_properties imagis_3038c_data = {
.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
.whoami_cmd = IST3038C_REG_CHIPID,
.whoami_val = IST3038C_WHOAMI,
+ .protocol_b = true,
};

#ifdef CONFIG_OF
--
2.39.2

2024-06-12 03:24:39

by Raymond Hackley

[permalink] [raw]

Subject: [PATCH 2/3] input/touchscreen: imagis: Add supports for Imagis IST3038

Imagis IST3038 is another variant of Imagis IST3038 IC, which has
a different register interface from IST3038C (possibly firmware defined).

Unlike IST3038C/IST3032C, IST3038 has different registers for commands,
which means IST3038 doesn't use protocol B.
Similar to IST3032C and maybe the other variants, IST3038 has touch keys
support, which provides KEY_APPSELECT and KEY_BACK.

Add support for IST3038 with touch keys.

Signed-off-by: Raymond Hackley <[email protected]>
---
drivers/input/touchscreen/imagis.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 886bcfc8497a..b2f4bc60721d 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -12,9 +12,16 @@
#include <linux/property.h>
#include <linux/regulator/consumer.h>

+#define IST30XX_REG_STATUS 0x20
+#define IST30XX_REG_CHIPID (0x40000000 | IST3038C_DIRECT_ACCESS)
+#define IST30XX_WHOAMI 0x30003000
+#define IST30XXA_WHOAMI 0x300a300a
+#define IST30XXB_WHOAMI 0x300b300b
+#define IST3038_WHOAMI 0x30383038
+
#define IST3032C_WHOAMI 0x32c
+#define IST3038C_WHOAMI 0x38c

-#define IST3038B_REG_STATUS 0x20
#define IST3038B_REG_CHIPID 0x30
#define IST3038B_WHOAMI 0x30380b

@@ -25,7 +32,6 @@
#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)
#define IST3038C_REG_INTR_MESSAGE (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS | 0x4)
-#define IST3038C_WHOAMI 0x38c
#define IST3038C_CHIP_ON_DELAY_MS 60
#define IST3038C_I2C_RETRY_COUNT 3
#define IST3038C_MAX_FINGER_NUM 10
@@ -397,9 +403,17 @@ static const struct imagis_properties imagis_3032c_data = {
.protocol_b = true,
};

+static const struct imagis_properties imagis_3038_data = {
+ .interrupt_msg_cmd = IST30XX_REG_STATUS,
+ .touch_coord_cmd = IST30XX_REG_STATUS,
+ .whoami_cmd = IST30XX_REG_CHIPID,
+ .whoami_val = IST3038_WHOAMI,
+ .touch_keys_supported = true,
+};
+
static const struct imagis_properties imagis_3038b_data = {
- .interrupt_msg_cmd = IST3038B_REG_STATUS,
- .touch_coord_cmd = IST3038B_REG_STATUS,
+ .interrupt_msg_cmd = IST30XX_REG_STATUS,
+ .touch_coord_cmd = IST30XX_REG_STATUS,
.whoami_cmd = IST3038B_REG_CHIPID,
.whoami_val = IST3038B_WHOAMI,
};
@@ -415,6 +429,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,ist3038", .data = &imagis_3038_data },
{ .compatible = "imagis,ist3038b", .data = &imagis_3038b_data },
{ .compatible = "imagis,ist3038c", .data = &imagis_3038c_data },
{ },
--
2.39.2

2024-06-12 08:05:57

by Krzysztof Kozlowski

[permalink] [raw]

Subject: Re: [PATCH 3/3] dt-bindings: input/touchscreen: imagis: Document ist3038

On 12/06/2024 05:21, Raymond Hackley wrote:
> Imagis IST3038 is a variant of Imagis touchscreen IC. Document it in
> imagis,ist3038c bindings.
>
> Signed-off-by: Raymond Hackley <[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 77ba280b3bdc..2aea21bfe6ac 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> @@ -15,6 +15,7 @@ properties:
>
> compatible:
> enum:
> + - imagis,ist3038
> - imagis,ist3032c

Don't add entries in random order. Keep alphabetical order.

Best regards,
Krzysztof

2024-06-12 08:11:38

by Krzysztof Kozlowski

[permalink] [raw]

Subject: Re: [PATCH 2/3] input/touchscreen: imagis: Add supports for Imagis IST3038

On 12/06/2024 05:21, Raymond Hackley wrote:
> +
> static const struct imagis_properties imagis_3038b_data = {
> - .interrupt_msg_cmd = IST3038B_REG_STATUS,
> - .touch_coord_cmd = IST3038B_REG_STATUS,
> + .interrupt_msg_cmd = IST30XX_REG_STATUS,
> + .touch_coord_cmd = IST30XX_REG_STATUS,
> .whoami_cmd = IST3038B_REG_CHIPID,
> .whoami_val = IST3038B_WHOAMI,
> };
> @@ -415,6 +429,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,ist3038", .data = &imagis_3038_data },

Where is the binding? Compatibles must be documented BEFORE they are used.

Best regards,
Krzysztof

2024-06-12 18:45:57

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 2/3] input/touchscreen: imagis: Add supports for Imagis IST3038

Hi Raymond,

kernel test robot noticed the following build warnings:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on robh/for-next linus/master v6.10-rc3 next-20240612]
[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/Raymond-Hackley/input-touchscreen-imagis-Clarify-the-usage-of-protocol_b/20240612-112300
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20240612032036.33103-3-raymondhackley%40protonmail.com
patch subject: [PATCH 2/3] input/touchscreen: imagis: Add supports for Imagis IST3038
config: x86_64-buildonly-randconfig-006-20240612 (https://download.01.org/0day-ci/archive/20240613/[email protected]/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240613/[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:397:39: warning: unused variable 'imagis_3032c_data' [-Wunused-const-variable]
397 | static const struct imagis_properties imagis_3032c_data = {
| ^~~~~~~~~~~~~~~~~
>> drivers/input/touchscreen/imagis.c:406:39: warning: unused variable 'imagis_3038_data' [-Wunused-const-variable]
406 | static const struct imagis_properties imagis_3038_data = {
| ^~~~~~~~~~~~~~~~
drivers/input/touchscreen/imagis.c:414:39: warning: unused variable 'imagis_3038b_data' [-Wunused-const-variable]
414 | static const struct imagis_properties imagis_3038b_data = {
| ^~~~~~~~~~~~~~~~~
drivers/input/touchscreen/imagis.c:421:39: warning: unused variable 'imagis_3038c_data' [-Wunused-const-variable]
421 | static const struct imagis_properties imagis_3038c_data = {
| ^~~~~~~~~~~~~~~~~
4 warnings generated.

vim +/imagis_3038_data +406 drivers/input/touchscreen/imagis.c

405
> 406 static const struct imagis_properties imagis_3038_data = {
407 .interrupt_msg_cmd = IST30XX_REG_STATUS,
408 .touch_coord_cmd = IST30XX_REG_STATUS,
409 .whoami_cmd = IST30XX_REG_CHIPID,
410 .whoami_val = IST3038_WHOAMI,
411 .touch_keys_supported = true,
412 };
413

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

2024-06-12 19:40:52

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 2/3] input/touchscreen: imagis: Add supports for Imagis IST3038

Hi Raymond,

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.10-rc3 next-20240612]
[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/Raymond-Hackley/input-touchscreen-imagis-Clarify-the-usage-of-protocol_b/20240612-112300
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20240612032036.33103-3-raymondhackley%40protonmail.com
patch subject: [PATCH 2/3] input/touchscreen: imagis: Add supports for Imagis IST3038
config: i386-randconfig-061-20240612 (https://download.01.org/0day-ci/archive/20240613/[email protected]/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240613/[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:421:39: warning: 'imagis_3038c_data' defined but not used [-Wunused-const-variable=]
421 | static const struct imagis_properties imagis_3038c_data = {
| ^~~~~~~~~~~~~~~~~
drivers/input/touchscreen/imagis.c:414:39: warning: 'imagis_3038b_data' defined but not used [-Wunused-const-variable=]
414 | static const struct imagis_properties imagis_3038b_data = {
| ^~~~~~~~~~~~~~~~~
>> drivers/input/touchscreen/imagis.c:406:39: warning: 'imagis_3038_data' defined but not used [-Wunused-const-variable=]
406 | static const struct imagis_properties imagis_3038_data = {
| ^~~~~~~~~~~~~~~~
drivers/input/touchscreen/imagis.c:397:39: warning: 'imagis_3032c_data' defined but not used [-Wunused-const-variable=]
397 | static const struct imagis_properties imagis_3032c_data = {
| ^~~~~~~~~~~~~~~~~

vim +/imagis_3038_data +406 drivers/input/touchscreen/imagis.c

405
> 406 static const struct imagis_properties imagis_3038_data = {
407 .interrupt_msg_cmd = IST30XX_REG_STATUS,
408 .touch_coord_cmd = IST30XX_REG_STATUS,
409 .whoami_cmd = IST30XX_REG_CHIPID,
410 .whoami_val = IST3038_WHOAMI,
411 .touch_keys_supported = true,
412 };
413

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