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

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

[Raymond Hackley]

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.

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.

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

[Raymond Hackley]

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 <raymondhackley@protonmail.com>
---
 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

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

[Raymond Hackley]

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 <raymondhackley@protonmail.com>
---
 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

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

[Krzysztof Kozlowski]

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

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

[kernel test robot]

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/202406130224.G2LpMsby-lkp@intel.com/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/202406130224.G2LpMsby-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406130224.G2LpMsby-lkp@intel.com/

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

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

[kernel test robot]

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/202406130336.wSgshW3L-lkp@intel.com/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/202406130336.wSgshW3L-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406130336.wSgshW3L-lkp@intel.com/

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

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

[Raymond Hackley]

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

Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
---
 .../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

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

[Krzysztof Kozlowski]

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 <raymondhackley@protonmail.com>
> ---
>  .../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