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

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

[Oliver Hartkopp]

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.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

---

v2: Resend due to wrong mail address header encoding.

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 9bf1116..25695bd 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -150,7 +150,19 @@ static int can_calc_bittiming(struct net_device *dev,
struct can_bittiming *bt)
 	bt->prop_seg = tseg1 / 2;
 	bt->phase_seg1 = tseg1 - bt->prop_seg;
 	bt->phase_seg2 = tseg2;
-	bt->sjw = 1;
+
+	/* check for sjw user settings */
+	if (!bt->sjw || !btc->sjw_max)
+		bt->sjw = 1;
+	else {
+		/* bt->sjw is at least 1 -> sanitize upper bound to sjw_max */
+		if (bt->sjw > btc->sjw_max)
+			bt->sjw = btc->sjw_max;
+		/* bt->sjw must not be higher than tseg2 */
+		if (tseg2 < bt->sjw)
+			bt->sjw = tseg2;
+	}
+
 	bt->brp = best_brp;
 	/* real bit-rate */
 	bt->bitrate = priv->clock.freq / (bt->brp * (tseg1 + tseg2 + 1));

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

[Wolfgang Grandegger]

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.
> 
> Signed-off-by: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
> 
> ---
> 
> v2: Resend due to wrong mail address header encoding.
> 
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 9bf1116..25695bd 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -150,7 +150,19 @@ static int can_calc_bittiming(struct net_device *dev,
> struct can_bittiming *bt)
>  	bt->prop_seg = tseg1 / 2;
>  	bt->phase_seg1 = tseg1 - bt->prop_seg;
>  	bt->phase_seg2 = tseg2;
> -	bt->sjw = 1;
> +
> +	/* check for sjw user settings */
> +	if (!bt->sjw || !btc->sjw_max)
> +		bt->sjw = 1;
> +	else {
> +		/* bt->sjw is at least 1 -> sanitize upper bound to sjw_max */
> +		if (bt->sjw > btc->sjw_max)
> +			bt->sjw = btc->sjw_max;
> +		/* bt->sjw must not be higher than tseg2 */
> +		if (tseg2 < bt->sjw)
> +			bt->sjw = tseg2;
> +	}
> +

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:

 $ ip link set can0 type can help
   Usage: ip link set DEVICE type can
       [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] |
       [ tq TQ prop-seg PROP_SEG phase-seg1 PHASE-SEG1
         phase-seg2 PHASE-SEG2 [ sjw SJW ] ]

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 ...".

See also:

http://lxr.linux.no/#linux+v3.0.4/Documentation/networking/can.txt#L742

Wolfgang.

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

[Oliver Hartkopp]

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?

> 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).

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

And btw. a help text can really be changed easily ;-)

-        [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] |
+        [ bitrate BITRATE [ sample-point SAMPLE-POINT] [sjw SJW] ] |

Regards,
Oliver

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

[Wolfgang Grandegger]

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.

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

[Oliver Hartkopp]

Hi Wolfgang,

i put Pavel and Andrey into CC, maybe they also have an opinion about this.
I just talked to a specialist who told me, that a higher SJW is better - if
possible. And if an expert told me so and i had a simple config interface to
fullfill it, it would be great for me ;-)

Here is a link to the patch and this discussion:

http://patchwork.ozlabs.org/patch/115932/

Regards,
Oliver

On 09/22/11 20:10, Wolfgang Grandegger wrote:

> 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.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Wolfgang Grandegger]

Hi Oliver,

On 09/22/2011 09:41 PM, Oliver Hartkopp wrote:
> Hi Wolfgang,
> 
> i put Pavel and Andrey into CC, maybe they also have an opinion about this.
> I just talked to a specialist who told me, that a higher SJW is better - if

Then let us set it to 4 (maximum), by default. But other documents
recommend a value of 1.

> possible. And if an expert told me so and i had a simple config interface to
> fullfill it, it would be great for me ;-)

Anyway, if SJW does not depend on other bit-timing parameters and it
usually works fine with the kernel calculated parameters I would no
longer vote against this patch.

