linux-kernel - Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

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: <20241128-monstrous-embargo-a665d921410d@wendy>
Date: Thu, 28 Nov 2024 10:36:16 +0000
From: Conor Dooley <conor.dooley@...rochip.com>
To: Stephen Boyd <sboyd@...nel.org>
CC: Conor Dooley <conor@...nel.org>, Jerome Brunet <jbrunet@...libre.com>,
	Neil Armstrong <neil.armstrong@...aro.org>, <linux-kernel@...r.kernel.org>,
	Daire McNamara <daire.mcnamara@...rochip.com>,
	<pierre-henry.moussay@...rochip.com>,
	<valentina.fernandezalanis@...rochip.com>, Michael Turquette
	<mturquette@...libre.com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Jassi Brar <jassisinghbrar@...il.com>, Lee Jones
	<lee@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
	<palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, Kevin Hilman
	<khilman@...libre.com>, Martin Blumenstingl
	<martin.blumenstingl@...glemail.com>, Philipp Zabel <p.zabel@...gutronix.de>,
	<linux-riscv@...ts.infradead.org>, <linux-clk@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-amlogic@...ts.infradead.org>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to
 common code

On Thu, Nov 14, 2024 at 05:29:54PM -0800, Stephen Boyd wrote:
> Quoting Conor Dooley (2024-11-06 04:56:25)
> > My use case doesn't
> > actually need the registration code changes either as, currently, only reg
> > gets set at runtime, but leaving that out is a level of incomplete I'd not
> > let myself away with.
> > Obviously shoving the extra members into the clk structs has the downside
> > of taking up a pointer and a offset worth of memory for each clock of
> > that type registered, but it is substantially easier to support devices
> > with multiple regmaps that way. Probably moot though since the approach you
> > suggested in the thread linked above that implements a clk_hw_get_regmap()
> > has to store a pointer to the regmap's identifier which would take up an
> > identical amount of memory.
> 
> We don't need to store the regmap identifier in the struct clk. We can
> store it in the 'struct clk_init_data' with some new field, and only do
> that when/if we actually need to. We would need to pass the init data to
> the clk_ops::init() callback though. We currently knock that out during
> registration so that clk_hw->init is NULL. Probably we can just set that
> to NULL after the init routine runs in __clk_core_init().
> 
> Long story short, don't add something to 'struct clk_core', 'struct
> clk', or 'struct clk_hw' for these details. We can have a 'struct
> clk_regmap_hw' that everyone else can build upon:
> 
>   struct clk_regmap_hw {
>         struct regmap *regmap;
>         struct clk_hw hw;
>   };

What's the point of this? I don't understand why you want to do this over
what clk_divider et al already do, where clk_hw and the iomem pointer
are in the struct itself.

> 
> and then set the regmap pointer during registration in
> clk_hw_init_regmap().
> 
> int clk_hw_init_regmap(struct clk_hw *hw)
> {
> 	struct device *dev;
> 	struct regmap *regmap;
> 	struct clk_regmap_hw *rhw;
> 
> 	rhw = clk_hw_to_clk_regmap_hw(hw);
> 
> 	dev = clk_hw_get_dev(hw);
> 	if (!dev)
> 		return -EINVAL;
> 
> 	regmap = dev_get_regmap(dev, hw->init->regmap_name);
> 	if (!regmap)
> 		return -EINVAL; // Print helpful message
> 	rhw->regmap = regmap;
> 
> 	return 0;
> }

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)