Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] HID: multitouch: Add quirk for HONOR GLO-GXXX touchpad
From: Jiri Kosina @ 2023-11-21  8:40 UTC (permalink / raw)
  To: Aoba K; +Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <DM6PR04MB412176BD43A29E1E873FB7E5CEA9A@DM6PR04MB4121.namprd04.prod.outlook.com>

On Tue, 7 Nov 2023, Aoba K wrote:

> Honor MagicBook 13 2023 has a touchpad which do not switch to the
> multitouch mode until the input mode feature is written by the host.
> The touchpad do report the input mode at touchpad(3), while itself
> working under mouse mode. As a workaround, it is possible to call
> MT_QUIRE_FORCE_GET_FEATURE to force set feature in mt_set_input_mode
> for such device.
> The touchpad reports as BLTP7853, which cannot retrive any useful
> manufacture information on the internel by this string at present.
> As the serial number of the laptop is GLO-G52, while DMI info reports
> the laptop serial number as GLO-GXXX, this workaround should applied
> to all models which has the GLO-GXXX.

Hi,

unfortunately, your patch has been whitespace-damaged by your mail client. 
Could you please fix that and resend?

Thanks,

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [PATCH 11/32] hid/picolcd_fb: Set FBINFO_VIRTFB flag
From: Jiri Kosina @ 2023-11-21  8:42 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: deller, javierm, linux-fbdev, dri-devel, Bruno Prémont,
	Benjamin Tissoires, linux-input
In-Reply-To: <20231115102954.7102-12-tzimmermann@suse.de>

On Wed, 15 Nov 2023, Thomas Zimmermann wrote:

> The picolcd_fb driver operates on system memory. Mark the framebuffer
> accordingly. Helpers operating on the framebuffer memory will test
> for the presence of this flag.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: linux-input@vger.kernel.org

Acked-by: Jiri Kosina <jkosina@suse.cz>

I guess this will go in as one series together, right?

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [PATCH] usb: hid: add ALWAYS_POLL quirk for Apple kb
From: Jiri Kosina @ 2023-11-21  8:50 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: benjamin.tissoires, linux-input
In-Reply-To: <20231114145430.3765-1-oneukum@suse.com>

On Tue, 14 Nov 2023, Oliver Neukum wrote:

> These devices disconnect if suspended without
> remote wakeup. They can operate with the standard
> driver.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>

Applied, thanks Oliver.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [PATCH 0/2] hid-asus: reset the backlight brightness level on resume
From: Jiri Kosina @ 2023-11-21  8:52 UTC (permalink / raw)
  To: Denis Benato; +Cc: Luke D. Jones, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <b4356286-368a-49ec-b1f8-d7e5e4afdc25@gmail.com>

On Fri, 17 Nov 2023, Denis Benato wrote:

> > From: Denis Benato <benato.denis96@gmail.com>
> 
> I want to express my gratitude toward Luke for his guidance and his help 
> in submitting this fix.
> 
> I confirm those patches were sent in my behalf.

Luke, as you were in the supply chain of the patches, could you please 
provide Signed-off-by: tags so that I can add them into the chain?

Thanks,

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [PATCH 11/32] hid/picolcd_fb: Set FBINFO_VIRTFB flag
From: Thomas Zimmermann @ 2023-11-21  8:56 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: deller, javierm, linux-fbdev, dri-devel, Bruno Prémont,
	Benjamin Tissoires, linux-input
In-Reply-To: <nycvar.YFH.7.76.2311210942200.29220@cbobk.fhfr.pm>


[-- Attachment #1.1: Type: text/plain, Size: 954 bytes --]

Hi

Am 21.11.23 um 09:42 schrieb Jiri Kosina:
> On Wed, 15 Nov 2023, Thomas Zimmermann wrote:
> 
>> The picolcd_fb driver operates on system memory. Mark the framebuffer
>> accordingly. Helpers operating on the framebuffer memory will test
>> for the presence of this flag.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
>> Cc: Jiri Kosina <jikos@kernel.org>
>> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Cc: linux-input@vger.kernel.org
> 
> Acked-by: Jiri Kosina <jkosina@suse.cz>
> 
> I guess this will go in as one series together, right?

Yes. I intend to move all this through the DRM trees.

Best regards
Thomas

> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply

* Re: [RFC v2 4/7] HID: i2c-hid: Move i2c_hid_finish_hwreset() to after reading the report-descriptor
From: Hans de Goede @ 2023-11-21  9:52 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Jiri Kosina, Benjamin Tissoires, Julian Sax, ahormann,
	Bruno Jesus, Dietrich, kloxdami, Tim Aldridge, Rene Wagner,
	Federico Ricchiuto, linux-input
In-Reply-To: <CAD=FV=U+emgVbnRT2yQonZ013CRmYXK1bxh8+xGGn5LCnOmL5Q@mail.gmail.com>

Hi Doug,

Thank you for the reviews.

On 11/20/23 23:07, Doug Anderson wrote:
> Hi,
> 
> On Mon, Nov 20, 2023 at 11:33 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> @@ -741,12 +741,9 @@ static int i2c_hid_parse(struct hid_device *hid)
>>                 return -EINVAL;
>>         }
>>
>> +       mutex_lock(&ihid->reset_lock);
>>         do {
>> -               mutex_lock(&ihid->reset_lock);
>>                 ret = i2c_hid_start_hwreset(ihid);
>> -               if (ret == 0)
>> -                       ret = i2c_hid_finish_hwreset(ihid);
>> -               mutex_unlock(&ihid->reset_lock);
>>                 if (ret)
>>                         msleep(1000);
>>         } while (tries-- > 0 && ret);
> 
> Right after this loop, you have:
> 
> if (ret)
>   return ret;
> 
> That will return with the mutex held. It needs to be a "goto
> abort_reset". You'd also need to init `use_override` then, I think.

Ah, good catch, I will fix this for the next version.

Assuming there will be a next version. Did you read the cover-letter
part about the moving of the wait for reset to after the descriptor
read not fixing the missing reset ack 100% but rather only 50% or
so of the time ?

And do you have any opinion on if we should still move forward with
this patch-set or not ?

> I'll also note that it seems awkward that
> `clear_bit(I2C_HID_RESET_PENDING, &ihid->flags)` is scattered in so
> many places for error handling, but I couldn't really find a better
> way to do it. :-P

I guess we could just no clear it? Only the wait for reset
wait_event_timeout() cares about its value and if we run that
a second time then the bit will be set to 1 again before calling
it anyways...    Not sure I like my own suggestion here, but
it is an option. I'm afraid it may bite us later thogh if we
ever decide to check for the bit in another place.

Regards,

Hans



^ permalink raw reply

* [PATCH v9 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-11-21 10:17 UTC (permalink / raw)
  To: linux-input, devicetree
  Cc: Anshul Dalal, Conor Dooley, Dmitry Torokhov,
	Thomas Weißschuh, linux-kernel, Krzysztof Kozlowski,
	Conor Dooley, Rob Herring, Krzysztof Kozlowski, Jeff LaBundy,
	linux-kernel-mentees

Adds bindings for the Adafruit Seesaw Gamepad.

The gamepad functions as an i2c device with the default address of 0x50
and has an IRQ pin that can be enabled in the driver to allow for a rising
edge trigger on each button press or joystick movement.

Product page:
  https://www.adafruit.com/product/5743
Arduino driver:
  https://github.com/adafruit/Adafruit_Seesaw

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>

---

Changes for v9:
- Added interrupt in example

v8: https://lore.kernel.org/lkml/20231108005337.45069-1-anshulusr@gmail.com/

Changes for v8:
- no updates

v7: https://lore.kernel.org/lkml/20231106164134.114668-1-anshulusr@gmail.com/

Changes for v7:
- no updates

v6: https://lore.kernel.org/lkml/20231027051819.81333-1-anshulusr@gmail.com/

Changes for v6:
- no updates

v5: https://lore.kernel.org/lkml/20231017034356.1436677-1-anshulusr@gmail.com/

Changes for v5:
- Added link to the datasheet

v4: https://lore.kernel.org/lkml/20231010184827.1213507-1-anshulusr@gmail.com/

Changes for v4:
- Fixed the URI for the id field
- Added `interrupts` property

v3: https://lore.kernel.org/linux-input/20231008185709.2448423-1-anshulusr@gmail.com/

Changes for v3:
- Updated id field to reflect updated file name from previous version
- Added `reg` property

v2: https://lore.kernel.org/linux-input/20231008172435.2391009-1-anshulusr@gmail.com/

Changes for v2:
- Renamed file to `adafruit,seesaw-gamepad.yaml`
- Removed quotes for `$id` and `$schema`
- Removed "Bindings for" from the description
- Changed node name to the generic name "joystick"
- Changed compatible to 'adafruit,seesaw-gamepad' instead of
  'adafruit,seesaw_gamepad'

v1: https://lore.kernel.org/linux-input/20231007144052.1535417-1-anshulusr@gmail.com/
---
 .../input/adafruit,seesaw-gamepad.yaml        | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml

diff --git a/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
new file mode 100644
index 000000000000..0f17e110f368
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/adafruit,seesaw-gamepad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Adafruit Mini I2C Gamepad with seesaw
+
+maintainers:
+  - Anshul Dalal <anshulusr@gmail.com>
+
+description: |
+  Adafruit Mini I2C Gamepad
+
+    +-----------------------------+
+    |   ___                       |
+    |  /   \               (X)    |
+    | |  S  |  __   __  (Y)   (A) |
+    |  \___/  |ST| |SE|    (B)    |
+    |                             |
+    +-----------------------------+
+
+  S -> 10-bit precision bidirectional analog joystick
+  ST -> Start
+  SE -> Select
+  X, A, B, Y -> Digital action buttons
+
+  Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
+  Product page: https://www.adafruit.com/product/5743
+  Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
+
+properties:
+  compatible:
+    const: adafruit,seesaw-gamepad
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description:
+      The gamepad's IRQ pin triggers a rising edge if interrupts are enabled.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        joystick@50 {
+            compatible = "adafruit,seesaw-gamepad";
+            interrupts = <18 IRQ_TYPE_EDGE_RISING>;
+            reg = <0x50>;
+        };
+    };
-- 
2.42.1


^ permalink raw reply related

