Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. Linux>
  3. Kernel
[PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM
darwish.07 at gmail
Jan 25, 2015, 9:27 PM
Post #1 of 5 (608 views)
Permalink
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

Update all of the can interface's state and error counters before
trying any skb allocation that can actually fail with -ENOMEM.

Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 181 ++++++++++++++++++++++----------------
1 files changed, 105 insertions(+), 76 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 7af379c..f57ce55 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -273,6 +273,10 @@ struct kvaser_msg {
} u;
} __packed;

+struct kvaser_usb_error_summary {
+ u8 channel, status, txerr, rxerr, error_factor;
+};
+
struct kvaser_usb_tx_urb_context {
struct kvaser_usb_net_priv *priv;
u32 echo_index;
@@ -615,6 +619,54 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
priv->tx_contexts[i].echo_index = MAX_TX_URBS;
}

+static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
+ const struct kvaser_usb_error_summary *es)
+{
+ struct net_device_stats *stats;
+ enum can_state new_state;
+
+ stats = &priv->netdev->stats;
+ new_state = priv->can.state;
+
+ netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
+
+ if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
+ priv->can.can_stats.bus_off++;
+ new_state = CAN_STATE_BUS_OFF;
+ } else if (es->status & M16C_STATE_BUS_PASSIVE) {
+ if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
+ priv->can.can_stats.error_passive++;
+ new_state = CAN_STATE_ERROR_PASSIVE;
+ } else if (es->status & M16C_STATE_BUS_ERROR) {
+ if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
+ ((es->txerr >= 96) || (es->rxerr >= 96))) {
+ priv->can.can_stats.error_warning++;
+ new_state = CAN_STATE_ERROR_WARNING;
+ } else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
+ ((es->txerr < 96) && (es->rxerr < 96))) {
+ new_state = CAN_STATE_ERROR_ACTIVE;
+ }
+ }
+
+ if (!es->status)
+ new_state = CAN_STATE_ERROR_ACTIVE;
+
+ if (priv->can.restart_ms &&
+ (priv->can.state >= CAN_STATE_BUS_OFF) &&
+ (new_state < CAN_STATE_BUS_OFF)) {
+ priv->can.can_stats.restarts++;
+ }
+
+ if (es->error_factor) {
+ priv->can.can_stats.bus_error++;
+ stats->rx_errors++;
+ }
+
+ priv->bec.txerr = es->txerr;
+ priv->bec.rxerr = es->rxerr;
+ priv->can.state = new_state;
+}
+
static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
const struct kvaser_msg *msg)
{
@@ -622,30 +674,30 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
struct sk_buff *skb;
struct net_device_stats *stats;
struct kvaser_usb_net_priv *priv;
- unsigned int new_state;
- u8 channel, status, txerr, rxerr, error_factor;
+ struct kvaser_usb_error_summary es = { };
+ enum can_state old_state;

switch (msg->id) {
case CMD_CAN_ERROR_EVENT:
- channel = msg->u.error_event.channel;
- status = msg->u.error_event.status;
- txerr = msg->u.error_event.tx_errors_count;
- rxerr = msg->u.error_event.rx_errors_count;
- error_factor = msg->u.error_event.error_factor;
+ es.channel = msg->u.error_event.channel;
+ es.status = msg->u.error_event.status;
+ es.txerr = msg->u.error_event.tx_errors_count;
+ es.rxerr = msg->u.error_event.rx_errors_count;
+ es.error_factor = msg->u.error_event.error_factor;
break;
case CMD_LOG_MESSAGE:
- channel = msg->u.log_message.channel;
- status = msg->u.log_message.data[0];
- txerr = msg->u.log_message.data[2];
- rxerr = msg->u.log_message.data[3];
- error_factor = msg->u.log_message.data[1];
+ es.channel = msg->u.log_message.channel;
+ es.status = msg->u.log_message.data[0];
+ es.txerr = msg->u.log_message.data[2];
+ es.rxerr = msg->u.log_message.data[3];
+ es.error_factor = msg->u.log_message.data[1];
break;
case CMD_CHIP_STATE_EVENT:
- channel = msg->u.chip_state_event.channel;
- status = msg->u.chip_state_event.status;
- txerr = msg->u.chip_state_event.tx_errors_count;
- rxerr = msg->u.chip_state_event.rx_errors_count;
- error_factor = 0;
+ es.channel = msg->u.chip_state_event.channel;
+ es.status = msg->u.chip_state_event.status;
+ es.txerr = msg->u.chip_state_event.tx_errors_count;
+ es.rxerr = msg->u.chip_state_event.rx_errors_count;
+ es.error_factor = 0;
break;
default:
dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
@@ -653,116 +705,93 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
return;
}

- if (channel >= dev->nchannels) {
+ if (es.channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
- "Invalid channel number (%d)\n", channel);
+ "Invalid channel number (%d)\n", es.channel);
return;
}

