| Message ID | 20220210163708.162866-1-markuss.broks@gmail.com |
|---|---|
| Headers | show |
| Series | Add support for Imagis touchscreens | expand |
On Thu, 10 Feb 2022 18:37:06 +0200, Markuss Broks wrote: > This patch adds device-tree bindings for the Imagis > IST3038C touch screen IC. > > Signed-off-by: Markuss Broks <markuss.broks@gmail.com> > --- > .../input/touchscreen/imagis,ist3038c.yaml | 78 +++++++++++++++++++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > 2 files changed, 80 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.example.dts:23.26-37.13: Warning (i2c_bus_reg): /example-0/i2c/touchscreen@48: I2C bus unit address format error, expected "50" doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1591241 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On 15/02/2022 16:15, Markuss Broks wrote: > This patch adds device-tree bindings for the Imagis > IST3038C touch screen IC. > > Signed-off-by: Markuss Broks <markuss.broks@gmail.com> > --- > .../input/touchscreen/imagis,ist3038c.yaml | 75 +++++++++++++++++++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > 2 files changed, 77 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > new file mode 100644 > index 000000000000..7b127817e1f6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml > @@ -0,0 +1,75 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/touchscreen/imagis,ist3038c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Imagis IST30XXC family touchscreen controller bindings > + > +maintainers: > + - Markuss Broks <markuss.broks@gmail.com> > + > +allOf: > + - $ref: touchscreen.yaml# > + > +properties: > + $nodename: > + pattern: "^touchscreen(@.*)?$" reg is required, so @ is not optional: ^touchscreen@[0-9a-f]+$ > + > + compatible: > + items: Do you expect here multiple compatibles? If not, the items are not needed. > + - enum: > + - imagis,ist3038c > + > + reg: > + description: I2C address You can skip description because it is fairly obvious, but instead you need maxItems. Alternatively, a list (items) with description defines max items. Best regards, Krzysztof
On 15/02/2022 16:15, Markuss Broks wrote: > 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.c > > diff --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. > +M: Markuss Broks <markuss.broks@gmail.com> > +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 <castet.matthieu@free.fr> > M: Stanislaw Gruszka <stf_xl@wp.pl> > 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. > + 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. > + > endif > diff --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