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

[Marc Kleine-Budde]

On 1/31/19 9:48 AM, Joakim Zhang wrote:
>> 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.

ok

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

If you double to FLEXCAN_TIMEOUT_US to 100, the loops in question will
spin at maximum the double time. But the loops are left as soon as the
condition is satisfied.

It will fix your problem with the 10 kbit/s bitrate. But if there is
some kind of problem with the IP core it will still fail, it just takes
double amount of time (100 Âs + overhead) until the function returns.

I don't see any harm in looping longer:
- The previous good case is unchanged.
- The error case takes double amount of time.
- Your problem is hopefully 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: signature.asc
Description: OpenPGP digital signature