Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210702225145.2643303-1-martin.blumenstingl@googlemail.com>
 <20210702225145.2643303-2-martin.blumenstingl@googlemail.com> <4eb964ac-4fff-b59d-2660-2f84d8af5901@gmail.com>
In-Reply-To: <4eb964ac-4fff-b59d-2660-2f84d8af5901@gmail.com>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Thu, 14 Oct 2021 14:11:37 +0200
Message-ID: <CAFBinCAVtd8gmcuvGU79-85CqhSU8a3mBCa_jweeZBd59u+amQ@mail.gmail.com>
Subject: Re: [PATCH v1 1/6] clk: divider: Implement and wire up
 .determine_rate by default
To:     Alex Bee <knaerzche@gmail.com>
Cc:     linux-clk@vger.kernel.org, sboyd@kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        =?UTF-8?Q?Heiko_St=C3=BCbner?= <heiko@sntech.de>,
        linux-rockchip@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Alex,

On Thu, Oct 14, 2021 at 11:55 AM Alex Bee <knaerzche@gmail.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