On 20/06/2022 12.27, Marc Kleine-Budde wrote:
> On 20.06.2022 09:09:09, Rasmus Villemoes wrote:
>>>> when I start the traffic test. And the dev_id reading still doesn't
>>>> work (though it's not really used for anything other than an
>>>> informative printk).
>>>
>>> What does it read on your board? But still that transfer should work.
>>
>> Depending on whether the miso pin has been configured with a weak
>> internal pull-up or pull-down, it reads either 0xff or 0x00 - and
>> that's also the case when I expand the read to all six e00 through e14
>> registers. So the chip isn't really driving miso, and I think that's
>> because CS is released between the two elements of the xfer array.
>
> Can you do a measurement of that MCP251XFD_REG_DEVID transfer?
>
> For testing I read the OSC register instead of the DEVID and print the
> value:
>
> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> index 537335d42d1d..79de59f9c145 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> @@ -1796,13 +1796,14 @@ mcp251xfd_register_get_dev_id(const struct mcp251xfd_priv *priv, u32 *dev_id,
> xfer[1].len = sizeof(dev_id);
> xfer[1].speed_hz = priv->spi_max_speed_hz_fast;
>
> - mcp251xfd_spi_cmd_read_nocrc(&buf_tx->cmd, MCP251XFD_REG_DEVID);
> + mcp251xfd_spi_cmd_read_nocrc(&buf_tx->cmd, MCP251XFD_REG_OSC);
>
> err = spi_sync_transfer(priv->spi, xfer, ARRAY_SIZE(xfer));
> if (err)
> goto out_kfree_buf_tx;
>
> *dev_id = be32_to_cpup((__be32 *)buf_rx->data);
> + pr_info("%s: reg_osc=0x%08x\n", __func__, *dev_id);
> *effective_speed_hz_slow = xfer[0].effective_speed_hz;
> *effective_speed_hz_fast = xfer[1].effective_speed_hz;
>
> And it gives me:
>
> | [ 171.051636] mcp251xfd_register_get_dev_id: reg_osc=0x68040000
>
> I just noticed the register contents isn't big endian, it's little
> endian. I'll send a patch. (The address information is big endian, and
> mcp251xfd_spi_cmd_read_nocrc is taking care of this.)
So the problem was that I was using native chip select, i.e. my pinmux
setting was
MX8MP_IOMUXC_ECSPI1_SS0__ECSPI1_SS0 0x00000144
and that means the controller only asserts the CS line for the duration
of one burst; so when the spi message contains two transfers, it
obviously breaks as the device saw the release of CS after the two
command/address bytes as the end of the transaction [then, from the
device's POV another begins, but there MOSI is all 0, so that may get
interpreted as a reset command, or perhaps it's ignored because it's not
precisely 16 0 bits - I wonder how the hardware designers thought
all-zeros was a good idea for a reset command].
Switching to gpio, i.e.
MX8MP_IOMUXC_ECSPI1_SS0__GPIO5_IO09 0x00000144
means CS is held low for the whole message, and I now read the expected
contents.
====
It's probably not anything to do with the mcp2518fd driver and this is
straying somewhat from the original problem, but I see that CS is held
low for a very long time after the last byte has been shifted in/out, on
the order of 0.5ms. https://ibb.co/n1Hq3YR shows the first write to OSC,
and zooming out shows https://ibb.co/12Z1YkM (or just looking at the
mouse-over info in the first one).
Similarly, there's a rather large gap between the two spi_transfers in
the case of the reading of dev_id (but, per the above, with CS correctly
held throughout): https://ibb.co/pyG1y96 .
I'm not sure if this is to be expected.
Rasmus
