* [PATCH] Input: tca6416 remove unused struct 'tca6416_drv_data'
From: linux @ 2024-05-23 0:29 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Dr. David Alan Gilbert
From: "Dr. David Alan Gilbert" <linux@treblig.org>
'tca6416_drv_data' is unused since the original
commit 30ba3ead0576 ("Input: add keypad driver for keys interfaced to
TCA6416").
Remove it.
Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
---
drivers/input/keyboard/tca6416-keypad.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/input/keyboard/tca6416-keypad.c b/drivers/input/keyboard/tca6416-keypad.c
index 677bc4baa5d1..b2060229bd0f 100644
--- a/drivers/input/keyboard/tca6416-keypad.c
+++ b/drivers/input/keyboard/tca6416-keypad.c
@@ -32,11 +32,6 @@ static const struct i2c_device_id tca6416_id[] = {
};
MODULE_DEVICE_TABLE(i2c, tca6416_id);
-struct tca6416_drv_data {
- struct input_dev *input;
- struct tca6416_button data[];
-};
-
struct tca6416_keypad_chip {
uint16_t reg_output;
uint16_t reg_direction;
--
2.45.1
^ permalink raw reply related
* Re: [PATCH v9 3/8] x86/vmware: Introduce VMware hypercall API
From: Alexey Makhalov @ 2024-05-22 23:39 UTC (permalink / raw)
To: Simon Horman
Cc: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, timothym, akaher,
dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, kirill.shutemov, Nadav Amit, Jeff Sipek
In-Reply-To: <20240511150225.GK2347895@kernel.org>
Hi Simon, apologize for long delay
On 5/11/24 8:02 AM, Simon Horman wrote:
>> diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
>
> ...
>
>> +static inline
>> +unsigned long vmware_hypercall3(unsigned long cmd, unsigned long in1,
>> + uint32_t *out1, uint32_t *out2)
>
> nit: u32 is preferred over uint32_t.
> Likewise elsewhere in this patch-set.
Good to know. Can you please shed a light on the reason?
I still see bunch of stdint style uint32_t in arch/x86.
> ...
>
>> /*
>> - * The high bandwidth in call. The low word of edx is presumed to have the
>> - * HB bit set.
>> + * High bandwidth calls are not supported on encrypted memory guests.
>> + * The caller should check cc_platform_has(CC_ATTR_MEM_ENCRYPT) and use
>> + * low bandwidth hypercall it memory encryption is set.
>> + * This assumption simplifies HB hypercall impementation to just I/O port
>
> nit: implementation
>
> checkpatch.pl --codespell is your friend
Thanks, that is useful!
>
>> + * based approach without alternative patching.
>> */
>
> ...
^ permalink raw reply
* Re: [PATCH v2 2/2] input: add driver for Hynitron CST816X touchscreen
From: Dmitry Torokhov @ 2024-05-22 22:11 UTC (permalink / raw)
To: Oleh Kuzhylnyi
Cc: linux-input, jeff, neil.armstrong, schnelle, arnd, hdegoede,
linux-kernel, robh, krzysztof.kozlowski+dt, conor+dt, devicetree,
artur.serhiienko, igor.opaniuk
In-Reply-To: <20240522203347.2263425-2-kuzhylol@gmail.com>
Hi Oleh,
On Wed, May 22, 2024 at 05:33:47PM -0300, Oleh Kuzhylnyi wrote:
> Introduce support for the Hynitron CST816X touchscreen controller
> used for 240×240 1.28-inch Round LCD Display Module manufactured
> by Waveshare Electronics. The driver is designed based on an Arduino
> implementation marked as under MIT License. This driver is written
> for a particular round display based on the CST816S controller, which
> is not compatiable with existing driver for Hynitron controllers.
>
> Signed-off-by: Oleh Kuzhylnyi <kuzhylol@gmail.com>
> ---
>
> Changes in v2:
> - Apply dev_err_probe() for better error handling
> - Remove redundant printing, remove dev_warn() message spamming
> - Get rid of PM since the touchscreen goes into sleep mode automatically
> - Get rid of IRQ control and IRQF_NO_AUTOEN flag
> - Reduce timer timeout up to 10ms to handle touch events faster
> - Skip registering of non-gesture CST816X_SWIPE event
> - Shift input_register_device() as a final call in probe() callback
> - Specify name of i2c_device_id explicitly
> - Update module description and fix typo
> - Add necessary spaces between lines
>
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/hynitron-cst816x.c | 306 +++++++++++++++++++
> 3 files changed, 319 insertions(+)
> create mode 100644 drivers/input/touchscreen/hynitron-cst816x.c
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index c821fe3ee794..02f40d0fbac0 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -481,6 +481,18 @@ config TOUCHSCREEN_HYNITRON_CSTXXX
> To compile this driver as a module, choose M here: the
> module will be called hynitron-cstxxx.
>
> +config TOUCHSCREEN_HYNITRON_CST816X
> + tristate "Hynitron CST816X touchscreen support"
> + depends on I2C
> + help
> + Say Y here if you have a touchscreen using a Hynitron
> + CST816X touchscreen controller.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called hynitron-cst816x.
> +
> config TOUCHSCREEN_ILI210X
> tristate "Ilitek ILI210X based touchscreen"
> depends on I2C
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index a81cb5aa21a5..a92a87417a97 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
> obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
> obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_SPI) += goodix_berlin_spi.o
> obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
> +obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CST816X) += hynitron-cst816x.o
> obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
> obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
> obj-$(CONFIG_TOUCHSCREEN_ILITEK) += ilitek_ts_i2c.o
> diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
> new file mode 100644
> index 000000000000..86715c3d1872
> --- /dev/null
> +++ b/drivers/input/touchscreen/hynitron-cst816x.c
> @@ -0,0 +1,306 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Driver for I2C connected Hynitron CST816X Touchscreen
> + *
> + * Copyright (C) 2024 Oleh Kuzhylnyi <kuzhylol@gmail.com>
> + */
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_irq.h>
> +#include <linux/timer.h>
> +
> +#define CST816X_MAX_X 240
> +#define CST816X_MAX_Y CST816X_MAX_X
> +
> +#define CST816X_EVENT_TIMEOUT_MS 10
> +
> +enum cst816x_registers {
> + CST816X_FRAME = 0x01,
> + CST816X_MOTION = 0xEC,
> +};
> +
> +enum cst816_gesture_code {
> + CST816X_SWIPE = 0x00,
> + CST816X_SWIPE_UP = 0x01,
> + CST816X_SWIPE_DOWN = 0x02,
> + CST816X_SWIPE_LEFT = 0x03,
> + CST816X_SWIPE_RIGHT = 0x04,
> + CST816X_SINGLE_TAP = 0x05,
> + CST816X_DOUBLE_TAP = 0x0B,
> + CST816X_LONG_PRESS = 0x0C,
> +};
> +
> +struct cst816x_info {
> + u8 gesture;
> + u8 x;
> + u8 y;
> +};
> +
> +struct cst816x_priv {
> + struct device *dev;
> + struct i2c_client *client;
> + struct gpio_desc *reset;
> + struct input_dev *input;
> + struct timer_list timer;
> + struct delayed_work dw;
> + struct cst816x_info info;
> +
> + u8 rxtx[8];
> +};
> +
> +struct cst816x_gesture_mapping {
> + enum cst816_gesture_code gesture_code;
> + size_t event_code;
Why size_t?
> +};
> +
> +static const struct cst816x_gesture_mapping cst816x_gesture_map[] = {
> + {CST816X_SWIPE, KEY_UNKNOWN},
> + {CST816X_SWIPE_UP, KEY_UP},
> + {CST816X_SWIPE_DOWN, KEY_DOWN},
> + {CST816X_SWIPE_LEFT, KEY_LEFT},
> + {CST816X_SWIPE_RIGHT, KEY_RIGHT},
> + {CST816X_SINGLE_TAP, BTN_TOUCH},
> + {CST816X_DOUBLE_TAP, BTN_TOOL_DOUBLETAP},
> + {CST816X_LONG_PRESS, BTN_TOOL_TRIPLETAP}
> +};
> +
> +static int cst816x_i2c_write_reg(struct cst816x_priv *priv, u8 reg, u8 cmd)
> +{
> + struct i2c_client *client;
> + struct i2c_msg xfer;
> + int rc;
> +
> + client = priv->client;
> +
> + priv->rxtx[0] = reg;
> + priv->rxtx[1] = cmd;
> +
> + xfer.addr = client->addr;
> + xfer.flags = 0;
> + xfer.len = 2;
> + xfer.buf = priv->rxtx;
> +
> + rc = i2c_transfer(client->adapter, &xfer, 1);
> + if (rc != 1) {
> + if (rc >= 0)
> + rc = -EIO;
> + } else {
> + rc = 0;
> + }
> +
> + if (rc < 0)
> + dev_err(&client->dev, "i2c tx err: %d\n", rc);
> +
> + return rc;
> +}
> +
> +static int cst816x_i2c_read_reg(struct cst816x_priv *priv, u8 reg)
> +{
> + struct i2c_client *client;
> + struct i2c_msg xfer[2];
> + int rc;
> +
> + client = priv->client;
> +
> + xfer[0].addr = client->addr;
> + xfer[0].flags = 0;
> + xfer[0].len = sizeof(reg);
> + xfer[0].buf = ®
> +
> + xfer[1].addr = client->addr;
> + xfer[1].flags = I2C_M_RD;
> + xfer[1].len = sizeof(priv->rxtx);
> + xfer[1].buf = priv->rxtx;
> +
> + rc = i2c_transfer(client->adapter, xfer, ARRAY_SIZE(xfer));
> + if (rc != ARRAY_SIZE(xfer)) {
> + if (rc >= 0)
> + rc = -EIO;
> + } else {
> + rc = 0;
> + }
> +
> + if (rc < 0)
> + dev_err(&client->dev, "i2c rx err: %d\n", rc);
> +
> + return rc;
> +}
> +
> +static int cst816x_setup_regs(struct cst816x_priv *priv)
> +{
> + return cst816x_i2c_write_reg(priv, CST816X_MOTION, CST816X_DOUBLE_TAP);
> +}
> +
> +static void report_gesture_event(const struct cst816x_priv *priv,
> + enum cst816_gesture_code gesture_code,
> + bool state)
> +{
> + const struct cst816x_gesture_mapping *lookup = NULL;
> +
> + for (u8 i = CST816X_SWIPE_UP; i < ARRAY_SIZE(cst816x_gesture_map); i++) {
> + if (cst816x_gesture_map[i].gesture_code == gesture_code) {
> + lookup = &cst816x_gesture_map[i];
Why don't you do
input_report_key(priv->input,
cst816x_gesture_map[i].event_code,
state);
right here?i No need for this "lookup" variable.
> + break;
> + }
> + }
> +
> + if (lookup)
> + input_report_key(priv->input, lookup->event_code, state);
> +}
> +
> +static int cst816x_process_touch(struct cst816x_priv *priv)
> +{
> + u8 *raw;
> + int rc;
> +
> + rc = cst816x_i2c_read_reg(priv, CST816X_FRAME);
> + if (!rc) {
> + raw = priv->rxtx;
> +
> + priv->info.gesture = raw[0];
> + priv->info.x = ((raw[2] & 0x0F) << 8) | raw[3];
> + priv->info.y = ((raw[4] & 0x0F) << 8) | raw[5];
> +
> + dev_dbg(priv->dev, "x: %d, y: %d, gesture: 0x%x\n",
> + priv->info.x, priv->info.y, priv->info.gesture);
> + }
> +
> + return rc;
> +}
> +
> +static int cst816x_register_input(struct cst816x_priv *priv)
> +{
> + priv->input = devm_input_allocate_device(priv->dev);
> + if (!priv->input)
> + return -ENOMEM;
> +
> + priv->input->name = "Hynitron CST816X Touchscreen";
> + priv->input->phys = "input/ts";
> + priv->input->id.bustype = BUS_I2C;
> + input_set_drvdata(priv->input, priv);
> +
> + for (u8 i = CST816X_SWIPE_UP; i < ARRAY_SIZE(cst816x_gesture_map); i++) {
> + input_set_capability(priv->input, EV_KEY,
> + cst816x_gesture_map[i].event_code);
> + }
> +
> + input_set_abs_params(priv->input, ABS_X, 0, CST816X_MAX_X, 0, 0);
> + input_set_abs_params(priv->input, ABS_Y, 0, CST816X_MAX_Y, 0, 0);
> + input_set_capability(priv->input, EV_ABS, ABS_X);
> + input_set_capability(priv->input, EV_ABS, ABS_Y);
No need for using input_set_capability() in conjunction with
input_set_abs_params().
> +
> + return input_register_device(priv->input);
> +}
> +
> +static void cst816x_reset(struct cst816x_priv *priv)
> +{
> + gpiod_set_value_cansleep(priv->reset, 0);
> + msleep(100);
> + gpiod_set_value_cansleep(priv->reset, 1);
> + msleep(100);
This code says that you put reset line into inactive state, wait for
100 msec, and then activate the reset and leave it active (i.e. the
device is inoperable) for the rest of the time.
The reason it is working for you is that you describe the line as
"active high" effectively inverting what the code logically does.
Please fix both the code here and the binding example (and your actual
device tree that you use).
> +}
> +
> +static void cst816x_timer_cb(struct timer_list *timer)
> +{
> + struct cst816x_priv *priv = from_timer(priv, timer, timer);
> +
> + report_gesture_event(priv, priv->info.gesture, false);
> + input_sync(priv->input);
> +}
> +
> +static void cst816x_dw_cb(struct work_struct *work)
> +{
> + struct cst816x_priv *priv =
> + container_of(work, struct cst816x_priv, dw.work);
> +
> + if (!cst816x_process_touch(priv)) {
> + input_report_abs(priv->input, ABS_X, priv->info.x);
> + input_report_abs(priv->input, ABS_Y, priv->info.y);
> + report_gesture_event(priv, priv->info.gesture, true);
> + input_sync(priv->input);
> +
> + mod_timer(&priv->timer,
> + jiffies + msecs_to_jiffies(CST816X_EVENT_TIMEOUT_MS));
> + }
> +}
> +
> +static irqreturn_t cst816x_irq_cb(int irq, void *cookie)
> +{
> + struct cst816x_priv *priv = (struct cst816x_priv *)cookie;
> +
> + schedule_delayed_work(&priv->dw, 0);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int cst816x_probe(struct i2c_client *client)
> +{
> + struct cst816x_priv *priv;
> + struct device *dev = &client->dev;
> + int rc;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = dev;
> + priv->client = client;
> +
> + INIT_DELAYED_WORK(&priv->dw, cst816x_dw_cb);
> + timer_setup(&priv->timer, cst816x_timer_cb, 0);
> +
> + priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(priv->reset))
> + return dev_err_probe(dev, PTR_ERR(priv->reset),
> + "reset gpio not found\n");
> +
> + if (priv->reset)
> + cst816x_reset(priv);
> +
> + rc = cst816x_setup_regs(priv);
> + if (rc)
> + return dev_err_probe(dev, rc, "regs setup failed\n");
> +
> + client->irq = of_irq_get(dev->of_node, 0);
> + if (client->irq <= 0)
> + return dev_err_probe(dev, client->irq, "irq lookup failed\n");
No, leave this to the I2C core to do. Just use client->irq that was set
up for you.
> +
> + rc = devm_request_threaded_irq(dev, client->irq, NULL, cst816x_irq_cb,
> + IRQF_ONESHOT, dev->driver->name, priv);
You have an "oneshot" threaded interrupt that from its handler schedules
work (which then uses timer to reschedule itself). This shows
fundamental misunderstanding of what a threaded interrupt is. They were
specifically introduced so that interrupt handler could interact with
"slow" devices like I2C controllers. You should be able to drop
the delayed work and the timer.
Does the device signal when finger leaves the surface?
> + if (rc)
> + return dev_err_probe(dev, client->irq, "irq request failed\n");
> +
> + return cst816x_register_input(priv);
u enable interrupt and only then allocate input device. Depending on
scheduling quirks this may blow up. Please allocate the input device
before registering interrupt handler, the rest of the input device
registration is OK to happen afterwards.
> +}
> +
> +static const struct i2c_device_id cst816x_id[] = {
> + { .name = "cst816s", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, cst816x_id);
> +
> +static const struct of_device_id cst816x_of_match[] = {
> + { .compatible = "hynitron,cst816s", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, cst816x_of_match);
> +
> +static struct i2c_driver cst816x_driver = {
> + .driver = {
> + .name = "cst816x",
> + .of_match_table = cst816x_of_match,
> + },
> + .id_table = cst816x_id,
> + .probe = cst816x_probe,
> +};
> +
> +module_i2c_driver(cst816x_driver);
> +
> +MODULE_AUTHOR("Oleh Kuzhylnyi <kuzhylol@gmail.com>");
> +MODULE_DESCRIPTION("Hynitron CST816X Touchscreen Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.30.2
>
--
Dmitry
^ permalink raw reply
* [PATCH v2 2/2] input: add driver for Hynitron CST816X touchscreen
From: Oleh Kuzhylnyi @ 2024-05-22 20:33 UTC (permalink / raw)
To: linux-input
Cc: dmitry.torokhov, jeff, neil.armstrong, schnelle, arnd, kuzhylol,
hdegoede, linux-kernel, robh, krzysztof.kozlowski+dt, conor+dt,
devicetree, artur.serhiienko, igor.opaniuk
In-Reply-To: <20240522203347.2263425-1-kuzhylol@gmail.com>
Introduce support for the Hynitron CST816X touchscreen controller
used for 240×240 1.28-inch Round LCD Display Module manufactured
by Waveshare Electronics. The driver is designed based on an Arduino
implementation marked as under MIT License. This driver is written
for a particular round display based on the CST816S controller, which
is not compatiable with existing driver for Hynitron controllers.
Signed-off-by: Oleh Kuzhylnyi <kuzhylol@gmail.com>
---
Changes in v2:
- Apply dev_err_probe() for better error handling
- Remove redundant printing, remove dev_warn() message spamming
- Get rid of PM since the touchscreen goes into sleep mode automatically
- Get rid of IRQ control and IRQF_NO_AUTOEN flag
- Reduce timer timeout up to 10ms to handle touch events faster
- Skip registering of non-gesture CST816X_SWIPE event
- Shift input_register_device() as a final call in probe() callback
- Specify name of i2c_device_id explicitly
- Update module description and fix typo
- Add necessary spaces between lines
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/hynitron-cst816x.c | 306 +++++++++++++++++++
3 files changed, 319 insertions(+)
create mode 100644 drivers/input/touchscreen/hynitron-cst816x.c
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index c821fe3ee794..02f40d0fbac0 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -481,6 +481,18 @@ config TOUCHSCREEN_HYNITRON_CSTXXX
To compile this driver as a module, choose M here: the
module will be called hynitron-cstxxx.
+config TOUCHSCREEN_HYNITRON_CST816X
+ tristate "Hynitron CST816X touchscreen support"
+ depends on I2C
+ help
+ Say Y here if you have a touchscreen using a Hynitron
+ CST816X touchscreen controller.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called hynitron-cst816x.
+
config TOUCHSCREEN_ILI210X
tristate "Ilitek ILI210X based touchscreen"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index a81cb5aa21a5..a92a87417a97 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_SPI) += goodix_berlin_spi.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
+obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CST816X) += hynitron-cst816x.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
obj-$(CONFIG_TOUCHSCREEN_ILITEK) += ilitek_ts_i2c.o
diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
new file mode 100644
index 000000000000..86715c3d1872
--- /dev/null
+++ b/drivers/input/touchscreen/hynitron-cst816x.c
@@ -0,0 +1,306 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for I2C connected Hynitron CST816X Touchscreen
+ *
+ * Copyright (C) 2024 Oleh Kuzhylnyi <kuzhylol@gmail.com>
+ */
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+#include <linux/timer.h>
+
+#define CST816X_MAX_X 240
+#define CST816X_MAX_Y CST816X_MAX_X
+
+#define CST816X_EVENT_TIMEOUT_MS 10
+
+enum cst816x_registers {
+ CST816X_FRAME = 0x01,
+ CST816X_MOTION = 0xEC,
+};
+
+enum cst816_gesture_code {
+ CST816X_SWIPE = 0x00,
+ CST816X_SWIPE_UP = 0x01,
+ CST816X_SWIPE_DOWN = 0x02,
+ CST816X_SWIPE_LEFT = 0x03,
+ CST816X_SWIPE_RIGHT = 0x04,
+ CST816X_SINGLE_TAP = 0x05,
+ CST816X_DOUBLE_TAP = 0x0B,
+ CST816X_LONG_PRESS = 0x0C,
+};
+
+struct cst816x_info {
+ u8 gesture;
+ u8 x;
+ u8 y;
+};
+
+struct cst816x_priv {
+ struct device *dev;
+ struct i2c_client *client;
+ struct gpio_desc *reset;
+ struct input_dev *input;
+ struct timer_list timer;
+ struct delayed_work dw;
+ struct cst816x_info info;
+
+ u8 rxtx[8];
+};
+
+struct cst816x_gesture_mapping {
+ enum cst816_gesture_code gesture_code;
+ size_t event_code;
+};
+
+static const struct cst816x_gesture_mapping cst816x_gesture_map[] = {
+ {CST816X_SWIPE, KEY_UNKNOWN},
+ {CST816X_SWIPE_UP, KEY_UP},
+ {CST816X_SWIPE_DOWN, KEY_DOWN},
+ {CST816X_SWIPE_LEFT, KEY_LEFT},
+ {CST816X_SWIPE_RIGHT, KEY_RIGHT},
+ {CST816X_SINGLE_TAP, BTN_TOUCH},
+ {CST816X_DOUBLE_TAP, BTN_TOOL_DOUBLETAP},
+ {CST816X_LONG_PRESS, BTN_TOOL_TRIPLETAP}
+};
+
+static int cst816x_i2c_write_reg(struct cst816x_priv *priv, u8 reg, u8 cmd)
+{
+ struct i2c_client *client;
+ struct i2c_msg xfer;
+ int rc;
+
+ client = priv->client;
+
+ priv->rxtx[0] = reg;
+ priv->rxtx[1] = cmd;
+
+ xfer.addr = client->addr;
+ xfer.flags = 0;
+ xfer.len = 2;
+ xfer.buf = priv->rxtx;
+
+ rc = i2c_transfer(client->adapter, &xfer, 1);
+ if (rc != 1) {
+ if (rc >= 0)
+ rc = -EIO;
+ } else {
+ rc = 0;
+ }
+
+ if (rc < 0)
+ dev_err(&client->dev, "i2c tx err: %d\n", rc);
+
+ return rc;
+}
+
+static int cst816x_i2c_read_reg(struct cst816x_priv *priv, u8 reg)
+{
+ struct i2c_client *client;
+ struct i2c_msg xfer[2];
+ int rc;
+
+ client = priv->client;
+
+ xfer[0].addr = client->addr;
+ xfer[0].flags = 0;
+ xfer[0].len = sizeof(reg);
+ xfer[0].buf = ®
+
+ xfer[1].addr = client->addr;
+ xfer[1].flags = I2C_M_RD;
+ xfer[1].len = sizeof(priv->rxtx);
+ xfer[1].buf = priv->rxtx;
+
+ rc = i2c_transfer(client->adapter, xfer, ARRAY_SIZE(xfer));
+ if (rc != ARRAY_SIZE(xfer)) {
+ if (rc >= 0)
+ rc = -EIO;
+ } else {
+ rc = 0;
+ }
+
+ if (rc < 0)
+ dev_err(&client->dev, "i2c rx err: %d\n", rc);
+
+ return rc;
+}
+
+static int cst816x_setup_regs(struct cst816x_priv *priv)
+{
+ return cst816x_i2c_write_reg(priv, CST816X_MOTION, CST816X_DOUBLE_TAP);
+}
+
+static void report_gesture_event(const struct cst816x_priv *priv,
+ enum cst816_gesture_code gesture_code,
+ bool state)
+{
+ const struct cst816x_gesture_mapping *lookup = NULL;
+
+ for (u8 i = CST816X_SWIPE_UP; i < ARRAY_SIZE(cst816x_gesture_map); i++) {
+ if (cst816x_gesture_map[i].gesture_code == gesture_code) {
+ lookup = &cst816x_gesture_map[i];
+ break;
+ }
+ }
+
+ if (lookup)
+ input_report_key(priv->input, lookup->event_code, state);
+}
+
+static int cst816x_process_touch(struct cst816x_priv *priv)
+{
+ u8 *raw;
+ int rc;
+
+ rc = cst816x_i2c_read_reg(priv, CST816X_FRAME);
+ if (!rc) {
+ raw = priv->rxtx;
+
+ priv->info.gesture = raw[0];
+ priv->info.x = ((raw[2] & 0x0F) << 8) | raw[3];
+ priv->info.y = ((raw[4] & 0x0F) << 8) | raw[5];
+
+ dev_dbg(priv->dev, "x: %d, y: %d, gesture: 0x%x\n",
+ priv->info.x, priv->info.y, priv->info.gesture);
+ }
+
+ return rc;
+}
+
+static int cst816x_register_input(struct cst816x_priv *priv)
+{
+ priv->input = devm_input_allocate_device(priv->dev);
+ if (!priv->input)
+ return -ENOMEM;
+
+ priv->input->name = "Hynitron CST816X Touchscreen";
+ priv->input->phys = "input/ts";
+ priv->input->id.bustype = BUS_I2C;
+ input_set_drvdata(priv->input, priv);
+
+ for (u8 i = CST816X_SWIPE_UP; i < ARRAY_SIZE(cst816x_gesture_map); i++) {
+ input_set_capability(priv->input, EV_KEY,
+ cst816x_gesture_map[i].event_code);
+ }
+
+ input_set_abs_params(priv->input, ABS_X, 0, CST816X_MAX_X, 0, 0);
+ input_set_abs_params(priv->input, ABS_Y, 0, CST816X_MAX_Y, 0, 0);
+ input_set_capability(priv->input, EV_ABS, ABS_X);
+ input_set_capability(priv->input, EV_ABS, ABS_Y);
+
+ return input_register_device(priv->input);
+}
+
+static void cst816x_reset(struct cst816x_priv *priv)
+{
+ gpiod_set_value_cansleep(priv->reset, 0);
+ msleep(100);
+ gpiod_set_value_cansleep(priv->reset, 1);
+ msleep(100);
+}
+
+static void cst816x_timer_cb(struct timer_list *timer)
+{
+ struct cst816x_priv *priv = from_timer(priv, timer, timer);
+
+ report_gesture_event(priv, priv->info.gesture, false);
+ input_sync(priv->input);
+}
+
+static void cst816x_dw_cb(struct work_struct *work)
+{
+ struct cst816x_priv *priv =
+ container_of(work, struct cst816x_priv, dw.work);
+
+ if (!cst816x_process_touch(priv)) {
+ input_report_abs(priv->input, ABS_X, priv->info.x);
+ input_report_abs(priv->input, ABS_Y, priv->info.y);
+ report_gesture_event(priv, priv->info.gesture, true);
+ input_sync(priv->input);
+
+ mod_timer(&priv->timer,
+ jiffies + msecs_to_jiffies(CST816X_EVENT_TIMEOUT_MS));
+ }
+}
+
+static irqreturn_t cst816x_irq_cb(int irq, void *cookie)
+{
+ struct cst816x_priv *priv = (struct cst816x_priv *)cookie;
+
+ schedule_delayed_work(&priv->dw, 0);
+
+ return IRQ_HANDLED;
+}
+
+static int cst816x_probe(struct i2c_client *client)
+{
+ struct cst816x_priv *priv;
+ struct device *dev = &client->dev;
+ int rc;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = dev;
+ priv->client = client;
+
+ INIT_DELAYED_WORK(&priv->dw, cst816x_dw_cb);
+ timer_setup(&priv->timer, cst816x_timer_cb, 0);
+
+ priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->reset))
+ return dev_err_probe(dev, PTR_ERR(priv->reset),
+ "reset gpio not found\n");
+
+ if (priv->reset)
+ cst816x_reset(priv);
+
+ rc = cst816x_setup_regs(priv);
+ if (rc)
+ return dev_err_probe(dev, rc, "regs setup failed\n");
+
+ client->irq = of_irq_get(dev->of_node, 0);
+ if (client->irq <= 0)
+ return dev_err_probe(dev, client->irq, "irq lookup failed\n");
+
+ rc = devm_request_threaded_irq(dev, client->irq, NULL, cst816x_irq_cb,
+ IRQF_ONESHOT, dev->driver->name, priv);
+ if (rc)
+ return dev_err_probe(dev, client->irq, "irq request failed\n");
+
+ return cst816x_register_input(priv);
+}
+
+static const struct i2c_device_id cst816x_id[] = {
+ { .name = "cst816s", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, cst816x_id);
+
+static const struct of_device_id cst816x_of_match[] = {
+ { .compatible = "hynitron,cst816s", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, cst816x_of_match);
+
+static struct i2c_driver cst816x_driver = {
+ .driver = {
+ .name = "cst816x",
+ .of_match_table = cst816x_of_match,
+ },
+ .id_table = cst816x_id,
+ .probe = cst816x_probe,
+};
+
+module_i2c_driver(cst816x_driver);
+
+MODULE_AUTHOR("Oleh Kuzhylnyi <kuzhylol@gmail.com>");
+MODULE_DESCRIPTION("Hynitron CST816X Touchscreen Driver");
+MODULE_LICENSE("GPL");
--
2.30.2
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: input: touchscreen: add Hynitron CST816X
From: Oleh Kuzhylnyi @ 2024-05-22 20:33 UTC (permalink / raw)
To: linux-input
Cc: dmitry.torokhov, jeff, neil.armstrong, schnelle, arnd, kuzhylol,
hdegoede, linux-kernel, robh, krzysztof.kozlowski+dt, conor+dt,
devicetree, artur.serhiienko, igor.opaniuk
Add documentation for the Hynitron CST816X touchscreen bindings.
Signed-off-by: Oleh Kuzhylnyi <kuzhylol@gmail.com>
---
Changes in v2:
- Apply pin definitions and DT headers
- Use generic name for DT node
- Drop status field
.../input/touchscreen/hynitron,cst816x.yaml | 56 +++++++++++++++++++
1 file changed, 56 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
diff --git a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
new file mode 100644
index 000000000000..22bd145db5ee
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/hynitron,cst816x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hynitron CST816X series touchscreen controller
+
+description: |
+ Bindings for Hynitron CST816X series touchscreen controllers.
+
+maintainers:
+ - Oleh Kuzhylnyi <kuzhylol@gmail.com>
+
+allOf:
+ - $ref: touchscreen.yaml#
+
+properties:
+ compatible:
+ enum:
+ - hynitron,cst816s
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - reset-gpios
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ touchscreen@15 {
+ compatible = "hynitron,cst816s";
+ reg = <0x15>;
+ interrupt-parent = <&gpio>;
+ interrupts = <4 IRQ_TYPE_EDGE_RISING>;
+ reset-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+...
--
2.30.2
^ permalink raw reply related
* Re: [PATCH] Input: ili210x - fix ili251x_read_touch_data() return value
From: Markus Elfring @ 2024-05-22 20:25 UTC (permalink / raw)
To: John Keeping, linux-input, kernel-janitors; +Cc: LKML, Dmitry Torokhov
In-Reply-To: <20240522100341.1650842-1-jkeeping@inmusicbrands.com>
…
> This fixes touch reporting when there are more than 6 active touches.
Does such information indicate a need for the tag “Fixes”?
…
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -255,14 +255,15 @@ static int ili251x_read_reg(struct i2c_client *client,
> static int ili251x_read_touch_data(struct i2c_client *client, u8 *data)
> {
> int error;
> + int ret;
>
> error = ili251x_read_reg_common(client, REG_TOUCHDATA,
> data, ILI251X_DATA_SIZE1, 0);
> if (!error && data[0] == 2) {
…
I suggest to define the variable “ret” in the shown if branch.
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v1 2/2] Input: Add driver for PixArt PS/2 touchpad
From: Dmitry Torokhov @ 2024-05-22 19:58 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Huacai Chen, loongson-kernel,
linux-input, Xiaotian Wu, Jon Xie, Jay Lee
In-Reply-To: <c200b74b5db63ae544be2bc037e6afa12137aa98.1715224143.git.zhoubinbin@loongson.cn>
Hi Binbin,
On Wed, May 15, 2024 at 04:58:57PM +0800, Binbin Zhou wrote:
> This patch introduces a driver for the PixArt PS/2 touchpad, which
> supports both clickpad and touchpad types.
>
> Co-developed-by: Jon Xie <jon_xie@pixart.com>
> Signed-off-by: Jon Xie <jon_xie@pixart.com>
> Co-developed-by: Jay Lee <jay_lee@pixart.com>
> Signed-off-by: Jay Lee <jay_lee@pixart.com>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
> drivers/input/mouse/Kconfig | 12 ++
> drivers/input/mouse/Makefile | 1 +
> drivers/input/mouse/pixart_ps2.c | 294 +++++++++++++++++++++++++++++
> drivers/input/mouse/pixart_ps2.h | 31 +++
> drivers/input/mouse/psmouse-base.c | 17 ++
> drivers/input/mouse/psmouse.h | 1 +
> 6 files changed, 356 insertions(+)
> create mode 100644 drivers/input/mouse/pixart_ps2.c
> create mode 100644 drivers/input/mouse/pixart_ps2.h
>
> diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> index 833b643f0616..8a27a20d04b0 100644
> --- a/drivers/input/mouse/Kconfig
> +++ b/drivers/input/mouse/Kconfig
> @@ -69,6 +69,18 @@ config MOUSE_PS2_LOGIPS2PP
>
> If unsure, say Y.
>
> +config MOUSE_PS2_PIXART
> + bool "PixArt PS/2 touchpad protocol extension" if EXPERT
> + default y
> + depends on MOUSE_PS2
> + help
> + This driver supports the PixArt PS/2 touchpad found in some
> + laptops.
> + Say Y here if you have a PixArt PS/2 TouchPad connected to
> + your system.
> +
> + If unsure, say Y.
> +
> config MOUSE_PS2_SYNAPTICS
> bool "Synaptics PS/2 mouse protocol extension" if EXPERT
> default y
> diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
> index a1336d5bee6f..563029551529 100644
> --- a/drivers/input/mouse/Makefile
> +++ b/drivers/input/mouse/Makefile
> @@ -32,6 +32,7 @@ psmouse-$(CONFIG_MOUSE_PS2_ELANTECH) += elantech.o
> psmouse-$(CONFIG_MOUSE_PS2_OLPC) += hgpk.o
> psmouse-$(CONFIG_MOUSE_PS2_LOGIPS2PP) += logips2pp.o
> psmouse-$(CONFIG_MOUSE_PS2_LIFEBOOK) += lifebook.o
> +psmouse-$(CONFIG_MOUSE_PS2_PIXART) += pixart_ps2.o
> psmouse-$(CONFIG_MOUSE_PS2_SENTELIC) += sentelic.o
> psmouse-$(CONFIG_MOUSE_PS2_TRACKPOINT) += trackpoint.o
> psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o
> diff --git a/drivers/input/mouse/pixart_ps2.c b/drivers/input/mouse/pixart_ps2.c
> new file mode 100644
> index 000000000000..532d84ab7654
> --- /dev/null
> +++ b/drivers/input/mouse/pixart_ps2.c
> @@ -0,0 +1,294 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Pixart Touchpad Controller 1336U PS2 driver
> + *
> + * Author: Jon Xie <jon_xie@pixart.com>
> + * Jay Lee <jay_lee@pixart.com>
> + * Further cleanup and restructuring by:
> + * Binbin Zhou <zhoubinbin@loongson.cn>
> + *
> + * Copyright (C) 2021-2024 Pixart Imaging
> + * Copyright (C) 2024 Loongson Technology Corporation Limited.
> + *
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/libps2.h>
> +#include <linux/input/mt.h>
> +#include <linux/serio.h>
> +#include <linux/slab.h>
> +#include "pixart_ps2.h"
> +
> +static int pixart_mode_detect(struct psmouse *psmouse)
> +{
> + u8 param[1];
> +
> + if (ps2_command(&psmouse->ps2dev, param, PIXART_CMD_REPORT_FORMAT))
> + return -EIO;
> +
> + return (param[0] == 1) ? PIXART_MODE_ABS : PIXART_MODE_REL;
> +}
> +
> +static int pixart_type_detect(struct psmouse *psmouse)
> +{
> + struct ps2dev *ps2dev = &psmouse->ps2dev;
> + u8 param[3];
You are not initializing this.
> +
> + param[0] = 0x0a;
> + ps2_command(ps2dev, param, PSMOUSE_CMD_SETRATE);
> + param[0] = 0x0;
> + ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES);
> + param[0] = 0x0;
> + ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES);
> + param[0] = 0x0;
> + ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES);
> + param[0] = 0x03;
> + ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES);
> + ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO);
> +
So here you might be comparing with random garbage.
I think it would be better if "param" was initialized and at least the
return value of the last ps2_command() checked to make sure the device
ACKed it and returned proper amount of data.
> + return (param[0] == 0x0e) ? PIXART_TYPE_TOUCHPAD : PIXART_TYPE_CLICKPAD;
> +}
> +
> +static int pixart_intellimouse_detect(struct psmouse *psmouse)
> +{
> + struct ps2dev *ps2dev = &psmouse->ps2dev;
> + u8 param[2];
> +
> + param[0] = 200;
> + ps2_command(ps2dev, param, PSMOUSE_CMD_SETRATE);
> + param[0] = 100;
> + ps2_command(ps2dev, param, PSMOUSE_CMD_SETRATE);
> + param[0] = 80;
> + ps2_command(ps2dev, param, PSMOUSE_CMD_SETRATE);
> + ps2_command(ps2dev, param, PSMOUSE_CMD_GETID);
> +
> + return (param[0] == 3) ? 0 : -1;
> +}
> +
> +static void pixart_reset(struct psmouse *psmouse)
> +{
> + ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
> + msleep(100);
> + psmouse_reset(psmouse);
> +}
> +
> +static psmouse_ret_t pixart_process_abs_packet(struct psmouse *psmouse)
> +{
> + struct pixart_data *priv = psmouse->private;
> + struct input_dev *dev = psmouse->dev;
> + u8 *packet = psmouse->packet;
> + u8 contact_cnt = (packet[0] >> 4) & 0x07;
> + unsigned int abs_x, abs_y;
> + int i, id, fingers = 0;
> + bool tip;
> +
> + if (contact_cnt > MAX_FINGERS || ((packet[0] & 0x8c) != 0x80))
> + return PSMOUSE_BAD_DATA;
> +
> + if (contact_cnt == MAX_FINGERS && psmouse->pktcnt < psmouse->pktsize)
> + return PSMOUSE_GOOD_DATA;
> + else if (contact_cnt == 0 && psmouse->pktcnt < 5)
> + return PSMOUSE_GOOD_DATA;
> + else if (psmouse->pktcnt < (3 * contact_cnt + 2))
> + return PSMOUSE_GOOD_DATA;
> +
> + for (i = 0; i < contact_cnt; i++) {
> + id = packet[3 * i + 3] & 0x07;
> + abs_y = ((packet[3 * i + 3] & 0x30) << 4) | packet[3 * i + 1];
> + abs_x = ((packet[3 * i + 3] & 0xc0) << 2) | packet[3 * i + 2];
> +
> + if (i == MAX_FINGERS - 1)
> + tip = packet[14] & (0x01 << 1);
Please use BIT().
> + else
> + tip = packet[3 * contact_cnt + 1] & (0x01 << (2 * i + 1));
> +
> + input_mt_slot(dev, id);
> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, tip);
> + if (tip) {
if (input_mt_report_slot_state(dev, MT_TOOL_FINGER, tip)) {
> + fingers++;
> + input_report_abs(dev, ABS_MT_POSITION_Y, abs_y);
> + input_report_abs(dev, ABS_MT_POSITION_X, abs_x);
> + }
> + }
> +
> + input_mt_drop_unused(dev);
Could you explain the protocol please? Does the device produce "lift
off" packets for contacts? Why do you need to call
input_mt_drop_unused() explicitly?
> + input_mt_report_pointer_emulation(dev, false);
> + input_mt_report_finger_count(dev, fingers);
> +
> + if (priv->type == PIXART_TYPE_CLICKPAD) {
> + input_report_key(dev, BTN_LEFT, packet[0] & 0x03);
> + } else {
> + input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> + }
> +
> + input_mt_report_pointer_emulation(dev, true);
Why the 2 calls to input_mt_report_pointer_emulation()?
> + input_sync(dev);
> +
> + return PSMOUSE_FULL_PACKET;
> +}
> +
> +static psmouse_ret_t pixart_process_rel_packet(struct psmouse *psmouse)
> +{
> + struct input_dev *dev = psmouse->dev;
> + u8 *packet = psmouse->packet;
> + int x = packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0;
> + int y = packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0;
> + int w = -(s8)packet[3];
> +
> + if (psmouse->pktcnt < psmouse->pktsize)
> + return PSMOUSE_GOOD_DATA;
> +
> + psmouse_report_standard_buttons(dev, packet[0]);
> + input_report_rel(dev, REL_X, x);
> + input_report_rel(dev, REL_Y, y);
> + input_report_rel(dev, REL_WHEEL, w);
> + input_sync(dev);
> +
> + return PSMOUSE_FULL_PACKET;
> +}
> +
> +static void pixart_disconnect(struct psmouse *psmouse)
> +{
> + psmouse_info(psmouse, "Device disconnect");
Please remove, no need to be noisy.
> + pixart_reset(psmouse);
> + kfree(psmouse->private);
> + psmouse->private = NULL;
> +}
> +
> +static int pixart_reconnect(struct psmouse *psmouse)
> +{
> + struct pixart_data *priv = psmouse->private;
> +
> + psmouse_info(psmouse, "Device reconnect");
> + pixart_reset(psmouse);
> +
> + priv->mode = pixart_mode_detect(psmouse);
> + if (priv->mode < 0) {
> + psmouse_err(psmouse, "Unable to detect the PixArt device");
> + return -EIO;
> + }
> +
> + if (priv->mode == PIXART_MODE_ABS)
> + ps2_command(&psmouse->ps2dev, NULL, PIXART_CMD_SWITCH_PROTO);
> +
> + return 0;
> +}
> +
> +static void pixart_set_abs_input_params(struct input_dev *dev, struct pixart_data *priv)
> +{
> + __clear_bit(EV_REL, dev->evbit);
> + __clear_bit(REL_X, dev->relbit);
> + __clear_bit(REL_Y, dev->relbit);
> + __clear_bit(BTN_MIDDLE, dev->keybit);
> +
> + __set_bit(EV_KEY, dev->evbit);
> + __set_bit(BTN_LEFT, dev->keybit);
> + __set_bit(BTN_RIGHT, dev->keybit);
> + if (priv->type == PIXART_TYPE_CLICKPAD) {
> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
> + __clear_bit(BTN_RIGHT, dev->keybit);
> + }
> + __set_bit(BTN_TOUCH, dev->keybit);
> + __set_bit(BTN_TOOL_FINGER, dev->keybit);
> + __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
> + __set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
> + __set_bit(BTN_TOOL_QUADTAP, dev->keybit);
> + __set_bit(INPUT_PROP_POINTER, dev->propbit);
> +
> + __set_bit(EV_ABS, dev->evbit);
> + input_set_abs_params(dev, ABS_X, 0, ABS_MAX_X, 0, 0);
> + input_set_abs_params(dev, ABS_Y, 0, ABS_MAX_Y, 0, 0);
> +
> + input_set_abs_params(dev, ABS_MT_POSITION_X, 0, ABS_MAX_X, 0, 0);
> + input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, ABS_MAX_Y, 0, 0);
> +
> + input_mt_init_slots(dev, SLOTS_NUMBER, 0);
> +}
> +
> +static void pixart_set_rel_input_params(struct input_dev *dev, struct pixart_data *priv)
> +{
> + __set_bit(EV_KEY, dev->evbit);
> + __set_bit(EV_REL, dev->evbit);
> + __set_bit(BTN_LEFT, dev->keybit);
> + __set_bit(BTN_RIGHT, dev->keybit);
> + __set_bit(REL_X, dev->relbit);
> + __set_bit(REL_Y, dev->relbit);
> + __set_bit(REL_WHEEL, dev->relbit);
> + __set_bit(INPUT_PROP_POINTER, dev->propbit);
> +}
> +
> +int pixart_detect(struct psmouse *psmouse, bool set_properties)
> +{
> + int mode, type;
> +
> + pixart_reset(psmouse);
> +
> + type = pixart_type_detect(psmouse);
This returns PIXART_TYPE_TOUCHPAD or PIXART_TYPE_CLICKPAD, and never
negative value.
> + if (type < 0)
> + return -ENODEV;
> +
> + mode = pixart_mode_detect(psmouse);
This relies on non-PixArt devices NAKing PIXART_CMD_REPORT_FORMAT
command. What other touchpad/mice have you tested this on to make sure
it does not mis-detect them?
> + if (mode < 0)
> + return -ENODEV;
> +
> + psmouse_info(psmouse, "Detect PixArt Device.");
No need to be this noisy.
> + if (set_properties) {
> + psmouse->vendor = "PixArt";
> + psmouse->name = (type == PIXART_TYPE_TOUCHPAD) ?
> + "touchpad" : "clickpad";
> + }
> +
> + return 0;
> +}
> +
> +int pixart_init(struct psmouse *psmouse)
> +{
> + struct pixart_data *priv;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + psmouse->private = priv;
> + pixart_reset(psmouse);
> +
> + priv->type = pixart_type_detect(psmouse);
> + if (priv->type < 0)
> + goto fail;
> +
> + priv->mode = pixart_mode_detect(psmouse);
> + if (priv->mode < 0) {
> + psmouse_err(psmouse, "Unable to initialize the PixArt device\n");
> + goto fail;
> + }
> +
> + if (priv->mode == PIXART_MODE_ABS) {
> + ps2_command(&psmouse->ps2dev, NULL, PIXART_CMD_SWITCH_PROTO);
> + pixart_set_abs_input_params(psmouse->dev, priv);
> + psmouse->protocol_handler = pixart_process_abs_packet;
> + psmouse->pktsize = 15;
> + } else {
> + pixart_reset(psmouse);
> + pixart_intellimouse_detect(psmouse);
> + pixart_set_rel_input_params(psmouse->dev, priv);
> + psmouse->protocol_handler = pixart_process_rel_packet;
> + psmouse->pktsize = 4;
Do we really need to handle this case? If the touchpad does not support
absolute packets can we fail the detection and simply rely on the normal
intellimouse detection and packet handling?
> + }
> +
> + psmouse->disconnect = pixart_disconnect;
> + psmouse->reconnect = pixart_reconnect;
> + psmouse->cleanup = pixart_reset;
> + /* resync is not supported yet */
> + psmouse->resync_time = 0;
> +
> + psmouse_info(psmouse, "Set device as Type: %x, mode: %x", priv->type, priv->mode);
> + return 0;
> +
> +fail:
> + pixart_reset(psmouse);
> + kfree(priv);
> + psmouse->private = NULL;
> + return -1;
> +}
> diff --git a/drivers/input/mouse/pixart_ps2.h b/drivers/input/mouse/pixart_ps2.h
> new file mode 100644
> index 000000000000..224ab3a4f739
> --- /dev/null
> +++ b/drivers/input/mouse/pixart_ps2.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _PIXART_PS2_H
> +#define _PIXART_PS2_H
> +
> +#include "psmouse.h"
> +
> +#define ABS_MAX_X 1023
> +#define ABS_MAX_Y 579
> +#define MAX_FINGERS 4
> +#define SLOTS_NUMBER MAX_FINGERS
> +
> +#define PIXART_CMD_REPORT_FORMAT 0x01d8
> +#define PIXART_CMD_SWITCH_PROTO 0x00de
> +
> +#define PIXART_MODE_REL 0
> +#define PIXART_MODE_ABS 1
> +
> +#define PIXART_TYPE_CLICKPAD 0
> +#define PIXART_TYPE_TOUCHPAD 1
> +
> +struct pixart_data {
> + int mode;
> + int type;
> + int x_max;
> + int y_max;
> +};
> +
> +int pixart_detect(struct psmouse *psmouse, bool set_properties);
> +int pixart_init(struct psmouse *psmouse);
> +
> +#endif /* _PIXART_PS2_H */
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index a0aac76b1e41..41af3460077d 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -36,6 +36,7 @@
> #include "focaltech.h"
> #include "vmmouse.h"
> #include "byd.h"
> +#include "pixart_ps2.h"
>
> #define DRIVER_DESC "PS/2 mouse driver"
>
> @@ -905,6 +906,15 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .detect = byd_detect,
> .init = byd_init,
> },
> +#endif
> +#ifdef CONFIG_MOUSE_PS2_PIXART
> + {
> + .type = PSMOUSE_PIXART,
> + .name = "PixArtPS/2",
> + .alias = "pixart",
> + .detect = pixart_detect,
> + .init = pixart_init,
> + },
> #endif
> {
> .type = PSMOUSE_AUTO,
> @@ -1172,6 +1182,13 @@ static int psmouse_extensions(struct psmouse *psmouse,
> return ret;
> }
>
> + /* Try PixArt touchpad */
> + if (max_proto > PSMOUSE_IMEX &&
> + psmouse_try_protocol(psmouse, PSMOUSE_PIXART, &max_proto,
> + set_properties, true)) {
> + return PSMOUSE_PIXART;
> + }
> +
> if (max_proto > PSMOUSE_IMEX) {
> if (psmouse_try_protocol(psmouse, PSMOUSE_GENPS,
> &max_proto, set_properties, true))
> diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
> index b05c6fbae7e3..23f7fa7243cb 100644
> --- a/drivers/input/mouse/psmouse.h
> +++ b/drivers/input/mouse/psmouse.h
> @@ -69,6 +69,7 @@ enum psmouse_type {
> PSMOUSE_BYD,
> PSMOUSE_SYNAPTICS_SMBUS,
> PSMOUSE_ELANTECH_SMBUS,
> + PSMOUSE_PIXART,
> PSMOUSE_AUTO /* This one should always be last */
> };
>
> --
> 2.43.0
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v1 1/2] Input: psmouse - Expanding the psmouse packet array
From: Dmitry Torokhov @ 2024-05-22 19:30 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Huacai Chen, loongson-kernel,
linux-input, Xiaotian Wu
In-Reply-To: <714216496603cd23c18d9d3e1cc52d8ba21e386d.1715224143.git.zhoubinbin@loongson.cn>
Hi Binbin,
On Wed, May 15, 2024 at 04:58:56PM +0800, Binbin Zhou wrote:
> According to the current design of PixArt touchpad hardware and FW, our
> single packet data width needs 11 bytes/15 bytes to receive the complete
> three-finger/four-finger data in absolute coordinate mode.
> Obviously, it exceeds the design of the native driver, and we try to
> extend the length of a single data packet to 16.
Please fold this into the patch adding PixArt protocol support. There is
no need to make this a separate patch.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: ili210x - fix ili251x_read_touch_data() return value
From: Dmitry Torokhov @ 2024-05-22 19:28 UTC (permalink / raw)
To: John Keeping; +Cc: linux-input, linux-kernel
In-Reply-To: <20240522100341.1650842-1-jkeeping@inmusicbrands.com>
On Wed, May 22, 2024 at 11:03:41AM +0100, John Keeping wrote:
> The caller of this function treats all non-zero values as an error, so
> the return value of i2c_master_recv() cannot be returned directly.
> Follow the same pattern as ili211x_read_touch_data() to return zero when
> the correct number of bytes is read and a negative error code otherwise.
>
> This fixes touch reporting when there are more than 6 active touches.
>
> Signed-off-by: John Keeping <jkeeping@inmusicbrands.com>
> ---
> drivers/input/touchscreen/ili210x.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index 31ffdc2a93f35..8846c6d10fc0d 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -255,14 +255,15 @@ static int ili251x_read_reg(struct i2c_client *client,
> static int ili251x_read_touch_data(struct i2c_client *client, u8 *data)
> {
> int error;
> + int ret;
>
> error = ili251x_read_reg_common(client, REG_TOUCHDATA,
> data, ILI251X_DATA_SIZE1, 0);
> if (!error && data[0] == 2) {
> - error = i2c_master_recv(client, data + ILI251X_DATA_SIZE1,
> - ILI251X_DATA_SIZE2);
> - if (error >= 0 && error != ILI251X_DATA_SIZE2)
> - error = -EIO;
Thanks for noticing this. Can we say
if (error >= 0)
error = error != ILI251X_DATA_SIZE2 ? -EIO : 0;
> + ret = i2c_master_recv(client, data + ILI251X_DATA_SIZE1,
> + ILI251X_DATA_SIZE2);
> + if (ret != ILI251X_DATA_SIZE2)
> + error = ret < 0 ? ret : -EIO;
> }
>
> return error;
> --
> 2.45.1
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] Input: edt-ft5x06 - add support for FocalTech FT5452 and FT8719
From: Dmitry Torokhov @ 2024-05-22 17:37 UTC (permalink / raw)
To: joelselvaraj.oss
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel
In-Reply-To: <20240521-add-support-ft5452-and-ft8719-touchscreen-v1-2-2a648ac7176b@gmail.com>
On Tue, May 21, 2024 at 09:02:58AM -0500, Joel Selvaraj via B4 Relay wrote:
> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>
> The driver is compatible with FocalTech FT5452 and FT8719 touchscreens
> too. FT5452 supports up to 5 touch points. FT8719 supports up to 10 touch
> points. Add compatible data for both of them.
>
> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: touchscreen: edt-ft5x06: Document FT5452 and FT8719 support
From: Dmitry Torokhov @ 2024-05-22 17:36 UTC (permalink / raw)
To: joelselvaraj.oss
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel
In-Reply-To: <20240521-add-support-ft5452-and-ft8719-touchscreen-v1-1-2a648ac7176b@gmail.com>
On Tue, May 21, 2024 at 09:02:57AM -0500, Joel Selvaraj via B4 Relay wrote:
> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>
> Document FocalTech FT5452 and FT8719 support by adding their compatibles.
>
> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: BUG: Framework Laptop 16 i2c-hid Based Touchpad Sometimes Fails To Initialize Properly On Early Boot
From: Arazil Songweaver @ 2024-05-22 17:34 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: linux-input, Jiri Kosina, Benjamin Tissoires, LKML
In-Reply-To: <75053193-92db-40ad-9658-3cdeb0f553be@leemhuis.info>
I was able to reproduce this bug on kernel 6.6.0 which should rule out a
recent regression. Kernel 6.5.0 refused to start with a modules error.
Going earlier than that is probably pointless given that we are talking
about hardware first released in January 2024.
This issue is also on the kernel.org Bugzilla at: 218836
-Arazil
On 5/21/24 8:44 AM, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 21.05.24 15:31, Arazil Songweaver wrote:
>> I was able to reproduce bug again with: Linux version
>> 6.9.0-1-git-10323-g8f6a15f095a6 (linux-git@archlinux) (gcc (GCC) 14.1.1
>> 20240507, GNU ld (GNU Binutils) 2.42.0) #1 SMP PREEMPT_DYNAMIC Tue, 21
>> May 2024 11:58:24 +0000 (not an official Arch Linux package)
>>
>> The relevant I2C_HID module needs to be built in to the kernel for the
>> bug to trigger on a consistent basis. Arch Linux and mkinitcpio is
>> currently getting around this issue by building I2C_HID as a module and
>> delaying the load of that module until the part when autoprobe starts
>> loading relevant kernel modules. The bug became (more) visible on Arch
>> Linux after an update to mkinitcpio moved the I2C_HID module up to the
>> beginning of the boot process.
>>
>> This is not a recent regression. In my testing, I was able to reproduce
>> this issue as far back as version 6.8. I did not test 6.7 or earlier
>> revisions yet.
> Okay, then I won't track this as a regression; might still be worth
> trying a few older kernels in a spare minute to see if it was introduced
> in the last 12 or 18 months and can be bisected.
>
> CCed Jiri and Benjamin nevertheless in case they missed this report on
> the lists.
>
> Ciao, Thorsten
>
>> On 5/21/24 5:57 AM, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> On 14.05.24 04:57, Arazil Songweaver wrote:
>>>> We are experiencing an issue where the touch pad on the Framework Laptop
>>>> 16 fails to initialize properly when the "i2c-hid" is loaded early in
>>>> the boot process. This issue is especially prominent when "i2c-hid" is
>>>> built directly into the kernel. When the "i2c-hid" module is built in,
>>>> the issue occurs roughly 50% of the time.
>>>> (https://community.frame.work/t/touchpad-not-working-since-update-archlinux/50304) Moving the module load to later in the boot process appears to resolve this issue by making initialization more likely to succeed. (https://gitlab.archlinux.org/archlinux/mkinitcpio/mkinitcpio/-/releases/v39.1)
>>>>
>>>> Kernel version: Linux artemis 6.9.0-1-git-01560-ga7c840ba5fa7 #1 SMP
>>>> PREEMPT_DYNAMIC Tue, 14 May 2024 01:49:25 +0000 x86_64 GNU/Linux
>>>>
>>>> I'm using the standard Arch Linux AUR "linux-git" package with the
>>>> following kernel configuration changes:
>>>>
>>>> CONFIG_I2C_DEBUG_CORE=y
>>>> CONFIG_I2C_DEBUG_ALGO=y
>>>> CONFIG_I2C_DEBUG_BUS=y
>>>> CONFIG_I2C_HID=y
>>>> CONFIG_I2C_HID_ACPI=y
>>>> CONFIG_I2C_HID_OF=y
>>>> CONFIG_I2C_HID_CORE=y
>>>>
>>>> We tried reverting the following patches without any behavior impact
>>>> (good or bad):
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.8.y&id=fb49deec375aa5beca4a5d71d7a74ec951872f28
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.8.y&id=418c5575d56410c6e186ab727bf32ae32447d497
>>>>
>>>> Impacted devices seem to include: "PIXA3854:00" and "i2c_designware
>>>> AMDI0010:03"
>>> Any news on this? If this is still unresolved I'll bring this to the
>>> attention of the right developer, if this is a recent regressions (it
>>> sounds like it, but it's not exactly clear; and from the first link
>>> above it sounds like it's partly due to a change in arch's approach to
>>> the initramfs).
>>>
>>> Ciao, Thorsten
^ permalink raw reply
* [PATCH] Fix spelling and grammar in driver doc
From: Luis Felipe Hernandez @ 2024-05-22 17:31 UTC (permalink / raw)
To: skhan, jikos, bentiss
Cc: Luis Felipe Hernandez, linux-kernel-mentees, linux-input,
linux-kernel
* XBox -> Xbox (lowercase 'b') as per official Microsoft branding
* the controller itself has more than one paddle, hence rephrasing
"exports the paddle" to reflext this
* rephrase the description of "those" usage to explicitly make reference
to the paddles which lends itself to replace the SDL statement to
them
* report -> reports grammatical fix in favor of present tense verb. Reports is refering to the act the action being performed by
the subject, in this case the kernel
* spelling fix: interpret
* capitalized first word in bullet points for consistency
Signed-off-by: Luis Felipe Hernandez <luis.hernandez093@gmail.com>
---
.../hid/bpf/progs/Microsoft__XBox-Elite-2.bpf.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/bpf/progs/Microsoft__XBox-Elite-2.bpf.c b/drivers/hid/bpf/progs/Microsoft__XBox-Elite-2.bpf.c
index c04abecab8ee..858cf20ebe2c 100644
--- a/drivers/hid/bpf/progs/Microsoft__XBox-Elite-2.bpf.c
+++ b/drivers/hid/bpf/progs/Microsoft__XBox-Elite-2.bpf.c
@@ -15,20 +15,19 @@ HID_BPF_CONFIG(
);
/*
- * When using the XBox Wireless Controller Elite 2 over Bluetooth,
- * the device exports the paddle on the back of the device as a single
+ * When using the Xbox Wireless Controller Elite 2 over Bluetooth,
+ * the device exports the paddles on the back of the device as a single
* bitfield value of usage "Assign Selection".
*
- * The kernel doesn't process those usages properly and report KEY_UNKNOWN
- * for it.
+ * The kernel doesn't process the paddles usage properly and reports KEY_UNKNOWN.
*
- * SDL doesn't know how to interprete that KEY_UNKNOWN and thus ignores the paddles.
+ * SDL doesn't know how to interpret KEY_UNKNOWN and thus ignores them.
*
* Given that over USB the kernel uses BTN_TRIGGER_HAPPY[5-8], we
- * can tweak the report descriptor to make the kernel interprete it properly:
- * - we need an application collection of gamepad (so we have to close the current
+ * can tweak the report descriptor to make the kernel interpret it properly:
+ * - We need an application collection of gamepad (so we have to close the current
* Consumer Control one)
- * - we need to change the usage to be buttons from 0x15 to 0x18
+ * - We need to change the usage to be buttons from 0x15 to 0x18
*/
#define OFFSET_ASSIGN_SELECTION 211
--
2.45.1
^ permalink raw reply related
* Re: [PATCH] Input: wacom_w8001: Check for string overflow from strscpy calls
From: Dmitry Torokhov @ 2024-05-22 17:23 UTC (permalink / raw)
To: Ping Cheng
Cc: linux-input, Aaron Armstrong Skomra, Jason Gerecke,
Joshua Dickens, Joshua Dickens
In-Reply-To: <CAF8JNhKqAOvxo3C1SaecaVP0uj4QeYpttybBgq_Jw7n18MLpcA@mail.gmail.com>
On Mon, May 20, 2024 at 08:55:30PM -0700, Ping Cheng wrote:
> On Mon, May 20, 2024 at 4:28 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>
> Hi Dmitry,
>
> > > This fix is the same as [1]. Without checking the return value,
> > > Wolfram's patch [2] fails our downstream build script. I'm adding my
> > > r-b, if it makes any difference ;).
> >
> > Could you please tell me how exactly it makes your build script to fail?
>
> We got an "unused-result warning". Jason made a temporary workaround
> at https://github.com/linuxwacom/input-wacom/commit/e83a9bb3e48d2d1b52ec709e60f73b9960d568e5.
I do not think strscpy is declared as __must_check. Do you have a repro
for the vanilla kernel build?
>
> > My concern is that the warnings are not actionable and therefore pretty
> > much worthless.
>
> The return value tells us which strscpy call(s) didn't have enough space.
Right, and what can be done about it? The driver does not control what
tty is being used for the serial port (and so can't control the prefix
of the 'phys'), we do not extend 'phys' (because it is used very
rarely). The only option is to truncate (which we already do).
So the warnings are not actionable.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller
From: Hans de Goede @ 2024-05-22 16:00 UTC (permalink / raw)
To: Krzysztof Kozlowski, joelselvaraj.oss, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <6f22e42d-8a06-4c24-93bd-25b6ac141cea@kernel.org>
Hi,
On 5/21/24 6:48 PM, Krzysztof Kozlowski wrote:
> On 21/05/2024 14:09, Joel Selvaraj via B4 Relay wrote:
>> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>>
>> Document the Novatek NVT touchscreen driver which is used in devices like
>
> driver? or device?
>
>> the Xiaomi Poco F1 [1]. Also, include the devictree binding file in the
>> MAINTAINERS file.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9
>>
>> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> ---
>> .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 ++++++++++++++++++++++
>> MAINTAINERS | 1 +
>> 2 files changed, 63 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> new file mode 100644
>> index 0000000000000..7839c6a028e4a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/touchscreen/novatek,nvt-ts.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Novatek NVT Touchscreen Controller
>> +
>> +maintainers:
>> + - Hans de Goede <hdegoede@redhat.com>
>> +
>> +allOf:
>> + - $ref: touchscreen.yaml#
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - novatek,nvt-ts
>
> That's too generic. Looking at your driver change, it is not even needed.
I wrote the novatek-nvt-ts driver for an Acer Iconia One 7 B1-750 tablet,
this is a x86 tablet which ships with Android as factory OS and because
Android OS images use kernels where everything is hardcoded the ACPI tables
do not describe this touchscreen. Instead the i2c_client for the touchscreen
is manually instantiated by some x86 platform glue code. Since it is
manually instantiated it uses i2c_device_id binding rather then OF/ACPI.
The generic "NVT-ts" i2c_device_id comes from me not knowing the controller
panel type back then. In the mean time I have learned that the B1-750 uses
NVT-NT11205 controller.
So what I think needs to happen here is add a preparation patch as first
patch to this series which basically does this:
diff --git a/drivers/input/touchscreen/novatek-nvt-ts.c b/drivers/input/touchscreen/novatek-nvt-ts.c
index 1a797e410a3f..224fd112b25a 100644
--- a/drivers/input/touchscreen/novatek-nvt-ts.c
+++ b/drivers/input/touchscreen/novatek-nvt-ts.c
@@ -278,7 +278,7 @@ static int nvt_ts_probe(struct i2c_client *client)
}
static const struct i2c_device_id nvt_ts_i2c_id[] = {
- { "NVT-ts" },
+ { "NT11205-ts" },
{ }
};
MODULE_DEVICE_TABLE(i2c, nvt_ts_i2c_id);
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index eb0e55c69dfe..5ecee6e66fb4 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -40,7 +40,7 @@ static const struct x86_i2c_client_info acer_b1_750_i2c_clients[] __initconst =
{
/* Novatek NVT-ts touchscreen */
.board_info = {
- .type = "NVT-ts",
+ .type = "NT11205-ts",
.addr = 0x34,
.dev_name = "NVT-ts",
},
This solves the too-generic ID problema nd can then be merged
together with the rest of the series through the input tree.
I'll give my ack as drivers/platform/x86 subsys maintainer for
merging the x86-android-tablets change this way.
>> + - novatek,nt36672a-ts
>
> Eh, we have already panel. Why there is a need for touchscreen binding
> (binding, not driver)?
I believe that the nt36672a identifier is an identifier for
a novatek display assembly which contains both a DSI display
panel as well as an I2C touchscreen. Since I2C devices need
to be children of the I2C controller we need a separate node
in the device tree for the I2c touchscreen-controller and since
it is a separate node it needs it own compatible I believe ?
Regards,
Hans
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller
From: Joel Selvaraj @ 2024-05-22 14:00 UTC (permalink / raw)
To: Krzysztof Kozlowski, Hans de Goede, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <6f22e42d-8a06-4c24-93bd-25b6ac141cea@kernel.org>
Hi Krzysztof Kozlowski,
On 5/21/24 11:48, Krzysztof Kozlowski wrote:
> On 21/05/2024 14:09, Joel Selvaraj via B4 Relay wrote:
>> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>>
>> Document the Novatek NVT touchscreen driver which is used in devices like
>
> driver? or device?
touchscreen "controller" would be correct I think. I will fix it in v2.
>> the Xiaomi Poco F1 [1]. Also, include the devictree binding file in the
>> MAINTAINERS file.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9
>>
>> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>> ---
>> .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 ++++++++++++++++++++++
>> MAINTAINERS | 1 +
>> 2 files changed, 63 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> new file mode 100644
>> index 0000000000000..7839c6a028e4a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/touchscreen/novatek,nvt-ts.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Novatek NVT Touchscreen Controller
>> +
>> +maintainers:
>> + - Hans de Goede <hdegoede@redhat.com>
>> +
>> +allOf:
>> + - $ref: touchscreen.yaml#
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - novatek,nvt-ts
>
> That's too generic. Looking at your driver change, it is not even needed.
>
>> + - novatek,nt36672a-ts
>
> Eh, we have already panel. Why there is a need for touchscreen binding
> (binding, not driver)?
I am not sure I understand this correctly. Help me a bit here. For
context, in mainline there is an existing driver for the novatek nvt
touchscreen controller. The driver did not have devicetree support. It
only had a i2c_device_id "NVT-ts". I don't know what is the variant of
that Novatek touchscreen controller. To use the driver in Xiaomi Poco
F1, I introduced a devicetree compatible for it "novatek,nvt-ts". The
However, the Novatek touchscreen controller present in Xiaomi Poco F1 is
"NT36672A" which has a different chip id than the one in existing
driver. So I created a separate compatible for this touchscreen
controller variant "novatek,nt36672a-ts". I used compatible data to
differentiate the two variants. Since there are two variants, I am
mentioning both here.
Between, the chip_id and wake_type are the only values that changes
between these two variants. And these are only checked during the probe
and is not used anywhere else in the code. If we remove this sanity
check during probing, then there is no need for two variants and we can
just keep the generic "novatek,nvt-ts".
Kindly let me know what is the correct thing to do here? How this should
be handled? I will be happy to address it in v2.
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + reset-gpios:
>> + maxItems: 1
>> +
>> + vcc-supply: true
>> + iovcc-supply: true
>> +
>> +unevaluatedProperties: false
>
> This goes after required:
Will fix in v2.
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> +
>
>
> Best regards,
> Krzysztof
Regards,
Joel Selvaraj
^ permalink raw reply
* Re: [PATCH] HID: usbhid: enable remote wakeup for mouse
From: Greg KH @ 2024-05-22 11:23 UTC (permalink / raw)
To: huanglei814
Cc: jikos, bentiss, linux-usb, linux-input, linux-kernel, huanglei
In-Reply-To: <20240522092257.19373-1-huanglei814@163.com>
On Wed, May 22, 2024 at 05:22:57PM +0800, huanglei814 wrote:
> From: huanglei <huanglei@kylinos.cn>
>
> This patch enables remote wakeup by default for USB mouse
> devices.
That is not a good idea. Please see the mailing list archives for the
past 20+ years for why this is the way it is.
If you know your device can support this, please set it in userspace,
but we can not change the default value at this point in time, sorry.
> Mouse can used to be wakeup devices, but the correct
> place to enable it depends on the device's bus; no single
> approach will work for all mouse devices. In particular, this
> covers only USB mouse (and then only those supporting the boot
> protocol).
And that is really not a wise choice, boot protocol mice have no
requirement that they support remote wakeup. So restricting it like
this really will not help, sorry.
Again, do this in userspace, that's why the interface is there to do so.
greg k-h
^ permalink raw reply
* [PATCH] Input: ili210x - fix ili251x_read_touch_data() return value
From: John Keeping @ 2024-05-22 10:03 UTC (permalink / raw)
To: linux-input; +Cc: John Keeping, Dmitry Torokhov, linux-kernel
The caller of this function treats all non-zero values as an error, so
the return value of i2c_master_recv() cannot be returned directly.
Follow the same pattern as ili211x_read_touch_data() to return zero when
the correct number of bytes is read and a negative error code otherwise.
This fixes touch reporting when there are more than 6 active touches.
Signed-off-by: John Keeping <jkeeping@inmusicbrands.com>
---
drivers/input/touchscreen/ili210x.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 31ffdc2a93f35..8846c6d10fc0d 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -255,14 +255,15 @@ static int ili251x_read_reg(struct i2c_client *client,
static int ili251x_read_touch_data(struct i2c_client *client, u8 *data)
{
int error;
+ int ret;
error = ili251x_read_reg_common(client, REG_TOUCHDATA,
data, ILI251X_DATA_SIZE1, 0);
if (!error && data[0] == 2) {
- error = i2c_master_recv(client, data + ILI251X_DATA_SIZE1,
- ILI251X_DATA_SIZE2);
- if (error >= 0 && error != ILI251X_DATA_SIZE2)
- error = -EIO;
+ ret = i2c_master_recv(client, data + ILI251X_DATA_SIZE1,
+ ILI251X_DATA_SIZE2);
+ if (ret != ILI251X_DATA_SIZE2)
+ error = ret < 0 ? ret : -EIO;
}
return error;
--
2.45.1
^ permalink raw reply related
* Re: [PATCH] HID: usbhid: enable remote wakeup for mouse
From: Oliver Neukum @ 2024-05-22 10:00 UTC (permalink / raw)
To: huanglei814, jikos, bentiss
Cc: linux-usb, linux-input, linux-kernel, huanglei
In-Reply-To: <20240522092257.19373-1-huanglei814@163.com>
On 22.05.24 11:22, huanglei814 wrote:
> From: huanglei <huanglei@kylinos.cn>
>
> This patch enables remote wakeup by default for USB mouse
> devices. Mouse can used to be wakeup devices, but the correct
> place to enable it depends on the device's bus; no single
> approach will work for all mouse devices. In particular, this
> covers only USB mouse (and then only those supporting the boot
> protocol).
Hi,
could you explain in the log why you want this to depend
on support for the boot protocol?
Regards
Oliver
^ permalink raw reply
* [PATCH] HID: usbhid: enable remote wakeup for mouse
From: huanglei814 @ 2024-05-22 9:22 UTC (permalink / raw)
To: jikos, bentiss; +Cc: linux-usb, linux-input, linux-kernel, huanglei
From: huanglei <huanglei@kylinos.cn>
This patch enables remote wakeup by default for USB mouse
devices. Mouse can used to be wakeup devices, but the correct
place to enable it depends on the device's bus; no single
approach will work for all mouse devices. In particular, this
covers only USB mouse (and then only those supporting the boot
protocol).
Signed-off-by: huanglei <huanglei@kylinos.cn>
---
drivers/hid/usbhid/hid-core.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index a90ed2ceae84..7ed3ab36426d 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1189,6 +1189,15 @@ static int usbhid_start(struct hid_device *hid)
device_set_wakeup_enable(&dev->dev, 1);
}
+ /* enable remote wakeup by default for all mouse
+ * devices supporting the boot protocol.
+ */
+ if (interface->desc.bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT &&
+ interface->desc.bInterfaceProtocol ==
+ USB_INTERFACE_PROTOCOL_MOUSE) {
+ device_set_wakeup_enable(&dev->dev, 1);
+ }
+
mutex_unlock(&usbhid->mutex);
return 0;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] HID: Use kvzalloc instead of kzalloc in hid_register_field()
From: Barry Song @ 2024-05-22 9:07 UTC (permalink / raw)
To: hailong.liu; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20240522080328.12317-1-hailong.liu@oppo.com>
On Wed, May 22, 2024 at 8:03 PM <hailong.liu@oppo.com> wrote:
>
> From: "Hailong.Liu" <hailong.liu@oppo.com>
>
> The function hid_register_field() might allocate more than 32k, which
> would use order-4 contiguous memory if the parameter usage exceeds
> 1024. However, after the system runs for a while, the memory can
> become heavily fragmented. This increases the likelihood of order-4 page
> allocation failure. Here’s the relevant log.
>
> [71553.093623]kworker/1: 0: page allocation failure: order:4, mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), nodemask=(null),cpuset=/,mems_allowed=0
> [71553.093669]Workqueue: events uhid_device_add_worker
> [71553.093683]Call trace:
> [71553.093687]: dump_backtrace+0xf4/0x118
> [71553.093696]: show_stack+0x18/0x24
> [71553.093702]: dump_stack_lvl+0x60/0x7c
> [71553.093710]: dump_stack+0x18/0x3c
> [71553.093717]: warn_alloc+0xf4/0x174
> [71553.093725]: __alloc_pages_slowpath+0x1ba0/0x1cac
> [71553.093732]: __alloc_pages+0x460/0x560
> [71553.093738]: __kmalloc_large_node+0xbc/0x1f8
> [71553.093746]: __kmalloc+0x144/0x254
> [71553.093752]: hid_add_field+0x13c/0x308
> [71553.093758]: hid_parser_main+0x250/0x298
> [71553.093765]: hid_open_report+0x214/0x30c
> [71553.093771]: mt_probe+0x130/0x258
> [71553.093778]: hid_device_probe+0x11c/0x1e4
> [71553.093784]: really_probe+0xe4/0x388
> [71553.093791]: __driver_probe_device+0xa0/0x12c
> [71553.093798]: driver_probe_device+0x44/0x214
> [71553.093804]: __device_attach_driver+0xdc/0x124
> [71553.093812]: bus_for_each_drv+0x88/0xec
> [71553.093818]: __device_attach+0x84/0x170
> [71553.093824]: device_initial_probe+0x14/0x20
> [71553.093831]: bus_probe_device+0x48/0xd0
> [71553.093836]: device_add+0x248/0x928
> [71553.093844]: hid_add_device+0xf8/0x1a4
> [71553.093850]: uhid_device_add_worker+0x24/0x144
> [71553.093857]: process_one_work+0x158/0x804
> [71553.093865]: worker_thread+0x15c/0x494
> [71553.093872]: kthread+0xf4/0x1e4
> [71553.093880]: ret_from_fork+0x10/0x20
>
> To fix the allocation failure, use kvzalloc() instead of kzalloc().
>
> Signed-off-by: Hailong.Liu <hailong.liu@oppo.com>
This makes a lot of sense from the perspective of mm.
Acked-by: Barry Song <baohua@kernel.org>
> ---
> drivers/hid/hid-core.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index de7a477d6665..574ec4873f41 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -95,9 +95,9 @@ static struct hid_field *hid_register_field(struct hid_report *report, unsigned
> return NULL;
> }
>
> - field = kzalloc((sizeof(struct hid_field) +
> - usages * sizeof(struct hid_usage) +
> - 3 * usages * sizeof(unsigned int)), GFP_KERNEL);
> + field = kvzalloc((sizeof(struct hid_field) +
> + usages * sizeof(struct hid_usage) +
> + 3 * usages * sizeof(unsigned int)), GFP_KERNEL);
> if (!field)
> return NULL;
>
> @@ -661,7 +661,7 @@ static void hid_free_report(struct hid_report *report)
> kfree(report->field_entries);
>
> for (n = 0; n < report->maxfield; n++)
> - kfree(report->field[n]);
> + kvfree(report->field[n]);
> kfree(report);
> }
>
> --
> 2.34.1
>
Thanks
Barry
^ permalink raw reply
* [PATCH] HID: Use kvzalloc instead of kzalloc in hid_register_field()
From: hailong.liu @ 2024-05-22 8:03 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, 21cnbao, Hailong.Liu
From: "Hailong.Liu" <hailong.liu@oppo.com>
The function hid_register_field() might allocate more than 32k, which
would use order-4 contiguous memory if the parameter usage exceeds
1024. However, after the system runs for a while, the memory can
become heavily fragmented. This increases the likelihood of order-4 page
allocation failure. Here’s the relevant log.
[71553.093623]kworker/1: 0: page allocation failure: order:4, mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), nodemask=(null),cpuset=/,mems_allowed=0
[71553.093669]Workqueue: events uhid_device_add_worker
[71553.093683]Call trace:
[71553.093687]: dump_backtrace+0xf4/0x118
[71553.093696]: show_stack+0x18/0x24
[71553.093702]: dump_stack_lvl+0x60/0x7c
[71553.093710]: dump_stack+0x18/0x3c
[71553.093717]: warn_alloc+0xf4/0x174
[71553.093725]: __alloc_pages_slowpath+0x1ba0/0x1cac
[71553.093732]: __alloc_pages+0x460/0x560
[71553.093738]: __kmalloc_large_node+0xbc/0x1f8
[71553.093746]: __kmalloc+0x144/0x254
[71553.093752]: hid_add_field+0x13c/0x308
[71553.093758]: hid_parser_main+0x250/0x298
[71553.093765]: hid_open_report+0x214/0x30c
[71553.093771]: mt_probe+0x130/0x258
[71553.093778]: hid_device_probe+0x11c/0x1e4
[71553.093784]: really_probe+0xe4/0x388
[71553.093791]: __driver_probe_device+0xa0/0x12c
[71553.093798]: driver_probe_device+0x44/0x214
[71553.093804]: __device_attach_driver+0xdc/0x124
[71553.093812]: bus_for_each_drv+0x88/0xec
[71553.093818]: __device_attach+0x84/0x170
[71553.093824]: device_initial_probe+0x14/0x20
[71553.093831]: bus_probe_device+0x48/0xd0
[71553.093836]: device_add+0x248/0x928
[71553.093844]: hid_add_device+0xf8/0x1a4
[71553.093850]: uhid_device_add_worker+0x24/0x144
[71553.093857]: process_one_work+0x158/0x804
[71553.093865]: worker_thread+0x15c/0x494
[71553.093872]: kthread+0xf4/0x1e4
[71553.093880]: ret_from_fork+0x10/0x20
To fix the allocation failure, use kvzalloc() instead of kzalloc().
Signed-off-by: Hailong.Liu <hailong.liu@oppo.com>
---
drivers/hid/hid-core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index de7a477d6665..574ec4873f41 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -95,9 +95,9 @@ static struct hid_field *hid_register_field(struct hid_report *report, unsigned
return NULL;
}
- field = kzalloc((sizeof(struct hid_field) +
- usages * sizeof(struct hid_usage) +
- 3 * usages * sizeof(unsigned int)), GFP_KERNEL);
+ field = kvzalloc((sizeof(struct hid_field) +
+ usages * sizeof(struct hid_usage) +
+ 3 * usages * sizeof(unsigned int)), GFP_KERNEL);
if (!field)
return NULL;
@@ -661,7 +661,7 @@ static void hid_free_report(struct hid_report *report)
kfree(report->field_entries);
for (n = 0; n < report->maxfield; n++)
- kfree(report->field[n]);
+ kvfree(report->field[n]);
kfree(report);
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: input: document Novatek NVT touchscreen controller
From: Krzysztof Kozlowski @ 2024-05-21 16:48 UTC (permalink / raw)
To: joelselvaraj.oss, Hans de Goede, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <20240521-nvt-ts-devicetree-regulator-support-v1-1-8d766c639dca@gmail.com>
On 21/05/2024 14:09, Joel Selvaraj via B4 Relay wrote:
> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>
> Document the Novatek NVT touchscreen driver which is used in devices like
driver? or device?
> the Xiaomi Poco F1 [1]. Also, include the devictree binding file in the
> MAINTAINERS file.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts?h=v6.9
>
> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
> ---
> .../bindings/input/touchscreen/novatek,nvt-ts.yaml | 62 ++++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 63 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
> new file mode 100644
> index 0000000000000..7839c6a028e4a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/novatek,nvt-ts.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Novatek NVT Touchscreen Controller
> +
> +maintainers:
> + - Hans de Goede <hdegoede@redhat.com>
> +
> +allOf:
> + - $ref: touchscreen.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - novatek,nvt-ts
That's too generic. Looking at your driver change, it is not even needed.
> + - novatek,nt36672a-ts
Eh, we have already panel. Why there is a need for touchscreen binding
(binding, not driver)?
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> +
> + vcc-supply: true
> + iovcc-supply: true
> +
> +unevaluatedProperties: false
This goes after required:
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: touchscreen: edt-ft5x06: Document FT5452 and FT8719 support
From: Krzysztof Kozlowski @ 2024-05-21 16:15 UTC (permalink / raw)
To: joelselvaraj.oss, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <20240521-add-support-ft5452-and-ft8719-touchscreen-v1-1-2a648ac7176b@gmail.com>
On 21/05/2024 16:02, Joel Selvaraj via B4 Relay wrote:
> From: Joel Selvaraj <joelselvaraj.oss@gmail.com>
>
> Document FocalTech FT5452 and FT8719 support by adding their compatibles.
>
> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>
> ---
> Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 2 ++
> 1 file changed, 2 insertions(+)
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v11 0/3] Input: Add TouchNetix axiom touchscreen driver
From: Kamel BOUHARA @ 2024-05-21 15:24 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Marco Felsch, Jeff LaBundy
Cc: catalin.popescu, mark.satterthwaite, Thomas Petazzoni,
Gregory Clement, bsp-development.geo
In-Reply-To: <20240419123829.120396-1-kamel.bouhara@bootlin.com>
Le 2024-04-19 14:38, Kamel Bouhara a écrit :
> Add a new driver for the TouchNetix's axiom family of touchscreen
> controller. This driver only support i2c and can be later adapted for
> SPI and USB support.
>
Hello Dmitry,
I just saw you are still applying patch and I wanted to know if this
series could be merged as well ?
Kamel
> --
> Changes in v11:
> - Fix regulators name to match dt-binding
> - Enable regulators before reset is asserted
>
> Changes in v10:
> - Set regulators as required
> - Enable power supply before reset
> - Fix ref count due to regulator requested twice
> - Rebase on v6.9-rc4
>
> Changes in v9:
> - Fix issue reported in
> https://lore.kernel.org/oe-kbuild-all/202402201157.BKo97uWl-lkp@intel.com/
> - Rebase on v6.8-rc2
>
> Changes in v8:
> - Fix missing call to input_report_slot_state()
> - Fix issue reported in
> https://lore.kernel.org/oe-kbuild-all/202402020623.8T1Ah513-lkp@intel.com/
>
> Changes in v7:
> - Remove startup time from dt-binding
> - Fix usage table not correctly populated
>
> Changes in v6:
> - Fix missing unevaluatedProperties.in dt-binding
> - Use __le16 to correctly deal with device endianness
> - Use standart kernel types s/char/u8/
> - Use regmap api as driver might support spi later
> - Use get_unaligned_le16() for the sake of clarity
> - Use devm_regulator_enable_optional()
>
> Changes in v5:
> - Fix wrong message constructed in axiom_i2c_read
> - Delay required between i2c reads is >= 250us
> - Do not split report reading in two phases as we'll
> have to wait 500us
> - Use lower-case in properties names
> - Make regulators properties are required in dt-binding
> - Fix bug report:
> https://lore.kernel.org/lkml/202312051457.y3N1q3sZ-lkp@intel.com/
> - Fix bug report:
> https://lore.kernel.org/lkml/6f8e3b64-5b21-4a50-8680-063ef7a93bdb@suswa.mountain/
>
> Changes in v4:
> - Cleanup unused headers and macros
> - Use standard kernel type
> - Namespace structures and functions
> - Use packed struct when possible to avoid bitfield operators
> - Fix missing break when address is found in
> axiom_populate_target_address()
> - Split reads in two steps for the reports, first length then report
> itself so we only read required bytes
> - Get poll-interval from devicetree
> - Add VDDI/VDDA regulators
> - Add a startup delay of 110 ms required after VDDA/VDDI is applied
> - Remove axiom_i2c_write() as it is no more used
>
> Changes in v3:
> - Remove irq-gpios property in dt-binding
> - Use a generic node name
> - Fix issues reported in
> https://lore.kernel.org/oe-kbuild-all/202310100300.oAC2M62R-lkp@intel.com/
>
> Changes in v2:
> - Add device tree binding documentation
> - Move core functions in axiom_i2c as we only care about i2c support
> now
> - Use static function when required
> - Use syntax dev_err_probe()
> - Add an hardware based reset
>
>
> Kamel Bouhara (3):
> dt-bindings: vendor-prefixes: Add TouchNetix AS
> dt-bindings: input: Add TouchNetix axiom touchscreen
> Input: Add TouchNetix axiom i2c touchscreen driver
>
> .../input/touchscreen/touchnetix,ax54a.yaml | 62 ++
> .../devicetree/bindings/vendor-prefixes.yaml | 2 +
> MAINTAINERS | 8 +
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/touchnetix_axiom.c | 657 ++++++++++++++++++
> 6 files changed, 742 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
> create mode 100644 drivers/input/touchscreen/touchnetix_axiom.c
>
> --
> 2.25.1
--
--
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox