| Message ID | 20240115144538.12018-6-max@enpas.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: playstation: DS4: LED bugfix, third-party gamepad support | expand |
Hi Max, On Mon, Jan 15, 2024 at 6:52 AM Max Staudt <max@enpas.org> wrote: > > Some third-party controllers never switch to the full 0x11 report. > > They keep sending the short 0x01 report, so let's parse that instead. > > Signed-off-by: Max Staudt <max@enpas.org> > --- > drivers/hid/hid-playstation.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c > index 2bf44bd3cc8a..086b0768fa51 100644 > --- a/drivers/hid/hid-playstation.c > +++ b/drivers/hid/hid-playstation.c > @@ -287,6 +287,8 @@ struct dualsense_output_report { > > #define DS4_INPUT_REPORT_USB 0x01 > #define DS4_INPUT_REPORT_USB_SIZE 64 > +#define DS4_INPUT_REPORT_BT_MINIMAL 0x01 > +#define DS4_INPUT_REPORT_BT_MINIMAL_SIZE 10 > #define DS4_INPUT_REPORT_BT 0x11 > #define DS4_INPUT_REPORT_BT_SIZE 78 > #define DS4_OUTPUT_REPORT_USB 0x05 > @@ -2198,6 +2200,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * > int battery_status, i, j; > uint16_t sensor_timestamp; > unsigned long flags; > + bool is_minimal = false; > > /* > * DualShock4 in USB uses the full HID report for reportID 1, but > @@ -2225,6 +2228,18 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * > ds4_report = &bt->common; > num_touch_reports = bt->num_touch_reports; > touch_reports = bt->touch_reports; > + } else if (hdev->bus == BUS_BLUETOOTH && > + report->id == DS4_INPUT_REPORT_BT_MINIMAL && > + size == DS4_INPUT_REPORT_BT_MINIMAL_SIZE) { > + /* Some third-party pads never switch to the full 0x11 report. > + * The short 0x01 report is 10 bytes long: > + * u8 report_id == 0x01 > + * u8 first_bytes_of_full_report[9] > + * So let's reuse the full report parser, and stop it after > + * parsing the buttons. > + */ > + ds4_report = (struct dualshock4_input_report_common *)&data[1]; > + is_minimal = true; > } else { > hid_err(hdev, "Unhandled reportID=%d\n", report->id); > return -1; > @@ -2258,6 +2273,9 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * > input_report_key(ds4->gamepad, BTN_MODE, ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME); > input_sync(ds4->gamepad); > > + if (is_minimal) > + goto finish_minimal; > + I would say let's turn this into a 'return 0'. The goto is not useful since there is no need for any common cleanup or some other common logic later. > /* Parse and calibrate gyroscope data. */ > for (i = 0; i < ARRAY_SIZE(ds4_report->gyro); i++) { > int raw_data = (short)le16_to_cpu(ds4_report->gyro[i]); > @@ -2365,6 +2383,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * > ps_dev->battery_status = battery_status; > spin_unlock_irqrestore(&ps_dev->lock, flags); > > +finish_minimal: > return 0; > } > > -- > 2.39.2 > > Thanks, Roderick
On 1/25/24 09:54, Roderick Colenbrander wrote: > I would say let's turn this into a 'return 0'. The goto is not useful > since there is no need for any common cleanup or some other common > logic later. Oops, that one slipped through the cracks. Thank you, will do! Max
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 2bf44bd3cc8a..086b0768fa51 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -287,6 +287,8 @@ struct dualsense_output_report { #define DS4_INPUT_REPORT_USB 0x01 #define DS4_INPUT_REPORT_USB_SIZE 64 +#define DS4_INPUT_REPORT_BT_MINIMAL 0x01 +#define DS4_INPUT_REPORT_BT_MINIMAL_SIZE 10 #define DS4_INPUT_REPORT_BT 0x11 #define DS4_INPUT_REPORT_BT_SIZE 78 #define DS4_OUTPUT_REPORT_USB 0x05 @@ -2198,6 +2200,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * int battery_status, i, j; uint16_t sensor_timestamp; unsigned long flags; + bool is_minimal = false; /* * DualShock4 in USB uses the full HID report for reportID 1, but @@ -2225,6 +2228,18 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * ds4_report = &bt->common; num_touch_reports = bt->num_touch_reports; touch_reports = bt->touch_reports; + } else if (hdev->bus == BUS_BLUETOOTH && + report->id == DS4_INPUT_REPORT_BT_MINIMAL && + size == DS4_INPUT_REPORT_BT_MINIMAL_SIZE) { + /* Some third-party pads never switch to the full 0x11 report. + * The short 0x01 report is 10 bytes long: + * u8 report_id == 0x01 + * u8 first_bytes_of_full_report[9] + * So let's reuse the full report parser, and stop it after + * parsing the buttons. + */ + ds4_report = (struct dualshock4_input_report_common *)&data[1]; + is_minimal = true; } else { hid_err(hdev, "Unhandled reportID=%d\n", report->id); return -1; @@ -2258,6 +2273,9 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * input_report_key(ds4->gamepad, BTN_MODE, ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME); input_sync(ds4->gamepad); + if (is_minimal) + goto finish_minimal; + /* Parse and calibrate gyroscope data. */ for (i = 0; i < ARRAY_SIZE(ds4_report->gyro); i++) { int raw_data = (short)le16_to_cpu(ds4_report->gyro[i]); @@ -2365,6 +2383,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * ps_dev->battery_status = battery_status; spin_unlock_irqrestore(&ps_dev->lock, flags); +finish_minimal: return 0; }
Some third-party controllers never switch to the full 0x11 report. They keep sending the short 0x01 report, so let's parse that instead. Signed-off-by: Max Staudt <max@enpas.org> --- drivers/hid/hid-playstation.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)