Wolfgang,

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

[Pavel Pisa]

Hello Oliver and Wolfgang,

On Friday 23 September 2011 09:24:20 Wolfgang Grandegger wrote:
> Hi Oliver,
>
> On 09/22/2011 09:41 PM, Oliver Hartkopp wrote:
> Then let us set it to 4 (maximum), by default. But other documents
> recommend a value of 1.

I am not expert for CAN timing nor I have time to go through
documentation at this moment. But I hope that I have some
sense/experience  for electronic and dynamic systems
and we have done more designs utilizing CAN at university
and at our company.

So there are some thought about SJW based on my experience.
There could be some inaccuracies, if you want better
analysis, I need time for that.

1) The CAN needs fast local roundrip time for correct operation
of dominant/recessive arbitration.
The time is sum of propagation from CAN controller chip,
propagation delay of optocoupler or other galvanic isolation,
slew rate and delay in Tx circuitry of transceiver, charging
the wire, Rx part of receiver, optocoupler, controller
Rx filtering and Rx,Tx logic level comparator.
Iw we count necessity to synchronize this between multiple
CAN nodes then whole roundtrip time has to be smaller than
50-80% of single bit bit time quantum.

2) From the noise and stabilization of voltage on the wire
the sampling point should be in the middle of bit pulse
delayed by round-trip delay from 1. This is 50% of bit
time (i.e. dependent on bitrate) + round trip delay (dependent
on delay in controller and concrete board circuitry).

3) But sampling point has to allow to decide about collision
before next bit Tx is started => it cannot be moved after
end of the given Tx bit time interval.

4) It is necessary to synchronize bit timing in all
CAN controllers during arbitration (ID sending) phase.
The first sync is done after receiving of the first
edge on the wire. It can be other node start of Tx
or our own Tx dominant level. Controller shifts its Tx
timing such way, that start of the next Tx bit interval
should result in Rx transition at the end the sensed
Rx bit. I.e. it counts only TSEG2 til next bit start.

5) There are some more things to consider if triple
sampling and filtering is enabled to suppress noise
on wire. It can be used only, if time quantum clocks
are fast enough to fit this sampling interval between
stabilization of delayed Rx logic level and end of Tx bit
time interval.

6) If the clock frequencies of all nodes CAN controllers
are guaranteed to be same, then no more synchronization
is necessary during message Tx/Rx (SJW=0). But that assumption
does not hold. That is why bitstuffing is used and guarantees
that there is at least one logic level transition (edge)
after each 6 bit time. If there is zero roundtrip propagation
delay and sampling in 50% of bit time interval then
maximal skew/frequency difference of the clocks could be

  (1+1/6*50%) - 1 i.e. 8%

This means, that SJW bigger than 8% of whole bit time
would not help to synchronize bitrate difference, because
for such case setup cannot work anyway. Propagation delay
is not zero then there is even less time left for sampling
point shift which would not cause incorrect bit data
detection so reasonable maximum is probably lower.

I expect that for reasonable precise setup of bitrate
when exact ratio is found and crystal based oscillators
there is the best option small SJW i.e. 1 or for very
fast TQ clock equivalent of 1% - 2% of bittime.
For nonexact ratio or low quality clocks sources,
bigger SJW values make sense. But big value has other
disadvantage. If there is bigger jitter in Tx/Rx delays
or clocks then it is propagated back to bit timing
and stability of system composed from multiple nodes
could be questionable. There is even bigger interval
where noise pulse could cause lost of the synchronization.

On base of above analysis, I think that blindly set SJW
on maximum is not good idea. It should be at least limited
to 5% of bit time. 

Because bit timing calculation is not what everybody
want to do again and again, the actual code tries
to hide differences of CAN controllers and provide
at least partially understandable knobs to user
with parameters independent on concrete setup low level
details to allow set bitrate for usual Joe user.
The basic parameters are chosen such way, that user need
not to care about actual TQ clock and selecting prescaller,
that is why sampling point is in percent of bittime.
SJW is more problematic, but may it be use of 2 or 5%
of bittime by default with assurance that zero is
replaced by one, would serve to most people pleasure.
May it be, that 0.1 of percent should be used as unit
for external parameter too.

