can: bittiming: can_calc_bittiming(): prefer small bit rate pre-scalers over larger...

The CiA (CAN in 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/f6a36d1461371a2f86ef0011a513712c.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
requested with a bit rate with a prescaler of 1:

| Data Bit timing parameters for xilinx_can_fd2i with 79.999999 MHz ref clock (cmd-line) using algo 'v4.8'
|  nominal                                  real  Bitrt    nom   real  SampP
|  Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP  Bitrate  Error  SampP  SampP  Error
| 12000000     12   2    2    2   1   1 11428571   4.8%  75.0%  71.4%   4.8%
| 10000000     25   1    1    1   1   2  9999999   0.0%  75.0%  75.0%   0.0%
|  8000000     12   3    3    3   1   1  7999999   0.0%  75.0%  70.0%   6.7%
|  5000000     50   1    1    1   1   4  4999999   0.0%  75.0%  75.0%   0.0%
|  4000000     62   1    1    1   1   5  3999999   0.0%  75.0%  75.0%   0.0%
|  2000000    125   1    1    1   1  10  1999999   0.0%  75.0%  75.0%   0.0%
|  1000000    250   1    1    1   1  20   999999   0.0%  75.0%  75.0%   0.0%

The bit timing parameter calculation algorithm iterates effectively
from low to high BRP values. It selects a new best parameter set, if
the sample point error of the current parameter set is equal or less
to old best parameter set.

If the given hardware constraints (clock rate and bit timing parameter
constants) don't allow a sample point error of 0, the algorithm will
first find a valid bit timing parameter set with a low BRP, but then
will accept parameter sets with higher BRPs that have the same sample
point error.

This patch changes the algorithm to only accept a new parameter set,
if the resulting sample point error is lower. This leads to the
following data bit timing parameter for the Versal ACAP board:

| Data Bit timing parameters for xilinx_can_fd2i with 79.999999 MHz ref clock (cmd-line) using algo 'can-next'
|  nominal                                  real  Bitrt    nom   real  SampP
|  Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP  Bitrate  Error  SampP  SampP  Error
| 12000000     12   2    2    2   1   1 11428571   4.8%  75.0%  71.4%   4.8%
| 10000000     12   2    3    2   1   1  9999999   0.0%  75.0%  75.0%   0.0%
|  8000000     12   3    3    3   1   1  7999999   0.0%  75.0%  70.0%   6.7%
|  5000000     12   5    6    4   1   1  4999999   0.0%  75.0%  75.0%   0.0%
|  4000000     12   7    7    5   1   1  3999999   0.0%  75.0%  75.0%   0.0%
|  2000000     12  14   15   10   1   1  1999999   0.0%  75.0%  75.0%   0.0%
|  1000000     25  14   15   10   1   2   999999   0.0%  75.0%  75.0%   0.0%

Note: Due to HW constraints a data bit rate of 1 MBit/s with BRP = 1 is not possible.

Link: https://lore.kernel.org/all/20220318144913.873614-1-mkl@pengutronix.de
Link: https://lore.kernel.org/all/20220113203004.jf2rqj2pirhgx72i@pengutronix.de
Cc: Srinivas Neeli <sneeli@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

diff --git a/drivers/net/can/dev/bittiming.c b/drivers/net/can/dev/bittiming.c
index 2103bcca9012b33876d0f419d51340e427fb20a6..c1e76f0a5064681f7bbad93849e0312edbbd7629 100644 (file)
--- a/drivers/net/can/dev/bittiming.c
+++ b/drivers/net/can/dev/bittiming.c
@@ -116,7 +116,7 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
 
                can_update_sample_point(btc, sample_point_nominal, tseg / 2,
                                        &tseg1, &tseg2, &sample_point_error);
-               if (sample_point_error > best_sample_point_error)
+               if (sample_point_error >= best_sample_point_error)
                        continue;
 
                best_sample_point_error = sample_point_error;