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

On 09/22/2011 06:26 PM, Oliver Hartkopp wrote:

On 09/22/11 15:07, Wolfgang Grandegger wrote:

quoted

Hi Oliver,

On 09/22/2011 12:28 PM, Oliver Hartkopp wrote:

quoted

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.

(..)

quoted

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.

quoted

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.