M_CAN message RAM initialization AppNote - was: Re: [PATCH V3 3/3] can: m_can: workaround for transmit data less than 4 bytes
b29396 at freescale.com
Fri Nov 7 00:34:50 PST 2014
On Thu, Nov 06, 2014 at 01:33:56PM +0100, Oliver Hartkopp wrote:
> On 06.11.2014 09:09, Dong Aisheng wrote:
> >On Thu, Nov 06, 2014 at 08:04:17AM +0100, Oliver Hartkopp wrote:
> >>To prevent the M_CAN controller detecting checksum errors when
> >>reading potentially uninitialized TX message RAM content to transmit
> >>CAN frames the TX message RAM has to be written with (any kind of)
> >>initial data.
> >The key point of the issue is that why M_CAN will read potentially uninitialized
> >TX message RAM content which should not happen?
> >e.g. for our case of the issue, if we sending a no data frame or a less
> >than 4 bytes frame, why m_can will read extra 4 bytes uninitialized/unset
> >data which seems not reasonable?
> > From IP code logic, it will read full 8 bytes of data no matter how many data
> >actually to be transfered which is strange.
> >For sending data over 4 bytes, since the Message RAM content will be filled
> >with the real data to be transfered so there's no such issue.
> E.g. think about the transfer of a CAN FD frame with 32 byte.
> When you only fill up content until 28 byte the last four bytes
> still remain uninitialized.
> Did you try this 28 byte use-case with an uninitialized TX message RAM ?
> cansend can0 123##1001122334566778899AABBCCDDEEFF001122334566778899AABB
> To me it looks too risky when we only initialize the first 8 byte.
I tried 28 byte case with two MX6SX SDB board and it worked.
root at imx6sxsabresd:~# cansend can0 123##1001122334566778899AABBC566778899AABB334
root at imx6sxsabresd:~# candump -x can0
can0 RX B - 123  00 11 22 33 45 66 77 88 99 AA BB CC DD EE FF 00 11 22 33 45 66 77 88 99 AA BB 00 00 00 00 00 00
I think this is mainly because the driver will ensure to write
the full 32 bytes to Message RAM even we only fill up content of
28 bytes. The remain 4 bytes written to M_RAM are default 0.
This seems avoid the possibility of reading uninitialized TX message RAM
The code is done as follows:
for (i = 0; i < cf->len; i += 4)
m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i / 4),
*(u32 *)(cf->data + i));
cf->len will be rounded to 32 in cansend.
> >>Then the code should memset() the entire TX FIFO element - and not
> >>only the 8 data bytes we are addressing now.
> >Our IC guy told me the issue only happened on transferring a data size
> >of less than 4 bytes and my test also proved that.
> 'less than'?
As i said before, from IP code logic, M_CAN will read extra data bytes
from TX buffer only for sending data less than 4 bytes.
cansnd can0 123#
cansnd can0 123#112233
Both case will read the full 8 byte from TX buffer even it sends no data
and a 3 bytes data.
cansnd can0 123#1122334455
it read 8 bytes
cansnd can0 123##1112233445566778899001122
it read 12 bytes.
No extra uninitialized data read.
> So you might try to use 26 bytes too:
> cansend can0 123##1001122334566778899AABBCCDDEEFF001122334566778899
It works too.
> >So i'm not sure memset() the entire TX FIFO element is neccesary...
> It's no big deal - so we should be defensive here.
> And memset() is not working as Marc pointed out in another mail.
> So we would need to loop with
> m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i), 0x0);
This simple loop may not work.
m_can_fifo_write is only for Tx Buffer.
Since Message RAM may be shared, we may want to initialize each part of
Message RAM used by this M_CAN controller.
Something like follows in probe() function:
/* initialize the entire Message RAM in use to avoid possible
* ECC/parity checksum errors when reading an uninitialized buffer
start = priv->mcfg[MRAM_SIDF].off;
end = priv->mcfg[MRAM_TXB].off +
priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
for (i = start; i < end; i += 4)
writel(0x0, priv->mram_base + i);
I will send a updated patch for this.
> >Do you think we could keep the current solution firstly and updated later
> >if needed?
> No :-)
> I would like to have all data bytes to be written at startup.
More information about the linux-arm-kernel