Re: [PATCH 2/3] clk: amlogic: drop clk_regmap tables

[Jerome Brunet <jbrunet@baylibre.com>]

On Tue 07 Jan 2025 at 13:28, Stephen Boyd <sboyd@kernel.org> wrote:

> Quoting Jerome Brunet (2025-01-07 06:46:41)
>> 
>> So, if it's OK, I'll resend this series with a temporary solution to
>> remove tables. Removing the table simplify the other clean-up I have
>> already line-up and avoid some unnecessary diffs. I'll circle back to
>> reworking the init_data afterward.
>> 
>
> Ok, sure. Let's see how it goes.


Hey Stephen,

While implementing a temporary solution I've been thinking about the
longer term one (I'd rather not end up stuck on a temporary one, so it
has been bothering me a bit)

I'll reformulate what I'm trying to acheive.

I'd like to register controller init hook to apply on all the clocks of
a particular type. The reason to do that is to drop the big clk_regmap
table that are a pain to maintain (in addition to be ugly). I hoped it
would also save a bit of memory.

The solutions we've been discussing so far feels like we are moving
around the problem, recreating the memory saved somewhere else,
perhaps in a more complicated way. I'd like to find something more
convinient to use, which does not scale the memory used with the number
of clock registered. The point is not a different hook for clk_hw after
all.

Here is an idea, how about list of hook identified by ops and controller ?

The node would look like this

struct clk_type_init_node {
       struct list_head	        entry;
       
       struct device_node	*of_node;
       struct device		*dev;
       const struct clk_ops	*ops;

       int (*init_hook)(struct clk_hw *hw);
};

The change would be minimal in core CCF, just searching the list for a
match in clk_register. On most platform the list would be empty so there
is virtually no penalty when it is not used.

From the controller, the usage would be very simple, just calling a
function before registering the clocks, something like:

int clk_type_register_dev_hook(struct device *dev,
                               const struct clk_ops *ops,
                               int (*init_hook)(struct clk_hw *hw))

or the 'of_node' equivalent.

I admit this is heavily inspired by how devres works :) but it does solve
the early clock controller problem and does not scale with the number of
clock registered.

Would you be ok with this approach ?

-- 
Jerome