Re: c_can driver sometimes sends first two bytes filled with zeros

[Wolfgang Grandegger <wg@grandegger.com>]

Hello Andy,

Am 10.06.2016 um 12:49 schrieb Andy Haydon:
> Hi Wolfgang,
>
> Wolfgang Grandegger <wg <at> grandegger.com> writes:
>
>>
>> Making "priv->write_regs() atomic is something I would try as well. My
>> suspicion is that something goes wrong writing(/reading) the controller
>> registers.
>
> We have also been looking at a similar issue over the last few days. In our
> case, bytes 0 and 1 and bytes 4 and 5 of an 8 byte message can be corrupted
> randomly - not necessarily zeroed. Bytes 2 and 3 and bytes 6 and 7 are never
> corrupted. The Tx task is running as a high priority real time thread in
> PREEMPT_RT.
>
> I've found that so far the only thing that fixes this issue is to perform
> the writes to the data registers as 32 bit operations something like this
> (for 3.18.20):
>
> --- a/drivers/net/can/c_can/c_can.c	2015-08-07 20:08:04.000000000 +0100
> +++ b/drivers/net/can/c_can/c_can.c	2016-06-09 14:46:51.497895357 +0100
> @@ -331,9 +332,19 @@
>
>   	priv->write_reg(priv, C_CAN_IFACE(MSGCTRL_REG, iface), ctrl);
>
> -	for (i = 0; i < frame->can_dlc; i += 2) {
> -		priv->write_reg(priv, C_CAN_IFACE(DATA1_REG, iface) + i / 2,
> -				frame->data[i] | (frame->data[i + 1] << 8));
> +// this doesn't work for Cyclone V
> +//	for (i = 0; i < frame->can_dlc; i += 2) {
> +//		priv->write_reg(priv, C_CAN_IFACE(DATA1_REG, iface) + i / 2,
> +//				frame->data[i] | (frame->data[i + 1] << 8));
> +
> +// replaced with this as a workaround
> +	u32 data = 0;
> +	for (i = 0; i < frame->can_dlc; i += 4) {
> +		data = (u32)frame->data[i];
> +		data |= (u32)frame->data[i + 1] << 8;
> +		data |= (u32)frame->data[i + 2] << 16;
> +		data |= (u32)frame->data[i + 3] << 24;
> +		priv->write_reg32(priv, C_CAN_IFACE(DATA1_REG, iface) + i /
> 2, data);
>   	}
>   }

Interesting observation! Richard (on CC now), does the patch also fix 
your similar issues?

> At the moment I can't explain this behaviour, or if this is really the only
> way to fix the issue for the Cyclone V.

Maybe the hardware does not like the 16-bit accesses. BTW: How is the 
device configured? C_CAN with 32-bit accesses or D_CAN? Anyway, real 
32-bit accesses would be faster anyway.

> The tests that I've done so far seem to indicate that the data is correct
> in the driver right up to the point at which it is actually written to the
> CAN peripheral registers, so I think that your suspicion is correct.

Maybe some hardware guys from Altera could help.

Wolfgang.