Re: [RESEND v6 6/8] arm64: dts: qcom: sc7280: Modify VA/RX/TX macro clock nodes for audioreach solution

[Krzysztof Kozlowski]

On 26/06/2023 14:24, Konrad Dybcio wrote:
> On 26.06.2023 13:13, Mohammad Rafi Shaik wrote:
>>
>> On 6/16/2023 4:59 PM, Konrad Dybcio wrote:
>>> On 16.06.2023 12:35, Mohammad Rafi Shaik wrote:
>>>> From: Srinivasa Rao Mandadapu <quic_srivasam@xxxxxxxxxxx>
>>>>
>>>> Modify VA, RX and TX macro and lpass_tlmm clock properties and
>>>> enable them. For audioreach solution mclk, npl and fsgen clocks
>>>> are enabled through the q6prm clock driver.
>>>>
>>>> Delete the power domain properties from VA, RX and TX macro,
>>>> for audioreach solution the macro, dcodec power domains enabled
>>>> through the q6prm clock driver.
>>>>
>>>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@xxxxxxxxxxx>
>>>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@xxxxxxxxxxx>
>>>> ---
>>> Maybe sc7280-audioreach.dtsi containing all these changes that could be
>>> reused by others would be in order?
>> Thanks for comment,
>>
>> yes, will create a common sc7280-audioreach.dtsi file, which will contain common audioreach changes
>> and could be reused by others.
>>>>   .../sc7280-herobrine-audioreach-wcd9385.dtsi  | 43 +++++++++++++++++++
>>>>   1 file changed, 43 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi
>>>> index 9daea1b25656..c02ca393378f 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-audioreach-wcd9385.dtsi
>>>> @@ -196,3 +196,46 @@ q6prmcc: clock-controller {
>>>>           };
>>>>       };
>>>>   };
>>>> +
>>>> +&lpass_rx_macro {
>>>> +    /delete-property/ power-domains;
>>>> +    /delete-property/ power-domain-names;
>>> Surely they shouldn't cause issues, even if the vote would be
>>> superfluous? They are still powered by these power domains, I'd assume?
>> No, In Audioreach case this macro and decodec clocks are not power by power domains,
>> this macro and decodec hw clocks are enrolled by q6prmcc clock voting.
> So the same piece of hardware is modeled differently twice?
> 
> i.e. the same GDSCs are reached once with register accesses and once
> registered as "Q6 vote clocks"?
> 
> that sounds like a bit of an overstep to register them with genpd and CCF
> depending on what entity controls them.. perhaps the "q6 vote clocks" could
> be remodeled as power domains as that's what they're ultimately seem to
> be referencing.. Krzysztof should have an opinion.

I think on SM8450 and newer these were already modeled as clocks, not
power domains. Anyway, for me, the previous/existing/coming code looks
like done by coincidence or copying some downstream choices, not with
any design in mind. Unfortunately, I don't know what to do with it now,
because the bindings were merged like that.


Best regards,
Krzysztof