[WIP] Separate system clock config for rp2xxx devices

[cibomahto]

Here is a first pass at separating the PLL configuration options for the rp2xxx devices, to allow the rp2350 to be configured for 150MHz. I tested (using a logic analyzer) that the UART, USB, I2C, and SPI generate roughly correct speeds.

I moved the configuration into the machine files, but it could be nice to allow a board to override them. If you have a suggestion for how to do that, please let me know.

I know of three issues that should be fixed before submitting the pull request:

• I2C is slow (90khz instead of 100khz), but this appears to be an existing issue and is similar on both parts.
• The 150MHz clock creates a non-integer 6.66ns period, which makes the PWM output inaccurate on the rp2350
• Currently, the board files have a separate, nonfuctional configuration for the on-board crystal. I created a separate issue for this: rp2040 / rp2350 xoscFreq setting is nonfuctional #4673 , but this touches on the same code.

cc @soypat @deadprogram

[deadprogram]

Thank you for the PR @cibomahto

Can you please run go fmt on the changed files?

/bin/sh: 1: node: not found
Unformatted:
  src/machine/machine_rp2_2040.go
  src/machine/machine_rp2_2350.go
make: *** [GNUmakefile:185: fmt-check] Error 1

[eliasnaur]

The rp2040 datasheet says that its cores are capable of 133 MHz. A cursory glance at the datasheet didn't mention any downside to that speed, so why is TinyGo running at 125 MHz? Perhaps it's because of non-integer periods that you mention in the PR description for rp2350? If so, this PR might consider rp2040 at 133 MHz for the same reasons you chose 150 MHz for rp2350.

Failing all that, a comment would be nice.

[cibomahto]

That's a good point, I don't know why it was running at 125 either, but this seems plausible.

[soypat]

Changing out the xoscFreq constant for a package level variable feels off. I also don't understand what we gained from switching out the constant for a xoscFreq field on clockCfg. In any case the constants could remain in each board-level file and the variable be set in a single package level file.

[cibomahto]

Changing out the xoscFreq constant for a package level variable feels off. I also don't understand what we gained from switching out the constant for a xoscFreq field on clockCfg. In any case the constants could remain in each board-level file and the variable be set in a single package level file.

As is, the xoscFreq constant doesn't work- you need to configure the PLLs to match the xoscFreq setting, and they were hard-coded in the clock configuration routine. Otherwise, you won't get the correct CPU clock speed, or the PLL might not actually start.

I guess some other options are:

• Port the PLL configuration solver to go, and run it during the configuration routine to determine the needed settings. Usually this isn't done because it is a little heavy for a boot routine, and there is some nuance about which configuration is best for a given design.
• Make a table of known PLL configurations for the given xoscFreq, and panic if the correct one isn't on the list.
• Drop xoscFreq, it's unlikely that any boards will use a different value than the default. This also implies that you can only run the RP2040 or RP2350 at a single clock speed, but maybe that's good enough.
• Copy all the PLL and clock settings into each board as constants, as you suggested, but then there is a lot of duplication.

In C-style apis, it's a common pattern to have default tables that you can pass to init routines, but I haven't been able to find something similar in TinyGo. The closest so far is for instance the SPI or I2C configuration objects (for example), but in this case the defaults are 'secret', and applied when fields are left at 0, which seems a little weird to me.