* [PATCH v9 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-11-21 10:17 UTC (permalink / raw)
  To: linux-input, devicetree
  Cc: Anshul Dalal, Conor Dooley, Dmitry Torokhov,
	Thomas Weißschuh, linux-kernel, Krzysztof Kozlowski,
	Conor Dooley, Rob Herring, Krzysztof Kozlowski, Jeff LaBundy,
	linux-kernel-mentees
In-Reply-To: <20231121101751.2189965-1-anshulusr@gmail.com>

Adds a driver for a mini gamepad that communicates over i2c, the gamepad
has bidirectional thumb stick input and six buttons.

The gamepad chip utilizes the open framework from Adafruit called 'Seesaw'
to transmit the ADC data for the joystick and digital pin state for the
buttons. I have only implemented the functionality required to receive the
thumb stick and button state.

Steps in reading the gamepad state over i2c:
  1. Reset the registers
  2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input
      `BUTTON_MASK`: A bit-map for the six digital pins internally
       connected to the joystick buttons.
  3. Enable internal pullup resistors for the `BUTTON_MASK`
  4. Bulk set the pin state HIGH for `BUTTON_MASK`
  5. Poll the device for button and joystick state done by:
      `seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)`

Product page:
  https://www.adafruit.com/product/5743
Arduino driver:
  https://github.com/adafruit/Adafruit_Seesaw

Driver tested on RPi Zero 2W

Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>

---

Changes for v9:
- Added of_device_id

v8: https://lore.kernel.org/lkml/20231108005337.45069-1-anshulusr@gmail.com/

Changes for v8:
- Updated invalid references to `adafruit_seesaw` to `adafruit-seesaw`

v7: https://lore.kernel.org/lkml/20231106164134.114668-1-anshulusr@gmail.com/

Changes for v7:
adafruit-seesaw.c
- Fixed formatting for macro definitions
- Made SEESAW_BUTTON_MASK static const
- Removed __be16 type for x and y fields of seesaw_data
- Used sparse_keymap implementation instead of custom keymap
- Used i2c_msg instead of i2c_master_send and recv in
  seesaw_register_read
- Use temporary variable `adc_data` to store data received from ADC
- Changed read_buf from u8[4] to __be32
- Use usleep_range instead of msleep
- Removed 'Reviewed-by: Thomas Weißschuh' due to large number of changes
  since last review
Kconfig:
- Added `select INPUT_SPARSEKMAP`

v6: https://lore.kernel.org/lkml/20231027051819.81333-1-anshulusr@gmail.com/

Changes for v6:
- Added TODO
- Removed `clang-format` directives
- Namespaced device buttons
- Removed `char physical_path[32]` field from `struct seesaw_gamepad`
- Added `packed` attribute to `struct seesaw_data`
- Moved from having booleans per button to single `u32 button_state`
- Added `seesaw_button_description` array to directly associate device
  buttons with respective keycodes
- Added wrapper functions `seesaw_register_` around `i2c_master_` API
- Ratelimited input error reporting with `dev_err_ratelimited`
- Removed `of_device_id`

v5: https://lore.kernel.org/lkml/20231017034356.1436677-1-anshulusr@gmail.com/

Changes for v5:
- Added link to the datasheet
- Added debug log message when `seesaw_read_data` fails

v4: https://lore.kernel.org/lkml/20231010184827.1213507-1-anshulusr@gmail.com/

Changes for v4:
- Changed `1UL << BUTTON_` to BIT(BUTTON_)
- Removed `hardware_id` field from `struct seesaw_gamepad`
- Removed redundant checks for the number of bytes written and received by
  `i2c_master_send` and `i2c_master_recv`
- Used `get_unaligned_be32` to instantiate `u32 result` from `read_buf`
- Changed  `result & (1UL << BUTTON_)` to
  `test_bit(BUTTON_, (long *)&result)`
- Changed `KBUILD_MODNAME` in id-tables to `SEESAW_DEVICE_NAME`
- Fixed formatting issues
- Changed button reporting:
    Since the gamepad had the action buttons in a non-standard layout:
         (X)
      (Y)   (A)
         (B)
    Therefore moved to using generic directional action button event codes
    instead of BTN_[ABXY].

v3: https://lore.kernel.org/linux-input/20231008185709.2448423-1-anshulusr@gmail.com/

Changes for v3:
- no updates

v2: https://lore.kernel.org/linux-input/20231008172435.2391009-1-anshulusr@gmail.com/

Changes for v2:
adafruit-seesaw.c:
- Renamed file from 'adafruit_seesaw.c'
- Changed device name from 'seesaw_gamepad' to 'seesaw-gamepad'
- Changed count parameter for receiving joystick x on line 118:
    `2` to `sizeof(write_buf)`
- Fixed invalid buffer size on line 123 and 126:
    `data->y` to `sizeof(data->y)`
- Added comment for the `mdelay(10)` on line 169
- Changed inconsistent indentation on line 271
Kconfig:
- Fixed indentation for the help text
- Updated module name
Makefile:
- Updated module object file name
MAINTAINERS:
- Updated file name for the driver and bindings

v1: https://lore.kernel.org/linux-input/20231007144052.1535417-1-anshulusr@gmail.com/
---
 MAINTAINERS                              |   7 +
 drivers/input/joystick/Kconfig           |  10 +
 drivers/input/joystick/Makefile          |   1 +
 drivers/input/joystick/adafruit-seesaw.c | 318 +++++++++++++++++++++++
 4 files changed, 336 insertions(+)
 create mode 100644 drivers/input/joystick/adafruit-seesaw.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 81d5fc0bba68..b3f101edc24b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -441,6 +441,13 @@ W:	http://wiki.analog.com/AD7879
 W:	https://ez.analog.com/linux-software-drivers
 F:	drivers/input/touchscreen/ad7879.c
 
+ADAFRUIT MINI I2C GAMEPAD
+M:	Anshul Dalal <anshulusr@gmail.com>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
+F:	drivers/input/joystick/adafruit-seesaw.c
+
 ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
 M:	Jiri Kosina <jikos@kernel.org>
 S:	Maintained
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index ac6925ce8366..7755e5b454d2 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -412,4 +412,14 @@ config JOYSTICK_SENSEHAT
 	  To compile this driver as a module, choose M here: the
 	  module will be called sensehat_joystick.
 
+config JOYSTICK_SEESAW
+	tristate "Adafruit Mini I2C Gamepad with Seesaw"
+	depends on I2C
+	select INPUT_SPARSEKMAP
+	help
+	  Say Y here if you want to use the Adafruit Mini I2C Gamepad.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called adafruit-seesaw.
+
 endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 3937535f0098..9976f596a920 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_JOYSTICK_N64)		+= n64joy.o
 obj-$(CONFIG_JOYSTICK_PSXPAD_SPI)	+= psxpad-spi.o
 obj-$(CONFIG_JOYSTICK_PXRC)		+= pxrc.o
 obj-$(CONFIG_JOYSTICK_QWIIC)		+= qwiic-joystick.o
+obj-$(CONFIG_JOYSTICK_SEESAW)		+= adafruit-seesaw.o
 obj-$(CONFIG_JOYSTICK_SENSEHAT)	+= sensehat-joystick.o
 obj-$(CONFIG_JOYSTICK_SIDEWINDER)	+= sidewinder.o
 obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
