* Re: [PATCH] HID: nvidia-shield: Add some null pointer checks to the hid-nvidia-shield.c
From: Rahul Rameshbabu @ 2024-01-19 6:19 UTC (permalink / raw)
To: Kunwu Chan; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20240119060714.258635-1-chentao@kylinos.cn>
On Fri, 19 Jan, 2024 14:07:14 +0800 Kunwu Chan <chentao@kylinos.cn> wrote:
> devm_kasprintf() returns a pointer to dynamically allocated memory
> which can be NULL upon failure. Ensure the allocation was successful
> by checking the pointer validity.
>
> Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
> ---
Just one comment about the subject for future submissions (this one is
fine). I would just leave the subject as "HID: nvidia-shield: Add some
null pointer checks". We know what file is being edited from the commit
content, so the additional filename detail isn't necessary.
Thanks for the patch. Really appreciate it.
Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
^ permalink raw reply
* [PATCH] HID: nintendo: Remove some unused functions
From: Jiapeng Chong @ 2024-01-19 7:27 UTC (permalink / raw)
To: djogorchock
Cc: jikos, benjamin.tissoires, linux-input, linux-kernel,
Jiapeng Chong, Abaci Robot
These functions are defined in the hid-nintendo.c file, but not called
elsewhere, so delete these unused functions.
drivers/hid/hid-nintendo.c:670:20: warning: unused function 'joycon_device_is_left_joycon'.
drivers/hid/hid-nintendo.c:674:20: warning: unused function 'joycon_device_is_right_joycon'
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7992
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
drivers/hid/hid-nintendo.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index ccc4032fb2b0..7ce6be0a8dee 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -667,16 +667,6 @@ struct joycon_ctlr {
* These helpers are most useful early during the HID probe or in conjunction
* with the capability helpers below.
*/
-static inline bool joycon_device_is_left_joycon(struct joycon_ctlr *ctlr)
-{
- return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONL;
-}
-
-static inline bool joycon_device_is_right_joycon(struct joycon_ctlr *ctlr)
-{
- return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONR;
-}
-
static inline bool joycon_device_is_procon(struct joycon_ctlr *ctlr)
{
return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_PROCON;
--
2.20.1.7.g153144c
^ permalink raw reply related
* [PATCH v7 0/4] Input: support overlay objects on touchscreens
From: Javier Carrasco @ 2024-01-19 7:43 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, Jeff LaBundy
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco,
Rob Herring
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, this series offers a documented,
device-tree-based solution by means of helper functions.
An implementation for a specific touchscreen driver is also included.
The functions in touch-overlay provide a simple workflow to acquire
physical objects from the device tree, map them into a list and generate
events according to the object descriptions.
This feature has been tested with a JT240MHQS-E3 display, which consists
of an st1624 as the base touchscreen and an overlay with two buttons and
a frame that clips its effective surface mounted on it.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
Changes in v7:
- General: return to a single input device implementation.
- touchscreen bindings: segment instead of button in the label
description.
- touch-overlay.c: define button-specific data inside segment struct.
- touch-overlay.c: remove fwnode_property_present() and check return
value of fwnode_property_read_u32() in touch_overlay_get_segment().
- touch-overlay.c: simplify return path in touch_overlay_map().
- Link to v6: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v6-0-d8a605975153@wolfvision.net
Changes in v6:
- General: use a single list to manage a single type of object.
- General: swap patches to have bindings preceding the code.
- touch-overlay.c: minor code-sytle fixes.
- Link to v5: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v5-0-ff6b5c4db693@wolfvision.net
Changes in v5:
- touchscreen bindings: move overlay common properties to a $def entry (Rob Herring)
- st1232 bindings: move overlays to the existing example instead of
making a new one (Rob Herring)
- Link to v4: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v4-0-5c6c0fc1eed6@wolfvision.net
Changes in v4:
- General: rename "touchscreen" to "touch" to include other consumers.
- PATCH 1/4: move touch-overlay feature to input core.
- PATCH 1/4, 3/4: set key caps and report key events without consumer's
intervention.
- PATCH 2/4: add missing 'required' field with the required properties.
- Link to v3: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v3-0-b4fb7fc4bab7@wolfvision.net
Changes in v3:
- General: rename "virtobj" and "virtual" to "overlay"
- PATCH 1/4: Make feature bool instead of tristate (selected by
supported touchscreens)
- Link to v2: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v2-0-f68a6bfe7a0f@wolfvision.net
Changes in v2:
- PATCH 1/4: remove preprocessor directives (the module is selected by
the drivers that support the feature). Typo in the commit message.
- PATCH 2/4: more detailed documentation. Images and examples were added.
- PATCH 3/4: select ts-virtobj automatically.
- Link to v1: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-0-5ae5e81bc264@wolfvision.net
---
Javier Carrasco (4):
dt-bindings: touchscreen: add touch-overlay property
Input: touch-overlay - Add touchscreen overlay handling
dt-bindings: input: touchscreen: st1232: add touch-overlay example
Input: st1232 - add touch overlays handling
.../input/touchscreen/sitronix,st1232.yaml | 29 +++
.../bindings/input/touchscreen/touchscreen.yaml | 119 ++++++++++
MAINTAINERS | 7 +
drivers/input/Makefile | 2 +-
drivers/input/touch-overlay.c | 250 +++++++++++++++++++++
drivers/input/touchscreen/st1232.c | 48 ++--
include/linux/input/touch-overlay.h | 22 ++
7 files changed, 462 insertions(+), 15 deletions(-)
---
base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
change-id: 20230510-feature-ts_virtobj_patch-e267540aae74
Best regards,
--
Javier Carrasco <javier.carrasco@wolfvision.net>
^ permalink raw reply
* [PATCH v7 1/4] dt-bindings: touchscreen: add touch-overlay property
From: Javier Carrasco @ 2024-01-19 7:43 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, Jeff LaBundy
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco,
Rob Herring
In-Reply-To: <20240119-feature-ts_virtobj_patch-v7-0-eda70985808f@wolfvision.net>
The touch-overlay encompasses a number of touch areas that define a
clipped touchscreen area and/or buttons with a specific functionality.
A clipped touchscreen area avoids getting events from regions that are
physically hidden by overlay frames.
For touchscreens with printed overlay buttons, sub-nodes with a suitable
key code can be defined to report key events instead of the original
touch events.
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
.../bindings/input/touchscreen/touchscreen.yaml | 119 +++++++++++++++++++++
1 file changed, 119 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
index 431c13335c40..eb1e86fa86c6 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
@@ -87,6 +87,125 @@ properties:
touchscreen-y-plate-ohms:
description: Resistance of the Y-plate in Ohms
+ touch-overlay:
+ description: list of nodes defining segments (touch areas) on the
+ touchscreen.
+
+ This object can be used to describe a series of segments to
+ restrict the region within touch events are reported or buttons
+ with a specific functionality.
+
+ This is of special interest if the touchscreen is shipped with a physical
+ overlay on top of it with a frame that hides some part of the original
+ touchscreen area. Printed buttons on that overlay are also a typical
+ use case.
+
+ A new touchscreen area is defined as a sub-node without a key code. If a
+ key code is defined in the sub-node, it will be interpreted as a button.
+
+ The x-origin and y-origin properties of a touchscreen area define the
+ offset of a new origin from where the touchscreen events are referenced.
+ This offset is applied to the events accordingly. The x-size and y-size
+ properties define the size of the touchscreen effective area.
+
+ The following example shows a new touchscreen area with the new origin
+ (0',0') for the touch events generated by the device.
+
+ Touchscreen (full area)
+ ┌────────────────────────────────────────┐
+ │ ┌───────────────────────────────┐ │
+ │ │ │ │
+ │ ├ y-size │ │
+ │ │ │ │
+ │ │ touchscreen area │ │
+ │ │ (no key code) │ │
+ │ │ │ │
+ │ │ x-size │ │
+ │ ┌└──────────────┴────────────────┘ │
+ │(0',0') │
+ ┌└────────────────────────────────────────┘
+ (0,0)
+
+ where (0',0') = (0+x-origin,0+y-origin)
+
+ Sub-nodes with key codes report the touch events on their surface as key
+ events instead.
+
+ The following example shows a touchscreen with a single button on it.
+
+ Touchscreen (full area)
+ ┌───────────────────────────────────┐
+ │ │
+ │ │
+ │ ┌─────────┐ │
+ │ │button 0 │ │
+ │ │KEY_POWER│ │
+ │ └─────────┘ │
+ │ │
+ │ │
+ ┌└───────────────────────────────────┘
+ (0,0)
+
+ Segments defining buttons and clipped toushcreen areas can be combined
+ as shown in the following example.
+ In that case only the events within the touchscreen area are reported
+ as touch events. Events within the button areas report their associated
+ key code. Any events outside the defined areas are ignored.
+
+ Touchscreen (full area)
+ ┌─────────┬──────────────────────────────┐
+ │ │ │
+ │ │ ┌───────────────────────┐ │
+ │ button 0│ │ │ │
+ │KEY_POWER│ │ │ │
+ │ │ │ │ │
+ ├─────────┤ │ touchscreen area │ │
+ │ │ │ (no key code) │ │
+ │ │ │ │ │
+ │ button 1│ │ │ │
+ │ KEY_INFO│ ┌└───────────────────────┘ │
+ │ │(0',0') │
+ ┌└─────────┴──────────────────────────────┘
+ (0,0)
+
+ type: object
+
+ patternProperties:
+ '^segment-':
+ type: object
+ description:
+ Each segment is represented as a sub-node.
+ properties:
+ x-origin:
+ description: horizontal origin of the node area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-origin:
+ description: vertical origin of the node area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ x-size:
+ description: horizontal resolution of the node area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-size:
+ description: vertical resolution of the node area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ label:
+ description: descriptive name of the segment
+ $ref: /schemas/types.yaml#/definitions/string
+
+ linux,code: true
+
+ required:
+ - x-origin
+ - y-origin
+ - x-size
+ - y-size
+
+ unevaluatedProperties: false
+
dependencies:
touchscreen-size-x: [ touchscreen-size-y ]
touchscreen-size-y: [ touchscreen-size-x ]
--
2.39.2
^ permalink raw reply related
* [PATCH v7 2/4] Input: touch-overlay - Add touchscreen overlay handling
From: Javier Carrasco @ 2024-01-19 7:43 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, Jeff LaBundy
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20240119-feature-ts_virtobj_patch-v7-0-eda70985808f@wolfvision.net>
Some touch devices 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 overlay objects via
device tree nodes.
These overlay objects consume the raw touch events and report the
expected input events depending on the object properties.
Note that the current implementation allows for multiple definitions
of touchscreen areas (regions that report touch events), but only the
first one will be used for the touchscreen device that the consumers
typically provide.
Should the need for multiple touchscreen areas arise, additional
touchscreen devices would be required at the consumer side.
There is no limitation in the number of touch areas defined as buttons.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
MAINTAINERS | 7 +
drivers/input/Makefile | 2 +-
drivers/input/touch-overlay.c | 250 ++++++++++++++++++++++++++++++++++++
include/linux/input/touch-overlay.h | 22 ++++
4 files changed, 280 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index a7c4cf8201e0..668687bf94df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21961,6 +21961,13 @@ L: platform-driver-x86@vger.kernel.org
S: Maintained
F: drivers/platform/x86/toshiba-wmi.c
+TOUCH OVERLAY
+M: Javier Carrasco <javier.carrasco@wolfvision.net>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/input/touch-overlay.c
+F: include/linux/input/touch-overlay.h
+
TPM DEVICE DRIVER
M: Peter Huewe <peterhuewe@gmx.de>
M: Jarkko Sakkinen <jarkko@kernel.org>
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index c78753274921..393e9f4d00dc 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -7,7 +7,7 @@
obj-$(CONFIG_INPUT) += input-core.o
input-core-y := input.o input-compat.o input-mt.o input-poller.o ff-core.o
-input-core-y += touchscreen.o
+input-core-y += touchscreen.o touch-overlay.o
obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
obj-$(CONFIG_INPUT_SPARSEKMAP) += sparse-keymap.o
diff --git a/drivers/input/touch-overlay.c b/drivers/input/touch-overlay.c
new file mode 100644
index 000000000000..42b6ad753a00
--- /dev/null
+++ b/drivers/input/touch-overlay.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Helper functions for overlay objects on touchscreens
+ *
+ * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/input/touch-overlay.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/property.h>
+
+struct touch_overlay_segment {
+ struct list_head list;
+ u32 x_origin;
+ u32 y_origin;
+ u32 x_size;
+ u32 y_size;
+ u32 key;
+ bool pressed;
+ int slot;
+};
+
+static int touch_overlay_get_segment(struct fwnode_handle *segment_node,
+ struct touch_overlay_segment *segment,
+ struct input_dev *input)
+{
+ int error;
+
+ error = fwnode_property_read_u32(segment_node, "x-origin",
+ &segment->x_origin);
+ if (error)
+ return error;
+
+ error = fwnode_property_read_u32(segment_node, "y-origin",
+ &segment->y_origin);
+ if (error)
+ return error;
+
+ error = fwnode_property_read_u32(segment_node, "x-size",
+ &segment->x_size);
+ if (error)
+ return error;
+
+ error = fwnode_property_read_u32(segment_node, "y-size",
+ &segment->y_size);
+ if (error)
+ return error;
+
+ error = fwnode_property_read_u32(segment_node, "linux,code",
+ &segment->key);
+ if (!error)
+ input_set_capability(input, EV_KEY, segment->key);
+ else if (error != -EINVAL)
+ return error;
+
+ return 0;
+}
+
+/**
+ * touch_overlay_map - map overlay objects from the device tree and set
+ * key capabilities if buttons are defined.
+ * @list: pointer to the list that will hold the segments
+ * @input: pointer to the already allocated input_dev
+ *
+ * Returns 0 on success and error number otherwise.
+ *
+ * If buttons are defined, key capabilities are set accordingly.
+ */
+int touch_overlay_map(struct list_head *list, struct input_dev *input)
+{
+ struct fwnode_handle *overlay, *fw_segment;
+ struct device *dev = input->dev.parent;
+ struct touch_overlay_segment *segment;
+ int error;
+
+ overlay = device_get_named_child_node(dev, "touch-overlay");
+ if (!overlay)
+ return 0;
+
+ fwnode_for_each_child_node(overlay, fw_segment) {
+ segment = devm_kzalloc(dev, sizeof(*segment), GFP_KERNEL);
+ if (!segment) {
+ error = -ENOMEM;
+ fwnode_handle_put(overlay);
+ break;
+ }
+ error = touch_overlay_get_segment(fw_segment, segment, input);
+ if (error) {
+ fwnode_handle_put(overlay);
+ break;
+ }
+ list_add_tail(&segment->list, list);
+ }
+
+ return error;
+}
+EXPORT_SYMBOL(touch_overlay_map);
+
+/**
+ * touch_overlay_get_touchscreen_abs - get abs size from the touchscreen area.
+ * @list: pointer to the list that holds the segments
+ * @x: horizontal abs
+ * @y: vertical abs
+ */
+void touch_overlay_get_touchscreen_abs(struct list_head *list, u16 *x, u16 *y)
+{
+ struct touch_overlay_segment *segment;
+ struct list_head *ptr;
+
+ list_for_each(ptr, list) {
+ segment = list_entry(ptr, struct touch_overlay_segment, list);
+ if (!segment->key) {
+ *x = segment->x_size - 1;
+ *y = segment->y_size - 1;
+ break;
+ }
+ }
+}
+EXPORT_SYMBOL(touch_overlay_get_touchscreen_abs);
+
+static bool touch_overlay_segment_event(struct touch_overlay_segment *seg,
+ u32 x, u32 y)
+{
+ if (!seg)
+ return false;
+
+ if (x >= seg->x_origin && x < (seg->x_origin + seg->x_size) &&
+ y >= seg->y_origin && y < (seg->y_origin + seg->y_size))
+ return true;
+
+ return false;
+}
+
+/**
+ * touch_overlay_mapped_touchscreen - check if a touchscreen area is mapped
+ * @list: pointer to the list that holds the segments
+ *
+ * Returns true if a touchscreen area is mapped or false otherwise.
+ */
+bool touch_overlay_mapped_touchscreen(struct list_head *list)
+{
+ struct touch_overlay_segment *segment;
+ struct list_head *ptr;
+
+ list_for_each(ptr, list) {
+ segment = list_entry(ptr, struct touch_overlay_segment, list);
+ if (!segment->key)
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(touch_overlay_mapped_touchscreen);
+
+static bool touch_overlay_event_on_ts(struct list_head *list, u32 *x, u32 *y)
+{
+ struct touch_overlay_segment *segment;
+ struct list_head *ptr;
+ bool valid_touch = true;
+
+ if (!x || !y)
+ return false;
+
+ list_for_each(ptr, list) {
+ segment = list_entry(ptr, struct touch_overlay_segment, list);
+ if (segment->key)
+ continue;
+
+ if (touch_overlay_segment_event(segment, *x, *y)) {
+ *x -= segment->x_origin;
+ *y -= segment->y_origin;
+ return true;
+ }
+ /* ignore touch events outside the defined area */
+ valid_touch = false;
+ }
+
+ return valid_touch;
+}
+
+static bool touch_overlay_button_event(struct input_dev *input,
+ struct touch_overlay_segment *segment,
+ const u32 *x, const u32 *y, u32 slot)
+{
+ bool contact = x && y;
+
+ if (!contact && segment->pressed && segment->slot == slot) {
+ segment->pressed = false;
+ input_report_key(input, segment->key, false);
+ input_sync(input);
+ return true;
+ } else if (contact && touch_overlay_segment_event(segment, *x, *y)) {
+ segment->pressed = true;
+ segment->slot = slot;
+ input_report_key(input, segment->key, true);
+ input_sync(input);
+ return true;
+ }
+
+ return false;
+}
+
+/**
+ * touch_overlay_process_event - process input events according to the overlay
+ * mapping. This function acts as a filter to release the calling driver from
+ * the events that are either related to overlay buttons or out of the overlay
+ * touchscreen area, if defined.
+ * @list: pointer to the list that holds the segments
+ * @input: pointer to the input device associated to the event
+ * @x: pointer to the x coordinate (NULL if not available - no contact)
+ * @y: pointer to the y coordinate (NULL if not available - no contact)
+ * @slot: slot associated to the event
+ *
+ * Returns true if the event was processed (reported for valid key events
+ * and dropped for events outside the overlay touchscreen area) or false
+ * if the event must be processed by the caller. In that case this function
+ * shifts the (x,y) coordinates to the overlay touchscreen axis if required.
+ */
+bool touch_overlay_process_event(struct list_head *list,
+ struct input_dev *input,
+ u32 *x, u32 *y, u32 slot)
+{
+ struct touch_overlay_segment *segment;
+ struct list_head *ptr;
+
+ /*
+ * buttons must be prioritized over overlay touchscreens to account for
+ * overlappings e.g. a button inside the touchscreen area.
+ */
+ list_for_each(ptr, list) {
+ segment = list_entry(ptr, struct touch_overlay_segment, list);
+ if (segment->key &&
+ touch_overlay_button_event(input, segment, x, y, slot)) {
+ return true;
+ }
+ }
+
+ /*
+ * valid touch events on the overlay touchscreen are left for the client
+ * to be processed/reported according to its (possibly) unique features.
+ */
+ return !touch_overlay_event_on_ts(list, x, y);
+}
+EXPORT_SYMBOL(touch_overlay_process_event);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Helper functions for overlay objects on touch devices");
diff --git a/include/linux/input/touch-overlay.h b/include/linux/input/touch-overlay.h
new file mode 100644
index 000000000000..cef05c46000d
--- /dev/null
+++ b/include/linux/input/touch-overlay.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#ifndef _TOUCH_OVERLAY
+#define _TOUCH_OVERLAY
+
+#include <linux/types.h>
+
+struct input_dev;
+
+int touch_overlay_map(struct list_head *list, struct input_dev *input);
+
+void touch_overlay_get_touchscreen_abs(struct list_head *list, u16 *x, u16 *y);
+
+bool touch_overlay_mapped_touchscreen(struct list_head *list);
+
+bool touch_overlay_process_event(struct list_head *list, struct input_dev *input,
+ u32 *x, u32 *y, u32 slot);
+
+#endif
--
2.39.2
^ permalink raw reply related
* [PATCH v7 3/4] dt-bindings: input: touchscreen: st1232: add touch-overlay example
From: Javier Carrasco @ 2024-01-19 7:43 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, Jeff LaBundy
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco,
Rob Herring
In-Reply-To: <20240119-feature-ts_virtobj_patch-v7-0-eda70985808f@wolfvision.net>
The touch-overaly feature adds support for segments (touch areas) on the
touchscreen surface that represent overlays with clipped touchscreen
areas and printed buttons.
Add nodes for a clipped touchscreen and overlay buttons to the existing
example.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
.../input/touchscreen/sitronix,st1232.yaml | 29 ++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
index 1d8ca19fd37a..e7ee7a0d74c4 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
@@ -37,6 +37,7 @@ unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/input/linux-event-codes.h>
i2c {
#address-cells = <1>;
#size-cells = <0>;
@@ -46,5 +47,33 @@ examples:
reg = <0x55>;
interrupts = <2 0>;
gpios = <&gpio1 166 0>;
+
+ touch-overlay {
+ segment-0 {
+ label = "Touchscreen";
+ x-origin = <0>;
+ x-size = <240>;
+ y-origin = <40>;
+ y-size = <280>;
+ };
+
+ segment-1a {
+ label = "Camera light";
+ linux,code = <KEY_LIGHTS_TOGGLE>;
+ x-origin = <40>;
+ x-size = <40>;
+ y-origin = <0>;
+ y-size = <40>;
+ };
+
+ segment-2a {
+ label = "Power";
+ linux,code = <KEY_POWER>;
+ x-origin = <160>;
+ x-size = <40>;
+ y-origin = <0>;
+ y-size = <40>;
+ };
+ };
};
};
--
2.39.2
^ permalink raw reply related
* [PATCH v7 4/4] Input: st1232 - add touch overlays handling
From: Javier Carrasco @ 2024-01-19 7:43 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, Jeff LaBundy
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20240119-feature-ts_virtobj_patch-v7-0-eda70985808f@wolfvision.net>
Use touch-overlay to support overlay objects such as buttons and a resized
frame defined in the device tree.
If buttons are provided, register an additional device to report key
events separately. A key event will be generated if the coordinates
of a touch event are within the area defined by the button properties.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
drivers/input/touchscreen/st1232.c | 48 +++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index 6475084aee1b..4fa31447dbc1 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/touch-overlay.h>
#define ST1232_TS_NAME "st1232-ts"
#define ST1633_TS_NAME "st1633-ts"
@@ -57,6 +58,7 @@ struct st1232_ts_data {
struct dev_pm_qos_request low_latency_req;
struct gpio_desc *reset_gpio;
const struct st_chip_info *chip_info;
+ struct list_head touch_overlay_list;
int read_buf_len;
u8 *read_buf;
};
@@ -138,14 +140,20 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
for (i = 0; i < ts->chip_info->max_fingers; i++) {
u8 *buf = &ts->read_buf[i * 4];
+ bool contact = buf[0] & BIT(7);
+ unsigned int x, y;
- if (buf[0] & BIT(7)) {
- unsigned int x = ((buf[0] & 0x70) << 4) | buf[1];
- unsigned int y = ((buf[0] & 0x07) << 8) | buf[2];
-
- touchscreen_set_mt_pos(&pos[n_contacts],
- &ts->prop, x, y);
+ if (contact) {
+ x = ((buf[0] & 0x70) << 4) | buf[1];
+ y = ((buf[0] & 0x07) << 8) | buf[2];
+ }
+ if (touch_overlay_process_event(&ts->touch_overlay_list, input,
+ contact ? &x : NULL,
+ contact ? &y : NULL, i))
+ continue;
+ if (contact) {
+ touchscreen_set_mt_pos(&pos[n_contacts], &ts->prop, x, y);
/* st1232 includes a z-axis / touch strength */
if (ts->chip_info->have_z)
z[n_contacts] = ts->read_buf[i + 6];
@@ -292,18 +300,30 @@ 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 overlay objects if defined in the device tree */
+ INIT_LIST_HEAD(&ts->touch_overlay_list);
+ error = touch_overlay_map(&ts->touch_overlay_list, input_dev);
+ if (error)
+ return error;
+
+ if (touch_overlay_mapped_touchscreen(&ts->touch_overlay_list)) {
+ /* Read resolution from the overlay touchscreen if defined */
+ touch_overlay_get_touchscreen_abs(&ts->touch_overlay_list,
+ &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,
--
2.39.2
^ permalink raw reply related
* [PATCH v2] Input: cros_ec_keyb: add support for base attached event
From: Ting Shen @ 2024-01-19 7:48 UTC (permalink / raw)
To: LKML
Cc: fshao, Ting Shen, Benson Leung, Dmitry Torokhov,
Greg Kroah-Hartman, Guenter Roeck, Tzung-Bi Shih,
Uwe Kleine-König, chrome-platform,
joewu (吳仲振), linux-input
There is a new type of ChromeOS detachable keyboard that talks to
the host via CrOS EC host command interface, rather than the USB
interface.
To trigger the firmware update daemon (hammerd) on this keyboard, a
signal is required to replace the typical USB hotplug event. This patch
addresses this by mapping the EC's BASE_ATTACHED event to SW_DOCK.
Signed-off-by: Ting Shen <phoenixshen@google.com>
---
Changes in v2:
- update commit message
drivers/input/keyboard/cros_ec_keyb.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 30678a34cf647..d2e0d89d4ffdf 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -128,6 +128,11 @@ static const struct cros_ec_bs_map cros_ec_keyb_bs[] = {
.code = SW_TABLET_MODE,
.bit = EC_MKBP_TABLET_MODE,
},
+ {
+ .ev_type = EV_SW,
+ .code = SW_DOCK,
+ .bit = EC_MKBP_BASE_ATTACHED,
+ },
};
/*
--
2.43.0.429.g432eaa2c6b-goog
^ permalink raw reply related
* Re: [PATCH] Input: cros_ec_keyb: add support for base attached event
From: Dmitry Torokhov @ 2024-01-19 8:13 UTC (permalink / raw)
To: ☆★☆ Ting ☆★☆ Shen
Cc: Ting Shen, LKML, fshao, Benson Leung, Greg Kroah-Hartman,
Guenter Roeck, Tzung-Bi Shih, Uwe Kleine-König,
chrome-platform, joewu (吳仲振), linux-input
In-Reply-To: <CAM106_uNOrpiovEuuyr2yUGXFpQqAiE_GQRFh2_j7v58cEvWbw@mail.gmail.com>
On Fri, Jan 19, 2024 at 03:47:32PM +0800, ☆★☆ Ting ☆★☆ Shen wrote:
> On Fri, Jan 19, 2024 at 1:55 AM Dmitry Torokhov <dmitry.torokhov@gmail.com>
> wrote:
>
> > Hi Ting,
> >
> > On Thu, Jan 18, 2024 at 04:52:43PM +0800, Ting Shen wrote:
> > > This CL maps ChromeOS EC's BASE_ATTACHED event to SW_DOCK,
> >
> > As far as I know none of the Chrome OS devices support classic docking
> > station (not talking about the new USB-C/TB ones).
> >
> > > to allow userspace detect that a keyboard is attached to the
> > > detachable device.
> >
> > Are we planning to have detachable keyboard that is not USB?
> >
> Yes this is for a non-USB keyboard. The keyboard is connected to EC, and EC
> forwards input events to the host.
>
> >
> > EC_MKBP_BASE_ATTACHED is processed by hid-google-hammer driver and,
> > together with the USB connector state, is used to form SW_TABLET_MODE.
> > The reason just the USB state was not enough is because the keyboard
> > could be attached to the lid "other way around" to be used as a stand.
> > In that configuration USB connection was not active. However userspace
> > still needed to know about this to activate tablet version of the UI.
> >
> > Please provide more information why you want this to be exposed via
> > cros_ec_keyb driver.
> >
> > Thanks.
> >
>
> The main purpose of this patch is to have some signal to trigger the
> firmware updater (hammerd), EC_MKBP_BASE_ATTACHED seems suitable.
> EC_MKBP_TABLET_MODE is also usable but may be too noisy, it fires whenever
> the user flips the keyboard.
I think this can be solved in userspace. You may consider emitting
uevent when seeing EC_MKBP_BASE_ATTACHED from the EC. SW_DOCK input event
is not really suitable here as it indicated a docking station being
attached.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: cros_ec_keyb: add support for base attached event
From: Dmitry Torokhov @ 2024-01-19 8:15 UTC (permalink / raw)
To: Ting Shen
Cc: LKML, fshao, Ting Shen, Benson Leung, Greg Kroah-Hartman,
Guenter Roeck, Tzung-Bi Shih, Uwe Kleine-König,
chrome-platform, joewu (吳仲振), linux-input
In-Reply-To: <20240119074831.2979671-1-phoenixshen@google.com>
On Fri, Jan 19, 2024 at 03:48:17PM +0800, Ting Shen wrote:
> There is a new type of ChromeOS detachable keyboard that talks to
> the host via CrOS EC host command interface, rather than the USB
> interface.
>
> To trigger the firmware update daemon (hammerd) on this keyboard, a
> signal is required to replace the typical USB hotplug event. This patch
> addresses this by mapping the EC's BASE_ATTACHED event to SW_DOCK.
I think emitting an uevent on attachment might make more sense here,
SW_DOCK is to signal attachment of a legacy docking station.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: Remove usage of the deprecated ida_simple_xx() API
From: Dmitry Torokhov @ 2024-01-19 8:18 UTC (permalink / raw)
To: Christophe JAILLET; +Cc: linux-kernel, kernel-janitors, linux-input
In-Reply-To: <a885de14beead2cc3c1c946f192b8b178dac696a.1705349930.git.christophe.jaillet@wanadoo.fr>
On Mon, Jan 15, 2024 at 09:19:04PM +0100, Christophe JAILLET wrote:
> ida_alloc() and ida_free() should be preferred to the deprecated
> ida_simple_get() and ida_simple_remove().
>
> Note that the upper limit of ida_simple_get() is exclusive, but the one of
> ida_alloc_range() is inclusive. So a -1 has been added when needed.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: xpad - add Lenovo Legion Go controllers
From: Brenton Simpson @ 2024-01-19 8:22 UTC (permalink / raw)
To: Vicki Pfau
Cc: Dmitry Torokhov, Hans de Goede, Cameron Gutman, Erica Taylor,
Ismael Ferreras Morezuelas, Jonathan Frederick, Matthias Benkmann,
Matthias Berndt, nate, Sam Lantinga, linux-input, linux-kernel,
trivial
In-Reply-To: <da6f1483-1ae2-40b5-9c1e-684321e12288@endrift.com>
Thanks Vicki. I didn't realize they were meant to be sorted.
Would it be appropriate to add comments explaining the sorting? The
second stanza, in particular, is sorted by the IDs rather than
lexicographically. If someone sorted it naively, they'd end up with a
bigger diff than expected.
It looks like a few others have escaped sorting; for instance,
"Microsoft X-Box One Elite 2 pad" appears in the wrong place.
If Dmitry wants to land this and then follow on with a sort + comment
commit (or do that first and then rebase this on top), that would be
great. I can take a stab too if that's helpful.
On Thu, Jan 18, 2024 at 10:16 PM Vicki Pfau <vi@endrift.com> wrote:
>
> Hi Brenton,
>
> On 1/18/24 10:35, Brenton Simpson wrote:
> > The Lenovo Legion Go is a handheld gaming system, similar to a Steam Deck.
> > It has a gamepad (including rear paddles), 3 gyroscopes, a trackpad,
> > volume buttons, a power button, and 2 LED ring lights.
> >
> > The Legion Go firmware presents these controls as a USB hub with various
> > devices attached. In its default state, the gamepad is presented as an
> > Xbox controller connected to this hub. (By holding a combination of
> > buttons, it can be changed to use the older DirectInput API.)
> >
> > This patch teaches the existing Xbox controller module `xpad` to bind to
> > the controller in the Legion Go, which enables support for the:
> >
> > - directional pad,
> > - analog sticks (including clicks),
> > - X, Y, A, B,
> > - start and select (or menu and capture),
> > - shoulder buttons, and
> > - rumble.
> >
> > The trackpad, touchscreen, volume controls, and power button are already
> > supported via existing kernel modules. Two of the face buttons, the
> > gyroscopes, rear paddles, and LEDs are not.
> >
> > After this patch lands, the Legion Go will be mostly functional in Linux,
> > out-of-the-box. The various components of the USB hub can be synthesized
> > into a single logical controller (including the additional buttons) in
> > userspace with [Handheld Daemon](https://github.com/hhd-dev/hhd), which
> > makes the Go fully functional.
> >
> > Signed-off-by: Brenton Simpson <appsforartists@google.com>
> > ---
> > drivers/input/joystick/xpad.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> > index f5c21565bb3c..ecfcea8740a0 100644
> > --- a/drivers/input/joystick/xpad.c
> > +++ b/drivers/input/joystick/xpad.c
> > @@ -127,6 +127,7 @@ static const struct xpad_device {
> > u8 mapping;
> > u8 xtype;
> > } xpad_device[] = {
> > + { 0x17ef, 0x6182, "Lenovo Legion Controller for Windows", 0, XTYPE_XBOX360 },
> > { 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
> > { 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
> > { 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
>
> Please keep the items in this list sorted.
>
> > @@ -459,6 +460,7 @@ static const signed short xpad_btn_paddles[] = {
> >
> > static const struct usb_device_id xpad_table[] = {
> > { USB_INTERFACE_INFO('X', 'B', 0) }, /* Xbox USB-IF not-approved class */
> > + XPAD_XBOX360_VENDOR(0x17ef), /* Lenovo */
> > XPAD_XBOX360_VENDOR(0x0079), /* GPD Win 2 controller */
> > XPAD_XBOX360_VENDOR(0x03eb), /* Wooting Keyboards (Legacy) */
> > XPAD_XBOXONE_VENDOR(0x03f0), /* HP HyperX Xbox One controllers */
>
> Ditto here.
>
> Dmitry will sometimes take patches and fix the sorting after they're
> submitted so you might not to resubmit, but for future reference, this
> list is intended to be sorted.
>
> Vicki
^ permalink raw reply
* Re: [PATCH v3] Input: adc-joystick: Handle inverted axes
From: Dmitry Torokhov @ 2024-01-19 8:29 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-input, contact, hdegoede, paul, peter.hutterer, svv,
biswarupp, Chris Morgan
In-Reply-To: <20240115192752.266367-1-macroalpha82@gmail.com>
Hi Chris,
On Mon, Jan 15, 2024 at 01:27:52PM -0600, Chris Morgan wrote:
>
> +static int adc_joystick_invert(struct input_dev *dev,
> + unsigned int axis, int val)
> +{
> + int min = dev->absinfo[axis].minimum;
> + int max = dev->absinfo[axis].maximum;
I changed this to input_abs_get_[min|max](dev, axis) to avoid peeking
into absinfo and applied.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] dt-bindings: input: silead,gsl1680: do not override firmware-name $ref
From: Dmitry Torokhov @ 2024-01-19 8:33 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel
In-Reply-To: <20240115182057.1610195-1-krzysztof.kozlowski@linaro.org>
On Mon, Jan 15, 2024 at 07:20:57PM +0100, Krzysztof Kozlowski wrote:
> dtschema package defines firmware-name as string-array, so individual
> bindings should not make it a string but instead just narrow the number
> of expected firmware file names.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Hans de Goede @ 2024-01-19 8:44 UTC (permalink / raw)
To: Pavel Machek, Werner Sembach
Cc: Jani Nikula, jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones,
linux-kernel, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds
In-Reply-To: <ZaljwLe7P+dXHEHb@duo.ucw.cz>
Hi,
On 1/18/24 18:45, Pavel Machek wrote:
> Hi!
>
>> We have an upcoming device that has a per-key keyboard backlight, but does
>> the control completely via a wmi/acpi interface. So no usable hidraw here
>> for a potential userspace driver implementation ...
>>
>> So a quick summary for the ideas floating in this thread so far:
>>
>> 1. Expand leds interface allowing arbitrary modes with semi arbitrary
>> optional attributes:
>
>> - Con:
>>
>> - Violates the simplicity paradigm of the leds interface (e.g. with
>> this one leds entry controls possible multiple leds)
>
> Let's not do this.
>
>> 2. Implement per-key keyboards as auxdisplay
>>
>> - Pro:
>>
>> - Already has a concept for led positions
>>
>> - Is conceptually closer to "multiple leds forming a singular entity"
>>
>> - Con:
>>
>> - No preexisting UPower support
>>
>> - No concept for special hardware lightning modes
>>
>> - No support for arbitrary led outlines yet (e.g. ISO style enter-key)
>
> Please do this one.
Ok, so based on the discussion so far and Pavel's feedback lets try to
design a custom userspace API for this. I do not believe that auxdisplay
is a good fit because:
- auxdisplay is just a directory name, it does not seem to clearly
define an API
- instead the deprecated /dev/fb API is used which is deprecated
- auxdisplays are very much displays (hence /dev/fb) they are typically
small LCD displays with a straight widthxheight grid of square pixels
- /dev/fb does gives us nothing for effects, zoned keyboard, etc.
So my proposal would be an ioctl interface (ioctl only no r/w)
using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev.
For per key controllable rgb LEDs we need to discuss a coordinate
system. I propose using a fixed size of 16 rows of 64 keys,
so 64x16 in standard WxH notation.
And then storing RGB in separate bytes, so userspace will then
always send a buffer of 192 bytes per line (64x3) x 14 rows
= 3072 bytes. With the kernel driver ignoring parts of
the buffer where there are no actual keys.
I would then like the map the standard 105 key layout onto this,
starting at x.y (column.row) coordinates of 16.6 (with 0.0 being
the top left). Leaving plenty of space on the left top and right
(and some on the bottom) for extra media key rows, macro keys, etc.
The idea to have the standard layout at a fixed place is to allow
userspace to have a database of preset patterns which will work
everywhere.
Note I say standard 105 key layout, but in reality for
defining the standardized part of the buffer we should
use the maximum amount of keys per row of all the standard layouts,
so for row 6 (the ESC row) and for extra keys on the right outside
the main block we use the standard layout as shown here:
http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg
For the main area of the keyboard looking at:
http://bopqehorizon.weebly.com/uploads/1/3/4/3/134337299/913246919_orig.png
We want to max rows per key, so this means that per row we use
(from the above image) :
row 7: 106/109 - JIS
row 8: 101/104 - ANSI
row 9: 102/105 - ISO
row 10: 104/107 - ABNT
row 11: 106/109 - JIS
(with row 7 being the main area top row)
This way we can address all the possible keys in the various
standard layouts in one standard wat and then the drivers can
just skip keys which are not there when preparing the buffer
to send to the hw / fw.
One open question is if we should add padding after the main
area so that the printscreen / ins / del / leftarrow of the
"middle" block of
http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg
all start at the same x (say 32) or we just pack these directly
after the main area.
And the same question for the numlock block, do we align
this to an x of say 36, or pack it ?
As for the actual IOCTL API I think there should be
the following ioctls:
1. A get-info ioctl returning a struct with the following members:
{
char name[64] /* Keyboard model name / identifier */
int row_begin[16]; /* The x address of the first available key per row. On a std 105key kbd this will be 16 for rows 6-11, 0 for other rows */
int row_end[16]; /* x+1 for the address of the last available key per row, end - begin gives number of keys in a row */
int rgb_zones; /* number of rgb zones for zoned keyboards. Note both
zones and per key addressing may be available if
effects are applied per zone. */
?
}
2. A set-leds ioctl which takes the earlier discussed 3092 bytes buffer
to set all the LEDs at once, only valid if at least one row has a non 0 lenght.
3. A set-zones ioctl which takes an array of bytes sized 3 * number-of-zones
containing RGB values for each zone
4. A enum_effects ioctl which takes a struct with the following members:
{
long size; /* Size of passed in struct including the size member itself */
long effects_mask[]
}
the idea being that there is an enum with effects, which gets extended
as we encounter more effects and the bitmask in effects_mask has a bit set
for each effects enum value which is supported. effects_mask is an array
so that we don't run out of bits. If older userspace only passes 1 long
(size == (2*sizeof(long)) when 2 are needed at some point in the future
then the kernel will simply only fill the first long.
5. A set_effect ioctl which takes a struct with the following members:
{
long size; /* Size of passed in struct including the size member itself */
int effect_nr; /* enum value of the effect to enable, 0 for disable effect */
int zone; /* zone to apply the effect to */
int speed; /* cycle speed of the effect in milli-hz */
char color1[3]; /* effect dependend may be unused. */
char color2[3]; /* effect dependend may be unused. */
}
Again the idea with the size member is that the struct can be extended with
new members if necessary and the kernel will supply a default value for
older userspaces which provide a smaller struct (note size being smaller
then sizeof(struct-v1) will invalid).
Note this is all just a rough sketch suggestions welcome!
Regards,
Hans
^ permalink raw reply
* Re: [PATCH] Input: goodix - Accept ACPI resources with gpio_count == 3 && gpio_int_idx == 0
From: Dmitry Torokhov @ 2024-01-19 8:55 UTC (permalink / raw)
To: Hans de Goede; +Cc: Bastien Nocera, linux-input
In-Reply-To: <20231223141650.10679-1-hdegoede@redhat.com>
On Sat, Dec 23, 2023 at 03:16:50PM +0100, Hans de Goede wrote:
> Some devices list 3 Gpio resources in the ACPI resource list for
> the touchscreen:
>
> 1. GpioInt resource pointing to the GPIO used for the interrupt
> 2. GpioIo resource pointing to the reset GPIO
> 3. GpioIo resource pointing to the GPIO used for the interrupt
>
> Note how the third extra GpioIo resource really is a duplicate
> of the GpioInt provided info.
>
> Ignore this extra GPIO, treating this setup the same as gpio_count == 2 &&
> gpio_int_idx == 0 fixes the touchscreen not working on the Thunderbook
> Colossus W803 rugged tablet and likely also on the CyberBook_T116K.
>
> Reported-by: Maarten van der Schrieck
> Closes: https://gitlab.com/AdyaAdya/goodix-touchscreen-linux-driver/-/issues/22
> Suggested-by: Maarten van der Schrieck
> Tested-by: Maarten van der Schrieck
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Jani Nikula @ 2024-01-19 10:51 UTC (permalink / raw)
To: Hans de Goede, Pavel Machek, Werner Sembach
Cc: jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds,
Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <6bbfdd62-e663-4a45-82f4-445069a8d690@redhat.com>
On Fri, 19 Jan 2024, Hans de Goede <hdegoede@redhat.com> wrote:
> For per key controllable rgb LEDs we need to discuss a coordinate
> system. I propose using a fixed size of 16 rows of 64 keys,
> so 64x16 in standard WxH notation.
>
> And then storing RGB in separate bytes, so userspace will then
> always send a buffer of 192 bytes per line (64x3) x 14 rows
> = 3072 bytes. With the kernel driver ignoring parts of
> the buffer where there are no actual keys.
>
> I would then like the map the standard 105 key layout onto this,
> starting at x.y (column.row) coordinates of 16.6 (with 0.0 being
> the top left). Leaving plenty of space on the left top and right
> (and some on the bottom) for extra media key rows, macro keys, etc.
>
> The idea to have the standard layout at a fixed place is to allow
> userspace to have a database of preset patterns which will work
> everywhere.
>
> Note I say standard 105 key layout, but in reality for
> defining the standardized part of the buffer we should
> use the maximum amount of keys per row of all the standard layouts,
> so for row 6 (the ESC row) and for extra keys on the right outside
> the main block we use the standard layout as shown here:
Doesn't the input stack already have to have pretty much all of this
already covered? I can view the keyboard layout in my desktop
environment, and it's a reasonably accurate match, even if unlikely to
be pixel perfect. But crucially, it has to have all the possible layouts
covered already.
And while I would personally hate it, you can imagine a use case where
you'd like a keypress to have a visual effect around the key you
pressed. A kind of force feedback, if you will. I don't actually know,
and correct me if I'm wrong, but feels like implementing that outside of
the input subsystem would be non-trivial.
Cc: Dmitry, could we at least have some input from the input subsystem
POV on this? AFAICT we have received none.
BR,
Jani.
>
> http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg
>
> For the main area of the keyboard looking at:
>
> http://bopqehorizon.weebly.com/uploads/1/3/4/3/134337299/913246919_orig.png
>
> We want to max rows per key, so this means that per row we use
> (from the above image) :
>
> row 7: 106/109 - JIS
> row 8: 101/104 - ANSI
> row 9: 102/105 - ISO
> row 10: 104/107 - ABNT
> row 11: 106/109 - JIS
>
> (with row 7 being the main area top row)
>
> This way we can address all the possible keys in the various
> standard layouts in one standard wat and then the drivers can
> just skip keys which are not there when preparing the buffer
> to send to the hw / fw.
>
> One open question is if we should add padding after the main
> area so that the printscreen / ins / del / leftarrow of the
> "middle" block of
>
> http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg
>
> all start at the same x (say 32) or we just pack these directly
> after the main area.
>
> And the same question for the numlock block, do we align
> this to an x of say 36, or pack it ?
>
>
> As for the actual IOCTL API I think there should be
> the following ioctls:
>
> 1. A get-info ioctl returning a struct with the following members:
>
> {
> char name[64] /* Keyboard model name / identifier */
> int row_begin[16]; /* The x address of the first available key per row. On a std 105key kbd this will be 16 for rows 6-11, 0 for other rows */
> int row_end[16]; /* x+1 for the address of the last available key per row, end - begin gives number of keys in a row */
> int rgb_zones; /* number of rgb zones for zoned keyboards. Note both
> zones and per key addressing may be available if
> effects are applied per zone. */
> ?
> }
>
> 2. A set-leds ioctl which takes the earlier discussed 3092 bytes buffer
> to set all the LEDs at once, only valid if at least one row has a non 0 lenght.
>
> 3. A set-zones ioctl which takes an array of bytes sized 3 * number-of-zones
> containing RGB values for each zone
>
> 4. A enum_effects ioctl which takes a struct with the following members:
>
> {
> long size; /* Size of passed in struct including the size member itself */
> long effects_mask[]
> }
>
> the idea being that there is an enum with effects, which gets extended
> as we encounter more effects and the bitmask in effects_mask has a bit set
> for each effects enum value which is supported. effects_mask is an array
> so that we don't run out of bits. If older userspace only passes 1 long
> (size == (2*sizeof(long)) when 2 are needed at some point in the future
> then the kernel will simply only fill the first long.
>
> 5. A set_effect ioctl which takes a struct with the following members:
>
> {
> long size; /* Size of passed in struct including the size member itself */
> int effect_nr; /* enum value of the effect to enable, 0 for disable effect */
> int zone; /* zone to apply the effect to */
> int speed; /* cycle speed of the effect in milli-hz */
> char color1[3]; /* effect dependend may be unused. */
> char color2[3]; /* effect dependend may be unused. */
> }
>
> Again the idea with the size member is that the struct can be extended with
> new members if necessary and the kernel will supply a default value for
> older userspaces which provide a smaller struct (note size being smaller
> then sizeof(struct-v1) will invalid).
>
>
> Note this is all just a rough sketch suggestions welcome!
>
> Regards,
>
> Hans
>
>
>
--
Jani Nikula, Intel
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2024-01-19 16:04 UTC (permalink / raw)
To: Hans de Goede, Pavel Machek
Cc: Jani Nikula, jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones,
linux-kernel, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds
In-Reply-To: <6bbfdd62-e663-4a45-82f4-445069a8d690@redhat.com>
Hi,
sorry have to resend, thunderbird html-ified the mail
Am 19.01.24 um 09:44 schrieb Hans de Goede:
> Hi,
>
> On 1/18/24 18:45, Pavel Machek wrote:
>> Hi!
>>
>>> We have an upcoming device that has a per-key keyboard backlight, but does
>>> the control completely via a wmi/acpi interface. So no usable hidraw here
>>> for a potential userspace driver implementation ...
>>>
>>> So a quick summary for the ideas floating in this thread so far:
>>>
>>> 1. Expand leds interface allowing arbitrary modes with semi arbitrary
>>> optional attributes:
>>> - Con:
>>>
>>> - Violates the simplicity paradigm of the leds interface (e.g. with
>>> this one leds entry controls possible multiple leds)
>> Let's not do this.
>>
>>> 2. Implement per-key keyboards as auxdisplay
>>>
>>> - Pro:
>>>
>>> - Already has a concept for led positions
>>>
>>> - Is conceptually closer to "multiple leds forming a singular entity"
>>>
>>> - Con:
>>>
>>> - No preexisting UPower support
>>>
>>> - No concept for special hardware lightning modes
>>>
>>> - No support for arbitrary led outlines yet (e.g. ISO style enter-key)
>> Please do this one.
> Ok, so based on the discussion so far and Pavel's feedback lets try to
> design a custom userspace API for this. I do not believe that auxdisplay
> is a good fit because:
>
> - auxdisplay is just a directory name, it does not seem to clearly
> define an API
>
> - instead the deprecated /dev/fb API is used which is deprecated
>
> - auxdisplays are very much displays (hence /dev/fb) they are typically
> small LCD displays with a straight widthxheight grid of square pixels
>
> - /dev/fb does gives us nothing for effects, zoned keyboard, etc.
I was just checking this and wanted to write something similar. When I wrote the
pro/con list I was mistaken that aux displays use either one of 2 APIs (charlcd
or fb), but I was mistaken. The 8 devices implemented there are actually using 5
different apis, some of them 2 at a time.
Just for reference the small list I wrote on the side just now:
arm-charlcd.c - own implementation without userspace interaction (just a static
text is displayed)
cfag12864b.c/cfag12864bfb.c - ks0108_isinited or register_framebuffer
hd44780.c - charlcd_register
ht16k33.c - linedisp_register or register_framebuffer
img-ascii-lcd.c - linedisp_register
ks0108.c - own implementetion using parport_register_dev_model
lcd2s.c - charlcd_register
panel.c - charlcd_register
> So my proposal would be an ioctl interface (ioctl only no r/w)
> using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev.
>
> For per key controllable rgb LEDs we need to discuss a coordinate
> system. I propose using a fixed size of 16 rows of 64 keys,
> so 64x16 in standard WxH notation.
>
> And then storing RGB in separate bytes, so userspace will then
> always send a buffer of 192 bytes per line (64x3) x 14 rows
> = 3072 bytes. With the kernel driver ignoring parts of
> the buffer where there are no actual keys.
The be sure the "14 rows" is a typo? And should be 16 rows?
> I would then like the map the standard 105 key layout onto this,
> starting at x.y (column.row) coordinates of 16.6 (with 0.0 being
> the top left). Leaving plenty of space on the left top and right
> (and some on the bottom) for extra media key rows, macro keys, etc.
>
> The idea to have the standard layout at a fixed place is to allow
> userspace to have a database of preset patterns which will work
> everywhere.
>
> Note I say standard 105 key layout, but in reality for
> defining the standardized part of the buffer we should
> use the maximum amount of keys per row of all the standard layouts,
> so for row 6 (the ESC row) and for extra keys on the right outside
> the main block we use the standard layout as shown here:
>
> http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg
>
> For the main area of the keyboard looking at:
>
> http://bopqehorizon.weebly.com/uploads/1/3/4/3/134337299/913246919_orig.png
>
> We want to max rows per key, so this means that per row we use
> (from the above image) :
>
> row 7: 106/109 - JIS
> row 8: 101/104 - ANSI
> row 9: 102/105 - ISO
> row 10: 104/107 - ABNT
> row 11: 106/109 - JIS
>
> (with row 7 being the main area top row)
>
> This way we can address all the possible keys in the various
> standard layouts in one standard wat and then the drivers can
> just skip keys which are not there when preparing the buffer
> to send to the hw / fw.
Some remarks here:
- Some keyboards might have two or more leds for big keys like (iso-)enter,
shift, capslock, num+, etc. that in theory are individually controllable by the
firmware. In windows drivers this is usually abstracted away, but could be
interesting for effects (e.g. if the top of iso-enter is separate from the
bottom of iso-enter like with one of our devices).
- In combination with this: The driver might not be able to tell if the actual
physical keyboard is ISO or ANSI, so it might not be able the correctly assign
the leds around enter correctly as being an own key or being part of ANSI- or
ISO-enter.
- Should the interface have different addresses for the different enter and num+
styles (or even the different length shifts and spacebars)?
One idea for this: Actually assign 1 value per line for tall keys per line, 3
(or maybe even 4, to have one spare) values per line for wide keys and 6 (or 8)
values for space. e.g.:
- Right shift would have 3 values in row 10. The first value might be the left
side of shift or the additional ABNT/JIS key. The 2nd Key might be the left side
or middle of shift and the third key might be the right side of shift or the
only value for the whole key. The additional ABNT/JIS key still also has a
dedicated value which is used by drivers which can differentiate between
physical layouts.
- Enter would have 3 values in row 8 and 3 values in row 9. With the same
disambiguation as the additional ABNT/JIS but this time for ansii-/ and iso-#
- Num+ would have 2 values, one row 8 and one in row 9. The one in row 9 might
control the whole key or might just control the lower half. The one in row 8
might be another key or the upper half
For the left half if the main block the leftmost value should be the "might be
the only relevant"-value while the right most value should be the "might be
another key"-value. For the right side of the main block this should be swapped.
Unused values should be adjacent to the "might be another key"-value, e.g.:
| Left shift value 1 | Left shift value 2 | Left shift value 3 | Left shift value 4 | 102nd key value
ISO/ANSI aware | Left shift color | Unused | Unused | Unused | 102nd key color
ISO non aware 1 led under shift | Left shift color | Unused | Unused | 102nd key color | Unused
ANSI non aware 1 led under shift | Left shift color | Unused | Unused | Unused | Unused
ISO non aware 2 leds under shift | Left shift left color | Left shift right color | Unused | 102nd key color | Unused
ANSI non aware 2 leds under shift | Left shift left color | Left shift right color | Unused | Unused | Unused
ISO non aware 3 leds under shift | Left shift left color | Left shift middle color | Left shift right color | 102nd key color | Unused
ANSI non aware 3 leds under shift | Left shift left color | Left shift middle color | Unused | Left shift right color | Unused
ANSI non aware 4 leds under shift | Left shift left color | Left shift middle left color | Left shift middle right color | Left shift right color | Unused
Like this with no information you can still reliable target the ANSI-shift
space, if you know it's an ISO keyboard from user space you can target shift and
102nd key, and if you have even more information you can have multi color shift
if the firmware supports it.
> One open question is if we should add padding after the main
> area so that the printscreen / ins / del / leftarrow of the
> "middle" block of
>
> http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg
>
> all start at the same x (say 32) or we just pack these directly
> after the main area.
>
> And the same question for the numlock block, do we align
> this to an x of say 36, or pack it ?
With all that padding around I think a little padding in the middle wouldn't
hurt. Would even suggest a min padding of 1 to have some reserved space in there.
> As for the actual IOCTL API I think there should be
> the following ioctls:
>
> 1. A get-info ioctl returning a struct with the following members:
>
> {
> char name[64] /* Keyboard model name / identifier */
> int row_begin[16]; /* The x address of the first available key per row. On a std 105key kbd this will be 16 for rows 6-11, 0 for other rows */
> int row_end[16]; /* x+1 for the address of the last available key per row, end - begin gives number of keys in a row */
I guess you meant x-1 for the address, aka row_end[16] points to the address
behind the last value so that you can iterate over the row with: i = row_begin;
i < row_end; ++i
> int rgb_zones; /* number of rgb zones for zoned keyboards. Note both
> zones and per key addressing may be available if
> effects are applied per zone. */
> ?
> }
>
> 2. A set-leds ioctl which takes the earlier discussed 3092 bytes buffer
> to set all the LEDs at once, only valid if at least one row has a non 0 lenght.
>
> 3. A set-zones ioctl which takes an array of bytes sized 3 * number-of-zones
> containing RGB values for each zone
>
> 4. A enum_effects ioctl which takes a struct with the following members:
>
> {
> long size; /* Size of passed in struct including the size member itself */
> long effects_mask[]
> }
>
> the idea being that there is an enum with effects, which gets extended
> as we encounter more effects and the bitmask in effects_mask has a bit set
> for each effects enum value which is supported. effects_mask is an array
> so that we don't run out of bits. If older userspace only passes 1 long
> (size == (2*sizeof(long)) when 2 are needed at some point in the future
> then the kernel will simply only fill the first long.
>
> 5. A set_effect ioctl which takes a struct with the following members:
>
> {
> long size; /* Size of passed in struct including the size member itself */
> int effect_nr; /* enum value of the effect to enable, 0 for disable effect */
> int zone; /* zone to apply the effect to */
Don't know if this is necessary, the keyboards I have seen so far apply firmware
effects globally.
> int speed; /* cycle speed of the effect in milli-hz */
I would split this into speed and speed_max and don't specify an actual unit.
The firmwares effects I have seen so far: If they have a speed value, it's some
low number interpreted as a proportional x/n * the max speed of this effect,
with n being some low number like 8 or 10.
But i don't know if such clearly named properties are even sensefull, see below.
> char color1[3]; /* effect dependend may be unused. */
> char color2[3]; /* effect dependend may be unused. */
> }
We can not predetermine how many colors we might need in the future.
Firmware effects can vary vastly in complexity, e.g. breathing can be a single
bit switch that just varies the brightness of whatever color setting is
currently applied. It could have an optional speed argument. It could have nth
additional color arguments to cycle through, it could have an optional randomize
bit that either randomizes the order of the defined colors or means that it is
picking completely random color ignoring the color settings if set.
Like this we could have a very fast explosion of the effects enum e.g.:
breathing, breathing_2_colors, breathing_3_colors, ... breathing_n_colors,
breathing_speed_controlled, breathing_speed_controlled_2_colors, ...
breathing_speed_controlled_n_colors_random_bit, etc.
Or we give up on generic names and just make something like:
tongfang_breathing_1, tongfang_scan_1, tongfang_breathing_2, clevo_breathing_1
Each with an own struct defined in a big .h file.
Otherwise I think the config struct needs to be dynamically created out of
information the driver gives to userspace.
> Again the idea with the size member is that the struct can be extended with
> new members if necessary and the kernel will supply a default value for
> older userspaces which provide a smaller struct (note size being smaller
> then sizeof(struct-v1) will invalid).
>
>
> Note this is all just a rough sketch suggestions welcome!
>
> Regards,
>
> Hans
>
>
>
Regards,
Werner
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2024-01-19 16:06 UTC (permalink / raw)
To: Jani Nikula, Hans de Goede, Pavel Machek
Cc: jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds,
Dmitry Torokhov, Benjamin Tissoires
In-Reply-To: <87bk9hppee.fsf@intel.com>
Hi,
Am 19.01.24 um 11:51 schrieb Jani Nikula:
> On Fri, 19 Jan 2024, Hans de Goede <hdegoede@redhat.com> wrote:
>> For per key controllable rgb LEDs we need to discuss a coordinate
>> system. I propose using a fixed size of 16 rows of 64 keys,
>> so 64x16 in standard WxH notation.
>>
>> And then storing RGB in separate bytes, so userspace will then
>> always send a buffer of 192 bytes per line (64x3) x 14 rows
>> = 3072 bytes. With the kernel driver ignoring parts of
>> the buffer where there are no actual keys.
>>
>> I would then like the map the standard 105 key layout onto this,
>> starting at x.y (column.row) coordinates of 16.6 (with 0.0 being
>> the top left). Leaving plenty of space on the left top and right
>> (and some on the bottom) for extra media key rows, macro keys, etc.
>>
>> The idea to have the standard layout at a fixed place is to allow
>> userspace to have a database of preset patterns which will work
>> everywhere.
>>
>> Note I say standard 105 key layout, but in reality for
>> defining the standardized part of the buffer we should
>> use the maximum amount of keys per row of all the standard layouts,
>> so for row 6 (the ESC row) and for extra keys on the right outside
>> the main block we use the standard layout as shown here:
> Doesn't the input stack already have to have pretty much all of this
> already covered? I can view the keyboard layout in my desktop
> environment, and it's a reasonably accurate match, even if unlikely to
> be pixel perfect. But crucially, it has to have all the possible layouts
> covered already.
>
> And while I would personally hate it, you can imagine a use case where
> you'd like a keypress to have a visual effect around the key you
> pressed. A kind of force feedback, if you will. I don't actually know,
> and correct me if I'm wrong, but feels like implementing that outside of
> the input subsystem would be non-trivial.
>
> Cc: Dmitry, could we at least have some input from the input subsystem
> POV on this? AFAICT we have received none.
>
>
> BR,
> Jani.
Don't forget: while we are currently discussing keyboards, in the future this
API imho should also be usefull for other RGB devices like mice, lightbars, etc.
Regards,
Werner
>
>
>> http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg
>>
>> For the main area of the keyboard looking at:
>>
>> http://bopqehorizon.weebly.com/uploads/1/3/4/3/134337299/913246919_orig.png
>>
>> We want to max rows per key, so this means that per row we use
>> (from the above image) :
>>
>> row 7: 106/109 - JIS
>> row 8: 101/104 - ANSI
>> row 9: 102/105 - ISO
>> row 10: 104/107 - ABNT
>> row 11: 106/109 - JIS
>>
>> (with row 7 being the main area top row)
>>
>> This way we can address all the possible keys in the various
>> standard layouts in one standard wat and then the drivers can
>> just skip keys which are not there when preparing the buffer
>> to send to the hw / fw.
>>
>> One open question is if we should add padding after the main
>> area so that the printscreen / ins / del / leftarrow of the
>> "middle" block of
>>
>> http://www.maxkeyboard.com/images/105_ISO_6_25_Key_Layout.jpg
>>
>> all start at the same x (say 32) or we just pack these directly
>> after the main area.
>>
>> And the same question for the numlock block, do we align
>> this to an x of say 36, or pack it ?
>>
>>
>> As for the actual IOCTL API I think there should be
>> the following ioctls:
>>
>> 1. A get-info ioctl returning a struct with the following members:
>>
>> {
>> char name[64] /* Keyboard model name / identifier */
>> int row_begin[16]; /* The x address of the first available key per row. On a std 105key kbd this will be 16 for rows 6-11, 0 for other rows */
>> int row_end[16]; /* x+1 for the address of the last available key per row, end - begin gives number of keys in a row */
>> int rgb_zones; /* number of rgb zones for zoned keyboards. Note both
>> zones and per key addressing may be available if
>> effects are applied per zone. */
>> ?
>> }
>>
>> 2. A set-leds ioctl which takes the earlier discussed 3092 bytes buffer
>> to set all the LEDs at once, only valid if at least one row has a non 0 lenght.
>>
>> 3. A set-zones ioctl which takes an array of bytes sized 3 * number-of-zones
>> containing RGB values for each zone
>>
>> 4. A enum_effects ioctl which takes a struct with the following members:
>>
>> {
>> long size; /* Size of passed in struct including the size member itself */
>> long effects_mask[]
>> }
>>
>> the idea being that there is an enum with effects, which gets extended
>> as we encounter more effects and the bitmask in effects_mask has a bit set
>> for each effects enum value which is supported. effects_mask is an array
>> so that we don't run out of bits. If older userspace only passes 1 long
>> (size == (2*sizeof(long)) when 2 are needed at some point in the future
>> then the kernel will simply only fill the first long.
>>
>> 5. A set_effect ioctl which takes a struct with the following members:
>>
>> {
>> long size; /* Size of passed in struct including the size member itself */
>> int effect_nr; /* enum value of the effect to enable, 0 for disable effect */
>> int zone; /* zone to apply the effect to */
>> int speed; /* cycle speed of the effect in milli-hz */
>> char color1[3]; /* effect dependend may be unused. */
>> char color2[3]; /* effect dependend may be unused. */
>> }
>>
>> Again the idea with the size member is that the struct can be extended with
>> new members if necessary and the kernel will supply a default value for
>> older userspaces which provide a smaller struct (note size being smaller
>> then sizeof(struct-v1) will invalid).
>>
>>
>> Note this is all just a rough sketch suggestions welcome!
>>
>> Regards,
>>
>> Hans
>>
>>
>>
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Dmitry Torokhov @ 2024-01-19 18:33 UTC (permalink / raw)
To: Jani Nikula
Cc: Hans de Goede, Pavel Machek, Werner Sembach, jikos,
Jelle van der Waa, Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds,
Benjamin Tissoires
In-Reply-To: <87bk9hppee.fsf@intel.com>
On Fri, Jan 19, 2024 at 12:51:21PM +0200, Jani Nikula wrote:
> On Fri, 19 Jan 2024, Hans de Goede <hdegoede@redhat.com> wrote:
> > For per key controllable rgb LEDs we need to discuss a coordinate
> > system. I propose using a fixed size of 16 rows of 64 keys,
> > so 64x16 in standard WxH notation.
> >
> > And then storing RGB in separate bytes, so userspace will then
> > always send a buffer of 192 bytes per line (64x3) x 14 rows
> > = 3072 bytes. With the kernel driver ignoring parts of
> > the buffer where there are no actual keys.
> >
> > I would then like the map the standard 105 key layout onto this,
> > starting at x.y (column.row) coordinates of 16.6 (with 0.0 being
> > the top left). Leaving plenty of space on the left top and right
> > (and some on the bottom) for extra media key rows, macro keys, etc.
> >
> > The idea to have the standard layout at a fixed place is to allow
> > userspace to have a database of preset patterns which will work
> > everywhere.
> >
> > Note I say standard 105 key layout, but in reality for
> > defining the standardized part of the buffer we should
> > use the maximum amount of keys per row of all the standard layouts,
> > so for row 6 (the ESC row) and for extra keys on the right outside
> > the main block we use the standard layout as shown here:
>
> Doesn't the input stack already have to have pretty much all of this
> already covered? I can view the keyboard layout in my desktop
> environment, and it's a reasonably accurate match, even if unlikely to
> be pixel perfect. But crucially, it has to have all the possible layouts
> covered already.
The kernel actually is not aware of the keyboard geometry, it had no
idea if you are dealing with a standard full 101/102 keys keyboard,
TKL or even smaller one, if it is split or not, maybe something like
Kinesis Advantage360. Arguably, it could potentially know about
101/TLK if vendors would program accurate descriptors into their
devices, but nobody does... And geometry is not a part of HID interface
at all. So your desktop environment makes an [un]educated guess.
>
> And while I would personally hate it, you can imagine a use case where
> you'd like a keypress to have a visual effect around the key you
> pressed. A kind of force feedback, if you will. I don't actually know,
> and correct me if I'm wrong, but feels like implementing that outside of
> the input subsystem would be non-trivial.
Actually I think it does not belong to the input subsystem as it is,
where the goal is to deliver keystrokes and gestures to userspace. The
"force feedback" kind of fits, but not really practical, again because
of lack of geometry info. It is also not really essential to be fully
and automatically handled by the kernel. So I think the best way is to
have an API that is flexible enough for the userspace solution to
control, and that is not restricted by the input core design. The
hardware drivers are not restricted to using a single API, they can
implement both an input device and whatever new "rgbled" and userspace
can associate them by topology/sysfs.
>
> Cc: Dmitry, could we at least have some input from the input subsystem
> POV on this? AFAICT we have received none.
Sorry, I was not CCed and I missed this on the mainling list.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH -next] Input: bcm-keypad - Remove redundant of_match_ptr()
From: Dmitry Torokhov @ 2024-01-19 19:22 UTC (permalink / raw)
To: Ruan Jinjie
Cc: linux-input, krzysztof.kozlowski, hdegoede, andy.shevchenko,
frank.li
In-Reply-To: <20230809101626.2664651-1-ruanjinjie@huawei.com>
On Wed, Aug 09, 2023 at 06:16:26PM +0800, Ruan Jinjie wrote:
> The driver depends on CONFIG_OF, it is not necessary to use
> of_match_ptr() here.
>
> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Remove some unused functions
From: Silvan Jegen @ 2024-01-19 20:12 UTC (permalink / raw)
To: Jiapeng Chong
Cc: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel,
Abaci Robot
In-Reply-To: <20240119072729.54499-1-jiapeng.chong@linux.alibaba.com>
Jiapeng Chong <jiapeng.chong@linux.alibaba.com> wrote:
> These functions are defined in the hid-nintendo.c file, but not called
> elsewhere, so delete these unused functions.
>
> drivers/hid/hid-nintendo.c:670:20: warning: unused function 'joycon_device_is_left_joycon'.
> drivers/hid/hid-nintendo.c:674:20: warning: unused function 'joycon_device_is_right_joycon'
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7992
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> ---
> drivers/hid/hid-nintendo.c | 10 ----------
> 1 file changed, 10 deletions(-)
Reviewed-by: Silvan Jegen <s.jegen@gmail.com>
>
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index ccc4032fb2b0..7ce6be0a8dee 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -667,16 +667,6 @@ struct joycon_ctlr {
> * These helpers are most useful early during the HID probe or in conjunction
> * with the capability helpers below.
> */
> -static inline bool joycon_device_is_left_joycon(struct joycon_ctlr *ctlr)
> -{
> - return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONL;
> -}
> -
> -static inline bool joycon_device_is_right_joycon(struct joycon_ctlr *ctlr)
> -{
> - return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONR;
> -}
> -
> static inline bool joycon_device_is_procon(struct joycon_ctlr *ctlr)
> {
> return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_PROCON;
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Pavel Machek @ 2024-01-19 20:15 UTC (permalink / raw)
To: Hans de Goede
Cc: Werner Sembach, Jani Nikula, jikos, Jelle van der Waa,
Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds
In-Reply-To: <6bbfdd62-e663-4a45-82f4-445069a8d690@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2494 bytes --]
Hi!
> >> 2. Implement per-key keyboards as auxdisplay
> >>
> >> - Pro:
> >>
> >> - Already has a concept for led positions
> >>
> >> - Is conceptually closer to "multiple leds forming a singular entity"
> >>
> >> - Con:
> >>
> >> - No preexisting UPower support
> >>
> >> - No concept for special hardware lightning modes
> >>
> >> - No support for arbitrary led outlines yet (e.g. ISO style enter-key)
> >
> > Please do this one.
>
> Ok, so based on the discussion so far and Pavel's feedback lets try to
> design a custom userspace API for this. I do not believe that auxdisplay
> is a good fit because:
Ok, so lets call this a "display". These days, framebuffers and drm
handles displays. My proposal is to use similar API as other displays.
> So my proposal would be an ioctl interface (ioctl only no r/w)
> using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev.
>
> For per key controllable rgb LEDs we need to discuss a coordinate
> system. I propose using a fixed size of 16 rows of 64 keys,
> so 64x16 in standard WxH notation.
>
> And then storing RGB in separate bytes, so userspace will then
> always send a buffer of 192 bytes per line (64x3) x 14 rows
> = 3072 bytes. With the kernel driver ignoring parts of
> the buffer where there are no actual keys.
That's really really weird interface. If you are doing RGB888 64x14,
lets make it a ... display? :-).
ioctl always sending 3072 bytes is really a hack.
Small displays exist and are quite common, surely we'd handle this as
a display:
https://pajenicko.cz/displeje/graficky-oled-displej-0-66-64x48-i2c-bily-wemos-d1-mini
It is 64x48.
And then there's this:
https://pajenicko.cz/displeje/maticovy-8x8-led-displej-s-radicem-max7219
and this:
https://pajenicko.cz/displeje/maticovy-8x32-led-displej-s-radicem-max7219
One of them is 8x8.
Surely those should be displays, too?
And yes, we'd probably want some extra ioctls on top, for example to
map from input device to this and back, and maybe for various effects,
too. And yes, I realize that display with holes in it and with some
pixels bigger than others is weird, but it still looks like a display
to me. (And phones have high-res displays with rounded corners and
holes in them, so... we'll need to deal with weird displays anyway).
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2024-01-19 20:22 UTC (permalink / raw)
To: Pavel Machek, Hans de Goede
Cc: Jani Nikula, jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones,
linux-kernel, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds
In-Reply-To: <ZarYSkzISyS+wuYR@duo.ucw.cz>
Hi,
Am 19.01.24 um 21:15 schrieb Pavel Machek:
> Hi!
>
>>>> 2. Implement per-key keyboards as auxdisplay
>>>>
>>>> - Pro:
>>>>
>>>> - Already has a concept for led positions
>>>>
>>>> - Is conceptually closer to "multiple leds forming a singular entity"
>>>>
>>>> - Con:
>>>>
>>>> - No preexisting UPower support
>>>>
>>>> - No concept for special hardware lightning modes
>>>>
>>>> - No support for arbitrary led outlines yet (e.g. ISO style enter-key)
>>> Please do this one.
>> Ok, so based on the discussion so far and Pavel's feedback lets try to
>> design a custom userspace API for this. I do not believe that auxdisplay
>> is a good fit because:
> Ok, so lets call this a "display". These days, framebuffers and drm
> handles displays. My proposal is to use similar API as other displays.
>
>> So my proposal would be an ioctl interface (ioctl only no r/w)
>> using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev.
>>
>> For per key controllable rgb LEDs we need to discuss a coordinate
>> system. I propose using a fixed size of 16 rows of 64 keys,
>> so 64x16 in standard WxH notation.
>>
>> And then storing RGB in separate bytes, so userspace will then
>> always send a buffer of 192 bytes per line (64x3) x 14 rows
>> = 3072 bytes. With the kernel driver ignoring parts of
>> the buffer where there are no actual keys.
> That's really really weird interface. If you are doing RGB888 64x14,
> lets make it a ... display? :-).
>
> ioctl always sending 3072 bytes is really a hack.
>
> Small displays exist and are quite common, surely we'd handle this as
> a display:
> https://pajenicko.cz/displeje/graficky-oled-displej-0-66-64x48-i2c-bily-wemos-d1-mini
> It is 64x48.
>
> And then there's this:
> https://pajenicko.cz/displeje/maticovy-8x8-led-displej-s-radicem-max7219
> and this:
> https://pajenicko.cz/displeje/maticovy-8x32-led-displej-s-radicem-max7219
>
> One of them is 8x8.
>
> Surely those should be displays, too?
But what about a light bar with, lets say, 3 zones. Is that a 3x1 display?
And what about a mouse having lit mousebuttons and a single led light bar at the
wrist: a 2x2 display, but one is thin but long and one is not used?
Regards,
Werner
>
> And yes, we'd probably want some extra ioctls on top, for example to
> map from input device to this and back, and maybe for various effects,
> too. And yes, I realize that display with holes in it and with some
> pixels bigger than others is weird, but it still looks like a display
> to me. (And phones have high-res displays with rounded corners and
> holes in them, so... we'll need to deal with weird displays anyway).
>
> Best regards,
> Pavel
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Pavel Machek @ 2024-01-19 20:32 UTC (permalink / raw)
To: Werner Sembach
Cc: Hans de Goede, Jani Nikula, jikos, Jelle van der Waa,
Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds
In-Reply-To: <36973f9d-bf67-417d-998c-ce24c38322c3@tuxedocomputers.com>
[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]
Hi!
> > > And then storing RGB in separate bytes, so userspace will then
> > > always send a buffer of 192 bytes per line (64x3) x 14 rows
> > > = 3072 bytes. With the kernel driver ignoring parts of
> > > the buffer where there are no actual keys.
> > That's really really weird interface. If you are doing RGB888 64x14,
> > lets make it a ... display? :-).
> >
> > ioctl always sending 3072 bytes is really a hack.
> >
> > Small displays exist and are quite common, surely we'd handle this as
> > a display:
> > https://pajenicko.cz/displeje/graficky-oled-displej-0-66-64x48-i2c-bily-wemos-d1-mini
> > It is 64x48.
> >
> > And then there's this:
> > https://pajenicko.cz/displeje/maticovy-8x8-led-displej-s-radicem-max7219
> > and this:
> > https://pajenicko.cz/displeje/maticovy-8x32-led-displej-s-radicem-max7219
> >
> > One of them is 8x8.
> >
> > Surely those should be displays, too?
>
> But what about a light bar with, lets say, 3 zones. Is that a 3x1 display?
>
> And what about a mouse having lit mousebuttons and a single led light bar at
> the wrist: a 2x2 display, but one is thin but long and one is not used?
So indeed LEDs can arranged into various shapes. Like a ring, or this:
* *
* * *
* *
https://pajenicko.cz/led-moduly?page=2
Dunno. Sounds like a display is still a best match for them. Some of
modules are RGB, some are single-color only, I'm sure there will be
various bit depths.
I guess we can do 3x1 and 2x2 displays. Or we could try to solve
keyboards and ignore those for now.
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox