Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] dt-bindings: Input: exc3000 - Support power supply regulators
From: Krzysztof Kozlowski @ 2023-07-03 16:25 UTC (permalink / raw)
  To: Mike Looijmans, devicetree, linux-input
  Cc: Conor Dooley, Dmitry Torokhov, Krzysztof Kozlowski, Rob Herring,
	linux-kernel
In-Reply-To: <20230703084536.8429-1-mike.looijmans@topic.nl>

On 03/07/2023 10:45, Mike Looijmans wrote:
> Add power supply regulator support to the exc3000 devices.
> This provides the devicetree bindings.

If there is going to be next version/resend, you could skip the last
sentence. It is a bit redundant.

In any case:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v5] HID: steelseries: Add support for Arctis 1 XBox
From: Benjamin Tissoires @ 2023-07-03 15:33 UTC (permalink / raw)
  To: kernel test robot
  Cc: Bastien Nocera, linux-input, oe-kbuild-all, linux-kernel,
	Jiri Kosina, Benjamin Tissoires
In-Reply-To: <202307032208.b5brKeCt-lkp@intel.com>


On Jul 03 2023, kernel test robot wrote:
> 
> Hi Bastien,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on hid/for-next]
> [also build test ERROR on linus/master v6.4 next-20230703]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Bastien-Nocera/HID-steelseries-Add-support-for-Arctis-1-XBox/20230703-183124
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
> patch link:    https://lore.kernel.org/r/20230703102918.9941-1-hadess%40hadess.net
> patch subject: [PATCH v5] HID: steelseries: Add support for Arctis 1 XBox
> config: parisc-randconfig-r032-20230703 (https://download.01.org/0day-ci/archive/20230703/202307032208.b5brKeCt-lkp@intel.com/config)
> compiler: hppa-linux-gcc (GCC) 12.3.0
> reproduce: (https://download.01.org/0day-ci/archive/20230703/202307032208.b5brKeCt-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202307032208.b5brKeCt-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    hppa-linux-ld: drivers/hid/hid-steelseries.o: in function `.LC10':
> >> hid-steelseries.c:(.rodata.cst4+0x8): undefined reference to `hid_is_usb'

Bastien, you need a depends on USB_HID IIRC in your Kconfig (at
least CONFIG_USB_HID is not set in that kernel test bot run).

Cheers,
Benjamin

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

^ permalink raw reply

* [PATCH v6 1/1] Input: gameport: add ISA and HAS_IOPORT dependencies
From: Niklas Schnelle @ 2023-07-03 15:23 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Arnd Bergmann, linux-input, linux-kernel
In-Reply-To: <20230703152355.3897456-1-schnelle@linux.ibm.com>

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. As ISA already implies HAS_IOPORT we can simply add
this dependency and guard sections of code using inb()/outb() as
alternative access methods.

Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/input/gameport/Kconfig    |  4 +++-
 drivers/input/gameport/gameport.c | 28 +++++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/input/gameport/Kconfig b/drivers/input/gameport/Kconfig
index 5a2c2fb3217d..fe73b26e647a 100644
--- a/drivers/input/gameport/Kconfig
+++ b/drivers/input/gameport/Kconfig
@@ -25,6 +25,7 @@ if GAMEPORT
 
 config GAMEPORT_NS558
 	tristate "Classic ISA and PnP gameport support"
+	depends on ISA
 	help
 	  Say Y here if you have an ISA or PnP gameport.
 
@@ -35,6 +36,7 @@ config GAMEPORT_NS558
 
 config GAMEPORT_L4
 	tristate "PDPI Lightning 4 gamecard support"
+	depends on ISA
 	help
 	  Say Y here if you have a PDPI Lightning 4 gamecard.
 
@@ -53,7 +55,7 @@ config GAMEPORT_EMU10K1
 
 config GAMEPORT_FM801
 	tristate "ForteMedia FM801 gameport support"
-	depends on PCI
+	depends on PCI && HAS_IOPORT
 	help
 	  Say Y here if you have ForteMedia FM801 PCI audio controller
 	  (Abit AU10, Genius Sound Maker, HP Workstation zx2000,
diff --git a/drivers/input/gameport/gameport.c b/drivers/input/gameport/gameport.c
index a1443320b419..bee935211554 100644
--- a/drivers/input/gameport/gameport.c
+++ b/drivers/input/gameport/gameport.c
@@ -519,12 +519,34 @@ EXPORT_SYMBOL(gameport_set_phys);
 
 static void gameport_default_trigger(struct gameport *gameport)
 {
+#ifdef CONFIG_HAS_IOPORT
 	outb(0xff, gameport->io);
+#endif
 }
 
 static unsigned char gameport_default_read(struct gameport *gameport)
 {
+#ifdef CONFIG_HAS_IOPORT
 	return inb(gameport->io);
+#else
+	return 0xff;
+#endif
+}
+
+static void gameport_setup_default_handlers(struct gameport *gameport)
+{
+	if (gameport->trigger && gameport->read)
+		return;
+
+	if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+		dev_err(&gameport->dev,
+			"I/O port access is required for %s (%s) but is not available\n",
+			gameport->phys, gameport->name);
+
+	if (!gameport->trigger)
+		gameport->trigger = gameport_default_trigger;
+	if (!gameport->read)
+		gameport->read = gameport_default_read;
 }
 
 /*
@@ -545,11 +567,7 @@ static void gameport_init_port(struct gameport *gameport)
 	if (gameport->parent)
 		gameport->dev.parent = &gameport->parent->dev;
 
-	if (!gameport->trigger)
-		gameport->trigger = gameport_default_trigger;
-	if (!gameport->read)
-		gameport->read = gameport_default_read;
-
+	gameport_setup_default_handlers(gameport);
 	INIT_LIST_HEAD(&gameport->node);
 	spin_lock_init(&gameport->timer_lock);
 	timer_setup(&gameport->poll_timer, gameport_run_poll_handler, 0);
-- 
2.39.2


^ permalink raw reply related

* [PATCH v6 0/1] Input: gameport: Handle HAS_IOPORT dependencies
From: Niklas Schnelle @ 2023-07-03 15:23 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Arnd Bergmann, linux-input, linux-kernel

This is a follow up to my ongoing effort of making the inb()/outb() and
similar I/O port accessors compile-time optional. Previously I sent this as
a complete treewide series titled "treewide: Remove I/O port accessors for
HAS_IOPORT=n" with the latest being its 5th version[0]. Now about half of
the per-subsystem patches have been merged so I'm changing over to stand
alone subsystem patches. These series are stand alone and should be merged
via the relevant tree such that with all subsystems complete we can follow
this up with the last patch[1] that will make the I/O port accessors
compile-time optional.

Thanks,
Niklas

Changes since v5 for Gameport:
- Added a dev_err() print when registering a port without .trigger() || .read()
  and disabled CONFIG_HAS_IOPORT (Dmitry Torokhov).
  I added this where the default functions are set instead of in the handlers
  themselves so there is naturally one error print per port.
- Rebased to deal with default handler functions.

[0] https://lore.kernel.org/all/20230516110038.2413224-1-schnelle@linux.ibm.com/
[1] https://lore.kernel.org/all/20230516110038.2413224-42-schnelle@linux.ibm.com/
[2] https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/

Niklas Schnelle (1):
  Input: gameport: add ISA and HAS_IOPORT dependencies

 drivers/input/gameport/Kconfig    |  4 +++-
 drivers/input/gameport/gameport.c | 28 +++++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 6 deletions(-)


base-commit: a901a3568fd26ca9c4a82d8bc5ed5b3ed844d451
-- 
2.39.2


^ permalink raw reply

* Re: [PATCH v5] HID: steelseries: Add support for Arctis 1 XBox
From: kernel test robot @ 2023-07-03 15:09 UTC (permalink / raw)
  To: Bastien Nocera, linux-input
  Cc: oe-kbuild-all, linux-kernel, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20230703102918.9941-1-hadess@hadess.net>

Hi Bastien,

kernel test robot noticed the following build errors:

[auto build test ERROR on hid/for-next]
[also build test ERROR on linus/master v6.4 next-20230703]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bastien-Nocera/HID-steelseries-Add-support-for-Arctis-1-XBox/20230703-183124
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link:    https://lore.kernel.org/r/20230703102918.9941-1-hadess%40hadess.net
patch subject: [PATCH v5] HID: steelseries: Add support for Arctis 1 XBox
config: parisc-randconfig-r032-20230703 (https://download.01.org/0day-ci/archive/20230703/202307032208.b5brKeCt-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230703/202307032208.b5brKeCt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307032208.b5brKeCt-lkp@intel.com/

All errors (new ones prefixed by >>):

   hppa-linux-ld: drivers/hid/hid-steelseries.o: in function `.LC10':
>> hid-steelseries.c:(.rodata.cst4+0x8): undefined reference to `hid_is_usb'

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

^ permalink raw reply

* Re: [PATCH v9 3/3] HID: cp2112: Fwnode Support
From: Andy Shevchenko @ 2023-07-03 10:57 UTC (permalink / raw)
  To: Daniel Kaehn, Niyas Sait
  Cc: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires,
	bartosz.golaszewski, dmitry.torokhov, devicetree, linux-input,
	ethan.twardy
In-Reply-To: <CAP+ZCCe3G_b3o=zWcqJNSFbs26yH6wzBbfHjkg2JuPeWgujfiQ@mail.gmail.com>

+Cc: Niyas, who is working a lot on filling the gaps in ACPI in comparison
     to DT in the Linux kernel. Perhaps he has some ideas or even better
     solutions.

On Mon, May 01, 2023 at 06:35:44PM -0500, Daniel Kaehn wrote:
> On Mon, Mar 20, 2023 at 9:10 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Mar 20, 2023 at 08:40:07AM -0500, Daniel Kaehn wrote:
> > > On Mon, Mar 20, 2023 at 8:00 AM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Mon, Mar 20, 2023 at 02:58:07PM +0200, Andy Shevchenko wrote:
> > > > > On Sun, Mar 19, 2023 at 03:48:02PM -0500, Danny Kaehn wrote:

...

> > > > > > +   device_for_each_child_node(&hdev->dev, child) {
> > > > > > +           name = fwnode_get_name(child);
> > > > > > +           ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> > > > > > +
> > > > > > +           if ((name && strcmp("i2c", name) == 0) || (!ret && addr == 0))
> > > > > > +                   device_set_node(&dev->adap.dev, child);
> > > > > > +           else if ((name && strcmp("gpio", name)) == 0 ||
> > > > > > +                                   (!ret && addr == 1))
> > > > > > +                   dev->gc.fwnode = child;
> > > > > > +   }
> > > > >
> > > > > Please, make addresses defined explicitly. You may also do it with node naming
> > > > > schema:
> > > > >
> > > > > #define CP2112_I2C_ADR                0
> > > > > #define CP2112_GPIO_ADR               1
> > > > >
> > > > > static const char * const cp2112_cell_names[] = {
> > > > >       [CP2112_I2C_ADR]        = "i2c",
> > > > >       [CP2112_GPIO_ADR]       = "gpio",
> > > > > };
> > > > >
> > > > >       device_for_each_child_node(&hdev->dev, child) {
> > > > >               name = fwnode_get_name(child);
> > > > >               if (name) {
> > > > >                       ret = match_string(cp2112_cell_names, ARRAY_SIZE(cp2112_cell_names), name);
> > > > >                       if (ret >= 0)
> > > > >                               addr = ret;
> > > > >               } else
> > > > >                       ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> > > > >               if (ret < 0)
> > > > >                       ...error handling if needed...
> > > > >
> > > > >               switch (addr) {
> > > > >               case CP2112_I2C_ADR:
> > > > >                       device_set_node(&dev->adap.dev, child);
> > > > >                       break;
> > > > >               case CP2112_GPIO_ADR:
> > > > >                       dev->gc.fwnode = child;
> > > > >                       break;
> > > > >               default:
> > > > >                       ...error handling...
> > > > >               }
> > > > >       }
> > > >
> > > > Btw, don't you use "reg" property for the child nodes? It would be better from
> > > > de facto used patterns (we have a couple of mode drivers that have a common
> > > > code to read "reg" or _ADR() and that code can be split into a helper and used
> > > > here).
> > >
> > > Named nodes _seem_ to be preferred in DT for when there isn't a logical /
> > > natural numbering to the child nodes. A.e. for USB, reg is used to specify
> > > which port, for I2C, which address on the bus, but for two parallel and
> > > independent functions on the same device, it seems named nodes would make
> > > more sense in DT. Many examples exist in mainline where named nodes are used
> > > in DT in this way.
> >
> > Okay, I'm not an expert in the DT preferable schemas, so I believe DT people
> > should answer on this.
> 
> Hello,
> 
> Thanks for all the time spent reviewing this thus far. Following up to
> see what my next steps might be.
> 
> It sounds like we might want some DT folks to weigh in on the strategy
> used for identifying the child I2C and GPIO nodes for the CP2112
> device before moving further toward applying this.
> 
> Since the DT list is on this thread (as well as Rob+Krzystof), and
> this has sat for a little while, I'm assuming that the ball is in my
> court to seek out an answer/opinion here. (I know folks get a lot of
> email, so apologies if the correct move would have been to wait a bit
> longer before following up! Not intending to be rude.)
> 
> Would it be appropriate / expected that I send a separate email thread
> to the DT mailing list on their opinion here? Or would that create
> more confusion/complexity in adding yet another thread? I did create a
> separate email thread for the initial DT vs. ACPI conversation we had
> about accessing children by name or index in a unified way due to the
> differences in upper/lower case and use-cases, but that
> (understandably) didn't seem to gain any traction.
> 
> Thanks for any insights!
> 
> Thanks,
> Danny Kaehn
> 
> > > One example is network cards which provide an mdio bus
> > > bind through the child "mdio". One example of a specifically a
> > > child i2c controller being bound to "i2c" can be found in
> > > pine64,pinephone-keyboard.yaml.
> > > But it's certainly possible this isn't the desired direction moving forward
> > > in DT -- my opinion should definitely be taken with a grain of salt. Maybe
> > > this is something I should follow up on with DT folks on that DT vs. ACPI
> > > thread made earlier.
> > >
> > > One thing I did notice when looking at the mfd subsystem is that most DT
> > > drivers actually match on the compatible string of the child nodes, a.e.
> > > "silabs,cp2112", "silabs,cp2112-gpio".  "silabs,cp2112-i2c". We could
> > > implement that here, but I think that would make more sense if we were to
> > > actually split the cp2112 into mfd & platform drivers, and additionally split
> > > the DT binding by function.
> >
> > IIRC (but might be very well mistaken) the compatible strings for children
> > are discouraged.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* [PATCH v5] HID: steelseries: Add support for Arctis 1 XBox
From: Bastien Nocera @ 2023-07-03 10:25 UTC (permalink / raw)
  To: linux-input; +Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires

Add support for the Steelseries Arctis 1 XBox headset. This driver
will export the battery information from the headset, as well as the
"wireless_status" property.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
v5:
- Move spinlock init as per bentiss review
- Use the already defined response length constant when parsing answers
- Avoid parsing non-battery events (fixes battery showing up as 3%
  for a couple of seconds in some rare circumstances)

v4:
- Guard against crash when using uhid
- Print the contents of the raw events for debugging

v3:
- Dependency is on USB not USB_HID

v2:
- Fix missing USB dependency
- Fix config option description

 drivers/hid/Kconfig           |   6 +-
 drivers/hid/hid-steelseries.c | 311 ++++++++++++++++++++++++++++++++--
 2 files changed, 300 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ce012f83253..afe1c6070602 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1048,9 +1048,11 @@ config STEAM_FF
 	Deck.
 
 config HID_STEELSERIES
-	tristate "Steelseries SRW-S1 steering wheel support"
+	tristate "Steelseries devices support"
+	depends on USB
 	help
-	Support for Steelseries SRW-S1 steering wheel
+	Support for Steelseries SRW-S1 steering wheel, and the Steelseries
+	Arctis 1 Wireless for XBox headset.
 
 config HID_SUNPLUS
 	tristate "Sunplus wireless desktop"
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index aae3afc4107a..495377686123 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *  HID driver for Steelseries SRW-S1
+ *  HID driver for Steelseries devices
  *
  *  Copyright (c) 2013 Simon Wood
+ *  Copyright (c) 2023 Bastien Nocera
  */
 
 /*
@@ -11,10 +12,28 @@
 #include <linux/device.h>
 #include <linux/hid.h>
 #include <linux/module.h>
+#include <linux/usb.h>
 #include <linux/leds.h>
 
 #include "hid-ids.h"
 
+#define STEELSERIES_SRWS1		BIT(0)
+#define STEELSERIES_ARCTIS_1		BIT(1)
+
+struct steelseries_device {
+	struct hid_device *hdev;
+	unsigned long quirks;
+
+	struct delayed_work battery_work;
+	spinlock_t lock;
+	bool removed;
+
+	struct power_supply_desc battery_desc;
+	struct power_supply *battery;
+	uint8_t battery_capacity;
+	bool headset_connected;
+};
+
 #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
     (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
 #define SRWS1_NUMBER_LEDS 15
@@ -353,9 +372,211 @@ static void steelseries_srws1_remove(struct hid_device *hdev)
 }
 #endif
 
+#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS	3000
+
+#define ARCTIS_1_BATTERY_RESPONSE_LEN		8
+const char arctis_1_battery_request[] = { 0x06, 0x12 };
+
+static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
+{
+	u8 *write_buf;
+	int ret;
+
+	/* Request battery information */
+	write_buf = kmemdup(arctis_1_battery_request, sizeof(arctis_1_battery_request), GFP_KERNEL);
+	if (!write_buf)
+		return -ENOMEM;
+
+	ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0],
+				 write_buf, sizeof(arctis_1_battery_request),
+				 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+	if (ret < sizeof(arctis_1_battery_request)) {
+		hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
+		ret = -ENODATA;
+	}
+	kfree(write_buf);
+	return ret;
+}
+
+static void steelseries_headset_fetch_battery(struct hid_device *hdev)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	int ret = 0;
+
+	if (sd->quirks & STEELSERIES_ARCTIS_1)
+		ret = steelseries_headset_arctis_1_fetch_battery(hdev);
+
+	if (ret < 0)
+		hid_dbg(hdev,
+			"Battery query failed (err: %d)\n", ret);
+}
+
+static void steelseries_headset_battery_timer_tick(struct work_struct *work)
+{
+	struct steelseries_device *sd = container_of(work,
+		struct steelseries_device, battery_work.work);
+	struct hid_device *hdev = sd->hdev;
+
+	steelseries_headset_fetch_battery(hdev);
+}
+
+static int steelseries_headset_battery_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct steelseries_device *sd = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = sd->headset_connected ?
+			POWER_SUPPLY_STATUS_DISCHARGING :
+			POWER_SUPPLY_STATUS_UNKNOWN;
+		break;
+	case POWER_SUPPLY_PROP_SCOPE:
+		val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = sd->battery_capacity;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+static void
+steelseries_headset_set_wireless_status(struct hid_device *hdev,
+					bool connected)
+{
+	struct usb_interface *intf;
+
+	if (!hid_is_usb(hdev))
+		return;
+
+	intf = to_usb_interface(hdev->dev.parent);
+	usb_set_wireless_status(intf, connected ?
+				USB_WIRELESS_STATUS_CONNECTED :
+				USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
+static enum power_supply_property steelseries_headset_battery_props[] = {
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_SCOPE,
+	POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steelseries_headset_battery_register(struct steelseries_device *sd)
+{
+	static atomic_t battery_no = ATOMIC_INIT(0);
+	struct power_supply_config battery_cfg = { .drv_data = sd, };
+	unsigned long n;
+	int ret;
+
+	sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+	sd->battery_desc.properties = steelseries_headset_battery_props;
+	sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
+	sd->battery_desc.get_property = steelseries_headset_battery_get_property;
+	sd->battery_desc.use_for_apm = 0;
+	n = atomic_inc_return(&battery_no) - 1;
+	sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
+						    "steelseries_headset_battery_%ld", n);
+	if (!sd->battery_desc.name)
+		return -ENOMEM;
+
+	/* avoid the warning of 0% battery while waiting for the first info */
+	steelseries_headset_set_wireless_status(sd->hdev, false);
+	sd->battery_capacity = 100;
+
+	sd->battery = devm_power_supply_register(&sd->hdev->dev,
+			&sd->battery_desc, &battery_cfg);
+	if (IS_ERR(sd->battery)) {
+		ret = PTR_ERR(sd->battery);
+		hid_err(sd->hdev,
+				"%s:power_supply_register failed with error %d\n",
+				__func__, ret);
+		return ret;
+	}
+	power_supply_powers(sd->battery, &sd->hdev->dev);
+
+	INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
+	steelseries_headset_fetch_battery(sd->hdev);
+
+	return 0;
+}
+
+static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	struct steelseries_device *sd;
+	int ret;
+
+	sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
+	if (!sd)
+		return -ENOMEM;
+	hid_set_drvdata(hdev, sd);
+	sd->hdev = hdev;
+	sd->quirks = id->driver_data;
+
+	if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+		return steelseries_srws1_probe(hdev, id);
+#else
+		return -ENODEV;
+#endif
+	}
+
+	ret = hid_parse(hdev);
+	if (ret)
+		return ret;
+
+	spin_lock_init(&sd->lock);
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (ret)
+		return ret;
+
+	if (steelseries_headset_battery_register(sd) < 0)
+		hid_err(sd->hdev,
+			"Failed to register battery for headset\n");
+
+	return ret;
+}
+
+static void steelseries_remove(struct hid_device *hdev)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	unsigned long flags;
+
+	if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+		steelseries_srws1_remove(hdev);
+#endif
+		return;
+	}
+
+	spin_lock_irqsave(&sd->lock, flags);
+	sd->removed = true;
+	spin_unlock_irqrestore(&sd->lock, flags);
+
+	cancel_delayed_work_sync(&sd->battery_work);
+
+	hid_hw_stop(hdev);
+}
+
 static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
 {
+	if (hdev->vendor != USB_VENDOR_ID_STEELSERIES ||
+	    hdev->product != USB_DEVICE_ID_STEELSERIES_SRWS1)
+		return rdesc;
+
 	if (*rsize >= 115 && rdesc[11] == 0x02 && rdesc[13] == 0xc8
 			&& rdesc[29] == 0xbb && rdesc[40] == 0xc5) {
 		hid_info(hdev, "Fixing up Steelseries SRW-S1 report descriptor\n");
@@ -365,22 +586,82 @@ static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc
 	return rdesc;
 }
 
-static const struct hid_device_id steelseries_srws1_devices[] = {
-	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
+static int steelseries_headset_raw_event(struct hid_device *hdev,
+					struct hid_report *report, u8 *read_buf,
+					int size)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	int capacity = sd->battery_capacity;
+	bool connected = sd->headset_connected;
+	unsigned long flags;
+
+	/* Not a headset */
+	if (sd->quirks & STEELSERIES_SRWS1)
+		return 0;
+
+	if (sd->quirks & STEELSERIES_ARCTIS_1) {
+		hid_dbg(sd->hdev,
+			"Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
+		if (size < ARCTIS_1_BATTERY_RESPONSE_LEN ||
+		    memcmp (read_buf, arctis_1_battery_request, sizeof(arctis_1_battery_request)))
+			return 0;
+		if (read_buf[2] == 0x01) {
+			connected = false;
+			capacity = 100;
+		} else {
+			connected = true;
+			capacity = read_buf[3];
+		}
+	}
+
+	if (connected != sd->headset_connected) {
+		hid_dbg(sd->hdev,
+			"Connected status changed from %sconnected to %sconnected\n",
+			sd->headset_connected ? "" : "not ",
+			connected ? "" : "not ");
+		sd->headset_connected = connected;
+		steelseries_headset_set_wireless_status(hdev, connected);
+	}
+
+	if (capacity != sd->battery_capacity) {
+		hid_dbg(sd->hdev,
+			"Battery capacity changed from %d%% to %d%%\n",
+			sd->battery_capacity, capacity);
+		sd->battery_capacity = capacity;
+		power_supply_changed(sd->battery);
+	}
+
+	spin_lock_irqsave(&sd->lock, flags);
+	if (!sd->removed)
+		schedule_delayed_work(&sd->battery_work,
+				msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
+	spin_unlock_irqrestore(&sd->lock, flags);
+
+	return 0;
+}
+
+static const struct hid_device_id steelseries_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
+	  .driver_data = STEELSERIES_SRWS1 },
+
+	{ /* SteelSeries Arctis 1 Wireless for XBox */
+	  HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
+	.driver_data = STEELSERIES_ARCTIS_1 },
+
 	{ }
 };
-MODULE_DEVICE_TABLE(hid, steelseries_srws1_devices);
-
-static struct hid_driver steelseries_srws1_driver = {
-	.name = "steelseries_srws1",
-	.id_table = steelseries_srws1_devices,
-#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
-    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
-	.probe = steelseries_srws1_probe,
-	.remove = steelseries_srws1_remove,
-#endif
-	.report_fixup = steelseries_srws1_report_fixup
+MODULE_DEVICE_TABLE(hid, steelseries_devices);
+
+static struct hid_driver steelseries_driver = {
+	.name = "steelseries",
+	.id_table = steelseries_devices,
+	.probe = steelseries_probe,
+	.remove = steelseries_remove,
+	.report_fixup = steelseries_srws1_report_fixup,
+	.raw_event = steelseries_headset_raw_event,
 };
 
-module_hid_driver(steelseries_srws1_driver);
+module_hid_driver(steelseries_driver);
 MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_AUTHOR("Simon Wood <simon@mungewell.org>");
-- 
2.41.0


^ permalink raw reply related

* [PATCH v4] HID: steelseries: Add support for Arctis 1 XBox
From: Bastien Nocera @ 2023-07-03  8:48 UTC (permalink / raw)
  To: linux-input; +Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires

Add support for the Steelseries Arctis 1 XBox headset. This driver
will export the battery information from the headset, as well as the
"wireless_status" property.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
v4:
- Guard against crash when using uhid
- Print the contents of the raw events for debugging

v3:
- Dependency is on USB not USB_HID

v2:
- Fix missing USB dependency
- Fix config option description

 drivers/hid/Kconfig           |   6 +-
 drivers/hid/hid-steelseries.c | 310 ++++++++++++++++++++++++++++++++--
 2 files changed, 299 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ce012f83253..afe1c6070602 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1048,9 +1048,11 @@ config STEAM_FF
 	Deck.
 
 config HID_STEELSERIES
-	tristate "Steelseries SRW-S1 steering wheel support"
+	tristate "Steelseries devices support"
+	depends on USB
 	help
-	Support for Steelseries SRW-S1 steering wheel
+	Support for Steelseries SRW-S1 steering wheel, and the Steelseries
+	Arctis 1 Wireless for XBox headset.
 
 config HID_SUNPLUS
 	tristate "Sunplus wireless desktop"
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index aae3afc4107a..93bc70ba47ab 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *  HID driver for Steelseries SRW-S1
+ *  HID driver for Steelseries devices
  *
  *  Copyright (c) 2013 Simon Wood
+ *  Copyright (c) 2023 Bastien Nocera
  */
 
 /*
@@ -11,10 +12,28 @@
 #include <linux/device.h>
 #include <linux/hid.h>
 #include <linux/module.h>
+#include <linux/usb.h>
 #include <linux/leds.h>
 
 #include "hid-ids.h"
 
+#define STEELSERIES_SRWS1		BIT(0)
+#define STEELSERIES_ARCTIS_1		BIT(1)
+
+struct steelseries_device {
+	struct hid_device *hdev;
+	unsigned long quirks;
+
+	struct delayed_work battery_work;
+	spinlock_t lock;
+	bool removed;
+
+	struct power_supply_desc battery_desc;
+	struct power_supply *battery;
+	uint8_t battery_capacity;
+	bool headset_connected;
+};
+
 #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
     (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
 #define SRWS1_NUMBER_LEDS 15
@@ -353,9 +372,211 @@ static void steelseries_srws1_remove(struct hid_device *hdev)
 }
 #endif
 
+#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS	3000
+
+#define ARCTIS_1_BATTERY_RESPONSE_LEN		8
+
+static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
+{
+	u8 *write_buf;
+	int ret;
+	char battery_request[2] = { 0x06, 0x12 };
+
+	/* Request battery information */
+	write_buf = kmemdup(battery_request, sizeof(battery_request), GFP_KERNEL);
+	if (!write_buf)
+		return -ENOMEM;
+
+	ret = hid_hw_raw_request(hdev, battery_request[0],
+				 write_buf, sizeof(battery_request),
+				 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+	if (ret < sizeof(battery_request)) {
+		hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
+		ret = -ENODATA;
+	}
+	kfree(write_buf);
+	return ret;
+}
+
+static void steelseries_headset_fetch_battery(struct hid_device *hdev)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	int ret = 0;
+
+	if (sd->quirks & STEELSERIES_ARCTIS_1)
+		ret = steelseries_headset_arctis_1_fetch_battery(hdev);
+
+	if (ret < 0)
+		hid_dbg(hdev,
+			"Battery query failed (err: %d)\n", ret);
+}
+
+static void steelseries_headset_battery_timer_tick(struct work_struct *work)
+{
+	struct steelseries_device *sd = container_of(work,
+		struct steelseries_device, battery_work.work);
+	struct hid_device *hdev = sd->hdev;
+
+	steelseries_headset_fetch_battery(hdev);
+}
+
+static int steelseries_headset_battery_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct steelseries_device *sd = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = sd->headset_connected ?
+			POWER_SUPPLY_STATUS_DISCHARGING :
+			POWER_SUPPLY_STATUS_UNKNOWN;
+		break;
+	case POWER_SUPPLY_PROP_SCOPE:
+		val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = sd->battery_capacity;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+static void
+steelseries_headset_set_wireless_status(struct hid_device *hdev,
+					bool connected)
+{
+	struct usb_interface *intf;
+
+	if (!hid_is_usb(hdev))
+		return;
+
+	intf = to_usb_interface(hdev->dev.parent);
+	usb_set_wireless_status(intf, connected ?
+				USB_WIRELESS_STATUS_CONNECTED :
+				USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
+static enum power_supply_property steelseries_headset_battery_props[] = {
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_SCOPE,
+	POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steelseries_headset_battery_register(struct steelseries_device *sd)
+{
+	static atomic_t battery_no = ATOMIC_INIT(0);
+	struct power_supply_config battery_cfg = { .drv_data = sd, };
+	unsigned long n;
+	int ret;
+
+	sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+	sd->battery_desc.properties = steelseries_headset_battery_props;
+	sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
+	sd->battery_desc.get_property = steelseries_headset_battery_get_property;
+	sd->battery_desc.use_for_apm = 0;
+	n = atomic_inc_return(&battery_no) - 1;
+	sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
+						    "steelseries_headset_battery_%ld", n);
+	if (!sd->battery_desc.name)
+		return -ENOMEM;
+
+	/* avoid the warning of 0% battery while waiting for the first info */
+	steelseries_headset_set_wireless_status(sd->hdev, false);
+	sd->battery_capacity = 100;
+
+	sd->battery = devm_power_supply_register(&sd->hdev->dev,
+			&sd->battery_desc, &battery_cfg);
+	if (IS_ERR(sd->battery)) {
+		ret = PTR_ERR(sd->battery);
+		hid_err(sd->hdev,
+				"%s:power_supply_register failed with error %d\n",
+				__func__, ret);
+		return ret;
+	}
+	power_supply_powers(sd->battery, &sd->hdev->dev);
+
+	INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
+	steelseries_headset_fetch_battery(sd->hdev);
+
+	return 0;
+}
+
+static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	struct steelseries_device *sd;
+	int ret;
+
+	sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
+	if (!sd)
+		return -ENOMEM;
+	hid_set_drvdata(hdev, sd);
+	sd->hdev = hdev;
+	sd->quirks = id->driver_data;
+
+	if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+		return steelseries_srws1_probe(hdev, id);
+#else
+		return -ENODEV;
+#endif
+	}
+
+	ret = hid_parse(hdev);
+	if (ret)
+		return ret;
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (ret)
+		return ret;
+
+	if (steelseries_headset_battery_register(sd) < 0)
+		hid_err(sd->hdev,
+			"Failed to register battery for headset\n");
+
+	spin_lock_init(&sd->lock);
+
+	return ret;
+}
+
+static void steelseries_remove(struct hid_device *hdev)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	unsigned long flags;
+
+	if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+		steelseries_srws1_remove(hdev);
+#endif
+		return;
+	}
+
+	spin_lock_irqsave(&sd->lock, flags);
+	sd->removed = true;
+	spin_unlock_irqrestore(&sd->lock, flags);
+
+	cancel_delayed_work_sync(&sd->battery_work);
+
+	hid_hw_stop(hdev);
+}
+
 static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
 {
+	if (hdev->vendor != USB_VENDOR_ID_STEELSERIES ||
+	    hdev->product != USB_DEVICE_ID_STEELSERIES_SRWS1)
+		return rdesc;
+
 	if (*rsize >= 115 && rdesc[11] == 0x02 && rdesc[13] == 0xc8
 			&& rdesc[29] == 0xbb && rdesc[40] == 0xc5) {
 		hid_info(hdev, "Fixing up Steelseries SRW-S1 report descriptor\n");
@@ -365,22 +586,81 @@ static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc
 	return rdesc;
 }
 
-static const struct hid_device_id steelseries_srws1_devices[] = {
-	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
+static int steelseries_headset_raw_event(struct hid_device *hdev,
+					struct hid_report *report, u8 *read_buf,
+					int size)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	int capacity = sd->battery_capacity;
+	bool connected = sd->headset_connected;
+	unsigned long flags;
+
+	/* Not a headset */
+	if (sd->quirks & STEELSERIES_SRWS1)
+		return 0;
+
+	if (sd->quirks & STEELSERIES_ARCTIS_1) {
+		hid_dbg(sd->hdev,
+			"Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
+		if (size < 8)
+			return 0;
+		if (read_buf[2] == 0x01) {
+			connected = false;
+			capacity = 100;
+		} else {
+			connected = true;
+			capacity = read_buf[3];
+		}
+	}
+
+	if (connected != sd->headset_connected) {
+		hid_dbg(sd->hdev,
+			"Connected status changed from %sconnected to %sconnected\n",
+			sd->headset_connected ? "" : "not ",
+			connected ? "" : "not ");
+		sd->headset_connected = connected;
+		steelseries_headset_set_wireless_status(hdev, connected);
+	}
+
+	if (capacity != sd->battery_capacity) {
+		hid_dbg(sd->hdev,
+			"Battery capacity changed from %d%% to %d%%\n",
+			sd->battery_capacity, capacity);
+		sd->battery_capacity = capacity;
+		power_supply_changed(sd->battery);
+	}
+
+	spin_lock_irqsave(&sd->lock, flags);
+	if (!sd->removed)
+		schedule_delayed_work(&sd->battery_work,
+				msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
+	spin_unlock_irqrestore(&sd->lock, flags);
+
+	return 0;
+}
+
+static const struct hid_device_id steelseries_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
+	  .driver_data = STEELSERIES_SRWS1 },
+
+	{ /* SteelSeries Arctis 1 Wireless for XBox */
+	  HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
+	.driver_data = STEELSERIES_ARCTIS_1 },
+
 	{ }
 };
-MODULE_DEVICE_TABLE(hid, steelseries_srws1_devices);
-
-static struct hid_driver steelseries_srws1_driver = {
-	.name = "steelseries_srws1",
-	.id_table = steelseries_srws1_devices,
-#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
-    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
-	.probe = steelseries_srws1_probe,
-	.remove = steelseries_srws1_remove,
-#endif
-	.report_fixup = steelseries_srws1_report_fixup
+MODULE_DEVICE_TABLE(hid, steelseries_devices);
+
+static struct hid_driver steelseries_driver = {
+	.name = "steelseries",
+	.id_table = steelseries_devices,
+	.probe = steelseries_probe,
+	.remove = steelseries_remove,
+	.report_fixup = steelseries_srws1_report_fixup,
+	.raw_event = steelseries_headset_raw_event,
 };
 
-module_hid_driver(steelseries_srws1_driver);
+module_hid_driver(steelseries_driver);
 MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_AUTHOR("Simon Wood <simon@mungewell.org>");
-- 
2.41.0


^ permalink raw reply related

* [PATCH 2/2] Input: exc3000 - Support power supply regulators
From: Mike Looijmans @ 2023-07-03  8:45 UTC (permalink / raw)
  To: devicetree, linux-input
  Cc: Mike Looijmans, Dmitry Torokhov, Uwe Kleine-König,
	linux-kernel
In-Reply-To: <20230703084536.8429-1-mike.looijmans@topic.nl>

Add power supply regulator support to the exc3000 devices.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>

---

 drivers/input/touchscreen/exc3000.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/touchscreen/exc3000.c b/drivers/input/touchscreen/exc3000.c
index 4af4c1e5d0da..3e50af8a4a2d 100644
--- a/drivers/input/touchscreen/exc3000.c
+++ b/drivers/input/touchscreen/exc3000.c
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/regulator/consumer.h>
 #include <linux/sizes.h>
 #include <linux/timer.h>
 #include <asm/unaligned.h>
@@ -360,6 +361,12 @@ static int exc3000_probe(struct i2c_client *client)
 	if (IS_ERR(data->reset))
 		return PTR_ERR(data->reset);
 
+	/* For proper reset sequence, enable power while reset asserted */
+	error = devm_regulator_get_enable_optional(&client->dev, "vdd");
+	if (error && error != -ENODEV)
+		dev_err_probe(&client->dev, error,
+			      "failed to request vdd regulator\n");
+
 	if (data->reset) {
 		msleep(EXC3000_RESET_MS);
 		gpiod_set_value_cansleep(data->reset, 0);
-- 
2.17.1


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topicproducts.com
W: www.topic.nl

Please consider the environment before printing this e-mail

^ permalink raw reply related

* [PATCH 1/2] dt-bindings: Input: exc3000 - Support power supply regulators
From: Mike Looijmans @ 2023-07-03  8:45 UTC (permalink / raw)
  To: devicetree, linux-input
  Cc: Mike Looijmans, Conor Dooley, Dmitry Torokhov,
	Krzysztof Kozlowski, Rob Herring, linux-kernel
In-Reply-To: <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.056940e6-2612-4bc3-a2f6-33121e64866d@emailsignatures365.codetwo.com>

Add power supply regulator support to the exc3000 devices.
This provides the devicetree bindings.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>

---

 .../devicetree/bindings/input/touchscreen/eeti,exc3000.yaml     | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/eeti,exc3000.yaml b/Documentation/devicetree/bindings/input/touchscreen/eeti,exc3000.yaml
index 007adbc89c14..9dc25d30a0a8 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/eeti,exc3000.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/eeti,exc3000.yaml
@@ -24,6 +24,8 @@ properties:
     maxItems: 1
   reset-gpios:
     maxItems: 1
+  vdd-supply:
+    description: Power supply regulator for the chip
   touchscreen-size-x: true
   touchscreen-size-y: true
   touchscreen-inverted-x: true
-- 
2.17.1


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topicproducts.com
W: www.topic.nl

Please consider the environment before printing this e-mail

^ permalink raw reply related

* Re: [PATCH] HID: i2c-hid: Block a rogue device on ASUS TUF A16
From: Benjamin Tissoires @ 2023-07-03  8:23 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: Friedrich Vock, linux-input, Natikar, Basavaraj,
	S-k, Shyam-sundar, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <9c10f6f2-b145-db5e-ebed-981222a72be7@amd.com>


On Jun 30 2023, Limonciello, Mario wrote:
[...]
> 
> Friderich and some people on CC are already aware of this, but mostly for
> Benjamin and Jiri I wanted to let you know that the additional register
> fetching comparing Windows and Linux allowed me to come up with a proper
> root cause.
> 
> This series has been sent out to fix the issue properly.

Great, thanks for the heads up :)

Cheers,
Benjamin


> 
> https://lore.kernel.org/linux-gpio/20230630194716.6497-1-mario.limonciello@amd.com/

^ permalink raw reply

* Re: [PATCH v3] HID: steelseries: Add support for Arctis 1 XBox
From: Benjamin Tissoires @ 2023-07-03  8:22 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: linux-input, linux-kernel, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20230630131217.51827-1-hadess@hadess.net>


On Jun 30 2023, Bastien Nocera wrote:
> 
> Add support for the Steelseries Arctis 1 XBox headset. This driver
> will export the battery information from the headset, as well as the
> "wireless_status" property.
> 
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> ---
> v3:
> - Dependency is on USB not USB_HID
> 
> v2:
> - Fix missing USB dependency
> - Fix config option description
> 
>  drivers/hid/Kconfig           |   6 +-
>  drivers/hid/hid-steelseries.c | 307 ++++++++++++++++++++++++++++++++--
>  2 files changed, 296 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 4ce012f83253..afe1c6070602 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -1048,9 +1048,11 @@ config STEAM_FF
>  	Deck.
>  
>  config HID_STEELSERIES
> -	tristate "Steelseries SRW-S1 steering wheel support"
> +	tristate "Steelseries devices support"
> +	depends on USB
>  	help
> -	Support for Steelseries SRW-S1 steering wheel
> +	Support for Steelseries SRW-S1 steering wheel, and the Steelseries
> +	Arctis 1 Wireless for XBox headset.
>  
>  config HID_SUNPLUS
>  	tristate "Sunplus wireless desktop"
> diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
> index aae3afc4107a..a9300a4244aa 100644
> --- a/drivers/hid/hid-steelseries.c
> +++ b/drivers/hid/hid-steelseries.c
> @@ -1,8 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - *  HID driver for Steelseries SRW-S1
> + *  HID driver for Steelseries devices
>   *
>   *  Copyright (c) 2013 Simon Wood
> + *  Copyright (c) 2023 Bastien Nocera
>   */
>  
>  /*
> @@ -11,10 +12,28 @@
>  #include <linux/device.h>
>  #include <linux/hid.h>
>  #include <linux/module.h>
> +#include <linux/usb.h>
>  #include <linux/leds.h>
>  
>  #include "hid-ids.h"
>  
> +#define STEELSERIES_SRWS1		BIT(0)
> +#define STEELSERIES_ARCTIS_1		BIT(1)
> +
> +struct steelseries_device {
> +	struct hid_device *hdev;
> +	unsigned long quirks;
> +
> +	struct delayed_work battery_work;
> +	spinlock_t lock;
> +	bool removed;
> +
> +	struct power_supply_desc battery_desc;
> +	struct power_supply *battery;
> +	uint8_t battery_capacity;
> +	bool headset_connected;
> +};
> +
>  #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
>      (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
>  #define SRWS1_NUMBER_LEDS 15
> @@ -353,9 +372,208 @@ static void steelseries_srws1_remove(struct hid_device *hdev)
>  }
>  #endif
>  
> +#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS	3000
> +
> +#define ARCTIS_1_BATTERY_RESPONSE_LEN		8
> +
> +static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
> +{
> +	u8 *write_buf;
> +	int ret;
> +	char battery_request[2] = { 0x06, 0x12 };
> +
> +	/* Request battery information */
> +	write_buf = kmemdup(battery_request, sizeof(battery_request), GFP_KERNEL);
> +	if (!write_buf)
> +		return -ENOMEM;
> +
> +	ret = hid_hw_raw_request(hdev, battery_request[0],
> +				 write_buf, sizeof(battery_request),
> +				 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> +	if (ret < sizeof(battery_request)) {
> +		hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
> +		ret = -ENODATA;
> +	}
> +	kfree(write_buf);
> +	return ret;
> +}
> +
> +static void steelseries_headset_fetch_battery(struct hid_device *hdev)
> +{
> +	struct steelseries_device *sd = hid_get_drvdata(hdev);
> +	int ret = 0;
> +
> +	if (sd->quirks & STEELSERIES_ARCTIS_1)
> +		ret = steelseries_headset_arctis_1_fetch_battery(hdev);
> +
> +	if (ret < 0)
> +		hid_dbg(hdev,
> +			"Battery query failed (err: %d)\n", ret);
> +}
> +
> +static void steelseries_headset_battery_timer_tick(struct work_struct *work)
> +{
> +	struct steelseries_device *sd = container_of(work,
> +		struct steelseries_device, battery_work.work);
> +	struct hid_device *hdev = sd->hdev;
> +
> +	steelseries_headset_fetch_battery(hdev);
> +}
> +
> +static int steelseries_headset_battery_get_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val)
> +{
> +	struct steelseries_device *sd = power_supply_get_drvdata(psy);
> +	int ret = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_PRESENT:
> +		val->intval = 1;
> +		break;
> +	case POWER_SUPPLY_PROP_STATUS:
> +		val->intval = sd->headset_connected ?
> +			POWER_SUPPLY_STATUS_DISCHARGING :
> +			POWER_SUPPLY_STATUS_UNKNOWN;
> +		break;
> +	case POWER_SUPPLY_PROP_SCOPE:
> +		val->intval = POWER_SUPPLY_SCOPE_DEVICE;
> +		break;
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		val->intval = sd->battery_capacity;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	return ret;
> +}
> +
> +static void
> +steelseries_headset_set_wireless_status(struct hid_device *hdev,
> +					bool connected)
> +{
> +	struct usb_interface *intf;
> +

You need to inset a `if (!hid_is_usb(hdev)) return;` here, so that a uhid
device will not oops the kernel.

> +	intf = to_usb_interface(hdev->dev.parent);
> +	usb_set_wireless_status(intf, connected ?
> +				USB_WIRELESS_STATUS_CONNECTED :
> +				USB_WIRELESS_STATUS_DISCONNECTED);
> +}
> +
> +static enum power_supply_property steelseries_headset_battery_props[] = {
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_SCOPE,
> +	POWER_SUPPLY_PROP_CAPACITY,
> +};
> +
> +static int steelseries_headset_battery_register(struct steelseries_device *sd)
> +{
> +	static atomic_t battery_no = ATOMIC_INIT(0);
> +	struct power_supply_config battery_cfg = { .drv_data = sd, };
> +	unsigned long n;
> +	int ret;
> +
> +	sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> +	sd->battery_desc.properties = steelseries_headset_battery_props;
> +	sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
> +	sd->battery_desc.get_property = steelseries_headset_battery_get_property;
> +	sd->battery_desc.use_for_apm = 0;
> +	n = atomic_inc_return(&battery_no) - 1;
> +	sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
> +						    "steelseries_headset_battery_%ld", n);
> +	if (!sd->battery_desc.name)
> +		return -ENOMEM;
> +
> +	/* avoid the warning of 0% battery while waiting for the first info */
> +	steelseries_headset_set_wireless_status(sd->hdev, false);
> +	sd->battery_capacity = 100;
> +
> +	sd->battery = devm_power_supply_register(&sd->hdev->dev,
> +			&sd->battery_desc, &battery_cfg);
> +	if (IS_ERR(sd->battery)) {
> +		ret = PTR_ERR(sd->battery);
> +		hid_err(sd->hdev,
> +				"%s:power_supply_register failed with error %d\n",
> +				__func__, ret);
> +		return ret;
> +	}
> +	power_supply_powers(sd->battery, &sd->hdev->dev);
> +
> +	INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
> +	steelseries_headset_fetch_battery(sd->hdev);
> +
> +	return 0;
> +}
> +
> +static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> +	struct steelseries_device *sd;
> +	int ret;
> +
> +	sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
> +	if (!sd)
> +		return -ENOMEM;

Nitpick: please add a new-line here

> +	hid_set_drvdata(hdev, sd);
> +	sd->hdev = hdev;
> +	sd->quirks = id->driver_data;
> +
> +	if (sd->quirks & STEELSERIES_SRWS1) {
> +#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
> +    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
> +		return steelseries_srws1_probe(hdev, id);
> +#else
> +		return -ENODEV;
> +#endif
> +	}
> +
> +	ret = hid_parse(hdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> +	if (ret)
> +		return ret;
> +
> +	if (steelseries_headset_battery_register(sd) < 0)
> +		hid_err(sd->hdev,
> +			"Failed to register battery for headset\n");
> +
> +	spin_lock_init(&sd->lock);

I'd say the spin_lock should be initialized before we register the
battery, given that steelseries_headset_battery_register() calls in the
workqueue already. No?

Actually the lock is used in .raw_event, so this should be called before
hid_hw_start()


> +
> +	return ret;
> +}
> +
> +static void steelseries_remove(struct hid_device *hdev)
> +{
> +	struct steelseries_device *sd = hid_get_drvdata(hdev);
> +	unsigned long flags;
> +
> +	if (sd->quirks & STEELSERIES_SRWS1) {
> +#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
> +    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
> +		steelseries_srws1_remove(hdev);
> +#endif
> +		return;
> +	}
> +
> +	spin_lock_irqsave(&sd->lock, flags);
> +	sd->removed = true;
> +	spin_unlock_irqrestore(&sd->lock, flags);
> +
> +	cancel_delayed_work_sync(&sd->battery_work);
> +
> +	hid_hw_stop(hdev);
> +}
> +
>  static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		unsigned int *rsize)
>  {
> +	if (hdev->vendor != USB_VENDOR_ID_STEELSERIES ||
> +	    hdev->product != USB_DEVICE_ID_STEELSERIES_SRWS1)
> +		return rdesc;
> +
>  	if (*rsize >= 115 && rdesc[11] == 0x02 && rdesc[13] == 0xc8
>  			&& rdesc[29] == 0xbb && rdesc[40] == 0xc5) {
>  		hid_info(hdev, "Fixing up Steelseries SRW-S1 report descriptor\n");
> @@ -365,22 +583,81 @@ static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc
>  	return rdesc;
>  }
>  
> -static const struct hid_device_id steelseries_srws1_devices[] = {
> -	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
> +static int steelseries_headset_raw_event(struct hid_device *hdev,
> +					struct hid_report *report, u8 *read_buf,
> +					int size)
> +{
> +	struct steelseries_device *sd = hid_get_drvdata(hdev);
> +	int capacity = sd->battery_capacity;
> +	bool connected = sd->headset_connected;
> +	unsigned long flags;
> +
> +	/* Not a headset */
> +	if (sd->quirks & STEELSERIES_SRWS1)
> +		return 0;
> +
> +	if (sd->quirks & STEELSERIES_ARCTIS_1) {
> +		hid_dbg(sd->hdev,
> +			"Parsing raw event for Arctis 1 headset (len: %d)\n", size);
> +		if (size < 8)
> +			return 0;
> +		if (read_buf[2] == 0x01) {
> +			connected = false;
> +			capacity = 100;
> +		} else {
> +			connected = true;
> +			capacity = read_buf[3];
> +		}
> +	}
> +
> +	if (connected != sd->headset_connected) {
> +		hid_dbg(sd->hdev,
> +			"Connected status changed from %sconnected to %sconnected\n",
> +			sd->headset_connected ? "" : "not ",
> +			connected ? "" : "not ");
> +		sd->headset_connected = connected;
> +		steelseries_headset_set_wireless_status(hdev, connected);
> +	}
> +
> +	if (capacity != sd->battery_capacity) {
> +		hid_dbg(sd->hdev,
> +			"Battery capacity changed from %d%% to %d%%\n",
> +			sd->battery_capacity, capacity);
> +		sd->battery_capacity = capacity;
> +		power_supply_changed(sd->battery);
> +	}
> +
> +	spin_lock_irqsave(&sd->lock, flags);
> +	if (!sd->removed)
> +		schedule_delayed_work(&sd->battery_work,
> +				msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
> +	spin_unlock_irqrestore(&sd->lock, flags);
> +
> +	return 0;
> +}
> +
> +static const struct hid_device_id steelseries_devices[] = {
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
> +	  .driver_data = STEELSERIES_SRWS1 },
> +
> +	{ /* SteelSeries Arctis 1 Wireless for XBox */
> +	  HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
> +	.driver_data = STEELSERIES_ARCTIS_1 },
> +
>  	{ }
>  };
> -MODULE_DEVICE_TABLE(hid, steelseries_srws1_devices);
> -
> -static struct hid_driver steelseries_srws1_driver = {
> -	.name = "steelseries_srws1",
> -	.id_table = steelseries_srws1_devices,
> -#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
> -    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
> -	.probe = steelseries_srws1_probe,
> -	.remove = steelseries_srws1_remove,
> -#endif
> -	.report_fixup = steelseries_srws1_report_fixup
> +MODULE_DEVICE_TABLE(hid, steelseries_devices);
> +
> +static struct hid_driver steelseries_driver = {
> +	.name = "steelseries",
> +	.id_table = steelseries_devices,
> +	.probe = steelseries_probe,
> +	.remove = steelseries_remove,
> +	.report_fixup = steelseries_srws1_report_fixup,
> +	.raw_event = steelseries_headset_raw_event,
>  };
>  
> -module_hid_driver(steelseries_srws1_driver);
> +module_hid_driver(steelseries_driver);
>  MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
> +MODULE_AUTHOR("Simon Wood <simon@mungewell.org>");
> -- 
> 2.41.0
> 

Few nitpicks here and there, but code looks good otherwise.

We are still in the merge window, so no commits targetting v6.6 will be
included until this is over (we should have 6.5-rc1 at the end of the
week IIRC).

Cheers,
Benjamin

^ permalink raw reply

* [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor
From: Riwen Lu @ 2023-07-03  6:01 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, dmitry.torokhov, linux, hdegoede,
	rrangel, u.kleine-koenig
  Cc: linux-input, linux-kernel, Riwen Lu

From: Riwen Lu <luriwen@kylinos.cn>

The format '%*ph' print up to 64 bytes long as a hex string with ' '
sepatator. Usually the size of report descriptor is larger than 64
bytes, so consider using print_hex_dump_debug to print out all of it for
better debugging.

Signed-off-by: Riwen Lu <luriwen@kylinos.cn>
---
 drivers/hid/i2c-hid/i2c-hid-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index efbba0465eef..8e97fc01c852 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -772,7 +772,9 @@ static int i2c_hid_parse(struct hid_device *hid)
 		}
 	}
 
-	i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc);
+	i2c_hid_dbg(ihid, "Report Descriptor\n");
+	print_hex_dump_debug("  ", DUMP_PREFIX_OFFSET, 16, 1,
+			rdesc, rsize, false);
 
 	ret = hid_parse_report(hid, rdesc, rsize);
 	if (!use_override)
-- 
2.25.1


^ permalink raw reply related

* RE: [PATCH V2] Input: synaptics-rmi4- Add a new feature for Forepad.
From: Marge Yang @ 2023-07-03  2:49 UTC (permalink / raw)
  To: Andi Shyti, Marge Yang
  Cc: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, David Chiu, Derek Cheng, Sam Tsai,
	Vincent Huang
In-Reply-To: <20230613204855.bm4wxneamkd76heq@intel.intel>

Hi Andi,
	I have modified it and send PATCH V3.
Thanks
Marge Yang

-----Original Message-----
From: Andi Shyti <andi.shyti@kernel.org> 
Sent: Wednesday, June 14, 2023 4:49 AM
To: Marge Yang <Marge.Yang@tw.synaptics.com>
Cc: dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; David Chiu <David.Chiu@tw.synaptics.com>; Derek Cheng <derek.cheng@tw.synaptics.com>; Sam Tsai <Sam.Tsai@synaptics.com>; Vincent Huang <Vincent.huang@tw.synaptics.com>
Subject: Re: [PATCH V2] Input: synaptics-rmi4- Add a new feature for Forepad.

CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.


Hi Marge,

[...]

> +static irqreturn_t rmi_f21_attention(int irq, void *ctx) {
> +     struct rmi_function *fn = ctx;
> +     struct f21_data *f21 = dev_get_drvdata(&fn->dev);
> +     int error;
> +
> +     error = rmi_read_block(fn->rmi_dev,
> +                             fn->fd.data_base_addr+RMI_F21_FORCE_CLICK_OFFSET,
> +                             f21->data_regs, 1);
> +     if (error) {
> +             dev_err(&fn->dev,
> +                     "%s: Failed to read f21 data registers: %d\n",
> +                     __func__, error);
> +             return IRQ_RETVAL(error);
> +     }
> +
> +     if (!!(f21->data_regs[0] & RMI_f21_FORCE_CLICK))

no need for double negation here.

Andi

> +             input_report_key(f21->input, f21->key_code, true);
> +     else
> +             input_report_key(f21->input, f21->key_code, false);
> +     return IRQ_HANDLED;
> +}

^ permalink raw reply

* [PATCH V3] Input: synaptics-rmi4- Add a new feature for Forepad.
From: Marge Yang @ 2023-07-03  2:47 UTC (permalink / raw)
  To: dmitry.torokhov, linux-input, linux-kernel, marge.yang
  Cc: david.chiu, derek.cheng, sam.tsai, vincent.huang

Forcepad devices will use F21, for click simulation
due to lack of a metal button, so we add F21 support
to make forcepad support click function.

Signed-off-by: Marge Yang <marge.yang@tw.synaptics.com>
---
 drivers/input/rmi4/Kconfig      |   8 +++
 drivers/input/rmi4/Makefile     |   1 +
 drivers/input/rmi4/rmi_bus.c    |   3 +
 drivers/input/rmi4/rmi_driver.h |   5 +-
 drivers/input/rmi4/rmi_f21.c    | 110 ++++++++++++++++++++++++++++++++
 5 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100644 drivers/input/rmi4/rmi_f21.c

diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index c0163b983ce6..37e2ba4918fd 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -108,6 +108,14 @@ config RMI4_F3A
 	  Function 3A provides GPIO support for RMI4 devices. This includes
 	  support for buttons on TouchPads and ClickPads.
 
+config RMI4_F21
+	bool "RMI4 Function 21 (PRESSURE)"
+	help
+	  Say Y here if you want to add support for RMI4 function 21.
+
+	  Function 21 provides buttons/pressure for RMI4 devices. This includes
+	  support for buttons/pressure on PressurePad.
+
 config RMI4_F54
 	bool "RMI4 Function 54 (Analog diagnostics)"
 	depends on VIDEO_DEV=y || (RMI4_CORE=m && VIDEO_DEV=m)
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 02f14c846861..ec4f08513d8b 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -11,6 +11,7 @@ rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
 rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
 rmi_core-$(CONFIG_RMI4_F34) += rmi_f34.o rmi_f34v7.o
 rmi_core-$(CONFIG_RMI4_F3A) += rmi_f3a.o
+rmi_core-$(CONFIG_RMI4_F21) += rmi_f21.o
 rmi_core-$(CONFIG_RMI4_F54) += rmi_f54.o
 rmi_core-$(CONFIG_RMI4_F55) += rmi_f55.o
 
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index f2e093b0b998..e12033aa7517 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -369,6 +369,9 @@ static struct rmi_function_handler *fn_handlers[] = {
 #ifdef CONFIG_RMI4_F3A
 	&rmi_f3a_handler,
 #endif
+#ifdef CONFIG_RMI4_F21
+	&rmi_f21_handler,
+#endif
 #ifdef CONFIG_RMI4_F54
 	&rmi_f54_handler,
 #endif
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 1c6c6086c0e5..57f258371bab 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -114,7 +114,9 @@ static inline int rmi_f03_overwrite_button(struct rmi_function *fn,
 }
 static inline void rmi_f03_commit_buttons(struct rmi_function *fn) {}
 #endif
-
+#ifdef CONFIG_RMI4_F21
+int rmi_f21_report_pressure(struct rmi_function *fn, int i);
+#endif
 #ifdef CONFIG_RMI4_F34
 int rmi_f34_create_sysfs(struct rmi_device *rmi_dev);
 void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev);
@@ -136,6 +138,7 @@ extern struct rmi_function_handler rmi_f12_handler;
 extern struct rmi_function_handler rmi_f30_handler;
 extern struct rmi_function_handler rmi_f34_handler;
 extern struct rmi_function_handler rmi_f3a_handler;
+extern struct rmi_function_handler rmi_f21_handler;
 extern struct rmi_function_handler rmi_f54_handler;
 extern struct rmi_function_handler rmi_f55_handler;
 #endif
diff --git a/drivers/input/rmi4/rmi_f21.c b/drivers/input/rmi4/rmi_f21.c
new file mode 100644
index 000000000000..bbb9ab12ae7c
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f21.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2012-2020 Synaptics Incorporated
+ */
+
+#include <linux/kernel.h>
+#include <linux/rmi.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include "rmi_driver.h"
+
+#define RMI_F21_FORCE_CLICK_OFFSET	8
+#define RMI_f21_FORCE_CLICK			0x01
+#define RMI_f21_DATA_REGS_MAX_SIZE	1
+#define RMI_f21_FORCEPAD_BUTTON_COUNT	1
+
+struct f21_data {
+	/* Query Data */
+	u8 data_regs[RMI_f21_DATA_REGS_MAX_SIZE];
+	struct input_dev *input;
+	u16 key_code;
+};
+
+static irqreturn_t rmi_f21_attention(int irq, void *ctx)
+{
+	struct rmi_function *fn = ctx;
+	struct f21_data *f21 = dev_get_drvdata(&fn->dev);
+	int error;
+
+	error = rmi_read_block(fn->rmi_dev,
+				fn->fd.data_base_addr+RMI_F21_FORCE_CLICK_OFFSET,
+				f21->data_regs, 1);
+	if (error) {
+		dev_err(&fn->dev,
+			"%s: Failed to read f21 data registers: %d\n",
+			__func__, error);
+		return IRQ_RETVAL(error);
+	}
+
+	if ((f21->data_regs[0] & RMI_f21_FORCE_CLICK))
+		input_report_key(f21->input, f21->key_code, true);
+	else
+		input_report_key(f21->input, f21->key_code, false);
+	return IRQ_HANDLED;
+}
+
+static int rmi_f21_config(struct rmi_function *fn)
+{
+	struct f21_data *f21 = dev_get_drvdata(&fn->dev);
+	struct rmi_driver *drv = fn->rmi_dev->driver;
+
+	if (!f21)
+		return 0;
+
+	drv->set_irq_bits(fn->rmi_dev, fn->irq_mask);
+
+	return 0;
+}
+
+static int rmi_f21_initialize(struct rmi_function *fn, struct f21_data *f21)
+{
+	struct input_dev *input = f21->input;
+	unsigned int button = BTN_LEFT;
+
+	f21->key_code = button;
+	input_set_capability(input, EV_KEY, f21->key_code);
+	input->keycode = &(f21->key_code);
+	input->keycodesize = sizeof(f21->key_code);
+	input->keycodemax = RMI_f21_FORCEPAD_BUTTON_COUNT;
+
+	__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
+
+	return 0;
+}
+
+static int rmi_f21_probe(struct rmi_function *fn)
+{
+	struct rmi_device *rmi_dev = fn->rmi_dev;
+	struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
+	struct f21_data *f21;
+	int error;
+
+	if (!drv_data->input) {
+		dev_info(&fn->dev, "f21: no input device found, ignoring\n");
+		return -ENXIO;
+	}
+
+	f21 = devm_kzalloc(&fn->dev, sizeof(*f21), GFP_KERNEL);
+	if (!f21)
+		return -ENOMEM;
+
+	f21->input = drv_data->input;
+
+	error = rmi_f21_initialize(fn, f21);
+	if (error)
+		return error;
+
+	dev_set_drvdata(&fn->dev, f21);
+	return 0;
+}
+
+struct rmi_function_handler rmi_f21_handler = {
+	.driver = {
+		.name = "rmi4_f21",
+	},
+	.func = 0x21,
+	.probe = rmi_f21_probe,
+	.config = rmi_f21_config,
+	.attention = rmi_f21_attention,
+};
-- 
2.25.1


^ permalink raw reply related

* [PATCH] Input: elan_i2c - Implement inhibit/uninhibit functions.
From: jingle.wu @ 2023-07-03  1:34 UTC (permalink / raw)
  To: linux-kernel, linux-input, dmitry.torokhov
  Cc: phoenix, josh.chen, dave.wang, jingle.wu

Add inhibit/uninhibit functions.

Due to some devices, the touchpad will be interfered with by
the screen noise when lid close.
---
 drivers/input/mouse/elan_i2c_core.c | 207 ++++++++++++++++++++++++++++
 1 file changed, 207 insertions(+)

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 5f0d75a45c80..4ea57f4c7bd4 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -56,6 +56,7 @@ struct elan_tp_data {
 	struct input_dev	*input;
 	struct input_dev	*tp_input; /* trackpoint input node */
 	struct regulator	*vcc;
+	struct list_head list;	/* for list of devices needing input handler */
 
 	const struct elan_transport_ops *ops;
 
@@ -63,6 +64,11 @@ struct elan_tp_data {
 	struct completion	fw_completion;
 	bool			in_fw_update;
 
+	struct work_struct	lid_work;
+	bool			lid_switch;
+	int			lid_value;
+	bool			in_inhibit;
+
 	struct mutex		sysfs_mutex;
 
 	unsigned int		max_x;
@@ -96,6 +102,9 @@ struct elan_tp_data {
 	u32			quirks;		/* Various quirks */
 };
 
+static struct workqueue_struct *elan_mode_wq;
+static LIST_HEAD(elan_devices_with_lid_handler);
+
 static u32 elan_i2c_lookup_quirks(u16 ic_type, u16 product_id)
 {
 	static const struct {
@@ -329,6 +338,74 @@ static int elan_initialize(struct elan_tp_data *data, bool skip_reset)
 	return error;
 }
 
+static int elan_reactivate(struct elan_tp_data *data)
+{
+	struct device *dev = &data->client->dev;
+	int error;
+
+	error = elan_set_power(data, true);
+	if (error)
+		dev_err(dev, "failed to restore power: %d\n", error);
+
+	error = data->ops->sleep_control(data->client, false);
+	if (error) {
+		dev_err(dev,
+			"failed to wake device up: %d\n", error);
+		return error;
+	}
+
+	return error;
+}
+
+static int elan_inhibit(struct input_dev *input_dev)
+{
+	struct elan_tp_data *data = input_get_drvdata(input_dev);
+	struct i2c_client *client = data->client;
+	int error;
+
+	dev_dbg(&client->dev, "inhibiting\n");
+	/*
+	 * We are taking the mutex to make sure sysfs operations are
+	 * complete before we attempt to bring the device into low[er]
+	 * power mode.
+	 */
+	error = mutex_lock_interruptible(&data->sysfs_mutex);
+	if (error)
+		return error;
+
+	disable_irq(client->irq);
+
+	error = elan_set_power(data, false);
+	if (error)
+		enable_irq(client->irq);
+
+	data->in_inhibit = true;
+	mutex_unlock(&data->sysfs_mutex);
+
+	return error;
+}
+
+static int elan_uninhibit(struct input_dev *input_dev)
+{
+	struct elan_tp_data *data = input_get_drvdata(input_dev);
+	struct i2c_client *client = data->client;
+	int error;
+
+	dev_dbg(&client->dev, "uninhibiting\n");
+	error = mutex_lock_interruptible(&data->sysfs_mutex);
+	if (error)
+		return error;
+
+	error = elan_reactivate(data);
+	if (error == 0)
+		enable_irq(client->irq);
+
+	data->in_inhibit = false;
+	mutex_unlock(&data->sysfs_mutex);
+
+	return error;
+}
+
 static int elan_query_device_info(struct elan_tp_data *data)
 {
 	int error;
@@ -1187,6 +1264,124 @@ static void elan_disable_regulator(void *_data)
 	regulator_disable(data->vcc);
 }
 
+static void lid_work_handler(struct work_struct *work)
+{
+	struct elan_tp_data *data = container_of(work, struct elan_tp_data,
+					    lid_work);
+
+	if (data->lid_value)
+		elan_inhibit(data->input);
+	else
+		elan_uninhibit(data->input);
+
+}
+
+static void elan_input_lid_event(struct input_handle *handle, unsigned int type,
+			     unsigned int code, int value)
+{
+	struct elan_tp_data *data, *n;
+
+	if (type == EV_SW && code == SW_LID) {
+		list_for_each_entry_safe(data, n, &elan_devices_with_lid_handler, list) {
+			data->lid_value = value;
+			queue_work(elan_mode_wq, &data->lid_work);
+		}
+	}
+
+}
+
+struct elan_input_lid {
+	struct input_handle handle;
+};
+
+static int elan_input_lid_connect(struct input_handler *handler,
+				struct input_dev *dev,
+				const struct input_device_id *id)
+{
+	struct elan_input_lid *lid;
+	char *name;
+	int error;
+
+	lid = kzalloc(sizeof(*lid), GFP_KERNEL);
+	if (!lid)
+		return -ENOMEM;
+	name = kasprintf(GFP_KERNEL, "elan-i2c-lid-%s", dev_name(&dev->dev));
+	if (!name) {
+		error = -ENOMEM;
+		goto err_free_lid;
+	}
+	lid->handle.dev = dev;
+	lid->handle.handler = handler;
+	lid->handle.name = name;
+	lid->handle.private = lid;
+	error = input_register_handle(&lid->handle);
+	if (error)
+		goto err_free_name;
+	error = input_open_device(&lid->handle);
+	if (error)
+		goto err_unregister_handle;
+	return 0;
+err_unregister_handle:
+	input_unregister_handle(&lid->handle);
+err_free_name:
+	kfree(name);
+err_free_lid:
+	kfree(lid);
+	return error;
+}
+
+static void elan_input_lid_disconnect(struct input_handle *handle)
+{
+	struct elan_input_lid *lid = handle->private;
+
+	input_close_device(handle);
+	input_unregister_handle(handle);
+	kfree(handle->name);
+	kfree(lid);
+}
+
+static const struct input_device_id elan_input_lid_ids[] = {
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_SWBIT,
+		.evbit = { BIT_MASK(EV_SW) },
+		.swbit = { [BIT_WORD(SW_LID)] = BIT_MASK(SW_LID) },
+	},
+	{ },
+};
+
+static struct input_handler elan_input_lid_handler = {
+	.event =	elan_input_lid_event,
+	.connect =	elan_input_lid_connect,
+	.disconnect =	elan_input_lid_disconnect,
+	.name =		"elan-i2c-lid",
+	.id_table =	elan_input_lid_ids,
+};
+
+static int elan_create_lid_handler(struct elan_tp_data *data)
+{
+	int error = 0;
+
+	elan_mode_wq = create_singlethread_workqueue("elan-i2c-lid");
+	if (elan_mode_wq == NULL)
+		return -ENOMEM;
+	error = input_register_handler(&elan_input_lid_handler);
+	if (error)
+		goto remove_wq;
+
+	data->lid_switch = true;
+	INIT_LIST_HEAD(&data->list);
+	INIT_WORK(&data->lid_work, lid_work_handler);
+	list_add_tail(&data->list, &elan_devices_with_lid_handler);
+
+	return 0;
+
+remove_wq:
+	data->lid_switch = false;
+	destroy_workqueue(elan_mode_wq);
+	elan_mode_wq = NULL;
+	return error;
+}
+
 static int elan_probe(struct i2c_client *client)
 {
 	const struct elan_transport_ops *transport_ops;
@@ -1325,6 +1520,10 @@ static int elan_probe(struct i2c_client *client)
 		}
 	}
 
+	error = elan_create_lid_handler(data);
+	if (error)
+		dev_err(dev, "failed to create lid handler: %d\n", error);
+
 	return 0;
 }
 
@@ -1334,6 +1533,10 @@ static int elan_suspend(struct device *dev)
 	struct elan_tp_data *data = i2c_get_clientdata(client);
 	int ret;
 
+	/* Wait for switch on completion */
+	if (data->lid_switch)
+		flush_workqueue(elan_mode_wq);
+
 	/*
 	 * We are taking the mutex to make sure sysfs operations are
 	 * complete before we attempt to bring the device into low[er]
@@ -1371,6 +1574,10 @@ static int elan_resume(struct device *dev)
 	struct elan_tp_data *data = i2c_get_clientdata(client);
 	int error;
 
+	/* Wait for switch on completion */
+	if (data->lid_switch)
+		flush_workqueue(elan_mode_wq);
+
 	if (!device_may_wakeup(dev)) {
 		error = regulator_enable(data->vcc);
 		if (error) {
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v3 1/4] Input: ts-overlay - Add touchscreen overlay object handling
From: Javier Carrasco @ 2023-07-01 20:58 UTC (permalink / raw)
  To: Jeff LaBundy
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
	linux-input, devicetree
In-Reply-To: <ZJj5VcHvfMMWMRx8@nixie71>

Hi Jeff,

On 26.06.23 04:35, Jeff LaBundy wrote:
>> +
>> +		dev_info(dev, "Added button at (%u, %u), size %ux%u, code=%u\n",
>> +			 btn[j].shape.x_origin, btn[j].shape.y_origin,
>> +			 btn[j].shape.x_size, btn[j].shape.y_size, btn[j].key);
> 
> This seems like a dev_dbg() to me.
> 
>> +		j++;
>> +	}
>> +
>> +	return 0;
>> +
>> +button_prop_cleanup:
>> +	fwnode_handle_put(child_btn);
>> +	return rc;
>> +}
>> +
>> +void ts_overlay_set_button_caps(struct ts_overlay_map *map,
>> +				struct input_dev *dev)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < map->button_count; i++)
>> +		input_set_capability(dev, EV_KEY, map->buttons[i].key);
>> +}
>> +EXPORT_SYMBOL(ts_overlay_set_button_caps);
> 
> I don't see a need to expose this function and require participating drivers
> to call it; we should just have one over-arching function for processing the
> overlay(s), akin to touchscreen_parse_properties but for the button input
> device in case the driver separates the button and touchscreen input devices.
> 
> That one function would then be responsible for parsing the overlay(s) and
> calling input_set_capability() on each button.
> 
I just realized that I did not reply anything about this, even though
there is a reason why I exposed this function and now that I am working
on the v4, some clarification might be required.

After it was decided that this feature should work with two different
devices (a touchscreen and a keypad), I registered a keypad in the
st1232.c probe function if overlay buttons were defined. I did not
register the device inside the new functions because I thought that I
would be hiding too much magic from the driver.

Instead I provided a function to check if a keypad was defined and
another one to set the capabilities (the one we are discussing). The
driver could just set any parameters it wants before registering the
device and use this function within that process. The parsing is not
missing, it is carried out before and the programmer does not need to
know the key capabilities to call this function.

So the process is as follows:
1.- Map overlay objects if they are defined.
2.- If there is a keypad, set the device properties you want it to have
(name, etc). The event keys were already parsed and can be set with
touch_overlay_set_button_caps()
3.- Register the device whenever and under the circumstances you like.

That is the current implementation, not necessarily the best one or the
one the community would prefer.
If that is preferred, the mapping function could for example register
the keypad and return a pointer to the keyboard, inferring the device
properties from the "main" device that will be registered anyways (e.g.
using its name + "-keypad") or passing them as parameters, which might
look a bit artificial. In that case the key capabilities would be
automatically set and this function would not be exposed any more.

The question is if we would be missing flexibility when setting the
device properties prior its registration and if the participating
drivers want this to be done under the hood. My solution is the one I
found less intrusive for the driver (at the cost of doing the
registration itself), but if there are good reasons for a different
implementation, I will be alright with it. If not, the driver will
control the keypad registration and will use this function to set the
key caps.

Sorry for the late reply to this particular point and especially for the
long text. But a clarification here might save us from another
discussion later on :)

Best regards,
Javier Carrasco

^ permalink raw reply

* Re: [PATCH 0/2] Input: xpad - Additional controller support
From: Vicki Pfau @ 2023-07-01  2:38 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input; +Cc: Pavel Rojtberg, Lyude Paul
In-Reply-To: <20230607012812.379640-1-vi@endrift.com>

Hello,

On 6/6/23 18:28, Vicki Pfau wrote:
> This series includes two patches: one is a fixed resubmit of a previously
> reverted patch (first applied as db7220c48d8d), and the other is a simple
> addition to the VID table for Xbox One controllers.
> 
> Sam Lantinga (1):
>   Input: xpad - Add GameSir VID for Xbox One controllers
> 
> Vicki Pfau (1):
>   Input: xpad - fix support for some third-party controllers
> 
>  drivers/input/joystick/xpad.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 

This series has been radio silent for over three weeks. It includes an easy patch as well as a fixed version of the previously reverted patch I submitted. Is there anything else I need to do? I almost forgot about it, but would like to make sure it's addressed at some point.

Thanks

^ permalink raw reply

* Re: [PATCH] HID: i2c-hid: Block a rogue device on ASUS TUF A16
From: Limonciello, Mario @ 2023-06-30 19:55 UTC (permalink / raw)
  To: Friedrich Vock, linux-input, Natikar, Basavaraj,
	S-k, Shyam-sundar
  Cc: Jiri Kosina, Benjamin Tissoires
In-Reply-To: <0a9ba51e-0b07-cd9d-32fa-d99596c34183@gmx.de>

On 6/27/2023 09:38, Friedrich Vock wrote:
> Hi,
> 
> On 26.06.23 17:10, Limonciello, Mario wrote:
>>
>> On 6/18/2023 10:05 PM, Mario Limonciello wrote:
>>> On 6/15/23 07:41, Friedrich Vock wrote:
>>>> Hi,
>>>>
>>>> sorry for taking so long to reply.
>>>>
>>>> On 02.06.23 20:43, Limonciello, Mario wrote:
>>>>> + some AMD guys
>>>>>
>>>>> On 5/30/2023 10:40 AM, Friedrich Vock wrote:
>>>>>> On these laptops, there seems to be a device that, when probed by
>>>>>> i2c-hid, constantly sends bogus interrupts and interferes with the
>>>>>> keyboard controller. When the device is enabled, it takes the
>>>>>> keyboard
>>>>>> around 8 seconds to register that keys are being pressed or released.
>>>>>
>>>>> Do you know what interrupt is firing constantly?
>>>>> Presumably it is the GPIO controller master interrupt, right?
>>>>> And it's for GPIO 7 (guessed from acpidump on one of the bug
>>>>> reports).
>>>>>
>>>>> To confirm check /proc/interrupts.
>>>> Seems likely that you guessed correctly. The corresponsing line in
>>>> /proc/interrupts (with the interrupts counts omitted):
>>>> 71:   amd_gpio    7  ITE5570:00
>>>
>>> OK.
>> I had asked in the past for R/W everything output to compare to
>> /sys/kernel/debug/gpio.
>>
>> I've added more explicit instructions how to get this to
>> the kernel bugzilla 217336 – keyboard not working Asus TUF FA617NS
>> (kernel.org) <https://bugzilla.kernel.org/show_bug.cgi?id=217336#c126>
> Thanks for this. R/W everything didn't work for the other people with
> the same models, so I spent this day getting Windows and R/W everything
> running myself. I managed to make it run and left a comment with the
> results in that bugzilla report.
>>>
>>>>>
>>>>> If it's not obvious which GPIO is firing there is also a dynamic
>>>>> debug statement in pinctrl-amd.c that may be helpful to figure
>>>>> this out.
>>>>>
>>>>> I would also suspect in Windows this doesn't happen.  If possible
>>>>> can you confirm that? Check in Device Manager what driver is bound
>>>>> to this device. Is it "inbox" from Microsoft or is it an ASUS
>>>>> specific driver?
>>>>>
>>>>> I wonder if the GPIO controller got programmed differently in
>>>>> Windows for some reason. We may want to confirm the values for
>>>>> GPIO registers from /sys/kernel/debug/gpio in Linux against those
>>>>> that are programmed in Windows.
>>>>>
>>>>> This can be accomplished using R/W everything in Windows.
>>>>
>>>> Unfortunately I don't have Windows installed on this system. I know
>>>> some
>>>> people with the Ryzen 9 7940HS model which might, I'll ask them if they
>>>> can give me the configuration on Windows and Linux.
>>>
>>> OK, I suggest directing everyone over to the bugs I linked and we
>>> should gather the relevant GPIO controller register dumps from
>>> Windows and Linux on the same hardware there.
>>>
>>>>
>>>>>
>>>>>>
>>>>>> Nothing I tried seemed to make the device work, and preventing the
>>>>>> device from being probed doesn't seem to break any functionality of
>>>>>> the laptop.
>>>>>>
>>>>>> Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
>>>>>
>>>>> There are a few bug reports that popped up around this issue that
>>>>> should
>>>>> probably also be tagged.
>>>>>
>>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336
>>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217493
>>>>>
>>>>>> ---
>>>>>>   drivers/hid/i2c-hid/i2c-hid-core.c       |  5 +++
>>>>>>   drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 48
>>>>>> ++++++++++++++++++++++++
>>>>>>   drivers/hid/i2c-hid/i2c-hid.h            |  3 ++
>>>>>>   3 files changed, 56 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
>>>>>> b/drivers/hid/i2c-hid/i2c-hid-core.c
>>>>>> index efbba0465eef..5f0686d058df 100644
>>>>>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>>>>>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>>>>>> @@ -1035,6 +1035,11 @@ int i2c_hid_core_probe(struct i2c_client
>>>>>> *client, struct i2chid_ops *ops,
>>>>>>
>>>>>>       ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>>>>>>
>>>>>> +    if (i2c_hid_device_blocked(hid->vendor, hid->product)) {
>>>>>> +        ret = -ENODEV;
>>>>>> +        goto err_irq;
>>>>>> +    }
>>>>>> +
>>>>> The thing I worry about here is that an unserviced interrupt can
>>>>> prevent the
>>>>> hardware from going into proper low power states; particularly at
>>>>> runtime.
>>>>>
>>>>> I think we should better understand what's going on before going down
>>>>> this
>>>>> path of just ignoring it.
>>>> Yeah, I guess I should've searched more for a proper explanation/fix
>>>> before submitting hacks like this. Let's see if this can be fixed in a
>>>> cleaner manner than preemptively disabling parts of the system.
>>>
>>> Can you please have a try with linux-next or apply this series:
>>> https://lore.kernel.org/linux-gpio/20230421120625.3366-1-mario.limonciello@amd.com/
>>>
>>>
>>> That does change some of the configuration for the GPIO controller
>>> registers to align how windows is handling debouncing.
>>>
>>> I don't think it will change the outcome of your bug, but I'd love
>>> to be surprised.
>> Any updates for this?
> Tried this out today. You won't be surprised, it didn't change anything.
>>>
>>>>>>       ret = hid_add_device(hid);
>>>>>>       if (ret) {
>>>>>>           if (ret != -ENODEV)
>>>>>> diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>>>>>> b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>>>>>> index 210f17c3a0be..d2c2806b64b4 100644
>>>>>> --- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>>>>>> +++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>>>>>> @@ -440,6 +440,38 @@ static const struct dmi_system_id
>>>>>> i2c_hid_dmi_quirk_table[] = {
>>>>>>       { }    /* Terminate list */
>>>>>>   };
>>>>>>
>>>>>> +static const struct hid_device_id i2c_hid_blocked_ite_device = {
>>>>>> +    HID_DEVICE(BUS_I2C, HID_GROUP_GENERIC, USB_VENDOR_ID_ITE,
>>>>>> 0x8051)
>>>>>> +};
>>>>>> +
>>>>>> +/*
>>>>>> + * This list contains devices that can't be activated at all, for
>>>>>> example
>>>>>> + * because activating them breaks other important parts of the
>>>>>> system.
>>>>>> + */
>>>>>> +static const struct dmi_system_id i2c_hid_dmi_block_table[] = {
>>>>>> +    /*
>>>>>> +     * On ASUS TUF Gaming A16 laptops, there is a device that will
>>>>>> make the
>>>>>> +     * keyboard controller stop working correctly and flood the CPU
>>>>>> with bogus
>>>>>> +     * interrupts.
>>>>>> +     */
>>>>>> +    {
>>>>>> +        .ident = "ASUS TUF Gaming A16 (Ryzen 7 7735HS)",
>>>>>> +        .matches = {
>>>>>> +            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
>>>>>> INC."),
>>>>>> +            DMI_MATCH(DMI_PRODUCT_NAME, "FA617NS"),
>>>>>> +        },
>>>>>> +        .driver_data = (void *)&i2c_hid_blocked_ite_device,
>>>>>> +    },
>>>>>> +    {
>>>>>> +        .ident = "ASUS TUF Gaming A16 (Ryzen 9 7940HS)",
>>>>>> +        .matches = {
>>>>>> +            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
>>>>>> INC."),
>>>>>> +            DMI_MATCH(DMI_PRODUCT_NAME, "FA617XS"),
>>>>>> +        },
>>>>>> +        .driver_data = (void *)&i2c_hid_blocked_ite_device,
>>>>>> +    },
>>>>>> +    { }    /* Terminate list */
>>>>> If this *does* end up being the best solution, I think it's better
>>>>> to patch in the DMI to gpiolib-acpi.c similar to other quirks for
>>>>> floating
>>>>> GPIOs.  Example:
>>>>>
>>>>> https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib-acpi.c#L1654
>>>>>
>>>>>
>>>>>
>>>>>> +};
>>>>>>
>>>>>>   struct i2c_hid_desc *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t
>>>>>> *i2c_name)
>>>>>>   {
>>>>>> @@ -492,3 +524,19 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor,
>>>>>> const u16 product)
>>>>>>
>>>>>>       return quirks;
>>>>>>   }
>>>>>> +
>>>>>> +bool i2c_hid_device_blocked(const u16 vendor, const u16 product)
>>>>>> +{
>>>>>> +    const struct dmi_system_id *system_id =
>>>>>> +            dmi_first_match(i2c_hid_dmi_block_table);
>>>>>> +
>>>>>> +    if (system_id) {
>>>>>> +        const struct hid_device_id *device_id =
>>>>>> +                (struct hid_device_id *)(system_id->driver_data);
>>>>>> +
>>>>>> +        if (device_id && device_id->vendor == vendor &&
>>>>>> +            device_id->product == product)
>>>>>> +            return true;
>>>>>> +    }
>>>>>> +    return false;
>>>>>> +}
>>>>>> diff --git a/drivers/hid/i2c-hid/i2c-hid.h
>>>>>> b/drivers/hid/i2c-hid/i2c-hid.h
>>>>>> index 2c7b66d5caa0..e17bdd758f39 100644
>>>>>> --- a/drivers/hid/i2c-hid/i2c-hid.h
>>>>>> +++ b/drivers/hid/i2c-hid/i2c-hid.h
>>>>>> @@ -10,6 +10,7 @@ struct i2c_hid_desc
>>>>>> *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name);
>>>>>>   char *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
>>>>>>                              unsigned int *size);
>>>>>>   u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product);
>>>>>> +bool i2c_hid_device_blocked(const u16 vendor, const u16 product);
>>>>>>   #else
>>>>>>   static inline struct i2c_hid_desc
>>>>>> *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name)
>>>>>> @@ -19,6 +20,8 @@ static inline char
>>>>>> *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
>>>>>>   { return NULL; }
>>>>>>   static inline u32 i2c_hid_get_dmi_quirks(const u16 vendor, const
>>>>>> u16 product)
>>>>>>   { return 0; }
>>>>>> +static inline bool i2c_hid_device_blocked(const u16 vendor, const
>>>>>> u16 product)
>>>>>> +{ return false; }
>>>>>>   #endif
>>>>>>
>>>>>>   /**
>>>>>> -- 
>>>>>> 2.40.1
>>>>>>
>>>>>>
>>>
> Thanks,
> Friedrich

Friderich and some people on CC are already aware of this, but mostly 
for Benjamin and Jiri I wanted to let you know that the additional 
register fetching comparing Windows and Linux allowed me to come up with 
a proper root cause.

This series has been sent out to fix the issue properly.

https://lore.kernel.org/linux-gpio/20230630194716.6497-1-mario.limonciello@amd.com/

^ permalink raw reply

* Re: [PATCH v3 1/4] Input: ts-overlay - Add touchscreen overlay object handling
From: Jeff LaBundy @ 2023-06-30 17:30 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
	linux-input, devicetree
In-Reply-To: <feb8931d-45e0-e701-d8a7-0f5e3c108a84@wolfvision.net>

Hi Javier,

On Thu, Jun 29, 2023 at 09:53:11AM +0200, Javier Carrasco wrote:
> Hi Jeff,
> 
> On 29.06.23 05:29, Jeff LaBundy wrote:
> > Hi Javier,
> > 
> > On Wed, Jun 28, 2023 at 08:44:51AM +0200, Javier Carrasco wrote:
> > 
> > [...]
> > 
> >>>>>> +static const char *const ts_overlay_names[] = {
> >>>>>> +	[TOUCHSCREEN] = "overlay-touchscreen",
> >>>>>
> >>>>> I'm a little confused why we need new code for this particular function; it's
> >>>>> what touchscreen-min-x/y and touchscreen-size-x/y were meant to define. Why
> >>>>> can't we keep using those?
> >>>>>
> >>>> According to the bindings, touchscreen-min-x/y define the minimum
> >>>> reported values, but the overlay-touchscreen is actually setting a new
> >>>> origin. Therefore I might be misusing those properties. On the other
> >>>> hand touchscreen-size-x/y would make more sense, but I also considered
> >>>> the case where someone would like to describe the real size of the
> >>>> touchscreen outside of the overlay node as well as the clipped size
> >>>> inside the node. In that case using the same property twice would be
> >>>> confusing.
> >>>> So in the end I thought that the origin/size properties are more precise
> >>>> and applicable for all objects and not only the overlay touchscreen.
> >>>> These properties are needed for the buttons anyways and in the future
> >>>> more overlay would use the same properties.
> >>>
> >>> Ah, I understand now. touchscreen-min-x/y define the lower limits of the axes
> >>> reported to input but they don't move the origin. I'm aligned with the reason
> >>> to introduce this function.
> >>>
> >>> This does beg the question as to whether we need two separate types of children
> >>> and related parsing code. Can we not simply have one overlay definition, and
> >>> make the decision as to whether we are dealing with a border or virtual button
> >>> based on whether 'linux,code' is present?
> >>>
> >> A single overlay definition would be possible, but in case more objects
> >> are added in the future, looking for single properties and then deciding
> >> what object it is might get messy pretty fast. You could end up needing
> >> a decision tree and the definition in the DT would get more complex.
> >>
> >> Now the decision tree is straightforward (linux,code -> button), but
> >> that might not always be the case. In the current implementation there
> >> are well-defined objects and adding a new one will never affect the
> >> parsing of the rest.
> >> Therefore I would like to keep it more readable and easily extendable.
> > 
> > As a potential customer of this feature, I'm ultimately looking to describe
> > the hardware as succinctly as possible. Currently we have two overlay types,
> > a border and button(s). The former will never have linux,code defined, while
> > the latter will. From my naive perspective, it seems redundant to define the
> > overlay types differently when their properties imply the difference already.
> > 
> > Ultimately it seems we are simply dealing with generic "segments" scattered
> > throughout a larger touch surface. These segments start to look something
> > like the following:
> > 
> > struct touch_segment {
> > 	unsigned int x_origin;
> > 	unsigned int y_origin;
> > 	unsigned int x_size;
> > 	unsigned int y_size;
> > 	unsigned int code;
> > };
> > 
> > You then have one exported function akin to touchscreen_parse_properties() that
> > simply walks the parent device looking for children named "touch-segment-0",
> > "touch-segment-1", etc. and parses the five properties, with the fifth (keycode)
> > being optional.
> > 
> > And then, you have one last exported function akin to touchscreen_report_pos()
> > that processes the touch coordinates. If the coordinates are in a given segment
> > and segment->code == KEY_RESERVED (i.e. linux,code was never given), then this
> > function simply passes the shifted coordinates to touchscreen_report_pos().
> > 
> > If however segment->code != KEY_RESERVED, it calls input_report_key() based on
> > whether the coordinates are within the segment. If this simplified solution
> > shrinks the code enough, it may even make sense to keep it in touchscreen.c
> > which this new feature is so tightly coupled to anyway.
> > 
> > I'm sure the devil is in the details however, and I understand the value in
> > future-proofing. Can you help me understand a potential future case where this
> > simplified view would break, and the existing definitions would be better?
> > 
> > Kind regards,
> > Jeff LaBundy
> 
> I agree that your approach would reduce the code and then moving this
> feature to touchscreen.c would be reasonable. So if in the end that is
> the desired solution, I will go for it. But there are some points where
> I think the bit of extra code would be worth it.
> 
> From a DT perspective, I can imagine some scenarios where a bunch of
> segments scattered around would be messy. An example would be a keypad
> with let's say N=9 buttons. It could be described easily with a buttons
> node and the keys inside. Understanding what the node describes would be
> straightforward as well, let alone N being much bigger.
> You could argue that the buttons node could have segments inside instead
> of buttons, but in the case where a cropped touchscreen is also
> described, you would end up with a segment outside the buttons node and
> the rest inside. That would reduce the parsing savings. Some labeling
> would help in that case, but that would be not as clear as the current
> implementation.

If we consider your nicely drawn hybrid example with two buttons and a border,
the parent would look something like the following:

touchscreen {
	compatible = "vendor,model";

	overlay-touchscreen {
		x-origin = ...;
		y-origin = ...;
		x-size = ...;
		y-size = ...;
	};

	overlay-buttons {
		button-0 {
			x-origin = ...;
			y-origin = ...;
			x-size = ...;
			y-size = ...;
			linux,code = <KEY_POWER>;
		};

		button-1 {
			x-origin = ...;
			y-origin = ...;
			x-size = ...;
			y-size = ...;
			linux,code = <KEY_INFO>;
		};
	};
};

Is that correct? My first impression was that it seems unnecessary to group
the button-0 and button-1 children under their own 'overlay-buttons' parent,
because there is nothing else inside 'overlay-buttons'. Stated another way,
it seems we are drawing an unnecessary imaginary line around the buttons. It
would be especially confusing for the case where the buttons are scattered
throughout the touch surface and have no logical or physical relationship.

My second impression is that 'overlay-touchscreen' and button-N are nearly
identical save for the 'linux,code' property that can be optional, which makes
it seem like these two should really just be the same node definition. If we
follow my suggestion, the example would collapse as follows:

touchscreen {
        compatible = "vendor,model";

	touch-segment-0 {
		/*
		 * this is the small subset of the entire surface that reports
		 * raw coordinates
		 */
		x-origin = ...;
		y-origin = ...;
		x-size = ...;
		y-size = ...;
	};

	touch-segment-1 {
		x-origin = ...;
		y-origin = ...;
		x-size = ...;
		y-size = ...;
		linux,code = <KEY_POWER>;
	};

	touch-segment-2 {
		x-origin = ...;
		y-origin = ...;
		x-size = ...;
		y-size = ...;
		linux,code = <KEY_INFO>;
	};
};

To me, this seems like a more authentic representation of a monolithic touch
surface. I can't imagine there is not some code savings and simplification to
be had if touch-segment-N can always assume it is a direct descendant of the
parent. In the present implementation, a common property such as 'x-size' may
be one or two generations below the parent depending on the context, which
the code has to keep track of.

> There is another point that I will just touch upon because I have no
> experience in the matter. I have seen that some keys use the
> 'linux,input-type' property to define themselves as keys, switches, etc.
> If that property or any other that I do not know is necessary for other
> implementations, the button object will cover them better than a generic
> segment where half of the properties would be meaningless in some
> scenarios. Buttons/keys are so ubiquitous that a dedicated object for
> them does not look that bad imho.

I think we would simply adopt the same rule I have proposed for 'linux,code',
that is, make these future properties optional and simply omit them in the
case of what is currently defined as 'overlay-touchscreen'. For something
like 'linux,input-type', you would simply declare it as dependening upon
'linux,code' in the binding.

That being said, 'linux,input-type' would not make sense here because we are
simply defining how to interpret momentary touch (touch coordinate or press/
release), so we would never have a switch here. However, nothing in either of
our proposals would prevent it from being used.

> But as I said, I do not want to make a strong statement here because I
> have seen that you maintain several bindings where this properties are
> present and I am not the right person to explain that to you... or
> actually anyone else out there :)

I personally feel my proposal is simpler, but I do not feel strongly about
it. There is nothing functionally incorrect about yours; it merely struck me
as placing some unnecessary burden on the DT author. From my perspective,
"less code is the best code" and we can always add complexity later as unique
use-cases arise.

Since we agree on the majority of the suggestions for v4, maybe a compromise
is to start on those for now, and keep my suggestion in mind as you work
through the non-controversial changes. If you still feel strongly about your
existing structure after that work is complete, then let us keep it.

> 
> Talking about the code itself, having a structure for buttons is handy
> because you can keep track of the button status (e.g. pressed) and in
> the end it is just a child of the base shape that is used for the
> overlay touchscreen. The same applies to any function that handles
> buttons: they just wrap around the shape functions and add the
> button-specific management. So if the parsing is taken aside, the code
> does not get much savings from that side and it is again much more
> readable and comprehensible.

Your module should not be storing button state; that is the job of the input
core. Instead, your module should only be reporting state; the input core
then decides if the state has changed and acts accordingly.

> 
> Thank you for your efforts to improve these patches and the constructive
> discussion.
> 
> Best regards,
> Javier Carrasco

Kind regards,
Jeff LaBundy

^ permalink raw reply

* Setting Synaptics Intertouch to 1
From: Lex van Teeffelen @ 2023-06-30 14:18 UTC (permalink / raw)
  To: linux-input@vger.kernel.org

Dear linux-input,

I was having some issues with my touchpad on my Lenovo Thinkpad T15 (gen 2), after reading some log messages, I noticed something among the lines of:

```
psmouse serio2: synaptics: Your touchpad says it can support a different bus. If i2c-hid and hid-rmi are not used, you might want to try setting psmouse.synaptics_intertouch to 1 and report this to linux-input@vger.kernel.org.
```

Both of the mentioned modules were not loaded by default.
So I decided to set the intertouch option in the grub config, and after a swift reboot I verified if the intertouch value had indeed changed:

```
cat /sys/module/psmouse/parameters/synaptics_intertouch 
1
```

The touchpad is now noticeably more responsive and less jittery, hence this email to report my findings.

As for the details:

- Hardware: Lenovo Thinkpad T15 (Gen 2)
- Distribution: Ubuntu 22.04.2 LTS
- Kernel: 5.19.0-45-generic
- Touchpad:

```
  *-input:2
       product: Synaptics TM3471-030
       physical id: 4
       logical name: input11
       logical name: /dev/input/event9
       logical name: /dev/input/mouse0
       capabilities: rmi
```


If there is more information required, feel free to ask, I am happy to help!


Kind regards,

L. van Teeffelen

^ permalink raw reply

* Re: [PATCH] HID: steelseries: Add support for Arctis 1 XBox
From: Bastien Nocera @ 2023-06-30 13:13 UTC (permalink / raw)
  To: kernel test robot, linux-input
  Cc: oe-kbuild-all, linux-kernel, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <202306300419.dmlkSoqD-lkp@intel.com>

On Fri, 2023-06-30 at 04:32 +0800, kernel test robot wrote:
> Hi Bastien,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on hid/for-next]
> [also build test ERROR on linus/master v6.4 next-20230629]
> [If your patch is applied to the wrong git tree, kindly drop us a
> note.
> And when submitting patch, we suggest to use '--base' as documented
> in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:   
> https://github.com/intel-lab-lkp/linux/commits/Bastien-Nocera/HID-steelseries-Add-support-for-Arctis-1-XBox/20230630-012252
> base:  
> https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
> patch link:   
> https://lore.kernel.org/r/20230629172041.3771-1-hadess%40hadess.net
> patch subject: [PATCH] HID: steelseries: Add support for Arctis 1
> XBox
> config: arc-randconfig-r035-20230629
> (https://download.01.org/0day-ci/archive/20230630/202306300419.dmlkSo
> qD-lkp@intel.com/config)
> compiler: arceb-elf-gcc (GCC) 12.3.0
> reproduce:
> (https://download.01.org/0day-ci/archive/20230630/202306300419.dmlkSo
> qD-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new
> version of
> the same patch/commit), kindly add following tags
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes:
> > https://lore.kernel.org/oe-kbuild-all/202306300419.dmlkSoqD-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    arceb-elf-ld: drivers/hid/hid-steelseries.o: in function
> `steelseries_headset_raw_event':
> > > hid-steelseries.c:(.text+0x344): undefined reference to
> > > `usb_set_wireless_status'
> > > arceb-elf-ld: hid-steelseries.c:(.text+0x344): undefined
> > > reference to `usb_set_wireless_status'
>    arceb-elf-ld: drivers/hid/hid-steelseries.o: in function
> `steelseries_probe':
>    hid-steelseries.c:(.text+0x790): undefined reference to
> `usb_set_wireless_status'
>    arceb-elf-ld: hid-steelseries.c:(.text+0x790): undefined reference
> to `usb_set_wireless_status'

The KConfig was missing a dependency on USB (and not USB HID...), fixed
in v3.

^ permalink raw reply

* [PATCH v3] HID: steelseries: Add support for Arctis 1 XBox
From: Bastien Nocera @ 2023-06-30 13:11 UTC (permalink / raw)
  To: linux-input; +Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires

Add support for the Steelseries Arctis 1 XBox headset. This driver
will export the battery information from the headset, as well as the
"wireless_status" property.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
v3:
- Dependency is on USB not USB_HID

v2:
- Fix missing USB dependency
- Fix config option description

 drivers/hid/Kconfig           |   6 +-
 drivers/hid/hid-steelseries.c | 307 ++++++++++++++++++++++++++++++++--
 2 files changed, 296 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ce012f83253..afe1c6070602 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1048,9 +1048,11 @@ config STEAM_FF
 	Deck.
 
 config HID_STEELSERIES
-	tristate "Steelseries SRW-S1 steering wheel support"
+	tristate "Steelseries devices support"
+	depends on USB
 	help
-	Support for Steelseries SRW-S1 steering wheel
+	Support for Steelseries SRW-S1 steering wheel, and the Steelseries
+	Arctis 1 Wireless for XBox headset.
 
 config HID_SUNPLUS
 	tristate "Sunplus wireless desktop"
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index aae3afc4107a..a9300a4244aa 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *  HID driver for Steelseries SRW-S1
+ *  HID driver for Steelseries devices
  *
  *  Copyright (c) 2013 Simon Wood
+ *  Copyright (c) 2023 Bastien Nocera
  */
 
 /*
@@ -11,10 +12,28 @@
 #include <linux/device.h>
 #include <linux/hid.h>
 #include <linux/module.h>
+#include <linux/usb.h>
 #include <linux/leds.h>
 
 #include "hid-ids.h"
 
+#define STEELSERIES_SRWS1		BIT(0)
+#define STEELSERIES_ARCTIS_1		BIT(1)
+
+struct steelseries_device {
+	struct hid_device *hdev;
+	unsigned long quirks;
+
+	struct delayed_work battery_work;
+	spinlock_t lock;
+	bool removed;
+
+	struct power_supply_desc battery_desc;
+	struct power_supply *battery;
+	uint8_t battery_capacity;
+	bool headset_connected;
+};
+
 #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
     (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
 #define SRWS1_NUMBER_LEDS 15
@@ -353,9 +372,208 @@ static void steelseries_srws1_remove(struct hid_device *hdev)
 }
 #endif
 
+#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS	3000
+
+#define ARCTIS_1_BATTERY_RESPONSE_LEN		8
+
+static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
+{
+	u8 *write_buf;
+	int ret;
+	char battery_request[2] = { 0x06, 0x12 };
+
+	/* Request battery information */
+	write_buf = kmemdup(battery_request, sizeof(battery_request), GFP_KERNEL);
+	if (!write_buf)
+		return -ENOMEM;
+
+	ret = hid_hw_raw_request(hdev, battery_request[0],
+				 write_buf, sizeof(battery_request),
+				 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+	if (ret < sizeof(battery_request)) {
+		hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
+		ret = -ENODATA;
+	}
+	kfree(write_buf);
+	return ret;
+}
+
+static void steelseries_headset_fetch_battery(struct hid_device *hdev)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	int ret = 0;
+
+	if (sd->quirks & STEELSERIES_ARCTIS_1)
+		ret = steelseries_headset_arctis_1_fetch_battery(hdev);
+
+	if (ret < 0)
+		hid_dbg(hdev,
+			"Battery query failed (err: %d)\n", ret);
+}
+
+static void steelseries_headset_battery_timer_tick(struct work_struct *work)
+{
+	struct steelseries_device *sd = container_of(work,
+		struct steelseries_device, battery_work.work);
+	struct hid_device *hdev = sd->hdev;
+
+	steelseries_headset_fetch_battery(hdev);
+}
+
+static int steelseries_headset_battery_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct steelseries_device *sd = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = sd->headset_connected ?
+			POWER_SUPPLY_STATUS_DISCHARGING :
+			POWER_SUPPLY_STATUS_UNKNOWN;
+		break;
+	case POWER_SUPPLY_PROP_SCOPE:
+		val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = sd->battery_capacity;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+static void
+steelseries_headset_set_wireless_status(struct hid_device *hdev,
+					bool connected)
+{
+	struct usb_interface *intf;
+
+	intf = to_usb_interface(hdev->dev.parent);
+	usb_set_wireless_status(intf, connected ?
+				USB_WIRELESS_STATUS_CONNECTED :
+				USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
+static enum power_supply_property steelseries_headset_battery_props[] = {
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_SCOPE,
+	POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steelseries_headset_battery_register(struct steelseries_device *sd)
+{
+	static atomic_t battery_no = ATOMIC_INIT(0);
+	struct power_supply_config battery_cfg = { .drv_data = sd, };
+	unsigned long n;
+	int ret;
+
+	sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+	sd->battery_desc.properties = steelseries_headset_battery_props;
+	sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
+	sd->battery_desc.get_property = steelseries_headset_battery_get_property;
+	sd->battery_desc.use_for_apm = 0;
+	n = atomic_inc_return(&battery_no) - 1;
+	sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
+						    "steelseries_headset_battery_%ld", n);
+	if (!sd->battery_desc.name)
+		return -ENOMEM;
+
+	/* avoid the warning of 0% battery while waiting for the first info */
+	steelseries_headset_set_wireless_status(sd->hdev, false);
+	sd->battery_capacity = 100;
+
+	sd->battery = devm_power_supply_register(&sd->hdev->dev,
+			&sd->battery_desc, &battery_cfg);
+	if (IS_ERR(sd->battery)) {
+		ret = PTR_ERR(sd->battery);
+		hid_err(sd->hdev,
+				"%s:power_supply_register failed with error %d\n",
+				__func__, ret);
+		return ret;
+	}
+	power_supply_powers(sd->battery, &sd->hdev->dev);
+
+	INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
+	steelseries_headset_fetch_battery(sd->hdev);
+
+	return 0;
+}
+
+static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	struct steelseries_device *sd;
+	int ret;
+
+	sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
+	if (!sd)
+		return -ENOMEM;
+	hid_set_drvdata(hdev, sd);
+	sd->hdev = hdev;
+	sd->quirks = id->driver_data;
+
+	if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+		return steelseries_srws1_probe(hdev, id);
+#else
+		return -ENODEV;
+#endif
+	}
+
+	ret = hid_parse(hdev);
+	if (ret)
+		return ret;
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (ret)
+		return ret;
+
+	if (steelseries_headset_battery_register(sd) < 0)
+		hid_err(sd->hdev,
+			"Failed to register battery for headset\n");
+
+	spin_lock_init(&sd->lock);
+
+	return ret;
+}
+
+static void steelseries_remove(struct hid_device *hdev)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	unsigned long flags;
+
+	if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+		steelseries_srws1_remove(hdev);
+#endif
+		return;
+	}
+
+	spin_lock_irqsave(&sd->lock, flags);
+	sd->removed = true;
+	spin_unlock_irqrestore(&sd->lock, flags);
+
+	cancel_delayed_work_sync(&sd->battery_work);
+
+	hid_hw_stop(hdev);
+}
+
 static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
 {
+	if (hdev->vendor != USB_VENDOR_ID_STEELSERIES ||
+	    hdev->product != USB_DEVICE_ID_STEELSERIES_SRWS1)
+		return rdesc;
+
 	if (*rsize >= 115 && rdesc[11] == 0x02 && rdesc[13] == 0xc8
 			&& rdesc[29] == 0xbb && rdesc[40] == 0xc5) {
 		hid_info(hdev, "Fixing up Steelseries SRW-S1 report descriptor\n");
@@ -365,22 +583,81 @@ static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc
 	return rdesc;
 }
 
-static const struct hid_device_id steelseries_srws1_devices[] = {
-	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
+static int steelseries_headset_raw_event(struct hid_device *hdev,
+					struct hid_report *report, u8 *read_buf,
+					int size)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	int capacity = sd->battery_capacity;
+	bool connected = sd->headset_connected;
+	unsigned long flags;
+
+	/* Not a headset */
+	if (sd->quirks & STEELSERIES_SRWS1)
+		return 0;
+
+	if (sd->quirks & STEELSERIES_ARCTIS_1) {
+		hid_dbg(sd->hdev,
+			"Parsing raw event for Arctis 1 headset (len: %d)\n", size);
+		if (size < 8)
+			return 0;
+		if (read_buf[2] == 0x01) {
+			connected = false;
+			capacity = 100;
+		} else {
+			connected = true;
+			capacity = read_buf[3];
+		}
+	}
+
+	if (connected != sd->headset_connected) {
+		hid_dbg(sd->hdev,
+			"Connected status changed from %sconnected to %sconnected\n",
+			sd->headset_connected ? "" : "not ",
+			connected ? "" : "not ");
+		sd->headset_connected = connected;
+		steelseries_headset_set_wireless_status(hdev, connected);
+	}
+
+	if (capacity != sd->battery_capacity) {
+		hid_dbg(sd->hdev,
+			"Battery capacity changed from %d%% to %d%%\n",
+			sd->battery_capacity, capacity);
+		sd->battery_capacity = capacity;
+		power_supply_changed(sd->battery);
+	}
+
+	spin_lock_irqsave(&sd->lock, flags);
+	if (!sd->removed)
+		schedule_delayed_work(&sd->battery_work,
+				msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
+	spin_unlock_irqrestore(&sd->lock, flags);
+
+	return 0;
+}
+
+static const struct hid_device_id steelseries_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
+	  .driver_data = STEELSERIES_SRWS1 },
+
+	{ /* SteelSeries Arctis 1 Wireless for XBox */
+	  HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
+	.driver_data = STEELSERIES_ARCTIS_1 },
+
 	{ }
 };
-MODULE_DEVICE_TABLE(hid, steelseries_srws1_devices);
-
-static struct hid_driver steelseries_srws1_driver = {
-	.name = "steelseries_srws1",
-	.id_table = steelseries_srws1_devices,
-#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
-    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
-	.probe = steelseries_srws1_probe,
-	.remove = steelseries_srws1_remove,
-#endif
-	.report_fixup = steelseries_srws1_report_fixup
+MODULE_DEVICE_TABLE(hid, steelseries_devices);
+
+static struct hid_driver steelseries_driver = {
+	.name = "steelseries",
+	.id_table = steelseries_devices,
+	.probe = steelseries_probe,
+	.remove = steelseries_remove,
+	.report_fixup = steelseries_srws1_report_fixup,
+	.raw_event = steelseries_headset_raw_event,
 };
 
-module_hid_driver(steelseries_srws1_driver);
+module_hid_driver(steelseries_driver);
 MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_AUTHOR("Simon Wood <simon@mungewell.org>");
-- 
2.41.0


^ permalink raw reply related

* [PATCH v2] HID: steelseries: Add support for Arctis 1 XBox
From: Bastien Nocera @ 2023-06-30 13:08 UTC (permalink / raw)
  To: linux-input; +Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires

Add support for the Steelseries Arctis 1 XBox headset. This driver
will export the battery information from the headset, as well as the
"wireless_status" property.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
v2:
- Fix missing USB dependency
- Fix config option description

 drivers/hid/Kconfig           |   6 +-
 drivers/hid/hid-steelseries.c | 307 ++++++++++++++++++++++++++++++++--
 2 files changed, 296 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ce012f83253..1bc99b33329f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1048,9 +1048,11 @@ config STEAM_FF
 	Deck.
 
 config HID_STEELSERIES
-	tristate "Steelseries SRW-S1 steering wheel support"
+	tristate "Steelseries devices support"
+	depends on USB_HID
 	help
-	Support for Steelseries SRW-S1 steering wheel
+	Support for Steelseries SRW-S1 steering wheel, and the Steelseries
+	Arctis 1 Wireless for XBox headset.
 
 config HID_SUNPLUS
 	tristate "Sunplus wireless desktop"
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index aae3afc4107a..a9300a4244aa 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *  HID driver for Steelseries SRW-S1
+ *  HID driver for Steelseries devices
  *
  *  Copyright (c) 2013 Simon Wood
+ *  Copyright (c) 2023 Bastien Nocera
  */
 
 /*
@@ -11,10 +12,28 @@
 #include <linux/device.h>
 #include <linux/hid.h>
 #include <linux/module.h>
+#include <linux/usb.h>
 #include <linux/leds.h>
 
 #include "hid-ids.h"
 
+#define STEELSERIES_SRWS1		BIT(0)
+#define STEELSERIES_ARCTIS_1		BIT(1)
+
+struct steelseries_device {
+	struct hid_device *hdev;
+	unsigned long quirks;
+
+	struct delayed_work battery_work;
+	spinlock_t lock;
+	bool removed;
+
+	struct power_supply_desc battery_desc;
+	struct power_supply *battery;
+	uint8_t battery_capacity;
+	bool headset_connected;
+};
+
 #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
     (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
 #define SRWS1_NUMBER_LEDS 15
@@ -353,9 +372,208 @@ static void steelseries_srws1_remove(struct hid_device *hdev)
 }
 #endif
 
+#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS	3000
+
+#define ARCTIS_1_BATTERY_RESPONSE_LEN		8
+
+static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
+{
+	u8 *write_buf;
+	int ret;
+	char battery_request[2] = { 0x06, 0x12 };
+
+	/* Request battery information */
+	write_buf = kmemdup(battery_request, sizeof(battery_request), GFP_KERNEL);
+	if (!write_buf)
+		return -ENOMEM;
+
+	ret = hid_hw_raw_request(hdev, battery_request[0],
+				 write_buf, sizeof(battery_request),
+				 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+	if (ret < sizeof(battery_request)) {
+		hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
+		ret = -ENODATA;
+	}
+	kfree(write_buf);
+	return ret;
+}
+
+static void steelseries_headset_fetch_battery(struct hid_device *hdev)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	int ret = 0;
+
+	if (sd->quirks & STEELSERIES_ARCTIS_1)
+		ret = steelseries_headset_arctis_1_fetch_battery(hdev);
+
+	if (ret < 0)
+		hid_dbg(hdev,
+			"Battery query failed (err: %d)\n", ret);
+}
+
+static void steelseries_headset_battery_timer_tick(struct work_struct *work)
+{
+	struct steelseries_device *sd = container_of(work,
+		struct steelseries_device, battery_work.work);
+	struct hid_device *hdev = sd->hdev;
+
+	steelseries_headset_fetch_battery(hdev);
+}
+
+static int steelseries_headset_battery_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct steelseries_device *sd = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = sd->headset_connected ?
+			POWER_SUPPLY_STATUS_DISCHARGING :
+			POWER_SUPPLY_STATUS_UNKNOWN;
+		break;
+	case POWER_SUPPLY_PROP_SCOPE:
+		val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = sd->battery_capacity;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+static void
+steelseries_headset_set_wireless_status(struct hid_device *hdev,
+					bool connected)
+{
+	struct usb_interface *intf;
+
+	intf = to_usb_interface(hdev->dev.parent);
+	usb_set_wireless_status(intf, connected ?
+				USB_WIRELESS_STATUS_CONNECTED :
+				USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
+static enum power_supply_property steelseries_headset_battery_props[] = {
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_SCOPE,
+	POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steelseries_headset_battery_register(struct steelseries_device *sd)
+{
+	static atomic_t battery_no = ATOMIC_INIT(0);
+	struct power_supply_config battery_cfg = { .drv_data = sd, };
+	unsigned long n;
+	int ret;
+
+	sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+	sd->battery_desc.properties = steelseries_headset_battery_props;
+	sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
+	sd->battery_desc.get_property = steelseries_headset_battery_get_property;
+	sd->battery_desc.use_for_apm = 0;
+	n = atomic_inc_return(&battery_no) - 1;
+	sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
+						    "steelseries_headset_battery_%ld", n);
+	if (!sd->battery_desc.name)
+		return -ENOMEM;
+
+	/* avoid the warning of 0% battery while waiting for the first info */
+	steelseries_headset_set_wireless_status(sd->hdev, false);
+	sd->battery_capacity = 100;
+
+	sd->battery = devm_power_supply_register(&sd->hdev->dev,
+			&sd->battery_desc, &battery_cfg);
+	if (IS_ERR(sd->battery)) {
+		ret = PTR_ERR(sd->battery);
+		hid_err(sd->hdev,
+				"%s:power_supply_register failed with error %d\n",
+				__func__, ret);
+		return ret;
+	}
+	power_supply_powers(sd->battery, &sd->hdev->dev);
+
+	INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
+	steelseries_headset_fetch_battery(sd->hdev);
+
+	return 0;
+}
+
+static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	struct steelseries_device *sd;
+	int ret;
+
+	sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
+	if (!sd)
+		return -ENOMEM;
+	hid_set_drvdata(hdev, sd);
+	sd->hdev = hdev;
+	sd->quirks = id->driver_data;
+
+	if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+		return steelseries_srws1_probe(hdev, id);
+#else
+		return -ENODEV;
+#endif
+	}
+
+	ret = hid_parse(hdev);
+	if (ret)
+		return ret;
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (ret)
+		return ret;
+
+	if (steelseries_headset_battery_register(sd) < 0)
+		hid_err(sd->hdev,
+			"Failed to register battery for headset\n");
+
+	spin_lock_init(&sd->lock);
+
+	return ret;
+}
+
+static void steelseries_remove(struct hid_device *hdev)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	unsigned long flags;
+
+	if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+		steelseries_srws1_remove(hdev);
+#endif
+		return;
+	}
+
+	spin_lock_irqsave(&sd->lock, flags);
+	sd->removed = true;
+	spin_unlock_irqrestore(&sd->lock, flags);
+
+	cancel_delayed_work_sync(&sd->battery_work);
+
+	hid_hw_stop(hdev);
+}
+
 static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
 {
+	if (hdev->vendor != USB_VENDOR_ID_STEELSERIES ||
+	    hdev->product != USB_DEVICE_ID_STEELSERIES_SRWS1)
+		return rdesc;
+
 	if (*rsize >= 115 && rdesc[11] == 0x02 && rdesc[13] == 0xc8
 			&& rdesc[29] == 0xbb && rdesc[40] == 0xc5) {
 		hid_info(hdev, "Fixing up Steelseries SRW-S1 report descriptor\n");
@@ -365,22 +583,81 @@ static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc
 	return rdesc;
 }
 
-static const struct hid_device_id steelseries_srws1_devices[] = {
-	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
+static int steelseries_headset_raw_event(struct hid_device *hdev,
+					struct hid_report *report, u8 *read_buf,
+					int size)
+{
+	struct steelseries_device *sd = hid_get_drvdata(hdev);
+	int capacity = sd->battery_capacity;
+	bool connected = sd->headset_connected;
+	unsigned long flags;
+
+	/* Not a headset */
+	if (sd->quirks & STEELSERIES_SRWS1)
+		return 0;
+
+	if (sd->quirks & STEELSERIES_ARCTIS_1) {
+		hid_dbg(sd->hdev,
+			"Parsing raw event for Arctis 1 headset (len: %d)\n", size);
+		if (size < 8)
+			return 0;
+		if (read_buf[2] == 0x01) {
+			connected = false;
+			capacity = 100;
+		} else {
+			connected = true;
+			capacity = read_buf[3];
+		}
+	}
+
+	if (connected != sd->headset_connected) {
+		hid_dbg(sd->hdev,
+			"Connected status changed from %sconnected to %sconnected\n",
+			sd->headset_connected ? "" : "not ",
+			connected ? "" : "not ");
+		sd->headset_connected = connected;
+		steelseries_headset_set_wireless_status(hdev, connected);
+	}
+
+	if (capacity != sd->battery_capacity) {
+		hid_dbg(sd->hdev,
+			"Battery capacity changed from %d%% to %d%%\n",
+			sd->battery_capacity, capacity);
+		sd->battery_capacity = capacity;
+		power_supply_changed(sd->battery);
+	}
+
+	spin_lock_irqsave(&sd->lock, flags);
+	if (!sd->removed)
+		schedule_delayed_work(&sd->battery_work,
+				msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
+	spin_unlock_irqrestore(&sd->lock, flags);
+
+	return 0;
+}
+
+static const struct hid_device_id steelseries_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
+	  .driver_data = STEELSERIES_SRWS1 },
+
+	{ /* SteelSeries Arctis 1 Wireless for XBox */
+	  HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
+	.driver_data = STEELSERIES_ARCTIS_1 },
+
 	{ }
 };
-MODULE_DEVICE_TABLE(hid, steelseries_srws1_devices);
-
-static struct hid_driver steelseries_srws1_driver = {
-	.name = "steelseries_srws1",
-	.id_table = steelseries_srws1_devices,
-#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
-    (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
-	.probe = steelseries_srws1_probe,
-	.remove = steelseries_srws1_remove,
-#endif
-	.report_fixup = steelseries_srws1_report_fixup
+MODULE_DEVICE_TABLE(hid, steelseries_devices);
+
+static struct hid_driver steelseries_driver = {
+	.name = "steelseries",
+	.id_table = steelseries_devices,
+	.probe = steelseries_probe,
+	.remove = steelseries_remove,
+	.report_fixup = steelseries_srws1_report_fixup,
+	.raw_event = steelseries_headset_raw_event,
 };
 
-module_hid_driver(steelseries_srws1_driver);
+module_hid_driver(steelseries_driver);
 MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_AUTHOR("Simon Wood <simon@mungewell.org>");
-- 
2.41.0


^ permalink raw reply related

* Re: [PATCH v3] HID: logitech-hidpp: Add wired USB id for Logitech G502 Lightspeed
From: Bastien Nocera @ 2023-06-30 11:51 UTC (permalink / raw)
  To: Stuart Hayhurst
  Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
	Filipe Laíns
In-Reply-To: <20230630113818.13005-1-stuart.a.hayhurst@gmail.com>

On Fri, 2023-06-30 at 12:38 +0100, Stuart Hayhurst wrote:
> Previously, support for the G502 had been attempted in commit
> '27fc32fd9417 ("HID: logitech-hidpp: add USB PID for a few more
> supported mice")'
> 
> This caused some issues and was reverted by
> 'addf3382c47c ("Revert "HID: logitech-hidpp: add USB PID for a few
> more
> supported mice"")'.
> 
> Since then, a new version of this mouse has been released (Lightpseed
> Wireless), and works correctly.
> 
> This device has support for battery reporting with the driver
> 
> Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>

Reviewed-by: Bastien Nocera <hadess@hadess.net>

> ---
> 
> Changes:
> v3:
>  - Add git commit titles
> v2:
>  - Add commit references
> 
> ---
>  drivers/hid/hid-logitech-hidpp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 5e1a412fd28f..94a045ef8e50 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4598,6 +4598,8 @@ static const struct hid_device_id
> hidpp_devices[] = {
>  
>         { /* Logitech G403 Wireless Gaming Mouse over USB */
>           HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC082) },
> +       { /* Logitech G502 Lightspeed Wireless Gaming Mouse over USB
> */
> +         HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08D) },
>         { /* Logitech G703 Gaming Mouse over USB */
>           HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC087) },
>         { /* Logitech G703 Hero Gaming Mouse over USB */


^ 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