diff --git a/drivers/input/joystick/adafruit-seesaw.c b/drivers/input/joystick/adafruit-seesaw.c
new file mode 100644
index 000000000000..856ca223df58
--- /dev/null
+++ b/drivers/input/joystick/adafruit-seesaw.c
@@ -0,0 +1,318 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 Anshul Dalal <anshulusr@gmail.com>
+ *
+ * Driver for Adafruit Mini I2C Gamepad
+ *
+ * Based on the work of:
+ *	Oleh Kravchenko (Sparkfun Qwiic Joystick driver)
+ *
+ * Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
+ * Product page: https://www.adafruit.com/product/5743
+ * Firmware and hardware sources: https://github.com/adafruit/Adafruit_Seesaw
+ *
+ * TODO:
+ *	- Add interrupt support
+ */
+
+#include <asm/unaligned.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#define SEESAW_DEVICE_NAME	     "seesaw-gamepad"
+
+#define SEESAW_STATUS_BASE	     0x00
+#define SEESAW_GPIO_BASE	     0x01
+#define SEESAW_ADC_BASE		     0x09
+
+#define SEESAW_GPIO_DIRCLR_BULK	     0x03
+#define SEESAW_GPIO_BULK	     0x04
+#define SEESAW_GPIO_BULK_SET	     0x05
+#define SEESAW_GPIO_PULLENSET	     0x0b
+
+#define SEESAW_STATUS_HW_ID	     0x01
+#define SEESAW_STATUS_SWRST	     0x7f
+
+#define SEESAW_ADC_OFFSET	     0x07
+
+#define SEESAW_BUTTON_A		     0x05
+#define SEESAW_BUTTON_B		     0x01
+#define SEESAW_BUTTON_X		     0x06
+#define SEESAW_BUTTON_Y		     0x02
+#define SEESAW_BUTTON_START	     0x10
+#define SEESAW_BUTTON_SELECT	     0x00
+
+#define SEESAW_ANALOG_X		     0x0e
+#define SEESAW_ANALOG_Y		     0x0f
+
+#define SEESAW_JOYSTICK_MAX_AXIS     1023
+#define SEESAW_JOYSTICK_FUZZ	     2
+#define SEESAW_JOYSTICK_FLAT	     4
+
+#define SEESAW_GAMEPAD_POLL_INTERVAL 16
+#define SEESAW_GAMEPAD_POLL_MIN	     8
+#define SEESAW_GAMEPAD_POLL_MAX	     32
+
+#define MSEC_PER_USEC		     1000
+
+static const u32 SEESAW_BUTTON_MASK =
+	BIT(SEESAW_BUTTON_A) | BIT(SEESAW_BUTTON_B) | BIT(SEESAW_BUTTON_X) |
+	BIT(SEESAW_BUTTON_Y) | BIT(SEESAW_BUTTON_START) |
+	BIT(SEESAW_BUTTON_SELECT);
+
+struct seesaw_gamepad {
+	struct input_dev *input_dev;
+	struct i2c_client *i2c_client;
+};
+
+struct seesaw_data {
+	u16 x;
+	u16 y;
+	u32 button_state;
+};
+
+static const struct key_entry seesaw_buttons_new[] = {
+	{ KE_KEY, SEESAW_BUTTON_A, .keycode = BTN_SOUTH },
+	{ KE_KEY, SEESAW_BUTTON_B, .keycode = BTN_EAST },
+	{ KE_KEY, SEESAW_BUTTON_X, .keycode = BTN_NORTH },
+	{ KE_KEY, SEESAW_BUTTON_Y, .keycode = BTN_WEST },
+	{ KE_KEY, SEESAW_BUTTON_START, .keycode = BTN_START },
+	{ KE_KEY, SEESAW_BUTTON_SELECT, .keycode = BTN_SELECT },
+	{ KE_END, 0 }
+};
+
+static int seesaw_register_read(struct i2c_client *client, u8 register_high,
+				u8 register_low, char *buf, int count)
+{
+	int ret;
+	u8 register_buf[2] = { register_high, register_low };
+
+	struct i2c_msg message_buf[2] = {
+		{
+			.addr = client->addr,
+			.flags = client->flags,
+			.len = sizeof(register_buf),
+			.buf = register_buf,
+		},
+		{
+			.addr = client->addr,
+			.flags = client->flags | I2C_M_RD,
+			.len = count,
+			.buf = buf,
+		},
+	};
+	ret = i2c_transfer(client->adapter, message_buf,
+			   ARRAY_SIZE(message_buf));
+
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int seesaw_register_write_u8(struct i2c_client *client, u8 register_high,
+				    u8 register_low, u8 value)
+{
+	int ret;
+	u8 write_buf[3] = { register_high, register_low, value };
+
+	ret = i2c_master_send(client, write_buf, sizeof(write_buf));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int seesaw_register_write_u32(struct i2c_client *client,
+				     u8 register_high, u8 register_low,
+				     u32 value)
+{
+	int ret;
+	u8 write_buf[6] = { register_high, register_low };
+
+	put_unaligned_be32(value, write_buf + 2);
+	ret = i2c_master_send(client, write_buf, sizeof(write_buf));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)
+{
+	int ret;
+	__be16 adc_data;
+	__be32 read_buf;
+
+	ret = seesaw_register_read(client, SEESAW_GPIO_BASE, SEESAW_GPIO_BULK,
+				   (char *)&read_buf, sizeof(read_buf));
+	if (ret)
+		return ret;
+
+	data->button_state = ~be32_to_cpu(read_buf);
+
+	ret = seesaw_register_read(client, SEESAW_ADC_BASE,
+				   SEESAW_ADC_OFFSET + SEESAW_ANALOG_X,
+				   (char *)&adc_data, sizeof(adc_data));
+	if (ret)
+		return ret;
+	/*
+	 * ADC reads left as max and right as 0, must be reversed since kernel
+	 * expects reports in opposite order.
+	 */
+	data->x = SEESAW_JOYSTICK_MAX_AXIS - be16_to_cpu(adc_data);
+
+	ret = seesaw_register_read(client, SEESAW_ADC_BASE,
+				   SEESAW_ADC_OFFSET + SEESAW_ANALOG_Y,
+				   (char *)&adc_data, sizeof(adc_data));
+	if (ret)
+		return ret;
+	data->y = be16_to_cpu(adc_data);
+
+	return 0;
+}
+
+static void seesaw_poll(struct input_dev *input)
+{
+	int err, i;
+	struct seesaw_gamepad *private = input_get_drvdata(input);
+	struct seesaw_data data;
+
+	err = seesaw_read_data(private->i2c_client, &data);
+	if (err) {
+		dev_err_ratelimited(&input->dev,
+				    "failed to read joystick state: %d\n", err);
+		return;
+	}
+
+	input_report_abs(input, ABS_X, data.x);
+	input_report_abs(input, ABS_Y, data.y);
+
+	for_each_set_bit(i, (long *)&SEESAW_BUTTON_MASK,
+			 BITS_PER_TYPE(SEESAW_BUTTON_MASK)) {
+		if (!sparse_keymap_report_event(
+			    input, i, data.button_state & BIT(i), false)) {
+			dev_err_ratelimited(&input->dev,
+					    "failed to report keymap event");
+		};
+	}
+
+	input_sync(input);
+}
+
+static int seesaw_probe(struct i2c_client *client)
+{
+	int ret;
+	u8 hardware_id;
+	struct seesaw_gamepad *seesaw;
+
+	ret = seesaw_register_write_u8(client, SEESAW_STATUS_BASE,
+				       SEESAW_STATUS_SWRST, 0xFF);
+	if (ret)
+		return ret;
+
+	/* Wait for the registers to reset before proceeding */
+	usleep_range(10 * MSEC_PER_USEC, 15 * MSEC_PER_USEC);
+
+	seesaw = devm_kzalloc(&client->dev, sizeof(*seesaw), GFP_KERNEL);
+	if (!seesaw)
+		return -ENOMEM;
+
+	ret = seesaw_register_read(client, SEESAW_STATUS_BASE,
+				   SEESAW_STATUS_HW_ID, &hardware_id,
+				   sizeof(hardware_id));
+	if (ret)
+		return ret;
+
+	dev_dbg(&client->dev, "Adafruit Seesaw Gamepad, Hardware ID: %02x\n",
+		hardware_id);
+
+	/* Set Pin Mode to input and enable pull-up resistors */
+	ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
+					SEESAW_GPIO_DIRCLR_BULK,
+					SEESAW_BUTTON_MASK);
+	if (ret)
+		return ret;
+	ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
+					SEESAW_GPIO_PULLENSET,
+					SEESAW_BUTTON_MASK);
+	if (ret)
+		return ret;
+	ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
+					SEESAW_GPIO_BULK_SET,
+					SEESAW_BUTTON_MASK);
+	if (ret)
+		return ret;
+
+	seesaw->i2c_client = client;
+	seesaw->input_dev = devm_input_allocate_device(&client->dev);
+	if (!seesaw->input_dev)
+		return -ENOMEM;
+
+	seesaw->input_dev->id.bustype = BUS_I2C;
+	seesaw->input_dev->name = "Adafruit Seesaw Gamepad";
+	seesaw->input_dev->phys = "i2c/" SEESAW_DEVICE_NAME;
+	input_set_drvdata(seesaw->input_dev, seesaw);
+	input_set_abs_params(seesaw->input_dev, ABS_X, 0,
+			     SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+			     SEESAW_JOYSTICK_FLAT);
+	input_set_abs_params(seesaw->input_dev, ABS_Y, 0,
+			     SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+			     SEESAW_JOYSTICK_FLAT);
+
+	ret = sparse_keymap_setup(seesaw->input_dev, seesaw_buttons_new, NULL);
+	if (ret) {
+		dev_err(&client->dev,
+			"failed to set up input device keymap: %d\n", ret);
+		return ret;
+	}
+
+	ret = input_setup_polling(seesaw->input_dev, seesaw_poll);
+	if (ret) {
+		dev_err(&client->dev, "failed to set up polling: %d\n", ret);
+		return ret;
+	}
+
+	input_set_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_INTERVAL);
+	input_set_max_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MAX);
+	input_set_min_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MIN);
+
+	ret = input_register_device(seesaw->input_dev);
+	if (ret) {
+		dev_err(&client->dev, "failed to register joystick: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct i2c_device_id seesaw_id_table[] = {
+	{ SEESAW_DEVICE_NAME, 0 },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, seesaw_id_table);
+
+static const struct of_device_id seesaw_of_table[] = {
+	{ .compatible = "adafruit,seesaw-gamepad"},
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, seesaw_of_table);
+
+static struct i2c_driver seesaw_driver = {
+	.driver = {
+		.name = SEESAW_DEVICE_NAME,
+		.of_match_table = of_match_ptr(seesaw_of_table),
+	},
+	.id_table = seesaw_id_table,
+	.probe = seesaw_probe,
+};
+module_i2c_driver(seesaw_driver);
+
+MODULE_AUTHOR("Anshul Dalal <anshulusr@gmail.com>");
+MODULE_DESCRIPTION("Adafruit Mini I2C Gamepad driver");
+MODULE_LICENSE("GPL");
-- 
2.42.1


^ permalink raw reply related

* [PATCH] Input: synaptics-rmi4 - remove no-op reset handler
From: Matthias Schiffer @ 2023-11-21 10:36 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, linux, Matthias Schiffer

None of the individual function drivers have ever implemented a reset()
operation, meaning that reset_one_function() and in turn
rmi_driver_process_reset_requests() have always been no-ops. Get rid of
them.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 drivers/input/rmi4/rmi_bus.h    |  1 -
 drivers/input/rmi4/rmi_driver.c | 38 ---------------------------------
 2 files changed, 39 deletions(-)

diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index 25df6320f9f1d..9b0860a8b06f9 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -80,7 +80,6 @@ struct rmi_function_handler {
 	int (*probe)(struct rmi_function *fn);
 	void (*remove)(struct rmi_function *fn);
 	int (*config)(struct rmi_function *fn);
-	int (*reset)(struct rmi_function *fn);
 	irqreturn_t (*attention)(int irq, void *ctx);
 	int (*suspend)(struct rmi_function *fn);
 	int (*resume)(struct rmi_function *fn);
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 258d5fe3d395c..131add4b9affd 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -57,25 +57,6 @@ void rmi_free_function_list(struct rmi_device *rmi_dev)
 	data->f34_container = NULL;
 }
 
-static int reset_one_function(struct rmi_function *fn)
-{
-	struct rmi_function_handler *fh;
-	int retval = 0;
-
-	if (!fn || !fn->dev.driver)
-		return 0;
-
-	fh = to_rmi_function_handler(fn->dev.driver);
-	if (fh->reset) {
-		retval = fh->reset(fn);
-		if (retval < 0)
-			dev_err(&fn->dev, "Reset failed with code %d.\n",
-				retval);
-	}
-
-	return retval;
-}
-
 static int configure_one_function(struct rmi_function *fn)
 {
 	struct rmi_function_handler *fh;
@@ -95,21 +76,6 @@ static int configure_one_function(struct rmi_function *fn)
 	return retval;
 }
 
-static int rmi_driver_process_reset_requests(struct rmi_device *rmi_dev)
-{
-	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
-	struct rmi_function *entry;
-	int retval;
-
-	list_for_each_entry(entry, &data->function_list, node) {
-		retval = reset_one_function(entry);
-		if (retval < 0)
-			return retval;
-	}
-
-	return 0;
-}
-
 static int rmi_driver_process_config_requests(struct rmi_device *rmi_dev)
 {
 	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
@@ -445,10 +411,6 @@ static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
 		return error;
 	}
 
-	error = rmi_driver_process_reset_requests(rmi_dev);
-	if (error < 0)
-		return error;
-
 	error = rmi_driver_process_config_requests(rmi_dev);
 	if (error < 0)
 		return error;
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/


^ permalink raw reply related

* Re: [PATCH v9 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Rob Herring @ 2023-11-21 11:37 UTC (permalink / raw)
  To: Anshul Dalal
  Cc: Conor Dooley, linux-kernel, Dmitry Torokhov, linux-kernel-mentees,
	Conor Dooley, Rob Herring, Krzysztof Kozlowski, Jeff LaBundy,
	devicetree, linux-input, Thomas Weißschuh,
	Krzysztof Kozlowski
In-Reply-To: <20231121101751.2189965-1-anshulusr@gmail.com>


On Tue, 21 Nov 2023 15:47:48 +0530, Anshul Dalal wrote:
> Adds bindings for the Adafruit Seesaw Gamepad.
> 
> The gamepad functions as an i2c device with the default address of 0x50
> and has an IRQ pin that can be enabled in the driver to allow for a rising
> edge trigger on each button press or joystick movement.
> 
> Product page:
>   https://www.adafruit.com/product/5743
> Arduino driver:
>   https://github.com/adafruit/Adafruit_Seesaw
> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
> 
> ---
> 
> Changes for v9:
> - Added interrupt in example
> 
> v8: https://lore.kernel.org/lkml/20231108005337.45069-1-anshulusr@gmail.com/
> 
> Changes for v8:
> - no updates
> 
> v7: https://lore.kernel.org/lkml/20231106164134.114668-1-anshulusr@gmail.com/
> 
> Changes for v7:
> - no updates
> 
> v6: https://lore.kernel.org/lkml/20231027051819.81333-1-anshulusr@gmail.com/
> 
> Changes for v6:
> - no updates
> 
> v5: https://lore.kernel.org/lkml/20231017034356.1436677-1-anshulusr@gmail.com/
> 
> Changes for v5:
> - Added link to the datasheet
> 
> v4: https://lore.kernel.org/lkml/20231010184827.1213507-1-anshulusr@gmail.com/
> 
> Changes for v4:
> - Fixed the URI for the id field
> - Added `interrupts` property
> 
> v3: https://lore.kernel.org/linux-input/20231008185709.2448423-1-anshulusr@gmail.com/
> 
> Changes for v3:
> - Updated id field to reflect updated file name from previous version
> - Added `reg` property
> 
> v2: https://lore.kernel.org/linux-input/20231008172435.2391009-1-anshulusr@gmail.com/
> 
> Changes for v2:
> - Renamed file to `adafruit,seesaw-gamepad.yaml`
> - Removed quotes for `$id` and `$schema`
> - Removed "Bindings for" from the description
> - Changed node name to the generic name "joystick"
> - Changed compatible to 'adafruit,seesaw-gamepad' instead of
>   'adafruit,seesaw_gamepad'
> 
> v1: https://lore.kernel.org/linux-input/20231007144052.1535417-1-anshulusr@gmail.com/
> ---
>  .../input/adafruit,seesaw-gamepad.yaml        | 61 +++++++++++++++++++
>  1 file changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.example.dts:30.34-35 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231121101751.2189965-1-anshulusr@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply

* Re: Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2023-11-21 11:33 UTC (permalink / raw)
  To: Pavel Machek, Jani Nikula, hdegoede, jikos
  Cc: Miguel Ojeda, Lee Jones, linux-kernel,
	dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds
In-Reply-To: <ZVvHG/Q+V6kCnfKZ@duo.ucw.cz>

Hi,

Am 20.11.23 um 21:52 schrieb Pavel Machek:
> Hi!
>
>>>> So... a bit of rationale. The keyboard does not really fit into the
>>>> LED subsystem; LEDs are expected to be independent ("hdd led") and not
>>>> a matrix of them.
>>> Makes sense.
>>>
>>>> We do see various strange displays these days -- they commonly have
>>>> rounded corners and holes in them. I'm not sure how that's currently
>>>> supported, but I believe it is reasonable to view keyboard as a
>>>> display with slightly weird placing of pixels.
>>>>
>>>> Plus, I'd really like to play tetris on one of those :-).
>>>>
>>>> So, would presenting them as auxdisplay be acceptable? Or are there
>>>> better options?
>>> It sounds like a fair use case -- auxdisplay are typically simple
>>> character-based or small graphical displays, e.g. 128x64, that may not
>>> be a "main" / usual screen as typically understood, but the concept is
>>> a bit fuzzy and we are a bit of a catch-all.
>>>
>>> And "keyboard backlight display with a pixel/color per-key" does not
>>> sound like a "main" screen, and having some cute effects displayed
>>> there are the kind of thing that one could do in the usual small
>>> graphical ones too. :)
>>>
>>> But if somebody prefers to create new categories (or subcategories
>>> within auxdisplay) to hold these, that could be nice too (in the
>>> latter case, I would perhaps suggest reorganizing all of the existing
>>> ones while at it).
>> One could also reasonably make the argument that controlling the
>> individual keyboard key backlights should be part of the input
>> subsystem. It's not a display per se. (Unless you actually have small
>> displays on the keycaps, and I think that's a thing too.)
> While it would not be completely crazy to do that... I believe the
> backlight is more of a display and less of a keyboard. Plus input
> subystem is very far away from supporting this, and we had no input
> from input people here.
>
> I don't think LED subsystem is right place for this, and I believe
> auxdisplay makes slightly more sense than input.
>
> Unless someone steps up, I'd suggest Werner tries to implement this as
> an auxdisplay. [And yes, this will not be simple task. RGB on LED is
> different from RGB on display. But there are other LED displays, so
> auxdisplay should handle this. Plus pixels are really funnily
> shaped. But displays with missing pixels -- aka holes for camera --
> are common in phones, and I believe we'll get variable pixel densities
> -- less dense over camera -- too. So displays will have to deal with
> these in the end.]

Another idea I want to throw in the mix:

Maybe the kernel is not the right place to implement this at all. RGB stuff is 
not at all standardized and every vendor is doing completely different 
interfaces, which does not fit the kernel userpsace apis desire to be uniformal 
and fixed. e.g. Auxdisplay might fit static setting of RGB values, but it does 
not fit the snake-effect mode, or the raindrops mode, or the 
4-different-colors-in-the-edges-breathing-and-color-cycling mode.

So my current idea: Implement these keyboards as a single zone RGB kbd_backlight 
in the leds interface to have something functional out of the box, but make it 
runtime disable-able if something like 
https://gitlab.com/CalcProgrammer1/OpenRGB wants to take over more fine granular 
control from userspace via hidraw.

Kind regards,

Werner

>
> Best regards,
> 								Pavel

^ permalink raw reply

* Re: [PATCH v5 06/17] platform/x86/amd/pmf: Add support to get inputs from other subsystems
From: Ilpo Järvinen @ 2023-11-21 12:02 UTC (permalink / raw)
  To: Shyam Sundar S K
  Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
	benjamin.tissoires, Patil.Reddy, mario.limonciello,
	platform-driver-x86, linux-input
In-Reply-To: <20231117080747.3643990-7-Shyam-sundar.S-k@amd.com>

[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]

On Fri, 17 Nov 2023, Shyam Sundar S K wrote:

> PMF driver sends changing inputs from each subystem to TA for evaluating
> the conditions in the policy binary.
> 
> Add initial support of plumbing in the PMF driver for Smart PC to get
> information from other subsystems in the kernel.
> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---

> +static int amd_pmf_get_battery_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> +	int val;
> +
> +	val = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_PRESENT);
> +	if (val >= 0 && val != 1)
> +		return -ENODEV;

