| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <DB8PR04MB67951C7B7C74E79FE61D06A5E6350@DB8PR04MB6795.eurprd04.prod.outlook.com>
Date: Mon, 28 Sep 2020 02:00:15 +0000
From: Joakim Zhang <qiangqing.zhang@....com>
To: Marc Kleine-Budde <mkl@...gutronix.de>,
"linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
Pankaj Bansal <pankaj.bansal@....com>
CC: dl-linux-imx <linux-imx@....com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for
ECC function
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@...gutronix.de>
> Sent: 2020年9月28日 3:52
> To: Joakim Zhang <qiangqing.zhang@....com>; linux-can@...r.kernel.org
> Cc: dl-linux-imx <linux-imx@....com>; netdev@...r.kernel.org
> Subject: Re: [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC
> function
>
> On 9/27/20 6:07 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. 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.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@....com>
> > ---
> > ChangeLogs:
> > V1->V2:
> > * update commit messages, add a datasheet reference.
> > * initialize block memory instead of trivial memory.
> > * inilialize reserved memory.
> > ---
> > drivers/net/can/flexcan.c | 67
> > +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 67 insertions(+)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index e86925134009..aca0fc40ae9b 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -309,6 +309,40 @@ struct flexcan_regs {
> >
> > static_assert(sizeof(struct flexcan_regs) == 0x4 + 0xc08);
> >
> > +/* Structure of memory need be initialized for ECC feature */ static
> > +const struct flexcan_ram_int {
> > + u32 offset;
> > + u16 len;
> > +} ram_init[] = {
> > + /* 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
> > + */
> > + {
> > + .offset = 0x80,
> > + .len = (0xadf - 0x80) / sizeof(u32) + 1,
> > + },
> > + /* 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
> > + */
> > + {
> > + .offset = 0xf28,
> > + .len = (0xfff - 0xf28) / sizeof(u32) + 1,
> > + },
> > +};
>
> As it's only two ranges, I think there's no need for this struct. Directly move
> code that into the for loops.
OK.
> > +
> > struct flexcan_devtype_data {
> > u32 quirks; /* quirks needed for different IP cores */
> > };
> > @@ -1292,6 +1326,36 @@ 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;
> > + int i;
> > +
> > + /* 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(®s->ctrl2);
> > + reg_ctrl2 |= FLEXCAN_CTRL2_WRMFRZ;
> > + priv->write(reg_ctrl2, ®s->ctrl2);
> > +
> > + for (i = 0; i < ram_init[0].len; i++)
> > + priv->write(0, (void __iomem *)regs + ram_init[0].offset +
> > +sizeof(u32) * i);
> > +
> > + if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> > + for (i = 0; i < ram_init[1].len; i++)
> > + priv->write(0, (void __iomem *)regs + ram_init[1].offset +
> > +sizeof(u32) * i);
> > +
> > + reg_ctrl2 &= ~FLEXCAN_CTRL2_WRMFRZ;
> > + priv->write(reg_ctrl2, ®s->ctrl2); }
> > +
> > /* flexcan_chip_start
> > *
> > * this functions is entered with clocks enabled @@ -1316,6 +1380,9
> > @@ static int flexcan_chip_start(struct net_device *dev)
> > if (err)
> > goto out_chip_disable;
> >
> > + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_DISABLE_MECR)
> > + flexcan_init_ram(dev);
>
> Can you test this on both layerscape SoCs (fsl,ls1021ar2-flexcan and
> fsl,lx2160ar1-flexcan)
Would ask Pankaj Bansal for help, since he is the owner of layerscape SoCs, these SoCs are unavailable at my side.
Best Regards,
Joakim Zhang
> > +
> > 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 |
Powered by blists - more mailing lists