Add a hid-stadiaff module to support rumble based force feedback on the
Google Stadia controller. This works using the HID output endpoint
exposed on both the USB and BLE interface.
Signed-off-by: Fabio Baltieri <[email protected]>
---
Hi, this adds rumble support to the stadia controller using the input
interface. Up to now this has only been implemented at application level
using hidraw:
https://source.chromium.org/chromium/chromium/src/+/main:device/gamepad/hid_haptic_gamepad.cc
Tested with fftest, works both over USB and BLE.
Changes from v1:
- renamed the module to hid-google-stadiaff.c
- added locking for passing the state to the worker code
- added a module removed check to prevent the work from rescheduling
drivers/hid/Kconfig | 7 ++
drivers/hid/Makefile | 1 +
drivers/hid/hid-google-stadiaff.c | 153 ++++++++++++++++++++++++++++++
drivers/hid/hid-ids.h | 1 +
4 files changed, 162 insertions(+)
create mode 100644 drivers/hid/hid-google-stadiaff.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 82f64fb31fda..f4f75d8a28ac 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -412,6 +412,13 @@ config HID_GOOGLE_HAMMER
help
Say Y here if you have a Google Hammer device.
+config HID_GOOGLE_STADIA_FF
+ tristate "Google Stadia force feedback"
+ select INPUT_FF_MEMLESS
+ help
+ Say Y here if you want to enable force feedback support for the Google
+ Stadia controller.
+
config HID_VIVALDI
tristate "Vivaldi Keyboard"
select HID_VIVALDI_COMMON
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 5d37cacbde33..18e9a3afecab 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_HID_GFRM) += hid-gfrm.o
obj-$(CONFIG_HID_GLORIOUS) += hid-glorious.o
obj-$(CONFIG_HID_VIVALDI_COMMON) += hid-vivaldi-common.o
obj-$(CONFIG_HID_GOOGLE_HAMMER) += hid-google-hammer.o
+obj-$(CONFIG_HID_GOOGLE_STADIA_FF) += hid-google-stadiaff.o
obj-$(CONFIG_HID_VIVALDI) += hid-vivaldi.o
obj-$(CONFIG_HID_GT683R) += hid-gt683r.o
obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
diff --git a/drivers/hid/hid-google-stadiaff.c b/drivers/hid/hid-google-stadiaff.c
new file mode 100644
index 000000000000..2628099e802c
--- /dev/null
+++ b/drivers/hid/hid-google-stadiaff.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Stadia controller rumble support.
+ *
+ * Copyright 2023 Google LLC
+ */
+
+#include <linux/hid.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+#define STADIA_FF_REPORT_ID 5
+
+struct stadiaff_device {
+ struct hid_device *hid;
+ struct hid_report *report;
+ spinlock_t lock;
+ bool removed;
+ uint16_t strong_magnitude;
+ uint16_t weak_magnitude;
+ struct work_struct work;
+};
+
+static void stadiaff_work(struct work_struct *work)
+{
+ struct stadiaff_device *stadiaff =
+ container_of(work, struct stadiaff_device, work);
+ struct hid_field *rumble_field = stadiaff->report->field[0];
+ unsigned long flags;
+
+ spin_lock_irqsave(&stadiaff->lock, flags);
+ rumble_field->value[0] = stadiaff->strong_magnitude;
+ rumble_field->value[1] = stadiaff->weak_magnitude;
+ spin_unlock_irqrestore(&stadiaff->lock, flags);
+
+ hid_hw_request(stadiaff->hid, stadiaff->report, HID_REQ_SET_REPORT);
+}
+
+static int stadiaff_play(struct input_dev *dev, void *data,
+ struct ff_effect *effect)
+{
+ struct hid_device *hid = input_get_drvdata(dev);
+ struct stadiaff_device *stadiaff = hid_get_drvdata(hid);
+ unsigned long flags;
+
+ spin_lock_irqsave(&stadiaff->lock, flags);
+ if (!stadiaff->removed) {
+ stadiaff->strong_magnitude = effect->u.rumble.strong_magnitude;
+ stadiaff->weak_magnitude = effect->u.rumble.weak_magnitude;
+ schedule_work(&stadiaff->work);
+ }
+ spin_unlock_irqrestore(&stadiaff->lock, flags);
+
+ return 0;
+}
+
+static int stadiaff_init(struct hid_device *hid)
+{
+ struct stadiaff_device *stadiaff;
+ struct hid_report *report;
+ struct hid_input *hidinput;
+ struct input_dev *dev;
+ int error;
+
+ if (list_empty(&hid->inputs)) {
+ hid_err(hid, "no inputs found\n");
+ return -ENODEV;
+ }
+ hidinput = list_entry(hid->inputs.next, struct hid_input, list);
+ dev = hidinput->input;
+
+ report = hid_validate_values(hid, HID_OUTPUT_REPORT,
+ STADIA_FF_REPORT_ID, 0, 2);
+ if (!report)
+ return -ENODEV;
+
+ stadiaff = devm_kzalloc(&hid->dev, sizeof(struct stadiaff_device),
+ GFP_KERNEL);
+ if (!stadiaff)
+ return -ENOMEM;
+
+ hid_set_drvdata(hid, stadiaff);
+
+ input_set_capability(dev, EV_FF, FF_RUMBLE);
+
+ error = input_ff_create_memless(dev, NULL, stadiaff_play);
+ if (error)
+ return error;
+
+ stadiaff->removed = false;
+ stadiaff->hid = hid;
+ stadiaff->report = report;
+ INIT_WORK(&stadiaff->work, stadiaff_work);
+ spin_lock_init(&stadiaff->lock);
+
+ hid_info(hid, "Force Feedback for Google Stadia controller\n");
+
+ return 0;
+}
+
+static int stadia_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "parse failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
+ if (ret) {
+ hid_err(hdev, "hw start failed\n");
+ return ret;
+ }
+
+ stadiaff_init(hdev);
+
+ return 0;
+}
+
+static void stadia_remove(struct hid_device *hid)
+{
+ struct stadiaff_device *stadiaff = hid_get_drvdata(hid);
+ unsigned long flags;
+
+ spin_lock_irqsave(&stadiaff->lock, flags);
+ stadiaff->removed = true;
+ spin_unlock_irqrestore(&stadiaff->lock, flags);
+
+ cancel_work_sync(&stadiaff->work);
+ hid_hw_stop(hid);
+}
+
+static const struct hid_device_id stadia_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_STADIA) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_STADIA) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, stadia_devices);
+
+static struct hid_driver stadia_driver = {
+ .name = "stadia",
+ .id_table = stadia_devices,
+ .probe = stadia_probe,
+ .remove = stadia_remove,
+};
+module_hid_driver(stadia_driver);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 63545cd307e5..cffd4ac609a0 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -525,6 +525,7 @@
#define USB_DEVICE_ID_GOOGLE_MOONBALL 0x5044
#define USB_DEVICE_ID_GOOGLE_DON 0x5050
#define USB_DEVICE_ID_GOOGLE_EEL 0x5057
+#define USB_DEVICE_ID_GOOGLE_STADIA 0x9400
#define USB_VENDOR_ID_GOTOP 0x08f2
#define USB_DEVICE_ID_SUPER_Q2 0x007f
--
2.40.1.521.gf1e218fcd8-goog
On Fri, 07 Jul, 2023 10:40:35 +0000 Fabio Baltieri <[email protected]> wrote:
> Add a hid-stadiaff module to support rumble based force feedback on the
> Google Stadia controller. This works using the HID output endpoint
> exposed on both the USB and BLE interface.
>
> Signed-off-by: Fabio Baltieri <[email protected]>
> ---
> +static int stadiaff_init(struct hid_device *hid)
> +{
> + struct stadiaff_device *stadiaff;
> + struct hid_report *report;
> + struct hid_input *hidinput;
> + struct input_dev *dev;
> + int error;
> +
> + if (list_empty(&hid->inputs)) {
> + hid_err(hid, "no inputs found\n");
> + return -ENODEV;
> + }
> + hidinput = list_entry(hid->inputs.next, struct hid_input, list);
> + dev = hidinput->input;
> +
> + report = hid_validate_values(hid, HID_OUTPUT_REPORT,
> + STADIA_FF_REPORT_ID, 0, 2);
> + if (!report)
> + return -ENODEV;
> +
> + stadiaff = devm_kzalloc(&hid->dev, sizeof(struct stadiaff_device),
> + GFP_KERNEL);
> + if (!stadiaff)
> + return -ENOMEM;
If we fail to allocate stadiaff, we abort init without ever initializing
the spinlock and work struct.
> +
> + hid_set_drvdata(hid, stadiaff);
> +
> + input_set_capability(dev, EV_FF, FF_RUMBLE);
> +
> + error = input_ff_create_memless(dev, NULL, stadiaff_play);
> + if (error)
> + return error;
Lets say input_ff_create_memless fails. The spinlock and work struct are
not properly initialized.
> +
> + stadiaff->removed = false;
> + stadiaff->hid = hid;
> + stadiaff->report = report;
> + INIT_WORK(&stadiaff->work, stadiaff_work);
> + spin_lock_init(&stadiaff->lock);
> +
> + hid_info(hid, "Force Feedback for Google Stadia controller\n");
> +
> + return 0;
> +}
> +
> +static int stadia_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + int ret;
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "parse failed\n");
> + return ret;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
> + if (ret) {
> + hid_err(hdev, "hw start failed\n");
> + return ret;
> + }
> +
> + stadiaff_init(hdev);
Is the intention for not error handling stadiaff_init to be able to use
the HID device even if haptics are not enabled? I think that's fine but
there are some design considerations that need to be made in order for
this code to be safe in the context of stadia_remove.
> +
> + return 0;
> +}
> +
> +static void stadia_remove(struct hid_device *hid)
> +{
> + struct stadiaff_device *stadiaff = hid_get_drvdata(hid);
stadiaff is unsafe to use if we failed to allocate memory for it and do
not set the hid device driver data. We would be dereferencing a
driver_data pointer never set/initialized by this driver in this error
path in stadiaff_init.
> + unsigned long flags;
> +
> + spin_lock_irqsave(&stadiaff->lock, flags);
> + stadiaff->removed = true;
> + spin_unlock_irqrestore(&stadiaff->lock, flags);
Attempting to lock/unlock a spinlock that may not have been initialized
if an error occurred in the stadiaff_init path.
> +
> + cancel_work_sync(&stadiaff->work);
Attempting to cancel work on a work_struct that may not be properly
initialized if an error occurred in stadiaff_init.
> + hid_hw_stop(hid);
> +}
Thanks,
-- Rahul Rameshbabu
Hi Rahul,
On Fri, Jul 07, 2023 at 10:51:48AM -0700, Rahul Rameshbabu wrote:
> On Fri, 07 Jul, 2023 10:40:35 +0000 Fabio Baltieri <[email protected]> wrote:
> > Add a hid-stadiaff module to support rumble based force feedback on the
> > Google Stadia controller. This works using the HID output endpoint
> > exposed on both the USB and BLE interface.
> >
> > Signed-off-by: Fabio Baltieri <[email protected]>
> > ---
> > +static int stadiaff_init(struct hid_device *hid)
> > +{
> > + struct stadiaff_device *stadiaff;
> > + struct hid_report *report;
> > + struct hid_input *hidinput;
> > + struct input_dev *dev;
> > + int error;
> > +
> > + if (list_empty(&hid->inputs)) {
> > + hid_err(hid, "no inputs found\n");
> > + return -ENODEV;
> > + }
> > + hidinput = list_entry(hid->inputs.next, struct hid_input, list);
> > + dev = hidinput->input;
> > +
> > + report = hid_validate_values(hid, HID_OUTPUT_REPORT,
> > + STADIA_FF_REPORT_ID, 0, 2);
> > + if (!report)
> > + return -ENODEV;
> > +
> > + stadiaff = devm_kzalloc(&hid->dev, sizeof(struct stadiaff_device),
> > + GFP_KERNEL);
> > + if (!stadiaff)
> > + return -ENOMEM;
>
> If we fail to allocate stadiaff, we abort init without ever initializing
> the spinlock and work struct.
>
> > +
> > + hid_set_drvdata(hid, stadiaff);
> > +
> > + input_set_capability(dev, EV_FF, FF_RUMBLE);
> > +
> > + error = input_ff_create_memless(dev, NULL, stadiaff_play);
> > + if (error)
> > + return error;
>
> Lets say input_ff_create_memless fails. The spinlock and work struct are
> not properly initialized.
>
> > +
> > + stadiaff->removed = false;
> > + stadiaff->hid = hid;
> > + stadiaff->report = report;
> > + INIT_WORK(&stadiaff->work, stadiaff_work);
> > + spin_lock_init(&stadiaff->lock);
> > +
> > + hid_info(hid, "Force Feedback for Google Stadia controller\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int stadia_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > +{
> > + int ret;
> > +
> > + ret = hid_parse(hdev);
> > + if (ret) {
> > + hid_err(hdev, "parse failed\n");
> > + return ret;
> > + }
> > +
> > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
> > + if (ret) {
> > + hid_err(hdev, "hw start failed\n");
> > + return ret;
> > + }
> > +
> > + stadiaff_init(hdev);
>
> Is the intention for not error handling stadiaff_init to be able to use
> the HID device even if haptics are not enabled? I think that's fine but
> there are some design considerations that need to be made in order for
> this code to be safe in the context of stadia_remove.
Sorry, no, the intention was to catch the error here and fail the probe,
that's the pattern on other hid haptic drivers as well, would not really
want to get too creative about it here. I shuffled the code around a bit
and somehow missed this check, I think it addresses all the potential
unallocated dereferecing you pointed out.
I'll send a v3 with the check, thanks for spotting this.
>
> > +
> > + return 0;
> > +}
> > +
> > +static void stadia_remove(struct hid_device *hid)
> > +{
> > + struct stadiaff_device *stadiaff = hid_get_drvdata(hid);
>
> stadiaff is unsafe to use if we failed to allocate memory for it and do
> not set the hid device driver data. We would be dereferencing a
> driver_data pointer never set/initialized by this driver in this error
> path in stadiaff_init.
>
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&stadiaff->lock, flags);
> > + stadiaff->removed = true;
> > + spin_unlock_irqrestore(&stadiaff->lock, flags);
>
> Attempting to lock/unlock a spinlock that may not have been initialized
> if an error occurred in the stadiaff_init path.
>
> > +
> > + cancel_work_sync(&stadiaff->work);
>
> Attempting to cancel work on a work_struct that may not be properly
> initialized if an error occurred in stadiaff_init.
>
> > + hid_hw_stop(hid);
> > +}
>
> Thanks,
>
> -- Rahul Rameshbabu