Re: [PATCH v2 14/17] can: bittiming: can_sjw_set_default(): use Phase Seg2 / 2 as default for SJW

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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,




[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux