On 9/28/20 8:02 PM, Joakim Zhang wrote:
> One issue was reported at a baremetal environment, which is used for
> FPGA verification. "The first transfer will fail for extended ID
> format(for both 2.0B and FD format), following frames can be transmitted
> and received successfully for extended format, and standard format don't
> have this issue. This issue occurred randomly with high possiblity, when
> it occurs, the transmitter will detect a BIT1 error, the receiver a CRC
> error. According to the spec, a non-correctable error may cause this
> transfer failure."
> 
> With FLEXCAN_QUIRK_DISABLE_MECR quirk, it supports correctable errors,
> disable non-correctable errors interrupt and freeze mode. Platform has
> ECC hardware support, but select this quirk, this issue may not come to
> light. Initialize all FlexCAN memory before accessing them, at least it
> can avoid non-correctable errors detected due to memory uninitialized.
> The internal region can't be initialized when the hardware doesn't support
> ECC.
> 
> According to IMX8MPRM, Rev.C, 04/2020. There is a NOTE at the section
> 11.8.3.13 Detection and correction of memory errors:
> "All FlexCAN memory must be initialized before starting its operation in
> order to have the parity bits in memory properly updated. CTRL2[WRMFRZ]
> grants write access to all memory positions that require initialization,
> ranging from 0x080 to 0xADF and from 0xF28 to 0xFFF when the CAN FD feature
> is enabled. The RXMGMASK, RX14MASK, RX15MASK, and RXFGMASK registers need to
> be initialized as well. MCR[RFEN] must not be set during memory 
> initialization."
> 
> Memory range from 0x080 to 0xADF, there are reserved memory (unimplemented
> by hardware, e.g. only configure 64 MBs), these memory can be initialized or 
> not.
> In this patch, initialize all flexcan memory which includes reserved memory.
> 
> In this patch, create FLEXCAN_QUIRK_SUPPORT_ECC for platforms which has ECC
> feature. If you have a ECC platform in your hand, please select this
> qurik to initialize all flexcan memory firstly, then you can select
> FLEXCAN_QUIRK_DISABLE_MECR to only enable correctable errors.
> 
> Signed-off-by: Joakim Zhang <[email protected]>
> ---
> ChangeLogs:
> V1->V2:
>       * update commit messages, add a datasheet reference.
>       * initialize block memory instead of trivial memory.
>       * inilialize reserved memory.
> V2->V3:
>       * add FLEXCAN_QUIRK_SUPPORT_ECC quirk.
>       * remove init_ram struct.
> ---
>  drivers/net/can/flexcan.c | 50 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index e86925134009..0ae7436ee6ef 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -239,6 +239,8 @@
>  #define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)
>  /* Support CAN-FD mode */
>  #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
> +/* support memory detection and correction */
> +#define FLEXCAN_QUIRK_SUPPORT_ECC BIT(10)
>  
>  /* Structure of the message buffer */
>  struct flexcan_mb {
> @@ -1292,6 +1294,51 @@ static void flexcan_set_bittiming(struct net_device 
> *dev)
>               return flexcan_set_bittiming_ctrl(dev);
>  }
>  
> +static void flexcan_init_ram(struct net_device *dev)
> +{
> +     struct flexcan_priv *priv = netdev_priv(dev);
> +     struct flexcan_regs __iomem *regs = priv->regs;
> +     u32 reg_ctrl2;
> +
> +     /* 11.8.3.13 Detection and correction of memory errors:
> +      * CTRL2[WRMFRZ] grants write access to all memory positions that
> +      * require initialization, ranging from 0x080 to 0xADF and
> +      * from 0xF28 to 0xFFF when the CAN FD feature is enabled.
> +      * The RXMGMASK, RX14MASK, RX15MASK, and RXFGMASK registers need to
> +      * be initialized as well. MCR[RFEN] must not be set during memory
> +      * initialization.
> +      */
> +     reg_ctrl2 = priv->read(&regs->ctrl2);
> +     reg_ctrl2 |= FLEXCAN_CTRL2_WRMFRZ;
> +     priv->write(reg_ctrl2, &regs->ctrl2);
> +
> +     /* ranging from 0x0080 to 0x0ADF, ram details as below list:
> +      * 0x0080--0x087F:      128 MBs
> +      * 0x0880--0x0A7F:      128 RXIMRs
> +      * 0x0A80--0x0A97:      6 RXFIRs
> +      * 0x0A98--0x0A9F:      Reserved
> +      * 0x0AA0--0x0AA3:      RXMGMASK
> +      * 0x0AA4--0x0AA7:      RXFGMASK
> +      * 0x0AA8--0x0AAB:      RX14MASK
> +      * 0x0AAC--0x0AAF:      RX15MASK
> +      * 0x0AB0--0x0ABF:      TX_SMB
> +      * 0x0AC0--0x0ACF:      RX_SMB0
> +      * 0x0AD0--0x0ADF:      RX_SMB1

I don't like to have the register definition here *again), we have struct
flexcan_regs for this.

> +      */
> +     memset_io((void __iomem *)regs + 0x80, 0, 0xadf - 0x80 + 1);

why the cast?

Can you use the "&regs->foo - &regs->bar + x" to get the length for the memset?

> +
> +     /* ranging from 0x0F28 to 0x0FFF when CAN FD feature is enabled,
> +      * ram details as below list:
> +      * 0x0F28--0x0F6F:      TX_SMB_FD
> +      * 0x0F70--0x0FB7:      RX_SMB0_FD
> +      * 0x0FB8--0x0FFF:      RX_SMB0_FD
> +      */
> +     memset_io((void __iomem *)regs + 0xf28, 0, 0xfff - 0xf28 + 1);

same here

> +
> +     reg_ctrl2 &= ~FLEXCAN_CTRL2_WRMFRZ;
> +     priv->write(reg_ctrl2, &regs->ctrl2);
> +}
> +
>  /* flexcan_chip_start
>   *
>   * this functions is entered with clocks enabled
> @@ -1316,6 +1363,9 @@ static int flexcan_chip_start(struct net_device *dev)
>       if (err)
>               goto out_chip_disable;
>  
> +     if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_ECC)
> +             flexcan_init_ram(dev);
> +
>       flexcan_set_bittiming(dev);
>  
>       /* MCR
> 

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: signature.asc
Description: OpenPGP digital signature

Reply via email to