2017-07-07 03:56:59

by Sean Wang

[permalink] [raw]
Subject: [PATCH v3 0/3] some fixups for MediaTek cpufreq driver

From: Sean Wang <[email protected]>

Changes since v2:
- correct the typo in the binding document

Changes since v1:
- drop those patches already accepted
- refine the commit messages and Kconfig dependency
- Kconfig menu entry and file name itself are updated with more
generic name to drop "MT8173" since this driver actually supports
all MediaTek SoCs.
- generate patchset again with git format-patch -C -M --thread=shallow
- fix binding examples by replacing '@' with '-' as the OPP nodes will
never have a "reg" property.

Hi,

The purpose of the series is
- (patch 1 to 3) to fix up current Mediatek cpufreq driver
can't work with the latest tree since one required CPU clock muxer missing
would cause the driver getting the resource fails when driver probe gets
called.
- (patch 4) to enable cpufreq feature on MT2701/MT7623 platform.
- (patch 5 to 6) to update the binding document to reflect
latest driver logic and add more examples guiding people how to apply for
Mediatek cpufreq driver.

Sean Wang (3):
cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC
dt-bindings: cpufreq: move MediaTek cpufreq dt-bindings document to
proper place
dt-bindings: cpufreq: enhance MediaTek cpufreq dt-binding document

.../devicetree/bindings/clock/mt8173-cpu-dvfs.txt | 83 -------
.../bindings/cpufreq/cpufreq-mediatek.txt | 247 +++++++++++++++++++++
drivers/cpufreq/Kconfig.arm | 8 +-
drivers/cpufreq/Makefile | 2 +-
.../cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} | 2 +
5 files changed, 254 insertions(+), 88 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt
create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
rename drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} (99%)

--
2.7.4


2017-07-07 03:57:01

by Sean Wang

[permalink] [raw]
Subject: [PATCH v3 3/3] dt-bindings: cpufreq: enhance MediaTek cpufreq dt-binding document

From: Sean Wang <[email protected]>

Update binding document with adding operating-points-v2 as the required
property and the cooling level as the optional properties and adding more
examples guiding people how to use MediaTek cpufreq driver for MediaTek
SoCs.

Signed-off-by: Sean Wang <[email protected]>
Acked-by: Viresh Kumar <[email protected]>
---
.../bindings/cpufreq/cpufreq-mediatek.txt | 170 ++++++++++++++++++++-
1 file changed, 167 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
index 52b457c..f640308 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
@@ -1,4 +1,5 @@
-Device Tree Clock bindins for CPU DVFS of Mediatek MT8173 SoC
+Binding for MediaTek's CPUFreq driver
+=====================================

Required properties:
- clocks: A list of phandle + clock-specifier pairs for the clocks listed in clock names.
@@ -9,6 +10,8 @@ Required properties:
transition and not stable yet.
Please refer to Documentation/devicetree/bindings/clk/clock-bindings.txt for
generic clock consumer properties.
+- operating-points-v2: Please refer to Documentation/devicetree/bindings/opp/opp.txt
+ for detail.
- proc-supply: Regulator for Vproc of CPU cluster.

