Increase PLL min VCO from 400MHz to 750MHz for improved stability across operating conditions #869

2022-06-16T11:33:28-05:00

liamfraser commented

2022-06-16 11:33:28 -05:00

(Migrated from github.com)

No description provided.

kilograham (Migrated from github.com) reviewed

2022-06-16 11:33:28 -05:00

lurch (Migrated from github.com) reviewed

2022-06-16 14:09:25 -05:00

src/rp2_common/hardware_clocks/scripts/vcocalc.py

					
				@ -5,7 +5,7 @@ import argparse

				parser = argparse.ArgumentParser(description="PLL parameter calculator")

It's just occurred to me that we should probably also assert that (args.vco_min <= args.vco_max) 😉

It's just occurred to me that we should probably also assert that `(args.vco_min <= args.vco_max)` :wink:

Wren6991 (Migrated from github.com) requested changes

2022-06-20 04:56:53 -05:00

src/rp2_common/hardware_clocks/scripts/vcocalc.py

					
				@ -5,7 +5,7 @@ import argparse

				parser = argparse.ArgumentParser(description="PLL parameter calculator")

Can we remove this assert please. The limits are set and checked by default, but I don't see any reason to stop people from raising limits if they want to. We're not cops.

Wren6991 (Migrated from github.com) approved these changes

2022-06-20 05:34:16 -05:00

lurch (Migrated from github.com) reviewed

2022-06-20 06:15:26 -05:00

src/rp2_common/hardware_clocks/scripts/vcocalc.py

					
				@ -5,7 +5,7 @@ import argparse

				parser = argparse.ArgumentParser(description="PLL parameter calculator")

But it does match the assert added in src/rp2_common/hardware_pll/pll.c ? 🤷‍♂️

But it _does_ match the assert added in `src/rp2_common/hardware_pll/pll.c` ? :man_shrugging:

Wren6991 (Migrated from github.com) reviewed

2022-06-20 06:22:04 -05:00

src/rp2_common/hardware_clocks/scripts/vcocalc.py

					
				@ -5,7 +5,7 @@ import argparse

				parser = argparse.ArgumentParser(description="PLL parameter calculator")

pll.c checks against the limits set by PICO_CONFIG_ defines. This assert was checking the limits themselves, which is different.

lurch (Migrated from github.com) reviewed

2022-06-20 06:31:54 -05:00

src/rp2_common/hardware_clocks/scripts/vcocalc.py

					
				@ -5,7 +5,7 @@ import argparse

				parser = argparse.ArgumentParser(description="PLL parameter calculator")

Hmmm, so should the PICO_PLL_VCO_ defines get turned into "full" PICO_CONFIG defines by adding the appropriate comment-lines like e.g. https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_platform/include/pico/platform.h#L24 ?

Hmmm, so should the `PICO_PLL_VCO_` defines get turned into "full" PICO_CONFIG defines by adding the appropriate comment-lines like e.g. https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_platform/include/pico/platform.h#L24 ?

Wren6991 (Migrated from github.com) reviewed

2022-06-20 07:54:07 -05:00

src/rp2_common/hardware_clocks/scripts/vcocalc.py

					
				@ -5,7 +5,7 @@ import argparse

				parser = argparse.ArgumentParser(description="PLL parameter calculator")

If you hit the assert you will discover the name of the define very quickly

kilograham (Migrated from github.com) reviewed

2022-06-20 07:57:56 -05:00

src/rp2_common/hardware_clocks/scripts/vcocalc.py

					
				@ -5,7 +5,7 @@ import argparse

				parser = argparse.ArgumentParser(description="PLL parameter calculator")

Hmmm, so should the PICO_PLL_VCO_ defines get turned into "full" PICO_CONFIG defines by adding the appropriate comment-lines like e.g. https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_platform/include/pico/platform.h#L24 ?

the reason i didn't is that i didn't expect many people would need to (i was going to include it in the release notes)

> Hmmm, so should the `PICO_PLL_VCO_` defines get turned into "full" PICO_CONFIG defines by adding the appropriate comment-lines like e.g. https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_platform/include/pico/platform.h#L24 ? the reason i didn't is that i didn't expect many people would need to (i was going to include it in the release notes)

Sign in to join this conversation.

No reviewers

kilograham

Wren6991