Hi, I do not know if it is because I have an older version of Si5351, but most registers are write only (you cannot read them back).
This affect all the functions that set bits using read-update-write.

I have modified my version to include a register cache;
modified the si5351_read(), si5351_write() and si5351_write_bulk()

Sometimes, if used intensively, PLL goes out of locking and never returns to it.

Appnote "Si5351 – Modifying the Feedback Multisynth Dividers Silicon Labs Timing Knowledge Base Article # 311668" states that:
"any changes to the feedback Multisynth will require a PLL reset. PLLA and PLLB are reset by setting register 177 bits 5 and 7 respectively to 1."

Please add these resets to the code where the PLL multisynths are updated.

Trying to figure out why my Si5351a cannot generate frequencies more than 112.5 MHz, I've found that two different lots of Si5351a exist on the market. According to various specs and comparison tables, one of them has multisynth divider limits 8..2050, another one - 6..1800. There is no way to determine programmatically which one is in use. Also, I don't know a way to determine which one it is by its enclosure marking.

Therefore, I suggest to set multisynth limits to:

define SI5351_MULTISYNTH_A_MIN 8

define SI5351_MULTISYNTH_A_MAX 1800

Also, maximum output frequency should be limited to 112500000 Hz.

I'm successfully using your code for my antenna analyzer project (using the board based on STM32 ARM Cortex M3), but had to make some changes in si5351 code.
There are several platform-dependent definitions like TW_xxx used in si5351.c. It does not allow building the code for other platforms. All such checks should be moved to i2c.c which is platform dependent.

PLL parameters are never going to be written as this check is bound to fail: pll_calc (a function pointer) will always differ from the null pointer (0).

Either:

• The check is not needed, and PLL parameters always need to be written (probable, serious fault)
• The check is needed, but the wrong comparison has been used (possible, serious fault)
• The check is not needed, and PLL parameters never need to be written (doubtful, redundant code)

Jason,
First thank you so very much for all your effort in providing the si5351c library. This has been extremely helpful to me as I modify the 'all band' 0-2 GHz disciplined transverter I'm working to make available on my Open Source Hardware Pages and the 2nd gen general purpose disciplined reference extracted from it using the Si5351 C. 1st gen is already in TAPR's hands for production and deployment for HamSci and other uses which have already netted important new insights into the ionosphere and propagation within amateur radio through FST4W spectral spreading measurements. In moving from the A to C I immediately went to your library which saved me a great deal of time. I apologize that I haven't yet been able to fully read your documentation or comments on this forum yet, things worked so well and so soon that I have been able to address HW and other issues for these devices I'm making.

I'd like to offer a suggestion that I think would greatly improve the C implementation. That is to use Continued Fraction techniques to obtain far more precision than simpler frequency setting techniques. These presently limit resolution to approximately SI5351_PLL_INPUT_CLKIN/10^6 which amounts to about 40 ppb for a 25 MHz source.

The new measurements and discoveries we are achieving with high precision even on HF require small-mHz accuracy to measure ionospheric characteristics and Doppler shift in the small mHz or few-ppt (parts per trillion) precision. The current library can not provide this but I think there may be a direct way to improve it. Franco Venturi K4VZ has already written code to calculate these new values which I think can provide about six orders of magnitude improvement in precision. While I haven't yet tried it I have confidence that it will work.

If your library could be revised to use this technique, borrowing from Franco's code as is useful, I think the results could be phenomenal. The present few kHz setting errors currently produced at microwave frequencies could drop to a few mHz (milli-Hz) in situations where the Si5351 is providing reference clocking. Of course this improvement would apply to all the user outputs on the 2nd generation reference I now have prototyped and operating.

While I think I am able to extend your library to make the ms calculation accessible myself, you are a far better coder than I am and undoubtedly much more proficient than I and the improvements would then be directly available to everyone using your efforts.
Having wide geographic area very high precision transmitters and receivers can allow high precision time of flight measurements, propagation study, geopositioning and other benefits to amateur radio and even radio science in general.

If it could help you, I am willing to provide you prototype hardware on which to develop and test these modifications. It does require a test set up with very high GNSS, OCXO or local 10 MHz disciplining such as rubidium standards. If you contact me directly I think I can provide Franco's code if you are interested in pursuing this.
Again thank you so much for what you've done and the great contribution it is.

Glenn Elmore n6gn
Fort Collins, CO

Several important registers that make significant influence on generated signals are at undefined state after powerup, namely, FANOUT register (187) and Spread Spectrum Parameters register (149). If, occasionally, fanout or spread spectrum will be enabled at powerup, the device will output unexpected signals.
These registers must be initialized explicitly.

There is no reason at all to use malloc for a 30 Bytes buffer.

The implications of using malloc and free on microcontrollers is quite severe - a simple 32 byte array should be used instead.

Probably the other issue reporting a memory leak leads to the same problem.

In current code, rational_best_approximation() almost always returns the same values of b and c that were passed to its input as rfrac and denom. The function has no numerical space for approximation: it can only perform GCD leaving ratio exactly the same. This approximation is useless and can be totally omitted, leaving the code with noticeable frequency setting errors.

The solution is to multiply both rfrac and denom by 4000ul to give some space for approximation within 32-bit range without overflowing the UINT32_MAX boundary.

And the ultimate, most precise but computationally heavy solution is to use 64-bit rfrac and denom, calculated from frequencies expressed as double floating points, and multiplied with very very big ULL number. Also, uint64_t internals inside rational_best_approximation(). But this way is hardly possible for 8-bit AVR which will do the computations for ages in this case. On ARM Cortex this way works like a charm, providing exceptional frequency setting accuracy.

si5351_set_pll() does not free allocated buffer.
BTW, why malloc's are used? Create 30-byte buffer on the stack and avoid all problems.

The frequency setting of either CLK1 or CLK2 outputs significantly impacts the frequency of CLK2 or CLK1 respectively by 100 KHz or more when operating at HF frequencies. The CLK that is set last is accurately set but the previously set CLK sharing the same PLL is frequency shifted.

This problem only affects CLK outputs sharing the same PLL. Since there are two PLL's the issue is only significant for applications where 3 outputs are needed as two of the outputs must share a PLL.

From pll_calc():

/* Factor calibration value into nominal crystal frequency */
/* Measured in parts-per-ten million */
ref_freq += (uint32_t)((double)(correction / 10000000.0) * (double)ref_freq)

Fixed-point is perhaps more precise, using a "long long" aka int64_t:

/* Factor calibration value into nominal crystal frequency */
/* Measured in parts-per-ten million */
ref_freq += (int32_t) ((((((int64_t)correction) << 31) /
    10000000LL) * ref_freq) >> 31);

Resulting linked binaries aren't that different in size (depending on optimization level in use).
Your mileage may vary.

Please look at line 138 at si5351.c:
if(pll_calc == 0)
Why does it evaluate function pointer here? The result will always be false.