[PATCH linux-can-next/flexcan 0/4] patch set for flexcan

[PATCH linux-can-next/flexcan 0/4] patch set for flexcan

[Joakim Zhang]

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: add CAN wakeup function for i.MX8
This single patch add stop mode support for i.MX8 serials, e.g.
i.MX8QM/QXP

can: flexcan: disable runtime PM if register flexcandev failed
Resend this patch as a small driver improvement.

Joakim Zhang (4):
  can: flexcan: initialize all flexcan memory for ECC function
  can: flexcan: add flexcan driver for i.MX8MP
  can: flexcan: add CAN wakeup function for i.MX8
  can: flexcan: disable runtime PM if register flexcandev failed

 drivers/net/can/flexcan.c | 184 ++++++++++++++++++++++++++++++++++----
 1 file changed, 169 insertions(+), 15 deletions(-)

-- 
2.17.1

[PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Joakim Zhang]

There is a NOTE at the section "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), these memory can be initialized or not.

Initialize all FlexCAN memory before accessing them, otherwise, memory
errors may be detected. The internal region cannot be initialized when
the hardware does not support ECC.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 92 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 90 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 286c67196592..f02f1de2bbca 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -292,7 +292,16 @@ struct flexcan_regs {
 	u32 rximr[64];		/* 0x880 - Not affected by Soft Reset */
 	u32 _reserved5[24];	/* 0x980 */
 	u32 gfwr_mx6;		/* 0x9e0 - MX6 */
-	u32 _reserved6[63];	/* 0x9e4 */
+	u32 _reserved6[39];	/* 0x9e4 */
+	u32 _rxfir[6];		/* 0xa80 */
+	u32 _reserved8[2];	/* 0xa98 */
+	u32 _rxmgmask;		/* 0xaa0 */
+	u32 _rxfgmask;		/* 0xaa4 */
+	u32 _rx14mask;		/* 0xaa8 */
+	u32 _rx15mask;		/* 0xaac */
+	u32 tx_smb[4];		/* 0xab0 */
+	u32 rx_smb0[4];		/* 0xac0 */
+	u32 rx_smb1[4];		/* 0xad0 */
 	u32 mecr;		/* 0xae0 */
 	u32 erriar;		/* 0xae4 */
 	u32 erridpr;		/* 0xae8 */
@@ -305,9 +314,13 @@ struct flexcan_regs {
 	u32 fdctrl;		/* 0xc00 - Not affected by Soft Reset */
 	u32 fdcbt;		/* 0xc04 - Not affected by Soft Reset */
 	u32 fdcrc;		/* 0xc08 */
+	u32 _reserved9[199];	/* 0xc0c */
+	u32 tx_smb_fd[18];	/* 0xf28 */
+	u32 rx_smb0_fd[18];	/* 0xf70 */
+	u32 rx_smb1_fd[18];	/* 0xfb8 */
 };
 
-static_assert(sizeof(struct flexcan_regs) == 0x4 + 0xc08);
+static_assert(sizeof(struct flexcan_regs) == 0x4 * 18 + 0xfb8);
 
 struct flexcan_devtype_data {
 	u32 quirks;		/* quirks needed for different IP cores */
@@ -1292,6 +1305,78 @@ 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, size;
+
+	/* CTRL2[WRMFRZ] grants write access to all memory positions that
+	 * require initialization. MCR[RFEN] must not be set during FlexCAN
+	 * memory initialization.
+	 */
+	reg_ctrl2 = priv->read(&regs->ctrl2);
+	reg_ctrl2 |= FLEXCAN_CTRL2_WRMFRZ;
+	priv->write(reg_ctrl2, &regs->ctrl2);
+
+	/* initialize MBs RAM */
+	size = sizeof(regs->mb) / sizeof(u32);
+	for (i = 0; i < size; i++)
+		priv->write(0, &regs->mb[0][0] + sizeof(u32) * i);
+
+	/* initialize RXIMRs RAM */
+	size = sizeof(regs->rximr) / sizeof(u32);
+	for (i = 0; i < size; i++)
+		priv->write(0, &regs->rximr[i]);
+
+	/* initialize RXFIRs RAM */
+	size = sizeof(regs->_rxfir) / sizeof(u32);
+	for (i = 0; i < size; i++)
+		priv->write(0, &regs->_rxfir[i]);
+
+	/* initialize RXMGMASK, RXFGMASK, RX14MASK, RX15MASK RAM */
+	priv->write(0, &regs->_rxmgmask);
+	priv->write(0, &regs->_rxfgmask);
+	priv->write(0, &regs->_rx14mask);
+	priv->write(0, &regs->_rx15mask);
+
+	/* initialize TX_SMB RAM */
+	size = sizeof(regs->tx_smb) / sizeof(u32);
+	for (i = 0; i < size; i++)
+		priv->write(0, &regs->tx_smb[i]);
+
+	/* initialize RX_SMB0 RAM */
+	size = sizeof(regs->rx_smb0) / sizeof(u32);
+	for (i = 0; i < size; i++)
+		priv->write(0, &regs->rx_smb0[i]);
+
+	/* initialize RX_SMB1 RAM */
+	size = sizeof(regs->rx_smb1) / sizeof(u32);
+	for (i = 0; i < size; i++)
+		priv->write(0, &regs->rx_smb1[i]);
+
+	if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
+		/* initialize TX_SMB_FD RAM */
+		size = sizeof(regs->tx_smb_fd) / sizeof(u32);
+		for (i = 0; i < size; i++)
+			priv->write(0, &regs->tx_smb_fd[i]);
+
+		/* initialize RX_SMB0_FD RAM */
+		size = sizeof(regs->rx_smb0_fd) / sizeof(u32);
+		for (i = 0; i < size; i++)
+			priv->write(0, &regs->rx_smb0_fd[i]);
+
+		/* initialize RX_SMB1_FD RAM */
+		size = sizeof(regs->rx_smb1_fd) / sizeof(u32);
+		for (i = 0; i < size; i++)
+			priv->write(0, &regs->rx_smb0_fd[i]);
+	}
+
+	reg_ctrl2 &= ~FLEXCAN_CTRL2_WRMFRZ;
+	priv->write(reg_ctrl2, &regs->ctrl2);
+}
+
 /* flexcan_chip_start
  *
  * this functions is entered with clocks enabled
@@ -1316,6 +1401,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

Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 6284 bytes --]

On 9/25/20 5:10 PM, Joakim Zhang wrote:
> There is a NOTE at the section "Detection and correction of memory errors":

Can you add a reference to one datasheet including name, revision and section?

> 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), these memory can be initialized or not.
> 
> Initialize all FlexCAN memory before accessing them, otherwise, memory
> errors may be detected. The internal region cannot be initialized when
> the hardware does not support ECC.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  drivers/net/can/flexcan.c | 92 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 286c67196592..f02f1de2bbca 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -292,7 +292,16 @@ struct flexcan_regs {
>  	u32 rximr[64];		/* 0x880 - Not affected by Soft Reset */
>  	u32 _reserved5[24];	/* 0x980 */
>  	u32 gfwr_mx6;		/* 0x9e0 - MX6 */
> -	u32 _reserved6[63];	/* 0x9e4 */
> +	u32 _reserved6[39];	/* 0x9e4 */
> +	u32 _rxfir[6];		/* 0xa80 */
> +	u32 _reserved8[2];	/* 0xa98 */
> +	u32 _rxmgmask;		/* 0xaa0 */
> +	u32 _rxfgmask;		/* 0xaa4 */
> +	u32 _rx14mask;		/* 0xaa8 */
> +	u32 _rx15mask;		/* 0xaac */
> +	u32 tx_smb[4];		/* 0xab0 */
> +	u32 rx_smb0[4];		/* 0xac0 */
> +	u32 rx_smb1[4];		/* 0xad0 */
>  	u32 mecr;		/* 0xae0 */
>  	u32 erriar;		/* 0xae4 */
>  	u32 erridpr;		/* 0xae8 */
> @@ -305,9 +314,13 @@ struct flexcan_regs {
>  	u32 fdctrl;		/* 0xc00 - Not affected by Soft Reset */
>  	u32 fdcbt;		/* 0xc04 - Not affected by Soft Reset */
>  	u32 fdcrc;		/* 0xc08 */
> +	u32 _reserved9[199];	/* 0xc0c */
> +	u32 tx_smb_fd[18];	/* 0xf28 */
> +	u32 rx_smb0_fd[18];	/* 0xf70 */
> +	u32 rx_smb1_fd[18];	/* 0xfb8 */
>  };
>  
> -static_assert(sizeof(struct flexcan_regs) == 0x4 + 0xc08);
> +static_assert(sizeof(struct flexcan_regs) == 0x4 * 18 + 0xfb8);
>  
>  struct flexcan_devtype_data {
>  	u32 quirks;		/* quirks needed for different IP cores */
> @@ -1292,6 +1305,78 @@ 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, size;
> +
> +	/* CTRL2[WRMFRZ] grants write access to all memory positions that
> +	 * require initialization. MCR[RFEN] must not be set during FlexCAN
> +	 * memory initialization.

Please add here the reference to the datasheet aswell.

> +	 */
> +	reg_ctrl2 = priv->read(&regs->ctrl2);
> +	reg_ctrl2 |= FLEXCAN_CTRL2_WRMFRZ;
> +	priv->write(reg_ctrl2, &regs->ctrl2);
> +
> +	/* initialize MBs RAM */
> +	size = sizeof(regs->mb) / sizeof(u32);
> +	for (i = 0; i < size; i++)
> +		priv->write(0, &regs->mb[0][0] + sizeof(u32) * i);

Can you create a "static const struct" holding the reg (or offset) + len and
loop over it. Something linke this?

const struct struct flexcan_ram_init ram_init[] {
	void __iomem *reg;
	u16 len;
} = {
	{
		.reg = regs->mb,	/* MB RAM */
		.len = sizeof(regs->mb), / sizeof(u32),
	}, {
		.reg = regs->rximr,	/* RXIMR RAM */
		.len = sizeof(regs->rximr),
	}, {
		...
	},
};


> +
> +	/* initialize RXIMRs RAM */
> +	size = sizeof(regs->rximr) / sizeof(u32);
> +	for (i = 0; i < size; i++)
> +		priv->write(0, &regs->rximr[i]);
> +
> +	/* initialize RXFIRs RAM */
> +	size = sizeof(regs->_rxfir) / sizeof(u32);
> +	for (i = 0; i < size; i++)
> +		priv->write(0, &regs->_rxfir[i]);
> +
> +	/* initialize RXMGMASK, RXFGMASK, RX14MASK, RX15MASK RAM */
> +	priv->write(0, &regs->_rxmgmask);
> +	priv->write(0, &regs->_rxfgmask);
> +	priv->write(0, &regs->_rx14mask);
> +	priv->write(0, &regs->_rx15mask);
> +
> +	/* initialize TX_SMB RAM */
> +	size = sizeof(regs->tx_smb) / sizeof(u32);
> +	for (i = 0; i < size; i++)
> +		priv->write(0, &regs->tx_smb[i]);
> +
> +	/* initialize RX_SMB0 RAM */
> +	size = sizeof(regs->rx_smb0) / sizeof(u32);
> +	for (i = 0; i < size; i++)
> +		priv->write(0, &regs->rx_smb0[i]);
> +
> +	/* initialize RX_SMB1 RAM */
> +	size = sizeof(regs->rx_smb1) / sizeof(u32);
> +	for (i = 0; i < size; i++)
> +		priv->write(0, &regs->rx_smb1[i]);
> +
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
> +		/* initialize TX_SMB_FD RAM */

and the same for the fd-mode

> +		size = sizeof(regs->tx_smb_fd) / sizeof(u32);
> +		for (i = 0; i < size; i++)
> +			priv->write(0, &regs->tx_smb_fd[i]);
> +
> +		/* initialize RX_SMB0_FD RAM */
> +		size = sizeof(regs->rx_smb0_fd) / sizeof(u32);
> +		for (i = 0; i < size; i++)
> +			priv->write(0, &regs->rx_smb0_fd[i]);
> +
> +		/* initialize RX_SMB1_FD RAM */
> +		size = sizeof(regs->rx_smb1_fd) / sizeof(u32);
> +		for (i = 0; i < size; i++)
> +			priv->write(0, &regs->rx_smb0_fd[i]);
> +	}
> +
> +	reg_ctrl2 &= ~FLEXCAN_CTRL2_WRMFRZ;
> +	priv->write(reg_ctrl2, &regs->ctrl2);
> +}
> +
>  /* flexcan_chip_start
>   *
>   * this functions is entered with clocks enabled
> @@ -1316,6 +1401,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
> 

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

RE: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 15:29
> 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 linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan
> memory for ECC function
> 
> On 9/25/20 5:10 PM, Joakim Zhang wrote:
> > There is a NOTE at the section "Detection and correction of memory errors":
> 
> Can you add a reference to one datasheet including name, revision and
> section?

Ok.

> > 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), these memory can be initialized or not.
> >
> > Initialize all FlexCAN memory before accessing them, otherwise, memory
> > errors may be detected. The internal region cannot be initialized when
> > the hardware does not support ECC.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  drivers/net/can/flexcan.c | 92
> > ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 90 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 286c67196592..f02f1de2bbca 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -292,7 +292,16 @@ struct flexcan_regs {
> >  	u32 rximr[64];		/* 0x880 - Not affected by Soft Reset */
> >  	u32 _reserved5[24];	/* 0x980 */
> >  	u32 gfwr_mx6;		/* 0x9e0 - MX6 */
> > -	u32 _reserved6[63];	/* 0x9e4 */
> > +	u32 _reserved6[39];	/* 0x9e4 */
> > +	u32 _rxfir[6];		/* 0xa80 */
> > +	u32 _reserved8[2];	/* 0xa98 */
> > +	u32 _rxmgmask;		/* 0xaa0 */
> > +	u32 _rxfgmask;		/* 0xaa4 */
> > +	u32 _rx14mask;		/* 0xaa8 */
> > +	u32 _rx15mask;		/* 0xaac */
> > +	u32 tx_smb[4];		/* 0xab0 */
> > +	u32 rx_smb0[4];		/* 0xac0 */
> > +	u32 rx_smb1[4];		/* 0xad0 */
> >  	u32 mecr;		/* 0xae0 */
> >  	u32 erriar;		/* 0xae4 */
> >  	u32 erridpr;		/* 0xae8 */
> > @@ -305,9 +314,13 @@ struct flexcan_regs {
> >  	u32 fdctrl;		/* 0xc00 - Not affected by Soft Reset */
> >  	u32 fdcbt;		/* 0xc04 - Not affected by Soft Reset */
> >  	u32 fdcrc;		/* 0xc08 */
> > +	u32 _reserved9[199];	/* 0xc0c */
> > +	u32 tx_smb_fd[18];	/* 0xf28 */
> > +	u32 rx_smb0_fd[18];	/* 0xf70 */
> > +	u32 rx_smb1_fd[18];	/* 0xfb8 */
> >  };
> >
> > -static_assert(sizeof(struct flexcan_regs) == 0x4 + 0xc08);
> > +static_assert(sizeof(struct flexcan_regs) == 0x4 * 18 + 0xfb8);
> >
> >  struct flexcan_devtype_data {
> >  	u32 quirks;		/* quirks needed for different IP cores */
> > @@ -1292,6 +1305,78 @@ 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, size;
> > +
> > +	/* CTRL2[WRMFRZ] grants write access to all memory positions that
> > +	 * require initialization. MCR[RFEN] must not be set during FlexCAN
> > +	 * memory initialization.
> 
> Please add here the reference to the datasheet aswell.

OK.

> > +	 */
> > +	reg_ctrl2 = priv->read(&regs->ctrl2);
> > +	reg_ctrl2 |= FLEXCAN_CTRL2_WRMFRZ;
> > +	priv->write(reg_ctrl2, &regs->ctrl2);
> > +
> > +	/* initialize MBs RAM */
> > +	size = sizeof(regs->mb) / sizeof(u32);
> > +	for (i = 0; i < size; i++)
> > +		priv->write(0, &regs->mb[0][0] + sizeof(u32) * i);
> 
> Can you create a "static const struct" holding the reg (or offset) + len and loop
> over it. Something linke this?
> 
> const struct struct flexcan_ram_init ram_init[] {
> 	void __iomem *reg;
> 	u16 len;
> } = {
> 	{
> 		.reg = regs->mb,	/* MB RAM */
> 		.len = sizeof(regs->mb), / sizeof(u32),
> 	}, {
> 		.reg = regs->rximr,	/* RXIMR RAM */
> 		.len = sizeof(regs->rximr),
> 	}, {
> 		...
> 	},
> };

Better solution, I will change into this way. Thanks.

> 
> > +
> > +	/* initialize RXIMRs RAM */
> > +	size = sizeof(regs->rximr) / sizeof(u32);
> > +	for (i = 0; i < size; i++)
> > +		priv->write(0, &regs->rximr[i]);
> > +
> > +	/* initialize RXFIRs RAM */
> > +	size = sizeof(regs->_rxfir) / sizeof(u32);
> > +	for (i = 0; i < size; i++)
> > +		priv->write(0, &regs->_rxfir[i]);
> > +
> > +	/* initialize RXMGMASK, RXFGMASK, RX14MASK, RX15MASK RAM */
> > +	priv->write(0, &regs->_rxmgmask);
> > +	priv->write(0, &regs->_rxfgmask);
> > +	priv->write(0, &regs->_rx14mask);
> > +	priv->write(0, &regs->_rx15mask);
> > +
> > +	/* initialize TX_SMB RAM */
> > +	size = sizeof(regs->tx_smb) / sizeof(u32);
> > +	for (i = 0; i < size; i++)
> > +		priv->write(0, &regs->tx_smb[i]);
> > +
> > +	/* initialize RX_SMB0 RAM */
> > +	size = sizeof(regs->rx_smb0) / sizeof(u32);
> > +	for (i = 0; i < size; i++)
> > +		priv->write(0, &regs->rx_smb0[i]);
> > +
> > +	/* initialize RX_SMB1 RAM */
> > +	size = sizeof(regs->rx_smb1) / sizeof(u32);
> > +	for (i = 0; i < size; i++)
> > +		priv->write(0, &regs->rx_smb1[i]);
> > +
> > +	if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
> > +		/* initialize TX_SMB_FD RAM */
> 
> and the same for the fd-mode

OK.

Best Regards,
Joakim Zhang
> > +		size = sizeof(regs->tx_smb_fd) / sizeof(u32);
> > +		for (i = 0; i < size; i++)
> > +			priv->write(0, &regs->tx_smb_fd[i]);
> > +
> > +		/* initialize RX_SMB0_FD RAM */
> > +		size = sizeof(regs->rx_smb0_fd) / sizeof(u32);
> > +		for (i = 0; i < size; i++)
> > +			priv->write(0, &regs->rx_smb0_fd[i]);
> > +
> > +		/* initialize RX_SMB1_FD RAM */
> > +		size = sizeof(regs->rx_smb1_fd) / sizeof(u32);
> > +		for (i = 0; i < size; i++)
> > +			priv->write(0, &regs->rx_smb0_fd[i]);
> > +	}
> > +
> > +	reg_ctrl2 &= ~FLEXCAN_CTRL2_WRMFRZ;
> > +	priv->write(reg_ctrl2, &regs->ctrl2); }
> > +
> >  /* flexcan_chip_start
> >   *
> >   * this functions is entered with clocks enabled @@ -1316,6 +1401,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
> >
> 
> 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 |

RE: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Joakim Zhang]

Hi Marc,

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 15:29
> 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 linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan
> memory for ECC function
> 
> On 9/25/20 5:10 PM, Joakim Zhang wrote:
> > There is a NOTE at the section "Detection and correction of memory errors":
> 
> Can you add a reference to one datasheet including name, revision and
> section?
> 
> > 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), these memory can be initialized or not.
> >
> > Initialize all FlexCAN memory before accessing them, otherwise, memory
> > errors may be detected. The internal region cannot be initialized when
> > the hardware does not support ECC.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > +	reg_ctrl2 = priv->read(&regs->ctrl2);
> > +	reg_ctrl2 |= FLEXCAN_CTRL2_WRMFRZ;
> > +	priv->write(reg_ctrl2, &regs->ctrl2);
> > +
> > +	/* initialize MBs RAM */
> > +	size = sizeof(regs->mb) / sizeof(u32);
> > +	for (i = 0; i < size; i++)
> > +		priv->write(0, &regs->mb[0][0] + sizeof(u32) * i);
> 

