linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Input: support virtual objects on touchscreens
@ 2023-05-10 13:50 Javier Carrasco
  2023-05-10 13:50 ` [PATCH 1/4] Input: ts-virtobj - Add touchsreen virtual object handling Javier Carrasco
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
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

Some touchscreens are shipped with a physical layer on top of them where
a number of buttons and a resized touchscreen surface might be available.

In order to generate proper key events by overlay buttons and adjust the
touch events to a clipped surface, these patches offer a documented,
device-tree-based solution by means of helper functions.
An implementation for a specific touchscreen driver is also included.

The functions in ts-virtobj provide a simple workflow to acquire
physical objects from the device tree, map them into the device driver
structures as virtual objects and generate events according to
the object descriptions.

This solution has been tested with a JT240MHQS-E3 display, which uses
the st1624 as a touchscreen and provides two overlay buttons and a frame
that clips its effective surface.

Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
Javier Carrasco (4):
      Input: ts-virtobj - Add touchsreen virtual object handling
      dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
      Input: st1232 - add virtual touchscreen and buttons handling
      dt-bindings: input: touchscreen: st1232: add example with ts-virtobj

 .../input/touchscreen/sitronix,st1232.yaml         |  40 +++
 .../bindings/input/touchscreen/touchscreen.yaml    |  54 ++++
 MAINTAINERS                                        |   7 +
 drivers/input/touchscreen/Kconfig                  |   9 +
 drivers/input/touchscreen/Makefile                 |   1 +
 drivers/input/touchscreen/st1232.c                 |  87 +++--
 drivers/input/touchscreen/ts-virtobj.c             | 360 +++++++++++++++++++++
 include/linux/input/ts-virtobj.h                   |  95 ++++++
 8 files changed, 635 insertions(+), 18 deletions(-)
---
base-commit: ac9a78681b921877518763ba0e89202254349d1b
change-id: 20230510-feature-ts_virtobj_patch-e267540aae74

Best regards,
-- 
Javier Carrasco <javier.carrasco@wolfvision.net>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/4] Input: ts-virtobj - Add touchsreen virtual object handling
  2023-05-10 13:50 [PATCH 0/4] Input: support virtual objects on touchscreens Javier Carrasco
@ 2023-05-10 13:50 ` Javier Carrasco
  2023-05-10 13:50 ` [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties Javier Carrasco
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
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

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	[flat|nested] 19+ messages in thread

* [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
  2023-05-10 13:50 [PATCH 0/4] Input: support virtual objects on touchscreens Javier Carrasco
  2023-05-10 13:50 ` [PATCH 1/4] Input: ts-virtobj - Add touchsreen virtual object handling Javier Carrasco
