[can,v3] can: mcp251xfd: mcp251xfd_get_tef_len(): fix length calculation

Commit Message

Marc Kleine-Budde Nov. 4, 2024, 4:42 p.m. UTC

Commit b8e0ddd36ce9 ("can: mcp251xfd: tef: prepare to workaround
broken TEF FIFO tail index erratum") introduced
mcp251xfd_get_tef_len() to get the number of unhandled transmit events
from the Transmit Event FIFO (TEF).

As the TEF has no head pointer, the driver uses the TX FIFO's tail
pointer instead, assuming that send frames are completed. However the
check for the TEF being full was not correct. This leads to the driver
stop working if the TEF is full.

Fix the TEF full check by assuming that if, from the driver's point of
view, there are no free TX buffers in the chip and the TX FIFO is
empty, all messages must have been sent and the TEF must therefore be
full.

Reported-by: Sven Schuchmann <[e-mail]>
Closes: https://patch.msgid.link/[e-mail]
Fixes: b8e0ddd36ce9 ("can: mcp251xfd: tef: prepare to workaround broken TEF FIFO tail index erratum")
Tested-by: Sven Schuchmann <[e-mail]>
Cc: [e-mail]
Signed-off-by: Marc Kleine-Budde <[e-mail]>
---
Changes in v3:
- add proper patch description
- added Sven's Tested-by
- Link to v2: https://patch.msgid.link/[e-mail]

Changes in v2:
- mcp251xfd_tx_fifo_sta_empty(): fix check if TX-FIFO is empty
- Link to RFC: https://patch.msgid.link/[e-mail]
---
 drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)


---
base-commit: 5ccdcdf186aec6b9111845fd37e1757e9b413e2f
change-id: 20241001-mcp251xfd-fix-length-calculation-09b6cc10aeb0

Best regards,

Comments

Simon Horman Nov. 6, 2024, 3:17 p.m. UTC | #1

On Mon, Nov 04, 2024 at 05:42:40PM +0100, Marc Kleine-Budde wrote:
> Commit b8e0ddd36ce9 ("can: mcp251xfd: tef: prepare to workaround
> broken TEF FIFO tail index erratum") introduced
> mcp251xfd_get_tef_len() to get the number of unhandled transmit events
> from the Transmit Event FIFO (TEF).
> 
> As the TEF has no head pointer, the driver uses the TX FIFO's tail
> pointer instead, assuming that send frames are completed. However the
> check for the TEF being full was not correct. This leads to the driver
> stop working if the TEF is full.
> 
> Fix the TEF full check by assuming that if, from the driver's point of
> view, there are no free TX buffers in the chip and the TX FIFO is
> empty, all messages must have been sent and the TEF must therefore be
> full.
> 
> Reported-by: Sven Schuchmann <[e-mail]>
> Closes: https://patch.msgid.link/[e-mail]
> Fixes: b8e0ddd36ce9 ("can: mcp251xfd: tef: prepare to workaround broken TEF FIFO tail index erratum")
> Tested-by: Sven Schuchmann <[e-mail]>
> Cc: [e-mail]
> Signed-off-by: Marc Kleine-Budde <[e-mail]>

Reviewed-by: Simon Horman <[e-mail]>

...

Patch

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c
index f732556d233a7be3b43f6f08e0b8f25732190104..d3ac865933fdf6c4ecdd80ad4d7accbff51eb0f8 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c
@@ -16,9 +16,9 @@ 
 
 #include "mcp251xfd.h"
 
-static inline bool mcp251xfd_tx_fifo_sta_full(u32 fifo_sta)
+static inline bool mcp251xfd_tx_fifo_sta_empty(u32 fifo_sta)
 {
-	return !(fifo_sta & MCP251XFD_REG_FIFOSTA_TFNRFNIF);
+	return fifo_sta & MCP251XFD_REG_FIFOSTA_TFERFFIF;
 }
 
 static inline int
@@ -122,7 +122,11 @@  mcp251xfd_get_tef_len(struct mcp251xfd_priv *priv, u8 *len_p)
 	if (err)
 		return err;
 
-	if (mcp251xfd_tx_fifo_sta_full(fifo_sta)) {
+	/* If the chip says the TX-FIFO is empty, but there are no TX
+	 * buffers free in the ring, we assume all have been sent.
+	 */
+	if (mcp251xfd_tx_fifo_sta_empty(fifo_sta) &&
+	    mcp251xfd_get_tx_free(tx_ring) == 0) {
 		*len_p = tx_ring->obj_num;
 		return 0;
 	}