[...]
> Can you create a "static const struct" holding the reg (or offset) + len and loop
> over it. Something linke this?
> 
> const struct struct flexcan_ram_init ram_init[] {
> 	void __iomem *reg;
> 	u16 len;
> } = {
> 	{
> 		.reg = regs->mb,	/* MB RAM */
> 		.len = sizeof(regs->mb), / sizeof(u32),
> 	}, {
> 		.reg = regs->rximr,	/* RXIMR RAM */
> 		.len = sizeof(regs->rximr),
> 	}, {
> 		...
> 	},
> };

In this version, I only initialize the implemented memory, so that it's a several trivial memory slice, reserved memory not initialized. Follow your point, I need create a global pointer for struct flexcan_reg,
i.e. static struct flexcan_regs *reg, so that we can use .reg = regs->mb in ram_init[], IMHO, I don't quite want to add this, or is there any better solution to get the reg/len value?

According to below notes and discussed with IP owner before, reserved memory also can be initialized. So I want to add two memory regions, and initialize them together,
this could be more clean. I will send out a V2, please let me know which one do you think is better?

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

Best Regards,
Joakim Zhang

Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 2063 bytes --]

On 9/27/20 10:01 AM, Joakim Zhang wrote:
> [...]
>> Can you create a "static const struct" holding the reg (or offset) + len and loop
>> over it. Something linke this?
>>
>> const struct struct flexcan_ram_init ram_init[] {
>> 	void __iomem *reg;
>> 	u16 len;
>> } = {
>> 	{
>> 		.reg = regs->mb,	/* MB RAM */
>> 		.len = sizeof(regs->mb), / sizeof(u32),
>> 	}, {
>> 		.reg = regs->rximr,	/* RXIMR RAM */
>> 		.len = sizeof(regs->rximr),
>> 	}, {
>> 		...
>> 	},
>> };
> 
> In this version, I only initialize the implemented memory, so that it's a
> several trivial memory slice, reserved memory not initialized. Follow your
> point, I need create a global pointer for struct flexcan_reg, i.e. static
> struct flexcan_regs *reg, so that we can use .reg = regs->mb in ram_init[],
> IMHO, I don't quite want to add this, or is there any better solution to get
> the reg/len value?