I hope that actual calculation works reasonably well.
But if it should be enhanced, then it would worth
to add additional parameter except crystal/controller
clock source freqency to the concrete boards drivers.
It should be measured round-trip delay of given/used
transceiver/optocouplers combination. This would
allow to have sampling point setup yet more independent
on given HW and same value could be used for different
bitrates. But there is still unknown parameter
capacity/length of connected wires so there is still
something left to user consideration.

Best wishes,


                Pavel Pisa
    e-mail:     pisa-/N2ztlQkxE7Ub/6JBqosbQ@public.gmane.org
    www:        http://cmp.felk.cvut.cz/~pisa
    university: http://dce.fel.cvut.cz/
    company:    http://www.pikron.com/


> > possible. And if an expert told me so and i had a simple config interface
> > to fullfill it, it would be great for me ;-)
>
> Anyway, if SJW does not depend on other bit-timing parameters and it
> usually works fine with the kernel calculated parameters I would no
> longer vote against this patch.
>
> Wolfgang,

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

[Oliver Hartkopp]

Hello Pavel, hello Wolfgang,

thanks for the detailed feedback!

Trying to summarize the text for me, i see these relevant topics (in my
probably limited view):

On 09/23/11 11:32, Pavel Pisa wrote:

> On Friday 23 September 2011 09:24:20 Wolfgang Grandegger wrote:
>> Then let us set it to 4 (maximum), by default. But other documents
>> recommend a value of 1.


Putting the SJW to it's max value by default - which is then reduced by tseg2
is obviously bad (see below). SJW = 1 should stay the default value.

(..)

> 6) If the clock frequencies of all nodes CAN controllers

> are guaranteed to be same, then no more synchronization
> is necessary during message Tx/Rx (SJW=0). But that assumption
> does not hold. That is why bitstuffing is used and guarantees
> that there is at least one logic level transition (edge)
> after each 6 bit time.


Ah - i see.

(..)


> I expect that for reasonable precise setup of bitrate
> when exact ratio is found and crystal based oscillators
> there is the best option small SJW i.e. 1 or for very
> fast TQ clock equivalent of 1% - 2% of bittime.


Which already was the default.

> For nonexact ratio or low quality clocks sources,
> bigger SJW values make sense. But big value has other
> disadvantage. If there is bigger jitter in Tx/Rx delays
> or clocks then it is propagated back to bit timing
> and stability of system composed from multiple nodes
> could be questionable. There is even bigger interval
> where noise pulse could cause lost of the synchronization.
> 
> On base of above analysis, I think that blindly set SJW
> on maximum is not good idea. It should be at least limited
> to 5% of bit time.


Ok.

 
> Because bit timing calculation is not what everybody
> want to do again and again, the actual code tries
> to hide differences of CAN controllers and provide
> at least partially understandable knobs to user
> with parameters independent on concrete setup low level
> details to allow set bitrate for usual Joe user.


Yes - like me ;-)

> The basic parameters are chosen such way, that user need
> not to care about actual TQ clock and selecting prescaller,
> that is why sampling point is in percent of bittime.


Btw. defining a sampling point other than CAN CIA recommendations could be
seen as a user specific requirement that is nothing for user Joe too.


> SJW is more problematic, but may it be use of 2 or 5%
> of bittime by default with assurance that zero is
> replaced by one, would serve to most people pleasure.
> May it be, that 0.1 of percent should be used as unit
> for external parameter too.


And then you would like to calculate the SJW based on this percent value? I'm
a bit concerned about this additional complexity (see below).

> I hope that actual calculation works reasonably well.

Yes, it does.

> But if it should be enhanced, then it would worth

> to add additional parameter except crystal/controller
> clock source freqency to the concrete boards drivers.
> It should be measured round-trip delay of given/used
> transceiver/optocouplers combination. This would
> allow to have sampling point setup yet more independent
> on given HW and same value could be used for different
> bitrates. But there is still unknown parameter
> capacity/length of connected wires so there is still
> something left to user consideration.