Optional properties:
@@ -17,9 +20,166 @@ Optional properties:
Vsram to fit SoC specific needs. When absent, the voltage scaling
flow is handled by hardware, hence no software "voltage tracking" is
needed.
+- #cooling-cells:
+- cooling-min-level:
+- cooling-max-level:
+ Please refer to Documentation/devicetree/bindings/thermal/thermal.txt
+ for detail.
+
+Example 1 (MT7623 SoC):
+
+ cpu_opp_table: opp_table {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-598000000 {
+ opp-hz = /bits/ 64 <598000000>;
+ opp-microvolt = <1050000>;
+ };
+
+ opp-747500000 {
+ opp-hz = /bits/ 64 <747500000>;
+ opp-microvolt = <1050000>;
+ };
+
+ opp-1040000000 {
+ opp-hz = /bits/ 64 <1040000000>;
+ opp-microvolt = <1150000>;
+ };
+
+ opp-1196000000 {
+ opp-hz = /bits/ 64 <1196000000>;
+ opp-microvolt = <1200000>;
+ };
+
+ opp-1300000000 {
+ opp-hz = /bits/ 64 <1300000000>;
+ opp-microvolt = <1300000>;
+ };
+ };
+
+ cpu0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a7";
+ reg = <0x0>;
+ clocks = <&infracfg CLK_INFRA_CPUSEL>,
+ <&apmixedsys CLK_APMIXED_MAINPLL>;
+ clock-names = "cpu", "intermediate";
+ operating-points-v2 = <&cpu_opp_table>;
+ #cooling-cells = <2>;
+ cooling-min-level = <0>;
+ cooling-max-level = <7>;
+ };
+ cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a7";
+ reg = <0x1>;
+ operating-points-v2 = <&cpu_opp_table>;
+ };
+ cpu@2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a7";
+ reg = <0x2>;
+ operating-points-v2 = <&cpu_opp_table>;
+ };
+ cpu@3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a7";
+ reg = <0x3>;
+ operating-points-v2 = <&cpu_opp_table>;
+ };
+
+Example 2 (MT8173 SoC):
+ cpu_opp_table_a: opp_table_a {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-507000000 {
+ opp-hz = /bits/ 64 <507000000>;
+ opp-microvolt = <859000>;
+ };
+
+ opp-702000000 {
+ opp-hz = /bits/ 64 <702000000>;
+ opp-microvolt = <908000>;
+ };
+
+ opp-1001000000 {
+ opp-hz = /bits/ 64 <1001000000>;
+ opp-microvolt = <983000>;
+ };
+
+ opp-1105000000 {
+ opp-hz = /bits/ 64 <1105000000>;
+ opp-microvolt = <1009000>;
+ };
+
+ opp-1183000000 {
+ opp-hz = /bits/ 64 <1183000000>;
+ opp-microvolt = <1028000>;
+ };
+
+ opp-1404000000 {
+ opp-hz = /bits/ 64 <1404000000>;
+ opp-microvolt = <1083000>;
+ };
+
+ opp-1508000000 {
+ opp-hz = /bits/ 64 <1508000000>;
+ opp-microvolt = <1109000>;
+ };
+
+ opp-1573000000 {
+ opp-hz = /bits/ 64 <1573000000>;
+ opp-microvolt = <1125000>;
+ };
+ };
+
+ cpu_opp_table_b: opp_table_b {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-507000000 {
+ opp-hz = /bits/ 64 <507000000>;
+ opp-microvolt = <828000>;
+ };
+
+ opp-702000000 {
+ opp-hz = /bits/ 64 <702000000>;
+ opp-microvolt = <867000>;
+ };
+
+ opp-1001000000 {
+ opp-hz = /bits/ 64 <1001000000>;
+ opp-microvolt = <927000>;
+ };
+
+ opp-1209000000 {
+ opp-hz = /bits/ 64 <1209000000>;
+ opp-microvolt = <968000>;
+ };
+
+ opp-1404000000 {
+ opp-hz = /bits/ 64 <1007000000>;
+ opp-microvolt = <1028000>;
+ };
+
+ opp-1612000000 {
+ opp-hz = /bits/ 64 <1612000000>;
+ opp-microvolt = <1049000>;
+ };
+
+ opp-1807000000 {
+ opp-hz = /bits/ 64 <1807000000>;
+ opp-microvolt = <1089000>;
+ };
+
+ opp-1989000000 {
+ opp-hz = /bits/ 64 <1989000000>;
+ opp-microvolt = <1125000>;
+ };
+ };

-Example:
---------
cpu0: cpu@0 {
device_type = "cpu";
compatible = "arm,cortex-a53";
@@ -29,6 +189,7 @@ Example:
clocks = <&infracfg CLK_INFRA_CA53SEL>,
<&apmixedsys CLK_APMIXED_MAINPLL>;
clock-names = "cpu", "intermediate";
+ operating-points-v2 = <&cpu_opp_table_a>;
};

cpu1: cpu@1 {
@@ -40,6 +201,7 @@ Example:
clocks = <&infracfg CLK_INFRA_CA53SEL>,
<&apmixedsys CLK_APMIXED_MAINPLL>;
clock-names = "cpu", "intermediate";
+ operating-points-v2 = <&cpu_opp_table_a>;
};

