| Message ID | cover.1604095004.git.pisa@cmp.felk.cvut.cz (mailing list archive) |
|---|---|
| Headers | show |
| Series | CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation | expand |
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
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
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
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
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
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 <jara.beran@gmail.com> 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 <charvj10@fel.cvut.cz> 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