Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, kuba@kernel.org, linux-can@vger.kernel.org,
	kernel@pengutronix.de,
	Stephane Grosjean <s.grosjean@peak-system.com>,
	Marc Kleine-Budde <mkl@pengutronix.de>
Subject: [PATCH net-next 37/46] can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes
Date: Mon, 26 Jul 2021 16:11:35 +0200	[thread overview]
Message-ID: <20210726141144.862529-38-mkl@pengutronix.de> (raw)
In-Reply-To: <20210726141144.862529-1-mkl@pengutronix.de>

From: Stephane Grosjean <s.grosjean@peak-system.com>

This patch updates old code by using the can_change_state() function
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.

Link: https://lore.kernel.org/r/20210715142842.35793-1-s.grosjean@peak-system.com
Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c | 166 ++++++------------------
 1 file changed, 43 insertions(+), 123 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index fea44e33c3dd..e36e60c3703a 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -452,145 +452,65 @@ 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 */
-		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;
 
-	case CAN_STATE_ERROR_ACTIVE:
-		cf->can_id |= CAN_ERR_CRTL;
-		cf->data[1] = CAN_ERR_CRTL_ACTIVE;
-
-		/* sync local copies of rxerr/txerr counters */
-		mc->pdev->bec.txerr = 0;
-		mc->pdev->bec.rxerr = 0;
-		break;
-
-	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");
+	if (n & PCAN_USB_ERROR_RXQOVR) {
+		/* data overrun interrupt */
+		netdev_dbg(mc->netdev, "data overrun interrupt\n");
+		mc->netdev->stats.rx_over_errors++;
+		mc->netdev->stats.rx_errors++;
+		if (cf) {
 			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++;
 		}
+	}
 
-		cf->data[6] = mc->pdev->bec.txerr;
-		cf->data[7] = mc->pdev->bec.rxerr;
+	if (n & PCAN_USB_ERROR_TXQFULL)
+		netdev_dbg(mc->netdev, "device Tx queue full)\n");
 
-		new_state = mc->pdev->dev.can.state;
-		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;
 	}
 
-	mc->pdev->dev.can.state = new_state;
+	/* 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;
+
+		can_change_state(mc->netdev, cf, tx_state, rx_state);
+
+		if (new_state == CAN_STATE_BUS_OFF) {
+			can_bus_off(mc->netdev);
+		} else if (cf && (cf->can_id & CAN_ERR_CRTL)) {
+			/* Supply TX/RX error counters in case of
+			 * controller error.
+			 */
+			cf->data[6] = mc->pdev->bec.txerr;
+			cf->data[7] = mc->pdev->bec.rxerr;
+		}
+	}
+
+	if (!skb)
+		return -ENOMEM;
 
 	if (status_len & PCAN_USB_STATUSLEN_TIMESTAMP) {
 		struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
-- 
2.30.2



  parent reply	other threads:[~2021-07-26 14:15 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-26 14:10 pull-request: can-next 2021-07-25 Marc Kleine-Budde
2021-07-26 14:10 ` [PATCH net-next 01/46] can: j1939: j1939_sk_sock_destruct(): correct a grammatical error Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 02/46] can: j1939: fix checkpatch warnings Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 03/46] can: j1939: replace fall through comment by fallthrough pseudo-keyword Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 04/46] can: j1939: j1939_session_completed(): use consistent name se_skb for the session skb Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 05/46] can: j1939: j1939_session_tx_dat(): use consistent name se_skcb for session skb control buffer Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 06/46] can: j1939: j1939_xtp_rx_dat_one(): use separate pointer " Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 07/46] can: rx-offload: add skb queue for use during ISR Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 08/46] can: rx-offload: can_rx_offload_irq_finish(): directly call napi_schedule() Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 09/46] can: rx-offload: can_rx_offload_threaded_irq_finish(): add new function to be called from threaded interrupt Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 10/46] can: bittiming: fix documentation for struct can_tdc Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 11/46] can: netlink: clear data_bittiming if FD is turned off Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 12/46] can: netlink: remove redundant check in can_validate() Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 13/46] dt-bindings: net: can: Document transceiver implementation as phy Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 14/46] can: m_can: Add support for transceiver " Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 15/46] can: m_can: use devm_platform_ioremap_resource_byname Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 16/46] can: m_can: remove support for custom bit timing Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 17/46] can: mcp251xfd: mcp251xfd_probe(): try to get crystal clock rate from property Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 18/46] can: mcp251xfd: Fix header block to clarify independence from OF Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 19/46] can: mcp251xfd: mcp251xfd_open(): request IRQ as shared Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 20/46] can: esd_usb2: use DEVICE_ATTR_RO() helper macro Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 21/46] can: janz-ican3: use DEVICE_ATTR_RO/RW() " Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 22/46] can: at91_can: use DEVICE_ATTR_RW() " Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 23/46] net: at91_can: remove redundant blank lines Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 24/46] net: at91_can: add blank line after declarations Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 25/46] net: at91_can: fix the code style issue about macro Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 26/46] net: at91_can: use BIT macro Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 27/46] net: at91_can: fix the alignment issue Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 28/46] net: at91_can: add braces {} to all arms of the statement Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 29/46] net: at91_can: remove redundant space Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 30/46] net: at91_can: fix the comments style issue Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 31/46] can: peak_pci: convert comments to network style comments Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 32/46] can: peak_pci: fix checkpatch warnings Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 33/46] can: peak_pci: Add name and FW version of the card in kernel buffer Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 34/46] can: peak_usb: pcan_usb_get_device_id(): read value only in case of success Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 35/46] can: peak_usb: PCAN-USB: add support of loopback and one-shot mode Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 36/46] can: peak_usb: pcan_usb_encode_msg(): add information Marc Kleine-Budde
2021-07-26 14:11 ` Marc Kleine-Budde [this message]
2021-07-26 14:11 ` [PATCH net-next 38/46] can: etas_es58x: fix three typos in author name and documentation Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 39/46] can: etas_es58x: use error pointer during device probing Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 40/46] can: etas_es58x: use devm_kzalloc() to allocate device resources Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 41/46] can: etas_es58x: add es58x_free_netdevs() to factorize code Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 42/46] can: etas_es58x: use sizeof and sizeof_field macros instead of constant values Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 43/46] can: etas_es58x: rewrite the message cast in es58{1,_fd}_tx_can_msg to increase readability Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 44/46] can: flexcan: add platform data header Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 45/46] can: flexcan: add mcf5441x support Marc Kleine-Budde
2021-07-26 14:11 ` [PATCH net-next 46/46] can: flexcan: update Kconfig to enable coldfire Marc Kleine-Budde
2021-07-26 21:20 ` pull-request: can-next 2021-07-25 patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210726141144.862529-38-mkl@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=davem@davemloft.net \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=s.grosjean@peak-system.com \
    --subject='Re: [PATCH net-next 37/46] can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).