linux-kernel - Re: [PATCH v8 1/2] Added Digiteq Automotive MGB4 driver

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aeaf46cd-205e-8111-c366-d4aa7223be4c@xs4all.nl>
Date:   Wed, 26 Jul 2023 11:30:32 +0200
From:   Hans Verkuil <hverkuil-cisco@...all.nl>
To:     tumic@...see.org, 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/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