* [PATCH V2 0/3] patch set for flexcan
@ 2020-09-27 16:07 Joakim Zhang
2020-09-27 16:07 ` [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function Joakim Zhang
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Joakim Zhang @ 2020-09-27 16:07 UTC (permalink / raw)
To: mkl, linux-can; +Cc: linux-imx, netdev
can: flexcan: initialize all flexcan memory for ECC function
can: flexcan: add flexcan driver for i.MX8MP
These two patches add i.MX8MP driver support.
can: flexcan: disable runtime PM if register flexcandev failed
Resend this patch as a small driver improvement.
Joakim Zhang (3):
can: flexcan: initialize all flexcan memory for ECC function
can: flexcan: add flexcan driver for i.MX8MP
can: flexcan: disable runtime PM if register flexcandev failed
drivers/net/can/flexcan.c | 78 +++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function 2020-09-27 16:07 [PATCH V2 0/3] patch set for flexcan Joakim Zhang @ 2020-09-27 16:07 ` Joakim Zhang 2020-09-27 19:51 ` Marc Kleine-Budde 2020-09-27 19:57 ` Marc Kleine-Budde 2020-09-27 16:08 ` [PATCH V2 2/3] can: flexcan: add flexcan driver for i.MX8MP Joakim Zhang 2020-09-27 16:08 ` [PATCH V2 3/3] can: flexcan: disable runtime PM if register flexcandev failed Joakim Zhang 2 siblings, 2 replies; 10+ messages in thread From: Joakim Zhang @ 2020-09-27 16:07 UTC (permalink / raw) To: mkl, linux-can; +Cc: linux-imx, netdev 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@nxp.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, + }, +}; + 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); + flexcan_set_bittiming(dev); /* MCR -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function 2020-09-27 16:07 ` [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function Joakim Zhang @ 2020-09-27 19:51 ` Marc Kleine-Budde 2020-09-28 2:00 ` Joakim Zhang 2020-09-27 19:57 ` Marc Kleine-Budde 1 sibling, 1 reply; 10+ messages in thread From: Marc Kleine-Budde @ 2020-09-27 19:51 UTC (permalink / raw) To: Joakim Zhang, linux-can; +Cc: linux-imx, netdev [-- Attachment #1.1: Type: text/plain, Size: 5738 bytes --] 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@nxp.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. > + > 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) > + > 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 #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function 2020-09-27 19:51 ` Marc Kleine-Budde @ 2020-09-28 2:00 ` Joakim Zhang 0 siblings, 0 replies; 10+ messages in thread From: Joakim Zhang @ 2020-09-28 2:00 UTC (permalink / raw) To: Marc Kleine-Budde, linux-can, Pankaj Bansal; +Cc: dl-linux-imx, netdev > -----Original Message----- > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: 2020年9月28日 3:52 > To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org > Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.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@nxp.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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function 2020-09-27 16:07 ` [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function Joakim Zhang 2020-09-27 19:51 ` Marc Kleine-Budde @ 2020-09-27 19:57 ` Marc Kleine-Budde 2020-09-28 2:29 ` Joakim Zhang 1 sibling, 1 reply; 10+ messages in thread From: Marc Kleine-Budde @ 2020-09-27 19:57 UTC (permalink / raw) To: Joakim Zhang, linux-can; +Cc: linux-imx, netdev [-- Attachment #1.1: Type: text/plain, Size: 1374 bytes --] On 9/27/20 6:07 PM, Joakim Zhang wrote: [...] > +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); As the write function only does endian conversion, and you're writing 0 here. What about using iowrite32_rep() and get rid of the for loop? 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: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function 2020-09-27 19:57 ` Marc Kleine-Budde @ 2020-09-28 2:29 ` Joakim Zhang 2020-09-28 5:23 ` Joakim Zhang 0 siblings, 1 reply; 10+ messages in thread From: Joakim Zhang @ 2020-09-28 2:29 UTC (permalink / raw) To: Marc Kleine-Budde, linux-can; +Cc: dl-linux-imx, netdev > -----Original Message----- > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: 2020年9月28日 3:58 > To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org > Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.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: > [...] > > > +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); > > As the write function only does endian conversion, and you're writing 0 here. > What about using iowrite32_rep() and get rid of the for loop? Thanks for this point, I will update in next version. Best Regards, Joakim Zhang > 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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function 2020-09-28 2:29 ` Joakim Zhang @ 2020-09-28 5:23 ` Joakim Zhang 2020-09-28 6:20 ` Marc Kleine-Budde 0 siblings, 1 reply; 10+ messages in thread From: Joakim Zhang @ 2020-09-28 5:23 UTC (permalink / raw) To: Marc Kleine-Budde, linux-can; +Cc: dl-linux-imx, netdev > -----Original Message----- > From: Joakim Zhang <qiangqing.zhang@nxp.com> > Sent: 2020年9月28日 10:29 > To: Marc Kleine-Budde <mkl@pengutronix.de>; linux-can@vger.kernel.org > Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.kernel.org > Subject: RE: [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC > function > > > > -----Original Message----- > > From: Marc Kleine-Budde <mkl@pengutronix.de> > > Sent: 2020年9月28日 3:58 > > To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org > > Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.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: > > [...] > > > > > +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); > > > > As the write function only does endian conversion, and you're writing 0 here. > > What about using iowrite32_rep() and get rid of the for loop? > > Thanks for this point, I will update in next version. Ahhh.. I check iowrite32_rep() writes a buf to single address, no shift for address. I prefer to use memset_io() here to initialize a block of io memory. What do you think? Best Regards, Joakim Zhang > Best Regards, > Joakim Zhang > > 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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function 2020-09-28 5:23 ` Joakim Zhang @ 2020-09-28 6:20 ` Marc Kleine-Budde 0 siblings, 0 replies; 10+ messages in thread From: Marc Kleine-Budde @ 2020-09-28 6:20 UTC (permalink / raw) To: Joakim Zhang, linux-can; +Cc: dl-linux-imx, netdev [-- Attachment #1.1: Type: text/plain, Size: 943 bytes --] On 9/28/20 7:23 AM, Joakim Zhang wrote: >>>> + for (i = 0; i < ram_init[0].len; i++) >>>> + priv->write(0, (void __iomem *)regs + ram_init[0].offset + >>>> +sizeof(u32) * i); >>> >>> As the write function only does endian conversion, and you're writing 0 here. >>> What about using iowrite32_rep() and get rid of the for loop? >> >> Thanks for this point, I will update in next version. > > Ahhh.. I check iowrite32_rep() writes a buf to single address, no shift for address. Doh! Right. > I prefer to use memset_io() here to initialize a block of io memory. What do > you think? Of course, that's the correct function for this! 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: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH V2 2/3] can: flexcan: add flexcan driver for i.MX8MP 2020-09-27 16:07 [PATCH V2 0/3] patch set for flexcan Joakim Zhang 2020-09-27 16:07 ` [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function Joakim Zhang @ 2020-09-27 16:08 ` Joakim Zhang 2020-09-27 16:08 ` [PATCH V2 3/3] can: flexcan: disable runtime PM if register flexcandev failed Joakim Zhang 2 siblings, 0 replies; 10+ messages in thread From: Joakim Zhang @ 2020-09-27 16:08 UTC (permalink / raw) To: mkl, linux-can; +Cc: linux-imx, netdev Add flexcan driver for i.MX8MP, which supports CAN FD and ECC. Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> --- ChangeLogs: V1->V2: * sort the order of the quirks by their value. --- drivers/net/can/flexcan.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index aca0fc40ae9b..4844dbf77c9c 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -214,6 +214,7 @@ * MX53 FlexCAN2 03.00.00.00 yes no no no no no * MX6s FlexCAN3 10.00.12.00 yes yes no no yes no * MX8QM FlexCAN3 03.00.23.00 yes yes no no yes yes + * MX8MP FlexCAN3 03.00.17.01 yes yes no yes yes yes * VF610 FlexCAN3 ? no yes no yes yes? no * LS1021A FlexCAN2 03.00.04.00 no yes no no yes no * LX2160A FlexCAN3 03.00.23.00 no yes no no yes yes @@ -410,6 +411,13 @@ static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = { FLEXCAN_QUIRK_SUPPORT_FD, }; +static struct flexcan_devtype_data fsl_imx8mp_devtype_data = { + .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS | + FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | + FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SETUP_STOP_MODE | + FLEXCAN_QUIRK_SUPPORT_FD, +}; + static const struct flexcan_devtype_data fsl_vf610_devtype_data = { .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS | FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | @@ -1912,6 +1920,7 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev) static const struct of_device_id flexcan_of_match[] = { { .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, }, + { .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, }, { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, }, { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, }, { .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data, }, -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH V2 3/3] can: flexcan: disable runtime PM if register flexcandev failed 2020-09-27 16:07 [PATCH V2 0/3] patch set for flexcan Joakim Zhang 2020-09-27 16:07 ` [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function Joakim Zhang 2020-09-27 16:08 ` [PATCH V2 2/3] can: flexcan: add flexcan driver for i.MX8MP Joakim Zhang @ 2020-09-27 16:08 ` Joakim Zhang 2 siblings, 0 replies; 10+ messages in thread From: Joakim Zhang @ 2020-09-27 16:08 UTC (permalink / raw) To: mkl, linux-can; +Cc: linux-imx, netdev Disable runtime PM if register flexcandev failed, and balance reference of usage_count. Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> --- ChangeLogs: V1->V2: * no changes. --- drivers/net/can/flexcan.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index 4844dbf77c9c..ee6d220a4b12 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -2075,6 +2075,8 @@ static int flexcan_probe(struct platform_device *pdev) return 0; failed_register: + pm_runtime_put_noidle(&pdev->dev); + pm_runtime_disable(&pdev->dev); free_candev(dev); return err; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-28 6:21 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-27 16:07 [PATCH V2 0/3] patch set for flexcan Joakim Zhang 2020-09-27 16:07 ` [PATCH V2 1/3] can: flexcan: initialize all flexcan memory for ECC function Joakim Zhang 2020-09-27 19:51 ` Marc Kleine-Budde 2020-09-28 2:00 ` Joakim Zhang 2020-09-27 19:57 ` Marc Kleine-Budde 2020-09-28 2:29 ` Joakim Zhang 2020-09-28 5:23 ` Joakim Zhang 2020-09-28 6:20 ` Marc Kleine-Budde 2020-09-27 16:08 ` [PATCH V2 2/3] can: flexcan: add flexcan driver for i.MX8MP Joakim Zhang 2020-09-27 16:08 ` [PATCH V2 3/3] can: flexcan: disable runtime PM if register flexcandev failed Joakim Zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).