stm32g431rb: PLL setting result to slow exccution

[baw-serafin]

Describe the bug
I have a small test program regitering an interrupt, the irq sets a semaphore and wake a Thread who toggles an output. On the 'nucleo_g4rb31' this is slower than on the 'f401re' even though it should have double the CPU-Freq. If I reconfigure the pll manually to 170Mhz the time goes down a lot.

In numbers:

• f401re 12.4 us
• g431rb 13.4us
• g431rb custom pll 5.3us

To Reproduce

• Build attached example

My PL settings:

CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=170000000
CONFIG_CLOCK_STM32_PLL_SRC_HSI=n
CONFIG_CLOCK_STM32_PLL_SRC_HSE=y
CONFIG_CLOCK_STM32_PLL_M_DIVISOR=6
CONFIG_CLOCK_STM32_PLL_N_MULTIPLIER=85
CONFIG_CLOCK_STM32_PLL_P_DIVISOR=7
CONFIG_CLOCK_STM32_PLL_Q_DIVISOR=2
CONFIG_CLOCK_STM32_PLL_R_DIVISOR=2

Expected behavior
g431 shoudl be fatser than f401

Impact
Slow excecution

Environment (please complete the following information):

• Debian 10
• Zehpyrtoolchain
• v2.1.0

[martinjaeger]

Maybe this is related to #22245?

The wrong SystemCoreClock setting happens also with HSE, but as far as I could see, the actual frequency is correct. We would need to know if your example program depends on SystemCoreClock variable in some way.

[baw-serafin]

No my program depends on nothing really, Sorry was nor really able to upload this correctly because it touches customer code etc so I will paste here:
main.c you can ignore the Configs and just use semaphore version

#include <zephyr.h>
#include <device.h>
#include <kernel.h>
#include <drivers/gpio.h>
#include <sys/printk.h>

#define PRIORITY -2 // USE COOPERATIVE SCHEDULING

#ifdef CONFIG_TASKSWITCHER_USE_QUEUE
K_MSGQ_DEFINE(input_queue, sizeof(uint32_t), 8, 4);
#else
K_SEM_DEFINE(input_sem, 0, 1)
#endif


void input_handler(struct device *port, struct gpio_callback *cb,
		    u32_t pins)
{
#ifdef CONFIG_TASKSWITCHER_USE_QUEUE
#ifdef CONFIG_TASKSWITCHER_NO_READBACK
	static uint32_t val = 0;
	val = val % 2;
	k_msgq_put(&input_queue, &val, K_NO_WAIT);
	val++;
#else //CONFIG_TASKSWITCHER_NO_READBACK
	int ret;
	uint32_t val;

	ret = gpio_pin_read(port, DT_GPIO_KEYS_IN_0_GPIOS_PIN, &val);
	k_msgq_put(&input_queue, &val, K_NO_WAIT);
#endif //CONFIG_TASKSWITCHER_NO_READBACK
#else //CONFIG_TASKSWITCHER_USE_QUEUE
	k_sem_give(&input_sem);
#endif //CONFIG_TASKSWITCHER_USE_QUEUE
}

void output_ctrl(void *p1, void *p2, void *p3) {

	struct device *outport;

	outport = device_get_binding(DT_GPIO_LEDS_OUT_0_GPIOS_CONTROLLER);
	if (!outport) {
		printk("Could not bind outport\n");
		return;
	}

	gpio_pin_configure(outport, DT_GPIO_LEDS_OUT_0_GPIOS_PIN, GPIO_DIR_OUT);
	gpio_pin_write(outport, DT_GPIO_LEDS_OUT_0_GPIOS_PIN, 1);

#ifdef CONFIG_TASKSWITCHER_USE_QUEUE
	uint32_t val;
	while(true) {
		k_msgq_get(&input_queue, &val, K_FOREVER);
		gpio_pin_write(outport, DT_GPIO_LEDS_OUT_0_GPIOS_PIN, val);
	}
#else
	uint32_t cnt = 0;
	while (true) {
		k_sem_take(&input_sem, K_FOREVER);
		gpio_pin_write(outport, DT_GPIO_LEDS_OUT_0_GPIOS_PIN, cnt % 2);
		cnt++;
	}
#endif
}

static struct gpio_callback gpio_cb;

void main(void)
{
	printk("Hello World! %s\n", CONFIG_BOARD);

	struct device *inport;

	inport = device_get_binding(DT_GPIO_KEYS_IN_0_GPIOS_CONTROLLER);
	if (!inport) {
		printk("Could not bind inport\n");
		return;
	}

	gpio_pin_configure(inport, DT_GPIO_KEYS_IN_0_GPIOS_PIN,
			   GPIO_DIR_IN | GPIO_INT |  GPIO_PUD_PULL_UP |
			   GPIO_INT_EDGE | GPIO_INT_DOUBLE_EDGE);

	gpio_init_callback(&gpio_cb, input_handler, BIT(DT_GPIO_KEYS_IN_0_GPIOS_PIN));

	gpio_add_callback(inport, &gpio_cb);
	gpio_pin_enable_callback(inport, DT_GPIO_KEYS_IN_0_GPIOS_PIN);
}

K_THREAD_DEFINE(out_thrd, CONFIG_MAIN_STACK_SIZE, output_ctrl, NULL, NULL, NULL,
		PRIORITY, 0, K_NO_WAIT);

The needed overlay (works for all nucleos just adjust the name): 'nucleo_.overlay'

/ {
	outputs {
		compatible = "gpio-leds";
		out_0: out_0 {
			gpios = <&arduino_header 14 0>;
			label = "Ooutput 0";
		};
	};

	inputs {
		compatible = "gpio-keys";
		in_0: in_0 {
			label = "Input 0";
			gpios = <&arduino_header 15 0>;
		};
	};
};

[FRASTM]

Note that on stm32g4xx, a system clock freq higher than 150MHz requires the main regulator in range 1 Boost mode.
So for setting the PLL at 170MHz, must the R1Mode be 0 in the Power control register 5
This is done using the LL_PWR_EnableRange1BoostMode()

[baw-serafin]

Note that on stm32g4xx, a system clock freq higher than 150MHz requires the main regulator in range 1 Boost mode.
So for setting the PLL at 170MHz, must the R1Mode be 0 in the Power control register 5
This is done using the LL_PWR_EnableRange1BoostMode()

I read that and ignored it as it worked for me. Not suggesting to increase the default to 170MhZ but 150Mhz should not be 2.5 time slower than 17'0Mhz - so something is wrong...

[FRASTM]

The AHB Prescaler should also be 1 (not divided)

[FRASTM]

Do you see the correct freq. on nucleo_g431rb with zephyrproject-rtos/hal_stm32#36

I also recommend adding PR #22311

[baw-serafin]

I can confirm that it behaves as expected on master with a delay of 6.1us which is just slightly slower than my settings. This is probably cause by the clock difference 150MHz instead of 170MHz. So the issue seems fixed on master

[erwango]

I'm closing the issue, as it is reported fixed on master.
#22245 remains open (with #22311 as fix)