Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f0f9d4d5-4603-7f62-ea18-ea7e120200c3@linaro.org>
Date:   Mon, 26 Jun 2023 17:26:07 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [RESEND v6 6/8] arm64: dts: qcom: sc7280: Modify VA/RX/TX macro
 clock nodes for audioreach solution
Content-Language: en-US
To:     Konrad Dybcio <konrad.dybcio@linaro.org>,
        Mohammad Rafi Shaik <quic_mohs@quicinc.com>,
        krzysztof.kozlowski+dt@linaro.org, swboyd@chromium.org,
        andersson@kernel.org, broonie@kernel.org, agross@kernel.org
Cc:     robh+dt@kernel.org, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        quic_rohkumar@quicinc.com, srinivas.kandagatla@linaro.org,
        dianders@chromium.org, judyhsiao@chromium.org,
        quic_visr@quicinc.com,
        Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
References: <20230616103534.4031331-1-quic_mohs@quicinc.com>
 <20230616103534.4031331-7-quic_mohs@quicinc.com>
 <7d2e580e-1861-d22f-e67d-726a2a69043e@linaro.org>
 <3441b3cb-29dd-691c-1e95-5bddd66c684a@quicinc.com>
 <af148e01-4026-a086-af78-a1e252fcc724@linaro.org>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <af148e01-4026-a086-af78-a1e252fcc724@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@quicinc.com>
>>>>
>>>> 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@quicinc.com>
>>>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>>>> ---
>>> 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