si5351: Write reserved bits of register 183 to zero on rev 0 parts

[martinling]

Per discussion on #1747.

[martinling]

Note that for rev 1 or higher, this now deliberately writes 0x12 to the reserved bits as per AN619, rather than just leaving whatever was read back at startup.

[Brumi-2021]

Thanks @martinling — and many thanks for opening #1811. I have reviewed the proposed approach and it is exactly the kind of backward-compatible solution I was hoping for: keeping the AN619 0x92 behaviour for Rev >= 1 while restoring the historical 0x80 behaviour only for Rev 0 devices.

I have also done some additional hardware testing today.

First, I have now confirmed that the affected Device 1 appears to contain an Si5351C, despite reporting REVID=0. In HackRF USB mode I can enable CLKOUT and freely program its output frequency (I tested the programmable CLKOUT functionality), unlike my HackRF One r9 / Si5351A hardware where the reduced output configuration imposes the known CLKOUT limitations.

More importantly, I was able to observe the effect of REG183 externally without opening the unit device 1 (Si5351C REVID =0). I enabled CLKOUT and measured the 10 MHz output with an oscilloscope while changing only REG183:

REG183 = 0x80 → stable 10 MHz CLKOUT and correct RF operation
REG183 = 0x92 → severe clock instability/disturbance and incorrect RF operation

I repeated the comparison using both the corresponding firmware versions and direct REG183 writes, with consistent results.

The other three tested devices using Si5351 A (1x hackrf one r9) and Si5351 C (1xhackrf one r1-r8 and 1xhackrf PRO), (all REVID =1) show no comparable CLKOUT degradation when switching between 0x80 and 0x92.

So this provides a physical clock-domain symptom matching the RF regression: on this Si5351C REVID=0 device, setting the reserved bits to 0x12 does not merely alter an otherwise unused/reserved state — it visibly destabilizes the generated clock.

Given these results, I am happy with 0x80 as the compatibility value for REVID=0 and with the approach taken in #1811.

Thanks again for taking this investigation seriously and for implementing the compatibility fix.

One very minor detail I noticed while reading the diff: the source comment currently says AN612; I believe that should be AN619.

==> } else {
/* But AN612 says to write 010010b to the reserved bits. */
set_XTAL_RESERVED(drv, 0x12);
}

Cheers,

[martinling]

One very minor detail I noticed while reading the diff: the source comment currently says AN612; I believe that should be AN619.

Good catch, thanks. Fixed that.

I've also added a second commit, to remove an ancient 2012 comment from @mossmann saying that register 183 reads as 0xE4 on power-up (i.e. 10pF default setting, with 0x24 in the reserved bits).

That must have been true on some hardware at the time, but current boards clearly start up with 0x12 in the reserved bits, as per AN619.