Skip to contentSkip to conversation

CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation

PATCH Superseded [email protected] 6 participants 17 messages Aug 3, 2020 – Aug 6, 2020 2 messages not in the archive

Subsystems (6)
Networking Network drivers Device tree bindings Documentation +2 more subsystems

v4 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

v4 cover CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation

From: Pavel Pisa <[email protected]>

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-ctu_can_fd).
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 related projects used and developed at the Faculty of the Electrical Engineering (http://www.fel.cvut.cz/en/)
of Czech Technical University (https://www.cvut.cz/en)
in Prague at 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 4 changes:
- 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

 .../devicetree/bindings/net/can/ctu,ctucanfd.yaml  |   70 ++
 .../devicetree/bindings/vendor-prefixes.yaml       |    2 +
 .../device_drivers/ctu/FSM_TXT_Buffer_user.png     |  Bin 0 -> 174807 bytes
 .../device_drivers/ctu/ctucanfd-driver.rst         |  635 +++++++++++
 drivers/net/can/Kconfig                            |    1 +
 drivers/net/can/Makefile                           |    1 +
 drivers/net/can/ctucanfd/Kconfig                   |   38 +
 drivers/net/can/ctucanfd/Makefile                  |   13 +
 drivers/net/can/ctucanfd/ctu_can_fd.c              | 1114 ++++++++++++++++++++
 drivers/net/can/ctucanfd/ctu_can_fd.h              |   88 ++
 drivers/net/can/ctucanfd/ctu_can_fd_frame.h        |  190 ++++
 drivers/net/can/ctucanfd/ctu_can_fd_hw.c           |  781 ++++++++++++++
 drivers/net/can/ctucanfd/ctu_can_fd_hw.h           |  917 ++++++++++++++++
 drivers/net/can/ctucanfd/ctu_can_fd_pci.c          |  316 ++++++
 drivers/net/can/ctucanfd/ctu_can_fd_platform.c     |  145 +++
 drivers/net/can/ctucanfd/ctu_can_fd_regs.h         |  972 +++++++++++++++++
 16 files changed, 5283 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml
 create mode 100644 Documentation/networking/device_drivers/ctu/FSM_TXT_Buffer_user.png
 create mode 100644 Documentation/networking/device_drivers/ctu/ctucanfd-driver.rst
 create mode 100644 drivers/net/can/ctucanfd/Kconfig
 create mode 100644 drivers/net/can/ctucanfd/Makefile
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd.c
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd.h
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_frame.h
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_hw.c
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_hw.h
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_pci.c
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_platform.c
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_regs.h
Signature · 1 line
2.11.0
author maintainer
#1 lore raw

PATCH v4 1/6 dt-bindings: vendor-prefix: add prefix for the Czech Technical University in Prague.

From: Pavel Pisa <[email protected]>

Following text added to make checkpatch happy.

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]>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0
index 967e78c5ec0a..dedb10f1b250 100644
@@ -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,.*":
Signature · 1 line
2.11.0
author maintainer
#2 lore raw

PATCH v4 2/6 dt-bindings: net: can: binding for CTU CAN FD open-source IP core.

In reply to [email protected]

From: Pavel Pisa <[email protected]>

The device-tree bindings for open-source CAN FD IP core which design started at Department of Measurement at Faculty of Electrical Engineering
of Czech Technical University in Prague.
The IP core main author is Ondrej Ille who continues on the core development even after finishing the studies.

The CTU CAN FD IP core main repository

https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core

The list of related CAN bus projects which we participate in

http://canbus.pages.fel.cvut.cz/

The commit text again to make checkpatch happy.

Signed-off-by Pavel Pisa <[email protected]>
---
 .../devicetree/bindings/net/can/ctu,ctucanfd.yaml  | 70 ++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +70 -0
new file mode 100644
index 000000000000..b74bfc951062
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/can/ctu,ctucanfd.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CTU CAN FD Open-source IP Core Device Tree Bindings
+
+description: |
+  Open-source CAN FD IP core developed at the Czech Technical University in Prague
+
+  The core sources and documentation on project page
+    [1] sources : https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core
+    [2] datasheet : https://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf
+
+  Integration in Xilinx Zynq SoC based system together with
+  OpenCores SJA1000 compatible controllers
+    [3] project : https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top
+  Martin Jerabek dimploma thesis with integration and testing
+  framework description
+    [4] PDF : https://dspace.cvut.cz/bitstream/handle/10467/80366/F3-DP-2019-Jerabek-Martin-Jerabek-thesis-2019-canfd.pdf
+
+maintainers:
+  - Pavel Pisa <[email protected]>
+  - Ondrej Ille <[email protected]>
+  - Martin Jerabek <[email protected]>
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - const: ctu,ctucanfd
+          - const: ctu,canfd-2
+      - const: ctu,ctucanfd
+
+  reg:
+    description:
+      mapping into bus address space, offset and size
+    maxItems: 1
+
+  interrupts:
+    description: |
+      interrupt source. For Zynq SoC system, format is <(is_spi) (number) (type)>
+      where is_spi defines if it is SPI (shared peripheral) interrupt,
+      the second number is translated to the vector by addition of 32
+      on Zynq-7000 systems and type is IRQ_TYPE_LEVEL_HIGH (4) for Zynq.
+    maxItems: 1
+
+  clocks:
+    description: |
+      phandle of reference clock (100 MHz is appropriate
+      for FPGA implementation on Zynq-7000 system).
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    ctu_can_fd_0: can@43c30000 {
+      compatible = "ctu,ctucanfd";
+      interrupts = <0 30 4>;
+      clocks = <&clkc 15>;
+      reg = <0x43c30000 0x10000>;
+    };
Signature · 1 line
2.11.0
#3 lore raw

Re: [PATCH v4 2/6] dt-bindings: net: can: binding for CTU CAN FD open-source IP core.

Hi!

Quoted 1 line

The commit text again to make checkpatch happy.

?

Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml line 34

Quoted 5 lines
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +5 -0
+    oneOf:
+      - items:
+          - const: ctu,ctucanfd
+          - const: ctu,canfd-2
+      - const: ctu,ctucanfd

For consistency, can we have ctu,canfd-1, ctu,canfd-2?

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#4 lore raw

Re: [PATCH v4 2/6] dt-bindings: net: can: binding for CTU CAN FD open-source IP core.

Quoted 7 lines · Pavel Machek

Hi!

Quoted 1 line

The commit text again to make checkpatch happy.

?

Quoted 7 lines

Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml line 34

Quoted 5 lines
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +5 -0
+    oneOf:
+      - items:
+          - const: ctu,ctucanfd
+          - const: ctu,canfd-2
+      - const: ctu,ctucanfd

For consistency, can we have ctu,canfd-1, ctu,canfd-2?

Make it ctu,ctucanfd-1, ctu,ctucanfd-2... to make it consistent with the file names.

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
maintainer
#5 lore raw

Re: [PATCH v4 2/6] dt-bindings: net: can: binding for CTU CAN FD open-source IP core.

Quoted 14 lines · Pavel Machek
Quoted 13 lines · Pavel Machek

Hi!

Quoted 1 line

The commit text again to make checkpatch happy.

?

Quoted 5 lines
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +5 -0
+    oneOf:
+      - items:
+          - const: ctu,ctucanfd
+          - const: ctu,canfd-2
+      - const: ctu,ctucanfd
Quoted 4 lines
Quoted 1 line

For consistency, can we have ctu,canfd-1, ctu,canfd-2?

Make it ctu,ctucanfd-1, ctu,ctucanfd-2... to make it consistent with the file names.

If you are going to do version numbers, please define where they come from. Hopefully some tag of the h/w IP version...

Better yet, put version numbers in the h/w registers itself and you don't need different compatibles.

Rob

author maintainer
#6 lore raw

Re: [PATCH v4 2/6] dt-bindings: net: can: binding for CTU CAN FD open-source IP core.

Hello Pavel and Rob,

thanks much for review.

Quoted 7 lines · On Thursday 06 of August 2020 16:47:13 Rob Herring wrote:
Quoted 6 lines · Pavel Machek
Quoted 5 lines · Pavel Machek

Hi!

Quoted 1 line

The commit text again to make checkpatch happy.

?

The checkpatch reports as a problem when there is no description of the patch. At least for patch

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

I consider that little pontificate but I have fullfiled its suggestion with remark, that in this case, It is not my intention to add these promotions. I remove the reference to patchcheck from these commit messages.

Quoted 8 lines
Quoted 8 lines
Quoted 7 lines

Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml line 34

Quoted 5 lines
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +5 -0
+    oneOf:
+      - items:
+          - const: ctu,ctucanfd
+          - const: ctu,canfd-2
+      - const: ctu,ctucanfd

For consistency, can we have ctu,canfd-1, ctu,canfd-2?

Quoted 8 lines
Quoted 2 lines

Make it ctu,ctucanfd-1, ctu,ctucanfd-2... to make it consistent with the file names.

If you are going to do version numbers, please define where they come from. Hopefully some tag of the h/w IP version...

Better yet, put version numbers in the h/w registers itself and you don't need different compatibles.

The actual major version of the core is 2. The minor intended for release was 1. But we wait for driver inclusion and release and IP core release has not been realized. Sources moved to 2.2-pre version and compiled core reports 2.2 now. There is added control bit for protocol exception behavior selection and minor enhancements in sync of standard and data rate bittimes starts.

Yes, version can be obtained from hardware. There is magic and version in the first core register. See 3.1.1 DEVICE_ID section of the manual (page 22/28)

http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf

As for the DT identifier we use "ctu,ctucanfd" in more projects already. Some devices are in the wild now. So I would prefer to keep compatibility with that name. Other name reflects that this driver is compatible with major version 2 of the core. It can be "ctu,ctucanfd-2". I am not sure if the repeat of "ctu" is good idea, but yes, full sources prefix is "ctucanfd". The second alias can be omitted alltogether. But I am not sure, there can be one day fundamental change between IP core versions which would be better handled by change of PCI ID and DT ID. It is questionable if attempt to keep single driver for more too different versions would be more manageable or convoluted than two fully independent ones. May it be we do not need to solve that because by that time it would be "ctu,ctucanxl".

At this time, our actual first first choic for the IP core identifier is ctu,ctucanfd.

As for the pointed description, I would remove them from version 5 according to your reference. My personal one is to keep documentation (even of actual/local functional setup) directly in the sources and mainline to find it out when I or somebody else need to recreate or update designs, my biological memory is already worn out by past events.

I am not sure if I should wait for subsystem maintainers review now or sent new patches version. I may get to its preparation tommorrow or may it be later because I want to take some time in countrysite/mountains.

Best wishes

Pavel

Signature · 9 lines
                Pavel Pisa
    phone:      +420 603531357
    e-mail:     [email protected]
    Department of Control Engineering FEE CVUT
    Karlovo namesti 13, 121 35, Prague 2
    university: http://dce.fel.cvut.cz/
    personal:   http://cmp.felk.cvut.cz/~pisa
    projects:   https://www.openhub.net/accounts/ppisa
    CAN related:http://canbus.pages.fel.cvut.cz/
maintainer
#7 lore raw

Re: [PATCH v4 2/6] dt-bindings: net: can: binding for CTU CAN FD open-source IP core.

In reply to [email protected]
Quoted 54 lines · On Mon, Aug 03, 2020 at 08:34:50PM +0200, [email protected] wrote:

From: Pavel Pisa <[email protected]>

The device-tree bindings for open-source CAN FD IP core which design started at Department of Measurement at Faculty of Electrical Engineering
of Czech Technical University in Prague.
The IP core main author is Ondrej Ille who continues on the core development even after finishing the studies.

The CTU CAN FD IP core main repository

https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core

The list of related CAN bus projects which we participate in

http://canbus.pages.fel.cvut.cz/

The commit text again to make checkpatch happy.

Signed-off-by Pavel Pisa <[email protected]>
---
 .../devicetree/bindings/net/can/ctu,ctucanfd.yaml  | 70 ++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +23 -0
new file mode 100644
index 000000000000..b74bfc951062
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/can/ctu,ctucanfd.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CTU CAN FD Open-source IP Core Device Tree Bindings
+
+description: |
+  Open-source CAN FD IP core developed at the Czech Technical University in Prague
+
+  The core sources and documentation on project page
+    [1] sources : https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core
+    [2] datasheet : https://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf
+
+  Integration in Xilinx Zynq SoC based system together with
+  OpenCores SJA1000 compatible controllers
+    [3] project : https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top
+  Martin Jerabek dimploma thesis with integration and testing
+  framework description
+    [4] PDF : https://dspace.cvut.cz/bitstream/handle/10467/80366/F3-DP-2019-Jerabek-Martin-Jerabek-thesis-2019-canfd.pdf
+
+maintainers:

Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml line 33

Quoted 10 lines
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +10 -0
+  - Pavel Pisa <[email protected]>
+  - Ondrej Ille <[email protected]>
+  - Martin Jerabek <[email protected]>
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - const: ctu,ctucanfd
+          - const: ctu,canfd-2

The order is supposed to be be most specific compatible first, but this seems backwards given 'ctu,ctucanfd' alone is allowed.

Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml line 38

Quoted 5 lines
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +5 -0
+      - const: ctu,ctucanfd
+
+  reg:
+    description:
+      mapping into bus address space, offset and size

No need to describe common properties unless you have something specific to this device to say.

Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml line 46

Quoted 8 lines
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +8 -0
+    maxItems: 1
+
+  interrupts:
+    description: |
+      interrupt source. For Zynq SoC system, format is <(is_spi) (number) (type)>
+      where is_spi defines if it is SPI (shared peripheral) interrupt,
+      the second number is translated to the vector by addition of 32
+      on Zynq-7000 systems and type is IRQ_TYPE_LEVEL_HIGH (4) for Zynq.

That's all outside the scope of this binding.

Quoted 27 lines
Documentation/devicetree/bindings/net/can/ctu,ctucanfd.yaml new file +24 -0
+    maxItems: 1
+
+  clocks:
+    description: |
+      phandle of reference clock (100 MHz is appropriate
+      for FPGA implementation on Zynq-7000 system).
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    ctu_can_fd_0: can@43c30000 {
+      compatible = "ctu,ctucanfd";
+      interrupts = <0 30 4>;
+      clocks = <&clkc 15>;
+      reg = <0x43c30000 0x10000>;
+    };
Signature · 1 line
2.11.0
author maintainer
#8 lore raw

PATCH v4 3/6 can: ctucanfd: add support for CTU CAN FD open-source IP core - bus independent part.

In reply to [email protected]

From: Martin Jerabek <[email protected]>

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 FGA based PCI Express board is available from project (https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd).

More about CAN related projects used and developed at the Faculty of the Electrical Engineering (http://www.fel.cvut.cz/en/)
of Czech Technical University (https://www.cvut.cz/en)
in at Prague http://canbus.pages.fel.cvut.cz/ .

Signed-off-by Martin Jerabek <[email protected]>
Signed-off-by Ondrej Ille <[email protected]>
Signed-off-by Pavel Pisa <[email protected]>
---
 drivers/net/can/Kconfig                     |    1 +
 drivers/net/can/Makefile                    |    1 +
 drivers/net/can/ctucanfd/Kconfig            |   18 +
 drivers/net/can/ctucanfd/Makefile           |    7 +
 drivers/net/can/ctucanfd/ctu_can_fd.c       | 1114 +++++++++++++++++++++++++++
 drivers/net/can/ctucanfd/ctu_can_fd.h       |   88 +++
 drivers/net/can/ctucanfd/ctu_can_fd_frame.h |  190 +++++
 drivers/net/can/ctucanfd/ctu_can_fd_hw.c    |  781 +++++++++++++++++++
 drivers/net/can/ctucanfd/ctu_can_fd_hw.h    |  917 ++++++++++++++++++++++
 drivers/net/can/ctucanfd/ctu_can_fd_regs.h  |  972 +++++++++++++++++++++++
 10 files changed, 4089 insertions(+)
 create mode 100644 drivers/net/can/ctucanfd/Kconfig
 create mode 100644 drivers/net/can/ctucanfd/Makefile
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd.c
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd.h
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_frame.h
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_hw.c
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_hw.h
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_regs.h
drivers/net/can/Kconfig +1 -0
index 17c166cc8482..458afc4b81f2 100644
@@ -168,6 +168,7 @@ config PCH_CAN
 
 source "drivers/net/can/c_can/Kconfig"
 source "drivers/net/can/cc770/Kconfig"
+source "drivers/net/can/ctucanfd/Kconfig"
 source "drivers/net/can/ifi_canfd/Kconfig"
 source "drivers/net/can/m_can/Kconfig"
 source "drivers/net/can/mscan/Kconfig"
drivers/net/can/Makefile +1 -0
index 22164300122d..28b39cd122f0 100644
@@ -21,6 +21,7 @@ obj-y				+= softing/
 obj-$(CONFIG_CAN_AT91)		+= at91_can.o
 obj-$(CONFIG_CAN_CC770)		+= cc770/
 obj-$(CONFIG_CAN_C_CAN)		+= c_can/
+obj-$(CONFIG_CAN_CTUCANFD)	+= ctucanfd/
 obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
 obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
 obj-$(CONFIG_CAN_IFI_CANFD)	+= ifi_canfd/
drivers/net/can/ctucanfd/Kconfig new file +18 -0
new file mode 100644
index 000000000000..0620111d57fd
@@ -0,0 +1,18 @@
+config CAN_CTUCANFD
+	tristate "CTU CAN-FD IP core"
+	help
+	  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 FGA based PCI Express board is available
+	  from project (https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd).
+	  More about CAN related projects used and developed at the Faculty
+	  of the Electrical Engineering (http://www.fel.cvut.cz/en/)
+	  of Czech Technical University in Prague (https://www.cvut.cz/en)
+	  at http://canbus.pages.fel.cvut.cz/ .
+
+if CAN_CTUCANFD
+
+endif
drivers/net/can/ctucanfd/Makefile new file +7 -0
new file mode 100644
index 000000000000..8d47008d0076
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Makefile for the CTU CAN-FD IP module drivers
+#
+
+obj-$(CONFIG_CAN_CTUCANFD) := ctucanfd.o
+ctucanfd-y := ctu_can_fd.o ctu_can_fd_hw.o
drivers/net/can/ctucanfd/ctu_can_fd.c new file +1114 -0

1,114 lines not on this page · raw message

drivers/net/can/ctucanfd/ctu_can_fd.h new file +88 -0

88 lines not on this page · raw message

drivers/net/can/ctucanfd/ctu_can_fd_frame.h new file +190 -0

190 lines not on this page · raw message

drivers/net/can/ctucanfd/ctu_can_fd_hw.c new file +781 -0

781 lines not on this page · raw message

drivers/net/can/ctucanfd/ctu_can_fd_hw.h new file +917 -0

788 lines not on this page · raw message

drivers/net/can/ctucanfd/ctu_can_fd_regs.h new file +972 -0

972 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line
2.11.0

Part of the diff is not shown here. View the raw message.

A message that is not in the archive (sent off-list or not yet received): 7360abc6087f63c34acdef6a2bf4b8c8cdbe9aa1.1596408856.git.pisa-/N2ztlQkxE7Ub/[email protected]

build bot
#10 lore raw

Re: [PATCH v4 3/6] can: ctucanfd: add support for CTU CAN FD open-source IP core - bus independent part.

Thank you for the patch! Perhaps something to improve:

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net-next/master sparc-next/master net/master linus/master v5.8 next-20200803] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/pisa-cmp-felk-cvut-cz/CTU-CAN-FD-open-source-IP-core-SocketCAN-driver-PCI-platform-integration-and-documentation/20200804-033027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/net/can/ctucanfd/ctu_can_fd.c: In function 'ctucan_chip_start':

>> drivers/net/can/ctucanfd/ctu_can_fd.c:198:46: warning: variable 'int_enamask_mask' set but not used [-Wunused-but-set-variable]
     198 |  union ctu_can_fd_int_stat int_ena, int_msk, int_enamask_mask;
         |                                              ^~~~~~~~~~~~~~~~

vim +/int_enamask_mask +198 drivers/net/can/ctucanfd/ctu_can_fd.c

186
187 /**
188 * ctucan_chip_start - This the drivers start routine
189 * @ndev: Pointer to net_device structure
190 *
191 * This is the drivers start routine.
192 *
193 * Return: 0 on success and failure value on error
194 */
195 static int ctucan_chip_start(struct net_device *ndev)
196 {
197 struct ctucan_priv *priv = netdev_priv(ndev);

 > 198		union ctu_can_fd_int_stat int_ena, int_msk, int_enamask_mask;

199 int err;
200 struct can_ctrlmode mode;
201
202 netdev_dbg(ndev, "ctucan_chip_start");
203
204 err = ctucan_reset(ndev);
205 if (err < 0)
206 return err;
207
208 priv->txb_prio = 0x01234567;
209 priv->txb_head = 0;
210 priv->txb_tail = 0;
211 priv->p.write_reg(&priv->p, CTU_CAN_FD_TX_PRIORITY, priv->txb_prio);
212
213 err = ctucan_set_bittiming(ndev);
214 if (err < 0)
215 return err;
216
217 err = ctucan_set_data_bittiming(ndev);
218 if (err < 0)
219 return err;
220
221 err = ctucan_set_secondary_sample_point(ndev);
222 if (err < 0)
223 return err;
224
225 /* Enable interrupts */
226 int_ena.u32 = 0;
227 int_ena.s.rbnei = 1;
228 int_ena.s.txbhci = 1;
229
230 int_ena.s.ewli = 1;
231 int_ena.s.fcsi = 1;
232
233 int_enamask_mask.u32 = 0xFFFFFFFF;
234
235 mode.flags = priv->can.ctrlmode;
236 mode.mask = 0xFFFFFFFF;
237 ctucan_hw_set_mode_reg(&priv->p, &mode);
238
239 /* One shot mode supported indirectly via Retransmit limit */
240 if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
241 ctucan_hw_set_ret_limit(&priv->p, true, 0);
242
243 /* Bus error reporting -> Allow Error interrupt */
244 if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) {
245 int_ena.s.ali = 1;
246 int_ena.s.bei = 1;
247 }
248
249 int_msk.u32 = ~int_ena.u32; /* mask all disabled interrupts */
250
251 /* It's after reset, so there is no need to clear anything */
252 ctucan_hw_int_mask_set(&priv->p, int_msk);
253 ctucan_hw_int_ena_set(&priv->p, int_ena);
254
255 /* Controller enters ERROR_ACTIVE on initial FCSI */
256 priv->can.state = CAN_STATE_STOPPED;
257
258 /* Enable the controller */
259 ctucan_hw_enable(&priv->p, true);
260
261 return 0;
262 }
263

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

#11 lore raw

Re: [PATCH v4 3/6] can: ctucanfd: add support for CTU CAN FD open-source IP core - bus independent part.

In reply to [email protected]

Hi!

Quoted 4 lines

More about CAN related projects used and developed at the Faculty of the Electrical Engineering (http://www.fel.cvut.cz/en/)
of Czech Technical University (https://www.cvut.cz/en)
in at Prague http://canbus.pages.fel.cvut.cz/ .

Should this go to Documentation, not a changelog?

drivers/net/can/ctucanfd/Kconfig line 14

Quoted 12 lines
drivers/net/can/ctucanfd/Kconfig new file +12 -0
+	help
+	  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 FGA based PCI Express board is available
+	  from project (https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd).
+	  More about CAN related projects used and developed at the Faculty
+	  of the Electrical Engineering (http://www.fel.cvut.cz/en/)
+	  of Czech Technical University in Prague (https://www.cvut.cz/en)
+	  at http://canbus.pages.fel.cvut.cz/ .

Again, links to university main mage here are little bit excessive.

Quoted 8 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +8 -0
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*******************************************************************************
+ *
+ * CTU CAN FD IP Core
+ * Copyright (C) 2015-2018
+ *
+ * Authors:
+ *     Ondrej Ille <[email protected]>

drivers/net/can/ctucanfd/ctu_can_fd.c line 18

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +10 -0
+ *     Martin Jerabek <[email protected]>
+ *     Jaroslav Beran <[email protected]>
+ *
+ * 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/)

Again, a bit too many links... and important information is missing:
who is the copyright holder?

Quoted 9 lines

+ * 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.

With SPDX, this should be removed.

drivers/net/can/ctucanfd/ctu_can_fd.c line 50

Quoted 3 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +3 -0
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Martin Jerabek");
+MODULE_DESCRIPTION("CTU CAN FD interface");

This normally goes to end of the file, MODULE_AUTHOR usually has <> section with email address.

Quoted 11 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +11 -0
+/* TX buffer rotation:
+ * - when a buffer transitions to empty state, rotate order and priorities
+ * - if more buffers seem to transition at the same time, rotate
+ *   by the number of buffers
+ * - it may be assumed that buffers transition to empty state in FIFO order
+ *   (because we manage priorities that way)
+ * - at frame filling, do not rotate anything, just increment buffer modulo
+ *   counter
+ */
+
+#define CTUCAN_FLAG_RX_FFW_BUFFERED	1

drivers/net/can/ctucanfd/ctu_can_fd.c line 83

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +10 -0
+
+static int ctucan_reset(struct net_device *ndev)
+{
+	int i;
+	struct ctucan_priv *priv = netdev_priv(ndev);
+
+	netdev_dbg(ndev, "ctucan_reset");
+
+	ctucan_hw_reset(&priv->p);
+	for (i = 0; i < 100; ++i) {

i++ would be usual kernel style.

drivers/net/can/ctucanfd/ctu_can_fd.c line 91

Quoted 8 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +8 -0
+		if (ctucan_hw_check_access(&priv->p))
+			return 0;
+		usleep_range(100, 200);
+	}
+
+	netdev_warn(ndev, "device did not leave reset\n");
+	return -ETIMEDOUT;
+}

This does extra sleep after last check_access... but I doubt that matters.

drivers/net/can/ctucanfd/ctu_can_fd.c line 111

Quoted 12 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +12 -0
+static int ctucan_set_bittiming(struct net_device *ndev)
+{
+	struct ctucan_priv *priv = netdev_priv(ndev);
+	struct can_bittiming *bt = &priv->can.bittiming;
+
+	netdev_dbg(ndev, "ctucan_set_bittiming");
+
+	if (ctucan_hw_is_enabled(&priv->p)) {
+		netdev_alert(ndev,
+			     "BUG! Cannot set bittiming - CAN is enabled\n");
+		return -EPERM;
+	}

alert is normally reserved for .. way higher severity.

Quoted 44 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +44 -0
+	/* Note that bt may be modified here */
+	ctucan_hw_set_nom_bittiming(&priv->p, bt);
+
+	return 0;
+}
+
+/**
+ * ctucan_set_data_bittiming - CAN set data bit timing routine
+ * @ndev:	Pointer to net_device structure
+ *
+ * This is the driver set data bittiming routine.
+ * Return: 0 on success and failure value on error
+ */
+static int ctucan_set_data_bittiming(struct net_device *ndev)
+{
+	struct ctucan_priv *priv = netdev_priv(ndev);
+	struct can_bittiming *dbt = &priv->can.data_bittiming;
+
+	netdev_dbg(ndev, "ctucan_set_data_bittiming");
+
+	if (ctucan_hw_is_enabled(&priv->p)) {
+		netdev_alert(ndev,
+			     "BUG! Cannot set bittiming - CAN is enabled\n");
+		return -EPERM;
+	}
+
+	/* Note that dbt may be modified here */
+	ctucan_hw_set_data_bittiming(&priv->p, dbt);
+
+	return 0;
+}
+
+/**
+ * ctucan_set_secondary_sample_point - CAN set secondary sample point routine
+ * @ndev:	Pointer to net_device structure
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int ctucan_set_secondary_sample_point(struct net_device *ndev)
+{
+	struct ctucan_priv *priv = netdev_priv(ndev);
+	struct can_bittiming *dbt = &priv->can.data_bittiming;
+	int ssp_offset = 0;
+	bool ssp_ena;

drivers/net/can/ctucanfd/ctu_can_fd.c line 166

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +10 -0
+
+	netdev_dbg(ndev, "ctucan_set_secondary_sample_point");
+
+	if (ctucan_hw_is_enabled(&priv->p)) {
+		netdev_alert(ndev,
+			     "BUG! Cannot set SSP - CAN is enabled\n");
+		return -EPERM;
+	}
+
+	// Use for bit-rates above 1 Mbits/s

/* */ style comment would be more common here.

Quoted 4 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +4 -0
+	if (dbt->bitrate > 1000000) {
+		ssp_ena = true;
+
+		// Calculate SSP in minimal time quanta

drivers/net/can/ctucanfd/ctu_can_fd.c line 180

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +10 -0
+		ssp_offset = (priv->can.clock.freq / 1000) *
+			      dbt->sample_point / dbt->bitrate;
+
+		if (ssp_offset > 127) {
+			netdev_warn(ndev, "SSP offset saturated to 127\n");
+			ssp_offset = 127;
+		}
+	} else {
+		ssp_ena = false;
+	}

Init ssp_ena to false, and you can get rid of else branch?

Quoted 5 lines

+/**
+ * ctucan_chip_start - This the drivers start routine
+ * @ndev: Pointer to net_device structure
+ *
+ * This is the drivers start routine.

driver's?

Quoted 2 lines

+/**
+ * ctucan_start_xmit - Starts the transmission

...

Quoted 1 line

+ * Return: 0 on success and failure value on error

Umm, no, AFAICT.

Quoted 1 line

+static int ctucan_start_xmit(struct sk_buff *skb, struct net_device *ndev)

Should it return netdev_tx_t ?

Quoted 3 lines

+/**
+ * xcan_rx - Is called from CAN isr to complete the received
+ * frame processing

Double space.

drivers/net/can/ctucanfd/ctu_can_fd.c line 408

Quoted 6 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +6 -0
+static const char *ctucan_state_to_str(enum can_state state)
+{
+	if (state >= CAN_STATE_MAX)
+		return "UNKNOWN";
+	return ctucan_state_strings[state];
+}

Is enum always unsigned?

Quoted 9 lines

+/**
+ * ctucan_err_interrupt - error frame Isr
+ * @ndev: net_device pointer
+ * @isr: interrupt status register value
+ *
+ * This is the CAN error interrupt and it will
+ * check the the type of error and forward the error
+ * frame to upper layers.
+ */

You are free to use 80 columns...

drivers/net/can/ctucanfd/ctu_can_fd.c line 448

Quoted 7 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +7 -0
+	skb = alloc_can_err_skb(ndev, &cf);
+
+	/* EWLI:  error warning limit condition met
+	 * FCSI: Fault confinement State changed
+	 * ALI:  arbitration lost (just informative)
+	 * BEI:  bus error interrupt
+	 */

Extra space before "error"... and something is wrong with big letters there.

Quoted 23 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +23 -0
+		if (state == CAN_STATE_BUS_OFF) {
+			priv->can.can_stats.bus_off++;
+			can_bus_off(ndev);
+			if (skb)
+				cf->can_id |= CAN_ERR_BUSOFF;
+		} else if (state == CAN_STATE_ERROR_PASSIVE) {
+			priv->can.can_stats.error_passive++;
+			if (skb) {
+				cf->can_id |= CAN_ERR_CRTL;
+				cf->data[1] = (berr.rxerr > 127) ?
+						CAN_ERR_CRTL_RX_PASSIVE :
+						CAN_ERR_CRTL_TX_PASSIVE;
+				cf->data[6] = berr.txerr;
+				cf->data[7] = berr.rxerr;
+			}
+		} else if (state == CAN_STATE_ERROR_WARNING) {
+			priv->can.can_stats.error_warning++;
+			if (skb) {
+				cf->can_id |= CAN_ERR_CRTL;
+				cf->data[1] |= (berr.txerr > berr.rxerr) ?
+					CAN_ERR_CRTL_TX_WARNING :
+					CAN_ERR_CRTL_RX_WARNING;
+				cf->data[6] = berr.txerr;

drivers/net/can/ctucanfd/ctu_can_fd.c line 492

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +10 -0
+				cf->data[7] = berr.rxerr;
+			}
+		} else if (state == CAN_STATE_ERROR_ACTIVE) {
+			cf->data[1] = CAN_ERR_CRTL_ACTIVE;
+			cf->data[6] = berr.txerr;
+			cf->data[7] = berr.rxerr;
+		} else {
+			netdev_warn(ndev, "    unhandled error state (%d:%s)!",
+				    state, ctucan_state_to_str(state));
+		}

This sounds like switch (state) to me?

drivers/net/can/ctucanfd/ctu_can_fd.c line 510

Quoted 5 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +5 -0
+	/* Check for Bus Error interrupt */
+	if (isr.s.bei) {
+		netdev_info(ndev, "  bus error");
+		priv->can.can_stats.bus_error++;
+		stats->tx_errors++; // TODO: really?

really? :-).

Quoted 50 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +50 -0
+		some_buffers_processed = false;
+		while ((int)(priv->txb_head - priv->txb_tail) > 0) {
+			u32 txb_idx = priv->txb_tail & priv->txb_mask;
+			u32 status = ctucan_hw_get_tx_status(&priv->p, txb_idx);
+
+			netdev_dbg(ndev, "TXI: TXB#%u: status 0x%x",
+				   txb_idx, status);
+
+			switch (status) {
+			case TXT_TOK:
+				netdev_dbg(ndev, "TXT_OK");
+				can_get_echo_skb(ndev, txb_idx);
+				stats->tx_packets++;
+				break;
+			case TXT_ERR:
+				/* This indicated that retransmit limit has been
+				 * reached. Obviously we should not echo the
+				 * frame, but also not indicate any kind
+				 * of error. If desired, it was already reported
+				 * (possible multiple times) on each arbitration
+				 * lost.
+				 */
+				netdev_warn(ndev, "TXB in Error state");
+				can_free_echo_skb(ndev, txb_idx);
+				stats->tx_dropped++;
+				break;
+			case TXT_ABT:
+				/* Same as for TXT_ERR, only with different
+				 * cause. We *could* re-queue the frame, but
+				 * multiqueue/abort is not supported yet anyway.
+				 */
+				netdev_warn(ndev, "TXB in Aborted state");
+				can_free_echo_skb(ndev, txb_idx);
+				stats->tx_dropped++;
+				break;
+			default:
+				/* Bug only if the first buffer is not finished,
+				 * otherwise it is pretty much expected
+				 */
+				if (first) {
+					netdev_err(ndev, "BUG: TXB#%u not in a finished state (0x%x)!",
+						   txb_idx, status);
+					spin_unlock_irqrestore(&priv->tx_lock,
+							       flags);
+					/* do not clear nor wake */
+					return;
+				}
+				goto clear;
+			}
+			priv->txb_tail++;

drivers/net/can/ctucanfd/ctu_can_fd.c line 692

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +10 -0
+			first = false;
+			some_buffers_processed = true;
+			/* Adjust priorities *before* marking the buffer
+			 * as empty.
+			 */
+			ctucan_rotate_txb_prio(ndev);
+			ctucan_hw_txt_set_empty(&priv->p, txb_idx);
+		}
+clear:
+		spin_unlock_irqrestore(&priv->tx_lock, flags);

Could some part of this be split into separate function?

drivers/net/can/ctucanfd/ctu_can_fd.c line 694

Quoted 1 line
drivers/net/can/ctucanfd/ctu_can_fd.c new file +1 -0
+		/* If no buffers were processed this time, wa cannot

we

drivers/net/can/ctucanfd/ctu_can_fd.c line 701

Quoted 7 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +7 -0
+		 * clear - that would introduce a race condition.
+		 */
+		if (some_buffers_processed) {
+			/* Clear the interrupt again as not to receive it again
+			 * for a buffer we already handled (possibly causing
+			 * the bug log)
+			 */

Not sure this is correct english.

Quoted 6 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +6 -0
+static irqreturn_t ctucan_interrupt(int irq, void *dev_id)
+{
+	struct net_device *ndev = (struct net_device *)dev_id;
+	struct ctucan_priv *priv = netdev_priv(ndev);
+	union ctu_can_fd_int_stat isr, icr;
+	int irq_loops = 0;

drivers/net/can/ctucanfd/ctu_can_fd.c line 777

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +9 -0
+
+	netdev_dbg(ndev, "ctucan_interrupt");
+
+	do {
+		/* Get the interrupt status */
+		isr = ctu_can_fd_int_sts(&priv->p);
 
+		}
+		/* Ignore RI, TI, LFI, RFI, BSI */
+	} while (irq_loops++ < 10000);

For loop would be more usual here.

drivers/net/can/ctucanfd/ctu_can_fd.c line 822

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +10 -0
+static void ctucan_chip_stop(struct net_device *ndev)
+{
+	struct ctucan_priv *priv = netdev_priv(ndev);
+	union ctu_can_fd_int_stat mask;
+
+	netdev_dbg(ndev, "ctucan_chip_stop");
+
+	mask.u32 = 0xffffffff;
+
+	/* Disable interrupts and disable can */

can->CAN?

drivers/net/can/ctucanfd/ctu_can_fd.c line 847

Quoted 8 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +8 -0
+	netdev_dbg(ndev, "ctucan_open");
+
+	ret = pm_runtime_get_sync(priv->dev);
+	if (ret < 0) {
+		netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
+			   __func__, ret);
+		return ret;
+	}

IIRC pm_runtime_get... is special, and you need to put even in the error case.

drivers/net/can/ctucanfd/ctu_can_fd.c line 846

Quoted 5 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +5 -0
+	ret = pm_runtime_get_sync(priv->dev);
+	if (ret < 0) {
+		netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
+			   __func__, ret);
+		return ret;

Same here.

drivers/net/can/ctucanfd/ctu_can_fd.c line 981

Quoted 4 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +4 -0
+int ctucan_suspend(struct device *dev)
+EXPORT_SYMBOL(ctucan_suspend);
+int ctucan_resume(struct device *dev)
+EXPORT_SYMBOL(ctucan_resume);

Exporting these is quite unusual.

drivers/net/can/ctucanfd/ctu_can_fd.c line 986

Quoted 4 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +4 -0
+int ctucan_probe_common(struct device *dev, void __iomem *addr,
+			int irq, unsigned int ntxbufs, unsigned long can_clk_rate,
+			int pm_enable_call, void (*set_drvdata_fnc)(struct device *dev,
+			struct net_device *ndev))

At least format it so that set_drvdata_fnc is on single line?

Quoted 1 line

+{

...

drivers/net/can/ctucanfd/ctu_can_fd.c line 1025

Quoted 2 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +2 -0
+	if (set_drvdata_fnc != NULL)
+		set_drvdata_fnc(dev, ndev);

No need for != NULL.

drivers/net/can/ctucanfd/ctu_can_fd.c line 1030

Quoted 5 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +5 -0
+	SET_NETDEV_DEV(ndev, dev);
+	ndev->netdev_ops = &ctucan_netdev_ops;
+
+	/* Getting the CAN can_clk info */
+	if (can_clk_rate == 0) {

!can_clk_rate would work, too.

Quoted 6 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +6 -0
+		priv->can_clk = devm_clk_get(dev, NULL);
+		if (IS_ERR(priv->can_clk)) {
+			dev_err(dev, "Device clock not found.\n");
+			ret = PTR_ERR(priv->can_clk);
+			goto err_free;
+		}

drivers/net/can/ctucanfd/ctu_can_fd.c line 1046

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +10 -0
+		can_clk_rate = clk_get_rate(priv->can_clk);
+	}
+
+	priv->p.write_reg = ctucan_hw_write32;
+	priv->p.read_reg = ctucan_hw_read32;
+
+	if (pm_enable_call)
+		pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {

put?

Quoted 5 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +5 -0
+	if ((priv->p.read_reg(&priv->p, CTU_CAN_FD_DEVICE_ID) &
+			    0xFFFF) != CTU_CAN_FD_ID) {
+		priv->p.write_reg = ctucan_hw_write32_be;
+		priv->p.read_reg = ctucan_hw_read32_be;
+		if ((priv->p.read_reg(&priv->p, CTU_CAN_FD_DEVICE_ID) &

drivers/net/can/ctucanfd/ctu_can_fd.c line 1066

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +10 -0
+			      0xFFFF) != CTU_CAN_FD_ID) {
+			netdev_err(ndev, "CTU_CAN_FD signature not found\n");
+			ret = -ENODEV;
+			goto err_disableclks;
+		}
+	}
+
+	ret = ctucan_reset(ndev);
+	if (ret < 0)
+		goto err_pmdisable;

Should be goto err_disableclks, AFAICT. Plus that label is quite confusing.

Quoted 5 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +5 -0
+static __init int ctucan_init(void)
+{
+	pr_info("%s CAN netdevice driver\n", DRV_NAME);
+
+	return 0;

drivers/net/can/ctucanfd/ctu_can_fd.c line 1114

Quoted 10 lines
drivers/net/can/ctucanfd/ctu_can_fd.c new file +9 -0
+}
+
+module_init(ctucan_init);
 
+static __exit void ctucan_exit(void)
+{
+	pr_info("%s: driver removed\n", DRV_NAME);
+}
+
+module_exit(ctucan_exit);

Remove?

drivers/net/can/ctucanfd/ctu_can_fd_hw.c line 35

Quoted 5 lines
drivers/net/can/ctucanfd/ctu_can_fd_hw.c new file +5 -0
+#ifndef __KERNEL__
+# include "ctu_can_fd_linux_defs.h"
+#else
+# include <linux/can/dev.h>
+#endif

Should always assume kernel?

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author maintainer
#12 lore raw

PATCH v4 4/6 can: ctucanfd: CTU CAN FD open-source IP core - PCI bus support.

In reply to [email protected]

From: Pavel Pisa <[email protected]>

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

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-ctu_can_fd .

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          |   9 +
 drivers/net/can/ctucanfd/Makefile         |   3 +
 drivers/net/can/ctucanfd/ctu_can_fd_pci.c | 316 ++++++++++++++++++++++++++++++
 3 files changed, 328 insertions(+)
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_pci.c
drivers/net/can/ctucanfd/Kconfig +9 -0
index 0620111d57fd..8a5f5d05fa72 100644
@@ -15,4 +15,13 @@ config CAN_CTUCANFD
 
 if CAN_CTUCANFD
 
+config CAN_CTUCANFD_PCI
+    tristate "CTU CAN-FD IP core PCI/PCIe driver"
+    depends on PCI
+	help
+	  This driver adds PCI/PCIe support for CTU CAN-FD IP core.
+	  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-ctu_can_fd .
+
 endif
drivers/net/can/ctucanfd/Makefile +3 -0
index 8d47008d0076..eb945260952d 100644
@@ -5,3 +5,6 @@
 
 obj-$(CONFIG_CAN_CTUCANFD) := ctucanfd.o
 ctucanfd-y := ctu_can_fd.o ctu_can_fd_hw.o
+
+obj-$(CONFIG_CAN_CTUCANFD_PCI) += ctucanfd_pci.o
+ctucanfd_pci-y := ctu_can_fd_pci.o
drivers/net/can/ctucanfd/ctu_can_fd_pci.c new file +316 -0

316 lines not on this page · raw message

Signature · 1 line
2.11.0
reviewer
#13 lore raw

Re: [PATCH v4 4/6] can: ctucanfd: CTU CAN FD open-source IP core - PCI bus support.

drivers/net/can/ctucanfd/Kconfig config CAN_CTUCANFD · line 21

Quoted 12 lines · On 8/3/20 11:34 AM, [email protected] wrote:
drivers/net/can/ctucanfd/Kconfig +4 -0
index 0620111d57fd..8a5f5d05fa72 100644
@@ -15,4 +15,13 @@ config CAN_CTUCANFD
 
 if CAN_CTUCANFD
 
+config CAN_CTUCANFD_PCI
+    tristate "CTU CAN-FD IP core PCI/PCIe driver"
+    depends on PCI
+	help

"help" should be indented with one tab only (no spaces).

drivers/net/can/ctucanfd/Kconfig config CAN_CTUCANFD · line 25

Quoted 4 lines
drivers/net/can/ctucanfd/Kconfig +4 -0
+	  This driver adds PCI/PCIe support for CTU CAN-FD IP core.
+	  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-ctu_can_fd .

help text should be indented with one tab + 2 spaces according to Documentation/process/coding-style.rst.

drivers/net/can/ctucanfd/Kconfig config CAN_CTUCANFD · line 27

Quoted 2 lines
drivers/net/can/ctucanfd/Kconfig +1 -0
+
 endif

thanks.

Signature · 1 line
~Randy

A message that is not in the archive (sent off-list or not yet received): bad059402032f82fa63aa51d2122589a8a2cf6fd.1596408856.git.pisa-/N2ztlQkxE7Ub/[email protected]

build bot
#15 lore raw

Re: [PATCH v4 4/6] can: ctucanfd: CTU CAN FD open-source IP core - PCI bus support.

Thank you for the patch! Perhaps something to improve:

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net-next/master sparc-next/master net/master linus/master v5.8 next-20200803] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/pisa-cmp-felk-cvut-cz/CTU-CAN-FD-open-source-IP-core-SocketCAN-driver-PCI-platform-integration-and-documentation/20200804-033027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/net/can/ctucanfd/ctu_can_fd_pci.c:79:31: warning: no previous prototype for 'ctucan_pci_get_bdata' [-Wmissing-prototypes]

79 | struct ctucan_pci_board_data *ctucan_pci_get_bdata(struct pci_dev *pdev)
| ^~~~~~~~~~~~~~~~~~~~

vim +/ctucan_pci_get_bdata +79 drivers/net/can/ctucanfd/ctu_can_fd_pci.c

    78	
  > 79	struct ctucan_pci_board_data *ctucan_pci_get_bdata(struct pci_dev *pdev)
    80	{
    81		return (struct ctucan_pci_board_data *)pci_get_drvdata(pdev);
    82	}
    83

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

author maintainer
#16 lore raw

PATCH v4 5/6 can: ctucanfd: CTU CAN FD open-source IP core - platform/SoC support.

In reply to [email protected]

From: Pavel Pisa <[email protected]>

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/b182/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               |  11 ++
 drivers/net/can/ctucanfd/Makefile              |   3 +
 drivers/net/can/ctucanfd/ctu_can_fd_platform.c | 145 +++++++++++++++++++++++++
 3 files changed, 159 insertions(+)
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_platform.c
drivers/net/can/ctucanfd/Kconfig +11 -0
index 8a5f5d05fa72..c0155d659373 100644
@@ -24,4 +24,15 @@ config CAN_CTUCANFD_PCI
 	  PCIe board with PiKRON.com designed transceiver riser shield is available
 	  at https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd .
 
+config CAN_CTUCANFD_PLATFORM
+	tristate "CTU CAN-FD IP core platform (FPGA, SoC) driver"
+	depends on OF
+	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 .
+
 endif
drivers/net/can/ctucanfd/Makefile +3 -0
index eb945260952d..a77ca72d534e 100644
@@ -8,3 +8,6 @@ ctucanfd-y := ctu_can_fd.o ctu_can_fd_hw.o
 
 obj-$(CONFIG_CAN_CTUCANFD_PCI) += ctucanfd_pci.o
 ctucanfd_pci-y := ctu_can_fd_pci.o
+
+obj-$(CONFIG_CAN_CTUCANFD_PLATFORM) += ctucanfd_platform.o
+ctucanfd_platform-y += ctu_can_fd_platform.o
drivers/net/can/ctucanfd/ctu_can_fd_platform.c new file +145 -0

145 lines not on this page · raw message

Signature · 1 line
2.11.0
author maintainer
#17 lore raw

PATCH v4 6/6 docs: ctucanfd: CTU CAN FD open-source IP core documentation.

In reply to [email protected]

From: Pavel Pisa <[email protected]>

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/FSM_TXT_Buffer_user.png     | Bin 0 -> 174807 bytes
 .../device_drivers/ctu/ctucanfd-driver.rst         | 635 +++++++++++++++++++++
 2 files changed, 635 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/ctu/FSM_TXT_Buffer_user.png
 create mode 100644 Documentation/networking/device_drivers/ctu/ctucanfd-driver.rst
Documentation/networking/device_drivers/ctu/FSM_TXT_Buffer_user.png new file binary +0 -0
new file mode 100644
index 0000000000000000000000000000000000000000..5dceb594fca1f668fbc2eb7c1fb5706e99137d28
GIT binary patch
Documentation/networking/device_drivers/ctu/ctucanfd-driver.rst new file +635 -0

635 lines not on this page · raw message

Signature · 1 line
2.11.0
maintainer
#18 lore raw
In reply to [email protected]
Quoted 13 lines · On Mon, 3 Aug 2020 20:34:48 +0200 [email protected] wrote:

From: Pavel Pisa <[email protected]>

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-ctu_can_fd).
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

Quoted 4 lines

More about CAN related projects used and developed at the Faculty of the Electrical Engineering (http://www.fel.cvut.cz/en/)
of Czech Technical University (https://www.cvut.cz/en)
in Prague at http://canbus.pages.fel.cvut.cz/ .

Patches 3 and 4 have warnings when built with W=1 C=1 flags.

Please also remove the uses of static inline in C sources. Those are rarely necessary and hide unused code warnings.