2020-10-30 22:24:58

by Pavel Pisa

[permalink] [raw]
Subject: [PATCH v7 0/6] CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation

This driver adds support for the CTU CAN FD open-source IP core.
More documentation and core sources at project page
(https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core).
The core integration to Xilinx Zynq system as platform driver
is available (https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top).
Implementation on Intel FPGA based PCI Express board is available
from project (https://gitlab.fel.cvut.cz/canbus/pcie-ctucanfd).
The CTU CAN FD core emulation send for review for QEMU mainline.
Development repository for QEMU emulation - ctu-canfd branch of
https://gitlab.fel.cvut.cz/canbus/qemu-canbus

More about CAN bus related projects used and developed at CTU FEE
on the guidepost page http://canbus.pages.fel.cvut.cz/ .

Martin Jerabek (1):
can: ctucanfd: add support for CTU CAN FD open-source IP core - bus
independent part.

Pavel Pisa (5):
dt-bindings: vendor-prefix: add prefix for the Czech Technical
University in Prague.
dt-bindings: net: can: binding for CTU CAN FD open-source IP core.
can: ctucanfd: CTU CAN FD open-source IP core - PCI bus support.
can: ctucanfd: CTU CAN FD open-source IP core - platform/SoC support.
docs: ctucanfd: CTU CAN FD open-source IP core documentation.

The version 7 changes:
- sent at 2020-10-31
- In response of Pavel Machek review, renamed files to match
directly module names. The core specification updated
to provide better description and match of the fields.
Driver headers, routines adjusted etc.. To achieve this,
registers HDL was regenerated and and its connection updated.
- CAN_STATE_* translation to text has been made robust to
Linux kernel define value changes/updates and the function
which uses table has moved after table for better readability.
- fsm_txt_buffer_user.svg redrawn from scratch to reduce
file to 16 kB.
- documentation updated, unified references to recently renamed
pcie-ctucanfd
- I have tried to fullfill request to cross-reference SocketCAN
document by :doc: or :ref: constructs in Sphinx way,
but without success. I reference geerated HTML on kernel.org
site for now.

The version 6 changes:
- sent at 2020-10-22
- the driver has been tested with 5.9 bigendian MIPS kernel
against QEMU CTU CAN FD model and correct behavior on PCIe
virtual board for big-endian system passed
- documentation updated to reflect inclusion of SocketCAN FD
and CTU CAN FD functional model support into QEMU mainline
- the integration for Cyclone V 5CSEMA4U23C6 based DE0-Nano-SoC
Terasic board used for SkodaAuto research projects at our
university has been clean up by its author (Jaroslav Beran)
and published
https://gitlab.fel.cvut.cz/canbus/intel-soc-ctucanfd
- Xilinx Zynq Microzed MZ_APO based target for automatic test
updated to Debian 10 base.

The version 5 changes:
- sent at 2020-08-15
- correct Kconfig formatting according to Randy Dunlap
- silence warnings reported by make W=1 C=1 flags.
Changes suggested by Jakub Kicinski
- big thanks for core patch review by Pavel Machek
resulting in more readability and formating updates
- fix power management errors found by Pavel Machek
- removed comments from d-t bindings as suggested by Rob Herring
- selected ctu,ctucanfd-2 as alternative name to ctu,ctucanfd
which allows to bind to actual major HDL core sources version 2.x
if for some reason driver adaptation would not work on version
read from the core
- line length limit relaxed to 100 characters on some cases
where it helps to readability

The version 4 changes:
- sent at 2020-08-04
- changes summary, 169 non-merge commits, 6 driver,
32 IP core sources enhancements and fixes, 58 tests
in master and about additional 30 iso-testbench
preparation branch.
- convert device-tree binding documentation to YAML
- QEMU model of CTU CAN FD IP core and generic extension
of QEMU CAN bus emulation developed by Jan Charvat.
- driver tested on QEMU emulated Malta big-endian MIPS
platform and big endian-support fixed.
- checkpatch from 5.4 kernel used to cleanup driver formatting
- header files generated from IP core IP-Xact description
updated to include protocol exception (pex) field.
Mechanism to set it from the driver is not provided yet.

The version 3 changes:
- sent at 2019-12-21
- adapts device tree bindings documentation according to
Rob Herring suggestions.
- the driver has been separated to individual modules for core support,
PCI bus integration and platform, SoC integration.
- the FPGA design has been cleaned up and CAN protocol FSM redesigned
by Ondrej Ille (the core redesign has been reason to pause attempts to driver
submission)
- the work from February 2019 on core, test framework and driver
1601 commits in total, 436 commits in the core sources, 144 commits
in the driver, 151 documentation, 502 in tests.
- not all continuous integration tests updated for latest design version yet
https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core/pipelines
- Zynq hardware in the loop test show no issues for after driver PCI and platform
separation and latest VHDL sources updates.
- driver code has been periodically tested on 4.18.5-rt3 and 4.19 long term
stable kernels.
- test of the patches before submission is run on 5.4 kernel
- the core has been integrated by Jaroslav Beran <[email protected]>
into Intel FPGA based SoC used in the tester developed for Skoda auto
at Department of Measurement, Faculty of Electrical Engineering,
Czech Technical University https://meas.fel.cvut.cz/ . He has contributed
feedback and fixes to the project.

The version 2 sent at 2019-02-27

The version 1 sent at 2019-02-22

Ondrej Ille has prepared the CTU CAN IP Core sources for new release.
We are waiting with it for the driver review, our intention
is to release IP when driver is reviewed and mainlined.

DKMS CTU CAN FD driver build by OpenBuildService to ease integration
into Debian systems when driver is not provided by the distribution

https://build.opensuse.org/package/show/home:ppisa/ctu_can_fd

Jan Charvat <[email protected]> finished work to extend already
mainlined QEMU SJA1000 and SocketCAN support to provide even CAN FD
support and CTU CAN FD core support.

https://gitlab.fel.cvut.cz/canbus/qemu-canbus/-/tree/ctu-canfd

The patches has been sent for review to QEMU mainlining list.

Thanks in advance to all who help us to deliver the project into public.

Thanks to all colleagues, reviewers and other providing feedback,
infrastructure and enthusiasm and motivation for open-source work.

Build infrastructure and hardware is provided by
Department of Control Engineering,
Faculty of Electrical Engineering,
Czech Technical University in Prague
https://dce.fel.cvut.cz/en

.../bindings/net/can/ctu,ctucanfd.yaml | 63 +
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
.../device_drivers/ctu/ctucanfd-driver.rst | 638 +++++++++
.../ctu/fsm_txt_buffer_user.svg | 151 +++
drivers/net/can/Kconfig | 1 +
drivers/net/can/Makefile | 1 +
drivers/net/can/ctucanfd/Kconfig | 34 +
drivers/net/can/ctucanfd/Makefile | 10 +
drivers/net/can/ctucanfd/ctucanfd.h | 87 ++
drivers/net/can/ctucanfd/ctucanfd_base.c | 1142 +++++++++++++++++
drivers/net/can/ctucanfd/ctucanfd_frame.h | 189 +++
drivers/net/can/ctucanfd/ctucanfd_hw.c | 751 +++++++++++
drivers/net/can/ctucanfd/ctucanfd_hw.h | 935 ++++++++++++++
drivers/net/can/ctucanfd/ctucanfd_pci.c | 316 +++++
drivers/net/can/ctucanfd/ctucanfd_platform.c | 142 ++
drivers/net/can/ctucanfd/ctucanfd_regs.h | 971 ++++++++++++++
16 files changed, 5433 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml
create mode 100644 Documentation/networking/device_drivers/ctu/ctucanfd-driver.rst
create mode 100644 Documentation/networking/device_drivers/ctu/fsm_txt_buffer_user.svg
create mode 100644 drivers/net/can/ctucanfd/Kconfig
create mode 100644 drivers/net/can/ctucanfd/Makefile
create mode 100644 drivers/net/can/ctucanfd/ctucanfd.h
create mode 100644 drivers/net/can/ctucanfd/ctucanfd_base.c
create mode 100644 drivers/net/can/ctucanfd/ctucanfd_frame.h
create mode 100644 drivers/net/can/ctucanfd/ctucanfd_hw.c
create mode 100644 drivers/net/can/ctucanfd/ctucanfd_hw.h
create mode 100644 drivers/net/can/ctucanfd/ctucanfd_pci.c
create mode 100644 drivers/net/can/ctucanfd/ctucanfd_platform.c
create mode 100644 drivers/net/can/ctucanfd/ctucanfd_regs.h

--
2.20.1


2020-10-30 22:31:46

by Pavel Pisa

[permalink] [raw]
Subject: [PATCH v7 1/6] dt-bindings: vendor-prefix: add prefix for the Czech Technical University in Prague.

The Czech Technical University in Prague (CTU) is one of
the biggest and oldest (founded 1707) technical universities
in Europe. The abbreviation in Czech language is ČVUT according
to official name in Czech language

České vysoké učení technické v Praze

The English translation

The Czech Technical University in Prague

The university pages in English

https://www.cvut.cz/en

Signed-off-by: Pavel Pisa <[email protected]>
Acked-by: Rob Herring <[email protected]>
Acked-by: Pavel Machek <[email protected]>
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 967e78c5ec0a..dedb10f1b250 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -215,6 +215,8 @@ patternProperties:
description: Hangzhou C-SKY Microsystems Co., Ltd
"^csq,.*":
description: Shenzen Chuangsiqi Technology Co.,Ltd.
+ "^ctu,.*":
+ description: Czech Technical University in Prague
"^cubietech,.*":
description: Cubietech, Ltd.
"^cypress,.*":
--
2.20.1

2020-10-30 22:34:10

by Pavel Pisa

[permalink] [raw]
Subject: [PATCH v7 6/6] docs: ctucanfd: CTU CAN FD open-source IP core documentation.

CTU CAN FD IP core documentation based on Martin Jeřábek's diploma theses
Open-source and Open-hardware CAN FD Protocol Support
https://dspace.cvut.cz/handle/10467/80366
.

Signed-off-by: Pavel Pisa <[email protected]>
Signed-off-by: Martin Jerabek <[email protected]>
Signed-off-by: Ondrej Ille <[email protected]>
---
.../device_drivers/ctu/ctucanfd-driver.rst | 638 ++++++++++++++++++
.../ctu/fsm_txt_buffer_user.svg | 151 +++++
2 files changed, 789 insertions(+)
create mode 100644 Documentation/networking/device_drivers/ctu/ctucanfd-driver.rst
create mode 100644 Documentation/networking/device_drivers/ctu/fsm_txt_buffer_user.svg

diff --git a/Documentation/networking/device_drivers/ctu/ctucanfd-driver.rst b/Documentation/networking/device_drivers/ctu/ctucanfd-driver.rst
new file mode 100644
index 000000000000..da5b0002e358
--- /dev/null
+++ b/Documentation/networking/device_drivers/ctu/ctucanfd-driver.rst
@@ -0,0 +1,638 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+CTU CAN FD Driver
+=================
+
+Author: Martin Jerabek <[email protected]>
+
+
+About CTU CAN FD IP Core
+------------------------
+
+`CTU CAN FD <https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core>`_
+is an open source soft core written in VHDL.
+It originated in 2015 as Ondrej Ille's project
+at the `Department of Measurement <https://meas.fel.cvut.cz/>`_
+of `FEE <http://www.fel.cvut.cz/en/>`_ at `CTU <http://www.fel.cvut.cz/en/>`_.
+
+The SocketCAN driver for Xilinx Zynq SoC based MicroZed board
+`Vivado integration <https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top>`_
+and Intel Cyclone V 5CSEMA4U23C6 based DE0-Nano-SoC Terasic board
+`QSys integration <https://gitlab.fel.cvut.cz/canbus/intel-soc-ctucanfd>`_
+has been developed as well as support for
+`PCIe integration <https://gitlab.fel.cvut.cz/canbus/pcie-ctucanfd>`_ of the core.
+
+In the case of Zynq, the core is connected via the APB system bus, which does
+not have enumeration support, and the device must be specified in Device Tree.
+This kind of devices is called platform device in the kernel and is
+handled by a platform device driver.
+
+The basic functional model of the CTU CAN FD peripheral has been
+accepted into QEMU mainline. See QEMU `CAN emulation support <https://git.qemu.org/?p=qemu.git;a=blob;f=docs/can.txt>`_
+for CAN FD buses, host connection and CTU CAN FD core emulation. The development
+version of emulation support can be cloned from ctu-canfd branch of QEMU local
+development `repository <https://gitlab.fel.cvut.cz/canbus/qemu-canbus>`_.
+
+
+About SocketCAN
+---------------
+
+SocketCAN is a standard common interface for CAN devices in the Linux
+kernel. As the name suggests, the bus is accessed via sockets, similarly
+to common network devices. The reasoning behind this is in depth
+described in `Linux SocketCAN <https://www.kernel.org/doc/html/latest/networking/can.html>`_.
+In short, it offers a
+natural way to implement and work with higher layer protocols over CAN,
+in the same way as, e.g., UDP/IP over Ethernet.
+
+Device probe
+~~~~~~~~~~~~
+
+Before going into detail about the structure of a CAN bus device driver,
+let's reiterate how the kernel gets to know about the device at all.
+Some buses, like PCI or PCIe, support device enumeration. That is, when
+the system boots, it discovers all the devices on the bus and reads
+their configuration. The kernel identifies the device via its vendor ID
+and device ID, and if there is a driver registered for this identifier
+combination, its probe method is invoked to populate the driver's
+instance for the given hardware. A similar situation goes with USB, only
+it allows for device hot-plug.
+
+The situation is different for peripherals which are directly embedded
+in the SoC and connected to an internal system bus (AXI, APB, Avalon,
+and others). These buses do not support enumeration, and thus the kernel
+has to learn about the devices from elsewhere. This is exactly what the
+Device Tree was made for.
+
+Device tree
+~~~~~~~~~~~
+
+An entry in device tree states that a device exists in the system, how
+it is reachable (on which bus it resides) and its configuration –
+registers address, interrupts and so on. An example of such a device
+tree is given in .
+
+.. code:: raw
+
+ / {
+ /* ... */
+ amba: amba {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "simple-bus";
+
+ CTU_CAN_FD_0: CTU_CAN_FD@43c30000 {
+ compatible = "ctu,ctucanfd";
+ interrupt-parent = <&intc>;
+ interrupts = <0 30 4>;
+ clocks = <&clkc 15>;
+ reg = <0x43c30000 0x10000>;
+ };
+ };
+ };
+
+
+.. _sec:socketcan:drv:
+
+Driver structure
+~~~~~~~~~~~~~~~~
+
+The driver can be divided into two parts – platform-dependent device
+discovery and set up, and platform-independent CAN network device
+implementation.
+
+.. _sec:socketcan:platdev:
+
+Platform device driver
+^^^^^^^^^^^^^^^^^^^^^^
+
+In the case of Zynq, the core is connected via the AXI system bus, which
+does not have enumeration support, and the device must be specified in
+Device Tree. This kind of devices is called *platform device* in the
+kernel and is handled by a *platform device driver*\ [1]_.
+
+A platform device driver provides the following things:
+
+- A *probe* function
+
+- A *remove* function
+
+- A table of *compatible* devices that the driver can handle
+
+The *probe* function is called exactly once when the device appears (or
+the driver is loaded, whichever happens later). If there are more
+devices handled by the same driver, the *probe* function is called for
+each one of them. Its role is to allocate and initialize resources
+required for handling the device, as well as set up low-level functions
+for the platform-independent layer, e.g., *read_reg* and *write_reg*.
+After that, the driver registers the device to a higher layer, in our
+case as a *network device*.
+
+The *remove* function is called when the device disappears, or the
+driver is about to be unloaded. It serves to free the resources
+allocated in *probe* and to unregister the device from higher layers.
+
+Finally, the table of *compatible* devices states which devices the
+driver can handle. The Device Tree entry ``compatible`` is matched
+against the tables of all *platform drivers*.
+
+.. code:: c
+
+ /* Match table for OF platform binding */
+ static const struct of_device_id ctucan_of_match[] = {
+ { .compatible = "ctu,canfd-2", },
+ { .compatible = "ctu,ctucanfd", },
+ { /* end of list */ },
+ };
+ MODULE_DEVICE_TABLE(of, ctucan_of_match);
+
+ static int ctucan_probe(struct platform_device *pdev);
+ static int ctucan_remove(struct platform_device *pdev);
+
+ static struct platform_driver ctucanfd_driver = {
+ .probe = ctucan_probe,
+ .remove = ctucan_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ .of_match_table = ctucan_of_match,
+ },
+ };
+ module_platform_driver(ctucanfd_driver);
+
+
+.. _sec:socketcan:netdev:
+
+Network device driver
+^^^^^^^^^^^^^^^^^^^^^
+
+Each network device must support at least these operations:
+
+- Bring the device up: ``ndo_open``
+
+- Bring the device down: ``ndo_close``
+
+- Submit TX frames to the device: ``ndo_start_xmit``
+
+- Signal TX completion and errors to the network subsystem: ISR
+
+- Submit RX frames to the network subsystem: ISR and NAPI
+
+There are two possible event sources: the device and the network
+subsystem. Device events are usually signaled via an interrupt, handled
+in an Interrupt Service Routine (ISR). Handlers for the events
+originating in the network subsystem are then specified in
+``struct net_device_ops``.
+
+When the device is brought up, e.g., by calling ``ip link set can0 up``,
+the driver’s function ``ndo_open`` is called. It should validate the
+interface configuration and configure and enable the device. The
+analogous opposite is ``ndo_close``, called when the device is being
+brought down, be it explicitly or implicitly.
+
+When the system should transmit a frame, it does so by calling
+``ndo_start_xmit``, which enqueues the frame into the device. If the
+device HW queue (FIFO, mailboxes or whatever the implementation is)
+becomes full, the ``ndo_start_xmit`` implementation informs the network
+subsystem that it should stop the TX queue (via ``netif_stop_queue``).
+It is then re-enabled later in ISR when the device has some space
+available again and is able to enqueue another frame.
+
+All the device events are handled in ISR, namely:
+
+#. **TX completion**. When the device successfully finishes transmitting
+ a frame, the frame is echoed locally. On error, an informative error
+ frame [2]_ is sent to the network subsystem instead. In both cases,
+ the software TX queue is resumed so that more frames may be sent.
+
+#. **Error condition**. If something goes wrong (e.g., the device goes
+ bus-off or RX overrun happens), error counters are updated, and
+ informative error frames are enqueued to SW RX queue.
+
+#. **RX buffer not empty**. In this case, read the RX frames and enqueue
+ them to SW RX queue. Usually NAPI is used as a middle layer (see ).
+
+.. _sec:socketcan:napi:
+
+NAPI
+~~~~
+
+The frequency of incoming frames can be high and the overhead to invoke
+the interrupt service routine for each frame can cause significant
+system load. There are multiple mechanisms in the Linux kernel to deal
+with this situation. They evolved over the years of Linux kernel
+development and enhancements. For network devices, the current standard
+is NAPI – *the New API*. It is similar to classical top-half/bottom-half
+interrupt handling in that it only acknowledges the interrupt in the ISR
+and signals that the rest of the processing should be done in softirq
+context. On top of that, it offers the possibility to *poll* for new
+frames for a while. This has a potential to avoid the costly round of
+enabling interrupts, handling an incoming IRQ in ISR, re-enabling the
+softirq and switching context back to softirq.
+
+More detailed documentation of NAPI may be found on the pages of Linux
+Foundation `<https://wiki.linuxfoundation.org/networking/napi>`_.
+
+Integrating the core to Xilinx Zynq
+-----------------------------------
+
+The core interfaces a simple subset of the Avalon
+`Avalon Interface Specifications <https://www.intel.com/content/dam/www/programmable/us/en/pdfs/literature/manual/mnl_avalon_spec.pdf>`_
+bus as it was originally used on
+Alterra FPGA chips, yet Xilinx natively interfaces with AXI
+`AMBA AXI and ACE Protocol Specification AXI3, AXI4, and AXI4-Lite, ACE and ACE-Lite <https://static.docs.arm.com/ihi0022/d/IHI0022D_amba_axi_protocol_spec.pdf>`_.
+The most obvious solution would be to use
+an Avalon/AXI bridge or implement some simple conversion entity.
+However, the core’s interface is half-duplex with no handshake
+signaling, whereas AXI is full duplex with two-way signaling. Moreover,
+even AXI-Lite slave interface is quite resource-intensive, and the
+flexibility and speed of AXI are not required for a CAN core.
+
+Thus a much simpler bus was chosen – APB (Advanced Peripheral Bus)
+`AMBA APB Protocol Specification v2.0 <https://static.docs.arm.com/ihi0024/c/IHI0024C_amba_apb_protocol_spec.pdf>`_.
+APB-AXI bridge is directly available in
+Xilinx Vivado, and the interface adaptor entity is just a few simple
+combinatorial assignments.
+
+Finally, to be able to include the core in a block diagram as a custom
+IP, the core, together with the APB interface, has been packaged as a
+Vivado component.
+
+CTU CAN FD Driver design
+------------------------
+
+The general structure of a CAN device driver has already been examined
+in . The next paragraphs provide a more detailed description of the CTU
+CAN FD core driver in particular.
+
+Low-level driver
+~~~~~~~~~~~~~~~~
+
+The core is not intended to be used solely with SocketCAN, and thus it
+is desirable to have an OS-independent low-level driver. This low-level
+driver can then be used in implementations of OS driver or directly
+either on bare metal or in a user-space application. Another advantage
+is that if the hardware slightly changes, only the low-level driver
+needs to be modified.
+
+The code [3]_ is in part automatically generated and in part written
+manually by the core author, with contributions of the thesis’ author.
+The low-level driver supports operations such as: set bit timing, set
+controller mode, enable/disable, read RX frame, write TX frame, and so
+on.
+
+Configuring bit timing
+~~~~~~~~~~~~~~~~~~~~~~
+
+On CAN, each bit is divided into four segments: SYNC, PROP, PHASE1, and
+PHASE2. Their duration is expressed in multiples of a Time Quantum
+(details in `CAN Specification, Version 2.0 <http://esd.cs.ucr.edu/webres/can20.pdf>`_, chapter 8).
+When configuring
+bitrate, the durations of all the segments (and time quantum) must be
+computed from the bitrate and Sample Point. This is performed
+independently for both the Nominal bitrate and Data bitrate for CAN FD.
+
+SocketCAN is fairly flexible and offers either highly customized
+configuration by setting all the segment durations manually, or a
+convenient configuration by setting just the bitrate and sample point
+(and even that is chosen automatically per Bosch recommendation if not
+specified). However, each CAN controller may have different base clock
+frequency and different width of segment duration registers. The
+algorithm thus needs the minimum and maximum values for the durations
+(and clock prescaler) and tries to optimize the numbers to fit both the
+constraints and the requested parameters.
+
+.. code:: c
+
+ struct can_bittiming_const {
+ char name[16]; /* Name of the CAN controller hardware */
+ __u32 tseg1_min; /* Time segment 1 = prop_seg + phase_seg1 */
+ __u32 tseg1_max;
+ __u32 tseg2_min; /* Time segment 2 = phase_seg2 */
+ __u32 tseg2_max;
+ __u32 sjw_max; /* Synchronisation jump width */
+ __u32 brp_min; /* Bit-rate prescaler */
+ __u32 brp_max;
+ __u32 brp_inc;
+ };
+
+
+[lst:can_bittiming_const]
+
+A curious reader will notice that the durations of the segments PROP_SEG
+and PHASE_SEG1 are not determined separately but rather combined and
+then, by default, the resulting TSEG1 is evenly divided between PROP_SEG
+and PHASE_SEG1. In practice, this has virtually no consequences as the
+sample point is between PHASE_SEG1 and PHASE_SEG2. In CTU CAN FD,
+however, the duration registers ``PROP`` and ``PH1`` have different
+widths (6 and 7 bits, respectively), so the auto-computed values might
+overflow the shorter register and must thus be redistributed among the
+two [4]_.
+
+Handling RX
+~~~~~~~~~~~
+
+Frame reception is handled in NAPI queue, which is enabled from ISR when
+the RXNE (RX FIFO Not Empty) bit is set. Frames are read one by one
+until either no frame is left in the RX FIFO or the maximum work quota
+has been reached for the NAPI poll run (see ). Each frame is then passed
+to the network interface RX queue.
+
+An incoming frame may be either a CAN 2.0 frame or a CAN FD frame. The
+way to distinguish between these two in the kernel is to allocate either
+``struct can_frame`` or ``struct canfd_frame``, the two having different
+sizes. In the controller, the information about the frame type is stored
+in the first word of RX FIFO.
+
+This brings us a chicken-egg problem: we want to allocate the ``skb``
+for the frame, and only if it succeeds, fetch the frame from FIFO;
+otherwise keep it there for later. But to be able to allocate the
+correct ``skb``, we have to fetch the first work of FIFO. There are
+several possible solutions:
+
+#. Read the word, then allocate. If it fails, discard the rest of the
+ frame. When the system is low on memory, the situation is bad anyway.
+
+#. Always allocate ``skb`` big enough for an FD frame beforehand. Then
+ tweak the ``skb`` internals to look like it has been allocated for
+ the smaller CAN 2.0 frame.
+
+#. Add option to peek into the FIFO instead of consuming the word.
+
+#. If the allocation fails, store the read word into driver’s data. On
+ the next try, use the stored word instead of reading it again.
+
+Option 1 is simple enough, but not very satisfying if we could do
+better. Option 2 is not acceptable, as it would require modifying the
+private state of an integral kernel structure. The slightly higher
+memory consumption is just a virtual cherry on top of the “cake”. Option
+3 requires non-trivial HW changes and is not ideal from the HW point of
+view.
+
+Option 4 seems like a good compromise, with its disadvantage being that
+a partial frame may stay in the FIFO for a prolonged time. Nonetheless,
+there may be just one owner of the RX FIFO, and thus no one else should
+see the partial frame (disregarding some exotic debugging scenarios).
+Basides, the driver resets the core on its initialization, so the
+partial frame cannot be “adopted” either. In the end, option 4 was
+selected [5]_.
+
+.. _subsec:ctucanfd:rxtimestamp:
+
+Timestamping RX frames
+^^^^^^^^^^^^^^^^^^^^^^
+
+The CTU CAN FD core reports the exact timestamp when the frame has been
+received. The timestamp is by default captured at the sample point of
+the last bit of EOF but is configurable to be captured at the SOF bit.
+The timestamp source is external to the core and may be up to 64 bits
+wide. At the time of writing, passing the timestamp from kernel to
+userspace is not yet implemented, but is planned in the future.
+
+Handling TX
+~~~~~~~~~~~
+
+The CTU CAN FD core has 4 independent TX buffers, each with its own
+state and priority. When the core wants to transmit, a TX buffer in
+Ready state with the highest priority is selected.
+
+The priorities are 3bit numbers in register TX_PRIORITY
+(nibble-aligned). This should be flexible enough for most use cases.
+SocketCAN, however, supports only one FIFO queue for outgoing
+frames [6]_. The buffer priorities may be used to simulate the FIFO
+behavior by assigning each buffer a distinct priority and *rotating* the
+priorities after a frame transmission is completed.
+
+In addition to priority rotation, the SW must maintain head and tail
+pointers into the FIFO formed by the TX buffers to be able to determine
+which buffer should be used for next frame (``txb_head``) and which
+should be the first completed one (``txb_tail``). The actual buffer
+indices are (obviously) modulo 4 (number of TX buffers), but the
+pointers must be at least one bit wider to be able to distinguish
+between FIFO full and FIFO empty – in this situation,
+:math:`txb\_head \equiv txb\_tail\ (\textrm{mod}\ 4)`. An example of how
+the FIFO is maintained, together with priority rotation, is depicted in
+
+|
+
++------+---+---+---+---+
+| TXB# | 0 | 1 | 2 | 3 |
++======+===+===+===+===+
+| Seq | A | B | C | |
++------+---+---+---+---+
+| Prio | 7 | 6 | 5 | 4 |
++------+---+---+---+---+
+| | | T | | H |
++------+---+---+---+---+
+
+|
+
++------+---+---+---+---+
+| TXB# | 0 | 1 | 2 | 3 |
++======+===+===+===+===+
+| Seq | | B | C | |
++------+---+---+---+---+
+| Prio | 4 | 7 | 6 | 5 |
++------+---+---+---+---+
+| | | T | | H |
++------+---+---+---+---+
+
+|
+
++------+---+---+---+---+----+
+| TXB# | 0 | 1 | 2 | 3 | 0’ |
++======+===+===+===+===+====+
+| Seq | E | B | C | D | |
++------+---+---+---+---+----+
+| Prio | 4 | 7 | 6 | 5 | |
++------+---+---+---+---+----+
+| | | T | | | H |
++------+---+---+---+---+----+
+
+|
+
+.. figure:: fsm_txt_buffer_user.svg
+
+ TX Buffer states with possible transitions
+
+.. _subsec:ctucanfd:txtimestamp:
+
+Timestamping TX frames
+^^^^^^^^^^^^^^^^^^^^^^
+
+When submitting a frame to a TX buffer, one may specify the timestamp at
+which the frame should be transmitted. The frame transmission may start
+later, but not sooner. Note that the timestamp does not participate in
+buffer prioritization – that is decided solely by the mechanism
+described above.
+
+Support for time-based packet transmission was recently merged to Linux
+v4.19 `Time-based packet transmission <https://lwn.net/Articles/748879/>`_,
+but it remains yet to be researched
+whether this functionality will be practical for CAN.
+
+Also similarly to retrieving the timestamp of RX frames, the core
+supports retrieving the timestamp of TX frames – that is the time when
+the frame was successfully delivered. The particulars are very similar
+to timestamping RX frames and are described in .
+
+Handling RX buffer overrun
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+When a received frame does no more fit into the hardware RX FIFO in its
+entirety, RX FIFO overrun flag (STATUS[DOR]) is set and Data Overrun
+Interrupt (DOI) is triggered. When servicing the interrupt, care must be
+taken first to clear the DOR flag (via COMMAND[CDO]) and after that
+clear the DOI interrupt flag. Otherwise, the interrupt would be
+immediately [7]_ rearmed.
+
+**Note**: During development, it was discussed whether the internal HW
+pipelining cannot disrupt this clear sequence and whether an additional
+dummy cycle is necessary between clearing the flag and the interrupt. On
+the Avalon interface, it indeed proved to be the case, but APB being
+safe because it uses 2-cycle transactions. Essentially, the DOR flag
+would be cleared, but DOI register’s Preset input would still be high
+the cycle when the DOI clear request would also be applied (by setting
+the register’s Reset input high). As Set had higher priority than Reset,
+the DOI flag would not be reset. This has been already fixed by swapping
+the Set/Reset priority (see issue #187).
+
+Reporting Error Passive and Bus Off conditions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+It may be desirable to report when the node reaches *Error Passive*,
+*Error Warning*, and *Bus Off* conditions. The driver is notified about
+error state change by an interrupt (EPI, EWLI), and then proceeds to
+determine the core’s error state by reading its error counters.
+
+There is, however, a slight race condition here – there is a delay
+between the time when the state transition occurs (and the interrupt is
+triggered) and when the error counters are read. When EPI is received,
+the node may be either *Error Passive* or *Bus Off*. If the node goes
+*Bus Off*, it obviously remains in the state until it is reset.
+Otherwise, the node is *or was* *Error Passive*. However, it may happen
+that the read state is *Error Warning* or even *Error Active*. It may be
+unclear whether and what exactly to report in that case, but I
+personally entertain the idea that the past error condition should still
+be reported. Similarly, when EWLI is received but the state is later
+detected to be *Error Passive*, *Error Passive* should be reported.
+
+
+CTU CAN FD Driver Sources Reference
+-----------------------------------
+
+.. kernel-doc:: drivers/net/can/ctucanfd/ctucanfd_hw.h
+ :internal:
+
+.. kernel-doc:: drivers/net/can/ctucanfd/ctucanfd_base.c
+ :internal:
+
+.. kernel-doc:: drivers/net/can/ctucanfd/ctucanfd_pci.c
+ :internal:
+
+.. kernel-doc:: drivers/net/can/ctucanfd/ctucanfd_platform.c
+ :internal:
+
+CTU CAN FD IP Core and Driver Development Acknowledgment
+---------------------------------------------------------
+
+* Odrej Ille <[email protected]>
+
+ * started the project as student at Department of Measurement, FEE, CTU
+ * invested great amount of personal time and enthusiasm to the project over years
+ * worked on more funded tasks
+
+* `Department of Measurement <https://meas.fel.cvut.cz/>`_,
+ `Faculty of Electrical Engineering <http://www.fel.cvut.cz/en/>`_,
+ `Czech Technical University <https://www.cvut.cz/en>`_
+
+ * is the main investor into the project over many years
+ * uses project in their CAN/CAN FD diagnostics framework for `Skoda Auto <https://www.skoda-auto.cz/>`_
+
+* `Digiteq Automotive <https://www.digiteqautomotive.com/en>`_
+
+ * funding of the project CAN FD Open Cores Support Linux Kernel Based Systems
+ * negotiated and paid CTU to allow public access to the project
+ * provided additional funding of the work
+
+* `Department of Control Engineering <https://dce.fel.cvut.cz/en>`_,
+ `Faculty of Electrical Engineering <http://www.fel.cvut.cz/en/>`_,
+ `Czech Technical University <https://www.cvut.cz/en>`_
+
+ * solving the project CAN FD Open Cores Support Linux Kernel Based Systems
+ * providing GitLab management
+ * virtual servers and computational power for continuous integration
+ * providing hardware for HIL continuous integration tests
+
+* `PiKRON Ltd. <http://pikron.com/>`_
+
+ * minor funding to initiate preparation of the project open-sourcing
+
+* Petr Porazil <[email protected]>
+
+ * design of PCIe transceiver addon board and assembly of boards
+ * design and assembly of MZ_APO baseboard for MicroZed/Zynq based system
+
+* Martin Jerabek <[email protected]>
+
+ * Linux driver development
+ * continuous integration platform architect and GHDL updates
+ * theses `Open-source and Open-hardware CAN FD Protocol Support <https://dspace.cvut.cz/bitstream/handle/10467/80366/F3-DP-2019-Jerabek-Martin-Jerabek-thesis-2019-canfd.pdf>`_
+
+* Jiri Novak <[email protected]>
+
+ * project initiation, management and use at Department of Measurement, FEE, CTU
+
+* Pavel Pisa <[email protected]>
+
+ * initiate open-sourcing, project coordination, management at Department of Control Engineering, FEE, CTU
+
+* Jaroslav Beran<[email protected]>
+
+ * system integration for Intel SoC, core and driver testing and updates
+
+* Carsten Emde (`OSADL <https://www.osadl.org/>`_)
+
+ * provided OSADL expertise to discuss IP core licensing
+ * pointed to possible deadlock for LGPL and CAN bus possible patent case which lead to relicense IP core design to BSD like license
+
+* Reiner Zitzmann and Holger Zeltwanger (`CAN in Automation <https://www.can-cia.org/>`_)
+
+ * provided suggestions and help to inform community about the project and invited us to events focused on CAN bus future development directions
+
+* Jan Charvat
+
+ * implemented CTU CAN FD functional model for QEMU which has been integrated into QEMU mainline (`docs/can.txt <https://git.qemu.org/?p=qemu.git;a=blob;f=docs/can.txt>`_)
+ * Bachelor theses Model of CAN FD Communication Controller for QEMU Emulator
+
+Notes
+-----
+
+
+.. [1]
+ Other buses have their own specific driver interface to set up the
+ device.
+
+.. [2]
+ Not to be mistaken with CAN Error Frame. This is a ``can_frame`` with
+ ``CAN_ERR_FLAG`` set and some error info in its ``data`` field.
+
+.. [3]
+ Available in in CTU CAN FD repository
+ `<https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core>`_
+
+.. [4]
+ As is done in the low-level driver functions
+ ``ctucan_hw_set_nom_bittiming`` and
+ ``ctucan_hw_set_data_bittiming``.
+
+.. [5]
+ At the time of writing this thesis, option 1 is still being used and
+ the modification is queued in gitlab issue #222
+
+.. [6]
+ Strictly speaking, multiple CAN TX queues are supported since v4.19
+ `can: enable multi-queue for SocketCAN devices <https://lore.kernel.org/patchwork/patch/913526/>`_ but no mainline driver is using
+ them yet.
+
+.. [7]
+ Or rather in the next clock cycle
diff --git a/Documentation/networking/device_drivers/ctu/fsm_txt_buffer_user.svg b/Documentation/networking/device_drivers/ctu/fsm_txt_buffer_user.svg
new file mode 100644
index 000000000000..b371650788f4
--- /dev/null
+++ b/Documentation/networking/device_drivers/ctu/fsm_txt_buffer_user.svg
@@ -0,0 +1,151 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg width="113.611mm" height="86.6873mm" version="1.1" viewBox="0 0 113.611 86.6873" xmlns="http://www.w3.org/2000/svg" xmlns:cc="http://creativecommons.org/ns#" xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
+ <defs>
+ <marker id="marker3667" overflow="visible" orient="auto">
+ <path transform="scale(-.6)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill="#28a4ff" fill-rule="evenodd" stroke="#28a4ff" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="marker3517" overflow="visible" orient="auto">
+ <path transform="scale(-.6)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill-rule="evenodd" stroke="#000" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="marker3373" overflow="visible" orient="auto">
+ <path transform="scale(-.6)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill-rule="evenodd" stroke="#000" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="marker3199" overflow="visible" orient="auto">
+ <path transform="scale(-.6)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill="#28a4ff" fill-rule="evenodd" stroke="#28a4ff" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="marker3037" overflow="visible" orient="auto">
+ <path transform="scale(-.6)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill="#28a4ff" fill-rule="evenodd" stroke="#28a4ff" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="marker2779" overflow="visible" orient="auto">
+ <path transform="scale(-.6)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill="#28a4ff" fill-rule="evenodd" stroke="#28a4ff" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="marker2477" overflow="visible" orient="auto">
+ <path transform="scale(.6) rotate(180) translate(0)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill="#28a4ff" fill-rule="evenodd" stroke="#28a4ff" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="marker2074" overflow="visible" orient="auto">
+ <path transform="scale(.6) rotate(180) translate(0)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill-rule="evenodd" stroke="#000" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="marker1964" overflow="visible" orient="auto">
+ <path transform="scale(.6) rotate(180) translate(0)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill-rule="evenodd" stroke="#000" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="marker1856" overflow="visible" orient="auto">
+ <path transform="scale(.6) rotate(180) translate(0)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill-rule="evenodd" stroke="#000" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <marker id="Arrow2Mend" overflow="visible" orient="auto">
+ <path transform="scale(.6) rotate(180) translate(0)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill-rule="evenodd" stroke="#000" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <filter id="filter1204" x="-4.19953e-6" y="-5.60084e-6" width="1.00001" height="1.00001" color-interpolation-filters="sRGB">
+ <feGaussianBlur stdDeviation="0.00018829868"/>
+ </filter>
+ <marker id="marker2074-3" overflow="visible" orient="auto">
+ <path transform="scale(-.6)" d="m8.71859 4.03374-10.9259-4.01772 10.9259-4.01772c-1.7455 2.37206-1.73544 5.61745-6e-7 8.03544z" fill="#28a4ff" fill-rule="evenodd" stroke="#28a4ff" stroke-linejoin="round" stroke-width=".625"/>
+ </marker>
+ <filter id="filter1204-6" x="-4.19953e-6" y="-5.60084e-6" width="1.00001" height="1.00001" color-interpolation-filters="sRGB">
+ <feGaussianBlur stdDeviation="0.00018829868"/>
+ </filter>
+ <filter id="filter1204-6-9" x="-4.19953e-6" y="-5.60084e-6" width="1.00001" height="1.00001" color-interpolation-filters="sRGB">
+ <feGaussianBlur stdDeviation="0.00018829868"/>
+ </filter>
+ <filter id="filter1204-6-2" x="-4.19953e-6" y="-5.60084e-6" width="1.00001" height="1.00001" color-interpolation-filters="sRGB">
+ <feGaussianBlur stdDeviation="0.00018829868"/>
+ </filter>
+ <filter id="filter1204-6-2-9" x="-4.19953e-6" y="-5.60084e-6" width="1.00001" height="1.00001" color-interpolation-filters="sRGB">
+ <feGaussianBlur stdDeviation="0.00018829868"/>
+ </filter>
+ <filter id="filter1204-6-2-9-4" x="-4.19953e-6" y="-5.60084e-6" width="1.00001" height="1.00001" color-interpolation-filters="sRGB">
+ <feGaussianBlur stdDeviation="0.00018829868"/>
+ </filter>
+ <filter id="filter1204-6-2-9-1" x="-4.19953e-6" y="-5.60084e-6" width="1.00001" height="1.00001" color-interpolation-filters="sRGB">
+ <feGaussianBlur stdDeviation="0.00018829868"/>
+ </filter>
+ <filter id="filter1204-6-2-9-1-3" x="-4.19953e-6" y="-5.60084e-6" width="1.00001" height="1.00001" color-interpolation-filters="sRGB">
+ <feGaussianBlur stdDeviation="0.00018829868"/>
+ </filter>
+ <filter id="filter1204-6-2-9-1-3-1" x="-4.19953e-6" y="-5.60084e-6" width="1.00001" height="1.00001" color-interpolation-filters="sRGB">
+ <feGaussianBlur stdDeviation="0.00018829868"/>
+ </filter>
+ </defs>
+ <metadata>
+ <rdf:RDF>
+ <cc:Work rdf:about="">
+ <dc:format>image/svg+xml</dc:format>
+ <dc:type rdf:resource="http://purl.org/dc/dcmitype/StillImage"/>
+ <dc:title/>
+ </cc:Work>
+ </rdf:RDF>
+ </metadata>
+ <g transform="translate(-49.0277 -104.823)">
+ <g>
+ <path d="m130.534 165.429h-71.1816v-17.5315" fill="none" marker-end="url(#marker2477)" stroke="#28a4ff" stroke-width=".6"/>
+ <path d="m145.034 122.959v-11.5914h-43.1215" fill="none" marker-end="url(#marker3037)" stroke="#28a4ff" stroke-width=".6"/>
+ <rect x="130.679" y="122.933" width="28.2965" height="45.2319" rx="0" ry="0" fill="#e5e5e5" stroke="#717171" stroke-linecap="square" stroke-width=".499999"/>
+ <path d="m102.044 116.236h23.3126l-0.13388 18.8185h19.9383v3.66603" fill="none" marker-end="url(#marker3199)" stroke="#28a4ff" stroke-width=".6"/>
+ <path d="m59.5006 138.391v-24.2517h20.6338" fill="none" marker-end="url(#marker2779)" stroke="#28a4ff" stroke-width=".6"/>
+ <rect x="78.1389" y="126.411" width="28.0037" height="35.0443" rx="0" ry="0" fill="#e5e5e5" stroke="#717171" stroke-linecap="square" stroke-width=".5"/>
+ </g>
+ <g fill="#ffcb35" stroke="#000" stroke-linecap="square">
+ <ellipse cx="92.1408" cy="114.239" rx="10.8866" ry="4.39308" stroke-width=".5"/>
+ <ellipse cx="92.1408" cy="134.185" rx="10.8866" ry="4.39308" stroke-width=".499999"/>
+ <ellipse cx="92.1408" cy="152.199" rx="10.8866" ry="4.39308" stroke-width=".499999"/>
+ </g>
+ <g fill="#28a4ff" stroke="#000" stroke-linecap="square" stroke-width=".499999">
+ <ellipse cx="144.827" cy="143.316" rx="10.8866" ry="4.39308"/>
+ <ellipse cx="144.827" cy="159.143" rx="10.8866" ry="4.39308"/>
+ <ellipse cx="59.4364" cy="142.823" rx="7.36455" ry="4.39308"/>
+ <ellipse cx="144.827" cy="129.196" rx="10.8866" ry="4.39308"/>
+ <ellipse cx="143.077" cy="180.53" rx="10.8866" ry="4.39308"/>
+ </g>
+ <ellipse cx="110.386" cy="180.53" rx="10.8866" ry="4.39308" fill="#ffcb35" stroke="#000" stroke-linecap="square" stroke-width=".499999"/>
+ <text x="110.90907" y="179.42688" font-size="3.175px" xml:space="preserve"><tspan x="110.90907" y="179.42688" dy="0.60000002" text-align="center" text-anchor="middle">Accessible</tspan><tspan x="110.90907" y="183.39563"><tspan font-size="3.175px" text-align="center" text-anchor="middle">for S</tspan>W</tspan></text>
+ <text x="143.5869" y="179.52795" xml:space="preserve"><tspan x="143.5869" y="179.52795" dy="1 0 0 0 0 0" font-family="sans-serif" font-size="2.82222px" text-align="center" text-anchor="middle" style="font-variant-caps:normal;font-variant-east-asian:normal;font-variant-ligatures:normal;font-variant-numeric:normal">Inaccessible</tspan><tspan x="143.5869" y="183.36786" font-size="3.175px"><tspan font-size="3.175px" text-align="center" text-anchor="middle">for S</tspan>W</tspan></text>
+ <g font-size="3.175px">
+ <text x="91.95018" y="115.29005" xml:space="preserve"><tspan x="91.95018" y="115.29005" font-size="3.175px"><tspan font-size="3.175px" text-align="center" text-anchor="middle">Ready</tspan></tspan></text>
+ <text x="145.25127" y="130.49019" xml:space="preserve"><tspan x="145.25127" y="130.49019" font-size="3.175px"><tspan font-size="3.175px" text-align="center" text-anchor="middle">TX OK</tspan></tspan></text>
+ <text x="145.31845" y="144.43121" xml:space="preserve"><tspan x="145.31845" y="144.43121" font-size="3.175px"><tspan font-size="3.175px" text-align="center" text-anchor="middle">Aborted</tspan></tspan></text>
+ <text x="145.40399" y="160.36035" xml:space="preserve"><tspan x="145.40399" y="160.36035" font-size="3.175px"><tspan font-size="3.175px" text-align="center" text-anchor="middle">TX failed</tspan></tspan></text>
+ <text x="91.823967" y="133.53941" text-align="center" text-anchor="middle" style="line-height:0.9" xml:space="preserve"><tspan x="91.823967" y="133.53941" text-align="center"><tspan font-size="3.175px" text-align="center" text-anchor="middle">TX in</tspan></tspan><tspan x="91.823967" y="136.39691" text-align="center">progress</tspan></text>
+ <text x="91.648918" y="151.84813" text-align="center" text-anchor="middle" style="line-height:0.9" xml:space="preserve"><tspan x="91.648918" y="151.84813" text-align="center"><tspan font-size="3.175px" text-align="center" text-anchor="middle">Abort in</tspan></tspan><tspan x="91.648918" y="154.70563" text-align="center">progress</tspan></text>
+ <text x="59.456043" y="143.91658" xml:space="preserve"><tspan x="59.456043" y="143.91658" font-size="3.175px"><tspan font-size="3.175px" text-align="center" text-anchor="middle">Empty</tspan></tspan></text>
+ </g>
+ <g fill="none">
+ <g stroke="#000">
+ <rect x="52.3943" y="171.63" width="106.581" height="16.601" rx="0" ry="0" stroke-linecap="square" stroke-width=".499999"/>
+ <g stroke-width=".6">
+ <path d="m106.383 159.046h26.4967" marker-end="url(#Arrow2Mend)"/>
+ <path d="m103.138 152.268h41.5564v-3.92426" marker-end="url(#marker1856)"/>
+ <path d="m106.38 129.354h17.7785"/>
+ <path d="m125.818 129.359h7.2418" marker-end="url(#marker1964)"/>
+ </g>
+ <path d="m124.169 129.354a0.959514 0.97091 0 0 1 0.47587-0.84557 0.959514 0.97091 0 0 1 0.96164-3e-3 0.959514 0.97091 0 0 1 0.48149 0.84231" stroke-linecap="square" stroke-width=".600001"/>
+ <path d="m55.7026 180.832h34.8131" marker-end="url(#marker2074)" stroke-width=".6"/>
+ </g>
+ <g>
+ <path d="m55.6464 185.744h34.8131" marker-end="url(#marker2074-3)" stroke="#28a4ff" stroke-width=".600001"/>
+ <g stroke-width=".6">
+ <path d="m94.0487 129.889v-10.6493" marker-end="url(#marker3373)" stroke="#000"/>
+ <path d="m89.7534 118.621v10.662" marker-end="url(#marker3517)" stroke="#000"/>
+ <path d="m92.119 138.812v7.9718" marker-end="url(#marker3667)" stroke="#28a4ff"/>
+ </g>
+ </g>
+ </g>
+ <text transform="matrix(.264583 0 0 .264583 91.8919 139.964)" x="26.959213" y="9.11724" fill="#2aa1ff" filter="url(#filter1204-6-2-9-1-3-1)" font-size="12px" stroke-width="3.77953" text-align="center" text-anchor="middle" style="line-height:1.1" xml:space="preserve"><tspan x="26.959213" y="9.11724" text-align="center">Set</tspan><tspan x="26.959213" y="22.31724" text-align="center">abort</tspan></text>
+ <text transform="translate(49.0277 104.823)" x="57.620724" y="16.855087" filter="url(#filter1204)" font-size="3.175px" text-align="center" text-anchor="middle" style="line-height:1.1" xml:space="preserve"><tspan x="57.620724" y="16.855087" text-align="center">Transmission</tspan><tspan x="57.620724" y="20.347588" text-align="center">unsuccesfull</tspan></text>
+ <g font-size="12px" stroke-width="3.77953" text-anchor="middle">
+ <text transform="matrix(.264583 0 0 .264583 68.5988 118.913)" x="38.824219" y="9.1171875" filter="url(#filter1204)" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="38.824219" y="9.1171875" text-align="center">Transmission</tspan><tspan x="38.824219" y="22.317188" text-align="center">starts</tspan></text>
+ <text transform="matrix(.264583 0 0 .264583 106.802 130.509)" x="38.824219" y="9.1171875" filter="url(#filter1204)" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="38.824219" y="9.1171875" text-align="center">Transmission</tspan><tspan x="38.824219" y="22.317188" text-align="center">succesfull</tspan></text>
+ <text transform="matrix(.264583 0 0 .264583 107.77 145.476)" x="38.824219" y="9.1171875" filter="url(#filter1204)" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="38.824219" y="9.1171875" text-align="center">Transmission</tspan><tspan x="38.824219" y="22.317188" text-align="center">sborted</tspan></text>
+ </g>
+ <g stroke-width="3.77953" text-anchor="middle">
+ <text transform="matrix(.264583 0 0 .264583 107.574 155.948)" x="38.824219" y="9.1171875" filter="url(#filter1204)" font-size="10.6667px" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="38.824219" y="9.1171875" text-align="center">Retransmit</tspan><tspan x="38.824219" y="20.850557" text-align="center">limit reached or</tspan><tspan x="38.824219" y="32.583927" text-align="center">node went bus off</tspan><tspan x="38.824219" y="44.317299" text-align="center"/></text>
+ <text transform="matrix(.264583 0 0 .264583 60.7127 177.384)" x="38.824539" y="9.1173134" filter="url(#filter1204-6)" font-size="12px" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="38.824539" y="9.1173134" font-size="12px" stroke-width="3.77953" text-align="center" text-anchor="middle">Transmission result</tspan></text>
+ <text transform="matrix(.264583 0 0 .264583 45.6885 173.226)" x="57.727047" y="9.11724" filter="url(#filter1204-6-9)" font-size="12px" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="57.727047" y="9.11724" font-size="12px" stroke-width="3.77953" text-align="center" text-anchor="middle">Legend:</tspan></text>
+ </g>
+ <g fill="#2aa1ff" font-size="12px" stroke-width="3.77953" text-anchor="middle">
+ <text transform="matrix(.264583 0 0 .264583 57.0045 182.079)" x="57.727047" y="9.11724" filter="url(#filter1204-6-2)" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="57.727047" y="9.11724" fill="#2aa1ff" font-size="12px" stroke-width="3.77953" text-align="center" text-anchor="middle">SW command</tspan></text>
+ <text transform="matrix(.264583 0 0 .264583 57.7865 110.104)" x="40.822609" y="9.11724" filter="url(#filter1204-6-2-9)" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="40.822609" y="9.11724" fill="#2aa1ff" font-size="12px" stroke-width="3.77953" text-align="center" text-anchor="middle">Set ready</tspan></text>
+ <text transform="matrix(.264583 0 0 .264583 116.893 107.491)" x="28.049065" y="9.1172523" filter="url(#filter1204-6-2-9-4)" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="28.049065" y="9.1172523" fill="#2aa1ff" font-size="12px" stroke-width="3.77953" text-align="center" text-anchor="middle">Set ready</tspan></text>
+ <text transform="matrix(.264583 0 0 .264583 87.5687 166.324)" x="28.049065" y="9.1172523" filter="url(#filter1204-6-2-9-1)" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="28.049065" y="9.1172523" fill="#2aa1ff" font-size="12px" stroke-width="3.77953" text-align="center" text-anchor="middle">Set empty</tspan></text>
+ <text transform="matrix(.264583 0 0 .264583 106.53 113.074)" x="30.228771" y="8.9063139" filter="url(#filter1204-6-2-9-1-3)" text-align="center" style="line-height:1.1" xml:space="preserve"><tspan x="30.228771" y="8.9063139" fill="#2aa1ff" font-size="12px" stroke-width="3.77953" text-align="center" text-anchor="middle">Set abort</tspan></text>
+ </g>
+ </g>
+</svg>
--
2.20.1

2020-10-30 22:34:40

by Pavel Pisa

[permalink] [raw]
Subject: [PATCH v7 5/6] can: ctucanfd: CTU CAN FD open-source IP core - platform/SoC support.

Platform bus adaptation for CTU CAN FD open-source IP core.

The core has been tested together with OpenCores SJA1000
modified to be CAN FD frames tolerant on MicroZed Zynq based
MZ_APO education kits designed by Petr Porazil from PiKRON.com
company. FPGA design

https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top.

The kit description at the Computer Architectures course pages

https://cw.fel.cvut.cz/wiki/courses/b35apo/documentation/mz_apo/start .

Kit carrier board and mechanics design source files

https://gitlab.com/pikron/projects/mz_apo/microzed_apo

The work is documented in Martin Jeřábek's diploma theses
Open-source and Open-hardware CAN FD Protocol Support

https://dspace.cvut.cz/handle/10467/80366
.

Signed-off-by: Pavel Pisa <[email protected]>
Signed-off-by: Martin Jerabek <[email protected]>
Signed-off-by: Ondrej Ille <[email protected]>
---
drivers/net/can/ctucanfd/Kconfig | 12 ++
drivers/net/can/ctucanfd/Makefile | 1 +
drivers/net/can/ctucanfd/ctucanfd_platform.c | 142 +++++++++++++++++++
3 files changed, 155 insertions(+)
create mode 100644 drivers/net/can/ctucanfd/ctucanfd_platform.c

diff --git a/drivers/net/can/ctucanfd/Kconfig b/drivers/net/can/ctucanfd/Kconfig
index 039df460cf0c..20a9a1b5ae8d 100644
--- a/drivers/net/can/ctucanfd/Kconfig
+++ b/drivers/net/can/ctucanfd/Kconfig
@@ -20,3 +20,15 @@ config CAN_CTUCANFD_PCI
The project providing FPGA design for Intel EP4CGX15 based DB4CGX15
PCIe board with PiKRON.com designed transceiver riser shield is available
at https://gitlab.fel.cvut.cz/canbus/pcie-ctucanfd .
+
+config CAN_CTUCANFD_PLATFORM
+ tristate "CTU CAN-FD IP core platform (FPGA, SoC) driver"
+ depends on CAN_CTUCANFD
+ depends on OF || COMPILE_TEST
+ help
+ The core has been tested together with OpenCores SJA1000
+ modified to be CAN FD frames tolerant on MicroZed Zynq based
+ MZ_APO education kits designed by Petr Porazil from PiKRON.com
+ company. FPGA design https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top.
+ The kit description at the Computer Architectures course pages
+ https://cw.fel.cvut.cz/b182/courses/b35apo/documentation/mz_apo/start .
diff --git a/drivers/net/can/ctucanfd/Makefile b/drivers/net/can/ctucanfd/Makefile
index b679859c7c9b..d4223812391d 100644
--- a/drivers/net/can/ctucanfd/Makefile
+++ b/drivers/net/can/ctucanfd/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_CAN_CTUCANFD) := ctucanfd.o
ctucanfd-y := ctucanfd_base.o ctucanfd_hw.o

