Re: [PATCH] can: bittiming: can_calc_bittiming(): prefer small bit rate pre-scalers over larger ones

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

 



On 19.03.2022 13:46:42, Pavel Pisa wrote:
> On Friday 18 of March 2022 15:49:13 Marc Kleine-Budde wrote:
> > The CiA (CAN into Automation) lists in their Newsletter 1/2018 in the
> > "Recommendation for the CAN FD bit-timing" [1] article several
> >
> > recommendations, one of them is:
> > | Recommendation 3: Choose BRPA and BRPD as low as possible
> >
> > [1]
> > https://can-newsletter.org/uploads/media/raw/f6a36d1461371a2f86ef0011a51371
> >2c.pdf
> >
> > With the current bit timing algorithm Srinivas Neeli noticed that on
> > the Xilinx Versal ACAP board the CAN data bit timing parameters are
> > not calculated optimally. For most bit rates, the bit rate
> > prescaler (BRP) is != 1, although it's possible to configure the
> 
> I have already thought about algorithm and optimal number of bitquanta
> per bittime. In the fact, I think than original LinCAN algorithm version
> which I have provided for SocketCAN used reversed preference for BPR.

It's complicated, at least for all evolutions of the algorithm we had in
the Linux kernel.

Side note: I've imported all Linux mainline algorithms into the
           can-calc-bit-timing tool available on my github:
           https://github.com/marckleinebudde/can-utils/tree/improve-can-calc-bit-timing
           Use the "--algo=" parameter to select the actual algorithm.
           Use diff to compare the bit timing:
           diff -u <(./can-calc-bit-timing --alg=v2.6.31 mcan-v3.1+) <(./can-calc-bit-timing --alg=v4.8 mcan-v3.1+)

For CAN bitrate/CAN clock rate/bit timing constant parameter
combinations where exact solutions are possible (no bit rate error and
no sample point errors are possible), the original algorithm will select
the solution with the lowest BRP. However, where exact solutions are not
possible, it will prefer those with larger BRP. With my proposed fix,
the behavior in the same for both cases.

> But I think that too small or too big ratio is bad.

It's complicated.

Valid solutions with the same (even optimal) bit rate and sample point
but with different BRPs are not equal. Why? All evolutions of the
algorithm use a default SJW of 1 (if no value if provided by the user -
the original algorithm even forces a fixed value of 1).

SJW == 1 means the SJW has the length of one Time Quanta (TQ). The length
of a TQ directly depends on the BRP:

    TQ = BRP / CAN Clock Frequency

This results in the SJW measured in time units, is not the same for
different BRPs and thus not the same for valid bit timing parameters
with different BRPs. So your statement is correct, at least while using
a fixed SJW == 1.

But, for CAN-FD we should use the same BRP for the arbitration and
data bit timing parameters. Why is that the case?
- Some controllers use the same BRP for the arbitration and the data bit
  timing.
- Different BRPs can result in a phase error when switching from
  arbitration to data bit timing
- For Transceiver Delay Compensation (TDC) to work, both BRPs must be 1.
  Although some controllers support 2, aswell.
- The CiA recommends it. See Recommendation 2:
  https://can-newsletter.org/uploads/media/raw/f6a36d1461371a2f86ef0011a513712c.pdf

> So for our actual NuttX ESP32C3 driver developed by our studnet Jan
> Charvat, I consider to add some mechanism how to specify optimal
> bitquanta per bittime ratio.

As elaborated above, this indirectly influences the size of the SJW. The
CiA has also a recommendation regarding SJW (same paper as above):

| Recommendation 5: Chose sjw D and sjw A as large as possible
|
| The maximal possible values are min(ps1 A/D, ps2 A/D). A large sjw A
| value allows the CAN node to resynchronize quickly to the transmitting
| node. A large sjw D value maximizes the CAN clock tolerance.

> The question is if that should be additional parameter in the struct
> can_bittiming_const
> 
>   https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/can/netlink.h#L47
> 
> or if that should be some constant defined for data bitrate and
> probably the second for arbitration bitrate.

The struct can_bittiming_const is user space API and cannot be changed.
If we need more parameters we would first convert the struct
can_bittiming_const based netlink type into a nested type. The nested
type then can be extended at will without breaking the API.

But this is independent if the enhancement of the bit timing algorithm.

> If there is interrest I try to provide some patch which would prefer
> the nearest suitable BPR to optimal timequanta to bittime ratio.

I'm really interested in your thoughts and patches. For easier testing,
I'm developing the algorithm in the can-calc-bit-timing tool, it can
easily be extended for more variants. Feel free to add a new variant.
Send me a PR on github or patches on this list.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: PGP signature


[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