On Thu. 2 Feb 2023 at 20:09, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> "The (Re-)Synchronization Jump Width (SJW) defines how far a
> resynchronization may move the Sample Point inside the limits defined
> by the Phase Buffer Segments to compensate for edge phase errors." [1]
>
> In other words, this means that the SJW parameter controls the CAN
> controller's tolerance to frequency errors compared to other CAN
> controllers.
>
> If the user space doesn't provide a SJW parameter, the
> kernel chooses a default value of 1. This has proven to be a good
> default value for CAN controllers, but no longer for modern
^^^^^^^^^^^^^^^
> controllers.
Are you missing a word here? You oppose CAN controllers to modern ones.
I think the point is Classical CAN only controllers vs. CAN-FD
controllers. A CAN-FD controller is able to sample at bitrates up to 5
or 8 Mbits and have maximum bitimming values five or eight times the
ones of a Classical CAN only controller (which is only capable of
sampling 1 Mbits).
I propose this instead:
This has proven to be a good default value for Classical CAN
controllers, but no longer for modern CAN-FD ones.
> In the past there were CAN controllers like the sja1000 with a rather
> limited range of bit timing parameters. For the standard bit rates
> this results in the following bit timing parameters:
>
> | Bit timing parameters for sja1000 with 8.000000 MHz ref clock
> | _----+--------------=> tseg1: 1 … 16
> | / / _---------=> tseg2: 1 … 8
> | | | / _-----=> sjw: 1 … 4
> | | | | / _-=> brp: 1 … 64 (inc: 1)
> | | | | | /
> | nominal | | | | | real Bitrt nom real SampP
> | Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error BTR0 BTR1
> | 1000000 125 2 3 2 1 1 1000000 0.0% 75.0% 75.0% 0.0% 0x00 0x14
> | 800000 125 3 4 2 1 1 800000 0.0% 80.0% 80.0% 0.0% 0x00 0x16
> | 666666 125 4 4 3 1 1 666666 0.0% 80.0% 75.0% 6.2% 0x00 0x27
> | 500000 125 6 7 2 1 1 500000 0.0% 87.5% 87.5% 0.0% 0x00 0x1c
> | 250000 250 6 7 2 1 2 250000 0.0% 87.5% 87.5% 0.0% 0x01 0x1c
> | 125000 500 6 7 2 1 4 125000 0.0% 87.5% 87.5% 0.0% 0x03 0x1c
> | 100000 625 6 7 2 1 5 100000 0.0% 87.5% 87.5% 0.0% 0x04 0x1c
> | 83333 750 6 7 2 1 6 83333 0.0% 87.5% 87.5% 0.0% 0x05 0x1c
> | 50000 1250 6 7 2 1 10 50000 0.0% 87.5% 87.5% 0.0% 0x09 0x1c
> | 33333 1875 6 7 2 1 15 33333 0.0% 87.5% 87.5% 0.0% 0x0e 0x1c
> | 20000 3125 6 7 2 1 25 20000 0.0% 87.5% 87.5% 0.0% 0x18 0x1c
> | 10000 6250 6 7 2 1 50 10000 0.0% 87.5% 87.5% 0.0% 0x31 0x1c
>
> The attentive reader will notice that the SJW is 1 in most cases,
> while the Seg2 phase is 2. Both values are given in TQ units, which in
> turn is a duration in nanoseconds.
>
> For example the 500 kbit/s configuration:
>
> | nominal real Bitrt nom real SampP
> | Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error BTR0 BTR1
> | 500000 125 6 7 2 1 1 500000 0.0% 87.5% 87.5% 0.0% 0x00 0x1c
>
> the TQ is 125ns, the Phase Seg2 is "2" (== 250ns), the SJW is "1" (==
> 125 ns).
>
> Looking at a more modern CAN controller like a mcp2518fd, it has wider
> bit timing registers.
>
> | Bit timing parameters for mcp251xfd with 40.000000 MHz ref clock
> | _----+--------------=> tseg1: 2 … 256
> | / / _---------=> tseg2: 1 … 128
> | | | / _-----=> sjw: 1 … 128
> | | | | / _-=> brp: 1 … 256 (inc: 1)
> | | | | | /
> | nominal | | | | | real Bitrt nom real SampP
> | Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error NBTCFG
> | 500000 25 34 35 10 1 1 500000 0.0% 87.5% 87.5% 0.0% 0x00440900
>
> The TQ is 25ns, the Phase Seg 2 is "10" (== 250ns), the SJW is "1" (==
> 25ns).
>
> Since the kernel chooses a default SJW of 1 regardless of the TQ, this
> leads to a much smaller SJW and thus much smaller tolerances to
> frequency errors.
>
> To maintain the same oscillator tolerances on controllers with wide
> bit timing registers, select a default SJW value of Phase Seg2 / 2
> unless Phase Seg 1 is less. This results in the following bit timing
> parameters:
>
> | Bit timing parameters for mcp251xfd with 40.000000 MHz ref clock
> | _----+--------------=> tseg1: 2 … 256
> | / / _---------=> tseg2: 1 … 128
> | | | / _-----=> sjw: 1 … 128
> | | | | / _-=> brp: 1 … 256 (inc: 1)
> | | | | | /
> | nominal | | | | | real Bitrt nom real SampP
> | Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error NBTCFG
> | 500000 25 34 35 10 5 1 500000 0.0% 87.5% 87.5% 0.0% 0x00440904
>
> The TQ is 25ns, the Phase Seg 2 is "10" (== 250ns), the SJW is "5" (==
> 125ns). Which is the same as on the sja1000 controller.
>
> [1] http://web.archive.org/http://www.oertel-halle.de/files/cia99paper.pdf
>
> Cc: Mark Bath <mark@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> ---
> drivers/net/can/dev/bittiming.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/can/dev/bittiming.c b/drivers/net/can/dev/bittiming.c
> index 68287b79afe8..55714e08ca3a 100644
> --- a/drivers/net/can/dev/bittiming.c
> +++ b/drivers/net/can/dev/bittiming.c
> @@ -11,8 +11,8 @@ void can_sjw_set_default(struct can_bittiming *bt)
> if (bt->sjw)
> return;
>
> - /* If user space provides no sjw, use 1 as default */
> - bt->sjw = 1;
> + /* If user space provides no sjw, use sane default of phase_seg2 / 2 */
> + bt->sjw = max(1U, min(bt->phase_seg1, bt->phase_seg2 / 2));
> }
>
> int can_sjw_check(const struct net_device *dev, const struct can_bittiming *bt,
