linux-kernel - Re: [PATCH 2/3] clk: amlogic: drop clk

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: <5109de7fe1a1a8467118daf80c7b7f8a.sboyd@kernel.org>
Date: Thu, 27 Feb 2025 14:55:00 -0800
From: Stephen Boyd <sboyd@...nel.org>
To: Jerome Brunet <jbrunet@...libre.com>
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

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.