Messages in this thread | | | Date | Thu, 27 Jul 2023 21:55:53 +0200 | Subject | Re: [PATCH v8 1/2] Added Digiteq Automotive MGB4 driver | From | Martin Tůma <> |
| |
On 26. 07. 23 11:30, Hans Verkuil wrote: > On 26/07/2023 11:14, Hans Verkuil wrote: >> Hi Martin, >> >> On 04/07/2023 15:13, tumic@gpxsee.org wrote: >>> From: Martin Tůma <martin.tuma@digiteqautomotive.com> >>> >>> Digiteq Automotive MGB4 is a modular frame grabber PCIe card for automotive >>> video interfaces. As for now, two modules - FPD-Link and GMSL - are >>> available and supported by the driver. The card has two inputs and two >>> outputs (FPD-Link only). >>> >>> In addition to the video interfaces it also provides a trigger signal >>> interface and a MTD interface for FPGA firmware upload. >>> >>> Signed-off-by: Martin Tůma <martin.tuma@digiteqautomotive.com> >>> --- >>> MAINTAINERS | 7 + >>> drivers/media/pci/Kconfig | 1 + >>> drivers/media/pci/Makefile | 1 + >>> drivers/media/pci/mgb4/Kconfig | 17 + >>> drivers/media/pci/mgb4/Makefile | 6 + >>> drivers/media/pci/mgb4/mgb4_cmt.c | 244 +++++++ >>> drivers/media/pci/mgb4/mgb4_cmt.h | 17 + >>> drivers/media/pci/mgb4/mgb4_core.c | 711 ++++++++++++++++++ >>> drivers/media/pci/mgb4/mgb4_core.h | 72 ++ >>> drivers/media/pci/mgb4/mgb4_dma.c | 123 ++++ >>> drivers/media/pci/mgb4/mgb4_dma.h | 18 + >>> drivers/media/pci/mgb4/mgb4_i2c.c | 140 ++++ >>> drivers/media/pci/mgb4/mgb4_i2c.h | 35 + >>> drivers/media/pci/mgb4/mgb4_io.h | 33 + >>> drivers/media/pci/mgb4/mgb4_regs.c | 30 + >>> drivers/media/pci/mgb4/mgb4_regs.h | 35 + >>> drivers/media/pci/mgb4/mgb4_sysfs.h | 18 + >>> drivers/media/pci/mgb4/mgb4_sysfs_in.c | 757 +++++++++++++++++++ >>> drivers/media/pci/mgb4/mgb4_sysfs_out.c | 700 ++++++++++++++++++ >>> drivers/media/pci/mgb4/mgb4_sysfs_pci.c | 71 ++ >>> drivers/media/pci/mgb4/mgb4_trigger.c | 208 ++++++ >>> drivers/media/pci/mgb4/mgb4_trigger.h | 8 + >>> drivers/media/pci/mgb4/mgb4_vin.c | 930 ++++++++++++++++++++++++ >>> drivers/media/pci/mgb4/mgb4_vin.h | 69 ++ >>> drivers/media/pci/mgb4/mgb4_vout.c | 594 +++++++++++++++ >>> drivers/media/pci/mgb4/mgb4_vout.h | 65 ++ >>> 26 files changed, 4910 insertions(+) >>> create mode 100644 drivers/media/pci/mgb4/Kconfig >>> create mode 100644 drivers/media/pci/mgb4/Makefile >>> create mode 100644 drivers/media/pci/mgb4/mgb4_cmt.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_cmt.h >>> create mode 100644 drivers/media/pci/mgb4/mgb4_core.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_core.h >>> create mode 100644 drivers/media/pci/mgb4/mgb4_dma.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_dma.h >>> create mode 100644 drivers/media/pci/mgb4/mgb4_i2c.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_i2c.h >>> create mode 100644 drivers/media/pci/mgb4/mgb4_io.h >>> create mode 100644 drivers/media/pci/mgb4/mgb4_regs.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_regs.h >>> create mode 100644 drivers/media/pci/mgb4/mgb4_sysfs.h >>> create mode 100644 drivers/media/pci/mgb4/mgb4_sysfs_in.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_sysfs_out.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_sysfs_pci.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_trigger.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_trigger.h >>> create mode 100644 drivers/media/pci/mgb4/mgb4_vin.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_vin.h >>> create mode 100644 drivers/media/pci/mgb4/mgb4_vout.c >>> create mode 100644 drivers/media/pci/mgb4/mgb4_vout.h >>> >> >> When I build with: make W=1 KCFLAGS=-Wmaybe-uninitialized >> >> I get this warning: >> >> drivers/media/pci/mgb4/mgb4_vout.c: In function 'mgb4_vout_create': >> drivers/media/pci/mgb4/mgb4_vout.c:473:27: warning: variable 'video' set but not used [-Wunused-but-set-variable] >> 473 | struct mgb4_regs *video; >> | ^~~~~ > > checkpatch.pl --strict also gives a lot of warnings. > > You can ignore the "line length of 131 exceeds 100 columns" warnings for the hex dump. > Those are OK in that particular case. > > But the suggested renamings would be good to implement to be consistent with other > drivers. > > Regarding these: > > WARNING: Missing a blank line after declarations > #3340: FILE: drivers/media/pci/mgb4/mgb4_trigger.c:93: > + u32 data; > + s64 ts __aligned(8); > > WARNING: externs should be avoided in .c files > #3340: FILE: drivers/media/pci/mgb4/mgb4_trigger.c:93: > + s64 ts __aligned(8); > > This seems to be standard iio coding style, even though checkpatch > gets confused by this. So let's leave this as-is. > > Regards, > > Hans
Hi Hans, I'm out of office till next week so I'm not able to re-check it now, but there were generally three kinds of warnings from checkpatch:
1.) line lengths in the tables 2.) the iio code style 3.) the rename suggestions
I'm glad you have no problem with 1 and 2 as the current implementation makes IMHO more sense than following some strict automatic rules. The renaming makes not much sense either for me, but I can rename the functions if required. I will have a look at it when I'm back at work.
M.
| |