This no longer handles errors and was not what I suggested. With 
splitting the check into two I meant this:

	val = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_PRESENT);
	if (val < 0)
		return val;
	if (val != 1)
		return -NODEV;

After fixing this, please add:

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

> +	in->ev_info.bat_percentage = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_CAPACITY);
> +	/* all values in mWh metrics */
> +	in->ev_info.bat_design = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN) /
> +		MILLIWATT_PER_WATT;
> +	in->ev_info.full_charge_capacity = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL) /
> +		MILLIWATT_PER_WATT;
> +	in->ev_info.drain_rate = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_POWER_NOW) /
> +		MILLIWATT_PER_WATT;
> +
> +	return 0;
> +}

-- 
 i.

^ permalink raw reply

* Re: [PATCH v5 10/17] platform/x86/amd/pmf: Add facility to dump TA inputs
From: Ilpo Järvinen @ 2023-11-21 12:06 UTC (permalink / raw)
  To: Shyam Sundar S K
  Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
	benjamin.tissoires, Patil.Reddy, mario.limonciello,
	platform-driver-x86, linux-input
In-Reply-To: <20231117080747.3643990-11-Shyam-sundar.S-k@amd.com>

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

On Fri, 17 Nov 2023, Shyam Sundar S K wrote:

> PMF driver sends constant inputs to TA which its gets via the other
> subsystems in the kernel. To debug certain TA issues knowing what inputs
> being sent to TA becomes critical. Add debug facility to the driver which
> can isolate Smart PC and TA related issues.
> 
> Also, make source_as_str() as non-static function as this helper is
> required outside of sps.c file.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply

* Re: [PATCH v5 11/17] platform/x86/amd/pmf: Add capability to sideload of policy binary
From: Ilpo Järvinen @ 2023-11-21 12:17 UTC (permalink / raw)
  To: Shyam Sundar S K
  Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
	benjamin.tissoires, Patil.Reddy, mario.limonciello,
	platform-driver-x86, linux-input
In-Reply-To: <20231117080747.3643990-12-Shyam-sundar.S-k@amd.com>

On Fri, 17 Nov 2023, Shyam Sundar S K wrote:

> A policy binary is OS agnostic, and the same policies are expected to work
> across the OSes.  At times it becomes difficult to debug when the policies
> inside the policy binaries starts to misbehave. Add a way to sideload such
> policies independently to debug them via a debugfs entry.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---

> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 5f10e5c6335e..f73663c629fe 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c

> +#ifdef CONFIG_AMD_PMF_DEBUG
> +static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> +				   size_t length, loff_t *pos)
> +{
> +	struct amd_pmf_dev *dev = filp->private_data;
> +	int ret;
> +
> +	/* Policy binary size cannot exceed POLICY_BUF_MAX_SZ */
> +	if (length > POLICY_BUF_MAX_SZ || length == 0)
> +		return -EINVAL;
> +
> +	dev->policy_sz = length;
> +	if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
> +		return -EFAULT;
> +
> +	ret = amd_pmf_start_policy_engine(dev);

Is this call safe against concurrent invocations from two racing writes?

Other than that, this change looked fine.

> +	if (ret)
> +		return -EINVAL;
> +
> +	return length;
> +}


-- 
 i.


^ permalink raw reply

* Re: Implement per-key keyboard backlight as auxdisplay?
From: Hans de Goede @ 2023-11-21 12:20 UTC (permalink / raw)
  To: Werner Sembach, Pavel Machek, Jani Nikula, jikos
  Cc: Miguel Ojeda, Lee Jones, linux-kernel,
	dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds
In-Reply-To: <f4137e34-c7fb-4f21-bc93-1496cbf61fdf@tuxedocomputers.com>

Hi Werner,

On 11/21/23 12:33, Werner Sembach wrote:
> Hi,
> 
> Am 20.11.23 um 21:52 schrieb Pavel Machek:
>> Hi!
>>
>>>>> So... a bit of rationale. The keyboard does not really fit into the
>>>>> LED subsystem; LEDs are expected to be independent ("hdd led") and not
>>>>> a matrix of them.
>>>> Makes sense.
>>>>
>>>>> We do see various strange displays these days -- they commonly have
>>>>> rounded corners and holes in them. I'm not sure how that's currently
>>>>> supported, but I believe it is reasonable to view keyboard as a
>>>>> display with slightly weird placing of pixels.
>>>>>
>>>>> Plus, I'd really like to play tetris on one of those :-).
>>>>>
>>>>> So, would presenting them as auxdisplay be acceptable? Or are there
>>>>> better options?
>>>> It sounds like a fair use case -- auxdisplay are typically simple
>>>> character-based or small graphical displays, e.g. 128x64, that may not
>>>> be a "main" / usual screen as typically understood, but the concept is
>>>> a bit fuzzy and we are a bit of a catch-all.
>>>>
>>>> And "keyboard backlight display with a pixel/color per-key" does not
>>>> sound like a "main" screen, and having some cute effects displayed
>>>> there are the kind of thing that one could do in the usual small
>>>> graphical ones too. :)
>>>>
>>>> But if somebody prefers to create new categories (or subcategories
>>>> within auxdisplay) to hold these, that could be nice too (in the
>>>> latter case, I would perhaps suggest reorganizing all of the existing
>>>> ones while at it).
>>> One could also reasonably make the argument that controlling the
>>> individual keyboard key backlights should be part of the input
>>> subsystem. It's not a display per se. (Unless you actually have small
>>> displays on the keycaps, and I think that's a thing too.)
>> While it would not be completely crazy to do that... I believe the
>> backlight is more of a display and less of a keyboard. Plus input
>> subystem is very far away from supporting this, and we had no input
>> from input people here.
>>
>> I don't think LED subsystem is right place for this, and I believe
>> auxdisplay makes slightly more sense than input.
>>
>> Unless someone steps up, I'd suggest Werner tries to implement this as
>> an auxdisplay. [And yes, this will not be simple task. RGB on LED is
>> different from RGB on display. But there are other LED displays, so
>> auxdisplay should handle this. Plus pixels are really funnily
>> shaped. But displays with missing pixels -- aka holes for camera --
>> are common in phones, and I believe we'll get variable pixel densities
>> -- less dense over camera -- too. So displays will have to deal with
>> these in the end.]
> 
> Another idea I want to throw in the mix:
> 
> Maybe the kernel is not the right place to implement this at all. RGB stuff is not at all standardized and every vendor is doing completely different interfaces, which does not fit the kernel userpsace apis desire to be uniformal and fixed. e.g. Auxdisplay might fit static setting of RGB values, but it does not fit the snake-effect mode, or the raindrops mode, or the 4-different-colors-in-the-edges-breathing-and-color-cycling mode.
> 
> So my current idea: Implement these keyboards as a single zone RGB kbd_backlight in the leds interface to have something functional out of the box, but make it runtime disable-able if something like https://gitlab.com/CalcProgrammer1/OpenRGB wants to take over more fine granular control from userspace via hidraw.

