On Sun, Dec 17, 2023 at 03:05:01PM -0800, Stephen Boyd wrote: > Quoting Ben Wolsieffer (2023-10-02 11:08:53) > > The stm32-power-config syscon (PWR peripheral) is used in this driver > > and the STM32 RTC driver to enable write access to backup domain > > registers. The syscon's clock has a gate controlled by this clock > > driver, but this clock is currently not registered in the device tree. > > This only happens to work currently because all relevant clock setup and > > RTC initialization happens before clk_disabled_unused(). After this > > point, all syscon register writes are ignored. > > Seems like we should mark those clks as CLK_IGNORE_UNUSED and add a > comment to that fact. That seems like a worse solution than specifying the clock dependency in the device tree. > > > > > If we simply add the syscon clock in the device tree, we end up with a > > circular dependency because the clock has not been registered at the > > point this driver requests the syscon. > > > > This patch avoids this circular dependency by moving the syscon lookup > > after the clocks are registered. This does appear to create a possible > > race condition where someone could attempt to perform an operation on a > > backup domain clock before the syscon has been initialized. This would > > result in the operation having no effect because backup domain writes > > could not be enabled. I'm not sure if this is a problem or if there is > > a way to avoid it. > > There's no comment in the code that says the regmap must be set there > instead of earlier. What's to stop someone from tripping over this > problem later? At the least, please add a comment. Yeah, I'll fix that. Do you have any thoughts on the race condition I described? Should I add some kind of locking to block enable/disable_power_domain_write_protection() until stm32f4_rcc_init() attempts to initialize the syscon? Thank you, Ben
