Skip to content

Commit

Permalink
Merge patch series "can: mcp251xfd: workaround for erratum DS80000789…
Browse files Browse the repository at this point in the history 
…E 6 of mcp2518fd"

Marc Kleine-Budde <mkl@pengutronix.de> says:

This patch series tries to work around erratum DS80000789E 6 of the
mcp2518fd, found by Stefan Althöfer, the other variants of the chip
family (mcp2517fd and mcp251863) are probably also affected.

Erratum DS80000789E 6 says "reading of the FIFOCI bits in the FIFOSTA
register for an RX FIFO may be corrupted". However observation shows
that this problem is not limited to RX FIFOs but also effects the TEF
FIFO.

In the bad case, the driver reads a too large head index. In the
original code, the driver always trusted the read value.

For the RX FIDO this caused old, already processed CAN frames or new,
incompletely written CAN frames to be (re-)processed.

To work around this issue, keep a per FIFO timestamp of the last valid
received CAN frame and compare against the timestamp of every received
CAN frame.

Further tests showed that this workaround can recognize old CAN
frames, but a small time window remains in which partially written CAN
frames are not recognized but then processed. These CAN frames have
the correct data and time stamps, but the DLC has not yet been
updated.

For the TEF FIFO the original driver already detects the error, update
the error handling with the knowledge that it is causes by this erratum.

Link: https://lore.kernel.org/all/20240628-mcp251xfd-workaround-erratum-6-v4-0-53586f168524@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
  • Loading branch information
Marc Kleine-Budde committed Jun 28, 2024
2 parents 69e2326 + 3a0a88f commit ae44fa9
Show file tree
Hide file tree
Showing 8 changed files with 269 additions and 201 deletions.
82 changes: 43 additions & 39 deletions drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// mcp251xfd - Microchip MCP251xFD Family CAN controller driver
//
// Copyright (c) 2019, 2020, 2021 Pengutronix,
// Copyright (c) 2019, 2020, 2021, 2023 Pengutronix,
// Marc Kleine-Budde <kernel@pengutronix.de>
//
// Based on:
Expand Down Expand Up @@ -744,6 +744,7 @@ static void mcp251xfd_chip_stop(struct mcp251xfd_priv *priv,

mcp251xfd_chip_interrupts_disable(priv);
mcp251xfd_chip_rx_int_disable(priv);
mcp251xfd_timestamp_stop(priv);
mcp251xfd_chip_sleep(priv);
}

Expand All @@ -763,6 +764,8 @@ static int mcp251xfd_chip_start(struct mcp251xfd_priv *priv)
if (err)
goto out_chip_stop;

mcp251xfd_timestamp_start(priv);

err = mcp251xfd_set_bittiming(priv);
if (err)
goto out_chip_stop;
Expand Down Expand Up @@ -791,7 +794,7 @@ static int mcp251xfd_chip_start(struct mcp251xfd_priv *priv)

return 0;

out_chip_stop:
out_chip_stop:
mcp251xfd_dump(priv);
mcp251xfd_chip_stop(priv, CAN_STATE_STOPPED);

Expand Down Expand Up @@ -867,18 +870,18 @@ static int mcp251xfd_get_berr_counter(const struct net_device *ndev,

static struct sk_buff *
mcp251xfd_alloc_can_err_skb(struct mcp251xfd_priv *priv,
struct can_frame **cf, u32 *timestamp)
struct can_frame **cf, u32 *ts_raw)
{
struct sk_buff *skb;
int err;

err = mcp251xfd_get_timestamp(priv, timestamp);
err = mcp251xfd_get_timestamp_raw(priv, ts_raw);
if (err)
return NULL;

skb = alloc_can_err_skb(priv->ndev, cf);
if (skb)
mcp251xfd_skb_set_timestamp(priv, skb, *timestamp);
mcp251xfd_skb_set_timestamp_raw(priv, skb, *ts_raw);

return skb;
}
Expand All @@ -889,7 +892,7 @@ static int mcp251xfd_handle_rxovif(struct mcp251xfd_priv *priv)
struct mcp251xfd_rx_ring *ring;
struct sk_buff *skb;
struct can_frame *cf;
u32 timestamp, rxovif;
u32 ts_raw, rxovif;
int err, i;

