STM32H7 MCO select and prescaler masks are incorrect

[attie]

Describe the bug

zephyr/include/zephyr/dt-bindings/clock/stm32_common_clocks.h

Lines 46 to 50 in 8a72d77

	#define STM32_DT_CLOCK_SELECT(val, mask, shift, reg) \
	((((reg) & STM32_DT_CLKSEL_REG_MASK) << STM32_DT_CLKSEL_REG_SHIFT) | \
	(((shift) & STM32_DT_CLKSEL_SHIFT_MASK) << STM32_DT_CLKSEL_SHIFT_SHIFT) | \
	(((mask) & STM32_DT_CLKSEL_MASK_MASK) << STM32_DT_CLKSEL_MASK_SHIFT) | \
	(((val) & STM32_DT_CLKSEL_VAL_MASK) << STM32_DT_CLKSEL_VAL_SHIFT))

zephyr/include/zephyr/dt-bindings/clock/stm32h7_clock.h

Lines 114 to 117 in 8a72d77

	#define MCO1_SEL(val) STM32_DT_CLOCK_SELECT((val), 0xF, 22, CFGR_REG)
	#define MCO1_PRE(val) STM32_DT_CLOCK_SELECT((val), 0x7, 18, CFGR_REG)
	#define MCO2_SEL(val) STM32_DT_CLOCK_SELECT((val), 0xF, 29, CFGR_REG)
	#define MCO2_PRE(val) STM32_DT_CLOCK_SELECT((val), 0x7, 25, CFGR_REG)

I believe the masks for these macros should be swapped (SEL vs PRE)... can someone confirm or deny this?

#define MCO1_SEL(val)           STM32_DT_CLOCK_SELECT((val), 0x7, 22, CFGR_REG)
#define MCO1_PRE(val)           STM32_DT_CLOCK_SELECT((val), 0xF, 18, CFGR_REG)
#define MCO2_SEL(val)           STM32_DT_CLOCK_SELECT((val), 0x7, 29, CFGR_REG)
#define MCO2_PRE(val)           STM32_DT_CLOCK_SELECT((val), 0xF, 25, CFGR_REG)

Steps to reproduce

Untested / unverified at the moment...

Relevant log output

None

Impact

Functional Limitation - Some features not working as expected, but system usable.

Environment

• Linux fef609931a42 6.5.0-35-generic #35~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue May 7 09:00:52 UTC 2 x86_64 x86_64 x86_64 GNU/Linux
• arm-none-eabi-gcc (Arm GNU Toolchain 13.2.rel1 (Build arm-13.7)) 13.2.1 20231009
• Zephyr 90cd350, still present in 8a72d77

Additional context

I'm using STM32H753, and luckily I don't think this will affect me, as I'm not using MCO2 (so the LSB of MCO2PRE won't be clobbered), and have low values for MCO1 prescaler (so MSB won't be masked off).

I've not verified values for the other families.

I'll try to set a large prescaler on MCO1 to confirm this if nobody else can confirm.

[github-actions]

Hi @attie! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[NickIOT72]

I made a comparison of the MCO register mask and shift values (PRE and SEL) set up on the file zephyr/include/zephyr/dt-bindings/clock/stm32f4_clock.h file with the RCC clock configuration register description obtained from the datasheet and all these parameters configuration match with their corresponding from the datasheet

zephyr/include/zephyr/dt-bindings/clock/stm32f4_clock.h

Lines 63 to 67 in 8a72d77

	#define I2S_SEL(val) STM32_DT_CLOCK_SELECT((val), 1, 23, CFGR_REG)
	#define MCO1_SEL(val) STM32_DT_CLOCK_SELECT((val), 0x3, 21, CFGR_REG)
	#define MCO1_PRE(val) STM32_DT_CLOCK_SELECT((val), 0x7, 24, CFGR_REG)
	#define MCO2_SEL(val) STM32_DT_CLOCK_SELECT((val), 0x3, 30, CFGR_REG)
	#define MCO2_PRE(val) STM32_DT_CLOCK_SELECT((val), 0x7, 27, CFGR_REG)

Based on this, I think there's a possibility the mask values of the MCO register defined on the file zephyr/include/zephyr/dt-bindings/clock/stm32h7_clock.h are swapped. Nevertheless, Let's wait for someone else can confirm or deny this suggestion.

[henrikbrixandersen]

Please use our bug template when reporting bugs. You need to edit this issue to include the information requested in https://github.com/zephyrproject-rtos/zephyr/blob/main/.github/ISSUE_TEMPLATE/001_bug_report.yml

[attie]

Please use our bug template when reporting bugs.

Apologies, the templates didn't appear when I created the issue (which I thought was odd)... will do that now

I've edited the original message, there isn't a lot of extra detail in there - I hope that's acceptable.
For UX reference, I created this issue from the source listing.

I see now that there are template options...

[gautierg-st]

This issue should be fixed as part of #97725 (which centers on something else but since it touches the STM32_DT_CLOCK_SELECT, I took advantage of it to add the fix).

[attie]

Thanks!