[PATCH] flex_can: Fix checking can_dlc

From: "phu.luuan" <redacted>

flexcan_start_xmit should write data to register when can_dlc > 4.
Because can_dlc is data length and it has value from 1 to 8. But CAN data
index has value from 0 to 7. So in case we have data in cf->data[4],
the can_dlc has value is more than 4.

Signed-off-by: Luu An Phu <redacted>
---
 drivers/net/can/flexcan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 0626dcf..0e3ff13 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c

@@ -5,6 +5,7 @@
  * Copyright (c) 2009 Sascha Hauer, Pengutronix
  * Copyright (c) 2010-2017 Pengutronix, Marc Kleine-Budde <kernel@pengutronix.de>
  * Copyright (c) 2014 David Jander, Protonic Holland
+ * Copyright 2017 NXP
  *
  * Based on code originally by Andrey Volkov <avolkov@varma-el.com>
  *

@@ -526,7 +527,7 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		data = be32_to_cpup((__be32 *)&cf->data[0]);
 		flexcan_write(data, &priv->tx_mb->data[0]);
 	}
-	if (cf->can_dlc > 3) {
+	if (cf->can_dlc > 4) {
 		data = be32_to_cpup((__be32 *)&cf->data[4]);
 		flexcan_write(data, &priv->tx_mb->data[1]);
 	}

-- 
1.9.1

This patch looks wrong to me.

On 12/19/2017 09:40 AM, Luu An Phu wrote:

From: "phu.luuan" <redacted>

flexcan_start_xmit should write data to register when can_dlc > 4.
Because can_dlc is data length and it has value from 1 to 8.

No. can_dlc can contain values from 0 to 8. Even 0 is a valid DLC.

This is correct as data[0 .. 3] are filled from the code above. And if can_dlc is greater than 3 (== 4 .. 8) the following 4 bytes are copied into the registers.

So everything is correct with the current code.

Regards,
Oliver

Answering myself after reading my own comment once more:

In fact the code fix seems to be correct but the commit comment was completely wrong which lead to my answer ...

can_dlc can hold values from 0 .. 8.

The first 4 bytes are placed in data[0..3]. When we have more(!) than 4 bytes in can_dlc, the bytes 5..8 are placed in data[4..7].

The good thing was, that the current check was more conservative than your suggestion so that we did not detect any data loss.

Please send a V2 patch with corrected commit message.

Thanks,
Oliver

ps.

quoted

From: "phu.luuan" <redacted>
+ * Copyright 2017 NXP

A one-liner contribution like this tiny fix usually does not lead to an attribution in the copyrights. Your contribution is already perfectly recorded by the Signed-off-by tag.