stats->rx_over_errors++;
Expand Down Expand Up @@ -924,14 +927,14 @@ static int mcp251xfd_handle_rxovif(struct mcp251xfd_priv *priv)
return err;
}

skb = mcp251xfd_alloc_can_err_skb(priv, &cf, &timestamp);
skb = mcp251xfd_alloc_can_err_skb(priv, &cf, &ts_raw);
if (!skb)
return 0;

cf->can_id |= CAN_ERR_CRTL;
cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;

err = can_rx_offload_queue_timestamp(&priv->offload, skb, timestamp);
err = can_rx_offload_queue_timestamp(&priv->offload, skb, ts_raw);
if (err)
stats->rx_fifo_errors++;

Expand All @@ -948,12 +951,12 @@ static int mcp251xfd_handle_txatif(struct mcp251xfd_priv *priv)
static int mcp251xfd_handle_ivmif(struct mcp251xfd_priv *priv)
{
struct net_device_stats *stats = &priv->ndev->stats;
u32 bdiag1, timestamp;
u32 bdiag1, ts_raw;
struct sk_buff *skb;
struct can_frame *cf = NULL;
int err;

err = mcp251xfd_get_timestamp(priv, &timestamp);
err = mcp251xfd_get_timestamp_raw(priv, &ts_raw);
if (err)
return err;

Expand Down Expand Up @@ -1035,8 +1038,8 @@ static int mcp251xfd_handle_ivmif(struct mcp251xfd_priv *priv)
if (!cf)
return 0;

mcp251xfd_skb_set_timestamp(priv, skb, timestamp);
err = can_rx_offload_queue_timestamp(&priv->offload, skb, timestamp);
mcp251xfd_skb_set_timestamp_raw(priv, skb, ts_raw);
err = can_rx_offload_queue_timestamp(&priv->offload, skb, ts_raw);
if (err)
stats->rx_fifo_errors++;

Expand All @@ -1049,7 +1052,7 @@ static int mcp251xfd_handle_cerrif(struct mcp251xfd_priv *priv)
struct sk_buff *skb;
struct can_frame *cf = NULL;
enum can_state new_state, rx_state, tx_state;
u32 trec, timestamp;
u32 trec, ts_raw;
int err;

err = regmap_read(priv->map_reg, MCP251XFD_REG_TREC, &trec);
Expand Down Expand Up @@ -1079,7 +1082,7 @@ static int mcp251xfd_handle_cerrif(struct mcp251xfd_priv *priv)
/* The skb allocation might fail, but can_change_state()
* handles cf == NULL.
*/
skb = mcp251xfd_alloc_can_err_skb(priv, &cf, &timestamp);
skb = mcp251xfd_alloc_can_err_skb(priv, &cf, &ts_raw);
can_change_state(priv->ndev, cf, tx_state, rx_state);

if (new_state == CAN_STATE_BUS_OFF) {
Expand Down Expand Up @@ -1110,7 +1113,7 @@ static int mcp251xfd_handle_cerrif(struct mcp251xfd_priv *priv)
cf->data[7] = bec.rxerr;
}

err = can_rx_offload_queue_timestamp(&priv->offload, skb, timestamp);
err = can_rx_offload_queue_timestamp(&priv->offload, skb, ts_raw);
if (err)
stats->rx_fifo_errors++;

Expand All @@ -1135,7 +1138,7 @@ mcp251xfd_handle_modif(const struct mcp251xfd_priv *priv, bool *set_normal_mode)
return 0;
}