- priv = dev->nets[channel];
+ priv = dev->nets[es.channel];
stats = &priv->netdev->stats;

+ /* Update all of the can interface's state and error counters before
+ * trying any skb allocation that can actually fail with -ENOMEM.
+ */
+ old_state = priv->can.state;
+ kvaser_usb_rx_error_update_can_state(priv, &es);
+
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
return;
}

- new_state = priv->can.state;
-
- netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
-
- if (status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
+ if (es.status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
cf->can_id |= CAN_ERR_BUSOFF;

- priv->can.can_stats.bus_off++;
if (!priv->can.restart_ms)
kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
-
netif_carrier_off(priv->netdev);
-
- new_state = CAN_STATE_BUS_OFF;
- } else if (status & M16C_STATE_BUS_PASSIVE) {
- if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
+ } else if (es.status & M16C_STATE_BUS_PASSIVE) {
+ if (old_state != CAN_STATE_ERROR_PASSIVE) {
cf->can_id |= CAN_ERR_CRTL;

- if (txerr || rxerr)
- cf->data[1] = (txerr > rxerr)
+ if (es.txerr || es.rxerr)
+ cf->data[1] = (es.txerr > es.rxerr)
? CAN_ERR_CRTL_TX_PASSIVE
: CAN_ERR_CRTL_RX_PASSIVE;
else
cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
CAN_ERR_CRTL_RX_PASSIVE;
-
- priv->can.can_stats.error_passive++;
}
-
- new_state = CAN_STATE_ERROR_PASSIVE;
- } else if (status & M16C_STATE_BUS_ERROR) {
- if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
- ((txerr >= 96) || (rxerr >= 96))) {
+ } else if (es.status & M16C_STATE_BUS_ERROR) {
+ if ((old_state < CAN_STATE_ERROR_WARNING) &&
+ ((es.txerr >= 96) || (es.rxerr >= 96))) {
cf->can_id |= CAN_ERR_CRTL;
- cf->data[1] = (txerr > rxerr)
+ cf->data[1] = (es.txerr > es.rxerr)
? CAN_ERR_CRTL_TX_WARNING
: CAN_ERR_CRTL_RX_WARNING;
-
- priv->can.can_stats.error_warning++;
- new_state = CAN_STATE_ERROR_WARNING;
- } else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
- ((txerr < 96) && (rxerr < 96))) {
+ } else if ((old_state > CAN_STATE_ERROR_ACTIVE) &&
+ ((es.txerr < 96) && (es.rxerr < 96))) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
-
- new_state = CAN_STATE_ERROR_ACTIVE;
}
}

- if (!status) {
+ if (!es.status) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
-
- new_state = CAN_STATE_ERROR_ACTIVE;
}

if (priv->can.restart_ms &&
- (priv->can.state >= CAN_STATE_BUS_OFF) &&
- (new_state < CAN_STATE_BUS_OFF)) {
+ (old_state >= CAN_STATE_BUS_OFF) &&
+ (priv->can.state < CAN_STATE_BUS_OFF)) {
cf->can_id |= CAN_ERR_RESTARTED;
netif_carrier_on(priv->netdev);
-
- priv->can.can_stats.restarts++;
}

