Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Mon, 2018-08-13 at 07:37 -0700, Andrey Smirnov wrote: > On Fri, Aug 10, 2018 at 4:38 AM Bastien Nocera > wrote: > > > > On Thu, 2018-08-09 at 17:17 -0700, Andrey Smirnov wrote: > > > Add HID quirk driver for Xbox One S controller over bluetooth. > > > > > > This driver only adds support for rumble. Standard controller > > > functionality is exposed by default HID driver. > > > > Did you manage to make the joypad work without hacks in the > > Bluetooth > > stack[1]? > > I was not aware this hack actually existed, but now that I see it I > think it explains why doing > > echo 1 > /sys/module/bluetooth/parameters/disable_ertm > > was necessary to make the controller connect over Bluetooth on my > machine. It looks like disabling ERTM just happens to have the same > effect as the hack that you mention. I'd like/plan to look into > finding a proper solution to replace that hack, but that'd probably > be > a separate patch. I gave it a try a couple of months back, but without much success. L2CAP is unfortunately a bit too low-level to know anything about the device which requested this. I think that some level of "STREAMING" support is needed for the Bluetooth audio portion of the device. Marcel was also interested in looking into the problem, but didn't get to it. Let me know if you manage to get anything working without the big hammer of disabling ERTM support for everything. Cheers
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Mon, 2018-08-13 at 07:37 -0700, Andrey Smirnov wrote: > On Fri, Aug 10, 2018 at 4:38 AM Bastien Nocera > wrote: > > > > On Thu, 2018-08-09 at 17:17 -0700, Andrey Smirnov wrote: > > > Add HID quirk driver for Xbox One S controller over bluetooth. > > > > > > This driver only adds support for rumble. Standard controller > > > functionality is exposed by default HID driver. > > > > Did you manage to make the joypad work without hacks in the > > Bluetooth > > stack[1]? > > I was not aware this hack actually existed, but now that I see it I > think it explains why doing > > echo 1 > /sys/module/bluetooth/parameters/disable_ertm > > was necessary to make the controller connect over Bluetooth on my > machine. It looks like disabling ERTM just happens to have the same > effect as the hack that you mention. I'd like/plan to look into > finding a proper solution to replace that hack, but that'd probably > be > a separate patch. I gave it a try a couple of months back, but without much success. L2CAP is unfortunately a bit too low-level to know anything about the device which requested this. I think that some level of "STREAMING" support is needed for the Bluetooth audio portion of the device. Marcel was also interested in looking into the problem, but didn't get to it. Let me know if you manage to get anything working without the big hammer of disabling ERTM support for everything. Cheers
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Fri, Aug 10, 2018 at 4:38 AM Bastien Nocera wrote: > > On Thu, 2018-08-09 at 17:17 -0700, Andrey Smirnov wrote: > > Add HID quirk driver for Xbox One S controller over bluetooth. > > > > This driver only adds support for rumble. Standard controller > > functionality is exposed by default HID driver. > > Did you manage to make the joypad work without hacks in the Bluetooth > stack[1]? I was not aware this hack actually existed, but now that I see it I think it explains why doing echo 1 > /sys/module/bluetooth/parameters/disable_ertm was necessary to make the controller connect over Bluetooth on my machine. It looks like disabling ERTM just happens to have the same effect as the hack that you mention. I'd like/plan to look into finding a proper solution to replace that hack, but that'd probably be a separate patch. Thanks, Andrey Smirnov
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Fri, Aug 10, 2018 at 4:38 AM Bastien Nocera wrote: > > On Thu, 2018-08-09 at 17:17 -0700, Andrey Smirnov wrote: > > Add HID quirk driver for Xbox One S controller over bluetooth. > > > > This driver only adds support for rumble. Standard controller > > functionality is exposed by default HID driver. > > Did you manage to make the joypad work without hacks in the Bluetooth > stack[1]? I was not aware this hack actually existed, but now that I see it I think it explains why doing echo 1 > /sys/module/bluetooth/parameters/disable_ertm was necessary to make the controller connect over Bluetooth on my machine. It looks like disabling ERTM just happens to have the same effect as the hack that you mention. I'd like/plan to look into finding a proper solution to replace that hack, but that'd probably be a separate patch. Thanks, Andrey Smirnov
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Fri, Aug 10, 2018 at 2:37 AM Benjamin Tissoires
wrote:
>
> Hi Andrew,
>
> On Fri, Aug 10, 2018 at 2:17 AM Andrey Smirnov
> wrote:
> >
> > Add HID quirk driver for Xbox One S controller over bluetooth.
> >
> > This driver only adds support for rumble. Standard controller
> > functionality is exposed by default HID driver.
> >
> > Cc: Dmitry Torokhov
> > Cc: Jiri Kosina
> > Cc: Benjamin Tissoires
> > Cc: linux-in...@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: Pierre-Loup A. Griffais
> > Signed-off-by: Juha Kuikka
> > Signed-off-by: Andrey Smirnov
> > ---
> > drivers/hid/Kconfig | 8 ++
> > drivers/hid/hid-ids.h | 1 +
> > drivers/hid/hid-microsoft.c | 142 ++--
> > drivers/hid/hid-quirks.c| 1 +
> > 4 files changed, 147 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index a49a10437c40..b2e983cb32f0 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -589,6 +589,14 @@ config HID_MICROSOFT
> > ---help---
> > Support for Microsoft devices that are not fully compliant with HID
> > standard.
> >
> > +config MICROSOFT_FF
> > + bool "Microsoft Xbox One S controller force feedback support"
> > + depends on HID_MICROSOFT
> > + select INPUT_FF_MEMLESS
> > + ---help---
> > + Say Y here if you have a Microsoft Xbox One S controller and want
> > to enable
> > + force feedback support for it.
> > +
>
> Is there really a need to have a kernel that doesn't enable FF on
> those particular devices?
> It adds a lot of #ifdefs in the code and I would think users want it
> enabled by default.
>
No, I don't think there is. This is just something that similar Sony
PS4 driver does, but I am more than happy to drop this option. Will do
in v2.
> > config HID_MONTEREY
> > tristate "Monterey Genius KB29E keyboard"
> > depends on HID
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index c7981ddd8776..c8b4bc7fe053 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -798,6 +798,7 @@
> > #define USB_DEVICE_ID_MS_TOUCH_COVER_2 0x07a7
> > #define USB_DEVICE_ID_MS_TYPE_COVER_20x07a9
> > #define USB_DEVICE_ID_MS_POWER_COVER 0x07da
> > +#define USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER 0x02fd
> >
> > #define USB_VENDOR_ID_MOJO 0x8282
> > #define USB_DEVICE_ID_RETRO_ADAPTER0x3201
> > diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
> > index 96e7d3231d2f..e2bd31d54e62 100644
> > --- a/drivers/hid/hid-microsoft.c
> > +++ b/drivers/hid/hid-microsoft.c
> > @@ -29,10 +29,29 @@
> > #define MS_NOGET 0x10
> > #define MS_DUPLICATE_USAGES0x20
> >
> > +struct ms_data {
> > + unsigned long quirks;
> > + struct hid_device *hdev;
> > + struct work_struct ff_worker;
> > + __u8 strong;
> > + __u8 weak;
> > + __u8 *output_report_dmabuf;
> > +};
> > +
> > +struct xb1s_ff_report {
> > + __u8report_id;
> > + __u8enable;
> > + __u8magnitude[4];
> > + __u8duration_10ms;
> > + __u8start_delay_10ms;
> > + __u8loop_count;
> > +};
> > +
> > static __u8 *ms_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> > unsigned int *rsize)
> > {
> > - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> > + struct ms_data *ms = hid_get_drvdata(hdev);
> > + unsigned long quirks = ms->quirks;
> >
> > /*
> > * Microsoft Wireless Desktop Receiver (Model 1028) has
> > @@ -134,7 +153,8 @@ static int ms_input_mapping(struct hid_device *hdev,
> > struct hid_input *hi,
> > struct hid_field *field, struct hid_usage *usage,
> > unsigned long **bit, int *max)
> > {
> > - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> > + struct ms_data *ms = hid_get_drvdata(hdev);
> > + unsigned long quirks = ms->quirks;
>
> Could you split the change of the quirks mechanism in its own patch.
> This will make the whole code easier to review and spot errors if
> there are.
>
Sure, sounds like a good idea, will do in v2.
> >
> > if (quirks & MS_ERGONOMY) {
> > int ret = ms_ergonomy_kb_quirk(hi, usage, bit, max);
> > @@ -153,7 +173,8 @@ static int ms_input_mapped(struct hid_device *hdev,
> > struct hid_input *hi,
> > struct hid_field *field, struct hid_usage *usage,
> > unsigned long **bit, int *max)
> > {
> > - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> > + struct ms_data *ms = hid_get_drvdata(hdev);
> > + unsigned long quirks = ms->quirks;
> >
> > if (quirks & MS_DUPLICATE_USAGES)
> > clear_bit(usage->code, *bit);
> > @@ -164,7 +185,8 @@ static int ms_input_mapped(struct hid_device *hdev,
> > struct hid_input *hi,
> >
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Fri, Aug 10, 2018 at 2:37 AM Benjamin Tissoires
wrote:
>
> Hi Andrew,
>
> On Fri, Aug 10, 2018 at 2:17 AM Andrey Smirnov
> wrote:
> >
> > Add HID quirk driver for Xbox One S controller over bluetooth.
> >
> > This driver only adds support for rumble. Standard controller
> > functionality is exposed by default HID driver.
> >
> > Cc: Dmitry Torokhov
> > Cc: Jiri Kosina
> > Cc: Benjamin Tissoires
> > Cc: linux-in...@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: Pierre-Loup A. Griffais
> > Signed-off-by: Juha Kuikka
> > Signed-off-by: Andrey Smirnov
> > ---
> > drivers/hid/Kconfig | 8 ++
> > drivers/hid/hid-ids.h | 1 +
> > drivers/hid/hid-microsoft.c | 142 ++--
> > drivers/hid/hid-quirks.c| 1 +
> > 4 files changed, 147 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index a49a10437c40..b2e983cb32f0 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -589,6 +589,14 @@ config HID_MICROSOFT
> > ---help---
> > Support for Microsoft devices that are not fully compliant with HID
> > standard.
> >
> > +config MICROSOFT_FF
> > + bool "Microsoft Xbox One S controller force feedback support"
> > + depends on HID_MICROSOFT
> > + select INPUT_FF_MEMLESS
> > + ---help---
> > + Say Y here if you have a Microsoft Xbox One S controller and want
> > to enable
> > + force feedback support for it.
> > +
>
> Is there really a need to have a kernel that doesn't enable FF on
> those particular devices?
> It adds a lot of #ifdefs in the code and I would think users want it
> enabled by default.
>
No, I don't think there is. This is just something that similar Sony
PS4 driver does, but I am more than happy to drop this option. Will do
in v2.
> > config HID_MONTEREY
> > tristate "Monterey Genius KB29E keyboard"
> > depends on HID
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index c7981ddd8776..c8b4bc7fe053 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -798,6 +798,7 @@
> > #define USB_DEVICE_ID_MS_TOUCH_COVER_2 0x07a7
> > #define USB_DEVICE_ID_MS_TYPE_COVER_20x07a9
> > #define USB_DEVICE_ID_MS_POWER_COVER 0x07da
> > +#define USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER 0x02fd
> >
> > #define USB_VENDOR_ID_MOJO 0x8282
> > #define USB_DEVICE_ID_RETRO_ADAPTER0x3201
> > diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
> > index 96e7d3231d2f..e2bd31d54e62 100644
> > --- a/drivers/hid/hid-microsoft.c
> > +++ b/drivers/hid/hid-microsoft.c
> > @@ -29,10 +29,29 @@
> > #define MS_NOGET 0x10
> > #define MS_DUPLICATE_USAGES0x20
> >
> > +struct ms_data {
> > + unsigned long quirks;
> > + struct hid_device *hdev;
> > + struct work_struct ff_worker;
> > + __u8 strong;
> > + __u8 weak;
> > + __u8 *output_report_dmabuf;
> > +};
> > +
> > +struct xb1s_ff_report {
> > + __u8report_id;
> > + __u8enable;
> > + __u8magnitude[4];
> > + __u8duration_10ms;
> > + __u8start_delay_10ms;
> > + __u8loop_count;
> > +};
> > +
> > static __u8 *ms_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> > unsigned int *rsize)
> > {
> > - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> > + struct ms_data *ms = hid_get_drvdata(hdev);
> > + unsigned long quirks = ms->quirks;
> >
> > /*
> > * Microsoft Wireless Desktop Receiver (Model 1028) has
> > @@ -134,7 +153,8 @@ static int ms_input_mapping(struct hid_device *hdev,
> > struct hid_input *hi,
> > struct hid_field *field, struct hid_usage *usage,
> > unsigned long **bit, int *max)
> > {
> > - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> > + struct ms_data *ms = hid_get_drvdata(hdev);
> > + unsigned long quirks = ms->quirks;
>
> Could you split the change of the quirks mechanism in its own patch.
> This will make the whole code easier to review and spot errors if
> there are.
>
Sure, sounds like a good idea, will do in v2.
> >
> > if (quirks & MS_ERGONOMY) {
> > int ret = ms_ergonomy_kb_quirk(hi, usage, bit, max);
> > @@ -153,7 +173,8 @@ static int ms_input_mapped(struct hid_device *hdev,
> > struct hid_input *hi,
> > struct hid_field *field, struct hid_usage *usage,
> > unsigned long **bit, int *max)
> > {
> > - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> > + struct ms_data *ms = hid_get_drvdata(hdev);
> > + unsigned long quirks = ms->quirks;
> >
> > if (quirks & MS_DUPLICATE_USAGES)
> > clear_bit(usage->code, *bit);
> > @@ -164,7 +185,8 @@ static int ms_input_mapped(struct hid_device *hdev,
> > struct hid_input *hi,
> >
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Thu, 2018-08-09 at 17:17 -0700, Andrey Smirnov wrote: > Add HID quirk driver for Xbox One S controller over bluetooth. > > This driver only adds support for rumble. Standard controller > functionality is exposed by default HID driver. Did you manage to make the joypad work without hacks in the Bluetooth stack[1]? [1]: https://github.com/ValveSoftware/steamos_kernel/commit/549c3dc10fa3749b3999549a2672b14bf0e9786d
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Thu, 2018-08-09 at 17:17 -0700, Andrey Smirnov wrote: > Add HID quirk driver for Xbox One S controller over bluetooth. > > This driver only adds support for rumble. Standard controller > functionality is exposed by default HID driver. Did you manage to make the joypad work without hacks in the Bluetooth stack[1]? [1]: https://github.com/ValveSoftware/steamos_kernel/commit/549c3dc10fa3749b3999549a2672b14bf0e9786d
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
Hi Andrew,
On Fri, Aug 10, 2018 at 2:17 AM Andrey Smirnov wrote:
>
> Add HID quirk driver for Xbox One S controller over bluetooth.
>
> This driver only adds support for rumble. Standard controller
> functionality is exposed by default HID driver.
>
> Cc: Dmitry Torokhov
> Cc: Jiri Kosina
> Cc: Benjamin Tissoires
> Cc: linux-in...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Pierre-Loup A. Griffais
> Signed-off-by: Juha Kuikka
> Signed-off-by: Andrey Smirnov
> ---
> drivers/hid/Kconfig | 8 ++
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-microsoft.c | 142 ++--
> drivers/hid/hid-quirks.c| 1 +
> 4 files changed, 147 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index a49a10437c40..b2e983cb32f0 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -589,6 +589,14 @@ config HID_MICROSOFT
> ---help---
> Support for Microsoft devices that are not fully compliant with HID
> standard.
>
> +config MICROSOFT_FF
> + bool "Microsoft Xbox One S controller force feedback support"
> + depends on HID_MICROSOFT
> + select INPUT_FF_MEMLESS
> + ---help---
> + Say Y here if you have a Microsoft Xbox One S controller and want to
> enable
> + force feedback support for it.
> +
Is there really a need to have a kernel that doesn't enable FF on
those particular devices?
It adds a lot of #ifdefs in the code and I would think users want it
enabled by default.
> config HID_MONTEREY
> tristate "Monterey Genius KB29E keyboard"
> depends on HID
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index c7981ddd8776..c8b4bc7fe053 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -798,6 +798,7 @@
> #define USB_DEVICE_ID_MS_TOUCH_COVER_2 0x07a7
> #define USB_DEVICE_ID_MS_TYPE_COVER_20x07a9
> #define USB_DEVICE_ID_MS_POWER_COVER 0x07da
> +#define USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER 0x02fd
>
> #define USB_VENDOR_ID_MOJO 0x8282
> #define USB_DEVICE_ID_RETRO_ADAPTER0x3201
> diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
> index 96e7d3231d2f..e2bd31d54e62 100644
> --- a/drivers/hid/hid-microsoft.c
> +++ b/drivers/hid/hid-microsoft.c
> @@ -29,10 +29,29 @@
> #define MS_NOGET 0x10
> #define MS_DUPLICATE_USAGES0x20
>
> +struct ms_data {
> + unsigned long quirks;
> + struct hid_device *hdev;
> + struct work_struct ff_worker;
> + __u8 strong;
> + __u8 weak;
> + __u8 *output_report_dmabuf;
> +};
> +
> +struct xb1s_ff_report {
> + __u8report_id;
> + __u8enable;
> + __u8magnitude[4];
> + __u8duration_10ms;
> + __u8start_delay_10ms;
> + __u8loop_count;
> +};
> +
> static __u8 *ms_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> unsigned int *rsize)
> {
> - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> + struct ms_data *ms = hid_get_drvdata(hdev);
> + unsigned long quirks = ms->quirks;
>
> /*
> * Microsoft Wireless Desktop Receiver (Model 1028) has
> @@ -134,7 +153,8 @@ static int ms_input_mapping(struct hid_device *hdev,
> struct hid_input *hi,
> struct hid_field *field, struct hid_usage *usage,
> unsigned long **bit, int *max)
> {
> - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> + struct ms_data *ms = hid_get_drvdata(hdev);
> + unsigned long quirks = ms->quirks;
Could you split the change of the quirks mechanism in its own patch.
This will make the whole code easier to review and spot errors if
there are.
>
> if (quirks & MS_ERGONOMY) {
> int ret = ms_ergonomy_kb_quirk(hi, usage, bit, max);
> @@ -153,7 +173,8 @@ static int ms_input_mapped(struct hid_device *hdev,
> struct hid_input *hi,
> struct hid_field *field, struct hid_usage *usage,
> unsigned long **bit, int *max)
> {
> - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> + struct ms_data *ms = hid_get_drvdata(hdev);
> + unsigned long quirks = ms->quirks;
>
> if (quirks & MS_DUPLICATE_USAGES)
> clear_bit(usage->code, *bit);
> @@ -164,7 +185,8 @@ static int ms_input_mapped(struct hid_device *hdev,
> struct hid_input *hi,
> static int ms_event(struct hid_device *hdev, struct hid_field *field,
> struct hid_usage *usage, __s32 value)
> {
> - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> + struct ms_data *ms = hid_get_drvdata(hdev);
> + unsigned long quirks = ms->quirks;
> struct input_dev *input;
>
> if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput ||
> @@ -219,12 +241,96 @@ static int ms_event(struct hid_device *hdev, struct
> hid_field *field,
>
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
Hi Andrew,
On Fri, Aug 10, 2018 at 2:17 AM Andrey Smirnov wrote:
>
> Add HID quirk driver for Xbox One S controller over bluetooth.
>
> This driver only adds support for rumble. Standard controller
> functionality is exposed by default HID driver.
>
> Cc: Dmitry Torokhov
> Cc: Jiri Kosina
> Cc: Benjamin Tissoires
> Cc: linux-in...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Pierre-Loup A. Griffais
> Signed-off-by: Juha Kuikka
> Signed-off-by: Andrey Smirnov
> ---
> drivers/hid/Kconfig | 8 ++
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-microsoft.c | 142 ++--
> drivers/hid/hid-quirks.c| 1 +
> 4 files changed, 147 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index a49a10437c40..b2e983cb32f0 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -589,6 +589,14 @@ config HID_MICROSOFT
> ---help---
> Support for Microsoft devices that are not fully compliant with HID
> standard.
>
> +config MICROSOFT_FF
> + bool "Microsoft Xbox One S controller force feedback support"
> + depends on HID_MICROSOFT
> + select INPUT_FF_MEMLESS
> + ---help---
> + Say Y here if you have a Microsoft Xbox One S controller and want to
> enable
> + force feedback support for it.
> +
Is there really a need to have a kernel that doesn't enable FF on
those particular devices?
It adds a lot of #ifdefs in the code and I would think users want it
enabled by default.
> config HID_MONTEREY
> tristate "Monterey Genius KB29E keyboard"
> depends on HID
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index c7981ddd8776..c8b4bc7fe053 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -798,6 +798,7 @@
> #define USB_DEVICE_ID_MS_TOUCH_COVER_2 0x07a7
> #define USB_DEVICE_ID_MS_TYPE_COVER_20x07a9
> #define USB_DEVICE_ID_MS_POWER_COVER 0x07da
> +#define USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER 0x02fd
>
> #define USB_VENDOR_ID_MOJO 0x8282
> #define USB_DEVICE_ID_RETRO_ADAPTER0x3201
> diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
> index 96e7d3231d2f..e2bd31d54e62 100644
> --- a/drivers/hid/hid-microsoft.c
> +++ b/drivers/hid/hid-microsoft.c
> @@ -29,10 +29,29 @@
> #define MS_NOGET 0x10
> #define MS_DUPLICATE_USAGES0x20
>
> +struct ms_data {
> + unsigned long quirks;
> + struct hid_device *hdev;
> + struct work_struct ff_worker;
> + __u8 strong;
> + __u8 weak;
> + __u8 *output_report_dmabuf;
> +};
> +
> +struct xb1s_ff_report {
> + __u8report_id;
> + __u8enable;
> + __u8magnitude[4];
> + __u8duration_10ms;
> + __u8start_delay_10ms;
> + __u8loop_count;
> +};
> +
> static __u8 *ms_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> unsigned int *rsize)
> {
> - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> + struct ms_data *ms = hid_get_drvdata(hdev);
> + unsigned long quirks = ms->quirks;
>
> /*
> * Microsoft Wireless Desktop Receiver (Model 1028) has
> @@ -134,7 +153,8 @@ static int ms_input_mapping(struct hid_device *hdev,
> struct hid_input *hi,
> struct hid_field *field, struct hid_usage *usage,
> unsigned long **bit, int *max)
> {
> - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> + struct ms_data *ms = hid_get_drvdata(hdev);
> + unsigned long quirks = ms->quirks;
Could you split the change of the quirks mechanism in its own patch.
This will make the whole code easier to review and spot errors if
there are.
>
> if (quirks & MS_ERGONOMY) {
> int ret = ms_ergonomy_kb_quirk(hi, usage, bit, max);
> @@ -153,7 +173,8 @@ static int ms_input_mapped(struct hid_device *hdev,
> struct hid_input *hi,
> struct hid_field *field, struct hid_usage *usage,
> unsigned long **bit, int *max)
> {
> - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> + struct ms_data *ms = hid_get_drvdata(hdev);
> + unsigned long quirks = ms->quirks;
>
> if (quirks & MS_DUPLICATE_USAGES)
> clear_bit(usage->code, *bit);
> @@ -164,7 +185,8 @@ static int ms_input_mapped(struct hid_device *hdev,
> struct hid_input *hi,
> static int ms_event(struct hid_device *hdev, struct hid_field *field,
> struct hid_usage *usage, __s32 value)
> {
> - unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
> + struct ms_data *ms = hid_get_drvdata(hdev);
> + unsigned long quirks = ms->quirks;
> struct input_dev *input;
>
> if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput ||
> @@ -219,12 +241,96 @@ static int ms_event(struct hid_device *hdev, struct
> hid_field *field,
>
