* [PATCH 0/6] input: himax_hx83112b: add support for HX83100A
@ 2024-05-04 2:04 Felix Kaechele
2024-05-04 2:04 ` [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A Felix Kaechele
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
This set of patches brings support for the Himax HX83100A touch
controller.
To properly bring the chip up, support for regulator handling is also
added.
I have no access to datasheets. So, like the original driver code
that's being extended here, this code is mostly based on the quite
convoluted, GPLv2 licensed manufacturer drivers for Android.
I included links to sources and references where appropriate.
A number of people tested this patch set on Lenovo ThinkSmart View
(CD-18781Y) devices. That device has a variant utilizing a Innolux
P080DDD-AB2 LCM. This LCM comes with the HX83100A.
I would really appreciate if people using HX83112B chips could give this
set a run to ensure nothing broke.
Thanks,
Felix
Felix Kaechele (6):
dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A
input: himax_hx83112b: add regulator handling
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 | 9 +
drivers/input/touchscreen/himax_hx83112b.c | 187 +++++++++++++++---
2 files changed, 166 insertions(+), 30 deletions(-)
base-commit: 7b4e0b39182cf5e677c1fc092a3ec40e621c25b6
--
2.44.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A
2024-05-04 2:04 [PATCH 0/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
@ 2024-05-04 2:04 ` Felix Kaechele
2024-05-04 12:34 ` Krzysztof Kozlowski
2024-05-04 2:04 ` [PATCH 2/6] input: himax_hx83112b: add regulator handling Felix Kaechele
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
This adds a compatible string for the Himax HX83100A touch controller
including the AVDD and VDD supply nodes used by this chip family.
Signed-off-by: Felix Kaechele <felix@kaechele.ca>
---
.../bindings/input/touchscreen/himax,hx83112b.yaml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
index f42b23d532eb..5809afedb9a2 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:
@@ -26,6 +27,12 @@ properties:
reset-gpios:
maxItems: 1
+ avdd-supply:
+ description: Analog power supply regulator
+
+ vdd-supply:
+ description: Digital power supply regulator
+
touchscreen-inverted-x: true
touchscreen-inverted-y: true
touchscreen-size-x: true
@@ -54,6 +61,8 @@ examples:
reg = <0x48>;
interrupt-parent = <&tlmm>;
interrupts = <65 IRQ_TYPE_LEVEL_LOW>;
+ avdd-supply = <&avdd_reg>;
+ vdd-supply = <&vdd_reg>;
touchscreen-size-x = <1080>;
touchscreen-size-y = <2160>;
reset-gpios = <&tlmm 64 GPIO_ACTIVE_LOW>;
--
2.44.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/6] input: himax_hx83112b: add regulator handling
2024-05-04 2:04 [PATCH 0/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
2024-05-04 2:04 ` [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A Felix Kaechele
@ 2024-05-04 2:04 ` Felix Kaechele
2024-05-04 12:37 ` Krzysztof Kozlowski
2024-05-04 2:04 ` [PATCH 3/6] input: himax_hx83112b: use more descriptive register defines Felix Kaechele
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
Handle regulators used on this chip family, namely AVDD and VDD. These
definitions are taken from the GPLv2 licensed vendor driver.
Signed-off-by: Felix Kaechele <felix@kaechele.ca>
Link: https://github.com/HimaxSoftware/HX83112_Android_Driver
---
drivers/input/touchscreen/himax_hx83112b.c | 48 ++++++++++++++++++++--
1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index 4f6609dcdef3..0a797789e548 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -19,10 +19,12 @@
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#define HIMAX_ID_83112B 0x83112b
#define HIMAX_MAX_POINTS 10
+#define HIMAX_MAX_SUPPLIES 2
#define HIMAX_REG_CFG_SET_ADDR 0x00
#define HIMAX_REG_CFG_INIT_READ 0x0c
@@ -50,6 +52,7 @@ struct himax_event {
static_assert(sizeof(struct himax_event) == 56);
struct himax_ts_data {
+ struct regulator_bulk_data supplies[HIMAX_MAX_SUPPLIES];
struct gpio_desc *gpiod_rst;
struct input_dev *input_dev;
struct i2c_client *client;
@@ -63,6 +66,11 @@ static const struct regmap_config himax_regmap_config = {
.val_format_endian = REGMAP_ENDIAN_LITTLE,
};
+static const char *const himax_supply_names[] = {
+ "avdd",
+ "vdd",
+};
+
static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
{
int error;
@@ -267,7 +275,7 @@ static irqreturn_t himax_irq_handler(int irq, void *dev_id)
static int himax_probe(struct i2c_client *client)
{
- int error;
+ int error, i;
struct device *dev = &client->dev;
struct himax_ts_data *ts;
@@ -290,11 +298,31 @@ static int himax_probe(struct i2c_client *client)
return error;
}
+ int num_supplies = ARRAY_SIZE(himax_supply_names);
+
+ for (i = 0; i < num_supplies; i++)
+ ts->supplies[i].supply = himax_supply_names[i];
+
+ error = devm_regulator_bulk_get(dev,
+ num_supplies,
+ ts->supplies);
+ if (error) {
+ dev_err(dev, "Failed to get supplies: %d\n", error);
+ return error;
+ }
+
+ error = regulator_bulk_enable(num_supplies,
+ ts->supplies);
+ if (error) {
+ dev_err(dev, "Failed to enable supplies: %d\n", error);
+ goto error_out;
+ }
+
ts->gpiod_rst = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
error = PTR_ERR_OR_ZERO(ts->gpiod_rst);
if (error) {
dev_err(dev, "Failed to get reset GPIO: %d\n", error);
- return error;
+ goto error_out;
}
himax_reset(ts);
@@ -305,15 +333,26 @@ static int himax_probe(struct i2c_client *client)
error = himax_input_register(ts);
if (error)
- return error;
+ goto error_out;
error = devm_request_threaded_irq(dev, client->irq, NULL,
himax_irq_handler, IRQF_ONESHOT,
client->name, ts);
if (error)
- return error;
+ goto error_out;
return 0;
+
+error_out:
+ regulator_bulk_disable(num_supplies, ts->supplies);
+ return error;
+}
+
+static void himax_remove(struct i2c_client *client)
+{
+ struct himax_ts_data *ts = i2c_get_clientdata(client);
+
+ regulator_bulk_disable(ARRAY_SIZE(himax_supply_names), ts->supplies);
}
static int himax_suspend(struct device *dev)
@@ -350,6 +389,7 @@ MODULE_DEVICE_TABLE(of, himax_of_match);
static struct i2c_driver himax_ts_driver = {
.probe = himax_probe,
+ .remove = himax_remove,
.id_table = himax_ts_id,
.driver = {
.name = "Himax-hx83112b-TS",
--
2.44.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/6] input: himax_hx83112b: use more descriptive register defines
2024-05-04 2:04 [PATCH 0/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
2024-05-04 2:04 ` [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A Felix Kaechele
2024-05-04 2:04 ` [PATCH 2/6] input: himax_hx83112b: add regulator handling Felix Kaechele
@ 2024-05-04 2:04 ` Felix Kaechele
2024-05-04 2:04 ` [PATCH 4/6] input: himax_hx83112b: implement MCU register reading Felix Kaechele
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
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 0a797789e548..ba5442cc0a24 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -26,12 +26,14 @@
#define HIMAX_MAX_POINTS 10
#define HIMAX_MAX_SUPPLIES 2
-#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
@@ -75,15 +77,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;
@@ -109,7 +112,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;
@@ -243,7 +246,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.44.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/6] input: himax_hx83112b: implement MCU register reading
2024-05-04 2:04 [PATCH 0/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
` (2 preceding siblings ...)
2024-05-04 2:04 ` [PATCH 3/6] input: himax_hx83112b: use more descriptive register defines Felix Kaechele
@ 2024-05-04 2:04 ` Felix Kaechele
2024-05-04 2:04 ` [PATCH 5/6] input: himax_hx83112b: add himax_chip struct for multi-chip support Felix Kaechele
2024-05-04 2:04 ` [PATCH 6/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
5 siblings, 0 replies; 11+ messages in thread
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
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 | 51 ++++++++++++++++++++--
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index ba5442cc0a24..0173ff394a99 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -29,9 +29,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
@@ -73,10 +77,34 @@ static const char *const himax_supply_names[] = {
"vdd",
};
-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;
@@ -86,7 +114,24 @@ 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;
@@ -112,7 +157,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.44.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/6] input: himax_hx83112b: add himax_chip struct for multi-chip support
2024-05-04 2:04 [PATCH 0/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
` (3 preceding siblings ...)
2024-05-04 2:04 ` [PATCH 4/6] input: himax_hx83112b: implement MCU register reading Felix Kaechele
@ 2024-05-04 2:04 ` Felix Kaechele
2024-05-04 2:04 ` [PATCH 6/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
5 siblings, 0 replies; 11+ messages in thread
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
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 | 53 +++++++++++++++-------
1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index 0173ff394a99..667611c5a018 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -21,8 +21,6 @@
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
-#define HIMAX_ID_83112B 0x83112b
-
#define HIMAX_MAX_POINTS 10
#define HIMAX_MAX_SUPPLIES 2
@@ -57,7 +55,17 @@ 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 regulator_bulk_data supplies[HIMAX_MAX_SUPPLIES];
struct gpio_desc *gpiod_rst;
struct input_dev *input_dev;
@@ -176,15 +184,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)
@@ -286,13 +291,20 @@ 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);
@@ -338,6 +350,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);
@@ -375,9 +388,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)
@@ -421,15 +436,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", 0 },
+ { "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.44.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/6] input: himax_hx83112b: add support for HX83100A
2024-05-04 2:04 [PATCH 0/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
` (4 preceding siblings ...)
2024-05-04 2:04 ` [PATCH 5/6] input: himax_hx83112b: add himax_chip struct for multi-chip support Felix Kaechele
@ 2024-05-04 2:04 ` Felix Kaechele
5 siblings, 0 replies; 11+ messages in thread
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
Cc: Paul Gale
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 | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index 667611c5a018..984baef495e1 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.
@@ -37,6 +40,8 @@
#define HIMAX_REG_ADDR_ICID 0x900000d0
+#define HX83100A_REG_FW_EVENT_STACK 0x90060000
+
#define HIMAX_INVALID_COORD 0xffff
struct himax_event_point {
@@ -291,6 +296,13 @@ static bool himax_verify_checksum(struct himax_ts_data *ts,
return true;
}
+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_read_events(struct himax_ts_data *ts,
struct himax_event *event,
size_t length)
@@ -436,6 +448,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,
@@ -443,6 +459,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 */ }
};
@@ -450,6 +467,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.44.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A
2024-05-04 2:04 ` [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A Felix Kaechele
@ 2024-05-04 12:34 ` Krzysztof Kozlowski
2024-05-07 18:48 ` Rob Herring
0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-04 12:34 UTC (permalink / raw)
To: Felix Kaechele, Job Noorman, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree,
linux-kernel
On 04/05/2024 04:04, Felix Kaechele wrote:
> This adds a compatible string for the Himax HX83100A touch controller
Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> including the AVDD and VDD supply nodes used by this chip family.
>
> Signed-off-by: Felix Kaechele <felix@kaechele.ca>
> ---
> .../bindings/input/touchscreen/himax,hx83112b.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
> index f42b23d532eb..5809afedb9a2 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:
> @@ -26,6 +27,12 @@ properties:
> reset-gpios:
> maxItems: 1
>
> + avdd-supply:
> + description: Analog power supply regulator
> +
> + vdd-supply:
> + description: Digital power supply regulator
These should not be allowed for other variant, so either you need
allOf:if:then disallowing them (: false) or just create another binding
file.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/6] input: himax_hx83112b: add regulator handling
2024-05-04 2:04 ` [PATCH 2/6] input: himax_hx83112b: add regulator handling Felix Kaechele
@ 2024-05-04 12:37 ` Krzysztof Kozlowski
2024-05-07 14:33 ` Felix Kaechele
0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-04 12:37 UTC (permalink / raw)
To: Felix Kaechele, Job Noorman, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree,
linux-kernel
On 04/05/2024 04:04, Felix Kaechele wrote:
> Handle regulators used on this chip family, namely AVDD and VDD. These
> definitions are taken from the GPLv2 licensed vendor driver.
>
> Signed-off-by: Felix Kaechele <felix@kaechele.ca>
> Link: https://github.com/HimaxSoftware/HX83112_Android_Driver
> ---
> drivers/input/touchscreen/himax_hx83112b.c | 48 ++++++++++++++++++++--
> 1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
> index 4f6609dcdef3..0a797789e548 100644
> --- a/drivers/input/touchscreen/himax_hx83112b.c
> +++ b/drivers/input/touchscreen/himax_hx83112b.c
> @@ -19,10 +19,12 @@
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>
> #define HIMAX_ID_83112B 0x83112b
>
> #define HIMAX_MAX_POINTS 10
> +#define HIMAX_MAX_SUPPLIES 2
>
> #define HIMAX_REG_CFG_SET_ADDR 0x00
> #define HIMAX_REG_CFG_INIT_READ 0x0c
> @@ -50,6 +52,7 @@ struct himax_event {
> static_assert(sizeof(struct himax_event) == 56);
>
> struct himax_ts_data {
> + struct regulator_bulk_data supplies[HIMAX_MAX_SUPPLIES];
> struct gpio_desc *gpiod_rst;
> struct input_dev *input_dev;
> struct i2c_client *client;
> @@ -63,6 +66,11 @@ static const struct regmap_config himax_regmap_config = {
> .val_format_endian = REGMAP_ENDIAN_LITTLE,
> };
>
> +static const char *const himax_supply_names[] = {
> + "avdd",
> + "vdd",
> +};
> +
That's confusing. Binding said only HX83100A family has regulators, but
you request for everyone.
> static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
> {
> int error;
> @@ -267,7 +275,7 @@ static irqreturn_t himax_irq_handler(int irq, void *dev_id)
>
> static int himax_probe(struct i2c_client *client)
> {
> - int error;
> + int error, i;
> struct device *dev = &client->dev;
> struct himax_ts_data *ts;
>
> @@ -290,11 +298,31 @@ static int himax_probe(struct i2c_client *client)
> return error;
> }
>
> + int num_supplies = ARRAY_SIZE(himax_supply_names);
> +
> + for (i = 0; i < num_supplies; i++)
> + ts->supplies[i].supply = himax_supply_names[i];
> +
> + error = devm_regulator_bulk_get(dev,
devm_regulator_bulk_get_enable and drop rest of the code here.
> + num_supplies,
> + ts->supplies);
Wrap it properly at 80, not one argument in one line.
> + if (error) {
> + dev_err(dev, "Failed to get supplies: %d\n", error);
return dev_err_probe()
> + return error;
> + }
> +
> + error = regulator_bulk_enable(num_supplies,
> + ts->supplies);
> + if (error) {
> + dev_err(dev, "Failed to enable supplies: %d\n", error);
> + goto error_out;
> + }
> +
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/6] input: himax_hx83112b: add regulator handling
2024-05-04 12:37 ` Krzysztof Kozlowski
@ 2024-05-07 14:33 ` Felix Kaechele
0 siblings, 0 replies; 11+ messages in thread
From: Felix Kaechele @ 2024-05-07 14:33 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
Thanks, Krzysztof! Really appreciate your input, as I'm currently not a
regular contributor to the kernel.
On 2024-05-04 08:37, Krzysztof Kozlowski wrote:
> On 04/05/2024 04:04, Felix Kaechele wrote:
>> Handle regulators used on this chip family, namely AVDD and VDD. These
>> definitions are taken from the GPLv2 licensed vendor driver.
>>
>> Signed-off-by: Felix Kaechele <felix@kaechele.ca>
>> Link: https://github.com/HimaxSoftware/HX83112_Android_Driver
>> ---
>> drivers/input/touchscreen/himax_hx83112b.c | 48 ++++++++++++++++++++--
>> 1 file changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
>> index 4f6609dcdef3..0a797789e548 100644
>> --- a/drivers/input/touchscreen/himax_hx83112b.c
>> +++ b/drivers/input/touchscreen/himax_hx83112b.c
>> @@ -19,10 +19,12 @@
>> #include <linux/interrupt.h>
>> #include <linux/kernel.h>
>> #include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>>
>> #define HIMAX_ID_83112B 0x83112b
>>
>> #define HIMAX_MAX_POINTS 10
>> +#define HIMAX_MAX_SUPPLIES 2
>>
>> #define HIMAX_REG_CFG_SET_ADDR 0x00
>> #define HIMAX_REG_CFG_INIT_READ 0x0c
>> @@ -50,6 +52,7 @@ struct himax_event {
>> static_assert(sizeof(struct himax_event) == 56);
>>
>> struct himax_ts_data {
>> + struct regulator_bulk_data supplies[HIMAX_MAX_SUPPLIES];
>> struct gpio_desc *gpiod_rst;
>> struct input_dev *input_dev;
>> struct i2c_client *client;
>> @@ -63,6 +66,11 @@ static const struct regmap_config himax_regmap_config = {
>> .val_format_endian = REGMAP_ENDIAN_LITTLE,
>> };
>>
>> +static const char *const himax_supply_names[] = {
>> + "avdd",
>> + "vdd",
>> +};
>> +
>
> That's confusing. Binding said only HX83100A family has regulators, but
> you request for everyone.
>
You're right. Looking at the vendor driver for each of models I could
see that it defined AVDD and VDD in both drivers. So I thought it could
make sense to offer it for the entire chip family, with which I meant
all HX831xx in this case.
But it seems after some more testing (and with this touch IC family
generally being a part of the display controller) the regulators are
sufficiently handled by the panel driver / bootloader for the use case
of having the touchscreen on when the display is on.
It wouldn't allow for waking the screen by using the touchscreen, and
while I'd have to go back to the original OS on the device to verify
this again, I don't remember that working on the original OS either.
So I'm thinking I may drop the whole regulator part of the patchset to
keep it smaller.
>> static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
>> {
>> int error;
>> @@ -267,7 +275,7 @@ static irqreturn_t himax_irq_handler(int irq, void *dev_id)
>>
>> static int himax_probe(struct i2c_client *client)
>> {
>> - int error;
>> + int error, i;
>> struct device *dev = &client->dev;
>> struct himax_ts_data *ts;
>>
>> @@ -290,11 +298,31 @@ static int himax_probe(struct i2c_client *client)
>> return error;
>> }
>>
>> + int num_supplies = ARRAY_SIZE(himax_supply_names);
>> +
>> + for (i = 0; i < num_supplies; i++)
>> + ts->supplies[i].supply = himax_supply_names[i];
>> +
>> + error = devm_regulator_bulk_get(dev,
>
> devm_regulator_bulk_get_enable and drop rest of the code here.
>
That's pretty neat. If I do decide to bring in regulator handling this
removes quite a bit of boilerplate.
>
>> + num_supplies,
>> + ts->supplies);
>
> Wrap it properly at 80, not one argument in one line.
>
>> + if (error) {
>> + dev_err(dev, "Failed to get supplies: %d\n", error);
>
> return dev_err_probe()
>
Same here. Thanks for the hint.
>> + return error;
>> + }
>> +
>> + error = regulator_bulk_enable(num_supplies,
>> + ts->supplies);
>> + if (error) {
>> + dev_err(dev, "Failed to enable supplies: %d\n", error);
>> + goto error_out;
>> + }
>> +
I'll be sending a v2 shortly.
Thanks again,
Felix
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A
2024-05-04 12:34 ` Krzysztof Kozlowski
@ 2024-05-07 18:48 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2024-05-07 18:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Felix Kaechele, Job Noorman, Dmitry Torokhov,
Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree,
linux-kernel
On Sat, May 04, 2024 at 02:34:08PM +0200, Krzysztof Kozlowski wrote:
> On 04/05/2024 04:04, Felix Kaechele wrote:
> > This adds a compatible string for the Himax HX83100A touch controller
>
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>
> > including the AVDD and VDD supply nodes used by this chip family.
> >
> > Signed-off-by: Felix Kaechele <felix@kaechele.ca>
> > ---
> > .../bindings/input/touchscreen/himax,hx83112b.yaml | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
> > index f42b23d532eb..5809afedb9a2 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:
> > @@ -26,6 +27,12 @@ properties:
> > reset-gpios:
> > maxItems: 1
> >
> > + avdd-supply:
> > + description: Analog power supply regulator
> > +
> > + vdd-supply:
> > + description: Digital power supply regulator
>
> These should not be allowed for other variant, so either you need
> allOf:if:then disallowing them (: false) or just create another binding
> file.
Or the commit message needs some explanation that the supplies also
apply to the 83112b as the existing binding has no supplies.
Rob
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-05-07 18:48 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-04 2:04 [PATCH 0/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
2024-05-04 2:04 ` [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A Felix Kaechele
2024-05-04 12:34 ` Krzysztof Kozlowski
2024-05-07 18:48 ` Rob Herring
2024-05-04 2:04 ` [PATCH 2/6] input: himax_hx83112b: add regulator handling Felix Kaechele
2024-05-04 12:37 ` Krzysztof Kozlowski
2024-05-07 14:33 ` Felix Kaechele
2024-05-04 2:04 ` [PATCH 3/6] input: himax_hx83112b: use more descriptive register defines Felix Kaechele
2024-05-04 2:04 ` [PATCH 4/6] input: himax_hx83112b: implement MCU register reading Felix Kaechele
2024-05-04 2:04 ` [PATCH 5/6] input: himax_hx83112b: add himax_chip struct for multi-chip support Felix Kaechele
2024-05-04 2:04 ` [PATCH 6/6] input: himax_hx83112b: add support for HX83100A Felix Kaechele
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).