@ 2023-05-10 13:50 ` Javier Carrasco
  2023-05-11  9:45   ` Krzysztof Kozlowski
  2023-05-10 13:50 ` [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling Javier Carrasco
  2023-05-10 13:50 ` [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj Javier Carrasco
  3 siblings, 1 reply; 19+ messages in thread
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

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	[flat|nested] 19+ messages in thread

* [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
  2023-05-10 13:50 [PATCH 0/4] Input: support virtual objects on touchscreens Javier Carrasco
  2023-05-10 13:50 ` [PATCH 1/4] Input: ts-virtobj - Add touchsreen virtual object handling Javier Carrasco
  2023-05-10 13:50 ` [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties Javier Carrasco
@ 2023-05-10 13:50 ` Javier Carrasco
  2023-05-13 22:38   ` kernel test robot
  2023-05-10 13:50 ` [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj Javier Carrasco
  3 siblings, 1 reply; 19+ messages in thread
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

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	[flat|nested] 19+ messages in thread

* [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj
  2023-05-10 13:50 [PATCH 0/4] Input: support virtual objects on touchscreens Javier Carrasco
                   ` (2 preceding siblings ...)
  2023-05-10 13:50 ` [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling Javier Carrasco
@ 2023-05-10 13:50 ` Javier Carrasco
  2023-05-10 14:59   ` Krzysztof Kozlowski
  3 siblings, 1 reply; 19+ messages in thread
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

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	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj
  2023-05-10 13:50 ` [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj Javier Carrasco
@ 2023-05-10 14:59   ` Krzysztof Kozlowski
  2023-05-11  6:26     ` Javier Carrasco
  0 siblings, 1 reply; 19+ messages in thread
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

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	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj
  2023-05-10 14:59   ` Krzysztof Kozlowski
@ 2023-05-11  6:26     ` Javier Carrasco
  0 siblings, 0 replies; 19+ messages in thread
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

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	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
  2023-05-10 13:50 ` [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties Javier Carrasco
@ 2023-05-11  9:45   ` Krzysztof Kozlowski
  2023-05-11 10:28     ` Javier Carrasco
  0 siblings, 1 reply; 19+ messages in thread
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

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	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
  2023-05-11  9:45   ` Krzysztof Kozlowski
@ 2023-05-11 10:28     ` Javier Carrasco
  2023-05-12  6:25       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
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

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	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
  2023-05-11 10:28     ` Javier Carrasco
@ 2023-05-12  6:25       ` Krzysztof Kozlowski
  2023-05-12  7:08         ` Michael Riesch
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-12  6:25 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

On 11/05/2023 12:28, Javier Carrasco wrote:
> 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.

So basically you still have the same touchscreen under the buttons and
these are not separate devices. Whether someone put a sticker on
touchscreen, does not really change hardware behavior and it's up to
system to handle this. What you are trying to do here is to create
virtual buttons through Devicetree and DT is not for that.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
  2023-05-12  6:25       ` Krzysztof Kozlowski
@ 2023-05-12  7:08         ` Michael Riesch
  2023-05-12  7:20           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Riesch @ 2023-05-12  7:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Javier Carrasco, Dmitry Torokhov,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
	Bastian Hecht
  Cc: linux-kernel, linux-input, devicetree

Hi Krzysztof,

On 5/12/23 08:25, Krzysztof Kozlowski wrote:
> On 11/05/2023 12:28, Javier Carrasco wrote:
>> 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.
> 
> So basically you still have the same touchscreen under the buttons and
> these are not separate devices. Whether someone put a sticker on
> touchscreen, does not really change hardware behavior and it's up to
> system to handle this. What you are trying to do here is to create
> virtual buttons through Devicetree and DT is not for that.

I have already addressed a similar statement here:
https://lore.kernel.org/lkml/20230504042927.GA1129520@quokka/t/#m1a93595c36535312df0061459a1da5e062de6c44
but let me extend this comment a bit.

The notion of "someone putting a sticker on touchscreen" does not really
reflect the use case we have in mind. We are talking about devices that
are shipped from the factory in a particular configuration, e.g.,

+-----------------------+---------+
|                       |  power  |
|                       |  button |
|   touchscreen         +---------+
|   (behind: display)   |  return |
|                       |  button |
+-----------------------+---------+

Here, the real touchscreen is larger than the display. The display is
behind the "touchscreen" part. Behind the buttons there are symbols
engraved in metal or printed foils or what not. I just would like to
make it clear that these symbols are not going to change.

We believe that the engraved or printed symbols actually define the
(expected) hardware behavior. Of course there is a virtual notion to
that, and of course it would be possible to let the power button work as
return button and vice versa in software. However, the user sees the
engraved or printed symbols (which are not going to change) and expects
the device to react appropriately.

Now if you suggest that the system (that is user space, right?) should
handle this, then the question would be which component is supposed to
handle the touchscreen events and react accordingly. I don't have an
answer to that and hope I don't need to find one. But independent of
that, a configuration file is required that defines the area of the
virtual buttons etc. Wouldn't this be similar to the (mostly) beloved
xorg.conf containing the definitions of input devices?

Best regards,
Michael

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
  2023-05-12  7:08         ` Michael Riesch
@ 2023-05-12  7:20           ` Krzysztof Kozlowski
  2023-05-12  8:13             ` Michael Riesch
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-12  7:20 UTC (permalink / raw)
  To: Michael Riesch, Javier Carrasco, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, Bastian Hecht
  Cc: linux-kernel, linux-input, devicetree

On 12/05/2023 09:08, Michael Riesch wrote:
> Hi Krzysztof,
> 
>>> 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.
>>
>> So basically you still have the same touchscreen under the buttons and
>> these are not separate devices. Whether someone put a sticker on
>> touchscreen, does not really change hardware behavior and it's up to
>> system to handle this. What you are trying to do here is to create
>> virtual buttons through Devicetree and DT is not for that.
> 
> I have already addressed a similar statement here:
> https://lore.kernel.org/lkml/20230504042927.GA1129520@quokka/t/#m1a93595c36535312df0061459a1da5e062de6c44
> but let me extend this comment a bit.
> 
> The notion of "someone putting a sticker on touchscreen" does not really
> reflect the use case we have in mind. We are talking about devices that
> are shipped from the factory in a particular configuration, e.g.,
> 
> +-----------------------+---------+
> |                       |  power  |
> |                       |  button |
> |   touchscreen         +---------+
> |   (behind: display)   |  return |
> |                       |  button |
> +-----------------------+---------+
> 
> Here, the real touchscreen is larger than the display. The display is
> behind the "touchscreen" part. Behind the buttons there are symbols
> engraved in metal or printed foils or what not. I just would like to
> make it clear that these symbols are not going to change.
> 
> We believe that the engraved or printed symbols actually define the
> (expected) hardware behavior. Of course there is a virtual notion to
> that, and of course it would be possible to let the power button work as
> return button and vice versa in software. However, the user sees the
> engraved or printed symbols (which are not going to change) and expects
> the device to react appropriately.

OK, I actually missed the concept that display is not equal to the
touchscreen ("screen" actually suggests display :) ). In your case here
it sounds good, but please put some parts of this description into this
common binding. The sketch above is nice, especially if you can point
where the virtual origin x/y are. Picture is thousands words.

> 
> Now if you suggest that the system (that is user space, right?) should
> handle this, then the question would be which component is supposed to
> handle the touchscreen events and react accordingly. I don't have an
> answer to that and hope I don't need to find one. But independent of
> that, a configuration file is required that defines the area of the
> virtual buttons etc. Wouldn't this be similar to the (mostly) beloved
> xorg.conf containing the definitions of input devices?

If the case was a bit different - e.g. display is everywhere - I could
easily imagine that on the device rotation you want to move
(re-position) the buttons. Or if user has some accessibility option
enabled, you want bigger buttons. Then it would be a prove that it must
be configured and managed from user-space. How, I don't know.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
  2023-05-12  7:20           ` Krzysztof Kozlowski
@ 2023-05-12  8:13             ` Michael Riesch
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Riesch @ 2023-05-12  8:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Javier Carrasco, Dmitry Torokhov,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
	Bastian Hecht
  Cc: linux-kernel, linux-input, devicetree

Hi Krzysztof,

On 5/12/23 09:20, Krzysztof Kozlowski wrote:
> On 12/05/2023 09:08, Michael Riesch wrote:
>> Hi Krzysztof,
>>
>>>> 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.
>>>
>>> So basically you still have the same touchscreen under the buttons and
>>> these are not separate devices. Whether someone put a sticker on
>>> touchscreen, does not really change hardware behavior and it's up to
>>> system to handle this. What you are trying to do here is to create
>>> virtual buttons through Devicetree and DT is not for that.
>>
>> I have already addressed a similar statement here:
>> https://lore.kernel.org/lkml/20230504042927.GA1129520@quokka/t/#m1a93595c36535312df0061459a1da5e062de6c44
>> but let me extend this comment a bit.
>>
>> The notion of "someone putting a sticker on touchscreen" does not really
>> reflect the use case we have in mind. We are talking about devices that
>> are shipped from the factory in a particular configuration, e.g.,
>>
>> +-----------------------+---------+
>> |                       |  power  |
>> |                       |  button |
>> |   touchscreen         +---------+
>> |   (behind: display)   |  return |
>> |                       |  button |
>> +-----------------------+---------+
>>
>> Here, the real touchscreen is larger than the display. The display is
>> behind the "touchscreen" part. Behind the buttons there are symbols
>> engraved in metal or printed foils or what not. I just would like to
>> make it clear that these symbols are not going to change.
>>
>> We believe that the engraved or printed symbols actually define the
>> (expected) hardware behavior. Of course there is a virtual notion to
>> that, and of course it would be possible to let the power button work as
>> return button and vice versa in software. However, the user sees the
>> engraved or printed symbols (which are not going to change) and expects
>> the device to react appropriately.
> 
> OK, I actually missed the concept that display is not equal to the
> touchscreen ("screen" actually suggests display :) ). In your case here
> it sounds good, but please put some parts of this description into this
> common binding. The sketch above is nice, especially if you can point
> where the virtual origin x/y are. Picture is thousands words.

I think this can be arranged :-)

>> Now if you suggest that the system (that is user space, right?) should
>> handle this, then the question would be which component is supposed to
>> handle the touchscreen events and react accordingly. I don't have an
>> answer to that and hope I don't need to find one. But independent of
>> that, a configuration file is required that defines the area of the
>> virtual buttons etc. Wouldn't this be similar to the (mostly) beloved
>> xorg.conf containing the definitions of input devices?
> 
> If the case was a bit different - e.g. display is everywhere - I could
> easily imagine that on the device rotation you want to move
> (re-position) the buttons. Or if user has some accessibility option
> enabled, you want bigger buttons. Then it would be a prove that it must
> be configured and managed from user-space. How, I don't know.

I fully agree here. If user space is able to draw the symbols, then
naturally it is also responsible for handling the events appropriately.
This could happen in an application with a GUI or on display
manager/compositor level (e.g., weston controller or similar).

But I take it that for the situation sketched above the device tree
approach is the correct one. Documentation should be improved, though.

Best regards,
Michael

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
  2023-05-10 13:50 ` [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling Javier Carrasco
@ 2023-05-13 22:38   ` kernel test robot
  2023-05-15  4:34     ` Javier Carrasco
  0 siblings, 1 reply; 19+ messages in thread
From: kernel test robot @ 2023-05-13 22:38 UTC (permalink / raw)
  To: Javier Carrasco, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, Bastian Hecht,
	Michael Riesch
  Cc: oe-kbuild-all, linux-kernel, linux-input, devicetree, Javier Carrasco

Hi Javier,

kernel test robot noticed the following build errors:

[auto build test ERROR on ac9a78681b921877518763ba0e89202254349d1b]

url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
base:   ac9a78681b921877518763ba0e89202254349d1b
patch link:    https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-3-5ae5e81bc264%40wolfvision.net
patch subject: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
config: arm-randconfig-m041-20230514 (https://download.01.org/0day-ci/archive/20230514/202305140640.VLcvhR5G-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
        git checkout 133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305140640.VLcvhR5G-lkp@intel.com/

All errors (new ones prefixed by >>):

   arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_parse_and_report':
>> st1232.c:(.text+0x148): undefined reference to `ts_virtobj_is_button_slot'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x15e): undefined reference to `ts_virtobj_button_press'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x16c): undefined reference to `ts_virtobj_mt_on_touchscreen'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x242): undefined reference to `ts_virtobj_mapped_buttons'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x266): undefined reference to `ts_virtobj_is_button_slot'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x276): undefined reference to `ts_virtobj_button_release'
   arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_probe':
>> st1232.c:(.text+0x42c): undefined reference to `ts_virtobj_map_objects'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x43c): undefined reference to `ts_virtobj_mapped_touchscreen'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x44a): undefined reference to `ts_virtobj_get_touchscreen_abs'
   arm-linux-gnueabi-ld: st1232.c:(.text+0x520): undefined reference to `ts_virtobj_mapped_buttons'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x542): undefined reference to `ts_virtobj_set_button_caps'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
  2023-05-13 22:38   ` kernel test robot
@ 2023-05-15  4:34     ` Javier Carrasco
  2023-05-15  6:43       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Javier Carrasco @ 2023-05-15  4:34 UTC (permalink / raw)
  To: kernel test robot, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, Bastian Hecht,
	Michael Riesch
  Cc: linux-kernel, linux-input, oe-kbuild-all

On 14.05.23 00:38, kernel test robot wrote:
> Hi Javier,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on ac9a78681b921877518763ba0e89202254349d1b]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
> base:   ac9a78681b921877518763ba0e89202254349d1b
> patch link:    https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-3-5ae5e81bc264%40wolfvision.net
> patch subject: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
> config: arm-randconfig-m041-20230514 (https://download.01.org/0day-ci/archive/20230514/202305140640.VLcvhR5G-lkp@intel.com/config)
> compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
>         git checkout 133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202305140640.VLcvhR5G-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_parse_and_report':
>>> st1232.c:(.text+0x148): undefined reference to `ts_virtobj_is_button_slot'
>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x15e): undefined reference to `ts_virtobj_button_press'
>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x16c): undefined reference to `ts_virtobj_mt_on_touchscreen'
>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x242): undefined reference to `ts_virtobj_mapped_buttons'
>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x266): undefined reference to `ts_virtobj_is_button_slot'
>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x276): undefined reference to `ts_virtobj_button_release'
>    arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_probe':
>>> st1232.c:(.text+0x42c): undefined reference to `ts_virtobj_map_objects'
>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x43c): undefined reference to `ts_virtobj_mapped_touchscreen'
>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x44a): undefined reference to `ts_virtobj_get_touchscreen_abs'
>    arm-linux-gnueabi-ld: st1232.c:(.text+0x520): undefined reference to `ts_virtobj_mapped_buttons'
>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x542): undefined reference to `ts_virtobj_set_button_caps'
> 
Apparently there is something wrong about the references from this patch
to a previous one from the same series ([PATCH 1/4] Input: ts-virtobj -
Add touchs[c]reen virtual object handling). The "url" link shows all
patches of this series in the right order though.

All these functions are declared in the linux/input/ts-virtobj.h header
and also inline-defined there if ts-virtobj is not selected. If it is
selected (either y or M), the functions are exported from
driver/input/touchscreen/ts-virtobj.c. According to the error report,
ts-virtobj was selected as a module.

I could build the kernel with the three possible configurations
(ts-virtobj y/n/M) for x86_64 as well as for arm64 with no errors or
warnings repeatedly, so I am a bit confused now. I am probably
missing something, but I do not know what.

I wonder if the new file where the functions are defined (ts-virtobj.c)
could not be found by some reason or if the test build does not like the
way I handled the function declaration/definition. Any hint or advice
would be more than welcome.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
  2023-05-15  4:34     ` Javier Carrasco
@ 2023-05-15  6:43       ` Krzysztof Kozlowski
  2023-05-15  8:33         ` Javier Carrasco
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-15  6:43 UTC (permalink / raw)
  To: 202305140640.VLcvhR5G-lkp, kernel test robot, Dmitry Torokhov,
	Rob Herring, Conor Dooley, Henrik Rydberg, Bastian Hecht,
	Michael Riesch
  Cc: linux-kernel, linux-input, oe-kbuild-all

On 15/05/2023 06:34, Javier Carrasco wrote:
> On 14.05.23 00:38, kernel test robot wrote:
>> Hi Javier,
>>
>> kernel test robot noticed the following build errors:
>>
>> [auto build test ERROR on ac9a78681b921877518763ba0e89202254349d1b]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
>> base:   ac9a78681b921877518763ba0e89202254349d1b
>> patch link:    https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-3-5ae5e81bc264%40wolfvision.net
>> patch subject: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
>> config: arm-randconfig-m041-20230514 (https://download.01.org/0day-ci/archive/20230514/202305140640.VLcvhR5G-lkp@intel.com/config)
>> compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
>> reproduce (this is a W=1 build):
>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>         chmod +x ~/bin/make.cross
>>         # https://github.com/intel-lab-lkp/linux/commit/133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
>>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>>         git fetch --no-tags linux-review Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
>>         git checkout 133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
>>         # save the config file
>>         mkdir build_dir && cp config build_dir/.config
>>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
>>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
>>
>> If you fix the issue, kindly add following tag where applicable
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Link: https://lore.kernel.org/oe-kbuild-all/202305140640.VLcvhR5G-lkp@intel.com/
>>
>> All errors (new ones prefixed by >>):
>>
>>    arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_parse_and_report':
>>>> st1232.c:(.text+0x148): undefined reference to `ts_virtobj_is_button_slot'
>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x15e): undefined reference to `ts_virtobj_button_press'
>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x16c): undefined reference to `ts_virtobj_mt_on_touchscreen'
>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x242): undefined reference to `ts_virtobj_mapped_buttons'
>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x266): undefined reference to `ts_virtobj_is_button_slot'
>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x276): undefined reference to `ts_virtobj_button_release'
>>    arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_probe':
>>>> st1232.c:(.text+0x42c): undefined reference to `ts_virtobj_map_objects'
>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x43c): undefined reference to `ts_virtobj_mapped_touchscreen'
>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x44a): undefined reference to `ts_virtobj_get_touchscreen_abs'
>>    arm-linux-gnueabi-ld: st1232.c:(.text+0x520): undefined reference to `ts_virtobj_mapped_buttons'
>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x542): undefined reference to `ts_virtobj_set_button_caps'
>>
> Apparently there is something wrong about the references from this patch
> to a previous one from the same series ([PATCH 1/4] Input: ts-virtobj -
> Add touchs[c]reen virtual object handling). The "url" link shows all
> patches of this series in the right order though.
> 
> All these functions are declared in the linux/input/ts-virtobj.h header
> and also inline-defined there if ts-virtobj is not selected. If it is
> selected (either y or M), the functions are exported from
> driver/input/touchscreen/ts-virtobj.c. According to the error report,
> ts-virtobj was selected as a module.
> 
> I could build the kernel with the three possible configurations
> (ts-virtobj y/n/M) for x86_64 as well as for arm64 with no errors or
> warnings repeatedly, so I am a bit confused now. I am probably
> missing something, but I do not know what.
> 
> I wonder if the new file where the functions are defined (ts-virtobj.c)
> could not be found by some reason or if the test build does not like the
> way I handled the function declaration/definition. Any hint or advice
> would be more than welcome.

The report is correct. Build driver builtin and your virtual as module.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
  2023-05-15  6:43       ` Krzysztof Kozlowski
