netdev - Re: [PATCH 1/1] can: esd: add support for esd GmbH PCIe/402 CAN interface family

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20210729071650.77e274e4zobv5uwo@pengutronix.de>
Date:   Thu, 29 Jul 2021 09:16:50 +0200
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     Stefan Mätje <Stefan.Maetje@....eu>
Cc:     linux-can@...r.kernel.org, Wolfgang Grandegger <wg@...ndegger.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] can: esd: add support for esd GmbH PCIe/402 CAN
 interface family

On 28.07.2021 22:36:47, Stefan Mätje wrote:
> This patch adds support for the PCI based PCIe/402 CAN interface family
> from esd GmbH that is available with various form factors
> (https://esd.eu/en/products/402-series-can-interfaces).
> 
> All boards utilize a FPGA based CAN controller solution developed
> by esd (esdACC). For more information on the esdACC see
> https://esd.eu/en/products/esdacc.

Thanks for the patch!

> This driver detects all available CAN interface boards but atm.
> operates the CAN-FD capable devices in Classic-CAN mode only!

Are you planing to change this?

> Signed-off-by: Stefan Mätje <Stefan.Maetje@....eu>

For now just some nitpicks:

Compilation throws this error message on 32 bit ARM:

| drivers/net/can/esd/esd402_pci.c: In function ‘pci402_init_dma’:                                                                                                                                                              
| drivers/net/can/esd/esd402_pci.c:304:32: warning: right shift count >= width of type [-Wshift-count-overflow]                                                                                                                 
|   304 |  iowrite32((u32)(card->dma_hnd >> 32),                                                                                                                                                                                                                  
|       |                                ^~                                                                                                                                                                                                                       
|   CHECK   /srv/work/frogger/socketcan/linux/drivers/net/can/esd/esd402_pci.c                                                                                                                                                                                    
| drivers/net/can/esd/esd402_pci.c:304:42: warning: shift too big (32) for type unsigned int                            

[...]

> diff --git a/drivers/net/can/esd/Makefile b/drivers/net/can/esd/Makefile
> new file mode 100644
> index 000000000000..a960e8b97c6f
> --- /dev/null
> +++ b/drivers/net/can/esd/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +#  Makefile for esd gmbh ESDACC controller driver
> +#
> +esd_402_pci-y := esdacc.o esd402_pci.o
> +
> +ifeq ($(CONFIG_CAN_ESD_402_PCI),)
> +obj-m += esd_402_pci.o
> +else
> +obj-$(CONFIG_CAN_ESD_402_PCI) += esd_402_pci.o
> +endif

Why do you build the driver, if it has not been enabled?

The straight forward way to build the driver would be:

| obj-$(CONFIG_CAN_ESD_402_PCI) += esd_402_pci.o
|
| esd_402_pci-objs := esdacc.o esd402_pci.o

You can rename the esd_402_pci.c to esd_402_pci-core.c to avoid
inconsistent naming, (C file is called esd402_pci.c, while the driver
module is esd_402_pci.ko)

Marc
-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)