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

[Marc Kleine-Budde <mkl@pengutronix.de>]

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