Re: [PATCH v3 2/2] Input: add Imagis touchscreen driver
From: Krzysztof Kozlowski <hidden>
Date: 2022-02-15 16:05:49
Also in:
linux-devicetree, lkml, phone-devel
On 15/02/2022 16:15, Markuss Broks wrote:
quoted hunk ↗ jump to hunk
Add support for the IST3038C touchscreen IC from Imagis, based on downstream driver. The driver supports multi-touch (10 touch points) The IST3038C IC supports touch keys, but the support isn't added because the touch screen used for testing doesn't utilize touch keys. Looking at the downstream driver, it is possible to add support for other Imagis ICs of IST30**C series. Signed-off-by: Markuss Broks <markuss.broks@gmail.com> --- MAINTAINERS | 6 + drivers/input/touchscreen/Kconfig | 10 + drivers/input/touchscreen/Makefile | 1 + drivers/input/touchscreen/imagis.c | 330 +++++++++++++++++++++++++++++ 4 files changed, 347 insertions(+) create mode 100644 drivers/input/touchscreen/imagis.cdiff --git a/MAINTAINERS b/MAINTAINERS index a899828a8d4e..f7f717ae926a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS@@ -9405,6 +9405,12 @@ F: Documentation/devicetree/bindings/iio/afe/current-sense-shunt.yaml F: Documentation/devicetree/bindings/iio/afe/voltage-divider.yaml F: drivers/iio/afe/iio-rescale.c +IMAGIS TOUCHSCREEN DRIVER
Please read the line 143-144 of this file.
quoted hunk ↗ jump to hunk
+M: Markuss Broks [off-list ref] +S: Maintained +F: Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml +F: drivers/input/touchscreen/imagis.c + IKANOS/ADI EAGLE ADSL USB DRIVER M: Matthieu Castet [off-list ref] M: Stanislaw Gruszka [off-list ref]diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index 2f6adfb7b938..6810b4b094e8 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig@@ -1367,4 +1367,14 @@ config TOUCHSCREEN_ZINITIX To compile this driver as a module, choose M here: the module will be called zinitix. +config TOUCHSCREEN_IMAGIS
Here and in Makefile - do not add entries at the end, but alphabetically. This reduces conflicts.
quoted hunk ↗ jump to hunk
+ tristate "Imagis touchscreen support" + depends on I2C + help + Say Y here if you have an Imagis IST30xxC touchscreen. + If unsure, say N. + + To compile this driver as a module, choose M here: the + module will be called imagis. + endifdiff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile index 39a8127cf6a5..989bb1d563d3 100644 --- a/drivers/input/touchscreen/Makefile +++ b/drivers/input/touchscreen/Makefile@@ -115,3 +115,4 @@ obj-$(CONFIG_TOUCHSCREEN_ROHM_BU21023) += rohm_bu21023.o obj-$(CONFIG_TOUCHSCREEN_RASPBERRYPI_FW) += raspberrypi-ts.o obj-$(CONFIG_TOUCHSCREEN_IQS5XX) += iqs5xx.o obj-$(CONFIG_TOUCHSCREEN_ZINITIX) += zinitix.o +obj-$(CONFIG_TOUCHSCREEN_IMAGIS) += imagis.o
(...)
+
+static int imagis_init_regulators(struct imagis_ts *ts)
+{
+ struct i2c_client *client = ts->client;
+ int error;
+
+ ts->supplies[0].supply = "vdd";
+ ts->supplies[1].supply = "vddio";
+ error = devm_regulator_bulk_get(&client->dev,
+ ARRAY_SIZE(ts->supplies),
+ ts->supplies);
+ if (error < 0) {
+ dev_err(&client->dev, "Failed to get regulators: %d\n", error);This should be also dev_err_probe() in case of deferred probing. On the other hand, you already handle printing in probe(), so why doing it twice?
+ return error;
+ }
+
+ return 0;
+}
+
+static int imagis_probe(struct i2c_client *i2c)
+{
+ struct device *dev;
+ struct imagis_ts *ts;
+ int chip_id, ret;
+
+ dev = &i2c->dev;
+
+ ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
+ ts->client = i2c;
+
+ ret = imagis_init_regulators(ts);
+ if (ret)
+ return dev_err_probe(dev, ret, "regulator init error: %d\n", ret);
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(ts->supplies),
+ ts->supplies);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to enable regulators: %d\n", ret);
+
+ msleep(IST3038C_CHIP_ON_DELAY);
+
+ ret = imagis_i2c_read_reg(ts, IST3038C_REG_CHIPID | IST3038C_DIRECT_ACCESS, &chip_id);
+ if (ret)You miss here and other error paths the regulator cleanup (disabling).
+ return dev_err_probe(dev, ret, "chip ID read failure: %d\n", ret); + + if (chip_id == IST3038C_WHOAMI) + dev_dbg(dev, "Detected IST3038C chip\n"); + else + return dev_err_probe(dev, -EINVAL, "unknown chip ID: 0x%x\n", chip_id); + + ret = devm_request_threaded_irq(dev, i2c->irq, + NULL, imagis_interrupt, + IRQF_ONESHOT | IRQF_TRIGGER_FALLING | IRQF_NO_AUTOEN,
The interrupt edge should come from DT, not be hard-coded. I think you can safely skip IRQF_TRIGGER_FALLING.
+ "imagis-touchscreen", ts); + if (ret) + return dev_err_probe(dev, ret, "IRQ allocation failure: %d\n", ret); + + ret = imagis_init_input_dev(ts); + if (ret) + return dev_err_probe(dev, ret, "input subsystem init error: %d\n", ret); + + return 0; +} +
Best regards, Krzysztof