Message-ID: <1j7c4i2xq5.fsf@starbuckisacylon.baylibre.com>
Date: Fri, 21 Mar 2025 16:46:10 +0100
From: Jerome Brunet <jbrunet@...libre.com>
To: Stephen Boyd <sboyd@...nel.org>
Cc: Kevin Hilman <khilman@...libre.com>,  Martin Blumenstingl
 <martin.blumenstingl@...glemail.com>,  Michael Turquette
 <mturquette@...libre.com>,  Neil Armstrong <neil.armstrong@...aro.org>,
  linux-clk@...r.kernel.org,  linux-kernel@...r.kernel.org,
  linux-amlogic@...ts.infradead.org,  linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/3] clk: amlogic: drop clk_regmap tables

On Thu 27 Feb 2025 at 14:55, Stephen Boyd <sboyd@...nel.org> wrote:

> Quoting Jerome Brunet (2025-01-15 07:58:55)
>> 
>> 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.
>
> What are the goals?
>
>  1. Drop clk_regmap table

That my first goal

>  2. Reduce memory

Would be nice

>  3. ??
>
>> 
>> 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.
>
> Why can't we register the clk at the same time? I don't understand why
> we want to search a list to match something up to what could be another
> argument to the clk registration API. Isn't this the same as 
>
>  clk_hw_register_data(struct device *dev, struct clk_hw *hw, const struct clk_register_data *data)
>
> Why is that hard to maintain? Is that because the clk driver is
> registering various different types of clks and it wants to do different
> stuff depending on the type of clk?

Exactly

> Why wouldn't wrapping the clk_ops
> in another struct and then using container_of to find the custom clk_ops
> not work there?

For this particular problem, it still does not scale well. There is more
than 20 different ops (and counting) for that clock type. Those would
need to be duplicated for each different way to get the regmap. That's
really not ideal

Side note: That's very interesting idea for another topic I'd like
address someday - not having all clock as global, but the just static data.
That would be a nice way to attach an allocator.

>
>> 
>> 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.
>> 
>
> I don't know if devres is a good model. It's about tracking allocations
> and things to undo later, not really to track things to do when called
> initially.

My point was more the decoupling it allows.
Maybe it is me being too picky, but what I'm trying to do is related to the
clock type, so it bothers me when it scales with the number of instances
instead of the type.

More generally, something devres-like allows to register an attribute
and link it to a group. Then the group members come and just pick what
they need. Whatever manages the attribute does not have to track
them. That is pretty much aligned with what I'm trying to do.

-- 
Jerome

Powered by blists - more mailing lists