cpu/samd5x: enable FDPLL1 at 200MHz

[dylad]

Contribution description

This PR allows to use the second FDPLL (the first one is used to generated the 120MHz frequency used by the core and some peripherals). The second FDPLL is setup to run at 200MHz which is the maximum allowed by this MCU.
In fact, I reused the existing function which setup FDPLL0 so it can be used in a generic way for both PLL (since they are the same IP).

I change the way the computation offset (left shift by 5) is done because 200MHz << 5 wouldn't fit inside an uint32_t and I wanted to avoid using an uint64_t here

Two additional commits are present for a small cleanup and a fix.

This is currently unused in our codebase, so it shouldn't impact this platform too much as the ONDEMAND bit is set. the FDPLL will not be running out of the box. But @gschorcht might need it pretty soon.

Testing procedure

This PR can be tested on a same54-xpro and an oscilloscope using the following the patch:

From 76490845ec72387b24116bdd364a61365c186aa1 Mon Sep 17 00:00:00 2001
From: Dylan Laduranty <dylan.laduranty@mesotic.com>
Date: Thu, 11 May 2023 17:42:16 +0200
Subject: [PATCH] removeme! for debug purpose

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>
---
 cpu/samd5x/cpu.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/cpu/samd5x/cpu.c b/cpu/samd5x/cpu.c
index f778991a5b..2866c8c9e5 100644
--- a/cpu/samd5x/cpu.c
+++ b/cpu/samd5x/cpu.c
@@ -220,7 +220,7 @@ static void fdpll_init(uint8_t idx, uint32_t f_cpu)
 }
 
 static void gclk_connect(uint8_t id, uint8_t src, uint32_t flags) {
-    GCLK->GENCTRL[id].reg = GCLK_GENCTRL_SRC(src) | GCLK_GENCTRL_GENEN | flags | GCLK_GENCTRL_IDC;
+    GCLK->GENCTRL[id].reg = GCLK_GENCTRL_SRC(src) | GCLK_GENCTRL_GENEN | flags | GCLK_GENCTRL_OE | GCLK_GENCTRL_IDC;
     while (GCLK->SYNCBUSY.reg & GCLK_SYNCBUSY_GENCTRL(id)) {}
 }
 
@@ -384,6 +384,12 @@ void cpu_init(void)
     dma_init();
 #endif
 
+    sam0_gclk_enable(SAM0_GCLK_200MHZ);
+    /* output both FDPLL (GCLK0 and GCLK4) to gpios */
+    gpio_init_mux(GPIO_PIN(PB, 14), GPIO_MUX_M);
+    gpio_init_mux(GPIO_PIN(PB, 10), GPIO_MUX_M);
+    /* PB14 -> EXT2    PB10 -> QSPI SCK */
+
     /* initialize stdio prior to periph_init() to allow use of DEBUG() there */
     early_init();
 
-- 
2.35.3

It will output both FDPLLs to PB14 and PB10. Their frequency can then be measured using an oscilloscope.

Issues/PRs references

None.

[dylad]

Actual results:

[riot-ci]

Murdock results

✔️ PASSED

6607ed1 cpu/samd5x: add support for FDPLL1 running at 200MHz

Success	Failures	Total	Runtime
6931	0	6931	10m:41s

Artifacts

• Documentation preview

[gschorcht]

Alternatively you could have used

Suggested change

	const uint32_t LDR = (f_cpu / 32768);
	const uint32_t LDR = (f_cpu >> 10); /* LDR = ((f_cpu << 5) / 32768) */

Should be the same as ((f_cpu << 5) / 32768). This might me more reproducible together with the former code for DPLLRATIO.

[dylad]

I'll give it a try.

[dylad]

@gschorcht I've applied your changes and squashed.
FDPLL1 looks good:

[benpicco]

This is currently unused in our codebase, so it shouldn't impact this platform too much as the ONDEMAND bit is set. the FDPLL will not be running out of the box.

Hm, are you sure about that? With examples/timer_periodic_wakeup I see an increase in current consumption from 3.16 mA to 4.98 mA on same54-xpro.

[benpicco]

Current consumption is back to normal when I remove this line.

[dylad]

Strange, the ONDEMAND bit is set and since nobody is connected yet to the associated GCLK it shouldn't impact the overall consumption...
I'll investigate.

[dylad]

Okay, I've found the reason why. ONDEMAND bit is "enable-protected".
I'll provide a fix later.
This also mean that the FDPLL0 is not running with ONDEMAND set which is a bug.

[gschorcht]

I don't have a scope at hand, but I can confirm that it seems to work. Independent on that the manual seems to be wrong regarding the description of the calculation of DIV as discussed with @dylad already, I get now the following SDCLK frequencies:

• 200 kHz during the card initialization and identification
• 12.5 MHz when the card is switch to the default speed of 25 MHz and
• 25 MHz after the HighSpeed capability test when the card is switched to the high speed mode of 50 MHz.

All speeds should be exactly the double.

Some debug output from _set_speed in sdhc.c:

data dir: /sd0
_set_speed fsdhc=400000 clk=200000000 div=250
_set_speed fsdhc=25000000 clk=200000000 div=4
_set_speed fsdhc=50000000 clk=200000000 div=2
_set_speed fsdhc=400000 clk=200000000 div=250
_set_speed fsdhc=25000000 clk=200000000 div=4
_set_speed fsdhc=50000000 clk=200000000 div=2
main(): This is RIOT! (Version: 2023.07-devel-286-gf2c28-cpu/samd5x/enable_fdpll1_test)

According to the manual, F_SDCLK = F_BASECLK / (2 × DIV). This should give for example for clk=200000000 and div=250 exactly the expected fsdhc=400000. But the result is a clock speed of only 200 kHz.

[dylad]

@benpicco I've slightly change the fdpll_init() function again.
The consumption should be back to normal.
I didn't change the behavior for FDPLL0. I'd like to have a deeper look at the consumption optimization. So this should belongs to another PR.

[gschorcht]

@benpicco Can you confirm that there is no change in power consumption if FDPLL1 isn't used?

[benpicco]

Now it hangs on init:

0x00000d78 in fdpll_init (flags=128 '\200', f_cpu=200000000, idx=1 '\001') at /home/benpicco/dev/RIOT/cpu/samd5x/cpu.c:218
218	    while (!(OSCCTRL->Dpll[idx].DPLLSTATUS.bit.CLKRDY &&
                     OSCCTRL->Dpll[idx].DPLLSTATUS.bit.LOCK)) {}

It works when I remove the ONDEMAND bit, but then power consumption is of course up again

--- a/cpu/samd5x/cpu.c
+++ b/cpu/samd5x/cpu.c
@@ -354,7 +354,7 @@ void cpu_init(void)
     xosc_init(0);
     xosc_init(1);
     fdpll_init(0, CLOCK_CORECLOCK * DPLL_DIV, 0);
-    fdpll_init(1, MHZ(200), OSCCTRL_DPLLCTRLA_ONDEMAND);
+    fdpll_init(1, MHZ(200), 0);
 
     /* select the source of the main clock */
     if (USE_DPLL) {

[dylad]

I'll have my board back tomorrow. I'll give it a try.

[dylad]

FDPLL0 no more blocks with ONDEMAND and power consumption is back to normal.

[benpicco]

bors merge

[bors]

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• Murdock
• static-tests
• tools-build-success

[dylad]

Thanks for the review & merge !