That sounds like a good approach to me. We are seeing the same with game controllers where steam and wine/proton also sometimes use hidraw mode to get access to all the crazy^W interesting features.

That would mean that all we need to standardize and the kernel <-> userspace API level is adding a standard way to disable the single zone RGB kbd_backlight support in the kernel.

Regards,

Hans



^ permalink raw reply

* [PATCH RESEND] HID: multitouch: Add quirk for HONOR GLO-GXXX touchpad
From: Aoba K @ 2023-11-21 12:23 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, Henrik Rydberg
  Cc: linux-input

Honor MagicBook 13 2023 has a touchpad which do not switch to the
multitouch mode until the input mode feature is written by the host.
The touchpad do report the input mode at touchpad(3), while itself
working under mouse mode. As a workaround, it is possible to call
MT_QUIRE_FORCE_GET_FEATURE to force set feature in mt_set_input_mode
for such device.
The touchpad reports as BLTP7853, which cannot retrive any useful
manufacture information on the internel by this string at present.
As the serial number of the laptop is GLO-G52, while DMI info reports
the laptop serial number as GLO-GXXX, this workaround should applied
to all models which has the GLO-GXXX.

Signed-off-by: Aoba K <nexp_0x17@outlook.com>
---
 drivers/hid/hid-multitouch.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index e098cc7b39..fd5b0637da 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -2046,6 +2046,11 @@ static const struct hid_device_id mt_devices[] = {
 		MT_USB_DEVICE(USB_VENDOR_ID_HANVON_ALT,
 			USB_DEVICE_ID_HANVON_ALT_MULTITOUCH) },
 
