Skip to content

Commit

Permalink
can: fix assignment of error location in CAN error messages
Browse files Browse the repository at this point in the history
As Dan Carpenter reported in http://marc.info/?l=linux-can&m=144793696016187
the assignment of the error location in CAN error messages had some bit wise
overlaps. Indeed the value to be assigned in data[3] is no bitfield but defines
a single value which points to a location inside the CAN frame on the wire.

This patch fixes the assignments for the error locations in error messages.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
  • Loading branch information
Oliver Hartkopp authored and Marc Kleine-Budde committed Nov 23, 2015
1 parent 7cecd9a commit ffd461f
Show file tree
Hide file tree
Showing 10 changed files with 18 additions and 28 deletions.
6 changes: 2 additions & 4 deletions drivers/net/can/c_can/c_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -975,8 +975,7 @@ static int c_can_handle_bus_err(struct net_device *dev,
break;
case LEC_ACK_ERROR:
netdev_dbg(dev, "ack error\n");
cf->data[3] |= (CAN_ERR_PROT_LOC_ACK |
CAN_ERR_PROT_LOC_ACK_DEL);
cf->data[3] = CAN_ERR_PROT_LOC_ACK;
break;
case LEC_BIT1_ERROR:
netdev_dbg(dev, "bit1 error\n");
Expand All @@ -988,8 +987,7 @@ static int c_can_handle_bus_err(struct net_device *dev,
break;
case LEC_CRC_ERROR:
netdev_dbg(dev, "CRC error\n");
cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
CAN_ERR_PROT_LOC_CRC_DEL);
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
break;
default:
break;
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/can/cc770/cc770.c
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ static int cc770_err(struct net_device *dev, u8 status)
cf->data[2] |= CAN_ERR_PROT_BIT0;
break;
case STAT_LEC_CRC:
cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
break;
}
}
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/can/flexcan.c
Original file line number Diff line number Diff line change
Expand Up @@ -535,13 +535,13 @@ static void do_bus_err(struct net_device *dev,
if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
netdev_dbg(dev, "ACK_ERR irq\n");
cf->can_id |= CAN_ERR_ACK;
cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
cf->data[3] = CAN_ERR_PROT_LOC_ACK;
tx_errors = 1;
}
if (reg_esr & FLEXCAN_ESR_CRC_ERR) {
netdev_dbg(dev, "CRC_ERR irq\n");
cf->data[2] |= CAN_ERR_PROT_BIT;
cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
rx_errors = 1;
}
if (reg_esr & FLEXCAN_ESR_FRM_ERR) {
Expand Down
6 changes: 2 additions & 4 deletions drivers/net/can/m_can/m_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,7 @@ static int m_can_handle_lec_err(struct net_device *dev,
break;
case LEC_ACK_ERROR:
netdev_dbg(dev, "ack error\n");
cf->data[3] |= (CAN_ERR_PROT_LOC_ACK |
CAN_ERR_PROT_LOC_ACK_DEL);
cf->data[3] = CAN_ERR_PROT_LOC_ACK;
break;
case LEC_BIT1_ERROR:
netdev_dbg(dev, "bit1 error\n");
Expand All @@ -513,8 +512,7 @@ static int m_can_handle_lec_err(struct net_device *dev,
break;
case LEC_CRC_ERROR:
netdev_dbg(dev, "CRC error\n");
cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
CAN_ERR_PROT_LOC_CRC_DEL);
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
break;
default:
break;
Expand Down
3 changes: 1 addition & 2 deletions drivers/net/can/pch_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,7 @@ static void pch_can_error(struct net_device *ndev, u32 status)
stats->rx_errors++;
break;
case PCH_CRC_ERR:
cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ |
CAN_ERR_PROT_LOC_CRC_DEL;
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
priv->can.can_stats.bus_error++;
stats->rx_errors++;
break;
Expand Down
6 changes: 3 additions & 3 deletions drivers/net/can/rcar_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ static void rcar_can_error(struct net_device *ndev)
tx_errors++;
writeb(~RCAR_CAN_ECSR_ADEF, &priv->regs->ecsr);
if (skb)
cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
cf->data[3] = CAN_ERR_PROT_LOC_ACK_DEL;
}
if (ecsr & RCAR_CAN_ECSR_BE0F) {
netdev_dbg(priv->ndev, "Bit Error (dominant)\n");
Expand All @@ -272,15 +272,15 @@ static void rcar_can_error(struct net_device *ndev)
rx_errors++;
writeb(~RCAR_CAN_ECSR_CEF, &priv->regs->ecsr);
if (skb)
cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
}
if (ecsr & RCAR_CAN_ECSR_AEF) {
netdev_dbg(priv->ndev, "ACK Error\n");
tx_errors++;
writeb(~RCAR_CAN_ECSR_AEF, &priv->regs->ecsr);
if (skb) {
cf->can_id |= CAN_ERR_ACK;
cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
cf->data[3] = CAN_ERR_PROT_LOC_ACK;
}
}
if (ecsr & RCAR_CAN_ECSR_FEF) {
Expand Down
6 changes: 2 additions & 4 deletions drivers/net/can/ti_hecc.c
Original file line number Diff line number Diff line change
Expand Up @@ -737,13 +737,11 @@ static int ti_hecc_error(struct net_device *ndev, int int_status,
}
if (err_status & HECC_CANES_CRCE) {
hecc_set_bit(priv, HECC_CANES, HECC_CANES_CRCE);
cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ |
CAN_ERR_PROT_LOC_CRC_DEL;
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
}
if (err_status & HECC_CANES_ACKE) {
hecc_set_bit(priv, HECC_CANES, HECC_CANES_ACKE);
cf->data[3] |= CAN_ERR_PROT_LOC_ACK |
CAN_ERR_PROT_LOC_ACK_DEL;
cf->data[3] = CAN_ERR_PROT_LOC_ACK;
}
}

Expand Down
5 changes: 2 additions & 3 deletions drivers/net/can/usb/kvaser_usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -944,10 +944,9 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;

if (es->leaf.error_factor & M16C_EF_ACKE)
cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
cf->data[3] = CAN_ERR_PROT_LOC_ACK;
if (es->leaf.error_factor & M16C_EF_CRCE)
cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
CAN_ERR_PROT_LOC_CRC_DEL);
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
if (es->leaf.error_factor & M16C_EF_FORME)
cf->data[2] |= CAN_ERR_PROT_FORM;
if (es->leaf.error_factor & M16C_EF_STFE)
Expand Down
3 changes: 1 addition & 2 deletions drivers/net/can/usb/usb_8dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,7 @@ static void usb_8dev_rx_err_msg(struct usb_8dev_priv *priv,
break;
case USB_8DEV_STATUSMSG_CRC:
cf->data[2] |= CAN_ERR_PROT_UNSPEC;
cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ |
CAN_ERR_PROT_LOC_CRC_DEL;
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
rx_errors = 1;
break;
case USB_8DEV_STATUSMSG_BIT0:
Expand Down
5 changes: 2 additions & 3 deletions drivers/net/can/xilinx_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ static void xcan_err_interrupt(struct net_device *ndev, u32 isr)
stats->tx_errors++;
if (skb) {
cf->can_id |= CAN_ERR_ACK;
cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
cf->data[3] = CAN_ERR_PROT_LOC_ACK;
}
}

Expand Down Expand Up @@ -654,8 +654,7 @@ static void xcan_err_interrupt(struct net_device *ndev, u32 isr)
stats->rx_errors++;
if (skb) {
cf->can_id |= CAN_ERR_PROT;
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ |
CAN_ERR_PROT_LOC_CRC_DEL;
cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
}
}
priv->can.can_stats.bus_error++;
Expand Down

0 comments on commit ffd461f

Please sign in to comment.