| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CADrjBPov=7t876dqpTS71j_xNFOrJv7_Ym7abYVLzjypoOYKng@mail.gmail.com>
Date: Tue, 21 Oct 2025 22:03:26 +0100
From: Peter Griffin <peter.griffin@...aro.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>,
André Draszik <andre.draszik@...aro.org>,
Tudor Ambarus <tudor.ambarus@...aro.org>, Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Sam Protsenko <semen.protsenko@...aro.org>,
Sylwester Nawrocki <s.nawrocki@...sung.com>, Chanwoo Choi <cw00.choi@...sung.com>,
Will McVicker <willmcvicker@...gle.com>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>, kernel-team@...roid.com
Subject: Re: [PATCH 8/9] clk: samsung: gs101: Enable auto_clock_gate mode for
each gs101 CMU
Hi Krzysztof,
Thanks for the review feedback.
On Tue, 21 Oct 2025 at 20:48, Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On 13/10/2025 22:51, Peter Griffin wrote:
> > Enable auto clock mode, and define the additional fields which are used
> > when this mode is enabled.
> >
> > /sys/kernel/debug/clk/clk_summary now reports approximately 308 running
> > clocks and 298 disabled clocks. Prior to this commit 586 clocks were
> > running and 17 disabled. To ensure compatability with older DTs the
>
> Typo
Will fix.
>
> > resource size is checked and an error issued if the DT needs updating.
>
> I fail to see how you keek it compatible. See further.
>
> >
> > Signed-off-by: Peter Griffin <peter.griffin@...aro.org>
> > ---
> > drivers/clk/samsung/clk-gs101.c | 80 +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 80 insertions(+)
> >
> > diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
> > index 70b26db9b95ad0b376d23f637c7683fbc8c8c600..baf41ae6c9e2480cb83531acf7eae190c6aff819 100644
> > --- a/drivers/clk/samsung/clk-gs101.c
> > +++ b/drivers/clk/samsung/clk-gs101.c
> > @@ -9,6 +9,7 @@
> > #include <linux/clk-provider.h>
> > #include <linux/mod_devicetable.h>
> > #include <linux/of.h>
> > +#include <linux/of_address.h>
> > #include <linux/platform_device.h>
> >
> > #include <dt-bindings/clock/google,gs101.h>
> > @@ -17,6 +18,8 @@
> > #include "clk-exynos-arm64.h"
> > #include "clk-pll.h"
> >
> > +int check_cmu_res_size(struct device_node *np);
> > +
> > /* NOTE: Must be equal to the last clock ID increased by one */
> > #define CLKS_NR_TOP (CLK_GOUT_CMU_TPU_UART + 1)
> > #define CLKS_NR_APM (CLK_APM_PLL_DIV16_APM + 1)
> > @@ -26,6 +29,10 @@
> > #define CLKS_NR_PERIC0 (CLK_GOUT_PERIC0_SYSREG_PERIC0_PCLK + 1)
> > #define CLKS_NR_PERIC1 (CLK_GOUT_PERIC1_SYSREG_PERIC1_PCLK + 1)
> >
> > +#define GS101_GATE_DBG_OFFSET 0x4000
> > +#define GS101_DRCG_EN_OFFSET 0x104
> > +#define GS101_MEMCLK_OFFSET 0x108
> > +
> > /* ---- CMU_TOP ------------------------------------------------------------- */
> >
> > /* Register Offset definitions for CMU_TOP (0x1e080000) */
> > @@ -1433,6 +1440,9 @@ static const struct samsung_cmu_info top_cmu_info __initconst = {
> > .nr_clk_ids = CLKS_NR_TOP,
> > .clk_regs = cmu_top_clk_regs,
> > .nr_clk_regs = ARRAY_SIZE(cmu_top_clk_regs),
> > + .auto_clock_gate = true,
> > + .gate_dbg_offset = GS101_GATE_DBG_OFFSET,
> > + .option_offset = CMU_CMU_TOP_CONTROLLER_OPTION,
> > };
> >
> > static void __init gs101_cmu_top_init(struct device_node *np)
> > @@ -1900,6 +1910,11 @@ static const struct samsung_gate_clock apm_gate_clks[] __initconst = {
> > CLK_CON_GAT_GOUT_BLK_APM_UID_XIU_DP_APM_IPCLKPORT_ACLK, 21, CLK_IS_CRITICAL, 0),
> > };
> >
> > +static const unsigned long dcrg_memclk_sysreg[] __initconst = {
> > + GS101_DRCG_EN_OFFSET,
> > + GS101_MEMCLK_OFFSET,
> > +};
> > +
> > static const struct samsung_cmu_info apm_cmu_info __initconst = {
> > .mux_clks = apm_mux_clks,
> > .nr_mux_clks = ARRAY_SIZE(apm_mux_clks),
> > @@ -1912,6 +1927,12 @@ static const struct samsung_cmu_info apm_cmu_info __initconst = {
> > .nr_clk_ids = CLKS_NR_APM,
> > .clk_regs = apm_clk_regs,
> > .nr_clk_regs = ARRAY_SIZE(apm_clk_regs),
> > + .sysreg_clk_regs = dcrg_memclk_sysreg,
> > + .nr_sysreg_clk_regs = ARRAY_SIZE(dcrg_memclk_sysreg),
> > + .auto_clock_gate = true,
> > + .gate_dbg_offset = GS101_GATE_DBG_OFFSET,
> > + .drcg_offset = GS101_DRCG_EN_OFFSET,
> > + .memclk_offset = GS101_MEMCLK_OFFSET,
> > };
> >
> > /* ---- CMU_HSI0 ------------------------------------------------------------ */
> > @@ -2375,7 +2396,14 @@ static const struct samsung_cmu_info hsi0_cmu_info __initconst = {
> > .nr_clk_ids = CLKS_NR_HSI0,
> > .clk_regs = hsi0_clk_regs,
> > .nr_clk_regs = ARRAY_SIZE(hsi0_clk_regs),
> > + .sysreg_clk_regs = dcrg_memclk_sysreg,
> > + .nr_sysreg_clk_regs = ARRAY_SIZE(dcrg_memclk_sysreg),
> > .clk_name = "bus",
> > + .auto_clock_gate = true,
> > + .gate_dbg_offset = GS101_GATE_DBG_OFFSET,
> > + .option_offset = HSI0_CMU_HSI0_CONTROLLER_OPTION,
> > + .drcg_offset = GS101_DRCG_EN_OFFSET,
> > + .memclk_offset = GS101_MEMCLK_OFFSET,
> > };
> >
> > /* ---- CMU_HSI2 ------------------------------------------------------------ */
> > @@ -2863,7 +2891,14 @@ static const struct samsung_cmu_info hsi2_cmu_info __initconst = {
> > .nr_clk_ids = CLKS_NR_HSI2,
> > .clk_regs = cmu_hsi2_clk_regs,
> > .nr_clk_regs = ARRAY_SIZE(cmu_hsi2_clk_regs),
> > + .sysreg_clk_regs = dcrg_memclk_sysreg,
> > + .nr_sysreg_clk_regs = ARRAY_SIZE(dcrg_memclk_sysreg),
> > .clk_name = "bus",
> > + .auto_clock_gate = true,
> > + .gate_dbg_offset = GS101_GATE_DBG_OFFSET,
> > + .option_offset = HSI2_CMU_HSI2_CONTROLLER_OPTION,
> > + .drcg_offset = GS101_DRCG_EN_OFFSET,
> > + .memclk_offset = GS101_MEMCLK_OFFSET,
> > };
> >
> > /* ---- CMU_MISC ------------------------------------------------------------ */
> > @@ -3423,11 +3458,37 @@ static const struct samsung_cmu_info misc_cmu_info __initconst = {
> > .nr_clk_ids = CLKS_NR_MISC,
> > .clk_regs = misc_clk_regs,
> > .nr_clk_regs = ARRAY_SIZE(misc_clk_regs),
> > + .sysreg_clk_regs = dcrg_memclk_sysreg,
> > + .nr_sysreg_clk_regs = ARRAY_SIZE(dcrg_memclk_sysreg),
> > .clk_name = "bus",
> > + .auto_clock_gate = true,
> > + .gate_dbg_offset = GS101_GATE_DBG_OFFSET,
> > + .option_offset = MISC_CMU_MISC_CONTROLLER_OPTION,
> > + .drcg_offset = GS101_DRCG_EN_OFFSET,
> > + .memclk_offset = GS101_MEMCLK_OFFSET,
> > };
> >
> > +/* for old DT compatbility with incorrect CMU size*/
> > +int check_cmu_res_size(struct device_node *np)
> > +{
> > + struct resource res;
> > + resource_size_t size;
> > +
> > + if (of_address_to_resource(np, 0, &res))
> > + return -ENODEV;
> > +
> > + size = resource_size(&res);
> > + if (size != 0x10000) {
> > + pr_warn("%pOF: resource to small. Please update your DT\n", np);
> > + return -ENODEV;
> > + }
> > + return 0;
> > +}
> > +
> > static void __init gs101_cmu_misc_init(struct device_node *np)
> > {
> > + if (check_cmu_res_size(np))
> > + return;
>
> You will not register CMU on old DTB.
By "compatible" I meant the driver detects an old DTB with an
incorrect reg size and issues an error message on the console to
update your DT (as opposed to crashing trying to access a register
that hasn't been mapped).
Is it enough to re-word the commit message to make it clearer what will happen?
An alternative might be to try registering all the gates in manual
mode, but that seems like it would add more complexity for not much
benefit. It would also require that clk_ignore_unused kernel parameter
to have been passed (as manual clock mode has never worked without it)
and whilst it might boot today I imagine it would bitrot fast as
additional CMUs are added (and thus probably crash in a much more
obscure way).
Peter
Powered by blists - more mailing lists