Re: [PATCH] can: flexcan: bump FLEXCAN_TIMEOUT_US to 250

[Rasmus Villemoes]

On 21/03/2019 16.30, Marc Kleine-Budde wrote:
> On 3/21/19 3:34 PM, Rasmus Villemoes wrote:
>> On 20/03/2019 14.18, Marc Kleine-Budde wrote:
>>> On 3/7/19 4:00 PM, Rasmus Villemoes wrote:
>>>> While trying to add support for the Flexcan modules on the MPC8309,
>>>> I'm hitting ETIMEDOUT in flexcan_chip_disable(). With this, probing
>>>> succeeds. Checking the leftover value of timeout with a primitive
>>>>
>>>>   pr_err("%s: timeout==%d\n", __func__, timeout);
>>>>
>>>> after the loop in chip_disable() typically shows values around 12-14,
>>>> i.e. suggesting that it takes about 110-130 us for the LPM_ACK bit to
>>>> appear. So a timeout value of about twice that seems reasonable.
>>>>
>>>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx>
>>>
>>> I've scheduled the patch by Joakim Zhang:
>>>
>>>     9daed89ae8a3 can: flexcan: fix timeout when set small bitrate
>>>
>>> that doubles the timeout to 100.
>>
>> Eh, ok, but that's not sufficient for the MPC8309 (I tried with 100 at
>> first, but as I write the minimally working timeout value turns out to
>> be about 140 us). Do you want me to send another patch on top of
>> 9daed89ae8a3, or how should I interpret the above?
> 
> Can you propose an updated commit message for Joakim Zhang's patch?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git/commit/?h=testing&id=9daed89ae8a3fc44ccd2b2bb9c3c4d3e3431904d
> 
> I'll add it by hand and increase the timeout to 250. Ok?

Sounds fine. In order not to change Joakim's commit log too much, how
about adding something like (with your initials, since you're next in
the sign-off chain)

[mkl: Meanwhile, Rasmus Villemoes reported that even with a timeout of
100, flexcan_probe() fails on the MPC8309, which requires a value of at
least 140 to work reliably. 250 works for everyone.]

? Feel free to edit as you wish.

Thanks,
Rasmus