Commit 07c7b6f6 authored by Thomas Gleixner's avatar Thomas Gleixner Committed by Marc Kleine-Budde

can: c_can: Fix the lost message handling

The lost message handling is broken in several ways.

1) Clearing the message lost flag is done by writing 0 to the
   message control register of the object.

   #define IF_MCONT_CLR_MSGLST    (0 << 14)

   That clears the object buffer configuration in the worst case,
   which results in a loss of the EOB flag. That leaves the FIFO chain
   without a limit and causes a complete lockup of the HW

2) In case that the error skb allocation fails, the code happily
   claims that it handed down a packet. Just an accounting bug, but ....

3) The code adds a lot of pointless overhead to that error case, where
   we need to get stuff done as fast as possible to avoid more packet
   loss.

   - printk an annoying error message
   - reread the object buffer for nothing

Fix is simple again:

  - Use the already known MSGCTRL content and only clear the MSGLST bit
  - Fix the buffer accounting by adding a proper return code
  - Remove the pointless operations
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parent 64f08f2f
...@@ -122,7 +122,6 @@ ...@@ -122,7 +122,6 @@
/* IFx message control */ /* IFx message control */
#define IF_MCONT_NEWDAT BIT(15) #define IF_MCONT_NEWDAT BIT(15)
#define IF_MCONT_MSGLST BIT(14) #define IF_MCONT_MSGLST BIT(14)
#define IF_MCONT_CLR_MSGLST (0 << 14)
#define IF_MCONT_INTPND BIT(13) #define IF_MCONT_INTPND BIT(13)
#define IF_MCONT_UMASK BIT(12) #define IF_MCONT_UMASK BIT(12)
#define IF_MCONT_TXIE BIT(11) #define IF_MCONT_TXIE BIT(11)
...@@ -411,27 +410,22 @@ static inline void c_can_activate_rx_msg_obj(struct net_device *dev, ...@@ -411,27 +410,22 @@ static inline void c_can_activate_rx_msg_obj(struct net_device *dev,
c_can_object_put(dev, iface, obj, IF_COMM_CONTROL); c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
} }
static void c_can_handle_lost_msg_obj(struct net_device *dev, static int c_can_handle_lost_msg_obj(struct net_device *dev,
int iface, int objno) int iface, int objno, u32 ctrl)
{ {
struct c_can_priv *priv = netdev_priv(dev);
struct net_device_stats *stats = &dev->stats; struct net_device_stats *stats = &dev->stats;
struct sk_buff *skb; struct c_can_priv *priv = netdev_priv(dev);
struct can_frame *frame; struct can_frame *frame;
struct sk_buff *skb;
netdev_err(dev, "msg lost in buffer %d\n", objno); ctrl &= ~(IF_MCONT_MSGLST | IF_MCONT_INTPND | IF_MCONT_NEWDAT);
priv->write_reg(priv, C_CAN_IFACE(MSGCTRL_REG, iface), ctrl);
c_can_object_get(dev, iface, objno, IF_COMM_ALL & ~IF_COMM_TXRQST);
priv->write_reg(priv, C_CAN_IFACE(MSGCTRL_REG, iface),
IF_MCONT_CLR_MSGLST);
c_can_object_put(dev, iface, objno, IF_COMM_CONTROL); c_can_object_put(dev, iface, objno, IF_COMM_CONTROL);
/* create an error msg */ /* create an error msg */
skb = alloc_can_err_skb(dev, &frame); skb = alloc_can_err_skb(dev, &frame);
if (unlikely(!skb)) if (unlikely(!skb))
return; return 0;
frame->can_id |= CAN_ERR_CRTL; frame->can_id |= CAN_ERR_CRTL;
frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW; frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
...@@ -439,6 +433,7 @@ static void c_can_handle_lost_msg_obj(struct net_device *dev, ...@@ -439,6 +433,7 @@ static void c_can_handle_lost_msg_obj(struct net_device *dev,
stats->rx_over_errors++; stats->rx_over_errors++;
netif_receive_skb(skb); netif_receive_skb(skb);
return 1;
} }
static int c_can_read_msg_object(struct net_device *dev, int iface, int ctrl) static int c_can_read_msg_object(struct net_device *dev, int iface, int ctrl)
...@@ -910,9 +905,13 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota) ...@@ -910,9 +905,13 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
C_CAN_IFACE(MSGCTRL_REG, IF_RX)); C_CAN_IFACE(MSGCTRL_REG, IF_RX));
if (msg_ctrl_save & IF_MCONT_MSGLST) { if (msg_ctrl_save & IF_MCONT_MSGLST) {
c_can_handle_lost_msg_obj(dev, IF_RX, msg_obj); int n;
num_rx_pkts++;
quota--; n = c_can_handle_lost_msg_obj(dev, IF_RX,
msg_obj,
msg_ctrl_save);
num_rx_pkts += n;
quota -=n;
continue; continue;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment