On Thu, May 12, 2022 at 9:05 PM Johnson Wang <johnson.wang@xxxxxxxxxxxx> wrote: > > On Wed, 2022-05-11 at 18:48 +0800, Chen-Yu Tsai wrote: > > On Mon, May 9, 2022 at 8:14 PM Johnson Wang < > > johnson.wang@xxxxxxxxxxxx> wrote: > > > > > > Hi Chen-Yu, > > > > > > On Tue, 2022-04-26 at 11:18 +0800, Chen-Yu Tsai wrote: > > > > On Mon, Apr 25, 2022 at 8:56 PM Johnson Wang < > > > > johnson.wang@xxxxxxxxxxxx> wrote: > > > > > > > > > > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > > > > > > > > > Signed-off-by: Johnson Wang <johnson.wang@xxxxxxxxxxxx> > > > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@xxxxxxxxxxxx> > > > > > --- > > > > > .../bindings/interconnect/mediatek,cci.yaml | 139 > > > > > ++++++++++++++++++ > > > > > 1 file changed, 139 insertions(+) > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/interconnect/mediatek,cci.yam > > > > > l > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/interconnect/mediatek,cci.y > > > > > aml > > > > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.y > > > > > aml > > > > > new file mode 100644 > > > > > index 000000000000..e5221e17d11b > > > > > --- /dev/null > > > > > +++ > > > > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.y > > > > > aml > > > > > @@ -0,0 +1,139 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: > > > > > > https://urldefense.com/v3/__http://devicetree.org/schemas/interconnect/mediatek,cci.yaml*__;Iw!!CTRNKA9wMg0ARbw!zuufEcqpKbditY3eqLTHpL8P8humMCyh4D4QWsximmw124tJUPE3ZBUyBqBtDlQ9pSDO$ > > > > > > > > > > +$schema: > > > > > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!zuufEcqpKbditY3eqLTHpL8P8humMCyh4D4QWsximmw124tJUPE3ZBUyBqBtDoE9YHyu$ > > > > > > > > > > + > > > > > +title: MediaTek Cache Coherent Interconnect (CCI) frequency > > > > > and > > > > > voltage scaling > > > > > + > > > > > +maintainers: > > > > > + - Jia-Wei Chang <jia-wei.chang@xxxxxxxxxxxx> > > > > > + > > > > > +description: | > > > > > + MediaTek Cache Coherent Interconnect (CCI) is a hardware > > > > > engine > > > > > used by > > > > > + MT8183 and MT8186 SoCs to scale the frequency and adjust the > > > > > voltage in > > > > > + hardware. It can also optimize the voltage to reduce the > > > > > power > > > > > consumption. > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + enum: > > > > > + - mediatek,mt8183-cci > > > > > + - mediatek,mt8186-cci > > > > > + > > > > > + clocks: > > > > > + items: > > > > > + - description: > > > > > + The multiplexer for clock input of CPU cluster. > > > > > > > > of the bus, not CPU cluster. > > > > > > Thanks for your suggestion. > > > I will correct it in the next version. > > > > > > > > > > > > + - description: > > > > > + A parent of "cpu" clock which is used as an > > > > > intermediate > > > > > clock source > > > > > + when the original CPU is under transition and not > > > > > stable > > > > > yet. > > > > > > > > This really should be handled in the clk controller, and not by > > > > every > > > > device > > > > that happens to take a clock from a mux with upstream PLLs that > > > > can > > > > change > > > > in clock rate. The end device hardware only takes one clock > > > > input. > > > > That's it. > > > > > > > > > > To make this intermediate clock works properly, this driver is also > > > responsible for handling the Vproc voltage and ensures the voltage > > > is > > > high enough to support intermediate clock rate. > > > > > > If we move intermediate clock rate control to clock driver, then > > > intermediate voltage control may be handled by the clock driver > > > itself > > > as well. > > > > > > We believe that is not reasonable because clock driver shouldn't > > > handle > > > voltage control. On the other hand, DVFS driver is more suitable > > > for > > > doing this job. > > > > Either way the DVFS driver handles the voltage change. > > > > Right now the driver is doing: > > > > 1. Raise voltage if scaling up > > 2. Mux CCI clock over to stable clock > > 3. Set rate for CCI PLL > > 4. Mux CCI clock back to CCI PLL > > 5. Drop voltage if scaling down > > > > I'm saying that the clock driver should handle 2+4 transparently when > > any > > driver requests a rate change on the CCI clock. So instead the driver > > would > > do: > > > > 1. Raise voltage if scaling up > > 2. Set rate for CCI _clock_ > > Here the clock driver would do: > > a. Mux CCI clock over to stable clock > > b. Change clock rate for original parent, i.e. the CCI PLL > > c. Mux CCI clock back to original parent, i.e. the CCI PLL > > and back to the devfreq driver ... > > 3. Drop voltage if scaling down > > > > Does that make sense? > > > > > > Regards > > ChenYu > > Hi Chen-Yu, > > Before we mux the CCI clock to an intermediate clock(MAINPLL), we must > ensure that regulator voltage is high enough (we call it intermediate > voltage) to support the intermediate clock rate. > > Based on this concept, if we move mux control to clock driver, there > will be a dilemma about which driver to adjust the voltage. > > 1)When DVFS calls clk_set_rate(), clock driver scales up the regulator > voltage to higher than intermediate voltage and then mux the CCI clock. > > This option is not reasonable because clock driver shouldn't handle the > regulators. > > > 2)DVFS scales up the regulator voltage, then calls clk_set_rate(). > Clock driver mux the CCI clock to the intermediate clock. > > This option isn't straightforward and makes one confused easily. For a > person who reads this driver, he may not understand why we adjust the > voltage before clk_set_rate(). > > That's why we put intermediate voltage/freq together in the DVFS. Thanks for the explanation. The intermediate clock's rate being higher than the lowest OPP is the key I missed. I can't think of a better way to describe this in DT. The intermediate clock's rate is stable, but it is set either through hardware reset defaults or firmware, so we can't just assume a given clock rate and hard code that. Having a direct reference to the clock seems simpler. Regards ChenYu
