| Message ID | 20220425125546.4129-2-johnson.wang@mediatek.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Chanwoo Choi |
| Headers | show |
| Series | Introduce MediaTek CCI devfreq driver | expand |
On 25/04/2022 14:55, Johnson Wang wrote: > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > --- Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Mon, Apr 25, 2022 at 8:56 PM Johnson Wang <johnson.wang@mediatek.com> wrote: > > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > --- > .../bindings/interconnect/mediatek,cci.yaml | 139 ++++++++++++++++++ > 1 file changed, 139 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > diff --git a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > new file mode 100644 > index 000000000000..e5221e17d11b > --- /dev/null > +++ b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > @@ -0,0 +1,139 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interconnect/mediatek,cci.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek Cache Coherent Interconnect (CCI) frequency and voltage scaling > + > +maintainers: > + - Jia-Wei Chang <jia-wei.chang@mediatek.com> > + > +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. > + - 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. > + > + clock-names: > + items: > + - const: cci > + - const: intermediate > + > + operating-points-v2: true > + opp-table: true > + > + proc-supply: > + description: > + Phandle of the regulator for CCI that provides the supply voltage. > + > + sram-supply: > + description: > + Phandle of the regulator for sram of CCI that provides the supply > + voltage. When it presents, the cci devfreq driver needs to do When it is present, the implementation needs to ... ChenYu > + "voltage tracking" to step by step scale up/down Vproc and Vsram to fit > + SoC specific needs. When absent, the voltage scaling flow is handled by > + hardware, hence no software "voltage tracking" is needed. > + > +required: > + - compatible > + - clocks > + - clock-names > + - operating-points-v2 > + - proc-supply > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/mt8183-clk.h> > + cci: cci { > + compatible = "mediatek,mt8183-cci"; > + clocks = <&mcucfg CLK_MCU_BUS_SEL>, > + <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>; > + clock-names = "cci", "intermediate"; > + operating-points-v2 = <&cci_opp>; > + proc-supply = <&mt6358_vproc12_reg>; > + }; > + > + cci_opp: opp-table-cci { > + compatible = "operating-points-v2"; > + opp-shared; > + opp2_00: opp-273000000 { > + opp-hz = /bits/ 64 <273000000>; > + opp-microvolt = <650000>; > + }; > + opp2_01: opp-338000000 { > + opp-hz = /bits/ 64 <338000000>; > + opp-microvolt = <687500>; > + }; > + opp2_02: opp-403000000 { > + opp-hz = /bits/ 64 <403000000>; > + opp-microvolt = <718750>; > + }; > + opp2_03: opp-463000000 { > + opp-hz = /bits/ 64 <463000000>; > + opp-microvolt = <756250>; > + }; > + opp2_04: opp-546000000 { > + opp-hz = /bits/ 64 <546000000>; > + opp-microvolt = <800000>; > + }; > + opp2_05: opp-624000000 { > + opp-hz = /bits/ 64 <624000000>; > + opp-microvolt = <818750>; > + }; > + opp2_06: opp-689000000 { > + opp-hz = /bits/ 64 <689000000>; > + opp-microvolt = <850000>; > + }; > + opp2_07: opp-767000000 { > + opp-hz = /bits/ 64 <767000000>; > + opp-microvolt = <868750>; > + }; > + opp2_08: opp-845000000 { > + opp-hz = /bits/ 64 <845000000>; > + opp-microvolt = <893750>; > + }; > + opp2_09: opp-871000000 { > + opp-hz = /bits/ 64 <871000000>; > + opp-microvolt = <906250>; > + }; > + opp2_10: opp-923000000 { > + opp-hz = /bits/ 64 <923000000>; > + opp-microvolt = <931250>; > + }; > + opp2_11: opp-962000000 { > + opp-hz = /bits/ 64 <962000000>; > + opp-microvolt = <943750>; > + }; > + opp2_12: opp-1027000000 { > + opp-hz = /bits/ 64 <1027000000>; > + opp-microvolt = <975000>; > + }; > + opp2_13: opp-1092000000 { > + opp-hz = /bits/ 64 <1092000000>; > + opp-microvolt = <1000000>; > + }; > + opp2_14: opp-1144000000 { > + opp-hz = /bits/ 64 <1144000000>; > + opp-microvolt = <1025000>; > + }; > + opp2_15: opp-1196000000 { > + opp-hz = /bits/ 64 <1196000000>; > + opp-microvolt = <1050000>; > + }; > + }; > -- > 2.18.0 > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi, On 22. 4. 25. 21:55, Johnson Wang wrote: > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > --- > .../bindings/interconnect/mediatek,cci.yaml | 139 ++++++++++++++++++ > 1 file changed, 139 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > diff --git a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > new file mode 100644 > index 000000000000..e5221e17d11b > --- /dev/null > +++ b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > @@ -0,0 +1,139 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interconnect/mediatek,cci.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek Cache Coherent Interconnect (CCI) frequency and voltage scaling > + > +maintainers: > + - Jia-Wei Chang <jia-wei.chang@mediatek.com> Why did you add your author information? Please add your author information. And add this dt-binding information to MAINTAINERS as following: because I cannot catch the later patch of modification. cwchoi00@chanwoo:~/kernel/linux.chanwoo$ d diff --git a/MAINTAINERS b/MAINTAINERS index edc96cdb85e8..a11e9c1947b7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5698,6 +5698,7 @@ L: linux-pm@vger.kernel.org S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git F: Documentation/devicetree/bindings/devfreq/ +F: Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml F: drivers/devfreq/ F: include/linux/devfreq.h F: include/trace/events/devfreq.h > + > +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. > + - 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. > + > + clock-names: > + items: > + - const: cci > + - const: intermediate > + > + operating-points-v2: true > + opp-table: true > + > + proc-supply: > + description: > + Phandle of the regulator for CCI that provides the supply voltage. > + > + sram-supply: > + description: > + Phandle of the regulator for sram of CCI that provides the supply > + voltage. When it presents, the cci devfreq driver needs to do > + "voltage tracking" to step by step scale up/down Vproc and Vsram to fit > + SoC specific needs. When absent, the voltage scaling flow is handled by > + hardware, hence no software "voltage tracking" is needed. > + > +required: > + - compatible > + - clocks > + - clock-names > + - operating-points-v2 > + - proc-supply > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/mt8183-clk.h> > + cci: cci { > + compatible = "mediatek,mt8183-cci"; > + clocks = <&mcucfg CLK_MCU_BUS_SEL>, > + <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>; > + clock-names = "cci", "intermediate"; > + operating-points-v2 = <&cci_opp>; > + proc-supply = <&mt6358_vproc12_reg>; > + }; > + > + cci_opp: opp-table-cci { > + compatible = "operating-points-v2"; > + opp-shared; > + opp2_00: opp-273000000 { > + opp-hz = /bits/ 64 <273000000>; > + opp-microvolt = <650000>; > + }; > + opp2_01: opp-338000000 { > + opp-hz = /bits/ 64 <338000000>; > + opp-microvolt = <687500>; > + }; > + opp2_02: opp-403000000 { > + opp-hz = /bits/ 64 <403000000>; > + opp-microvolt = <718750>; > + }; > + opp2_03: opp-463000000 { > + opp-hz = /bits/ 64 <463000000>; > + opp-microvolt = <756250>; > + }; > + opp2_04: opp-546000000 { > + opp-hz = /bits/ 64 <546000000>; > + opp-microvolt = <800000>; > + }; > + opp2_05: opp-624000000 { > + opp-hz = /bits/ 64 <624000000>; > + opp-microvolt = <818750>; > + }; > + opp2_06: opp-689000000 { > + opp-hz = /bits/ 64 <689000000>; > + opp-microvolt = <850000>; > + }; > + opp2_07: opp-767000000 { > + opp-hz = /bits/ 64 <767000000>; > + opp-microvolt = <868750>; > + }; > + opp2_08: opp-845000000 { > + opp-hz = /bits/ 64 <845000000>; > + opp-microvolt = <893750>; > + }; > + opp2_09: opp-871000000 { > + opp-hz = /bits/ 64 <871000000>; > + opp-microvolt = <906250>; > + }; > + opp2_10: opp-923000000 { > + opp-hz = /bits/ 64 <923000000>; > + opp-microvolt = <931250>; > + }; > + opp2_11: opp-962000000 { > + opp-hz = /bits/ 64 <962000000>; > + opp-microvolt = <943750>; > + }; > + opp2_12: opp-1027000000 { > + opp-hz = /bits/ 64 <1027000000>; > + opp-microvolt = <975000>; > + }; > + opp2_13: opp-1092000000 { > + opp-hz = /bits/ 64 <1092000000>; > + opp-microvolt = <1000000>; > + }; > + opp2_14: opp-1144000000 { > + opp-hz = /bits/ 64 <1144000000>; > + opp-microvolt = <1025000>; > + }; > + opp2_15: opp-1196000000 { > + opp-hz = /bits/ 64 <1196000000>; > + opp-microvolt = <1050000>; > + }; > + };
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@mediatek.com> wrote: > > > > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > --- > > .../bindings/interconnect/mediatek,cci.yaml | 139 > > ++++++++++++++++++ > > 1 file changed, 139 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > new file mode 100644 > > index 000000000000..e5221e17d11b > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > @@ -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@mediatek.com> > > + > > +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. > > + > > + clock-names: > > + items: > > + - const: cci > > + - const: intermediate > > + > > + operating-points-v2: true > > + opp-table: true > > + > > + proc-supply: > > + description: > > + Phandle of the regulator for CCI that provides the supply > > voltage. > > + > > + sram-supply: > > + description: > > + Phandle of the regulator for sram of CCI that provides the > > supply > > + voltage. When it presents, the cci devfreq driver needs to > > do > > When it is present, the implementation needs to ... > > ChenYu I will modify it in the next version. BRs, Johnson Wang > > > + "voltage tracking" to step by step scale up/down Vproc and > > Vsram to fit > > + SoC specific needs. When absent, the voltage scaling flow is > > handled by > > + hardware, hence no software "voltage tracking" is needed. > > + > > +required: > > + - compatible > > + - clocks > > + - clock-names > > + - operating-points-v2 > > + - proc-supply > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/mt8183-clk.h> > > + cci: cci { > > + compatible = "mediatek,mt8183-cci"; > > + clocks = <&mcucfg CLK_MCU_BUS_SEL>, > > + <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>; > > + clock-names = "cci", "intermediate"; > > + operating-points-v2 = <&cci_opp>; > > + proc-supply = <&mt6358_vproc12_reg>; > > + }; > > + > > + cci_opp: opp-table-cci { > > + compatible = "operating-points-v2"; > > + opp-shared; > > + opp2_00: opp-273000000 { > > + opp-hz = /bits/ 64 <273000000>; > > + opp-microvolt = <650000>; > > + }; > > + opp2_01: opp-338000000 { > > + opp-hz = /bits/ 64 <338000000>; > > + opp-microvolt = <687500>; > > + }; > > + opp2_02: opp-403000000 { > > + opp-hz = /bits/ 64 <403000000>; > > + opp-microvolt = <718750>; > > + }; > > + opp2_03: opp-463000000 { > > + opp-hz = /bits/ 64 <463000000>; > > + opp-microvolt = <756250>; > > + }; > > + opp2_04: opp-546000000 { > > + opp-hz = /bits/ 64 <546000000>; > > + opp-microvolt = <800000>; > > + }; > > + opp2_05: opp-624000000 { > > + opp-hz = /bits/ 64 <624000000>; > > + opp-microvolt = <818750>; > > + }; > > + opp2_06: opp-689000000 { > > + opp-hz = /bits/ 64 <689000000>; > > + opp-microvolt = <850000>; > > + }; > > + opp2_07: opp-767000000 { > > + opp-hz = /bits/ 64 <767000000>; > > + opp-microvolt = <868750>; > > + }; > > + opp2_08: opp-845000000 { > > + opp-hz = /bits/ 64 <845000000>; > > + opp-microvolt = <893750>; > > + }; > > + opp2_09: opp-871000000 { > > + opp-hz = /bits/ 64 <871000000>; > > + opp-microvolt = <906250>; > > + }; > > + opp2_10: opp-923000000 { > > + opp-hz = /bits/ 64 <923000000>; > > + opp-microvolt = <931250>; > > + }; > > + opp2_11: opp-962000000 { > > + opp-hz = /bits/ 64 <962000000>; > > + opp-microvolt = <943750>; > > + }; > > + opp2_12: opp-1027000000 { > > + opp-hz = /bits/ 64 <1027000000>; > > + opp-microvolt = <975000>; > > + }; > > + opp2_13: opp-1092000000 { > > + opp-hz = /bits/ 64 <1092000000>; > > + opp-microvolt = <1000000>; > > + }; > > + opp2_14: opp-1144000000 { > > + opp-hz = /bits/ 64 <1144000000>; > > + opp-microvolt = <1025000>; > > + }; > > + opp2_15: opp-1196000000 { > > + opp-hz = /bits/ 64 <1196000000>; > > + opp-microvolt = <1050000>; > > + }; > > + }; > > -- > > 2.18.0 > > > > > > _______________________________________________ > > Linux-mediatek mailing list > > Linux-mediatek@lists.infradead.org > > https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-mediatek__;!!CTRNKA9wMg0ARbw!zuufEcqpKbditY3eqLTHpL8P8humMCyh4D4QWsximmw124tJUPE3ZBUyBqBtDvdNpOFZ$ > >
On 22. 5. 9. 21:09, Chanwoo Choi wrote: > Hi, > > On 22. 4. 25. 21:55, Johnson Wang wrote: >> Add devicetree binding of MediaTek CCI on MT8183 and MT8186. >> >> Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> >> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> >> --- >> .../bindings/interconnect/mediatek,cci.yaml | 139 ++++++++++++++++++ >> 1 file changed, 139 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml >> b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml >> new file mode 100644 >> index 000000000000..e5221e17d11b >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml >> @@ -0,0 +1,139 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/interconnect/mediatek,cci.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: MediaTek Cache Coherent Interconnect (CCI) frequency and >> voltage scaling >> + >> +maintainers: >> + - Jia-Wei Chang <jia-wei.chang@mediatek.com> > > Why did you add your author information? I'm sorry. I missed 'not' above sentence. IMHO, please add your author information. (snip)
Hi Chanwoo, On Mon, 2022-05-09 at 21:09 +0900, Chanwoo Choi wrote: > Hi, > > On 22. 4. 25. 21:55, Johnson Wang wrote: > > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > --- > > .../bindings/interconnect/mediatek,cci.yaml | 139 > > ++++++++++++++++++ > > 1 file changed, 139 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > new file mode 100644 > > index 000000000000..e5221e17d11b > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > @@ -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!z6ogArqzuIzPR3TYO1aW-Z-scpuZJxIriWMofdfnvrKTXAYBBLZeitAPIKyZayMYZGsR$ > > > > +$schema: > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!z6ogArqzuIzPR3TYO1aW-Z-scpuZJxIriWMofdfnvrKTXAYBBLZeitAPIKyZa9f2pALd$ > > > > + > > +title: MediaTek Cache Coherent Interconnect (CCI) frequency and > > voltage scaling > > + > > +maintainers: > > + - Jia-Wei Chang <jia-wei.chang@mediatek.com> > > Why did you add your author information? > Please add your author information. Sorry, I don't really understand what you mean. Could you please explain your advice again? The author of this driver is 'Jia-Wei Chang'. We have added author information to the driver code and this binding document as above. > > And add this dt-binding information to MAINTAINERS > as following: because I cannot catch the later patch > of modification. > > cwchoi00@chanwoo:~/kernel/linux.chanwoo$ d > diff --git a/MAINTAINERS b/MAINTAINERS > index edc96cdb85e8..a11e9c1947b7 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5698,6 +5698,7 @@ L: linux-pm@vger.kernel.org > S: Maintained > T: git > git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git > F: Documentation/devicetree/bindings/devfreq/ > +F: Documentation/devicetree/bindings/interconnect/mediatek,cci.y > aml > F: drivers/devfreq/ > F: include/linux/devfreq.h > F: include/trace/events/devfreq.h > I will add it in the next version. BRs, Johnson Wang > > > + > > +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. > > + - 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. > > + > > + clock-names: > > + items: > > + - const: cci > > + - const: intermediate > > + > > + operating-points-v2: true > > + opp-table: true > > + > > + proc-supply: > > + description: > > + Phandle of the regulator for CCI that provides the supply > > voltage. > > + > > + sram-supply: > > + description: > > + Phandle of the regulator for sram of CCI that provides the > > supply > > + voltage. When it presents, the cci devfreq driver needs to > > do > > + "voltage tracking" to step by step scale up/down Vproc and > > Vsram to fit > > + SoC specific needs. When absent, the voltage scaling flow is > > handled by > > + hardware, hence no software "voltage tracking" is needed. > > + > > +required: > > + - compatible > > + - clocks > > + - clock-names > > + - operating-points-v2 > > + - proc-supply > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/mt8183-clk.h> > > + cci: cci { > > + compatible = "mediatek,mt8183-cci"; > > + clocks = <&mcucfg CLK_MCU_BUS_SEL>, > > + <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>; > > + clock-names = "cci", "intermediate"; > > + operating-points-v2 = <&cci_opp>; > > + proc-supply = <&mt6358_vproc12_reg>; > > + }; > > + > > + cci_opp: opp-table-cci { > > + compatible = "operating-points-v2"; > > + opp-shared; > > + opp2_00: opp-273000000 { > > + opp-hz = /bits/ 64 <273000000>; > > + opp-microvolt = <650000>; > > + }; > > + opp2_01: opp-338000000 { > > + opp-hz = /bits/ 64 <338000000>; > > + opp-microvolt = <687500>; > > + }; > > + opp2_02: opp-403000000 { > > + opp-hz = /bits/ 64 <403000000>; > > + opp-microvolt = <718750>; > > + }; > > + opp2_03: opp-463000000 { > > + opp-hz = /bits/ 64 <463000000>; > > + opp-microvolt = <756250>; > > + }; > > + opp2_04: opp-546000000 { > > + opp-hz = /bits/ 64 <546000000>; > > + opp-microvolt = <800000>; > > + }; > > + opp2_05: opp-624000000 { > > + opp-hz = /bits/ 64 <624000000>; > > + opp-microvolt = <818750>; > > + }; > > + opp2_06: opp-689000000 { > > + opp-hz = /bits/ 64 <689000000>; > > + opp-microvolt = <850000>; > > + }; > > + opp2_07: opp-767000000 { > > + opp-hz = /bits/ 64 <767000000>; > > + opp-microvolt = <868750>; > > + }; > > + opp2_08: opp-845000000 { > > + opp-hz = /bits/ 64 <845000000>; > > + opp-microvolt = <893750>; > > + }; > > + opp2_09: opp-871000000 { > > + opp-hz = /bits/ 64 <871000000>; > > + opp-microvolt = <906250>; > > + }; > > + opp2_10: opp-923000000 { > > + opp-hz = /bits/ 64 <923000000>; > > + opp-microvolt = <931250>; > > + }; > > + opp2_11: opp-962000000 { > > + opp-hz = /bits/ 64 <962000000>; > > + opp-microvolt = <943750>; > > + }; > > + opp2_12: opp-1027000000 { > > + opp-hz = /bits/ 64 <1027000000>; > > + opp-microvolt = <975000>; > > + }; > > + opp2_13: opp-1092000000 { > > + opp-hz = /bits/ 64 <1092000000>; > > + opp-microvolt = <1000000>; > > + }; > > + opp2_14: opp-1144000000 { > > + opp-hz = /bits/ 64 <1144000000>; > > + opp-microvolt = <1025000>; > > + }; > > + opp2_15: opp-1196000000 { > > + opp-hz = /bits/ 64 <1196000000>; > > + opp-microvolt = <1050000>; > > + }; > > + }; > >
On Tue, May 10, 2022 at 11:59 AM Johnson Wang <johnson.wang@mediatek.com> wrote: > > Hi Chanwoo, > > On Mon, 2022-05-09 at 21:09 +0900, Chanwoo Choi wrote: > > Hi, > > > > On 22. 4. 25. 21:55, Johnson Wang wrote: > > > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > --- > > > .../bindings/interconnect/mediatek,cci.yaml | 139 > > > ++++++++++++++++++ > > > 1 file changed, 139 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > > > > diff --git > > > a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > new file mode 100644 > > > index 000000000000..e5221e17d11b > > > --- /dev/null > > > +++ > > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > @@ -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!z6ogArqzuIzPR3TYO1aW-Z-scpuZJxIriWMofdfnvrKTXAYBBLZeitAPIKyZayMYZGsR$ > > > > > > +$schema: > > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!z6ogArqzuIzPR3TYO1aW-Z-scpuZJxIriWMofdfnvrKTXAYBBLZeitAPIKyZa9f2pALd$ > > > > > > + > > > +title: MediaTek Cache Coherent Interconnect (CCI) frequency and > > > voltage scaling > > > + > > > +maintainers: > > > + - Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > > Why did you add your author information? > > Please add your author information. > > Sorry, I don't really understand what you mean. > Could you please explain your advice again? > > The author of this driver is 'Jia-Wei Chang'. > We have added author information to the driver code and this binding > document as above. Firstly, sorry for the confusion. I have discussed this patch with you. I think that you (Johnson Wang) should be added to 'maintainers' of this binding document as following: +maintainers: + - Jia-Wei Chang <jia-wei.chang@mediatek.com> + - Johnson Wang <johnson.wang@mediatek.com> > > > > > And add this dt-binding information to MAINTAINERS > > as following: because I cannot catch the later patch > > of modification. > > > > cwchoi00@chanwoo:~/kernel/linux.chanwoo$ d > > diff --git a/MAINTAINERS b/MAINTAINERS > > index edc96cdb85e8..a11e9c1947b7 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -5698,6 +5698,7 @@ L: linux-pm@vger.kernel.org > > S: Maintained > > T: git > > git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git > > F: Documentation/devicetree/bindings/devfreq/ > > +F: Documentation/devicetree/bindings/interconnect/mediatek,cci.y > > aml > > F: drivers/devfreq/ > > F: include/linux/devfreq.h > > F: include/trace/events/devfreq.h > > > > I will add it in the next version. > > BRs, > Johnson Wang > > > > > + > > > +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. > > > + - 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. > > > + > > > + clock-names: > > > + items: > > > + - const: cci > > > + - const: intermediate > > > + > > > + operating-points-v2: true > > > + opp-table: true > > > + > > > + proc-supply: > > > + description: > > > + Phandle of the regulator for CCI that provides the supply > > > voltage. > > > + > > > + sram-supply: > > > + description: > > > + Phandle of the regulator for sram of CCI that provides the > > > supply > > > + voltage. When it presents, the cci devfreq driver needs to > > > do > > > + "voltage tracking" to step by step scale up/down Vproc and > > > Vsram to fit > > > + SoC specific needs. When absent, the voltage scaling flow is > > > handled by > > > + hardware, hence no software "voltage tracking" is needed. > > > + > > > +required: > > > + - compatible > > > + - clocks > > > + - clock-names > > > + - operating-points-v2 > > > + - proc-supply > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/clock/mt8183-clk.h> > > > + cci: cci { > > > + compatible = "mediatek,mt8183-cci"; > > > + clocks = <&mcucfg CLK_MCU_BUS_SEL>, > > > + <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>; > > > + clock-names = "cci", "intermediate"; > > > + operating-points-v2 = <&cci_opp>; > > > + proc-supply = <&mt6358_vproc12_reg>; > > > + }; > > > + > > > + cci_opp: opp-table-cci { > > > + compatible = "operating-points-v2"; > > > + opp-shared; > > > + opp2_00: opp-273000000 { > > > + opp-hz = /bits/ 64 <273000000>; > > > + opp-microvolt = <650000>; > > > + }; > > > + opp2_01: opp-338000000 { > > > + opp-hz = /bits/ 64 <338000000>; > > > + opp-microvolt = <687500>; > > > + }; > > > + opp2_02: opp-403000000 { > > > + opp-hz = /bits/ 64 <403000000>; > > > + opp-microvolt = <718750>; > > > + }; > > > + opp2_03: opp-463000000 { > > > + opp-hz = /bits/ 64 <463000000>; > > > + opp-microvolt = <756250>; > > > + }; > > > + opp2_04: opp-546000000 { > > > + opp-hz = /bits/ 64 <546000000>; > > > + opp-microvolt = <800000>; > > > + }; > > > + opp2_05: opp-624000000 { > > > + opp-hz = /bits/ 64 <624000000>; > > > + opp-microvolt = <818750>; > > > + }; > > > + opp2_06: opp-689000000 { > > > + opp-hz = /bits/ 64 <689000000>; > > > + opp-microvolt = <850000>; > > > + }; > > > + opp2_07: opp-767000000 { > > > + opp-hz = /bits/ 64 <767000000>; > > > + opp-microvolt = <868750>; > > > + }; > > > + opp2_08: opp-845000000 { > > > + opp-hz = /bits/ 64 <845000000>; > > > + opp-microvolt = <893750>; > > > + }; > > > + opp2_09: opp-871000000 { > > > + opp-hz = /bits/ 64 <871000000>; > > > + opp-microvolt = <906250>; > > > + }; > > > + opp2_10: opp-923000000 { > > > + opp-hz = /bits/ 64 <923000000>; > > > + opp-microvolt = <931250>; > > > + }; > > > + opp2_11: opp-962000000 { > > > + opp-hz = /bits/ 64 <962000000>; > > > + opp-microvolt = <943750>; > > > + }; > > > + opp2_12: opp-1027000000 { > > > + opp-hz = /bits/ 64 <1027000000>; > > > + opp-microvolt = <975000>; > > > + }; > > > + opp2_13: opp-1092000000 { > > > + opp-hz = /bits/ 64 <1092000000>; > > > + opp-microvolt = <1000000>; > > > + }; > > > + opp2_14: opp-1144000000 { > > > + opp-hz = /bits/ 64 <1144000000>; > > > + opp-microvolt = <1025000>; > > > + }; > > > + opp2_15: opp-1196000000 { > > > + opp-hz = /bits/ 64 <1196000000>; > > > + opp-microvolt = <1050000>; > > > + }; > > > + }; > > > > >
On Mon, May 9, 2022 at 8:14 PM Johnson Wang <johnson.wang@mediatek.com> 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@mediatek.com> wrote: > > > > > > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > --- > > > .../bindings/interconnect/mediatek,cci.yaml | 139 > > > ++++++++++++++++++ > > > 1 file changed, 139 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > > > > diff --git > > > a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > new file mode 100644 > > > index 000000000000..e5221e17d11b > > > --- /dev/null > > > +++ > > > b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml > > > @@ -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@mediatek.com> > > > + > > > +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
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@mediatek.com> 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@mediatek.com> wrote: > > > > > > > > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > > --- > > > > .../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@mediatek.com> > > > > + > > > > +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. BRs, Johnson Wang
On Thu, May 12, 2022 at 9:05 PM Johnson Wang <johnson.wang@mediatek.com> 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@mediatek.com> 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@mediatek.com> wrote: > > > > > > > > > > Add devicetree binding of MediaTek CCI on MT8183 and MT8186. > > > > > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > > > --- > > > > > .../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@mediatek.com> > > > > > + > > > > > +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
diff --git a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml new file mode 100644 index 000000000000..e5221e17d11b --- /dev/null +++ b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml @@ -0,0 +1,139 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interconnect/mediatek,cci.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek Cache Coherent Interconnect (CCI) frequency and voltage scaling + +maintainers: + - Jia-Wei Chang <jia-wei.chang@mediatek.com> + +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. + - 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. + + clock-names: + items: + - const: cci + - const: intermediate + + operating-points-v2: true + opp-table: true + + proc-supply: + description: + Phandle of the regulator for CCI that provides the supply voltage. + + sram-supply: + description: + Phandle of the regulator for sram of CCI that provides the supply + voltage. When it presents, the cci devfreq driver needs to do + "voltage tracking" to step by step scale up/down Vproc and Vsram to fit + SoC specific needs. When absent, the voltage scaling flow is handled by + hardware, hence no software "voltage tracking" is needed. + +required: + - compatible + - clocks + - clock-names + - operating-points-v2 + - proc-supply + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/mt8183-clk.h> + cci: cci { + compatible = "mediatek,mt8183-cci"; + clocks = <&mcucfg CLK_MCU_BUS_SEL>, + <&topckgen CLK_TOP_ARMPLL_DIV_PLL1>; + clock-names = "cci", "intermediate"; + operating-points-v2 = <&cci_opp>; + proc-supply = <&mt6358_vproc12_reg>; + }; + + cci_opp: opp-table-cci { + compatible = "operating-points-v2"; + opp-shared; + opp2_00: opp-273000000 { + opp-hz = /bits/ 64 <273000000>; + opp-microvolt = <650000>; + }; + opp2_01: opp-338000000 { + opp-hz = /bits/ 64 <338000000>; + opp-microvolt = <687500>; + }; + opp2_02: opp-403000000 { + opp-hz = /bits/ 64 <403000000>; + opp-microvolt = <718750>; + }; + opp2_03: opp-463000000 { + opp-hz = /bits/ 64 <463000000>; + opp-microvolt = <756250>; + }; + opp2_04: opp-546000000 { + opp-hz = /bits/ 64 <546000000>; + opp-microvolt = <800000>; + }; + opp2_05: opp-624000000 { + opp-hz = /bits/ 64 <624000000>; + opp-microvolt = <818750>; + }; + opp2_06: opp-689000000 { + opp-hz = /bits/ 64 <689000000>; + opp-microvolt = <850000>; + }; + opp2_07: opp-767000000 { + opp-hz = /bits/ 64 <767000000>; + opp-microvolt = <868750>; + }; + opp2_08: opp-845000000 { + opp-hz = /bits/ 64 <845000000>; + opp-microvolt = <893750>; + }; + opp2_09: opp-871000000 { + opp-hz = /bits/ 64 <871000000>; + opp-microvolt = <906250>; + }; + opp2_10: opp-923000000 { + opp-hz = /bits/ 64 <923000000>; + opp-microvolt = <931250>; + }; + opp2_11: opp-962000000 { + opp-hz = /bits/ 64 <962000000>; + opp-microvolt = <943750>; + }; + opp2_12: opp-1027000000 { + opp-hz = /bits/ 64 <1027000000>; + opp-microvolt = <975000>; + }; + opp2_13: opp-1092000000 { + opp-hz = /bits/ 64 <1092000000>; + opp-microvolt = <1000000>; + }; + opp2_14: opp-1144000000 { + opp-hz = /bits/ 64 <1144000000>; + opp-microvolt = <1025000>; + }; + opp2_15: opp-1196000000 { + opp-hz = /bits/ 64 <1196000000>; + opp-microvolt = <1050000>; + }; + };