- if (error_factor) {
- priv->can.can_stats.bus_error++;
- stats->rx_errors++;
-
+ if (es.error_factor) {
cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;

- if (error_factor & M16C_EF_ACKE)
+ if (es.error_factor & M16C_EF_ACKE)
cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
- if (error_factor & M16C_EF_CRCE)
+ if (es.error_factor & M16C_EF_CRCE)
cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
CAN_ERR_PROT_LOC_CRC_DEL);
- if (error_factor & M16C_EF_FORME)
+ if (es.error_factor & M16C_EF_FORME)
cf->data[2] |= CAN_ERR_PROT_FORM;
- if (error_factor & M16C_EF_STFE)
+ if (es.error_factor & M16C_EF_STFE)
cf->data[2] |= CAN_ERR_PROT_STUFF;
- if (error_factor & M16C_EF_BITE0)
+ if (es.error_factor & M16C_EF_BITE0)
cf->data[2] |= CAN_ERR_PROT_BIT0;
- if (error_factor & M16C_EF_BITE1)
+ if (es.error_factor & M16C_EF_BITE1)
cf->data[2] |= CAN_ERR_PROT_BIT1;
- if (error_factor & M16C_EF_TRE)
+ if (es.error_factor & M16C_EF_TRE)
cf->data[2] |= CAN_ERR_PROT_TX;
}

- cf->data[6] = txerr;
- cf->data[7] = rxerr;
-
- priv->bec.txerr = txerr;
- priv->bec.rxerr = rxerr;
-
- priv->can.state = new_state;
+ cf->data[6] = es.txerr;
+ cf->data[7] = es.rxerr;

stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
@@ -786,6 +815,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
}

if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
+ stats->rx_over_errors++;
+ stats->rx_errors++;
+
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
@@ -795,9 +827,6 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
cf->can_id |= CAN_ERR_CRTL;
cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;