One option is not to make it a global variable, but to move it into the
function, then you have the reg pointer available.

> According to below notes and discussed with IP owner before, reserved memory
> also can be initialized. So I want to add two memory regions, and initialize
> them together, this could be more clean. I will send out a V2, please let me
> know which one do you think is better?

If it's OK on all SoCs to initialize the complete RAM area, just do it. Then we
can get rid of the proposed struct at all.

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

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

RE: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月28日 3:56
> 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 linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan
> memory for ECC function
> 
> On 9/27/20 10:01 AM, Joakim Zhang wrote:
> > [...]
> >> Can you create a "static const struct" holding the reg (or offset) +
> >> len and loop over it. Something linke this?
> >>
> >> const struct struct flexcan_ram_init ram_init[] {
> >> 	void __iomem *reg;
> >> 	u16 len;
> >> } = {
> >> 	{
> >> 		.reg = regs->mb,	/* MB RAM */
> >> 		.len = sizeof(regs->mb), / sizeof(u32),
> >> 	}, {
> >> 		.reg = regs->rximr,	/* RXIMR RAM */
> >> 		.len = sizeof(regs->rximr),
> >> 	}, {
> >> 		...
> >> 	},
> >> };
> >
> > In this version, I only initialize the implemented memory, so that
> > it's a several trivial memory slice, reserved memory not initialized.
> > Follow your point, I need create a global pointer for struct
> > flexcan_reg, i.e. static struct flexcan_regs *reg, so that we can use
> > .reg = regs->mb in ram_init[], IMHO, I don't quite want to add this,
> > or is there any better solution to get the reg/len value?
> 
> One option is not to make it a global variable, but to move it into the function,
> then you have the reg pointer available.

