2022-04-08 05:56:09

by Johnson Wang

[permalink] [raw]
Subject: [PATCH v2 0/2] Introduce MediaTek CCI devfreq driver

The Cache Coherent Interconnect (CCI) is the management of cache
coherency by hardware. CCI DEVFREQ is DVFS driver for power saving by
scaling clock frequency and supply voltage of CCI. CCI uses the same
input clock source and power rail as LITTLE CPUs on Mediatek SoCs.

This series depends on:
Chanwoo's repo: kernel/git/chanwoo/linux.git
branch: devfreq-testing
[1]: PM / devfreq: Export devfreq_get_freq_range symbol within devfreq
[2]: PM / devfreq: Add cpu based scaling support to passive governor
[3]: PM / devfreq: passive: Reduce duplicate code when passive_devfreq case
[4]: PM / devfreq: passive: Update frequency when start governor

Changes in v2:
- Take MT8183 as example in binding document.
- Use dev_err() instead of pr_err().
- Use 'goto' statement to handle error case.
- Clean up driver code.

Johnson Wang (2):
dt-bindings: devfreq: mediatek: Add mtk cci devfreq dt-bindings
PM / devfreq: mediatek: Introduce MediaTek CCI devfreq driver

.../devicetree/bindings/devfreq/mtk-cci.yaml | 72 +++
drivers/devfreq/Kconfig | 10 +
drivers/devfreq/Makefile | 1 +
drivers/devfreq/mtk-cci-devfreq.c | 479 ++++++++++++++++++
4 files changed, 562 insertions(+)
create mode 100644 Documentation/devicetree/bindings/devfreq/mtk-cci.yaml
create mode 100644 drivers/devfreq/mtk-cci-devfreq.c

--
2.18.0


2022-04-08 05:56:10

by Johnson Wang

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings: devfreq: mediatek: Add mtk cci devfreq dt-bindings

Add devicetree binding of mtk cci devfreq on MediaTek SoC.

Signed-off-by: Johnson Wang <[email protected]>
Signed-off-by: Jia-Wei Chang <[email protected]>
---
.../devicetree/bindings/devfreq/mtk-cci.yaml | 72 +++++++++++++++++++
1 file changed, 72 insertions(+)
create mode 100644 Documentation/devicetree/bindings/devfreq/mtk-cci.yaml

diff --git a/Documentation/devicetree/bindings/devfreq/mtk-cci.yaml b/Documentation/devicetree/bindings/devfreq/mtk-cci.yaml
new file mode 100644
index 000000000000..ef4ea951025c
--- /dev/null
+++ b/Documentation/devicetree/bindings/devfreq/mtk-cci.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/devfreq/mtk-cci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Cache Coherent Interconnect (CCI) frequency and voltage scaling
+
+maintainers:
+ - Jia-Wei Chang <[email protected]>
+
+description: |
+ MediaTek Cache Coherent Interconnect (CCI) uses the software devfreq module
+ to scale the clock frequency and adjust the voltage. MediaTek CCI shares
+ the same power supplies with CPU, so the scheduling involves with CPUfreq.
+
+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:
+ description:
+ For details, please refer to
+ Documentation/devicetree/bindings/opp/opp-v2.yaml
+
+ 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>;
+ };
--
2.18.0

2022-04-08 09:10:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: devfreq: mediatek: Add mtk cci devfreq dt-bindings

On 08/04/2022 07:21, Johnson Wang wrote:
> Add devicetree binding of mtk cci devfreq on MediaTek SoC.
>

Thank you for your patch. There is something to discuss/improve.

> Signed-off-by: Johnson Wang <[email protected]>
> Signed-off-by: Jia-Wei Chang <[email protected]>
> ---
> .../devicetree/bindings/devfreq/mtk-cci.yaml | 72 +++++++++++++++++++
> 1 file changed, 72 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/devfreq/mtk-cci.yaml

Filename with vendor prefix, so something like:

mediatek,cci.yaml

