* [PATCH 3/7] Input: libps2 - rework handling of command response
From: Dmitry Torokhov @ 2023-05-11 18:52 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-1-dmitry.torokhov@gmail.com>
It is not entirely correct that libps2 sets PS2_FLAG_CMD1 after
the device acknowledges each byte sent to the device by the host.
Rework the code so that PS2_FLAG_CMD1 and PS2_FLAG_CMD are set only once,
at the beginning of PS/2 command execution.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/serio/libps2.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 399cda0d34f5..d09450eca9a7 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -247,14 +247,19 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
serio_pause_rx(ps2dev->serio);
+ /* Some mice do not ACK the "get ID" command, prepare to handle this. */
ps2dev->flags = command == PS2_CMD_GETID ? PS2_FLAG_WAITID : 0;
ps2dev->cmdcnt = receive;
- if (receive && param)
- for (i = 0; i < receive; i++)
- ps2dev->cmdbuf[(receive - 1) - i] = param[i];
+ if (receive) {
+ /* Indicate that we expect response to the command. */
+ ps2dev->flags |= PS2_FLAG_CMD | PS2_FLAG_CMD1;
+ if (param)
+ for (i = 0; i < receive; i++)
+ ps2dev->cmdbuf[(receive - 1) - i] = param[i];
+ }
/*
- * Some devices (Synaptics) peform the reset before
+ * Some devices (Synaptics) perform the reset before
* ACKing the reset command, and so it can take a long
* time before the ACK arrives.
*/
@@ -434,11 +439,8 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
return true;
}
- if (!ps2dev->nak) {
+ if (!ps2dev->nak)
ps2dev->flags &= ~PS2_FLAG_NAK;
- if (ps2dev->cmdcnt)
- ps2dev->flags |= PS2_FLAG_CMD | PS2_FLAG_CMD1;
- }
ps2dev->flags &= ~PS2_FLAG_ACK;
wake_up(&ps2dev->wait);
--
2.40.1.606.ga4b1b128d6-goog
^ permalink raw reply related
* [PATCH 2/7] Input: libps2 - remove special handling of ACK for command byte
From: Dmitry Torokhov @ 2023-05-11 18:52 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-1-dmitry.torokhov@gmail.com>
When getting unexpected data while waiting for an acknowledgement it does
not matter what command phase is currently executed, and ps2_handle_ack()
should indicate that no further processing is needed for the received data
byte. Remove PS2_FLAG_ACK_CMD and associated handling.
Note that while it is possible to make ps2_handle_ack (and
ps2_handle_repsonse) return void, it will be done when the code will be
converted to common PS/2 interrupt handler later.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/serio/libps2.c | 9 ++-------
include/linux/libps2.h | 1 -
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 764990723847..399cda0d34f5 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -253,9 +253,6 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
for (i = 0; i < receive; i++)
ps2dev->cmdbuf[(receive - 1) - i] = param[i];
- /* Signal that we are sending the command byte */
- ps2dev->flags |= PS2_FLAG_ACK_CMD;
-
/*
* Some devices (Synaptics) peform the reset before
* ACKing the reset command, and so it can take a long
@@ -267,9 +264,7 @@ int __ps2_command(struct ps2dev *ps2dev, u8 *param, unsigned int command)
if (rc)
goto out_reset_flags;
- /* Now we are sending command parameters, if any */
- ps2dev->flags &= ~PS2_FLAG_ACK_CMD;
-
+ /* Send command parameters, if any. */
for (i = 0; i < send; i++) {
rc = ps2_do_sendbyte(ps2dev, param[i], 200, 2);
if (rc)
@@ -436,7 +431,7 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
*/
dev_dbg(&ps2dev->serio->dev, "unexpected %#02x\n", data);
ps2dev->flags &= ~PS2_FLAG_WAITID;
- return ps2dev->flags & PS2_FLAG_ACK_CMD;
+ return true;
}
if (!ps2dev->nak) {
diff --git a/include/linux/libps2.h b/include/linux/libps2.h
index 53f7e4d0f4b7..193dd53ad18b 100644
--- a/include/linux/libps2.h
+++ b/include/linux/libps2.h
@@ -28,7 +28,6 @@
#define PS2_FLAG_CMD1 BIT(2) /* Waiting for the first byte of command response */
#define PS2_FLAG_WAITID BIT(3) /* Command executing is GET ID */
#define PS2_FLAG_NAK BIT(4) /* Last transmission was NAKed */
-#define PS2_FLAG_ACK_CMD BIT(5) /* Waiting to ACK the command (first) byte */
struct ps2dev {
struct serio *serio;
--
2.40.1.606.ga4b1b128d6-goog
^ permalink raw reply related
* [PATCH 1/7] Input: libps2 - attach ps2dev instances as serio port's drvdata
From: Dmitry Torokhov @ 2023-05-11 18:52 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
In-Reply-To: <20230511185252.386941-1-dmitry.torokhov@gmail.com>
In preparation of having unified interrupt handler for PS/2 devices,
instead of attaching instances of psmouse and atkbd structures as serio's
driver data, switch to attaching ps2dev instances.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/atkbd.c | 23 +++++++++++++-------
drivers/input/mouse/psmouse-base.c | 35 +++++++++++++++++-------------
drivers/input/mouse/psmouse.h | 2 ++
drivers/input/mouse/synaptics.c | 10 ++++-----
drivers/input/mouse/trackpoint.c | 2 +-
drivers/input/serio/libps2.c | 1 +
6 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 246958795f60..2fb2ad73e796 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -309,12 +309,19 @@ static ssize_t atkbd_show_function_row_physmap(struct atkbd *atkbd, char *buf)
return vivaldi_function_row_physmap_show(&atkbd->vdata, buf);
}
+static struct atkbd *atkbd_from_serio(struct serio *serio)
+{
+ struct ps2dev *ps2dev = serio_get_drvdata(serio);
+
+ return container_of(ps2dev, struct atkbd, ps2dev);
+}
+
static umode_t atkbd_attr_is_visible(struct kobject *kobj,
struct attribute *attr, int i)
{
struct device *dev = kobj_to_dev(kobj);
struct serio *serio = to_serio_port(dev);
- struct atkbd *atkbd = serio_get_drvdata(serio);
+ struct atkbd *atkbd = atkbd_from_serio(serio);
if (attr == &atkbd_attr_function_row_physmap.attr &&
!atkbd->vdata.num_function_row_keys)
@@ -399,7 +406,7 @@ static unsigned int atkbd_compat_scancode(struct atkbd *atkbd, unsigned int code
static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
unsigned int flags)
{
- struct atkbd *atkbd = serio_get_drvdata(serio);
+ struct atkbd *atkbd = atkbd_from_serio(serio);
struct input_dev *dev = atkbd->dev;
unsigned int code = data;
int scroll = 0, hscroll = 0, click = -1;
@@ -909,7 +916,7 @@ static int atkbd_reset_state(struct atkbd *atkbd)
static void atkbd_cleanup(struct serio *serio)
{
- struct atkbd *atkbd = serio_get_drvdata(serio);
+ struct atkbd *atkbd = atkbd_from_serio(serio);
atkbd_disable(atkbd);
ps2_command(&atkbd->ps2dev, NULL, ATKBD_CMD_RESET_DEF);
@@ -922,7 +929,7 @@ static void atkbd_cleanup(struct serio *serio)
static void atkbd_disconnect(struct serio *serio)
{
- struct atkbd *atkbd = serio_get_drvdata(serio);
+ struct atkbd *atkbd = atkbd_from_serio(serio);
atkbd_disable(atkbd);
@@ -1188,7 +1195,7 @@ static void atkbd_set_device_attrs(struct atkbd *atkbd)
static void atkbd_parse_fwnode_data(struct serio *serio)
{
- struct atkbd *atkbd = serio_get_drvdata(serio);
+ struct atkbd *atkbd = atkbd_from_serio(serio);
struct device *dev = &serio->dev;
int n;
@@ -1295,7 +1302,7 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv)
static int atkbd_reconnect(struct serio *serio)
{
- struct atkbd *atkbd = serio_get_drvdata(serio);
+ struct atkbd *atkbd = atkbd_from_serio(serio);
struct serio_driver *drv = serio->drv;
int retval = -1;
@@ -1389,7 +1396,7 @@ static ssize_t atkbd_attr_show_helper(struct device *dev, char *buf,
ssize_t (*handler)(struct atkbd *, char *))
{
struct serio *serio = to_serio_port(dev);
- struct atkbd *atkbd = serio_get_drvdata(serio);
+ struct atkbd *atkbd = atkbd_from_serio(serio);
return handler(atkbd, buf);
}
@@ -1398,7 +1405,7 @@ static ssize_t atkbd_attr_set_helper(struct device *dev, const char *buf, size_t
ssize_t (*handler)(struct atkbd *, const char *, size_t))
{
struct serio *serio = to_serio_port(dev);
- struct atkbd *atkbd = serio_get_drvdata(serio);
+ struct atkbd *atkbd = atkbd_from_serio(serio);
int retval;
retval = mutex_lock_interruptible(&atkbd->mutex);
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index c9a7e87b273e..ed5376099fba 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -116,6 +116,13 @@ static DEFINE_MUTEX(psmouse_mutex);
static struct workqueue_struct *kpsmoused_wq;
+struct psmouse *psmouse_from_serio(struct serio *serio)
+{
+ struct ps2dev *ps2dev = serio_get_drvdata(serio);
+
+ return container_of(ps2dev, struct psmouse, ps2dev);
+}
+
void psmouse_report_standard_buttons(struct input_dev *dev, u8 buttons)
{
input_report_key(dev, BTN_LEFT, buttons & BIT(0));
@@ -336,7 +343,7 @@ static void psmouse_handle_oob_data(struct psmouse *psmouse, u8 data)
static irqreturn_t psmouse_interrupt(struct serio *serio,
u8 data, unsigned int flags)
{
- struct psmouse *psmouse = serio_get_drvdata(serio);
+ struct psmouse *psmouse = psmouse_from_serio(serio);
if (psmouse->state == PSMOUSE_IGNORE)
goto out;
@@ -1344,7 +1351,7 @@ static void psmouse_resync(struct work_struct *work)
goto out;
if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
- parent = serio_get_drvdata(serio->parent);
+ parent = psmouse_from_serio(serio->parent);
psmouse_deactivate(parent);
}
@@ -1428,13 +1435,13 @@ static void psmouse_resync(struct work_struct *work)
*/
static void psmouse_cleanup(struct serio *serio)
{
- struct psmouse *psmouse = serio_get_drvdata(serio);
+ struct psmouse *psmouse = psmouse_from_serio(serio);
struct psmouse *parent = NULL;
mutex_lock(&psmouse_mutex);
if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
- parent = serio_get_drvdata(serio->parent);
+ parent = psmouse_from_serio(serio->parent);
psmouse_deactivate(parent);
}
@@ -1476,7 +1483,7 @@ static void psmouse_cleanup(struct serio *serio)
*/
static void psmouse_disconnect(struct serio *serio)
{
- struct psmouse *psmouse = serio_get_drvdata(serio);
+ struct psmouse *psmouse = psmouse_from_serio(serio);
struct psmouse *parent = NULL;
mutex_lock(&psmouse_mutex);
@@ -1489,7 +1496,7 @@ static void psmouse_disconnect(struct serio *serio)
mutex_lock(&psmouse_mutex);
if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
- parent = serio_get_drvdata(serio->parent);
+ parent = psmouse_from_serio(serio->parent);
psmouse_deactivate(parent);
}
@@ -1588,7 +1595,7 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
* connected to this port can be successfully identified
*/
if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
- parent = serio_get_drvdata(serio->parent);
+ parent = psmouse_from_serio(serio->parent);
psmouse_deactivate(parent);
}
@@ -1604,8 +1611,6 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
psmouse_set_state(psmouse, PSMOUSE_INITIALIZING);
- serio_set_drvdata(serio, psmouse);
-
error = serio_open(serio, drv);
if (error)
goto err_clear_drvdata;
@@ -1676,7 +1681,7 @@ static int psmouse_connect(struct serio *serio, struct serio_driver *drv)
static int __psmouse_reconnect(struct serio *serio, bool fast_reconnect)
{
- struct psmouse *psmouse = serio_get_drvdata(serio);
+ struct psmouse *psmouse = psmouse_from_serio(serio);
struct psmouse *parent = NULL;
int (*reconnect_handler)(struct psmouse *);
enum psmouse_type type;
@@ -1695,7 +1700,7 @@ static int __psmouse_reconnect(struct serio *serio, bool fast_reconnect)
}
if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
- parent = serio_get_drvdata(serio->parent);
+ parent = psmouse_from_serio(serio->parent);
psmouse_deactivate(parent);
}
@@ -1794,7 +1799,7 @@ ssize_t psmouse_attr_show_helper(struct device *dev, struct device_attribute *de
{
struct serio *serio = to_serio_port(dev);
struct psmouse_attribute *attr = to_psmouse_attr(devattr);
- struct psmouse *psmouse = serio_get_drvdata(serio);
+ struct psmouse *psmouse = psmouse_from_serio(serio);
if (psmouse->protocol->smbus_companion &&
devattr != &psmouse_attr_protocol.dattr)
@@ -1815,7 +1820,7 @@ ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *dev
if (retval)
goto out;
- psmouse = serio_get_drvdata(serio);
+ psmouse = psmouse_from_serio(serio);
if (psmouse->protocol->smbus_companion &&
devattr != &psmouse_attr_protocol.dattr) {
@@ -1830,7 +1835,7 @@ ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *dev
}
if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
- parent = serio_get_drvdata(serio->parent);
+ parent = psmouse_from_serio(serio->parent);
psmouse_deactivate(parent);
}
@@ -1925,7 +1930,7 @@ static ssize_t psmouse_attr_set_protocol(struct psmouse *psmouse, void *data, co
}
if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
- parent = serio_get_drvdata(serio->parent);
+ parent = psmouse_from_serio(serio->parent);
if (parent->pt_deactivate)
parent->pt_deactivate(parent);
}
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 64c3a5d3fb3e..4d8acfe0d82a 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -130,6 +130,8 @@ struct psmouse {
void (*pt_deactivate)(struct psmouse *psmouse);
};
+struct psmouse *psmouse_from_serio(struct serio *serio);
+
void psmouse_queue_work(struct psmouse *psmouse, struct delayed_work *work,
unsigned long delay);
int psmouse_reset(struct psmouse *psmouse);
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index fa021af8506e..ada299ec5bba 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -628,7 +628,7 @@ static void synaptics_set_rate(struct psmouse *psmouse, unsigned int rate)
****************************************************************************/
static int synaptics_pt_write(struct serio *serio, u8 c)
{
- struct psmouse *parent = serio_get_drvdata(serio->parent);
+ struct psmouse *parent = psmouse_from_serio(serio->parent);
u8 rate_param = SYN_PS_CLIENT_CMD; /* indicates that we want pass-through port */
int error;
@@ -645,7 +645,7 @@ static int synaptics_pt_write(struct serio *serio, u8 c)
static int synaptics_pt_start(struct serio *serio)
{
- struct psmouse *parent = serio_get_drvdata(serio->parent);
+ struct psmouse *parent = psmouse_from_serio(serio->parent);
struct synaptics_data *priv = parent->private;
serio_pause_rx(parent->ps2dev.serio);
@@ -657,7 +657,7 @@ static int synaptics_pt_start(struct serio *serio)
static void synaptics_pt_stop(struct serio *serio)
{
- struct psmouse *parent = serio_get_drvdata(serio->parent);
+ struct psmouse *parent = psmouse_from_serio(serio->parent);
struct synaptics_data *priv = parent->private;
serio_pause_rx(parent->ps2dev.serio);
@@ -672,7 +672,7 @@ static int synaptics_is_pt_packet(u8 *buf)
static void synaptics_pass_pt_packet(struct serio *ptport, u8 *packet)
{
- struct psmouse *child = serio_get_drvdata(ptport);
+ struct psmouse *child = psmouse_from_serio(ptport);
if (child && child->state == PSMOUSE_ACTIVATED) {
serio_interrupt(ptport, packet[1], 0);
@@ -688,7 +688,7 @@ static void synaptics_pass_pt_packet(struct serio *ptport, u8 *packet)
static void synaptics_pt_activate(struct psmouse *psmouse)
{
struct synaptics_data *priv = psmouse->private;
- struct psmouse *child = serio_get_drvdata(priv->pt_port);
+ struct psmouse *child = psmouse_from_serio(priv->pt_port);
/* adjust the touchpad to child's choice of protocol */
if (child) {
diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index 4a86b3e31d3b..5f6643b69a2c 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -216,7 +216,7 @@ static umode_t trackpoint_is_attr_visible(struct kobject *kobj,
{
struct device *dev = kobj_to_dev(kobj);
struct serio *serio = to_serio_port(dev);
- struct psmouse *psmouse = serio_get_drvdata(serio);
+ struct psmouse *psmouse = psmouse_from_serio(serio);
return trackpoint_is_attr_available(psmouse, attr) ? attr->mode : 0;
}
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 3e19344eda93..764990723847 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -382,6 +382,7 @@ void ps2_init(struct ps2dev *ps2dev, struct serio *serio)
lockdep_set_subclass(&ps2dev->cmd_mutex, serio->depth);
init_waitqueue_head(&ps2dev->wait);
ps2dev->serio = serio;
+ serio_set_drvdata(serio, ps2dev);
}
EXPORT_SYMBOL(ps2_init);
--
2.40.1.606.ga4b1b128d6-goog
^ permalink raw reply related
* [PATCH 0/7] libps2: be more tolerant when processing commands
From: Dmitry Torokhov @ 2023-05-11 18:52 UTC (permalink / raw)
To: linux-input; +Cc: Raul E Rangel, linux-kernel
Hi,
The main reason for this patch series is to deal with the case when
EC/keyboard controller has already latched a scancode in the output
buffer at the same time the host (kernel) is sending a PS/2 command to
the controller/device. The device should stop scanning (keyboard) or
sending coordinate data (mouse), and instead send acknowledge (0xfa) and
then potentially command response, but if the output buffer already
contains scancode byte it can not be substituted with an ACK byte.
The typical scenario for this is user activating a CapsLock function,
with host sending command to toggle CapsLock LED. If at the same time
the keyboard transmitting break code for the key the kernel may mistake
it for garbage command response and get confused.
To work around this scenario, instead of simply dropping the non-ACK/NAK
byte we will pass it on to atkbd/psmouse for normal processing.
In addition to the above there a couple more assorted cleanups and
fixes.
Thanks.
Dmitry Torokhov (7):
Input: libps2 - attach ps2dev instances as serio port's drvdata
Input: libps2 - remove special handling of ACK for command byte
Input: libps2 - rework handling of command response
Input: libps2 - fix NAK handling
Input: libps2 - fix aborting PS/2 commands
Input: libps2 - introduce common interrupt handler
Input: libps2 - do not discard non-ack bytes when controlling LEDs
drivers/input/keyboard/atkbd.c | 94 ++++-----
drivers/input/mouse/psmouse-base.c | 86 +++++----
drivers/input/mouse/psmouse.h | 2 +
drivers/input/mouse/synaptics.c | 10 +-
drivers/input/mouse/trackpoint.c | 2 +-
drivers/input/serio/libps2.c | 293 +++++++++++++++++++++--------
include/linux/libps2.h | 62 +++---
7 files changed, 350 insertions(+), 199 deletions(-)
--
Dmitry
^ permalink raw reply
* Re: [RESEND PATCH] dt-bindings: input: cypress,cyapa: convert to dtschema
From: Dmitry Torokhov @ 2023-05-11 18:37 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel, Rob Herring
In-Reply-To: <20230511102559.175088-1-krzysztof.kozlowski@linaro.org>
On Thu, May 11, 2023 at 12:25:59PM +0200, Krzysztof Kozlowski wrote:
> Convert the Cypress All Points Addressable (APA) I2C Touchpad / Trackpad
> bindings to DT schema.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS e96220bce5176ed2309f77f061dcc0430b82b25e
From: kernel test robot @ 2023-05-11 16:32 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: e96220bce5176ed2309f77f061dcc0430b82b25e Input: adxl34x - do not hardcode interrupt trigger type
elapsed time: 741m
configs tested: 259
configs skipped: 28
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allyesconfig gcc
alpha buildonly-randconfig-r001-20230509 gcc
alpha buildonly-randconfig-r002-20230511 gcc
alpha buildonly-randconfig-r005-20230509 gcc
alpha buildonly-randconfig-r005-20230511 gcc
alpha buildonly-randconfig-r006-20230510 gcc
alpha buildonly-randconfig-r006-20230511 gcc
alpha defconfig gcc
alpha randconfig-r005-20230509 gcc
alpha randconfig-r014-20230510 gcc
alpha randconfig-r015-20230510 gcc
alpha randconfig-r015-20230511 gcc
alpha randconfig-r016-20230509 gcc
arc allyesconfig gcc
arc buildonly-randconfig-r001-20230511 gcc
arc buildonly-randconfig-r002-20230510 gcc
arc defconfig gcc
arc randconfig-r016-20230509 gcc
arc randconfig-r022-20230509 gcc
arc randconfig-r023-20230509 gcc
arc randconfig-r025-20230509 gcc
arc randconfig-r031-20230511 gcc
arc randconfig-r036-20230511 gcc
arc randconfig-r043-20230509 gcc
arc randconfig-r043-20230510 gcc
arc randconfig-r043-20230511 gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm buildonly-randconfig-r004-20230509 gcc
arm defconfig gcc
arm randconfig-r013-20230510 gcc
arm randconfig-r013-20230511 clang
arm randconfig-r015-20230511 clang
arm randconfig-r024-20230511 clang
arm randconfig-r035-20230511 gcc
arm randconfig-r046-20230509 gcc
arm randconfig-r046-20230510 gcc
arm randconfig-r046-20230511 clang
arm rpc_defconfig gcc
arm64 allyesconfig gcc
arm64 buildonly-randconfig-r002-20230511 clang
arm64 buildonly-randconfig-r006-20230509 gcc
arm64 defconfig gcc
arm64 randconfig-r002-20230509 gcc
arm64 randconfig-r034-20230509 gcc
csky buildonly-randconfig-r001-20230509 gcc
csky buildonly-randconfig-r002-20230509 gcc
csky buildonly-randconfig-r003-20230509 gcc
csky buildonly-randconfig-r003-20230510 gcc
csky buildonly-randconfig-r006-20230511 gcc
csky defconfig gcc
csky randconfig-r004-20230511 gcc
csky randconfig-r006-20230509 gcc
csky randconfig-r011-20230509 gcc
csky randconfig-r033-20230509 gcc
csky randconfig-r034-20230510 gcc
csky randconfig-r034-20230511 gcc
hexagon buildonly-randconfig-r002-20230511 clang
hexagon randconfig-r024-20230509 clang
hexagon randconfig-r024-20230511 clang
hexagon randconfig-r026-20230511 clang
hexagon randconfig-r041-20230509 clang
hexagon randconfig-r041-20230510 clang
hexagon randconfig-r041-20230511 clang
hexagon randconfig-r045-20230509 clang
hexagon randconfig-r045-20230510 clang
hexagon randconfig-r045-20230511 clang
i386 allnoconfig clang
i386 allyesconfig gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-a001 gcc
i386 randconfig-a002 clang
i386 randconfig-a003 gcc
i386 randconfig-a004 clang
i386 randconfig-a005 gcc
i386 randconfig-a006 clang
i386 randconfig-a011 clang
i386 randconfig-a012 gcc
i386 randconfig-a013 clang
i386 randconfig-a014 gcc
i386 randconfig-a015 clang
i386 randconfig-a016 gcc
ia64 allmodconfig gcc
ia64 defconfig gcc
ia64 generic_defconfig gcc
ia64 randconfig-r002-20230511 gcc
ia64 randconfig-r006-20230511 gcc
ia64 randconfig-r023-20230511 gcc
ia64 randconfig-r033-20230509 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch buildonly-randconfig-r001-20230510 gcc
loongarch buildonly-randconfig-r005-20230509 gcc
loongarch defconfig gcc
loongarch randconfig-r006-20230511 gcc
loongarch randconfig-r014-20230509 gcc
loongarch randconfig-r021-20230509 gcc
loongarch randconfig-r021-20230511 gcc
loongarch randconfig-r024-20230511 gcc
m68k allmodconfig gcc
m68k defconfig gcc
m68k randconfig-r002-20230511 gcc
m68k randconfig-r003-20230511 gcc
m68k randconfig-r004-20230509 gcc
m68k randconfig-r013-20230509 gcc
m68k randconfig-r014-20230511 gcc
m68k randconfig-r025-20230509 gcc
m68k randconfig-r025-20230511 gcc
m68k randconfig-r032-20230510 gcc
microblaze buildonly-randconfig-r001-20230511 gcc
microblaze buildonly-randconfig-r002-20230509 gcc
microblaze buildonly-randconfig-r004-20230511 gcc
microblaze buildonly-randconfig-r006-20230511 gcc
microblaze mmu_defconfig gcc
microblaze randconfig-r003-20230511 gcc
microblaze randconfig-r004-20230509 gcc
microblaze randconfig-r005-20230511 gcc
microblaze randconfig-r014-20230511 gcc
microblaze randconfig-r015-20230509 gcc
microblaze randconfig-r021-20230509 gcc
microblaze randconfig-r034-20230511 gcc
microblaze randconfig-r035-20230509 gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips buildonly-randconfig-r004-20230511 gcc
mips maltasmvp_defconfig gcc
mips maltaup_xpa_defconfig gcc
mips randconfig-r011-20230511 clang
mips randconfig-r024-20230509 gcc
mips randconfig-r031-20230510 clang
mips randconfig-r036-20230511 gcc
nios2 buildonly-randconfig-r001-20230509 gcc
nios2 buildonly-randconfig-r006-20230511 gcc
nios2 defconfig gcc
nios2 randconfig-r003-20230509 gcc
nios2 randconfig-r005-20230511 gcc
nios2 randconfig-r016-20230511 gcc
nios2 randconfig-r022-20230511 gcc
nios2 randconfig-r031-20230509 gcc
nios2 randconfig-r032-20230509 gcc
nios2 randconfig-r036-20230511 gcc
openrisc buildonly-randconfig-r002-20230509 gcc
openrisc buildonly-randconfig-r003-20230511 gcc
openrisc buildonly-randconfig-r006-20230509 gcc
openrisc randconfig-r001-20230509 gcc
openrisc randconfig-r005-20230511 gcc
openrisc randconfig-r022-20230511 gcc
openrisc randconfig-r025-20230511 gcc
openrisc randconfig-r032-20230511 gcc
openrisc randconfig-r033-20230511 gcc
openrisc randconfig-r036-20230509 gcc
parisc defconfig gcc
parisc randconfig-r011-20230511 gcc
parisc randconfig-r013-20230509 gcc
parisc randconfig-r025-20230511 gcc
parisc randconfig-r026-20230511 gcc
parisc randconfig-r031-20230511 gcc
parisc randconfig-r034-20230511 gcc
parisc randconfig-r035-20230509 gcc
parisc randconfig-r035-20230511 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc buildonly-randconfig-r001-20230511 gcc
powerpc buildonly-randconfig-r002-20230511 gcc
powerpc buildonly-randconfig-r003-20230511 gcc
powerpc buildonly-randconfig-r004-20230509 clang
powerpc buildonly-randconfig-r004-20230511 gcc
powerpc buildonly-randconfig-r005-20230511 gcc
powerpc mpc83xx_defconfig gcc
powerpc ppc6xx_defconfig gcc
powerpc randconfig-r001-20230511 clang
powerpc randconfig-r003-20230509 gcc
powerpc randconfig-r014-20230511 gcc
powerpc randconfig-r023-20230509 clang
powerpc randconfig-r023-20230511 gcc
powerpc randconfig-r024-20230509 clang
powerpc randconfig-r026-20230509 clang
powerpc randconfig-r026-20230511 gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv buildonly-randconfig-r004-20230509 clang
riscv buildonly-randconfig-r005-20230511 gcc
riscv defconfig gcc
riscv randconfig-r002-20230511 clang
riscv randconfig-r004-20230511 clang
riscv randconfig-r006-20230511 clang
riscv randconfig-r012-20230511 gcc
riscv randconfig-r013-20230511 gcc
riscv randconfig-r026-20230509 clang
riscv randconfig-r035-20230511 clang
riscv randconfig-r042-20230509 clang
riscv randconfig-r042-20230510 clang
riscv randconfig-r042-20230511 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 buildonly-randconfig-r001-20230511 gcc
s390 buildonly-randconfig-r005-20230509 clang
s390 defconfig gcc
s390 randconfig-r013-20230509 clang
s390 randconfig-r013-20230511 gcc
s390 randconfig-r033-20230511 clang
s390 randconfig-r044-20230509 clang
s390 randconfig-r044-20230510 clang
s390 randconfig-r044-20230511 gcc
sh allmodconfig gcc
sh buildonly-randconfig-r003-20230509 gcc
sh buildonly-randconfig-r004-20230511 gcc
sh buildonly-randconfig-r005-20230511 gcc
sh randconfig-r003-20230509 gcc
sh randconfig-r015-20230509 gcc
sh randconfig-r015-20230511 gcc
sh randconfig-r022-20230509 gcc
sh randconfig-r023-20230511 gcc
sh randconfig-r036-20230510 gcc
sh sdk7786_defconfig gcc
sh titan_defconfig gcc
sparc buildonly-randconfig-r003-20230511 gcc
sparc buildonly-randconfig-r004-20230509 gcc
sparc defconfig gcc
sparc randconfig-r011-20230511 gcc
sparc randconfig-r012-20230511 gcc
sparc randconfig-r016-20230510 gcc
sparc randconfig-r021-20230511 gcc
sparc randconfig-r022-20230509 gcc
sparc randconfig-r036-20230509 gcc
sparc64 buildonly-randconfig-r003-20230509 gcc
sparc64 randconfig-r005-20230509 gcc
sparc64 randconfig-r011-20230510 gcc
sparc64 randconfig-r012-20230510 gcc
sparc64 randconfig-r032-20230509 gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-a001 clang
x86_64 randconfig-a002 gcc
x86_64 randconfig-a003 clang
x86_64 randconfig-a004 gcc
x86_64 randconfig-a005 clang
x86_64 randconfig-a006 gcc
x86_64 randconfig-a011 gcc
x86_64 randconfig-a012 clang
x86_64 randconfig-a013 gcc
x86_64 randconfig-a014 clang
x86_64 randconfig-a015 gcc
x86_64 randconfig-a016 clang
x86_64 randconfig-k001 clang
x86_64 rhel-8.3 gcc
xtensa buildonly-randconfig-r001-20230509 gcc
xtensa buildonly-randconfig-r005-20230510 gcc
xtensa common_defconfig gcc
xtensa randconfig-r002-20230509 gcc
xtensa randconfig-r004-20230509 gcc
xtensa randconfig-r032-20230511 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: gpio: Convert STMPE GPIO to YAML schema
From: Bartosz Golaszewski @ 2023-05-11 14:58 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, Krzysztof Kozlowski, Maxime Coquelin,
Alexandre Torgue, Dmitry Torokhov, Lee Jones, Philippe Schenker,
Stefan Agner, Marek Vasut, Steffen Trumtrar, linux-gpio,
devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
linux-input
In-Reply-To: <20230426-stmpe-dt-bindings-v4-1-36fdd53d9919@linaro.org>
On Mon, May 8, 2023 at 2:35 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> This rewrites the STMPE GPIO bindings to a YAML schema.
>
> We add the properties that are used in the widely used
> STMPE GPIO device nodes found in the wild, most notably
> interrupt support, so interrupt-cells and
> interrupt-controller is now part of the bindings.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v3->v4:
> - Realize that there were actually some old bindings so we
> need to get rid of them as part of this patch.
> - Fix blank lines after description: keyword.
> ChangeLog v2->v3:
> - Use a compact hog node schema backed by the standard hog
> schema.
> ChangeLog v1->v2:
> - New patch split off from the MFD patch.
> ---
> .../devicetree/bindings/gpio/gpio-stmpe.txt | 17 -------
> .../devicetree/bindings/gpio/st,stmpe-gpio.yaml | 53 ++++++++++++++++++++++
> 2 files changed, 53 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt
> deleted file mode 100644
> index b33f8f02c0d7..000000000000
> --- a/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -STMPE gpio
> -----------
> -
> -Required properties:
> - - compatible: "st,stmpe-gpio"
> -
> -Optional properties:
> - - st,norequest-mask: bitmask specifying which GPIOs should _not_ be requestable
> - due to different usage (e.g. touch, keypad)
> -
> -Node should be child node of stmpe node to which it belongs.
> -
> -Example:
> - stmpe_gpio {
> - compatible = "st,stmpe-gpio";
> - st,norequest-mask = <0x20>; //gpio 5 can't be used
> - };
> diff --git a/Documentation/devicetree/bindings/gpio/st,stmpe-gpio.yaml b/Documentation/devicetree/bindings/gpio/st,stmpe-gpio.yaml
> new file mode 100644
> index 000000000000..22c0cae73425
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/st,stmpe-gpio.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/st,stmpe-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: STMicroelectonics Port Expander (STMPE) GPIO Block
> +
> +description:
> + STMicroelectronics Port Expander (STMPE) is a series of slow
> + bus controllers for various expanded peripherals such as GPIO, keypad,
> + touchscreen, ADC, PWM or rotator. It can contain one or several different
> + peripherals connected to SPI or I2C. These bindings pertain to the
> + GPIO portions of these expanders.
> +
> +maintainers:
> + - Linus Walleij <linus.walleij@linaro.org>
> +
> +properties:
> + compatible:
> + const: st,stmpe-gpio
> +
> + "#gpio-cells":
> + const: 2
> +
> + "#interrupt-cells":
> + const: 2
> +
> + gpio-controller: true
> +
> + interrupt-controller: true
> +
> + st,norequest-mask:
> + description:
> + A bitmask of GPIO lines that cannot be requested because for
> + for example not being connected to anything on the system
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> +patternProperties:
> + "^.+-hog(-[0-9]+)?$":
> + type: object
> +
> + required:
> + - gpio-hog
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - "#gpio-cells"
> + - "#interrupt-cells"
> + - gpio-controller
> + - interrupt-controller
>
> --
> 2.34.1
>
Applied, thanks!
Bart
^ permalink raw reply
* Re: [regression] Since kernel 6.3.1 logitech unify receiver not working properly
From: Thorsten Leemhuis @ 2023-05-11 11:58 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: open list:HID CORE LAYER, LKML, Linux kernel regressions list,
guy.b
In-Reply-To: <9b987585-0834-bb8c-3414-283c29f3f2ab@leemhuis.info>
Hi, Thorsten here, the Linux kernel's regression tracker.
On 08.05.23 11:55, Linux regression tracking (Thorsten Leemhuis) wrote:
> Hi, Thorsten here, the Linux kernel's regression tracker.
>
> I noticed a regression report in bugzilla.kernel.org. As many (most?)
> kernel developers don't keep an eye on it, I decided to forward it by mail.
>
> Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=217412 :
TWIMC: a few other users (three or four iirc) showed up in that ticket
and reported problems with the receiver, albeit the symptoms are not
exactly the same for all of them, so there might be more than one problem.
I'll try to motivate the affected users to perform a bisection. But
would be great if those with more knowledge about this code could
briefly look into the ticket, maybe the details the users shared allows
one of you to guess what causes this.
Ciao, Thorsten
>> guy.b 2023-05-07 07:37:34 UTC
>>
>> Hello,
>>
>> Since kernel 6.3.1 the boot process hangs (~ 5 seconds) by uevent triggering with the following errors :
>>
>> logitech-hidpp-device 0003:046D:405E.0004: hidpp_devicenametype_get_count: received protocol error 0x07
>>
>>
>> The logs about logitech input:
>>
>> usb 1-8: new full-speed USB device number 2 using xhci_hcd
>> mai 06 11:54:24 Cockpit kernel: usb 1-8: New USB device found, idVendor=046d, idProduct=c52b, bcdDevice=24.10
>> mai 06 11:54:24 Cockpit kernel: usb 1-8: New USB device strings: Mfr=1, Product=2, SerialNumber=0
>> mai 06 11:54:24 Cockpit kernel: usb 1-8: Product: USB Receiver
>> mai 06 11:54:24 Cockpit kernel: usb 1-8: Manufacturer: Logitech
>> mai 06 11:54:24 Cockpit kernel: input: Logitech USB Receiver as /devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.0/0003:046D:C52B.0001/input/input4
>> mai 06 11:54:24 Cockpit kernel: hid-generic 0003:046D:C52B.0001: input,hidraw0: USB HID v1.11 Keyboard [Logitech USB Receiver] on usb-0000:00:14.0-8/input0
>> mai 06 11:54:24 Cockpit kernel: input: Logitech USB Receiver Mouse as /devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.1/0003:046D:C52B.0002/input/input5
>> mai 06 11:54:24 Cockpit kernel: input: Logitech USB Receiver Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.1/0003:046D:C52B.0002/input/input6
>> mai 06 11:54:24 Cockpit kernel: input: Logitech USB Receiver System Control as /devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.1/0003:046D:C52B.0002/input/input7
>> mai 06 11:54:24 Cockpit kernel: hid-generic 0003:046D:C52B.0002: input,hiddev96,hidraw1: USB HID v1.11 Mouse [Logitech USB Receiver] on usb-0000:00:14.0-8/input1
>> mai 06 11:54:24 Cockpit kernel: hid-generic 0003:046D:C52B.0003: hiddev97,hidraw2: USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-8/input2
>> mai 06 11:54:24 Cockpit kernel: usbcore: registered new interface driver usbhid
>> mai 06 11:54:24 Cockpit kernel: usbhid: USB HID core driver
>> mai 06 11:54:24 Cockpit kernel: logitech-djreceiver 0003:046D:C52B.0003: hiddev96,hidraw0: USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-8/input2
>> mai 06 11:54:24 Cockpit kernel: input: Logitech Wireless Device PID:405e Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.2/0003:046D:C52B.0003/0003:046D:405E.0004/input/input9
>> mai 06 11:54:24 Cockpit kernel: input: Logitech Wireless Device PID:405e Mouse as /devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.2/0003:046D:C52B.0003/0003:046D:405E.0004/input/input10
>> mai 06 11:54:24 Cockpit kernel: hid-generic 0003:046D:405E.0004: input,hidraw1: USB HID v1.11 Keyboard [Logitech Wireless Device PID:405e] on usb-0000:00:14.0-8/input2:1
>> mai 06 11:54:24 Cockpit kernel: input: Logitech Wireless Device PID:2010 Keyboard as /devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.2/0003:046D:C52B.0003/0003:046D:2010.0005/input/input14
>> mai 06 11:54:24 Cockpit kernel: hid-generic 0003:046D:2010.0005: input,hidraw2: USB HID v1.11 Keyboard [Logitech Wireless Device PID:2010] on usb-0000:00:14.0-8/input2:2
>> mai 06 11:54:24 Cockpit kernel: logitech-hidpp-device 0003:046D:405E.0004: HID++ 4.5 device connected.
>> mai 06 11:54:24 Cockpit kernel: logitech-hidpp-device 0003:046D:405E.0004: hidpp_devicenametype_get_count: received protocol error 0x07
>> mai 06 11:54:24 Cockpit kernel: input: Logitech Wireless Device PID:405e as /devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.2/0003:046D:C52B.0003/0003:046D:405E.0004/input/input18
>> mai 06 11:54:24 Cockpit kernel: logitech-hidpp-device 0003:046D:405E.0004: input,hidraw1: USB HID v1.11 Keyboard [Logitech Wireless Device PID:405e] on usb-0000:00:14.0-8/input2:1
>> mai 06 11:54:24 Cockpit kernel: input: Logitech Wireless Device PID:2010 as /devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.2/0003:046D:C52B.0003/0003:046D:2010.0005/input/input19
>> mai 06 11:54:24 Cockpit kernel: logitech-hidpp-device 0003:046D:2010.0005: input,hidraw2: USB HID v1.11 Keyboard [Logitech Wireless Device PID:2010] on usb-0000:00:14.0-8/input2:2
>>
>> Next, once booted and remove the unify receiver and plug it again there is a massive lag (~ 15 seconds) before that the receiver get ready for the mouse and keyboard to be functional with following errors :
>>
>> kernel: logitech-hidpp-device 0003:046D:405E.0022: hidpp_devicenametype_get_count: received protocol error 0x07
>> kernel: logitech-hidpp-device 0003:046D:405E.0023: Couldn't get wheel multiplier (error -110)
>>
>> Unify receiver with K800 keyboard and M720 Triathlon mouse paired.
>>
>> This happens on my desktop computer but not on my laptop with a unify receiver and a marathon M705 mouse.
>>
>> Both computer are on Archlinux and up to date.
>>
>> On the desktop the boot is fine without the unify receiver.
>>
>> Let me know if you need more info.
>>
>> Thank you.
>
>
> See the ticket for more details.
>
> Note, there are two users affected by this (see Comment 5 for the
> second), but you have to use bugzilla to reach the second reporter, as I
> sadly[1] can not simply CCed them in mails like this (the initial
> reporter gave permission).
>
>
> [TLDR for the rest of this mail: I'm adding this report to the list of
> tracked Linux kernel regressions; the text you find below is based on a
> few templates paragraphs you might have encountered already in similar
> form.]
>
> BTW, let me use this mail to also add the report to the list of tracked
> regressions to ensure it's doesn't fall through the cracks:
>
> #regzbot introduced: v6.2..v6.3
> https://bugzilla.kernel.org/show_bug.cgi?id=217412
> #regzbot title: input: hid: logitech unify receiver not working properly
> #regzbot ignore-activity
>
> This isn't a regression? This issue or a fix for it are already
> discussed somewhere else? It was fixed already? You want to clarify when
> the regression started to happen? Or point out I got the title or
> something else totally wrong? Then just reply and tell me -- ideally
> while also telling regzbot about it, as explained by the page listed in
> the footer of this mail.
>
> Developers: When fixing the issue, remember to add 'Link:' tags pointing
> to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
> this thread sees some discussion). See page linked in footer for details.
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> [1] because bugzilla.kernel.org tells users upon registration their
> "email address will never be displayed to logged out users"
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Javier Carrasco @ 2023-05-11 10:28 UTC (permalink / raw)
To: Krzysztof Kozlowski, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, Bastian Hecht,
Michael Riesch
Cc: linux-kernel, linux-input, devicetree
In-Reply-To: <280ab18d-bbfa-9920-5f1b-d069fd866e1f@linaro.org>
On 11.05.23 11:45, Krzysztof Kozlowski wrote:
> On 10/05/2023 15:50, Javier Carrasco wrote:
>> The virtual-touchscreen object defines an area within the touchscreen
>> where touch events are reported and their coordinates get converted to
>> the virtual origin. This object avoids getting events from areas that
>> are physically hidden by overlay frames.
>>
>> For touchscreens where overlay buttons on the touchscreen surface are
>> provided, the virtual-buttons object contains a node for every button
>> and the key event that should be reported when pressed.
>
> Hm, I don't understand - are these separate physical buttons? If so, why
> would they be part of touchscreen? Where do the wires go?
Those buttons are actually printed on some overlays which are mounted on
top of the touchscreen and they are used to provide a predefined
functionality to that touchscreen region. Any touchscreen with such a
physical overlay with buttons printed on them or clipped touchscreen
areas might use this functionality.
These buttons are actually physical i.e. printed and with a given
functionality, but still part of the touchscreen as the physical device
is not aware of them. Therefore they only make sense in the touchscreen
context.
>
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
>> ---
>> .../bindings/input/touchscreen/touchscreen.yaml | 54 ++++++++++++++++++++++
>> 1 file changed, 54 insertions(+)
>>
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* [RESEND PATCH] dt-bindings: input: cypress,cyapa: convert to dtschema
From: Krzysztof Kozlowski @ 2023-05-11 10:25 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree, linux-kernel
Cc: Krzysztof Kozlowski, Rob Herring
Convert the Cypress All Points Addressable (APA) I2C Touchpad / Trackpad
bindings to DT schema.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../bindings/input/cypress,cyapa.txt | 42 ----------------
.../bindings/input/cypress,cyapa.yaml | 49 +++++++++++++++++++
2 files changed, 49 insertions(+), 42 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/cypress,cyapa.txt
create mode 100644 Documentation/devicetree/bindings/input/cypress,cyapa.yaml
diff --git a/Documentation/devicetree/bindings/input/cypress,cyapa.txt b/Documentation/devicetree/bindings/input/cypress,cyapa.txt
deleted file mode 100644
index d3db65916a36..000000000000
--- a/Documentation/devicetree/bindings/input/cypress,cyapa.txt
+++ /dev/null
@@ -1,42 +0,0 @@
-Cypress I2C Touchpad
-
-Required properties:
-- compatible: must be "cypress,cyapa".
-- reg: I2C address of the chip.
-- interrupts: interrupt to which the chip is connected (see interrupt
- binding[0]).
-
-Optional properties:
-- wakeup-source: touchpad can be used as a wakeup source.
-- pinctrl-names: should be "default" (see pinctrl binding [1]).
-- pinctrl-0: a phandle pointing to the pin settings for the device (see
- pinctrl binding [1]).
-- vcc-supply: a phandle for the regulator supplying 3.3V power.
-
-[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
-[1]: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
-
-Example:
- &i2c0 {
- /* ... */
-
- /* Cypress Gen3 touchpad */
- touchpad@67 {
- compatible = "cypress,cyapa";
- reg = <0x67>;
- interrupt-parent = <&gpio>;
- interrupts = <2 IRQ_TYPE_EDGE_FALLING>; /* GPIO 2 */
- wakeup-source;
- };
-
- /* Cypress Gen5 and later touchpad */
- touchpad@24 {
- compatible = "cypress,cyapa";
- reg = <0x24>;
- interrupt-parent = <&gpio>;
- interrupts = <2 IRQ_TYPE_EDGE_FALLING>; /* GPIO 2 */
- wakeup-source;
- };
-
- /* ... */
- };
diff --git a/Documentation/devicetree/bindings/input/cypress,cyapa.yaml b/Documentation/devicetree/bindings/input/cypress,cyapa.yaml
new file mode 100644
index 000000000000..29515151abe9
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cypress,cyapa.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cypress,cyapa.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cypress All Points Addressable (APA) I2C Touchpad / Trackpad
+
+maintainers:
+ - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
+
+properties:
+ compatible:
+ const: cypress,cyapa
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ wakeup-source: true
+
+ vcc-supply:
+ description: 3.3V power
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ trackpad@67 {
+ reg = <0x67>;
+ compatible = "cypress,cyapa";
+ interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpx1>;
+ wakeup-source;
+ };
+ };
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Krzysztof Kozlowski @ 2023-05-11 9:45 UTC (permalink / raw)
To: Javier Carrasco, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, Bastian Hecht,
Michael Riesch
Cc: linux-kernel, linux-input, devicetree
In-Reply-To: <20230510-feature-ts_virtobj_patch-v1-2-5ae5e81bc264@wolfvision.net>
On 10/05/2023 15:50, Javier Carrasco wrote:
> The virtual-touchscreen object defines an area within the touchscreen
> where touch events are reported and their coordinates get converted to
> the virtual origin. This object avoids getting events from areas that
> are physically hidden by overlay frames.
>
> For touchscreens where overlay buttons on the touchscreen surface are
> provided, the virtual-buttons object contains a node for every button
> and the key event that should be reported when pressed.
Hm, I don't understand - are these separate physical buttons? If so, why
would they be part of touchscreen? Where do the wires go?
>
> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
> ---
> .../bindings/input/touchscreen/touchscreen.yaml | 54 ++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj
From: Javier Carrasco @ 2023-05-11 6:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bastian Hecht, Michael Riesch, Rob Herring, Conor Dooley,
linux-input, Krzysztof Kozlowski, Henrik Rydberg, devicetree,
linux-kernel, Dmitry Torokhov
In-Reply-To: <20230510145911.66jxevntvzrghoct@krzk-bin>
On 10.05.23 16:59, Krzysztof Kozlowski wrote:
> On Wed, 10 May 2023 15:50:49 +0200, Javier Carrasco wrote:
>> The st1232 driver supports the virtual-touchscreen and virtual-buttons
>> objects defined in the generic touchscreen bindings and implemented in
>> the ts-virtobj module. Add an example where nodes for a virtual
>> touchscreen and virtual buttons are defined.
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
>> ---
>> .../input/touchscreen/sitronix,st1232.yaml | 40 ++++++++++++++++++++++
>> 1 file changed, 40 insertions(+)
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.example.dtb: touchscreen@55: Unevaluated properties are not allowed ('virtual-buttons', 'virtual-touchscreen' were unexpected)
> From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
>
Thanks for your feedback.
This patch depends on: 'PATCH 2/4: dt-bindings: touchscreen: add
virtual-touchscreen and virtual-buttons properties' from the same
series, where 'virtual-buttons' and 'virtual-touchscreen' are defined in
'touchscreen.yaml'.
I could only reproduce the error after reverting the patch it depends
on. Otherwise it passes the tests successfully and with no warning
reports with dt-schema 04.2023 and yamllint 1.31.0
> See https://patchwork.ozlabs.org/patch/1779521
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
^ permalink raw reply
* Re: [PATCH] Input: adxl34x - do not hardcode interrupt trigger type
From: Dmitry Torokhov @ 2023-05-11 0:28 UTC (permalink / raw)
To: Marek Vasut; +Cc: linux-input, Michael Hennerich
In-Reply-To: <20230509203555.549158-1-marex@denx.de>
On Tue, May 09, 2023 at 10:35:55PM +0200, Marek Vasut wrote:
> Instead of hardcoding IRQ trigger type to IRQF_TRIGGER_HIGH, let's
> respect the settings specified in the firmware description.
>
> Fixes: e27c729219ad2 ("Input: add driver for ADXL345/346 Digital Accelerometers")
> Signed-off-by: Marek Vasut <marex@denx.de>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: google: Don't use devm for hid_hw_stop()
From: Dmitry Torokhov @ 2023-05-11 0:25 UTC (permalink / raw)
To: Stephen Boyd
Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, patches,
linux-input
In-Reply-To: <CAE-0n52bv1-VaQikOV6hdFmuRyPBX6YV7MT=2+xrpReJecrgbQ@mail.gmail.com>
On Wed, May 10, 2023 at 01:50:01PM -0700, Stephen Boyd wrote:
> Quoting Dmitry Torokhov (2023-05-10 13:24:08)
> > On Wed, May 10, 2023 at 11:51:31AM -0700, Stephen Boyd wrote:
> > > Quoting Dmitry Torokhov (2023-05-05 17:06:07)
> > > > On Fri, May 05, 2023 at 04:24:16PM -0700, Stephen Boyd wrote:
> > > > >
> > > > ...
> > > > > Unfortunately, the hid google hammer driver hand rolls a devm function
> > > > > to call hid_hw_stop() when the driver is unbound and implements an
> > > > > hid_driver::remove() function. The driver core doesn't call the devm
> > > > > release functions until _after_ the bus unbinds the driver, so the order
> > > > > of operations is like this:
> > > >
> > > > Excellent analysis, but the problem is not limited to the hammer driver
> > > > (potentially) and shalt be dealt with appropriately, at the HID bus
> > > > level.
> > >
> > > Thanks. I thought of the bus level approach as well, but I was trying to
> > > keep the fix isolated to the driver that had the problem. I'd like to
> > > get the fix into the stable kernel, as this fixes a regression
> > > introduced by commit d950db3f80a8 ("HID: google: switch to devm when
> > > registering keyboard backlight LED") in v5.18.
> > >
> > > Is the bus level approach going to be acceptable as a stable backport?
> >
> > Sure, why not given the kind of stuff flowing into stable kernels. At
> > least this would be fixing real issue that can be triggered with a real
> > device.
>
> Hmm, ok. I was worried it would be too much "new code" vs. fixing
> something.
>
> > >
> > > This got me thinking that maybe both of these approaches are wrong.
> > > Maybe the call to hid_close_report() should be removed from
> > > hid_device_remove() instead.
> > >
> > > The device is being removed from the bus when hid_device_remove() is
> > > called, but it hasn't been released yet. Other devices like the hidinput
> > > device are referencing the hdev device because they set the hdev as
> > > their parent. Basically, child devices are still bound to some sort of
> > > driver or subsystem when the parent hdev is unbound from its driver,
> > > leading to a state where the child drivers could still access the hdev
> > > while it is being destroyed. If we remove the hid_close_report() call
> > > from this function it will eventually be called by hid_device_release()
> > > when the last reference to the device is dropped, i.e. when the child
> > > devices all get destroyed. In the case of hid-google-hammer, that would
> > > be when hid_hw_stop() is called from the devm release function by driver
> > > core.
> > >
> > > The benefit of this approach is that we don't allocate a devres group
> > > for all the hid devices when only two drivers need it. The possible
> > > downside is that we keep the report around while the device exists but
> > > has no driver bound to it.
> > >
> > > Here's a totally untested patch for that.
> > >
> > > ---8<----
> > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > > index 22623eb4f72f..93905e200cae 100644
> > > --- a/drivers/hid/hid-core.c
> > > +++ b/drivers/hid/hid-core.c
> > > @@ -1211,8 +1211,8 @@ int hid_open_report(struct hid_device *device)
> > > hid_parser_reserved
> > > };
> > >
> > > - if (WARN_ON(device->status & HID_STAT_PARSED))
> > > - return -EBUSY;
> > > + if (device->status & HID_STAT_PARSED)
> > > + hid_close_report(device);
> > >
> > > start = device->dev_rdesc;
> > > if (WARN_ON(!start))
> > > @@ -2662,7 +2662,6 @@ static void hid_device_remove(struct device *dev)
> > > hdrv->remove(hdev);
> > > else /* default remove */
> > > hid_hw_stop(hdev);
> > > - hid_close_report(hdev);
> > > hdev->driver = NULL;
> > > }
> >
> > This will probably work, but it I consider this still being fragile as
> > at some point we might want to add some more unwinding, and we'll run
> > into this issue again. I would feel much safer if the order of release
> > followed (inversely) order of allocations more closely.
> >
>
> Sorry, I'm not following here. How is it fragile? Are you saying that if
> we want to add devm calls into the bus layer itself the order of release
> won't be inverse of allocation/creation?
What I was trying to say is that later someone else might be tempted to
add more traditional-style resources and non-devm-unwinding for them.
Having an explicit devres groups gives exact point when driver-allocated resources
are released, and makes patch authors take it into consideration.
If everything is devm-controlled then we do not need a separate devres
group.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: google: Don't use devm for hid_hw_stop()
From: Stephen Boyd @ 2023-05-10 20:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, patches,
linux-input
In-Reply-To: <ZFv9aKZlZbfK1cVr@google.com>
Quoting Dmitry Torokhov (2023-05-10 13:24:08)
> On Wed, May 10, 2023 at 11:51:31AM -0700, Stephen Boyd wrote:
> > Quoting Dmitry Torokhov (2023-05-05 17:06:07)
> > > On Fri, May 05, 2023 at 04:24:16PM -0700, Stephen Boyd wrote:
> > > >
> > > ...
> > > > Unfortunately, the hid google hammer driver hand rolls a devm function
> > > > to call hid_hw_stop() when the driver is unbound and implements an
> > > > hid_driver::remove() function. The driver core doesn't call the devm
> > > > release functions until _after_ the bus unbinds the driver, so the order
> > > > of operations is like this:
> > >
> > > Excellent analysis, but the problem is not limited to the hammer driver
> > > (potentially) and shalt be dealt with appropriately, at the HID bus
> > > level.
> >
> > Thanks. I thought of the bus level approach as well, but I was trying to
> > keep the fix isolated to the driver that had the problem. I'd like to
> > get the fix into the stable kernel, as this fixes a regression
> > introduced by commit d950db3f80a8 ("HID: google: switch to devm when
> > registering keyboard backlight LED") in v5.18.
> >
> > Is the bus level approach going to be acceptable as a stable backport?
>
> Sure, why not given the kind of stuff flowing into stable kernels. At
> least this would be fixing real issue that can be triggered with a real
> device.
Hmm, ok. I was worried it would be too much "new code" vs. fixing
something.
> >
> > This got me thinking that maybe both of these approaches are wrong.
> > Maybe the call to hid_close_report() should be removed from
> > hid_device_remove() instead.
> >
> > The device is being removed from the bus when hid_device_remove() is
> > called, but it hasn't been released yet. Other devices like the hidinput
> > device are referencing the hdev device because they set the hdev as
> > their parent. Basically, child devices are still bound to some sort of
> > driver or subsystem when the parent hdev is unbound from its driver,
> > leading to a state where the child drivers could still access the hdev
> > while it is being destroyed. If we remove the hid_close_report() call
> > from this function it will eventually be called by hid_device_release()
> > when the last reference to the device is dropped, i.e. when the child
> > devices all get destroyed. In the case of hid-google-hammer, that would
> > be when hid_hw_stop() is called from the devm release function by driver
> > core.
> >
> > The benefit of this approach is that we don't allocate a devres group
> > for all the hid devices when only two drivers need it. The possible
> > downside is that we keep the report around while the device exists but
> > has no driver bound to it.
> >
> > Here's a totally untested patch for that.
> >
> > ---8<----
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > index 22623eb4f72f..93905e200cae 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -1211,8 +1211,8 @@ int hid_open_report(struct hid_device *device)
> > hid_parser_reserved
> > };
> >
> > - if (WARN_ON(device->status & HID_STAT_PARSED))
> > - return -EBUSY;
> > + if (device->status & HID_STAT_PARSED)
> > + hid_close_report(device);
> >
> > start = device->dev_rdesc;
> > if (WARN_ON(!start))
> > @@ -2662,7 +2662,6 @@ static void hid_device_remove(struct device *dev)
> > hdrv->remove(hdev);
> > else /* default remove */
> > hid_hw_stop(hdev);
> > - hid_close_report(hdev);
> > hdev->driver = NULL;
> > }
>
> This will probably work, but it I consider this still being fragile as
> at some point we might want to add some more unwinding, and we'll run
> into this issue again. I would feel much safer if the order of release
> followed (inversely) order of allocations more closely.
>
Sorry, I'm not following here. How is it fragile? Are you saying that if
we want to add devm calls into the bus layer itself the order of release
won't be inverse of allocation/creation?
^ permalink raw reply
* Re: [PATCH] HID: google: Don't use devm for hid_hw_stop()
From: Dmitry Torokhov @ 2023-05-10 20:24 UTC (permalink / raw)
To: Stephen Boyd
Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, patches,
linux-input
In-Reply-To: <CAE-0n521MhmdWjEb0-xwnPLQz07bMCGyXokZ3L87azYcw6_C_Q@mail.gmail.com>
On Wed, May 10, 2023 at 11:51:31AM -0700, Stephen Boyd wrote:
> Quoting Dmitry Torokhov (2023-05-05 17:06:07)
> > On Fri, May 05, 2023 at 04:24:16PM -0700, Stephen Boyd wrote:
> > >
> > ...
> > > Unfortunately, the hid google hammer driver hand rolls a devm function
> > > to call hid_hw_stop() when the driver is unbound and implements an
> > > hid_driver::remove() function. The driver core doesn't call the devm
> > > release functions until _after_ the bus unbinds the driver, so the order
> > > of operations is like this:
> >
> > Excellent analysis, but the problem is not limited to the hammer driver
> > (potentially) and shalt be dealt with appropriately, at the HID bus
> > level.
>
> Thanks. I thought of the bus level approach as well, but I was trying to
> keep the fix isolated to the driver that had the problem. I'd like to
> get the fix into the stable kernel, as this fixes a regression
> introduced by commit d950db3f80a8 ("HID: google: switch to devm when
> registering keyboard backlight LED") in v5.18.
>
> Is the bus level approach going to be acceptable as a stable backport?
Sure, why not given the kind of stuff flowing into stable kernels. At
least this would be fixing real issue that can be triggered with a real
device.
>
> Is it a problem to call hid_hw_stop() directly? I suppose for the
> hid-google-hammer driver we don't want to leave the led sysfs node
> hanging around after the hid_hw_stop() function has been called either,
> so some sort of forced ejection of the devm led device is needed and the
> bus level approach helps there.
>
> I was curious to see if anything else had this problem so I did this
> poor grep to find code that isn't calling hid_hw_stop() from probe or
> remove:
>
> git grep -W hid_hw_stop | grep .c= | grep -v probe | grep -v remove
>
> and I got this list (minus hid core which doesn't matter):
>
> drivers/hid/hid-google-hammer.c=static void hammer_stop(void *hdev)
> drivers/hid/hid-mcp2221.c=static void mcp2221_hid_unregister(void *ptr)
> drivers/hid/hid-wiimote-core.c=static void wiimote_destroy(struct
> wiimote_data *wdata)
> drivers/hid/wacom_sys.c=static int wacom_parse_and_register(struct
> wacom *wacom, bool wireless)
> drivers/hid/wacom_sys.c=static void wacom_wireless_work(struct
> work_struct *work)
> drivers/hid/wacom_sys.c=static void wacom_mode_change_work(struct
> work_struct *work)
>
> The wacom_sys.c ones look OK because they're during workqueues that are
> probably flushed, and wiimote_destroy() is called from an error path or
> driver remove, so it is also OK. But mcp2221_hid_unregister() has the
> same problem.
>
> If you look at drivers/hid/hid-mcp2221.c you'll see this comment above
> mcp2221_remove() too:
>
> /* This is needed to be sure hid_hw_stop() isn't called twice by the
> subsystem */
> static void mcp2221_remove(struct hid_device *hdev)
>
> which is kinda weird. Why can't we have a devm_hid_hw_start() API that
> tells the hid bus to not call hid_hw_stop() at all in
> hid_device_remove()? That would allow us to avoid this pitfall where
> everything is moved to devm and the driver has no remove function at all
> and we forget to populate an empty one. Instead, the bus layer can know
> that hardware will be stopped with devm later.
So yes, this is another option: all bus code should exclusively use
devm* API and can not use non-managed resources. This for HID includes
disconnecting hiddev, hidraw and hidinput handlers/drivers.
FTR, I think having devm_hid_hw_start() would be nice.
>
> >
> > Actually, it is not even limited to HID, but exists in most buses with
> > non-trivial ->remove() implementation. For example I fixed similar issue
> > in I2C in 5b5475826c52 ("i2c: ensure timely release of driver-allocated
> > resources"). I tried fixing it in SPI but Mark has some objections, and
> > wanted to fix it in the driver core, so I was thinking about it and then
> > dropped the ball. At this time I do not think fixing it at driver core
> > makes logic any clearer, so I think we just need to fix a handful of
> > buses.
>
> Do you have a link to that discussion?
https://lore.kernel.org/lkml/YFf2RD931nq3RudJ@google.com/
>
> -------
>
> This got me thinking that maybe both of these approaches are wrong.
> Maybe the call to hid_close_report() should be removed from
> hid_device_remove() instead.
>
> The device is being removed from the bus when hid_device_remove() is
> called, but it hasn't been released yet. Other devices like the hidinput
> device are referencing the hdev device because they set the hdev as
> their parent. Basically, child devices are still bound to some sort of
> driver or subsystem when the parent hdev is unbound from its driver,
> leading to a state where the child drivers could still access the hdev
> while it is being destroyed. If we remove the hid_close_report() call
> from this function it will eventually be called by hid_device_release()
> when the last reference to the device is dropped, i.e. when the child
> devices all get destroyed. In the case of hid-google-hammer, that would
> be when hid_hw_stop() is called from the devm release function by driver
> core.
>
> The benefit of this approach is that we don't allocate a devres group
> for all the hid devices when only two drivers need it. The possible
> downside is that we keep the report around while the device exists but
> has no driver bound to it.
>
> Here's a totally untested patch for that.
>
> ---8<----
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 22623eb4f72f..93905e200cae 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1211,8 +1211,8 @@ int hid_open_report(struct hid_device *device)
> hid_parser_reserved
> };
>
> - if (WARN_ON(device->status & HID_STAT_PARSED))
> - return -EBUSY;
> + if (device->status & HID_STAT_PARSED)
> + hid_close_report(device);
>
> start = device->dev_rdesc;
> if (WARN_ON(!start))
> @@ -2662,7 +2662,6 @@ static void hid_device_remove(struct device *dev)
> hdrv->remove(hdev);
> else /* default remove */
> hid_hw_stop(hdev);
> - hid_close_report(hdev);
> hdev->driver = NULL;
> }
This will probably work, but it I consider this still being fragile as
at some point we might want to add some more unwinding, and we'll run
into this issue again. I would feel much safer if the order of release
followed (inversely) order of allocations more closely.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: synaptics-rmi4 - retry reading SMBus version on resume
From: Dmitry Torokhov @ 2023-05-10 20:06 UTC (permalink / raw)
To: Jeffery Miller
Cc: Andrew Duggan, Jonathan Denose, jdenose, Lyude Paul, loic.poulain,
benjamin.tissoires, Andrew Duggan, Jonathan Cameron,
Maximilian Luz, Miguel Ojeda, Uwe Kleine-König, linux-input,
linux-kernel
In-Reply-To: <20230510192731.300786-1-jefferymiller@google.com>
On Wed, May 10, 2023 at 07:27:22PM +0000, Jeffery Miller wrote:
> On resume there can be a period of time after the
> preceding serio_resume -> psmouse_deactivate call
> where calls to rmi_smb_get_version fail with
> -ENXIO.
>
> The call path in rmi_smb_resume is rmi_smb_resume -> rmi_smb_reset ->
> rmi_smb_enable_smbus_mode -> rmi_smb_get_version where
> this failure would occur.
>
> Adding a retry loop ensures that after rmi_smb_reset returns
> the following rmi_driver_resume calls in rmi_smbus_resume can
> succeed.
>
> This behavior was seen on a Lenovo T440p machine that required
> a delay of approximately 7-12ms.
> The retry limit of 5 is chosen to be larger than
> this observed delay.
>
> With this patch the trimmed resume logs look similar to:
> ```
> psmouse serio1: PM: calling serio_resume+0x0/0x8c @ 5399, parent: i8042
> [5399] libps2:__ps2_command:316: psmouse serio1: f5 [] - 0/00000000 []
> psmouse serio1: PM: serio_resume+0x0/0x8c returned 0 after 3259 usecs
> ...
> rmi4_smbus 0-002c: PM: calling rmi_smb_resume ... @ 5454, parent: i2c-0
> ...
> [5454] i2c_i801:i801_check_post:414: i801_smbus 0000:00:1f.3: No response
> smbus_result: i2c-0 a=02c f=0000 c=fd BYTE_DATA rd res=-6
> rmi4_smbus 0-002c: failed to get SMBus version number!
> rmi4_smbus 0-002c: sleeping to retry getting the SMBus version number
> ...
> rmi4_smbus 0-002c: PM: rmi_smb_resume ... returned 0 after 21351 usecs
> ```
>
> Signed-off-by: Jeffery Miller <jefferymiller@google.com>
> ---
>
> Early boot dmesg include:
> ```
> rmi4_smbus 0-002c: registering SMbus-connected sensor
> rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: TM2722-001, fw id: 0
> ```
>
> The resume order looks correct. The `psmouse serio1` resume returns
> before the rmi_smb_resume is called showing the patch from
> https://lore.kernel.org/all/89456fcd-a113-4c82-4b10-a9bcaefac68f@google.com/
> is applied and working for that ordering.
>
> I attempted to try to rule out some interaction between the concurrent
> input resume calls for other i8042 devices.
> Adding a 7ms delay after psmouse_deactivate which is called in the
> preceding psmouse serio1 serio_resume function also allows
> this version call to succeed.
I am not really fond of adding random repeats in the code base. Andrew,
do you know if the Synaptics device needs certain delay when switching
to SMbus mode?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] Input: synaptics-rmi4 - retry reading SMBus version on resume
From: Jeffery Miller @ 2023-05-10 19:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Jonathan Denose, jdenose, Lyude Paul, loic.poulain,
benjamin.tissoires, Andrew Duggan, Jeffery Miller,
Jonathan Cameron, Maximilian Luz, Miguel Ojeda,
Uwe Kleine-König, linux-input, linux-kernel
On resume there can be a period of time after the
preceding serio_resume -> psmouse_deactivate call
where calls to rmi_smb_get_version fail with
-ENXIO.
The call path in rmi_smb_resume is rmi_smb_resume -> rmi_smb_reset ->
rmi_smb_enable_smbus_mode -> rmi_smb_get_version where
this failure would occur.
Adding a retry loop ensures that after rmi_smb_reset returns
the following rmi_driver_resume calls in rmi_smbus_resume can
succeed.
This behavior was seen on a Lenovo T440p machine that required
a delay of approximately 7-12ms.
The retry limit of 5 is chosen to be larger than
this observed delay.
With this patch the trimmed resume logs look similar to:
```
psmouse serio1: PM: calling serio_resume+0x0/0x8c @ 5399, parent: i8042
[5399] libps2:__ps2_command:316: psmouse serio1: f5 [] - 0/00000000 []
psmouse serio1: PM: serio_resume+0x0/0x8c returned 0 after 3259 usecs
...
rmi4_smbus 0-002c: PM: calling rmi_smb_resume ... @ 5454, parent: i2c-0
...
[5454] i2c_i801:i801_check_post:414: i801_smbus 0000:00:1f.3: No response
smbus_result: i2c-0 a=02c f=0000 c=fd BYTE_DATA rd res=-6
rmi4_smbus 0-002c: failed to get SMBus version number!
rmi4_smbus 0-002c: sleeping to retry getting the SMBus version number
...
rmi4_smbus 0-002c: PM: rmi_smb_resume ... returned 0 after 21351 usecs
```
Signed-off-by: Jeffery Miller <jefferymiller@google.com>
---
Early boot dmesg include:
```
rmi4_smbus 0-002c: registering SMbus-connected sensor
rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: TM2722-001, fw id: 0
```
The resume order looks correct. The `psmouse serio1` resume returns
before the rmi_smb_resume is called showing the patch from
https://lore.kernel.org/all/89456fcd-a113-4c82-4b10-a9bcaefac68f@google.com/
is applied and working for that ordering.
I attempted to try to rule out some interaction between the concurrent
input resume calls for other i8042 devices.
Adding a 7ms delay after psmouse_deactivate which is called in the
preceding psmouse serio1 serio_resume function also allows
this version call to succeed.
If the rmi_smb_probe device_disable_async_suspend patch is applied
it will also avoid this issue. However the time between
the psmouse_deactivate call for serio_resume and rmi_smb_resume
was over 60ms on my test machine. This would naturally be long
enough to avoid this particular delay.
drivers/input/rmi4/rmi_smbus.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 4bf0e1df6a4a..386e80ae141b 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -43,15 +43,26 @@ static int rmi_smb_get_version(struct rmi_smb_xport *rmi_smb)
{
struct i2c_client *client = rmi_smb->client;
int retval;
+ int tries = 0;
/* Check if for SMBus new version device by reading version byte. */
- retval = i2c_smbus_read_byte_data(client, SMB_PROTOCOL_VERSION_ADDRESS);
- if (retval < 0) {
- dev_err(&client->dev, "failed to get SMBus version number!\n");
- return retval;
- }
+ do {
+ if (tries > 0) {
+ dev_warn(&client->dev, "sleeping to retry getting the SMBus version number\n");
+ fsleep(5000);
+ }
+ retval = i2c_smbus_read_byte_data(client,
+ SMB_PROTOCOL_VERSION_ADDRESS);
+ if (retval >= 0)
+ return retval + 1;
- return retval + 1;
+ dev_err(&client->dev, "failed to get SMBus version number!\n");
+ /* On resume the read of the version can
+ * momentarily return -ENXIO.
+ * Retry to allow additional time for it to succeed.
+ */
+ } while (retval == -ENXIO && tries++ < 5);
+ return retval;
}
/* SMB block write - wrapper over ic2_smb_write_block */
--
2.40.1.606.ga4b1b128d6-goog
^ permalink raw reply related
* Re: [PATCH] HID: google: Don't use devm for hid_hw_stop()
From: Stephen Boyd @ 2023-05-10 18:51 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, patches,
linux-input
In-Reply-To: <ZFWZ785FRHDii/+5@google.com>
Quoting Dmitry Torokhov (2023-05-05 17:06:07)
> On Fri, May 05, 2023 at 04:24:16PM -0700, Stephen Boyd wrote:
> >
> ...
> > Unfortunately, the hid google hammer driver hand rolls a devm function
> > to call hid_hw_stop() when the driver is unbound and implements an
> > hid_driver::remove() function. The driver core doesn't call the devm
> > release functions until _after_ the bus unbinds the driver, so the order
> > of operations is like this:
>
> Excellent analysis, but the problem is not limited to the hammer driver
> (potentially) and shalt be dealt with appropriately, at the HID bus
> level.
Thanks. I thought of the bus level approach as well, but I was trying to
keep the fix isolated to the driver that had the problem. I'd like to
get the fix into the stable kernel, as this fixes a regression
introduced by commit d950db3f80a8 ("HID: google: switch to devm when
registering keyboard backlight LED") in v5.18.
Is the bus level approach going to be acceptable as a stable backport?
Is it a problem to call hid_hw_stop() directly? I suppose for the
hid-google-hammer driver we don't want to leave the led sysfs node
hanging around after the hid_hw_stop() function has been called either,
so some sort of forced ejection of the devm led device is needed and the
bus level approach helps there.
I was curious to see if anything else had this problem so I did this
poor grep to find code that isn't calling hid_hw_stop() from probe or
remove:
git grep -W hid_hw_stop | grep .c= | grep -v probe | grep -v remove
and I got this list (minus hid core which doesn't matter):
drivers/hid/hid-google-hammer.c=static void hammer_stop(void *hdev)
drivers/hid/hid-mcp2221.c=static void mcp2221_hid_unregister(void *ptr)
drivers/hid/hid-wiimote-core.c=static void wiimote_destroy(struct
wiimote_data *wdata)
drivers/hid/wacom_sys.c=static int wacom_parse_and_register(struct
wacom *wacom, bool wireless)
drivers/hid/wacom_sys.c=static void wacom_wireless_work(struct
work_struct *work)
drivers/hid/wacom_sys.c=static void wacom_mode_change_work(struct
work_struct *work)
The wacom_sys.c ones look OK because they're during workqueues that are
probably flushed, and wiimote_destroy() is called from an error path or
driver remove, so it is also OK. But mcp2221_hid_unregister() has the
same problem.
If you look at drivers/hid/hid-mcp2221.c you'll see this comment above
mcp2221_remove() too:
/* This is needed to be sure hid_hw_stop() isn't called twice by the
subsystem */
static void mcp2221_remove(struct hid_device *hdev)
which is kinda weird. Why can't we have a devm_hid_hw_start() API that
tells the hid bus to not call hid_hw_stop() at all in
hid_device_remove()? That would allow us to avoid this pitfall where
everything is moved to devm and the driver has no remove function at all
and we forget to populate an empty one. Instead, the bus layer can know
that hardware will be stopped with devm later.
>
> Actually, it is not even limited to HID, but exists in most buses with
> non-trivial ->remove() implementation. For example I fixed similar issue
> in I2C in 5b5475826c52 ("i2c: ensure timely release of driver-allocated
> resources"). I tried fixing it in SPI but Mark has some objections, and
> wanted to fix it in the driver core, so I was thinking about it and then
> dropped the ball. At this time I do not think fixing it at driver core
> makes logic any clearer, so I think we just need to fix a handful of
> buses.
Do you have a link to that discussion?
-------
This got me thinking that maybe both of these approaches are wrong.
Maybe the call to hid_close_report() should be removed from
hid_device_remove() instead.
The device is being removed from the bus when hid_device_remove() is
called, but it hasn't been released yet. Other devices like the hidinput
device are referencing the hdev device because they set the hdev as
their parent. Basically, child devices are still bound to some sort of
driver or subsystem when the parent hdev is unbound from its driver,
leading to a state where the child drivers could still access the hdev
while it is being destroyed. If we remove the hid_close_report() call
from this function it will eventually be called by hid_device_release()
when the last reference to the device is dropped, i.e. when the child
devices all get destroyed. In the case of hid-google-hammer, that would
be when hid_hw_stop() is called from the devm release function by driver
core.
The benefit of this approach is that we don't allocate a devres group
for all the hid devices when only two drivers need it. The possible
downside is that we keep the report around while the device exists but
has no driver bound to it.
Here's a totally untested patch for that.
---8<----
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 22623eb4f72f..93905e200cae 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1211,8 +1211,8 @@ int hid_open_report(struct hid_device *device)
hid_parser_reserved
};
- if (WARN_ON(device->status & HID_STAT_PARSED))
- return -EBUSY;
+ if (device->status & HID_STAT_PARSED)
+ hid_close_report(device);
start = device->dev_rdesc;
if (WARN_ON(!start))
@@ -2662,7 +2662,6 @@ static void hid_device_remove(struct device *dev)
hdrv->remove(hdev);
else /* default remove */
hid_hw_stop(hdev);
- hid_close_report(hdev);
hdev->driver = NULL;
}
^ permalink raw reply related
* Re: [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj
From: Krzysztof Kozlowski @ 2023-05-10 14:59 UTC (permalink / raw)
To: Javier Carrasco
Cc: Bastian Hecht, Michael Riesch, Rob Herring, Conor Dooley,
linux-input, Krzysztof Kozlowski, Henrik Rydberg, devicetree,
linux-kernel, Dmitry Torokhov
In-Reply-To: <20230510-feature-ts_virtobj_patch-v1-4-5ae5e81bc264@wolfvision.net>
On Wed, 10 May 2023 15:50:49 +0200, Javier Carrasco wrote:
> The st1232 driver supports the virtual-touchscreen and virtual-buttons
> objects defined in the generic touchscreen bindings and implemented in
> the ts-virtobj module. Add an example where nodes for a virtual
> touchscreen and virtual buttons are defined.
>
> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
> ---
> .../input/touchscreen/sitronix,st1232.yaml | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.example.dtb: touchscreen@55: Unevaluated properties are not allowed ('virtual-buttons', 'virtual-touchscreen' were unexpected)
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
See https://patchwork.ozlabs.org/patch/1779521
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply
* Re: [PATCH 1/2] Input: cs40l26: Support for CS40L26 Boosted Haptic Amplifier
From: Jeff LaBundy @ 2023-05-10 13:55 UTC (permalink / raw)
To: Charles Keepax
Cc: Fred Treven, dmitry.torokhov@gmail.com, Ben Bright,
James Ogletree, lee@kernel.org, jdelvare@suse.de, joel@jms.id.au,
cy_huang@richtek.com, rdunlap@infradead.org,
eajames@linux.ibm.com, ping.bai@nxp.com, msp@baylibre.com,
arnd@arndb.de, bartosz.golaszewski@linaro.org,
linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
patches@opensource.cirrus.com
In-Reply-To: <20230509102257.GG68926@ediswmail.ad.cirrus.com>
Hi Charles,
On Tue, May 09, 2023 at 10:22:57AM +0000, Charles Keepax wrote:
> On Sat, May 06, 2023 at 03:30:17PM -0500, Jeff LaBundy wrote:
> > On Thu, May 04, 2023 at 09:51:37PM +0000, Fred Treven wrote:
> > > >> +const struct dev_pm_ops cs40l26_pm_ops = {
> > > >> + SET_RUNTIME_PM_OPS(cs40l26_suspend, cs40l26_resume, NULL)
> > > >> + SET_SYSTEM_SLEEP_PM_OPS(cs40l26_sys_suspend, cs40l26_sys_resume)
> > > >> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(cs40l26_sys_suspend_noirq, cs40l26_sys_resume_noirq)
> > > >> +};
> > > >> +EXPORT_SYMBOL_GPL(cs40l26_pm_ops);
> > > >
> > > > Please use latest macros (e.g. DEFINE_SIMPLE_DEV_PM_OPS).
> > >
> > > When looking at these *_PM_OPS* macros that replace the deprecated versions,
> > > it is unclear to me how to maintain support for *_sys_* and
> > > *_sys_*_noirq* functions. Would these all need to be separately defined
> > > via DEFINE_SIMPLE_DEV_PM_OPS?
> > > Would the *_sys_* definitions still be defined through a struct i.e.
> > > const struct dev_pm_ops cs40l26_sys_pm_ops which is then exported as it
> > > is in my initial submission?
> > > I’m unsure how to handle these cases with the latest macros.
> >
> > I don't happen to see macros for suspend_noirq and resume_noirq, so maybe you
> > cannot use macros here after all and will instead have to fall back to tacking
> > on __maybe_unused to these callbacks to accommodate the !CONFIG_PM case.
> >
>
> Correct this device can not presently use the simple macros.
>
> > That being said, what are you ultimately trying to accomplish here with these
> > noirq variants? For example the print statement says "early resume" when in
> > fact a different callback exists for that (resume_early).
> >
> > On that note, why to disable interrupts during system suspend? I can imagine a
> > use-case where a customer ties the output of a force sensor to a CS40L26 GPIO
> > for low-latency haptic trigger, and then the CS40L26 interrupt output to the
> > SoC as a wake-up trigger. Does the part not support this use-case? I vaguely
> > seem to remember an issue with this on L25.
> >
> > Also, why is the logic inverted for the noirq variants? These are simply meant
> > to accommodate additional tasks that need a guarantee the device's interrupt
> > handler is not running (for example, clear or acknowledge a pending interrupt).
> > In case I have misunderstood the intent, please let me know.
> >
>
> This is a generic issue with devices that use PM runtime, but
> also have IRQs. The system suspend process re-enables IRQs before it
> re-enables the PM runtime. This means if your IRQ handler uses PM
> runtime and you get an IRQ in that window things don't work. The
> simplest solution is to disable IRQs across the window. Ideally
> one day this would probably get fixed in the PM core, but that is
> likely a massively non-trivial amount of work.
>
> To be clear the code allows IRQs whilst in system suspend (aka wakes)
> and whilst resumed. As the IRQ output of the chip is level based, the
> temporary disable only causes a slight delay in handling the IRQ.
ACK, thanks for the clarification. A couple suggestions for you Fred:
1. Update the print statements in the noirq variants; currently they say
late/early when in fact these are separate callbacks that occur directly
before/after the noirq callbacks (unless you meant to use late/early)?
2. Consider some comments here to highlight that this driver relies upon
PM callbacks from its interrupt handler and hence disables them briefly
throughout suspend/resume to accommodate the order of operations mentioned
above.
>
> > One last gripe, then I promise to stop bringing it up :) But the mental gymnastics
> > required to explain the no-fewer-than-six PM callbacks used here, as well as how
> > to support the !CONFIG_PM case, are in some ways additional nudges toward getting
> > rid of this massive amount of PM overhead and relying on the device's internal
> > power management as so many modern input devices now do. As a rule of thumb, if
> > you're having to jump through a lot of hoops to do simple things that others seem
> > to be doing with less work, something is wrong.
> >
>
> I am not sure there are significant issues supporting the
> !CONFIG_PM case, you need a couple __maybe_unused's. What issues
> are you expecting here? Yeah ok you get worse power consumption
> in that case, but you did turn off power management, presumably
> you were not that concerned about power consumption.
No concerns really, all I am saying is that in the decision whether to simplify
the driver and allow the DSP FW to naturally manage the device's power state, or
duplicate the logic in the driver and gain explicit control, the option for the
device to behave the same and save a few extra mW regardless of platform config
is one argument for the former (DSP FW).
>
> > In your defense, however, you are unlikely to come across many devices that do
> > not enable CONFIG_PM given this device's target application. That being said, it
> > is not unheard of for OEMs building wall-powered devices to enable CONFIG_PM but
> > inhibit system suspend using a wake_lock because of some HW bug.
> >
>
> Again remember the system vs runtime suspend here. Holding a
> system wake lock will have no effect on the runtime PM.
Good call.
>
> > Therefore, it seems a bit unfortunate that those use-cases wouldn't get to enjoy
> > the power savings this devices offers. That's just my $.02; I also understand
> > the reasons behind the current implementation and won't push you to change it.
>
> The power savings from not blocking suspend are tiny, at least
> outwith the !CONFIG_PM case. The driver is only blocking hibernate
> when it is actively talking to the device, during which time the
> device will very likely not be hibernating anyway.
>
> I think really it is up to Fred and Ben who are supporting the
> driver. If they feel the device will work reliably that way,
> I certainly won't stand in the way. But I would be keen to avoid
> a situation where all the downstream implementations (ie. most
> of the testing) use PM runtime and the upstream code is full of
> corner cases that haven't been ironed out, so I would like to
> know they are going to be moving our customers over to this new
> mode of operation if they decide to switch to it. Certainly you
> are not wrong that it would save a fair amount of code from the
> driver and make it look a lot cleaner.
Agreed; I would maintain parity with the battle-tested implementation that is out
in the wild.
>
> Thanks,
> Charles
Kind regards,
Jeff LaBundy
^ permalink raw reply
* [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Javier Carrasco @ 2023-05-10 13:50 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v1-0-5ae5e81bc264@wolfvision.net>
The virtual-touchscreen object defines an area within the touchscreen
where touch events are reported and their coordinates get converted to
the virtual origin. This object avoids getting events from areas that
are physically hidden by overlay frames.
For touchscreens where overlay buttons on the touchscreen surface are
provided, the virtual-buttons object contains a node for every button
and the key event that should be reported when pressed.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
.../bindings/input/touchscreen/touchscreen.yaml | 54 ++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
index 895592da9626..869be007eb6f 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
@@ -80,6 +80,60 @@ properties:
touchscreen-y-plate-ohms:
description: Resistance of the Y-plate in Ohms
+ virtual-touchscreen:
+ description: Clipped touchscreen area
+ type: object
+
+ properties:
+ x-origin:
+ description: horizontal origin of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-origin:
+ description: vertical origin of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ x-size:
+ description: horizontal resolution of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-size:
+ description: vertical resolution of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ virtual-buttons:
+ description: list of nodes defining the buttons on the touchscreen
+ type: object
+
+ patternProperties:
+ '^button-':
+ type: object
+ description:
+ Each button (key) is represented as a sub-node.
+
+ properties:
+ label:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: descriptive name of the button
+
+ linux,code: true
+
+ x-origin:
+ description: horizontal origin of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-origin:
+ description: vertical origin of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ x-size:
+ description: horizontal resolution of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-size:
+ description: vertical resolution of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
dependencies:
touchscreen-size-x: [ touchscreen-size-y ]
touchscreen-size-y: [ touchscreen-size-x ]
--
2.39.2
^ permalink raw reply related
* [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
From: Javier Carrasco @ 2023-05-10 13:50 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v1-0-5ae5e81bc264@wolfvision.net>
Use ts-virtobj to support overlay objects such as buttons and resized
frames defined in the device tree.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
drivers/input/touchscreen/st1232.c | 87 ++++++++++++++++++++++++++++++--------
1 file changed, 69 insertions(+), 18 deletions(-)
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index f49566dc96f8..b8139b7daa40 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -22,6 +22,7 @@
#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/input/ts-virtobj.h>
#define ST1232_TS_NAME "st1232-ts"
#define ST1633_TS_NAME "st1633-ts"
@@ -56,6 +57,8 @@ struct st1232_ts_data {
struct touchscreen_properties prop;
struct dev_pm_qos_request low_latency_req;
struct gpio_desc *reset_gpio;
+ struct input_dev *virtual_keypad;
+ struct ts_virtobj_map *map;
const struct st_chip_info *chip_info;
int read_buf_len;
u8 *read_buf;
@@ -129,10 +132,12 @@ static int st1232_ts_read_resolution(struct st1232_ts_data *ts, u16 *max_x,
static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
{
- struct input_dev *input = ts->input_dev;
+ struct input_dev *tscreen = ts->input_dev;
+ __maybe_unused struct input_dev *keypad = ts->virtual_keypad;
struct input_mt_pos pos[ST_TS_MAX_FINGERS];
u8 z[ST_TS_MAX_FINGERS];
int slots[ST_TS_MAX_FINGERS];
+ __maybe_unused bool button_pressed[ST_TS_MAX_FINGERS];
int n_contacts = 0;
int i;
@@ -143,6 +148,15 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
unsigned int x = ((buf[0] & 0x70) << 4) | buf[1];
unsigned int y = ((buf[0] & 0x07) << 8) | buf[2];
+ /* forward button presses to the keypad input device */
+ if (ts_virtobj_is_button_slot(ts->map, i) ||
+ ts_virtobj_button_press(ts->map, keypad, x, y, i)) {
+ button_pressed[i] = true;
+ continue;
+ }
+ /* Ignore events out of the virtual screen if defined */
+ if (!ts_virtobj_mt_on_touchscreen(ts->map, &x, &y))
+ continue;
touchscreen_set_mt_pos(&pos[n_contacts],
&ts->prop, x, y);
@@ -154,18 +168,25 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
}
}
- input_mt_assign_slots(input, slots, pos, n_contacts, 0);
+ input_mt_assign_slots(tscreen, slots, pos, n_contacts, 0);
for (i = 0; i < n_contacts; i++) {
- input_mt_slot(input, slots[i]);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
- input_report_abs(input, ABS_MT_POSITION_X, pos[i].x);
- input_report_abs(input, ABS_MT_POSITION_Y, pos[i].y);
+ input_mt_slot(tscreen, slots[i]);
+ input_mt_report_slot_state(tscreen, MT_TOOL_FINGER, true);
+ input_report_abs(tscreen, ABS_MT_POSITION_X, pos[i].x);
+ input_report_abs(tscreen, ABS_MT_POSITION_Y, pos[i].y);
if (ts->chip_info->have_z)
- input_report_abs(input, ABS_MT_TOUCH_MAJOR, z[i]);
+ input_report_abs(tscreen, ABS_MT_TOUCH_MAJOR, z[i]);
+ }
+ input_mt_sync_frame(tscreen);
+ input_sync(tscreen);
+
+ if (ts_virtobj_mapped_buttons(ts->map)) {
+ for (i = 0; i < ts->chip_info->max_fingers; i++)
+ if (ts_virtobj_is_button_slot(ts->map, i) &&
+ !button_pressed[i])
+ ts_virtobj_button_release(ts->map, keypad, i);
+ input_sync(keypad);
}
-
- input_mt_sync_frame(input);
- input_sync(input);
return n_contacts;
}
@@ -226,6 +247,7 @@ static int st1232_ts_probe(struct i2c_client *client)
const struct st_chip_info *match;
struct st1232_ts_data *ts;
struct input_dev *input_dev;
+ struct input_dev __maybe_unused *virtual_keypad;
u16 max_x, max_y;
int error;
@@ -292,18 +314,28 @@ static int st1232_ts_probe(struct i2c_client *client)
if (error)
return error;
- /* Read resolution from the chip */
- error = st1232_ts_read_resolution(ts, &max_x, &max_y);
- if (error) {
- dev_err(&client->dev,
- "Failed to read resolution: %d\n", error);
- return error;
- }
-
if (ts->chip_info->have_z)
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
ts->chip_info->max_area, 0, 0);
+ /* map virtual objects if defined in the device tree */
+ ts->map = ts_virtobj_map_objects(&ts->client->dev, ts->input_dev);
+ if (IS_ERR(ts->map))
+ return PTR_ERR(ts->map);
+
+ if (ts_virtobj_mapped_touchscreen(ts->map)) {
+ /* Read resolution from the virtual touchscreen if defined*/
+ ts_virtobj_get_touchscreen_abs(ts->map, &max_x, &max_y);
+ } else {
+ /* Read resolution from the chip */
+ error = st1232_ts_read_resolution(ts, &max_x, &max_y);
+ if (error) {
+ dev_err(&client->dev,
+ "Failed to read resolution: %d\n", error);
+ return error;
+ }
+ }
+
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, max_x, 0, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
@@ -335,6 +367,25 @@ static int st1232_ts_probe(struct i2c_client *client)
return error;
}
+ /* Register keypad input device if virtual buttons were defined */
+ if (ts_virtobj_mapped_buttons(ts->map)) {
+ virtual_keypad = devm_input_allocate_device(&client->dev);
+ if (!virtual_keypad)
+ return -ENOMEM;
+
+ ts->virtual_keypad = virtual_keypad;
+ virtual_keypad->name = "st1232-keypad";
+ virtual_keypad->id.bustype = BUS_I2C;
+ ts_virtobj_set_button_caps(ts->map, virtual_keypad);
+ error = input_register_device(ts->virtual_keypad);
+ if (error) {
+ dev_err(&client->dev,
+ "Unable to register %s input device\n",
+ virtual_keypad->name);
+ return error;
+ }
+ }
+
i2c_set_clientdata(client, ts);
return 0;
--
2.39.2
^ permalink raw reply related
* [PATCH 1/4] Input: ts-virtobj - Add touchsreen virtual object handling
From: Javier Carrasco @ 2023-05-10 13:50 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v1-0-5ae5e81bc264@wolfvision.net>
Some touchscreens provide mechanical overlays with different objects
like buttons or clipped touchscreen surfaces.
In order to support these objects, add a series of helper functions
to the input subsystem to transform them into virtual objects via
device tree nodes.
These virtual objects consume the raw touch events and report the
expected input events depending on the object properties.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
MAINTAINERS | 7 +
drivers/input/touchscreen/Kconfig | 9 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/ts-virtobj.c | 360 +++++++++++++++++++++++++++++++++
include/linux/input/ts-virtobj.h | 95 +++++++++
5 files changed, 472 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 7e0b87d5aa2e..296f71bcfe92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21434,6 +21434,13 @@ W: https://github.com/srcres258/linux-doc
T: git git://github.com/srcres258/linux-doc.git doc-zh-tw
F: Documentation/translations/zh_TW/
+TOUCHSCREEN VIRTUAL OBJECTS
+M: Javier Carrasco <javier.carrasco@wolfvision.net>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/input/touchscreen/ts-virtobj.c
+F: include/linux/input/ts-virtobj.h
+
TTY LAYER
M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
M: Jiri Slaby <jirislaby@kernel.org>
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 143ff43c67ae..276f6e0b914b 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1388,4 +1388,13 @@ config TOUCHSCREEN_HIMAX_HX83112B
To compile this driver as a module, choose M here: the
module will be called himax_hx83112b.
+config TOUCHSCREEN_TS_VIRTOBJ
+ tristate "Touchscreen Virtual Objects"
+ help
+ Say Y here if you are using a touchscreen driver that supports
+ printed overlays with keys or a clipped touchscreen area.
+
+ To compile this feature as a module, choose M here: the
+ module will be called ts-virtobj.
+
endif
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 159cd5136fdb..dc315d58a03b 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -117,3 +117,4 @@ obj-$(CONFIG_TOUCHSCREEN_RASPBERRYPI_FW) += raspberrypi-ts.o
obj-$(CONFIG_TOUCHSCREEN_IQS5XX) += iqs5xx.o
obj-$(CONFIG_TOUCHSCREEN_ZINITIX) += zinitix.o
obj-$(CONFIG_TOUCHSCREEN_HIMAX_HX83112B) += himax_hx83112b.o
+obj-$(CONFIG_TOUCHSCREEN_TS_VIRTOBJ) += ts-virtobj.o
diff --git a/drivers/input/touchscreen/ts-virtobj.c b/drivers/input/touchscreen/ts-virtobj.c
new file mode 100644
index 000000000000..56c137fc49a3
--- /dev/null
+++ b/drivers/input/touchscreen/ts-virtobj.c
@@ -0,0 +1,360 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Helper functions for virtual objects on touchscreens
+ *
+ * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#include <linux/property.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/module.h>
+#include <linux/input/ts-virtobj.h>
+
+#if IS_ENABLED(CONFIG_TOUCHSCREEN_TS_VIRTOBJ)
+
+enum ts_virtobj_valid_objects {
+ TOUCHSCREEN,
+ BUTTON,
+};
+
+static const char *const ts_virtobj_names[] = {
+ [TOUCHSCREEN] = "virtual-touchscreen",
+ [BUTTON] = "virtual-buttons",
+};
+
+struct ts_virtobj_shape {
+ u32 x_origin;
+ u32 y_origin;
+ u32 x_size;
+ u32 y_size;
+};
+
+struct ts_virtobj_button {
+ struct ts_virtobj_shape shape;
+ u32 key;
+ bool pressed;
+ int slot;
+};
+
+static int ts_virtobj_get_shape_properties(struct fwnode_handle *child_node,
+ struct ts_virtobj_shape *shape)
+{
+ int rc;
+
+ rc = fwnode_property_read_u32(child_node, "x-origin", &shape->x_origin);
+ if (rc < 0)
+ return rc;
+
+ rc = fwnode_property_read_u32(child_node, "y-origin", &shape->y_origin);
+ if (rc < 0)
+ return rc;
+
+ rc = fwnode_property_read_u32(child_node, "x-size", &shape->x_size);
+ if (rc < 0)
+ return rc;
+
+ rc = fwnode_property_read_u32(child_node, "y-size", &shape->y_size);
+ if (rc < 0)
+ return rc;
+
+ return 0;
+}
+
+static int ts_virtobj_get_button_properties(struct device *dev,
+ struct fwnode_handle *child_node,
+ struct ts_virtobj_button *btn)
+{
+ struct fwnode_handle *child_btn;
+ int rc;
+ int j = 0;
+
+ fwnode_for_each_child_node(child_node, child_btn) {
+ rc = ts_virtobj_get_shape_properties(child_btn, &btn[j].shape);
+ if (rc < 0)
+ goto button_prop_cleanup;
+
+ rc = fwnode_property_read_u32(child_btn, "linux,code",
+ &btn[j].key);
+ if (rc < 0)
+ goto button_prop_cleanup;
+
+ dev_info(dev, "Added button at (%u, %u), size %ux%u, code=%u\n",
+ btn[j].shape.x_origin, btn[j].shape.y_origin,
+ btn[j].shape.x_size, btn[j].shape.y_size, btn[j].key);
+ j++;
+ }
+
+ return 0;
+
+button_prop_cleanup:
+ fwnode_handle_put(child_btn);
+ return rc;
+}
+
+void ts_virtobj_set_button_caps(struct ts_virtobj_map *map,
+ struct input_dev *dev)
+{
+ int i;
+
+ for (i = 0; i < map->button_count; i++)
+ input_set_capability(dev, EV_KEY, map->buttons[i].key);
+}
+EXPORT_SYMBOL(ts_virtobj_set_button_caps);
+
+static int ts_virtobj_count_buttons(struct device *dev)
+{
+ struct fwnode_handle *child_node;
+ struct fwnode_handle *child_button;
+ int count = 0;
+
+ child_node = device_get_named_child_node(dev, ts_virtobj_names[BUTTON]);
+ if (!child_node)
+ return 0;
+
+ fwnode_for_each_child_node(child_node, child_button)
+ count++;
+ fwnode_handle_put(child_node);
+
+ return count;
+}
+
+static int ts_virtobj_map_touchscreen(struct device *dev,
+ struct ts_virtobj_map *map)
+{
+ struct fwnode_handle *child;
+ int rc = 0;
+
+ child = device_get_named_child_node(dev, ts_virtobj_names[TOUCHSCREEN]);
+ if (!child)
+ goto touchscreen_ret;
+
+ map->touchscreen =
+ devm_kzalloc(dev, sizeof(*map->touchscreen), GFP_KERNEL);
+ if (!map->touchscreen) {
+ rc = -ENOMEM;
+ goto touchscreen_handle;
+ }
+ rc = ts_virtobj_get_shape_properties(child, map->touchscreen);
+ if (rc < 0)
+ goto touchscreen_free;
+
+ map->virtual_touchscreen = true;
+ dev_info(dev, "Added virtual touchscreen at (%u, %u), size %u x %u\n",
+ map->touchscreen->x_origin, map->touchscreen->y_origin,
+ map->touchscreen->x_size, map->touchscreen->y_size);
+
+ rc = 0;
+ goto touchscreen_handle;
+
+touchscreen_free:
+ devm_kfree(dev, map->touchscreen);
+touchscreen_handle:
+ fwnode_handle_put(child);
+touchscreen_ret:
+ return rc;
+}
+
+static int ts_virtobj_map_buttons(struct device *dev,
+ struct ts_virtobj_map *map,
+ struct input_dev *input)
+{
+ struct fwnode_handle *child;
+ u32 button_count;
+ int rc = 0;
+
+ button_count = ts_virtobj_count_buttons(dev);
+ if (button_count) {
+ map->buttons = devm_kcalloc(dev, button_count,
+ sizeof(*map->buttons), GFP_KERNEL);
+ if (!map->buttons) {
+ rc = -ENOMEM;
+ goto map_buttons_ret;
+ }
+ child = device_get_named_child_node(dev,
+ ts_virtobj_names[BUTTON]);
+ if (unlikely(!child))
+ goto map_buttons_free;
+
+ rc = ts_virtobj_get_button_properties(dev, child, map->buttons);
+ if (rc < 0)
+ goto map_buttons_free;
+
+ map->button_count = button_count;
+ }
+
+ return 0;
+
+map_buttons_free:
+ devm_kfree(dev, map->buttons);
+map_buttons_ret:
+ return rc;
+}
+
+static bool ts_virtobj_defined_objects(struct device *dev)
+{
+ struct fwnode_handle *child;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(ts_virtobj_names); i++) {
+ child = device_get_named_child_node(dev, ts_virtobj_names[i]);
+ if (child) {
+ fwnode_handle_put(child);
+ return true;
+ }
+ fwnode_handle_put(child);
+ }
+
+ return false;
+}
+
+struct ts_virtobj_map *ts_virtobj_map_objects(struct device *dev,
+ struct input_dev *input)
+{
+ struct ts_virtobj_map *map = NULL;
+ int rc;
+
+ if (!ts_virtobj_defined_objects(dev))
+ return NULL;
+
+ map = devm_kzalloc(dev, sizeof(*map), GFP_KERNEL);
+ if (!map) {
+ rc = -ENOMEM;
+ goto objects_err;
+ }
+ rc = ts_virtobj_map_touchscreen(dev, map);
+ if (rc < 0)
+ goto objects_free;
+
+ rc = ts_virtobj_map_buttons(dev, map, input);
+ if (rc < 0)
+ goto objects_free;
+
+ return map;
+
+objects_free:
+ devm_kfree(dev, map);
+objects_err:
+ return ERR_PTR(rc);
+}
+EXPORT_SYMBOL(ts_virtobj_map_objects);
+
+void ts_virtobj_get_touchscreen_abs(struct ts_virtobj_map *map, u16 *x, u16 *y)
+{
+ *x = map->touchscreen->x_size - 1;
+ *y = map->touchscreen->y_size - 1;
+}
+EXPORT_SYMBOL(ts_virtobj_get_touchscreen_abs);
+
+static bool ts_virtobj_shape_event(struct ts_virtobj_shape *shape, u32 x, u32 y)
+{
+ if (!shape)
+ return false;
+
+ if (x >= shape->x_origin && x < (shape->x_origin + shape->x_size) &&
+ y >= shape->y_origin && y < (shape->y_origin + shape->y_size))
+ return true;
+
+ return false;
+}
+
+static bool ts_virtobj_touchscreen_event(struct ts_virtobj_shape *touchscreen,
+ u32 *x, u32 *y)
+{
+ if (ts_virtobj_shape_event(touchscreen, *x, *y)) {
+ *x -= touchscreen->x_origin;
+ *y -= touchscreen->y_origin;
+ return true;
+ }
+
+ return false;
+}
+
+bool ts_virtobj_mapped_touchscreen(struct ts_virtobj_map *map)
+{
+ if (!map || !map->virtual_touchscreen)
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL(ts_virtobj_mapped_touchscreen);
+
+bool ts_virtobj_mapped_buttons(struct ts_virtobj_map *map)
+{
+ if (!map || !map->button_count)
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL(ts_virtobj_mapped_buttons);
+
+bool ts_virtobj_mt_on_touchscreen(struct ts_virtobj_map *map, u32 *x, u32 *y)
+{
+ if (!ts_virtobj_mapped_touchscreen(map))
+ return true;
+
+ if (!ts_virtobj_touchscreen_event(map->touchscreen, x, y))
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL(ts_virtobj_mt_on_touchscreen);
+
+bool ts_virtobj_button_press(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 x, u32 y, u32 slot)
+{
+ int i;
+
+ if (!ts_virtobj_mapped_buttons(map))
+ return false;
+
+ for (i = 0; i < map->button_count; i++) {
+ if (ts_virtobj_shape_event(&map->buttons[i].shape, x, y)) {
+ input_report_key(input, map->buttons[i].key, 1);
+ map->buttons[i].pressed = true;
+ map->buttons[i].slot = slot;
+ return true;
+ }
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(ts_virtobj_button_press);
+
+bool ts_virtobj_is_button_slot(struct ts_virtobj_map *map, int slot)
+{
+ int i;
+
+ if (!map || !map->button_count)
+ return false;
+
+ for (i = 0; i < map->button_count; i++) {
+ if (map->buttons[i].pressed && map->buttons[i].slot == slot)
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(ts_virtobj_is_button_slot);
+
+void ts_virtobj_button_release(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 slot)
+{
+ int i;
+
+ if (!map || !map->button_count)
+ return;
+
+ for (i = 0; i < map->button_count; i++) {
+ if (map->buttons[i].pressed && map->buttons[i].slot == slot) {
+ input_report_key(input, map->buttons[i].key, 0);
+ map->buttons[i].pressed = false;
+ }
+ }
+}
+EXPORT_SYMBOL(ts_virtobj_button_release);
+
+#endif
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Helper functions for virtual objects on touchscreens");
diff --git a/include/linux/input/ts-virtobj.h b/include/linux/input/ts-virtobj.h
new file mode 100644
index 000000000000..5f9d0059451e
--- /dev/null
+++ b/include/linux/input/ts-virtobj.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#ifndef _TS_VIRTOBJ
+#define _TS_VIRTOBJ
+
+#include <linux/types.h>
+
+struct input_dev;
+struct device;
+
+struct ts_virtobj_map {
+ struct ts_virtobj_shape *touchscreen;
+ bool virtual_touchscreen;
+ struct ts_virtobj_button *buttons;
+ u32 button_count;
+};
+
+#if IS_ENABLED(CONFIG_TOUCHSCREEN_TS_VIRTOBJ)
+struct ts_virtobj_map *ts_virtobj_map_objects(struct device *dev,
+ struct input_dev *input);
+
+void ts_virtobj_get_touchscreen_abs(struct ts_virtobj_map *map, u16 *x, u16 *y);
+
+bool ts_virtobj_mapped_touchscreen(struct ts_virtobj_map *map);
+
+bool ts_virtobj_mapped_buttons(struct ts_virtobj_map *map);
+
+bool ts_virtobj_mt_on_touchscreen(struct ts_virtobj_map *map, u32 *x, u32 *y);
+
+bool ts_virtobj_button_press(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 x, u32 y, u32 slot);
+
+bool ts_virtobj_is_button_slot(struct ts_virtobj_map *map, int slot);
+
+void ts_virtobj_button_release(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 slot);
+
+void ts_virtobj_set_button_caps(struct ts_virtobj_map *map,
+ struct input_dev *dev);
+#else
+static inline struct ts_virtobj_map *
+ts_virtobj_map_objects(struct device *dev, struct input_dev *input)
+{
+ return NULL;
+}
+
+static inline void ts_virtobj_get_touchscreen_abs(struct ts_virtobj_map *map,
+ u16 *x, u16 *y)
+{
+}
+
+static inline bool ts_virtobj_mapped_touchscreen(struct ts_virtobj_map *map)
+{
+ return false;
+}
+
+static inline bool ts_virtobj_mapped_buttons(struct ts_virtobj_map *map)
+{
+ return false;
+}
+
+static inline bool ts_virtobj_mt_on_touchscreen(struct ts_virtobj_map *map,
+ u32 *x, u32 *y)
+{
+ return true;
+}
+
+static inline bool ts_virtobj_button_press(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 x,
+ u32 y, u32 slot)
+{
+ return false;
+}
+
+static inline bool ts_virtobj_is_button_slot(struct ts_virtobj_map *map,
+ int slot)
+{
+ return false;
+}
+
+static inline void ts_virtobj_button_release(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 slot)
+{
+}
+
+static inline void ts_virtobj_set_button_caps(struct ts_virtobj_map *map,
+ struct input_dev *dev)
+{
+}
+#endif
+
+#endif
--
2.39.2
^ permalink raw reply related
* [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj
From: Javier Carrasco @ 2023-05-10 13:50 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v1-0-5ae5e81bc264@wolfvision.net>
The st1232 driver supports the virtual-touchscreen and virtual-buttons
objects defined in the generic touchscreen bindings and implemented in
the ts-virtobj module. Add an example where nodes for a virtual
touchscreen and virtual buttons are defined.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
.../input/touchscreen/sitronix,st1232.yaml | 40 ++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
index 1d8ca19fd37a..97a2c063b47c 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
@@ -48,3 +48,43 @@ examples:
gpios = <&gpio1 166 0>;
};
};
+ - |
+ #include <dt-bindings/input/linux-event-codes.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@55 {
+ compatible = "sitronix,st1232";
+ reg = <0x55>;
+ interrupts = <2 0>;
+ gpios = <&gpio1 166 0>;
+
+ virtual-touchscreen {
+ x-origin = <0>;
+ x-size = <240>;
+ y-origin = <40>;
+ y-size = <280>;
+ };
+
+ virtual-buttons {
+ button-light {
+ label = "Camera light";
+ linux,code = <KEY_LIGHTS_TOGGLE>;
+ x-origin = <40>;
+ x-size = <40>;
+ y-origin = <0>;
+ y-size = <40>;
+ };
+
+ button-suspend {
+ label = "Suspend";
+ linux,code = <KEY_SUSPEND>;
+ x-origin = <160>;
+ x-size = <40>;
+ y-origin = <0>;
+ y-size = <40>;
+ };
+ };
+ };
+ };
--
2.39.2
^ permalink raw reply related
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