Will take into account if later we also need implement this struct.

> > According to below notes and discussed with IP owner before, reserved
> > memory also can be initialized. So I want to add two memory regions,
> > and initialize them together, this could be more clean. I will send
> > out a V2, please let me know which one do you think is better?
> 
> If it's OK on all SoCs to initialize the complete RAM area, just do it. Then we can
> get rid of the proposed struct at all.

Should be OK according to IP guys feedbacks.

I am checking layerscape's CAN section:

There is no ECC section in LS1021A 
https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-multicore-processors/layerscape-1021a-dual-core-communications-processor-with-lcd-controller:LS1021A?tab=Documentation_Tab


ECC section in LX2160A, also contains the same NOTE as i.MX8MP.
https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-multicore-processors/layerscape-lx2160a-multicore-communications-processor:LX2160A?tab=Documentation_Tab


Hi @Pankaj Bansal, could you please also have a check?

Best Regards,
Joakim Zhang
> > "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."
> 
> 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 |

Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 2225 bytes --]

On 9/28/20 4:27 AM, Joakim Zhang wrote:
>> If it's OK on all SoCs to initialize the complete RAM area, just do it. Then we can
>> get rid of the proposed struct at all.
> 
> Should be OK according to IP guys feedbacks.

Good!

> 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 |
> 		FLEXCAN_QUIRK_BROKEN_PERR_STATE,
> };
> 
> static const struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
> 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> 		FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> 		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
> };
> 
> static const struct flexcan_devtype_data fsl_lx2160a_r1_devtype_data = {
> 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> 		FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> 		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_SUPPORT_FD,
> };

> I am checking layerscape's CAN section:
> 
> There is no ECC section in LS1021A 
> https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-multicore-processors/layerscape-1021a-dual-core-communications-processor-with-lcd-controller:LS1021A?tab=Documentation_Tab

Hmmm, why does the LS1021A have "FLEXCAN_QUIRK_DISABLE_MECR"? The bits in the
ctrl2 and the mecr register itself used in the quirk are marked as reserved in
this datasheet....

Can @Pankaj Bansal clarify this?

> ECC section in LX2160A, also contains the same NOTE as i.MX8MP.
> https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-multicore-processors/layerscape-lx2160a-multicore-communications-processor:LX2160A?tab=Documentation_Tab

> Hi @Pankaj Bansal, could you please also have a check?
Can someone check the vf610, too?

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

RE: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月28日 15:01
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org;
> Pankaj Bansal <pankaj.bansal@nxp.com>
> Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan
> memory for ECC function
> 
> On 9/28/20 4:27 AM, Joakim Zhang wrote:
> >> If it's OK on all SoCs to initialize the complete RAM area, just do
> >> it. Then we can get rid of the proposed struct at all.
> >
> > Should be OK according to IP guys feedbacks.
> 
> Good!
> 
> > 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 |
> > 		FLEXCAN_QUIRK_BROKEN_PERR_STATE,
> > };
> >
> > static const struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
> > 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> > 		FLEXCAN_QUIRK_DISABLE_MECR |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> > 		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
> > };
> >
> > static const struct flexcan_devtype_data fsl_lx2160a_r1_devtype_data = {
> > 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> > 		FLEXCAN_QUIRK_DISABLE_MECR |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> > 		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> FLEXCAN_QUIRK_SUPPORT_FD, };
> 
> > I am checking layerscape's CAN section:
> >
> > There is no ECC section in LS1021A
> > https://www.nxp.com/products/processors-and-microcontrollers/arm-proce
> > ssors/layerscape-multicore-processors/layerscape-1021a-dual-core-commu
> > nications-processor-with-lcd-controller:LS1021A?tab=Documentation_Tab
> 
> Hmmm, why does the LS1021A have "FLEXCAN_QUIRK_DISABLE_MECR"? The
> bits in the
> ctrl2 and the mecr register itself used in the quirk are marked as reserved in
> this datasheet....
> 
> Can @Pankaj Bansal clarify this?
> 
> > ECC section in LX2160A, also contains the same NOTE as i.MX8MP.
> > https://www.nxp.com/products/processors-and-microcontrollers/arm-proce
> > ssors/layerscape-multicore-processors/layerscape-lx2160a-multicore-com
> > munications-processor:LX2160A?tab=Documentation_Tab
> 
> > Hi @Pankaj Bansal, could you please also have a check?
> Can someone check the vf610, too?

I check the VF610 RM just now, indeed it has ECC feature, there is also a NOTE in "12.1.4.13 Detection and Correction of Memory Errors" section:

All FlexCAN memory must be initialized before starting its
operation in order to have the parity bits in memory properly
updated. The WRMFRZ bit in Control 2 Register (CTRL2)
grants write access to all memory positions from 0x080 to
0xADF.

Best Regards,
Joakim Zhang
> 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 |

Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 787 bytes --]

On 9/28/20 9:58 AM, Joakim Zhang wrote:
>> Can someone check the vf610, too?
> 
> I check the VF610 RM just now, indeed it has ECC feature, there is also a NOTE in "12.1.4.13 Detection and Correction of Memory Errors" section:
> 
> All FlexCAN memory must be initialized before starting its
> operation in order to have the parity bits in memory properly
> updated. The WRMFRZ bit in Control 2 Register (CTRL2)
> grants write access to all memory positions from 0x080 to
> 0xADF.

Sounds good.

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

RE: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月28日 16:21
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org;
> Pankaj Bansal <pankaj.bansal@nxp.com>
> Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan
> memory for ECC function
> 
> On 9/28/20 9:58 AM, Joakim Zhang wrote:
> >> Can someone check the vf610, too?
> >
> > I check the VF610 RM just now, indeed it has ECC feature, there is also a
> NOTE in "12.1.4.13 Detection and Correction of Memory Errors" section:
> >
> > All FlexCAN memory must be initialized before starting its operation
> > in order to have the parity bits in memory properly updated. The
> > WRMFRZ bit in Control 2 Register (CTRL2) grants write access to all
> > memory positions from 0x080 to 0xADF.
> 
> Sounds good.


Could I send out a V3 to review firstly, then wait Pankaj have time to do the test?

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 |

Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 430 bytes --]

On 9/28/20 10:36 AM, Joakim Zhang wrote:
> Could I send out a V3 to review firstly, then wait Pankaj have time to do the
> test?

sure, go ahead.

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

Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1317 bytes --]

On 9/25/20 5:10 PM, Joakim Zhang wrote:
> There is a NOTE at the section "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), these memory can be initialized or not.
> 
> Initialize all FlexCAN memory before accessing them, otherwise, memory
> errors may be detected. The internal region cannot be initialized when
> the hardware does not support ECC.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>

Is this whole patch valid/compatible with the mx7,too?

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

RE: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 15:33
> 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 linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan
> memory for ECC function
> 
> On 9/25/20 5:10 PM, Joakim Zhang wrote:
> > There is a NOTE at the section "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), these memory can be initialized or not.
> >
> > Initialize all FlexCAN memory before accessing them, otherwise, memory
> > errors may be detected. The internal region cannot be initialized when
> > the hardware does not support ECC.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> 
> Is this whole patch valid/compatible with the mx7,too?

Hi Marc,

I notice it just now, seems lack of patch for imx firmware in upstream, that will always export scu symbols.
include/linux/firmware/imx/svc/misc.h

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 |

Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 569 bytes --]

On 9/25/20 9:38 AM, Joakim Zhang wrote:
> I notice it just now, seems lack of patch for imx firmware in upstream, that
> will always export scu symbols. include/linux/firmware/imx/svc/misc.h

That will affect "can: flexcan: add CAN wakeup function for i.MX8" not this
patch, right?

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

RE: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 16:11
> 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 linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan
> memory for ECC function
> 
> On 9/25/20 9:38 AM, Joakim Zhang wrote:
> > I notice it just now, seems lack of patch for imx firmware in
> > upstream, that will always export scu symbols.
> > include/linux/firmware/imx/svc/misc.h
> 
> That will affect "can: flexcan: add CAN wakeup function for i.MX8" not this
> patch, right?

Hi Marc,

Yes, only affect "can: flexcan: add CAN wakeup function for i.MX8", I will remove this patch first.
Sorry, I replied in a wrong place.

"can: flexcan: initialize all flexcan memory for ECC function" for this patch, I find this issue in i.MX8MP, which is the first SoC implements ECC for i.MX
I think this patch should compatible with others which has ECC support, but I don't have one to have a test.

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 |

Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1079 bytes --]

On 9/25/20 10:51 AM, Joakim Zhang wrote:
>>> I notice it just now, seems lack of patch for imx firmware in
>>> upstream, that will always export scu symbols.
>>> include/linux/firmware/imx/svc/misc.h
>>
>> That will affect "can: flexcan: add CAN wakeup function for i.MX8" not this
>> patch, right?
> 
> Yes, only affect "can: flexcan: add CAN wakeup function for i.MX8", I will
> remove this patch first.

ok

> Sorry, I replied in a wrong place.

np

> "can: flexcan: initialize all flexcan memory for ECC function" for this
> patch, I find this issue in i.MX8MP, which is the first SoC implements ECC
> for i.MX

What about the mx7?

> I think this patch should compatible with others which has ECC support, but I
> don't have one to have a test.

What about the mx7?

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