cpu2: cpu@100 {
@@ -51,6 +213,7 @@ Example:
clocks = <&infracfg CLK_INFRA_CA57SEL>,
<&apmixedsys CLK_APMIXED_MAINPLL>;
clock-names = "cpu", "intermediate";
+ operating-points-v2 = <&cpu_opp_table_b>;
};

cpu3: cpu@101 {
@@ -62,6 +225,7 @@ Example:
clocks = <&infracfg CLK_INFRA_CA57SEL>,
<&apmixedsys CLK_APMIXED_MAINPLL>;
clock-names = "cpu", "intermediate";
+ operating-points-v2 = <&cpu_opp_table_b>;
};

&cpu0 {
--
2.7.4

2017-07-07 03:57:05

by Sean Wang

[permalink] [raw]
Subject: [PATCH v3 1/3] cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC

From: Sean Wang <[email protected]>

MT2701/MT7623 is a 32-bit ARMv7 based quad-core (4 * Cortex-A7) with
single cluster and this hardware is also compatible with the existing
driver through enabling CPU frequency feature with operating-points-v2
bindings. Also, this driver actually supports all MediaTek SoCs, the
Kconfig menu entry and file name itself should be updated with more
generic name to drop "MT8173"

Signed-off-by: Sean Wang <[email protected]>
Acked-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/Kconfig.arm | 8 ++++----
drivers/cpufreq/Makefile | 2 +-
drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} | 2 ++
3 files changed, 7 insertions(+), 5 deletions(-)
rename drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} (99%)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 74ed7e9..79aece7 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -87,14 +87,14 @@ config ARM_KIRKWOOD_CPUFREQ
This adds the CPUFreq driver for Marvell Kirkwood
SoCs.

-config ARM_MT8173_CPUFREQ
- tristate "Mediatek MT8173 CPUFreq support"
+config ARM_MEDIATEK_CPUFREQ
+ tristate "CPU Frequency scaling support for MediaTek SoCs"
depends on ARCH_MEDIATEK && REGULATOR
- depends on ARM64 || (ARM_CPU_TOPOLOGY && COMPILE_TEST)
+ depends on ARM || ARM64 || COMPILE_TEST
depends on !CPU_THERMAL || THERMAL
select PM_OPP
help
- This adds the CPUFreq driver support for Mediatek MT8173 SoC.
+ This adds the CPUFreq driver support for MediaTek SoCs.

