* Re: [PATCH] input: uinput: Drop checks for abs_min > abs_max
From: Dmitry Torokhov @ 2023-12-24 8:03 UTC (permalink / raw)
To: Hans de Goede
Cc: Paul Cercueil, Peter Hutterer, Chris Morgan, linux-input, svv,
biswarupp, contact, Chris Morgan
In-Reply-To: <63ad6de2-8f24-47c1-b00d-588c22f6877f@redhat.com>
On Sat, Dec 23, 2023 at 04:16:46PM +0100, Hans de Goede wrote:
> Hi,
>
> On 12/23/23 16:01, Paul Cercueil wrote:
> > Hi Hans,
> >
> > Le samedi 23 décembre 2023 à 15:29 +0100, Hans de Goede a écrit :
> >> Hi Paul,
> >>
> >> On 12/20/23 14:39, Paul Cercueil wrote:
> >>> Hi Dmitry,
> >>>
> >>> Le mardi 19 décembre 2023 à 17:53 -0800, Dmitry Torokhov a écrit :
> >>>> Hi Paul,
> >>>>
> >>>> On Wed, Dec 20, 2023 at 01:38:39AM +0100, Paul Cercueil wrote:
> >>>>> Hi Peter,
> >>>>>
> >>>>> Le mercredi 20 décembre 2023 à 09:51 +1000, Peter Hutterer a
> >>>>> écrit :
> >>>>>> On Mon, Dec 18, 2023 at 11:16:53AM -0600, Chris Morgan wrote:
> >>>>>>> From: Chris Morgan <macromorgan@hotmail.com>
> >>>>>>>
> >>>>>>> Stop checking if the minimum abs value is greater than the
> >>>>>>> maximum
> >>>>>>> abs
> >>>>>>> value. When the axis is inverted this condition is allowed.
> >>>>>>> Without
> >>>>>>> relaxing this check, it is not possible to use uinput on
> >>>>>>> devices in
> >>>>>>> userspace with an inverted axis, such as the adc-joystick
> >>>>>>> found
> >>>>>>> on
> >>>>>>> many handheld gaming devices.
> >>>>>>
> >>>>>> As mentioned in the other thread [1] a fair bit of userspace
> >>>>>> relies
> >>>>>> on
> >>>>>> that general assumption so removing it will likely cause all
> >>>>>> sorts of
> >>>>>> issues.
> >>>>>
> >>>>> There is some userspace that works with it though, so why
> >>>>> restrict
> >>>>> it
> >>>>> artificially?
> >>>>>
> >>>>> The fact that some other userspace code wouldn't work with it
> >>>>> sounds a
> >>>>> bit irrelevant. They just never encountered that min>max usage
> >>>>> before.
> >>>>>
> >>>>> And removing this check won't cause all sort of issues, why
> >>>>> would
> >>>>> it?
> >>>>> It's not like the current software actively probes min>max and
> >>>>> crash
> >>>>> badly if it doesn't return -EINVAL...
> >>>>
> >>>> It will cause weird movements because calculations expect min be
> >>>> the
> >>>> minimum, and max the maximum, and not encode left/right or
> >>>> up/down.
> >>>> Putting this into adc joystick binding was a mistake.
> >>>
> >>> I don't see why it was a mistake, it's only one of the ways to
> >>> specify
> >>> that the axis is inverted. This information is between the firmware
> >>> (DT) and the kernel, that doesn't mean the information has to be
> >>> relayed as-is to the userspace.
> >>>
> >>> Unlike what you wrote in your other answer, when talking about
> >>> input
> >>> the kernel doesn't really normalize anything - it gives you the
> >>> min/max
> >>> values, and the raw samples, not normalized samples (they don't get
> >>> translated to a pre-specified range, or even clamped).
> >>>
> >>> I don't really like the idea of having the driver tamper with the
> >>> samples, but if the specification really is that max>min, then it
> >>> would
> >>> be up to evdev/joydev (if the individual drivers are allowed
> >>> min>max)
> >>> or adc-joystick (if they are not) to process the samples.
> >>
> >> I don't see why a driver, especially a userspace driver which
> >> then injects things back into the kernel using uinput, would
> >> not take care of inverting the samples itself and then just
> >> present userspace with normalized data where min is simply 0
> >> (as result of normalization as part of inversion) and
> >> max is (original_max - original_min).
> >
> > Yes, I totally agree.
> >
> > What I was saying is, as Chris is only "piping" events from adc-
> > joystick into uinput, that the problem is more that evdev/joydev don't
> > handle axis inversion and provide min>max values that most of the
> > userspace (and some kernel drivers e.g. uinput) don't support.
>
> Ah I see, that sounds like a joydev adc-joystick / driver bug
> to me then.
joydev/mousedev/evdev are simply consumers of events coming from the
drivers that handle hardware. Even though they reside in the kernel,
they still consumers of events, much like userspace is, and they operate
under the same assumption that if min and max are specified then max is
not less than min.
We always had HW drivers invert the axis to match our coordinate system
(for absolute coordinates 0,0 is in the lower left corner, for relative
right and up are positive and left and down are negative). You can see
that in psmouse (psmouse_report_standard_motion) and synaptics drivers,
one of the earliest in the kernel.
The rest of the stack operates under this assumption.
>
> >> Note that this is exactly what is being done for touchscreens,
> >> where having the touchscreen mounted e.g. upside-down is
> >> a long standing issue and this is thus also a long solved issue,
> >> see: drivers/input/touchscreen.c which contains generic
> >> code for parsing device-properties including swapped / inverted
> >> axis as well as generic code for reporting the position to the
> >> input core, where the helpers from drivers/input/touchscreen.c
> >> take care of the swap + invert including normalization when
> >> doing inversion.
> >>
> >> Specifically this contains in touchscreen_parse_properties() :
> >>
> >> prop->max_x = input_abs_get_max(input, axis_x);
> >> prop->max_y = input_abs_get_max(input, axis_y);
> >>
> >> if (prop->invert_x) {
> >> absinfo = &input->absinfo[axis_x];
> >> absinfo->maximum -= absinfo->minimum;
> >> absinfo->minimum = 0;
> >> }
> >>
> >> if (prop->invert_y) {
> >> absinfo = &input->absinfo[axis_y];
> >> absinfo->maximum -= absinfo->minimum;
> >> absinfo->minimum = 0;
> >> }
> >>
> >> and then when reporting touches:
> >>
> >> void touchscreen_report_pos(struct input_dev *input,
> >> const struct touchscreen_properties
> >> *prop,
> >> unsigned int x, unsigned int y,
> >> bool multitouch)
> >> {
> >> if (prop->invert_x)
> >> x = prop->max_x - x;
> >>
> >> if (prop->invert_y)
> >> y = prop->max_y - y;
> >>
> >> if (prop->swap_x_y)
> >> swap(x, y);
> >>
> >> input_report_abs(input, multitouch ? ABS_MT_POSITION_X :
> >> ABS_X, x);
> >> input_report_abs(input, multitouch ? ABS_MT_POSITION_Y :
> >> ABS_Y, y);
> >> }
> >>
> >> One of the tasks of a driver / the kernel is to provide some
> >> level of hardware abstraction to isolate userspace from
> >> hw details. IMHO taking care of the axis-inversion for userspace
> >> with something like the above is part of the kernels' HAL task.
> >
> > Totally agree, but this is not done anywhere, is it? evdev seems to
> > just pass the hardware values alongside some basic meta-data (min/max
> > values, fuzz etc.), it does not tamper with the data. Should evdev
> > handle axis inversion? Should it be in adc-joystick (and every other
> > driver that needs that) instead?
>
> For touchcreens we have chosen to have a set of generic helpers
> and then make using those helpers the responsibility of the driver.
>
> Part of the reason for doing this is because some touchscreen drivers
> already were doing axis inversion inside the driver triggering on
> things like e.g. DMI matches, or maybe custom pre standardization
> device properties, etc.
>
> So the decision was made to add a set of helpers and convert drivers
> one by one. Where drivers can e.g. still set prop->invert_x manually,
> but then they also need to take care of the min/max adjustments
> manually (min is typically 0 for touchscreens though).
>
> I expect that there will also be enough existing special handling
> in the joystick code that piece-meal conversion using helpers
> is likely best.
Not only touchscreens and joysticks. As I mentioned, PS/2 mice have
their reported relative Y motion inverted since forever, touchpads like
Synaptics or Elan invert Y axis as well, and so on.
>
> With that said having axis inversion support in the evdev core
> does sound interesting, but that means also storing the max-value
> inside the core for abs axis and this will likely be a big
> change / lots of work.
evdev is not the only consumer, so if anything it should be in the input
core, but there are enough quirks that I think touchscreen helpers are
the best, at least for now.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: SoC button array: add mapping for airplane mode button
From: Dmitry Torokhov @ 2023-12-24 7:44 UTC (permalink / raw)
To: Werner Sembach; +Cc: Christoffer Sandberg, linux-input, linux-kernel
In-Reply-To: <20231215171718.80229-1-wse@tuxedocomputers.com>
On Fri, Dec 15, 2023 at 06:17:18PM +0100, Werner Sembach wrote:
> From: Christoffer Sandberg <cs@tuxedo.de>
>
> This add a mapping for the airplane mode button on the TUXEDO Pulse Gen3.
>
> While it is physically a key it behaves more like a switch, sending a key
> down on first press and a key up on 2nd press. Therefor the switch event is
> used here. Besides this behaviour it uses the HID usage-id 0xc6 (Wireless
> Radio Button) and not 0xc8 (Wireless Radio Slider Switch), but since
> neither 0xc6 nor 0xc8 are currently implemented at all in soc_button_array
> this not to standard behaviour is not put behind a quirk for the moment.
>
> Signed-off-by: Christoffer Sandberg <cs@tuxedo.de>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [PATCH v3 2/4] dt-bindings: touchscreen: neonode,zforce: Use standard properties
From: Andreas Kemnade @ 2023-12-23 22:12 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg, andreas,
linus.walleij, Jonathan.Cameron, u.kleine-koenig, heiko,
linux-input, devicetree, linux-kernel, linux-arm-kernel
Cc: Rob Herring
In-Reply-To: <20231223221213.774868-1-andreas@kemnade.info>
Enable touchscreen orientation to be specified by using standard
properties.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/input/touchscreen/neonode,zforce.yaml | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
index c39662815a6c5..c2ee89b76ea13 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
@@ -9,6 +9,9 @@ title: Neonode infrared touchscreen controller
maintainers:
- Heiko Stuebner <heiko@sntech.de>
+allOf:
+ - $ref: touchscreen.yaml#
+
properties:
compatible:
const: neonode,zforce
@@ -26,9 +29,11 @@ properties:
maxItems: 1
x-size:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
y-size:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
vdd-supply: true
@@ -38,8 +43,6 @@ required:
- reg
- interrupts
- reset-gpios
- - x-size
- - y-size
unevaluatedProperties: false
@@ -60,8 +63,10 @@ examples:
reset-gpios = <&gpio5 9 0>; /* RST */
irq-gpios = <&gpio5 6 0>; /* IRQ, optional */
- x-size = <800>;
- y-size = <600>;
+ touchscreen-min-x = <0>;
+ touchscreen-size-x = <800>;
+ touchscreen-min-y = <0>;
+ touchscreen-size-y = <600>;
};
};
...
--
2.39.2
^ permalink raw reply related
* [PATCH v3 4/4] ARM: dts: imx6sl-tolino-shine2hd: fix touchscreen rotation
From: Andreas Kemnade @ 2023-12-23 22:12 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg, andreas,
linus.walleij, Jonathan.Cameron, u.kleine-koenig, heiko,
linux-input, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20231223221213.774868-1-andreas@kemnade.info>
The display is in landscape orientation, but the touchscreen is in portrait
orientation. Specify that properly in the devicetree.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts b/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts
index 815119c12bd48..5636fb3661e8a 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts
+++ b/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts
@@ -141,8 +141,10 @@ zforce: touchscreen@50 {
interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
vdd-supply = <&ldo1_reg>;
reset-gpios = <&gpio5 9 GPIO_ACTIVE_LOW>;
- x-size = <1072>;
- y-size = <1448>;
+ touchscreen-size-x = <1072>;
+ touchscreen-size-y = <1448>;
+ touchscreen-swapped-x-y;
+ touchscreen-inverted-x;
};
/* TODO: TPS65185 PMIC for E Ink at 0x68 */
--
2.39.2
^ permalink raw reply related
* [PATCH v3 3/4] Input: zforce_ts: Accept standard touchscreen properties
From: Andreas Kemnade @ 2023-12-23 22:12 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg, andreas,
linus.walleij, Jonathan.Cameron, u.kleine-koenig, heiko,
linux-input, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20231223221213.774868-1-andreas@kemnade.info>
Only driver-specific properties were accepted, change it
to use the now-available standard properties.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
drivers/input/touchscreen/zforce_ts.c | 36 +++++++++++++--------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 5be5112845e1e..f4397497bbe94 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -20,6 +20,7 @@
#include <linux/device.h>
#include <linux/sysfs.h>
#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
#include <linux/platform_data/zforce_ts.h>
#include <linux/regulator/consumer.h>
#include <linux/of.h>
@@ -106,6 +107,7 @@ struct zforce_point {
struct zforce_ts {
struct i2c_client *client;
struct input_dev *input;
+ struct touchscreen_properties prop;
const struct zforce_ts_platdata *pdata;
char phys[32];
@@ -266,7 +268,6 @@ static int zforce_setconfig(struct zforce_ts *ts, char b1)
static int zforce_start(struct zforce_ts *ts)
{
struct i2c_client *client = ts->client;
- const struct zforce_ts_platdata *pdata = ts->pdata;
int ret;
dev_dbg(&client->dev, "starting device\n");
@@ -277,7 +278,7 @@ static int zforce_start(struct zforce_ts *ts)
return ret;
}
- ret = zforce_resolution(ts, pdata->x_max, pdata->y_max);
+ ret = zforce_resolution(ts, ts->prop.max_x, ts->prop.max_y);
if (ret) {
dev_err(&client->dev, "Unable to set resolution, %d\n", ret);
goto error;
@@ -337,7 +338,6 @@ static int zforce_stop(struct zforce_ts *ts)
static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
{
struct i2c_client *client = ts->client;
- const struct zforce_ts_platdata *pdata = ts->pdata;
struct zforce_point point;
int count, i, num = 0;
@@ -355,8 +355,8 @@ static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
point.coord_y =
payload[9 * i + 4] << 8 | payload[9 * i + 3];
- if (point.coord_x > pdata->x_max ||
- point.coord_y > pdata->y_max) {
+ if (point.coord_x > ts->prop.max_x ||
+ point.coord_y > ts->prop.max_y) {
dev_warn(&client->dev, "coordinates (%d,%d) invalid\n",
point.coord_x, point.coord_y);
point.coord_x = point.coord_y = 0;
@@ -390,10 +390,11 @@ static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
point.state != STATE_UP);
if (point.state != STATE_UP) {
- input_report_abs(ts->input, ABS_MT_POSITION_X,
- point.coord_x);
- input_report_abs(ts->input, ABS_MT_POSITION_Y,
- point.coord_y);
+ touchscreen_report_pos(ts->input,
+ &ts->prop,
+ point.coord_x,
+ point.coord_y,
+ true);
input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
point.area_major);
input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
@@ -719,15 +720,8 @@ static struct zforce_ts_platdata *zforce_parse_dt(struct device *dev)
return ERR_PTR(-ENOMEM);
}
- if (of_property_read_u32(np, "x-size", &pdata->x_max)) {
- dev_err(dev, "failed to get x-size property\n");
- return ERR_PTR(-EINVAL);
- }
-
- if (of_property_read_u32(np, "y-size", &pdata->y_max)) {
- dev_err(dev, "failed to get y-size property\n");
- return ERR_PTR(-EINVAL);
- }
+ of_property_read_u32(np, "x-size", &pdata->x_max);
+ of_property_read_u32(np, "y-size", &pdata->y_max);
return pdata;
}
@@ -856,6 +850,12 @@ static int zforce_probe(struct i2c_client *client)
input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
pdata->y_max, 0, 0);
+ touchscreen_parse_properties(input_dev, true, &ts->prop);
+ if (ts->prop.max_x == 0 || ts->prop.max_y == 0) {
+ dev_err(&client->dev, "no size specified\n");
+ return -EINVAL;
+ }
+
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
ZFORCE_MAX_AREA, 0, 0);
input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR, 0,
--
2.39.2
^ permalink raw reply related
* [PATCH v3 1/4] dt-bindings: touchscreen: convert neonode,zforce to json-schema
From: Andreas Kemnade @ 2023-12-23 22:12 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg, andreas,
linus.walleij, Jonathan.Cameron, u.kleine-koenig, heiko,
linux-input, devicetree, linux-kernel, linux-arm-kernel
Cc: Krzysztof Kozlowski
In-Reply-To: <20231223221213.774868-1-andreas@kemnade.info>
Convert Neonode infrared touchscreen controller binding to DT schema.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../input/touchscreen/neonode,zforce.yaml | 67 +++++++++++++++++++
.../bindings/input/touchscreen/zforce_ts.txt | 34 ----------
2 files changed, 67 insertions(+), 34 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
diff --git a/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
new file mode 100644
index 0000000000000..c39662815a6c5
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/neonode,zforce.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Neonode infrared touchscreen controller
+
+maintainers:
+ - Heiko Stuebner <heiko@sntech.de>
+
+properties:
+ compatible:
+ const: neonode,zforce
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ irq-gpios:
+ maxItems: 1
+
+ x-size:
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-size:
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ vdd-supply: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - reset-gpios
+ - x-size
+ - y-size
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@50 {
+ compatible = "neonode,zforce";
+ reg = <0x50>;
+ interrupts = <2 0>;
+ vdd-supply = <®_zforce_vdd>;
+
+ reset-gpios = <&gpio5 9 0>; /* RST */
+ irq-gpios = <&gpio5 6 0>; /* IRQ, optional */
+
+ x-size = <800>;
+ y-size = <600>;
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
deleted file mode 100644
index e3c27c4fd9c85..0000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-* Neonode infrared touchscreen controller
-
-Required properties:
-- compatible: must be "neonode,zforce"
-- reg: I2C address of the chip
-- interrupts: interrupt to which the chip is connected
-- reset-gpios: reset gpio the chip is connected to
-- x-size: horizontal resolution of touchscreen
-- y-size: vertical resolution of touchscreen
-
-Optional properties:
-- irq-gpios : interrupt gpio the chip is connected to
-- vdd-supply: Regulator controlling the controller supply
-
-Example:
-
- i2c@00000000 {
- /* ... */
-
- zforce_ts@50 {
- compatible = "neonode,zforce";
- reg = <0x50>;
- interrupts = <2 0>;
- vdd-supply = <®_zforce_vdd>;
-
- reset-gpios = <&gpio5 9 0>; /* RST */
- irq-gpios = <&gpio5 6 0>; /* IRQ, optional */
-
- x-size = <800>;
- y-size = <600>;
- };
-
- /* ... */
- };
--
2.39.2
^ permalink raw reply related
* [PATCH v3 0/4] Input: zforce_ts: standard properties
From: Andreas Kemnade @ 2023-12-23 22:12 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg, andreas,
linus.walleij, Jonathan.Cameron, u.kleine-koenig, heiko,
linux-input, devicetree, linux-kernel, linux-arm-kernel
Accept standard touchscreen properties to also enable specification
of touchscreen orientation.
Changes in V3:
- adding R-bys, no change in code
Changes in V2:
- correct mail address in .yaml
Andreas Kemnade (4):
dt-bindings: touchscreen: convert neonode,zforce to json-schema
dt-bindings: touchscreen: neonode,zforce: Use standard properties
Input: zforce_ts: Accept standard touchscreen properties
ARM: dts: imx6sl-tolino-shine2hd: fix touchscreen rotation
.../input/touchscreen/neonode,zforce.yaml | 72 +++++++++++++++++++
.../bindings/input/touchscreen/zforce_ts.txt | 34 ---------
.../dts/nxp/imx/imx6sl-tolino-shine2hd.dts | 6 +-
drivers/input/touchscreen/zforce_ts.c | 36 +++++-----
4 files changed, 94 insertions(+), 54 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
--
2.39.2
^ permalink raw reply
* Re: [git pull] Input updates for v6.7-rc6
From: pr-tracker-bot @ 2023-12-23 20:21 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <ZYaQvgIr99ixMKBS@google.com>
The pull request you sent on Fri, 22 Dec 2023 23:48:14 -0800:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.7-rc6
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/fa655abe42c65e7e4ad52baf280dadfb571c110e
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* [PATCH] HID: lenovo: Add middleclick_workaround sysfs knob for cptkbd
From: Mikhail Khvainitski @ 2023-12-23 19:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Mikhail Khvainitski, iam, jekhor, u-v
In-Reply-To: <CAMMabwPd-m7a+EQV7zb=wU52=P1FkqFU1dg9=gyvaS1EP5tX3Q@mail.gmail.com>
Previous attempt to autodetect well-behaving patched firmware
introduced in commit 46a0a2c96f0f ("HID: lenovo: Detect quirk-free fw
on cptkbd and stop applying workaround") has shown that there are
false-positives on original firmware (on both 1st gen and 2nd gen
keyboards) which causes the middle button click workaround to be
mistakenly disabled.
This commit adds explicit parameter to sysfs to control this
workaround.
Fixes: 46a0a2c96f0f ("HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround")
Fixes: 43527a0094c1 ("HID: lenovo: Restrict detection of patched firmware only to USB cptkbd")
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
---
drivers/hid/hid-lenovo.c | 57 +++++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 18 deletions(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 149a3c74346b..f86c1ea83a03 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -54,10 +54,10 @@ struct lenovo_drvdata {
/* 0: Up
* 1: Down (undecided)
* 2: Scrolling
- * 3: Patched firmware, disable workaround
*/
u8 middlebutton_state;
bool fn_lock;
+ bool middleclick_workaround_cptkbd;
};
#define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
@@ -621,6 +621,36 @@ static ssize_t attr_sensitivity_store_cptkbd(struct device *dev,
return count;
}
+static ssize_t attr_middleclick_workaround_show_cptkbd(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
+
+ return snprintf(buf, PAGE_SIZE, "%u\n",
+ cptkbd_data->middleclick_workaround_cptkbd);
+}
+
+static ssize_t attr_middleclick_workaround_store_cptkbd(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
+ int value;
+
+ if (kstrtoint(buf, 10, &value))
+ return -EINVAL;
+ if (value < 0 || value > 1)
+ return -EINVAL;
+
+ cptkbd_data->middleclick_workaround_cptkbd = !!value;
+
+ return count;
+}
+
static struct device_attribute dev_attr_fn_lock =
__ATTR(fn_lock, S_IWUSR | S_IRUGO,
@@ -632,10 +662,16 @@ static struct device_attribute dev_attr_sensitivity_cptkbd =
attr_sensitivity_show_cptkbd,
attr_sensitivity_store_cptkbd);
+static struct device_attribute dev_attr_middleclick_workaround_cptkbd =
+ __ATTR(middleclick_workaround, S_IWUSR | S_IRUGO,
+ attr_middleclick_workaround_show_cptkbd,
+ attr_middleclick_workaround_store_cptkbd);
+
static struct attribute *lenovo_attributes_cptkbd[] = {
&dev_attr_fn_lock.attr,
&dev_attr_sensitivity_cptkbd.attr,
+ &dev_attr_middleclick_workaround_cptkbd.attr,
NULL
};
@@ -686,23 +722,7 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
{
struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
- if (cptkbd_data->middlebutton_state != 3) {
- /* REL_X and REL_Y events during middle button pressed
- * are only possible on patched, bug-free firmware
- * so set middlebutton_state to 3
- * to never apply workaround anymore
- */
- if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD &&
- cptkbd_data->middlebutton_state == 1 &&
- usage->type == EV_REL &&
- (usage->code == REL_X || usage->code == REL_Y)) {
- cptkbd_data->middlebutton_state = 3;
- /* send middle button press which was hold before */
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 1);
- input_sync(field->hidinput->input);
- }
-
+ if (cptkbd_data->middleclick_workaround_cptkbd) {
/* "wheel" scroll events */
if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
usage->code == REL_HWHEEL)) {
@@ -1166,6 +1186,7 @@ static int lenovo_probe_cptkbd(struct hid_device *hdev)
cptkbd_data->middlebutton_state = 0;
cptkbd_data->fn_lock = true;
cptkbd_data->sensitivity = 0x05;
+ cptkbd_data->middleclick_workaround_cptkbd = true;
lenovo_features_set_cptkbd(hdev);
ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_cptkbd);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] HID: lenovo: Restrict detection of patched firmware only to USB cptkbd
From: Mikhail Khvoinitsky @ 2023-12-23 18:04 UTC (permalink / raw)
To: Uli v. d. Ohe
Cc: jkosina, benjamin.tissoires, iam, jekhor, linux-input,
linux-kernel
In-Reply-To: <3a29c0a6-9b53-433d-a83d-5b68a87c1155@mailbox.org>
Hello.
Thank you for the report, sorry about that. I've received one more
report of the same issue by email.
So this means that the only reliable way is to add a sysfs parameter.
I'll send a patch.
On Fri, 22 Dec 2023 at 17:32, Uli v. d. Ohe <u-v@mailbox.org> wrote:
>
> I get buggy middle button scrolling behavior on my USB Compact Keyboard
> (i.e., "1st gen") with original, unmodified firmware and the patch (of
> Sep. 23).
>
> Sometimes the keyboard sends REL_X events while the middle button is
> pressed. Thus the old "workaround" is disabled and middle button
> scrolling henceforth exhibits the known buggy behavior.
>
> Explicitly, I can confirm that the following values occur, leading to
> erroneous disabling of the workaround:
>
> cptkbd_data->middlebutton_state == 1
> usage->type == 2 [i.e., EV_REL]
> usage->code == 0 [i.e., REL_X]
>
> The keyboard is identified by lsusb as:
> ID 17ef:6047 Lenovo ThinkPad Compact Keyboard with TrackPoint
>
> This was tested with kernel 6.1.67 which contains the backported patch
> (commit 6e2076cad8873cc2a9f96e4becab35425c3656dc).
>
> I didn't test the latest patch of Dec. 12. However, I don't expect it to
> fix my issue as the only added condition
> hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
> should be satisfied, which I understand is also the intention.
> (USB_DEVICE_ID_LENOVO_CUSBKBD == 0x6047, which is the device ID of my
> keyboard as reported by lsusb.)
>
> Best regards,
> Uli
^ permalink raw reply
* Re: [PATCH] input: uinput: Drop checks for abs_min > abs_max
From: Hans de Goede @ 2023-12-23 15:16 UTC (permalink / raw)
To: Paul Cercueil, Dmitry Torokhov
Cc: Peter Hutterer, Chris Morgan, linux-input, svv, biswarupp,
contact, Chris Morgan
In-Reply-To: <0ac9f339380ca3acd76eff065238599f39cde039.camel@crapouillou.net>
Hi,
On 12/23/23 16:01, Paul Cercueil wrote:
> Hi Hans,
>
> Le samedi 23 décembre 2023 à 15:29 +0100, Hans de Goede a écrit :
>> Hi Paul,
>>
>> On 12/20/23 14:39, Paul Cercueil wrote:
>>> Hi Dmitry,
>>>
>>> Le mardi 19 décembre 2023 à 17:53 -0800, Dmitry Torokhov a écrit :
>>>> Hi Paul,
>>>>
>>>> On Wed, Dec 20, 2023 at 01:38:39AM +0100, Paul Cercueil wrote:
>>>>> Hi Peter,
>>>>>
>>>>> Le mercredi 20 décembre 2023 à 09:51 +1000, Peter Hutterer a
>>>>> écrit :
>>>>>> On Mon, Dec 18, 2023 at 11:16:53AM -0600, Chris Morgan wrote:
>>>>>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>>>>>
>>>>>>> Stop checking if the minimum abs value is greater than the
>>>>>>> maximum
>>>>>>> abs
>>>>>>> value. When the axis is inverted this condition is allowed.
>>>>>>> Without
>>>>>>> relaxing this check, it is not possible to use uinput on
>>>>>>> devices in
>>>>>>> userspace with an inverted axis, such as the adc-joystick
>>>>>>> found
>>>>>>> on
>>>>>>> many handheld gaming devices.
>>>>>>
>>>>>> As mentioned in the other thread [1] a fair bit of userspace
>>>>>> relies
>>>>>> on
>>>>>> that general assumption so removing it will likely cause all
>>>>>> sorts of
>>>>>> issues.
>>>>>
>>>>> There is some userspace that works with it though, so why
>>>>> restrict
>>>>> it
>>>>> artificially?
>>>>>
>>>>> The fact that some other userspace code wouldn't work with it
>>>>> sounds a
>>>>> bit irrelevant. They just never encountered that min>max usage
>>>>> before.
>>>>>
>>>>> And removing this check won't cause all sort of issues, why
>>>>> would
>>>>> it?
>>>>> It's not like the current software actively probes min>max and
>>>>> crash
>>>>> badly if it doesn't return -EINVAL...
>>>>
>>>> It will cause weird movements because calculations expect min be
>>>> the
>>>> minimum, and max the maximum, and not encode left/right or
>>>> up/down.
>>>> Putting this into adc joystick binding was a mistake.
>>>
>>> I don't see why it was a mistake, it's only one of the ways to
>>> specify
>>> that the axis is inverted. This information is between the firmware
>>> (DT) and the kernel, that doesn't mean the information has to be
>>> relayed as-is to the userspace.
>>>
>>> Unlike what you wrote in your other answer, when talking about
>>> input
>>> the kernel doesn't really normalize anything - it gives you the
>>> min/max
>>> values, and the raw samples, not normalized samples (they don't get
>>> translated to a pre-specified range, or even clamped).
>>>
>>> I don't really like the idea of having the driver tamper with the
>>> samples, but if the specification really is that max>min, then it
>>> would
>>> be up to evdev/joydev (if the individual drivers are allowed
>>> min>max)
>>> or adc-joystick (if they are not) to process the samples.
>>
>> I don't see why a driver, especially a userspace driver which
>> then injects things back into the kernel using uinput, would
>> not take care of inverting the samples itself and then just
>> present userspace with normalized data where min is simply 0
>> (as result of normalization as part of inversion) and
>> max is (original_max - original_min).
>
> Yes, I totally agree.
>
> What I was saying is, as Chris is only "piping" events from adc-
> joystick into uinput, that the problem is more that evdev/joydev don't
> handle axis inversion and provide min>max values that most of the
> userspace (and some kernel drivers e.g. uinput) don't support.
Ah I see, that sounds like a joydev adc-joystick / driver bug
to me then.
>> Note that this is exactly what is being done for touchscreens,
>> where having the touchscreen mounted e.g. upside-down is
>> a long standing issue and this is thus also a long solved issue,
>> see: drivers/input/touchscreen.c which contains generic
>> code for parsing device-properties including swapped / inverted
>> axis as well as generic code for reporting the position to the
>> input core, where the helpers from drivers/input/touchscreen.c
>> take care of the swap + invert including normalization when
>> doing inversion.
>>
>> Specifically this contains in touchscreen_parse_properties() :
>>
>> prop->max_x = input_abs_get_max(input, axis_x);
>> prop->max_y = input_abs_get_max(input, axis_y);
>>
>> if (prop->invert_x) {
>> absinfo = &input->absinfo[axis_x];
>> absinfo->maximum -= absinfo->minimum;
>> absinfo->minimum = 0;
>> }
>>
>> if (prop->invert_y) {
>> absinfo = &input->absinfo[axis_y];
>> absinfo->maximum -= absinfo->minimum;
>> absinfo->minimum = 0;
>> }
>>
>> and then when reporting touches:
>>
>> void touchscreen_report_pos(struct input_dev *input,
>> const struct touchscreen_properties
>> *prop,
>> unsigned int x, unsigned int y,
>> bool multitouch)
>> {
>> if (prop->invert_x)
>> x = prop->max_x - x;
>>
>> if (prop->invert_y)
>> y = prop->max_y - y;
>>
>> if (prop->swap_x_y)
>> swap(x, y);
>>
>> input_report_abs(input, multitouch ? ABS_MT_POSITION_X :
>> ABS_X, x);
>> input_report_abs(input, multitouch ? ABS_MT_POSITION_Y :
>> ABS_Y, y);
>> }
>>
>> One of the tasks of a driver / the kernel is to provide some
>> level of hardware abstraction to isolate userspace from
>> hw details. IMHO taking care of the axis-inversion for userspace
>> with something like the above is part of the kernels' HAL task.
>
> Totally agree, but this is not done anywhere, is it? evdev seems to
> just pass the hardware values alongside some basic meta-data (min/max
> values, fuzz etc.), it does not tamper with the data. Should evdev
> handle axis inversion? Should it be in adc-joystick (and every other
> driver that needs that) instead?
For touchcreens we have chosen to have a set of generic helpers
and then make using those helpers the responsibility of the driver.
Part of the reason for doing this is because some touchscreen drivers
already were doing axis inversion inside the driver triggering on
things like e.g. DMI matches, or maybe custom pre standardization
device properties, etc.
So the decision was made to add a set of helpers and convert drivers
one by one. Where drivers can e.g. still set prop->invert_x manually,
but then they also need to take care of the min/max adjustments
manually (min is typically 0 for touchscreens though).
I expect that there will also be enough existing special handling
in the joystick code that piece-meal conversion using helpers
is likely best.
With that said having axis inversion support in the evdev core
does sound interesting, but that means also storing the max-value
inside the core for abs axis and this will likely be a big
change / lots of work.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH] input: uinput: Drop checks for abs_min > abs_max
From: Paul Cercueil @ 2023-12-23 15:01 UTC (permalink / raw)
To: Hans de Goede, Dmitry Torokhov
Cc: Peter Hutterer, Chris Morgan, linux-input, svv, biswarupp,
contact, Chris Morgan
In-Reply-To: <954f6537-15d5-42db-94b5-d148d4942870@redhat.com>
Hi Hans,
Le samedi 23 décembre 2023 à 15:29 +0100, Hans de Goede a écrit :
> Hi Paul,
>
> On 12/20/23 14:39, Paul Cercueil wrote:
> > Hi Dmitry,
> >
> > Le mardi 19 décembre 2023 à 17:53 -0800, Dmitry Torokhov a écrit :
> > > Hi Paul,
> > >
> > > On Wed, Dec 20, 2023 at 01:38:39AM +0100, Paul Cercueil wrote:
> > > > Hi Peter,
> > > >
> > > > Le mercredi 20 décembre 2023 à 09:51 +1000, Peter Hutterer a
> > > > écrit :
> > > > > On Mon, Dec 18, 2023 at 11:16:53AM -0600, Chris Morgan wrote:
> > > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > > >
> > > > > > Stop checking if the minimum abs value is greater than the
> > > > > > maximum
> > > > > > abs
> > > > > > value. When the axis is inverted this condition is allowed.
> > > > > > Without
> > > > > > relaxing this check, it is not possible to use uinput on
> > > > > > devices in
> > > > > > userspace with an inverted axis, such as the adc-joystick
> > > > > > found
> > > > > > on
> > > > > > many handheld gaming devices.
> > > > >
> > > > > As mentioned in the other thread [1] a fair bit of userspace
> > > > > relies
> > > > > on
> > > > > that general assumption so removing it will likely cause all
> > > > > sorts of
> > > > > issues.
> > > >
> > > > There is some userspace that works with it though, so why
> > > > restrict
> > > > it
> > > > artificially?
> > > >
> > > > The fact that some other userspace code wouldn't work with it
> > > > sounds a
> > > > bit irrelevant. They just never encountered that min>max usage
> > > > before.
> > > >
> > > > And removing this check won't cause all sort of issues, why
> > > > would
> > > > it?
> > > > It's not like the current software actively probes min>max and
> > > > crash
> > > > badly if it doesn't return -EINVAL...
> > >
> > > It will cause weird movements because calculations expect min be
> > > the
> > > minimum, and max the maximum, and not encode left/right or
> > > up/down.
> > > Putting this into adc joystick binding was a mistake.
> >
> > I don't see why it was a mistake, it's only one of the ways to
> > specify
> > that the axis is inverted. This information is between the firmware
> > (DT) and the kernel, that doesn't mean the information has to be
> > relayed as-is to the userspace.
> >
> > Unlike what you wrote in your other answer, when talking about
> > input
> > the kernel doesn't really normalize anything - it gives you the
> > min/max
> > values, and the raw samples, not normalized samples (they don't get
> > translated to a pre-specified range, or even clamped).
> >
> > I don't really like the idea of having the driver tamper with the
> > samples, but if the specification really is that max>min, then it
> > would
> > be up to evdev/joydev (if the individual drivers are allowed
> > min>max)
> > or adc-joystick (if they are not) to process the samples.
>
> I don't see why a driver, especially a userspace driver which
> then injects things back into the kernel using uinput, would
> not take care of inverting the samples itself and then just
> present userspace with normalized data where min is simply 0
> (as result of normalization as part of inversion) and
> max is (original_max - original_min).
Yes, I totally agree.
What I was saying is, as Chris is only "piping" events from adc-
joystick into uinput, that the problem is more that evdev/joydev don't
handle axis inversion and provide min>max values that most of the
userspace (and some kernel drivers e.g. uinput) don't support.
> Note that this is exactly what is being done for touchscreens,
> where having the touchscreen mounted e.g. upside-down is
> a long standing issue and this is thus also a long solved issue,
> see: drivers/input/touchscreen.c which contains generic
> code for parsing device-properties including swapped / inverted
> axis as well as generic code for reporting the position to the
> input core, where the helpers from drivers/input/touchscreen.c
> take care of the swap + invert including normalization when
> doing inversion.
>
> Specifically this contains in touchscreen_parse_properties() :
>
> prop->max_x = input_abs_get_max(input, axis_x);
> prop->max_y = input_abs_get_max(input, axis_y);
>
> if (prop->invert_x) {
> absinfo = &input->absinfo[axis_x];
> absinfo->maximum -= absinfo->minimum;
> absinfo->minimum = 0;
> }
>
> if (prop->invert_y) {
> absinfo = &input->absinfo[axis_y];
> absinfo->maximum -= absinfo->minimum;
> absinfo->minimum = 0;
> }
>
> and then when reporting touches:
>
> void touchscreen_report_pos(struct input_dev *input,
> const struct touchscreen_properties
> *prop,
> unsigned int x, unsigned int y,
> bool multitouch)
> {
> if (prop->invert_x)
> x = prop->max_x - x;
>
> if (prop->invert_y)
> y = prop->max_y - y;
>
> if (prop->swap_x_y)
> swap(x, y);
>
> input_report_abs(input, multitouch ? ABS_MT_POSITION_X :
> ABS_X, x);
> input_report_abs(input, multitouch ? ABS_MT_POSITION_Y :
> ABS_Y, y);
> }
>
> One of the tasks of a driver / the kernel is to provide some
> level of hardware abstraction to isolate userspace from
> hw details. IMHO taking care of the axis-inversion for userspace
> with something like the above is part of the kernels' HAL task.
Totally agree, but this is not done anywhere, is it? evdev seems to
just pass the hardware values alongside some basic meta-data (min/max
values, fuzz etc.), it does not tamper with the data. Should evdev
handle axis inversion? Should it be in adc-joystick (and every other
driver that needs that) instead?
>
> Regards,
>
> Hans
Cheers,
-Paul
^ permalink raw reply
* Re: [PATCH] input: uinput: Drop checks for abs_min > abs_max
From: Hans de Goede @ 2023-12-23 14:29 UTC (permalink / raw)
To: Paul Cercueil, Dmitry Torokhov
Cc: Peter Hutterer, Chris Morgan, linux-input, svv, biswarupp,
contact, Chris Morgan
In-Reply-To: <4e902e8ff60e21a74a87887e272f6751d3837c71.camel@crapouillou.net>
Hi Paul,
On 12/20/23 14:39, Paul Cercueil wrote:
> Hi Dmitry,
>
> Le mardi 19 décembre 2023 à 17:53 -0800, Dmitry Torokhov a écrit :
>> Hi Paul,
>>
>> On Wed, Dec 20, 2023 at 01:38:39AM +0100, Paul Cercueil wrote:
>>> Hi Peter,
>>>
>>> Le mercredi 20 décembre 2023 à 09:51 +1000, Peter Hutterer a
>>> écrit :
>>>> On Mon, Dec 18, 2023 at 11:16:53AM -0600, Chris Morgan wrote:
>>>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>>>
>>>>> Stop checking if the minimum abs value is greater than the
>>>>> maximum
>>>>> abs
>>>>> value. When the axis is inverted this condition is allowed.
>>>>> Without
>>>>> relaxing this check, it is not possible to use uinput on
>>>>> devices in
>>>>> userspace with an inverted axis, such as the adc-joystick found
>>>>> on
>>>>> many handheld gaming devices.
>>>>
>>>> As mentioned in the other thread [1] a fair bit of userspace
>>>> relies
>>>> on
>>>> that general assumption so removing it will likely cause all
>>>> sorts of
>>>> issues.
>>>
>>> There is some userspace that works with it though, so why restrict
>>> it
>>> artificially?
>>>
>>> The fact that some other userspace code wouldn't work with it
>>> sounds a
>>> bit irrelevant. They just never encountered that min>max usage
>>> before.
>>>
>>> And removing this check won't cause all sort of issues, why would
>>> it?
>>> It's not like the current software actively probes min>max and
>>> crash
>>> badly if it doesn't return -EINVAL...
>>
>> It will cause weird movements because calculations expect min be the
>> minimum, and max the maximum, and not encode left/right or up/down.
>> Putting this into adc joystick binding was a mistake.
>
> I don't see why it was a mistake, it's only one of the ways to specify
> that the axis is inverted. This information is between the firmware
> (DT) and the kernel, that doesn't mean the information has to be
> relayed as-is to the userspace.
>
> Unlike what you wrote in your other answer, when talking about input
> the kernel doesn't really normalize anything - it gives you the min/max
> values, and the raw samples, not normalized samples (they don't get
> translated to a pre-specified range, or even clamped).
>
> I don't really like the idea of having the driver tamper with the
> samples, but if the specification really is that max>min, then it would
> be up to evdev/joydev (if the individual drivers are allowed min>max)
> or adc-joystick (if they are not) to process the samples.
I don't see why a driver, especially a userspace driver which
then injects things back into the kernel using uinput, would
not take care of inverting the samples itself and then just
present userspace with normalized data where min is simply 0
(as result of normalization as part of inversion) and
max is (original_max - original_min).
Note that this is exactly what is being done for touchscreens,
where having the touchscreen mounted e.g. upside-down is
a long standing issue and this is thus also a long solved issue,
see: drivers/input/touchscreen.c which contains generic
code for parsing device-properties including swapped / inverted
axis as well as generic code for reporting the position to the
input core, where the helpers from drivers/input/touchscreen.c
take care of the swap + invert including normalization when
doing inversion.
Specifically this contains in touchscreen_parse_properties() :
prop->max_x = input_abs_get_max(input, axis_x);
prop->max_y = input_abs_get_max(input, axis_y);
if (prop->invert_x) {
absinfo = &input->absinfo[axis_x];
absinfo->maximum -= absinfo->minimum;
absinfo->minimum = 0;
}
if (prop->invert_y) {
absinfo = &input->absinfo[axis_y];
absinfo->maximum -= absinfo->minimum;
absinfo->minimum = 0;
}
and then when reporting touches:
void touchscreen_report_pos(struct input_dev *input,
const struct touchscreen_properties *prop,
unsigned int x, unsigned int y,
bool multitouch)
{
if (prop->invert_x)
x = prop->max_x - x;
if (prop->invert_y)
y = prop->max_y - y;
if (prop->swap_x_y)
swap(x, y);
input_report_abs(input, multitouch ? ABS_MT_POSITION_X : ABS_X, x);
input_report_abs(input, multitouch ? ABS_MT_POSITION_Y : ABS_Y, y);
}
One of the tasks of a driver / the kernel is to provide some
level of hardware abstraction to isolate userspace from
hw details. IMHO taking care of the axis-inversion for userspace
with something like the above is part of the kernels' HAL task.
Regards,
Hans
^ permalink raw reply
* [PATCH] Input: goodix - Accept ACPI resources with gpio_count == 3 && gpio_int_idx == 0
From: Hans de Goede @ 2023-12-23 14:16 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, linux-input
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>
---
Note Maarten has requested for their email to not be made public.
---
drivers/input/touchscreen/goodix.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index af32fbe57b63..b068ff8afbc9 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -884,7 +884,8 @@ static int goodix_add_acpi_gpio_mappings(struct goodix_ts_data *ts)
}
}
- if (ts->gpio_count == 2 && ts->gpio_int_idx == 0) {
+ /* Some devices with gpio_int_idx 0 list a third unused GPIO */
+ if ((ts->gpio_count == 2 || ts->gpio_count == 3) && ts->gpio_int_idx == 0) {
ts->irq_pin_access_method = IRQ_PIN_ACCESS_ACPI_GPIO;
gpio_mapping = acpi_goodix_int_first_gpios;
} else if (ts->gpio_count == 2 && ts->gpio_int_idx == 1) {
--
2.41.0
^ permalink raw reply related
* Re: [PATCH v2 1/2] dt-bindings: input: atmel,maxtouch: add poweroff-sleep property
From: Krzysztof Kozlowski @ 2023-12-23 13:54 UTC (permalink / raw)
To: Stefan Eichenberger, nick, dmitry.torokhov, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nicolas.ferre,
alexandre.belloni, claudiu.beznea, linus.walleij,
francesco.dolcini
Cc: linux-input, devicetree, linux-arm-kernel, linux-kernel,
Stefan Eichenberger
In-Reply-To: <20231222095258.33369-2-eichest@gmail.com>
On 22/12/2023 10:52, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> Add a new property to indicate that the device should power off rather
> than use deep sleep. Deep sleep is a feature of the controller that
> expects the controller to remain powered in suspend. However, if a
> display shares its regulator with the touch controller, we may want to
> do a power off so that the display and touch controller do not use any
> power.
>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v6 00/40] ep93xx device tree conversion
From: Nikita Shubin @ 2023-12-23 9:12 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Michael Turquette, Stephen Boyd, Sebastian Reichel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Wim Van Sebroeck,
Guenter Roeck, Thierry Reding, Uwe Kleine-König, Mark Brown,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Damien Le Moal, Sergey Shtylyov, Dmitry Torokhov, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-kernel,
linux-gpio, linux-clk, linux-pm, devicetree, dmaengine,
linux-watchdog, linux-pwm, linux-spi, netdev, linux-mtd,
linux-ide, linux-input, linux-sound, Arnd Bergmann,
Bartosz Golaszewski, Krzysztof Kozlowski, Andrew Lunn
In-Reply-To: <ZXnxBtqbneUMbvwq@smile.fi.intel.com>
Hello Andy!
On Wed, 2023-12-13 at 19:59 +0200, Andy Shevchenko wrote:
> On Tue, Dec 12, 2023 at 11:20:17AM +0300, Nikita Shubin wrote:
> > No major changes since last version all changes are cometic.
> >
> > Following patches require attention from Stephen Boyd, as they were
> > converted to aux_dev as suggested:
> >
> > - ARM: ep93xx: add regmap aux_dev
> > - clk: ep93xx: add DT support for Cirrus EP93xx
> >
> > DMA related patches still require Acked or Reviewed tags.
> >
> > got approval LGTM from Miquel:
> > - mtd: rawnand: add support for ts72xx
> > Link: https://lore.kernel.org/lkml/20231004103911.2aa65354@xps-13/
> >
> > new patches:
> >
> > ARM: ep93xx: Add terminator to gpiod_lookup_table
> > - fixed terminator in gpiod_lockup_table
> >
> > So mostly all patches got approval.
> >
> > Patches should be now formated with '--histogram'
>
> It _feels_ like some tags might be missing.
> In any case I suggest to use `b4` tool to retrieve tags when
> preparing
> the next version:
>
> git checkout -b vXX v6.7-rcX
> b4 am -slt $MSG_ID_OF_v(XX-1)
> git am ...
> git rebase --interactive ... # to address comments
>
I moved to b4 a few iterations ago:
```
Calculating patch-ids from commits, this may take a moment...
Checking change-id "20230605-ep93xx-01c76317e2d2"
Grabbing search results from lore.kernel.org
Grabbing thread from lore.kernel.org/all/20230424123522.18302-1-
nikita.shubin%40maquefel.me/t.mbox.gz
---
NOTE: some trailers ignored due to from/email mismatches:
! Trailer: Tested-by: Michael Peters <mpeters@embeddedTS.com>
Msg From: Kris Bahnsen <kris@embeddedTS.com>
NOTE: Rerun with -S to apply them anyway
No trailer updates found.
```
I haven't found any missing tags, that b4 didn't apply, the ones above
refer to a very old iteration and were given to cover letter and i
don't feel like they need to be included.
^ permalink raw reply
* Re: [PATCH v2 0/9] Support light color temperature and chromaticity
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-12-23 8:14 UTC (permalink / raw)
To: Thomas Weißschuh, Basavaraj Natikar
Cc: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
linux-input, linux-iio, regressions
In-Reply-To: <4441bd6b-01cd-4f26-bf85-bde2e1bf404e@t-8ch.de>
[TLDR: This mail in primarily relevant for Linux regression tracking. A
change or fix related to the regression discussed in this thread was
posted or applied, but it did not use a Closes: tag to point to the
report, as Linus and the documentation call for. Things happen, no
worries -- but now the regression tracking bot needs to be told manually
about the fix. See link in footer if these mails annoy you.]
On 07.12.23 00:39, Thomas Weißschuh wrote:
> On 2023-09-19 13:40:45+0530, Basavaraj Natikar wrote:
> [...]
> This series is breaking probing of hid-sensor-als on Framework 13 AMD
> laptops [0].
> [...]
> #regzbot introduced: 5f05285df691b1e82108eead7165feae238c95ef
> #regzbot monitor: https://bugzilla.kernel.org/show_bug.cgi?id=218223
#regzbot fix: d4005431673929
#regzbot ignore-activity
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
^ permalink raw reply
* [git pull] Input updates for v6.7-rc6
From: Dmitry Torokhov @ 2023-12-23 7:48 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.7-rc6
to receive updates for the input subsystem. You will get:
- a quirk to AT keyboard driver to skip issuing "GET ID" command when 8042
is in translated mode and the device is a laptop/portable, because the
"GET ID" command makes a bunch of recent laptops unhappy
- a quirk to i8042 to disable multiplexed mode on Acer P459-G2-M which
causes issues on resume
- psmouse will activate native RMI4 protocol support for touchpad on
ThinkPad L14 G1
- addition of Razer Wolverine V2 ID to xpad gamepad driver
- mapping for airplane mode button in soc_button_array driver for TUXEDO
laptops
- improved error handling in ipaq-micro-keys driver
- amimouse being prepared for platform remove callback returning void
Changelog:
---------
Christoffer Sandberg (1):
Input: soc_button_array - add mapping for airplane mode button
Esther Shimanovich (1):
Input: i8042 - add nomux quirk for Acer P459-G2-M
Hans de Goede (1):
Input: atkbd - skip ATKBD_CMD_GETID in translated mode
Haoran Liu (1):
Input: ipaq-micro-keys - add error handling for devm_kmemdup
José Pekkarinen (1):
Input: psmouse - enable Synaptics InterTouch for ThinkPad L14 G1
Luca Weiss (1):
Input: xpad - add Razer Wolverine V2 support
Uwe Kleine-König (1):
Input: amimouse - convert to platform remove callback returning void
Diffstat:
--------
drivers/input/joystick/xpad.c | 1 +
drivers/input/keyboard/atkbd.c | 46 +++++++++++++++++++++++++++++---
drivers/input/keyboard/ipaq-micro-keys.c | 3 +++
drivers/input/misc/soc_button_array.c | 5 ++++
drivers/input/mouse/amimouse.c | 5 ++--
drivers/input/mouse/synaptics.c | 1 +
drivers/input/serio/i8042-acpipnpio.h | 8 ++++++
7 files changed, 62 insertions(+), 7 deletions(-)
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v1] dt-bindings: input: convert drv266x to json-schema
From: Dmitry Torokhov @ 2023-12-23 7:24 UTC (permalink / raw)
To: Anshul Dalal
Cc: linux-kernel, linux-input, devicetree, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-kernel-mentees
In-Reply-To: <20231221183109.684325-1-anshulusr@gmail.com>
On Fri, Dec 22, 2023 at 12:01:08AM +0530, Anshul Dalal wrote:
> Convert devicetree binding documentation for ti drv2665 and drv2667
> haptics driver to json-schema. The previously two separate bindings have
> been merged into a single drv266x.yaml.
>
> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [PATCH v3 1/2] Input: ilitek_ts_i2c - avoid wrong input subsystem sync
From: Francesco Dolcini @ 2023-12-22 18:31 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Emanuele Ghidoli, linux-input, linux-kernel, Francesco Dolcini
In-Reply-To: <20231222183114.30775-1-francesco@dolcini.it>
From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
For different reasons i2c transaction may fail or
report id message content may be wrong.
Avoid sync the input subsystem if message cannot be parsed.
An input subsystem sync without points is interpreted as
"nothing is touching the screen" while normally this is not the case.
Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v3: added reviewed by
---
drivers/input/touchscreen/ilitek_ts_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index fc4e39b6651a..250133f0d68f 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -203,9 +203,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
ilitek_touch_down(ts, id, x, y);
}
-err_sync_frame:
input_mt_sync_frame(input);
input_sync(input);
+err_sync_frame:
return error;
}
--
2.25.1
^ permalink raw reply related
* [PATCH v3 2/2] Input: ilitek_ts_i2c - add report id message validation
From: Francesco Dolcini @ 2023-12-22 18:31 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Emanuele Ghidoli, linux-input, linux-kernel, Francesco Dolcini
In-Reply-To: <20231222183114.30775-1-francesco@dolcini.it>
From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
Ilitek touch IC driver answer to plain i2c read request, after it has
generated an interrupt request, with a report id message starting
with an identifier and a series of points.
If a request is sent unsolicited by an interrupt request the answer
do not contain this identifier.
Add a test to discard these messages, making the driver robust to
spurious interrupt requests.
Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v3: added reviewed by
---
drivers/input/touchscreen/ilitek_ts_i2c.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index 250133f0d68f..557362490244 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -37,6 +37,8 @@
#define ILITEK_TP_CMD_GET_MCU_VER 0x61
#define ILITEK_TP_CMD_GET_IC_MODE 0xC0
+#define ILITEK_TP_I2C_REPORT_ID 0x48
+
#define REPORT_COUNT_ADDRESS 61
#define ILITEK_SUPPORT_MAX_POINT 40
@@ -163,6 +165,11 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
goto err_sync_frame;
}
+ if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
+ dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
+ goto err_sync_frame;
+ }
+
report_max_point = buf[REPORT_COUNT_ADDRESS];
if (report_max_point > ts->max_tp) {
dev_err(dev, "FW report max point:%d > panel info. max:%d\n",
--
2.25.1
^ permalink raw reply related
* [PATCH v3 0/2] Input: ilitek_ts_i2c - Fix spurious input events
From: Francesco Dolcini @ 2023-12-22 18:31 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Francesco Dolcini, linux-input, linux-kernel
From: Francesco Dolcini <francesco.dolcini@toradex.com>
A couple of fixes to prevent spurious events when the data buffer is not the
expected one.
v3:
- added reviewed-by and take over series from emanuele
v2:
- initial series, sent by mistake as v2 instead of v1
Emanuele Ghidoli (2):
Input: ilitek_ts_i2c - avoid wrong input subsystem sync
Input: ilitek_ts_i2c - add report id message validation
drivers/input/touchscreen/ilitek_ts_i2c.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH] HID: sensor-hub: Enable hid core report processing for all devices
From: Benjamin Tissoires @ 2023-12-22 18:27 UTC (permalink / raw)
To: linux-input, linux-iio, Yauhen Kharuzhy
Cc: Daniel Thompson, linux-kernel, Jiri Kosina, Jonathan Cameron,
Srinivas Pandruvada, Benjamin Tissoires
In-Reply-To: <20231219231503.1506801-1-jekhor@gmail.com>
On Wed, 20 Dec 2023 01:15:03 +0200, Yauhen Kharuzhy wrote:
> After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-function
> sensor devices") hub devices are claimed by hidraw driver in hid_connect().
> This causes stoppping of processing HID reports by hid core due to
> optimization.
>
> In such case, the hid-sensor-custom driver cannot match a known custom
> sensor in hid_sensor_custom_get_known() because it try to check custom
> properties which weren't filled from the report because hid core didn't
> parsed it.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.8/sensor-hub), thanks!
[1/1] HID: sensor-hub: Enable hid core report processing for all devices
https://git.kernel.org/hid/hid/c/8e2f79f41a5d
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: atmel,maxtouch: add poweroff-sleep property
From: Linus Walleij @ 2023-12-22 17:52 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: nick, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
nicolas.ferre, alexandre.belloni, claudiu.beznea,
francesco.dolcini, linux-input, devicetree, linux-arm-kernel,
linux-kernel, Stefan Eichenberger
In-Reply-To: <20231222095258.33369-2-eichest@gmail.com>
On Fri, Dec 22, 2023 at 10:53 AM Stefan Eichenberger <eichest@gmail.com> wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> Add a new property to indicate that the device should power off rather
> than use deep sleep. Deep sleep is a feature of the controller that
> expects the controller to remain powered in suspend. However, if a
> display shares its regulator with the touch controller, we may want to
> do a power off so that the display and touch controller do not use any
> power.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
This LGTM:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [RFC PATCH 1/5] dt-bindings: mfd: add entry for the Marvell 88PM88X PMICs
From: Karel Balej @ 2023-12-22 17:25 UTC (permalink / raw)
To: Rob Herring
Cc: Karel Balej, Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, linux-input, devicetree, linux-kernel,
Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20231218151718.GA3827526-robh@kernel.org>
Rob,
thank you very much for your feedback.
On Mon Dec 18, 2023 at 4:17 PM CET, Rob Herring wrote:
> > + Marvell 88PM880 and 88PM886 are two similar PMICs providing
> > + several functions such as onkey, regulators or battery and
> > + charger. Both seem to come in two revisions -- A0 and A1.
> > +
> > +properties:
> > + compatible:
> > + const: marvell,88pm886-a1
>
> The description talks about 4 different devices, but only 1 here.
>
> Do you expect to need A0 support? Devices with these PMICs should be
> known and few, right?
I know of three smartphones which have 88PM886 and all of them (at least
the revisions that have been tested) seem to use A1. So no, I don't know
of any device that would need A0, but I wanted have the driver ready in
case somebody needed to add it later. What change do you then suggest?
Thank you and kind regards,
K. B.
^ 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