Hi Jelle,
On Fri, Sep 09, 2016 at 08:58:36PM +0200, Jelle van der Waa wrote:
> This is a basic driver for the Zeitec ZET6223 I2C touchscreen controllers.
> The driver does not support firmware loading, which is not required for
> all tablets which contain this chip.
>
> Signed-off-by: Jelle van der Waa <jelle@xxxxxxxx>
> ---
> .../bindings/input/touchscreen/zet6223.txt | 33 ++++
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> drivers/input/touchscreen/Kconfig | 11 ++
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/zet6223.c | 190 +++++++++++++++++++++
> 5 files changed, 236 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zet6223.txt
> create mode 100644 drivers/input/touchscreen/zet6223.c
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt b/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt
> new file mode 100644
> index 0000000..bdb2c55
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt
> @@ -0,0 +1,33 @@
> +* Zeitec ZET6223 I2C touchscreen controller
> +
> +Required properties:
> + - compatible : "zeitec,zet6223"
> + - reg : I2C slave address of the chip (0x76)
> + - interrupt-parent : a phandle pointing to the interrupt controller
> + serving the interrupt for this chip
> + - interrupts : interrupt specification for the zet6223 interrupt
> + - power-gpios : Specification for the pin connected to the gsl1680's
> + shutdown input. This needs to be driven high to take the
> + gsl1680 out of its low power state
Is it truly power GPIO and not active-low reset GPIO that you need to
driver low for certain period of time before releasing, to re-initialize
the chip?
> +
> +Optional properties:
> +
> +- touchscreen-size-x : See touchscreen.txt
> +- touchscreen-size-y : See touchscreen.txt
> +- touchscreen-inverted-x : See touchscreen.txt
> +- touchscreen-inverted-y : See touchscreen.txt
> +- touchscreen-swapped-x-y : See touchscreen.txt
> +
> +Example:
> +
> +i2c@00000000 {
> +
> + zet6223: touchscreen@76 {
> + compatible = "zeitec,zet6223";
> + reg = <0x76>;
> + interrupt-parent = <&pio>;
> + interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>
> + power-gpios = <&pio 1 3 GPIO_ACTIVE_HIGH>;
> + };
> +
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 1992aa9..1c2cf45 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -295,5 +295,6 @@ xillybus Xillybus Ltd.
> xlnx Xilinx
> zyxel ZyXEL Communications Corp.
> zarlink Zarlink Semiconductor
> +zeitec ZEITEC Semiconductor Co., LTD.
> zii Zodiac Inflight Innovations
> zte ZTE Corp.
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 2fb1f43..1fa4671 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1206,4 +1206,15 @@ config TOUCHSCREEN_ROHM_BU21023
> To compile this driver as a module, choose M here: the
> module will be called bu21023_ts.
>
> +config TOUCHSCREEN_ZET6223
> + tristate "Zeitec ZET6223 touchscreen driver"
> + depends on I2C
> + help
> + Say Y here if you have a touchscreen using Zeitec ZET6223
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called zet6223.
> +
> endif
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index b4373d6..af4608d 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -99,3 +99,4 @@ obj-$(CONFIG_TOUCHSCREEN_TPS6507X) += tps6507x-ts.o
> obj-$(CONFIG_TOUCHSCREEN_ZFORCE) += zforce_ts.o
> obj-$(CONFIG_TOUCHSCREEN_COLIBRI_VF50) += colibri-vf50-ts.o
> obj-$(CONFIG_TOUCHSCREEN_ROHM_BU21023) += rohm_bu21023.o
> +obj-$(CONFIG_TOUCHSCREEN_ZET6223) += zet6223.o
> diff --git a/drivers/input/touchscreen/zet6223.c b/drivers/input/touchscreen/zet6223.c
> new file mode 100644
> index 0000000..661ca0f
> --- /dev/null
> +++ b/drivers/input/touchscreen/zet6223.c
> @@ -0,0 +1,190 @@
> +/*
> + * Copyright (C) 2016, Jelle van der Waa <jelle@xxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <asm/unaligned.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/module.h>
> +
> +#define ZET6223_CMD_INFO 0xB2
> +#define ZET6223_CMD_INFO_LENGTH 17
> +#define ZET6223_VALID_PACKET 0x3c
> +
> +struct zet6223_data {
> + struct gpio_desc *power_gpios;
> + struct i2c_client *client;
> + struct input_dev *input;
> + struct touchscreen_properties prop;
> + u8 fingernum;
> +};
> +
> +static int zet6223_start(struct input_dev *dev)
> +{
> + struct zet6223_data *data = input_get_drvdata(dev);
> +
> + enable_irq(data->client->irq);
> +
> + return 0;
Wierd indent.
> +}
> +
> +static void zet6223_stop(struct input_dev *dev)
> +{
> + struct zet6223_data *data = input_get_drvdata(dev);
> +
> + disable_irq(data->client->irq);
Same here.
> +}
> +
> +static irqreturn_t irqreturn_t_zet6223(int irq, void *dev_id)
> +{
> + struct zet6223_data *data = dev_id;
> + // First 3 bytes are an identifier, two bytes of finger data.
> + // X, Y data per finger is 4 bytes.
No C++ style comments in the kernel please.
> + u8 bufsize = 3 + 4 * data->fingernum;
> + u8 buf[bufsize];
> + u8 i;
> + int ret;
> +
> + ret = i2c_master_recv(data->client, buf, bufsize);
> + if (ret != bufsize)
> + return IRQ_HANDLED;
I think we'd be interested to know if i2c comm is not working. A
[ratelimited] message would do us good.
> +
> + if (buf[0] != ZET6223_VALID_PACKET)
> + return IRQ_HANDLED;
> +
Hmm, should we do:
u16 finger_bits = get_unaligned_be16(buf + 1);
> + for (i = 0; i < data->fingernum; i++) {
> + if (!(buf[i / 8 + 1] & (0x80 >> (i % 8))))
> + continue;
if (!(finger_bits & BIT(15 - i))
continue;
> +
> + input_mt_slot(data->input, i);
Hmm, that would imply that firmware keeps accurate track of contacts
instead of just reporting N contact points and having kernel/userspace
figure out if they match old contacts or not. If that is so: great, but
why you request in-kernel tracking then (INPUT_MT_TRACK).
> + input_mt_report_slot_state(data->input, MT_TOOL_FINGER, true);
> + input_event(data->input, EV_ABS, ABS_MT_POSITION_X,
> + ((buf[i + 3] >> 4) << 8) + buf[i + 4]);
> + input_event(data->input, EV_ABS, ABS_MT_POSITION_Y,
> + ((buf[i + 3] & 0xF) << 8) + buf[i + 5]);
> + }
> +
> + input_mt_sync_frame(data->input);
> + input_sync(data->input);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int zet6223_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct zet6223_data *data;
> + struct input_dev *input;
> + u8 buf[ZET6223_CMD_INFO_LENGTH];
> + u8 cmd = ZET6223_CMD_INFO;
> + int ret, error;
> +
> + if (!client->irq) {
> + dev_err(dev, "Error no irq specified\n");
> + return -EINVAL;
> + }
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->power_gpios = devm_gpiod_get(dev, "power", GPIOD_OUT_HIGH);
> + if (IS_ERR(data->power_gpios)) {
> + error = PTR_ERR(data->power_gpios);
> + if (error != -EPROBE_DEFER)
> + dev_err(dev, "Error getting power gpio: %d\n", error);
> + return error;
> + }
Don't you need to wait, at least a little, after powering up the
device, for it to initialize.
> +
> + ret = i2c_master_send(client, &cmd, 1);
> + if (ret < 0) {
> + dev_err(dev, "touchpanel info cmd failed: %d\n", ret);
> + return -ENODEV;
> + }
> +
> + ret = i2c_master_recv(client, buf, ZET6223_CMD_INFO_LENGTH);
> + if (ret < 0) {
> + dev_err(dev, "cannot retrieve touchpanel info: %d\n", ret);
> + return -ENODEV;
> + }
> +
> + data->fingernum = buf[15] & 0x7F;
You only have 2 bytes - 16 bits - to report fingers. 127 max is way too
big.
> +
> + input = devm_input_allocate_device(dev);
> + if (!input)
> + return -ENOMEM;
> +
> + input_set_abs_params(input, ABS_MT_POSITION_X, 0,
> + get_unaligned_le16(&buf[8]), 0, 0);
> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0,
> + get_unaligned_le16(&buf[10]), 0, 0);
> + touchscreen_parse_properties(input, true, &data->prop);
> +
> + input->name = client->name;
> + input->id.bustype = BUS_I2C;
> + input->dev.parent = dev;
> + input->open = zet6223_start;
> + input->close = zet6223_stop;
> +
> + ret = input_mt_init_slots(input, data->fingernum,
> + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED | INPUT_MT_TRACK);
> + if (ret)
> + return ret;
> +
> + data->client = client;
> + data->input = input;
> +
> + input_set_drvdata(input, data);
> +
> + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> + irqreturn_t_zet6223, IRQF_ONESHOT, client->name, data);
> + if (ret) {
> + dev_err(dev, "Error requesting irq: %d\n", ret);
> + return ret;
> + }
> +
> + zet6223_stop(input);
> +
> + ret = input_register_device(input);
> + if (ret)
> + return ret;
> +
> + i2c_set_clientdata(client, data);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id zet6223_id[] = {
> + { "zet6223", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, zet6223_id);
I'd rather we have proper of match table as well.
> +
> +static struct i2c_driver zet6223_driver = {
> + .driver = {
> + .name = "zet6223",
> + },
> + .probe = zet6223_probe,
> + .id_table = zet6223_id
> +};
> +
> +module_i2c_driver(zet6223_driver);
> +
> +MODULE_AUTHOR("Jelle van der Waa <jelle@xxxxxxxx>");
> +MODULE_DESCRIPTION("ZEITEC zet622x I2C touchscreen driver");
> +MODULE_LICENSE("GPL");
> --
> 2.9.3
>
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