/* According to MCP2517FD errata DS80000792B 1., during a TX
/* According to MCP2517FD errata DS80000792C 1., during a TX
* MAB underflow, the controller will transition to Restricted
* Operation Mode or Listen Only Mode (depending on SERR2LOM).
*
Expand Down Expand Up @@ -1180,7 +1183,7 @@ static int mcp251xfd_handle_serrif(struct mcp251xfd_priv *priv)

/* TX MAB underflow
*
* According to MCP2517FD Errata DS80000792B 1. a TX MAB
* According to MCP2517FD Errata DS80000792C 1. a TX MAB
* underflow is indicated by SERRIF and MODIF.
*
* In addition to the effects mentioned in the Errata, there
Expand Down Expand Up @@ -1224,7 +1227,7 @@ static int mcp251xfd_handle_serrif(struct mcp251xfd_priv *priv)

/* RX MAB overflow
*
* According to MCP2517FD Errata DS80000792B 1. a RX MAB
* According to MCP2517FD Errata DS80000792C 1. a RX MAB
* overflow is indicated by SERRIF.
*
* In addition to the effects mentioned in the Errata, (most
Expand Down Expand Up @@ -1331,7 +1334,8 @@ mcp251xfd_handle_eccif(struct mcp251xfd_priv *priv, bool set_normal_mode)
return err;

/* Errata Reference:
* mcp2517fd: DS80000789B, mcp2518fd: DS80000792C 2.
* mcp2517fd: DS80000789C 3., mcp2518fd: DS80000792E 2.,
* mcp251863: DS80000984A 2.
*
* ECC single error correction does not work in all cases:
*
Expand Down Expand Up @@ -1576,7 +1580,7 @@ static irqreturn_t mcp251xfd_irq(int irq, void *dev_id)
handled = IRQ_HANDLED;
} while (1);

out_fail:
out_fail:
can_rx_offload_threaded_irq_finish(&priv->offload);

netdev_err(priv->ndev, "IRQ handler returned %d (intf=0x%08x).\n",
Expand Down Expand Up @@ -1610,11 +1614,12 @@ static int mcp251xfd_open(struct net_device *ndev)
if (err)
goto out_mcp251xfd_ring_free;

mcp251xfd_timestamp_init(priv);

err = mcp251xfd_chip_start(priv);
if (err)
goto out_transceiver_disable;

mcp251xfd_timestamp_init(priv);
clear_bit(MCP251XFD_FLAGS_DOWN, priv->flags);
can_rx_offload_enable(&priv->offload);

Expand All @@ -1641,22 +1646,21 @@ static int mcp251xfd_open(struct net_device *ndev)

return 0;

out_free_irq:
out_free_irq:
free_irq(spi->irq, priv);
out_destroy_workqueue:
out_destroy_workqueue:
destroy_workqueue(priv->wq);
out_can_rx_offload_disable:
out_can_rx_offload_disable:
can_rx_offload_disable(&priv->offload);
set_bit(MCP251XFD_FLAGS_DOWN, priv->flags);
mcp251xfd_timestamp_stop(priv);
out_transceiver_disable:
out_transceiver_disable:
mcp251xfd_transceiver_disable(priv);
out_mcp251xfd_ring_free:
out_mcp251xfd_ring_free:
mcp251xfd_ring_free(priv);
out_pm_runtime_put:
out_pm_runtime_put:
mcp251xfd_chip_stop(priv, CAN_STATE_STOPPED);
pm_runtime_put(ndev->dev.parent);
out_close_candev:
out_close_candev:
close_candev(ndev);

return err;
Expand All @@ -1674,7 +1678,6 @@ static int mcp251xfd_stop(struct net_device *ndev)
free_irq(ndev->irq, priv);
destroy_workqueue(priv->wq);
can_rx_offload_disable(&priv->offload);
mcp251xfd_timestamp_stop(priv);
mcp251xfd_chip_stop(priv, CAN_STATE_STOPPED);
mcp251xfd_transceiver_disable(priv);
mcp251xfd_ring_free(priv);
Expand Down Expand Up @@ -1820,9 +1823,9 @@ mcp251xfd_register_get_dev_id(const struct mcp251xfd_priv *priv, u32 *dev_id,
*effective_speed_hz_slow = xfer[0].effective_speed_hz;
*effective_speed_hz_fast = xfer[1].effective_speed_hz;

out_kfree_buf_tx:
out_kfree_buf_tx:
kfree(buf_tx);
out_kfree_buf_rx:
out_kfree_buf_rx:
kfree(buf_rx);

return err;
Expand Down Expand Up @@ -1936,13 +1939,13 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv)

return 0;

out_unregister_candev:
out_unregister_candev:
unregister_candev(ndev);
out_chip_sleep:
out_chip_sleep:
mcp251xfd_chip_sleep(priv);
out_runtime_disable:
out_runtime_disable:
pm_runtime_disable(ndev->dev.parent);
out_runtime_put_noidle:
out_runtime_put_noidle:
pm_runtime_put_noidle(ndev->dev.parent);
mcp251xfd_clks_and_vdd_disable(priv);

Expand Down Expand Up @@ -2095,7 +2098,8 @@ static int mcp251xfd_probe(struct spi_device *spi)
priv->devtype_data = *(struct mcp251xfd_devtype_data *)spi_get_device_match_data(spi);

/* Errata Reference:
* mcp2517fd: DS80000792C 5., mcp2518fd: DS80000789C 4.
* mcp2517fd: DS80000792C 5., mcp2518fd: DS80000789E 4.,
* mcp251863: DS80000984A 4.
*
* The SPI can write corrupted data to the RAM at fast SPI
* speeds:
Expand Down Expand Up @@ -2155,9 +2159,9 @@ static int mcp251xfd_probe(struct spi_device *spi)

return 0;

out_can_rx_offload_del:
out_can_rx_offload_del:
can_rx_offload_del(&priv->offload);
out_free_candev:
out_free_candev:
spi->max_speed_hz = priv->spi_max_speed_hz_orig;

free_candev(ndev);
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/can/spi/mcp251xfd/mcp251xfd-dump.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ static void mcp251xfd_dump_registers(const struct mcp251xfd_priv *priv,
kfree(buf);
}

out:
out:
mcp251xfd_dump_header(iter, MCP251XFD_DUMP_OBJECT_TYPE_REG, reg);
}

Expand Down
2 changes: 1 addition & 1 deletion drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ mcp251xfd_regmap_crc_read(void *context,

return err;
}
out:
out:
memcpy(val_buf, buf_rx->data, val_len);

return 0;
Expand Down
5 changes: 5 additions & 0 deletions drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ mcp251xfd_ring_init_rx(struct mcp251xfd_priv *priv, u16 *base, u8 *fifo_nr)
int i, j;

mcp251xfd_for_each_rx_ring(priv, rx_ring, i) {
rx_ring->last_valid = timecounter_read(&priv->tc);
rx_ring->head = 0;
rx_ring->tail = 0;
rx_ring->base = *base;
Expand Down Expand Up @@ -485,6 +486,8 @@ int mcp251xfd_ring_alloc(struct mcp251xfd_priv *priv)
clear_bit(MCP251XFD_FLAGS_FD_MODE, priv->flags);
}

tx_ring->obj_num_shift_to_u8 = BITS_PER_TYPE(tx_ring->obj_num) -
ilog2(tx_ring->obj_num);
tx_ring->obj_size = tx_obj_size;

rem = priv->rx_obj_num;
Expand All @@ -507,6 +510,8 @@ int mcp251xfd_ring_alloc(struct mcp251xfd_priv *priv)
}

rx_ring->obj_num = rx_obj_num;
rx_ring->obj_num_shift_to_u8 = BITS_PER_TYPE(rx_ring->obj_num_shift_to_u8) -
ilog2(rx_obj_num);
rx_ring->obj_size = rx_obj_size;
priv->rx[i] = rx_ring;
}
Expand Down
Loading

0 comments on commit ae44fa9

Please sign in to comment.