Re: mcf5441x: flexcan FIFO and mailbox mode (was: Re: [PATCH v5 5/5] can: flexcan: add mcf5441x support)

[Angelo Dureghello <angelo@kernel-space.org>]

Hi Marc,

On Wed, 19 Jan 2022, Marc Kleine-Budde wrote:

> On 19.01.2022 07:25:21, Angelo Dureghello wrote:
>>> we're currently discussing the option that the user of a flexcan can
>>> switch between RX-FIFO and RX via mailboxes.
>>>
>>> I noticed that the mcf5441x currently is configured for FIFO mode. Have
>>> you tested the driver in mailbox mode?
>>>
>>> The reason that some cores use the FIFO mode is, that they cannot
>>> receive RTR CAN frames in mailbox mode. According to the IP core
>>> overview table, the mcf5441x can receive RTR frames.
>>>
>>> If the IP core supports reception of RTR frames, mailbox mode should be
>>> used, as it makes use of more buffers (16-2) instead of 6 in FIFO mode.
>>> Should we activate mailbox mode for the mcf5441x?
>>
>> Ok, not sure why i selected FIFO mode initially, my application
>> actually is quite simple. Will try the switch to mailbox, sure,
>> looking into this.
>
> Thanks for coming back to me. The mailbox mode performs better under
> heavy load, as you have more buffers available.
>
> If you're using a recent kernel, the flexcan driver has been moved to:
> | drivers/net/can/flexcan/flexcan-core.c
>
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 0bff1884d5cc..aa0b7efb5ca6 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -296,7 +296,8 @@ static_assert(sizeof(struct flexcan_regs) ==  0x4 * 18 + 0xfb8);
> static const struct flexcan_devtype_data fsl_mcf5441x_devtype_data = {
>        .quirks = FLEXCAN_QUIRK_BROKEN_PERR_STATE |
>                FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_NR_MB_16 |
> -               FLEXCAN_QUIRK_SUPPPORT_RX_FIFO,
> +               FLEXCAN_QUIRK_SUPPPORT_RX_MAILBOX |
> +               FLEXCAN_QUIRK_SUPPPORT_RX_MAILBOX_RTR,
> };
>
> On older kernel with drivers/net/can/flexcan.c you need:
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 7734229aa078..538b26619460 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -382,7 +382,8 @@ struct flexcan_priv {
>
> static const struct flexcan_devtype_data fsl_mcf5441x_devtype_data = {
>        .quirks = FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> -               FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_NR_MB_16,
> +               FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_NR_MB_16 |
> +               FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
> };
>
> static const struct flexcan_devtype_data fsl_p1010_devtype_data = {
>
> Please apply appropriate change and check if the driver still works if
> you RX with full CAN bus load. Please also test if you can still receive
> RTR frames.
>

I tested this patch:

-------------------- drivers/net/can/flexcan/flexcan-core.c 
index 0bff1884d5cc..daeeb6250347 100644
@@ -296,7 +296,10 @@ static_assert(sizeof(struct flexcan_regs) ==  0x4 * 
18 + 0xfb8);
  static const struct flexcan_devtype_data fsl_mcf5441x_devtype_data = {
  	.quirks = FLEXCAN_QUIRK_BROKEN_PERR_STATE |
  		FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_NR_MB_16 |
-		FLEXCAN_QUIRK_SUPPPORT_RX_FIFO,
+		FLEXCAN_QUIRK_USE_RX_MAILBOX |
+		FLEXCAN_QUIRK_SUPPPORT_RX_FIFO |
+		FLEXCAN_QUIRK_SUPPPORT_RX_MAILBOX |
+		FLEXCAN_QUIRK_SUPPPORT_RX_MAILBOX_RTR,
  };

  static const struct flexcan_devtype_data fsl_p1010_devtype_data = {

Bus load PC -> mcf5441x tested by

ip link set can0 type can bitrate 1000000
ip link set can0 up
cangen can0 -g 0

On target (mcf54415)
candump can0

It works, even better then FIFO mode.

While unfortunately, RTR rx does not work. Tested it by putting
a trace in flexcan_mailbox_read()

 	if (reg_ctrl & FLEXCAN_MB_CNT_RTR) {
 		printk("%s() RX RTR frame\n", __func__);
 		cfd->can_id |= CAN_RTR_FLAG;
 	}

on host pc i used:

cangen can0 -R

No sign of RTR in rx. While in FIFO mode i can see it.


> regards,
> Marc
>

Regards,
angelo

> -- 
> 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 |
>