RFC [PATCH v2 1/2] can: flexcan: fix mailbox initialization

RFC [PATCH v2 1/2] can: flexcan: fix mailbox initialization

[Marc Kleine-Budde]

Picking up David's patch....

RFC: [PATCH v2 1/2] can: flexcan: Correctly initialize mailboxes

[Marc Kleine-Budde]

From: David Jander <david@protonic.nl>

Apparently mailboxes may contain random data at startup, causing some of
them being prepared for message reception. This causes overruns being
missed or even confusing the IRQ check for trasmitted messages, increasing
the transmit counter instead of the error counter.

Signed-off-by: David Jander <david@protonic.nl>
[mkl: adjust starting value of loop]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Changes since v1:
- don't remove existing initialization of FLEXCAN_TX_BUF_ID
- start loop at FLEXCAN_TX_BUF_ID + 1

Marc

 drivers/net/can/flexcan.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 630c7bf..6ec49bd 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -801,6 +801,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	int err;
 	u32 reg_mcr, reg_ctrl;
+	int i;
 
 	/* enable module */
 	err = flexcan_chip_enable(priv);
@@ -870,6 +871,11 @@ static int flexcan_chip_start(struct net_device *dev)
 	/* Abort any pending TX, mark Mailbox as INACTIVE */
 	flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
 		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
