Hi Michael, Apologies for completely missing your v1 post for this driver. It was posted just before the ELCE and it looks like it just fell through the cracks. Anyway, I did a quick scan and found a few high-level things that need to be addressed before I will start reviewing: 1) Please add the output of 'v4l2-compliance -s' to this cover letter (test with a valid source connected). Obviously any failures should be addressed, and if possible also all warnings. 2) Add entries for the new drivers to the MAINTAINERS file. 3) Since you added SPDX lines you can drop the actual license texts. It's one or the other, not both. 4) I see references to a firmware, but it appears to be a firmware that's loaded from an on-board eeprom or something, not an external firmware file. Correct? If I'm wrong and it is an external fw file, then where does it come from? 5) s_dv_timings is missing. That can't be right. Drivers shall *never* change timings automatically when they detect new timings. Instead they should send the SOURCE_CHANGE event to userspace, userspace will stop streaming, call QUERY_DV_TIMINGS and, if a valid signal was detected, call S_DV_TIMINGS with the new timings and restart streaming. Regards, Hans On 11/26/2018 07:09 PM, Michael Grzeschik wrote: > This series adds support for the Startech usb2hdcapm framegrabber. The > code is based on the external kernel module code from Steven Toth's > github page: > > https://github.com/stoth68000/hdcapm/ > > We applied checkpatch.pl --strict and cleaned up the 80 character > length, whitespace issues and replaced simple printks with appropriate > v4l2_* or dev_* helpers, used WARN_ON instead of BUG and changed all > errors and warnings checkpatch was complaining about. > > Steven Toth (2): > media: mst3367: add support for mstar mst3367 HDMI RX > media: hdcapm: add support for usb2hdcapm hdmi2usb framegrabber from > startech > > drivers/media/i2c/Kconfig | 10 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/mst3367.c | 1104 ++++++++++++++++++ > drivers/media/usb/Kconfig | 1 + > drivers/media/usb/Makefile | 1 + > drivers/media/usb/hdcapm/Kconfig | 11 + > drivers/media/usb/hdcapm/Makefile | 3 + > drivers/media/usb/hdcapm/hdcapm-buffer.c | 230 ++++ > drivers/media/usb/hdcapm/hdcapm-compressor.c | 782 +++++++++++++ > drivers/media/usb/hdcapm/hdcapm-core.c | 743 ++++++++++++ > drivers/media/usb/hdcapm/hdcapm-i2c.c | 332 ++++++ > drivers/media/usb/hdcapm/hdcapm-reg.h | 111 ++ > drivers/media/usb/hdcapm/hdcapm-video.c | 665 +++++++++++ > drivers/media/usb/hdcapm/hdcapm.h | 283 +++++ > include/media/i2c/mst3367.h | 29 + > 15 files changed, 4306 insertions(+) > create mode 100644 drivers/media/i2c/mst3367.c > create mode 100644 drivers/media/usb/hdcapm/Kconfig > create mode 100644 drivers/media/usb/hdcapm/Makefile > create mode 100644 drivers/media/usb/hdcapm/hdcapm-buffer.c > create mode 100644 drivers/media/usb/hdcapm/hdcapm-compressor.c > create mode 100644 drivers/media/usb/hdcapm/hdcapm-core.c > create mode 100644 drivers/media/usb/hdcapm/hdcapm-i2c.c > create mode 100644 drivers/media/usb/hdcapm/hdcapm-reg.h > create mode 100644 drivers/media/usb/hdcapm/hdcapm-video.c > create mode 100644 drivers/media/usb/hdcapm/hdcapm.h > create mode 100644 include/media/i2c/mst3367.h >
