| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 27 Jul 2023 21:55:53 +0200
From: Martin Tůma <tumic@...see.org>
To: Hans Verkuil <hverkuil-cisco@...all.nl>,
Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
Martin Tůma <martin.tuma@...iteqautomotive.com>
Subject: Re: [PATCH v8 1/2] Added Digiteq Automotive MGB4 driver
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@...see.org wrote:
>>> From: Martin Tůma <martin.tuma@...iteqautomotive.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@...iteqautomotive.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.
Powered by blists - more mailing lists