stianeikeland / go-rpio

:electric_plug: Raspberry Pi GPIO library for go-lang
MIT License
2.16k stars 221 forks source link

Pi4 PWM frequency wrong #76

Closed b3nn0 closed 2 years ago

b3nn0 commented 2 years ago

Hi, I noticed some oddities with the PWM frequency when using this library, so I double-checked with a scope.. and indeed it's off. e.g. when using pin.Freq(64000) I'd expect a frequency of 64khz. However, I get 1.8khz on my scope. This seems to be because of a wrong assumption about the oscillator frequency. In rpio.go, it says

const sourceFreq = 19200000 // oscilator frequency`

However, this is not correct for the PI4. According to https://raspberrypi.stackexchange.com/questions/100622/is-default-pwm-frequency-19-2mhz-in-raspberry-3#:~:text=1%20Answer&text=The%20crystal%20oscillator%20is%2019.2,has%20a%2054%20MHz%20oscillator the PI4 has a frequency of 54Mhz. And indeed, changing the code to

const sourceFreq = 54000000 // oscilator frequency

fixes it somewhat. However, it is still wrong by a factor of 100 - I now get a 640Hz PWM instead of a 64k PWM.

Changing it to

const sourceFreq = 540000 // oscilator frequency

now finally gives a correct result (but I guess the method is wrong - I assume the real bug is somewhere further down, but didn't check further).

drahoslove commented 2 years ago

Hi @b3nn0, you are right about the oscillator frequency. There should be some check in the init function to determine the correct version and set the correct oscillator frequency.

But did you really get a higher output frequency by lowering the source frequency? That is weird, are you sure about that? It should IMHO be 54000000, not 540000. You have to take into account the effect of DuctyCycle - the second param of DutyCycle effectively divides the source frequency. You probably had it set to 100, so you should have also multiplied the desired output frequency by 100 before passing it to the Freq function for it to work as intended.

Do you think you could create a PR for that? Unfortunately, I don't have access to working Pi4 now by myself.

b3nn0 commented 2 years ago

Ah yes, you are correct about the 100. I thought SetFreq was fixed, and the second DutyCycle param was just to make it a bit more simple to use as percentages, but now that I think about it, it makes sense I guess. I will check if I can do a PR. Not familiar with the code at all, but shouldn't be too hard I guess.