As i remember from the discussions for the existing algorithm the complexity
is already high enough, so that adding these new details may lead to new
problems (in kernel space), we do not see so far. I think, if someone want's
to go THAT deep the already existing 'expert mode' with the time-quanta based
setting of the bitrate is the right choice.

The in-kernel bittiming calculation is the best approach for user Joe as you
already pointed out. Besides the sampling point (which has a impact on the
bittiming calculation) the tuning of the SJW value would be a second feature
for 'advanced' users that can be modified from the outside.

As we still preserve the reasonable default of SJW=1, adding a new SJW option
to tune just only the resulting SJW value looks risk-free to me.

(..)

>> Anyway, if SJW does not depend on other bit-timing parameters and it
>> usually works fine with the kernel calculated parameters I would no
>> longer vote against this patch.


That's nice. Your Acked-by would be great.

I would provide a patch for the 'ip' tools help text then. Fortunately there
is no code change necessary in iplink_can.c and this functionality can be used
with all iproute2 versions if the kernel supports it. And if not, it's really
easy to patch ;-)

Many thanks,
Oliver

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

[Wolfgang Grandegger]

Hi Pavel,

On 09/23/2011 11:32 AM, Pavel Pisa wrote:
> Hello Oliver and Wolfgang,
> 
> On Friday 23 September 2011 09:24:20 Wolfgang Grandegger wrote:
>> Hi Oliver,
>>
>> On 09/22/2011 09:41 PM, Oliver Hartkopp wrote:
>> Then let us set it to 4 (maximum), by default. But other documents
>> recommend a value of 1.
> 
> I am not expert for CAN timing nor I have time to go through
> documentation at this moment. But I hope that I have some
> sense/experience  for electronic and dynamic systems
> and we have done more designs utilizing CAN at university
> and at our company.
> 
> So there are some thought about SJW based on my experience.
> There could be some inaccuracies, if you want better
> analysis, I need time for that.
> 
> 1) The CAN needs fast local roundrip time for correct operation
> of dominant/recessive arbitration.
> The time is sum of propagation from CAN controller chip,
> propagation delay of optocoupler or other galvanic isolation,
> slew rate and delay in Tx circuitry of transceiver, charging
> the wire, Rx part of receiver, optocoupler, controller
> Rx filtering and Rx,Tx logic level comparator.
> Iw we count necessity to synchronize this between multiple
> CAN nodes then whole roundtrip time has to be smaller than
> 50-80% of single bit bit time quantum.
> 
> 2) From the noise and stabilization of voltage on the wire
> the sampling point should be in the middle of bit pulse
> delayed by round-trip delay from 1. This is 50% of bit
> time (i.e. dependent on bitrate) + round trip delay (dependent
> on delay in controller and concrete board circuitry).
> 
> 3) But sampling point has to allow to decide about collision
> before next bit Tx is started => it cannot be moved after
> end of the given Tx bit time interval.
> 
> 4) It is necessary to synchronize bit timing in all
> CAN controllers during arbitration (ID sending) phase.
> The first sync is done after receiving of the first
> edge on the wire. It can be other node start of Tx
> or our own Tx dominant level. Controller shifts its Tx
> timing such way, that start of the next Tx bit interval
> should result in Rx transition at the end the sensed
> Rx bit. I.e. it counts only TSEG2 til next bit start.
> 
> 5) There are some more things to consider if triple
> sampling and filtering is enabled to suppress noise
> on wire. It can be used only, if time quantum clocks
> are fast enough to fit this sampling interval between
> stabilization of delayed Rx logic level and end of Tx bit
> time interval.
> 
> 6) If the clock frequencies of all nodes CAN controllers
> are guaranteed to be same, then no more synchronization
> is necessary during message Tx/Rx (SJW=0). But that assumption
> does not hold. That is why bitstuffing is used and guarantees
> that there is at least one logic level transition (edge)
> after each 6 bit time. If there is zero roundtrip propagation
> delay and sampling in 50% of bit time interval then
> maximal skew/frequency difference of the clocks could be
> 
>   (1+1/6*50%) - 1 i.e. 8%
> 
> This means, that SJW bigger than 8% of whole bit time
> would not help to synchronize bitrate difference, because
> for such case setup cannot work anyway. Propagation delay
> is not zero then there is even less time left for sampling
> point shift which would not cause incorrect bit data
> detection so reasonable maximum is probably lower.
> 
> I expect that for reasonable precise setup of bitrate
> when exact ratio is found and crystal based oscillators
> there is the best option small SJW i.e. 1 or for very
> fast TQ clock equivalent of 1% - 2% of bittime.
> For nonexact ratio or low quality clocks sources,
> bigger SJW values make sense. But big value has other
> disadvantage. If there is bigger jitter in Tx/Rx delays
> or clocks then it is propagated back to bit timing
> and stability of system composed from multiple nodes
> could be questionable. There is even bigger interval
> where noise pulse could cause lost of the synchronization.
> 
> On base of above analysis, I think that blindly set SJW
> on maximum is not good idea. It should be at least limited
> to 5% of bit time. 
> 
> Because bit timing calculation is not what everybody
> want to do again and again, the actual code tries
> to hide differences of CAN controllers and provide
> at least partially understandable knobs to user
> with parameters independent on concrete setup low level
> details to allow set bitrate for usual Joe user.
> The basic parameters are chosen such way, that user need
> not to care about actual TQ clock and selecting prescaller,
> that is why sampling point is in percent of bittime.
> SJW is more problematic, but may it be use of 2 or 5%
> of bittime by default with assurance that zero is
> replaced by one, would serve to most people pleasure.
> May it be, that 0.1 of percent should be used as unit
> for external parameter too.
> 
> I hope that actual calculation works reasonably well.
> But if it should be enhanced, then it would worth
> to add additional parameter except crystal/controller
> clock source freqency to the concrete boards drivers.
> It should be measured round-trip delay of given/used
> transceiver/optocouplers combination. This would
> allow to have sampling point setup yet more independent
> on given HW and same value could be used for different
> bitrates. But there is still unknown parameter
> capacity/length of connected wires so there is still
> something left to user consideration.

