Hi, I do not know much about PCI, so no thorough review of that part. Here is a set of small fixes or nitpicks. The driver starts to look good. If things go right, expect to receive my reviewed-by tag in the next iteration. On Wed. 8 Nov. 2023 at 08:30, Stefan Mätje <stefan.maetje@xxxxxx> 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. > > This driver detects all available CAN interface board variants of > the family but atm. operates the CAN-FD capable devices in > Classic-CAN mode only! A later patch will introduce the CAN-FD > functionality in this driver. The driver already introduces pieces of code for CAN-FD, for example struct can_bittiming_const pci402_bittiming_const_canfd. Would it make sense to remove those for now and send them altogether in the follow-up patch which will add CAN-FD? > Co-developed-by: Thomas Körper <thomas.koerper@xxxxxx> > Signed-off-by: Thomas Körper <thomas.koerper@xxxxxx> > Signed-off-by: Stefan Mätje <stefan.maetje@xxxxxx> > --- > 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 | 512 ++++++++++++++++ > drivers/net/can/esd/esdacc.c | 771 +++++++++++++++++++++++++ > drivers/net/can/esd/esdacc.h | 393 +++++++++++++ > 7 files changed, 1697 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 > > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig > index 649453a3c858..b9a65b44ccef 100644 > --- a/drivers/net/can/Kconfig > +++ b/drivers/net/can/Kconfig > @@ -217,6 +217,7 @@ config CAN_XILINXCAN > source "drivers/net/can/c_can/Kconfig" > source "drivers/net/can/cc770/Kconfig" > source "drivers/net/can/ctucanfd/Kconfig" > +source "drivers/net/can/esd/Kconfig" > source "drivers/net/can/ifi_canfd/Kconfig" > source "drivers/net/can/m_can/Kconfig" > source "drivers/net/can/mscan/Kconfig" > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile > index ff8f76295d13..4669cd51e7bf 100644 > --- a/drivers/net/can/Makefile > +++ b/drivers/net/can/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_CAN_VXCAN) += vxcan.o > obj-$(CONFIG_CAN_SLCAN) += slcan/ > > obj-y += dev/ > +obj-y += esd/ > obj-y += rcar/ > obj-y += spi/ > obj-y += usb/ > diff --git a/drivers/net/can/esd/Kconfig b/drivers/net/can/esd/Kconfig > new file mode 100644 > index 000000000000..54bfc366634c > --- /dev/null > +++ b/drivers/net/can/esd/Kconfig > @@ -0,0 +1,12 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +config CAN_ESD_402_PCI > + tristate "esd electronics gmbh CAN-PCI(e)/402 family" > + depends on PCI && HAS_DMA > + help > + Support for C402 card family from esd electronics gmbh. > + This card family is based on the ESDACC CAN controller and > + available in several form factors: PCI, PCIe, PCIe Mini, > + M.2 PCIe, CPCIserial, PMC, XMC (see https://esd.eu/en) > + > + This driver can also be built as a module. In this case the > + module will be called esd_402_pci. > diff --git a/drivers/net/can/esd/Makefile b/drivers/net/can/esd/Makefile > new file mode 100644 > index 000000000000..5dd2d470c286 > --- /dev/null > +++ b/drivers/net/can/esd/Makefile > @@ -0,0 +1,7 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Makefile for esd gmbh ESDACC controller driver > +# > +esd_402_pci-objs := esdacc.o esd_402_pci-core.o > + > +obj-$(CONFIG_CAN_ESD_402_PCI) += esd_402_pci.o > diff --git a/drivers/net/can/esd/esd_402_pci-core.c b/drivers/net/can/esd/esd_402_pci-core.c > new file mode 100644 > index 000000000000..5793d41c17c2 > --- /dev/null > +++ b/drivers/net/can/esd/esd_402_pci-core.c > @@ -0,0 +1,512 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright (C) 2015 - 2016 Thomas Körper, esd electronic system design gmbh > + * Copyright (C) 2017 - 2022 Stefan Mätje, esd electronics gmbh > + */ > + > +#include <linux/can/dev.h> > +#include <linux/can.h> > +#include <linux/can/netlink.h> > +#include <linux/delay.h> > +#include <linux/dma-mapping.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/netdevice.h> > +#include <linux/pci.h> > + > +#include "esdacc.h" > + > +#define ESD_PCI_DEVICE_ID_PCIE402 0x0402 > + > +#define PCI402_FPGA_VER_MIN 0x003d > +#define PCI402_MAX_CORES 6 > +#define PCI402_BAR 0 > +#define PCI402_IO_OV_OFFS 0 > +#define PCI402_IO_PCIEP_OFFS 0x10000 > +#define PCI402_IO_LEN_TOTAL 0x20000 > +#define PCI402_IO_LEN_CORE 0x2000 > +#define PCI402_PCICFG_MSICAP 0x50 > + > +#define PCI402_DMA_MASK DMA_BIT_MASK(32) > +#define PCI402_DMA_SIZE ALIGN(0x10000, PAGE_SIZE) > + > +#define PCI402_PCIEP_OF_INT_ENABLE 0x0050 > +#define PCI402_PCIEP_OF_BM_ADDR_LO 0x1000 > +#define PCI402_PCIEP_OF_BM_ADDR_HI 0x1004 > +#define PCI402_PCIEP_OF_MSI_ADDR_LO 0x1008 > +#define PCI402_PCIEP_OF_MSI_ADDR_HI 0x100c > + > +/* The BTR register capabilities described by the can_bittiming_const structures > + * below are valid since ESDACC version 0x0032. > + */ > + > +/* Used if the ESDACC FPGA is built as CAN-Classic version. */ > +static const struct can_bittiming_const pci402_bittiming_const = { > + .name = "esd_402", > + .tseg1_min = 1, > + .tseg1_max = 16, > + .tseg2_min = 1, > + .tseg2_max = 8, > + .sjw_max = 4, > + .brp_min = 1, > + .brp_max = 512, > + .brp_inc = 1, > +}; > + > +/* Used if the ESDACC FPGA is built as CAN-FD version. */ > +static const struct can_bittiming_const pci402_bittiming_const_canfd = { > + .name = "esd_402fd", > + .tseg1_min = 1, > + .tseg1_max = 256, > + .tseg2_min = 1, > + .tseg2_max = 128, > + .sjw_max = 128, > + .brp_min = 1, > + .brp_max = 256, > + .brp_inc = 1, > +}; > + > +static const struct net_device_ops pci402_acc_netdev_ops = { > + .ndo_open = acc_open, > + .ndo_stop = acc_close,can_ > + .ndo_start_xmit = acc_start_xmit, > + .ndo_change_mtu = can_change_mtu ^ Generic comment: add a comma after the last entry to have a cleaner diff if members get added later on. Please also populate net_device_ops.ndo_eth_ioctl() and netdev->ethtool_ops for the hardware timestamp support. Use below commit as a reference: https://git.kernel.org/torvalds/c/1d46efa0008a > +}; > + > +struct pci402_card { > + /* Actually mapped io space, all other iomem derived from this */ > + void __iomem *addr; > + void __iomem *addr_pciep; > + > + void *dma_buf; > + dma_addr_t dma_hnd; > + > + struct acc_ov ov; > + struct acc_core *cores; > + > + bool msi_enabled; > +}; > + > +static irqreturn_t pci402_interrupt(int irq, void *dev_id) > +{ > + struct pci_dev *pdev = dev_id; > + struct pci402_card *card = pci_get_drvdata(pdev); > + irqreturn_t irq_status; > + > + irq_status = acc_card_interrupt(&card->ov, card->cores); > + > + return irq_status; > +} > + > +static int pci402_set_msiconfig(struct pci_dev *pdev) > +{ > + struct pci402_card *card = pci_get_drvdata(pdev); > + u32 addr_lo_offs = 0; > + u32 addr_lo = 0; > + u32 addr_hi = 0; > + u32 data = 0; > + u16 csr = 0; > + int err; > + > + /* The FPGA hard IP PCIe core implements a 64-bit MSI Capability > + * Register Format > + */ > + err = pci_read_config_word(pdev, PCI402_PCICFG_MSICAP + PCI_MSI_FLAGS, &csr); > + if (err) > + goto failed; > + > + err = pci_read_config_dword(pdev, PCI402_PCICFG_MSICAP + PCI_MSI_ADDRESS_LO, > + &addr_lo); > + if (err) > + goto failed; > + err = pci_read_config_dword(pdev, PCI402_PCICFG_MSICAP + PCI_MSI_ADDRESS_HI, > + &addr_hi); > + if (err) > + goto failed; > + > + err = pci_read_config_dword(pdev, PCI402_PCICFG_MSICAP + PCI_MSI_DATA_64, > + &data); > + if (err) > + goto failed; > + > + addr_lo_offs = addr_lo & 0x0000ffff; > + addr_lo &= 0xffff0000; > + > + if (addr_hi) > + addr_lo |= 1; /* To enable 64-Bit addressing in PCIe endpoint */ > + > + if (!(csr & PCI_MSI_FLAGS_ENABLE)) { > + err = -EINVAL; > + goto failed; > + } > + > + iowrite32(addr_lo, card->addr_pciep + PCI402_PCIEP_OF_MSI_ADDR_LO); > + iowrite32(addr_hi, card->addr_pciep + PCI402_PCIEP_OF_MSI_ADDR_HI); > + acc_ov_write32(&card->ov, ACC_OV_OF_MSI_ADDRESSOFFSET, addr_lo_offs); > + acc_ov_write32(&card->ov, ACC_OV_OF_MSI_DATA, data); > + > + return 0; > + > +failed: > + pci_warn(pdev, "Error while setting MSI configuration:\n" > + "CSR: 0x%.4x, addr: 0x%.8x%.8x, offs: 0x%.4x, data: 0x%.8x\n", > + csr, addr_hi, addr_lo, addr_lo_offs, data); > + > + return err; > +} > + > +static int pci402_init_card(struct pci_dev *pdev) > +{ > + struct pci402_card *card = pci_get_drvdata(pdev); > + > + card->ov.addr = card->addr + PCI402_IO_OV_OFFS; > + card->addr_pciep = card->addr + PCI402_IO_PCIEP_OFFS; > + > + acc_reset_fpga(&card->ov); > + acc_init_ov(&card->ov, &pdev->dev); > + > + if (card->ov.version < PCI402_FPGA_VER_MIN) { > + pci_err(pdev, > + "ESDACC version (0x%.4x) outdated, please update\n", > + card->ov.version); > + return -EINVAL; > + } > + > + if (card->ov.timestamp_frequency != ACC_TS_FREQ_80MHZ) { > + pci_err(pdev, > + "esdACC timestamp frequency of %uHz not supported by driver. Aborted.\n", > + card->ov.timestamp_frequency); > + return -EINVAL; > + } > + > + if (card->ov.active_cores > PCI402_MAX_CORES) { > + pci_err(pdev, > + "Card has more active cores (%u) than supported by driver. Aborted.\n", ^^^^ >From the Linux coding style: Printing numbers in parentheses (%d) adds no value and should be avoided. Ref: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#printing-kernel-messages > + card->ov.active_cores); > + return -EINVAL; > + } > + card->cores = devm_kcalloc(&pdev->dev, card->ov.active_cores, > + sizeof(struct acc_core), GFP_KERNEL); > + if (!card->cores) > + return -ENOMEM; > + > + if (card->ov.features & ACC_OV_REG_FEAT_MASK_CANFD) { > + pci_warn(pdev, > + "ESDACC with CAN-FD feature detected. This driver doesn't support CAN-FD yet.\n"); > + } > + > +#ifdef __LITTLE_ENDIAN > + /* So card converts all busmastered data to LE for us: */ > + acc_ov_set_bits(&card->ov, ACC_OV_OF_MODE, > + ACC_OV_REG_MODE_MASK_ENDIAN_LITTLE); > +#endif > + > + return 0; > +} > + > +static int pci402_init_interrupt(struct pci_dev *pdev) > +{ > + struct pci402_card *card = pci_get_drvdata(pdev); > + int err; > + > + err = pci_enable_msi(pdev); > + if (!err) { > + err = pci402_set_msiconfig(pdev); > + if (!err) { > + card->msi_enabled = true; > + acc_ov_set_bits(&card->ov, ACC_OV_OF_MODE, > + ACC_OV_REG_MODE_MASK_MSI_ENABLE); > + pci_info(pdev, "MSI enabled\n"); Consider using dev_dbg() instead of dev_info() here. > + } > + } > + > + err = devm_request_irq(&pdev->dev, pdev->irq, pci402_interrupt, > + IRQF_SHARED, dev_name(&pdev->dev), pdev); > + if (err) > + goto failure_msidis; > + > + iowrite32(1, card->addr_pciep + PCI402_PCIEP_OF_INT_ENABLE); > + > + return 0; > + > +failure_msidis: > + if (card->msi_enabled) { > + acc_ov_clear_bits(&card->ov, ACC_OV_OF_MODE, > + ACC_OV_REG_MODE_MASK_MSI_ENABLE); > + pci_disable_msi(pdev); > + card->msi_enabled = false; > + } > + > + return err; > +} > + > +static void pci402_finish_interrupt(struct pci_dev *pdev) > +{ > + struct pci402_card *card = pci_get_drvdata(pdev); > + > + iowrite32(0, card->addr_pciep + PCI402_PCIEP_OF_INT_ENABLE); > + devm_free_irq(&pdev->dev, pdev->irq, pdev); > + > + if (card->msi_enabled) { > + acc_ov_clear_bits(&card->ov, ACC_OV_OF_MODE, > + ACC_OV_REG_MODE_MASK_MSI_ENABLE); > + pci_disable_msi(pdev); > + card->msi_enabled = false; > + } > +} > + > +static int pci402_init_dma(struct pci_dev *pdev) > +{ > + struct pci402_card *card = pci_get_drvdata(pdev); > + int err; > + > + err = dma_set_coherent_mask(&pdev->dev, PCI402_DMA_MASK); > + if (err) { > + pci_err(pdev, "DMA set mask failed!\n"); > + return err; > + } > + > + /* The ESDACC DMA engine needs the DMA buffer aligned to a 64k > + * boundary. The DMA API guarantees to align the returned buffer to the > + * smallest PAGE_SIZE order which is greater than or equal to the > + * requested size. With PCI402_DMA_SIZE == 64kB this suffices here. > + */ > + card->dma_buf = dma_alloc_coherent(&pdev->dev, PCI402_DMA_SIZE, > + &card->dma_hnd, GFP_KERNEL); > + if (!card->dma_buf) > + return -ENOMEM; > + > + acc_init_bm_ptr(&card->ov, card->cores, card->dma_buf); > + > + iowrite32(card->dma_hnd, > + card->addr_pciep + PCI402_PCIEP_OF_BM_ADDR_LO); > + iowrite32(0, card->addr_pciep + PCI402_PCIEP_OF_BM_ADDR_HI); > + > + pci_set_master(pdev); > + > + acc_ov_set_bits(&card->ov, ACC_OV_OF_MODE, > + ACC_OV_REG_MODE_MASK_BM_ENABLE); > + > + return 0; > +} > + > +static void pci402_finish_dma(struct pci_dev *pdev) > +{ > + struct pci402_card *card = pci_get_drvdata(pdev); > + int i; > + > + acc_ov_clear_bits(&card->ov, ACC_OV_OF_MODE, > + ACC_OV_REG_MODE_MASK_BM_ENABLE); > + > + pci_clear_master(pdev); > + > + iowrite32(0, card->addr_pciep + PCI402_PCIEP_OF_BM_ADDR_LO); > + iowrite32(0, card->addr_pciep + PCI402_PCIEP_OF_BM_ADDR_HI); > + > + card->ov.bmfifo.messages = NULL; > + card->ov.bmfifo.irq_cnt = NULL; > + for (i = 0; i < card->ov.active_cores; i++) { > + struct acc_core *core = &card->cores[i]; > + > + core->bmfifo.messages = NULL; > + core->bmfifo.irq_cnt = NULL; > + } > + > + dma_free_coherent(&pdev->dev, PCI402_DMA_SIZE, card->dma_buf, > + card->dma_hnd); > + card->dma_buf = NULL; > +} > + > +static int pci402_init_cores(struct pci_dev *pdev) > +{ > + struct pci402_card *card = pci_get_drvdata(pdev); > + int err; > + int i; > + > + for (i = 0; i < card->ov.active_cores; i++) { > + struct acc_core *core = &card->cores[i]; > + struct acc_net_priv *priv; > + struct net_device *netdev; > + u32 fifo_config; > + > + core->addr = card->ov.addr + (i + 1) * PCI402_IO_LEN_CORE; > + > + fifo_config = acc_read32(core, ACC_CORE_OF_TXFIFO_CONFIG); > + core->tx_fifo_size = (fifo_config >> 24); > + if (core->tx_fifo_size <= 1) { > + pci_err(pdev, "Invalid tx_fifo_size!\n"); > + err = -EINVAL; > + goto failure; > + } > + > + netdev = alloc_candev(sizeof(*priv), core->tx_fifo_size); > + if (!netdev) { > + err = -ENOMEM; > + goto failure; > + } > + core->netdev = netdev; > + > + netdev->flags |= IFF_ECHO; > + netdev->dev_port = i; > + netdev->netdev_ops = &pci402_acc_netdev_ops; > + SET_NETDEV_DEV(netdev, &pdev->dev); > + > + priv = netdev_priv(netdev); > + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK | > + CAN_CTRLMODE_LISTENONLY | > + CAN_CTRLMODE_BERR_REPORTING | > + CAN_CTRLMODE_CC_LEN8_DLC; > + > + priv->can.clock.freq = card->ov.core_frequency; > + if (card->ov.features & ACC_OV_REG_FEAT_MASK_CANFD) > + priv->can.bittiming_const = &pci402_bittiming_const_canfd; > + else > + priv->can.bittiming_const = &pci402_bittiming_const; > + priv->can.do_set_bittiming = acc_set_bittiming; > + priv->can.do_set_mode = acc_set_mode; > + priv->can.do_get_berr_counter = acc_get_berr_counter; > + > + priv->core = core; > + priv->ov = &card->ov; > + > + err = register_candev(netdev); > + if (err) { > + free_candev(core->netdev); > + core->netdev = NULL; > + goto failure; > + } > + > + netdev_info(netdev, "registered\n"); Isn't this already reported by the core? Please check and if redundant, remove (same comment for the unregister below). > + } > + > + return 0; > + > +failure: > + for (i--; i >= 0; i--) { > + struct acc_core *core = &card->cores[i]; > + > + netdev_info(core->netdev, "unregistering...\n"); > + unregister_candev(core->netdev); > + > + free_candev(core->netdev); > + core->netdev = NULL; > + } Nitpick: this cleanup loop has the same content as the loop in pci402_finish_cores(). Might be worth factorizing the code in an helper function that would free a single core. > + > + return err; > +} > + > +static void pci402_finish_cores(struct pci_dev *pdev) > +{ > + struct pci402_card *card = pci_get_drvdata(pdev); > + int i; > + > + for (i = 0; i < card->ov.active_cores; i++) { > + struct acc_core *core = &card->cores[i]; > + > + netdev_info(core->netdev, "unregister\n"); > + unregister_candev(core->netdev); > + > + free_candev(core->netdev); > + core->netdev = NULL; > + } > +} > + > +static int pci402_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > +{ > + struct pci402_card *card = NULL; > + int err; > + > + err = pci_enable_device(pdev); > + if (err) > + return err; > + > + card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL); > + if (!card) { > + err = -ENOMEM; > + goto failure_disable_pci; > + } > + > + pci_set_drvdata(pdev, card); > + > + err = pci_request_regions(pdev, pci_name(pdev)); > + if (err) > + goto failure_disable_pci; > + > + card->addr = pci_iomap(pdev, PCI402_BAR, PCI402_IO_LEN_TOTAL); > + if (!card->addr) { > + err = -ENOMEM; > + goto failure_release_regions; > + } > + > + err = pci402_init_card(pdev); > + if (err) > + goto failure_unmap; > + > + err = pci402_init_dma(pdev); > + if (err) > + goto failure_unmap; > + > + err = pci402_init_interrupt(pdev); > + if (err) > + goto failure_finish_dma; > + > + err = pci402_init_cores(pdev); > + if (err) > + goto failure_finish_interrupt; > + > + return 0; > + > +failure_finish_interrupt: > + pci402_finish_interrupt(pdev); > + > +failure_finish_dma: > + pci402_finish_dma(pdev); > + > +failure_unmap: > + pci_iounmap(pdev, card->addr); > + > +failure_release_regions: > + pci_release_regions(pdev); > + > +failure_disable_pci: > + pci_disable_device(pdev); > + > + return err; > +} > + > +static void pci402_remove(struct pci_dev *pdev) > +{ > + struct pci402_card *card = pci_get_drvdata(pdev); > + > + pci402_finish_interrupt(pdev); > + pci402_finish_cores(pdev); > + pci402_finish_dma(pdev); > + pci_iounmap(pdev, card->addr); > + pci_release_regions(pdev); > + pci_disable_device(pdev); > +} > + > +static const struct pci_device_id pci402_tbl[] = { > + { > + .vendor = PCI_VENDOR_ID_ESDGMBH, > + .device = ESD_PCI_DEVICE_ID_PCIE402, > + .subvendor = PCI_VENDOR_ID_ESDGMBH, > + .subdevice = PCI_ANY_ID, > + }, > + { 0, } > +}; > +MODULE_DEVICE_TABLE(pci, pci402_tbl); > + > +static struct pci_driver pci402_driver = { > + .name = KBUILD_MODNAME, > + .id_table = pci402_tbl, > + .probe = pci402_probe, > + .remove = pci402_remove, > +}; > +module_pci_driver(pci402_driver); > + > +MODULE_DESCRIPTION("Socket-CAN driver for esd CAN 402 card family with esdACC core on PCIe"); > +MODULE_AUTHOR("Thomas Körper <socketcan@xxxxxx>"); > +MODULE_AUTHOR("Stefan Mätje <stefan.maetje@xxxxxx>"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/net/can/esd/esdacc.c b/drivers/net/can/esd/esdacc.c > new file mode 100644 > index 000000000000..49017c986c70 > --- /dev/null > +++ b/drivers/net/can/esd/esdacc.c > @@ -0,0 +1,771 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright (C) 2015 - 2016 Thomas Körper, esd electronic system design gmbh > + * Copyright (C) 2017 - 2022 Stefan Mätje, esd electronics gmbh ^^^^ 2023? (Same comment for other files) > + */ > + > +#include "esdacc.h" > + > +#include <linux/bitfield.h> > +#include <linux/delay.h> > +#include <linux/io.h> > +#include <linux/ktime.h> > + > +/* ecc value of esdACC equals SJA1000's ECC register */ > +#define ACC_ECC_SEG 0x1f > +#define ACC_ECC_DIR 0x20 > +#define ACC_ECC_BIT 0x00 > +#define ACC_ECC_FORM 0x40 > +#define ACC_ECC_STUFF 0x80 > +#define ACC_ECC_MASK 0xc0 > + > +#define ACC_BM_IRQ_UNMASK_ALL 0x55555555U > +#define ACC_BM_IRQ_MASK_ALL 0xaaaaaaaaU > +#define ACC_BM_IRQ_MASK 0x2U > +#define ACC_BM_IRQ_UNMASK 0x1U > +#define ACC_BM_LENFLAG_TX 0x20 > + > +#define ACC_REG_STATUS_IDX_STATUS_DOS 16 > +#define ACC_REG_STATUS_IDX_STATUS_ES 17 > +#define ACC_REG_STATUS_IDX_STATUS_EP 18 > +#define ACC_REG_STATUS_IDX_STATUS_BS 19 > +#define ACC_REG_STATUS_IDX_STATUS_RBS 20 > +#define ACC_REG_STATUS_IDX_STATUS_RS 21 > +#define ACC_REG_STATUS_MASK_STATUS_DOS BIT(ACC_REG_STATUS_IDX_STATUS_DOS) > +#define ACC_REG_STATUS_MASK_STATUS_ES BIT(ACC_REG_STATUS_IDX_STATUS_ES) > +#define ACC_REG_STATUS_MASK_STATUS_EP BIT(ACC_REG_STATUS_IDX_STATUS_EP) > +#define ACC_REG_STATUS_MASK_STATUS_BS BIT(ACC_REG_STATUS_IDX_STATUS_BS) > +#define ACC_REG_STATUS_MASK_STATUS_RBS BIT(ACC_REG_STATUS_IDX_STATUS_RBS) > +#define ACC_REG_STATUS_MASK_STATUS_RS BIT(ACC_REG_STATUS_IDX_STATUS_RS) Many of these macros are not used: ./include/linux/ctype.h:43:19: warning: declaration of ‘isdigit’ shadows a built-in function [-Wshadow] 43 | static inline int isdigit(int c) | ^~~~~~~ drivers/net/can/esd/esdacc.c:24: warning: macro "ACC_BM_IRQ_UNMASK" is not used [-Wunused-macros] 24 | #define ACC_BM_IRQ_UNMASK 0x1U | drivers/net/can/esd/esdacc.c:27: warning: macro "ACC_REG_STATUS_IDX_STATUS_DOS" is not used [-Wunused-macros] 27 | #define ACC_REG_STATUS_IDX_STATUS_DOS 16 | drivers/net/can/esd/esdacc.c:37: warning: macro "ACC_REG_STATUS_MASK_STATUS_RBS" is not used [-Wunused-macros] 37 | #define ACC_REG_STATUS_MASK_STATUS_RBS BIT(ACC_REG_STATUS_IDX_STATUS_RBS) | drivers/net/can/esd/esdacc.c:38: warning: macro "ACC_REG_STATUS_MASK_STATUS_RS" is not used [-Wunused-macros] 38 | #define ACC_REG_STATUS_MASK_STATUS_RS BIT(ACC_REG_STATUS_IDX_STATUS_RS) | drivers/net/can/esd/esdacc.c:32: warning: macro "ACC_REG_STATUS_IDX_STATUS_RS" is not used [-Wunused-macros] 32 | #define ACC_REG_STATUS_IDX_STATUS_RS 21 | drivers/net/can/esd/esdacc.c:31: warning: macro "ACC_REG_STATUS_IDX_STATUS_RBS" is not used [-Wunused-macros] 31 | #define ACC_REG_STATUS_IDX_STATUS_RBS 20 | drivers/net/can/esd/esdacc.c:33: warning: macro "ACC_REG_STATUS_MASK_STATUS_DOS" is not used [-Wunused-macros] 33 | #define ACC_REG_STATUS_MASK_STATUS_DOS BIT(ACC_REG_STATUS_IDX_STATUS_DOS) | drivers/net/can/esd/esdacc.c:22: warning: macro "ACC_BM_IRQ_MASK_ALL" is not used [-Wunused-macros] 22 | #define ACC_BM_IRQ_MASK_ALL 0xaaaaaaaaU > +static void acc_resetmode_enter(struct acc_core *core) > +{ > + acc_set_bits(core, ACC_CORE_OF_CTRL_MODE, > + ACC_REG_CONTROL_MASK_MODE_RESETMODE); > + > + /* Read back reset mode bit to flush PCI write posting */ > + acc_resetmode_entered(core); > +} > + > +static void acc_resetmode_leave(struct acc_core *core) > +{ > + acc_clear_bits(core, ACC_CORE_OF_CTRL_MODE, > + ACC_REG_CONTROL_MASK_MODE_RESETMODE); > + > + /* Read back reset mode bit to flush PCI write posting */ > + acc_resetmode_entered(core); > +} > + > +static void acc_txq_put(struct acc_core *core, u32 acc_id, u8 acc_dlc, > + const void *data) > +{ > + acc_write32_noswap(core, ACC_CORE_OF_TXFIFO_DATA_1, > + *((const u32 *)(data + 4))); > + acc_write32_noswap(core, ACC_CORE_OF_TXFIFO_DATA_0, > + *((const u32 *)data)); > + acc_write32(core, ACC_CORE_OF_TXFIFO_DLC, acc_dlc); > + /* CAN id must be written at last. This write starts TX. */ > + acc_write32(core, ACC_CORE_OF_TXFIFO_ID, acc_id); > +} > + > +/* Convert timestamp from esdACC time stamp ticks to ns > + * > + * The conversion factor ts2ns from time stamp counts to ns is basically > + * ts2ns = NSEC_PER_SEC / timestamp_frequency > + * > + * We handle here only a fixed timestamp frequency of 80MHz. The > + * resulting ts2ns factor would be 12.5. > + * > + * At the end we multiply by 12 and add the half of the HW timestamp > + * to get a multiplication by 12.5. This way any overflow is > + * avoided until ktime_t itself overflows. > + */ > +#define ACC_TS_FACTOR (NSEC_PER_SEC / ACC_TS_FREQ_80MHZ) > +#define ACC_TS_80MHZ_SHIFT 1 > + > +static ktime_t acc_ts2ktime(struct acc_ov *ov, u64 ts) > +{ > + u64 ns; > + > + ns = (ts * ACC_TS_FACTOR) + (ts >> ACC_TS_80MHZ_SHIFT); > + > + return ns_to_ktime(ns); > +} > + > +void acc_init_ov(struct acc_ov *ov, struct device *dev) > +{ > + u32 temp; > + > + temp = acc_ov_read32(ov, ACC_OV_OF_VERSION); > + ov->version = temp; > + ov->features = (temp >> 16); > + > + temp = acc_ov_read32(ov, ACC_OV_OF_INFO); > + ov->total_cores = temp; > + ov->active_cores = (temp >> 8); > + > + ov->core_frequency = acc_ov_read32(ov, ACC_OV_OF_CANCORE_FREQ); > + ov->timestamp_frequency = acc_ov_read32(ov, ACC_OV_OF_TS_FREQ_LO); > + > + /* Depending on ESDACC feature NEW_PSC enable the new prescaler > + * or adjust core_frequency according to the implicit division by 2. > + */ > + if (ov->features & ACC_OV_REG_FEAT_MASK_NEW_PSC) { > + acc_ov_set_bits(ov, ACC_OV_OF_MODE, > + ACC_OV_REG_MODE_MASK_NEW_PSC_ENABLE); > + } else { > + ov->core_frequency /= 2; > + } > + > + dev_info(dev, > + "ESDACC v%u, freq: %u/%u, feat/strap: 0x%x/0x%x, cores: %u/%u\n", > + ov->version, ov->core_frequency, ov->timestamp_frequency, > + ov->features, acc_ov_read32(ov, ACC_OV_OF_INFO) >> 16, > + ov->active_cores, ov->total_cores); Consider using dev_dbg() instead of dev_info() here. > +} > + > +void acc_init_bm_ptr(struct acc_ov *ov, struct acc_core *cores, const void *mem) > +{ > + unsigned int u; > + > + /* DMA buffer layout as follows where N is the number of CAN cores > + * implemented in the FPGA, i.e. N = ov->total_cores > + * > + * Layout Section size > + * +-----------------------+ > + * | FIFO Card/Overview | ACC_CORE_DMABUF_SIZE > + * | | > + * +-----------------------+ > + * | FIFO Core0 | ACC_CORE_DMABUF_SIZE > + * | | > + * +-----------------------+ > + * | ... | ... > + * | | > + * +-----------------------+ > + * | FIFO CoreN | ACC_CORE_DMABUF_SIZE > + * | | > + * +-----------------------+ > + * | irq_cnt Card/Overview | sizeof(u32) > + * +-----------------------+ > + * | irq_cnt Core0 | sizeof(u32) > + * +-----------------------+ > + * | ... | ... > + * +-----------------------+ > + * | irq_cnt CoreN | sizeof(u32) > + * +-----------------------+ Nitpick, instead of drawing all the cells in ascii art, please prefer a more frugal style. Example: Layout Section size --------------------------------------------- FIFO Card/Overview ACC_CORE_DMABUF_SIZE FIFO Core0 ACC_CORE_DMABUF_SIZE ... ... > + */ > + ov->bmfifo.messages = mem; > + ov->bmfifo.irq_cnt = mem + (ov->total_cores + 1U) * ACC_CORE_DMABUF_SIZE; > + > + for (u = 0U; u < ov->active_cores; u++) { > + struct acc_core *core = &cores[u]; > + > + core->bmfifo.messages = mem + (u + 1U) * ACC_CORE_DMABUF_SIZE; > + core->bmfifo.irq_cnt = ov->bmfifo.irq_cnt + (u + 1U); > + } > +} > + > +int acc_open(struct net_device *netdev) > +{ > + struct acc_net_priv *priv = netdev_priv(netdev); > + struct acc_core *core = priv->core; > + u32 tx_fifo_status; > + u32 ctrl_mode; > + int err; > + > + /* Retry to enter RESET mode if out of sync. */ > + if (priv->can.state != CAN_STATE_STOPPED) { > + netdev_warn(netdev, "Entered %s() with bad can.state: %s\n", > + __func__, can_get_state_str(priv->can.state)); > + acc_resetmode_enter(core); > + priv->can.state = CAN_STATE_STOPPED; > + } > + > + err = open_candev(netdev); > + if (err) > + return err; > + > + ctrl_mode = ACC_REG_CONTROL_MASK_IE_RXTX | > + ACC_REG_CONTROL_MASK_IE_TXERROR | > + ACC_REG_CONTROL_MASK_IE_ERRWARN | > + ACC_REG_CONTROL_MASK_IE_OVERRUN | > + ACC_REG_CONTROL_MASK_IE_ERRPASS; > + > + if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) > + ctrl_mode |= ACC_REG_CONTROL_MASK_IE_BUSERR; > + > + if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) > + ctrl_mode |= ACC_REG_CONTROL_MASK_MODE_LOM; > + > + acc_set_bits(core, ACC_CORE_OF_CTRL_MODE, ctrl_mode); > + > + acc_resetmode_leave(core); > + priv->can.state = CAN_STATE_ERROR_ACTIVE; > + > + /* Resync TX FIFO indices to HW state after (re-)start. */ > + tx_fifo_status = acc_read32(core, ACC_CORE_OF_TXFIFO_STATUS); > + core->tx_fifo_head = tx_fifo_status & 0xff; > + core->tx_fifo_tail = (tx_fifo_status >> 8) & 0xff; > + > + netif_start_queue(netdev); > + return 0; > +} > + > +int acc_close(struct net_device *netdev) > +{ > + struct acc_net_priv *priv = netdev_priv(netdev); > + struct acc_core *core = priv->core; > + > + acc_clear_bits(core, ACC_CORE_OF_CTRL_MODE, > + ACC_REG_CONTROL_MASK_IE_RXTX | > + ACC_REG_CONTROL_MASK_IE_TXERROR | > + ACC_REG_CONTROL_MASK_IE_ERRWARN | > + ACC_REG_CONTROL_MASK_IE_OVERRUN | > + ACC_REG_CONTROL_MASK_IE_ERRPASS | > + ACC_REG_CONTROL_MASK_IE_BUSERR); > + > + netif_stop_queue(netdev); > + acc_resetmode_enter(core); > + priv->can.state = CAN_STATE_STOPPED; > + > + /* Mark pending TX requests to be aborted after controller restart. */ > + acc_write32(core, ACC_CORE_OF_TX_ABORT_MASK, 0xffff); > + > + /* ACC_REG_CONTROL_MASK_MODE_LOM is only accessible in RESET mode */ > + acc_clear_bits(core, ACC_CORE_OF_CTRL_MODE, > + ACC_REG_CONTROL_MASK_MODE_LOM); > + > + close_candev(netdev); > + return 0; > +} > + > +netdev_tx_t acc_start_xmit(struct sk_buff *skb, struct net_device *netdev) > +{ > + struct acc_net_priv *priv = netdev_priv(netdev); > + struct acc_core *core = priv->core; > + struct can_frame *cf = (struct can_frame *)skb->data; > + u8 tx_fifo_head = core->tx_fifo_head; > + int fifo_usage; > + u32 acc_id; > + u8 acc_dlc; > + > + if (can_dropped_invalid_skb(netdev, skb)) > + return NETDEV_TX_OK; > + > + /* Access core->tx_fifo_tail only once because it may be changed > + * from the interrupt level. > + */ > + fifo_usage = tx_fifo_head - core->tx_fifo_tail; > + if (fifo_usage < 0) > + fifo_usage += core->tx_fifo_size; > + > + if (fifo_usage >= core->tx_fifo_size - 1) { > + netdev_err(core->netdev, > + "BUG: TX ring full when queue awake!\n"); > + netif_stop_queue(netdev); > + return NETDEV_TX_BUSY; > + } > + > + if (fifo_usage == core->tx_fifo_size - 2) > + netif_stop_queue(netdev); > + > + acc_dlc = can_get_cc_dlc(cf, priv->can.ctrlmode); > + if (cf->can_id & CAN_RTR_FLAG) > + acc_dlc |= ACC_CAN_RTR_FLAG; > + > + if (cf->can_id & CAN_EFF_FLAG) { > + acc_id = cf->can_id & CAN_EFF_MASK; > + acc_id |= ACC_CAN_EFF_FLAG; > + } else { > + acc_id = cf->can_id & CAN_SFF_MASK; > + } > + > + can_put_echo_skb(skb, netdev, core->tx_fifo_head, 0); > + > + tx_fifo_head++; > + if (tx_fifo_head >= core->tx_fifo_size) > + tx_fifo_head = 0U; Those three lines are repeated several times when setting tx_fifo_head or tx_fifo_tail. I suggest adding an helper function (e.g. acc_inc_fifo_idx()) to factorize all that. > + core->tx_fifo_head = tx_fifo_head; > + > + acc_txq_put(core, acc_id, acc_dlc, cf->data); > + > + return NETDEV_TX_OK; > +} > + > +int acc_get_berr_counter(const struct net_device *netdev, > + struct can_berr_counter *bec) > +{ > + struct acc_net_priv *priv = netdev_priv(netdev); > + u32 core_status = acc_read32(priv->core, ACC_CORE_OF_STATUS); > + > + bec->txerr = (core_status >> 8) & 0xff; > + bec->rxerr = core_status & 0xff; > + > + return 0; > +} > + > +int acc_set_mode(struct net_device *netdev, enum can_mode mode) > +{ > + struct acc_net_priv *priv = netdev_priv(netdev); > + > + switch (mode) { > + case CAN_MODE_START: > + /* Paranoid FIFO index check. */ > + { > + const u32 tx_fifo_status = > + acc_read32(priv->core, ACC_CORE_OF_TXFIFO_STATUS); > + const u8 hw_fifo_head = tx_fifo_status; > + > + if (hw_fifo_head != priv->core->tx_fifo_head || > + hw_fifo_head != priv->core->tx_fifo_tail) { > + netdev_warn(netdev, > + "TX FIFO mismatch: T %2u H %2u; TFHW %#08x\n", > + priv->core->tx_fifo_tail, > + priv->core->tx_fifo_head, > + tx_fifo_status); > + } > + } > + acc_resetmode_leave(priv->core); > + /* To leave the bus-off state the esdACC controller begins > + * here a grace period where it counts 128 "idle conditions" (each > + * of 11 consecutive recessive bits) on the bus as required > + * by the CAN spec. > + * > + * During this time the TX FIFO may still contain already > + * aborted "zombie" frames that are only drained from the FIFO > + * at the end of the grace period. > + * > + * To not to interfere with this drain process we don't > + * call netif_wake_queue() here. When the controller reaches > + * the error-active state again, it informs us about that > + * with an acc_bmmsg_errstatechange message. Then > + * netif_wake_queue() is called from > + * handle_core_msg_errstatechange() instead. > + */ > + break; > + > + default: > + return -EOPNOTSUPP; > + } > + > + return 0; > +} > + > +int acc_set_bittiming(struct net_device *netdev) > +{ > + struct acc_net_priv *priv = netdev_priv(netdev); > + const struct can_bittiming *bt = &priv->can.bittiming; > + u32 brp; > + u32 btr; > + > + if (priv->ov->features & ACC_OV_REG_FEAT_MASK_CANFD) { > + u32 fbtr = 0; > + > + netdev_dbg(netdev, "bit timing: brp %u, prop %u, ph1 %u ph2 %u, sjw %u\n", > + bt->brp, bt->prop_seg, > + bt->phase_seg1, bt->phase_seg2, bt->sjw); > + > + brp = FIELD_PREP(ACC_REG_BRP_FD_MASK_BRP, bt->brp - 1); > + > + btr = FIELD_PREP(ACC_REG_BTR_FD_MASK_TSEG1, bt->phase_seg1 + bt->prop_seg - 1); > + btr |= FIELD_PREP(ACC_REG_BTR_FD_MASK_TSEG2, bt->phase_seg2 - 1); > + btr |= FIELD_PREP(ACC_REG_BTR_FD_MASK_SJW, bt->sjw - 1); > + > + /* Keep order of accesses to ACC_CORE_OF_BRP and ACC_CORE_OF_BTR. */ > + acc_write32(priv->core, ACC_CORE_OF_BRP, brp); > + acc_write32(priv->core, ACC_CORE_OF_BTR, btr); > + > + netdev_dbg(netdev, "ESDACC: BRP %u, NBTR 0x%08x, DBTR 0x%08x", > + brp, btr, fbtr); > + } else { > + netdev_dbg(netdev, "bit timing: brp %u, prop %u, ph1 %u ph2 %u, sjw %u\n", > + bt->brp, bt->prop_seg, > + bt->phase_seg1, bt->phase_seg2, bt->sjw); > + > + brp = FIELD_PREP(ACC_REG_BRP_CL_MASK_BRP, bt->brp - 1); > + > + btr = FIELD_PREP(ACC_REG_BTR_CL_MASK_TSEG1, bt->phase_seg1 + bt->prop_seg - 1); > + btr |= FIELD_PREP(ACC_REG_BTR_CL_MASK_TSEG2, bt->phase_seg2 - 1); > + btr |= FIELD_PREP(ACC_REG_BTR_CL_MASK_SJW, bt->sjw - 1); > + > + /* Keep order of accesses to ACC_CORE_OF_BRP and ACC_CORE_OF_BTR. */ > + acc_write32(priv->core, ACC_CORE_OF_BRP, brp); > + acc_write32(priv->core, ACC_CORE_OF_BTR, btr); > + > + netdev_dbg(netdev, "ESDACC: BRP %u, BTR 0x%08x", brp, btr); > + } > + > + return 0; > +} > + > +static void handle_core_msg_rxtxdone(struct acc_core *core, > + const struct acc_bmmsg_rxtxdone *msg) > +{ > + struct acc_net_priv *priv = netdev_priv(core->netdev); > + struct net_device_stats *stats = &core->netdev->stats; > + struct sk_buff *skb; > + > + if (msg->dlc.rxtx.len & ACC_BM_LENFLAG_TX) { > + u8 tx_fifo_tail = core->tx_fifo_tail; > + > + if (core->tx_fifo_head == tx_fifo_tail) { > + netdev_warn(core->netdev, > + "TX interrupt, but queue is empty!?\n"); > + return; > + } > + > + /* Direct access echo skb to attach HW time stamp. */ > + skb = priv->can.echo_skb[tx_fifo_tail]; > + if (skb) { > + skb_hwtstamps(skb)->hwtstamp = > + acc_ts2ktime(priv->ov, msg->ts); > + } > + > + stats->tx_packets++; > + stats->tx_bytes += can_get_echo_skb(core->netdev, tx_fifo_tail, > + NULL); > + > + tx_fifo_tail++; > + if (tx_fifo_tail >= core->tx_fifo_size) > + tx_fifo_tail = 0U; > + core->tx_fifo_tail = tx_fifo_tail; > + > + netif_wake_queue(core->netdev); > + > + } else { > + struct can_frame *cf; > + > + skb = alloc_can_skb(core->netdev, &cf); > + if (!skb) { > + stats->rx_dropped++; > + return; > + } > + > + cf->can_id = msg->id & CAN_EFF_MASK; Semantically, you want the exclude the EFF, RTR and ERR flags, right? If so, use CAN_ERR_MASK instead of CAN_EFF_MASK. https://elixir.bootlin.com/linux/v6.6/source/include/uapi/linux/can.h#L63 Please also check the other use of CAN_EFF_MASK throughout the driver. > + if (msg->id & ACC_CAN_EFF_FLAG) > + cf->can_id |= CAN_EFF_FLAG; > + > + can_frame_set_cc_len(cf, msg->dlc.rx.len & ACC_CAN_DLC_MASK, > + priv->can.ctrlmode); > + > + if (msg->dlc.rx.len & ACC_CAN_RTR_FLAG) { > + cf->can_id |= CAN_RTR_FLAG; > + } else { > + memcpy(cf->data, msg->data, cf->len); > + stats->rx_bytes += cf->len; > + } > + stats->rx_packets++; > + > + skb_hwtstamps(skb)->hwtstamp = acc_ts2ktime(priv->ov, msg->ts); > + > + netif_rx(skb); > + } > +} > + > +static void handle_core_msg_txabort(struct acc_core *core, > + const struct acc_bmmsg_txabort *msg) > +{ > + struct net_device_stats *stats = &core->netdev->stats; > + u8 tx_fifo_tail = core->tx_fifo_tail; > + u32 abort_mask = msg->abort_mask; /* u32 extend to avoid warnings later */ > + > + /* The abort_mask shows which frames were aborted in ESDACC's FIFO. */ > + while (tx_fifo_tail != core->tx_fifo_head && (abort_mask)) { > + const u32 tail_mask = (1U << tx_fifo_tail); > + > + if (!(abort_mask & tail_mask)) > + break; > + abort_mask &= ~tail_mask; > + > + can_free_echo_skb(core->netdev, tx_fifo_tail, NULL); > + stats->tx_dropped++; > + stats->tx_aborted_errors++; > + > + tx_fifo_tail++; > + if (tx_fifo_tail >= core->tx_fifo_size) > + tx_fifo_tail = 0; > + } > + core->tx_fifo_tail = tx_fifo_tail; > + if (abort_mask) > + netdev_warn(core->netdev, "Unhandled aborted messages\n"); > + > + if (!acc_resetmode_entered(core)) > + netif_wake_queue(core->netdev); > +} > + > +static void handle_core_msg_overrun(struct acc_core *core, > + const struct acc_bmmsg_overrun *msg) > +{ > + struct acc_net_priv *priv = netdev_priv(core->netdev); > + struct net_device_stats *stats = &core->netdev->stats; > + struct can_frame *cf; > + struct sk_buff *skb; > + > + /* lost_cnt may be 0 if not supported by ESDACC version */ > + if (msg->lost_cnt) { > + stats->rx_errors += msg->lost_cnt; > + stats->rx_over_errors += msg->lost_cnt; > + } else { > + stats->rx_errors++; > + stats->rx_over_errors++; > + } > + > + skb = alloc_can_err_skb(core->netdev, &cf); > + if (!skb) { > + stats->rx_dropped++; > + return; > + } > + > + cf->can_id |= CAN_ERR_CRTL; > + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW; > + > + skb_hwtstamps(skb)->hwtstamp = acc_ts2ktime(priv->ov, msg->ts); > + > + netif_rx(skb); > +} > + > +static void handle_core_msg_buserr(struct acc_core *core, > + const struct acc_bmmsg_buserr *msg) > +{ > + struct acc_net_priv *priv = netdev_priv(core->netdev); > + struct net_device_stats *stats = &core->netdev->stats; > + struct can_frame *cf; > + struct sk_buff *skb; > + const u32 reg_status = msg->reg_status; > + const u8 rxerr = reg_status; > + const u8 txerr = (reg_status >> 8); > + u8 can_err_prot_type = 0U; > + > + priv->can.can_stats.bus_error++; > + > + /* Error occurred during transmission? */ > + if ((msg->ecc & ACC_ECC_DIR) == 0) { > + can_err_prot_type |= CAN_ERR_PROT_TX; > + stats->tx_errors++; > + } else { > + stats->rx_errors++; > + } Nitpick: check the "boolean true" branch first: if (msg->ecc & ACC_ECC_DIR) { stats->rx_errors++; } else { can_err_prot_type |= CAN_ERR_PROT_TX; stats->tx_errors++; } > + /* Determine error type */ > + switch (msg->ecc & ACC_ECC_MASK) { > + case ACC_ECC_BIT: > + can_err_prot_type |= CAN_ERR_PROT_BIT; > + break; > + case ACC_ECC_FORM: > + can_err_prot_type |= CAN_ERR_PROT_FORM; > + break; > + case ACC_ECC_STUFF: > + can_err_prot_type |= CAN_ERR_PROT_STUFF; > + break; > + default: > + can_err_prot_type |= CAN_ERR_PROT_UNSPEC; > + break; > + } > + > + skb = alloc_can_err_skb(core->netdev, &cf); > + if (!skb) { > + stats->rx_dropped++; The CAN err skb is a socket CAN concept. It does not represent an actual CAN frame. So no need to count that one as an rx_dropped. > + return; > + } > + > + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; Set the CAN_ERR_CNT flag to inform the userland that the error counters are available. > + > + /* Set protocol error type */ > + cf->data[2] = can_err_prot_type; > + /* Set error location */ > + cf->data[3] = msg->ecc & ACC_ECC_SEG; > + > + /* Insert CAN TX and RX error counters. */ > + cf->data[6] = txerr; > + cf->data[7] = rxerr; > + > + skb_hwtstamps(skb)->hwtstamp = acc_ts2ktime(priv->ov, msg->ts); > + > + netif_rx(skb); > +} > + > +static void > +handle_core_msg_errstatechange(struct acc_core *core, > + const struct acc_bmmsg_errstatechange *msg) > +{ > + struct acc_net_priv *priv = netdev_priv(core->netdev); > + struct net_device_stats *stats = &core->netdev->stats; > + struct can_frame *cf = NULL; > + struct sk_buff *skb; > + const u32 reg_status = msg->reg_status; > + const u8 rxerr = reg_status; > + const u8 txerr = (reg_status >> 8); > + enum can_state new_state; > + > + if (reg_status & ACC_REG_STATUS_MASK_STATUS_BS) { > + new_state = CAN_STATE_BUS_OFF; > + } else if (reg_status & ACC_REG_STATUS_MASK_STATUS_EP) { > + new_state = CAN_STATE_ERROR_PASSIVE; > + } else if (reg_status & ACC_REG_STATUS_MASK_STATUS_ES) { > + new_state = CAN_STATE_ERROR_WARNING; > + } else { > + new_state = CAN_STATE_ERROR_ACTIVE; > + if (priv->can.state == CAN_STATE_BUS_OFF) { > + /* See comment in acc_set_mode() for CAN_MODE_START */ > + netif_wake_queue(core->netdev); > + } > + } > + > + skb = alloc_can_err_skb(core->netdev, &cf); > + > + if (new_state != priv->can.state) { > + enum can_state tx_state, rx_state; > + > + tx_state = (txerr >= rxerr) ? > + new_state : CAN_STATE_ERROR_ACTIVE; > + rx_state = (rxerr >= txerr) ? > + new_state : CAN_STATE_ERROR_ACTIVE; > + > + /* Always call can_change_state() to update the state > + * even if alloc_can_err_skb() may have failed. > + * can_change_state() can cope with a NULL cf pointer. > + */ > + can_change_state(core->netdev, cf, tx_state, rx_state); > + } > + > + if (skb) { > + cf->data[6] = txerr; > + cf->data[7] = rxerr; Set the CAN_ERR_CNT flag to inform the userland that the error counters are available. > + > + skb_hwtstamps(skb)->hwtstamp = acc_ts2ktime(priv->ov, msg->ts); > + > + netif_rx(skb); > + } else { > + stats->rx_dropped++; The CAN err skb is a socket CAN concept. It does not represent an actual CAN frame. So no need to count that one as an rx_dropped. > + } > + > + if (new_state == CAN_STATE_BUS_OFF) { > + acc_write32(core, ACC_CORE_OF_TX_ABORT_MASK, 0xffff); > + can_bus_off(core->netdev); > + } > +} > + > +static void handle_core_interrupt(struct acc_core *core) > +{ > + u32 msg_fifo_head = core->bmfifo.local_irq_cnt & 0xff; > + > + while (core->bmfifo.msg_fifo_tail != msg_fifo_head) { > + const union acc_bmmsg *msg = > + &core->bmfifo.messages[core->bmfifo.msg_fifo_tail]; > + > + switch (msg->msg_id) { > + case BM_MSG_ID_RXTXDONE: > + handle_core_msg_rxtxdone(core, &msg->rxtxdone); > + break; > + > + case BM_MSG_ID_TXABORT: > + handle_core_msg_txabort(core, &msg->txabort); > + break; > + > + case BM_MSG_ID_OVERRUN: > + handle_core_msg_overrun(core, &msg->overrun); > + break; > + > + case BM_MSG_ID_BUSERR: > + handle_core_msg_buserr(core, &msg->buserr); > + break; > + > + case BM_MSG_ID_ERRPASSIVE: > + case BM_MSG_ID_ERRWARN: > + handle_core_msg_errstatechange(core, > + &msg->errstatechange); > + break; > + > + default: > + /* Ignore all other BM messages (like the CAN-FD messages) */ > + break; > + } > + > + core->bmfifo.msg_fifo_tail = > + (core->bmfifo.msg_fifo_tail + 1) & 0xff; > + } > +} > + > +/** > + * acc_card_interrupt() - handle the interrupts of an esdACC FPGA > + * > + * @ov: overview module structure > + * @cores: array of core structures > + * > + * This function handles all interrupts pending for the overview module and the > + * CAN cores of the esdACC FPGA. > + * > + * It examines for all cores (the overview module core and the CAN cores) > + * the bmfifo.irq_cnt and compares it with the previously saved > + * bmfifo.local_irq_cnt. An IRQ is pending if they differ. The esdACC FPGA > + * updates the bmfifo.irq_cnt values by DMA. > + * > + * The pending interrupts are masked by writing to the IRQ mask register at > + * ACC_OV_OF_BM_IRQ_MASK. This register has for each core a two bit command > + * field evaluated as follows: > + * > + * Define, bit pattern: meaning > + * 00: no action > + * ACC_BM_IRQ_UNMASK, 01: unmask interrupt > + * ACC_BM_IRQ_MASK, 10: mask interrupt > + * 11: no action > + * > + * For each CAN core with a pending IRQ handle_core_interrupt() handles all > + * busmaster messages from the message FIFO. The last handled message (FIFO > + * index) is written to the CAN core to acknowledge its handling. > + * > + * Last step is to unmask all interrupts in the FPGA using > + * ACC_BM_IRQ_UNMASK_ALL. If using kdoc style, then also document the return value. c.f. this W=2 error message: drivers/net/can/esd/esdacc.c:719: warning: No description found for return value of 'acc_card_interrupt' > + */ > +irqreturn_t acc_card_interrupt(struct acc_ov *ov, struct acc_core *cores) > +{ > + u32 irqmask; > + int i; > + > + /* First we look for whom interrupts are pending, card/overview > + * or any of the cores. Two bits in irqmask are used for each; > + * Each two bit field is set to ACC_BM_IRQ_MASK if an IRQ is > + * pending. > + */ > + irqmask = 0; > + if (READ_ONCE(*ov->bmfifo.irq_cnt) != ov->bmfifo.local_irq_cnt) { > + irqmask |= ACC_BM_IRQ_MASK; > + ov->bmfifo.local_irq_cnt = READ_ONCE(*ov->bmfifo.irq_cnt); > + } > + > + for (i = 0; i < ov->active_cores; i++) { > + struct acc_core *core = &cores[i]; > + > + if (READ_ONCE(*core->bmfifo.irq_cnt) != core->bmfifo.local_irq_cnt) { > + irqmask |= (ACC_BM_IRQ_MASK << (2 * (i + 1))); > + core->bmfifo.local_irq_cnt = READ_ONCE(*core->bmfifo.irq_cnt); > + } > + } > + > + if (!irqmask) > + return IRQ_NONE; > + > + /* At second we tell the card we're working on them by writing irqmask, > + * call handle_{ov|core}_interrupt and then acknowledge the > + * interrupts by writing irq_cnt: > + */ > + acc_ov_write32(ov, ACC_OV_OF_BM_IRQ_MASK, irqmask); > + > + if (irqmask & ACC_BM_IRQ_MASK) { > + /* handle_ov_interrupt(); - no use yet. */ > + acc_ov_write32(ov, ACC_OV_OF_BM_IRQ_COUNTER, > + ov->bmfifo.local_irq_cnt); > + } > + > + for (i = 0; i < ov->active_cores; i++) { > + struct acc_core *core = &cores[i]; > + > + if (irqmask & (ACC_BM_IRQ_MASK << (2 * (i + 1)))) { > + handle_core_interrupt(core); > + acc_write32(core, ACC_OV_OF_BM_IRQ_COUNTER, > + core->bmfifo.local_irq_cnt); > + } > + } > + > + acc_ov_write32(ov, ACC_OV_OF_BM_IRQ_MASK, ACC_BM_IRQ_UNMASK_ALL); > + > + return IRQ_HANDLED; > +} > diff --git a/drivers/net/can/esd/esdacc.h b/drivers/net/can/esd/esdacc.h > new file mode 100644 > index 000000000000..73651bc1d52c > --- /dev/null > +++ b/drivers/net/can/esd/esdacc.h > @@ -0,0 +1,393 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright (C) 2015 - 2016 Thomas Körper, esd electronic system design gmbh > + * Copyright (C) 2017 - 2022 Stefan Mätje, esd electronics gmbh > + */ > + > +#include <linux/bits.h> > +#include <linux/can/dev.h> > +#include <linux/kernel.h> > +#include <linux/netdevice.h> > +#include <linux/units.h> > + > +#define ACC_TS_FREQ_80MHZ (80 * HZ_PER_MHZ) > + > +#define ACC_CAN_EFF_FLAG 0x20000000 > +#define ACC_CAN_RTR_FLAG 0x10 > +#define ACC_CAN_DLC_MASK 0x0f > + > +#define ACC_OV_OF_PROBE 0x0000 > +#define ACC_OV_OF_VERSION 0x0004 > +#define ACC_OV_OF_INFO 0x0008 > +#define ACC_OV_OF_CANCORE_FREQ 0x000c > +#define ACC_OV_OF_TS_FREQ_LO 0x0010 > +#define ACC_OV_OF_TS_FREQ_HI 0x0014 > +#define ACC_OV_OF_IRQ_STATUS_CORES 0x0018 > +#define ACC_OV_OF_TS_CURR_LO 0x001c > +#define ACC_OV_OF_TS_CURR_HI 0x0020 > +#define ACC_OV_OF_IRQ_STATUS 0x0028 > +#define ACC_OV_OF_MODE 0x002c > +#define ACC_OV_OF_BM_IRQ_COUNTER 0x0070 > +#define ACC_OV_OF_BM_IRQ_MASK 0x0074 > +#define ACC_OV_OF_MSI_DATA 0x0080 > +#define ACC_OV_OF_MSI_ADDRESSOFFSET 0x0084 The #define indentation style is not consistant between the different files. Please pick one (I suggest the single space to make it easier to add more defines later on). > +/* Feature flags are contained in the upper 16 bit of the version > + * register at ACC_OV_OF_VERSION but only used with these masks after > + * extraction into an extra variable => (xx - 16). > + */ > +#define ACC_OV_REG_FEAT_IDX_CANFD (27 - 16) > +#define ACC_OV_REG_FEAT_IDX_NEW_PSC (28 - 16) > +#define ACC_OV_REG_FEAT_MASK_CANFD BIT(ACC_OV_REG_FEAT_IDX_CANFD) > +#define ACC_OV_REG_FEAT_MASK_NEW_PSC BIT(ACC_OV_REG_FEAT_IDX_NEW_PSC) > + > +#define ACC_OV_REG_MODE_MASK_ENDIAN_LITTLE 0x00000001 > +#define ACC_OV_REG_MODE_MASK_BM_ENABLE 0x00000002 > +#define ACC_OV_REG_MODE_MASK_MODE_LED 0x00000004 > +#define ACC_OV_REG_MODE_MASK_TIMER 0x00000070 > +#define ACC_OV_REG_MODE_MASK_TIMER_ENABLE 0x00000010 > +#define ACC_OV_REG_MODE_MASK_TIMER_ONE_SHOT 0x00000020 > +#define ACC_OV_REG_MODE_MASK_TIMER_ABSOLUTE 0x00000040 > +#define ACC_OV_REG_MODE_MASK_TS_SRC 0x00000180 > +#define ACC_OV_REG_MODE_MASK_I2C_ENABLE 0x00000800 > +#define ACC_OV_REG_MODE_MASK_MSI_ENABLE 0x00004000 > +#define ACC_OV_REG_MODE_MASK_NEW_PSC_ENABLE 0x00008000 > +#define ACC_OV_REG_MODE_MASK_FPGA_RESET 0x80000000 > + > +#define ACC_CORE_OF_CTRL_MODE 0x0000 > +#define ACC_CORE_OF_STATUS_IRQ 0x0008 > +#define ACC_CORE_OF_BRP 0x000c > +#define ACC_CORE_OF_BTR 0x0010 > +#define ACC_CORE_OF_FBTR 0x0014 > +#define ACC_CORE_OF_STATUS 0x0030 > +#define ACC_CORE_OF_TXFIFO_CONFIG 0x0048 > +#define ACC_CORE_OF_TXFIFO_STATUS 0x004c > +#define ACC_CORE_OF_TX_STATUS_IRQ 0x0050 > +#define ACC_CORE_OF_TX_ABORT_MASK 0x0054 > +#define ACC_CORE_OF_BM_IRQ_COUNTER 0x0070 > +#define ACC_CORE_OF_TXFIFO_ID 0x00c0 > +#define ACC_CORE_OF_TXFIFO_DLC 0x00c4 > +#define ACC_CORE_OF_TXFIFO_DATA_0 0x00c8 > +#define ACC_CORE_OF_TXFIFO_DATA_1 0x00cc > + > +#define ACC_REG_CONTROL_IDX_MODE_RESETMODE 0 > +#define ACC_REG_CONTROL_IDX_MODE_LOM 1 > +#define ACC_REG_CONTROL_IDX_MODE_STM 2 > +#define ACC_REG_CONTROL_IDX_MODE_TRANSEN 5 > +#define ACC_REG_CONTROL_IDX_MODE_TS 6 > +#define ACC_REG_CONTROL_IDX_MODE_SCHEDULE 7 > +#define ACC_REG_CONTROL_MASK_MODE_RESETMODE \ > + BIT(ACC_REG_CONTROL_IDX_MODE_RESETMODE) > +#define ACC_REG_CONTROL_MASK_MODE_LOM \ > + BIT(ACC_REG_CONTROL_IDX_MODE_LOM) > +#define ACC_REG_CONTROL_MASK_MODE_STM \ > + BIT(ACC_REG_CONTROL_IDX_MODE_STM) > +#define ACC_REG_CONTROL_MASK_MODE_TRANSEN \ > + BIT(ACC_REG_CONTROL_IDX_MODE_TRANSEN) > +#define ACC_REG_CONTROL_MASK_MODE_TS \ > + BIT(ACC_REG_CONTROL_IDX_MODE_TS) > +#define ACC_REG_CONTROL_MASK_MODE_SCHEDULE \ > + BIT(ACC_REG_CONTROL_IDX_MODE_SCHEDULE) > + > +#define ACC_REG_CONTROL_IDX_IE_RXTX 8 > +#define ACC_REG_CONTROL_IDX_IE_TXERROR 9 > +#define ACC_REG_CONTROL_IDX_IE_ERRWARN 10 > +#define ACC_REG_CONTROL_IDX_IE_OVERRUN 11 > +#define ACC_REG_CONTROL_IDX_IE_TSI 12 > +#define ACC_REG_CONTROL_IDX_IE_ERRPASS 13 > +#define ACC_REG_CONTROL_IDX_IE_BUSERR 15 > +#define ACC_REG_CONTROL_MASK_IE_RXTX BIT(ACC_REG_CONTROL_IDX_IE_RXTX) > +#define ACC_REG_CONTROL_MASK_IE_TXERROR BIT(ACC_REG_CONTROL_IDX_IE_TXERROR) > +#define ACC_REG_CONTROL_MASK_IE_ERRWARN BIT(ACC_REG_CONTROL_IDX_IE_ERRWARN) > +#define ACC_REG_CONTROL_MASK_IE_OVERRUN BIT(ACC_REG_CONTROL_IDX_IE_OVERRUN) > +#define ACC_REG_CONTROL_MASK_IE_TSI BIT(ACC_REG_CONTROL_IDX_IE_TSI) > +#define ACC_REG_CONTROL_MASK_IE_ERRPASS BIT(ACC_REG_CONTROL_IDX_IE_ERRPASS) > +#define ACC_REG_CONTROL_MASK_IE_BUSERR BIT(ACC_REG_CONTROL_IDX_IE_BUSERR) > + > +/* BRP and BTR register layout for CAN-Classic version */ > +#define ACC_REG_BRP_CL_MASK_BRP GENMASK(8, 0) > +#define ACC_REG_BTR_CL_MASK_TSEG1 GENMASK(3, 0) > +#define ACC_REG_BTR_CL_MASK_TSEG2 GENMASK(18, 16) > +#define ACC_REG_BTR_CL_MASK_SJW GENMASK(25, 24) > + > +/* BRP and BTR register layout for CAN-FD version */ > +#define ACC_REG_BRP_FD_MASK_BRP GENMASK(7, 0) > +#define ACC_REG_BTR_FD_MASK_TSEG1 GENMASK(7, 0) > +#define ACC_REG_BTR_FD_MASK_TSEG2 GENMASK(22, 16) > +#define ACC_REG_BTR_FD_MASK_SJW GENMASK(30, 24) > + > +/* 256 BM_MSGs of 32 byte size */ > +#define ACC_CORE_DMAMSG_SIZE 32U > +#define ACC_CORE_DMABUF_SIZE (256U * ACC_CORE_DMAMSG_SIZE) > + > +enum acc_bmmsg_id { > + BM_MSG_ID_RXTXDONE = 0x01, > + BM_MSG_ID_TXABORT = 0x02, > + BM_MSG_ID_OVERRUN = 0x03, > + BM_MSG_ID_BUSERR = 0x04, > + BM_MSG_ID_ERRPASSIVE = 0x05, > + BM_MSG_ID_ERRWARN = 0x06, > + BM_MSG_ID_TIMESLICE = 0x07, > + BM_MSG_ID_HWTIMER = 0x08, > + BM_MSG_ID_HOTPLUG = 0x09, > +}; > + > +/* The struct acc_bmmsg_* structure declarations that follow here provide > + * access to the ring buffer of bus master messages maintained by the FPGA > + * bus master engine. All bus master messages have the same size of > + * ACC_CORE_DMAMSG_SIZE and a minimum alignment of ACC_CORE_DMAMSG_SIZE in > + * memory. > + * > + * All structure members are natural aligned. Therefore we should not need > + * a __packed attribute. All struct acc_bmmsg_* declarations have at least > + * reserved* members to fill the structure to the full ACC_CORE_DMAMSG_SIZE. > + * > + * A failure of this property due padding will be detected at compile time > + * by static_assert(sizeof(union acc_bmmsg) == ACC_CORE_DMAMSG_SIZE). > + */ > + > +struct acc_bmmsg_rxtxdone { > + u8 msg_id; > + u8 txfifo_level; > + u8 reserved1[2]; > + u8 txtsfifo_level; > + u8 reserved2[3]; > + u32 id; > + union { > + struct { > + u8 len; > + u8 reserved0; Technically, this one is not reserved because is it used in both rx and tx structures. > + u8 bits; > + u8 state; > + } rxtx; > + struct { > + u8 len; > + u8 msg_lost; > + u8 bits; > + u8 state; > + } rx; > + struct { > + u8 len; > + u8 txfifo_idx; > + u8 bits; > + u8 state; > + } tx; > + } dlc; ^^^ This looks like a misnomer to me: the union contains more than the dlc. What about: struct { u8 len; union { u8 rx_msg_lost; u8 tx_fifo_idx; } __packed; u8 bits; u8 state; } rxtx; in place of that union? > + u8 data[8]; Nitpick: u8 data[CAN_MAX_DLEN]; > + /* Time stamps in struct acc_ov::timestamp_frequency ticks. */ > + u64 ts; > +}; > + > +struct acc_bmmsg_txabort { > + u8 msg_id; > + u8 txfifo_level; > + u16 abort_mask; > + u8 txtsfifo_level; > + u8 reserved2[1]; > + u16 abort_mask_txts; > + u64 ts; > + u32 reserved3[4]; > +}; > + > +struct acc_bmmsg_overrun { > + u8 msg_id; > + u8 txfifo_level; > + u8 lost_cnt; > + u8 reserved1; > + u8 txtsfifo_level; > + u8 reserved2[3]; > + u64 ts; > + u32 reserved3[4]; > +}; > + > +struct acc_bmmsg_buserr { > + u8 msg_id; > + u8 txfifo_level; > + u8 ecc; > + u8 reserved1; > + u8 txtsfifo_level; > + u8 reserved2[3]; > + u64 ts; > + u32 reg_status; > + u32 reg_btr; > + u32 reserved3[2]; > +}; > + > +struct acc_bmmsg_errstatechange { > + u8 msg_id; > + u8 txfifo_level; > + u8 reserved1[2]; > + u8 txtsfifo_level; > + u8 reserved2[3]; > + u64 ts; > + u32 reg_status; > + u32 reserved3[3]; > +}; > + > +struct acc_bmmsg_timeslice { > + u8 msg_id; > + u8 txfifo_level; > + u8 reserved1[2]; > + u8 txtsfifo_level; > + u8 reserved2[3]; > + u64 ts; > + u32 reserved3[4]; > +}; > + > +struct acc_bmmsg_hwtimer { > + u8 msg_id; > + u8 reserved1[3]; > + u32 reserved2[1]; > + u64 timer; > + u32 reserved3[4]; > +}; > + > +struct acc_bmmsg_hotplug { > + u8 msg_id; > + u8 reserved1[3]; > + u32 reserved2[7]; > +}; > + > +union acc_bmmsg { > + u8 msg_id; > + struct acc_bmmsg_rxtxdone rxtxdone; > + struct acc_bmmsg_txabort txabort; > + struct acc_bmmsg_overrun overrun; > + struct acc_bmmsg_buserr buserr; > + struct acc_bmmsg_errstatechange errstatechange; > + struct acc_bmmsg_timeslice timeslice; > + struct acc_bmmsg_hwtimer hwtimer; > +}; > + > +/* Check size of union acc_bmmsg to be of expected size. */ > +static_assert(sizeof(union acc_bmmsg) == ACC_CORE_DMAMSG_SIZE); > + > +struct acc_bmfifo { > + const union acc_bmmsg *messages; > + /* irq_cnt points to an u32 value where the ESDACC FPGA deposits > + * the bm_fifo head index in coherent DMA memory. Only bits 7..0 > + * are valid. Use READ_ONCE() to access this memory location. > + */ > + const u32 *irq_cnt; > + u32 local_irq_cnt; > + u32 msg_fifo_tail; > +}; > + > +struct acc_core { > + void __iomem *addr; > + struct net_device *netdev; > + struct acc_bmfifo bmfifo; > + u8 tx_fifo_size; > + u8 tx_fifo_head; > + u8 tx_fifo_tail; > +}; > + > +struct acc_ov { > + void __iomem *addr; > + struct acc_bmfifo bmfifo; > + u32 timestamp_frequency; > + u32 core_frequency; > + u16 version; > + u16 features; > + u8 total_cores; > + u8 active_cores; > +}; > + > +struct acc_net_priv { > + struct can_priv can; /* must be the first member! */ > + struct acc_core *core; > + struct acc_ov *ov; > +}; > + > +static inline u32 acc_read32(struct acc_core *core, unsigned short offs) > +{ > + return ioread32be(core->addr + offs); > +} > + > +static inline void acc_write32(struct acc_core *core, > + unsigned short offs, u32 v) > +{ > + iowrite32be(v, core->addr + offs); > +} > + > +static inline void acc_write32_noswap(struct acc_core *core, > + unsigned short offs, u32 v) > +{ > + iowrite32(v, core->addr + offs); > +} > + > +static inline void acc_set_bits(struct acc_core *core, > + unsigned short offs, u32 mask) > +{ > + u32 v = acc_read32(core, offs); > + > + v |= mask; > + acc_write32(core, offs, v); > +} > + > +static inline void acc_clear_bits(struct acc_core *core, > + unsigned short offs, u32 mask) > +{ > + u32 v = acc_read32(core, offs); > + > + v &= ~mask; > + acc_write32(core, offs, v); > +} > + > +static inline int acc_resetmode_entered(struct acc_core *core) > +{ > + u32 ctrl = acc_read32(core, ACC_CORE_OF_CTRL_MODE); > + > + return (ctrl & ACC_REG_CONTROL_MASK_MODE_RESETMODE) != 0; > +} > + > +static inline u32 acc_ov_read32(struct acc_ov *ov, unsigned short offs) > +{ > + return ioread32be(ov->addr + offs); > +} > + > +static inline void acc_ov_write32(struct acc_ov *ov, > + unsigned short offs, u32 v) > +{ > + iowrite32be(v, ov->addr + offs); > +} > + > +static inline void acc_ov_set_bits(struct acc_ov *ov, > + unsigned short offs, u32 b) > +{ > + u32 v = acc_ov_read32(ov, offs); > + > + v |= b; > + acc_ov_write32(ov, offs, v); > +} > + > +static inline void acc_ov_clear_bits(struct acc_ov *ov, > + unsigned short offs, u32 b) > +{ > + u32 v = acc_ov_read32(ov, offs);. > + > + v &= ~b; > + acc_ov_write32(ov, offs, v); > +} > + > +static inline void acc_reset_fpga(struct acc_ov *ov) > +{ > + acc_ov_write32(ov, ACC_OV_OF_MODE, ACC_OV_REG_MODE_MASK_FPGA_RESET); > + > + /* Also reset I^2C, to re-detect card addons at every driver start: */ > + acc_ov_clear_bits(ov, ACC_OV_OF_MODE, ACC_OV_REG_MODE_MASK_I2C_ENABLE); > + mdelay(2); > + acc_ov_set_bits(ov, ACC_OV_OF_MODE, ACC_OV_REG_MODE_MASK_I2C_ENABLE); > + mdelay(10); Maybe add a macro to replace those two delays magic numbers? > +} > + > +void acc_init_ov(struct acc_ov *ov, struct device *dev); > +void acc_init_bm_ptr(struct acc_ov *ov, struct acc_core *cores, > + const void *mem); > +int acc_open(struct net_device *netdev); > +int acc_close(struct net_device *netdev); > +netdev_tx_t acc_start_xmit(struct sk_buff *skb, struct net_device *netdev); > +int acc_get_berr_counter(const struct net_device *netdev, > + struct can_berr_counter *bec); > +int acc_set_mode(struct net_device *netdev, enum can_mode mode); > +int acc_set_bittiming(struct net_device *netdev); > +irqreturn_t acc_card_interrupt(struct acc_ov *ov, struct acc_core *cores); > -- > 2.34.1 > >
