netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, linux-can@vger.kernel.org,
	kernel@pengutronix.de, Thomas Gleixner <tglx@linutronix.de>,
	Marc Kleine-Budde <mkl@pengutronix.de>
Subject: [PATCH 12/26] can: c_can: Work around C_CAN RX wreckage
Date: Fri, 25 Apr 2014 00:00:15 +0200	[thread overview]
Message-ID: <1398376829-6771-13-git-send-email-mkl@pengutronix.de> (raw)
In-Reply-To: <1398376829-6771-1-git-send-email-mkl@pengutronix.de>

From: Thomas Gleixner <tglx@linutronix.de>

Alexander reported that the new optimized handling of the RX fifo
causes random packet loss on Intel PCH C_CAN hardware.

After a few fruitless debugging sessions I got hold of a PCH (eg20t)
afflicted system. That machine does not have the CAN interface wired
up, but it was possible to reproduce the issue with the HW loopback
mode.

As Alexander observed correctly, clearing the NewDat flag along with
reading out the message buffer causes that issue on C_CAN, while D_CAN
handles that correctly.

Instead of restoring the original message buffer handling horror the
following workaround solves the issue:

    transfer buffer to IF without clearing the NewDat
    handle the message
    clear NewDat bit

That's similar to the original code but conditional for C_CAN.

I really wonder why all user manuals (C_CAN, Intel PCH and some more)
recommend to clear the NewDat bit right away. The knows it all Oracle
operated by Gurgle does not unearth any useful information either. I
simply cannot believe that we are the first to uncover that HW issue.

Reported-and-tested-by: Alexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.c | 13 ++++++++++---
 drivers/net/can/c_can/c_can.h |  1 +
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index c1a8684..5d43c5a 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -647,6 +647,10 @@ static int c_can_start(struct net_device *dev)
 	if (err)
 		return err;
 
+	/* Setup the command for new messages */
+	priv->comm_rcv_high = priv->type != BOSCH_D_CAN ?
+		IF_COMM_RCV_LOW : IF_COMM_RCV_HIGH;
+
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	/* reset tx helper pointers and the rx mask */
@@ -791,14 +795,15 @@ static u32 c_can_adjust_pending(u32 pend)
 	return pend & ~((1 << lasts) - 1);
 }
 
-static inline void c_can_rx_object_get(struct net_device *dev, u32 obj)
+static inline void c_can_rx_object_get(struct net_device *dev,
+				       struct c_can_priv *priv, u32 obj)
 {
 #ifdef CONFIG_CAN_C_CAN_STRICT_FRAME_ORDERING
 	if (obj < C_CAN_MSG_RX_LOW_LAST)
 		c_can_object_get(dev, IF_RX, obj, IF_COMM_RCV_LOW);
 	else
 #endif
-		c_can_object_get(dev, IF_RX, obj, IF_COMM_RCV_HIGH);
+		c_can_object_get(dev, IF_RX, obj, priv->comm_rcv_high);
 }
 
 static inline void c_can_rx_finalize(struct net_device *dev,
@@ -813,6 +818,8 @@ static inline void c_can_rx_finalize(struct net_device *dev,
 		c_can_activate_all_lower_rx_msg_obj(dev, IF_RX);
 	}
 #endif
+	if (priv->type != BOSCH_D_CAN)
+		c_can_object_get(dev, IF_RX, obj, IF_COMM_CLR_NEWDAT);
 }
 
 static int c_can_read_objects(struct net_device *dev, struct c_can_priv *priv,
@@ -823,7 +830,7 @@ static int c_can_read_objects(struct net_device *dev, struct c_can_priv *priv,
 	while ((obj = ffs(pend)) && quota > 0) {
 		pend &= ~BIT(obj - 1);
 
-		c_can_rx_object_get(dev, obj);
+		c_can_rx_object_get(dev, priv, obj);
 		ctrl = priv->read_reg(priv, C_CAN_IFACE(MSGCTRL_REG, IF_RX));
 
 		if (ctrl & IF_MCONT_MSGLST) {
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index cd91960..792944c 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -198,6 +198,7 @@ struct c_can_priv {
 	u32 __iomem *raminit_ctrlreg;
 	unsigned int instance;
 	void (*raminit) (const struct c_can_priv *priv, bool enable);
+	u32 comm_rcv_high;
 	u32 rxmasked;
 	u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
 };
-- 
1.9.0.279.gdc9e3eb


  parent reply	other threads:[~2014-04-24 22:00 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-24 22:00 pull-request: can 2014-04-24 Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 01/26] can: c_can_pci: Set the type of the IP core Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 02/26] can: c_can: Fix startup logic Marc Kleine-Budde
2014-04-29  7:29   ` Yegor Yefremov
2014-05-02 12:37     ` Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 03/26] can: c_can: Make bus off interrupt disable logic work Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 04/26] can: c_can: Do not access skb after net_receive_skb() Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 05/26] can: c_can: Handle state change correctly Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 06/26] can: c_can: Fix berr reporting Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 07/26] can: c_can: Always update error stats Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 08/26] can: c_can: Simplify buffer reenabling Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 09/26] can: c_can: Avoid status register update for D_CAN Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 10/26] can: c_can: Get rid of pointless interrupts Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 11/26] can: c_can: Disable rx split as workaround Marc Kleine-Budde
2014-04-24 22:00 ` Marc Kleine-Budde [this message]
2014-04-24 22:00 ` [PATCH 13/26] can: c_can: Cleanup irq enable/disable Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 14/26] can: c_can: Cleanup c_can_read_msg_object() Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 15/26] can: c_can: Cleanup setup of receive buffers Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 16/26] can: c_can: Cleanup c_can_inval_msg_object() Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 17/26] can: c_can: Cleanup c_can_msg_obj_put/get() Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 18/26] can: c_can: Cleanup c_can_write_msg_object() Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 19/26] can: c_can: Use proper u32 variables in c_can_write_msg_object() Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 20/26] can: c_can: Remove tx locking Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 21/26] can: c_can: Speed up tx buffer invalidation Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 22/26] can: c_can: use proper type for 'instance' Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 23/26] can: c_can_pci: enable PCI bus master only for MSI Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 24/26] can: sja1000_isa: add locking for indirect register access mode Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 25/26] can: fix return value from can_get_bittiming() Marc Kleine-Budde
2014-04-24 22:00 ` [PATCH 26/26] can: slcan: Fix spinlock variant Marc Kleine-Budde
2014-04-24 22:04 ` pull-request: can 2014-04-24 Marc Kleine-Budde
2014-04-26 16:31 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1398376829-6771-13-git-send-email-mkl@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=davem@davemloft.net \
    --cc=kernel@pengutronix.de \
    --cc=linux-can@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).