Thanks for your detailed explanation. It clearly shows that adjusting
SJW is non-trivial and nothing a normal CAN user should deal with. When
adjusting SJW, do you also need to tweek other bit-timing parameters,
e.g. tq? I mean, would "ip link set can0 type can bitrate x
sampling-point y sjw z" work for your setup or do you need to use the
expert mode setting via "ip link set can0 type can tq ..." anyway?

Wolfgang.

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

[Oliver Hartkopp]

On 09/24/11 09:22, Wolfgang Grandegger wrote:

> On 09/23/2011 11:32 AM, Pavel Pisa wrote:

>> On base of above analysis, I think that blindly set SJW
>> on maximum is not good idea. It should be at least limited
>> to 5% of bit time. 


(..)


>> SJW is more problematic, but may it be use of 2 or 5%
>> of bittime by default with assurance that zero is
>> replaced by one, would serve to most people pleasure.


(..)

>> But there is still unknown parameter
>> capacity/length of connected wires so there is still
>> something left to user consideration.
> 
> Thanks for your detailed explanation. It clearly shows that adjusting
> SJW is non-trivial and nothing a normal CAN user should deal with. When
> adjusting SJW, do you also need to tweek other bit-timing parameters,
> e.g. tq? I mean, would "ip link set can0 type can bitrate x
> sampling-point y sjw z" work for your setup or do you need to use the
> expert mode setting via "ip link set can0 type can tq ..." anyway?


Hello Wolfgang,

i double checked a specification where i got my requirement to influence the
SJW value from. It says (non literally):

    "Put the SJW to the highest possible value only reduced by tseg2."

The fact that this requirement might no fit to any CAN setup or may not be  in
best academical shape is not my problem. My requirement is to provide an easy
way to move the SJW away from it's default value, which is currently
hard-coded to 1 in the can-dev framework when using it's bittiming calculation
function.