@ 2023-05-15  8:33         ` Javier Carrasco
  2023-05-15  8:41           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Javier Carrasco @ 2023-05-15  8:33 UTC (permalink / raw)
  To: Krzysztof Kozlowski, 202305140640.VLcvhR5G-lkp,
	kernel test robot, Dmitry Torokhov, Rob Herring, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch
  Cc: linux-kernel, linux-input, oe-kbuild-all

On 15.05.23 08:43, Krzysztof Kozlowski wrote:
> On 15/05/2023 06:34, Javier Carrasco wrote:
>> On 14.05.23 00:38, kernel test robot wrote:
>>> Hi Javier,
>>>
>>> kernel test robot noticed the following build errors:
>>>
>>> [auto build test ERROR on ac9a78681b921877518763ba0e89202254349d1b]
>>>
>>> url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
>>> base:   ac9a78681b921877518763ba0e89202254349d1b
>>> patch link:    https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-3-5ae5e81bc264%40wolfvision.net
>>> patch subject: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
>>> config: arm-randconfig-m041-20230514 (https://download.01.org/0day-ci/archive/20230514/202305140640.VLcvhR5G-lkp@intel.com/config)
>>> compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
>>> reproduce (this is a W=1 build):
>>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>         chmod +x ~/bin/make.cross
>>>         # https://github.com/intel-lab-lkp/linux/commit/133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
>>>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>>>         git fetch --no-tags linux-review Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
>>>         git checkout 133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
>>>         # save the config file
>>>         mkdir build_dir && cp config build_dir/.config
>>>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
>>>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Link: https://lore.kernel.org/oe-kbuild-all/202305140640.VLcvhR5G-lkp@intel.com/
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>    arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_parse_and_report':
>>>>> st1232.c:(.text+0x148): undefined reference to `ts_virtobj_is_button_slot'
>>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x15e): undefined reference to `ts_virtobj_button_press'
>>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x16c): undefined reference to `ts_virtobj_mt_on_touchscreen'
>>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x242): undefined reference to `ts_virtobj_mapped_buttons'
>>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x266): undefined reference to `ts_virtobj_is_button_slot'
>>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x276): undefined reference to `ts_virtobj_button_release'
>>>    arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_probe':
>>>>> st1232.c:(.text+0x42c): undefined reference to `ts_virtobj_map_objects'
>>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x43c): undefined reference to `ts_virtobj_mapped_touchscreen'
>>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x44a): undefined reference to `ts_virtobj_get_touchscreen_abs'
>>>    arm-linux-gnueabi-ld: st1232.c:(.text+0x520): undefined reference to `ts_virtobj_mapped_buttons'
>>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x542): undefined reference to `ts_virtobj_set_button_caps'
>>>
>> Apparently there is something wrong about the references from this patch
>> to a previous one from the same series ([PATCH 1/4] Input: ts-virtobj -
>> Add touchs[c]reen virtual object handling). The "url" link shows all
>> patches of this series in the right order though.
>>
>> All these functions are declared in the linux/input/ts-virtobj.h header
>> and also inline-defined there if ts-virtobj is not selected. If it is
>> selected (either y or M), the functions are exported from
>> driver/input/touchscreen/ts-virtobj.c. According to the error report,
>> ts-virtobj was selected as a module.
>>
>> I could build the kernel with the three possible configurations
>> (ts-virtobj y/n/M) for x86_64 as well as for arm64 with no errors or
>> warnings repeatedly, so I am a bit confused now. I am probably
>> missing something, but I do not know what.
>>
>> I wonder if the new file where the functions are defined (ts-virtobj.c)
>> could not be found by some reason or if the test build does not like the
>> way I handled the function declaration/definition. Any hint or advice
>> would be more than welcome.
> 
> The report is correct. Build driver builtin and your virtual as module.

