Message-ID: <df4c76eb-aec7-823e-28f9-5ba96cc200c6@linaro.org>
Date:   Thu, 26 Jan 2023 16:34:11 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.0
Subject: Re: [PATCH v4 0/6] Add MSM8939 SoC support with two devices
To:     Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
        Stephan Gerhold <stephan@gerhold.net>
Cc:     agross@kernel.org, andersson@kernel.org, djakov@kernel.org,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        benl@squareup.com, shawn.guo@linaro.org, fabien.parent@linaro.org,
        leo.yan@linaro.org, dmitry.baryshkov@linaro.org
References: <20230123023127.1186619-1-bryan.odonoghue@linaro.org>
 <42baa874-c926-9111-b0b3-2df2562d8de6@linaro.org>
 <Y86CPmgvAi+kChQI@gerhold.net>
 <87192098-b7f4-060f-9274-933d974c0a7d@linaro.org>
Content-Language: en-US
From:   Konrad Dybcio <konrad.dybcio@linaro.org>
In-Reply-To: <87192098-b7f4-060f-9274-933d974c0a7d@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 26.01.2023 16:29, Bryan O'Donoghue wrote:
> On 23/01/2023 12:49, Stephan Gerhold wrote:
>>> - Adds gcc dsi1pll and dsi1pllbyte to gcc clock list.
>>>    Reviewing the silicon documentation we see dsi0_phy_pll is used to clock
>>>    GCC_BYTE1_CFG_RCGR : SRC_SEL
>>>    Root Source Select
>>>    000 : cxo
>>>    001 : dsi0_phy_pll_out_byteclk
>>>    010 : GPLL0_OUT_AUX
>>>    011 : gnd
>>>    100 : gnd
>>>    101 : gnd
>>>    110 : gnd
>>>    111 : reserved - Stephan/Bryan
>>>
>> I'm confused. Are you not contradicting yourself here? You say that
>> dsi0_phy_pll (dsi ZERO) is used to clock GCC_BYTE1_CFG_RCGR. Then why
>> do you add dsi1_phy_pll (dsi ONE) to the gcc clock list?
> 
> So my understanding of the clock tree here is that dsi0_phy_pll_out_byteclk is a legacy name.
> 
> Its perfectly possible to have DSI0 and DSI0_PHY switched off and to have DSI1/DSI1_PHY operable.
> 
> dsi0_phy_pll_out_byteclk is perhaps an unfortunate name and probably should have been renamed.
> 
>> To me this looks like a confirmation of what downstream does, that both
>> DSI byte clocks are actually sourced from the dsi0_phy and the PLL of
> 
> A better name would have been dsiX_phy_pll_out_byteclk.
I believe Stephan is just confused what the clock source of both
pairs of GCC DSI clocks are, as you're suggesting that:

phy_clock0
  |_gcc_clock0

and

phy_clock0 (yes, zero)
  |_gcc_clock1

whereas on most other SoCs the following is true:

phy_clock0
  |_gcc_clock0

phy_clock1
  |_gcc_clock_1

Konrad
> 
> ---
> bod