- stats->rx_over_errors++;
- stats->rx_errors++;
-
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
netif_rx(skb);
--
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM [ In reply to ]
andri.yngvason at marel
Jan 26, 2015, 2:28 AM
Post #2 of 5 (547 views)
Permalink
Quoting Ahmed S. Darwish (2015-01-26 05:27:19)
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>
> Update all of the can interface's state and error counters before
> trying any skb allocation that can actually fail with -ENOMEM.
>
> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> ---
> drivers/net/can/usb/kvaser_usb.c | 181 ++++++++++++++++++++++----------------
> 1 files changed, 105 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index 7af379c..f57ce55 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -273,6 +273,10 @@ struct kvaser_msg {
> } u;
> } __packed;
>
> +struct kvaser_usb_error_summary {
> + u8 channel, status, txerr, rxerr, error_factor;
> +};
> +
> struct kvaser_usb_tx_urb_context {
> struct kvaser_usb_net_priv *priv;
> u32 echo_index;
> @@ -615,6 +619,54 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> }
>
> +static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
> + const struct kvaser_usb_error_summary *es)
> +{
> + struct net_device_stats *stats;
> + enum can_state new_state;
> +
> + stats = &priv->netdev->stats;
> + new_state = priv->can.state;
> +
> + netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
> +
> + if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> + priv->can.can_stats.bus_off++;
> + new_state = CAN_STATE_BUS_OFF;
> + } else if (es->status & M16C_STATE_BUS_PASSIVE) {
> + if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
> + priv->can.can_stats.error_passive++;
> + new_state = CAN_STATE_ERROR_PASSIVE;
> + } else if (es->status & M16C_STATE_BUS_ERROR) {
> + if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> + ((es->txerr >= 96) || (es->rxerr >= 96))) {
> + priv->can.can_stats.error_warning++;
> + new_state = CAN_STATE_ERROR_WARNING;
> + } else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
> + ((es->txerr < 96) && (es->rxerr < 96))) {
> + new_state = CAN_STATE_ERROR_ACTIVE;
> + }
> + }
> +
> + if (!es->status)
> + new_state = CAN_STATE_ERROR_ACTIVE;
> +
> + if (priv->can.restart_ms &&
> + (priv->can.state >= CAN_STATE_BUS_OFF) &&
> + (new_state < CAN_STATE_BUS_OFF)) {
> + priv->can.can_stats.restarts++;
> + }
> +
> + if (es->error_factor) {
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + }
> +
> + priv->bec.txerr = es->txerr;
> + priv->bec.rxerr = es->rxerr;
> + priv->can.state = new_state;
> +}
> +
> static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> const struct kvaser_msg *msg)
> {
> @@ -622,30 +674,30 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> struct sk_buff *skb;
> struct net_device_stats *stats;
> struct kvaser_usb_net_priv *priv;
> - unsigned int new_state;
> - u8 channel, status, txerr, rxerr, error_factor;
> + struct kvaser_usb_error_summary es = { };
> + enum can_state old_state;
>
> switch (msg->id) {
> case CMD_CAN_ERROR_EVENT:
> - channel = msg->u.error_event.channel;
> - status = msg->u.error_event.status;
> - txerr = msg->u.error_event.tx_errors_count;
> - rxerr = msg->u.error_event.rx_errors_count;
> - error_factor = msg->u.error_event.error_factor;
> + es.channel = msg->u.error_event.channel;
> + es.status = msg->u.error_event.status;
> + es.txerr = msg->u.error_event.tx_errors_count;
> + es.rxerr = msg->u.error_event.rx_errors_count;
> + es.error_factor = msg->u.error_event.error_factor;
> break;
> case CMD_LOG_MESSAGE:
> - channel = msg->u.log_message.channel;
> - status = msg->u.log_message.data[0];
> - txerr = msg->u.log_message.data[2];
> - rxerr = msg->u.log_message.data[3];
> - error_factor = msg->u.log_message.data[1];
> + es.channel = msg->u.log_message.channel;
> + es.status = msg->u.log_message.data[0];
> + es.txerr = msg->u.log_message.data[2];
> + es.rxerr = msg->u.log_message.data[3];
> + es.error_factor = msg->u.log_message.data[1];
> break;
> case CMD_CHIP_STATE_EVENT:
> - channel = msg->u.chip_state_event.channel;
> - status = msg->u.chip_state_event.status;
> - txerr = msg->u.chip_state_event.tx_errors_count;
> - rxerr = msg->u.chip_state_event.rx_errors_count;
> - error_factor = 0;
> + es.channel = msg->u.chip_state_event.channel;
> + es.status = msg->u.chip_state_event.status;
> + es.txerr = msg->u.chip_state_event.tx_errors_count;
> + es.rxerr = msg->u.chip_state_event.rx_errors_count;
> + es.error_factor = 0;
> break;
> default:
> dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> @@ -653,116 +705,93 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> return;
> }
>
> - if (channel >= dev->nchannels) {
> + if (es.channel >= dev->nchannels) {
> dev_err(dev->udev->dev.parent,
> - "Invalid channel number (%d)\n", channel);
> + "Invalid channel number (%d)\n", es.channel);
> return;
> }
>
> - priv = dev->nets[channel];
> + priv = dev->nets[es.channel];
> stats = &priv->netdev->stats;
>
> + /* Update all of the can interface's state and error counters before
> + * trying any skb allocation that can actually fail with -ENOMEM.
> + */
> + old_state = priv->can.state;
> + kvaser_usb_rx_error_update_can_state(priv, &es);
> +
> skb = alloc_can_err_skb(priv->netdev, &cf);
> if (!skb) {
> stats->rx_dropped++;
> return;
> }
>
> - new_state = priv->can.state;
> -
> - netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
> -
> - if (status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> + if (es.status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> cf->can_id |= CAN_ERR_BUSOFF;
>
> - priv->can.can_stats.bus_off++;
> if (!priv->can.restart_ms)
> kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> -
> netif_carrier_off(priv->netdev);
> -
> - new_state = CAN_STATE_BUS_OFF;
> - } else if (status & M16C_STATE_BUS_PASSIVE) {
> - if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
> + } else if (es.status & M16C_STATE_BUS_PASSIVE) {
> + if (old_state != CAN_STATE_ERROR_PASSIVE) {
> cf->can_id |= CAN_ERR_CRTL;
>
> - if (txerr || rxerr)
> - cf->data[1] = (txerr > rxerr)
> + if (es.txerr || es.rxerr)
> + cf->data[1] = (es.txerr > es.rxerr)
> ? CAN_ERR_CRTL_TX_PASSIVE
> : CAN_ERR_CRTL_RX_PASSIVE;
> else
> cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
> CAN_ERR_CRTL_RX_PASSIVE;
> -
> - priv->can.can_stats.error_passive++;
> }
> -
> - new_state = CAN_STATE_ERROR_PASSIVE;
> - } else if (status & M16C_STATE_BUS_ERROR) {
> - if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> - ((txerr >= 96) || (rxerr >= 96))) {
> + } else if (es.status & M16C_STATE_BUS_ERROR) {
> + if ((old_state < CAN_STATE_ERROR_WARNING) &&
> + ((es.txerr >= 96) || (es.rxerr >= 96))) {
> cf->can_id |= CAN_ERR_CRTL;
> - cf->data[1] = (txerr > rxerr)
> + cf->data[1] = (es.txerr > es.rxerr)
> ? CAN_ERR_CRTL_TX_WARNING
> : CAN_ERR_CRTL_RX_WARNING;
> -
> - priv->can.can_stats.error_warning++;
> - new_state = CAN_STATE_ERROR_WARNING;
> - } else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
> - ((txerr < 96) && (rxerr < 96))) {
> + } else if ((old_state > CAN_STATE_ERROR_ACTIVE) &&
> + ((es.txerr < 96) && (es.rxerr < 96))) {
> cf->can_id |= CAN_ERR_PROT;
> cf->data[2] = CAN_ERR_PROT_ACTIVE;
> -
> - new_state = CAN_STATE_ERROR_ACTIVE;
> }
> }
>
> - if (!status) {
> + if (!es.status) {
> cf->can_id |= CAN_ERR_PROT;
> cf->data[2] = CAN_ERR_PROT_ACTIVE;
> -
> - new_state = CAN_STATE_ERROR_ACTIVE;
> }
>
> if (priv->can.restart_ms &&
> - (priv->can.state >= CAN_STATE_BUS_OFF) &&
> - (new_state < CAN_STATE_BUS_OFF)) {
> + (old_state >= CAN_STATE_BUS_OFF) &&
> + (priv->can.state < CAN_STATE_BUS_OFF)) {
> cf->can_id |= CAN_ERR_RESTARTED;
> netif_carrier_on(priv->netdev);
> -
> - priv->can.can_stats.restarts++;
> }
>
> - if (error_factor) {
> - priv->can.can_stats.bus_error++;
> - stats->rx_errors++;
> -
> + if (es.error_factor) {
> cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
>
> - if (error_factor & M16C_EF_ACKE)
> + if (es.error_factor & M16C_EF_ACKE)
> cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> - if (error_factor & M16C_EF_CRCE)
> + if (es.error_factor & M16C_EF_CRCE)
> cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> CAN_ERR_PROT_LOC_CRC_DEL);
> - if (error_factor & M16C_EF_FORME)
> + if (es.error_factor & M16C_EF_FORME)
> cf->data[2] |= CAN_ERR_PROT_FORM;
> - if (error_factor & M16C_EF_STFE)
> + if (es.error_factor & M16C_EF_STFE)
> cf->data[2] |= CAN_ERR_PROT_STUFF;
> - if (error_factor & M16C_EF_BITE0)
> + if (es.error_factor & M16C_EF_BITE0)
> cf->data[2] |= CAN_ERR_PROT_BIT0;
> - if (error_factor & M16C_EF_BITE1)
> + if (es.error_factor & M16C_EF_BITE1)
> cf->data[2] |= CAN_ERR_PROT_BIT1;
> - if (error_factor & M16C_EF_TRE)
> + if (es.error_factor & M16C_EF_TRE)
> cf->data[2] |= CAN_ERR_PROT_TX;
> }
>
> - cf->data[6] = txerr;
> - cf->data[7] = rxerr;
> -
> - priv->bec.txerr = txerr;
> - priv->bec.rxerr = rxerr;
> -
> - priv->can.state = new_state;
> + cf->data[6] = es.txerr;
> + cf->data[7] = es.rxerr;
>
> stats->rx_packets++;
> stats->rx_bytes += cf->can_dlc;
> @@ -786,6 +815,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> }
>
> if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> + stats->rx_over_errors++;
> + stats->rx_errors++;
> +
> skb = alloc_can_err_skb(priv->netdev, &cf);
> if (!skb) {
> stats->rx_dropped++;
> @@ -795,9 +827,6 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> cf->can_id |= CAN_ERR_CRTL;
> cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
>
> - stats->rx_over_errors++;
> - stats->rx_errors++;
> -
> stats->rx_packets++;
> stats->rx_bytes += cf->can_dlc;
> netif_rx(skb);
> --
> 1.7.7.6

Looks good to me.

--
Andri
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM [ In reply to ]
mkl at pengutronix
Jan 26, 2015, 2:50 AM
Post #3 of 5 (548 views)
Permalink
On 01/26/2015 11:28 AM, Andri Yngvason wrote:
> Quoting Ahmed S. Darwish (2015-01-26 05:27:19)
>> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>>
>> Update all of the can interface's state and error counters before
>> trying any skb allocation that can actually fail with -ENOMEM.
>>
>> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>> ---
>> drivers/net/can/usb/kvaser_usb.c | 181 ++++++++++++++++++++++----------------
>> 1 files changed, 105 insertions(+), 76 deletions(-)
[..]

> Looks good to me.

Can I add your Acked-by to 5-7?

Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |

Attached Files:

Re: [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM [ In reply to ]
andri.yngvason at marel
Jan 26, 2015, 2:52 AM
Post #4 of 5 (547 views)
Permalink
Quoting Marc Kleine-Budde (2015-01-26 10:50:12)
> On 01/26/2015 11:28 AM, Andri Yngvason wrote:
> > Quoting Ahmed S. Darwish (2015-01-26 05:27:19)
> >> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> >>
> >> Update all of the can interface's state and error counters before
> >> trying any skb allocation that can actually fail with -ENOMEM.
> >>
> >> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> >> ---
> >> drivers/net/can/usb/kvaser_usb.c | 181 ++++++++++++++++++++++----------------
> >> 1 files changed, 105 insertions(+), 76 deletions(-)
> [..]
>
> > Looks good to me.
>
> Can I add your Acked-by to 5-7?
>
Yes.

--
Andri
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM [ In reply to ]
mkl at pengutronix
Jan 26, 2015, 2:53 AM
Post #5 of 5 (547 views)
Permalink
On 01/26/2015 11:52 AM, Andri Yngvason wrote:
> Quoting Marc Kleine-Budde (2015-01-26 10:50:12)
>> On 01/26/2015 11:28 AM, Andri Yngvason wrote:
>>> Quoting Ahmed S. Darwish (2015-01-26 05:27:19)
>>>> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>>>>
>>>> Update all of the can interface's state and error counters before
>>>> trying any skb allocation that can actually fail with -ENOMEM.
>>>>
>>>> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>>> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>>>> ---
>>>> drivers/net/can/usb/kvaser_usb.c | 181 ++++++++++++++++++++++----------------
>>>> 1 files changed, 105 insertions(+), 76 deletions(-)
>> [..]
>>
>>> Looks good to me.
>>
>> Can I add your Acked-by to 5-7?
>>
> Yes.

Tnx.

Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |

Attached Files:

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal