[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8c1f0a4f-8a8b-41e7-b7f1-4c5a38ec7c1a@linaro.org>
Date: Tue, 20 Feb 2024 11:53:22 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Sam Protsenko <semen.protsenko@...aro.org>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Chanwoo Choi <cw00.choi@...sung.com>
Cc: Alim Akhtar <alim.akhtar@...sung.com>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Tomasz Figa <tomasz.figa@...il.com>,
linux-samsung-soc@...r.kernel.org, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 07/16] clk: samsung: Pass actual clock controller base
address to CPU_CLK()
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
Powered by blists - more mailing lists