obj-$(CONFIG_CAN_CTUCANFD_PCI) += ctucanfd_pci.o
+obj-$(CONFIG_CAN_CTUCANFD_PLATFORM) += ctucanfd_platform.o
diff --git a/drivers/net/can/ctucanfd/ctucanfd_platform.c b/drivers/net/can/ctucanfd/ctucanfd_platform.c
new file mode 100644
index 000000000000..d8146cfb2f39
--- /dev/null
+++ b/drivers/net/can/ctucanfd/ctucanfd_platform.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*******************************************************************************
+ *
+ * CTU CAN FD IP Core
+ *
+ * Copyright (C) 2015-2018 Ondrej Ille <[email protected]> FEE CTU
+ * Copyright (C) 2018-2020 Ondrej Ille <[email protected]> self-funded
+ * Copyright (C) 2018-2019 Martin Jerabek <[email protected]> FEE CTU
+ * Copyright (C) 2018-2020 Pavel Pisa <[email protected]> FEE CTU/self-funded
+ *
+ * Project advisors:
+ * Jiri Novak <[email protected]>
+ * Pavel Pisa <[email protected]>
+ *
+ * Department of Measurement (http://meas.fel.cvut.cz/)
+ * Faculty of Electrical Engineering (http://www.fel.cvut.cz)
+ * Czech Technical University (http://www.cvut.cz/)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ ******************************************************************************/
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include "ctucanfd.h"
+
+#define DRV_NAME "ctucanfd"
+
+static void ctucan_platform_set_drvdata(struct device *dev,
+ struct net_device *ndev)
+{
+ struct platform_device *pdev = container_of(dev, struct platform_device,
+ dev);
+
+ platform_set_drvdata(pdev, ndev);
+}
+
+/**
+ * ctucan_platform_probe - Platform registration call
+ * @pdev: Handle to the platform device structure
+ *
+ * This function does all the memory allocation and registration for the CAN
+ * device.
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int ctucan_platform_probe(struct platform_device *pdev)
+{
+ struct resource *res; /* IO mem resources */
+ struct device *dev = &pdev->dev;
+ void __iomem *addr;
+ int ret;
+ unsigned int ntxbufs;
+ int irq;
+
+ /* Get the virtual base address for the device */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ addr = devm_ioremap_resource(dev, res);
+ if (IS_ERR(addr)) {
+ dev_err(dev, "Cannot remap address.\n");
+ ret = PTR_ERR(addr);
+ goto err;
+ }
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev, "Cannot find interrupt.\n");
+ ret = irq;
+ goto err;
+ }
+
+ /* Number of tx bufs might be change in HW for future. If so,
+ * it will be passed as property via device tree
+ */
+ ntxbufs = 4;
+ ret = ctucan_probe_common(dev, addr, irq, ntxbufs, 0,
+ 1, ctucan_platform_set_drvdata);
+
+ if (ret < 0)
+ platform_set_drvdata(pdev, NULL);
+
+err:
+ return ret;
+}
+
+/**
+ * ctucan_platform_remove - Unregister the device after releasing the resources
+ * @pdev: Handle to the platform device structure
+ *
+ * This function frees all the resources allocated to the device.
+ * Return: 0 always
+ */
+static int ctucan_platform_remove(struct platform_device *pdev)
+{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct ctucan_priv *priv = netdev_priv(ndev);
+
+ netdev_dbg(ndev, "ctucan_remove");
+
+ unregister_candev(ndev);
+ pm_runtime_disable(&pdev->dev);
+ netif_napi_del(&priv->napi);
+ free_candev(ndev);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ctucan_platform_pm_ops, ctucan_suspend, ctucan_resume);
+
+/* Match table for OF platform binding */
+static const struct of_device_id ctucan_of_match[] = {
+ { .compatible = "ctu,ctucanfd-2", },
+ { .compatible = "ctu,ctucanfd", },
+ { /* end of list */ },
+};
+MODULE_DEVICE_TABLE(of, ctucan_of_match);
+
+static struct platform_driver ctucanfd_driver = {
+ .probe = ctucan_platform_probe,
+ .remove = ctucan_platform_remove,
+ .driver = {
+ .name = DRV_NAME,
+ .pm = &ctucan_platform_pm_ops,
+ .of_match_table = ctucan_of_match,
+ },
+};
+
+module_platform_driver(ctucanfd_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Martin Jerabek");
+MODULE_DESCRIPTION("CTU CAN FD for platform");
--
2.20.1

2020-10-31 11:38:24

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation

On 10/30/20 11:19 PM, Pavel Pisa wrote:
> This driver adds support for the CTU CAN FD open-source IP core.

Please fix the following checkpatch warnings/errors:

----------------------------------------
drivers/net/can/ctucanfd/ctucanfd_base.c
----------------------------------------
WARNING: Possible repeated word: 'the'
#296: FILE: drivers/net/can/ctucanfd/ctucanfd_base.c:296:
+ * This check the drivers state and calls the
+ * the corresponding modes to set.

WARNING: Possible repeated word: 'the'
#445: FILE: drivers/net/can/ctucanfd/ctucanfd_base.c:445:
+ * This is the CAN error interrupt and it will check the the type of error

WARNING: quoted string split across lines
#466: FILE: drivers/net/can/ctucanfd/ctucanfd_base.c:466:
+ netdev_info(ndev, "%s: ISR = 0x%08x, rxerr %d, txerr %d,"
+ " error type %u, pos %u, ALC id_field %u, bit %u\n",

CHECK: Alignment should match open parenthesis
#637: FILE: drivers/net/can/ctucanfd/ctucanfd_base.c:637:
+ ctucan_netdev_dbg(ndev, "%s: from 0x%08x to 0x%08x\n",
+ __func__, priv->txb_prio, prio);

CHECK: Alignment should match open parenthesis
#673: FILE: drivers/net/can/ctucanfd/ctucanfd_base.c:673:
+ ctucan_netdev_dbg(ndev, "TXI: TXB#%u: status 0x%x\n",
+ txb_idx, status);

CHECK: Alignment should match open parenthesis
#808: FILE: drivers/net/can/ctucanfd/ctucanfd_base.c:808:
+ ctucan_netdev_dbg(ndev, "some ERR interrupt: clearing 0x%08x\n",
+ icr.u32);

total: 0 errors, 3 warnings, 3 checks, 1142 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

drivers/net/can/ctucanfd/ctucanfd_base.c has style problems, please review.
-----------------------------------------
drivers/net/can/ctucanfd/ctucanfd_frame.h
-----------------------------------------
CHECK: Please don't use multiple blank lines
#46: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:46:
+
+

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#49: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:49:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#104: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:104:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#120: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:120:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#128: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:128:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#136: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:136:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#154: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:154:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#172: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:172:
+ uint32_t u32;

total: 0 errors, 0 warnings, 8 checks, 189 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

drivers/net/can/ctucanfd/ctucanfd_frame.h has style problems, please review.
-----------------------------------
drivers/net/can/ctucanfd/ctucanfd.h
-----------------------------------
total: 0 errors, 0 warnings, 0 checks, 87 lines checked

drivers/net/can/ctucanfd/ctucanfd.h has no obvious style problems and is ready for submission.
--------------------------------------
drivers/net/can/ctucanfd/ctucanfd_hw.c
--------------------------------------
CHECK: Please don't use multiple blank lines
#30: FILE: drivers/net/can/ctucanfd/ctucanfd_hw.c:30:
+
+

WARNING: Possible repeated word: 'from'
#40: FILE: drivers/net/can/ctucanfd/ctucanfd_hw.c:40:
+ * generated from from IP-XACT/cactus helps to driver to hardware

CHECK: Alignment should match open parenthesis
#98: FILE: drivers/net/can/ctucanfd/ctucanfd_hw.c:98:
+static u32 ctucan_hw_hwid_to_id(union ctu_can_fd_identifier_w hwid,
+ enum ctu_can_fd_frame_format_w_ide type)

total: 0 errors, 1 warnings, 2 checks, 751 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

drivers/net/can/ctucanfd/ctucanfd_hw.c has style problems, please review.
--------------------------------------
drivers/net/can/ctucanfd/ctucanfd_hw.h
--------------------------------------
WARNING: networking block comments don't use an empty /* line, use /* Comment...
#56: FILE: drivers/net/can/ctucanfd/ctucanfd_hw.h:56:
+/*
+ * Status macros -> pass "ctu_can_get_status" result

WARNING: networking block comments don't use an empty /* line, use /* Comment...
#84: FILE: drivers/net/can/ctucanfd/ctucanfd_hw.h:84:
+/*
+ * Interrupt macros -> pass "ctu_can_fd_int_sts" result

CHECK: Alignment should match open parenthesis
#759: FILE: drivers/net/can/ctucanfd/ctucanfd_hw.h:759:
+static inline void ctucan_hw_txt_buf_give_command(struct ctucan_hw_priv *priv,
+ union ctu_can_fd_tx_command cmd, u8 buf)

total: 0 errors, 2 warnings, 1 checks, 935 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

drivers/net/can/ctucanfd/ctucanfd_hw.h has style problems, please review.
---------------------------------------
drivers/net/can/ctucanfd/ctucanfd_pci.c
---------------------------------------
total: 0 errors, 0 warnings, 0 checks, 316 lines checked

drivers/net/can/ctucanfd/ctucanfd_pci.c has no obvious style problems and is ready for submission.
--------------------------------------------
drivers/net/can/ctucanfd/ctucanfd_platform.c
--------------------------------------------
total: 0 errors, 0 warnings, 0 checks, 142 lines checked

drivers/net/can/ctucanfd/ctucanfd_platform.c has no obvious style problems and is ready for submission.
----------------------------------------
drivers/net/can/ctucanfd/ctucanfd_regs.h
----------------------------------------
CHECK: Please don't use multiple blank lines
#100: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:100:
+
+

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#103: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:103:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#124: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:124:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#217: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:217:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#245: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:245:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#269: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:269:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#305: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:305:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#319: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:319:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#333: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:333:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#347: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:347:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#361: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:361:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#381: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:381:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#407: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:407:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#431: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:431:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#450: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:450:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#465: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:465:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#487: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:487:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#501: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:501:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#515: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:515:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#529: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:529:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#543: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:543:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#557: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:557:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#571: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:571:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#585: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:585:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#599: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:599:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#652: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:652:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#670: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:670:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#688: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:688:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#718: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:718:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#726: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:726:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#756: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:756:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#784: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:784:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#810: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:810:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#863: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:863:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#890: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:890:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#898: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:898:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#906: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:906:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#948: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:948:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#956: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:956:
+ uint32_t u32;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#964: FILE: drivers/net/can/ctucanfd/ctucanfd_regs.h:964:
+ uint32_t u32;

total: 0 errors, 0 warnings, 40 checks, 971 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

drivers/net/can/ctucanfd/ctucanfd_regs.h has style problems, please review.

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

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 |


Attachments:
signature.asc (499.00 B)
OpenPGP digital signature

2020-10-31 11:44:06

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation

On 10/30/20 11:19 PM, Pavel Pisa wrote:
> This driver adds support for the CTU CAN FD open-source IP core.

Please fix the following spelling mistakes:

--- a/drivers/net/can/ctucanfd/ctucanfd_base.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
@@ -752,7 +752,7 @@ static void ctucan_tx_interrupt(struct net_device *ndev)
/**
* ctucan_interrupt - CAN Isr
* @irq: irq number
- * @dev_id: device id poniter
+ * @dev_id: device id pointer
*
* This is the CTU CAN FD ISR. It checks for the type of interrupt
* and invokes the corresponding ISR.
diff --git a/drivers/net/can/ctucanfd/ctucanfd_hw.h b/drivers/net/can/ctucanfd/ctucanfd_hw.h
index 7d562f41ca52..2fd2416de46d 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_hw.h
+++ b/drivers/net/can/ctucanfd/ctucanfd_hw.h
@@ -211,7 +211,7 @@ bool ctucan_hw_set_ret_limit(struct ctucan_hw_priv *priv, bool enable,
* CAN_CTRLMODE_LISTENONLY - No frame is transmitted, no dominant bit is
* sent on the bus.
*
- * CAN_CTRLMODE_3_SAMPLES - Tripple sampling mode
+ * CAN_CTRLMODE_3_SAMPLES - Triple sampling mode
*
* CAN_CTRLMODE_FD - Flexible data-rate support. When not set, Core
* does not accept CAN FD Frames and interprets,
@@ -680,7 +680,7 @@ void ctucan_hw_set_rx_tsop(struct ctucan_hw_priv *priv,
*
* @priv: Private info
*
- * Return: The firts word of received frame
+ * Return: The first word of received frame
*/
static inline union ctu_can_fd_frame_format_w
ctu_can_fd_read_rx_ffw(struct ctucan_hw_priv *priv)
@@ -908,7 +908,7 @@ static inline union ctu_can_fd_debug_register
* ctucan_hw_read_timestamp - Read timestamp value which is used internally
* by CTU CAN FD Core.
*
- * Reads timestamp twice and checks consistency betwen upper and
+ * Reads timestamp twice and checks consistency between upper and
* lower timestamp word.
*
* @priv: Private info

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 |


Attachments:
signature.asc (499.00 B)
OpenPGP digital signature

2020-11-03 10:05:51

by Pavel Pisa

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation

Hello Marc,

thanks for response

On Saturday 31 of October 2020 12:35:11 Marc Kleine-Budde wrote:
> On 10/30/20 11:19 PM, Pavel Pisa wrote:
> > This driver adds support for the CTU CAN FD open-source IP core.
>
> Please fix the following checkpatch warnings/errors:

Yes I recheck with actual checkpatch, I have used 5.4 one
and may it be overlooked something during last upadates.

> -----------------------------------------
> drivers/net/can/ctucanfd/ctucanfd_frame.h
> -----------------------------------------
> CHECK: Please don't use multiple blank lines
> #46: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:46:

OK, we find a reason for this blank line in header generator.

> CHECK: Prefer kernel type 'u32' over 'uint32_t'
> #49: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:49:
> + uint32_t u32;

In this case, please confirm that even your personal opinion
is against uint32_t in headers, you request the change.

uint32_t is used in many kernel headers and in this case
allows our tooling to use headers for mutual test of HDL
design match with HW access in the C.

If the reasons to remove uint32_t prevails, we need to
separate Linux generator from the one used for other
purposes. When we add Linux mode then we can revamp
headers even more and in such case we can even invest
time to switch from structure bitfields to plain bitmask
defines. It is quite lot of work and takes some time,
but if there is consensus I do it during next weeks,
I would like to see what is preferred way to define
registers bitfields. I personally like RTEMS approach
for which we have prepared generator from parsed PDFs
when we added BSP for TMS570

https://git.rtems.org/rtems/tree/bsps/arm/tms570/include/bsp/ti_herc/reg_dcan.h#n152

Other solution I like (biased, because I have even designed it)
is

#define __val2mfld(mask,val) (((mask)&~((mask)<<1))*(val)&(mask))
#define __mfld2val(mask,val) (((val)&(mask))/((mask)&~((mask)<<1)))
https://gitlab.com/pikron/sw-base/sysless/-/blob/master/arch/arm/generic/defines/cpu_def.h#L314

Which allows to use simple masks, i.e.
#define SSP_CR0_DSS_m 0x000f /* Data Size Select (num bits - 1) */
#define SSP_CR0_FRF_m 0x0030 /* Frame Format: 0 SPI, 1 TI, 2 Microwire */
#define SSP_CR0_CPOL_m 0x0040 /* SPI Clock Polarity. 0 low between frames, 1 high */ #

https://gitlab.com/pikron/sw-base/sysless/-/blob/master/libs4c/spi/spi_lpcssp.c#L46

in the sources

lpcssp_drv->ssp_regs->CR0 =
__val2mfld(SSP_CR0_DSS_m, lpcssp_drv->data16_fl? 16 - 1 : 8 - 1) |
__val2mfld(SSP_CR0_FRF_m, 0) |
(msg->size_mode & SPI_MODE_CPOL? SSP_CR0_CPOL_m: 0) |
(msg->size_mode & SPI_MODE_CPHA? SSP_CR0_CPHA_m: 0) |
__val2mfld(SSP_CR0_SCR_m, rate);

https://gitlab.com/pikron/sw-base/sysless/-/blob/master/libs4c/spi/spi_lpcssp.c#L217

If you have some preferred Linux style then please send us pointers.
In the fact, Ondrej Ille has based his structure bitfileds style
on the other driver included in the Linux kernel and it seems
to be a problem now. So when I invest my time, I want to use style
which pleases me and others.

Thanks for the support and best wishes,

Pavel Pisa

2020-11-03 11:30:33

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation

On 11/3/20 11:00 AM, Pavel Pisa wrote:
> On Saturday 31 of October 2020 12:35:11 Marc Kleine-Budde wrote:
>> On 10/30/20 11:19 PM, Pavel Pisa wrote:
>>> This driver adds support for the CTU CAN FD open-source IP core.
>>
>> Please fix the following checkpatch warnings/errors:
>
> Yes I recheck with actual checkpatch, I have used 5.4 one
> and may it be overlooked something during last upadates.

I used the lastest one from linus/master :)

>> -----------------------------------------
>> drivers/net/can/ctucanfd/ctucanfd_frame.h
>> -----------------------------------------
>> CHECK: Please don't use multiple blank lines
>> #46: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:46:
>
> OK, we find a reason for this blank line in header generator.
>
>> CHECK: Prefer kernel type 'u32' over 'uint32_t'
>> #49: FILE: drivers/net/can/ctucanfd/ctucanfd_frame.h:49:
>> + uint32_t u32;
>
> In this case, please confirm that even your personal opinion
> is against uint32_t in headers, you request the change.

confirmed :)

