summaryrefslogtreecommitdiffstats
path: root/drivers/net/can/spi/mcp251x.c
diff options
context:
space:
mode:
authorVincent Mailhol <mailhol.vincent@wanadoo.fr>2021-12-07 21:15:31 +0900
committerMarc Kleine-Budde <mkl@pengutronix.de>2022-01-05 12:09:05 +0100
commitcc4b08c31b5c51352f258032cc65e884b3e61e6a (patch)
tree1ff8f441190194792f5da507b6952d1ced79ae36 /drivers/net/can/spi/mcp251x.c
parent8e674ca74244eac1cd85c6e9a89b588a03c55ff7 (diff)
downloadlinux-cc4b08c31b5c51352f258032cc65e884b3e61e6a.tar.gz
linux-cc4b08c31b5c51352f258032cc65e884b3e61e6a.tar.bz2
linux-cc4b08c31b5c51352f258032cc65e884b3e61e6a.zip
can: do not increase tx_bytes statistics for RTR frames
The actual payload length of the CAN Remote Transmission Request (RTR) frames is always 0, i.e. no payload is transmitted on the wire. However, those RTR frames still use the DLC to indicate the length of the requested frame. As such, net_device_stats::tx_bytes should not be increased when sending RTR frames. The function can_get_echo_skb() already returns the correct length, even for RTR frames (c.f. [1]). However, for historical reasons, the drivers do not use can_get_echo_skb()'s return value and instead, most of them store a temporary length (or dlc) in some local structure or array. Using the return value of can_get_echo_skb() solves the issue. After doing this, such length/dlc fields become unused and so this patch does the adequate cleaning when needed. This patch fixes all the CAN drivers. Finally, can_get_echo_skb() is decorated with the __must_check attribute in order to force future drivers to correctly use its return value (else the compiler would emit a warning). [1] commit ed3320cec279 ("can: dev: __can_get_echo_skb(): fix real payload length return value for RTR frames") Link: https://lore.kernel.org/all/20211207121531.42941-6-mailhol.vincent@wanadoo.fr Cc: Nicolas Ferre <nicolas.ferre@microchip.com> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Ludovic Desroches <ludovic.desroches@microchip.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Chen-Yu Tsai <wens@csie.org> Cc: Jernej Skrabec <jernej.skrabec@gmail.com> Cc: Yasushi SHOJI <yashi@spacecubics.com> Cc: Oliver Hartkopp <socketcan@hartkopp.net> Cc: Stephane Grosjean <s.grosjean@peak-system.com> Cc: Andreas Larsson <andreas@gaisler.com> Tested-by: Jimmy Assarsson <extja@kvaser.com> # kvaser Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> Acked-by: Stefan Mätje <stefan.maetje@esd.eu> # esd_usb2 Tested-by: Stefan Mätje <stefan.maetje@esd.eu> # esd_usb2 [mkl: add conversion for grcan] Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Diffstat (limited to 'drivers/net/can/spi/mcp251x.c')
-rw-r--r--drivers/net/can/spi/mcp251x.c25
1 files changed, 13 insertions, 12 deletions
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index 0da965da32cb..025e07cb7439 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -237,7 +237,6 @@ struct mcp251x_priv {
u8 *spi_rx_buf;
struct sk_buff *tx_skb;
- int tx_len;
struct workqueue_struct *wq;
struct work_struct tx_work;
@@ -250,6 +249,8 @@ struct mcp251x_priv {
#define AFTER_SUSPEND_POWER 4
#define AFTER_SUSPEND_RESTART 8
int restart_tx;
+ bool tx_busy;
+
struct regulator *power;
struct regulator *transceiver;
struct clk *clk;
@@ -272,13 +273,13 @@ static void mcp251x_clean(struct net_device *net)
{
struct mcp251x_priv *priv = netdev_priv(net);
- if (priv->tx_skb || priv->tx_len)
+ if (priv->tx_skb || priv->tx_busy)
net->stats.tx_errors++;
dev_kfree_skb(priv->tx_skb);
- if (priv->tx_len)
+ if (priv->tx_busy)
can_free_echo_skb(priv->net, 0, NULL);
priv->tx_skb = NULL;
- priv->tx_len = 0;
+ priv->tx_busy = false;
}
/* Note about handling of error return of mcp251x_spi_trans: accessing
@@ -785,7 +786,7 @@ static netdev_tx_t mcp251x_hard_start_xmit(struct sk_buff *skb,
struct mcp251x_priv *priv = netdev_priv(net);
struct spi_device *spi = priv->spi;
- if (priv->tx_skb || priv->tx_len) {
+ if (priv->tx_skb || priv->tx_busy) {
dev_warn(&spi->dev, "hard_xmit called while tx busy\n");
return NETDEV_TX_BUSY;
}
@@ -1010,7 +1011,7 @@ static void mcp251x_tx_work_handler(struct work_struct *ws)
if (frame->len > CAN_FRAME_MAX_DATA_LEN)
frame->len = CAN_FRAME_MAX_DATA_LEN;
mcp251x_hw_tx(spi, frame, 0);
- priv->tx_len = 1 + frame->len;
+ priv->tx_busy = true;
can_put_echo_skb(priv->tx_skb, net, 0, 0);
priv->tx_skb = NULL;
}
@@ -1176,12 +1177,12 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
break;
if (intf & CANINTF_TX) {
- net->stats.tx_packets++;
- net->stats.tx_bytes += priv->tx_len - 1;
can_led_event(net, CAN_LED_EVENT_TX);
- if (priv->tx_len) {
- can_get_echo_skb(net, 0, NULL);
- priv->tx_len = 0;
+ if (priv->tx_busy) {
+ net->stats.tx_packets++;
+ net->stats.tx_bytes += can_get_echo_skb(net, 0,
+ NULL);
+ priv->tx_busy = false;
}
netif_wake_queue(net);
}
@@ -1208,7 +1209,7 @@ static int mcp251x_open(struct net_device *net)
priv->force_quit = 0;
priv->tx_skb = NULL;
- priv->tx_len = 0;
+ priv->tx_busy = false;
if (!dev_fwnode(&spi->dev))
flags = IRQF_TRIGGER_FALLING;