+	/* Clear and invalidate all mailboxes first */
+	for (i = FLEXCAN_TX_BUF_ID + 1; i < ARRAY_SIZE(regs->cantxfg; i++) {
+		flexcan_write(FLEXCAN_MB_CNT_CODE(0),
+		      &regs->cantxfg[i].can_ctrl);
+	}
 
 	/* acceptance mask/acceptance code (accept everything) */
 	flexcan_write(0x0, &regs->rxgmask);
-- 
2.1.0

RFC: [PATCH v2 2/2] can: flexcan: increase FLEXCAN_MCR_MAXMB() macro to 7 bits

[Marc Kleine-Budde]

This patch increases the mask in the FLEXCAN_MCR_MAXMB() to 7 bits as in the
newer flexcan cores the MAXMB field is 7 bits wide.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---

Changes since v1:
- new

 drivers/net/can/flexcan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 6ec49bd..44ba836 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -62,7 +62,7 @@
 #define FLEXCAN_MCR_BCC			BIT(16)
 #define FLEXCAN_MCR_LPRIO_EN		BIT(13)
 #define FLEXCAN_MCR_AEN			BIT(12)
-#define FLEXCAN_MCR_MAXMB(x)		((x) & 0x1f)
+#define FLEXCAN_MCR_MAXMB(x)		((x) & 0x7f)
 #define FLEXCAN_MCR_IDAM_A		(0 << 8)
 #define FLEXCAN_MCR_IDAM_B		(1 << 8)
 #define FLEXCAN_MCR_IDAM_C		(2 << 8)
-- 
2.1.0

Re: RFC: [PATCH v2 1/2] can: flexcan: Correctly initialize mailboxes

[David Jander]

On Tue,  2 Sep 2014 17:03:35 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> From: David Jander <david@protonic.nl>
> 
> Apparently mailboxes may contain random data at startup, causing some of
> them being prepared for message reception. This causes overruns being
> missed or even confusing the IRQ check for trasmitted messages, increasing
> the transmit counter instead of the error counter.
> 
> Signed-off-by: David Jander <david@protonic.nl>
> [mkl: adjust starting value of loop]
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> Changes since v1:
> - don't remove existing initialization of FLEXCAN_TX_BUF_ID
> - start loop at FLEXCAN_TX_BUF_ID + 1
> 
> Marc
> 
>  drivers/net/can/flexcan.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 630c7bf..6ec49bd 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -801,6 +801,7 @@ static int flexcan_chip_start(struct net_device *dev)
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	int err;
>  	u32 reg_mcr, reg_ctrl;
> +	int i;
>  
>  	/* enable module */
>  	err = flexcan_chip_enable(priv);
> @@ -870,6 +871,11 @@ static int flexcan_chip_start(struct net_device *dev)
>  	/* Abort any pending TX, mark Mailbox as INACTIVE */
>  	flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
>  		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> +	/* Clear and invalidate all mailboxes first */
> +	for (i = FLEXCAN_TX_BUF_ID + 1; i < ARRAY_SIZE(regs->cantxfg; i++) {
> +		flexcan_write(FLEXCAN_MB_CNT_CODE(0),
> +		      &regs->cantxfg[i].can_ctrl);
> +	}
>  
>  	/* acceptance mask/acceptance code (accept everything) */
>  	flexcan_write(0x0, &regs->rxgmask);

This assumes that FLEXCAN_TX_BUF_ID is the ID of the first available MB. Other
than that, it should work I believe.
OTOH, since this loop is more like a memset(...0...), wouldn't it be nice to
just clear all MB's CODE registers and then initialize TX_BUF (and others)
afterwards?

Best regards,

-- 
David Jander
Protonic Holland.

Re: RFC: [PATCH v2 1/2] can: flexcan: Correctly initialize mailboxes

[Marc Kleine-Budde]

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

On 09/03/2014 08:58 AM, David Jander wrote:
> On Tue,  2 Sep 2014 17:03:35 +0200
> Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> 
>> From: David Jander <david@protonic.nl>
>>
>> Apparently mailboxes may contain random data at startup, causing some of
>> them being prepared for message reception. This causes overruns being
>> missed or even confusing the IRQ check for trasmitted messages, increasing
>> the transmit counter instead of the error counter.
>>
>> Signed-off-by: David Jander <david@protonic.nl>
>> [mkl: adjust starting value of loop]
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>> Changes since v1:
>> - don't remove existing initialization of FLEXCAN_TX_BUF_ID
>> - start loop at FLEXCAN_TX_BUF_ID + 1
>>
>> Marc
>>
>>  drivers/net/can/flexcan.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>> index 630c7bf..6ec49bd 100644
>> --- a/drivers/net/can/flexcan.c
>> +++ b/drivers/net/can/flexcan.c
>> @@ -801,6 +801,7 @@ static int flexcan_chip_start(struct net_device *dev)
>>  	struct flexcan_regs __iomem *regs = priv->base;
>>  	int err;
>>  	u32 reg_mcr, reg_ctrl;
>> +	int i;
>>  
>>  	/* enable module */
>>  	err = flexcan_chip_enable(priv);
>> @@ -870,6 +871,11 @@ static int flexcan_chip_start(struct net_device *dev)
>>  	/* Abort any pending TX, mark Mailbox as INACTIVE */
>>  	flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
>>  		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>> +	/* Clear and invalidate all mailboxes first */
>> +	for (i = FLEXCAN_TX_BUF_ID + 1; i < ARRAY_SIZE(regs->cantxfg; i++) {
>> +		flexcan_write(FLEXCAN_MB_CNT_CODE(0),
>> +		      &regs->cantxfg[i].can_ctrl);
>> +	}
>>  
>>  	/* acceptance mask/acceptance code (accept everything) */
>>  	flexcan_write(0x0, &regs->rxgmask);
> 
> This assumes that FLEXCAN_TX_BUF_ID is the ID of the first available MB. Other
> than that, it should work I believe.
> OTOH, since this loop is more like a memset(...0...), wouldn't it be nice to
> just clear all MB's CODE registers and then initialize TX_BUF (and others)
> afterwards?

See patch v3, I've changed the order, first do the loop, then initialize
FLEXCAN_TX_BUF_ID as 0x4. I've pushed this series to the testing branch
of the linux-can repo on gitorious. Make your imx6 errata fix patch
based on this branch.

You probably have to change FLEXCAN_TX_BUF_ID and/or introduce a new
define for the invalid mailbox. When your patch is ready we can look at
the patches and see if it makes sense to shuffle some changes around.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: RFC: [PATCH v2 1/2] can: flexcan: Correctly initialize mailboxes

[David Jander]

On Tue,  2 Sep 2014 17:03:35 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> From: David Jander <david@protonic.nl>
> 
> Apparently mailboxes may contain random data at startup, causing some of
> them being prepared for message reception. This causes overruns being
> missed or even confusing the IRQ check for trasmitted messages, increasing
> the transmit counter instead of the error counter.
> 
> Signed-off-by: David Jander <david@protonic.nl>
> [mkl: adjust starting value of loop]
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> Changes since v1:
> - don't remove existing initialization of FLEXCAN_TX_BUF_ID
> - start loop at FLEXCAN_TX_BUF_ID + 1
> 
> Marc
> 
>  drivers/net/can/flexcan.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 630c7bf..6ec49bd 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -801,6 +801,7 @@ static int flexcan_chip_start(struct net_device *dev)
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	int err;
>  	u32 reg_mcr, reg_ctrl;
> +	int i;
>  
>  	/* enable module */
>  	err = flexcan_chip_enable(priv);
> @@ -870,6 +871,11 @@ static int flexcan_chip_start(struct net_device *dev)
>  	/* Abort any pending TX, mark Mailbox as INACTIVE */
>  	flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
>  		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> +	/* Clear and invalidate all mailboxes first */
> +	for (i = FLEXCAN_TX_BUF_ID + 1; i < ARRAY_SIZE(regs->cantxfg; i++) {

Oops! You are missing a closing parenthesis in this line.
I will assume you amend this commit and base mine on top of that...

> +		flexcan_write(FLEXCAN_MB_CNT_CODE(0),
> +		      &regs->cantxfg[i].can_ctrl);
> +	}
>  
>  	/* acceptance mask/acceptance code (accept everything) */
>  	flexcan_write(0x0, &regs->rxgmask);

Best regards,

-- 
David Jander
Protonic Holland.

Re: RFC: [PATCH v2 1/2] can: flexcan: Correctly initialize mailboxes

[Marc Kleine-Budde]

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

On 09/03/2014 04:34 PM, David Jander wrote:
> On Tue,  2 Sep 2014 17:03:35 +0200
> Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> 
>> From: David Jander <david@protonic.nl>
>>
>> Apparently mailboxes may contain random data at startup, causing some of
>> them being prepared for message reception. This causes overruns being
>> missed or even confusing the IRQ check for trasmitted messages, increasing
>> the transmit counter instead of the error counter.
>>
>> Signed-off-by: David Jander <david@protonic.nl>
>> [mkl: adjust starting value of loop]
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>> Changes since v1:
>> - don't remove existing initialization of FLEXCAN_TX_BUF_ID
>> - start loop at FLEXCAN_TX_BUF_ID + 1
>>
>> Marc
>>
>>  drivers/net/can/flexcan.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>> index 630c7bf..6ec49bd 100644
>> --- a/drivers/net/can/flexcan.c
>> +++ b/drivers/net/can/flexcan.c
>> @@ -801,6 +801,7 @@ static int flexcan_chip_start(struct net_device *dev)
>>  	struct flexcan_regs __iomem *regs = priv->base;
>>  	int err;
>>  	u32 reg_mcr, reg_ctrl;
>> +	int i;
>>  
>>  	/* enable module */
>>  	err = flexcan_chip_enable(priv);
>> @@ -870,6 +871,11 @@ static int flexcan_chip_start(struct net_device *dev)
>>  	/* Abort any pending TX, mark Mailbox as INACTIVE */
>>  	flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
>>  		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>> +	/* Clear and invalidate all mailboxes first */
>> +	for (i = FLEXCAN_TX_BUF_ID + 1; i < ARRAY_SIZE(regs->cantxfg; i++) {
> 
> Oops! You are missing a closing parenthesis in this line.
> I will assume you amend this commit and base mine on top of that...

Doh! I introduced this problem, when converting the 64 to
ARRAY_SIZE(...without the closing ).

Fixed.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]