Message-ID: <f848061a-763e-fbf2-860c-758373e953df@linaro.org>
Date:   Sat, 11 Mar 2023 14:29:38 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v7 6/9] interconnect: qcom: rpm: Handle interface clocks
Content-Language: en-US
To:     Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Georgi Djakov <djakov@kernel.org>
Cc:     linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230228-topic-qos-v7-0-815606092fff@linaro.org>
 <20230228-topic-qos-v7-6-815606092fff@linaro.org>
 <68a5d81a-5de8-798a-c150-d74c8ad38cb7@linaro.org>
From:   Bryan O'Donoghue <bryan.odonoghue@linaro.org>
In-Reply-To: <68a5d81a-5de8-798a-c150-d74c8ad38cb7@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/03/2023 14:01, Dmitry Baryshkov wrote:
>>
>> Limit the number of bus_clocks to 2 (which is the maximum that SMD
>> RPM interconnect supports anyway) and handle non-scaling clocks
>> separately. Update MSM8996 and SDM660 drivers to make sure they do
>> not regress with this change.
>>
>> This unfortunately has to be done in one patch to prevent either
>> compile errors or broken bisect.
> 
> Can we determine somehow if the intf clocks are required for the whole 
> NoC or just for a single node? I don't think that clocks like a0noc_ufs 
> are requiered to be up for the whole aggre_noc. Instead they probably 
> have to be enabled only when UFS makes use of the NoC (in other words 
> when is has voted for the bandwidth).

Its probably worthwhile experimenting to see if the *ufs*_clk can/should 
be added to the UFS device list of clocks.

I hadn't realised we were talking about enabling the intf clocks always 
but, actually that is what we are talking about isn't it ?

It seems pretty unlikely that other devices would depend on clocks named 
*ufs*

OTOH "clk-aggre2-noc-clk-no-rate" may be used across different nodes, 
seems like a pretty generic name, though still suspicious it is bundled 
with UFS, likely it is only required for UFS ?!?

Konrad can you try:

1. Moving the intf_clks/QoS clocks be contained to UFS alone ?
2. If that doesn't work just the *ufs* clocks be put there with
2a. "clk-aggre2-noc-clk-no-rate" alone added to the intf_clk list

It seems wrong to be enabling ufs related NoC clocks unless we are 
actually switching on UFS.

---
bod