> uint32_t is used in many kernel headers and in this case
> allows our tooling to use headers for mutual test of HDL
> design match with HW access in the C.

It's probably historically related :)

> If the reasons to remove uint32_t prevails, we need to
> separate Linux generator from the one used for other
> purposes. When we add Linux mode then we can revamp
> headers even more and in such case we can even invest
> time to switch from structure bitfields to plain bitmask
> defines.

This is another point I wanted to address. Obviously checkpatch doesn't complain
about bitfields, but it's frowned upon.

> It is quite lot of work and takes some time,
> but if there is consensus I do it during next weeks,
> I would like to see what is preferred way to define
> registers bitfields. I personally like RTEMS approach
> for which we have prepared generator from parsed PDFs
> when we added BSP for TMS570
>
> https://git.rtems.org/rtems/tree/bsps/arm/tms570/include/bsp/ti_herc/reg_dcan.h#n152

The current Linux way is to define bitmask with GENMASK() and single bit mask
with BIT().

For example the mcp251xfd driver:

First the register offset:
> #define MCP251XFD_REG_CON 0x00

Then a bitmask:
> #define MCP251XFD_REG_CON_TXBWS_MASK GENMASK(31, 28)

And a single bit:
> #define MCP251XFD_REG_CON_ABAT BIT(27)