+	/* HONOR GLO-GXXX panel */
+	{ .driver_data = MT_CLS_VTL,
+		HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
+			0x347d, 0x7853) },
+
 	/* Ilitek dual touch panel */
 	{  .driver_data = MT_CLS_NSMU,
 		MT_USB_DEVICE(USB_VENDOR_ID_ILITEK,

base-commit: 4ea4ed22b57846facd9cb4af5f67cb7bd2792cf3
-- 
2.42.0



^ permalink raw reply related

* [PATCH v10 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-11-21 12:34 UTC (permalink / raw)
  To: linux-input, devicetree
  Cc: Anshul Dalal, Conor Dooley, Dmitry Torokhov,
	Thomas Weißschuh, linux-kernel, Krzysztof Kozlowski,
	Conor Dooley, Rob Herring, Krzysztof Kozlowski, Jeff LaBundy,
	linux-kernel-mentees

Adds bindings for the Adafruit Seesaw Gamepad.

The gamepad functions as an i2c device with the default address of 0x50
and has an IRQ pin that can be enabled in the driver to allow for a rising
edge trigger on each button press or joystick movement.

Product page:
  https://www.adafruit.com/product/5743
Arduino driver:
  https://github.com/adafruit/Adafruit_Seesaw

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>

---

Changes for v10:
- Added interrupt-controller/irq.h header

v9: https://lore.kernel.org/lkml/20231121101751.2189965-1-anshulusr@gmail.com/

Changes for v9:
- Added interrupt in example

v8: https://lore.kernel.org/lkml/20231108005337.45069-1-anshulusr@gmail.com/

Changes for v8:
- no updates

v7: https://lore.kernel.org/lkml/20231106164134.114668-1-anshulusr@gmail.com/

Changes for v7:
- no updates

v6: https://lore.kernel.org/lkml/20231027051819.81333-1-anshulusr@gmail.com/

Changes for v6:
- no updates

v5: https://lore.kernel.org/lkml/20231017034356.1436677-1-anshulusr@gmail.com/

Changes for v5:
- Added link to the datasheet

v4: https://lore.kernel.org/lkml/20231010184827.1213507-1-anshulusr@gmail.com/

Changes for v4:
- Fixed the URI for the id field
- Added `interrupts` property

v3: https://lore.kernel.org/linux-input/20231008185709.2448423-1-anshulusr@gmail.com/

Changes for v3:
- Updated id field to reflect updated file name from previous version
- Added `reg` property

v2: https://lore.kernel.org/linux-input/20231008172435.2391009-1-anshulusr@gmail.com/

Changes for v2:
- Renamed file to `adafruit,seesaw-gamepad.yaml`
- Removed quotes for `$id` and `$schema`
- Removed "Bindings for" from the description
- Changed node name to the generic name "joystick"
- Changed compatible to 'adafruit,seesaw-gamepad' instead of
  'adafruit,seesaw_gamepad'

v1: https://lore.kernel.org/linux-input/20231007144052.1535417-1-anshulusr@gmail.com/
---
 .../input/adafruit,seesaw-gamepad.yaml        | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml

diff --git a/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
new file mode 100644
index 000000000000..5e86f6de6978
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/adafruit,seesaw-gamepad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Adafruit Mini I2C Gamepad with seesaw
+
+maintainers:
+  - Anshul Dalal <anshulusr@gmail.com>
+
+description: |
+  Adafruit Mini I2C Gamepad
+
+    +-----------------------------+
+    |   ___                       |
+    |  /   \               (X)    |
+    | |  S  |  __   __  (Y)   (A) |
+    |  \___/  |ST| |SE|    (B)    |
+    |                             |
+    +-----------------------------+
+
+  S -> 10-bit precision bidirectional analog joystick
+  ST -> Start
+  SE -> Select
+  X, A, B, Y -> Digital action buttons
+
+  Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
+  Product page: https://www.adafruit.com/product/5743
+  Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
+
+properties:
+  compatible:
+    const: adafruit,seesaw-gamepad
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description:
+      The gamepad's IRQ pin triggers a rising edge if interrupts are enabled.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        joystick@50 {
+            compatible = "adafruit,seesaw-gamepad";
+            interrupts = <18 IRQ_TYPE_EDGE_RISING>;
+            reg = <0x50>;
+        };
+    };
-- 
2.42.1


^ permalink raw reply related

* [PATCH v10 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-11-21 12:34 UTC (permalink / raw)
  To: linux-input, devicetree
  Cc: Anshul Dalal, Conor Dooley, Dmitry Torokhov,
	Thomas Weißschuh, linux-kernel, Krzysztof Kozlowski,
	Conor Dooley, Rob Herring, Krzysztof Kozlowski, Jeff LaBundy,
	linux-kernel-mentees
In-Reply-To: <20231121123409.2231115-1-anshulusr@gmail.com>

Adds a driver for a mini gamepad that communicates over i2c, the gamepad
has bidirectional thumb stick input and six buttons.

The gamepad chip utilizes the open framework from Adafruit called 'Seesaw'
to transmit the ADC data for the joystick and digital pin state for the
buttons. I have only implemented the functionality required to receive the
thumb stick and button state.

Steps in reading the gamepad state over i2c:
  1. Reset the registers
  2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input
      `BUTTON_MASK`: A bit-map for the six digital pins internally
       connected to the joystick buttons.
  3. Enable internal pullup resistors for the `BUTTON_MASK`
  4. Bulk set the pin state HIGH for `BUTTON_MASK`
  5. Poll the device for button and joystick state done by:
      `seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)`

Product page:
  https://www.adafruit.com/product/5743
Arduino driver:
  https://github.com/adafruit/Adafruit_Seesaw

Driver tested on RPi Zero 2W

Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>

---

Changes for v10:
- no updates

v9: https://lore.kernel.org/lkml/20231121101751.2189965-2-anshulusr@gmail.com/

Changes for v9:
- Added of_device_id

v8: https://lore.kernel.org/lkml/20231108005337.45069-1-anshulusr@gmail.com/

Changes for v8:
- Updated invalid references to `adafruit_seesaw` to `adafruit-seesaw`

v7: https://lore.kernel.org/lkml/20231106164134.114668-1-anshulusr@gmail.com/

Changes for v7:
adafruit-seesaw.c
- Fixed formatting for macro definitions
- Made SEESAW_BUTTON_MASK static const
- Removed __be16 type for x and y fields of seesaw_data
- Used sparse_keymap implementation instead of custom keymap
- Used i2c_msg instead of i2c_master_send and recv in
  seesaw_register_read
- Use temporary variable `adc_data` to store data received from ADC
- Changed read_buf from u8[4] to __be32
- Use usleep_range instead of msleep
- Removed 'Reviewed-by: Thomas Weißschuh' due to large number of changes
  since last review
Kconfig:
- Added `select INPUT_SPARSEKMAP`

v6: https://lore.kernel.org/lkml/20231027051819.81333-1-anshulusr@gmail.com/

Changes for v6:
- Added TODO
- Removed `clang-format` directives
- Namespaced device buttons
- Removed `char physical_path[32]` field from `struct seesaw_gamepad`
- Added `packed` attribute to `struct seesaw_data`
- Moved from having booleans per button to single `u32 button_state`
- Added `seesaw_button_description` array to directly associate device
  buttons with respective keycodes
- Added wrapper functions `seesaw_register_` around `i2c_master_` API
- Ratelimited input error reporting with `dev_err_ratelimited`
- Removed `of_device_id`

v5: https://lore.kernel.org/lkml/20231017034356.1436677-1-anshulusr@gmail.com/

Changes for v5:
- Added link to the datasheet
- Added debug log message when `seesaw_read_data` fails

v4: https://lore.kernel.org/lkml/20231010184827.1213507-1-anshulusr@gmail.com/

Changes for v4:
- Changed `1UL << BUTTON_` to BIT(BUTTON_)
- Removed `hardware_id` field from `struct seesaw_gamepad`
- Removed redundant checks for the number of bytes written and received by
  `i2c_master_send` and `i2c_master_recv`
- Used `get_unaligned_be32` to instantiate `u32 result` from `read_buf`
- Changed  `result & (1UL << BUTTON_)` to
  `test_bit(BUTTON_, (long *)&result)`
- Changed `KBUILD_MODNAME` in id-tables to `SEESAW_DEVICE_NAME`
- Fixed formatting issues
- Changed button reporting:
    Since the gamepad had the action buttons in a non-standard layout:
         (X)
      (Y)   (A)
         (B)
    Therefore moved to using generic directional action button event codes
    instead of BTN_[ABXY].

v3: https://lore.kernel.org/linux-input/20231008185709.2448423-1-anshulusr@gmail.com/

Changes for v3:
- no updates

v2: https://lore.kernel.org/linux-input/20231008172435.2391009-1-anshulusr@gmail.com/

Changes for v2:
adafruit-seesaw.c:
- Renamed file from 'adafruit_seesaw.c'
- Changed device name from 'seesaw_gamepad' to 'seesaw-gamepad'
- Changed count parameter for receiving joystick x on line 118:
    `2` to `sizeof(write_buf)`
- Fixed invalid buffer size on line 123 and 126:
    `data->y` to `sizeof(data->y)`
- Added comment for the `mdelay(10)` on line 169
- Changed inconsistent indentation on line 271
Kconfig:
- Fixed indentation for the help text
- Updated module name
Makefile:
- Updated module object file name
MAINTAINERS:
- Updated file name for the driver and bindings

v1: https://lore.kernel.org/linux-input/20231007144052.1535417-1-anshulusr@gmail.com/
---
 MAINTAINERS                              |   7 +
 drivers/input/joystick/Kconfig           |  10 +
 drivers/input/joystick/Makefile          |   1 +
 drivers/input/joystick/adafruit-seesaw.c | 318 +++++++++++++++++++++++
 4 files changed, 336 insertions(+)
 create mode 100644 drivers/input/joystick/adafruit-seesaw.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 81d5fc0bba68..b3f101edc24b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -441,6 +441,13 @@ W:	http://wiki.analog.com/AD7879
 W:	https://ez.analog.com/linux-software-drivers
 F:	drivers/input/touchscreen/ad7879.c
 
+ADAFRUIT MINI I2C GAMEPAD
+M:	Anshul Dalal <anshulusr@gmail.com>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
+F:	drivers/input/joystick/adafruit-seesaw.c
+
 ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
 M:	Jiri Kosina <jikos@kernel.org>
 S:	Maintained
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index ac6925ce8366..7755e5b454d2 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -412,4 +412,14 @@ config JOYSTICK_SENSEHAT
 	  To compile this driver as a module, choose M here: the
 	  module will be called sensehat_joystick.
 
+config JOYSTICK_SEESAW
+	tristate "Adafruit Mini I2C Gamepad with Seesaw"
+	depends on I2C
+	select INPUT_SPARSEKMAP
+	help
+	  Say Y here if you want to use the Adafruit Mini I2C Gamepad.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called adafruit-seesaw.
+
 endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 3937535f0098..9976f596a920 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_JOYSTICK_N64)		+= n64joy.o
 obj-$(CONFIG_JOYSTICK_PSXPAD_SPI)	+= psxpad-spi.o
 obj-$(CONFIG_JOYSTICK_PXRC)		+= pxrc.o
 obj-$(CONFIG_JOYSTICK_QWIIC)		+= qwiic-joystick.o
+obj-$(CONFIG_JOYSTICK_SEESAW)		+= adafruit-seesaw.o
 obj-$(CONFIG_JOYSTICK_SENSEHAT)	+= sensehat-joystick.o
 obj-$(CONFIG_JOYSTICK_SIDEWINDER)	+= sidewinder.o
 obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
diff --git a/drivers/input/joystick/adafruit-seesaw.c b/drivers/input/joystick/adafruit-seesaw.c
new file mode 100644
index 000000000000..856ca223df58
--- /dev/null
+++ b/drivers/input/joystick/adafruit-seesaw.c
@@ -0,0 +1,318 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 Anshul Dalal <anshulusr@gmail.com>
+ *
+ * Driver for Adafruit Mini I2C Gamepad
+ *
+ * Based on the work of:
+ *	Oleh Kravchenko (Sparkfun Qwiic Joystick driver)
+ *
+ * Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
+ * Product page: https://www.adafruit.com/product/5743
+ * Firmware and hardware sources: https://github.com/adafruit/Adafruit_Seesaw
+ *
+ * TODO:
+ *	- Add interrupt support
+ */
+
+#include <asm/unaligned.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#define SEESAW_DEVICE_NAME	     "seesaw-gamepad"
+
+#define SEESAW_STATUS_BASE	     0x00
+#define SEESAW_GPIO_BASE	     0x01
+#define SEESAW_ADC_BASE		     0x09
+
+#define SEESAW_GPIO_DIRCLR_BULK	     0x03
+#define SEESAW_GPIO_BULK	     0x04
+#define SEESAW_GPIO_BULK_SET	     0x05
+#define SEESAW_GPIO_PULLENSET	     0x0b
+
+#define SEESAW_STATUS_HW_ID	     0x01
+#define SEESAW_STATUS_SWRST	     0x7f
+
+#define SEESAW_ADC_OFFSET	     0x07
+
+#define SEESAW_BUTTON_A		     0x05
+#define SEESAW_BUTTON_B		     0x01
+#define SEESAW_BUTTON_X		     0x06
+#define SEESAW_BUTTON_Y		     0x02
+#define SEESAW_BUTTON_START	     0x10
+#define SEESAW_BUTTON_SELECT	     0x00
+
+#define SEESAW_ANALOG_X		     0x0e
+#define SEESAW_ANALOG_Y		     0x0f
+
+#define SEESAW_JOYSTICK_MAX_AXIS     1023
+#define SEESAW_JOYSTICK_FUZZ	     2
+#define SEESAW_JOYSTICK_FLAT	     4
+
+#define SEESAW_GAMEPAD_POLL_INTERVAL 16
+#define SEESAW_GAMEPAD_POLL_MIN	     8
+#define SEESAW_GAMEPAD_POLL_MAX	     32
+
+#define MSEC_PER_USEC		     1000
+
+static const u32 SEESAW_BUTTON_MASK =
+	BIT(SEESAW_BUTTON_A) | BIT(SEESAW_BUTTON_B) | BIT(SEESAW_BUTTON_X) |
+	BIT(SEESAW_BUTTON_Y) | BIT(SEESAW_BUTTON_START) |
+	BIT(SEESAW_BUTTON_SELECT);
+
+struct seesaw_gamepad {
+	struct input_dev *input_dev;
+	struct i2c_client *i2c_client;
+};
+
+struct seesaw_data {
+	u16 x;
+	u16 y;
+	u32 button_state;
+};
+
+static const struct key_entry seesaw_buttons_new[] = {
+	{ KE_KEY, SEESAW_BUTTON_A, .keycode = BTN_SOUTH },
+	{ KE_KEY, SEESAW_BUTTON_B, .keycode = BTN_EAST },
+	{ KE_KEY, SEESAW_BUTTON_X, .keycode = BTN_NORTH },
+	{ KE_KEY, SEESAW_BUTTON_Y, .keycode = BTN_WEST },
+	{ KE_KEY, SEESAW_BUTTON_START, .keycode = BTN_START },
+	{ KE_KEY, SEESAW_BUTTON_SELECT, .keycode = BTN_SELECT },
+	{ KE_END, 0 }
+};
+
+static int seesaw_register_read(struct i2c_client *client, u8 register_high,
+				u8 register_low, char *buf, int count)
+{
+	int ret;
+	u8 register_buf[2] = { register_high, register_low };
+
+	struct i2c_msg message_buf[2] = {
+		{
+			.addr = client->addr,
+			.flags = client->flags,
+			.len = sizeof(register_buf),
+			.buf = register_buf,
+		},
+		{
+			.addr = client->addr,
+			.flags = client->flags | I2C_M_RD,
+			.len = count,
+			.buf = buf,
+		},
+	};
+	ret = i2c_transfer(client->adapter, message_buf,
+			   ARRAY_SIZE(message_buf));
+
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int seesaw_register_write_u8(struct i2c_client *client, u8 register_high,
+				    u8 register_low, u8 value)
+{
+	int ret;
+	u8 write_buf[3] = { register_high, register_low, value };
+
+	ret = i2c_master_send(client, write_buf, sizeof(write_buf));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int seesaw_register_write_u32(struct i2c_client *client,
+				     u8 register_high, u8 register_low,
+				     u32 value)
+{
+	int ret;
+	u8 write_buf[6] = { register_high, register_low };
+
+	put_unaligned_be32(value, write_buf + 2);
+	ret = i2c_master_send(client, write_buf, sizeof(write_buf));
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)
+{
+	int ret;
+	__be16 adc_data;
+	__be32 read_buf;
+
+	ret = seesaw_register_read(client, SEESAW_GPIO_BASE, SEESAW_GPIO_BULK,
+				   (char *)&read_buf, sizeof(read_buf));
+	if (ret)
+		return ret;
+
+	data->button_state = ~be32_to_cpu(read_buf);
+
+	ret = seesaw_register_read(client, SEESAW_ADC_BASE,
+				   SEESAW_ADC_OFFSET + SEESAW_ANALOG_X,
+				   (char *)&adc_data, sizeof(adc_data));
+	if (ret)
+		return ret;
+	/*
+	 * ADC reads left as max and right as 0, must be reversed since kernel
+	 * expects reports in opposite order.
+	 */
+	data->x = SEESAW_JOYSTICK_MAX_AXIS - be16_to_cpu(adc_data);
+
+	ret = seesaw_register_read(client, SEESAW_ADC_BASE,
+				   SEESAW_ADC_OFFSET + SEESAW_ANALOG_Y,
+				   (char *)&adc_data, sizeof(adc_data));
+	if (ret)
+		return ret;
+	data->y = be16_to_cpu(adc_data);
+
+	return 0;
+}
+
+static void seesaw_poll(struct input_dev *input)
+{
+	int err, i;
+	struct seesaw_gamepad *private = input_get_drvdata(input);
+	struct seesaw_data data;
+
+	err = seesaw_read_data(private->i2c_client, &data);
+	if (err) {
+		dev_err_ratelimited(&input->dev,
+				    "failed to read joystick state: %d\n", err);
+		return;
+	}
+
+	input_report_abs(input, ABS_X, data.x);
+	input_report_abs(input, ABS_Y, data.y);
+
+	for_each_set_bit(i, (long *)&SEESAW_BUTTON_MASK,
+			 BITS_PER_TYPE(SEESAW_BUTTON_MASK)) {
+		if (!sparse_keymap_report_event(
+			    input, i, data.button_state & BIT(i), false)) {
+			dev_err_ratelimited(&input->dev,
+					    "failed to report keymap event");
+		};
+	}
+
+	input_sync(input);
+}
+
+static int seesaw_probe(struct i2c_client *client)
+{
+	int ret;
+	u8 hardware_id;
+	struct seesaw_gamepad *seesaw;
+
+	ret = seesaw_register_write_u8(client, SEESAW_STATUS_BASE,
+				       SEESAW_STATUS_SWRST, 0xFF);
+	if (ret)
+		return ret;
+
+	/* Wait for the registers to reset before proceeding */
+	usleep_range(10 * MSEC_PER_USEC, 15 * MSEC_PER_USEC);
+
+	seesaw = devm_kzalloc(&client->dev, sizeof(*seesaw), GFP_KERNEL);
+	if (!seesaw)
+		return -ENOMEM;
+
+	ret = seesaw_register_read(client, SEESAW_STATUS_BASE,
+				   SEESAW_STATUS_HW_ID, &hardware_id,
+				   sizeof(hardware_id));
+	if (ret)
+		return ret;
+
+	dev_dbg(&client->dev, "Adafruit Seesaw Gamepad, Hardware ID: %02x\n",
+		hardware_id);
+
+	/* Set Pin Mode to input and enable pull-up resistors */
+	ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
+					SEESAW_GPIO_DIRCLR_BULK,
+					SEESAW_BUTTON_MASK);
+	if (ret)
+		return ret;
+	ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
+					SEESAW_GPIO_PULLENSET,
+					SEESAW_BUTTON_MASK);
+	if (ret)
+		return ret;
+	ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
+					SEESAW_GPIO_BULK_SET,
+					SEESAW_BUTTON_MASK);
+	if (ret)
+		return ret;
+
+	seesaw->i2c_client = client;
+	seesaw->input_dev = devm_input_allocate_device(&client->dev);
+	if (!seesaw->input_dev)
+		return -ENOMEM;
+
+	seesaw->input_dev->id.bustype = BUS_I2C;
+	seesaw->input_dev->name = "Adafruit Seesaw Gamepad";
+	seesaw->input_dev->phys = "i2c/" SEESAW_DEVICE_NAME;
+	input_set_drvdata(seesaw->input_dev, seesaw);
+	input_set_abs_params(seesaw->input_dev, ABS_X, 0,
+			     SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+			     SEESAW_JOYSTICK_FLAT);
+	input_set_abs_params(seesaw->input_dev, ABS_Y, 0,
+			     SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+			     SEESAW_JOYSTICK_FLAT);
+
+	ret = sparse_keymap_setup(seesaw->input_dev, seesaw_buttons_new, NULL);
+	if (ret) {
+		dev_err(&client->dev,
+			"failed to set up input device keymap: %d\n", ret);
+		return ret;
+	}
+
+	ret = input_setup_polling(seesaw->input_dev, seesaw_poll);
+	if (ret) {
+		dev_err(&client->dev, "failed to set up polling: %d\n", ret);
+		return ret;
+	}
+
+	input_set_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_INTERVAL);
+	input_set_max_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MAX);
+	input_set_min_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MIN);
+
+	ret = input_register_device(seesaw->input_dev);
+	if (ret) {
+		dev_err(&client->dev, "failed to register joystick: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct i2c_device_id seesaw_id_table[] = {
+	{ SEESAW_DEVICE_NAME, 0 },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, seesaw_id_table);
+
+static const struct of_device_id seesaw_of_table[] = {
+	{ .compatible = "adafruit,seesaw-gamepad"},
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, seesaw_of_table);
+
+static struct i2c_driver seesaw_driver = {
+	.driver = {
+		.name = SEESAW_DEVICE_NAME,
+		.of_match_table = of_match_ptr(seesaw_of_table),
+	},
+	.id_table = seesaw_id_table,
+	.probe = seesaw_probe,
+};
+module_i2c_driver(seesaw_driver);
+
+MODULE_AUTHOR("Anshul Dalal <anshulusr@gmail.com>");
+MODULE_DESCRIPTION("Adafruit Mini I2C Gamepad driver");
+MODULE_LICENSE("GPL");
-- 
2.42.1


^ permalink raw reply related

* Re: [PATCH v10 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Krzysztof Kozlowski @ 2023-11-21 12:52 UTC (permalink / raw)
  To: Anshul Dalal, linux-input, devicetree
  Cc: Conor Dooley, Dmitry Torokhov, Thomas Weißschuh,
	linux-kernel, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
	Jeff LaBundy, linux-kernel-mentees
In-Reply-To: <20231121123409.2231115-1-anshulusr@gmail.com>

On 21/11/2023 13:34, Anshul Dalal wrote:
> Adds bindings for the Adafruit Seesaw Gamepad.
> 
> The gamepad functions as an i2c device with the default address of 0x50
> and has an IRQ pin that can be enabled in the driver to allow for a rising
> edge trigger on each button press or joystick movement.
> 
> Product page:
>   https://www.adafruit.com/product/5743
> Arduino driver:
>   https://github.com/adafruit/Adafruit_Seesaw
> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>

Previously you sent untested patch. Did you test this one?

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v10 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-11-21 12:53 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-input, devicetree
  Cc: Conor Dooley, Dmitry Torokhov, Thomas Weißschuh,
	linux-kernel, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
	Jeff LaBundy, linux-kernel-mentees
In-Reply-To: <1bd2a6cc-aa01-4370-be19-88ffa524086e@linaro.org>



On 11/21/23 18:22, Krzysztof Kozlowski wrote:
> On 21/11/2023 13:34, Anshul Dalal wrote:
>> Adds bindings for the Adafruit Seesaw Gamepad.
>>
>> The gamepad functions as an i2c device with the default address of 0x50
>> and has an IRQ pin that can be enabled in the driver to allow for a rising
>> edge trigger on each button press or joystick movement.
>>
>> Product page:
>>   https://www.adafruit.com/product/5743
>> Arduino driver:
>>   https://github.com/adafruit/Adafruit_Seesaw
>>
>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
> 
> Previously you sent untested patch. Did you test this one?

Yes, I have tested this one.

Regards,
Anshul

^ permalink raw reply

* Re: Fwd: Logitech G915 Wireless Keyboard acts weird on 6.6.0
From: Bagas Sanjaya @ 2023-11-21 13:09 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Linux Regressions, Linux Input Devices,
	Thorsten Leemhuis
  Cc: Mavroudis Chatzilazaridis, Filipe Laíns, Bastien Nocera,
	Jiri Kosina, LinuxCat, Marcelo, Takashi Iwai, Hans de Goede,
	Linus Torvalds
In-Reply-To: <6929ebbf-f2e0-4cd4-addc-1e33ecf3277f@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2610 bytes --]

Hi Thorsten and all,

On Thu, Nov 02, 2023 at 09:11:42PM +0700, Bagas Sanjaya wrote:
> Hi,
> 
> I notice a regression report on Bugzilla [1]. Quoting from it:
> 
> > Hello,
> > After upgrading from 6.5.9 to 6.6.0, my keyboard started acting really weird in its wireless mode, key actions sent are completely wrong, see video attached. 
> > 
> > Most keys are perceived as either E, 3 or F7, with F8 and <, as well. 
> > 
> > Modifier keys (CTRL, ALT, ALT GR, Shift and Super) are working normally, as well as media control keys (pause/play, previous, next, mute and sound up/down).
> > 
> > The keyboard works as expected if it's wired.
> 
> Another reporter bisected the regression:
> 
> > Bisected to 
> > 
> > 9d1bd9346241cd6963b58da7ffb7ed303285f684 is the first bad commit
> > commit 9d1bd9346241cd6963b58da7ffb7ed303285f684
> > Author: Mavroudis Chatzilazaridis <mavchatz@protonmail.com>
> > Date: Sun Jul 16 18:23:44 2023 +0000
> > 
> > HID: logitech-dj: Add support for a new lightspeed receiver iteration
> > 
> > The lightspeed receiver for the Pro X Superlight uses 13 byte mouse reports
> > without a report id. The workaround for such cases has been adjusted to
> > handle these larger packets.
> > 
> > The device now reports the status of its battery in wireless mode and
> > libratbag now recognizes the device and it can be configured with Piper.
> > 
> > https://github.com/libratbag/libratbag/pull/1122
> > 
> > Co-developed-by: Filipe Laíns <lains@riseup.net>
> > Signed-off-by: Filipe Laíns <lains@riseup.net>
> > Signed-off-by: Mavroudis Chatzilazaridis <mavchatz@protonmail.com>
> > Reviewed-by: Bastien Nocera <hadess@hadess.net>
> > Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> > 
> > drivers/hid/hid-ids.h | 1 +
> > drivers/hid/hid-logitech-dj.c | 11 ++++++++---
> > 2 files changed, 9 insertions(+), 3 deletions(-)
> 
> See Bugzilla for the full thread.
> 
> Anyway, I'm adding this regression to regzbot:
> 
> #regzbot introduced: 9d1bd9346241cd https://bugzilla.kernel.org/show_bug.cgi?id=218094
> #regzbot title: Logitech G915 Wireless Keyboard key event only detects few key codes
> #regzbot link: https://streamable.com/ac6l8u
> 

There's no reply from culprit author nor linux-input people (did they miss
this regression?). And on Bugzilla, other reporters replied that reverting
the culprit fixed the regression.

