[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@xxxxxxxxxxxxxxx)?
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
