* [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers @ 2022-08-22 6:09 Marcus Folkesson 2022-08-22 6:09 ` [RESEND PATCH 2/2] MAINTAINERS: Add entry " Marcus Folkesson 2022-08-23 9:49 ` [RESEND PATCH 1/2] HID: Add driver " Benjamin Tissoires 0 siblings, 2 replies; 16+ messages in thread From: Marcus Folkesson @ 2022-08-22 6:09 UTC (permalink / raw) To: Jonathan Corbet, Marcus Folkesson, Jiri Kosina, Benjamin Tissoires Cc: linux-doc, linux-kernel, linux-input Several RC Simulator Controllers are HID compliant with similar interface. Add support for these controllers: - Phoenix RC (HID variant) - Car VRC2.0 - Real Flight G5/G6/G7 - Aero Fly, FMS - OrangeRX FrSky Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> --- Documentation/hid/index.rst | 1 + Documentation/hid/rcsim.rst | 142 ++++++++++++++++ drivers/hid/Kconfig | 11 ++ drivers/hid/Makefile | 1 + drivers/hid/hid-ids.h | 5 + drivers/hid/hid-rcsim.c | 315 ++++++++++++++++++++++++++++++++++++ 6 files changed, 475 insertions(+) create mode 100644 Documentation/hid/rcsim.rst create mode 100644 drivers/hid/hid-rcsim.c diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst index e50f513c579c..e5813d264f37 100644 --- a/Documentation/hid/index.rst +++ b/Documentation/hid/index.rst @@ -17,3 +17,4 @@ Human Interface Devices (HID) hid-alps intel-ish-hid amd-sfh-hid + rcsim diff --git a/Documentation/hid/rcsim.rst b/Documentation/hid/rcsim.rst new file mode 100644 index 000000000000..1a031f7189cb --- /dev/null +++ b/Documentation/hid/rcsim.rst @@ -0,0 +1,142 @@ +.. SPDX-License-Identifier: GPL-2.0 + +================================= +rcsim - RC Simulator Controllers +================================= + +:Author: Marcus Folkesson <marcus.folkesson@gmail.com> + +This driver let you use your own RC controller plugged +into your computer using an HID compatible USB dongle. + +There are several HID compatible USB dongles from different +vendors. The driver currently supports: + +- Phoenix RC (HID variant) (8ch) +- Car VRC2.0 (2ch) +- Real Flight G5/G6/G7 (6ch) +- Aero Fly, FMS (8ch) +- OrangeRX FrSky (6ch) + +Many RC controllers is able to configure which stick goes to which channel. +This is also configurable in most simulators, so a matching is not necessary. + +Supported dongles +================== + +PhoenixRC +---------- + +The PhoenixRC has one HID compatible variant which is supported by this driver. +The controller has support for 8 analog channels. + +The driver is generating the following input event for on channels: + ++---------+----------------+ +| Channel | Event | ++=========+================+ +| 1 | ABS_Y | ++---------+----------------+ +| 2 | ABS_X | ++---------+----------------+ +| 3 | ABS_RY | ++---------+----------------+ +| 4 | ABS_RX | ++---------+----------------+ +| 5 | ABS_RUDDER | ++---------+----------------+ +| 6 | ABS_THROTTLE | ++---------+----------------+ +| 7 | ABS_Z | ++---------+----------------+ +| 8 | ABS_RZ | ++---------+----------------+ + +VRC2.0 +---------- +VRC2.0 is a controller for RC Cars. +The controller has support for 2 analog channels. + +The driver is generating the following input event for on channels: + ++---------+----------------+ +| Channel | Event | ++=========+================+ +| 1 | ABS_GAS | ++---------+----------------+ +| 2 | ABS_WHEEL | ++---------+----------------+ + +RealFlight +---------- + +This driver supports Realflight G4-G7 and above +The controller has support for 4 analog channels and two buttons. + +The driver is generating the following input event for on channels: + ++---------+----------------+ +| Channel | Event | ++=========+================+ +| 1 | ABS_Y | ++---------+----------------+ +| 2 | ABS_X | ++---------+----------------+ +| 3 | ABS_RY | ++---------+----------------+ +| 4 | ABS_RX | ++---------+----------------+ +| 5 | BTN_A | ++---------+----------------+ +| 6 | BTN_B | ++---------+----------------+ + +XTR+G2+FMS Controllers +-------------------------------- + +The controllers has support for 8 analog channels. + +The driver is generating the following input event for on channels: + ++---------+----------------+ +| Channel | Event | ++=========+================+ +| 1 | ABS_Y | ++---------+----------------+ +| 2 | ABS_X | ++---------+----------------+ +| 3 | ABS_RY | ++---------+----------------+ +| 4 | ABS_RX | ++---------+----------------+ +| 5 | ABS_RUDDER | ++---------+----------------+ +| 6 | ABS_THROTTLE | ++---------+----------------+ +| 7 | ABS_Z | ++---------+----------------+ +| 8 | ABS_RZ | ++---------+----------------+ + +OrangeRX +---------- + +The controllers has support for 6 analog channels. + +The driver is generating the following input event for on channels: + ++---------+----------------+ +| Channel | Event | ++=========+================+ +| 1 | ABS_Y | ++---------+----------------+ +| 2 | ABS_X | ++---------+----------------+ +| 3 | ABS_RY | ++---------+----------------+ +| 4 | ABS_RX | ++---------+----------------+ +| 5 | ABS_RUDDER | ++---------+----------------+ +| 6 | ABS_THROTTLE | ++---------+----------------+ diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 70da5931082f..d8313d36086c 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -957,6 +957,17 @@ config HID_RAZER Support for Razer devices that are not fully compliant with the HID standard. +config HID_RCSIM + tristate "RC Simulator Controllers" + depends on HID + help + Support for several HID compatible RC Simulator Controllers including + - Phoenix RC + - Car VRC2.0 + - Real Flight + - Aero Fly, FMS + - OrangeRX FrSky + config HID_PRIMAX tristate "Primax non-fully HID-compliant devices" depends on HID diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index cac2cbe26d11..85d50ab352ee 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -102,6 +102,7 @@ obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o obj-$(CONFIG_HID_PLAYSTATION) += hid-playstation.o obj-$(CONFIG_HID_PRIMAX) += hid-primax.o obj-$(CONFIG_HID_RAZER) += hid-razer.o +obj-$(CONFIG_HID_RCSIM) += hid-rcsim.o obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o obj-$(CONFIG_HID_RETRODE) += hid-retrode.o obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index d9eb676abe96..baf5f74d5bed 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -1381,6 +1381,11 @@ #define USB_VENDOR_ID_MULTIPLE_1781 0x1781 #define USB_DEVICE_ID_RAPHNET_4NES4SNES_OLD 0x0a9d +#define USB_DEVICE_ID_PHOENIXRC 0x0898 +#define USB_DEVICE_ID_REALFLIGHT 0x0e56 + +#define USB_VENDOR_ID_DIPLING 0x0B9B +#define USB_DEVICE_ID_DIPLING_RCCONTROLLER 0x4012 #define USB_VENDOR_ID_DRACAL_RAPHNET 0x289b #define USB_DEVICE_ID_RAPHNET_2NES2SNES 0x0002 diff --git a/drivers/hid/hid-rcsim.c b/drivers/hid/hid-rcsim.c new file mode 100644 index 000000000000..0f214cb5816a --- /dev/null +++ b/drivers/hid/hid-rcsim.c @@ -0,0 +1,315 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Driver for several HID compatible RC Simulator Controllers. + * Currently supported controllers are: + * + * - Phoenix RC (HID variant) (8ch) + * - Car VRC2.0 (2ch) + * - Real Flight G5/G6/G7 (6ch) + * - Aero Fly, FMS (8ch) + * - OrangeRX FrSky (6ch) + * + * Copyright (C) 2022 Marcus Folkesson <marcus.folkesson@gmail.com> + */ + +#include <linux/bitfield.h> +#include <linux/device.h> +#include <linux/input.h> +#include <linux/hid.h> +#include <linux/module.h> +#include <linux/usb.h> + +#include "hid-ids.h" + +/* + * Some of these VID/PID are probably "borrowed", so keep them locally and + * do not populate hid-ids.h with those. + */ + +/* PHOENIXRC Controlloer (HID variant) */ +#define PHOENIXRC_VID (USB_VENDOR_ID_MULTIPLE_1781) +#define PHOENIXRC_PID (USB_DEVICE_ID_PHOENIXRC) +#define PHOENIXRC_DSIZE (8) + +/* VRC2 Controlloer */ +#define VRC2_VID (0x07c0) +#define VRC2_PID (0x1125) +#define VRC2_DSIZE (7) + +/* Realflight G4-&7 and Above Controller */ +#define REALFLIGHT_VID (USB_VENDOR_ID_MULTIPLE_1781) +#define REALFLIGHT_PID (USB_DEVICE_ID_REALFLIGHT) +#define REALFLIGHT_DSIZE (8) + +#define REALFLIGHT_BTN_A BIT(0) +#define REALFLIGHT_BTN_B BIT(1) + +/* XTR+G2+FMS Controller */ +#define XTRG2FMS_VID (USB_VENDOR_ID_DIPLING) +#define XTRG2FMS_PID (USB_DEVICE_ID_DIPLING_RCCONTROLLER) +#define XTRG2FMS_DSIZE (8) + +#define XTRG2FMS_X_HI GENMASK(3, 2) +#define XTRG2FMS_Y_HI GENMASK(1, 0) +#define XTRG2FMS_RX_HI GENMASK(7, 6) +#define XTRG2FMS_RY_HI GENMASK(5, 4) +#define XTRG2FMS_ALT1_HI GENMASK(3, 2) +#define XTRG2FMS_ALT2_HI GENMASK(1, 0) + +/* OrangeRX FrSky */ +#define ORANGERX_VID (0x0451) +#define ORANGERX_PID (0x16a5) +#define ORANGERX_DSIZE (8) + +enum rcsim_controller { + PHOENIXRC, + VRC2, + REALFLIGHT, + XTRG2FMS, + ORANGERX +}; + +struct rcsim_priv { + struct hid_device *hdev; + struct input_dev *input; + enum rcsim_controller controller; + u8 alt; +}; + +static int rcsim_open(struct input_dev *dev) +{ + struct rcsim_priv *priv = input_get_drvdata(dev); + + return hid_hw_open(priv->hdev); +} + +static void rcsim_close(struct input_dev *dev) +{ + struct rcsim_priv *priv = input_get_drvdata(dev); + + hid_hw_close(priv->hdev); +} + +static int rcsim_setup_input(struct rcsim_priv *priv) +{ + struct input_dev *input; + + input = devm_input_allocate_device(&priv->hdev->dev); + if (!input) + return -ENOMEM; + + input->id.bustype = priv->hdev->bus; + input->id.vendor = priv->hdev->vendor; + input->id.product = priv->hdev->product; + input->id.version = priv->hdev->bus; + input->phys = priv->hdev->phys; + input->uniq = priv->hdev->uniq; + input->open = rcsim_open; + input->close = rcsim_close; + + input_set_drvdata(input, priv); + + switch (priv->controller) { + case PHOENIXRC: + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); + input_set_abs_params(input, ABS_Z, 0, 255, 0, 0); + input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0); + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); + input->name = "RC Simuator Controller PhoenixRC"; + break; + case VRC2: + input_set_abs_params(input, ABS_GAS, 0, 2048, 0, 0); + input_set_abs_params(input, ABS_WHEEL, 0, 2048, 0, 0); + input->name = "RC Simuator Controller VRC2.0"; + break; + case REALFLIGHT: + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); + input_set_capability(input, EV_KEY, BTN_A); + input_set_capability(input, EV_KEY, BTN_B); + input->name = "RC Simuator Controller Realflight"; + break; + case XTRG2FMS: + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_Z, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_RZ, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_RUDDER, 0, 1024, 0, 0); + input_set_abs_params(input, ABS_THROTTLE, 0, 1024, 0, 0); + input->name = "RC Simuator Controller AeroFly, FMS"; + priv->alt = 0; + break; + case ORANGERX: + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); + input->name = "RC Simuator Controller OrangeRX FrSky"; + break; + }; + + priv->input = input; + return input_register_device(priv->input); +} + +static int rcsim_raw_event(struct hid_device *hdev, + struct hid_report *report, + u8 *raw_data, int size) +{ + struct rcsim_priv *priv = hid_get_drvdata(hdev); + u16 value; + + switch (priv->controller) { + case PHOENIXRC: + if (size != PHOENIXRC_DSIZE) + break; + + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ + input_report_abs(priv->input, ABS_X, raw_data[2]); + input_report_abs(priv->input, ABS_Y, raw_data[0]); + input_report_abs(priv->input, ABS_RX, raw_data[4]); + input_report_abs(priv->input, ABS_RY, raw_data[3]); + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); + + /* Z and RZ are sent every other time */ + if (priv->alt) + input_report_abs(priv->input, ABS_Z, raw_data[7]); + else + input_report_abs(priv->input, ABS_RZ, raw_data[7]); + + priv->alt ^= 1; + break; + case VRC2: + if (size != VRC2_DSIZE) + break; + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); + input_report_abs(priv->input, ABS_GAS, value); + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); + input_report_abs(priv->input, ABS_WHEEL, value); + break; + case REALFLIGHT: + if (size != REALFLIGHT_DSIZE) + break; + input_report_abs(priv->input, ABS_X, raw_data[2]); + input_report_abs(priv->input, ABS_Y, raw_data[1]); + input_report_abs(priv->input, ABS_RX, raw_data[5]); + input_report_abs(priv->input, ABS_RY, raw_data[3]); + input_report_abs(priv->input, ABS_MISC, raw_data[4]); + input_report_key(priv->input, BTN_A, + raw_data[7] & REALFLIGHT_BTN_A); + input_report_key(priv->input, BTN_B, + raw_data[7] & REALFLIGHT_BTN_B); + break; + case XTRG2FMS: + if (size != XTRG2FMS_DSIZE) + break; + + /* X, RX, Y and RY are sent every time */ + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); + value = (value << 8) | raw_data[1]; + input_report_abs(priv->input, ABS_X, value); + + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); + value = (value << 8) | raw_data[2]; + input_report_abs(priv->input, ABS_Y, value); + + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); + value = (value << 8) | raw_data[0]; + input_report_abs(priv->input, ABS_RX, value); + + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); + value = (value << 8) | raw_data[4]; + input_report_abs(priv->input, ABS_RY, value); + + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); + value = (value << 8) | raw_data[6]; + if (priv->alt) + input_report_abs(priv->input, ABS_Z, value); + else + input_report_abs(priv->input, ABS_RUDDER, value); + + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); + value = (value << 8) | raw_data[5]; + if (priv->alt) + input_report_abs(priv->input, ABS_RZ, value); + else + input_report_abs(priv->input, ABS_THROTTLE, value); + + priv->alt ^= 1; + break; + case ORANGERX: + if (size != ORANGERX_DSIZE) + break; + input_report_abs(priv->input, ABS_X, raw_data[0]); + input_report_abs(priv->input, ABS_Y, raw_data[2]); + input_report_abs(priv->input, ABS_RX, raw_data[3]); + input_report_abs(priv->input, ABS_RY, raw_data[1]); + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); + break; + }; + + input_sync(priv->input); + return 0; +} + +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) +{ + struct device *dev = &hdev->dev; + struct rcsim_priv *priv; + int ret; + + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) + return -ENODEV; + + ret = hid_parse(hdev); + if (ret) + return ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->hdev = hdev; + priv->controller = id->driver_data; + hid_set_drvdata(hdev, priv); + + ret = rcsim_setup_input(priv); + if (ret) + return ret; + + return hid_hw_start(hdev, HID_CONNECT_HIDRAW); +} + +static const struct hid_device_id rcsim_devices[] = { + { HID_USB_DEVICE(PHOENIXRC_VID, PHOENIXRC_PID), .driver_data = PHOENIXRC }, + { HID_USB_DEVICE(VRC2_VID, VRC2_PID), .driver_data = VRC2 }, + { HID_USB_DEVICE(REALFLIGHT_VID, REALFLIGHT_PID), .driver_data = REALFLIGHT }, + { HID_USB_DEVICE(XTRG2FMS_VID, XTRG2FMS_PID), .driver_data = XTRG2FMS }, + { HID_USB_DEVICE(ORANGERX_VID, ORANGERX_PID), .driver_data = ORANGERX }, + { /* Sentinel */ } +}; +MODULE_DEVICE_TABLE(hid, rcsim_devices); + +static struct hid_driver rcsim_driver = { + .name = "rcsim", + .id_table = rcsim_devices, + .probe = rcsim_probe, + .raw_event = rcsim_raw_event, +}; +module_hid_driver(rcsim_driver); + +MODULE_AUTHOR("Marcus Folkesson <marcus.folkesson@gmail.com>"); +MODULE_LICENSE("GPL"); -- 2.37.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RESEND PATCH 2/2] MAINTAINERS: Add entry for RC Simulator Controllers 2022-08-22 6:09 [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers Marcus Folkesson @ 2022-08-22 6:09 ` Marcus Folkesson 2022-08-23 9:49 ` [RESEND PATCH 1/2] HID: Add driver " Benjamin Tissoires 1 sibling, 0 replies; 16+ messages in thread From: Marcus Folkesson @ 2022-08-22 6:09 UTC (permalink / raw) To: Jonathan Corbet, Marcus Folkesson, Jiri Kosina, Benjamin Tissoires Cc: linux-doc, linux-kernel, linux-input Add an entry for RC Simulator Controllers and add myself as maintainers of this driver. Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> --- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 64379c699903..216b9f021f72 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16896,6 +16896,13 @@ M: Patrick Lerda <patrick9876@free.fr> S: Maintained F: drivers/media/rc/ir-rcmm-decoder.c +RC SIMULATOR CONTROLLERS +M: Marcus Folkesson <marcus.folkesson@gmail.com> +L: linux-input@vger.kernel.org +S: Maintained +F: Documentation/hid/rcsim.rst +F: drivers/hid/hid-rcsim.c + RCUTORTURE TEST FRAMEWORK M: "Paul E. McKenney" <paulmck@kernel.org> M: Josh Triplett <josh@joshtriplett.org> -- 2.37.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-22 6:09 [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers Marcus Folkesson 2022-08-22 6:09 ` [RESEND PATCH 2/2] MAINTAINERS: Add entry " Marcus Folkesson @ 2022-08-23 9:49 ` Benjamin Tissoires 2022-08-23 15:18 ` Marcus Folkesson 2023-03-25 16:08 ` Pavel Machek 1 sibling, 2 replies; 16+ messages in thread From: Benjamin Tissoires @ 2022-08-23 9:49 UTC (permalink / raw) To: Marcus Folkesson Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER Hi Marcus, [and sorry for the delay in the review of your patches] On Mon, Aug 22, 2022 at 8:04 AM Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > Several RC Simulator Controllers are HID compliant with similar > interface. > > Add support for these controllers: > - Phoenix RC (HID variant) > - Car VRC2.0 > - Real Flight G5/G6/G7 > - Aero Fly, FMS > - OrangeRX FrSky > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > --- > Documentation/hid/index.rst | 1 + > Documentation/hid/rcsim.rst | 142 ++++++++++++++++ > drivers/hid/Kconfig | 11 ++ > drivers/hid/Makefile | 1 + > drivers/hid/hid-ids.h | 5 + > drivers/hid/hid-rcsim.c | 315 ++++++++++++++++++++++++++++++++++++ > 6 files changed, 475 insertions(+) > create mode 100644 Documentation/hid/rcsim.rst > create mode 100644 drivers/hid/hid-rcsim.c > > diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst > index e50f513c579c..e5813d264f37 100644 > --- a/Documentation/hid/index.rst > +++ b/Documentation/hid/index.rst > @@ -17,3 +17,4 @@ Human Interface Devices (HID) > hid-alps > intel-ish-hid > amd-sfh-hid > + rcsim > diff --git a/Documentation/hid/rcsim.rst b/Documentation/hid/rcsim.rst > new file mode 100644 > index 000000000000..1a031f7189cb > --- /dev/null > +++ b/Documentation/hid/rcsim.rst > @@ -0,0 +1,142 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +================================= > +rcsim - RC Simulator Controllers > +================================= > + > +:Author: Marcus Folkesson <marcus.folkesson@gmail.com> > + > +This driver let you use your own RC controller plugged > +into your computer using an HID compatible USB dongle. > + > +There are several HID compatible USB dongles from different > +vendors. The driver currently supports: > + > +- Phoenix RC (HID variant) (8ch) > +- Car VRC2.0 (2ch) > +- Real Flight G5/G6/G7 (6ch) > +- Aero Fly, FMS (8ch) > +- OrangeRX FrSky (6ch) > + > +Many RC controllers is able to configure which stick goes to which channel. > +This is also configurable in most simulators, so a matching is not necessary. > + > +Supported dongles > +================== > + > +PhoenixRC > +---------- > + > +The PhoenixRC has one HID compatible variant which is supported by this driver. > +The controller has support for 8 analog channels. > + > +The driver is generating the following input event for on channels: > + > ++---------+----------------+ > +| Channel | Event | > ++=========+================+ > +| 1 | ABS_Y | > ++---------+----------------+ > +| 2 | ABS_X | > ++---------+----------------+ > +| 3 | ABS_RY | > ++---------+----------------+ > +| 4 | ABS_RX | > ++---------+----------------+ > +| 5 | ABS_RUDDER | > ++---------+----------------+ > +| 6 | ABS_THROTTLE | > ++---------+----------------+ > +| 7 | ABS_Z | > ++---------+----------------+ > +| 8 | ABS_RZ | > ++---------+----------------+ > + > +VRC2.0 > +---------- > +VRC2.0 is a controller for RC Cars. > +The controller has support for 2 analog channels. > + > +The driver is generating the following input event for on channels: > + > ++---------+----------------+ > +| Channel | Event | > ++=========+================+ > +| 1 | ABS_GAS | > ++---------+----------------+ > +| 2 | ABS_WHEEL | > ++---------+----------------+ > + > +RealFlight > +---------- > + > +This driver supports Realflight G4-G7 and above > +The controller has support for 4 analog channels and two buttons. > + > +The driver is generating the following input event for on channels: > + > ++---------+----------------+ > +| Channel | Event | > ++=========+================+ > +| 1 | ABS_Y | > ++---------+----------------+ > +| 2 | ABS_X | > ++---------+----------------+ > +| 3 | ABS_RY | > ++---------+----------------+ > +| 4 | ABS_RX | > ++---------+----------------+ > +| 5 | BTN_A | > ++---------+----------------+ > +| 6 | BTN_B | > ++---------+----------------+ > + > +XTR+G2+FMS Controllers > +-------------------------------- > + > +The controllers has support for 8 analog channels. > + > +The driver is generating the following input event for on channels: > + > ++---------+----------------+ > +| Channel | Event | > ++=========+================+ > +| 1 | ABS_Y | > ++---------+----------------+ > +| 2 | ABS_X | > ++---------+----------------+ > +| 3 | ABS_RY | > ++---------+----------------+ > +| 4 | ABS_RX | > ++---------+----------------+ > +| 5 | ABS_RUDDER | > ++---------+----------------+ > +| 6 | ABS_THROTTLE | > ++---------+----------------+ > +| 7 | ABS_Z | > ++---------+----------------+ > +| 8 | ABS_RZ | > ++---------+----------------+ > + > +OrangeRX > +---------- > + > +The controllers has support for 6 analog channels. > + > +The driver is generating the following input event for on channels: > + > ++---------+----------------+ > +| Channel | Event | > ++=========+================+ > +| 1 | ABS_Y | > ++---------+----------------+ > +| 2 | ABS_X | > ++---------+----------------+ > +| 3 | ABS_RY | > ++---------+----------------+ > +| 4 | ABS_RX | > ++---------+----------------+ > +| 5 | ABS_RUDDER | > ++---------+----------------+ > +| 6 | ABS_THROTTLE | > ++---------+----------------+ > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index 70da5931082f..d8313d36086c 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -957,6 +957,17 @@ config HID_RAZER > Support for Razer devices that are not fully compliant with the > HID standard. > > +config HID_RCSIM > + tristate "RC Simulator Controllers" > + depends on HID > + help > + Support for several HID compatible RC Simulator Controllers including > + - Phoenix RC > + - Car VRC2.0 > + - Real Flight > + - Aero Fly, FMS > + - OrangeRX FrSky > + > config HID_PRIMAX > tristate "Primax non-fully HID-compliant devices" > depends on HID > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index cac2cbe26d11..85d50ab352ee 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -102,6 +102,7 @@ obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o > obj-$(CONFIG_HID_PLAYSTATION) += hid-playstation.o > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o > obj-$(CONFIG_HID_RAZER) += hid-razer.o > +obj-$(CONFIG_HID_RCSIM) += hid-rcsim.o General rule of thumbs, we try to name the drivers after their vendors, unless we know we have a generic driver. Here, this driver seems to be really tied to a small set of devices, and thus I don't think we can call it "generic". > obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index d9eb676abe96..baf5f74d5bed 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -1381,6 +1381,11 @@ > > #define USB_VENDOR_ID_MULTIPLE_1781 0x1781 > #define USB_DEVICE_ID_RAPHNET_4NES4SNES_OLD 0x0a9d > +#define USB_DEVICE_ID_PHOENIXRC 0x0898 > +#define USB_DEVICE_ID_REALFLIGHT 0x0e56 > + > +#define USB_VENDOR_ID_DIPLING 0x0B9B > +#define USB_DEVICE_ID_DIPLING_RCCONTROLLER 0x4012 > > #define USB_VENDOR_ID_DRACAL_RAPHNET 0x289b > #define USB_DEVICE_ID_RAPHNET_2NES2SNES 0x0002 > diff --git a/drivers/hid/hid-rcsim.c b/drivers/hid/hid-rcsim.c > new file mode 100644 > index 000000000000..0f214cb5816a > --- /dev/null > +++ b/drivers/hid/hid-rcsim.c > @@ -0,0 +1,315 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Driver for several HID compatible RC Simulator Controllers. > + * Currently supported controllers are: > + * > + * - Phoenix RC (HID variant) (8ch) > + * - Car VRC2.0 (2ch) > + * - Real Flight G5/G6/G7 (6ch) > + * - Aero Fly, FMS (8ch) > + * - OrangeRX FrSky (6ch) > + * > + * Copyright (C) 2022 Marcus Folkesson <marcus.folkesson@gmail.com> > + */ > + > +#include <linux/bitfield.h> > +#include <linux/device.h> > +#include <linux/input.h> > +#include <linux/hid.h> > +#include <linux/module.h> > +#include <linux/usb.h> > + > +#include "hid-ids.h" > + > +/* > + * Some of these VID/PID are probably "borrowed", so keep them locally and > + * do not populate hid-ids.h with those. > + */ > + > +/* PHOENIXRC Controlloer (HID variant) */ > +#define PHOENIXRC_VID (USB_VENDOR_ID_MULTIPLE_1781) > +#define PHOENIXRC_PID (USB_DEVICE_ID_PHOENIXRC) > +#define PHOENIXRC_DSIZE (8) > + > +/* VRC2 Controlloer */ > +#define VRC2_VID (0x07c0) > +#define VRC2_PID (0x1125) > +#define VRC2_DSIZE (7) > + > +/* Realflight G4-&7 and Above Controller */ > +#define REALFLIGHT_VID (USB_VENDOR_ID_MULTIPLE_1781) > +#define REALFLIGHT_PID (USB_DEVICE_ID_REALFLIGHT) > +#define REALFLIGHT_DSIZE (8) > + > +#define REALFLIGHT_BTN_A BIT(0) > +#define REALFLIGHT_BTN_B BIT(1) > + > +/* XTR+G2+FMS Controller */ > +#define XTRG2FMS_VID (USB_VENDOR_ID_DIPLING) > +#define XTRG2FMS_PID (USB_DEVICE_ID_DIPLING_RCCONTROLLER) > +#define XTRG2FMS_DSIZE (8) > + > +#define XTRG2FMS_X_HI GENMASK(3, 2) > +#define XTRG2FMS_Y_HI GENMASK(1, 0) > +#define XTRG2FMS_RX_HI GENMASK(7, 6) > +#define XTRG2FMS_RY_HI GENMASK(5, 4) > +#define XTRG2FMS_ALT1_HI GENMASK(3, 2) > +#define XTRG2FMS_ALT2_HI GENMASK(1, 0) > + > +/* OrangeRX FrSky */ > +#define ORANGERX_VID (0x0451) > +#define ORANGERX_PID (0x16a5) > +#define ORANGERX_DSIZE (8) > + > +enum rcsim_controller { > + PHOENIXRC, > + VRC2, > + REALFLIGHT, > + XTRG2FMS, > + ORANGERX > +}; > + > +struct rcsim_priv { > + struct hid_device *hdev; > + struct input_dev *input; > + enum rcsim_controller controller; > + u8 alt; > +}; > + > +static int rcsim_open(struct input_dev *dev) > +{ > + struct rcsim_priv *priv = input_get_drvdata(dev); > + > + return hid_hw_open(priv->hdev); > +} > + > +static void rcsim_close(struct input_dev *dev) > +{ > + struct rcsim_priv *priv = input_get_drvdata(dev); > + > + hid_hw_close(priv->hdev); > +} > + > +static int rcsim_setup_input(struct rcsim_priv *priv) > +{ > + struct input_dev *input; > + > + input = devm_input_allocate_device(&priv->hdev->dev); > + if (!input) > + return -ENOMEM; > + > + input->id.bustype = priv->hdev->bus; > + input->id.vendor = priv->hdev->vendor; > + input->id.product = priv->hdev->product; > + input->id.version = priv->hdev->bus; > + input->phys = priv->hdev->phys; > + input->uniq = priv->hdev->uniq; > + input->open = rcsim_open; > + input->close = rcsim_close; > + > + input_set_drvdata(input, priv); > + > + switch (priv->controller) { > + case PHOENIXRC: > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_Z, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > + input->name = "RC Simuator Controller PhoenixRC"; > + break; > + case VRC2: > + input_set_abs_params(input, ABS_GAS, 0, 2048, 0, 0); > + input_set_abs_params(input, ABS_WHEEL, 0, 2048, 0, 0); > + input->name = "RC Simuator Controller VRC2.0"; > + break; > + case REALFLIGHT: > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > + input_set_capability(input, EV_KEY, BTN_A); > + input_set_capability(input, EV_KEY, BTN_B); > + input->name = "RC Simuator Controller Realflight"; > + break; > + case XTRG2FMS: > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_Z, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_RZ, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_RUDDER, 0, 1024, 0, 0); > + input_set_abs_params(input, ABS_THROTTLE, 0, 1024, 0, 0); > + input->name = "RC Simuator Controller AeroFly, FMS"; > + priv->alt = 0; > + break; > + case ORANGERX: > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > + input->name = "RC Simuator Controller OrangeRX FrSky"; > + break; > + }; > + > + priv->input = input; > + return input_register_device(priv->input); > +} You are basically rewriting hid-input.c, which is suboptimal. I guess the report descriptor provided by these devices are basically useless, and so you have to parse the reports yourself in the raw_event callback. But instead of manually doing that, why not overwrite the report descriptor (with .rdesc_fixup) and declare here all of the data that needs to be exported. You could remove basically everything in this driver by just providing a fixed report descriptor. > + > +static int rcsim_raw_event(struct hid_device *hdev, > + struct hid_report *report, > + u8 *raw_data, int size) > +{ > + struct rcsim_priv *priv = hid_get_drvdata(hdev); > + u16 value; > + > + switch (priv->controller) { > + case PHOENIXRC: > + if (size != PHOENIXRC_DSIZE) > + break; > + > + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ > + input_report_abs(priv->input, ABS_X, raw_data[2]); > + input_report_abs(priv->input, ABS_Y, raw_data[0]); > + input_report_abs(priv->input, ABS_RX, raw_data[4]); > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > + > + /* Z and RZ are sent every other time */ > + if (priv->alt) > + input_report_abs(priv->input, ABS_Z, raw_data[7]); > + else > + input_report_abs(priv->input, ABS_RZ, raw_data[7]); > + > + priv->alt ^= 1; > + break; > + case VRC2: > + if (size != VRC2_DSIZE) > + break; > + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); > + input_report_abs(priv->input, ABS_GAS, value); > + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); > + input_report_abs(priv->input, ABS_WHEEL, value); > + break; > + case REALFLIGHT: > + if (size != REALFLIGHT_DSIZE) > + break; > + input_report_abs(priv->input, ABS_X, raw_data[2]); > + input_report_abs(priv->input, ABS_Y, raw_data[1]); > + input_report_abs(priv->input, ABS_RX, raw_data[5]); > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > + input_report_abs(priv->input, ABS_MISC, raw_data[4]); > + input_report_key(priv->input, BTN_A, > + raw_data[7] & REALFLIGHT_BTN_A); > + input_report_key(priv->input, BTN_B, > + raw_data[7] & REALFLIGHT_BTN_B); > + break; > + case XTRG2FMS: > + if (size != XTRG2FMS_DSIZE) > + break; > + > + /* X, RX, Y and RY are sent every time */ > + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); > + value = (value << 8) | raw_data[1]; > + input_report_abs(priv->input, ABS_X, value); > + > + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); > + value = (value << 8) | raw_data[2]; > + input_report_abs(priv->input, ABS_Y, value); > + > + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); > + value = (value << 8) | raw_data[0]; > + input_report_abs(priv->input, ABS_RX, value); > + > + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); > + value = (value << 8) | raw_data[4]; > + input_report_abs(priv->input, ABS_RY, value); > + > + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ > + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > + value = (value << 8) | raw_data[6]; > + if (priv->alt) > + input_report_abs(priv->input, ABS_Z, value); > + else > + input_report_abs(priv->input, ABS_RUDDER, value); > + > + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > + value = (value << 8) | raw_data[5]; > + if (priv->alt) > + input_report_abs(priv->input, ABS_RZ, value); > + else > + input_report_abs(priv->input, ABS_THROTTLE, value); > + > + priv->alt ^= 1; > + break; > + case ORANGERX: > + if (size != ORANGERX_DSIZE) > + break; > + input_report_abs(priv->input, ABS_X, raw_data[0]); > + input_report_abs(priv->input, ABS_Y, raw_data[2]); > + input_report_abs(priv->input, ABS_RX, raw_data[3]); > + input_report_abs(priv->input, ABS_RY, raw_data[1]); > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > + break; > + }; > + > + input_sync(priv->input); > + return 0; > +} > + > +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) > +{ > + struct device *dev = &hdev->dev; > + struct rcsim_priv *priv; > + int ret; > + > + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) > + return -ENODEV; You are not accessing anything in the USB stack, so there is no need to prevent regression tests that could inject uhid devices to your drivers. Cheers, Benjamin > + > + ret = hid_parse(hdev); > + if (ret) > + return ret; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->hdev = hdev; > + priv->controller = id->driver_data; > + hid_set_drvdata(hdev, priv); > + > + ret = rcsim_setup_input(priv); > + if (ret) > + return ret; > + > + return hid_hw_start(hdev, HID_CONNECT_HIDRAW); > +} > + > +static const struct hid_device_id rcsim_devices[] = { > + { HID_USB_DEVICE(PHOENIXRC_VID, PHOENIXRC_PID), .driver_data = PHOENIXRC }, > + { HID_USB_DEVICE(VRC2_VID, VRC2_PID), .driver_data = VRC2 }, > + { HID_USB_DEVICE(REALFLIGHT_VID, REALFLIGHT_PID), .driver_data = REALFLIGHT }, > + { HID_USB_DEVICE(XTRG2FMS_VID, XTRG2FMS_PID), .driver_data = XTRG2FMS }, > + { HID_USB_DEVICE(ORANGERX_VID, ORANGERX_PID), .driver_data = ORANGERX }, > + { /* Sentinel */ } > +}; > +MODULE_DEVICE_TABLE(hid, rcsim_devices); > + > +static struct hid_driver rcsim_driver = { > + .name = "rcsim", > + .id_table = rcsim_devices, > + .probe = rcsim_probe, > + .raw_event = rcsim_raw_event, > +}; > +module_hid_driver(rcsim_driver); > + > +MODULE_AUTHOR("Marcus Folkesson <marcus.folkesson@gmail.com>"); > +MODULE_LICENSE("GPL"); > -- > 2.37.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-23 9:49 ` [RESEND PATCH 1/2] HID: Add driver " Benjamin Tissoires @ 2022-08-23 15:18 ` Marcus Folkesson 2022-08-23 16:43 ` Benjamin Tissoires 2023-03-25 16:08 ` Pavel Machek 1 sibling, 1 reply; 16+ messages in thread From: Marcus Folkesson @ 2022-08-23 15:18 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER [-- Attachment #1: Type: text/plain, Size: 22032 bytes --] Thank you Benjamin, On Tue, Aug 23, 2022 at 11:49:59AM +0200, Benjamin Tissoires wrote: > Hi Marcus, > > [and sorry for the delay in the review of your patches] > > On Mon, Aug 22, 2022 at 8:04 AM Marcus Folkesson > <marcus.folkesson@gmail.com> wrote: > > > > Several RC Simulator Controllers are HID compliant with similar > > interface. > > > > Add support for these controllers: > > - Phoenix RC (HID variant) > > - Car VRC2.0 > > - Real Flight G5/G6/G7 > > - Aero Fly, FMS > > - OrangeRX FrSky > > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > > --- > > Documentation/hid/index.rst | 1 + > > Documentation/hid/rcsim.rst | 142 ++++++++++++++++ > > drivers/hid/Kconfig | 11 ++ > > drivers/hid/Makefile | 1 + > > drivers/hid/hid-ids.h | 5 + > > drivers/hid/hid-rcsim.c | 315 ++++++++++++++++++++++++++++++++++++ > > 6 files changed, 475 insertions(+) > > create mode 100644 Documentation/hid/rcsim.rst > > create mode 100644 drivers/hid/hid-rcsim.c > > > > diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst > > index e50f513c579c..e5813d264f37 100644 > > --- a/Documentation/hid/index.rst > > +++ b/Documentation/hid/index.rst > > @@ -17,3 +17,4 @@ Human Interface Devices (HID) > > hid-alps > > intel-ish-hid > > amd-sfh-hid > > + rcsim > > diff --git a/Documentation/hid/rcsim.rst b/Documentation/hid/rcsim.rst > > new file mode 100644 > > index 000000000000..1a031f7189cb > > --- /dev/null > > +++ b/Documentation/hid/rcsim.rst > > @@ -0,0 +1,142 @@ > > +.. SPDX-License-Identifier: GPL-2.0 > > + > > +================================= > > +rcsim - RC Simulator Controllers > > +================================= > > + > > +:Author: Marcus Folkesson <marcus.folkesson@gmail.com> > > + > > +This driver let you use your own RC controller plugged > > +into your computer using an HID compatible USB dongle. > > + > > +There are several HID compatible USB dongles from different > > +vendors. The driver currently supports: > > + > > +- Phoenix RC (HID variant) (8ch) > > +- Car VRC2.0 (2ch) > > +- Real Flight G5/G6/G7 (6ch) > > +- Aero Fly, FMS (8ch) > > +- OrangeRX FrSky (6ch) > > + > > +Many RC controllers is able to configure which stick goes to which channel. > > +This is also configurable in most simulators, so a matching is not necessary. > > + > > +Supported dongles > > +================== > > + > > +PhoenixRC > > +---------- > > + > > +The PhoenixRC has one HID compatible variant which is supported by this driver. > > +The controller has support for 8 analog channels. > > + > > +The driver is generating the following input event for on channels: > > + > > ++---------+----------------+ > > +| Channel | Event | > > ++=========+================+ > > +| 1 | ABS_Y | > > ++---------+----------------+ > > +| 2 | ABS_X | > > ++---------+----------------+ > > +| 3 | ABS_RY | > > ++---------+----------------+ > > +| 4 | ABS_RX | > > ++---------+----------------+ > > +| 5 | ABS_RUDDER | > > ++---------+----------------+ > > +| 6 | ABS_THROTTLE | > > ++---------+----------------+ > > +| 7 | ABS_Z | > > ++---------+----------------+ > > +| 8 | ABS_RZ | > > ++---------+----------------+ > > + > > +VRC2.0 > > +---------- > > +VRC2.0 is a controller for RC Cars. > > +The controller has support for 2 analog channels. > > + > > +The driver is generating the following input event for on channels: > > + > > ++---------+----------------+ > > +| Channel | Event | > > ++=========+================+ > > +| 1 | ABS_GAS | > > ++---------+----------------+ > > +| 2 | ABS_WHEEL | > > ++---------+----------------+ > > + > > +RealFlight > > +---------- > > + > > +This driver supports Realflight G4-G7 and above > > +The controller has support for 4 analog channels and two buttons. > > + > > +The driver is generating the following input event for on channels: > > + > > ++---------+----------------+ > > +| Channel | Event | > > ++=========+================+ > > +| 1 | ABS_Y | > > ++---------+----------------+ > > +| 2 | ABS_X | > > ++---------+----------------+ > > +| 3 | ABS_RY | > > ++---------+----------------+ > > +| 4 | ABS_RX | > > ++---------+----------------+ > > +| 5 | BTN_A | > > ++---------+----------------+ > > +| 6 | BTN_B | > > ++---------+----------------+ > > + > > +XTR+G2+FMS Controllers > > +-------------------------------- > > + > > +The controllers has support for 8 analog channels. > > + > > +The driver is generating the following input event for on channels: > > + > > ++---------+----------------+ > > +| Channel | Event | > > ++=========+================+ > > +| 1 | ABS_Y | > > ++---------+----------------+ > > +| 2 | ABS_X | > > ++---------+----------------+ > > +| 3 | ABS_RY | > > ++---------+----------------+ > > +| 4 | ABS_RX | > > ++---------+----------------+ > > +| 5 | ABS_RUDDER | > > ++---------+----------------+ > > +| 6 | ABS_THROTTLE | > > ++---------+----------------+ > > +| 7 | ABS_Z | > > ++---------+----------------+ > > +| 8 | ABS_RZ | > > ++---------+----------------+ > > + > > +OrangeRX > > +---------- > > + > > +The controllers has support for 6 analog channels. > > + > > +The driver is generating the following input event for on channels: > > + > > ++---------+----------------+ > > +| Channel | Event | > > ++=========+================+ > > +| 1 | ABS_Y | > > ++---------+----------------+ > > +| 2 | ABS_X | > > ++---------+----------------+ > > +| 3 | ABS_RY | > > ++---------+----------------+ > > +| 4 | ABS_RX | > > ++---------+----------------+ > > +| 5 | ABS_RUDDER | > > ++---------+----------------+ > > +| 6 | ABS_THROTTLE | > > ++---------+----------------+ > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > index 70da5931082f..d8313d36086c 100644 > > --- a/drivers/hid/Kconfig > > +++ b/drivers/hid/Kconfig > > @@ -957,6 +957,17 @@ config HID_RAZER > > Support for Razer devices that are not fully compliant with the > > HID standard. > > > > +config HID_RCSIM > > + tristate "RC Simulator Controllers" > > + depends on HID > > + help > > + Support for several HID compatible RC Simulator Controllers including > > + - Phoenix RC > > + - Car VRC2.0 > > + - Real Flight > > + - Aero Fly, FMS > > + - OrangeRX FrSky > > + > > config HID_PRIMAX > > tristate "Primax non-fully HID-compliant devices" > > depends on HID > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > > index cac2cbe26d11..85d50ab352ee 100644 > > --- a/drivers/hid/Makefile > > +++ b/drivers/hid/Makefile > > @@ -102,6 +102,7 @@ obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o > > obj-$(CONFIG_HID_PLAYSTATION) += hid-playstation.o > > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o > > obj-$(CONFIG_HID_RAZER) += hid-razer.o > > +obj-$(CONFIG_HID_RCSIM) += hid-rcsim.o > > General rule of thumbs, we try to name the drivers after their > vendors, unless we know we have a generic driver. > > Here, this driver seems to be really tied to a small set of devices, > and thus I don't think we can call it "generic". Got it. > > > obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o > > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o > > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > index d9eb676abe96..baf5f74d5bed 100644 > > --- a/drivers/hid/hid-ids.h > > +++ b/drivers/hid/hid-ids.h > > @@ -1381,6 +1381,11 @@ > > > > #define USB_VENDOR_ID_MULTIPLE_1781 0x1781 > > #define USB_DEVICE_ID_RAPHNET_4NES4SNES_OLD 0x0a9d > > +#define USB_DEVICE_ID_PHOENIXRC 0x0898 > > +#define USB_DEVICE_ID_REALFLIGHT 0x0e56 > > + > > +#define USB_VENDOR_ID_DIPLING 0x0B9B > > +#define USB_DEVICE_ID_DIPLING_RCCONTROLLER 0x4012 > > > > #define USB_VENDOR_ID_DRACAL_RAPHNET 0x289b > > #define USB_DEVICE_ID_RAPHNET_2NES2SNES 0x0002 > > diff --git a/drivers/hid/hid-rcsim.c b/drivers/hid/hid-rcsim.c > > new file mode 100644 > > index 000000000000..0f214cb5816a > > --- /dev/null > > +++ b/drivers/hid/hid-rcsim.c > > @@ -0,0 +1,315 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Driver for several HID compatible RC Simulator Controllers. > > + * Currently supported controllers are: > > + * > > + * - Phoenix RC (HID variant) (8ch) > > + * - Car VRC2.0 (2ch) > > + * - Real Flight G5/G6/G7 (6ch) > > + * - Aero Fly, FMS (8ch) > > + * - OrangeRX FrSky (6ch) > > + * > > + * Copyright (C) 2022 Marcus Folkesson <marcus.folkesson@gmail.com> > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/device.h> > > +#include <linux/input.h> > > +#include <linux/hid.h> > > +#include <linux/module.h> > > +#include <linux/usb.h> > > + > > +#include "hid-ids.h" > > + > > +/* > > + * Some of these VID/PID are probably "borrowed", so keep them locally and > > + * do not populate hid-ids.h with those. > > + */ > > + > > +/* PHOENIXRC Controlloer (HID variant) */ > > +#define PHOENIXRC_VID (USB_VENDOR_ID_MULTIPLE_1781) > > +#define PHOENIXRC_PID (USB_DEVICE_ID_PHOENIXRC) > > +#define PHOENIXRC_DSIZE (8) > > + > > +/* VRC2 Controlloer */ > > +#define VRC2_VID (0x07c0) > > +#define VRC2_PID (0x1125) > > +#define VRC2_DSIZE (7) > > + > > +/* Realflight G4-&7 and Above Controller */ > > +#define REALFLIGHT_VID (USB_VENDOR_ID_MULTIPLE_1781) > > +#define REALFLIGHT_PID (USB_DEVICE_ID_REALFLIGHT) > > +#define REALFLIGHT_DSIZE (8) > > + > > +#define REALFLIGHT_BTN_A BIT(0) > > +#define REALFLIGHT_BTN_B BIT(1) > > + > > +/* XTR+G2+FMS Controller */ > > +#define XTRG2FMS_VID (USB_VENDOR_ID_DIPLING) > > +#define XTRG2FMS_PID (USB_DEVICE_ID_DIPLING_RCCONTROLLER) > > +#define XTRG2FMS_DSIZE (8) > > + > > +#define XTRG2FMS_X_HI GENMASK(3, 2) > > +#define XTRG2FMS_Y_HI GENMASK(1, 0) > > +#define XTRG2FMS_RX_HI GENMASK(7, 6) > > +#define XTRG2FMS_RY_HI GENMASK(5, 4) > > +#define XTRG2FMS_ALT1_HI GENMASK(3, 2) > > +#define XTRG2FMS_ALT2_HI GENMASK(1, 0) > > + > > +/* OrangeRX FrSky */ > > +#define ORANGERX_VID (0x0451) > > +#define ORANGERX_PID (0x16a5) > > +#define ORANGERX_DSIZE (8) > > + > > +enum rcsim_controller { > > + PHOENIXRC, > > + VRC2, > > + REALFLIGHT, > > + XTRG2FMS, > > + ORANGERX > > +}; > > + > > +struct rcsim_priv { > > + struct hid_device *hdev; > > + struct input_dev *input; > > + enum rcsim_controller controller; > > + u8 alt; > > +}; > > + > > +static int rcsim_open(struct input_dev *dev) > > +{ > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > + > > + return hid_hw_open(priv->hdev); > > +} > > + > > +static void rcsim_close(struct input_dev *dev) > > +{ > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > + > > + hid_hw_close(priv->hdev); > > +} > > + > > +static int rcsim_setup_input(struct rcsim_priv *priv) > > +{ > > + struct input_dev *input; > > + > > + input = devm_input_allocate_device(&priv->hdev->dev); > > + if (!input) > > + return -ENOMEM; > > + > > + input->id.bustype = priv->hdev->bus; > > + input->id.vendor = priv->hdev->vendor; > > + input->id.product = priv->hdev->product; > > + input->id.version = priv->hdev->bus; > > + input->phys = priv->hdev->phys; > > + input->uniq = priv->hdev->uniq; > > + input->open = rcsim_open; > > + input->close = rcsim_close; > > + > > + input_set_drvdata(input, priv); > > + > > + switch (priv->controller) { > > + case PHOENIXRC: > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_Z, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > + input->name = "RC Simuator Controller PhoenixRC"; > > + break; > > + case VRC2: > > + input_set_abs_params(input, ABS_GAS, 0, 2048, 0, 0); > > + input_set_abs_params(input, ABS_WHEEL, 0, 2048, 0, 0); > > + input->name = "RC Simuator Controller VRC2.0"; > > + break; > > + case REALFLIGHT: > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > + input_set_capability(input, EV_KEY, BTN_A); > > + input_set_capability(input, EV_KEY, BTN_B); > > + input->name = "RC Simuator Controller Realflight"; > > + break; > > + case XTRG2FMS: > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_Z, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_RZ, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_RUDDER, 0, 1024, 0, 0); > > + input_set_abs_params(input, ABS_THROTTLE, 0, 1024, 0, 0); > > + input->name = "RC Simuator Controller AeroFly, FMS"; > > + priv->alt = 0; > > + break; > > + case ORANGERX: > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > + input->name = "RC Simuator Controller OrangeRX FrSky"; > > + break; > > + }; > > + > > + priv->input = input; > > + return input_register_device(priv->input); > > +} > > You are basically rewriting hid-input.c, which is suboptimal. Ouch. I will have a look at hid-input, thanks. > > I guess the report descriptor provided by these devices are basically > useless, and so you have to parse the reports yourself in the > raw_event callback. Yep. > > But instead of manually doing that, why not overwrite the report > descriptor (with .rdesc_fixup) and declare here all of the data that Do you mean .report_fixup? > needs to be exported. You could remove basically everything in this > driver by just providing a fixed report descriptor. What you are aiming for is to fixup the report descriptor and let the generic hid-raw driver handle the rest, or do I get you wrong? How is the report mapped to certain events then? I do read at [1] but it is not obvious how to put it together. Most drivers I've looked at that is using .report_fixup just fix broken reports. I guess these reports are not "broken", just.. odd? > > > + > > +static int rcsim_raw_event(struct hid_device *hdev, > > + struct hid_report *report, > > + u8 *raw_data, int size) > > +{ > > + struct rcsim_priv *priv = hid_get_drvdata(hdev); > > + u16 value; > > + > > + switch (priv->controller) { > > + case PHOENIXRC: > > + if (size != PHOENIXRC_DSIZE) > > + break; > > + > > + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > + input_report_abs(priv->input, ABS_Y, raw_data[0]); > > + input_report_abs(priv->input, ABS_RX, raw_data[4]); > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > + > > + /* Z and RZ are sent every other time */ > > + if (priv->alt) > > + input_report_abs(priv->input, ABS_Z, raw_data[7]); > > + else > > + input_report_abs(priv->input, ABS_RZ, raw_data[7]); > > + > > + priv->alt ^= 1; > > + break; > > + case VRC2: > > + if (size != VRC2_DSIZE) > > + break; > > + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); > > + input_report_abs(priv->input, ABS_GAS, value); > > + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); > > + input_report_abs(priv->input, ABS_WHEEL, value); > > + break; > > + case REALFLIGHT: > > + if (size != REALFLIGHT_DSIZE) > > + break; > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > + input_report_abs(priv->input, ABS_Y, raw_data[1]); > > + input_report_abs(priv->input, ABS_RX, raw_data[5]); > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > + input_report_abs(priv->input, ABS_MISC, raw_data[4]); > > + input_report_key(priv->input, BTN_A, > > + raw_data[7] & REALFLIGHT_BTN_A); > > + input_report_key(priv->input, BTN_B, > > + raw_data[7] & REALFLIGHT_BTN_B); > > + break; > > + case XTRG2FMS: > > + if (size != XTRG2FMS_DSIZE) > > + break; > > + > > + /* X, RX, Y and RY are sent every time */ > > + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); > > + value = (value << 8) | raw_data[1]; > > + input_report_abs(priv->input, ABS_X, value); > > + > > + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); > > + value = (value << 8) | raw_data[2]; > > + input_report_abs(priv->input, ABS_Y, value); > > + > > + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); > > + value = (value << 8) | raw_data[0]; > > + input_report_abs(priv->input, ABS_RX, value); > > + > > + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); > > + value = (value << 8) | raw_data[4]; > > + input_report_abs(priv->input, ABS_RY, value); > > + > > + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ > > + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > > + value = (value << 8) | raw_data[6]; > > + if (priv->alt) > > + input_report_abs(priv->input, ABS_Z, value); > > + else > > + input_report_abs(priv->input, ABS_RUDDER, value); > > + > > + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > > + value = (value << 8) | raw_data[5]; > > + if (priv->alt) > > + input_report_abs(priv->input, ABS_RZ, value); > > + else > > + input_report_abs(priv->input, ABS_THROTTLE, value); > > + > > + priv->alt ^= 1; > > + break; > > + case ORANGERX: > > + if (size != ORANGERX_DSIZE) > > + break; > > + input_report_abs(priv->input, ABS_X, raw_data[0]); > > + input_report_abs(priv->input, ABS_Y, raw_data[2]); > > + input_report_abs(priv->input, ABS_RX, raw_data[3]); > > + input_report_abs(priv->input, ABS_RY, raw_data[1]); > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > + break; > > + }; > > + > > + input_sync(priv->input); > > + return 0; > > +} > > + > > +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) > > +{ > > + struct device *dev = &hdev->dev; > > + struct rcsim_priv *priv; > > + int ret; > > + > > + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) > > + return -ENODEV; > > You are not accessing anything in the USB stack, so there is no need > to prevent regression tests that could inject uhid devices to your > drivers. Ok, thanks. > > Cheers, > Benjamin > Best regards, Marcus Folkesson [1] https://www.usb.org/hid [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-23 15:18 ` Marcus Folkesson @ 2022-08-23 16:43 ` Benjamin Tissoires 2022-08-25 6:48 ` Marcus Folkesson 2022-08-28 14:18 ` Marcus Folkesson 0 siblings, 2 replies; 16+ messages in thread From: Benjamin Tissoires @ 2022-08-23 16:43 UTC (permalink / raw) To: Marcus Folkesson Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER On Tue, Aug 23, 2022 at 5:13 PM Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > Thank you Benjamin, > > On Tue, Aug 23, 2022 at 11:49:59AM +0200, Benjamin Tissoires wrote: > > Hi Marcus, > > > > [and sorry for the delay in the review of your patches] > > > > On Mon, Aug 22, 2022 at 8:04 AM Marcus Folkesson > > <marcus.folkesson@gmail.com> wrote: > > > > > > Several RC Simulator Controllers are HID compliant with similar > > > interface. > > > > > > Add support for these controllers: > > > - Phoenix RC (HID variant) > > > - Car VRC2.0 > > > - Real Flight G5/G6/G7 > > > - Aero Fly, FMS > > > - OrangeRX FrSky > > > > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > > > --- > > > Documentation/hid/index.rst | 1 + > > > Documentation/hid/rcsim.rst | 142 ++++++++++++++++ > > > drivers/hid/Kconfig | 11 ++ > > > drivers/hid/Makefile | 1 + > > > drivers/hid/hid-ids.h | 5 + > > > drivers/hid/hid-rcsim.c | 315 ++++++++++++++++++++++++++++++++++++ > > > 6 files changed, 475 insertions(+) > > > create mode 100644 Documentation/hid/rcsim.rst > > > create mode 100644 drivers/hid/hid-rcsim.c > > > > > > diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst > > > index e50f513c579c..e5813d264f37 100644 > > > --- a/Documentation/hid/index.rst > > > +++ b/Documentation/hid/index.rst > > > @@ -17,3 +17,4 @@ Human Interface Devices (HID) > > > hid-alps > > > intel-ish-hid > > > amd-sfh-hid > > > + rcsim > > > diff --git a/Documentation/hid/rcsim.rst b/Documentation/hid/rcsim.rst > > > new file mode 100644 > > > index 000000000000..1a031f7189cb > > > --- /dev/null > > > +++ b/Documentation/hid/rcsim.rst > > > @@ -0,0 +1,142 @@ > > > +.. SPDX-License-Identifier: GPL-2.0 > > > + > > > +================================= > > > +rcsim - RC Simulator Controllers > > > +================================= > > > + > > > +:Author: Marcus Folkesson <marcus.folkesson@gmail.com> > > > + > > > +This driver let you use your own RC controller plugged > > > +into your computer using an HID compatible USB dongle. > > > + > > > +There are several HID compatible USB dongles from different > > > +vendors. The driver currently supports: > > > + > > > +- Phoenix RC (HID variant) (8ch) > > > +- Car VRC2.0 (2ch) > > > +- Real Flight G5/G6/G7 (6ch) > > > +- Aero Fly, FMS (8ch) > > > +- OrangeRX FrSky (6ch) > > > + > > > +Many RC controllers is able to configure which stick goes to which channel. > > > +This is also configurable in most simulators, so a matching is not necessary. > > > + > > > +Supported dongles > > > +================== > > > + > > > +PhoenixRC > > > +---------- > > > + > > > +The PhoenixRC has one HID compatible variant which is supported by this driver. > > > +The controller has support for 8 analog channels. > > > + > > > +The driver is generating the following input event for on channels: > > > + > > > ++---------+----------------+ > > > +| Channel | Event | > > > ++=========+================+ > > > +| 1 | ABS_Y | > > > ++---------+----------------+ > > > +| 2 | ABS_X | > > > ++---------+----------------+ > > > +| 3 | ABS_RY | > > > ++---------+----------------+ > > > +| 4 | ABS_RX | > > > ++---------+----------------+ > > > +| 5 | ABS_RUDDER | > > > ++---------+----------------+ > > > +| 6 | ABS_THROTTLE | > > > ++---------+----------------+ > > > +| 7 | ABS_Z | > > > ++---------+----------------+ > > > +| 8 | ABS_RZ | > > > ++---------+----------------+ > > > + > > > +VRC2.0 > > > +---------- > > > +VRC2.0 is a controller for RC Cars. > > > +The controller has support for 2 analog channels. > > > + > > > +The driver is generating the following input event for on channels: > > > + > > > ++---------+----------------+ > > > +| Channel | Event | > > > ++=========+================+ > > > +| 1 | ABS_GAS | > > > ++---------+----------------+ > > > +| 2 | ABS_WHEEL | > > > ++---------+----------------+ > > > + > > > +RealFlight > > > +---------- > > > + > > > +This driver supports Realflight G4-G7 and above > > > +The controller has support for 4 analog channels and two buttons. > > > + > > > +The driver is generating the following input event for on channels: > > > + > > > ++---------+----------------+ > > > +| Channel | Event | > > > ++=========+================+ > > > +| 1 | ABS_Y | > > > ++---------+----------------+ > > > +| 2 | ABS_X | > > > ++---------+----------------+ > > > +| 3 | ABS_RY | > > > ++---------+----------------+ > > > +| 4 | ABS_RX | > > > ++---------+----------------+ > > > +| 5 | BTN_A | > > > ++---------+----------------+ > > > +| 6 | BTN_B | > > > ++---------+----------------+ > > > + > > > +XTR+G2+FMS Controllers > > > +-------------------------------- > > > + > > > +The controllers has support for 8 analog channels. > > > + > > > +The driver is generating the following input event for on channels: > > > + > > > ++---------+----------------+ > > > +| Channel | Event | > > > ++=========+================+ > > > +| 1 | ABS_Y | > > > ++---------+----------------+ > > > +| 2 | ABS_X | > > > ++---------+----------------+ > > > +| 3 | ABS_RY | > > > ++---------+----------------+ > > > +| 4 | ABS_RX | > > > ++---------+----------------+ > > > +| 5 | ABS_RUDDER | > > > ++---------+----------------+ > > > +| 6 | ABS_THROTTLE | > > > ++---------+----------------+ > > > +| 7 | ABS_Z | > > > ++---------+----------------+ > > > +| 8 | ABS_RZ | > > > ++---------+----------------+ > > > + > > > +OrangeRX > > > +---------- > > > + > > > +The controllers has support for 6 analog channels. > > > + > > > +The driver is generating the following input event for on channels: > > > + > > > ++---------+----------------+ > > > +| Channel | Event | > > > ++=========+================+ > > > +| 1 | ABS_Y | > > > ++---------+----------------+ > > > +| 2 | ABS_X | > > > ++---------+----------------+ > > > +| 3 | ABS_RY | > > > ++---------+----------------+ > > > +| 4 | ABS_RX | > > > ++---------+----------------+ > > > +| 5 | ABS_RUDDER | > > > ++---------+----------------+ > > > +| 6 | ABS_THROTTLE | > > > ++---------+----------------+ > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > > index 70da5931082f..d8313d36086c 100644 > > > --- a/drivers/hid/Kconfig > > > +++ b/drivers/hid/Kconfig > > > @@ -957,6 +957,17 @@ config HID_RAZER > > > Support for Razer devices that are not fully compliant with the > > > HID standard. > > > > > > +config HID_RCSIM > > > + tristate "RC Simulator Controllers" > > > + depends on HID > > > + help > > > + Support for several HID compatible RC Simulator Controllers including > > > + - Phoenix RC > > > + - Car VRC2.0 > > > + - Real Flight > > > + - Aero Fly, FMS > > > + - OrangeRX FrSky > > > + > > > config HID_PRIMAX > > > tristate "Primax non-fully HID-compliant devices" > > > depends on HID > > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > > > index cac2cbe26d11..85d50ab352ee 100644 > > > --- a/drivers/hid/Makefile > > > +++ b/drivers/hid/Makefile > > > @@ -102,6 +102,7 @@ obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o > > > obj-$(CONFIG_HID_PLAYSTATION) += hid-playstation.o > > > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o > > > obj-$(CONFIG_HID_RAZER) += hid-razer.o > > > +obj-$(CONFIG_HID_RCSIM) += hid-rcsim.o > > > > General rule of thumbs, we try to name the drivers after their > > vendors, unless we know we have a generic driver. > > > > Here, this driver seems to be really tied to a small set of devices, > > and thus I don't think we can call it "generic". > > Got it. > > > > > > obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o > > > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o > > > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > > index d9eb676abe96..baf5f74d5bed 100644 > > > --- a/drivers/hid/hid-ids.h > > > +++ b/drivers/hid/hid-ids.h > > > @@ -1381,6 +1381,11 @@ > > > > > > #define USB_VENDOR_ID_MULTIPLE_1781 0x1781 > > > #define USB_DEVICE_ID_RAPHNET_4NES4SNES_OLD 0x0a9d > > > +#define USB_DEVICE_ID_PHOENIXRC 0x0898 > > > +#define USB_DEVICE_ID_REALFLIGHT 0x0e56 > > > + > > > +#define USB_VENDOR_ID_DIPLING 0x0B9B > > > +#define USB_DEVICE_ID_DIPLING_RCCONTROLLER 0x4012 > > > > > > #define USB_VENDOR_ID_DRACAL_RAPHNET 0x289b > > > #define USB_DEVICE_ID_RAPHNET_2NES2SNES 0x0002 > > > diff --git a/drivers/hid/hid-rcsim.c b/drivers/hid/hid-rcsim.c > > > new file mode 100644 > > > index 000000000000..0f214cb5816a > > > --- /dev/null > > > +++ b/drivers/hid/hid-rcsim.c > > > @@ -0,0 +1,315 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Driver for several HID compatible RC Simulator Controllers. > > > + * Currently supported controllers are: > > > + * > > > + * - Phoenix RC (HID variant) (8ch) > > > + * - Car VRC2.0 (2ch) > > > + * - Real Flight G5/G6/G7 (6ch) > > > + * - Aero Fly, FMS (8ch) > > > + * - OrangeRX FrSky (6ch) > > > + * > > > + * Copyright (C) 2022 Marcus Folkesson <marcus.folkesson@gmail.com> > > > + */ > > > + > > > +#include <linux/bitfield.h> > > > +#include <linux/device.h> > > > +#include <linux/input.h> > > > +#include <linux/hid.h> > > > +#include <linux/module.h> > > > +#include <linux/usb.h> > > > + > > > +#include "hid-ids.h" > > > + > > > +/* > > > + * Some of these VID/PID are probably "borrowed", so keep them locally and > > > + * do not populate hid-ids.h with those. > > > + */ > > > + > > > +/* PHOENIXRC Controlloer (HID variant) */ > > > +#define PHOENIXRC_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > +#define PHOENIXRC_PID (USB_DEVICE_ID_PHOENIXRC) > > > +#define PHOENIXRC_DSIZE (8) > > > + > > > +/* VRC2 Controlloer */ > > > +#define VRC2_VID (0x07c0) > > > +#define VRC2_PID (0x1125) > > > +#define VRC2_DSIZE (7) > > > + > > > +/* Realflight G4-&7 and Above Controller */ > > > +#define REALFLIGHT_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > +#define REALFLIGHT_PID (USB_DEVICE_ID_REALFLIGHT) > > > +#define REALFLIGHT_DSIZE (8) > > > + > > > +#define REALFLIGHT_BTN_A BIT(0) > > > +#define REALFLIGHT_BTN_B BIT(1) > > > + > > > +/* XTR+G2+FMS Controller */ > > > +#define XTRG2FMS_VID (USB_VENDOR_ID_DIPLING) > > > +#define XTRG2FMS_PID (USB_DEVICE_ID_DIPLING_RCCONTROLLER) > > > +#define XTRG2FMS_DSIZE (8) > > > + > > > +#define XTRG2FMS_X_HI GENMASK(3, 2) > > > +#define XTRG2FMS_Y_HI GENMASK(1, 0) > > > +#define XTRG2FMS_RX_HI GENMASK(7, 6) > > > +#define XTRG2FMS_RY_HI GENMASK(5, 4) > > > +#define XTRG2FMS_ALT1_HI GENMASK(3, 2) > > > +#define XTRG2FMS_ALT2_HI GENMASK(1, 0) > > > + > > > +/* OrangeRX FrSky */ > > > +#define ORANGERX_VID (0x0451) > > > +#define ORANGERX_PID (0x16a5) > > > +#define ORANGERX_DSIZE (8) > > > + > > > +enum rcsim_controller { > > > + PHOENIXRC, > > > + VRC2, > > > + REALFLIGHT, > > > + XTRG2FMS, > > > + ORANGERX > > > +}; > > > + > > > +struct rcsim_priv { > > > + struct hid_device *hdev; > > > + struct input_dev *input; > > > + enum rcsim_controller controller; > > > + u8 alt; > > > +}; > > > + > > > +static int rcsim_open(struct input_dev *dev) > > > +{ > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > + > > > + return hid_hw_open(priv->hdev); > > > +} > > > + > > > +static void rcsim_close(struct input_dev *dev) > > > +{ > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > + > > > + hid_hw_close(priv->hdev); > > > +} > > > + > > > +static int rcsim_setup_input(struct rcsim_priv *priv) > > > +{ > > > + struct input_dev *input; > > > + > > > + input = devm_input_allocate_device(&priv->hdev->dev); > > > + if (!input) > > > + return -ENOMEM; > > > + > > > + input->id.bustype = priv->hdev->bus; > > > + input->id.vendor = priv->hdev->vendor; > > > + input->id.product = priv->hdev->product; > > > + input->id.version = priv->hdev->bus; > > > + input->phys = priv->hdev->phys; > > > + input->uniq = priv->hdev->uniq; > > > + input->open = rcsim_open; > > > + input->close = rcsim_close; > > > + > > > + input_set_drvdata(input, priv); > > > + > > > + switch (priv->controller) { > > > + case PHOENIXRC: > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_Z, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > + input->name = "RC Simuator Controller PhoenixRC"; > > > + break; > > > + case VRC2: > > > + input_set_abs_params(input, ABS_GAS, 0, 2048, 0, 0); > > > + input_set_abs_params(input, ABS_WHEEL, 0, 2048, 0, 0); > > > + input->name = "RC Simuator Controller VRC2.0"; > > > + break; > > > + case REALFLIGHT: > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > + input_set_capability(input, EV_KEY, BTN_A); > > > + input_set_capability(input, EV_KEY, BTN_B); > > > + input->name = "RC Simuator Controller Realflight"; > > > + break; > > > + case XTRG2FMS: > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_Z, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_RZ, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_RUDDER, 0, 1024, 0, 0); > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 1024, 0, 0); > > > + input->name = "RC Simuator Controller AeroFly, FMS"; > > > + priv->alt = 0; > > > + break; > > > + case ORANGERX: > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > + input->name = "RC Simuator Controller OrangeRX FrSky"; > > > + break; > > > + }; > > > + > > > + priv->input = input; > > > + return input_register_device(priv->input); > > > +} > > > > You are basically rewriting hid-input.c, which is suboptimal. > > Ouch. I will have a look at hid-input, thanks. > > > > > I guess the report descriptor provided by these devices are basically > > useless, and so you have to parse the reports yourself in the > > raw_event callback. > > Yep. > > > > > But instead of manually doing that, why not overwrite the report > > descriptor (with .rdesc_fixup) and declare here all of the data that > Do you mean .report_fixup? yes, sorry :/ > > > needs to be exported. You could remove basically everything in this > > driver by just providing a fixed report descriptor. > > What you are aiming for is to fixup the report descriptor and let the > generic hid-raw driver handle the rest, or do I get you wrong? yep, exactly > > How is the report mapped to certain events then? Have a look at hid_configure_usage in hid-input.c [3]. Most of HID events are mapped to input events with a one to one mapping. > > I do read at [1] but it is not obvious how to put it together. > Most drivers I've looked at that is using .report_fixup just fix broken > reports. I guess these reports are not "broken", just.. odd? Have a look at [2], lots of full report descriptors :) And in your case, the reports are incomplete, not odd. > > > > > > > + > > > +static int rcsim_raw_event(struct hid_device *hdev, > > > + struct hid_report *report, > > > + u8 *raw_data, int size) > > > +{ > > > + struct rcsim_priv *priv = hid_get_drvdata(hdev); > > > + u16 value; > > > + > > > + switch (priv->controller) { > > > + case PHOENIXRC: > > > + if (size != PHOENIXRC_DSIZE) > > > + break; > > > + > > > + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > + input_report_abs(priv->input, ABS_Y, raw_data[0]); > > > + input_report_abs(priv->input, ABS_RX, raw_data[4]); > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > + > > > + /* Z and RZ are sent every other time */ > > > + if (priv->alt) > > > + input_report_abs(priv->input, ABS_Z, raw_data[7]); > > > + else > > > + input_report_abs(priv->input, ABS_RZ, raw_data[7]); > > > + > > > + priv->alt ^= 1; > > > + break; > > > + case VRC2: > > > + if (size != VRC2_DSIZE) > > > + break; > > > + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); > > > + input_report_abs(priv->input, ABS_GAS, value); > > > + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); > > > + input_report_abs(priv->input, ABS_WHEEL, value); > > > + break; > > > + case REALFLIGHT: > > > + if (size != REALFLIGHT_DSIZE) > > > + break; > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > + input_report_abs(priv->input, ABS_Y, raw_data[1]); > > > + input_report_abs(priv->input, ABS_RX, raw_data[5]); > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > + input_report_abs(priv->input, ABS_MISC, raw_data[4]); > > > + input_report_key(priv->input, BTN_A, > > > + raw_data[7] & REALFLIGHT_BTN_A); > > > + input_report_key(priv->input, BTN_B, > > > + raw_data[7] & REALFLIGHT_BTN_B); > > > + break; > > > + case XTRG2FMS: > > > + if (size != XTRG2FMS_DSIZE) > > > + break; > > > + > > > + /* X, RX, Y and RY are sent every time */ > > > + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); > > > + value = (value << 8) | raw_data[1]; > > > + input_report_abs(priv->input, ABS_X, value); > > > + > > > + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); > > > + value = (value << 8) | raw_data[2]; > > > + input_report_abs(priv->input, ABS_Y, value); > > > + > > > + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); > > > + value = (value << 8) | raw_data[0]; > > > + input_report_abs(priv->input, ABS_RX, value); > > > + > > > + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); > > > + value = (value << 8) | raw_data[4]; > > > + input_report_abs(priv->input, ABS_RY, value); > > > + > > > + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ > > > + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > > > + value = (value << 8) | raw_data[6]; > > > + if (priv->alt) > > > + input_report_abs(priv->input, ABS_Z, value); > > > + else > > > + input_report_abs(priv->input, ABS_RUDDER, value); > > > + > > > + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > > > + value = (value << 8) | raw_data[5]; > > > + if (priv->alt) > > > + input_report_abs(priv->input, ABS_RZ, value); > > > + else > > > + input_report_abs(priv->input, ABS_THROTTLE, value); > > > + > > > + priv->alt ^= 1; > > > + break; > > > + case ORANGERX: > > > + if (size != ORANGERX_DSIZE) > > > + break; > > > + input_report_abs(priv->input, ABS_X, raw_data[0]); > > > + input_report_abs(priv->input, ABS_Y, raw_data[2]); > > > + input_report_abs(priv->input, ABS_RX, raw_data[3]); > > > + input_report_abs(priv->input, ABS_RY, raw_data[1]); > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > + break; > > > + }; > > > + > > > + input_sync(priv->input); > > > + return 0; > > > +} > > > + > > > +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) > > > +{ > > > + struct device *dev = &hdev->dev; > > > + struct rcsim_priv *priv; > > > + int ret; > > > + > > > + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) > > > + return -ENODEV; > > > > You are not accessing anything in the USB stack, so there is no need > > to prevent regression tests that could inject uhid devices to your > > drivers. > > Ok, thanks. > > > > > Cheers, > > Benjamin > > > > Best regards, > Marcus Folkesson > > [1] https://www.usb.org/hid > If you need help in writing report descriptors, I can give you some, but the easiest might be for you to start from the report descriptor in hid-sony.c. I used to have a tool to dynamically write a report descriptor, but I'm not sure it still works... FYI, I just re-read rcsim_raw_event() and there is stuff that would require more than just a report descriptor fixup (the fact that some data is sent every other report is not good and will need some manual handling though). Cheers, Benjamin [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-uclogic-rdesc.c [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-input.c#n817 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-23 16:43 ` Benjamin Tissoires @ 2022-08-25 6:48 ` Marcus Folkesson 2022-08-30 12:45 ` Benjamin Tissoires 2022-08-28 14:18 ` Marcus Folkesson 1 sibling, 1 reply; 16+ messages in thread From: Marcus Folkesson @ 2022-08-25 6:48 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER [-- Attachment #1: Type: text/plain, Size: 10706 bytes --] Hi Benjamin, Thank you! I have a few questions regarding the report descriptor, please see below. On Tue, Aug 23, 2022 at 06:43:47PM +0200, Benjamin Tissoires wrote: > On Tue, Aug 23, 2022 at 5:13 PM Marcus Folkesson > <marcus.folkesson@gmail.com> wrote: > > > > Thank you Benjamin, > > > > On Tue, Aug 23, 2022 at 11:49:59AM +0200, Benjamin Tissoires wrote: > > > Hi Marcus, > > > > > > [and sorry for the delay in the review of your patches] > > > > > > On Mon, Aug 22, 2022 at 8:04 AM Marcus Folkesson > > > <marcus.folkesson@gmail.com> wrote: [...] > > > > + }; > > > > + > > > > + priv->input = input; > > > > + return input_register_device(priv->input); > > > > +} > > > > > > You are basically rewriting hid-input.c, which is suboptimal. > > > > Ouch. I will have a look at hid-input, thanks. > > > > > > > > I guess the report descriptor provided by these devices are basically > > > useless, and so you have to parse the reports yourself in the > > > raw_event callback. > > > > Yep. > > > > > > > > But instead of manually doing that, why not overwrite the report > > > descriptor (with .rdesc_fixup) and declare here all of the data that > > Do you mean .report_fixup? > > yes, sorry :/ > > > > > > needs to be exported. You could remove basically everything in this > > > driver by just providing a fixed report descriptor. > > > > What you are aiming for is to fixup the report descriptor and let the > > generic hid-raw driver handle the rest, or do I get you wrong? > > yep, exactly > > > > > How is the report mapped to certain events then? > > Have a look at hid_configure_usage in hid-input.c [3]. Most of HID > events are mapped to input events with a one to one mapping. > > > > > I do read at [1] but it is not obvious how to put it together. > > Most drivers I've looked at that is using .report_fixup just fix broken > > reports. I guess these reports are not "broken", just.. odd? > > Have a look at [2], lots of full report descriptors :) > > And in your case, the reports are incomplete, not odd. Got it. I've parsed [4] the report descriptor for VRC2, and I guess it does not looks that good: 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) 0x09, 0x00, // Usage (Undefined) 0xA1, 0x01, // Collection (Application) 0x15, 0x00, // Logical Minimum (0) 0x25, 0x01, // Logical Maximum (1) 0x75, 0x01, // Report Size (1) 0x05, 0x09, // Usage Page (Button) 0x19, 0x01, // Usage Minimum (0x01) 0x29, 0x3F, // Usage Maximum (0x3F) 0x95, 0x40, // Report Count (64) 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection The Usage should rather be Joystick than undefined, the other fields does also looks wrong to me. The data for each axis (WHEEL and GAS) is 11 bit long (Logical maximum 2047 ?), how does the report descriptor map to the actual data in terms of offset, order and length? > > > > > > > > > > > > + > > > > +static int rcsim_raw_event(struct hid_device *hdev, > > > > + struct hid_report *report, > > > > + u8 *raw_data, int size) > > > > +{ > > > > + struct rcsim_priv *priv = hid_get_drvdata(hdev); > > > > + u16 value; > > > > + > > > > + switch (priv->controller) { > > > > + case PHOENIXRC: > > > > + if (size != PHOENIXRC_DSIZE) > > > > + break; > > > > + > > > > + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > + input_report_abs(priv->input, ABS_Y, raw_data[0]); > > > > + input_report_abs(priv->input, ABS_RX, raw_data[4]); > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > + > > > > + /* Z and RZ are sent every other time */ > > > > + if (priv->alt) > > > > + input_report_abs(priv->input, ABS_Z, raw_data[7]); > > > > + else > > > > + input_report_abs(priv->input, ABS_RZ, raw_data[7]); > > > > + > > > > + priv->alt ^= 1; > > > > + break; > > > > + case VRC2: > > > > + if (size != VRC2_DSIZE) > > > > + break; > > > > + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); > > > > + input_report_abs(priv->input, ABS_GAS, value); > > > > + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); > > > > + input_report_abs(priv->input, ABS_WHEEL, value); > > > > + break; > > > > + case REALFLIGHT: > > > > + if (size != REALFLIGHT_DSIZE) > > > > + break; > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > + input_report_abs(priv->input, ABS_Y, raw_data[1]); > > > > + input_report_abs(priv->input, ABS_RX, raw_data[5]); > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > + input_report_abs(priv->input, ABS_MISC, raw_data[4]); > > > > + input_report_key(priv->input, BTN_A, > > > > + raw_data[7] & REALFLIGHT_BTN_A); > > > > + input_report_key(priv->input, BTN_B, > > > > + raw_data[7] & REALFLIGHT_BTN_B); > > > > + break; > > > > + case XTRG2FMS: > > > > + if (size != XTRG2FMS_DSIZE) > > > > + break; > > > > + > > > > + /* X, RX, Y and RY are sent every time */ > > > > + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); > > > > + value = (value << 8) | raw_data[1]; > > > > + input_report_abs(priv->input, ABS_X, value); > > > > + > > > > + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); > > > > + value = (value << 8) | raw_data[2]; > > > > + input_report_abs(priv->input, ABS_Y, value); > > > > + > > > > + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); > > > > + value = (value << 8) | raw_data[0]; > > > > + input_report_abs(priv->input, ABS_RX, value); > > > > + > > > > + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); > > > > + value = (value << 8) | raw_data[4]; > > > > + input_report_abs(priv->input, ABS_RY, value); > > > > + > > > > + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ > > > > + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > > > > + value = (value << 8) | raw_data[6]; > > > > + if (priv->alt) > > > > + input_report_abs(priv->input, ABS_Z, value); > > > > + else > > > > + input_report_abs(priv->input, ABS_RUDDER, value); > > > > + > > > > + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > > > > + value = (value << 8) | raw_data[5]; > > > > + if (priv->alt) > > > > + input_report_abs(priv->input, ABS_RZ, value); > > > > + else > > > > + input_report_abs(priv->input, ABS_THROTTLE, value); > > > > + > > > > + priv->alt ^= 1; > > > > + break; > > > > + case ORANGERX: > > > > + if (size != ORANGERX_DSIZE) > > > > + break; > > > > + input_report_abs(priv->input, ABS_X, raw_data[0]); > > > > + input_report_abs(priv->input, ABS_Y, raw_data[2]); > > > > + input_report_abs(priv->input, ABS_RX, raw_data[3]); > > > > + input_report_abs(priv->input, ABS_RY, raw_data[1]); > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > + break; > > > > + }; > > > > + > > > > + input_sync(priv->input); > > > > + return 0; > > > > +} > > > > + > > > > +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) > > > > +{ > > > > + struct device *dev = &hdev->dev; > > > > + struct rcsim_priv *priv; > > > > + int ret; > > > > + > > > > + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) > > > > + return -ENODEV; > > > > > > You are not accessing anything in the USB stack, so there is no need > > > to prevent regression tests that could inject uhid devices to your > > > drivers. > > > > Ok, thanks. > > > > > > > > Cheers, > > > Benjamin > > > > > > > Best regards, > > Marcus Folkesson > > > > [1] https://www.usb.org/hid > > > > If you need help in writing report descriptors, I can give you some, > but the easiest might be for you to start from the report descriptor > in hid-sony.c. I used to have a tool to dynamically write a report > descriptor, but I'm not sure it still works... I think at least some advice would be great :-) The VRC2 would be the most simple of those, it only has 2 axis with resolution of 11-bit. If you have time, would you please give some advice what a report descriptor would look like and I could probably come up with something for the others. > > FYI, I just re-read rcsim_raw_event() and there is stuff that would > require more than just a report descriptor fixup (the fact that some > data is sent every other report is not good and will need some manual > handling though). Is the fact that more than one button share the same byte hard to describe in the report? value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); value = (value << 8) | raw_data[6]; ... value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); value = (value << 8) | raw_data[5]; > > Cheers, > Benjamin > Best regards Marcus Folkesson > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-uclogic-rdesc.c > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-input.c#n817 [4] https://eleccelerator.com/usbdescreqparser/ [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-25 6:48 ` Marcus Folkesson @ 2022-08-30 12:45 ` Benjamin Tissoires 2022-09-15 7:34 ` Marcus Folkesson 0 siblings, 1 reply; 16+ messages in thread From: Benjamin Tissoires @ 2022-08-30 12:45 UTC (permalink / raw) To: Marcus Folkesson Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER On Thu, Aug 25, 2022 at 8:44 AM Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > Hi Benjamin, > > Thank you! > I have a few questions regarding the report descriptor, please see > below. > > On Tue, Aug 23, 2022 at 06:43:47PM +0200, Benjamin Tissoires wrote: [...] > > Got it. > I've parsed [4] the report descriptor for VRC2, and I guess it does not looks > that good: > > 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) > 0x09, 0x00, // Usage (Undefined) > 0xA1, 0x01, // Collection (Application) > 0x15, 0x00, // Logical Minimum (0) > 0x25, 0x01, // Logical Maximum (1) > 0x75, 0x01, // Report Size (1) > 0x05, 0x09, // Usage Page (Button) > 0x19, 0x01, // Usage Minimum (0x01) > 0x29, 0x3F, // Usage Maximum (0x3F) > 0x95, 0x40, // Report Count (64) > 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) > 0xC0, // End Collection > > The Usage should rather be Joystick than undefined, the other > fields does also looks wrong to me. yep, as you saw already :) > > The data for each axis (WHEEL and GAS) is 11 bit long (Logical maximum > 2047 ?), how does the report descriptor map to the actual data in terms > of offset, order and length? You probably already got it, but it all depends on how much data is used on the wire: - if you have 2 bytes used per each value, then the report size will be 16 and you set the logical max to 2047 - if the values are packed, then the report size is 11, and then you probably have to add padding *after* the 22 bits. > > > > > > > > > > > > > > > > > > + > > > > > +static int rcsim_raw_event(struct hid_device *hdev, > > > > > + struct hid_report *report, > > > > > + u8 *raw_data, int size) > > > > > +{ > > > > > + struct rcsim_priv *priv = hid_get_drvdata(hdev); > > > > > + u16 value; > > > > > + > > > > > + switch (priv->controller) { > > > > > + case PHOENIXRC: > > > > > + if (size != PHOENIXRC_DSIZE) > > > > > + break; > > > > > + > > > > > + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ > > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[0]); > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[4]); > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > > + > > > > > + /* Z and RZ are sent every other time */ > > > > > + if (priv->alt) > > > > > + input_report_abs(priv->input, ABS_Z, raw_data[7]); > > > > > + else > > > > > + input_report_abs(priv->input, ABS_RZ, raw_data[7]); > > > > > + > > > > > + priv->alt ^= 1; > > > > > + break; > > > > > + case VRC2: > > > > > + if (size != VRC2_DSIZE) > > > > > + break; > > > > > + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); > > > > > + input_report_abs(priv->input, ABS_GAS, value); > > > > > + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); > > > > > + input_report_abs(priv->input, ABS_WHEEL, value); > > > > > + break; > > > > > + case REALFLIGHT: > > > > > + if (size != REALFLIGHT_DSIZE) > > > > > + break; > > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[1]); > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[5]); > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > > + input_report_abs(priv->input, ABS_MISC, raw_data[4]); > > > > > + input_report_key(priv->input, BTN_A, > > > > > + raw_data[7] & REALFLIGHT_BTN_A); > > > > > + input_report_key(priv->input, BTN_B, > > > > > + raw_data[7] & REALFLIGHT_BTN_B); > > > > > + break; > > > > > + case XTRG2FMS: > > > > > + if (size != XTRG2FMS_DSIZE) > > > > > + break; > > > > > + > > > > > + /* X, RX, Y and RY are sent every time */ > > > > > + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); > > > > > + value = (value << 8) | raw_data[1]; > > > > > + input_report_abs(priv->input, ABS_X, value); > > > > > + > > > > > + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); > > > > > + value = (value << 8) | raw_data[2]; > > > > > + input_report_abs(priv->input, ABS_Y, value); > > > > > + > > > > > + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); > > > > > + value = (value << 8) | raw_data[0]; > > > > > + input_report_abs(priv->input, ABS_RX, value); > > > > > + > > > > > + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); > > > > > + value = (value << 8) | raw_data[4]; > > > > > + input_report_abs(priv->input, ABS_RY, value); > > > > > + > > > > > + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ > > > > > + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > > > > > + value = (value << 8) | raw_data[6]; > > > > > + if (priv->alt) > > > > > + input_report_abs(priv->input, ABS_Z, value); > > > > > + else > > > > > + input_report_abs(priv->input, ABS_RUDDER, value); > > > > > + > > > > > + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > > > > > + value = (value << 8) | raw_data[5]; > > > > > + if (priv->alt) > > > > > + input_report_abs(priv->input, ABS_RZ, value); > > > > > + else > > > > > + input_report_abs(priv->input, ABS_THROTTLE, value); > > > > > + > > > > > + priv->alt ^= 1; > > > > > + break; > > > > > + case ORANGERX: > > > > > + if (size != ORANGERX_DSIZE) > > > > > + break; > > > > > + input_report_abs(priv->input, ABS_X, raw_data[0]); > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[2]); > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[3]); > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[1]); > > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > > + break; > > > > > + }; > > > > > + > > > > > + input_sync(priv->input); > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) > > > > > +{ > > > > > + struct device *dev = &hdev->dev; > > > > > + struct rcsim_priv *priv; > > > > > + int ret; > > > > > + > > > > > + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) > > > > > + return -ENODEV; > > > > > > > > You are not accessing anything in the USB stack, so there is no need > > > > to prevent regression tests that could inject uhid devices to your > > > > drivers. > > > > > > Ok, thanks. > > > > > > > > > > > Cheers, > > > > Benjamin > > > > > > > > > > Best regards, > > > Marcus Folkesson > > > > > > [1] https://www.usb.org/hid > > > > > > > If you need help in writing report descriptors, I can give you some, > > but the easiest might be for you to start from the report descriptor > > in hid-sony.c. I used to have a tool to dynamically write a report > > descriptor, but I'm not sure it still works... > > I think at least some advice would be great :-) > > The VRC2 would be the most simple of those, it only has 2 axis with > resolution of 11-bit. > If you have time, would you please give some advice what a report descriptor would look > like and I could probably come up with something for the others. > > > > > FYI, I just re-read rcsim_raw_event() and there is stuff that would > > require more than just a report descriptor fixup (the fact that some > > data is sent every other report is not good and will need some manual > > handling though). > > > Is the fact that more than one button share the same > byte hard to describe in the report? No, this is actually easy to describe. You say that there is one usage of "something" which has a report size of 1 bit, and then you have another usage of "something else" with the same report size. But usually you have to add padding after to make up to 8 bits (so 6 bits in that case). I was referring to the case where you are parsing the same bit on the wire, and give a different usage based if you have received an odd or an even number of reports. In that case, we probably need to use move this bit to a const field in the original report descriptor and say that the data is now not const: - initial report (completely random example): X (2 bytes) | Y (2 bytes) | button this_or_that (1 bit, depending of odd or even received reports) | 7 bits of padding - we can declare it as: X (2 bytes) | Y (2 bytes) | button this (1 bit) | button that (1 bit) | 6 bits of padding and have raw_event do the translation for the button "this_or_that" that would populate the correct bit in the translated report. Cheers, Benjamin > > value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > value = (value << 8) | raw_data[6]; > > ... > value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > value = (value << 8) | raw_data[5]; > > > > > > Cheers, > > Benjamin > > > > Best regards > Marcus Folkesson > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-uclogic-rdesc.c > > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-input.c#n817 > [4] https://eleccelerator.com/usbdescreqparser/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-30 12:45 ` Benjamin Tissoires @ 2022-09-15 7:34 ` Marcus Folkesson 2022-09-15 7:35 ` Benjamin Tissoires 0 siblings, 1 reply; 16+ messages in thread From: Marcus Folkesson @ 2022-09-15 7:34 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER [-- Attachment #1: Type: text/plain, Size: 1551 bytes --] Hi Benjamin, On Tue, Aug 30, 2022 at 02:45:11PM +0200, Benjamin Tissoires wrote: > On Thu, Aug 25, 2022 at 8:44 AM Marcus Folkesson > <marcus.folkesson@gmail.com> wrote: > > [...] > > > > > > Is the fact that more than one button share the same > > byte hard to describe in the report? > > No, this is actually easy to describe. You say that there is one usage > of "something" which has a report size of 1 bit, and then you have > another usage of "something else" with the same report size. > > But usually you have to add padding after to make up to 8 bits (so 6 > bits in that case). > > I was referring to the case where you are parsing the same bit on the > wire, and give a different usage based if you have received an odd or > an even number of reports. In that case, we probably need to use move > this bit to a const field in the original report descriptor and say > that the data is now not const: > > - initial report (completely random example): > X (2 bytes) | Y (2 bytes) | button this_or_that (1 bit, depending of > odd or even received reports) | 7 bits of padding > - we can declare it as: > X (2 bytes) | Y (2 bytes) | button this (1 bit) | button that (1 > bit) | 6 bits of padding How about if there is no unused bytes? The XTRG2FMS has 8 10-bit channels and use every byte in the report. Should I specify 8 8-bit channels instead and fix that in raw_event? If so, should I only use 8bit values then? (Are you at the ELCE conference btw?) Best regards Marcus Folkesson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-09-15 7:34 ` Marcus Folkesson @ 2022-09-15 7:35 ` Benjamin Tissoires 2022-09-19 13:32 ` Benjamin Tissoires 0 siblings, 1 reply; 16+ messages in thread From: Benjamin Tissoires @ 2022-09-15 7:35 UTC (permalink / raw) To: Marcus Folkesson Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER On Thu, Sep 15, 2022 at 8:28 AM Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > Hi Benjamin, > > On Tue, Aug 30, 2022 at 02:45:11PM +0200, Benjamin Tissoires wrote: > > On Thu, Aug 25, 2022 at 8:44 AM Marcus Folkesson > > <marcus.folkesson@gmail.com> wrote: > > > > > > [...] > > > > > > > > > Is the fact that more than one button share the same > > > byte hard to describe in the report? > > > > No, this is actually easy to describe. You say that there is one usage > > of "something" which has a report size of 1 bit, and then you have > > another usage of "something else" with the same report size. > > > > But usually you have to add padding after to make up to 8 bits (so 6 > > bits in that case). > > > > I was referring to the case where you are parsing the same bit on the > > wire, and give a different usage based if you have received an odd or > > an even number of reports. In that case, we probably need to use move > > this bit to a const field in the original report descriptor and say > > that the data is now not const: > > > > - initial report (completely random example): > > X (2 bytes) | Y (2 bytes) | button this_or_that (1 bit, depending of > > odd or even received reports) | 7 bits of padding > > - we can declare it as: > > X (2 bytes) | Y (2 bytes) | button this (1 bit) | button that (1 > > bit) | 6 bits of padding > > How about if there is no unused bytes? > > The XTRG2FMS has 8 10-bit channels and use every byte in the report. > Should I specify 8 8-bit channels instead and fix that in raw_event? > If so, should I only use 8bit values then? If I am not wrong, you should be able to add another byte in the report descriptor, as long as your raw_event function always adds it. Though now that I am typing it, I am actually wondering if this will work. You can always try, there is a chance it'll work, but I can't remember if it'll result in a timeout on the USB front because it'll expect one more byte that will never arrive. > > (Are you at the ELCE conference btw?) I was at Plumbers this week, but got an extra day today. But yeah, I'm in Dublin today. Cheers, Benjamin > > Best regards > Marcus Folkesson ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-09-15 7:35 ` Benjamin Tissoires @ 2022-09-19 13:32 ` Benjamin Tissoires 2023-01-10 14:48 ` Marcus Folkesson 0 siblings, 1 reply; 16+ messages in thread From: Benjamin Tissoires @ 2022-09-19 13:32 UTC (permalink / raw) To: Marcus Folkesson Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER On 9/15/22 09:35, Benjamin Tissoires wrote: > On Thu, Sep 15, 2022 at 8:28 AM Marcus Folkesson > <marcus.folkesson@gmail.com> wrote: >> >> Hi Benjamin, >> >> On Tue, Aug 30, 2022 at 02:45:11PM +0200, Benjamin Tissoires wrote: >>> On Thu, Aug 25, 2022 at 8:44 AM Marcus Folkesson >>> <marcus.folkesson@gmail.com> wrote: >>>> >> >> >> [...] >>>> >>>> >>>> Is the fact that more than one button share the same >>>> byte hard to describe in the report? >>> >>> No, this is actually easy to describe. You say that there is one usage >>> of "something" which has a report size of 1 bit, and then you have >>> another usage of "something else" with the same report size. >>> >>> But usually you have to add padding after to make up to 8 bits (so 6 >>> bits in that case). >>> >>> I was referring to the case where you are parsing the same bit on the >>> wire, and give a different usage based if you have received an odd or >>> an even number of reports. In that case, we probably need to use move >>> this bit to a const field in the original report descriptor and say >>> that the data is now not const: >>> >>> - initial report (completely random example): >>> X (2 bytes) | Y (2 bytes) | button this_or_that (1 bit, depending of >>> odd or even received reports) | 7 bits of padding >>> - we can declare it as: >>> X (2 bytes) | Y (2 bytes) | button this (1 bit) | button that (1 >>> bit) | 6 bits of padding >> >> How about if there is no unused bytes? >> >> The XTRG2FMS has 8 10-bit channels and use every byte in the report. >> Should I specify 8 8-bit channels instead and fix that in raw_event? >> If so, should I only use 8bit values then? > > If I am not wrong, you should be able to add another byte in the > report descriptor, as long as your raw_event function always adds it. > Though now that I am typing it, I am actually wondering if this will > work. You can always try, there is a chance it'll work, but I can't > remember if it'll result in a timeout on the USB front because it'll > expect one more byte that will never arrive. I am back home, and I just tested that. I had a doubt, and it is indeed failing. You need the following change for this to be working (I need to send it as a proper patch after assessing it hasn't side effects) --- diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 13cce286247e..f37ffe2bd488 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -275,6 +275,7 @@ static void hid_irq_in(struct urb *urb) int status; switch (urb->status) { + case -EOVERFLOW: /* happens with modified report descriptors */ case 0: /* success */ usbhid->retry_delay = 0; if (!test_bit(HID_OPENED, &usbhid->iofl)) --- Cheers, Benjamin > >> >> (Are you at the ELCE conference btw?) > > I was at Plumbers this week, but got an extra day today. But yeah, I'm > in Dublin today. > > Cheers, > Benjamin > >> >> Best regards >> Marcus Folkesson ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-09-19 13:32 ` Benjamin Tissoires @ 2023-01-10 14:48 ` Marcus Folkesson 0 siblings, 0 replies; 16+ messages in thread From: Marcus Folkesson @ 2023-01-10 14:48 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER [-- Attachment #1: Type: text/plain, Size: 1040 bytes --] Hi Benjamin, On Mon, Sep 19, 2022 at 03:32:37PM +0200, Benjamin Tissoires wrote: > > [...] > > I am back home, and I just tested that. I had a doubt, and it is indeed > failing. You need the following change for this to be working (I need to > send it as a proper patch after assessing it hasn't side effects) Did it come up with any side effects? :-) > > --- > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c > index 13cce286247e..f37ffe2bd488 100644 > --- a/drivers/hid/usbhid/hid-core.c > +++ b/drivers/hid/usbhid/hid-core.c > @@ -275,6 +275,7 @@ static void hid_irq_in(struct urb *urb) > int status; > switch (urb->status) { > + case -EOVERFLOW: /* happens with modified report descriptors */ > case 0: /* success */ > usbhid->retry_delay = 0; > if (!test_bit(HID_OPENED, &usbhid->iofl)) > --- > > Cheers, > Benjamin > Best regards, Marcus Folkesson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-23 16:43 ` Benjamin Tissoires 2022-08-25 6:48 ` Marcus Folkesson @ 2022-08-28 14:18 ` Marcus Folkesson 2022-08-30 12:35 ` Benjamin Tissoires 1 sibling, 1 reply; 16+ messages in thread From: Marcus Folkesson @ 2022-08-28 14:18 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER [-- Attachment #1: Type: text/plain, Size: 27982 bytes --] Hi, On Tue, Aug 23, 2022 at 06:43:47PM +0200, Benjamin Tissoires wrote: > On Tue, Aug 23, 2022 at 5:13 PM Marcus Folkesson > <marcus.folkesson@gmail.com> wrote: > > > > Thank you Benjamin, > > > > On Tue, Aug 23, 2022 at 11:49:59AM +0200, Benjamin Tissoires wrote: > > > Hi Marcus, > > > > > > [and sorry for the delay in the review of your patches] > > > > > > On Mon, Aug 22, 2022 at 8:04 AM Marcus Folkesson > > > <marcus.folkesson@gmail.com> wrote: > > > > > > > > Several RC Simulator Controllers are HID compliant with similar > > > > interface. > > > > > > > > Add support for these controllers: > > > > - Phoenix RC (HID variant) > > > > - Car VRC2.0 > > > > - Real Flight G5/G6/G7 > > > > - Aero Fly, FMS > > > > - OrangeRX FrSky > > > > > > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > > > > --- > > > > Documentation/hid/index.rst | 1 + > > > > Documentation/hid/rcsim.rst | 142 ++++++++++++++++ > > > > drivers/hid/Kconfig | 11 ++ > > > > drivers/hid/Makefile | 1 + > > > > drivers/hid/hid-ids.h | 5 + > > > > drivers/hid/hid-rcsim.c | 315 ++++++++++++++++++++++++++++++++++++ > > > > 6 files changed, 475 insertions(+) > > > > create mode 100644 Documentation/hid/rcsim.rst > > > > create mode 100644 drivers/hid/hid-rcsim.c > > > > > > > > diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst > > > > index e50f513c579c..e5813d264f37 100644 > > > > --- a/Documentation/hid/index.rst > > > > +++ b/Documentation/hid/index.rst > > > > @@ -17,3 +17,4 @@ Human Interface Devices (HID) > > > > hid-alps > > > > intel-ish-hid > > > > amd-sfh-hid > > > > + rcsim > > > > diff --git a/Documentation/hid/rcsim.rst b/Documentation/hid/rcsim.rst > > > > new file mode 100644 > > > > index 000000000000..1a031f7189cb > > > > --- /dev/null > > > > +++ b/Documentation/hid/rcsim.rst > > > > @@ -0,0 +1,142 @@ > > > > +.. SPDX-License-Identifier: GPL-2.0 > > > > + > > > > +================================= > > > > +rcsim - RC Simulator Controllers > > > > +================================= > > > > + > > > > +:Author: Marcus Folkesson <marcus.folkesson@gmail.com> > > > > + > > > > +This driver let you use your own RC controller plugged > > > > +into your computer using an HID compatible USB dongle. > > > > + > > > > +There are several HID compatible USB dongles from different > > > > +vendors. The driver currently supports: > > > > + > > > > +- Phoenix RC (HID variant) (8ch) > > > > +- Car VRC2.0 (2ch) > > > > +- Real Flight G5/G6/G7 (6ch) > > > > +- Aero Fly, FMS (8ch) > > > > +- OrangeRX FrSky (6ch) > > > > + > > > > +Many RC controllers is able to configure which stick goes to which channel. > > > > +This is also configurable in most simulators, so a matching is not necessary. > > > > + > > > > +Supported dongles > > > > +================== > > > > + > > > > +PhoenixRC > > > > +---------- > > > > + > > > > +The PhoenixRC has one HID compatible variant which is supported by this driver. > > > > +The controller has support for 8 analog channels. > > > > + > > > > +The driver is generating the following input event for on channels: > > > > + > > > > ++---------+----------------+ > > > > +| Channel | Event | > > > > ++=========+================+ > > > > +| 1 | ABS_Y | > > > > ++---------+----------------+ > > > > +| 2 | ABS_X | > > > > ++---------+----------------+ > > > > +| 3 | ABS_RY | > > > > ++---------+----------------+ > > > > +| 4 | ABS_RX | > > > > ++---------+----------------+ > > > > +| 5 | ABS_RUDDER | > > > > ++---------+----------------+ > > > > +| 6 | ABS_THROTTLE | > > > > ++---------+----------------+ > > > > +| 7 | ABS_Z | > > > > ++---------+----------------+ > > > > +| 8 | ABS_RZ | > > > > ++---------+----------------+ > > > > + > > > > +VRC2.0 > > > > +---------- > > > > +VRC2.0 is a controller for RC Cars. > > > > +The controller has support for 2 analog channels. > > > > + > > > > +The driver is generating the following input event for on channels: > > > > + > > > > ++---------+----------------+ > > > > +| Channel | Event | > > > > ++=========+================+ > > > > +| 1 | ABS_GAS | > > > > ++---------+----------------+ > > > > +| 2 | ABS_WHEEL | > > > > ++---------+----------------+ > > > > + > > > > +RealFlight > > > > +---------- > > > > + > > > > +This driver supports Realflight G4-G7 and above > > > > +The controller has support for 4 analog channels and two buttons. > > > > + > > > > +The driver is generating the following input event for on channels: > > > > + > > > > ++---------+----------------+ > > > > +| Channel | Event | > > > > ++=========+================+ > > > > +| 1 | ABS_Y | > > > > ++---------+----------------+ > > > > +| 2 | ABS_X | > > > > ++---------+----------------+ > > > > +| 3 | ABS_RY | > > > > ++---------+----------------+ > > > > +| 4 | ABS_RX | > > > > ++---------+----------------+ > > > > +| 5 | BTN_A | > > > > ++---------+----------------+ > > > > +| 6 | BTN_B | > > > > ++---------+----------------+ > > > > + > > > > +XTR+G2+FMS Controllers > > > > +-------------------------------- > > > > + > > > > +The controllers has support for 8 analog channels. > > > > + > > > > +The driver is generating the following input event for on channels: > > > > + > > > > ++---------+----------------+ > > > > +| Channel | Event | > > > > ++=========+================+ > > > > +| 1 | ABS_Y | > > > > ++---------+----------------+ > > > > +| 2 | ABS_X | > > > > ++---------+----------------+ > > > > +| 3 | ABS_RY | > > > > ++---------+----------------+ > > > > +| 4 | ABS_RX | > > > > ++---------+----------------+ > > > > +| 5 | ABS_RUDDER | > > > > ++---------+----------------+ > > > > +| 6 | ABS_THROTTLE | > > > > ++---------+----------------+ > > > > +| 7 | ABS_Z | > > > > ++---------+----------------+ > > > > +| 8 | ABS_RZ | > > > > ++---------+----------------+ > > > > + > > > > +OrangeRX > > > > +---------- > > > > + > > > > +The controllers has support for 6 analog channels. > > > > + > > > > +The driver is generating the following input event for on channels: > > > > + > > > > ++---------+----------------+ > > > > +| Channel | Event | > > > > ++=========+================+ > > > > +| 1 | ABS_Y | > > > > ++---------+----------------+ > > > > +| 2 | ABS_X | > > > > ++---------+----------------+ > > > > +| 3 | ABS_RY | > > > > ++---------+----------------+ > > > > +| 4 | ABS_RX | > > > > ++---------+----------------+ > > > > +| 5 | ABS_RUDDER | > > > > ++---------+----------------+ > > > > +| 6 | ABS_THROTTLE | > > > > ++---------+----------------+ > > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > > > index 70da5931082f..d8313d36086c 100644 > > > > --- a/drivers/hid/Kconfig > > > > +++ b/drivers/hid/Kconfig > > > > @@ -957,6 +957,17 @@ config HID_RAZER > > > > Support for Razer devices that are not fully compliant with the > > > > HID standard. > > > > > > > > +config HID_RCSIM > > > > + tristate "RC Simulator Controllers" > > > > + depends on HID > > > > + help > > > > + Support for several HID compatible RC Simulator Controllers including > > > > + - Phoenix RC > > > > + - Car VRC2.0 > > > > + - Real Flight > > > > + - Aero Fly, FMS > > > > + - OrangeRX FrSky > > > > + > > > > config HID_PRIMAX > > > > tristate "Primax non-fully HID-compliant devices" > > > > depends on HID > > > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > > > > index cac2cbe26d11..85d50ab352ee 100644 > > > > --- a/drivers/hid/Makefile > > > > +++ b/drivers/hid/Makefile > > > > @@ -102,6 +102,7 @@ obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o > > > > obj-$(CONFIG_HID_PLAYSTATION) += hid-playstation.o > > > > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o > > > > obj-$(CONFIG_HID_RAZER) += hid-razer.o > > > > +obj-$(CONFIG_HID_RCSIM) += hid-rcsim.o > > > > > > General rule of thumbs, we try to name the drivers after their > > > vendors, unless we know we have a generic driver. > > > > > > Here, this driver seems to be really tied to a small set of devices, > > > and thus I don't think we can call it "generic". > > > > Got it. > > > > > > > > > obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o > > > > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o > > > > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ > > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > > > index d9eb676abe96..baf5f74d5bed 100644 > > > > --- a/drivers/hid/hid-ids.h > > > > +++ b/drivers/hid/hid-ids.h > > > > @@ -1381,6 +1381,11 @@ > > > > > > > > #define USB_VENDOR_ID_MULTIPLE_1781 0x1781 > > > > #define USB_DEVICE_ID_RAPHNET_4NES4SNES_OLD 0x0a9d > > > > +#define USB_DEVICE_ID_PHOENIXRC 0x0898 > > > > +#define USB_DEVICE_ID_REALFLIGHT 0x0e56 > > > > + > > > > +#define USB_VENDOR_ID_DIPLING 0x0B9B > > > > +#define USB_DEVICE_ID_DIPLING_RCCONTROLLER 0x4012 > > > > > > > > #define USB_VENDOR_ID_DRACAL_RAPHNET 0x289b > > > > #define USB_DEVICE_ID_RAPHNET_2NES2SNES 0x0002 > > > > diff --git a/drivers/hid/hid-rcsim.c b/drivers/hid/hid-rcsim.c > > > > new file mode 100644 > > > > index 000000000000..0f214cb5816a > > > > --- /dev/null > > > > +++ b/drivers/hid/hid-rcsim.c > > > > @@ -0,0 +1,315 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > +/* > > > > + * Driver for several HID compatible RC Simulator Controllers. > > > > + * Currently supported controllers are: > > > > + * > > > > + * - Phoenix RC (HID variant) (8ch) > > > > + * - Car VRC2.0 (2ch) > > > > + * - Real Flight G5/G6/G7 (6ch) > > > > + * - Aero Fly, FMS (8ch) > > > > + * - OrangeRX FrSky (6ch) > > > > + * > > > > + * Copyright (C) 2022 Marcus Folkesson <marcus.folkesson@gmail.com> > > > > + */ > > > > + > > > > +#include <linux/bitfield.h> > > > > +#include <linux/device.h> > > > > +#include <linux/input.h> > > > > +#include <linux/hid.h> > > > > +#include <linux/module.h> > > > > +#include <linux/usb.h> > > > > + > > > > +#include "hid-ids.h" > > > > + > > > > +/* > > > > + * Some of these VID/PID are probably "borrowed", so keep them locally and > > > > + * do not populate hid-ids.h with those. > > > > + */ > > > > + > > > > +/* PHOENIXRC Controlloer (HID variant) */ > > > > +#define PHOENIXRC_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > > +#define PHOENIXRC_PID (USB_DEVICE_ID_PHOENIXRC) > > > > +#define PHOENIXRC_DSIZE (8) > > > > + > > > > +/* VRC2 Controlloer */ > > > > +#define VRC2_VID (0x07c0) > > > > +#define VRC2_PID (0x1125) > > > > +#define VRC2_DSIZE (7) > > > > + > > > > +/* Realflight G4-&7 and Above Controller */ > > > > +#define REALFLIGHT_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > > +#define REALFLIGHT_PID (USB_DEVICE_ID_REALFLIGHT) > > > > +#define REALFLIGHT_DSIZE (8) > > > > + > > > > +#define REALFLIGHT_BTN_A BIT(0) > > > > +#define REALFLIGHT_BTN_B BIT(1) > > > > + > > > > +/* XTR+G2+FMS Controller */ > > > > +#define XTRG2FMS_VID (USB_VENDOR_ID_DIPLING) > > > > +#define XTRG2FMS_PID (USB_DEVICE_ID_DIPLING_RCCONTROLLER) > > > > +#define XTRG2FMS_DSIZE (8) > > > > + > > > > +#define XTRG2FMS_X_HI GENMASK(3, 2) > > > > +#define XTRG2FMS_Y_HI GENMASK(1, 0) > > > > +#define XTRG2FMS_RX_HI GENMASK(7, 6) > > > > +#define XTRG2FMS_RY_HI GENMASK(5, 4) > > > > +#define XTRG2FMS_ALT1_HI GENMASK(3, 2) > > > > +#define XTRG2FMS_ALT2_HI GENMASK(1, 0) > > > > + > > > > +/* OrangeRX FrSky */ > > > > +#define ORANGERX_VID (0x0451) > > > > +#define ORANGERX_PID (0x16a5) > > > > +#define ORANGERX_DSIZE (8) > > > > + > > > > +enum rcsim_controller { > > > > + PHOENIXRC, > > > > + VRC2, > > > > + REALFLIGHT, > > > > + XTRG2FMS, > > > > + ORANGERX > > > > +}; > > > > + > > > > +struct rcsim_priv { > > > > + struct hid_device *hdev; > > > > + struct input_dev *input; > > > > + enum rcsim_controller controller; > > > > + u8 alt; > > > > +}; > > > > + > > > > +static int rcsim_open(struct input_dev *dev) > > > > +{ > > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > > + > > > > + return hid_hw_open(priv->hdev); > > > > +} > > > > + > > > > +static void rcsim_close(struct input_dev *dev) > > > > +{ > > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > > + > > > > + hid_hw_close(priv->hdev); > > > > +} > > > > + > > > > +static int rcsim_setup_input(struct rcsim_priv *priv) > > > > +{ > > > > + struct input_dev *input; > > > > + > > > > + input = devm_input_allocate_device(&priv->hdev->dev); > > > > + if (!input) > > > > + return -ENOMEM; > > > > + > > > > + input->id.bustype = priv->hdev->bus; > > > > + input->id.vendor = priv->hdev->vendor; > > > > + input->id.product = priv->hdev->product; > > > > + input->id.version = priv->hdev->bus; > > > > + input->phys = priv->hdev->phys; > > > > + input->uniq = priv->hdev->uniq; > > > > + input->open = rcsim_open; > > > > + input->close = rcsim_close; > > > > + > > > > + input_set_drvdata(input, priv); > > > > + > > > > + switch (priv->controller) { > > > > + case PHOENIXRC: > > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_Z, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > > + input->name = "RC Simuator Controller PhoenixRC"; > > > > + break; > > > > + case VRC2: > > > > + input_set_abs_params(input, ABS_GAS, 0, 2048, 0, 0); > > > > + input_set_abs_params(input, ABS_WHEEL, 0, 2048, 0, 0); > > > > + input->name = "RC Simuator Controller VRC2.0"; > > > > + break; > > > > + case REALFLIGHT: > > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > > + input_set_capability(input, EV_KEY, BTN_A); > > > > + input_set_capability(input, EV_KEY, BTN_B); > > > > + input->name = "RC Simuator Controller Realflight"; > > > > + break; > > > > + case XTRG2FMS: > > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_Z, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_RZ, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 1024, 0, 0); > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 1024, 0, 0); > > > > + input->name = "RC Simuator Controller AeroFly, FMS"; > > > > + priv->alt = 0; > > > > + break; > > > > + case ORANGERX: > > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > > + input->name = "RC Simuator Controller OrangeRX FrSky"; > > > > + break; > > > > + }; > > > > + > > > > + priv->input = input; > > > > + return input_register_device(priv->input); > > > > +} > > > > > > You are basically rewriting hid-input.c, which is suboptimal. > > > > Ouch. I will have a look at hid-input, thanks. > > > > > > > > I guess the report descriptor provided by these devices are basically > > > useless, and so you have to parse the reports yourself in the > > > raw_event callback. > > > > Yep. > > > > > > > > But instead of manually doing that, why not overwrite the report > > > descriptor (with .rdesc_fixup) and declare here all of the data that > > Do you mean .report_fixup? > > yes, sorry :/ > > > > > > needs to be exported. You could remove basically everything in this > > > driver by just providing a fixed report descriptor. > > > > What you are aiming for is to fixup the report descriptor and let the > > generic hid-raw driver handle the rest, or do I get you wrong? > > yep, exactly > > > > > How is the report mapped to certain events then? > > Have a look at hid_configure_usage in hid-input.c [3]. Most of HID > events are mapped to input events with a one to one mapping. > > > > > I do read at [1] but it is not obvious how to put it together. > > Most drivers I've looked at that is using .report_fixup just fix broken > > reports. I guess these reports are not "broken", just.. odd? > > Have a look at [2], lots of full report descriptors :) > > And in your case, the reports are incomplete, not odd. > > > > > > > > > > > > + > > > > +static int rcsim_raw_event(struct hid_device *hdev, > > > > + struct hid_report *report, > > > > + u8 *raw_data, int size) > > > > +{ > > > > + struct rcsim_priv *priv = hid_get_drvdata(hdev); > > > > + u16 value; > > > > + > > > > + switch (priv->controller) { > > > > + case PHOENIXRC: > > > > + if (size != PHOENIXRC_DSIZE) > > > > + break; > > > > + > > > > + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > + input_report_abs(priv->input, ABS_Y, raw_data[0]); > > > > + input_report_abs(priv->input, ABS_RX, raw_data[4]); > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > + > > > > + /* Z and RZ are sent every other time */ > > > > + if (priv->alt) > > > > + input_report_abs(priv->input, ABS_Z, raw_data[7]); > > > > + else > > > > + input_report_abs(priv->input, ABS_RZ, raw_data[7]); > > > > + > > > > + priv->alt ^= 1; > > > > + break; > > > > + case VRC2: > > > > + if (size != VRC2_DSIZE) > > > > + break; > > > > + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); > > > > + input_report_abs(priv->input, ABS_GAS, value); > > > > + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); > > > > + input_report_abs(priv->input, ABS_WHEEL, value); > > > > + break; > > > > + case REALFLIGHT: > > > > + if (size != REALFLIGHT_DSIZE) > > > > + break; > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > + input_report_abs(priv->input, ABS_Y, raw_data[1]); > > > > + input_report_abs(priv->input, ABS_RX, raw_data[5]); > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > + input_report_abs(priv->input, ABS_MISC, raw_data[4]); > > > > + input_report_key(priv->input, BTN_A, > > > > + raw_data[7] & REALFLIGHT_BTN_A); > > > > + input_report_key(priv->input, BTN_B, > > > > + raw_data[7] & REALFLIGHT_BTN_B); > > > > + break; > > > > + case XTRG2FMS: > > > > + if (size != XTRG2FMS_DSIZE) > > > > + break; > > > > + > > > > + /* X, RX, Y and RY are sent every time */ > > > > + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); > > > > + value = (value << 8) | raw_data[1]; > > > > + input_report_abs(priv->input, ABS_X, value); > > > > + > > > > + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); > > > > + value = (value << 8) | raw_data[2]; > > > > + input_report_abs(priv->input, ABS_Y, value); > > > > + > > > > + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); > > > > + value = (value << 8) | raw_data[0]; > > > > + input_report_abs(priv->input, ABS_RX, value); > > > > + > > > > + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); > > > > + value = (value << 8) | raw_data[4]; > > > > + input_report_abs(priv->input, ABS_RY, value); > > > > + > > > > + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ > > > > + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > > > > + value = (value << 8) | raw_data[6]; > > > > + if (priv->alt) > > > > + input_report_abs(priv->input, ABS_Z, value); > > > > + else > > > > + input_report_abs(priv->input, ABS_RUDDER, value); > > > > + > > > > + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > > > > + value = (value << 8) | raw_data[5]; > > > > + if (priv->alt) > > > > + input_report_abs(priv->input, ABS_RZ, value); > > > > + else > > > > + input_report_abs(priv->input, ABS_THROTTLE, value); > > > > + > > > > + priv->alt ^= 1; > > > > + break; > > > > + case ORANGERX: > > > > + if (size != ORANGERX_DSIZE) > > > > + break; > > > > + input_report_abs(priv->input, ABS_X, raw_data[0]); > > > > + input_report_abs(priv->input, ABS_Y, raw_data[2]); > > > > + input_report_abs(priv->input, ABS_RX, raw_data[3]); > > > > + input_report_abs(priv->input, ABS_RY, raw_data[1]); > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > + break; > > > > + }; > > > > + > > > > + input_sync(priv->input); > > > > + return 0; > > > > +} > > > > + > > > > +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) > > > > +{ > > > > + struct device *dev = &hdev->dev; > > > > + struct rcsim_priv *priv; > > > > + int ret; > > > > + > > > > + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) > > > > + return -ENODEV; > > > > > > You are not accessing anything in the USB stack, so there is no need > > > to prevent regression tests that could inject uhid devices to your > > > drivers. > > > > Ok, thanks. > > > > > > > > Cheers, > > > Benjamin > > > > > > > Best regards, > > Marcus Folkesson > > > > [1] https://www.usb.org/hid > > > > If you need help in writing report descriptors, I can give you some, > but the easiest might be for you to start from the report descriptor > in hid-sony.c. I used to have a tool to dynamically write a report > descriptor, but I'm not sure it still works... I've worked with the report descriptor for VRC2, and have come up with something that works. static __u8 vrc2_rdesc_fixed[] = { 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) 0x09, 0x04, // Usage (Joystick) 0xA1, 0x01, // Collection (Application) 0x09, 0x01, // Usage (Pointer) 0xA1, 0x00, // Collection (Physical) 0x09, 0x30, // Usage (X) 0x09, 0x31, // Usage (Y) 0x15, 0x00, // Logical Minimum (0) 0x26, 0xFF, 0x07, // Logical Maximum (2047) 0x35, 0x00, // Physical Minimum (0) 0x46, 0xFF, 0x00, // Physical Maximum (255) 0x75, 0x10, // Report Size (16) 0x95, 0x02, // Report Count (2) 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection 0x95, 0x01, // Report Count (1) 0x75, 0x18, // Report Size (24) 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) 0x09, 0x00, // Usage (Undefined) 0x15, 0x00, // Logical Minimum (0) 0x26, 0xFF, 0x00, // Logical Maximum (255) 0x75, 0x08, // Report Size (8) 0x95, 0x08, // Report Count (8) 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) 0xC0, // End Collection }; It is a result of reading specifications and looking at various examples. These lines are taken from an example, and I do not know why I need those. I think the constant report size is wrong (should be 32 to fill up) and I do not know what the Feature is used for. 0x95, 0x01, // Report Count (1) 0x75, 0x18, // Report Size (24) 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) 0x09, 0x00, // Usage (Undefined) 0x15, 0x00, // Logical Minimum (0) 0x26, 0xFF, 0x00, // Logical Maximum (255) 0x75, 0x08, // Report Size (8) 0x95, 0x08, // Report Count (8) 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) If I omit any of these lines then I only get tons of kernel: usb 3-12.3.1.3.3: input irq status -75 received In the kernel log. > > FYI, I just re-read rcsim_raw_event() and there is stuff that would > require more than just a report descriptor fixup (the fact that some > data is sent every other report is not good and will need some manual > handling though). > > Cheers, > Benjamin > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-uclogic-rdesc.c > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-input.c#n817 > Best regards Marcus Folkesson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-28 14:18 ` Marcus Folkesson @ 2022-08-30 12:35 ` Benjamin Tissoires 2022-08-30 14:43 ` Marcus Folkesson 0 siblings, 1 reply; 16+ messages in thread From: Benjamin Tissoires @ 2022-08-30 12:35 UTC (permalink / raw) To: Marcus Folkesson Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER Hi Marcus, On Sun, Aug 28, 2022 at 4:13 PM Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > Hi, > > On Tue, Aug 23, 2022 at 06:43:47PM +0200, Benjamin Tissoires wrote: > > On Tue, Aug 23, 2022 at 5:13 PM Marcus Folkesson > > <marcus.folkesson@gmail.com> wrote: > > > > > > > Thank you Benjamin, > > > > > > On Tue, Aug 23, 2022 at 11:49:59AM +0200, Benjamin Tissoires wrote: > > > > Hi Marcus, > > > > > > > > [and sorry for the delay in the review of your patches] > > > > > > > > On Mon, Aug 22, 2022 at 8:04 AM Marcus Folkesson > > > > <marcus.folkesson@gmail.com> wrote: > > > > > > > > > > Several RC Simulator Controllers are HID compliant with similar > > > > > interface. > > > > > > > > > > Add support for these controllers: > > > > > - Phoenix RC (HID variant) > > > > > - Car VRC2.0 > > > > > - Real Flight G5/G6/G7 > > > > > - Aero Fly, FMS > > > > > - OrangeRX FrSky > > > > > > > > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > > > > > --- > > > > > Documentation/hid/index.rst | 1 + > > > > > Documentation/hid/rcsim.rst | 142 ++++++++++++++++ > > > > > drivers/hid/Kconfig | 11 ++ > > > > > drivers/hid/Makefile | 1 + > > > > > drivers/hid/hid-ids.h | 5 + > > > > > drivers/hid/hid-rcsim.c | 315 ++++++++++++++++++++++++++++++++++++ > > > > > 6 files changed, 475 insertions(+) > > > > > create mode 100644 Documentation/hid/rcsim.rst > > > > > create mode 100644 drivers/hid/hid-rcsim.c > > > > > > > > > > diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst > > > > > index e50f513c579c..e5813d264f37 100644 > > > > > --- a/Documentation/hid/index.rst > > > > > +++ b/Documentation/hid/index.rst > > > > > @@ -17,3 +17,4 @@ Human Interface Devices (HID) > > > > > hid-alps > > > > > intel-ish-hid > > > > > amd-sfh-hid > > > > > + rcsim > > > > > diff --git a/Documentation/hid/rcsim.rst b/Documentation/hid/rcsim.rst > > > > > new file mode 100644 > > > > > index 000000000000..1a031f7189cb > > > > > --- /dev/null > > > > > +++ b/Documentation/hid/rcsim.rst > > > > > @@ -0,0 +1,142 @@ > > > > > +.. SPDX-License-Identifier: GPL-2.0 > > > > > + > > > > > +================================= > > > > > +rcsim - RC Simulator Controllers > > > > > +================================= > > > > > + > > > > > +:Author: Marcus Folkesson <marcus.folkesson@gmail.com> > > > > > + > > > > > +This driver let you use your own RC controller plugged > > > > > +into your computer using an HID compatible USB dongle. > > > > > + > > > > > +There are several HID compatible USB dongles from different > > > > > +vendors. The driver currently supports: > > > > > + > > > > > +- Phoenix RC (HID variant) (8ch) > > > > > +- Car VRC2.0 (2ch) > > > > > +- Real Flight G5/G6/G7 (6ch) > > > > > +- Aero Fly, FMS (8ch) > > > > > +- OrangeRX FrSky (6ch) > > > > > + > > > > > +Many RC controllers is able to configure which stick goes to which channel. > > > > > +This is also configurable in most simulators, so a matching is not necessary. > > > > > + > > > > > +Supported dongles > > > > > +================== > > > > > + > > > > > +PhoenixRC > > > > > +---------- > > > > > + > > > > > +The PhoenixRC has one HID compatible variant which is supported by this driver. > > > > > +The controller has support for 8 analog channels. > > > > > + > > > > > +The driver is generating the following input event for on channels: > > > > > + > > > > > ++---------+----------------+ > > > > > +| Channel | Event | > > > > > ++=========+================+ > > > > > +| 1 | ABS_Y | > > > > > ++---------+----------------+ > > > > > +| 2 | ABS_X | > > > > > ++---------+----------------+ > > > > > +| 3 | ABS_RY | > > > > > ++---------+----------------+ > > > > > +| 4 | ABS_RX | > > > > > ++---------+----------------+ > > > > > +| 5 | ABS_RUDDER | > > > > > ++---------+----------------+ > > > > > +| 6 | ABS_THROTTLE | > > > > > ++---------+----------------+ > > > > > +| 7 | ABS_Z | > > > > > ++---------+----------------+ > > > > > +| 8 | ABS_RZ | > > > > > ++---------+----------------+ > > > > > + > > > > > +VRC2.0 > > > > > +---------- > > > > > +VRC2.0 is a controller for RC Cars. > > > > > +The controller has support for 2 analog channels. > > > > > + > > > > > +The driver is generating the following input event for on channels: > > > > > + > > > > > ++---------+----------------+ > > > > > +| Channel | Event | > > > > > ++=========+================+ > > > > > +| 1 | ABS_GAS | > > > > > ++---------+----------------+ > > > > > +| 2 | ABS_WHEEL | > > > > > ++---------+----------------+ > > > > > + > > > > > +RealFlight > > > > > +---------- > > > > > + > > > > > +This driver supports Realflight G4-G7 and above > > > > > +The controller has support for 4 analog channels and two buttons. > > > > > + > > > > > +The driver is generating the following input event for on channels: > > > > > + > > > > > ++---------+----------------+ > > > > > +| Channel | Event | > > > > > ++=========+================+ > > > > > +| 1 | ABS_Y | > > > > > ++---------+----------------+ > > > > > +| 2 | ABS_X | > > > > > ++---------+----------------+ > > > > > +| 3 | ABS_RY | > > > > > ++---------+----------------+ > > > > > +| 4 | ABS_RX | > > > > > ++---------+----------------+ > > > > > +| 5 | BTN_A | > > > > > ++---------+----------------+ > > > > > +| 6 | BTN_B | > > > > > ++---------+----------------+ > > > > > + > > > > > +XTR+G2+FMS Controllers > > > > > +-------------------------------- > > > > > + > > > > > +The controllers has support for 8 analog channels. > > > > > + > > > > > +The driver is generating the following input event for on channels: > > > > > + > > > > > ++---------+----------------+ > > > > > +| Channel | Event | > > > > > ++=========+================+ > > > > > +| 1 | ABS_Y | > > > > > ++---------+----------------+ > > > > > +| 2 | ABS_X | > > > > > ++---------+----------------+ > > > > > +| 3 | ABS_RY | > > > > > ++---------+----------------+ > > > > > +| 4 | ABS_RX | > > > > > ++---------+----------------+ > > > > > +| 5 | ABS_RUDDER | > > > > > ++---------+----------------+ > > > > > +| 6 | ABS_THROTTLE | > > > > > ++---------+----------------+ > > > > > +| 7 | ABS_Z | > > > > > ++---------+----------------+ > > > > > +| 8 | ABS_RZ | > > > > > ++---------+----------------+ > > > > > + > > > > > +OrangeRX > > > > > +---------- > > > > > + > > > > > +The controllers has support for 6 analog channels. > > > > > + > > > > > +The driver is generating the following input event for on channels: > > > > > + > > > > > ++---------+----------------+ > > > > > +| Channel | Event | > > > > > ++=========+================+ > > > > > +| 1 | ABS_Y | > > > > > ++---------+----------------+ > > > > > +| 2 | ABS_X | > > > > > ++---------+----------------+ > > > > > +| 3 | ABS_RY | > > > > > ++---------+----------------+ > > > > > +| 4 | ABS_RX | > > > > > ++---------+----------------+ > > > > > +| 5 | ABS_RUDDER | > > > > > ++---------+----------------+ > > > > > +| 6 | ABS_THROTTLE | > > > > > ++---------+----------------+ > > > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > > > > index 70da5931082f..d8313d36086c 100644 > > > > > --- a/drivers/hid/Kconfig > > > > > +++ b/drivers/hid/Kconfig > > > > > @@ -957,6 +957,17 @@ config HID_RAZER > > > > > Support for Razer devices that are not fully compliant with the > > > > > HID standard. > > > > > > > > > > +config HID_RCSIM > > > > > + tristate "RC Simulator Controllers" > > > > > + depends on HID > > > > > + help > > > > > + Support for several HID compatible RC Simulator Controllers including > > > > > + - Phoenix RC > > > > > + - Car VRC2.0 > > > > > + - Real Flight > > > > > + - Aero Fly, FMS > > > > > + - OrangeRX FrSky > > > > > + > > > > > config HID_PRIMAX > > > > > tristate "Primax non-fully HID-compliant devices" > > > > > depends on HID > > > > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > > > > > index cac2cbe26d11..85d50ab352ee 100644 > > > > > --- a/drivers/hid/Makefile > > > > > +++ b/drivers/hid/Makefile > > > > > @@ -102,6 +102,7 @@ obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o > > > > > obj-$(CONFIG_HID_PLAYSTATION) += hid-playstation.o > > > > > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o > > > > > obj-$(CONFIG_HID_RAZER) += hid-razer.o > > > > > +obj-$(CONFIG_HID_RCSIM) += hid-rcsim.o > > > > > > > > General rule of thumbs, we try to name the drivers after their > > > > vendors, unless we know we have a generic driver. > > > > > > > > Here, this driver seems to be really tied to a small set of devices, > > > > and thus I don't think we can call it "generic". > > > > > > Got it. > > > > > > > > > > > > obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o > > > > > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o > > > > > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ > > > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > > > > index d9eb676abe96..baf5f74d5bed 100644 > > > > > --- a/drivers/hid/hid-ids.h > > > > > +++ b/drivers/hid/hid-ids.h > > > > > @@ -1381,6 +1381,11 @@ > > > > > > > > > > #define USB_VENDOR_ID_MULTIPLE_1781 0x1781 > > > > > #define USB_DEVICE_ID_RAPHNET_4NES4SNES_OLD 0x0a9d > > > > > +#define USB_DEVICE_ID_PHOENIXRC 0x0898 > > > > > +#define USB_DEVICE_ID_REALFLIGHT 0x0e56 > > > > > + > > > > > +#define USB_VENDOR_ID_DIPLING 0x0B9B > > > > > +#define USB_DEVICE_ID_DIPLING_RCCONTROLLER 0x4012 > > > > > > > > > > #define USB_VENDOR_ID_DRACAL_RAPHNET 0x289b > > > > > #define USB_DEVICE_ID_RAPHNET_2NES2SNES 0x0002 > > > > > diff --git a/drivers/hid/hid-rcsim.c b/drivers/hid/hid-rcsim.c > > > > > new file mode 100644 > > > > > index 000000000000..0f214cb5816a > > > > > --- /dev/null > > > > > +++ b/drivers/hid/hid-rcsim.c > > > > > @@ -0,0 +1,315 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > > +/* > > > > > + * Driver for several HID compatible RC Simulator Controllers. > > > > > + * Currently supported controllers are: > > > > > + * > > > > > + * - Phoenix RC (HID variant) (8ch) > > > > > + * - Car VRC2.0 (2ch) > > > > > + * - Real Flight G5/G6/G7 (6ch) > > > > > + * - Aero Fly, FMS (8ch) > > > > > + * - OrangeRX FrSky (6ch) > > > > > + * > > > > > + * Copyright (C) 2022 Marcus Folkesson <marcus.folkesson@gmail.com> > > > > > + */ > > > > > + > > > > > +#include <linux/bitfield.h> > > > > > +#include <linux/device.h> > > > > > +#include <linux/input.h> > > > > > +#include <linux/hid.h> > > > > > +#include <linux/module.h> > > > > > +#include <linux/usb.h> > > > > > + > > > > > +#include "hid-ids.h" > > > > > + > > > > > +/* > > > > > + * Some of these VID/PID are probably "borrowed", so keep them locally and > > > > > + * do not populate hid-ids.h with those. > > > > > + */ > > > > > + > > > > > +/* PHOENIXRC Controlloer (HID variant) */ > > > > > +#define PHOENIXRC_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > > > +#define PHOENIXRC_PID (USB_DEVICE_ID_PHOENIXRC) > > > > > +#define PHOENIXRC_DSIZE (8) > > > > > + > > > > > +/* VRC2 Controlloer */ > > > > > +#define VRC2_VID (0x07c0) > > > > > +#define VRC2_PID (0x1125) > > > > > +#define VRC2_DSIZE (7) > > > > > + > > > > > +/* Realflight G4-&7 and Above Controller */ > > > > > +#define REALFLIGHT_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > > > +#define REALFLIGHT_PID (USB_DEVICE_ID_REALFLIGHT) > > > > > +#define REALFLIGHT_DSIZE (8) > > > > > + > > > > > +#define REALFLIGHT_BTN_A BIT(0) > > > > > +#define REALFLIGHT_BTN_B BIT(1) > > > > > + > > > > > +/* XTR+G2+FMS Controller */ > > > > > +#define XTRG2FMS_VID (USB_VENDOR_ID_DIPLING) > > > > > +#define XTRG2FMS_PID (USB_DEVICE_ID_DIPLING_RCCONTROLLER) > > > > > +#define XTRG2FMS_DSIZE (8) > > > > > + > > > > > +#define XTRG2FMS_X_HI GENMASK(3, 2) > > > > > +#define XTRG2FMS_Y_HI GENMASK(1, 0) > > > > > +#define XTRG2FMS_RX_HI GENMASK(7, 6) > > > > > +#define XTRG2FMS_RY_HI GENMASK(5, 4) > > > > > +#define XTRG2FMS_ALT1_HI GENMASK(3, 2) > > > > > +#define XTRG2FMS_ALT2_HI GENMASK(1, 0) > > > > > + > > > > > +/* OrangeRX FrSky */ > > > > > +#define ORANGERX_VID (0x0451) > > > > > +#define ORANGERX_PID (0x16a5) > > > > > +#define ORANGERX_DSIZE (8) > > > > > + > > > > > +enum rcsim_controller { > > > > > + PHOENIXRC, > > > > > + VRC2, > > > > > + REALFLIGHT, > > > > > + XTRG2FMS, > > > > > + ORANGERX > > > > > +}; > > > > > + > > > > > +struct rcsim_priv { > > > > > + struct hid_device *hdev; > > > > > + struct input_dev *input; > > > > > + enum rcsim_controller controller; > > > > > + u8 alt; > > > > > +}; > > > > > + > > > > > +static int rcsim_open(struct input_dev *dev) > > > > > +{ > > > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > > > + > > > > > + return hid_hw_open(priv->hdev); > > > > > +} > > > > > + > > > > > +static void rcsim_close(struct input_dev *dev) > > > > > +{ > > > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > > > + > > > > > + hid_hw_close(priv->hdev); > > > > > +} > > > > > + > > > > > +static int rcsim_setup_input(struct rcsim_priv *priv) > > > > > +{ > > > > > + struct input_dev *input; > > > > > + > > > > > + input = devm_input_allocate_device(&priv->hdev->dev); > > > > > + if (!input) > > > > > + return -ENOMEM; > > > > > + > > > > > + input->id.bustype = priv->hdev->bus; > > > > > + input->id.vendor = priv->hdev->vendor; > > > > > + input->id.product = priv->hdev->product; > > > > > + input->id.version = priv->hdev->bus; > > > > > + input->phys = priv->hdev->phys; > > > > > + input->uniq = priv->hdev->uniq; > > > > > + input->open = rcsim_open; > > > > > + input->close = rcsim_close; > > > > > + > > > > > + input_set_drvdata(input, priv); > > > > > + > > > > > + switch (priv->controller) { > > > > > + case PHOENIXRC: > > > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_Z, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > > > + input->name = "RC Simuator Controller PhoenixRC"; > > > > > + break; > > > > > + case VRC2: > > > > > + input_set_abs_params(input, ABS_GAS, 0, 2048, 0, 0); > > > > > + input_set_abs_params(input, ABS_WHEEL, 0, 2048, 0, 0); > > > > > + input->name = "RC Simuator Controller VRC2.0"; > > > > > + break; > > > > > + case REALFLIGHT: > > > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > > > + input_set_capability(input, EV_KEY, BTN_A); > > > > > + input_set_capability(input, EV_KEY, BTN_B); > > > > > + input->name = "RC Simuator Controller Realflight"; > > > > > + break; > > > > > + case XTRG2FMS: > > > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_Z, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_RZ, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 1024, 0, 0); > > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 1024, 0, 0); > > > > > + input->name = "RC Simuator Controller AeroFly, FMS"; > > > > > + priv->alt = 0; > > > > > + break; > > > > > + case ORANGERX: > > > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > > > + input->name = "RC Simuator Controller OrangeRX FrSky"; > > > > > + break; > > > > > + }; > > > > > + > > > > > + priv->input = input; > > > > > + return input_register_device(priv->input); > > > > > +} > > > > > > > > You are basically rewriting hid-input.c, which is suboptimal. > > > > > > Ouch. I will have a look at hid-input, thanks. > > > > > > > > > > > I guess the report descriptor provided by these devices are basically > > > > useless, and so you have to parse the reports yourself in the > > > > raw_event callback. > > > > > > Yep. > > > > > > > > > > > But instead of manually doing that, why not overwrite the report > > > > descriptor (with .rdesc_fixup) and declare here all of the data that > > > Do you mean .report_fixup? > > > > yes, sorry :/ > > > > > > > > > needs to be exported. You could remove basically everything in this > > > > driver by just providing a fixed report descriptor. > > > > > > What you are aiming for is to fixup the report descriptor and let the > > > generic hid-raw driver handle the rest, or do I get you wrong? > > > > yep, exactly > > > > > > > > How is the report mapped to certain events then? > > > > Have a look at hid_configure_usage in hid-input.c [3]. Most of HID > > events are mapped to input events with a one to one mapping. > > > > > > > > I do read at [1] but it is not obvious how to put it together. > > > Most drivers I've looked at that is using .report_fixup just fix broken > > > reports. I guess these reports are not "broken", just.. odd? > > > > Have a look at [2], lots of full report descriptors :) > > > > And in your case, the reports are incomplete, not odd. > > > > > > > > > > > > > > > > > + > > > > > +static int rcsim_raw_event(struct hid_device *hdev, > > > > > + struct hid_report *report, > > > > > + u8 *raw_data, int size) > > > > > +{ > > > > > + struct rcsim_priv *priv = hid_get_drvdata(hdev); > > > > > + u16 value; > > > > > + > > > > > + switch (priv->controller) { > > > > > + case PHOENIXRC: > > > > > + if (size != PHOENIXRC_DSIZE) > > > > > + break; > > > > > + > > > > > + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ > > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[0]); > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[4]); > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > > + > > > > > + /* Z and RZ are sent every other time */ > > > > > + if (priv->alt) > > > > > + input_report_abs(priv->input, ABS_Z, raw_data[7]); > > > > > + else > > > > > + input_report_abs(priv->input, ABS_RZ, raw_data[7]); > > > > > + > > > > > + priv->alt ^= 1; > > > > > + break; > > > > > + case VRC2: > > > > > + if (size != VRC2_DSIZE) > > > > > + break; > > > > > + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); > > > > > + input_report_abs(priv->input, ABS_GAS, value); > > > > > + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); > > > > > + input_report_abs(priv->input, ABS_WHEEL, value); > > > > > + break; > > > > > + case REALFLIGHT: > > > > > + if (size != REALFLIGHT_DSIZE) > > > > > + break; > > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[1]); > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[5]); > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > > + input_report_abs(priv->input, ABS_MISC, raw_data[4]); > > > > > + input_report_key(priv->input, BTN_A, > > > > > + raw_data[7] & REALFLIGHT_BTN_A); > > > > > + input_report_key(priv->input, BTN_B, > > > > > + raw_data[7] & REALFLIGHT_BTN_B); > > > > > + break; > > > > > + case XTRG2FMS: > > > > > + if (size != XTRG2FMS_DSIZE) > > > > > + break; > > > > > + > > > > > + /* X, RX, Y and RY are sent every time */ > > > > > + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); > > > > > + value = (value << 8) | raw_data[1]; > > > > > + input_report_abs(priv->input, ABS_X, value); > > > > > + > > > > > + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); > > > > > + value = (value << 8) | raw_data[2]; > > > > > + input_report_abs(priv->input, ABS_Y, value); > > > > > + > > > > > + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); > > > > > + value = (value << 8) | raw_data[0]; > > > > > + input_report_abs(priv->input, ABS_RX, value); > > > > > + > > > > > + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); > > > > > + value = (value << 8) | raw_data[4]; > > > > > + input_report_abs(priv->input, ABS_RY, value); > > > > > + > > > > > + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ > > > > > + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > > > > > + value = (value << 8) | raw_data[6]; > > > > > + if (priv->alt) > > > > > + input_report_abs(priv->input, ABS_Z, value); > > > > > + else > > > > > + input_report_abs(priv->input, ABS_RUDDER, value); > > > > > + > > > > > + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > > > > > + value = (value << 8) | raw_data[5]; > > > > > + if (priv->alt) > > > > > + input_report_abs(priv->input, ABS_RZ, value); > > > > > + else > > > > > + input_report_abs(priv->input, ABS_THROTTLE, value); > > > > > + > > > > > + priv->alt ^= 1; > > > > > + break; > > > > > + case ORANGERX: > > > > > + if (size != ORANGERX_DSIZE) > > > > > + break; > > > > > + input_report_abs(priv->input, ABS_X, raw_data[0]); > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[2]); > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[3]); > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[1]); > > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > > + break; > > > > > + }; > > > > > + > > > > > + input_sync(priv->input); > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) > > > > > +{ > > > > > + struct device *dev = &hdev->dev; > > > > > + struct rcsim_priv *priv; > > > > > + int ret; > > > > > + > > > > > + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) > > > > > + return -ENODEV; > > > > > > > > You are not accessing anything in the USB stack, so there is no need > > > > to prevent regression tests that could inject uhid devices to your > > > > drivers. > > > > > > Ok, thanks. > > > > > > > > > > > Cheers, > > > > Benjamin > > > > > > > > > > Best regards, > > > Marcus Folkesson > > > > > > [1] https://www.usb.org/hid > > > > > > > If you need help in writing report descriptors, I can give you some, > > but the easiest might be for you to start from the report descriptor > > in hid-sony.c. I used to have a tool to dynamically write a report > > descriptor, but I'm not sure it still works... > > I've worked with the report descriptor for VRC2, and have come up with > something that works. Great that you managed to fill in most of the gaps :) > > > static __u8 vrc2_rdesc_fixed[] = { > 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) > 0x09, 0x04, // Usage (Joystick) > 0xA1, 0x01, // Collection (Application) > 0x09, 0x01, // Usage (Pointer) > 0xA1, 0x00, // Collection (Physical) > 0x09, 0x30, // Usage (X) > 0x09, 0x31, // Usage (Y) > 0x15, 0x00, // Logical Minimum (0) > 0x26, 0xFF, 0x07, // Logical Maximum (2047) > 0x35, 0x00, // Physical Minimum (0) > 0x46, 0xFF, 0x00, // Physical Maximum (255) > 0x75, 0x10, // Report Size (16) > 0x95, 0x02, // Report Count (2) > 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) > 0xC0, // End Collection > 0x95, 0x01, // Report Count (1) > 0x75, 0x18, // Report Size (24) > 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) > 0x09, 0x00, // Usage (Undefined) > 0x15, 0x00, // Logical Minimum (0) > 0x26, 0xFF, 0x00, // Logical Maximum (255) > 0x75, 0x08, // Report Size (8) > 0x95, 0x08, // Report Count (8) > 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) > 0xC0, // End Collection > }; > > > It is a result of reading specifications and looking at various > examples. > > These lines are taken from an example, and I do not know why I need > those. I think the constant report size is wrong (should be 32 to Do you have an example of the actual data sent on the wire? hexdump or hid-recorder should be enough for me to get what is missing. As I read the report descriptors, we had: * old one: - opening of application collection of usage undefined - 64 buttons (logical max of 1), usage being being 0x01 and 0x3F (63) -> so the last bit is ignored Which would translate to a report of the following (each 0 being a separate button, | is marking bytes): 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, X -> 64 * 1 = 64 bits ()last bit being ignored because of its duplicated usage with the previous one) * new one: - opening of an application collection of usage Joystick - opening of a physical collection of type Pointer - 16 bits for X, with logical max of 2047 and physical max of 255 - 16 bits for Y, with logical max of 2047 and physical max of 255 - 1 const element of size 24 that has to be ignored - one feature that should not be there :) So it would translate into the following (c being constant bit): X (2 bytes) | Y (2 bytes) | c, c, c, c, c, c, c, c | c, c, c, c, c, c, c, c | c, c, c, c, c, c, c, c -> 24 + 2*8 + 2*8 = 56 bits So I agree with you, you are missing 16 bits > fill up) and I do not know what the Feature is used for. You should not have to declare a feature. A feature is a special HID endpoint that you can use to talk to the HID device. By emitting special HID commands, (HID_REPORT_GET/HID_REPORT_SET) you can control how the device works. But if your current report descriptor doesn't have a feature declared, you should not declare it. > > 0x95, 0x01, // Report Count (1) > 0x75, 0x18, // Report Size (24) > 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) > 0x09, 0x00, // Usage (Undefined) > 0x15, 0x00, // Logical Minimum (0) > 0x26, 0xFF, 0x00, // Logical Maximum (255) > 0x75, 0x08, // Report Size (8) > 0x95, 0x08, // Report Count (8) > 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) > > If I omit any of these lines then I only get tons of > > kernel: usb 3-12.3.1.3.3: input irq status -75 received I assume the feature in this particular case helps the driver to understand that you need to allocate a 64 bits chunk of memory to communicate with the device. I would advise to use the following report instead: ---- static __u8 vrc2_rdesc_fixed[] = { 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) 0x09, 0x04, // Usage (Joystick) 0xA1, 0x01, // Collection (Application) 0x09, 0x01, // Usage (Pointer) 0xA1, 0x00, // Collection (Physical) 0x09, 0x30, // Usage (X) 0x09, 0x31, // Usage (Y) 0x15, 0x00, // Logical Minimum (0) 0x26, 0xFF, 0x07, // Logical Maximum (2047) 0x35, 0x00, // Physical Minimum (0) 0x46, 0xFF, 0x00, // Physical Maximum (255) 0x75, 0x10, // Report Size (16) 0x95, 0x02, // Report Count (2) 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection 0x75, 0x08, // Report Size (8) 0x95, 0x04, // Report Count (4) 0x81, 0x03, // Input (Cnst,Var,Abs) 0xC0, // End Collection }; --- Few changes: - use of 4 bytes of padding, instead of 1 24bits slot of padding - changed the padding from 0x01 to 0x03 to remove the "array" const definition which is tricky to handle - removed the feature report. Cheers, Benjamin > > In the kernel log. > > > > > > FYI, I just re-read rcsim_raw_event() and there is stuff that would > > require more than just a report descriptor fixup (the fact that some > > data is sent every other report is not good and will need some manual > > handling though). > > > > Cheers, > > Benjamin > > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-uclogic-rdesc.c > > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-input.c#n817 > > > > > Best regards > Marcus Folkesson ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-30 12:35 ` Benjamin Tissoires @ 2022-08-30 14:43 ` Marcus Folkesson 2022-08-30 15:01 ` Benjamin Tissoires 0 siblings, 1 reply; 16+ messages in thread From: Marcus Folkesson @ 2022-08-30 14:43 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER [-- Attachment #1: Type: text/plain, Size: 38299 bytes --] Hi Benjamin, Thank you for your detailed response! On Tue, Aug 30, 2022 at 02:35:54PM +0200, Benjamin Tissoires wrote: > Hi Marcus, > > On Sun, Aug 28, 2022 at 4:13 PM Marcus Folkesson > <marcus.folkesson@gmail.com> wrote: > > > > Hi, > > > > On Tue, Aug 23, 2022 at 06:43:47PM +0200, Benjamin Tissoires wrote: > > > On Tue, Aug 23, 2022 at 5:13 PM Marcus Folkesson > > > <marcus.folkesson@gmail.com> wrote: > > > > > > > > > > Thank you Benjamin, > > > > > > > > On Tue, Aug 23, 2022 at 11:49:59AM +0200, Benjamin Tissoires wrote: > > > > > Hi Marcus, > > > > > > > > > > [and sorry for the delay in the review of your patches] > > > > > > > > > > On Mon, Aug 22, 2022 at 8:04 AM Marcus Folkesson > > > > > <marcus.folkesson@gmail.com> wrote: > > > > > > > > > > > > Several RC Simulator Controllers are HID compliant with similar > > > > > > interface. > > > > > > > > > > > > Add support for these controllers: > > > > > > - Phoenix RC (HID variant) > > > > > > - Car VRC2.0 > > > > > > - Real Flight G5/G6/G7 > > > > > > - Aero Fly, FMS > > > > > > - OrangeRX FrSky > > > > > > > > > > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > > > > > > --- > > > > > > Documentation/hid/index.rst | 1 + > > > > > > Documentation/hid/rcsim.rst | 142 ++++++++++++++++ > > > > > > drivers/hid/Kconfig | 11 ++ > > > > > > drivers/hid/Makefile | 1 + > > > > > > drivers/hid/hid-ids.h | 5 + > > > > > > drivers/hid/hid-rcsim.c | 315 ++++++++++++++++++++++++++++++++++++ > > > > > > 6 files changed, 475 insertions(+) > > > > > > create mode 100644 Documentation/hid/rcsim.rst > > > > > > create mode 100644 drivers/hid/hid-rcsim.c > > > > > > > > > > > > diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst > > > > > > index e50f513c579c..e5813d264f37 100644 > > > > > > --- a/Documentation/hid/index.rst > > > > > > +++ b/Documentation/hid/index.rst > > > > > > @@ -17,3 +17,4 @@ Human Interface Devices (HID) > > > > > > hid-alps > > > > > > intel-ish-hid > > > > > > amd-sfh-hid > > > > > > + rcsim > > > > > > diff --git a/Documentation/hid/rcsim.rst b/Documentation/hid/rcsim.rst > > > > > > new file mode 100644 > > > > > > index 000000000000..1a031f7189cb > > > > > > --- /dev/null > > > > > > +++ b/Documentation/hid/rcsim.rst > > > > > > @@ -0,0 +1,142 @@ > > > > > > +.. SPDX-License-Identifier: GPL-2.0 > > > > > > + > > > > > > +================================= > > > > > > +rcsim - RC Simulator Controllers > > > > > > +================================= > > > > > > + > > > > > > +:Author: Marcus Folkesson <marcus.folkesson@gmail.com> > > > > > > + > > > > > > +This driver let you use your own RC controller plugged > > > > > > +into your computer using an HID compatible USB dongle. > > > > > > + > > > > > > +There are several HID compatible USB dongles from different > > > > > > +vendors. The driver currently supports: > > > > > > + > > > > > > +- Phoenix RC (HID variant) (8ch) > > > > > > +- Car VRC2.0 (2ch) > > > > > > +- Real Flight G5/G6/G7 (6ch) > > > > > > +- Aero Fly, FMS (8ch) > > > > > > +- OrangeRX FrSky (6ch) > > > > > > + > > > > > > +Many RC controllers is able to configure which stick goes to which channel. > > > > > > +This is also configurable in most simulators, so a matching is not necessary. > > > > > > + > > > > > > +Supported dongles > > > > > > +================== > > > > > > + > > > > > > +PhoenixRC > > > > > > +---------- > > > > > > + > > > > > > +The PhoenixRC has one HID compatible variant which is supported by this driver. > > > > > > +The controller has support for 8 analog channels. > > > > > > + > > > > > > +The driver is generating the following input event for on channels: > > > > > > + > > > > > > ++---------+----------------+ > > > > > > +| Channel | Event | > > > > > > ++=========+================+ > > > > > > +| 1 | ABS_Y | > > > > > > ++---------+----------------+ > > > > > > +| 2 | ABS_X | > > > > > > ++---------+----------------+ > > > > > > +| 3 | ABS_RY | > > > > > > ++---------+----------------+ > > > > > > +| 4 | ABS_RX | > > > > > > ++---------+----------------+ > > > > > > +| 5 | ABS_RUDDER | > > > > > > ++---------+----------------+ > > > > > > +| 6 | ABS_THROTTLE | > > > > > > ++---------+----------------+ > > > > > > +| 7 | ABS_Z | > > > > > > ++---------+----------------+ > > > > > > +| 8 | ABS_RZ | > > > > > > ++---------+----------------+ > > > > > > + > > > > > > +VRC2.0 > > > > > > +---------- > > > > > > +VRC2.0 is a controller for RC Cars. > > > > > > +The controller has support for 2 analog channels. > > > > > > + > > > > > > +The driver is generating the following input event for on channels: > > > > > > + > > > > > > ++---------+----------------+ > > > > > > +| Channel | Event | > > > > > > ++=========+================+ > > > > > > +| 1 | ABS_GAS | > > > > > > ++---------+----------------+ > > > > > > +| 2 | ABS_WHEEL | > > > > > > ++---------+----------------+ > > > > > > + > > > > > > +RealFlight > > > > > > +---------- > > > > > > + > > > > > > +This driver supports Realflight G4-G7 and above > > > > > > +The controller has support for 4 analog channels and two buttons. > > > > > > + > > > > > > +The driver is generating the following input event for on channels: > > > > > > + > > > > > > ++---------+----------------+ > > > > > > +| Channel | Event | > > > > > > ++=========+================+ > > > > > > +| 1 | ABS_Y | > > > > > > ++---------+----------------+ > > > > > > +| 2 | ABS_X | > > > > > > ++---------+----------------+ > > > > > > +| 3 | ABS_RY | > > > > > > ++---------+----------------+ > > > > > > +| 4 | ABS_RX | > > > > > > ++---------+----------------+ > > > > > > +| 5 | BTN_A | > > > > > > ++---------+----------------+ > > > > > > +| 6 | BTN_B | > > > > > > ++---------+----------------+ > > > > > > + > > > > > > +XTR+G2+FMS Controllers > > > > > > +-------------------------------- > > > > > > + > > > > > > +The controllers has support for 8 analog channels. > > > > > > + > > > > > > +The driver is generating the following input event for on channels: > > > > > > + > > > > > > ++---------+----------------+ > > > > > > +| Channel | Event | > > > > > > ++=========+================+ > > > > > > +| 1 | ABS_Y | > > > > > > ++---------+----------------+ > > > > > > +| 2 | ABS_X | > > > > > > ++---------+----------------+ > > > > > > +| 3 | ABS_RY | > > > > > > ++---------+----------------+ > > > > > > +| 4 | ABS_RX | > > > > > > ++---------+----------------+ > > > > > > +| 5 | ABS_RUDDER | > > > > > > ++---------+----------------+ > > > > > > +| 6 | ABS_THROTTLE | > > > > > > ++---------+----------------+ > > > > > > +| 7 | ABS_Z | > > > > > > ++---------+----------------+ > > > > > > +| 8 | ABS_RZ | > > > > > > ++---------+----------------+ > > > > > > + > > > > > > +OrangeRX > > > > > > +---------- > > > > > > + > > > > > > +The controllers has support for 6 analog channels. > > > > > > + > > > > > > +The driver is generating the following input event for on channels: > > > > > > + > > > > > > ++---------+----------------+ > > > > > > +| Channel | Event | > > > > > > ++=========+================+ > > > > > > +| 1 | ABS_Y | > > > > > > ++---------+----------------+ > > > > > > +| 2 | ABS_X | > > > > > > ++---------+----------------+ > > > > > > +| 3 | ABS_RY | > > > > > > ++---------+----------------+ > > > > > > +| 4 | ABS_RX | > > > > > > ++---------+----------------+ > > > > > > +| 5 | ABS_RUDDER | > > > > > > ++---------+----------------+ > > > > > > +| 6 | ABS_THROTTLE | > > > > > > ++---------+----------------+ > > > > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > > > > > index 70da5931082f..d8313d36086c 100644 > > > > > > --- a/drivers/hid/Kconfig > > > > > > +++ b/drivers/hid/Kconfig > > > > > > @@ -957,6 +957,17 @@ config HID_RAZER > > > > > > Support for Razer devices that are not fully compliant with the > > > > > > HID standard. > > > > > > > > > > > > +config HID_RCSIM > > > > > > + tristate "RC Simulator Controllers" > > > > > > + depends on HID > > > > > > + help > > > > > > + Support for several HID compatible RC Simulator Controllers including > > > > > > + - Phoenix RC > > > > > > + - Car VRC2.0 > > > > > > + - Real Flight > > > > > > + - Aero Fly, FMS > > > > > > + - OrangeRX FrSky > > > > > > + > > > > > > config HID_PRIMAX > > > > > > tristate "Primax non-fully HID-compliant devices" > > > > > > depends on HID > > > > > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > > > > > > index cac2cbe26d11..85d50ab352ee 100644 > > > > > > --- a/drivers/hid/Makefile > > > > > > +++ b/drivers/hid/Makefile > > > > > > @@ -102,6 +102,7 @@ obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o > > > > > > obj-$(CONFIG_HID_PLAYSTATION) += hid-playstation.o > > > > > > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o > > > > > > obj-$(CONFIG_HID_RAZER) += hid-razer.o > > > > > > +obj-$(CONFIG_HID_RCSIM) += hid-rcsim.o > > > > > > > > > > General rule of thumbs, we try to name the drivers after their > > > > > vendors, unless we know we have a generic driver. > > > > > > > > > > Here, this driver seems to be really tied to a small set of devices, > > > > > and thus I don't think we can call it "generic". > > > > > > > > Got it. > > > > > > > > > > > > > > > obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o > > > > > > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o > > > > > > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ > > > > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > > > > > index d9eb676abe96..baf5f74d5bed 100644 > > > > > > --- a/drivers/hid/hid-ids.h > > > > > > +++ b/drivers/hid/hid-ids.h > > > > > > @@ -1381,6 +1381,11 @@ > > > > > > > > > > > > #define USB_VENDOR_ID_MULTIPLE_1781 0x1781 > > > > > > #define USB_DEVICE_ID_RAPHNET_4NES4SNES_OLD 0x0a9d > > > > > > +#define USB_DEVICE_ID_PHOENIXRC 0x0898 > > > > > > +#define USB_DEVICE_ID_REALFLIGHT 0x0e56 > > > > > > + > > > > > > +#define USB_VENDOR_ID_DIPLING 0x0B9B > > > > > > +#define USB_DEVICE_ID_DIPLING_RCCONTROLLER 0x4012 > > > > > > > > > > > > #define USB_VENDOR_ID_DRACAL_RAPHNET 0x289b > > > > > > #define USB_DEVICE_ID_RAPHNET_2NES2SNES 0x0002 > > > > > > diff --git a/drivers/hid/hid-rcsim.c b/drivers/hid/hid-rcsim.c > > > > > > new file mode 100644 > > > > > > index 000000000000..0f214cb5816a > > > > > > --- /dev/null > > > > > > +++ b/drivers/hid/hid-rcsim.c > > > > > > @@ -0,0 +1,315 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > > > +/* > > > > > > + * Driver for several HID compatible RC Simulator Controllers. > > > > > > + * Currently supported controllers are: > > > > > > + * > > > > > > + * - Phoenix RC (HID variant) (8ch) > > > > > > + * - Car VRC2.0 (2ch) > > > > > > + * - Real Flight G5/G6/G7 (6ch) > > > > > > + * - Aero Fly, FMS (8ch) > > > > > > + * - OrangeRX FrSky (6ch) > > > > > > + * > > > > > > + * Copyright (C) 2022 Marcus Folkesson <marcus.folkesson@gmail.com> > > > > > > + */ > > > > > > + > > > > > > +#include <linux/bitfield.h> > > > > > > +#include <linux/device.h> > > > > > > +#include <linux/input.h> > > > > > > +#include <linux/hid.h> > > > > > > +#include <linux/module.h> > > > > > > +#include <linux/usb.h> > > > > > > + > > > > > > +#include "hid-ids.h" > > > > > > + > > > > > > +/* > > > > > > + * Some of these VID/PID are probably "borrowed", so keep them locally and > > > > > > + * do not populate hid-ids.h with those. > > > > > > + */ > > > > > > + > > > > > > +/* PHOENIXRC Controlloer (HID variant) */ > > > > > > +#define PHOENIXRC_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > > > > +#define PHOENIXRC_PID (USB_DEVICE_ID_PHOENIXRC) > > > > > > +#define PHOENIXRC_DSIZE (8) > > > > > > + > > > > > > +/* VRC2 Controlloer */ > > > > > > +#define VRC2_VID (0x07c0) > > > > > > +#define VRC2_PID (0x1125) > > > > > > +#define VRC2_DSIZE (7) > > > > > > + > > > > > > +/* Realflight G4-&7 and Above Controller */ > > > > > > +#define REALFLIGHT_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > > > > +#define REALFLIGHT_PID (USB_DEVICE_ID_REALFLIGHT) > > > > > > +#define REALFLIGHT_DSIZE (8) > > > > > > + > > > > > > +#define REALFLIGHT_BTN_A BIT(0) > > > > > > +#define REALFLIGHT_BTN_B BIT(1) > > > > > > + > > > > > > +/* XTR+G2+FMS Controller */ > > > > > > +#define XTRG2FMS_VID (USB_VENDOR_ID_DIPLING) > > > > > > +#define XTRG2FMS_PID (USB_DEVICE_ID_DIPLING_RCCONTROLLER) > > > > > > +#define XTRG2FMS_DSIZE (8) > > > > > > + > > > > > > +#define XTRG2FMS_X_HI GENMASK(3, 2) > > > > > > +#define XTRG2FMS_Y_HI GENMASK(1, 0) > > > > > > +#define XTRG2FMS_RX_HI GENMASK(7, 6) > > > > > > +#define XTRG2FMS_RY_HI GENMASK(5, 4) > > > > > > +#define XTRG2FMS_ALT1_HI GENMASK(3, 2) > > > > > > +#define XTRG2FMS_ALT2_HI GENMASK(1, 0) > > > > > > + > > > > > > +/* OrangeRX FrSky */ > > > > > > +#define ORANGERX_VID (0x0451) > > > > > > +#define ORANGERX_PID (0x16a5) > > > > > > +#define ORANGERX_DSIZE (8) > > > > > > + > > > > > > +enum rcsim_controller { > > > > > > + PHOENIXRC, > > > > > > + VRC2, > > > > > > + REALFLIGHT, > > > > > > + XTRG2FMS, > > > > > > + ORANGERX > > > > > > +}; > > > > > > + > > > > > > +struct rcsim_priv { > > > > > > + struct hid_device *hdev; > > > > > > + struct input_dev *input; > > > > > > + enum rcsim_controller controller; > > > > > > + u8 alt; > > > > > > +}; > > > > > > + > > > > > > +static int rcsim_open(struct input_dev *dev) > > > > > > +{ > > > > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > > > > + > > > > > > + return hid_hw_open(priv->hdev); > > > > > > +} > > > > > > + > > > > > > +static void rcsim_close(struct input_dev *dev) > > > > > > +{ > > > > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > > > > + > > > > > > + hid_hw_close(priv->hdev); > > > > > > +} > > > > > > + > > > > > > +static int rcsim_setup_input(struct rcsim_priv *priv) > > > > > > +{ > > > > > > + struct input_dev *input; > > > > > > + > > > > > > + input = devm_input_allocate_device(&priv->hdev->dev); > > > > > > + if (!input) > > > > > > + return -ENOMEM; > > > > > > + > > > > > > + input->id.bustype = priv->hdev->bus; > > > > > > + input->id.vendor = priv->hdev->vendor; > > > > > > + input->id.product = priv->hdev->product; > > > > > > + input->id.version = priv->hdev->bus; > > > > > > + input->phys = priv->hdev->phys; > > > > > > + input->uniq = priv->hdev->uniq; > > > > > > + input->open = rcsim_open; > > > > > > + input->close = rcsim_close; > > > > > > + > > > > > > + input_set_drvdata(input, priv); > > > > > > + > > > > > > + switch (priv->controller) { > > > > > > + case PHOENIXRC: > > > > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_Z, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > > > > + input->name = "RC Simuator Controller PhoenixRC"; > > > > > > + break; > > > > > > + case VRC2: > > > > > > + input_set_abs_params(input, ABS_GAS, 0, 2048, 0, 0); > > > > > > + input_set_abs_params(input, ABS_WHEEL, 0, 2048, 0, 0); > > > > > > + input->name = "RC Simuator Controller VRC2.0"; > > > > > > + break; > > > > > > + case REALFLIGHT: > > > > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > > > > + input_set_capability(input, EV_KEY, BTN_A); > > > > > > + input_set_capability(input, EV_KEY, BTN_B); > > > > > > + input->name = "RC Simuator Controller Realflight"; > > > > > > + break; > > > > > > + case XTRG2FMS: > > > > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_Z, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RZ, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 1024, 0, 0); > > > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 1024, 0, 0); > > > > > > + input->name = "RC Simuator Controller AeroFly, FMS"; > > > > > > + priv->alt = 0; > > > > > > + break; > > > > > > + case ORANGERX: > > > > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > > > > + input->name = "RC Simuator Controller OrangeRX FrSky"; > > > > > > + break; > > > > > > + }; > > > > > > + > > > > > > + priv->input = input; > > > > > > + return input_register_device(priv->input); > > > > > > +} > > > > > > > > > > You are basically rewriting hid-input.c, which is suboptimal. > > > > > > > > Ouch. I will have a look at hid-input, thanks. > > > > > > > > > > > > > > I guess the report descriptor provided by these devices are basically > > > > > useless, and so you have to parse the reports yourself in the > > > > > raw_event callback. > > > > > > > > Yep. > > > > > > > > > > > > > > But instead of manually doing that, why not overwrite the report > > > > > descriptor (with .rdesc_fixup) and declare here all of the data that > > > > Do you mean .report_fixup? > > > > > > yes, sorry :/ > > > > > > > > > > > > needs to be exported. You could remove basically everything in this > > > > > driver by just providing a fixed report descriptor. > > > > > > > > What you are aiming for is to fixup the report descriptor and let the > > > > generic hid-raw driver handle the rest, or do I get you wrong? > > > > > > yep, exactly > > > > > > > > > > > How is the report mapped to certain events then? > > > > > > Have a look at hid_configure_usage in hid-input.c [3]. Most of HID > > > events are mapped to input events with a one to one mapping. > > > > > > > > > > > I do read at [1] but it is not obvious how to put it together. > > > > Most drivers I've looked at that is using .report_fixup just fix broken > > > > reports. I guess these reports are not "broken", just.. odd? > > > > > > Have a look at [2], lots of full report descriptors :) > > > > > > And in your case, the reports are incomplete, not odd. > > > > > > > > > > > > > > > > > > > > > > + > > > > > > +static int rcsim_raw_event(struct hid_device *hdev, > > > > > > + struct hid_report *report, > > > > > > + u8 *raw_data, int size) > > > > > > +{ > > > > > > + struct rcsim_priv *priv = hid_get_drvdata(hdev); > > > > > > + u16 value; > > > > > > + > > > > > > + switch (priv->controller) { > > > > > > + case PHOENIXRC: > > > > > > + if (size != PHOENIXRC_DSIZE) > > > > > > + break; > > > > > > + > > > > > > + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ > > > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[0]); > > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[4]); > > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > > > + > > > > > > + /* Z and RZ are sent every other time */ > > > > > > + if (priv->alt) > > > > > > + input_report_abs(priv->input, ABS_Z, raw_data[7]); > > > > > > + else > > > > > > + input_report_abs(priv->input, ABS_RZ, raw_data[7]); > > > > > > + > > > > > > + priv->alt ^= 1; > > > > > > + break; > > > > > > + case VRC2: > > > > > > + if (size != VRC2_DSIZE) > > > > > > + break; > > > > > > + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); > > > > > > + input_report_abs(priv->input, ABS_GAS, value); > > > > > > + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); > > > > > > + input_report_abs(priv->input, ABS_WHEEL, value); > > > > > > + break; > > > > > > + case REALFLIGHT: > > > > > > + if (size != REALFLIGHT_DSIZE) > > > > > > + break; > > > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[1]); > > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[5]); > > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > > > + input_report_abs(priv->input, ABS_MISC, raw_data[4]); > > > > > > + input_report_key(priv->input, BTN_A, > > > > > > + raw_data[7] & REALFLIGHT_BTN_A); > > > > > > + input_report_key(priv->input, BTN_B, > > > > > > + raw_data[7] & REALFLIGHT_BTN_B); > > > > > > + break; > > > > > > + case XTRG2FMS: > > > > > > + if (size != XTRG2FMS_DSIZE) > > > > > > + break; > > > > > > + > > > > > > + /* X, RX, Y and RY are sent every time */ > > > > > > + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); > > > > > > + value = (value << 8) | raw_data[1]; > > > > > > + input_report_abs(priv->input, ABS_X, value); > > > > > > + > > > > > > + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); > > > > > > + value = (value << 8) | raw_data[2]; > > > > > > + input_report_abs(priv->input, ABS_Y, value); > > > > > > + > > > > > > + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); > > > > > > + value = (value << 8) | raw_data[0]; > > > > > > + input_report_abs(priv->input, ABS_RX, value); > > > > > > + > > > > > > + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); > > > > > > + value = (value << 8) | raw_data[4]; > > > > > > + input_report_abs(priv->input, ABS_RY, value); > > > > > > + > > > > > > + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ > > > > > > + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > > > > > > + value = (value << 8) | raw_data[6]; > > > > > > + if (priv->alt) > > > > > > + input_report_abs(priv->input, ABS_Z, value); > > > > > > + else > > > > > > + input_report_abs(priv->input, ABS_RUDDER, value); > > > > > > + > > > > > > + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > > > > > > + value = (value << 8) | raw_data[5]; > > > > > > + if (priv->alt) > > > > > > + input_report_abs(priv->input, ABS_RZ, value); > > > > > > + else > > > > > > + input_report_abs(priv->input, ABS_THROTTLE, value); > > > > > > + > > > > > > + priv->alt ^= 1; > > > > > > + break; > > > > > > + case ORANGERX: > > > > > > + if (size != ORANGERX_DSIZE) > > > > > > + break; > > > > > > + input_report_abs(priv->input, ABS_X, raw_data[0]); > > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[2]); > > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[3]); > > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[1]); > > > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > > > + break; > > > > > > + }; > > > > > > + > > > > > > + input_sync(priv->input); > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) > > > > > > +{ > > > > > > + struct device *dev = &hdev->dev; > > > > > > + struct rcsim_priv *priv; > > > > > > + int ret; > > > > > > + > > > > > > + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) > > > > > > + return -ENODEV; > > > > > > > > > > You are not accessing anything in the USB stack, so there is no need > > > > > to prevent regression tests that could inject uhid devices to your > > > > > drivers. > > > > > > > > Ok, thanks. > > > > > > > > > > > > > > Cheers, > > > > > Benjamin > > > > > > > > > > > > > Best regards, > > > > Marcus Folkesson > > > > > > > > [1] https://www.usb.org/hid > > > > > > > > > > If you need help in writing report descriptors, I can give you some, > > > but the easiest might be for you to start from the report descriptor > > > in hid-sony.c. I used to have a tool to dynamically write a report > > > descriptor, but I'm not sure it still works... > > > > I've worked with the report descriptor for VRC2, and have come up with > > something that works. > > Great that you managed to fill in most of the gaps :) > > > > > > > static __u8 vrc2_rdesc_fixed[] = { > > 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) > > 0x09, 0x04, // Usage (Joystick) > > 0xA1, 0x01, // Collection (Application) > > 0x09, 0x01, // Usage (Pointer) > > 0xA1, 0x00, // Collection (Physical) > > 0x09, 0x30, // Usage (X) > > 0x09, 0x31, // Usage (Y) > > 0x15, 0x00, // Logical Minimum (0) > > 0x26, 0xFF, 0x07, // Logical Maximum (2047) > > 0x35, 0x00, // Physical Minimum (0) > > 0x46, 0xFF, 0x00, // Physical Maximum (255) > > 0x75, 0x10, // Report Size (16) > > 0x95, 0x02, // Report Count (2) > > 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) > > 0xC0, // End Collection > > 0x95, 0x01, // Report Count (1) > > 0x75, 0x18, // Report Size (24) > > 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) > > 0x09, 0x00, // Usage (Undefined) > > 0x15, 0x00, // Logical Minimum (0) > > 0x26, 0xFF, 0x00, // Logical Maximum (255) > > 0x75, 0x08, // Report Size (8) > > 0x95, 0x08, // Report Count (8) > > 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) > > 0xC0, // End Collection > > }; > > > > > > It is a result of reading specifications and looking at various > > examples. > > > > These lines are taken from an example, and I do not know why I need > > those. I think the constant report size is wrong (should be 32 to > > Do you have an example of the actual data sent on the wire? hexdump or > hid-recorder should be enough for me to get what is missing. Sorry, this will be a little confusing. I just realized that there are actually two hidraw devices showing up. One with this descriptor: 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) 0x09, 0x00, // Usage (Undefined) 0xA1, 0x01, // Collection (Application) 0x15, 0x00, // Logical Minimum (0) 0x25, 0x01, // Logical Maximum (1) 0x75, 0x01, // Report Size (1) 0x05, 0x09, // Usage Page (Button) 0x19, 0x01, // Usage Minimum (0x01) 0x29, 0x3F, // Usage Maximum (0x3F) 0x95, 0x40, // Report Count (64) 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection Output from hid-recorder: E: 000074.735891 8 20 00 ed 03 00 00 00 cc # Button: 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 1 1 1 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 1 0 0 1 , 1 At a first glance, it did look as the button bits follows the same pattern as the joystick axis in the descriptor below. It is hard to debug though, as it causes random clicks and button presses everywhere... and it driving me nuts. The other one looks like this: 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) 0x09, 0x04, // Usage (Joystick) 0xA1, 0x01, // Collection (Application) 0x09, 0x01, // Usage (Pointer) 0xA1, 0x00, // Collection (Physical) 0x09, 0x30, // Usage (X) 0x09, 0x31, // Usage (Y) 0x09, 0x32, // Usage (Z) 0x15, 0x00, // Logical Minimum (0) 0x26, 0xFF, 0x07, // Logical Maximum (2047) 0x35, 0x00, // Physical Minimum (0) 0x46, 0xFF, 0x00, // Physical Maximum (255) 0x75, 0x10, // Report Size (16) 0x95, 0x03, // Report Count (3) 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection 0x05, 0x09, // Usage Page (Button) 0x19, 0x01, // Usage Minimum (0x01) 0x29, 0x02, // Usage Maximum (0x02) 0x15, 0x00, // Logical Minimum (0) 0x25, 0x01, // Logical Maximum (1) 0x95, 0x02, // Report Count (2) 0x75, 0x01, // Report Size (1) 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0x95, 0x01, // Report Count (1) 0x75, 0x06, // Report Size (6) 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) 0x09, 0x00, // Usage (0x00) 0x15, 0x00, // Logical Minimum (0) 0x26, 0xFF, 0x00, // Logical Maximum (255) 0x75, 0x08, // Report Size (8) 0x95, 0x08, // Report Count (8) 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) 0xC0, // End Collection Output from hid-recorder: E: 000013.240054 7 00 00 00 00 80 00 00 # X: 110 | Y: 122 | Z: 0 | Button: 0 0 | # The controller does only support 2 axis (X/Y) and no buttons, so this descriptor is wrong in any way. I see that it makes use of Feature as well. Does this feature have something to do with the first HID device? > > As I read the report descriptors, we had: > * old one: > - opening of application collection of usage undefined > - 64 buttons (logical max of 1), usage being being 0x01 and 0x3F > (63) -> so the last bit is ignored > > Which would translate to a report of the following (each 0 being a > separate button, | is marking bytes): > > 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, > 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, > 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, X > > -> 64 * 1 = 64 bits > > ()last bit being ignored because of its duplicated usage with the previous one) > > * new one: > - opening of an application collection of usage Joystick > - opening of a physical collection of type Pointer > - 16 bits for X, with logical max of 2047 and physical max of 255 > - 16 bits for Y, with logical max of 2047 and physical max of 255 > - 1 const element of size 24 that has to be ignored > - one feature that should not be there :) > > So it would translate into the following (c being constant bit): > X (2 bytes) | Y (2 bytes) | c, c, c, c, c, c, c, c | c, c, c, c, c, c, > c, c | c, c, c, c, c, c, c, c > > -> 24 + 2*8 + 2*8 = 56 bits > > So I agree with you, you are missing 16 bits > > > fill up) and I do not know what the Feature is used for. > > You should not have to declare a feature. A feature is a special HID > endpoint that you can use to talk to the HID device. By emitting > special HID commands, (HID_REPORT_GET/HID_REPORT_SET) you can control > how the device works. But if your current report descriptor doesn't > have a feature declared, you should not declare it. > > > > > 0x95, 0x01, // Report Count (1) > > 0x75, 0x18, // Report Size (24) > > 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) > > 0x09, 0x00, // Usage (Undefined) > > 0x15, 0x00, // Logical Minimum (0) > > 0x26, 0xFF, 0x00, // Logical Maximum (255) > > 0x75, 0x08, // Report Size (8) > > 0x95, 0x08, // Report Count (8) > > 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) > > > > If I omit any of these lines then I only get tons of > > > > kernel: usb 3-12.3.1.3.3: input irq status -75 received > > I assume the feature in this particular case helps the driver to > understand that you need to allocate a 64 bits chunk of memory to > communicate with the device. > > I would advise to use the following report instead: > > ---- > static __u8 vrc2_rdesc_fixed[] = { > 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) > 0x09, 0x04, // Usage (Joystick) > 0xA1, 0x01, // Collection (Application) > 0x09, 0x01, // Usage (Pointer) > 0xA1, 0x00, // Collection (Physical) > 0x09, 0x30, // Usage (X) > 0x09, 0x31, // Usage (Y) > 0x15, 0x00, // Logical Minimum (0) > 0x26, 0xFF, 0x07, // Logical Maximum (2047) > 0x35, 0x00, // Physical Minimum (0) > 0x46, 0xFF, 0x00, // Physical Maximum (255) > 0x75, 0x10, // Report Size (16) > 0x95, 0x02, // Report Count (2) > 0x81, 0x02, // Input (Data,Var,Abs,No > Wrap,Linear,Preferred State,No Null Position) > 0xC0, // End Collection > 0x75, 0x08, // Report Size (8) > 0x95, 0x04, // Report Count (4) > 0x81, 0x03, // Input (Cnst,Var,Abs) > 0xC0, // End Collection > }; > --- This report is more of what I want it to look like. I got the same "usb 3-12.3.1.3.1: input irq status -75 received" error when using this descriptor though. I think it is beacause it is applied to both hid-devices? > > Few changes: > - use of 4 bytes of padding, instead of 1 24bits slot of padding > - changed the padding from 0x01 to 0x03 to remove the "array" const > definition which is tricky to handle > - removed the feature report. > > Cheers, > Benjamin > > > > > In the kernel log. > > > > > > > > > > FYI, I just re-read rcsim_raw_event() and there is stuff that would > > > require more than just a report descriptor fixup (the fact that some > > > data is sent every other report is not good and will need some manual > > > handling though). > > > > > > Cheers, > > > Benjamin > > > > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-uclogic-rdesc.c > > > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-input.c#n817 > > > > > > > > > Best regards > > Marcus Folkesson > Best regards, Marcus Folkesson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-30 14:43 ` Marcus Folkesson @ 2022-08-30 15:01 ` Benjamin Tissoires 0 siblings, 0 replies; 16+ messages in thread From: Benjamin Tissoires @ 2022-08-30 15:01 UTC (permalink / raw) To: Marcus Folkesson Cc: Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER On Tue, Aug 30, 2022 at 4:38 PM Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > Hi Benjamin, > > Thank you for your detailed response! > > On Tue, Aug 30, 2022 at 02:35:54PM +0200, Benjamin Tissoires wrote: > > Hi Marcus, > > > > On Sun, Aug 28, 2022 at 4:13 PM Marcus Folkesson > > <marcus.folkesson@gmail.com> wrote: > > > > > > Hi, > > > > > > On Tue, Aug 23, 2022 at 06:43:47PM +0200, Benjamin Tissoires wrote: > > > > On Tue, Aug 23, 2022 at 5:13 PM Marcus Folkesson > > > > <marcus.folkesson@gmail.com> wrote: > > > > > > > > > > > > > Thank you Benjamin, > > > > > > > > > > On Tue, Aug 23, 2022 at 11:49:59AM +0200, Benjamin Tissoires wrote: > > > > > > Hi Marcus, > > > > > > > > > > > > [and sorry for the delay in the review of your patches] > > > > > > > > > > > > On Mon, Aug 22, 2022 at 8:04 AM Marcus Folkesson > > > > > > <marcus.folkesson@gmail.com> wrote: > > > > > > > > > > > > > > Several RC Simulator Controllers are HID compliant with similar > > > > > > > interface. > > > > > > > > > > > > > > Add support for these controllers: > > > > > > > - Phoenix RC (HID variant) > > > > > > > - Car VRC2.0 > > > > > > > - Real Flight G5/G6/G7 > > > > > > > - Aero Fly, FMS > > > > > > > - OrangeRX FrSky > > > > > > > > > > > > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > > > > > > > --- > > > > > > > Documentation/hid/index.rst | 1 + > > > > > > > Documentation/hid/rcsim.rst | 142 ++++++++++++++++ > > > > > > > drivers/hid/Kconfig | 11 ++ > > > > > > > drivers/hid/Makefile | 1 + > > > > > > > drivers/hid/hid-ids.h | 5 + > > > > > > > drivers/hid/hid-rcsim.c | 315 ++++++++++++++++++++++++++++++++++++ > > > > > > > 6 files changed, 475 insertions(+) > > > > > > > create mode 100644 Documentation/hid/rcsim.rst > > > > > > > create mode 100644 drivers/hid/hid-rcsim.c > > > > > > > > > > > > > > diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst > > > > > > > index e50f513c579c..e5813d264f37 100644 > > > > > > > --- a/Documentation/hid/index.rst > > > > > > > +++ b/Documentation/hid/index.rst > > > > > > > @@ -17,3 +17,4 @@ Human Interface Devices (HID) > > > > > > > hid-alps > > > > > > > intel-ish-hid > > > > > > > amd-sfh-hid > > > > > > > + rcsim > > > > > > > diff --git a/Documentation/hid/rcsim.rst b/Documentation/hid/rcsim.rst > > > > > > > new file mode 100644 > > > > > > > index 000000000000..1a031f7189cb > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/hid/rcsim.rst > > > > > > > @@ -0,0 +1,142 @@ > > > > > > > +.. SPDX-License-Identifier: GPL-2.0 > > > > > > > + > > > > > > > +================================= > > > > > > > +rcsim - RC Simulator Controllers > > > > > > > +================================= > > > > > > > + > > > > > > > +:Author: Marcus Folkesson <marcus.folkesson@gmail.com> > > > > > > > + > > > > > > > +This driver let you use your own RC controller plugged > > > > > > > +into your computer using an HID compatible USB dongle. > > > > > > > + > > > > > > > +There are several HID compatible USB dongles from different > > > > > > > +vendors. The driver currently supports: > > > > > > > + > > > > > > > +- Phoenix RC (HID variant) (8ch) > > > > > > > +- Car VRC2.0 (2ch) > > > > > > > +- Real Flight G5/G6/G7 (6ch) > > > > > > > +- Aero Fly, FMS (8ch) > > > > > > > +- OrangeRX FrSky (6ch) > > > > > > > + > > > > > > > +Many RC controllers is able to configure which stick goes to which channel. > > > > > > > +This is also configurable in most simulators, so a matching is not necessary. > > > > > > > + > > > > > > > +Supported dongles > > > > > > > +================== > > > > > > > + > > > > > > > +PhoenixRC > > > > > > > +---------- > > > > > > > + > > > > > > > +The PhoenixRC has one HID compatible variant which is supported by this driver. > > > > > > > +The controller has support for 8 analog channels. > > > > > > > + > > > > > > > +The driver is generating the following input event for on channels: > > > > > > > + > > > > > > > ++---------+----------------+ > > > > > > > +| Channel | Event | > > > > > > > ++=========+================+ > > > > > > > +| 1 | ABS_Y | > > > > > > > ++---------+----------------+ > > > > > > > +| 2 | ABS_X | > > > > > > > ++---------+----------------+ > > > > > > > +| 3 | ABS_RY | > > > > > > > ++---------+----------------+ > > > > > > > +| 4 | ABS_RX | > > > > > > > ++---------+----------------+ > > > > > > > +| 5 | ABS_RUDDER | > > > > > > > ++---------+----------------+ > > > > > > > +| 6 | ABS_THROTTLE | > > > > > > > ++---------+----------------+ > > > > > > > +| 7 | ABS_Z | > > > > > > > ++---------+----------------+ > > > > > > > +| 8 | ABS_RZ | > > > > > > > ++---------+----------------+ > > > > > > > + > > > > > > > +VRC2.0 > > > > > > > +---------- > > > > > > > +VRC2.0 is a controller for RC Cars. > > > > > > > +The controller has support for 2 analog channels. > > > > > > > + > > > > > > > +The driver is generating the following input event for on channels: > > > > > > > + > > > > > > > ++---------+----------------+ > > > > > > > +| Channel | Event | > > > > > > > ++=========+================+ > > > > > > > +| 1 | ABS_GAS | > > > > > > > ++---------+----------------+ > > > > > > > +| 2 | ABS_WHEEL | > > > > > > > ++---------+----------------+ > > > > > > > + > > > > > > > +RealFlight > > > > > > > +---------- > > > > > > > + > > > > > > > +This driver supports Realflight G4-G7 and above > > > > > > > +The controller has support for 4 analog channels and two buttons. > > > > > > > + > > > > > > > +The driver is generating the following input event for on channels: > > > > > > > + > > > > > > > ++---------+----------------+ > > > > > > > +| Channel | Event | > > > > > > > ++=========+================+ > > > > > > > +| 1 | ABS_Y | > > > > > > > ++---------+----------------+ > > > > > > > +| 2 | ABS_X | > > > > > > > ++---------+----------------+ > > > > > > > +| 3 | ABS_RY | > > > > > > > ++---------+----------------+ > > > > > > > +| 4 | ABS_RX | > > > > > > > ++---------+----------------+ > > > > > > > +| 5 | BTN_A | > > > > > > > ++---------+----------------+ > > > > > > > +| 6 | BTN_B | > > > > > > > ++---------+----------------+ > > > > > > > + > > > > > > > +XTR+G2+FMS Controllers > > > > > > > +-------------------------------- > > > > > > > + > > > > > > > +The controllers has support for 8 analog channels. > > > > > > > + > > > > > > > +The driver is generating the following input event for on channels: > > > > > > > + > > > > > > > ++---------+----------------+ > > > > > > > +| Channel | Event | > > > > > > > ++=========+================+ > > > > > > > +| 1 | ABS_Y | > > > > > > > ++---------+----------------+ > > > > > > > +| 2 | ABS_X | > > > > > > > ++---------+----------------+ > > > > > > > +| 3 | ABS_RY | > > > > > > > ++---------+----------------+ > > > > > > > +| 4 | ABS_RX | > > > > > > > ++---------+----------------+ > > > > > > > +| 5 | ABS_RUDDER | > > > > > > > ++---------+----------------+ > > > > > > > +| 6 | ABS_THROTTLE | > > > > > > > ++---------+----------------+ > > > > > > > +| 7 | ABS_Z | > > > > > > > ++---------+----------------+ > > > > > > > +| 8 | ABS_RZ | > > > > > > > ++---------+----------------+ > > > > > > > + > > > > > > > +OrangeRX > > > > > > > +---------- > > > > > > > + > > > > > > > +The controllers has support for 6 analog channels. > > > > > > > + > > > > > > > +The driver is generating the following input event for on channels: > > > > > > > + > > > > > > > ++---------+----------------+ > > > > > > > +| Channel | Event | > > > > > > > ++=========+================+ > > > > > > > +| 1 | ABS_Y | > > > > > > > ++---------+----------------+ > > > > > > > +| 2 | ABS_X | > > > > > > > ++---------+----------------+ > > > > > > > +| 3 | ABS_RY | > > > > > > > ++---------+----------------+ > > > > > > > +| 4 | ABS_RX | > > > > > > > ++---------+----------------+ > > > > > > > +| 5 | ABS_RUDDER | > > > > > > > ++---------+----------------+ > > > > > > > +| 6 | ABS_THROTTLE | > > > > > > > ++---------+----------------+ > > > > > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > > > > > > index 70da5931082f..d8313d36086c 100644 > > > > > > > --- a/drivers/hid/Kconfig > > > > > > > +++ b/drivers/hid/Kconfig > > > > > > > @@ -957,6 +957,17 @@ config HID_RAZER > > > > > > > Support for Razer devices that are not fully compliant with the > > > > > > > HID standard. > > > > > > > > > > > > > > +config HID_RCSIM > > > > > > > + tristate "RC Simulator Controllers" > > > > > > > + depends on HID > > > > > > > + help > > > > > > > + Support for several HID compatible RC Simulator Controllers including > > > > > > > + - Phoenix RC > > > > > > > + - Car VRC2.0 > > > > > > > + - Real Flight > > > > > > > + - Aero Fly, FMS > > > > > > > + - OrangeRX FrSky > > > > > > > + > > > > > > > config HID_PRIMAX > > > > > > > tristate "Primax non-fully HID-compliant devices" > > > > > > > depends on HID > > > > > > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > > > > > > > index cac2cbe26d11..85d50ab352ee 100644 > > > > > > > --- a/drivers/hid/Makefile > > > > > > > +++ b/drivers/hid/Makefile > > > > > > > @@ -102,6 +102,7 @@ obj-$(CONFIG_HID_PLANTRONICS) += hid-plantronics.o > > > > > > > obj-$(CONFIG_HID_PLAYSTATION) += hid-playstation.o > > > > > > > obj-$(CONFIG_HID_PRIMAX) += hid-primax.o > > > > > > > obj-$(CONFIG_HID_RAZER) += hid-razer.o > > > > > > > +obj-$(CONFIG_HID_RCSIM) += hid-rcsim.o > > > > > > > > > > > > General rule of thumbs, we try to name the drivers after their > > > > > > vendors, unless we know we have a generic driver. > > > > > > > > > > > > Here, this driver seems to be really tied to a small set of devices, > > > > > > and thus I don't think we can call it "generic". > > > > > > > > > > Got it. > > > > > > > > > > > > > > > > > > obj-$(CONFIG_HID_REDRAGON) += hid-redragon.o > > > > > > > obj-$(CONFIG_HID_RETRODE) += hid-retrode.o > > > > > > > obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \ > > > > > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > > > > > > index d9eb676abe96..baf5f74d5bed 100644 > > > > > > > --- a/drivers/hid/hid-ids.h > > > > > > > +++ b/drivers/hid/hid-ids.h > > > > > > > @@ -1381,6 +1381,11 @@ > > > > > > > > > > > > > > #define USB_VENDOR_ID_MULTIPLE_1781 0x1781 > > > > > > > #define USB_DEVICE_ID_RAPHNET_4NES4SNES_OLD 0x0a9d > > > > > > > +#define USB_DEVICE_ID_PHOENIXRC 0x0898 > > > > > > > +#define USB_DEVICE_ID_REALFLIGHT 0x0e56 > > > > > > > + > > > > > > > +#define USB_VENDOR_ID_DIPLING 0x0B9B > > > > > > > +#define USB_DEVICE_ID_DIPLING_RCCONTROLLER 0x4012 > > > > > > > > > > > > > > #define USB_VENDOR_ID_DRACAL_RAPHNET 0x289b > > > > > > > #define USB_DEVICE_ID_RAPHNET_2NES2SNES 0x0002 > > > > > > > diff --git a/drivers/hid/hid-rcsim.c b/drivers/hid/hid-rcsim.c > > > > > > > new file mode 100644 > > > > > > > index 000000000000..0f214cb5816a > > > > > > > --- /dev/null > > > > > > > +++ b/drivers/hid/hid-rcsim.c > > > > > > > @@ -0,0 +1,315 @@ > > > > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > > > > +/* > > > > > > > + * Driver for several HID compatible RC Simulator Controllers. > > > > > > > + * Currently supported controllers are: > > > > > > > + * > > > > > > > + * - Phoenix RC (HID variant) (8ch) > > > > > > > + * - Car VRC2.0 (2ch) > > > > > > > + * - Real Flight G5/G6/G7 (6ch) > > > > > > > + * - Aero Fly, FMS (8ch) > > > > > > > + * - OrangeRX FrSky (6ch) > > > > > > > + * > > > > > > > + * Copyright (C) 2022 Marcus Folkesson <marcus.folkesson@gmail.com> > > > > > > > + */ > > > > > > > + > > > > > > > +#include <linux/bitfield.h> > > > > > > > +#include <linux/device.h> > > > > > > > +#include <linux/input.h> > > > > > > > +#include <linux/hid.h> > > > > > > > +#include <linux/module.h> > > > > > > > +#include <linux/usb.h> > > > > > > > + > > > > > > > +#include "hid-ids.h" > > > > > > > + > > > > > > > +/* > > > > > > > + * Some of these VID/PID are probably "borrowed", so keep them locally and > > > > > > > + * do not populate hid-ids.h with those. > > > > > > > + */ > > > > > > > + > > > > > > > +/* PHOENIXRC Controlloer (HID variant) */ > > > > > > > +#define PHOENIXRC_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > > > > > +#define PHOENIXRC_PID (USB_DEVICE_ID_PHOENIXRC) > > > > > > > +#define PHOENIXRC_DSIZE (8) > > > > > > > + > > > > > > > +/* VRC2 Controlloer */ > > > > > > > +#define VRC2_VID (0x07c0) > > > > > > > +#define VRC2_PID (0x1125) > > > > > > > +#define VRC2_DSIZE (7) > > > > > > > + > > > > > > > +/* Realflight G4-&7 and Above Controller */ > > > > > > > +#define REALFLIGHT_VID (USB_VENDOR_ID_MULTIPLE_1781) > > > > > > > +#define REALFLIGHT_PID (USB_DEVICE_ID_REALFLIGHT) > > > > > > > +#define REALFLIGHT_DSIZE (8) > > > > > > > + > > > > > > > +#define REALFLIGHT_BTN_A BIT(0) > > > > > > > +#define REALFLIGHT_BTN_B BIT(1) > > > > > > > + > > > > > > > +/* XTR+G2+FMS Controller */ > > > > > > > +#define XTRG2FMS_VID (USB_VENDOR_ID_DIPLING) > > > > > > > +#define XTRG2FMS_PID (USB_DEVICE_ID_DIPLING_RCCONTROLLER) > > > > > > > +#define XTRG2FMS_DSIZE (8) > > > > > > > + > > > > > > > +#define XTRG2FMS_X_HI GENMASK(3, 2) > > > > > > > +#define XTRG2FMS_Y_HI GENMASK(1, 0) > > > > > > > +#define XTRG2FMS_RX_HI GENMASK(7, 6) > > > > > > > +#define XTRG2FMS_RY_HI GENMASK(5, 4) > > > > > > > +#define XTRG2FMS_ALT1_HI GENMASK(3, 2) > > > > > > > +#define XTRG2FMS_ALT2_HI GENMASK(1, 0) > > > > > > > + > > > > > > > +/* OrangeRX FrSky */ > > > > > > > +#define ORANGERX_VID (0x0451) > > > > > > > +#define ORANGERX_PID (0x16a5) > > > > > > > +#define ORANGERX_DSIZE (8) > > > > > > > + > > > > > > > +enum rcsim_controller { > > > > > > > + PHOENIXRC, > > > > > > > + VRC2, > > > > > > > + REALFLIGHT, > > > > > > > + XTRG2FMS, > > > > > > > + ORANGERX > > > > > > > +}; > > > > > > > + > > > > > > > +struct rcsim_priv { > > > > > > > + struct hid_device *hdev; > > > > > > > + struct input_dev *input; > > > > > > > + enum rcsim_controller controller; > > > > > > > + u8 alt; > > > > > > > +}; > > > > > > > + > > > > > > > +static int rcsim_open(struct input_dev *dev) > > > > > > > +{ > > > > > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > > > > > + > > > > > > > + return hid_hw_open(priv->hdev); > > > > > > > +} > > > > > > > + > > > > > > > +static void rcsim_close(struct input_dev *dev) > > > > > > > +{ > > > > > > > + struct rcsim_priv *priv = input_get_drvdata(dev); > > > > > > > + > > > > > > > + hid_hw_close(priv->hdev); > > > > > > > +} > > > > > > > + > > > > > > > +static int rcsim_setup_input(struct rcsim_priv *priv) > > > > > > > +{ > > > > > > > + struct input_dev *input; > > > > > > > + > > > > > > > + input = devm_input_allocate_device(&priv->hdev->dev); > > > > > > > + if (!input) > > > > > > > + return -ENOMEM; > > > > > > > + > > > > > > > + input->id.bustype = priv->hdev->bus; > > > > > > > + input->id.vendor = priv->hdev->vendor; > > > > > > > + input->id.product = priv->hdev->product; > > > > > > > + input->id.version = priv->hdev->bus; > > > > > > > + input->phys = priv->hdev->phys; > > > > > > > + input->uniq = priv->hdev->uniq; > > > > > > > + input->open = rcsim_open; > > > > > > > + input->close = rcsim_close; > > > > > > > + > > > > > > > + input_set_drvdata(input, priv); > > > > > > > + > > > > > > > + switch (priv->controller) { > > > > > > > + case PHOENIXRC: > > > > > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_Z, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > > > > > + input->name = "RC Simuator Controller PhoenixRC"; > > > > > > > + break; > > > > > > > + case VRC2: > > > > > > > + input_set_abs_params(input, ABS_GAS, 0, 2048, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_WHEEL, 0, 2048, 0, 0); > > > > > > > + input->name = "RC Simuator Controller VRC2.0"; > > > > > > > + break; > > > > > > > + case REALFLIGHT: > > > > > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > > > > > + input_set_capability(input, EV_KEY, BTN_A); > > > > > > > + input_set_capability(input, EV_KEY, BTN_B); > > > > > > > + input->name = "RC Simuator Controller Realflight"; > > > > > > > + break; > > > > > > > + case XTRG2FMS: > > > > > > > + input_set_abs_params(input, ABS_X, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_Y, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RX, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RY, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_Z, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RZ, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 1024, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 1024, 0, 0); > > > > > > > + input->name = "RC Simuator Controller AeroFly, FMS"; > > > > > > > + priv->alt = 0; > > > > > > > + break; > > > > > > > + case ORANGERX: > > > > > > > + input_set_abs_params(input, ABS_X, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_Y, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RX, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RY, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_RUDDER, 0, 255, 0, 0); > > > > > > > + input_set_abs_params(input, ABS_THROTTLE, 0, 255, 0, 0); > > > > > > > + input->name = "RC Simuator Controller OrangeRX FrSky"; > > > > > > > + break; > > > > > > > + }; > > > > > > > + > > > > > > > + priv->input = input; > > > > > > > + return input_register_device(priv->input); > > > > > > > +} > > > > > > > > > > > > You are basically rewriting hid-input.c, which is suboptimal. > > > > > > > > > > Ouch. I will have a look at hid-input, thanks. > > > > > > > > > > > > > > > > > I guess the report descriptor provided by these devices are basically > > > > > > useless, and so you have to parse the reports yourself in the > > > > > > raw_event callback. > > > > > > > > > > Yep. > > > > > > > > > > > > > > > > > But instead of manually doing that, why not overwrite the report > > > > > > descriptor (with .rdesc_fixup) and declare here all of the data that > > > > > Do you mean .report_fixup? > > > > > > > > yes, sorry :/ > > > > > > > > > > > > > > > needs to be exported. You could remove basically everything in this > > > > > > driver by just providing a fixed report descriptor. > > > > > > > > > > What you are aiming for is to fixup the report descriptor and let the > > > > > generic hid-raw driver handle the rest, or do I get you wrong? > > > > > > > > yep, exactly > > > > > > > > > > > > > > How is the report mapped to certain events then? > > > > > > > > Have a look at hid_configure_usage in hid-input.c [3]. Most of HID > > > > events are mapped to input events with a one to one mapping. > > > > > > > > > > > > > > I do read at [1] but it is not obvious how to put it together. > > > > > Most drivers I've looked at that is using .report_fixup just fix broken > > > > > reports. I guess these reports are not "broken", just.. odd? > > > > > > > > Have a look at [2], lots of full report descriptors :) > > > > > > > > And in your case, the reports are incomplete, not odd. > > > > > > > > > > > > > > > > > > > > > > > > > > > + > > > > > > > +static int rcsim_raw_event(struct hid_device *hdev, > > > > > > > + struct hid_report *report, > > > > > > > + u8 *raw_data, int size) > > > > > > > +{ > > > > > > > + struct rcsim_priv *priv = hid_get_drvdata(hdev); > > > > > > > + u16 value; > > > > > > > + > > > > > > > + switch (priv->controller) { > > > > > > > + case PHOENIXRC: > > > > > > > + if (size != PHOENIXRC_DSIZE) > > > > > > > + break; > > > > > > > + > > > > > > > + /* X, RX, Y and RY, RUDDER and THROTTLE are sent every time */ > > > > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[0]); > > > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[4]); > > > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > > > > + > > > > > > > + /* Z and RZ are sent every other time */ > > > > > > > + if (priv->alt) > > > > > > > + input_report_abs(priv->input, ABS_Z, raw_data[7]); > > > > > > > + else > > > > > > > + input_report_abs(priv->input, ABS_RZ, raw_data[7]); > > > > > > > + > > > > > > > + priv->alt ^= 1; > > > > > > > + break; > > > > > > > + case VRC2: > > > > > > > + if (size != VRC2_DSIZE) > > > > > > > + break; > > > > > > > + value = (raw_data[1] << 8 | raw_data[0]) & GENMASK(10, 0); > > > > > > > + input_report_abs(priv->input, ABS_GAS, value); > > > > > > > + value = (raw_data[3] << 8 | raw_data[2]) & GENMASK(10, 0); > > > > > > > + input_report_abs(priv->input, ABS_WHEEL, value); > > > > > > > + break; > > > > > > > + case REALFLIGHT: > > > > > > > + if (size != REALFLIGHT_DSIZE) > > > > > > > + break; > > > > > > > + input_report_abs(priv->input, ABS_X, raw_data[2]); > > > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[1]); > > > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[5]); > > > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[3]); > > > > > > > + input_report_abs(priv->input, ABS_MISC, raw_data[4]); > > > > > > > + input_report_key(priv->input, BTN_A, > > > > > > > + raw_data[7] & REALFLIGHT_BTN_A); > > > > > > > + input_report_key(priv->input, BTN_B, > > > > > > > + raw_data[7] & REALFLIGHT_BTN_B); > > > > > > > + break; > > > > > > > + case XTRG2FMS: > > > > > > > + if (size != XTRG2FMS_DSIZE) > > > > > > > + break; > > > > > > > + > > > > > > > + /* X, RX, Y and RY are sent every time */ > > > > > > > + value = FIELD_GET(XTRG2FMS_X_HI, raw_data[3]); > > > > > > > + value = (value << 8) | raw_data[1]; > > > > > > > + input_report_abs(priv->input, ABS_X, value); > > > > > > > + > > > > > > > + value = FIELD_GET(XTRG2FMS_Y_HI, raw_data[3]); > > > > > > > + value = (value << 8) | raw_data[2]; > > > > > > > + input_report_abs(priv->input, ABS_Y, value); > > > > > > > + > > > > > > > + value = FIELD_GET(XTRG2FMS_RX_HI, raw_data[3]); > > > > > > > + value = (value << 8) | raw_data[0]; > > > > > > > + input_report_abs(priv->input, ABS_RX, value); > > > > > > > + > > > > > > > + value = FIELD_GET(XTRG2FMS_RY_HI, raw_data[3]); > > > > > > > + value = (value << 8) | raw_data[4]; > > > > > > > + input_report_abs(priv->input, ABS_RY, value); > > > > > > > + > > > > > > > + /* Z, RZ, RUDDER and THROTTLE are sent every other time */ > > > > > > > + value = FIELD_GET(XTRG2FMS_ALT1_HI, raw_data[7]); > > > > > > > + value = (value << 8) | raw_data[6]; > > > > > > > + if (priv->alt) > > > > > > > + input_report_abs(priv->input, ABS_Z, value); > > > > > > > + else > > > > > > > + input_report_abs(priv->input, ABS_RUDDER, value); > > > > > > > + > > > > > > > + value = FIELD_GET(XTRG2FMS_ALT2_HI, raw_data[7]); > > > > > > > + value = (value << 8) | raw_data[5]; > > > > > > > + if (priv->alt) > > > > > > > + input_report_abs(priv->input, ABS_RZ, value); > > > > > > > + else > > > > > > > + input_report_abs(priv->input, ABS_THROTTLE, value); > > > > > > > + > > > > > > > + priv->alt ^= 1; > > > > > > > + break; > > > > > > > + case ORANGERX: > > > > > > > + if (size != ORANGERX_DSIZE) > > > > > > > + break; > > > > > > > + input_report_abs(priv->input, ABS_X, raw_data[0]); > > > > > > > + input_report_abs(priv->input, ABS_Y, raw_data[2]); > > > > > > > + input_report_abs(priv->input, ABS_RX, raw_data[3]); > > > > > > > + input_report_abs(priv->input, ABS_RY, raw_data[1]); > > > > > > > + input_report_abs(priv->input, ABS_RUDDER, raw_data[5]); > > > > > > > + input_report_abs(priv->input, ABS_THROTTLE, raw_data[6]); > > > > > > > + break; > > > > > > > + }; > > > > > > > + > > > > > > > + input_sync(priv->input); > > > > > > > + return 0; > > > > > > > +} > > > > > > > + > > > > > > > +static int rcsim_probe(struct hid_device *hdev, const struct hid_device_id *id) > > > > > > > +{ > > > > > > > + struct device *dev = &hdev->dev; > > > > > > > + struct rcsim_priv *priv; > > > > > > > + int ret; > > > > > > > + > > > > > > > + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver)) > > > > > > > + return -ENODEV; > > > > > > > > > > > > You are not accessing anything in the USB stack, so there is no need > > > > > > to prevent regression tests that could inject uhid devices to your > > > > > > drivers. > > > > > > > > > > Ok, thanks. > > > > > > > > > > > > > > > > > Cheers, > > > > > > Benjamin > > > > > > > > > > > > > > > > Best regards, > > > > > Marcus Folkesson > > > > > > > > > > [1] https://www.usb.org/hid > > > > > > > > > > > > > If you need help in writing report descriptors, I can give you some, > > > > but the easiest might be for you to start from the report descriptor > > > > in hid-sony.c. I used to have a tool to dynamically write a report > > > > descriptor, but I'm not sure it still works... > > > > > > I've worked with the report descriptor for VRC2, and have come up with > > > something that works. > > > > Great that you managed to fill in most of the gaps :) > > > > > > > > > > > static __u8 vrc2_rdesc_fixed[] = { > > > 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) > > > 0x09, 0x04, // Usage (Joystick) > > > 0xA1, 0x01, // Collection (Application) > > > 0x09, 0x01, // Usage (Pointer) > > > 0xA1, 0x00, // Collection (Physical) > > > 0x09, 0x30, // Usage (X) > > > 0x09, 0x31, // Usage (Y) > > > 0x15, 0x00, // Logical Minimum (0) > > > 0x26, 0xFF, 0x07, // Logical Maximum (2047) > > > 0x35, 0x00, // Physical Minimum (0) > > > 0x46, 0xFF, 0x00, // Physical Maximum (255) > > > 0x75, 0x10, // Report Size (16) > > > 0x95, 0x02, // Report Count (2) > > > 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) > > > 0xC0, // End Collection > > > 0x95, 0x01, // Report Count (1) > > > 0x75, 0x18, // Report Size (24) > > > 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) > > > 0x09, 0x00, // Usage (Undefined) > > > 0x15, 0x00, // Logical Minimum (0) > > > 0x26, 0xFF, 0x00, // Logical Maximum (255) > > > 0x75, 0x08, // Report Size (8) > > > 0x95, 0x08, // Report Count (8) > > > 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) > > > 0xC0, // End Collection > > > }; > > > > > > > > > It is a result of reading specifications and looking at various > > > examples. > > > > > > These lines are taken from an example, and I do not know why I need > > > those. I think the constant report size is wrong (should be 32 to > > > > Do you have an example of the actual data sent on the wire? hexdump or > > hid-recorder should be enough for me to get what is missing. > > Sorry, this will be a little confusing. > > I just realized that there are actually two hidraw devices showing > up. > > One with this descriptor: > 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) > 0x09, 0x00, // Usage (Undefined) > 0xA1, 0x01, // Collection (Application) > 0x15, 0x00, // Logical Minimum (0) > 0x25, 0x01, // Logical Maximum (1) > 0x75, 0x01, // Report Size (1) > 0x05, 0x09, // Usage Page (Button) > 0x19, 0x01, // Usage Minimum (0x01) > 0x29, 0x3F, // Usage Maximum (0x3F) > 0x95, 0x40, // Report Count (64) > 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) > 0xC0, // End Collection > > Output from hid-recorder: > E: 000074.735891 8 20 00 ed 03 00 00 00 cc > # Button: 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 1 1 1 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 1 0 0 1 , 1 > > At a first glance, it did look as the button bits follows the same > pattern as the joystick axis in the descriptor below. It is hard to > debug though, as it causes random clicks and button presses > everywhere... and it driving me nuts. Maybe the easiest is to ignore that hid device in .probe: - if the report descriptor size is 23, return -ENODEV. And then see if you get all the correct events with the second HID device. > > > The other one looks like this: > 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) > 0x09, 0x04, // Usage (Joystick) > 0xA1, 0x01, // Collection (Application) > 0x09, 0x01, // Usage (Pointer) > 0xA1, 0x00, // Collection (Physical) > 0x09, 0x30, // Usage (X) > 0x09, 0x31, // Usage (Y) > 0x09, 0x32, // Usage (Z) > 0x15, 0x00, // Logical Minimum (0) > 0x26, 0xFF, 0x07, // Logical Maximum (2047) > 0x35, 0x00, // Physical Minimum (0) > 0x46, 0xFF, 0x00, // Physical Maximum (255) > 0x75, 0x10, // Report Size (16) > 0x95, 0x03, // Report Count (3) > 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) > 0xC0, // End Collection > 0x05, 0x09, // Usage Page (Button) > 0x19, 0x01, // Usage Minimum (0x01) > 0x29, 0x02, // Usage Maximum (0x02) > 0x15, 0x00, // Logical Minimum (0) > 0x25, 0x01, // Logical Maximum (1) > 0x95, 0x02, // Report Count (2) > 0x75, 0x01, // Report Size (1) > 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) > 0x95, 0x01, // Report Count (1) > 0x75, 0x06, // Report Size (6) > 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) > 0x09, 0x00, // Usage (0x00) > 0x15, 0x00, // Logical Minimum (0) > 0x26, 0xFF, 0x00, // Logical Maximum (255) > 0x75, 0x08, // Report Size (8) > 0x95, 0x08, // Report Count (8) > 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) > 0xC0, // End Collection > > Output from hid-recorder: > E: 000013.240054 7 00 00 00 00 80 00 00 > # X: 110 | Y: 122 | Z: 0 | Button: 0 0 | # > > > The controller does only support 2 axis (X/Y) and no buttons, so this > descriptor is wrong in any way. Well, if the values are always 0, it is not so terrible. It will look like a proper controller with buttons that are never pressed, which is good enough in 99% of the cases. > I see that it makes use of Feature as well. > Does this feature have something to do with the first HID device? The fact that you have 2 hidraw devices, means that you have 2 separate USB endpoints on the device. Virtually, you have 2 entirely separated USB devices in one. Features are defined per USB endpoint. So you can not define a feature on one device if the device doesn't support it, and the other way around. > > > > > As I read the report descriptors, we had: > > * old one: > > - opening of application collection of usage undefined > > - 64 buttons (logical max of 1), usage being being 0x01 and 0x3F > > (63) -> so the last bit is ignored > > > > Which would translate to a report of the following (each 0 being a > > separate button, | is marking bytes): > > > > 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, > > 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, > > 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, 0 | 0, 0, 0, 0, 0, 0, 0, X > > > > -> 64 * 1 = 64 bits > > > > ()last bit being ignored because of its duplicated usage with the previous one) > > > > * new one: > > - opening of an application collection of usage Joystick > > - opening of a physical collection of type Pointer > > - 16 bits for X, with logical max of 2047 and physical max of 255 > > - 16 bits for Y, with logical max of 2047 and physical max of 255 > > - 1 const element of size 24 that has to be ignored > > - one feature that should not be there :) > > > > So it would translate into the following (c being constant bit): > > X (2 bytes) | Y (2 bytes) | c, c, c, c, c, c, c, c | c, c, c, c, c, c, > > c, c | c, c, c, c, c, c, c, c > > > > -> 24 + 2*8 + 2*8 = 56 bits > > > > So I agree with you, you are missing 16 bits > > > > > fill up) and I do not know what the Feature is used for. > > > > You should not have to declare a feature. A feature is a special HID > > endpoint that you can use to talk to the HID device. By emitting > > special HID commands, (HID_REPORT_GET/HID_REPORT_SET) you can control > > how the device works. But if your current report descriptor doesn't > > have a feature declared, you should not declare it. > > > > > > > > 0x95, 0x01, // Report Count (1) > > > 0x75, 0x18, // Report Size (24) > > > 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) > > > 0x09, 0x00, // Usage (Undefined) > > > 0x15, 0x00, // Logical Minimum (0) > > > 0x26, 0xFF, 0x00, // Logical Maximum (255) > > > 0x75, 0x08, // Report Size (8) > > > 0x95, 0x08, // Report Count (8) > > > 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) > > > > > > If I omit any of these lines then I only get tons of > > > > > > kernel: usb 3-12.3.1.3.3: input irq status -75 received > > > > I assume the feature in this particular case helps the driver to > > understand that you need to allocate a 64 bits chunk of memory to > > communicate with the device. > > > > I would advise to use the following report instead: > > > > ---- > > static __u8 vrc2_rdesc_fixed[] = { > > 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) > > 0x09, 0x04, // Usage (Joystick) > > 0xA1, 0x01, // Collection (Application) > > 0x09, 0x01, // Usage (Pointer) > > 0xA1, 0x00, // Collection (Physical) > > 0x09, 0x30, // Usage (X) > > 0x09, 0x31, // Usage (Y) > > 0x15, 0x00, // Logical Minimum (0) > > 0x26, 0xFF, 0x07, // Logical Maximum (2047) > > 0x35, 0x00, // Physical Minimum (0) > > 0x46, 0xFF, 0x00, // Physical Maximum (255) > > 0x75, 0x10, // Report Size (16) > > 0x95, 0x02, // Report Count (2) > > 0x81, 0x02, // Input (Data,Var,Abs,No > > Wrap,Linear,Preferred State,No Null Position) > > 0xC0, // End Collection > > 0x75, 0x08, // Report Size (8) > > 0x95, 0x04, // Report Count (4) > > 0x81, 0x03, // Input (Cnst,Var,Abs) > > 0xC0, // End Collection > > }; > > --- > > This report is more of what I want it to look like. > > I got the same "usb 3-12.3.1.3.1: input irq status -75 received" error > when using this descriptor though. > I think it is beacause it is applied to both hid-devices? Yes, you need to check which one you are applying the fixup. The easiest for that is to check for the current report descriptor size. If it's the one you expect, change it. If not, ignore it. But let's first have a look at ignoring the first hidraw device entirely and see if it solves the problem for this one. Cheers, Benjamin > > > > > > Few changes: > > - use of 4 bytes of padding, instead of 1 24bits slot of padding > > - changed the padding from 0x01 to 0x03 to remove the "array" const > > definition which is tricky to handle > > - removed the feature report. > > > > Cheers, > > Benjamin > > > > > > > > In the kernel log. > > > > > > > > > > > > > > FYI, I just re-read rcsim_raw_event() and there is stuff that would > > > > require more than just a report descriptor fixup (the fact that some > > > > data is sent every other report is not good and will need some manual > > > > handling though). > > > > > > > > Cheers, > > > > Benjamin > > > > > > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-uclogic-rdesc.c > > > > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-input.c#n817 > > > > > > > > > > > > > Best regards > > > Marcus Folkesson > > > > Best regards, > Marcus Folkesson ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers 2022-08-23 9:49 ` [RESEND PATCH 1/2] HID: Add driver " Benjamin Tissoires 2022-08-23 15:18 ` Marcus Folkesson @ 2023-03-25 16:08 ` Pavel Machek 1 sibling, 0 replies; 16+ messages in thread From: Pavel Machek @ 2023-03-25 16:08 UTC (permalink / raw) To: Benjamin Tissoires Cc: Marcus Folkesson, Jonathan Corbet, Jiri Kosina, Linux Doc Mailing List, lkml, open list:HID CORE LAYER Hi! > > +Many RC controllers is able to configure which stick goes to which channel. > > +This is also configurable in most simulators, so a matching is not necessary. While it is configurable, we should try to do the good job "by default". Configuring controllers can take hours, and it is not fun. > > +The driver is generating the following input event for on channels: > > + > > ++---------+----------------+ > > +| Channel | Event | > > ++=========+================+ > > +| 1 | ABS_Y | > > ++---------+----------------+ > > +| 2 | ABS_X | > > ++---------+----------------+ > > +| 3 | ABS_RY | > > ++---------+----------------+ > > +| 4 | ABS_RX | > > ++---------+----------------+ > > +| 5 | BTN_A | > > ++---------+----------------+ > > +| 6 | BTN_B | > > ++---------+----------------+ If one of these is normally used as throttle and rudder, it should be marked as such... User can then remap it if he does not like the mapping. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-03-25 16:08 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-22 6:09 [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers Marcus Folkesson 2022-08-22 6:09 ` [RESEND PATCH 2/2] MAINTAINERS: Add entry " Marcus Folkesson 2022-08-23 9:49 ` [RESEND PATCH 1/2] HID: Add driver " Benjamin Tissoires 2022-08-23 15:18 ` Marcus Folkesson 2022-08-23 16:43 ` Benjamin Tissoires 2022-08-25 6:48 ` Marcus Folkesson 2022-08-30 12:45 ` Benjamin Tissoires 2022-09-15 7:34 ` Marcus Folkesson 2022-09-15 7:35 ` Benjamin Tissoires 2022-09-19 13:32 ` Benjamin Tissoires 2023-01-10 14:48 ` Marcus Folkesson 2022-08-28 14:18 ` Marcus Folkesson 2022-08-30 12:35 ` Benjamin Tissoires 2022-08-30 14:43 ` Marcus Folkesson 2022-08-30 15:01 ` Benjamin Tissoires 2023-03-25 16:08 ` Pavel Machek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).