Re: ram initialization on mcp2518fd

* Re: ram initialization on mcp2518fd
       [not found]   ` <441514ec-7b43-e11c-09b5-bdaf7fca0077@prevas.dk>
@ 2022-06-16 11:21     ` Rasmus Villemoes
  2022-06-16 12:40       ` Marc Kleine-Budde
  0 siblings, 1 reply; 9+ messages in thread
From: Rasmus Villemoes @ 2022-06-16 11:21 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: info, linux-can

[resending without image attached so that it makes it to the linux-can list]

On 16/06/2022 12.11, Marc Kleine-Budde wrote:
> Hello Rasmus,
>
> can we move this discussion to the Linux-CAN mailing list
> (linux-can@vger.kernel.org)?

Absolutely. Cc'ed.

> On 16.06.2022 11:13:41, Rasmus Villemoes wrote:
>> I have a board based on imx8mp with two mcp2518fd devices (sitting on
>> two different spi busses). I'm having some trouble getting them to
>> work.
>
> The ecspi or the qspi?
>

ecspi

>> A logic analyzer shows that the spi communication works (mostly) fine
>> [1], up until mcp251xfd_chip_ecc_init(). In there, it seems that the
>> regmap_raw_write() ends up trying to do byte-by-byte transfers - i.e.,
>> the chip select is only asserted for 8 clock cycles, then deasserted,
>> and asserted again for the next byte.
>
> What about the other transfers done before the
> mcp251xfd_chip_ecc_init()? Do the look correct?

Yes. For example the very first write done from mcp251xfd_chip_start()
-> mcp251xfd_chip_softreset{,_do}() -> mcp251xfd_chip_clock_enable()
looks like https://ibb.co/xMbdsSq , and the MOSI signal looks right (0xa
= crc
write to 0xe00 aka OSC, 4 value bytes, the 0x00000060 value in little
endian, and the crc partly off-screen).

>> Unfortunately, I can't physically put more than two probes on at a
>> time, but I have captured (clk, mosi) at one point and (clk, cs) at
>> another, and in both cases one sees these 8-clock bursts, which should
>> never appear with this device.
>
> ACK
>
>> Obviously, that means that the RAM doesn't get initialized, because
>> all the device sees is a bunch of aborted spi transactions. I can't
>> really figure out why this happens, maybe I'm missing some setting in
>> DT or elsewhere; currently I have
>
> Can you show me your SPI host controller node, too?

Yes, it's essentially the one from imx8mp.dtsi, i.e.

	ecspi1: spi@30820000 {
		#address-cells = <1>;
		#size-cells = <0>;
		compatible = "fsl,imx8mp-ecspi", "fsl,imx6ul-ecspi";
		reg = <0x30820000 0x10000>;
		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&clk IMX8MP_CLK_ECSPI1_ROOT>,
			 <&clk IMX8MP_CLK_ECSPI1_ROOT>;
		clock-names = "ipg", "per";
		assigned-clock-rates = <80000000>;
		assigned-clocks = <&clk IMX8MP_CLK_ECSPI1>;
		assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>;
		dmas = <&sdma1 0 7 1>, <&sdma1 1 7 2>;
		dma-names = "rx", "tx";
		status = "disabled";
	};

with this in the board .dts:

&ecspi1 {
	status = "okay";
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_ecspi1>;
	cs-gpios = <&gpio5 9 GPIO_ACTIVE_LOW>;

	CAN_C: spi@0 {
		reg = <0>;
		compatible = "microchip,mcp2518fd";
		/* clocks = <&cdce937 6>; */
		clocks = <&clk_40MHz>;
		interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_LOW>;
		spi-max-frequency = <10000000>;
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_CAN_C>;
	};
};

It's currently (for bad reasons) the nxp kernel, hence the
fsl,imx6ul-ecspi compatible, but changing that to the one in mainline,
fsl,imx51-ecspi, doesn't change the symptoms.

>> 	CAN_C: spi@0 {
>> 		reg = <0>;
>> 		compatible = "microchip,mcp2518fd";
>> 		clocks = <&clk_40MHz>;
>> 		interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_LOW>;
>> 		spi-max-frequency = <17000000>;
>> 		pinctrl-names = "default";
>> 		pinctrl-0 = <&pinctrl_CAN_C>;
>> 	};
>>
>> with the pinctrl being for the irq gpio. I have also tried lowering
>> spi-max-frequency to 10MHz (currently it ends up effectively using
>> 16MHz),
>
> The driver uses 85% of clock/2 or spi-max-frequency, whatever is lower.

Yes; using 10MHz was just an attempt to see if using an even lower
frequency than the maximum implied by the errata (and implemented in the
driver) would make a difference.

>> [1] One thing I have noticed is that we probably want to do
>>
>> -       xfer[1].len = sizeof(dev_id);
>> +       xfer[1].len = sizeof(*dev_id);
>
> Fixed. Do you want to appear as Reported-by in the patch?

Yes please.

>> But: the reading doesn't seem to work; it's as if the device doesn't
>> drive MISO at all at this point, because when I configure the pin with
>> a weak internal pull-up, I get 0xffffffff, while if I use pinmux
>> settings with a weak pull-down I get 0 - which is the "correct"
>> answer, but probably not for the right reason.
>
> The device id is read after detecting the chip and the
> mcp251xfd_chip_ecc_init(). Does the chip detection work properly?

It appears so, adding a print of the osc variable after the
regmap_read() in mcp251xfd_register_chip_detect() gives

mcp251xfd_register_chip_detect: osc = 0x00000468

so the LPMEN bit seems to have been set (and the other bits show
expected values). And it's true that the dev_id is read after this,
through mcp251xfd_register_done(), which is why I'm puzzled that it
doesn't seem to work.

Are you sure about the ecc_init() part? AFAICT that's only called from
chip_start, which in turn is only called when the device is brought up.

>> I'm on a 5.10.y kernel, but I don't see any commits related to this
>> since then (other than the error handling in
>> mcp251xfd_register_get_dev_id()).
>
> Which kernel are you using exactly? Mainline or freescale/nxp
> downstream?

Currently NXP,  lf-5.10.72-2.2.0 aka a68e31b63f86. I'll see if I can
manage to make a mainline kernel boot.

> Have you enabled DMA on SPI?

Not explicitly, but nor have I done anything to disable/not enable it,
so I'm not really sure what the right answer is. Is that a CONFIG_ knob
or module/kernel parameter?

Thanks,
Rasmus

^ permalink raw reply	[flat|nested] 9+ messages in thread