Re: [PATCH net-next v2] candev: allow SJW user setting for bittiming calculation

[Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>]

On 09/22/2011 06:26 PM, Oliver Hartkopp wrote:
> On 09/22/11 15:07, Wolfgang Grandegger wrote:
> 
>> Hi Oliver,
>>
>> On 09/22/2011 12:28 PM, Oliver Hartkopp wrote:
>>> This patch adds support for SJW user settings to not set the synchronization
>>> jump width (SJW) to 1 in any case when using the in-kernel bittiming
>>> calculation.
>>>
>>> The ip-tool from iproute2 already supports to pass the user defined SJW
>>> value. The given SJW value is sanitized with the controller specific sjw_max
>>> and the calculated tseg2 value. As the SJW can have values up to 4 providing
>>> this value will lead to the maximum possible SJW automatically. A higher SJW
>>> allows higher controller oscillator tolerances.
> 
> (..)
> 
> 
>> As I already said, I expected "bt->sjw" to be always 0 when
>> can_calc_bittiming() is called. But the software somehow allows to
>> preset "bt->sjw", which is not intended as the help of the ip utility shows:
> 
> 
> What was the technical reason for this 'not intended' that time?

Fiddling with SJW is something for *expert* only. You need to know
what's going on the bus (electrically). To be clear, I'm not such an expert.

>> I actually hesitate to extend can_calc_bittiming(). I suggest using an
>> external tool to calculate proper bit-timing parameters and set them
>> with "ip link set DEVICE type can tq ...".
> 
> 
> Yes, i know the possibility to specify the bittiming via time-quanta (tq).

This is what we called the "expert mode".

> But i think the in-kernel bittiming calculation is pretty good and solves most
> usual cases. For me it's just an additional requirement to specify SJW as

What's your reason to increase SJW? Likely because you get bus errors
with the standard settings. How should it be set? Likely you need to
fiddle with other parameters as well. For sure, no business for a normal
CAN user.

> MIN(tseg2, max_sjw) to extend the clock source tolerance - which can be
> provided as an additional option to the bittiming calculation without pain
> (IMO). If i can 'tune' the sample point, why should i not have any influence
> to the synchronization jump width generation then?

Bitrate and sample point is something obvious for normal users.

> And btw. a help text can really be changed easily ;-)
> 
> -        [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] |
> +        [ bitrate BITRATE [ sample-point SAMPLE-POINT] [sjw SJW] ] |

Well, I have stronger arguments against it ;-).

Any other opinions or suggestions?

Wolfgang.