On 29.08.2023 11:56, Krzysztof Kozlowski wrote: > On 29/08/2023 11:18, Mateusz Majewski wrote: >> This clock only controls the register operations. The gain in power >> efficiency is therefore quite dubious, while there is price of added >> complexity that is important to get right (as a register operation might >> outright hang the CPU if the clock is not enabled). > So once it is done right, this stops being argument. The benefit is to > keep this clock disabled most of the time, which now we lost. > > I don't find this patch correct approach. I've suggested this change while playing with this driver. For me turning AHB clock on/off during normal driver operation seems to be over-engineering and really gives no real power saving benefits, especially if thermal driver is the only one that does such fine-grained clock management (none of the Exynos supported in mainline does that). Removing it simplifies code and makes it easier to understand or read, as the current code already was somehow problematic to understand and unintuitive: https://lore.kernel.org/all/c3258cb2-9a56-d048-5738-1132331a157d@xxxxxxxxxx/ Taking into account that the driver is not really maintained, making it simpler without noticeable feature loss counts as a benefit for me. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
