linux-kernel - Re: [PATCH v1 1/6] clk: divider: Implement and wire up .determine

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Oct 2021 14:11:37 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Alex Bee <knaerzche@...il.com>
Cc:     linux-clk@...r.kernel.org, sboyd@...nel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Heiko Stübner <heiko@...ech.de>,
        linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v1 1/6] clk: divider: Implement and wire up
 .determine_rate by default

Hi Alex,

On Thu, Oct 14, 2021 at 11:55 AM Alex Bee <knaerzche@...il.com> wrote:
[...]
> This breaks lot of clocks for Rockchip which intensively uses
> composites,  i.e. those clocks will always stay at the initial parent,
> which in some cases  is the XTAL clock and I strongly guess it is the
> same for other platforms,  which use composite clocks having more than
> one parent (e.g. mediatek, ti ...)
Sorry for that and thanks for bisecting this!

> Example (RK3399)
> clk_sdio is set (initialized) with XTAL (24 MHz) as parent in u-boot.
> It will always stay at this parent, even if the mmc driver sets a rate
> of  200 MHz (fails, as the nature of things), which should switch it to
> any of its possible parent PLLs defined in
> mux_pll_src_cpll_gpll_npll_ppll_upll_24m_p (see clk-rk3399.c)  - which
> never happens.
My question to Stephen et. al. is: where is the correct place to solve this?
What I came up with so far (in no particular order):
1) not using clk-composite from clock drivers and letting CCF take
care of re-parenting clocks as needed (and as specified with
CLK_SET_RATE_NO_REPARENT)
2) clk-composite.c: extending the logic so "rate" clocks with
.determine_rate include the existing logic which only applies to
.round_rate (which means clk-composite.c is then responsible for
finding the best possible parent clock)
3) clk-divider.c: extending the logic here to account for clk_hws with
multiple parents

For 3) I am wondering whether this would even work because it seems
that clk-composite uses multiple struct clk_hw.
Letting the divider handle multiple parents means it would need to
know about the information which is only available in mux_hw - whereas
clk-composite currently passes rate_hw (struct clk_hw for the
divider).

I am happy to work on a patch for this if I can get some help with
testing (since I don't have any board with Rockchip SoC).

> Reverting this commit makes it work again: Unless there is a quick and
> obvious fix for that, I guess this should be done for 5.15 - even if the
> real issue is somewhere else.
Reverting this patch is fine from the Amlogic SoC point of view.
The main goal was to clean up / improve the CCF code.
Nothing (that I am aware of) is going to break in Amlogic land if we
revert this.



Best regards,
Martin