| Message ID | 1418213396-743-7-git-send-email-l.majewski@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Eduardo Valentin |
| Headers | show |
On Wed, Dec 10, 2014 at 01:09:45PM +0100, Lukasz Majewski wrote: > Presented patch aims to move data necessary for correct CPU cooling device > configuration from exynos_tmu_data.c to device tree. I believe the patch title is misleading. Looks like you are changing something at cpu cooling, but in fact, you are changing DTS files. I would suggest you to use a prefix like 'arm: dts: ....' > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > --- > Changes for v2: > - None > --- > arch/arm/boot/dts/exynos4210-trats.dts | 15 ++++++++++++ > arch/arm/boot/dts/exynos4210.dtsi | 20 ++++++++++++++++ > arch/arm/boot/dts/exynos4212.dtsi | 20 ++++++++++++++++ > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 15 ++++++++++++ > arch/arm/boot/dts/exynos4412-trats2.dts | 15 ++++++++++++ > arch/arm/boot/dts/exynos4412.dtsi | 32 +++++++++++++++++++++++++ > arch/arm/boot/dts/exynos5250.dtsi | 20 +++++++++++++++- > 7 files changed, 136 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts > index b59019c..d9dd9a7 100644 > --- a/arch/arm/boot/dts/exynos4210-trats.dts > +++ b/arch/arm/boot/dts/exynos4210-trats.dts > @@ -428,6 +428,21 @@ > status = "okay"; > }; > > + thermal-zones { > + cpu_thermal: cpu-thermal { > + cooling-maps { > + map0 { > + /* Corresponds to 800MHz at freq_table */ > + cooling-device = <&cpu0 2 2>; > + }; > + map1 { > + /* Corresponds to 200MHz at freq_table */ > + cooling-device = <&cpu0 4 4>; > + }; > + }; > + }; > + }; > + > camera { > pinctrl-names = "default"; > pinctrl-0 = <>; > diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi > index 807bb5b..10e8915 100644 > --- a/arch/arm/boot/dts/exynos4210.dtsi > +++ b/arch/arm/boot/dts/exynos4210.dtsi > @@ -41,6 +41,26 @@ > #clock-cells = <1>; > }; > > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu0: cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + reg = <0x900>; > + cooling-min-level = <4>; > + cooling-max-level = <2>; > + #cooling-cells = <2>; /* min followed by max */ > + }; > + > + cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + reg = <0x901>; > + }; > + }; > + > sysram@02020000 { > compatible = "mmio-sram"; > reg = <0x02020000 0x20000>; > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi > index 3c00e6e..6405954 100644 > --- a/arch/arm/boot/dts/exynos4212.dtsi > +++ b/arch/arm/boot/dts/exynos4212.dtsi > @@ -22,6 +22,26 @@ > / { > compatible = "samsung,exynos4212", "samsung,exynos4"; > > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu0: cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + reg = <0xA00>; > + cooling-min-level = <13>; > + cooling-max-level = <7>; > + #cooling-cells = <2>; /* min followed by max */ > + }; > + > + cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + reg = <0xA01>; > + }; > + }; > + > combiner: interrupt-controller@10440000 { > samsung,combiner-nr = <18>; > }; > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > index eb1c08c..15d45f0 100644 > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > @@ -375,6 +375,21 @@ > vtmu-supply = <&ldo10_reg>; > status = "okay"; > }; > + > + thermal-zones { > + cpu_thermal: cpu-thermal { > + cooling-maps { > + map0 { > + /* Corresponds to 800MHz at freq_table */ > + cooling-device = <&cpu0 7 7>; > + }; > + map1 { > + /* Corresponds to 200MHz at freq_table */ > + cooling-device = <&cpu0 13 13>; > + }; > + }; > + }; > + }; > }; > > &pinctrl_1 { > diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts > index 121430d..396c525 100644 > --- a/arch/arm/boot/dts/exynos4412-trats2.dts > +++ b/arch/arm/boot/dts/exynos4412-trats2.dts > @@ -786,4 +786,19 @@ > pulldown-ohm = <100000>; /* 100K */ > io-channels = <&adc 2>; /* Battery temperature */ > }; > + > + thermal-zones { > + cpu_thermal: cpu-thermal { > + cooling-maps { > + map0 { > + /* Corresponds to 800MHz at freq_table */ > + cooling-device = <&cpu0 7 7>; > + }; > + map1 { > + /* Corresponds to 200MHz at freq_table */ > + cooling-device = <&cpu0 13 13>; > + }; > + }; > + }; > + }; > }; > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi > index d8bc059e..9ed8925 100644 > --- a/arch/arm/boot/dts/exynos4412.dtsi > +++ b/arch/arm/boot/dts/exynos4412.dtsi > @@ -22,6 +22,38 @@ > / { > compatible = "samsung,exynos4412", "samsung,exynos4"; > > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu0: cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + reg = <0xA00>; > + cooling-min-level = <13>; > + cooling-max-level = <7>; > + #cooling-cells = <2>; /* min followed by max */ > + }; > + > + cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + reg = <0xA01>; > + }; > + > + cpu@2 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + reg = <0xA02>; > + }; > + > + cpu@3 { > + device_type = "cpu"; > + compatible = "arm,cortex-a9"; > + reg = <0xA03>; > + }; > + }; > + > combiner: interrupt-controller@10440000 { > samsung,combiner-nr = <20>; > }; > diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi > index d55c1a2..a5bbc1a 100644 > --- a/arch/arm/boot/dts/exynos5250.dtsi > +++ b/arch/arm/boot/dts/exynos5250.dtsi > @@ -58,11 +58,14 @@ > #address-cells = <1>; > #size-cells = <0>; > > - cpu@0 { > + cpu0: cpu@0 { > device_type = "cpu"; > compatible = "arm,cortex-a15"; > reg = <0>; > clock-frequency = <1700000000>; > + cooling-min-level = <15>; > + cooling-max-level = <9>; > + #cooling-cells = <2>; /* min followed by max */ > }; > cpu@1 { > device_type = "cpu"; > @@ -241,6 +244,21 @@ > clock-names = "tmu_apbif"; > }; > > + thermal-zones { > + cpu_thermal: cpu-thermal { > + cooling-maps { > + map0 { > + /* Corresponds to 800MHz at freq_table */ > + cooling-device = <&cpu0 9 9>; > + }; > + map1 { > + /* Corresponds to 200MHz at freq_table */ > + cooling-device = <&cpu0 15 15>; > + }; > + }; > + }; > + }; > + > serial@12C00000 { > clocks = <&clock CLK_UART0>, <&clock CLK_SCLK_UART0>; > clock-names = "uart", "clk_uart_baud0"; > -- > 2.0.0.rc2 >
Hi Eduardo, > > Presented patch aims to move data necessary for correct CPU cooling > > device configuration from exynos_tmu_data.c to device tree. > > I believe the patch title is misleading. Looks like you are changing > something at cpu cooling, but in fact, you are changing DTS files. I > would suggest you to use a prefix like 'arm: dts: ....' Now this patch name is: thermal: cpu_cooling: dts: ...... All the code in this patch adds bindings for 'thermal-zone' to map cooling device to proper trip points. In fact this is solely related to cpu_cooling... From the above, I think that the already provided convention is more suitable and "arm: dts: cpu_cooling: thermal" seems a bit awkward for me. If you don't regard my justification as valid, then I will fix this.
On Mon, Jan 12, 2015 at 03:09:16PM +0100, Lukasz Majewski wrote: > Hi Eduardo, > > > > Presented patch aims to move data necessary for correct CPU cooling > > > device configuration from exynos_tmu_data.c to device tree. > > > > I believe the patch title is misleading. Looks like you are changing > > something at cpu cooling, but in fact, you are changing DTS files. I > > would suggest you to use a prefix like 'arm: dts: ....' > > Now this patch name is: > thermal: cpu_cooling: dts: ...... > > All the code in this patch adds bindings for 'thermal-zone' to map > cooling device to proper trip points. > In fact this is solely related to cpu_cooling... > > From the above, I think that the already provided convention is more > suitable and "arm: dts: cpu_cooling: thermal" seems a bit awkward for > me. > > If you don't regard my justification as valid, then I will fix this. Still, the naming is confusing. With that prefix you are saying this patch is something generic about cpu cooling and thermal dts. And that is not what this patch is about. Saying "arm: dts: add cpu cooling bindings for Exynos" is a short and direct subject. Besides, you have the plus to call the attention of the ARM and device tree maintainers. It will be also in my radar. The original subject also makes me think you are dealing with C code, while the former says already upfront that you are talking about arm dts bindings. Cheers, > > -- > Best regards, > > Lukasz Majewski > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Hi Eduardo, > > On Mon, Jan 12, 2015 at 03:09:16PM +0100, Lukasz Majewski wrote: > > Hi Eduardo, > > > > > > Presented patch aims to move data necessary for correct CPU > > > > cooling device configuration from exynos_tmu_data.c to device > > > > tree. > > > > > > I believe the patch title is misleading. Looks like you are > > > changing something at cpu cooling, but in fact, you are changing > > > DTS files. I would suggest you to use a prefix like 'arm: > > > dts: ....' > > > > Now this patch name is: > > thermal: cpu_cooling: dts: ...... > > > > All the code in this patch adds bindings for 'thermal-zone' to map > > cooling device to proper trip points. > > In fact this is solely related to cpu_cooling... > > > > From the above, I think that the already provided convention is more > > suitable and "arm: dts: cpu_cooling: thermal" seems a bit awkward > > for me. > > > > If you don't regard my justification as valid, then I will fix this. > > Still, the naming is confusing. With that prefix you are saying this > patch is something generic about cpu cooling and thermal dts. And that > is not what this patch is about. > > Saying "arm: dts: add cpu cooling bindings for Exynos" is a short and > direct subject. Besides, you have the plus to call the attention of > the ARM and device tree maintainers. It will be also in my radar. Fair enough. Thanks for clarification. > > > The original subject also makes me think you are dealing with C code, > while the former says already upfront that you are talking about arm > dts bindings. > > Cheers, > > > > -- > > Best regards, > > > > Lukasz Majewski > > > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts index b59019c..d9dd9a7 100644 --- a/arch/arm/boot/dts/exynos4210-trats.dts +++ b/arch/arm/boot/dts/exynos4210-trats.dts @@ -428,6 +428,21 @@ status = "okay"; }; + thermal-zones { + cpu_thermal: cpu-thermal { + cooling-maps { + map0 { + /* Corresponds to 800MHz at freq_table */ + cooling-device = <&cpu0 2 2>; + }; + map1 { + /* Corresponds to 200MHz at freq_table */ + cooling-device = <&cpu0 4 4>; + }; + }; + }; + }; + camera { pinctrl-names = "default"; pinctrl-0 = <>; diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index 807bb5b..10e8915 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -41,6 +41,26 @@ #clock-cells = <1>; }; + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu0: cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + reg = <0x900>; + cooling-min-level = <4>; + cooling-max-level = <2>; + #cooling-cells = <2>; /* min followed by max */ + }; + + cpu@1 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + reg = <0x901>; + }; + }; + sysram@02020000 { compatible = "mmio-sram"; reg = <0x02020000 0x20000>; diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi index 3c00e6e..6405954 100644 --- a/arch/arm/boot/dts/exynos4212.dtsi +++ b/arch/arm/boot/dts/exynos4212.dtsi @@ -22,6 +22,26 @@ / { compatible = "samsung,exynos4212", "samsung,exynos4"; + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu0: cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + reg = <0xA00>; + cooling-min-level = <13>; + cooling-max-level = <7>; + #cooling-cells = <2>; /* min followed by max */ + }; + + cpu@1 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + reg = <0xA01>; + }; + }; + combiner: interrupt-controller@10440000 { samsung,combiner-nr = <18>; }; diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi index eb1c08c..15d45f0 100644 --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi @@ -375,6 +375,21 @@ vtmu-supply = <&ldo10_reg>; status = "okay"; }; + + thermal-zones { + cpu_thermal: cpu-thermal { + cooling-maps { + map0 { + /* Corresponds to 800MHz at freq_table */ + cooling-device = <&cpu0 7 7>; + }; + map1 { + /* Corresponds to 200MHz at freq_table */ + cooling-device = <&cpu0 13 13>; + }; + }; + }; + }; }; &pinctrl_1 { diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts index 121430d..396c525 100644 --- a/arch/arm/boot/dts/exynos4412-trats2.dts +++ b/arch/arm/boot/dts/exynos4412-trats2.dts @@ -786,4 +786,19 @@ pulldown-ohm = <100000>; /* 100K */ io-channels = <&adc 2>; /* Battery temperature */ }; + + thermal-zones { + cpu_thermal: cpu-thermal { + cooling-maps { + map0 { + /* Corresponds to 800MHz at freq_table */ + cooling-device = <&cpu0 7 7>; + }; + map1 { + /* Corresponds to 200MHz at freq_table */ + cooling-device = <&cpu0 13 13>; + }; + }; + }; + }; }; diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi index d8bc059e..9ed8925 100644 --- a/arch/arm/boot/dts/exynos4412.dtsi +++ b/arch/arm/boot/dts/exynos4412.dtsi @@ -22,6 +22,38 @@ / { compatible = "samsung,exynos4412", "samsung,exynos4"; + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu0: cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + reg = <0xA00>; + cooling-min-level = <13>; + cooling-max-level = <7>; + #cooling-cells = <2>; /* min followed by max */ + }; + + cpu@1 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + reg = <0xA01>; + }; + + cpu@2 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + reg = <0xA02>; + }; + + cpu@3 { + device_type = "cpu"; + compatible = "arm,cortex-a9"; + reg = <0xA03>; + }; + }; + combiner: interrupt-controller@10440000 { samsung,combiner-nr = <20>; }; diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index d55c1a2..a5bbc1a 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -58,11 +58,14 @@ #address-cells = <1>; #size-cells = <0>; - cpu@0 { + cpu0: cpu@0 { device_type = "cpu"; compatible = "arm,cortex-a15"; reg = <0>; clock-frequency = <1700000000>; + cooling-min-level = <15>; + cooling-max-level = <9>; + #cooling-cells = <2>; /* min followed by max */ }; cpu@1 { device_type = "cpu"; @@ -241,6 +244,21 @@ clock-names = "tmu_apbif"; }; + thermal-zones { + cpu_thermal: cpu-thermal { + cooling-maps { + map0 { + /* Corresponds to 800MHz at freq_table */ + cooling-device = <&cpu0 9 9>; + }; + map1 { + /* Corresponds to 200MHz at freq_table */ + cooling-device = <&cpu0 15 15>; + }; + }; + }; + }; + serial@12C00000 { clocks = <&clock CLK_UART0>, <&clock CLK_SCLK_UART0>; clock-names = "uart", "clk_uart_baud0";
Presented patch aims to move data necessary for correct CPU cooling device configuration from exynos_tmu_data.c to device tree. Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> --- Changes for v2: - None --- arch/arm/boot/dts/exynos4210-trats.dts | 15 ++++++++++++ arch/arm/boot/dts/exynos4210.dtsi | 20 ++++++++++++++++ arch/arm/boot/dts/exynos4212.dtsi | 20 ++++++++++++++++ arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 15 ++++++++++++ arch/arm/boot/dts/exynos4412-trats2.dts | 15 ++++++++++++ arch/arm/boot/dts/exynos4412.dtsi | 32 +++++++++++++++++++++++++ arch/arm/boot/dts/exynos5250.dtsi | 20 +++++++++++++++- 7 files changed, 136 insertions(+), 1 deletion(-)