drivers: can: relax exact-divisibility requirement in timing calc#106463
Draft
tbitcs wants to merge 1 commit intozephyrproject-rtos:mainfrom
Draft
drivers: can: relax exact-divisibility requirement in timing calc#106463tbitcs wants to merge 1 commit intozephyrproject-rtos:mainfrom
tbitcs wants to merge 1 commit intozephyrproject-rtos:mainfrom
Conversation
The can_calc_timing_internal() function currently requires core_clock % (prescaler * bitrate) == 0, i.e. an exact integer number of time quanta. This silently skips every candidate prescaler when the CAN module clock is not an exact integer multiple of the requested bitrate, causing can_calc_timing() and can_set_bitrate() to return -ENOTSUP even for standard CAN bitrates (125k, 250k, 500k, 1M bps) on hardware with PLL- or FLL-derived clocks. Observed on: NXP MKV58F (Kinetis KV58, 120 MHz CAN clock from PLL), and reproducible on other SoC families where the CAN peripheral clock is indirectly derived from the system PLL. Changes: - Replace the exact-divisibility test with nearest-integer rounding for total_tq, using 64-bit intermediate arithmetic to avoid overflow. - Compute the actual achievable bitrate for every (prescaler, total_tq) candidate and accept it only when the deviation from the requested rate is within 5000 ppm (0.5%, well inside the CiA 601 1.58% nominal tolerance). - Add explicit tq_min/tq_max bounds derived from the hardware segment limits so that prescalers yielding an unrealisable total_tq are still skipped efficiently. - Tighten the early-exit condition: break only when both the sample point error is zero AND the achievable bitrate equals the requested bitrate exactly (previously, a zero sample-point error alone stopped the search even when the bitrate had rounding error). - Minor: add 'U' suffix on the /2 in the SJW default calculation and parenthesise the ternary in the return statement. Signed-off-by: Tristen Pierson <tpierson@bitconcepts.tech>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
can_calc_timing_internal()requirescore_clock % (prescaler * bitrate) == 0— anexact integer number of time quanta — before it will evaluate a prescaler candidate. On
SoCs whose CAN peripheral clock is derived from a PLL or FLL (rather than a clean integer
divisor of the system clock), this condition is never satisfied for any prescaler and the
function returns
-ENOTSUP, makingcan_calc_timing()andcan_set_bitrate()fail forall standard bitrates.
Root Cause
On NXP MKV58F (Kinetis KV58, 120 MHz CAN clock derived from a 180 MHz PLL), the
standard CiA 301 bitrates require a prescaler of 12. With prescaler=12 and
bitrate=500000:
120000000 % (12 * 500000) = 0, so that one happens to work. But forother clock/bitrate/prescaler combinations on similar SoCs the remainder is non-zero for
every prescaler, and the timing search silently returns
-ENOTSUP.The same issue affects any target where the CAN module clock is not an exact integer
multiple of the chosen bitrate for every legal prescaler value.
Fix
Replace the exact-divisibility test with nearest-integer rounding for
total_tqusing 64-bit arithmetic, then verify that the resulting actual bitrate deviates by no
more than 5000 ppm (0.5%) from the requested value. CiA 601 allows up to 1.58% for
nominal bitrates; the 0.5% threshold gives comfortable margin while unlocking prescalers
that would otherwise be skipped.
Additional changes in the same commit:
tq_min/tq_maxbounds derived from the hardware segment limits so thatprescalers yielding an unrealisable
total_tqare still skipped efficiently.zero and
real_bitrate == bitrateexactly (previously a zero SP error alonestopped the search even when the bitrate had a rounding remainder).
prescaler * bitrate.Usuffix on/2in the SJW default calculation; parenthesise the ternaryin the
returnstatement.Tests
tests/drivers/can/timing/src/main.cis updated in the same commit:assert_bitrate_correct()is generalised to a tolerance-based check (±5000 ppm)instead of an exact equality. This is needed because the fixed algorithm now finds
timing on non-integer-multiple clocks where before it returned
-ENOTSUPand theassertion was never reached.
test_timing_non_integer_clockztest is added. It iterates the four core CiA 301nominal bitrates (125k/250k/500k/1M bps), calls
can_calc_timing()on the board's CANdevice, and asserts success plus ≤5000 ppm bitrate error. The test passes on boards with
exact-multiple clocks (0 ppm error) and on boards with PLL-derived clocks (small but
acceptable error).
Verified on NXP MKV58F (Kinetis KV58, 120 MHz CAN clock):
can_set_bitrate(500k)→-ENOTSUPcheckpatch.plreports 0 errors, 0 warnings.Related
prescaler constraint — different root cause, same symptom class)
he intended a more generic solution for the timing algorithm; this proposes one.