[PATCH 0/5] can: peak_usb: upgrade driver for PCAN-USB

[PATCH 0/5] can: peak_usb: upgrade driver for PCAN-USB

[Stephane Grosjean]

This bundle of patches allows to upgrade the old code of the device
driver for the PCAN-USB made by PEAK-System GmbH. In particular:

- it adds support for loopback and one-shot modes when the version of the
  embedded firmware allows it
- it corrects the reading of error counters
- it updates the management of bus state changes using the functions
  exported by the can-dev module and the error counters values

Stephane Grosjean (5):
  can: peak_usb: pcan_usb_get_device_id(): read value only in case of
    success
  can: peak_usb: PCAN-USB: add support of loopback and one-short mode
  can: peak_usb: pcan_usb_handle_bus_evt(): fix reading rxerr/txerr
    values
  can: peak_usb: pcan_usb_encode_msg(): adds information
  can: peak_usb: upgrades the handling of bus state changes

 drivers/net/can/usb/peak_usb/pcan_usb.c | 238 ++++++++++--------------
 1 file changed, 99 insertions(+), 139 deletions(-)

-- 
2.25.1

[PATCH 1/5] can: peak_usb: pcan_usb_get_device_id(): read value only in case of success

[Stephane Grosjean]

In case of error, reading value from response argument is useless.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 1d6f77252f01..9f3e16684e28 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -384,7 +384,8 @@ static int pcan_usb_get_device_id(struct peak_usb_device *dev, u32 *device_id)
 	if (err)
 		netdev_err(dev->netdev, "getting device id failure: %d\n", err);
 
-	*device_id = args[0];
+	else
+		*device_id = args[0];
 
 	return err;
 }
-- 
2.25.1

[PATCH 2/5] can: peak_usb: PCAN-USB: add support of loopback and one-short mode

[Stephane Grosjean]

The CAN - USB PCAN-USB interface is able to generate one-shot frames as
well as loopback frames that it transmits starting from version 4.1 of its
firmware.

This patch allows to add these one-shot and loopback functions to the
driver when the embedded firmware allows it.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c | 50 +++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 9f3e16684e28..4c86cf58eac3 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -73,6 +73,10 @@
 #define PCAN_USB_STATUSLEN_RTR		(1 << 4)
 #define PCAN_USB_STATUSLEN_DLC		(0xf)
 
+/* PCAN-USB 4.1 CAN Id tx extended flags */
+#define PCAN_USB_TX_SRR			0x01	/* SJA1000 SRR command */
+#define PCAN_USB_TX_AT			0x02	/* SJA1000 AT command */
+
 /* PCAN-USB error flags */
 #define PCAN_USB_ERROR_TXFULL		0x01
 #define PCAN_USB_ERROR_RXQOVR		0x02
@@ -705,6 +709,7 @@ static int pcan_usb_decode_data(struct pcan_usb_msg_context *mc, u8 status_len)
 	struct sk_buff *skb;
 	struct can_frame *cf;
 	struct skb_shared_hwtstamps *hwts;
+	u32 can_id_flags;
 
 	skb = alloc_can_skb(mc->netdev, &cf);
 	if (!skb)
@@ -714,13 +719,15 @@ static int pcan_usb_decode_data(struct pcan_usb_msg_context *mc, u8 status_len)
 		if ((mc->ptr + 4) > mc->end)
 			goto decode_failed;
 
-		cf->can_id = get_unaligned_le32(mc->ptr) >> 3 | CAN_EFF_FLAG;
+		can_id_flags = get_unaligned_le32(mc->ptr);
+		cf->can_id = can_id_flags >> 3 | CAN_EFF_FLAG;
 		mc->ptr += 4;
 	} else {
 		if ((mc->ptr + 2) > mc->end)
 			goto decode_failed;
 
-		cf->can_id = get_unaligned_le16(mc->ptr) >> 5;
+		can_id_flags = get_unaligned_le16(mc->ptr);
+		cf->can_id = can_id_flags >> 5;
 		mc->ptr += 2;
 	}
 