RE: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 17:03
> 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 linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan
> memory for ECC function
> 
> On 9/25/20 10:51 AM, Joakim Zhang wrote:
> >>> I notice it just now, seems lack of patch for imx firmware in
> >>> upstream, that will always export scu symbols.
> >>> include/linux/firmware/imx/svc/misc.h
> >>
> >> That will affect "can: flexcan: add CAN wakeup function for i.MX8"
> >> not this patch, right?
> >
> > Yes, only affect "can: flexcan: add CAN wakeup function for i.MX8", I
> > will remove this patch first.
> 
> ok
> 
> > Sorry, I replied in a wrong place.
> 
> np
> 
> > "can: flexcan: initialize all flexcan memory for ECC function" for
> > this patch, I find this issue in i.MX8MP, which is the first SoC
> > implements ECC for i.MX
> 
> What about the mx7?
> 
> > I think this patch should compatible with others which has ECC
> > support, but I don't have one to have a test.
> 
> What about the mx7?

As I know only i.MX7D integrated in Flexcan without ECC, I am not quite understand what you meaning.
Could you explain more?

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 |

Re: [PATCH linux-can-next/flexcan 1/4] can: flexcan: initialize all flexcan memory for ECC function

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 909 bytes --]

On 9/25/20 11:09 AM, Joakim Zhang wrote:
>>> "can: flexcan: initialize all flexcan memory for ECC function" for
>>> this patch, I find this issue in i.MX8MP, which is the first SoC
>>> implements ECC for i.MX
>>
>> What about the mx7?
>>> I think this patch should compatible with others which has ECC
>>> support, but I don't have one to have a test.
>>
>> What about the mx7?
> 
> As I know only i.MX7D integrated in Flexcan without ECC, I am not quite understand what you meaning.
> Could you explain more?

Doh! Looking at the table in driver corrects me and says the VF610 has ECC (not
the imx7).

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

[PATCH linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver for i.MX8MP

[Joakim Zhang]

Add flexcan driver for i.MX8MP, which supports CAN FD and ECC.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 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 f02f1de2bbca..8c8753f77764 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
@@ -389,6 +390,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_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
+		FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE |
+		FLEXCAN_QUIRK_DISABLE_MECR,
+};
+
 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 |
@@ -1932,6 +1940,7 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
 }
 
 static const struct of_device_id flexcan_of_match[] = {
+	{ .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },
 	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
 	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
 	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
-- 
2.17.1

Re: [PATCH linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver for i.MX8MP

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 2641 bytes --]

On 9/25/20 5:10 PM, Joakim Zhang wrote:
> Add flexcan driver for i.MX8MP, which supports CAN FD and ECC.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  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 f02f1de2bbca..8c8753f77764 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
> @@ -389,6 +390,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_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> +		FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE |
> +		FLEXCAN_QUIRK_DISABLE_MECR,

Can you sort the order of the quirks by their value?

> +};
> +
>  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 |
> @@ -1932,6 +1940,7 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id flexcan_of_match[] = {
> +	{ .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },
>  	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
>  	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
>  	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
> 

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

RE: [PATCH linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver for i.MX8MP

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 15:37
> 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 linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver
> for i.MX8MP
> 
> On 9/25/20 5:10 PM, Joakim Zhang wrote:
> > Add flexcan driver for i.MX8MP, which supports CAN FD and ECC.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  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 f02f1de2bbca..8c8753f77764 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
> > @@ -389,6 +390,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_USE_OFF_TIMESTAMP |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> > +		FLEXCAN_QUIRK_SUPPORT_FD |
> FLEXCAN_QUIRK_SETUP_STOP_MODE |
> > +		FLEXCAN_QUIRK_DISABLE_MECR,
> 
> Can you sort the order of the quirks by their value?

Ok, I have not noticed such details before, sorry.

Best Regards,
Joakim Zhang
> > +};
> > +
> >  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 | @@
> > -1932,6 +1940,7 @@ static int flexcan_setup_stop_mode(struct
> > platform_device *pdev)  }
> >
> >  static const struct of_device_id flexcan_of_match[] = {
> > +	{ .compatible = "fsl,imx8mp-flexcan", .data =
> > +&fsl_imx8mp_devtype_data, },
> >  	{ .compatible = "fsl,imx8qm-flexcan", .data =
> &fsl_imx8qm_devtype_data, },
> >  	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
> >  	{ .compatible = "fsl,imx28-flexcan", .data =
> > &fsl_imx28_devtype_data, },
> >
> 
> 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 |

Re: [PATCH linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver for i.MX8MP

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1204 bytes --]

On 9/25/20 5:10 PM, Joakim Zhang wrote:
> Add flexcan driver for i.MX8MP, which supports CAN FD and ECC.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  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 f02f1de2bbca..8c8753f77764 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

This doesn't apply to net-next/master. The MX8QM indented differently.

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

RE: [PATCH linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver for i.MX8MP

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 17:05
> 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 linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver
> for i.MX8MP
> 
> On 9/25/20 5:10 PM, Joakim Zhang wrote:
> > Add flexcan driver for i.MX8MP, which supports CAN FD and ECC.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  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 f02f1de2bbca..8c8753f77764 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
> 
> This doesn't apply to net-next/master. The MX8QM indented differently.

Need I rebase on net-next/master in next version? This patch set is made from linux-can-next/flexcan branch.

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 |

Re: [PATCH linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver for i.MX8MP

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 608 bytes --]

On 9/25/20 11:11 AM, Joakim Zhang wrote:
>> This doesn't apply to net-next/master. The MX8QM indented differently.
> 
> Need I rebase on net-next/master in next version? This patch set is made from
> linux-can-next/flexcan branch.
Yes, the flexcan patches are already in David's tree, so please base on
net-next/master.

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

RE: [PATCH linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver for i.MX8MP

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 17:36
> 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 linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver
> for i.MX8MP
> 
> On 9/25/20 11:11 AM, Joakim Zhang wrote:
> >> This doesn't apply to net-next/master. The MX8QM indented differently.
> >
> > Need I rebase on net-next/master in next version? This patch set is
> > made from linux-can-next/flexcan branch.
> Yes, the flexcan patches are already in David's tree, so please base on
> net-next/master.

Ok, another I want to indicate is that, i.MX8MP RM has not released to public, I only have internal review version and specific RM of i.MX8MP generated by IP owners.

So I am unable to give a website link for this, just add the name, version, section.

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 |

Re: [PATCH linux-can-next/flexcan 2/4] can: flexcan: add flexcan driver for i.MX8MP

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 892 bytes --]

On 9/25/20 11:48 AM, Joakim Zhang wrote:
>>> Need I rebase on net-next/master in next version? This patch set is
>>> made from linux-can-next/flexcan branch.
>> Yes, the flexcan patches are already in David's tree, so please base on
>> net-next/master.
> 
> Ok, another I want to indicate is that, i.MX8MP RM has not released to
> public, I only have internal review version and specific RM of i.MX8MP
> generated by IP owners.

> So I am unable to give a website link for this, just add the name, version,
> section.

No problem, we can change the section if needed once the datasheet is public.

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

[PATCH linux-can-next/flexcan 3/4] can: flexcan: add CAN wakeup function for i.MX8

[Joakim Zhang]

The System Controller Firmware (SCFW) is a low-level system function
which runs on a dedicated Cortex-M core to provide power, clock, and
resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
(QM, QP), and i.MX8QX (QXP, DX).

SCU driver manages the IPC interface between host CPU and the
SCU firmware running on M4.

For i.MX8, stop mode request is controlled by System Controller Unit(SCU)
firmware.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 81 ++++++++++++++++++++++++++++++++-------
 1 file changed, 68 insertions(+), 13 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8c8753f77764..41b52cb56f93 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -9,6 +9,7 @@
 //
 // Based on code originally by Andrey Volkov <avolkov@varma-el.com>
 
+#include <dt-bindings/firmware/imx/rsrc.h>
 #include <linux/bitfield.h>
 #include <linux/can.h>
 #include <linux/can/dev.h>
@@ -17,6 +18,7 @@
 #include <linux/can/rx-offload.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/firmware/imx/sci.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/mfd/syscon.h>
@@ -240,6 +242,8 @@
 #define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)
 /* Support CAN-FD mode */
 #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
+/* Use System Controller Firmware */
+#define FLEXCAN_QUIRK_USE_SCFW BIT(10)
 
 /* Structure of the message buffer */
 struct flexcan_mb {
@@ -358,6 +362,9 @@ struct flexcan_priv {
 	struct regulator *reg_xceiver;
 	struct flexcan_stop_mode stm;
 
+	/* IPC handle when enable stop mode by System Controller firmware(scfw) */
+	struct imx_sc_ipc *sc_ipc_handle;
+
 	/* Read and Write APIs */
 	u32 (*read)(void __iomem *addr);
 	void (*write)(u32 val, void __iomem *addr);
@@ -387,7 +394,8 @@ static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
 static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
 		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
-		FLEXCAN_QUIRK_SUPPORT_FD,
+		FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE |
+		FLEXCAN_QUIRK_USE_SCFW,
 };
 
 static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
@@ -546,6 +554,25 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable)
 	priv->write(reg_mcr, &regs->mcr);
 }
 
+static void flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool enabled)
+{
+	struct device_node *np = priv->dev->of_node;
+	u32 rsrc_id, val;
+	int idx;
+
+	idx = of_alias_get_id(np, "can");
+	if (idx == 0)
+		rsrc_id = IMX_SC_R_CAN_0;
+	else if (idx == 1)
+		rsrc_id = IMX_SC_R_CAN_1;
+	else
+		rsrc_id = IMX_SC_R_CAN_2;
+
+	val = enabled ? 1 : 0;
+	/* stop mode request */
+	imx_sc_misc_set_control(priv->sc_ipc_handle, rsrc_id, IMX_SC_C_IPG_STOP, val);
+}
+
 static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
 {
 	struct flexcan_regs __iomem *regs = priv->regs;
@@ -555,9 +582,12 @@ static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
 	reg_mcr |= FLEXCAN_MCR_SLF_WAK;
 	priv->write(reg_mcr, &regs->mcr);
 
-	/* enable stop request */
-	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
-			   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
+	 /* enable stop request */
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
+		flexcan_stop_mode_enable_scfw(priv, true);
+	else
+		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+				   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
 
 	return flexcan_low_power_enter_ack(priv);
 }
