Skip to content

Commit

Permalink
can: xilinx_can: avoid non-requested bus error frames
Browse files Browse the repository at this point in the history 
Userspace can signal with CAN_CTRLMODE_BERR_REPORTING whether they need
reporting of bus errors (CAN_ERR_BUSERROR) or not.

However, xilinx_can driver currently always sends CAN_ERR_BUSERROR
frames to userspace on bus errors.

To improve performance on error conditions when bus error reporting is
not needed, avoid sending CAN_ERR_BUSERROR frames unless requested via
CAN_CTRLMODE_BERR_REPORTING.

The error interrupt is still kept enabled as there is no dedicated state
transition interrupt, but just disabling error frame submission still
yields a significant performance improvement. In a simple test with
continuous bus errors and no userspace programs reading/writing CAN I
saw system CPU load reduced by 1/3.

Tested on a ZynqMP board with CAN-FD v1.0.

Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
State: upstream (91e9f2c)
  • Loading branch information
@anssih @michalsimek
anssih authored and michalsimek committed Oct 2, 2020
1 parent 0ef3948 commit bb064c2
Showing 1 changed file with 48 additions and 41 deletions.
89 changes: 48 additions & 41 deletions drivers/net/can/xilinx_can.c
View file
Expand Up @@ -472,7 +472,13 @@ static int xcan_chip_start(struct net_device *ndev)
if (err < 0)
return err;

/* Enable interrupts */
/* Enable interrupts
*
* We enable the ERROR interrupt even with
* CAN_CTRLMODE_BERR_REPORTING disabled as there is no
* dedicated interrupt for a state change to
* ERROR_WARNING/ERROR_PASSIVE.
*/
ier = XCAN_IXR_TXOK_MASK | XCAN_IXR_BSOFF_MASK |
XCAN_IXR_WKUP_MASK | XCAN_IXR_SLP_MASK |
XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK |
Expand Down Expand Up @@ -983,12 +989,9 @@ static void xcan_err_interrupt(struct net_device *ndev, u32 isr)
{
struct xcan_priv *priv = netdev_priv(ndev);
struct net_device_stats *stats = &ndev->stats;
struct can_frame *cf;
struct sk_buff *skb;
struct can_frame cf = { };
u32 err_status;

skb = alloc_can_err_skb(ndev, &cf);

err_status = priv->read_reg(priv, XCAN_ESR_OFFSET);
priv->write_reg(priv, XCAN_ESR_OFFSET, err_status);

Expand All @@ -998,101 +1001,105 @@ static void xcan_err_interrupt(struct net_device *ndev, u32 isr)
/* Leave device in Config Mode in bus-off state */
priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
can_bus_off(ndev);
if (skb)
cf->can_id |= CAN_ERR_BUSOFF;
cf.can_id |= CAN_ERR_BUSOFF;
} else {
enum can_state new_state = xcan_current_error_state(ndev);

if (new_state != priv->can.state)
xcan_set_error_state(ndev, new_state, skb ? cf : NULL);
xcan_set_error_state(ndev, new_state, &cf);
}

/* Check for Arbitration lost interrupt */
if (isr & XCAN_IXR_ARBLST_MASK) {
priv->can.can_stats.arbitration_lost++;
if (skb) {
cf->can_id |= CAN_ERR_LOSTARB;
cf->data[0] = CAN_ERR_LOSTARB_UNSPEC;
}
cf.can_id |= CAN_ERR_LOSTARB;
cf.data[0] = CAN_ERR_LOSTARB_UNSPEC;
}

/* Check for RX FIFO Overflow interrupt */
if (isr & XCAN_IXR_RXOFLW_MASK) {
stats->rx_over_errors++;
stats->rx_errors++;
if (skb) {
cf->can_id |= CAN_ERR_CRTL;
cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
}
cf.can_id |= CAN_ERR_CRTL;
cf.data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
}

/* Check for RX Match Not Finished interrupt */
if (isr & XCAN_IXR_RXMNF_MASK) {
stats->rx_dropped++;
stats->rx_errors++;
netdev_err(ndev, "RX match not finished, frame discarded\n");
if (skb) {
cf->can_id |= CAN_ERR_CRTL;
cf->data[1] |= CAN_ERR_CRTL_UNSPEC;
}
cf.can_id |= CAN_ERR_CRTL;
cf.data[1] |= CAN_ERR_CRTL_UNSPEC;
}

/* Check for error interrupt */
if (isr & XCAN_IXR_ERROR_MASK) {
if (skb)
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
bool berr_reporting = false;

if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) {
berr_reporting = true;
cf.can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
}

/* Check for Ack error interrupt */
if (err_status & XCAN_ESR_ACKER_MASK) {
stats->tx_errors++;
if (skb) {
cf->can_id |= CAN_ERR_ACK;
cf->data[3] = CAN_ERR_PROT_LOC_ACK;
if (berr_reporting) {
cf.can_id |= CAN_ERR_ACK;
cf.data[3] = CAN_ERR_PROT_LOC_ACK;
}
}

/* Check for Bit error interrupt */
if (err_status & XCAN_ESR_BERR_MASK) {
stats->tx_errors++;
if (skb) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_BIT;
if (berr_reporting) {
cf.can_id |= CAN_ERR_PROT;
cf.data[2] = CAN_ERR_PROT_BIT;
}
}

/* Check for Stuff error interrupt */
if (err_status & XCAN_ESR_STER_MASK) {
stats->rx_errors++;
if (skb) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_STUFF;
if (berr_reporting) {
cf.can_id |= CAN_ERR_PROT;
cf.data[2] = CAN_ERR_PROT_STUFF;
}
}

/* Check for Form error interrupt */
if (err_status & XCAN_ESR_FMER_MASK) {
stats->rx_errors++;
if (skb) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_FORM;
if (berr_reporting) {
cf.can_id |= CAN_ERR_PROT;
cf.data[2] = CAN_ERR_PROT_FORM;
}
}

/* Check for CRC error interrupt */
if (err_status & XCAN_ESR_CRCER_MASK) {
stats->rx_errors++;
if (skb) {
cf->can_id |= CAN_ERR_PROT;
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
if (berr_reporting) {
cf.can_id |= CAN_ERR_PROT;
cf.data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
}
}
priv->can.can_stats.bus_error++;
}

if (skb) {
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
netif_rx(skb);
if (cf.can_id) {
struct can_frame *skb_cf;
struct sk_buff *skb = alloc_can_err_skb(ndev, &skb_cf);

if (skb) {
skb_cf->can_id |= cf.can_id;
memcpy(skb_cf->data, cf.data, CAN_ERR_DLC);
stats->rx_packets++;
stats->rx_bytes += CAN_ERR_DLC;
netif_rx(skb);
}
}

netdev_dbg(ndev, "%s: error status register:0x%x\n",
Expand Down

0 comments on commit bb064c2

Please sign in to comment.