Re: [PATCH 7/7] can: m_can: workaround for transmit data less than 4 bytes

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3478 bytes --]

On 11/04/2014 10:27 AM, Dong Aisheng wrote:
>>>> It should be possible to change the for loop to go always to 8, or
>>>> simply unroll the loop:
>>>>
>>>> /* errata description goes here */
>>>> m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(0), *(u32 *)(cf->data + 0));
>>>> m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(1), *(u32 *)(cf->data + 4));
>>>>
>>>
>>> Yes, i tried to fix it as follows.
>>>
>>> /* FIXME: we meet an IC issue that we have to write the full 8
>>>  * bytes (whatever value for the second word) in Message RAM to
>>>  * avoid bit error for transmit data less than 4 bytes
>>>  */
>>> if (cf->len <= 4) {
>>>         m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(0),
>>>                          *(u32 *)(cf->data + 0));
>>>         m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(1),
>>>                          *(u32 *)(cf->data + 4));
>>> } else {
>>>         for (i = 0; i < cf->len; i += 4)
>>>                 m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i / 4),
>>>                                  *(u32 *)(cf->data + i));
>>>
>>> Will update the patch.
>>
>> Both branches of the above if are doing the same thing, I think you can
>> replace the while if ... else ... for with this:
>>
> 
> Not the same thing.
> The later one will cover payload up to 64 bytes.

Doh! I'm not used to CAN-FD, yet :) However, I'll apply this fix before
adding the CAN-FD support.

>> /* errata description goes here */
>> m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(0), *(u32 *)(cf->data + 0));
>> m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(1), *(u32 *)(cf->data + 4));
>>
>> However if writing to DATA(0) and DATA(1) once in the open() function is
>> enough this code should stay as it is.
> 
> I tried put them into open() function and the quick test showed it worked.
> 
> Do you think it's ok to put things into open() function for this issue
> as follows?
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 065e4f1..ca55988 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -901,6 +901,15 @@ static void m_can_chip_config(struct net_device *dev)
>         /* set bittiming params */
>         m_can_set_bittiming(dev);
> 
> +       /* We meet an IC issue that we have to write the full 8

At least on the *insert SoC name here*, an issue with the Message RAM
was discovered. Sending CAN frames with dlc less than 4 bytes will lead
to bit errors, when the first 8 bytes of the Message RAM have not been
initialized (i.e. written to). To work around this issue, the first 8
bytes are initialized here.

> +        * bytes (whatever value for the second word) in Message RAM to
> +        * avoid bit error for transmit data less than 4 bytes at the first
> +        * time. By initializing the first 8 bytes of tx buffer before using
> +        * it can avoid such issue.
> +        */
> +       m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(0), 0x0);
> +       m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(1), 0x0);
> +
>         m_can_config_endisable(priv, false);
>  }

Can you trigger the issue when sending CAN-FD frames with dlc > 8 && dlc
< 64?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]