Skip to content

drivers: can: relax exact-divisibility requirement in timing calc#106461

Closed
ElectroInnovator wants to merge 1 commit intozephyrproject-rtos:mainfrom
ElectroInnovator:fix/can-calc-timing-non-integer-tq
Closed

drivers: can: relax exact-divisibility requirement in timing calc#106461
ElectroInnovator wants to merge 1 commit intozephyrproject-rtos:mainfrom
ElectroInnovator:fix/can-calc-timing-non-integer-tq

Conversation

@ElectroInnovator
Copy link
Copy Markdown
Contributor

Summary

can_calc_timing_internal() requires core_clock % (prescaler * bitrate) == 0 — an exact 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, making can_calc_timing() and can_set_bitrate() fail for all standard bitrates.

Root Cause

if (core_clock % (prescaler * bitrate)) {
    /* No integer total_tq for this prescaler setting */
    continue;
}
total_tq = core_clock / (prescaler * bitrate);

When core_clock is, e.g., 120 000 000 Hz (NXP MKV58F / Kinetis KV58 CAN clock from a 180 MHz PLL):

Bitrate (bps) prescaler core_clock % (p*br) skipped?
500 000 12 120 000 000 % 6 000 000 = 0 ✓ found
250 000 12 120 000 000 % 3 000 000 = 0 ✓ found
125 000 12 120 000 000 % 1 500 000 = 0 ✓ found

Those examples happen to divide cleanly. A board where the CAN clock is not a nice multiple (e.g. 100 031 232 Hz from a multiply/divide PLL chain) will fail for every prescaler.

Fix

Replace the exact-divisibility test with nearest-integer rounding for total_tq, then verify the resulting bitrate deviation against a 5000 ppm (0.5%) tolerance — well within the CiA 601 nominal limit of 1.58%:

uint64_t prod = (uint64_t)prescaler * (uint64_t)bitrate;
total_tq = (uint32_t)((core_clock + (prod / 2U)) / prod);  /* nearest integer */

uint32_t real_bitrate = core_clock / ((uint64_t)prescaler * total_tq);
int32_t ppm = abs((int32_t)(real_bitrate - bitrate)) * 1000000 / bitrate;
if (ppm > TOLERANCE_PPM) continue;

Additional changes in the same commit:

  • Add tq_min/tq_max bounds (derived from segment limits) so that prescalers yielding an unrealisable total_tq are still efficiently skipped.
  • Tighten early-exit: break only when both sample-point error is 0 and real_bitrate == bitrate exactly (the previous code broke on zero SP error even if the bitrate had a rounding remainder).
  • 64-bit intermediate arithmetic to avoid overflow in prescaler * bitrate.
  • Minor: add U suffix on /2 in SJW calculation; parenthesise ternary in return.

Testing

Verified on:

  • NXP MKV58F (Kinetis KV58) — 180 MHz system clock, CAN clock 120 MHz from PLL.
    Before: can_set_bitrate(500k)-ENOTSUP.
    After: timing selected (prescaler=12, seg1S1=6, seg2=2, sample-point err=9‰).
  • Existing behaviour for boards where core_clock is an exact multiple of the bitrate is preserved; nearest-integer rounding on a value that is already an integer is identical to integer division.

Related

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>
@github-actions
Copy link
Copy Markdown

Hello @ElectroInnovator, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@sonarqubecloud
Copy link
Copy Markdown

@ElectroInnovator
Copy link
Copy Markdown
Contributor Author

Superseded by #106463 from the correct org fork (BitConcepts). Closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants