| Message ID | CAPsNqc427=u2B02fPpi3Dtw-K7dtSGfphq3qi-PeBNTRRmDVTg@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | HID: sony: Fix SHANWAN PS3 GamePad rumbling on USB again | expand |
On Fri, 28 Dec 2018 10:26:14 +0800 liquid <outmatch@gmail.com> wrote: > >From 6323ffb83e23eab9a08208063f9dba27c6a0d228 Mon Sep 17 00:00:00 2001 > From: Hongye Yuan <outmatch@gmail.com> > Date: Thu, 27 Dec 2018 09:41:23 +0800 > Subject: [PATCH] HID: sony: Fix SHANWAN PS3 GamePad rumbling on USB again > > - The SHANWAN DS3 clone joystick requires HID Output Reports via Interrupt EP. > - Added a quirk for SHANWAN PS3 GamePad. > > Signed-off-by: Hongye Yuan <outmatch@gmail.com> Hi Hongye, I am CCing Roderick Colenbrander who is the de-facto hid-sony maintainer. Maybe Roderick should be mentioned in MAINTAINERS. I guess you can skip linux-usb and Greg on the next iteration to spare them some traffic. Please consider using "git send-email" to send patches created with "git format-patch" to avoid problems with formatting, for instance I don't see any TABs in the patch below, all TABs seem to have been converted to spaces and the patch does not apply. Make sure to use ./scripts/checkpatch.pl to spot other problems. Some other minor comments are inlined below. > --- > drivers/hid/hid-sony.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c > index 9671a4bad643..842f370cfec2 100644 > --- a/drivers/hid/hid-sony.c > +++ b/drivers/hid/hid-sony.c > @@ -58,6 +58,7 @@ > #define FUTUREMAX_DANCE_MAT BIT(13) > #define NSG_MR5U_REMOTE_BT BIT(14) > #define NSG_MR7U_REMOTE_BT BIT(15) > +#define SHANWAN_GAMEPAD BIT(16) > > #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT) > #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB | MOTION_CONTROLLER_BT) > @@ -1490,6 +1491,7 @@ static int sony_register_sensors(struct sony_sc *sc) > */ > static int sixaxis_set_operational_usb(struct hid_device *hdev) > { > + struct sony_sc *sc = hid_get_drvdata(hdev); > const int buf_size = > max(SIXAXIS_REPORT_0xF2_SIZE, SIXAXIS_REPORT_0xF5_SIZE); > u8 *buf; > @@ -1521,7 +1523,7 @@ static int sixaxis_set_operational_usb(struct > hid_device *hdev) > * But the USB interrupt would cause SHANWAN controllers to > * start rumbling non-stop. > */ > - if (strcmp(hdev->name, "SHANWAN PS3 GamePad")) { > + if (sc->quirks & SHANWAN_GAMEPAD) { I think you are inverting the logic here, the original test meant to check that the controller was NOT a "SHANWAN PS3 GamePad" before doing step 3. Maybe this can be made more explicit with the change below (ideally in a preliminary patch): ----------------------------------------------------------------------- diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c index 9671a4bad643..949305cfa199 100644 --- a/drivers/hid/hid-sony.c +++ b/drivers/hid/hid-sony.c @@ -1519,14 +1519,15 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev) /* * But the USB interrupt would cause SHANWAN controllers to - * start rumbling non-stop. + * start rumbling non-stop, so skip step 3 for these controllers. */ - if (strcmp(hdev->name, "SHANWAN PS3 GamePad")) { - ret = hid_hw_output_report(hdev, buf, 1); - if (ret < 0) { - hid_info(hdev, "can't set operational mode: step 3, ignoring\n"); - ret = 0; - } + if (!strcmp(hdev->name, "SHANWAN PS3 GamePad")) + goto out; + + ret = hid_hw_output_report(hdev, buf, 1); + if (ret < 0) { + hid_info(hdev, "can't set operational mode: step 3, ignoring\n"); + ret = 0; } out: ----------------------------------------------------------------------- Unfortunately I cannot test it. After that change you can check for (sc->quirks & SHANWAN_GAMEPAD) in your patch. > ret = hid_hw_output_report(hdev, buf, 1); > if (ret < 0) { > hid_info(hdev, "can't set operational mode: step 3, ignoring\n"); > @@ -2097,9 +2099,13 @@ static void sixaxis_send_output_report(struct > sony_sc *sc) > } > } > > - hid_hw_raw_request(sc->hdev, report->report_id, (u8 *)report, > - sizeof(struct sixaxis_output_report), > - HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); > + /* SHANWAN controllers require sending output reports via intr channel */ > + if (sc->quirks & SHANWAN_GAMEPAD) > + hid_hw_output_report(sc->hdev, (u8 *)report, > + sizeof(struct sixaxis_output_report)); > + else hid_hw_raw_request(sc->hdev, report->report_id, (u8 *)report, > + sizeof(struct sixaxis_output_report), > + HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); The body of the else condition needs to go on a new line, with an extra indentation level. Also, this makes me wonder if the problem might be about HID quirks, can you try playing with hdev->quirks settings in sony_input_configured()? Ciao, Antonio > } > > static void dualshock4_send_output_report(struct sony_sc *sc) > @@ -2811,6 +2817,9 @@ static int sony_probe(struct hid_device *hdev, > const struct hid_device_id *id) > if (!strcmp(hdev->name, "FutureMax Dance Mat")) > quirks |= FUTUREMAX_DANCE_MAT; > > + if (!strcmp(hdev->name, "SHANWAN PS3 GamePad")) > + quirks |= SHANWAN_GAMEPAD; > + > sc = devm_kzalloc(&hdev->dev, sizeof(*sc), GFP_KERNEL); > if (sc == NULL) { > hid_err(hdev, "can't alloc sony descriptor\n"); > -- > 2.20.1
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c index 9671a4bad643..842f370cfec2 100644 --- a/drivers/hid/hid-sony.c +++ b/drivers/hid/hid-sony.c @@ -58,6 +58,7 @@ #define FUTUREMAX_DANCE_MAT BIT(13) #define NSG_MR5U_REMOTE_BT BIT(14) #define NSG_MR7U_REMOTE_BT BIT(15) +#define SHANWAN_GAMEPAD BIT(16) #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT) #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB | MOTION_CONTROLLER_BT) @@ -1490,6 +1491,7 @@ static int sony_register_sensors(struct sony_sc *sc) */ static int sixaxis_set_operational_usb(struct hid_device *hdev) { + struct sony_sc *sc = hid_get_drvdata(hdev); const int buf_size = max(SIXAXIS_REPORT_0xF2_SIZE, SIXAXIS_REPORT_0xF5_SIZE); u8 *buf; @@ -1521,7 +1523,7 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev) * But the USB interrupt would cause SHANWAN controllers to * start rumbling non-stop. */ - if (strcmp(hdev->name, "SHANWAN PS3 GamePad")) { + if (sc->quirks & SHANWAN_GAMEPAD) { ret = hid_hw_output_report(hdev, buf, 1); if (ret < 0) {