[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <9096086a-3bae-be33-7961-f9db24f301da@linaro.org>
Date: Mon, 2 Oct 2023 11:13:00 +0200
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Johan Hovold <johan@...nel.org>
Cc: Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Vinod Koul <vkoul@...nel.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH 2/2] clk: qcom: gcc-sc8280xp: Don't keep display AHB
clocks always-on
On 10/2/23 10:34, Johan Hovold wrote:
> On Sat, Sep 30, 2023 at 06:44:47PM +0200, Konrad Dybcio wrote:
>> On 9/30/23 11:39, Johan Hovold wrote:
>>> On Fri, Sep 29, 2023 at 03:38:53PM +0200, Konrad Dybcio wrote:
>>>> These clocks are consumed by the dispcc[01] clock controllers, so there's
>>>> no reason to keep them on from gcc probe. Remove that hack.
>>>
>>> Eh, how did you test this patch?
>
>> Oehh you're right, I didn't notice that I still had clk_ignore_unused :/
>
> That doesn't matter since these clocks are never even registered with
> the clock framework.
That's the point, if it was missing and was not enabled I would have
noticed display not working (unless the bootloader left it on, which it
did for at least the mdss instance with the eDP panel)
>
> But you'd notice that if you try to verify the clock state by looking at
> /sys/kernel/debug/clk/clk_summary for example.
>
>>> The GCC_DISP_AHB_CLK clocks are not modelled by the clock driver
>>> currently so nothing is guaranteeing them to be enabled if we were to
>>> apply this patch. They just happen to be left on by the bootloader on
>>> some machines currently (well at least one of them is on one machine).
>
>> What fooled me is that despite not being modeled by the clock driver, it
>> is defined in bindings and referenced in the device tree.
>>
>> Another thing I'll fix up!
>
> Right, a number of Qualcomm SoCs apparently fail to register these
> clocks. You should start by determining why that is as I assume (hope)
> it was done for a reason.
The reason is, downstream lazily enables clocks because people decided
they don't leak much power and are still enabled after a clock
controller reset (or something) and we started blindly copying that
around 2017 :/
>
> Then the Qualcomm drivers use sloppy bulk clock look-up and enable so
> that an integrator would never even notice when clocks are missing. Once
> the clocks are registered, that could be tightened up as well.
Yeah the current state of things is not great.
Konrad
Powered by blists - more mailing lists