FYI, there's similar Bugzilla report on [1].

Also Cc'ed Linus.

Thanks.

[1]: https://bugzilla.kernel.org/show_bug.cgi?id=218172

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2023-11-21 13:29 UTC (permalink / raw)
  To: Hans de Goede, Pavel Machek, Jani Nikula, jikos
  Cc: Miguel Ojeda, Lee Jones, linux-kernel,
	dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds
In-Reply-To: <8096a042-83bd-4b9f-b633-79e86995c9b8@redhat.com>


Am 21.11.23 um 13:20 schrieb Hans de Goede:
> Hi Werner,
>
> On 11/21/23 12:33, Werner Sembach wrote:
>> Hi,
>>
>> Am 20.11.23 um 21:52 schrieb Pavel Machek:
>>> Hi!
>>>
>>>>>> So... a bit of rationale. The keyboard does not really fit into the
>>>>>> LED subsystem; LEDs are expected to be independent ("hdd led") and not
>>>>>> a matrix of them.
>>>>> Makes sense.
>>>>>
>>>>>> We do see various strange displays these days -- they commonly have
>>>>>> rounded corners and holes in them. I'm not sure how that's currently
>>>>>> supported, but I believe it is reasonable to view keyboard as a
>>>>>> display with slightly weird placing of pixels.
>>>>>>
>>>>>> Plus, I'd really like to play tetris on one of those :-).
>>>>>>
>>>>>> So, would presenting them as auxdisplay be acceptable? Or are there
>>>>>> better options?
>>>>> It sounds like a fair use case -- auxdisplay are typically simple
>>>>> character-based or small graphical displays, e.g. 128x64, that may not
>>>>> be a "main" / usual screen as typically understood, but the concept is
>>>>> a bit fuzzy and we are a bit of a catch-all.
>>>>>
>>>>> And "keyboard backlight display with a pixel/color per-key" does not
>>>>> sound like a "main" screen, and having some cute effects displayed
>>>>> there are the kind of thing that one could do in the usual small
>>>>> graphical ones too. :)
>>>>>
>>>>> But if somebody prefers to create new categories (or subcategories
>>>>> within auxdisplay) to hold these, that could be nice too (in the
>>>>> latter case, I would perhaps suggest reorganizing all of the existing
>>>>> ones while at it).
>>>> One could also reasonably make the argument that controlling the
>>>> individual keyboard key backlights should be part of the input
>>>> subsystem. It's not a display per se. (Unless you actually have small
>>>> displays on the keycaps, and I think that's a thing too.)
>>> While it would not be completely crazy to do that... I believe the
>>> backlight is more of a display and less of a keyboard. Plus input
>>> subystem is very far away from supporting this, and we had no input
>>> from input people here.
>>>
>>> I don't think LED subsystem is right place for this, and I believe
>>> auxdisplay makes slightly more sense than input.
>>>
>>> Unless someone steps up, I'd suggest Werner tries to implement this as
>>> an auxdisplay. [And yes, this will not be simple task. RGB on LED is
>>> different from RGB on display. But there are other LED displays, so
>>> auxdisplay should handle this. Plus pixels are really funnily
>>> shaped. But displays with missing pixels -- aka holes for camera --
>>> are common in phones, and I believe we'll get variable pixel densities
>>> -- less dense over camera -- too. So displays will have to deal with
>>> these in the end.]
>> Another idea I want to throw in the mix:
>>
>> Maybe the kernel is not the right place to implement this at all. RGB stuff is not at all standardized and every vendor is doing completely different interfaces, which does not fit the kernel userpsace apis desire to be uniformal and fixed. e.g. Auxdisplay might fit static setting of RGB values, but it does not fit the snake-effect mode, or the raindrops mode, or the 4-different-colors-in-the-edges-breathing-and-color-cycling mode.
>>
>> So my current idea: Implement these keyboards as a single zone RGB kbd_backlight in the leds interface to have something functional out of the box, but make it runtime disable-able if something like https://gitlab.com/CalcProgrammer1/OpenRGB wants to take over more fine granular control from userspace via hidraw.
> That sounds like a good approach to me. We are seeing the same with game controllers where steam and wine/proton also sometimes use hidraw mode to get access to all the crazy^W interesting features.
>
> That would mean that all we need to standardize and the kernel <-> userspace API level is adding a standard way to disable the single zone RGB kbd_backlight support in the kernel.