config ARM_OMAP2PLUS_CPUFREQ
bool "TI OMAP2+"
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index b7e78f0..4956f5d 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -58,7 +58,7 @@ obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ) += exynos5440-cpufreq.o
obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o
obj-$(CONFIG_ARM_IMX6Q_CPUFREQ) += imx6q-cpufreq.o
obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ) += kirkwood-cpufreq.o
-obj-$(CONFIG_ARM_MT8173_CPUFREQ) += mt8173-cpufreq.o
+obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ) += mtk-cpufreq.o
obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o
obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mtk-cpufreq.c
similarity index 99%
rename from drivers/cpufreq/mt8173-cpufreq.c
rename to drivers/cpufreq/mtk-cpufreq.c
index fd1886f..481ec77 100644
--- a/drivers/cpufreq/mt8173-cpufreq.c
+++ b/drivers/cpufreq/mtk-cpufreq.c
@@ -575,6 +575,8 @@ static struct platform_driver mt8173_cpufreq_platdrv = {

/* List of machines supported by this driver */
static const struct of_device_id mt8173_cpufreq_machines[] __initconst = {
+ { .compatible = "mediatek,mt2701", },
+ { .compatible = "mediatek,mt7623", },
{ .compatible = "mediatek,mt817x", },
{ .compatible = "mediatek,mt8173", },
{ .compatible = "mediatek,mt8176", },
--
2.7.4

2017-07-07 03:57:35

by Sean Wang

[permalink] [raw]
Subject: [PATCH v3 2/3] dt-bindings: cpufreq: move MediaTek cpufreq dt-bindings document to proper place

From: Sean Wang <[email protected]>

The old place is Documentation/devicetree/bindings/clock/ that would
let people hard to find how to use MediaTek cpufreq driver, so moving
it to the appropriate place as other cpufreq drivers done would be
better.

Signed-off-by: Sean Wang <[email protected]>
Acked-by: Viresh Kumar <[email protected]>
---
.../bindings/{clock/mt8173-cpu-dvfs.txt => cpufreq/cpufreq-mediatek.txt} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename Documentation/devicetree/bindings/{clock/mt8173-cpu-dvfs.txt => cpufreq/cpufreq-mediatek.txt} (100%)

diff --git a/Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
similarity index 100%
rename from Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt
rename to Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
--
2.7.4

2017-07-07 07:30:56

by Jean Delvare

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC

Hi Sean,

On ven., 2017-07-07 at 11:56 +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> MT2701/MT7623 is a 32-bit ARMv7 based quad-core (4 * Cortex-A7) with
> single cluster and this hardware is also compatible with the existing
> driver through enabling CPU frequency feature with operating-points-v2
> bindings. Also, this driver actually supports all MediaTek SoCs, the
> Kconfig menu entry and file name itself should be updated with more
> generic name to drop "MT8173"
>
> Signed-off-by: Sean Wang <[email protected]>
> Acked-by: Viresh Kumar <[email protected]>
> ---
> drivers/cpufreq/Kconfig.arm | 8 ++++----
> drivers/cpufreq/Makefile | 2 +-
> drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} | 2 ++
> 3 files changed, 7 insertions(+), 5 deletions(-)
> rename drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} (99%)
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 74ed7e9..79aece7 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -87,14 +87,14 @@ config ARM_KIRKWOOD_CPUFREQ
> This adds the CPUFreq driver for Marvell Kirkwood
> SoCs.
>
> -config ARM_MT8173_CPUFREQ
> - tristate "Mediatek MT8173 CPUFreq support"
> +config ARM_MEDIATEK_CPUFREQ
> + tristate "CPU Frequency scaling support for MediaTek SoCs"
> depends on ARCH_MEDIATEK && REGULATOR
> - depends on ARM64 || (ARM_CPU_TOPOLOGY && COMPILE_TEST)
> + depends on ARM || ARM64 || COMPILE_TEST

This statement no longer serves any purpose. The whole Kconfig.arm file
depends on ARM || ARM64, so you can drop it. As a matter of fact no
other entry in this file has the dependency.

> depends on !CPU_THERMAL || THERMAL
> select PM_OPP
> help
> - This adds the CPUFreq driver support for Mediatek MT8173 SoC.
> + This adds the CPUFreq driver support for MediaTek SoCs.
>
> config ARM_OMAP2PLUS_CPUFREQ
> bool "TI OMAP2+"
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index b7e78f0..4956f5d 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -58,7 +58,7 @@ obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ) += exynos5440-cpufreq.o
> obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o
> obj-$(CONFIG_ARM_IMX6Q_CPUFREQ) += imx6q-cpufreq.o
> obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ) += kirkwood-cpufreq.o
> -obj-$(CONFIG_ARM_MT8173_CPUFREQ) += mt8173-cpufreq.o
> +obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ) += mtk-cpufreq.o

I would have preferred mediatek-cpufreq. 3-letter abbreviations are
likely to collide and cause confusion at some point in time. However it
seems I am late to the party, I see that there are already many files
named mtk-* or *-mtk.*... So, so be it.

Also the file describing the bindings is named cpufreq-mediatek.txt,
which is inconsistent... But the situation is the same for a few other
cpufreq drivers already, so I suppose this is acceptable.

> obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
> obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o
> obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
> diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mtk-cpufreq.c
> similarity index 99%
> rename from drivers/cpufreq/mt8173-cpufreq.c
> rename to drivers/cpufreq/mtk-cpufreq.c
> index fd1886f..481ec77 100644
> --- a/drivers/cpufreq/mt8173-cpufreq.c
> +++ b/drivers/cpufreq/mtk-cpufreq.c
> @@ -575,6 +575,8 @@ static struct platform_driver mt8173_cpufreq_platdrv = {
>
> /* List of machines supported by this driver */
> static const struct of_device_id mt8173_cpufreq_machines[] __initconst = {
> + { .compatible = "mediatek,mt2701", },
> + { .compatible = "mediatek,mt7623", },
> { .compatible = "mediatek,mt817x", },
> { .compatible = "mediatek,mt8173", },
> { .compatible = "mediatek,mt8176", },

Reviewed-by: Jean Delvare <[email protected]>

--
Jean Delvare
SUSE L3 Support

2017-07-08 06:30:41

by Sean Wang

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC

On Fri, 2017-07-07 at 09:30 +0200, Jean Delvare wrote:
> Hi Sean,
>
> On ven., 2017-07-07 at 11:56 +0800, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > MT2701/MT7623 is a 32-bit ARMv7 based quad-core (4 * Cortex-A7) with
> > single cluster and this hardware is also compatible with the existing
> > driver through enabling CPU frequency feature with operating-points-v2
> > bindings. Also, this driver actually supports all MediaTek SoCs, the
> > Kconfig menu entry and file name itself should be updated with more
> > generic name to drop "MT8173"
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > Acked-by: Viresh Kumar <[email protected]>
> > ---
> > drivers/cpufreq/Kconfig.arm | 8 ++++----
> > drivers/cpufreq/Makefile | 2 +-
> > drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} | 2 ++
> > 3 files changed, 7 insertions(+), 5 deletions(-)
> > rename drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} (99%)
> >
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index 74ed7e9..79aece7 100644
> > --- a/drivers/cpufreq/Kconfig.arm
> > +++ b/drivers/cpufreq/Kconfig.arm
> > @@ -87,14 +87,14 @@ config ARM_KIRKWOOD_CPUFREQ
> > This adds the CPUFreq driver for Marvell Kirkwood
> > SoCs.
> >
> > -config ARM_MT8173_CPUFREQ
> > - tristate "Mediatek MT8173 CPUFreq support"
> > +config ARM_MEDIATEK_CPUFREQ
> > + tristate "CPU Frequency scaling support for MediaTek SoCs"
> > depends on ARCH_MEDIATEK && REGULATOR
> > - depends on ARM64 || (ARM_CPU_TOPOLOGY && COMPILE_TEST)
> > + depends on ARM || ARM64 || COMPILE_TEST
>
> This statement no longer serves any purpose. The whole Kconfig.arm file
> depends on ARM || ARM64, so you can drop it. As a matter of fact no
> other entry in this file has the dependency.
>

Okay, the whole line seemed redundant for that's always true.
I will drop it in the next version.

> > depends on !CPU_THERMAL || THERMAL
> > select PM_OPP
> > help
> > - This adds the CPUFreq driver support for Mediatek MT8173 SoC.
> > + This adds the CPUFreq driver support for MediaTek SoCs.
> >
> > config ARM_OMAP2PLUS_CPUFREQ
> > bool "TI OMAP2+"
> > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> > index b7e78f0..4956f5d 100644
> > --- a/drivers/cpufreq/Makefile
> > +++ b/drivers/cpufreq/Makefile
> > @@ -58,7 +58,7 @@ obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ) += exynos5440-cpufreq.o
> > obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o
> > obj-$(CONFIG_ARM_IMX6Q_CPUFREQ) += imx6q-cpufreq.o
> > obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ) += kirkwood-cpufreq.o
> > -obj-$(CONFIG_ARM_MT8173_CPUFREQ) += mt8173-cpufreq.o
> > +obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ) += mtk-cpufreq.o
>
> I would have preferred mediatek-cpufreq. 3-letter abbreviations are
> likely to collide and cause confusion at some point in time. However it
> seems I am late to the party, I see that there are already many files
> named mtk-* or *-mtk.*... So, so be it.
>
> Also the file describing the bindings is named cpufreq-mediatek.txt,
> which is inconsistent... But the situation is the same for a few other
> cpufreq drivers already, so I suppose this is acceptable.
>

Agreed. I also prefer using whole name with mediatek as the
prefix than just abbreviation used. I will update it also in the
next version.

