can: esd_usb: Make use of can_change_state() and relocate checking skb for NULL
[ Upstream commit9684b000a8
] Start a rework initiated by Vincents remarks "You should not report the greatest of txerr and rxerr but the one which actually increased." [1] and "As far as I understand, those flags should be set only when the threshold is reached" [2] . Therefore make use of can_change_state() to (among others) set the flags CAN_ERR_CRTL_[RT]X_WARNING and CAN_ERR_CRTL_[RT]X_PASSIVE, maintain CAN statistic counters for error_warning, error_passive and bus_off. Relocate testing alloc_can_err_skb() for NULL to the end of esd_usb_rx_event(), to have things like can_bus_off(), can_change_state() working even in out of memory conditions. Fixes:96d8e90382
("can: Add driver for esd CAN-USB/2 device") Signed-off-by: Frank Jungclaus <frank.jungclaus@esd.eu> Link: [1] https://lore.kernel.org/all/CAMZ6RqKGBWe15aMkf8-QLf-cOQg99GQBebSm+1wEzTqHgvmNuw@mail.gmail.com/ Link: [2] https://lore.kernel.org/all/CAMZ6Rq+QBO1yTX_o6GV0yhdBj-RzZSRGWDZBS0fs7zbSTy4hmA@mail.gmail.com/ Link: https://lore.kernel.org/all/20230216190450.3901254-3-frank.jungclaus@esd.eu Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
08d2f67795
commit
ccf81ec1f1
|
@ -239,41 +239,42 @@ static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
|
|||
msg->msg.rx.dlc, state, ecc, rxerr, txerr);
|
||||
|
||||
skb = alloc_can_err_skb(priv->netdev, &cf);
|
||||
if (skb == NULL) {
|
||||
stats->rx_dropped++;
|
||||
return;
|
||||
}
|
||||
|
||||
if (state != priv->old_state) {
|
||||
enum can_state tx_state, rx_state;
|
||||
enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
|
||||
|
||||
priv->old_state = state;
|
||||
|
||||
switch (state & ESD_BUSSTATE_MASK) {
|
||||
case ESD_BUSSTATE_BUSOFF:
|
||||
priv->can.state = CAN_STATE_BUS_OFF;
|
||||
cf->can_id |= CAN_ERR_BUSOFF;
|
||||
priv->can.can_stats.bus_off++;
|
||||
new_state = CAN_STATE_BUS_OFF;
|
||||
can_bus_off(priv->netdev);
|
||||
break;
|
||||
case ESD_BUSSTATE_WARN:
|
||||
priv->can.state = CAN_STATE_ERROR_WARNING;
|
||||
priv->can.can_stats.error_warning++;
|
||||
new_state = CAN_STATE_ERROR_WARNING;
|
||||
break;
|
||||
case ESD_BUSSTATE_ERRPASSIVE:
|
||||
priv->can.state = CAN_STATE_ERROR_PASSIVE;
|
||||
priv->can.can_stats.error_passive++;
|
||||
new_state = CAN_STATE_ERROR_PASSIVE;
|
||||
break;
|
||||
default:
|
||||
priv->can.state = CAN_STATE_ERROR_ACTIVE;
|
||||
new_state = CAN_STATE_ERROR_ACTIVE;
|
||||
txerr = 0;
|
||||
rxerr = 0;
|
||||
break;
|
||||
}
|
||||
} else {
|
||||
|
||||
if (new_state != priv->can.state) {
|
||||
tx_state = (txerr >= rxerr) ? new_state : 0;
|
||||
rx_state = (txerr <= rxerr) ? new_state : 0;
|
||||
can_change_state(priv->netdev, cf,
|
||||
tx_state, rx_state);
|
||||
}
|
||||
} else if (skb) {
|
||||
priv->can.can_stats.bus_error++;
|
||||
stats->rx_errors++;
|
||||
|
||||
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR |
|
||||
CAN_ERR_CNT;
|
||||
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
|
||||
|
||||
switch (ecc & SJA1000_ECC_MASK) {
|
||||
case SJA1000_ECC_BIT:
|
||||
|
@ -295,21 +296,20 @@ static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
|
|||
|
||||
/* Bit stream position in CAN frame as the error was detected */
|
||||
cf->data[3] = ecc & SJA1000_ECC_SEG;
|
||||
|
||||
if (priv->can.state == CAN_STATE_ERROR_WARNING ||
|
||||
priv->can.state == CAN_STATE_ERROR_PASSIVE) {
|
||||
cf->data[1] = (txerr > rxerr) ?
|
||||
CAN_ERR_CRTL_TX_PASSIVE :
|
||||
CAN_ERR_CRTL_RX_PASSIVE;
|
||||
}
|
||||
cf->data[6] = txerr;
|
||||
cf->data[7] = rxerr;
|
||||
}
|
||||
|
||||
priv->bec.txerr = txerr;
|
||||
priv->bec.rxerr = rxerr;
|
||||
|
||||
netif_rx(skb);
|
||||
if (skb) {
|
||||
cf->can_id |= CAN_ERR_CNT;
|
||||
cf->data[6] = txerr;
|
||||
cf->data[7] = rxerr;
|
||||
|
||||
netif_rx(skb);
|
||||
} else {
|
||||
stats->rx_dropped++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue