linux-kernel - Re: [PATCH 3/3] clk: samsung: allow runtime PM with auto clock gating

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <872f487ab2c5f6c2b1b2d7f7ee3cc26beec7970a.camel@linaro.org>
Date: Mon, 19 Jan 2026 15:22:16 +0000
From: André Draszik <andre.draszik@...aro.org>
To: Krzysztof Kozlowski <krzk@...nel.org>, Sylwester Nawrocki	
 <s.nawrocki@...sung.com>, Chanwoo Choi <cw00.choi@...sung.com>, Alim Akhtar
	 <alim.akhtar@...sung.com>, Michael Turquette <mturquette@...libre.com>, 
 Stephen Boyd <sboyd@...nel.org>
Cc: Peter Griffin <peter.griffin@...aro.org>, Tudor Ambarus	
 <tudor.ambarus@...aro.org>, Will McVicker <willmcvicker@...gle.com>, Juan
 Yescas <jyescas@...gle.com>, linux-samsung-soc@...r.kernel.org,
 linux-clk@...r.kernel.org, 	linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 3/3] clk: samsung: allow runtime PM with auto clock
 gating

On Mon, 2026-01-19 at 15:00 +0000, André Draszik wrote:
> Hi Krzysztof,
> 
> On Sat, 2026-01-17 at 20:25 +0100, Krzysztof Kozlowski wrote:
> > On 09/01/2026 18:27, André Draszik wrote:
> > > When automatic clock gating is enabled, runtime PM (RPM) isn't entered
> > > even if enabled for a CMU if a sysreg clock exists and is provided by
> > > this CMU (as is generally the case).
> > > 
> > > The reason is that this driver acquires a CMU's sysreg registers using
> > > syscon_regmap_lookup_by_phandle() which ends up preparing the sysreg
> > > clock. Given the sysreg clock is provided by this CMU, this CMU's usage
> > > count is therefore bumped and RPM can not be entered as this CMU never
> > > becomes idle.
> > > 
> > > Switch to using device_node_to_regmap() which doesn't handle resources
> > > (the clock), leaving the CMU's usage count unaffected.
> > > 
> > > Note1: sysreg clock handling is completely removed with this commit
> > 
> > I miss where do you remove in this commit the sysreg clock handling?
> 
> The CMU driver originally used syscon_regmap_lookup_by_phandle() for sysreg,
> which does clk_get() and clk_prepare(), and then implicitly and for each
> register access clk_enable()/disable() for the clock specified in DT for the
> (sysreg) node.
> 
> This commit changes it to using device_node_to_regmap(), which does nothing
> with the clock (or any other resources). I opted to have the CMU driver still
> do a clk_get(), so that the relationship is still visible, e.g. in
> $debugfs/clk/clk_summary.
> 
> > > because sysreg register access is only required during suspend/resume.
> > > In the runtime suspend case, we would have to enable the clock to read
> > > the registers, but we can not do that as that would cause a resume of
> > > this driver which is not allowed. This is not a problem because we
> > > would only need to handle the clock manually if automatic clock gating
> > > wasn't enabled in the first. This code is only relevant if automatic
> > > clock gating is enabled, though.
> > > 
> > > Fixes: 298fac4f4b96 ("clk: samsung: Implement automatic clock gating mode for CMUs")
> > > Signed-off-by: André Draszik <andre.draszik@...aro.org>
> > > ---
> > >  drivers/clk/samsung/clk.c | 92 +++++++++++++++++++++++++++++++++++------------
> > >  drivers/clk/samsung/clk.h |  2 ++
> > >  2 files changed, 71 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> > > index 9f68f079fd552f8dfb6898dbfb47dec0e84c626c..6515df81fcbc79b90f5262843e67575f6a4e0dda 100644
> > > --- a/drivers/clk/samsung/clk.c
> > > +++ b/drivers/clk/samsung/clk.c
> > > @@ -9,11 +9,13 @@
> > >   */
> > >  
> > >  #include <linux/slab.h>
> > > +#include <linux/clk.h>
> > >  #include <linux/clkdev.h>
> > >  #include <linux/clk-provider.h>
> > >  #include <linux/io.h>
> > >  #include <linux/mfd/syscon.h>
> > >  #include <linux/mod_devicetable.h>
> > > +#include <linux/of.h>
> > >  #include <linux/of_address.h>
> > >  #include <linux/regmap.h>
> > >  #include <linux/syscore_ops.h>
> > > @@ -489,6 +491,50 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> > >  		samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
> > >  }
> > >  
> > > +static int samsung_get_sysreg_regmap(struct device_node *np,
> > > +				     struct samsung_clk_provider *ctx)
> > > +{
> > > +	struct device_node *sysreg_np;
> > > +	struct clk *sysreg_clk;
> > > +	struct regmap *regmap;
> > > +	int ret;
> > > +
> > > +	sysreg_np = of_parse_phandle(np, "samsung,sysreg", 0);
> > > +	if (!sysreg_np)
> > > +		return -ENODEV;
> > > +
> > > +	sysreg_clk = of_clk_get(sysreg_np, 0);
> > 
> > I don't think you should be poking clock of some other device. This
> > clearly breaks encapsulation. What sysreg is needs to do to access
> > registers, is only business of sysreg. No other drivers should
> > re-implement this.
> 
> Similarly, to how other drivers (like UFS and peric/USI) also poke in
> the sysreg region in a similar way, the CMU related parts of sysreg are
> handled in this driver, though, as only the CMU driver knows how to
> configure them based on the gating (auto vs. manual).
> 
> No separate driver was sysreg was added when this code was introduced.
> The clk_get() used to be part of syscon_regmap_lookup_by_phandle(), and
> I chose to keep only that, as per above.
> 
> I propose to add an actual sysreg driver once the need arises, then
> this can also move, but for now keeping it here seems straight forward.
> 
> Is that OK with you?

Forgot to say, I can of course remove all clock related code (clk_get() etc).

What do you think? Is that acceptable?


CHeers,
Andre'