All of lore.kernel.org
 help / color / mirror / Atom feed
* mcf5441x: flexcan FIFO and mailbox mode (was: Re: [PATCH v5 5/5] can: flexcan: add mcf5441x support)
@ 2022-01-06 11:18 Marc Kleine-Budde
  2022-01-19  6:25 ` Angelo Dureghello
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-01-06 11:18 UTC (permalink / raw)
  To: Angelo Dureghello
  Cc: gerg, geert, linux-m68k, linux-can, qiangqing.zhang, Dario Binacchi

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]

Hello Angelo,

On 02.07.2021 11:48:41, Angelo Dureghello wrote:
> Add flexcan support for NXP ColdFire mcf5441x family.
> 
> This flexcan module is quite similar to imx6 flexcan module, but
> with some exceptions:
> 
> - 3 separate interrupt sources, MB, BOFF and ERR,
> - implements 16 mb only,
> - m68k architecture is not supporting devicetrees, so a
>   platform data check/case has been added,
> - ColdFire is m68k, so big-endian cpu, with a little-endian flexcan
>   module.

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?

regards,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: mcf5441x: flexcan FIFO and mailbox mode (was: Re: [PATCH v5 5/5] can: flexcan: add mcf5441x support)
  2022-01-06 11:18 mcf5441x: flexcan FIFO and mailbox mode (was: Re: [PATCH v5 5/5] can: flexcan: add mcf5441x support) Marc Kleine-Budde
@ 2022-01-19  6:25 ` Angelo Dureghello
  2022-01-19  6:38   ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Angelo Dureghello @ 2022-01-19  6:25 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: gerg, geert, linux-m68k, linux-can, qiangqing.zhang, Dario Binacchi

Hi Marc,

On Thu, 6 Jan 2022, Marc Kleine-Budde wrote:

> Hello Angelo,
>
> On 02.07.2021 11:48:41, Angelo Dureghello wrote:
>> Add flexcan support for NXP ColdFire mcf5441x family.
>>
>> This flexcan module is quite similar to imx6 flexcan module, but
>> with some exceptions:
>>
>> - 3 separate interrupt sources, MB, BOFF and ERR,
>> - implements 16 mb only,
>> - m68k architecture is not supporting devicetrees, so a
>>   platform data check/case has been added,
>> - ColdFire is m68k, so big-endian cpu, with a little-endian flexcan
>>   module.
>
> 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.


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: mcf5441x: flexcan FIFO and mailbox mode (was: Re: [PATCH v5 5/5] can: flexcan: add mcf5441x support)
  2022-01-19  6:25 ` Angelo Dureghello
@ 2022-01-19  6:38   ` Marc Kleine-Budde
  2022-01-20 23:35     ` Angelo Dureghello
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-01-19  6:38 UTC (permalink / raw)
  To: Angelo Dureghello
  Cc: gerg, geert, linux-m68k, linux-can, qiangqing.zhang, Dario Binacchi

[-- Attachment #1: Type: text/plain, Size: 2910 bytes --]

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.

regards,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: mcf5441x: flexcan FIFO and mailbox mode (was: Re: [PATCH v5 5/5] can: flexcan: add mcf5441x support)
  2022-01-19  6:38   ` Marc Kleine-Budde
@ 2022-01-20 23:35     ` Angelo Dureghello
  2022-01-21  8:49       ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Angelo Dureghello @ 2022-01-20 23:35 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: gerg, geert, linux-m68k, linux-can, qiangqing.zhang, Dario Binacchi

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: mcf5441x: flexcan FIFO and mailbox mode (was: Re: [PATCH v5 5/5] can: flexcan: add mcf5441x support)
  2022-01-20 23:35     ` Angelo Dureghello
@ 2022-01-21  8:49       ` Marc Kleine-Budde
  2022-01-21 10:16         ` Angelo Dureghello
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-01-21  8:49 UTC (permalink / raw)
  To: Angelo Dureghello
  Cc: gerg, geert, linux-m68k, linux-can, qiangqing.zhang, Dario Binacchi

[-- Attachment #1: Type: text/plain, Size: 2432 bytes --]

On 21.01.2022 00:35:43, Angelo Dureghello wrote:
> > 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.

What's the difference, does it produce less overruns?

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

Ok, then I enable the mailbox support for the mcf5441x and fix the note
that the IP core doesn't support RTR reception via mailboxes. See the
following patch:

| https://lore.kernel.org/all/20220121084425.3141218-1-mkl@pengutronix.de

If you have time, please test and add your Tested-by.

You can switch to mailbox mode during runtime, if the interface is down:

| ethtool --set-priv-flags can0 rx-rtr off

With ethtool you can query the number of used RX and TX buffers:

| ethtool --show-ring can0

The FIFO mode uses 6 RX buffers, while mailbox mode will probably use
14.

regards,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: mcf5441x: flexcan FIFO and mailbox mode (was: Re: [PATCH v5 5/5] can: flexcan: add mcf5441x support)
  2022-01-21  8:49       ` Marc Kleine-Budde
@ 2022-01-21 10:16         ` Angelo Dureghello
  0 siblings, 0 replies; 6+ messages in thread
From: Angelo Dureghello @ 2022-01-21 10:16 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: gerg, geert, linux-m68k, linux-can, qiangqing.zhang, Dario Binacchi

Hi Marc,

On Fri, 21 Jan 2022, Marc Kleine-Budde wrote:

> On 21.01.2022 00:35:43, Angelo Dureghello wrote:
>>> 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.
>
> What's the difference, does it produce less overruns?
>

In FIFO mode, i had a conditions where the mcf system
was stuck. Had to reboot. Not clear what's happen btw, should try to 
reproduce this.

>> 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.
>
> Ok, then I enable the mailbox support for the mcf5441x and fix the note
> that the IP core doesn't support RTR reception via mailboxes. See the
> following patch:
>
> | https://lore.kernel.org/all/20220121084425.3141218-1-mkl@pengutronix.de
>
> If you have time, please test and add your Tested-by.
>
> You can switch to mailbox mode during runtime, if the interface is down:
>
> | ethtool --set-priv-flags can0 rx-rtr off
>
> With ethtool you can query the number of used RX and TX buffers:
>
> | ethtool --show-ring can0
>
> The FIFO mode uses 6 RX buffers, while mailbox mode will probably use
> 14.
>

My mcf5441x small system does not have ethtool available
right now. Will test this as soon as i can.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-01-21 10:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-06 11:18 mcf5441x: flexcan FIFO and mailbox mode (was: Re: [PATCH v5 5/5] can: flexcan: add mcf5441x support) Marc Kleine-Budde
2022-01-19  6:25 ` Angelo Dureghello
2022-01-19  6:38   ` Marc Kleine-Budde
2022-01-20 23:35     ` Angelo Dureghello
2022-01-21  8:49       ` Marc Kleine-Budde
2022-01-21 10:16         ` Angelo Dureghello

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.