zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.06k stars 6.18k forks source link

stm32: clock_control_on might return before peripheral is properly clocked #73751

Open taltenbach opened 1 month ago

taltenbach commented 1 month ago

Describe the bug According to RM0433, section 8.5.10, the following sequence must be used on STM32H7 MCUs when enabling a peripheral's clock to guarantee the peripheral is properly clocked before accessing its registers:

  1. Set the corresponding PERxEN bit in the RCC_xxxxENR register.
  2. Read back the RCC_xxxxENR register.
  3. Perform a dummy read operation to any register of the enabled peripheral.

(Note that this sequence assumes the domain in which the peripheral is located is in DRun mode. Otherwise, additional steps are required.)

However, the current implementation of clock_control_on on STM32H7 devices currently only performs step 1 and 2 (see here).

This means an access to a register of the enabled peripheral just after the call to clock_control_on might silently fail, leading to hard to investigate issues. This is probably especially true for peripherals having a low clock frequency, in comparison to the CPU clock.

I suspect this issue to also impact other STM32 series. For example, RM0481, section 11.4.16, suggests that a similar sequence should be followed on STM32H5 MCUs.

Relates to https://github.com/zephyrproject-rtos/zephyr/issues/73580.

To Reproduce In theory, it the following sequence could fail on an STM32 MCU:

Expected behavior The peripheral P is properly clock after clock_control_on has returned and the access to the register R succeeds.

Impact According to the reference manual, the peripheral P might not be properly clocked when clock_control_on returns, which means the access to the register R might silently fail.

Logs and console output N/A

Environment

Additional context Relevant quotes from the RM0433 8.5.10:

A maximum delay of two periods of the enabled clock may occur between the enable command and the first rising edge of the clock. The enable command can be the rising edge of the PERxEN bits of RCC_xxxxENR registers, or a kernel clock request asserted by a peripheral.

The following sequence can be followed to avoid this issue: a) Enable the peripheral clocks (i.e. allocate the peripheral) by writing the corresponding PERxEN bit to ‘1’ in the RCC_xxxxENR register, b) Read back the RCCxxxxENR register to make sure that the previous write operation is not pending into a write buffer. c) [...] (step not needed if the domain is in DRun mode)_ d) Perform a dummy read operation into a register of the enabled peripheral. This operation will take at least 2 clock cycles, which is equal to the max delay of the enable command. e) The peripheral can then be used.

erwango commented 1 week ago

Thanks for raising this issue.

This description is correct but some notes should be added:

Expected behavior The peripheral P is properly clock after clock_control_on has returned and the access to the register R succeeds.

Not exactly. API clock_control_on specifies: " On success, the clock is enabled and ready when this function returns.". hence that the clock is enabled and nothing else. Nothing is specified on the requesting peripheral.

Coming to the way to fix this, the most simple would be performing systematic dummy read in each peripheral driver following return from clock_control_on. We can also think of alternatives like getting the requesting peripheral to provide a reg address where the dummy read should be performed in the opaque clock_control_subsys_t structure provided as argument to clock_control_on and have the clock_control driver performing the read. Both have pro's and cons but I'd prefer the first one.