Hi Dmitry, Thanks a lot for the submission. It is in a much better form than the previous version. I do have a few general comments and then will comment on each commit. On Thu, Dec 30, 2021 at 12:11 AM Dmitry Antipov <daantipov@xxxxxxxxx> wrote: > > Surface Duo devices use a touch digitizer that communicates to the main > SoC via SPI and presents itself as a HID device. This patch's goal is to > add the spi-hid transport driver to drivers/hid. The driver follows the > publically available HID Over SPI Protocol Specification version 1.0. I managed to find the spec on the website, but ideally could you add a link here and in your 5/5 patch so we keep a trace of it? > > In the initial commits there are some HID core changes to support a SPI > device, followed by extensions to hid_driver and hid_ll_driver structs > to allow for some error-handling logic delegation from the transport > layer to the device driver, and finally the SPI HID transport driver. The 2 other comments I have are: - your patches need to have the same Signed-off-by: line than the From: line. If you can't use your email from your SoB line, just add in the commit description the from line. For example, the formatted patch would be: --- From: Submitter <submitter@xxxxxxxxx> Subject: This is patch one From: Author Name <author@xxxxxxxxxxx> This is the description Signed-off-by: Author Name <author@xxxxxxxxxxx> --- This should allow checkpatch to not complain about it too (tip: use (and fix) `./scripts/checkpatch.pl -g ...HEAD~5` on your tree before submitting) - Please use a version numbering when submitting patches (well, at least use v2 here). We already saw a first submission, and adding a v2 and the changes is nicer for the reviewers to know what we can assume that have been fixed. More onto each patch. Cheers, Benjamin > > Dmitry Antipov (5): > HID: Add BUS_SPI support when printing out device info in > hid_connect() > HID: define HID_SPI_DEVICE macro in hid.h > HID: add on_transport_error() field to struct hid_driver > HID: add reset() field to struct hid_ll_driver > HID: add spi-hid, transport driver for HID over SPI bus > > arch/arm64/configs/defconfig | 1 + > drivers/hid/Kconfig | 2 + > drivers/hid/Makefile | 1 + > drivers/hid/hid-core.c | 3 + > drivers/hid/spi-hid/Kconfig | 25 + > drivers/hid/spi-hid/Makefile | 12 + > drivers/hid/spi-hid/spi-hid-core.c | 1487 +++++++++++++++++++++++++++ > drivers/hid/spi-hid/spi-hid-core.h | 201 ++++ > drivers/hid/spi-hid/spi-hid_trace.h | 197 ++++ > drivers/hid/spi-hid/trace.c | 11 + > include/linux/hid.h | 24 + > 11 files changed, 1964 insertions(+) > create mode 100644 drivers/hid/spi-hid/Kconfig > create mode 100644 drivers/hid/spi-hid/Makefile > create mode 100644 drivers/hid/spi-hid/spi-hid-core.c > create mode 100644 drivers/hid/spi-hid/spi-hid-core.h > create mode 100644 drivers/hid/spi-hid/spi-hid_trace.h > create mode 100644 drivers/hid/spi-hid/trace.c > > -- > 2.25.1 >