As for the file naming for other drivers, i feel it's okay for me
changing into mediatek as file prefix, but it still depends on
maintainers or original authors if they likes this.


Sean

> > obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
> > obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o
> > obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
> > diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mtk-cpufreq.c
> > similarity index 99%
> > rename from drivers/cpufreq/mt8173-cpufreq.c
> > rename to drivers/cpufreq/mtk-cpufreq.c
> > index fd1886f..481ec77 100644
> > --- a/drivers/cpufreq/mt8173-cpufreq.c
> > +++ b/drivers/cpufreq/mtk-cpufreq.c
> > @@ -575,6 +575,8 @@ static struct platform_driver mt8173_cpufreq_platdrv = {
> >
> > /* List of machines supported by this driver */
> > static const struct of_device_id mt8173_cpufreq_machines[] __initconst = {
> > + { .compatible = "mediatek,mt2701", },
> > + { .compatible = "mediatek,mt7623", },
> > { .compatible = "mediatek,mt817x", },
> > { .compatible = "mediatek,mt8173", },
> > { .compatible = "mediatek,mt8176", },
>
> Reviewed-by: Jean Delvare <[email protected]>
>


2017-07-10 12:06:05

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC



On 07/08/2017 08:30 AM, Sean Wang wrote:
> On Fri, 2017-07-07 at 09:30 +0200, Jean Delvare wrote:
>> Hi Sean,
>>
>> On ven., 2017-07-07 at 11:56 +0800, [email protected] wrote:
>>> From: Sean Wang <[email protected]>
>>>
>>> MT2701/MT7623 is a 32-bit ARMv7 based quad-core (4 * Cortex-A7) with
>>> single cluster and this hardware is also compatible with the existing
>>> driver through enabling CPU frequency feature with operating-points-v2
>>> bindings. Also, this driver actually supports all MediaTek SoCs, the
>>> Kconfig menu entry and file name itself should be updated with more
>>> generic name to drop "MT8173"
>>>
>>> Signed-off-by: Sean Wang <[email protected]>
>>> Acked-by: Viresh Kumar <[email protected]>
>>> ---
>>> drivers/cpufreq/Kconfig.arm | 8 ++++----
>>> drivers/cpufreq/Makefile | 2 +-
>>> drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} | 2 ++
>>> 3 files changed, 7 insertions(+), 5 deletions(-)
>>> rename drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} (99%)
>>>
>>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>>> index 74ed7e9..79aece7 100644
>>> --- a/drivers/cpufreq/Kconfig.arm
>>> +++ b/drivers/cpufreq/Kconfig.arm
>>> @@ -87,14 +87,14 @@ config ARM_KIRKWOOD_CPUFREQ
>>> This adds the CPUFreq driver for Marvell Kirkwood
>>> SoCs.
>>>
>>> -config ARM_MT8173_CPUFREQ
>>> - tristate "Mediatek MT8173 CPUFreq support"
>>> +config ARM_MEDIATEK_CPUFREQ
>>> + tristate "CPU Frequency scaling support for MediaTek SoCs"
>>> depends on ARCH_MEDIATEK && REGULATOR
>>> - depends on ARM64 || (ARM_CPU_TOPOLOGY && COMPILE_TEST)
>>> + depends on ARM || ARM64 || COMPILE_TEST
>>
>> This statement no longer serves any purpose. The whole Kconfig.arm file
>> depends on ARM || ARM64, so you can drop it. As a matter of fact no
>> other entry in this file has the dependency.
>>
>
> Okay, the whole line seemed redundant for that's always true.
> I will drop it in the next version.
>
>>> depends on !CPU_THERMAL || THERMAL
>>> select PM_OPP
>>> help
>>> - This adds the CPUFreq driver support for Mediatek MT8173 SoC.
>>> + This adds the CPUFreq driver support for MediaTek SoCs.
>>>
>>> config ARM_OMAP2PLUS_CPUFREQ
>>> bool "TI OMAP2+"
>>> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
>>> index b7e78f0..4956f5d 100644
>>> --- a/drivers/cpufreq/Makefile
>>> +++ b/drivers/cpufreq/Makefile
>>> @@ -58,7 +58,7 @@ obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ) += exynos5440-cpufreq.o
>>> obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o
>>> obj-$(CONFIG_ARM_IMX6Q_CPUFREQ) += imx6q-cpufreq.o
>>> obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ) += kirkwood-cpufreq.o
>>> -obj-$(CONFIG_ARM_MT8173_CPUFREQ) += mt8173-cpufreq.o
>>> +obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ) += mtk-cpufreq.o
>>
>> I would have preferred mediatek-cpufreq. 3-letter abbreviations are
>> likely to collide and cause confusion at some point in time. However it
>> seems I am late to the party, I see that there are already many files
>> named mtk-* or *-mtk.*... So, so be it.
>>
>> Also the file describing the bindings is named cpufreq-mediatek.txt,
>> which is inconsistent... But the situation is the same for a few other
>> cpufreq drivers already, so I suppose this is acceptable.
>>
>
> Agreed. I also prefer using whole name with mediatek as the
> prefix than just abbreviation used. I will update it also in the
> next version.
>
> As for the file naming for other drivers, i feel it's okay for me
> changing into mediatek as file prefix, but it still depends on
> maintainers or original authors if they likes this.
>