As almost everything is already done (only the provided SJW is not evaluated
in dev.c) this patch is IMO an valid option to support it. If Joe user can
tune the sampling point, why should he not be able to influence the SJW if he
thinks, he knows what he's doing?

Especially as the good working bittiming calculation is not touched in any way
and the default SJW remains 1 (even if '0' is provided as user input).

Regards,
Oliver

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

[Wolfgang Grandegger]

On 09/27/2011 07:32 PM, Oliver Hartkopp wrote:
> On 09/24/11 09:22, Wolfgang Grandegger wrote:
> 
>> On 09/23/2011 11:32 AM, Pavel Pisa wrote:
> 
>>> On base of above analysis, I think that blindly set SJW
>>> on maximum is not good idea. It should be at least limited
>>> to 5% of bit time. 
> 
> 
> (..)
> 
> 
>>> SJW is more problematic, but may it be use of 2 or 5%
>>> of bittime by default with assurance that zero is
>>> replaced by one, would serve to most people pleasure.
> 
> 
> (..)
> 
>>> But there is still unknown parameter
>>> capacity/length of connected wires so there is still
>>> something left to user consideration.
>>
>> Thanks for your detailed explanation. It clearly shows that adjusting
>> SJW is non-trivial and nothing a normal CAN user should deal with. When
>> adjusting SJW, do you also need to tweek other bit-timing parameters,
>> e.g. tq? I mean, would "ip link set can0 type can bitrate x
>> sampling-point y sjw z" work for your setup or do you need to use the
>> expert mode setting via "ip link set can0 type can tq ..." anyway?
> 
> 
> Hello Wolfgang,
> 
> i double checked a specification where i got my requirement to influence the
> SJW value from. It says (non literally):
> 
>     "Put the SJW to the highest possible value only reduced by tseg2."
> 
> The fact that this requirement might no fit to any CAN setup or may not be  in
> best academical shape is not my problem. My requirement is to provide an easy
> way to move the SJW away from it's default value, which is currently
> hard-coded to 1 in the can-dev framework when using it's bittiming calculation
> function.

OK.

> As almost everything is already done (only the provided SJW is not evaluated
> in dev.c) this patch is IMO an valid option to support it. If Joe user can
> tune the sampling point, why should he not be able to influence the SJW if he
> thinks, he knows what he's doing?
> 
> Especially as the good working bittiming calculation is not touched in any way
> and the default SJW remains 1 (even if '0' is provided as user input).

Still not sure if it's useful. Anyway, just added my acked-by to your patch.

Thanks,

Wolfgang.

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

[Wolfgang Grandegger]

Hi Pavel,

On 09/23/2011 11:32 AM, Pavel Pisa wrote:
> Hello Oliver and Wolfgang,
...
> I hope that actual calculation works reasonably well.
> But if it should be enhanced, then it would worth
> to add additional parameter except crystal/controller
> clock source freqency to the concrete boards drivers.
> It should be measured round-trip delay of given/used
> transceiver/optocouplers combination. This would
> allow to have sampling point setup yet more independent
> on given HW and same value could be used for different
> bitrates. But there is still unknown parameter
> capacity/length of connected wires so there is still
> something left to user consideration.

The actual in-kernel bit-timing calculation is known to work well for
most, not all, CAN users, especially with "good" clock source
frequencies (a multiple of 8 MHz). That's fine and intended. The results
of that algorithm are not always satisfactory. Such cases should then be
handled in user-space, either manually or by an advanced algorithm. In
the BerliOS "can-utils" there we have already "can-calc-bit-timing". Any
enhancements and improvements would be nice to share, indeed. Do you
already have/use some code for the enhancement mentioned above?

Wolfgang.

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

[Wolfgang Grandegger]

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.
> 
> Signed-off-by: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Thanks,

Wolfgang.

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

[Kurt Van Dijck]

On Thu, Sep 22, 2011 at 12:28:54PM +0200, 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.
> 
Since both iproute2 & the bittiming calculation seem to be ready for it,
activating the SJW setting looks like a good idea.

Thanks,
Kurt

Acked-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>