@@ -568,8 +598,11 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
 	u32 reg_mcr;
 
 	/* remove stop request */
-	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
-			   1 << priv->stm.req_bit, 0);
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
+		flexcan_stop_mode_enable_scfw(priv, false);
+	else
+		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+				   1 << priv->stm.req_bit, 0);
 
 	reg_mcr = priv->read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
@@ -1927,11 +1960,6 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
 		gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_bit,
 		priv->stm.ack_gpr, priv->stm.ack_bit);
 
-	device_set_wakeup_capable(&pdev->dev, true);
-
-	if (of_property_read_bool(np, "wakeup-source"))
-		device_set_wakeup_enable(&pdev->dev, true);
-
 	return 0;
 
 out_put_node:
@@ -1939,6 +1967,23 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
 	return ret;
 }
 
+static int flexcan_setup_stop_mode_scfw(struct platform_device *pdev)
+{
+	struct net_device *dev = platform_get_drvdata(pdev);
+	struct flexcan_priv *priv;
+	int ret;
+
+	priv = netdev_priv(dev);
+
+	ret = imx_scu_get_handle(&priv->sc_ipc_handle);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "get ipc handle used by SCU failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static const struct of_device_id flexcan_of_match[] = {
 	{ .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },
 	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
@@ -2088,9 +2133,19 @@ static int flexcan_probe(struct platform_device *pdev)
 	devm_can_led_init(dev);
 
 	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
-		err = flexcan_setup_stop_mode(pdev);
-		if (err)
+		if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
+			err = flexcan_setup_stop_mode_scfw(pdev);
+		else
+			err = flexcan_setup_stop_mode(pdev);
+
+		if (err) {
 			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
+		} else {
+			device_set_wakeup_capable(&pdev->dev, true);
+
+			if (of_property_read_bool(pdev->dev.of_node, "wakeup-source"))
+				device_set_wakeup_enable(&pdev->dev, true);
+		}
 	}
 
 	return 0;
-- 
2.17.1

Re: [PATCH linux-can-next/flexcan 3/4] can: flexcan: add CAN wakeup function for i.MX8

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1049 bytes --]

On 9/25/20 5:10 PM, Joakim Zhang wrote:
> The System Controller Firmware (SCFW) is a low-level system function
> which runs on a dedicated Cortex-M core to provide power, clock, and
> resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> (QM, QP), and i.MX8QX (QXP, DX).
> 
> SCU driver manages the IPC interface between host CPU and the
> SCU firmware running on M4.
> 
> For i.MX8, stop mode request is controlled by System Controller Unit(SCU)
> firmware.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---

Just for reference, this fails to build with:

ERROR: modpost: "imx_scu_get_handle" [drivers/net/can/flexcan.ko] undefined!
ERROR: modpost: "imx_sc_misc_set_control" [drivers/net/can/flexcan.ko] undefined!

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

RE: [PATCH linux-can-next/flexcan 3/4] can: flexcan: add CAN wakeup function for i.MX8

[Joakim Zhang]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 17:44
> 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 linux-can-next/flexcan 3/4] can: flexcan: add CAN wakeup
> function for i.MX8
> 
> On 9/25/20 5:10 PM, Joakim Zhang wrote:
> > The System Controller Firmware (SCFW) is a low-level system function
> > which runs on a dedicated Cortex-M core to provide power, clock, and
> > resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> > (QM, QP), and i.MX8QX (QXP, DX).
> >
> > SCU driver manages the IPC interface between host CPU and the SCU
> > firmware running on M4.
> >
> > For i.MX8, stop mode request is controlled by System Controller
> > Unit(SCU) firmware.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> 
> Just for reference, this fails to build with:
> 
> ERROR: modpost: "imx_scu_get_handle" [drivers/net/can/flexcan.ko]
> undefined!
> ERROR: modpost: "imx_sc_misc_set_control" [drivers/net/can/flexcan.ko]
> undefined!