Also please put it in the "interconnect" directory.

>
> diff --git a/Documentation/devicetree/bindings/devfreq/mtk-cci.yaml b/Documentation/devicetree/bindings/devfreq/mtk-cci.yaml
> new file mode 100644
> index 000000000000..ef4ea951025c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/mtk-cci.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/devfreq/mtk-cci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Cache Coherent Interconnect (CCI) frequency and voltage scaling
> +
> +maintainers:
> + - Jia-Wei Chang <[email protected]>
> +
> +description: |
> + MediaTek Cache Coherent Interconnect (CCI) uses the software devfreq module

Do not reference software implementation (devfreq).

> + to scale the clock frequency and adjust the voltage. MediaTek CCI shares
> + the same power supplies with CPU, so the scheduling involves with CPUfreq.

The same - cpufreq.

Instead, focus on the hardware, what do you describe here?

> +
> +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:
> + description:
> + For details, please refer to
> + Documentation/devicetree/bindings/opp/opp-v2.yaml

No need for description. Just "operating-points-v2: true".

"opp-table:true" could stay. My previous comment about its removal was a
wrong advice, because opp-table is used for a table being a children of
this device node.

Best regards,
Krzysztof

2022-04-12 22:36:21

by Johnson Wang

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: devfreq: mediatek: Add mtk cci devfreq dt-bindings

Hi Krzysztof,
On Fri, 2022-04-08 at 10:17 +0200, Krzysztof Kozlowski wrote:
> On 08/04/2022 07:21, Johnson Wang wrote:
> > Add devicetree binding of mtk cci devfreq on MediaTek SoC.
> >
>
> Thank you for your patch. There is something to discuss/improve.
>
> > Signed-off-by: Johnson Wang <[email protected]>
> > Signed-off-by: Jia-Wei Chang <[email protected]>
> > ---
> > .../devicetree/bindings/devfreq/mtk-cci.yaml | 72
> > +++++++++++++++++++
> > 1 file changed, 72 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/devfreq/mtk-
> > cci.yaml
>
> Filename with vendor prefix, so something like:
>
> mediatek,cci.yaml

Thank you for your review.
I will take your advice in the next version.
>
> Also please put it in the "interconnect" directory.
>

I don't really know about "interconnect".
However, it looks like a Linux framework about data transfer and "NoC".

While this cci driver is more like a power managment which is
responsible for adjusting voltages and frequencies.
In my opinion, "devfreq" should be more suitable.

Please correct me if my understanding is wrong.

> > diff --git a/Documentation/devicetree/bindings/devfreq/mtk-cci.yaml
> > b/Documentation/devicetree/bindings/devfreq/mtk-cci.yaml
> > new file mode 100644
> > index 000000000000..ef4ea951025c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/devfreq/mtk-cci.yaml
> > @@ -0,0 +1,72 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > https://urldefense.com/v3/__http://devicetree.org/schemas/devfreq/mtk-cci.yaml*__;Iw!!CTRNKA9wMg0ARbw!yd_wfLu2nSv0GsJZOGP1S8McsGD9A2SC4Qe0Xg1wEb_yEMVcqHRqdvs-M8YKOGckaagO$
> >
> > +$schema:
> > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!yd_wfLu2nSv0GsJZOGP1S8McsGD9A2SC4Qe0Xg1wEb_yEMVcqHRqdvs-M8YKOERouJvA$
> >
> > +
> > +title: MediaTek Cache Coherent Interconnect (CCI) frequency and
> > voltage scaling
> > +
> > +maintainers:
> > + - Jia-Wei Chang <[email protected]>
> > +
> > +description: |
> > + MediaTek Cache Coherent Interconnect (CCI) uses the software
> > devfreq module
>
> Do not reference software implementation (devfreq).

I will modify it in the next version.

>
> > + to scale the clock frequency and adjust the voltage. MediaTek
> > CCI shares
> > + the same power supplies with CPU, so the scheduling involves
> > with CPUfreq.
>
> The same - cpufreq.
>
> Instead, focus on the hardware, what do you describe here?