If you want to clean that up, be my guest. Other low-hanging fruits to
clean up would be the config options. Right now we have MEDIATEK, MTK
and MT65 in kconfig, which is a mess.

Regards,
Matthias

>
> Sean
>
>>> obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
>>> obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o
>>> obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
>>> diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mtk-cpufreq.c
>>> similarity index 99%
>>> rename from drivers/cpufreq/mt8173-cpufreq.c
>>> rename to drivers/cpufreq/mtk-cpufreq.c
>>> index fd1886f..481ec77 100644
>>> --- a/drivers/cpufreq/mt8173-cpufreq.c
>>> +++ b/drivers/cpufreq/mtk-cpufreq.c
>>> @@ -575,6 +575,8 @@ static struct platform_driver mt8173_cpufreq_platdrv = {
>>>
>>> /* List of machines supported by this driver */
>>> static const struct of_device_id mt8173_cpufreq_machines[] __initconst = {
>>> + { .compatible = "mediatek,mt2701", },
>>> + { .compatible = "mediatek,mt7623", },
>>> { .compatible = "mediatek,mt817x", },
>>> { .compatible = "mediatek,mt8173", },
>>> { .compatible = "mediatek,mt8176", },
>>
>> Reviewed-by: Jean Delvare <[email protected]>
>>
>
>

2017-07-10 14:58:57

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] dt-bindings: cpufreq: move MediaTek cpufreq dt-bindings document to proper place

On Fri, Jul 07, 2017 at 11:56:46AM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> The old place is Documentation/devicetree/bindings/clock/ that would
> let people hard to find how to use MediaTek cpufreq driver, so moving
> it to the appropriate place as other cpufreq drivers done would be
> better.
>
> Signed-off-by: Sean Wang <[email protected]>
> Acked-by: Viresh Kumar <[email protected]>
> ---
> .../bindings/{clock/mt8173-cpu-dvfs.txt => cpufreq/cpufreq-mediatek.txt} | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> rename Documentation/devicetree/bindings/{clock/mt8173-cpu-dvfs.txt => cpufreq/cpufreq-mediatek.txt} (100%)

Acked-by: Rob Herring <[email protected]>

2017-07-10 15:02:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] dt-bindings: cpufreq: enhance MediaTek cpufreq dt-binding document

On Fri, Jul 07, 2017 at 11:56:47AM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Update binding document with adding operating-points-v2 as the required
> property and the cooling level as the optional properties and adding more
> examples guiding people how to use MediaTek cpufreq driver for MediaTek
> SoCs.
>
> Signed-off-by: Sean Wang <[email protected]>
> Acked-by: Viresh Kumar <[email protected]>
> ---
> .../bindings/cpufreq/cpufreq-mediatek.txt | 170 ++++++++++++++++++++-
> 1 file changed, 167 insertions(+), 3 deletions(-)

Acked-by: Rob Herring <[email protected]>