You are right, that was the configuration I was missing, which I
honestly did not even consider when I tested this feature.

The problem seems to be that I am using the IS_ENABLED macro which
returns true if selected as a module, but the define is in that case
CONFIG_TOUCHSCREEN_TS_VIRTOBJ_MODULE instead of
CONFIG_TOUCHSCREEN_TS_VIRTOBJ. As I am using the latter define in the
Makefile, it does not get compiled.

I am testing the IS_REACHABLE macro for the header instead, which I
think is the right one in this case and actually fixes the bug. I am
telling you that just in case I am talking nonsense to save everyone
from a pointless v2.
But first I will go through all possible combinations between my virtual
and a touchscreen driver before I submit a broken v2, where I will also
provide an improved documentation as you suggested.
Thanks a lot again!

> 
> Best regards,
> Krzysztof
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
  2023-05-15  8:33         ` Javier Carrasco
@ 2023-05-15  8:41           ` Krzysztof Kozlowski
  2023-05-15  9:08             ` Javier Carrasco
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-15  8:41 UTC (permalink / raw)
  To: Javier Carrasco, 202305140640.VLcvhR5G-lkp, kernel test robot,
	Dmitry Torokhov, Rob Herring, Conor Dooley, Henrik Rydberg,
	Bastian Hecht, Michael Riesch
  Cc: linux-kernel, linux-input, oe-kbuild-all

On 15/05/2023 10:33, Javier Carrasco wrote:
>>> All these functions are declared in the linux/input/ts-virtobj.h header
>>> and also inline-defined there if ts-virtobj is not selected. If it is
>>> selected (either y or M), the functions are exported from
>>> driver/input/touchscreen/ts-virtobj.c. According to the error report,
>>> ts-virtobj was selected as a module.
>>>
>>> I could build the kernel with the three possible configurations
>>> (ts-virtobj y/n/M) for x86_64 as well as for arm64 with no errors or
>>> warnings repeatedly, so I am a bit confused now. I am probably
>>> missing something, but I do not know what.
>>>
>>> I wonder if the new file where the functions are defined (ts-virtobj.c)
>>> could not be found by some reason or if the test build does not like the
>>> way I handled the function declaration/definition. Any hint or advice
>>> would be more than welcome.
>>
>> The report is correct. Build driver builtin and your virtual as module.
> 
> You are right, that was the configuration I was missing, which I
> honestly did not even consider when I tested this feature.
> 
> The problem seems to be that I am using the IS_ENABLED macro which
> returns true if selected as a module, but the define is in that case
> CONFIG_TOUCHSCREEN_TS_VIRTOBJ_MODULE instead of
> CONFIG_TOUCHSCREEN_TS_VIRTOBJ. As I am using the latter define in the
> Makefile, it does not get compiled.

This could be proper solution for build failure but does not look like
the proper solution for entire choice/design. The questions are: why
TOUCHSCREEN_TS_VIRTOBJ should be selectable by user? How can user know
that he needs a driver with VIRTOBJ?

I think he cannot and your config help option suggests that:
"you are using a touchscreen driver that supports printed overlays".
What driver supports or not, is a job for kernel developers. Not for
kernel configurators or distros. User should never investigate whether
his touchscreen driver support printed overlays. Why would user like to
dig into kernel to know that? Thus either your driver should select
VIRTOBJ or depend on it.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
  2023-05-15  8:41           ` Krzysztof Kozlowski
