Re: [PATCH RFT v2 01/14] dt-bindings: clock: qcom,rpmcc: Add a way to enable unused clock cleanup

From: Konrad Dybcio
Date: Tue Apr 11 2023 - 17:35:12 EST




On 7.04.2023 22:17, Konrad Dybcio wrote:
>
>
> On 6.04.2023 16:44, Konrad Dybcio wrote:
>>
>>
>> On 17.03.2023 19:20, Stephen Boyd wrote:
>>> Quoting Konrad Dybcio (2023-03-16 17:31:34)
>>>>
>>>> On 16.03.2023 23:58, Rob Herring wrote:
>>>>> On Wed, Mar 08, 2023 at 10:35:17PM +0100, Konrad Dybcio wrote:
>>>>>>
>>>>>> + qcom,clk-disable-unused:
>>>>>> + type: boolean
>>>>>> + description:
>>>>>> + Indicates whether unused RPM clocks can be shut down with the common
>>>>>> + unused clock cleanup. Requires a functional interconnect driver.
>>>>>
>>>>> I don't think this should be QCom specific. Come up with something
>>>>> common (which will probably have some debate).
>>>> Generally the opposite (ignoring unused clocks during the cleanup) is
>>>> the thing you need to opt into.
>>>>
>>>> I can however see how (especially with the focus on not breaking things
>>>> for older DTs) somebody else may also decide to only allow them to be
>>>> cleaned up conditionally (by marking the clocks that were enabled earlier
>>>> as enabled in Linux OR not addding clk.flags |= CLK_IGNORE_UNUSED) as we
>>>> do here.
>>>>
>>>> Stephen, Rob, would `clk-disable-unused` be a fitting generic property
>>>> name for that? Should we also think about `clk-ignore-unused` as a
>>>> clock-controller-specific alternative to the CCF-wide clk_ignore_unused
>>>> cmdline?
>>>>
>>>
>>> There are multiple threads on the list about disabling unused clks.
>>> Moving the decision to disable unused clks to a DT property is yet
>>> another approach. I'd rather not do that, because it really isn't
>>> describing the hardware configuration. If anything, I'd expect the
>>> property to be describing which clks are enabled by the firmware and
>>> then leave the decision to disable them because they're unused up to the
>>> software.
>> After some more thinking, I realized that this could be made opt-in
>> simply with driver_data..
>>
>> WDYT?
> ..on a re-evaluation, obviously not a great idea.. Old DTBs will not
> be happy about that.
Another idea would be to yank out the not-very-useful "qcom,rpmcc"
fallback compatible and present .is_enabled etc. when it's absent..

Directly checking for the interconnect handle to rpmcc is not possible,
as interconnect requires rpmcc.. And then somebody's interconnect
driver may not be "good enough" (like 8996 and pre-6.3 DTs).

Konrad
>
> Konrad
>>
>> Konrad