| Message ID | 20211201220328.3079270-1-stefan.maetje@esd.eu (mailing list archive) |
|---|---|
| Headers | show |
| Series | can: esd: add support for esd GmbH PCIe/402 CAN interface | expand |
Hi,
this is really patch v6. Missed to change the email subject. My bad.
Best regards,
Stefan Mätje
Am Mittwoch, den 01.12.2021, 23:03 +0100 schrieb Stefan Mätje:
> The purpose of this patch is to introduce a new CAN driver to support
> the esd GmbH 402 family of CAN interface boards. The hardware design
> is based on a CAN controller implemented in a FPGA attached to a
> PCIe link.
>
> More information on these boards can be found following the links
> included in the commit message.
>
> This patch supports all boards but will operate the CAN-FD capable
> boards only in Classic-CAN mode. The CAN-FD support will be added
> when the initial patch has stabilized.
>
> The patch is reuses the previous work of my former colleague:
> Link:
> https://lore.kernel.org/linux-can/1426592308-23817-1-git-send-email-thomas.koerper@esd.eu/
>
> *Note*: scripts/checkpatch.pl still emits the following warnings:
> - esd_402_pci-core.c:270: Possible unnecessary 'out of memory' message
> This error message is there to tell the user that the DMA allocation
> failed and not an allocation for normal kernel memory.
> - esdacc.h:255: The irq_cnt pointer is still declared volatile and
> this has a reason and is explained in detail in the header
> referencing the exception noted in volatile-considered-harmful.rst.
>
> The patch is based on the linux-can-next testing branch.
>
> Changes in v6:
> - Fixed the statistic handling of RX overrun errors and increase
> net_device_stats::rx_errors instead of net_device_stats::rx_dropped.
> - Added a patch to not increase rx statistics when generating a CAN
> rx error message frame as suggested on the linux-can list.
> - Added a patch to not not increase rx_bytes statistics for RTR frames
> as suggested on the linux-can list.
>
> The last two patches change the statistics handling from the previous
> style used in other drivers to the newly suggested one.
>
Hi,
is there any chance to get this patch
https://lore.kernel.org/linux-can/20211201220328.3079270-1-stefan.maetje@esd.eu/
in the upcoming
5.17 release?
The patch still applies cleanly to linux-can-next "testing" at this tag:
https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tag/?h=linux-can-next-for-5.17-20220105
If it helps I would provide a patch v7 that is rebased to the mentioned
tag.
Any comment is appreciated.
Best regards,
Stefan Mätje
Am Mittwoch, den 01.12.2021, 22:09 +0000 schrieb Stefan Mätje:
> Hi,
>
> this is really patch v6. Missed to change the email subject. My bad.
>
> Best regards,
> Stefan Mätje
>
> Am Mittwoch, den 01.12.2021, 23:03 +0100 schrieb Stefan Mätje:
> > The purpose of this patch is to introduce a new CAN driver to support
> > the esd GmbH 402 family of CAN interface boards. The hardware design
> > is based on a CAN controller implemented in a FPGA attached to a
> > PCIe link.
> >
> > More information on these boards can be found following the links
> > included in the commit message.
> >
> > This patch supports all boards but will operate the CAN-FD capable
> > boards only in Classic-CAN mode. The CAN-FD support will be added
> > when the initial patch has stabilized.
> >
> > The patch is reuses the previous work of my former colleague:
> > Link:
> > https://lore.kernel.org/linux-can/1426592308-23817-1-git-send-email-thomas.koerper@esd.eu/
> >
> > *Note*: scripts/checkpatch.pl still emits the following warnings:
> > - esd_402_pci-core.c:270: Possible unnecessary 'out of memory' message
> > This error message is there to tell the user that the DMA allocation
> > failed and not an allocation for normal kernel memory.
> > - esdacc.h:255: The irq_cnt pointer is still declared volatile and
> > this has a reason and is explained in detail in the header
> > referencing the exception noted in volatile-considered-harmful.rst.
> >
> > The patch is based on the linux-can-next testing branch.
> >
> > Changes in v6:
> > - Fixed the statistic handling of RX overrun errors and increase
> > net_device_stats::rx_errors instead of net_device_stats::rx_dropped.
> > - Added a patch to not increase rx statistics when generating a CAN
> > rx error message frame as suggested on the linux-can list.
> > - Added a patch to not not increase rx_bytes statistics for RTR frames
> > as suggested on the linux-can list.
> >
> > The last two patches change the statistics handling from the previous
> > style used in other drivers to the newly suggested one.
> >
On 01.12.2021 23:03:24, Stefan Mätje wrote: > *Note*: scripts/checkpatch.pl still emits the following warnings: > - esd_402_pci-core.c:270: Possible unnecessary 'out of memory' message > This error message is there to tell the user that the DMA allocation > failed and not an allocation for normal kernel memory. The kernel takes care of printing a error message in case the DMA mem allocation fails. This is why checkpatch asks you to remove that message. > - esdacc.h:255: The irq_cnt pointer is still declared volatile and > this has a reason and is explained in detail in the header > referencing the exception noted in volatile-considered-harmful.rst. I'll look into this, I'll probably ask you to explain the IRQ demux to me :) regards, Marc
Am Dienstag, den 25.01.2022, 17:25 +0100 schrieb Marc Kleine-Budde: > On 01.12.2021 23:03:24, Stefan Mätje wrote: > > *Note*: scripts/checkpatch.pl still emits the following warnings: > > - esd_402_pci-core.c:270: Possible unnecessary 'out of memory' message > > This error message is there to tell the user that the DMA allocation > > failed and not an allocation for normal kernel memory. > > The kernel takes care of printing a error message in case the DMA mem > allocation fails. This is why checkpatch asks you to remove that message. Hello Marc, I've triggered a kernel failure message for the DMA allocation by exceeding the DMA allocation limit. If you say it is clear from that message that the DMA allocation fails (see included kernel messages below) I'll throw out the extra "DMA alloc failed" message from the esd_402_pci driver. The driver's message is shown for both boards in my computer while the kernel message is printed only once. See the kernel messages below. Best regards, Stefan System Design Phone: +49-511-37298-146 E-Mail: stefan.maetje@esd.eu
On 27.01.2022 19:14:04, Stefan Mätje wrote: > > The kernel takes care of printing a error message in case the DMA mem > > allocation fails. This is why checkpatch asks you to remove that message. > > I've triggered a kernel failure message for the DMA allocation by exceeding > the DMA allocation limit. If you say it is clear from that message that the > DMA allocation fails (see included kernel messages below) I'll throw out the > extra "DMA alloc failed" message from the esd_402_pci driver. The driver's > message is shown for both boards in my computer while the kernel message > is printed only once. > > See the kernel messages below. > [ 778.083489] ------------[ cut here ]------------ > [ 778.083490] WARNING: CPU: 0 PID: 24545 at mm/page_alloc.c:5344 __alloc_pages+0x292/0x330 > [ 778.083496] Modules linked in: esd_402_pci(+) tcp_diag inet_diag unix_diag nfsv3 nfs_acl rpcsec_gss_krb5 auth_rpcgss nfsv4 nfs > lockd grace fscache netfs binfmt_misc nls_iso8859_1 intel_rapl_msr mei_hdcp intel_rapl_common snd_hda_codec_hdmi > x86_pkg_temp_thermal intel_powerclamp snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio coretemp kvm_intel kvm > crct10dif_pclmul ghash_clmulni_intel aesni_intel crypto_simd snd_hda_intel snd_intel_dspcfg cryptd snd_intel_sdw_acpi rapl > snd_hda_codec intel_cstate snd_hda_core esd_usb2 input_leds snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi i915 > serio_raw at24 efi_pstore video ttm snd_seq drm_kms_helper plx_pci sja1000 snd_seq_device cec snd_timer rc_core can_dev i2c_algo_bit > snd fb_sys_fops syscopyarea sysfillrect mei_me sysimgblt soundcore mei mac_hid sch_fq_codel msr parport_pc ppdev lp drm parport > sunrpc ip_tables x_tables autofs4 gpio_ich crc32_pclmul psmouse i2c_i801 r8169 realtek ahci i2c_smbus libahci lpc_ich wmi > [ 778.083547] [last unloaded: esd_402_pci] > [ 778.083549] CPU: 0 PID: 24545 Comm: modprobe Not tainted 5.16.0-rc7-gcc-9+ #2 > [ 778.083551] Hardware name: Acer Veriton M2610G/Veriton M2610G, BIOS P01-B1 06/18/2012 > [ 778.083553] RIP: 0010:__alloc_pages+0x292/0x330 > [ 778.083556] Code: d4 89 94 7e 0f 85 3d ff ff ff e8 e8 ab d2 ff e9 33 ff ff ff e8 bf cf fb ff 48 89 c7 e9 a5 fe ff ff 41 81 e4 00 > 20 00 00 75 8e <0f> 0b eb 8a a9 00 00 08 00 75 6a 44 89 e1 80 e1 7f a9 00 00 04 00 > [ 778.083557] RSP: 0018:ffffb4e84240b918 EFLAGS: 00010246 > [ 778.083559] RAX: 0000000000000000 RBX: 0000000000000a24 RCX: 0000000000000000 > [ 778.083561] RDX: 0000000000000000 RSI: 000000000000000c RDI: 0000000000000a24 > [ 778.083562] RBP: ffffb4e84240b970 R08: 00000000ffffffff R09: 00000000ffffffff > [ 778.083563] R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000 > [ 778.083564] R13: 000000000000000c R14: ffff9a08c0e3d0d0 R15: 0000000001000000 > [ 778.083565] FS: 00007f7d730df540(0000) GS:ffff9a09f7a00000(0000) knlGS:0000000000000000 > [ 778.083567] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 778.083568] CR2: 00007ffe83526b40 CR3: 0000000008128006 CR4: 00000000000606f0 > [ 778.083570] Call Trace: > [ 778.083571] <TASK> VVVVVVVVVVVVVVVVVVVVVVVV > [ 778.083574] __dma_direct_alloc_pages+0x8e/0x120 > [ 778.083578] dma_direct_alloc+0x66/0x2a0 > [ 778.083580] dma_alloc_attrs+0x3e/0x50 ^^^^^^^^^^^^^^^ > [ 778.083582] pci402_probe+0x211/0x64a [esd_402_pci] > [ 778.083586] ? kernfs_link_sibling+0x99/0xe0 > [ 778.083590] local_pci_probe+0x4b/0x90 > [ 778.083594] ? pci_match_device+0xde/0x130 > [ 778.083597] pci_device_probe+0xd8/0x1d0 > [ 778.083600] really_probe+0x1d2/0x3d0 > [ 778.083604] __driver_probe_device+0x109/0x180 > [ 778.083607] driver_probe_device+0x23/0xa0 > [ 778.083610] __driver_attach+0xbd/0x160 > [ 778.083613] ? __device_attach_driver+0xe0/0xe0 > [ 778.083616] bus_for_each_dev+0x7e/0xc0 > [ 778.083619] driver_attach+0x1e/0x20 > [ 778.083621] bus_add_driver+0x152/0x1f0 > [ 778.083624] driver_register+0x74/0xd0 > [ 778.083626] ? 0xffffffffc0c2e000 > [ 778.083627] __pci_register_driver+0x68/0x70 > [ 778.083630] pci402_driver_init+0x23/0x1000 [esd_402_pci] > [ 778.083633] do_one_initcall+0x48/0x210 > [ 778.083636] ? kmem_cache_alloc_trace+0x32e/0x3f0 > [ 778.083639] do_init_module+0x62/0x250 > [ 778.083641] load_module+0x261c/0x2890 > [ 778.083645] __do_sys_finit_module+0xbf/0x120 > [ 778.083646] ? __do_sys_finit_module+0xbf/0x120 > [ 778.083649] __x64_sys_finit_module+0x1a/0x20 > [ 778.083650] do_syscall_64+0x3b/0xc0 > [ 778.083654] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 778.083657] RIP: 0033:0x7f7d7322489d > [ 778.083658] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b > 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 f5 0c 00 f7 d8 64 89 01 48 > [ 778.083660] RSP: 002b:00007ffe83529b68 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 > [ 778.083662] RAX: ffffffffffffffda RBX: 000055db638f40d0 RCX: 00007f7d7322489d > [ 778.083663] RDX: 0000000000000000 RSI: 000055db638f4730 RDI: 0000000000000003 > [ 778.083664] RBP: 0000000000040000 R08: 0000000000000000 R09: 0000000000000000 > [ 778.083665] R10: 0000000000000003 R11: 0000000000000246 R12: 000055db638f4730 > [ 778.083666] R13: 0000000000000000 R14: 000055db638f43e0 R15: 000055db638f40d0 > [ 778.083669] </TASK> > [ 778.083670] ---[ end trace 8ac4b05d87d41ee5 ]--- > [ 778.083671] esd_402_pci 0000:01:00.0: DMA alloc failed! > [ 778.083710] esd_402_pci: probe of 0000:01:00.0 failed with error -12 > [ 778.095803] esd_402_pci 0000:05:00.0: ESDACC v72, freq: 80000000/80000000, feat/strap: 0x3c90/0x3d, cores: 2/4 > [ 778.095810] acc_init_ov:146: esd_402_pci 0000:05:00.0: ESDACC ts2ns: numerator 25, denominator 2 > [ 778.095812] esd_402_pci 0000:05:00.0: ESDACC with CAN-FD feature detected. This driver doesn't support CAN-FD yet. > [ 778.095822] esd_402_pci 0000:05:00.0: DMA alloc failed! > [ 778.095882] esd_402_pci: probe of 0000:05:00.0 failed with error -12 Looking at the back trace, at least to a kernel developer, it's clear that this is DMA allocation failure. Marc
On 01.12.2021 23:03:24, Stefan Mätje wrote: > The purpose of this patch is to introduce a new CAN driver to support > the esd GmbH 402 family of CAN interface boards. The hardware design > is based on a CAN controller implemented in a FPGA attached to a > PCIe link. > > More information on these boards can be found following the links > included in the commit message. > > This patch supports all boards but will operate the CAN-FD capable > boards only in Classic-CAN mode. The CAN-FD support will be added > when the initial patch has stabilized. > > The patch is reuses the previous work of my former colleague: > Link: https://lore.kernel.org/linux-can/1426592308-23817-1-git-send-email-thomas.koerper@esd.eu/ > > *Note*: scripts/checkpatch.pl still emits the following warnings: > - esd_402_pci-core.c:270: Possible unnecessary 'out of memory' message > This error message is there to tell the user that the DMA allocation > failed and not an allocation for normal kernel memory. > - esdacc.h:255: The irq_cnt pointer is still declared volatile and > this has a reason and is explained in detail in the header > referencing the exception noted in volatile-considered-harmful.rst. > > The patch is based on the linux-can-next testing branch. > > Changes in v6: > - Fixed the statistic handling of RX overrun errors and increase > net_device_stats::rx_errors instead of net_device_stats::rx_dropped. > - Added a patch to not increase rx statistics when generating a CAN > rx error message frame as suggested on the linux-can list. > - Added a patch to not not increase rx_bytes statistics for RTR frames > as suggested on the linux-can list. > > The last two patches change the statistics handling from the previous > style used in other drivers to the newly suggested one. Please squash the last 2 patches into patch 2. Marc
On 01.12.2021 23:03:24, Stefan Mätje wrote: > The purpose of this patch is to introduce a new CAN driver to support > the esd GmbH 402 family of CAN interface boards. The hardware design > is based on a CAN controller implemented in a FPGA attached to a > PCIe link. > > More information on these boards can be found following the links > included in the commit message. > > This patch supports all boards but will operate the CAN-FD capable > boards only in Classic-CAN mode. The CAN-FD support will be added > when the initial patch has stabilized. > > The patch is reuses the previous work of my former colleague: > Link: https://lore.kernel.org/linux-can/1426592308-23817-1-git-send-email-thomas.koerper@esd.eu/ > > *Note*: scripts/checkpatch.pl still emits the following warnings: > - esd_402_pci-core.c:270: Possible unnecessary 'out of memory' message > This error message is there to tell the user that the DMA allocation > failed and not an allocation for normal kernel memory. > - esdacc.h:255: The irq_cnt pointer is still declared volatile and > this has a reason and is explained in detail in the header > referencing the exception noted in volatile-considered-harmful.rst. I think you can use READ_ONCE() instead of the volatile. Marc
The purpose of this patch is to introduce a new CAN driver to support the esd GmbH 402 family of CAN interface boards. The hardware design is based on a CAN controller implemented in a FPGA attached to a PCIe link. More information on these boards can be found following the links included in the commit message. This patch supports all boards but will operate the CAN-FD capable boards only in Classic-CAN mode. The CAN-FD support will be added when the initial patch has stabilized. The patch is reuses the previous work of my former colleague: Link: https://lore.kernel.org/linux-can/1426592308-23817-1-git-send-email-thomas.koerper@esd.eu/ *Note*: scripts/checkpatch.pl still emits the following warnings: - esd_402_pci-core.c:270: Possible unnecessary 'out of memory' message This error message is there to tell the user that the DMA allocation failed and not an allocation for normal kernel memory. - esdacc.h:255: The irq_cnt pointer is still declared volatile and this has a reason and is explained in detail in the header referencing the exception noted in volatile-considered-harmful.rst. The patch is based on the linux-can-next testing branch. Changes in v6: - Fixed the statistic handling of RX overrun errors and increase net_device_stats::rx_errors instead of net_device_stats::rx_dropped. - Added a patch to not increase rx statistics when generating a CAN rx error message frame as suggested on the linux-can list. - Added a patch to not not increase rx_bytes statistics for RTR frames as suggested on the linux-can list. The last two patches change the statistics handling from the previous style used in other drivers to the newly suggested one. Changes in v5: - Added the initialization for netdev::dev_port as it is implemented for another CAN driver. See https://lore.kernel.org/linux-can/20211026180553.1953189-1-mailhol.vincent@wanadoo.fr/ Changes in v4: - Fixed the build failure on ARCH=arm64 that was found by the Intel kernel test robot. See https://lore.kernel.org/linux-can/202109120608.7ZbQXkRh-lkp@intel.com/ Removed error monitoring code that used GCC's built-in compiler functions for atomic access (__sync_* functions). GCC versions after 9 (tested with "gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04)") don't implement the intrinsic atomic as in-line code but call "__aarch64_ldadd4_acq_rel" on arm64. This GCC support function is not exported by the kernel and therefore the module build post-processing fails. Removed that code because the error monitoring never showed a problem during the development this year. Changes in v3: - Rework the bus-off restart logic in acc_set_mode() and handle_core_msg_errstatechange() to call netif_wake_queue() from the error active event. - Changed pci402_init_card() to allocate a variable sized array of struct acc_core using devm_kcalloc() instead of using a fixed size array in struct pci402_card. - Changed handle_core_msg_txabort() to release aborted TX frames in TX FIFO order. - Fixed the acc_close() function to abort all pending TX request in esdACC controller. - Fixed counting of transmit aborts in handle_core_msg_txabort(). It is now done like in can_flush_echo_skb(). - Fixed handle_core_msg_buserr() to create error frames including the CAN RX and TX error counters that were missing. - Fixed acc_set_bittiming() neither to touch LOM mode setting of esdACC controller nor to enter or leave RESET mode. The esdACC controller is going active on the CAN bus in acc_open() and is going inactive (RESET mode) again in acc_close(). - Rely on the automatic release of memory fetched by devm_kzalloc(). But still use devm_irq_free() explicitely to make sure that the interrupt handler is disconnected at that point. This avoids a possible crash in non-MSI mode due to the IRQ triggered by another device on the same PCI IRQ line. - Changed to use DMA map API instead of pci_*_consistent compatibility wrappers. - Fixed stale email references and updated copyright information. - Removed any traces of future CAN-FD support. Changes in v2: - Avoid warning triggered by -Wshift-count-overflow on architectures with 32-bit dma_addr_t. - Fixed Makefile not to build the kernel module always. Doing this renamed esd402_pci.c to esd_402_pci-core.c as recommended by Marc. Stefan Mätje (4): MAINTAINERS: add Stefan Mätje as maintainer for the esd electronics GmbH CAN drivers can: esd: add support for esd GmbH PCIe/402 CAN interface family can: esd_402_pci: do not increase rx statistics when generating a CAN rx error message frame can: esd_402_pci: do not increase rx_bytes statistics for RTR frames MAINTAINERS | 8 + drivers/net/can/Kconfig | 1 + drivers/net/can/Makefile | 1 + drivers/net/can/esd/Kconfig | 12 + drivers/net/can/esd/Makefile | 7 + drivers/net/can/esd/esd_402_pci-core.c | 502 ++++++++++++++++ drivers/net/can/esd/esdacc.c | 772 +++++++++++++++++++++++++ drivers/net/can/esd/esdacc.h | 380 ++++++++++++ 8 files changed, 1683 insertions(+) create mode 100644 drivers/net/can/esd/Kconfig create mode 100644 drivers/net/can/esd/Makefile create mode 100644 drivers/net/can/esd/esd_402_pci-core.c create mode 100644 drivers/net/can/esd/esdacc.c create mode 100644 drivers/net/can/esd/esdacc.h base-commit: f659c5c7de7982018bb961cf1f9960e60f526bdf