search

afiskon / stm32-si5351

HAL-based Si5351 driver for STM32
https://eax.me/stm32-si5351/
MIT License
48 stars 13 forks source link

FIX calculation of t, e.g. using 1048576 i.e. 2^20 causes an overflow… #1

Closed tkuschel closed 2 years ago

tkuschel commented 2 years ago

… for the register using z. When using your calculation for instance with correction si5351Correction set to 0 and given exactly the frequency value of 2^20 that is 1048576; that value >> 20 gives exactly 1 but within the next line if(Fclk & 0xFFFFF) that fails and t remains 1. So the z becomes 1048576 which is one bit too high :-) I think you simply can replace it with a one liner: t = (Fxtal >> 20) + 1; which correctly adds this for the divisor t

Thank you Aleksander for sharing your code. I just looked at it, but never installed it, because I definitely try to make a separate library with more then one instances. So I can drive several Si5351 on my STM32L4A6ZG ... Thomas

afiskon commented 2 years ago

@tkuschel hi Thomas and many thanks for the report.

You are right that there is a bug here. I will reproduce the issue and then make sure that the proposed patch fixes it.

I just looked at it, but never installed it, because I definitely try to make a separate library with more then one instances. So I can drive several Si5351 on my STM32L4A6ZG ...

Sounds like a valuable feature to me. Feel free submitting a corresponding PR.

afiskon commented 2 years ago

@tkuschel I verified your fix and applied it (it needed a little clean up, see 850a0ddc ). I also modified tests/*.py scripts accordingly. For the record, because of how si5351_writeBulk() is implemented in practice the register was not overflowed. Going from 1048576 Hz to 1048575 Hz changed the frequency 1 Hz UP instead of changing it DOWN.