@@ -743,6 +750,10 @@ static int pcan_usb_decode_data(struct pcan_usb_msg_context *mc, u8 status_len)
 
 		memcpy(cf->data, mc->ptr, cf->len);
 		mc->ptr += rec_len;
+
+		/* Ignore next byte (client private id) if SRR bit is set */
+		if (can_id_flags & PCAN_USB_TX_SRR)
+			mc->ptr++;
 	}
 
 	/* convert timestamp into kernel time */
@@ -820,6 +831,7 @@ static int pcan_usb_encode_msg(struct peak_usb_device *dev, struct sk_buff *skb,
 	struct net_device *netdev = dev->netdev;
 	struct net_device_stats *stats = &netdev->stats;
 	struct can_frame *cf = (struct can_frame *)skb->data;
+	u32 can_id_flags = cf->can_id & CAN_ERR_MASK;
 	u8 *pc;
 
 	obuf[0] = 2;
@@ -838,12 +850,28 @@ static int pcan_usb_encode_msg(struct peak_usb_device *dev, struct sk_buff *skb,
 		*pc |= PCAN_USB_STATUSLEN_EXT_ID;
 		pc++;
 
-		put_unaligned_le32((cf->can_id & CAN_ERR_MASK) << 3, pc);
+		can_id_flags <<= 3;
+
+		if (dev->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
+			can_id_flags |= PCAN_USB_TX_SRR;
+
+		if (dev->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
+			can_id_flags |= PCAN_USB_TX_AT;
+
+		put_unaligned_le32(can_id_flags, pc);
 		pc += 4;
 	} else {
 		pc++;
 
-		put_unaligned_le16((cf->can_id & CAN_ERR_MASK) << 5, pc);
+		can_id_flags <<= 5;
+
+		if (dev->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
+			can_id_flags |= PCAN_USB_TX_SRR;
+
+		if (dev->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
+			can_id_flags |= PCAN_USB_TX_AT;
+
+		put_unaligned_le16(can_id_flags, pc);
 		pc += 2;
 	}
 
@@ -853,6 +881,10 @@ static int pcan_usb_encode_msg(struct peak_usb_device *dev, struct sk_buff *skb,
 		pc += cf->len;
 	}
 
+	/* SRR bit needs a writer id (useless here) */
+	if (can_id_flags & PCAN_USB_TX_SRR)
+		*pc++ = 0x80;
+
 	obuf[(*size)-1] = (u8)(stats->tx_packets & 0xff);
 
 	return 0;
@@ -927,6 +959,16 @@ static int pcan_usb_init(struct peak_usb_device *dev)
 		return err;
 	}
 
+	/* Since rev 4.1, PCAN-USB is able to make single-shot as well as
+	 * looped back frames.
+	 */
+	if (dev->device_rev >= 41) {
+		struct can_priv *priv = netdev_priv(dev->netdev);
+
+		priv->ctrlmode_supported |= CAN_CTRLMODE_ONE_SHOT |
+					    CAN_CTRLMODE_LOOPBACK;
+	}
+
 	dev_info(dev->netdev->dev.parent,
 		 "PEAK-System %s adapter hwrev %u serial %08X (%u channel)\n",
 		 pcan_usb.name, dev->device_rev, serial_number,
-- 
2.25.1

Re: [PATCH 2/5] can: peak_usb: PCAN-USB: add support of loopback and one-short mode

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On 25.06.2021 15:09:28, Stephane Grosjean wrote:
> The CAN - USB PCAN-USB interface is able to generate one-shot frames as
> well as loopback frames that it transmits starting from version 4.1 of its
> firmware.
> 
> This patch allows to add these one-shot and loopback functions to the
> driver when the embedded firmware allows it.

Does it makes sense print a firmware update available message if the
adapter doesn't run the latest firmware?

Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 3/5] can: peak_usb: pcan_usb_handle_bus_evt(): fix reading rxerr/txerr values

[Stephane Grosjean]

This patch fixes an incorrect way of reading error counters in messages
received for this purpose from the PCAN-USB interface. These messages
inform about the increase or decrease of the error counters, whose values
are placed in bytes 1 and 2 of the message data (not 0 and 1).

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 4c86cf58eac3..341ace36570b 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -121,7 +121,8 @@
 #define PCAN_USB_BERR_MASK	(PCAN_USB_ERR_RXERR | PCAN_USB_ERR_TXERR)
 
 /* identify bus event packets with rx/tx error counters */
-#define PCAN_USB_ERR_CNT		0x80
+#define PCAN_USB_ERR_CNT_DEC		0x00	/* counters are decreasing */
+#define PCAN_USB_ERR_CNT_INC		0x80	/* counters are increasing */
 
 /* private to PCAN-USB adapter */
 struct pcan_usb {
@@ -613,11 +614,12 @@ static int pcan_usb_handle_bus_evt(struct pcan_usb_msg_context *mc, u8 ir)
 
 	/* acccording to the content of the packet */
 	switch (ir) {
-	case PCAN_USB_ERR_CNT:
+	case PCAN_USB_ERR_CNT_DEC:
+	case PCAN_USB_ERR_CNT_INC:
 
 		/* save rx/tx error counters from in the device context */
-		pdev->bec.rxerr = mc->ptr[0];
-		pdev->bec.txerr = mc->ptr[1];
+		pdev->bec.rxerr = mc->ptr[1];
+		pdev->bec.txerr = mc->ptr[2];
 		break;
 
 	default:
-- 
2.25.1

Re: [PATCH 3/5] can: peak_usb: pcan_usb_handle_bus_evt(): fix reading rxerr/txerr values

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On 25.06.2021 15:09:29, Stephane Grosjean wrote:
> This patch fixes an incorrect way of reading error counters in messages
> received for this purpose from the PCAN-USB interface. These messages
> inform about the increase or decrease of the error counters, whose values
> are placed in bytes 1 and 2 of the message data (not 0 and 1).
> 
> Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>

Added to linux-can/testing, added a fixes tag and stable on Cc.

regards,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 4/5] can: peak_usb: pcan_usb_encode_msg(): adds information

[Stephane Grosjean]

This patch adds information by replacing hard-coded values with their
symbol and adding comments.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 341ace36570b..7d18bc6911f5 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -63,6 +63,8 @@
 
 #define PCAN_USB_MSG_HEADER_LEN		2
 
+#define PCAN_USB_MSG_TX_CAN		2	/* Tx msg is a CAN frame */
+
 /* PCAN-USB adapter internal clock (MHz) */
 #define PCAN_USB_CRYSTAL_HZ		16000000
 
@@ -836,8 +838,8 @@ static int pcan_usb_encode_msg(struct peak_usb_device *dev, struct sk_buff *skb,
 	u32 can_id_flags = cf->can_id & CAN_ERR_MASK;
 	u8 *pc;
 
-	obuf[0] = 2;
-	obuf[1] = 1;
+	obuf[0] = PCAN_USB_MSG_TX_CAN;
+	obuf[1] = 1;	/* only one CAN frame is stored in the packet */
 
 	pc = obuf + PCAN_USB_MSG_HEADER_LEN;
 
-- 
2.25.1

[PATCH 5/5] can: peak_usb: upgrades the handling of bus state changes

[Stephane Grosjean]

This patch updates old code by using the functions published since by the
socket-can module. In particular, this new code better manages the change
of bus state by also using the value of the error counters that the driver
now systematically asks for when initializing the channel.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c | 169 ++++++------------------
 1 file changed, 41 insertions(+), 128 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 7d18bc6911f5..1ab3be8dbb83 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -453,146 +453,59 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
 {
 	struct sk_buff *skb;
 	struct can_frame *cf;
-	enum can_state new_state;
+	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
 
 	/* ignore this error until 1st ts received */
 	if (n == PCAN_USB_ERROR_QOVR)
 		if (!mc->pdev->time_ref.tick_count)
 			return 0;
 
-	new_state = mc->pdev->dev.can.state;
-
-	switch (mc->pdev->dev.can.state) {
-	case CAN_STATE_ERROR_ACTIVE:
-		if (n & PCAN_USB_ERROR_BUS_LIGHT) {
-			new_state = CAN_STATE_ERROR_WARNING;
-			break;
-		}
-		fallthrough;
-
-	case CAN_STATE_ERROR_WARNING:
-		if (n & PCAN_USB_ERROR_BUS_HEAVY) {
-			new_state = CAN_STATE_ERROR_PASSIVE;
-			break;
-		}
-		if (n & PCAN_USB_ERROR_BUS_OFF) {
-			new_state = CAN_STATE_BUS_OFF;
-			break;
-		}
-		if (n & ~PCAN_USB_ERROR_BUS) {
-			/*
-			 * trick to bypass next comparison and process other
-			 * errors
-			 */
-			new_state = CAN_STATE_MAX;
-			break;
-		}
-		if ((n & PCAN_USB_ERROR_BUS_LIGHT) == 0) {
-			/* no error (back to active state) */
-			new_state = CAN_STATE_ERROR_ACTIVE;
-			break;
-		}
-		break;
-
-	case CAN_STATE_ERROR_PASSIVE:
-		if (n & PCAN_USB_ERROR_BUS_OFF) {
-			new_state = CAN_STATE_BUS_OFF;
-			break;
-		}
-		if (n & PCAN_USB_ERROR_BUS_LIGHT) {
-			new_state = CAN_STATE_ERROR_WARNING;
-			break;
-		}
-		if (n & ~PCAN_USB_ERROR_BUS) {
-			/*
-			 * trick to bypass next comparison and process other
-			 * errors
-			 */
-			new_state = CAN_STATE_MAX;
-			break;
-		}
-
-		if ((n & PCAN_USB_ERROR_BUS_HEAVY) == 0) {
-			/* no error (back to warning state) */
-			new_state = CAN_STATE_ERROR_WARNING;
-			break;
-		}
-		break;
-
-	default:
-		/* do nothing waiting for restart */
+	if (n & PCAN_USB_ERROR_TXQFULL) {
+		netdev_dbg(mc->netdev, "device Tx queue full)\n");
 		return 0;
 	}
 
-	/* donot post any error if current state didn't change */
-	if (mc->pdev->dev.can.state == new_state)
-		return 0;
-
 	/* allocate an skb to store the error frame */
 	skb = alloc_can_err_skb(mc->netdev, &cf);
 	if (!skb)
-		return -ENOMEM;
-
-	switch (new_state) {
-	case CAN_STATE_BUS_OFF:
-		cf->can_id |= CAN_ERR_BUSOFF;
-		mc->pdev->dev.can.can_stats.bus_off++;
-		can_bus_off(mc->netdev);
-		break;
-
-	case CAN_STATE_ERROR_PASSIVE:
-		cf->can_id |= CAN_ERR_CRTL;
-		cf->data[1] = (mc->pdev->bec.txerr > mc->pdev->bec.rxerr) ?
-				CAN_ERR_CRTL_TX_PASSIVE :
-				CAN_ERR_CRTL_RX_PASSIVE;
-		cf->data[6] = mc->pdev->bec.txerr;
-		cf->data[7] = mc->pdev->bec.rxerr;
-
-		mc->pdev->dev.can.can_stats.error_passive++;
-		break;
-
-	case CAN_STATE_ERROR_WARNING:
-		cf->can_id |= CAN_ERR_CRTL;
-		cf->data[1] = (mc->pdev->bec.txerr > mc->pdev->bec.rxerr) ?
-				CAN_ERR_CRTL_TX_WARNING :
-				CAN_ERR_CRTL_RX_WARNING;
-		cf->data[6] = mc->pdev->bec.txerr;
-		cf->data[7] = mc->pdev->bec.rxerr;
-
-		mc->pdev->dev.can.can_stats.error_warning++;
-		break;
+		return 0;
 
-	case CAN_STATE_ERROR_ACTIVE:
+	if (n & PCAN_USB_ERROR_RXQOVR) {
+		/* data overrun interrupt */
+		netdev_dbg(mc->netdev, "data overrun interrupt\n");
 		cf->can_id |= CAN_ERR_CRTL;
-		cf->data[1] = CAN_ERR_CRTL_ACTIVE;
+		cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
+		mc->netdev->stats.rx_over_errors++;
+		mc->netdev->stats.rx_errors++;
+	}
 
-		/* sync local copies of rxerr/txerr counters */
-		mc->pdev->bec.txerr = 0;
-		mc->pdev->bec.rxerr = 0;
-		break;
+	if (n & PCAN_USB_ERROR_BUS_OFF) {
+		new_state = CAN_STATE_BUS_OFF;
+	} else if (n & PCAN_USB_ERROR_BUS_HEAVY) {
+		new_state = ((mc->pdev->bec.txerr >= 128) ||
+			     (mc->pdev->bec.rxerr >= 128)) ?
+				CAN_STATE_ERROR_PASSIVE :
+				CAN_STATE_ERROR_WARNING;
+	} else {
+		new_state = CAN_STATE_ERROR_ACTIVE;
+	}
 
-	default:
-		/* CAN_STATE_MAX (trick to handle other errors) */
-		if (n & PCAN_USB_ERROR_TXQFULL)
-			netdev_dbg(mc->netdev, "device Tx queue full)\n");
-
-		if (n & PCAN_USB_ERROR_RXQOVR) {
-			netdev_dbg(mc->netdev, "data overrun interrupt\n");
-			cf->can_id |= CAN_ERR_CRTL;
-			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
-			mc->netdev->stats.rx_over_errors++;
-			mc->netdev->stats.rx_errors++;
-		}
+	/* handle change of state */
+	if (new_state != mc->pdev->dev.can.state) {
+		enum can_state tx_state =
+			(mc->pdev->bec.txerr >= mc->pdev->bec.rxerr) ?
+				new_state : 0;
+		enum can_state rx_state =
+			(mc->pdev->bec.txerr <= mc->pdev->bec.rxerr) ?
+				new_state : 0;
 
-		cf->data[6] = mc->pdev->bec.txerr;
-		cf->data[7] = mc->pdev->bec.rxerr;
+		can_change_state(mc->netdev, cf, tx_state, rx_state);
 
-		new_state = mc->pdev->dev.can.state;
-		break;
+		/* things must be done even in case of OOM */
+		if (new_state == CAN_STATE_BUS_OFF)
+			can_bus_off(mc->netdev);
 	}
 
-	mc->pdev->dev.can.state = new_state;
-
 	if (status_len & PCAN_USB_STATUSLEN_TIMESTAMP) {
 		struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
 
@@ -921,14 +834,14 @@ static int pcan_usb_start(struct peak_usb_device *dev)
 	pdev->bec.rxerr = 0;
 	pdev->bec.txerr = 0;
 
-	/* be notified on error counter changes (if requested by user) */
-	if (dev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) {
-		err = pcan_usb_set_err_frame(dev, PCAN_USB_BERR_MASK);
-		if (err)
-			netdev_warn(dev->netdev,
-				    "Asking for BERR reporting error %u\n",
-				    err);
-	}
+	/* always ask the device for BERR reporting, to be able to switch from
+	 * WARNING to PASSIVE state
+	 */
+	err = pcan_usb_set_err_frame(dev, PCAN_USB_BERR_MASK);
+	if (err)
+		netdev_warn(dev->netdev,
+			    "Asking for BERR reporting error %u\n",
+			    err);
 
 	/* if revision greater than 3, can put silent mode on/off */
 	if (dev->device_rev > 3) {
-- 
2.25.1

Re: [PATCH 5/5] can: peak_usb: upgrades the handling of bus state changes

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 3474 bytes --]

On 25.06.2021 15:09:31, Stephane Grosjean wrote:
> This patch updates old code by using the functions published since by the
> socket-can module. In particular, this new code better manages the change
> of bus state by also using the value of the error counters that the driver
> now systematically asks for when initializing the channel.
> 
> Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
> ---
>  drivers/net/can/usb/peak_usb/pcan_usb.c | 169 ++++++------------------
>  1 file changed, 41 insertions(+), 128 deletions(-)
> 
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
> index 7d18bc6911f5..1ab3be8dbb83 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
> @@ -453,146 +453,59 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
>  {
>  	struct sk_buff *skb;
>  	struct can_frame *cf;
> -	enum can_state new_state;
> +	enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
>  
>  	/* ignore this error until 1st ts received */
>  	if (n == PCAN_USB_ERROR_QOVR)
>  		if (!mc->pdev->time_ref.tick_count)
>  			return 0;
>  
> -	new_state = mc->pdev->dev.can.state;
> -
> -	switch (mc->pdev->dev.can.state) {
> -	case CAN_STATE_ERROR_ACTIVE:
> -		if (n & PCAN_USB_ERROR_BUS_LIGHT) {
> -			new_state = CAN_STATE_ERROR_WARNING;
> -			break;
> -		}
> -		fallthrough;
> -
> -	case CAN_STATE_ERROR_WARNING:
> -		if (n & PCAN_USB_ERROR_BUS_HEAVY) {
> -			new_state = CAN_STATE_ERROR_PASSIVE;
> -			break;
> -		}
> -		if (n & PCAN_USB_ERROR_BUS_OFF) {
> -			new_state = CAN_STATE_BUS_OFF;
> -			break;
> -		}
> -		if (n & ~PCAN_USB_ERROR_BUS) {
> -			/*
> -			 * trick to bypass next comparison and process other
> -			 * errors
> -			 */
> -			new_state = CAN_STATE_MAX;
> -			break;
> -		}
> -		if ((n & PCAN_USB_ERROR_BUS_LIGHT) == 0) {
> -			/* no error (back to active state) */
> -			new_state = CAN_STATE_ERROR_ACTIVE;
> -			break;
> -		}
> -		break;
> -
> -	case CAN_STATE_ERROR_PASSIVE:
> -		if (n & PCAN_USB_ERROR_BUS_OFF) {
> -			new_state = CAN_STATE_BUS_OFF;
> -			break;
> -		}
> -		if (n & PCAN_USB_ERROR_BUS_LIGHT) {
> -			new_state = CAN_STATE_ERROR_WARNING;
> -			break;
> -		}
> -		if (n & ~PCAN_USB_ERROR_BUS) {
> -			/*
> -			 * trick to bypass next comparison and process other
> -			 * errors
> -			 */
> -			new_state = CAN_STATE_MAX;
> -			break;
> -		}
> -
> -		if ((n & PCAN_USB_ERROR_BUS_HEAVY) == 0) {
> -			/* no error (back to warning state) */
> -			new_state = CAN_STATE_ERROR_WARNING;
> -			break;
> -		}
> -		break;
> -
> -	default:
> -		/* do nothing waiting for restart */
> +	if (n & PCAN_USB_ERROR_TXQFULL) {
> +		netdev_dbg(mc->netdev, "device Tx queue full)\n");
>  		return 0;
>  	}
>  
> -	/* donot post any error if current state didn't change */
> -	if (mc->pdev->dev.can.state == new_state)
> -		return 0;
> -
>  	/* allocate an skb to store the error frame */
>  	skb = alloc_can_err_skb(mc->netdev, &cf);

Please update the driver, to do the statistics and state update, even if
the allocation of the skb fails.

Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/5] can: peak_usb: upgrade driver for PCAN-USB

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

On 25.06.2021 15:09:26, Stephane Grosjean wrote:
> This bundle of patches allows to upgrade the old code of the device
> driver for the PCAN-USB made by PEAK-System GmbH. In particular:
> 
> - it adds support for loopback and one-shot modes when the version of the
>   embedded firmware allows it
> - it corrects the reading of error counters
> - it updates the management of bus state changes using the functions
>   exported by the can-dev module and the error counters values
> 
> Stephane Grosjean (5):
>   can: peak_usb: pcan_usb_get_device_id(): read value only in case of
>     success
>   can: peak_usb: PCAN-USB: add support of loopback and one-short mode
>   can: peak_usb: pcan_usb_handle_bus_evt(): fix reading rxerr/txerr
>     values
>   can: peak_usb: pcan_usb_encode_msg(): adds information
>   can: peak_usb: upgrades the handling of bus state changes

I've added 3 to linux-can/testing and 1,2,4 to linux-can-next/testing.
Some changes suggested to 5, you only have to resend this one.

regards,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]