[PATCH v3 0/5] himax_hx83112b: add support for HX83100A

[PATCH v3 0/5] himax_hx83112b: add support for HX83100A

[Felix Kaechele]

Original discussion around v2 here: https://lore.kernel.org/all/20240511121245.109644-1-felix@kaechele.ca/

Changes from v2:
- reworded dt-bindings commit message to specifiy how the HX83100A is different
  from other chips in this series to justify having its own compatible string
- added Acked-By from Conor that was given in the v2 thread to commit message

I've done some more testing in the meantime and haven't found any issues.
I haven't heard back from anyone with an HX83112B, but the HX83100A works as
intended with these changes and I assume the HX83112B will too.

Thanks,
Felix

Felix Kaechele (5):
  dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A
  input: himax_hx83112b: use more descriptive register defines
  input: himax_hx83112b: implement MCU register reading
  input: himax_hx83112b: add himax_chip struct for multi-chip support
  input: himax_hx83112b: add support for HX83100A

 .../input/touchscreen/himax,hx83112b.yaml     |   1 +
 drivers/input/touchscreen/himax_hx83112b.c    | 135 ++++++++++++++----
 2 files changed, 110 insertions(+), 26 deletions(-)


base-commit: 9b9247397e2e20016031e59f76dae563b79b6ee2
-- 
2.45.2

[PATCH v3 1/5] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A

[Felix Kaechele]

Add a compatible string for the Himax HX83100A touch controller.

The HX83100A presents touch events on the internal bus rather than
offering a dedicated event register like the other chips in this family
do.

Signed-off-by: Felix Kaechele <felix@kaechele.ca>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../devicetree/bindings/input/touchscreen/himax,hx83112b.yaml    | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
index f42b23d532eb..f5cfacb5e966 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
@@ -15,6 +15,7 @@ allOf:
 properties:
   compatible:
     enum:
+      - himax,hx83100a
       - himax,hx83112b
 
   reg:
-- 
2.45.2

[PATCH v3 2/5] input: himax_hx83112b: use more descriptive register defines

[Felix Kaechele]

Himax uses an AHB-style bus to communicate with different parts of the
display driver and touch controller system.
Use more descriptive names for the register and address defines.
The names were taken from a driver submission for the similar HX83102J
chip.

Signed-off-by: Felix Kaechele <felix@kaechele.ca>
Link: https://lore.kernel.org/all/TY0PR06MB561105A3386E9D76F429110D9E0F2@TY0PR06MB5611.apcprd06.prod.outlook.com/
---
 drivers/input/touchscreen/himax_hx83112b.c | 23 ++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index bafabd06dabc..d6c4a68eac23 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -24,12 +24,14 @@
 
 #define HIMAX_MAX_POINTS		10
 
-#define HIMAX_REG_CFG_SET_ADDR		0x00
-#define HIMAX_REG_CFG_INIT_READ		0x0c
-#define HIMAX_REG_CFG_READ_VALUE	0x08
-#define HIMAX_REG_READ_EVENT		0x30
+#define HIMAX_AHB_ADDR_BYTE_0			0x00
+#define HIMAX_AHB_ADDR_RDATA_BYTE_0		0x08
+#define HIMAX_AHB_ADDR_ACCESS_DIRECTION		0x0c
+#define HIMAX_AHB_ADDR_EVENT_STACK		0x30
 
-#define HIMAX_CFG_PRODUCT_ID		0x900000d0
+#define HIMAX_AHB_CMD_ACCESS_DIRECTION_READ	0x00
+
+#define HIMAX_REG_ADDR_ICID			0x900000d0
 
 #define HIMAX_INVALID_COORD		0xffff
 
@@ -67,15 +69,16 @@ static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
 {
 	int error;
 
-	error = regmap_write(ts->regmap, HIMAX_REG_CFG_SET_ADDR, address);
+	error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_BYTE_0, address);
 	if (error)
 		return error;
 
-	error = regmap_write(ts->regmap, HIMAX_REG_CFG_INIT_READ, 0x0);
+	error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_ACCESS_DIRECTION,
+			     HIMAX_AHB_CMD_ACCESS_DIRECTION_READ);
 	if (error)
 		return error;
 
-	error = regmap_read(ts->regmap, HIMAX_REG_CFG_READ_VALUE, dst);
+	error = regmap_read(ts->regmap, HIMAX_AHB_ADDR_RDATA_BYTE_0, dst);
 	if (error)
 		return error;
 
@@ -101,7 +104,7 @@ static int himax_read_product_id(struct himax_ts_data *ts, u32 *product_id)
 {
 	int error;
 
-	error = himax_read_config(ts, HIMAX_CFG_PRODUCT_ID, product_id);
+	error = himax_read_config(ts, HIMAX_REG_ADDR_ICID, product_id);
 	if (error)
 		return error;
 
@@ -235,7 +238,7 @@ static int himax_handle_input(struct himax_ts_data *ts)
 	int error;
 	struct himax_event event;
 
-	error = regmap_raw_read(ts->regmap, HIMAX_REG_READ_EVENT, &event,
+	error = regmap_raw_read(ts->regmap, HIMAX_AHB_ADDR_EVENT_STACK, &event,
 				sizeof(event));
 	if (error) {
 		dev_err(&ts->client->dev, "Failed to read input event: %d\n",
-- 
2.45.2

[PATCH v3 3/5] input: himax_hx83112b: implement MCU register reading

[Felix Kaechele]

Implement reading from the MCU in a more universal fashion. This allows
properly handling reads of more than 4 bytes using the AHB FIFO
implemented in the chip.

Signed-off-by: Felix Kaechele <felix@kaechele.ca>
---
 drivers/input/touchscreen/himax_hx83112b.c | 50 ++++++++++++++++++++--
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index d6c4a68eac23..c9cc69bf242a 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -27,9 +27,13 @@
 #define HIMAX_AHB_ADDR_BYTE_0			0x00
 #define HIMAX_AHB_ADDR_RDATA_BYTE_0		0x08
 #define HIMAX_AHB_ADDR_ACCESS_DIRECTION		0x0c
+#define HIMAX_AHB_ADDR_INCR4			0x0d
+#define HIMAX_AHB_ADDR_CONTI			0x13
 #define HIMAX_AHB_ADDR_EVENT_STACK		0x30
 
 #define HIMAX_AHB_CMD_ACCESS_DIRECTION_READ	0x00
+#define HIMAX_AHB_CMD_INCR4			0x10
+#define HIMAX_AHB_CMD_CONTI			0x31
 
 #define HIMAX_REG_ADDR_ICID			0x900000d0
 
@@ -65,10 +69,34 @@ static const struct regmap_config himax_regmap_config = {
 	.val_format_endian = REGMAP_ENDIAN_LITTLE,
 };
 
-static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
+static int himax_bus_enable_burst(struct himax_ts_data *ts)
 {
 	int error;
 
+	error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_CONTI,
+			     HIMAX_AHB_CMD_CONTI);
+	if (error)
+		return error;
+
+	error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_INCR4,
+			     HIMAX_AHB_CMD_INCR4);
+	if (error)
+		return error;
+
+	return 0;
+}
+
+static int himax_bus_read(struct himax_ts_data *ts, u32 address, void *dst,
+			  size_t length)
+{
+	int error;
+
+	if (length > 4) {
+		error = himax_bus_enable_burst(ts);
+		if (error)
+			return error;
+	}
+
 	error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_BYTE_0, address);
 	if (error)
 		return error;
@@ -78,7 +106,23 @@ static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
 	if (error)
 		return error;
 
-	error = regmap_read(ts->regmap, HIMAX_AHB_ADDR_RDATA_BYTE_0, dst);
+	if (length > 4)
+		error = regmap_noinc_read(ts->regmap, HIMAX_AHB_ADDR_RDATA_BYTE_0,
+					  dst, length);
+	else
+		error = regmap_read(ts->regmap, HIMAX_AHB_ADDR_RDATA_BYTE_0,
+				    dst);
+	if (error)
+		return error;
+
+	return 0;
+}
+
+static int himax_read_mcu(struct himax_ts_data *ts, u32 address, u32 *dst)
+{
+	int error;
+
+	error = himax_bus_read(ts, address, dst, sizeof(dst));
 	if (error)
 		return error;
 
@@ -104,7 +148,7 @@ static int himax_read_product_id(struct himax_ts_data *ts, u32 *product_id)
 {
 	int error;
 
-	error = himax_read_config(ts, HIMAX_REG_ADDR_ICID, product_id);
+	error = himax_read_mcu(ts, HIMAX_REG_ADDR_ICID, product_id);
 	if (error)
 		return error;
 
-- 
2.45.2

[PATCH v3 4/5] input: himax_hx83112b: add himax_chip struct for multi-chip support

[Felix Kaechele]

In preparation for HX83100A support allow defining separate functions
for specific chip operations.

Signed-off-by: Felix Kaechele <felix@kaechele.ca>
---
 drivers/input/touchscreen/himax_hx83112b.c | 51 +++++++++++++++-------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index c9cc69bf242a..5092a357c332 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -20,8 +20,6 @@
 #include <linux/kernel.h>
 #include <linux/regmap.h>
 
-#define HIMAX_ID_83112B			0x83112b
-
 #define HIMAX_MAX_POINTS		10
 
 #define HIMAX_AHB_ADDR_BYTE_0			0x00
@@ -55,7 +53,16 @@ struct himax_event {
 
 static_assert(sizeof(struct himax_event) == 56);
 
+struct himax_ts_data;
+struct himax_chip {
+	u32 id;
+	int (*check_id)(struct himax_ts_data *ts);
+	int (*read_events)(struct himax_ts_data *ts, struct himax_event *event,
+			   size_t length);
+};
+
 struct himax_ts_data {
+	const struct himax_chip *chip;
 	struct gpio_desc *gpiod_rst;
 	struct input_dev *input_dev;
 	struct i2c_client *client;
@@ -167,15 +174,12 @@ static int himax_check_product_id(struct himax_ts_data *ts)
 
 	dev_dbg(&ts->client->dev, "Product id: %x\n", product_id);
 
-	switch (product_id) {
-	case HIMAX_ID_83112B:
+	if (product_id == ts->chip->id)
 		return 0;
 
-	default:
-		dev_err(&ts->client->dev,
-			"Unknown product id: %x\n", product_id);
-		return -EINVAL;
-	}
+	dev_err(&ts->client->dev, "Unknown product id: %x\n",
+		product_id);
+	return -EINVAL;
 }
 
 static int himax_input_register(struct himax_ts_data *ts)
@@ -277,13 +281,19 @@ static bool himax_verify_checksum(struct himax_ts_data *ts,
 	return true;
 }
 
+static int himax_read_events(struct himax_ts_data *ts,
+			     struct himax_event *event, size_t length)
+{
+	return regmap_raw_read(ts->regmap, HIMAX_AHB_ADDR_EVENT_STACK, event,
+			       length);
+}
+
 static int himax_handle_input(struct himax_ts_data *ts)
 {
 	int error;
 	struct himax_event event;
 
-	error = regmap_raw_read(ts->regmap, HIMAX_AHB_ADDR_EVENT_STACK, &event,
-				sizeof(event));
+	error = ts->chip->read_events(ts, &event, sizeof(event));
 	if (error) {
 		dev_err(&ts->client->dev, "Failed to read input event: %d\n",
 			error);
@@ -329,6 +339,7 @@ static int himax_probe(struct i2c_client *client)
 
 	i2c_set_clientdata(client, ts);
 	ts->client = client;
+	ts->chip = i2c_get_match_data(client);
 
 	ts->regmap = devm_regmap_init_i2c(client, &himax_regmap_config);
 	error = PTR_ERR_OR_ZERO(ts->regmap);
@@ -346,9 +357,11 @@ static int himax_probe(struct i2c_client *client)
 
 	himax_reset(ts);
 
-	error = himax_check_product_id(ts);
-	if (error)
-		return error;
+	if (ts->chip->check_id) {
+		error = himax_check_product_id(ts);
+		if (error)
+			return error;
+	}
 
 	error = himax_input_register(ts);
 	if (error)
@@ -381,15 +394,21 @@ static int himax_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(himax_pm_ops, himax_suspend, himax_resume);
 
+static const struct himax_chip hx83112b_chip = {
+	.id = 0x83112b,
+	.check_id = himax_check_product_id,
+	.read_events = himax_read_events,
+};
+
 static const struct i2c_device_id himax_ts_id[] = {
-	{ "hx83112b" },
+	{ "hx83112b", (kernel_ulong_t)&hx83112b_chip },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(i2c, himax_ts_id);
 
 #ifdef CONFIG_OF
 static const struct of_device_id himax_of_match[] = {
-	{ .compatible = "himax,hx83112b" },
+	{ .compatible = "himax,hx83112b", .data = &hx83112b_chip },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, himax_of_match);
-- 
2.45.2

[PATCH v3 5/5] input: himax_hx83112b: add support for HX83100A

[Felix Kaechele]

The HX83100A is a bit of an outlier in the Himax HX831xxx series of
touch controllers as it requires reading touch events through the AHB
interface of the MCU rather than providing a dedicated FIFO address like
the other chips do.
This patch implements the specific read function and introduces the
HX83100A chip with an appropriate i2c ID and DT compatible string.

The HX83100A doesn't have a straightforward way to do chip
identification, which is why it is not implemented in this patch.

Tested on: Lenovo ThinkSmart View (CD-18781Y) / Innolux P080DDD-AB2 LCM

Signed-off-by: Felix Kaechele <felix@kaechele.ca>
Tested-by: Paul Gale <paul@siliconpixel.com>
---
 drivers/input/touchscreen/himax_hx83112b.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index 5092a357c332..9ed3bccde4ac 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -4,6 +4,9 @@
  *
  * Copyright (C) 2022 Job Noorman <job@noorman.info>
  *
+ * HX83100A support
+ * Copyright (C) 2024 Felix Kaechele <felix@kaechele.ca>
+ *
  * This code is based on "Himax Android Driver Sample Code for QCT platform":
  *
  * Copyright (C) 2017 Himax Corporation.
@@ -35,6 +38,8 @@
 
 #define HIMAX_REG_ADDR_ICID			0x900000d0
 
+#define HX83100A_REG_FW_EVENT_STACK		0x90060000
+
 #define HIMAX_INVALID_COORD		0xffff
 
 struct himax_event_point {
@@ -288,6 +293,12 @@ static int himax_read_events(struct himax_ts_data *ts,
 			       length);
 }
 
+static int hx83100a_read_events(struct himax_ts_data *ts,
+				struct himax_event *event, size_t length)
+{
+	return himax_bus_read(ts, HX83100A_REG_FW_EVENT_STACK, event, length);
+};
+
 static int himax_handle_input(struct himax_ts_data *ts)
 {
 	int error;
@@ -394,6 +405,10 @@ static int himax_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(himax_pm_ops, himax_suspend, himax_resume);
 
+static const struct himax_chip hx83100a_chip = {
+	.read_events = hx83100a_read_events,
+};
+
 static const struct himax_chip hx83112b_chip = {
 	.id = 0x83112b,
 	.check_id = himax_check_product_id,
@@ -401,6 +416,7 @@ static const struct himax_chip hx83112b_chip = {
 };
 
 static const struct i2c_device_id himax_ts_id[] = {
+	{ "hx83100a", (kernel_ulong_t)&hx83100a_chip },
 	{ "hx83112b", (kernel_ulong_t)&hx83112b_chip },
 	{ /* sentinel */ }
 };
@@ -408,6 +424,7 @@ MODULE_DEVICE_TABLE(i2c, himax_ts_id);
 
 #ifdef CONFIG_OF
 static const struct of_device_id himax_of_match[] = {
+	{ .compatible = "himax,hx83100a", .data = &hx83100a_chip },
 	{ .compatible = "himax,hx83112b", .data = &hx83112b_chip },
 	{ /* sentinel */ }
 };
-- 
2.45.2

Re: [PATCH v3 0/5] himax_hx83112b: add support for HX83100A

[Dmitry Torokhov]

On Thu, Jun 20, 2024 at 10:50:01AM -0400, Felix Kaechele wrote:
> Original discussion around v2 here: https://lore.kernel.org/all/20240511121245.109644-1-felix@kaechele.ca/
> 
> Changes from v2:
> - reworded dt-bindings commit message to specifiy how the HX83100A is different
>   from other chips in this series to justify having its own compatible string
> - added Acked-By from Conor that was given in the v2 thread to commit message
> 
> I've done some more testing in the meantime and haven't found any issues.
> I haven't heard back from anyone with an HX83112B, but the HX83100A works as
> intended with these changes and I assume the HX83112B will too.

Applied the lot, thank you.

-- 
Dmitry