Thread (2 messages) 2 messages, 2 authors, 2025-02-27

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

From: Stephen Boyd <sboyd@kernel.org>
Date: 2025-02-27 22:55:02
Also in: linux-amlogic, linux-clk, lkml

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
 2. Reduce memory
 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? Why wouldn't wrapping the clk_ops
in another struct and then using container_of to find the custom clk_ops
not work there?
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help