Skip to content

Commit 9684b00

Browse files
Frank Jungclausmarckleinebudde
authored andcommitted
can: esd_usb: Make use of can_change_state() and relocate checking skb for NULL
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: 96d8e90 ("can: Add driver for esd CAN-USB/2 device") Signed-off-by: Frank Jungclaus <[email protected]> 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/[email protected] Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent 118469f commit 9684b00

File tree

1 file changed

+25
-25
lines changed

1 file changed

+25
-25
lines changed

drivers/net/can/usb/esd_usb.c

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -239,41 +239,42 @@ static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
239239
msg->msg.rx.dlc, state, ecc, rxerr, txerr);
240240

241241
skb = alloc_can_err_skb(priv->netdev, &cf);
242-
if (skb == NULL) {
243-
stats->rx_dropped++;
244-
return;
245-
}
246242

247243
if (state != priv->old_state) {
244+
enum can_state tx_state, rx_state;
245+
enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
246+
248247
priv->old_state = state;
249248

250249
switch (state & ESD_BUSSTATE_MASK) {
251250
case ESD_BUSSTATE_BUSOFF:
252-
priv->can.state = CAN_STATE_BUS_OFF;
253-
cf->can_id |= CAN_ERR_BUSOFF;
254-
priv->can.can_stats.bus_off++;
251+
new_state = CAN_STATE_BUS_OFF;
255252
can_bus_off(priv->netdev);
256253
break;
257254
case ESD_BUSSTATE_WARN:
258-
priv->can.state = CAN_STATE_ERROR_WARNING;
259-
priv->can.can_stats.error_warning++;
255+
new_state = CAN_STATE_ERROR_WARNING;
260256
break;
261257
case ESD_BUSSTATE_ERRPASSIVE:
262-
priv->can.state = CAN_STATE_ERROR_PASSIVE;
263-
priv->can.can_stats.error_passive++;
258+
new_state = CAN_STATE_ERROR_PASSIVE;
264259
break;
265260
default:
266-
priv->can.state = CAN_STATE_ERROR_ACTIVE;
261+
new_state = CAN_STATE_ERROR_ACTIVE;
267262
txerr = 0;
268263
rxerr = 0;
269264
break;
270265
}
271-
} else {
266+
267+
if (new_state != priv->can.state) {
268+
tx_state = (txerr >= rxerr) ? new_state : 0;
269+
rx_state = (txerr <= rxerr) ? new_state : 0;
270+
can_change_state(priv->netdev, cf,
271+
tx_state, rx_state);
272+
}
273+
} else if (skb) {
272274
priv->can.can_stats.bus_error++;
273275
stats->rx_errors++;
274276

275-
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR |
276-
CAN_ERR_CNT;
277+
cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
277278

278279
switch (ecc & SJA1000_ECC_MASK) {
279280
case SJA1000_ECC_BIT:
@@ -295,21 +296,20 @@ static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
295296

296297
/* Bit stream position in CAN frame as the error was detected */
297298
cf->data[3] = ecc & SJA1000_ECC_SEG;
298-
299-
if (priv->can.state == CAN_STATE_ERROR_WARNING ||
300-
priv->can.state == CAN_STATE_ERROR_PASSIVE) {
301-
cf->data[1] = (txerr > rxerr) ?
302-
CAN_ERR_CRTL_TX_PASSIVE :
303-
CAN_ERR_CRTL_RX_PASSIVE;
304-
}
305-
cf->data[6] = txerr;
306-
cf->data[7] = rxerr;
307299
}
308300

309301
priv->bec.txerr = txerr;
310302
priv->bec.rxerr = rxerr;
311303

312-
netif_rx(skb);
304+
if (skb) {
305+
cf->can_id |= CAN_ERR_CNT;
306+
cf->data[6] = txerr;
307+
cf->data[7] = rxerr;
308+
309+
netif_rx(skb);
310+
} else {
311+
stats->rx_dropped++;
312+
}
313313
}
314314
}
315315

0 commit comments

Comments
 (0)