netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Stefan Mätje" <Stefan.Maetje@esd.eu>
To: "linux-can@vger.kernel.org" <linux-can@vger.kernel.org>,
	"mkl@pengutronix.de" <mkl@pengutronix.de>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "wg@grandegger.com" <wg@grandegger.com>,
	"edumazet@google.com" <edumazet@google.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"pabeni@redhat.com" <pabeni@redhat.com>
Subject: Re: [PATCH v11 0/2] can: esd: add support for esd GmbH PCIe/402 CAN interface
Date: Wed, 20 Dec 2023 15:25:57 +0000	[thread overview]
Message-ID: <77d81c492c78a8ce1b51e71af5607917f6aebd5d.camel@esd.eu> (raw)
In-Reply-To: <20231122160211.2110448-1-stefan.maetje@esd.eu>

Hi,

are there any comments on this patch besides the ones from Vincent?
Is there anything I can do to pave the path for this patch?

Best regards,
    Stefan

Am Mittwoch, dem 22.11.2023 um 17:02 +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/
> 
> 
> The patch is based on the linux-can-next main branch.
> 
> Changed in v11:
>   No functional, only editorial changes due to feedback on v10.
>   - Make lifetime of macros used for hardware timestamp calculation
>     very short by #undef-ing them after use.
>   - Fixed insertion order of new entry in MAINTAINERS file.
> 
> Changed in v10:
>   Most changes due to feedback by Vincent Mailhol
>   https://lore.kernel.org/linux-can/CAMZ6RqLOAC930GNOU+pWuoi6FgYwFOuFrSyAzVjvE2fuVgy8oA@mail.gmail.com/
>   - Add support for ethtool operations by using default operations 
>     provided by the can_dev module for drivers with hardware time
>     stamp support.
>   - Factor out core unregistration into pci402_unregister_core().
>   - Factor out getting next TX fifo index into acc_tx_fifo_next().
>   - Stop counting alloc_can_err_skb() failures in rx_dropped statistic.
>   - Add CAN_ERR_CNT flag in CAN error frames as needed.
>   - Rework function acc_reset_fpga(). To clear I^2C bus enable bit
>     is not necessary after FPGA reset.
>   - Simplify struct acc_bmmsg_rxtxdone layout.
>   - Additional non functional changes due to feedback by Vincent
>   - Some spelling corrections: ESDACC -> esdACC
> 
> Changes in v9:
>   - Fix returning success error code in case of allocation failure in
>     pci402_probe().
> 
> Changes in v8:
>   - Rebased to 6.6-rc2 on linux-can-next branch main
> 
> Changes in v7:
>   - Numerous changes. Find the quoted with inline comments about changes
>     below after the changes list. Stuff that I don't understand and
>     where I have questions is marked with ????.
>     Unfortunately I will be AFK till 28th of November.
> 
> 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.
> 
> previous versions:
> v1 - https://lore.kernel.org/linux-can/20210728203647.15240-1-Stefan.Maetje@esd.eu/
> v2 - https://lore.kernel.org/linux-can/20210730173805.3926-1-Stefan.Maetje@esd.eu/
> v3 - https://lore.kernel.org/linux-can/20210908164640.23243-1-stefan.maetje@esd.eu/
> v4 - https://lore.kernel.org/linux-can/20210916172152.5127-1-stefan.maetje@esd.eu/
> v5 - https://lore.kernel.org/linux-can/20211109155326.2608822-1-stefan.maetje@esd.eu/
> v6 - https://lore.kernel.org/linux-can/20211201220328.3079270-1-stefan.maetje@esd.eu/
> v7 - https://lore.kernel.org/linux-can/20221106224156.3619334-1-stefan.maetje@esd.eu/
> v8 - https://lore.kernel.org/linux-can/20231025141635.1459606-1-stefan.maetje@esd.eu/
> v9 - https://lore.kernel.org/linux-can/20231107184103.2802678-1-stefan.maetje@esd.eu/
> v10 - https://lore.kernel.org/linux-can/20231120175657.4070921-1-stefan.maetje@esd.eu/
> 
> 
> Stefan Mätje (2):
>   MAINTAINERS: add Stefan Mätje as maintainer for the esd electronics
>     GmbH PCIe/402 CAN drivers
>   can: esd: add support for esd GmbH PCIe/402 CAN interface family
> 
>  MAINTAINERS                            |   7 +
>  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 | 514 +++++++++++++++++
>  drivers/net/can/esd/esdacc.c           | 764 +++++++++++++++++++++++++
>  drivers/net/can/esd/esdacc.h           | 356 ++++++++++++
>  8 files changed, 1662 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: 93e7eca853ca0087b129433630ddd89288d2b8b4

  parent reply	other threads:[~2023-12-20 15:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-22 16:02 [PATCH v11 0/2] can: esd: add support for esd GmbH PCIe/402 CAN interface Stefan Mätje
2023-11-22 16:02 ` [PATCH v11 1/2] MAINTAINERS: add Stefan Mätje as maintainer for the esd electronics GmbH PCIe/402 CAN drivers Stefan Mätje
2023-11-22 16:02 ` [PATCH v11 2/2] can: esd: add support for esd GmbH PCIe/402 CAN interface family Stefan Mätje
2023-12-20 15:25 ` Stefan Mätje [this message]
2024-01-29 12:46   ` [PATCH v11 0/2] can: esd: add support for esd GmbH PCIe/402 CAN interface Stefan Mätje

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=77d81c492c78a8ce1b51e71af5607917f6aebd5d.camel@esd.eu \
    --to=stefan.maetje@esd.eu \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).