Yes, I build at my side, due to scu symbols have not export on non-scu systems.

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 |

Re: [PATCH linux-can-next/flexcan 3/4] can: flexcan: add CAN wakeup function for i.MX8

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 7372 bytes --]

On 9/25/20 5:10 PM, Joakim Zhang wrote:
> The System Controller Firmware (SCFW) is a low-level system function
> which runs on a dedicated Cortex-M core to provide power, clock, and
> resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> (QM, QP), and i.MX8QX (QXP, DX).
> 
> SCU driver manages the IPC interface between host CPU and the
> SCU firmware running on M4.
> 
> For i.MX8, stop mode request is controlled by System Controller Unit(SCU)
> firmware.

As you mentioned in the other mail, some functions are missing from the
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  drivers/net/can/flexcan.c | 81 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 68 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 8c8753f77764..41b52cb56f93 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -9,6 +9,7 @@
>  //
>  // Based on code originally by Andrey Volkov <avolkov@varma-el.com>
>  
> +#include <dt-bindings/firmware/imx/rsrc.h>
>  #include <linux/bitfield.h>
>  #include <linux/can.h>
>  #include <linux/can/dev.h>
> @@ -17,6 +18,7 @@
>  #include <linux/can/rx-offload.h>
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/firmware/imx/sci.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/mfd/syscon.h>
> @@ -240,6 +242,8 @@
>  #define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)

rename this into "FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR"

>  /* Support CAN-FD mode */
>  #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
> +/* Use System Controller Firmware */
> +#define FLEXCAN_QUIRK_USE_SCFW BIT(10)

...and this into FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW

>  
>  /* Structure of the message buffer */
>  struct flexcan_mb {
> @@ -358,6 +362,9 @@ struct flexcan_priv {
>  	struct regulator *reg_xceiver;
>  	struct flexcan_stop_mode stm;
>  
> +	/* IPC handle when enable stop mode by System Controller firmware(scfw) */
> +	struct imx_sc_ipc *sc_ipc_handle;
> +
>  	/* Read and Write APIs */
>  	u32 (*read)(void __iomem *addr);
>  	void (*write)(u32 val, void __iomem *addr);
> @@ -387,7 +394,8 @@ static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
>  static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
>  	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
>  		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> -		FLEXCAN_QUIRK_SUPPORT_FD,
> +		FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE |
> +		FLEXCAN_QUIRK_USE_SCFW,
>  };
>  
>  static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
> @@ -546,6 +554,25 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable)
>  	priv->write(reg_mcr, &regs->mcr);
>  }
>  
> +static void flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool enabled)
> +{
> +	struct device_node *np = priv->dev->of_node;
> +	u32 rsrc_id, val;
> +	int idx;
> +
> +	idx = of_alias_get_id(np, "can");
> +	if (idx == 0)
> +		rsrc_id = IMX_SC_R_CAN_0;
> +	else if (idx == 1)
> +		rsrc_id = IMX_SC_R_CAN_1;
> +	else
> +		rsrc_id = IMX_SC_R_CAN_2;

This looks too fragile to me. Better add a property to the DT which indicates
the index.

> +
> +	val = enabled ? 1 : 0;

Please use an if() here.

> +	/* stop mode request */
> +	imx_sc_misc_set_control(priv->sc_ipc_handle, rsrc_id, IMX_SC_C_IPG_STOP, val);
> +}
> +
>  static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
>  {
>  	struct flexcan_regs __iomem *regs = priv->regs;
> @@ -555,9 +582,12 @@ static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
>  	reg_mcr |= FLEXCAN_MCR_SLF_WAK;
>  	priv->write(reg_mcr, &regs->mcr);
>  
> -	/* enable stop request */
> -	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> -			   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
> +	 /* enable stop request */
> +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
> +		flexcan_stop_mode_enable_scfw(priv, true);

error handling?

> +	else
> +		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +				   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
>  
>  	return flexcan_low_power_enter_ack(priv);
>  }
> @@ -568,8 +598,11 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
>  	u32 reg_mcr;
>  
>  	/* remove stop request */
> -	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> -			   1 << priv->stm.req_bit, 0);
> +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
> +		flexcan_stop_mode_enable_scfw(priv, false);

error handling?

> +	else
> +		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +				   1 << priv->stm.req_bit, 0);
>  
>  	reg_mcr = priv->read(&regs->mcr);
>  	reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
> @@ -1927,11 +1960,6 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
>  		gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_bit,
>  		priv->stm.ack_gpr, priv->stm.ack_bit);
>  
> -	device_set_wakeup_capable(&pdev->dev, true);
> -
> -	if (of_property_read_bool(np, "wakeup-source"))
> -		device_set_wakeup_enable(&pdev->dev, true);
> -
>  	return 0;
>  
>  out_put_node:
> @@ -1939,6 +1967,23 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +static int flexcan_setup_stop_mode_scfw(struct platform_device *pdev)
> +{
> +	struct net_device *dev = platform_get_drvdata(pdev);
> +	struct flexcan_priv *priv;
> +	int ret;
> +
> +	priv = netdev_priv(dev);
> +
> +	ret = imx_scu_get_handle(&priv->sc_ipc_handle);

this function can return -EPROBE_DEFER

https://elixir.bootlin.com/linux/latest/source/drivers/firmware/imx/imx-scu.c#L97

> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "get ipc handle used by SCU failed\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct of_device_id flexcan_of_match[] = {
>  	{ .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },
>  	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
> @@ -2088,9 +2133,19 @@ static int flexcan_probe(struct platform_device *pdev)
>  	devm_can_led_init(dev);
>  
>  	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
> -		err = flexcan_setup_stop_mode(pdev);

what about renaming the flexcan_setup_stop_mode() to
flexcan_setup_stop_mode_gpr() and moving this below into a function called
flexcan_setup_stop_mode().

> -		if (err)
> +		if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
> +			err = flexcan_setup_stop_mode_scfw(pdev);
> +		else
> +			err = flexcan_setup_stop_mode(pdev);
> +
> +		if (err) {
>  			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
> +		} else {
> +			device_set_wakeup_capable(&pdev->dev, true);
> +
> +			if (of_property_read_bool(pdev->dev.of_node, "wakeup-source"))
> +				device_set_wakeup_enable(&pdev->dev, true);
> +		}
>  	}
>  
>  	return 0;
> 

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

[PATCH linux-can-next/flexcan 4/4] can: flexcan: disable runtime PM if register flexcandev failed

[Joakim Zhang]

Disable runtime PM if register flexcandev failed, and balance reference
of usage_count.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 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 41b52cb56f93..dd9845146982 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -2151,6 +2151,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