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/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 fill first find a valid bit timing parameter set with a low BRP, but then 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/20220113203004.jf2rqj2pirhgx72i@xxxxxxxxxxxxxx Cc: Srinivas Neeli <sneeli@xxxxxxxxxx> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> --- drivers/net/can/dev/bittiming.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/dev/bittiming.c b/drivers/net/can/dev/bittiming.c index 2103bcca9012..c1e76f0a5064 100644 --- 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; -- 2.35.1