On 10/11/18 5:01 PM, Alexander Stein wrote:
> Essentially this patch moves the Tx mailbox to position 63, regardless of
> timestamp based offloading or Rx FIFO.
> So mainly the iflag register usage regarding Tx has changed. The rest is
> consolidating Rx FIFO and timestamp offloading as they now use both the
> same Tx mailbox.
> The reason is a very annoying behavior regarding sending RTR frames when
> _not_ using Rx FIFO:
> If a Tx mailbox sent an RTR frame it becomes an Rx mailbox. For that
> reason flexcan_irq disables the Tx mailbox again. But if during the time
> the RTR was sent and the Tx mailbox is disabled a new CAN frames is
> received, it is lost without notice. The reason is that so-called "Move-in"
> process starts from the lowest mailbox which happen to be a Tx mailbox set
> to EMPTY.
>
> Steps to reproduce (I used an imx7d):
> 1. generate regular bursts of messages
> 2. send an RTR from flexcan with higher priority than burst messages every
> 1ms, e.g. cangen -I 0x100 -L 0 -g 1 -R can0
> 3. notice a lost message without notification after some seconds
>
> When running an iperf in parallel this problem is occurring even more
> frequently.
> Using filters is not possible as at least one single CAN-ID is allowed.
> Handling the Tx MB during Rx is also not possible as there is no race-free
> disable of Rx MB.
>
> There is still a slight window when the described problem can occur. But
> for that all Rx MB must be in use which is essentially next to an overrun.
> Still there will be no indication if it ever occurs.
>
> Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/net/can/flexcan.c | 52 ++++++++++++++++++++-------------------
> 1 file changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index c9d30480aae9..83cc9e6d8ba1 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -983,16 +985,17 @@ static int flexcan_chip_start(struct net_device *dev)
> priv->write(reg_ctrl2, ®s->ctrl2);
> }
>
> - /* clear and invalidate all mailboxes first */
> - for (i = priv->tx_mb_idx; i < ARRAY_SIZE(regs->mb); i++) {
> - priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
> - ®s->mb[i].can_ctrl);
> - }
> -
> if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
> - for (i = priv->offload.mb_first; i <= priv->offload.mb_last; i++)
> + for (i = priv->offload.mb_first; i <= priv->offload.mb_last; i++) {
> priv->write(FLEXCAN_MB_CODE_RX_EMPTY,
> ®s->mb[i].can_ctrl);
> + }
> + } else {
> + /* clear and invalidate unused mailboxes first */
> + for (i = FLEXCAN_TX_MB_RESERVED_OFF_FIFO; i <= ARRAY_SIZE(regs->mb); i++) {
> + priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
> + ®s->mb[i].can_ctrl);
> + }
> }
Any particular reason, why you've moved the loop into the else branch?
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 |
Attachment:
signature.asc
Description: OpenPGP digital signature
