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

[Stephen Boyd <sboyd@kernel.org>]

Quoting Conor Dooley (2024-11-28 02:36:16)
> 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.

Can you give an example? I don't understand what you're suggesting. I
prefer a struct clk_regmap_hw like above so that the existing struct
clk_hw in the kernel aren't increased by a pointer. SoC drivers can use
the same struct as a replacement for their struct clk_hw member today.