I will focus on hardware description in the next version.

>
> > +
> > +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:
> > + description:
> > + For details, please refer to
> > + Documentation/devicetree/bindings/opp/opp-v2.yaml
>
> No need for description. Just "operating-points-v2: true".
>
> "opp-table:true" could stay. My previous comment about its removal
> was a
> wrong advice, because opp-table is used for a table being a children
> of
> this device node.
>
> Best regards,
> Krzysztof


I will remove it and add "opp-table:true"(also example) in the next
version.

Thanks.

BRs,
Johnson Wang

2022-04-12 23:47:25

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: devfreq: mediatek: Add mtk cci devfreq dt-bindings

On 11/04/2022 14:10, Johnson Wang wrote:
>> Also please put it in the "interconnect" directory.
>>
>
> I don't really know about "interconnect".
> However, it looks like a Linux framework about data transfer and "NoC".
>
> While this cci driver is more like a power managment which is
> responsible for adjusting voltages and frequencies.
> In my opinion, "devfreq" should be more suitable.
>
> Please correct me if my understanding is wrong.

devfreq is a Linux mechanism, not a real device/hardware. We try to put
the bindings in directories/subsystems matching the hardware, therefore
devfreq is not appropriate.

Whether interconnect - or other subsystem - is appropriate, I am not
sure. To me this looks exactly like bus bandwidth management and you
even use "interconnect" in several places. So interconnect matches.

Best regards,
Krzysztof

2022-04-14 11:20:14

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: devfreq: mediatek: Add mtk cci devfreq dt-bindings

On 22. 4. 12. 18:17, Krzysztof Kozlowski wrote:
> On 11/04/2022 14:10, Johnson Wang wrote:
>>> Also please put it in the "interconnect" directory.
>>>
>>
>> I don't really know about "interconnect".
>> However, it looks like a Linux framework about data transfer and "NoC".
>>
>> While this cci driver is more like a power managment which is
>> responsible for adjusting voltages and frequencies.
>> In my opinion, "devfreq" should be more suitable.
>>
>> Please correct me if my understanding is wrong.
>
> devfreq is a Linux mechanism, not a real device/hardware. We try to put
> the bindings in directories/subsystems matching the hardware, therefore
> devfreq is not appropriate.
>
> Whether interconnect - or other subsystem - is appropriate, I am not
> sure. To me this looks exactly like bus bandwidth management and you
> even use "interconnect" in several places. So interconnect matches.

I think that 'Documentation/devicetree/bindings/arm/mediatek' is proper.

--
Best Regards,
Samsung Electronics
Chanwoo Choi

2022-04-15 12:32:07

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: devfreq: mediatek: Add mtk cci devfreq dt-bindings

Krzysztof Kozlowski <[email protected]> writes:

> On 11/04/2022 14:10, Johnson Wang wrote:
>>> Also please put it in the "interconnect" directory.
>>>
>>
>> I don't really know about "interconnect".
>> However, it looks like a Linux framework about data transfer and "NoC".
>>
>> While this cci driver is more like a power managment which is
>> responsible for adjusting voltages and frequencies.
>> In my opinion, "devfreq" should be more suitable.
>>
>> Please correct me if my understanding is wrong.
>
> devfreq is a Linux mechanism, not a real device/hardware. We try to put
> the bindings in directories/subsystems matching the hardware, therefore
> devfreq is not appropriate.
>
> Whether interconnect - or other subsystem - is appropriate, I am not
> sure. To me this looks exactly like bus bandwidth management and you
> even use "interconnect" in several places. So interconnect matches.

I agree with Krzysztof that "interconnect" is the right place.

I'm pretty sure CCI stands for "cache coherent interconnect". At least
that's what it means for the Arm IP. The Mediatek IP being described
here certainly seems like the same thing. It's just that the only
aspects being described (so far) are the DVFS parts. Even so, I still
think it belongs in "interconnect"

Kevin