see:
https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/net/can/spi/mcp251xfd/mcp251xfd.h#L24

The masks are used with FIELD_GET, FIELD_PREP.

For example:
https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c#L1386

> Other solution I like (biased, because I have even designed it)
> is
>
> #define __val2mfld(mask,val) (((mask)&~((mask)<<1))*(val)&(mask))
> #define __mfld2val(mask,val) (((val)&(mask))/((mask)&~((mask)<<1)))
> https://gitlab.com/pikron/sw-base/sysless/-/blob/master/arch/arm/generic/defines/cpu_def.h#L314
>
> Which allows to use simple masks, i.e.
> #define SSP_CR0_DSS_m 0x000f /* Data Size Select (num bits - 1) */
> #define SSP_CR0_FRF_m 0x0030 /* Frame Format: 0 SPI, 1 TI, 2 Microwire */
> #define SSP_CR0_CPOL_m 0x0040 /* SPI Clock Polarity. 0 low between frames, 1 high */ #
>
> https://gitlab.com/pikron/sw-base/sysless/-/blob/master/libs4c/spi/spi_lpcssp.c#L46
>
> in the sources
>
> lpcssp_drv->ssp_regs->CR0 =
> __val2mfld(SSP_CR0_DSS_m, lpcssp_drv->data16_fl? 16 - 1 : 8 - 1) |
> __val2mfld(SSP_CR0_FRF_m, 0) |
> (msg->size_mode & SPI_MODE_CPOL? SSP_CR0_CPOL_m: 0) |
> (msg->size_mode & SPI_MODE_CPHA? SSP_CR0_CPHA_m: 0) |
> __val2mfld(SSP_CR0_SCR_m, rate);
>
> https://gitlab.com/pikron/sw-base/sysless/-/blob/master/libs4c/spi/spi_lpcssp.c#L217
>
> If you have some preferred Linux style then please send us pointers.
> In the fact, Ondrej Ille has based his structure bitfileds style
> on the other driver included in the Linux kernel and it seems
> to be a problem now. So when I invest my time, I want to use style
> which pleases me and others.

Hope that helps,
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 |


Attachments:
signature.asc (499.00 B)
OpenPGP digital signature

2020-11-03 16:15:26

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation

On 11/3/20 2:36 PM, Ondrej Ille wrote:
> Hello Marc,
>
> thank you for review, I appreciate it. We will process all your notes, and get
> rid of uin32_t and bitfields then.
>
> As Pavel pointed out, there are user space tests using this stuff, so it is
> not just search and replace work. We will extend our IP-XACT generation
> toolchain (what a strong word for bunch of python scripts...), to generate
> Linux specific headers with GEN_MASK and BIT then.

Fine!
> It will take some time, since we have to modify quite a lot of stuff and
> re-test it then, but we will try to do it fast. Btw, do you agree with
> separation of HW specific part of driver into "_hw" file, or would you
> preffer to get rid of this abstraction layer? If we should get rid of it, we
> will, but it would take even more time to do it.

I haven't looked at the HW abstraction yet, but will do next. Usually Linux is
considered the HW abstraction layer :)

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 |


Attachments:
signature.asc (499.00 B)
OpenPGP digital signature