RE: [PATCH] can: flexcan: fix timeout when set small bitrate

[Joakim Zhang]

Hi Marc,

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> Sent: 2019å1æ31æ 15:34
> To: Joakim Zhang <qiangqing.zhang@xxxxxxx>; linux-can@xxxxxxxxxxxxxxx
> Cc: wg@xxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; Aisheng
> Dong <aisheng.dong@xxxxxxx>
> Subject: Re: [PATCH] can: flexcan: fix timeout when set small bitrate
> 
> On 1/31/19 7:58 AM, Joakim Zhang wrote:
> > From: Dong Aisheng <aisheng.dong@xxxxxxx>
> >
> > Current we can meet timeout issue when setting a small bitrate like
> > 10000 as follows:
> > root@imx6qdlsolo:~# ip link set can0 up type can bitrate 10000 A link
> > change request failed with some changes committed already.
> > Interface can0 may have been left with an inconsistent configuration,
> > please check.
> > RTNETLINK answers: Connection timed out
> 
> Which SoC are you using? Which clock rate has the flexcan IP core?

  We tested on i.MX6 series boards and all met this issue. And ipg clock rate is 66MHZ, per clock rate is 30MHZ.

> > It is caused by calling of flexcan_chip_unfreeze() timeout.
> >
> > Originally the code is using usleep_range(10, 20) for unfreeze
> > operation, but the patch (8badd65 can: flexcan: avoid calling
> > usleep_range from interrupt context) changed it into udelay(10) which
> > is only a half delay of before, there're also some other delay changes.
> >
> > After only changed unfreeze delay back to udelay(20), the issue is gone.
> > So other timeout values are kept the same as 8badd65 changed.
> 
> Can you change FLEXCAN_TIMEOUT_US instead?

  Of course, we can change FLEXCAN_TIMEOUT_US to 100, but this will extend the time of enable/disable/softreset.
Which method do you think is better?

Best Regards,
Joakim Zhang

> > Signed-off-by: Dong Aisheng <aisheng.dong@xxxxxxx>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@xxxxxxx>
> > ---
> >  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 2bca867bcfaa..1d3a9053bbeb 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -530,7 +530,7 @@ static int flexcan_chip_unfreeze(struct flexcan_priv
> *priv)
> >  	priv->write(reg, &regs->mcr);
> >
> >  	while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> > -		udelay(10);
> > +		udelay(20);
> >
> >  	if (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
> >  		return -ETIMEDOUT;
> >
> 
> 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   |