Implementing MCP2517FD module on Verdin

spasoye · February 24, 2021, 6:01pm

@Edward big thanks for the explanation, that really makes sense now.

This is my current external CAN node definition and Im still having the same errors in dmesg:

&pinctrl_gpio4{
       /* changing gpio5_27 pin settings */ 
	fsl,pins = <
		MX8MM_IOMUXC_UART3_TXD_GPIO5_IO27		0x1c4	/* SODIMM 212 */
	>;
};

&ecspi2 {
	#address-cells = <1>;
	#size-cells = <0>;
	cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
	/* This property is required, even if marked as obsolete in the doku */
	fsl,spi-num-chipselects = <1>;
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_ecspi2>;
	status = "okay";
	
	can3: can@0 {
		compatible = "microchip,mcp2517fd";
		clocks = <&clk20m>;
		gpio-controller;
		/* dev board gpio4 pin -> gpio5_27 */
		interrupt-parent = <&gpio5>;
		interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
		microchip,clock-allways-on;
		microchip,clock-out-div = <1>;
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_gpio4>;
		reg = <0>;
		spi-max-frequency = <2000000>;
		status = "okay";
	};
};

Edward · February 24, 2021, 6:30pm

I opened relevant imx8mm-verdin.dtsi to comment regarding can@ settings interrupt-parent, interrupts and pinctrl-0.

Which pin actually you connected to MCP251x INT? pinctrl_can2_int in above mentioned dtsi is MX8MM_IOMUXC_GPIO1_IO07_GPIO1_IO7. GPIO interrupt-parent should match that and you should have &gpio1 there. Interrupts setting should as well match GPIO1 IO line, which is 7 and not 14. IRQ type for MCP251x should be level sensitive, not edge. I have GPIO_ACTIVE_LOW, which evaluates to the same number as IRQ_TYPE_EDGE_FALLING Linux include’s, though clearly it should be level sensitive, not edge. In many examples where interrupt parent is GPIO I see GPIO_xxx in interrupts, not IRQ_XXX. Well, endless learning, yet another thing to figure.

Edward

spasoye · February 24, 2021, 7:18pm

I used pinctrl_gpio4 (MX8MM_IOMUXC_UART3_TXD_GPIO5_IO27).

can2 and can1 node in imx8mm-verdin-v1.1.dtsi are both edge sensitive I decided to follow that. I noticed that it is more reliable when using IRQ_TYPE_LEVEL_LOW,

irq.h states:

#define IRQ_TYPE_EDGE_FALLING	2

gpio.h:

#define GPIO_ACTIVE_LOW 1

Edward · February 24, 2021, 7:47pm

Yes, it seems I noticed it bad in includes.

Yeah, looks like I followed bad example, it should be IRQ_TYPE_LEVEL_LOW, not GPIO_ACTIVE_LOW. I wonder how did it work at all, but I didn’t loose single message in my heavy load tests, just quite high CPU load, which I thought is OK for VF61. I’ll check what happens with right setting. It may explain why Marc Kleine-Budde driver didn’t work for me.

Thanks,

Edward

andrecurvello.tx · February 24, 2021, 8:17pm

Hi @Ralph / @spasoye,

Judging by your CAN Controller, it’s using a 40 MHz oscillator, instead of the 20 MHz we reference in our DTS.

That said, you need to reference a 40 MHz oscillator for it.

You can do that by the following:

/* fixed clock dedicated to 40MHz SPI CAN controller */
clk40m: oscillator {
       compatible = "fixed-clock";
        #clock-cells = <0>;
        clock-frequency = <40000000>;
};

....

