On 16/02/2024 23:32, Sam Protsenko wrote: > The documentation for struct exynos_cpuclk says .ctrl_base field should > contain the controller base address. But in reality all Exynos clock > drivers are passing CPU_SRC register address via CPU_CLK() macro, which > in turn gets assigned to mentioned .ctrl_base field. Because CPU_SRC > address usually already has 0x200 offset from controller's base, all > other register offsets in clk-cpu.c (like DIVs and MUXes) are specified > as offsets from CPU_SRC offset, and not from controller's base. That > makes things confusing and not consistent with register offsets provided > in Exynis clock drivers, also breaking the contract for .ctrl_base field Typo: Exynos > as described in struct exynos_cpuclk doc. Rework all register offsets in > clk-cpu.c to be actual offsets from controller's base, and fix offsets > provided to CPU_CLK() macro in all Exynos clock drivers. Change is fine and makes sense on devices having separate CPU clock controller. That's not the case for: 1. Exynos3250: dedicated CPU clock controller space, but we merged it into one driver/binding. 2. Exynos4 and 5250: no obvious dedicated CPU clock controller, but register layout suggests that there is such, just not explicit. In all these cases you provide not the correct offset against explicit or implicit CPU base, but from main clock controller base. Mention it briefly in above commit msg. Best regards, Krzysztof