I would suggest a simple "enable" entry. Default is 1. When set to 0 the kernel 
driver no longer does anything.

Questions:

- Should the driver try to reset the settings to boot default? Or just leave the 
device in the current state? With the former I could see issues that they 
keyboard is flashing when changing from kernelspace control to userspace 
control. With the later the burden on bringing the device to a know state lies 
with the userspace driver.

- Should this be a optional entry that only shows up on drivers supporting it, 
or could this implemented in a generic way affecting all current led entries?

- I guess UPower integration for the userspace driver could be archived with 
https://www.kernel.org/doc/html/latest/leds/uleds.html however this limited to 
brightness atm, so when accent colors actually come to UPower this would also 
need some expansion to be able to pass a preferred color to the userspace driver 
(regardless of what that driver is then doing with that information).

On a different note: This approach does currently not cover the older EC 
controlled 3 zone keyboards from clevo. Here only the kernel has access access 
to the device so the kernel driver has to expose all functionality somehow. 
Should this be done by an arbitrarily designed platform device?

Kind regards,

Werner

>
> Regards,
>
> Hans
>
>

^ permalink raw reply

* Re: Fwd: Logitech G915 Wireless Keyboard acts weird on 6.6.0
From: Jiri Kosina @ 2023-11-21 13:37 UTC (permalink / raw)
  To: Bagas Sanjaya
  Cc: Linux Kernel Mailing List, Linux Regressions, Linux Input Devices,
	Thorsten Leemhuis, Mavroudis Chatzilazaridis, Filipe Laíns,
	Bastien Nocera, LinuxCat, Marcelo, Takashi Iwai, Hans de Goede,
	Linus Torvalds
In-Reply-To: <ZVyr-of1X4RudpWG@archie.me>

On Tue, 21 Nov 2023, Bagas Sanjaya wrote:

> Hi Thorsten and all,
> 
> On Thu, Nov 02, 2023 at 09:11:42PM +0700, Bagas Sanjaya wrote:
> > Hi,
> > 
> > I notice a regression report on Bugzilla [1]. Quoting from it:
> > 
> > > Hello,
> > > After upgrading from 6.5.9 to 6.6.0, my keyboard started acting really weird in its wireless mode, key actions sent are completely wrong, see video attached. 
> > > 
> > > Most keys are perceived as either E, 3 or F7, with F8 and <, as well. 
> > > 
> > > Modifier keys (CTRL, ALT, ALT GR, Shift and Super) are working normally, as well as media control keys (pause/play, previous, next, mute and sound up/down).
> > > 
> > > The keyboard works as expected if it's wired.
> > 
> > Another reporter bisected the regression:
> > 
> > > Bisected to 
> > > 
> > > 9d1bd9346241cd6963b58da7ffb7ed303285f684 is the first bad commit
> > > commit 9d1bd9346241cd6963b58da7ffb7ed303285f684
> > > Author: Mavroudis Chatzilazaridis <mavchatz@protonmail.com>
> > > Date: Sun Jul 16 18:23:44 2023 +0000
> > > 
> > > HID: logitech-dj: Add support for a new lightspeed receiver iteration
> > > 
> > > The lightspeed receiver for the Pro X Superlight uses 13 byte mouse reports
> > > without a report id. The workaround for such cases has been adjusted to
> > > handle these larger packets.
> > > 
> > > The device now reports the status of its battery in wireless mode and
> > > libratbag now recognizes the device and it can be configured with Piper.
> > > 
> > > https://github.com/libratbag/libratbag/pull/1122
> > > 
> > > Co-developed-by: Filipe Laíns <lains@riseup.net>
> > > Signed-off-by: Filipe Laíns <lains@riseup.net>
> > > Signed-off-by: Mavroudis Chatzilazaridis <mavchatz@protonmail.com>
> > > Reviewed-by: Bastien Nocera <hadess@hadess.net>
> > > Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> > > 
> > > drivers/hid/hid-ids.h | 1 +
> > > drivers/hid/hid-logitech-dj.c | 11 ++++++++---
> > > 2 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > See Bugzilla for the full thread.
> > 
> > Anyway, I'm adding this regression to regzbot:
> > 
> > #regzbot introduced: 9d1bd9346241cd https://bugzilla.kernel.org/show_bug.cgi?id=218094
> > #regzbot title: Logitech G915 Wireless Keyboard key event only detects few key codes
> > #regzbot link: https://streamable.com/ac6l8u
> > 
> 
> There's no reply from culprit author nor linux-input people (did they miss
> this regression?). And on Bugzilla, other reporters replied that reverting
> the culprit fixed the regression.
> 
> FYI, there's similar Bugzilla report on [1].

As there was no reaction from Mavroudis in order to figure out why he is 
not observing the issues the other reporters do and what to do to fix 
those, I already do have revert in my queue for -rc3.

My first guess would be that the extra buttons in the extended report are 
not properly reflected in the emulated report descriptor, but that 
wouldn't explain why it worked for the author of the commit.

So revert it is, and once Marvoudis resurfaces, we can try again for some 
of later releases.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: Fwd: Logitech G915 Wireless Keyboard acts weird on 6.6.0
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-11-21 13:53 UTC (permalink / raw)
  To: Bagas Sanjaya, Linux Kernel Mailing List, Linux Regressions,
	Linux Input Devices
  Cc: Mavroudis Chatzilazaridis, Filipe Laíns, Bastien Nocera,
	Jiri Kosina, LinuxCat, Marcelo, Takashi Iwai, Hans de Goede,
	Linus Torvalds, Benjamin Tissoires
In-Reply-To: <ZVyr-of1X4RudpWG@archie.me>

On 21.11.23 14:09, Bagas Sanjaya wrote:
> On Thu, Nov 02, 2023 at 09:11:42PM +0700, Bagas Sanjaya wrote:
>> I notice a regression report on Bugzilla [1]. Quoting from it:
>> 
>>> Hello, After upgrading from 6.5.9 to 6.6.0, my keyboard started
>>> acting really weird in its wireless mode, key actions sent are
>>> completely wrong, see video attached.
>>> 
>>> Most keys are perceived as either E, 3 or F7, with F8 and <, as
>>> well.
> [...]
>>> 9d1bd9346241cd6963b58da7ffb7ed303285f684 is the first bad commit 
>>> commit 9d1bd9346241cd6963b58da7ffb7ed303285f684 Author: Mavroudis
>>> Chatzilazaridis <mavchatz@protonmail.com> Date: Sun Jul 16
>>> 18:23:44 2023 +0000
>>> 
>>> HID: logitech-dj: Add support for a new lightspeed receiver
>>> iteration
> [...]
>> 
>> https://bugzilla.kernel.org/show_bug.cgi?id=218094 
> 
> There's no reply from culprit author nor linux-input people (did they
> miss this regression?).

I guess part of the problem is that Bastien got reassigned and might not
care about the kernel anymore.

Another part of it is that Jiri was CCed, but Benjamin was not.

Ideally of course Mavroudis Chatzilazaridis, the culprit's author would
look into this, but from a quick search on lore it looks like Mavroudis
is not a regular kernel contributor and thus might not even know how we
expect situations like this to be handled.

> And on Bugzilla, other reporters replied that
> reverting the culprit fixed the regression.

From Takashi's comments like
https://bugzilla.kernel.org/show_bug.cgi?id=218094#c33 it sounds like
this can be fixed by resolving another regression as discussed earlier
today here:
https://lore.kernel.org/all/87edgjo2kr.wl-tiwai@suse.de/

I think that might be the better solution, but Takashi, Hans, or the
input people will know best.

> FYI, there's similar Bugzilla report on [1].
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=218172

Not sure, that might be a different problem, guess Hans is the best to
judge.

> Also Cc'ed Linus.

Linus can speak for himself, but I guess he gets enough mail already.
I'd say in a situation like this it thus might best to not CC him;
instead poke me when things apparently are not handled well, then we
together can decide if it's worth bringing Linus in.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

^ permalink raw reply

* Re: Fwd: Logitech G915 Wireless Keyboard acts weird on 6.6.0
From: Bagas Sanjaya @ 2023-11-21 13:58 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Linux Kernel Mailing List, Linux Regressions, Linux Input Devices,
	Thorsten Leemhuis, Mavroudis Chatzilazaridis, Filipe Laíns,
	Bastien Nocera, LinuxCat, Marcelo, Takashi Iwai, Hans de Goede,
	Linus Torvalds
In-Reply-To: <nycvar.YFH.7.76.2311211435050.29220@cbobk.fhfr.pm>

[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]

On Tue, Nov 21, 2023 at 02:37:52PM +0100, Jiri Kosina wrote:
> On Tue, 21 Nov 2023, Bagas Sanjaya wrote:
> 
> > Hi Thorsten and all,
> > 
> > <snip>...
> > There's no reply from culprit author nor linux-input people (did they miss
> > this regression?). And on Bugzilla, other reporters replied that reverting
> > the culprit fixed the regression.
> > 
> > FYI, there's similar Bugzilla report on [1].
> 
> As there was no reaction from Mavroudis in order to figure out why he is 
> not observing the issues the other reporters do and what to do to fix 
> those, I already do have revert in my queue for -rc3.
> 
> My first guess would be that the extra buttons in the extended report are 
> not properly reflected in the emulated report descriptor, but that 
> wouldn't explain why it worked for the author of the commit.
> 
> So revert it is, and once Marvoudis resurfaces, we can try again for some 
> of later releases.
> 

Thanks for letting us know!

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox