Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <4af8e7a4-0506-a08c-f294-d055fb463af8@quicinc.com>
Date:   Mon, 20 Nov 2023 12:12:48 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH V2 2/4] clk: qcom: Add Global Clock controller (GCC)
 driver for X1E80100
Content-Language: en-US
To:     Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        <andersson@kernel.org>, <mturquette@baylibre.com>,
        <sboyd@kernel.org>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
CC:     <agross@kernel.org>, <conor+dt@kernel.org>,
        <quic_tdas@quicinc.com>, <quic_rjendra@quicinc.com>,
        <linux-arm-msm@vger.kernel.org>, <linux-clk@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <neil.armstrong@linaro.org>, <abel.vesa@linaro.org>,
        <quic_tsoni@quicinc.com>
References: <20231117092737.28362-1-quic_sibis@quicinc.com>
 <20231117092737.28362-3-quic_sibis@quicinc.com>
 <ec9d03f7-7158-4309-9a04-b08c69b89f39@linaro.org>
 <2e0d2c55-fb2f-4903-a555-f51019942c6e@linaro.org>
 <c8cf229b-4d15-4eca-bc4b-61dc67d63e91@linaro.org>
From:   Sibi Sankar <quic_sibis@quicinc.com>
In-Reply-To: <c8cf229b-4d15-4eca-bc4b-61dc67d63e91@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/18/23 07:22, Bryan O'Donoghue wrote:
> On 18/11/2023 00:06, Konrad Dybcio wrote:
>> On 17.11.2023 21:50, Bryan O'Donoghue wrote:
>>> On 17/11/2023 09:27, Sibi Sankar wrote:
>>>> * Use shared ops in the x1e80100 gcc driver [Bryan].
>>>
>>> This looks better to me now / more consistent with what we have in 
>>> sc8280xp - where we do try to hit suspend and => retention/parking 
>>> matters.
>> Parking the clock is separate from putting the system to sleep.
> 
> Yes but several of our clocks want to be parked, not switched off.. 
> which obviously does matter in suspend.
> 
>> IIUC we usually use shared ops on clocks that may have different users
>> (e.g. not only controlled by Linux) and/or that are crucial to the
>> functioning of hardware (like AXI clocks, which if gated would make
>> the system crash on any access attempt, from any subsystem, unless
>> turned on beforehand)
> 
> My question here for Sibi, is why sdcc2_apss_clk_src differs here from 
> sc8280xp?
> 
> Is it wrong on sc8280xp or if correct sc8280xp then why is it not 
> replicated here ?
> 
> https://lore.kernel.org/linux-arm-msm/e857c853-51ef-8314-2a21-fa6fd25162ca@quicinc.com/

Bryan,

5e4b7e82d497 ("clk: qcom: gcc-sdm845: Use floor ops for sdcc clks")

The floor_ops was introduced in sdm845 ^^ and later propagated to all
other QC SoCs later on. It makes sense to do the same for sc8280xp as
well.

> 
> Also @Sibi I realise alot of this code is autogenerated - it would be 
> worthwhile finding/fixing the script that does the generation to plug in 
> shared_ops instead of floor_ops if the input material has the necessary 
> flags.

floor_ops part isn't auto-generated (it comes out as shared_ops,
but like you said it might make sense to include it as part of the
generation process itself.

-Sibi

> 
> ---
> bod