can3: ... {
   clocks = <&clk40m>;
...

Can you try with this setup?

Edward · February 24, 2021, 8:19pm

@spasoye,

My MCP2518FD click board came with clock set to 40MHz. Like I see on your picture it is 40MHz as well. I DT you have clocks set to 20MHz.

Edward

Ralph · February 25, 2021, 8:07am

Hi @andrecurvello.tx ,
we are using the module with a 20MHz clock as it can be configured with a jumper between 40 and 20MHz. Therefore the 20MHz setting should be fine.

andrecurvello.tx · February 25, 2021, 1:50pm

Hi @Ralph and @spasoye , can you send me your complete device-tree file?

spasoye · February 25, 2021, 2:22pm

@andrecurvello.tx you are right, jumper on my CAN controller is set to 40MHz.
I added this to root node:
clk40m: oscillator-1 {
compatible = “fixed-clock”;
#clock-cells = <0>;
clock-frequency = <40000000>;
};

And changed can3 clocks property to clk40m. But I still get:
mcp25xxfd spi1.0 can1: CAN Bus error experienced

Do you think that maybe the problem is with INT pin configuration (interrupt pin on MCP) ? Is there a way to check if interrupt on gpio5_27 is generated in linux?

Edward · February 25, 2021, 2:38pm

@spasoye ,

Since you don’t get CRC error from driver - SPI comms are OK. Bus OFF is the sign that you have no proper wiring to your CAN communication partners, or they are operating at different bit rates.
Proper interrupt will be required to receive messages, you can send message without it.
Since you have the scope attached, disconnect your device from CAN bus, try sending some message via MCP2517 and take waveform. Find the minimum time gap between two adjacent edges. 1/dt will tell you arbitration bitrate your MCP2517 operates at. Something like that could be used to determine data bit rate, but you need to first establish proper connection at arbitration rate.
BTW how did you connect click board to CAN bus? Just CAN-L and CAN-H or indeed GND as well? GND is required.

spasoye · February 25, 2021, 4:04pm

@Edward I discovered stupid problem, 5V wire to MCP module was disconnected. It works now, I had no need for GND and 5V.

spasoye · February 25, 2021, 4:15pm

@andrecurvello.tx @Edward @Ralph I finally implemented exetrnal CAN. Thanks for your help.
This is my wiring (from up above) that I used:

And here is my modified .dts, @andrecurvello.tx I will eventualy convert this to overlay as you already mentioned, @Edward I left edge trigger for now:

// SPDX-License-Identifier: GPL-2.0+ OR MIT
/*
 * Copyright 2019 Toradex
 */

/dts-v1/;

#include "imx8mm-verdin.dtsi"
#include "imx8mm-verdin-wifi.dtsi"
#include "imx8mm-verdin-dev.dtsi"
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/clock/imx8mm-clock.h>

/ {
	model = "Toradex Verdin iMX8M Mini WB on Verdin Development Board";
	compatible = "toradex,verdin-imx8mm-wifi-dev",
		     "toradex,verdin-imx8mm-wifi",
		     "toradex,verdin-imx8mm",
		     "fsl,imx8mm";

	clk40m: oscillator-1 {
		compatible = "fixed-clock";
		#clock-cells = <0>;
		clock-frequency = <40000000>;
	};
};

&iomuxc {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_gpio3>, <&pinctrl_gpio7>,
		    <&pinctrl_gpio8>, <&pinctrl_gpio_hog1>,
		    <&pinctrl_gpio_hog2>, <&pinctrl_gpio_hog3>,
		    <&pinctrl_sai5>, <&pinctrl_pmic_tpm_ena>;
};

&pinctrl_gpio4{
	fsl,pins = <
		MX8MM_IOMUXC_UART3_TXD_GPIO5_IO27		0x1c4	/* SODIMM 212 */
	>;
};

&spidev20 {
	status = "disabled";
};

&ecspi2 {
	#address-cells = <1>;
	#size-cells = <0>;
	cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
	/* This property is required, even if marked as obsolete in the doku */
	fsl,spi-num-chipselects = <1>;
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_ecspi2>;
	status = "okay";

	can3: can@0 {
		compatible = "microchip,mcp2517fd";
		clocks = <&clk40m>;
		gpio-controller;
		/* dev board gpio4 pin -> gpio5_27 */
		interrupt-parent = <&gpio5>;
		interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
		microchip,clock-allways-on;
		microchip,clock-out-div = <1>;
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_gpio4>;
		reg = <0>;
		spi-max-frequency = <2000000>;
		status = "okay";
	};
};

Edward · February 25, 2021, 4:16pm

Congratulations!

andrecurvello.tx · February 25, 2021, 5:07pm

I’m glad that this was a very simple solution.

Either way, we managed to set and discuss very useful information here.

Thanks for the support, @Edward.

Edward · February 25, 2021, 5:47pm

Still some words to add. I don’t want to disturb you further, just try paying attention when you have time:

There’s not small difference between IRQ_TYPE_EDGE_FALLING and IRQ_TYPE_LEVEL_LOW. I think driver should match MCP251xFD interrupt output type, which is active low.
spi-max-frequency may be too low for busier bus, as well lower SPI clock means longer delay from event on CAN bus Top allowed MCP2518FD SPI clock is 20MHz, not 2.
As I understand, since there up to 2 identical MCP251x’s on Verdin, Toradex DT uses GPIO for chip select. ECSPI is limited to one CS, so emulating 2 CS’s using GPIO’s is way to go without utilizing 2nd ECPI. Unless there are problems in ECSPI driver, automatic HW chip select should be used when single SPI slave is connected to one ECSPI. According to RM automatic CS is good up to 4096 bits long SPI messages, which is more than enough for MCP251xFD. If you are interested, cs-gpios in DT tells driver to toggle specific GPIOs instead of using automatic CS. As well have a look at IOMUX:
```
pinctrl_ecspi2: ecspi2grp {
	fsl,pins = <
		MX8MM_IOMUXC_ECSPI2_SCLK_ECSPI2_SCLK		0x4	/* SODIMM 196 */
		MX8MM_IOMUXC_ECSPI2_MOSI_ECSPI2_MOSI		0x4	/* SODIMM 200 */
		MX8MM_IOMUXC_ECSPI2_MISO_ECSPI2_MISO		0x1c4	/* SODIMM 198 */
		MX8MM_IOMUXC_ECSPI2_SS0_GPIO5_IO13		0x1c4	/* SODIMM 202 */
	>;
};
```

For automatic CS SS0 line should be MX8MM_IOMUXC_ECSPI2_SS0_ECSPI2_SS0. You may check defines in imx8mm-pinfunc.h.

I’m off, sorry.

Regards,
Edward

andrecurvello.tx · February 27, 2021, 1:10am

Hi @Edward,

Good observations!

Thanks.

Best regards,
André Curvello

Ralph · March 10, 2021, 11:08am

Hi @andrecurvello.tx ,

I ran into the CAN bus errors due to missing nodes on the network.

I thought a termination on the wire should be enough for proper transmission, but it is not. It also works for me now. Thanks a lot for your support!

Another thing that I found out is that the SPI communication does not work with the SPI hardware chip-select signal MX8MM_IOMUXC_ECSPI2_SS0_ECSPI2_SS0. If I try to use this, I get CRC single/double bit errors in the SPI communication.

Instead, I need to use the GPIO based chip-select that works fine for me now.
Do you know the reason for that issue?

I also have other SPI slaves in use and they work fine with the hardware chip-selects on the same IMX8…

Edward · March 10, 2021, 2:45pm

Hi @Ralph

Please don’t mix CRC with ECC. CRC is checksum calculation to verify data sent/received over SPI to MCP2517/18. It can’t have single or double bit errors, only OK or KO. Some MCP25xx commands include CRC sending to master or expect CRC to be added while receiving data from master. And ECC is internal MCP2517/18 RAM contents protection.

Hardware CS works well for me on VF61/VF50, but DSPI is there, not ECSPI. For some reason hardware CS on iMX6ULL and iMX7D leads to MCP2518 ECC errors. Looks like initial fill-all-MCP2518-data-RAM procedure fails, which is used to initialize RAM ECC bits. Driver initializes ECC bits by filling all the RAM. 256 bytes of dummy bytes, then next 256 bytes, etc until all RAM is written. Reducing SPI clock rate didn’t help. But it helped to disable ECSPI DMA!

If I initialize ECC RAM using non HW-CS or DMA=off DT, and not power cycle MCP2518FD, then HW-CS + DMA seem working.. until the big burst of TX messages, which makes driver reporting TEF errors…

Edward

andrecurvello.tx · March 10, 2021, 6:12pm

Hi @Ralph

Do you have an Oscilloscope with you over there?

Could you “watch” the MX8MM_IOMUXC_ECSPI2_SS0_ECSPI2_SS0 signal during the exercise of the CAN Controller?

Can you also show us the errors you receive when using this ChipSelect signal?

Best regards,
André Curvello

andrecurvello.tx · March 10, 2021, 6:14pm

Good observation @Edward.

I’ll discuss those points internally.

Best regards,
André Curvello