Hi 2021. február 5., péntek 18:01 keltezéssel, Benjamin Tissoires írta: > Hi, > > On Thu, Jan 28, 2021 at 6:27 PM Roderick Colenbrander > roderick@xxxxxxxxxx wrote: > > > From: Roderick Colenbrander <roderick.colenbrander@xxxxxxxx> > > Hi, > > This is hopefully the final revision of this patch series. Patch v4 had > > a rebase issue of a part of the sensors patch for which a part had moved > > to the end of the series. This has been fixed. I have double, no triple > > checked the patches. Made sure they build using a 'rebase -x' script > > and also ran the hid-tools tests on the final driver. > > Thanks to everyone who provided feedback through the mailing list or privately. > > As suggested by Benjamin on the 'v4' version of this email, if you were > > involed in the review or testing of this series and would like some credit, > > please provide a reviewed-by or tested-by tag. > > Changes since v4: > > > > - Fixed bad rebase of ps_sensors_create, moved it to appropriate patch. > > Barnabás, any comments on this version? > > As soon as I get your rev-by, we can apply the series, just in time for 5.12. > Sorry for not responding earlier, I have been relatively busy lately. I have taken another look at the final source file. I have a couple comments for Roderick: - `player_ids` array should be `static const` as far as I can see; - there are a couple devm_kasprintf() calls which are not checked; - power_supply_powers() call is not checked - I think either a comment should mention that it's not considered a fatal error, or checked There are also other more minor things, formatting inconsistencies, but I cannot see anything else, so with the aforementioned things fixed, if you want: Reviewed-by: Barnabás Pőcze <pobrn@xxxxxxxxxxxxxx> Regards, Barnabás Pőcze > Roderick, I do see a few checkpath errors that could be fixed, but I > won't hole the series against: > HID: playstation: add DualSense battery support. -> WARNING: Missing a > blank line after declarations > HID: playstation: report DualSense hardware and firmware version. -> > WARNING: Consider renaming function(s) 'ps_show_firmware_version' to > 'firmware_version_show' (and same for ps_show_hardware_version) > > Also, there is a weird sparse error: > +drivers/hid/hid-playstation.c:xxx:1:.error: static assertion failed: > "sizeof(struct dualsense_input_report) == DS_INPUT_REPORT_USB_SIZE - > 1" > +drivers/hid/hid-playstation.c:xxx:1:.error: static assertion failed: > "sizeof(struct dualsense_output_report_bt) == > DS_OUTPUT_REPORT_BT_SIZE" > > It's weird because it only fails while running sparse, when the normal > compilation is just fine, and the assert is correctly evaluated. > > Anyway, the series is good from my Point of View, but I'd like to get > the reviewers some credits. > > Cheers, > Benjamin > > > Thanks, > > Roderick Colenbrander > > Sony Interactive Entertainment, LLC > > Roderick Colenbrander (13): > > HID: playstation: initial DualSense USB support. > > HID: playstation: use DualSense MAC address as unique identifier. > > HID: playstation: add DualSense battery support. > > HID: playstation: add DualSense touchpad support. > > HID: playstation: add DualSense accelerometer and gyroscope support. > > HID: playstation: track devices in list. > > HID: playstation: add DualSense Bluetooth support. > > HID: playstation: add DualSense classic rumble support. > > HID: playstation: add DualSense lightbar support > > HID: playstation: add microphone mute support for DualSense. > > HID: playstation: add DualSense player LEDs support. > > HID: playstation: DualSense set LEDs to default player id. > > HID: playstation: report DualSense hardware and firmware version. > > MAINTAINERS | 6 + > > drivers/hid/Kconfig | 21 + > > drivers/hid/Makefile | 1 + > > drivers/hid/hid-ids.h | 1 + > > drivers/hid/hid-playstation.c | 1492 +++++++++++++++++++++++++++++++++ > > 5 files changed, 1521 insertions(+) > > create mode 100644 drivers/hid/hid-playstation.c > > -- > > 2.26.2