@ 2023-05-15  9:08             ` Javier Carrasco
  0 siblings, 0 replies; 19+ messages in thread
From: Javier Carrasco @ 2023-05-15  9:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Dmitry Torokhov, Rob Herring, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch
  Cc: linux-kernel, linux-input, oe-kbuild-all



On 15.05.23 10:41, Krzysztof Kozlowski wrote:
> On 15/05/2023 10:33, Javier Carrasco wrote:
>>>> All these functions are declared in the linux/input/ts-virtobj.h header
>>>> and also inline-defined there if ts-virtobj is not selected. If it is
>>>> selected (either y or M), the functions are exported from
>>>> driver/input/touchscreen/ts-virtobj.c. According to the error report,
>>>> ts-virtobj was selected as a module.
>>>>
>>>> I could build the kernel with the three possible configurations
>>>> (ts-virtobj y/n/M) for x86_64 as well as for arm64 with no errors or
>>>> warnings repeatedly, so I am a bit confused now. I am probably
>>>> missing something, but I do not know what.
>>>>
>>>> I wonder if the new file where the functions are defined (ts-virtobj.c)
>>>> could not be found by some reason or if the test build does not like the
>>>> way I handled the function declaration/definition. Any hint or advice
>>>> would be more than welcome.
>>>
>>> The report is correct. Build driver builtin and your virtual as module.
>>
>> You are right, that was the configuration I was missing, which I
>> honestly did not even consider when I tested this feature.
>>
>> The problem seems to be that I am using the IS_ENABLED macro which
>> returns true if selected as a module, but the define is in that case
>> CONFIG_TOUCHSCREEN_TS_VIRTOBJ_MODULE instead of
>> CONFIG_TOUCHSCREEN_TS_VIRTOBJ. As I am using the latter define in the
>> Makefile, it does not get compiled.
> 
> This could be proper solution for build failure but does not look like
> the proper solution for entire choice/design. The questions are: why
> TOUCHSCREEN_TS_VIRTOBJ should be selectable by user? How can user know
> that he needs a driver with VIRTOBJ?
> 
> I think he cannot and your config help option suggests that:
> "you are using a touchscreen driver that supports printed overlays".
> What driver supports or not, is a job for kernel developers. Not for
> kernel configurators or distros. User should never investigate whether
> his touchscreen driver support printed overlays. Why would user like to
> dig into kernel to know that? Thus either your driver should select
> VIRTOBJ or depend on it.
> 
I was thinking about the minimal savings (inline functions that just
return a value and the size of the ts-virtobj object itself) that could
be obtained if the touchscreen does not need the functionality at all.
But probably those savings are negligible and there will not be many
cases where the user will know if that applies.

I agree, the feature will be selected by drivers that support it. That
actually simplifies the code a little bit.

> Best regards,
> Krzysztof
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-05-15  9:08 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 13:50 [PATCH 0/4] Input: support virtual objects on touchscreens Javier Carrasco
2023-05-10 13:50 ` [PATCH 1/4] Input: ts-virtobj - Add touchsreen virtual object handling Javier Carrasco
2023-05-10 13:50 ` [PATCH 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties Javier Carrasco
2023-05-11  9:45   ` Krzysztof Kozlowski
2023-05-11 10:28     ` Javier Carrasco
2023-05-12  6:25       ` Krzysztof Kozlowski
2023-05-12  7:08         ` Michael Riesch
2023-05-12  7:20           ` Krzysztof Kozlowski
2023-05-12  8:13             ` Michael Riesch
2023-05-10 13:50 ` [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling Javier Carrasco
2023-05-13 22:38   ` kernel test robot
2023-05-15  4:34     ` Javier Carrasco
2023-05-15  6:43       ` Krzysztof Kozlowski
2023-05-15  8:33         ` Javier Carrasco
2023-05-15  8:41           ` Krzysztof Kozlowski
2023-05-15  9:08             ` Javier Carrasco
2023-05-10 13:50 ` [PATCH 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-virtobj Javier Carrasco
2023-05-10 14:59   ` Krzysztof Kozlowski
2023-05-11  6:26     ` Javier Carrasco

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).