Hi Keneko-san,
Thanks for your work.
Comments to this patch applies to all patches in this series.
On 2018-12-21 05:49:39 +0900, Yoshihiro Kaneko wrote:
> From: Dien Pham <dien.pham.ry@xxxxxxxxxxx>
>
> [dien.pham.ry: arm64: dts: r8a7795: Add support IPA for CA53 core]
>
> Setup a thermal zone driven by SoC temperature sensor.
> Create passive trip points and bind them to CPUFreq cooling
> device that supports power extension.
>
> In R-Car Gen3, IPA is supportted for only one channel
> (on H3/M3/M3N board, it is channel THS3). Reason:
> Currently, IPA controls base on only CPU temperature.
> And only one thermal channel is assembled closest
> CPU cores is selected as target of IPA.
> If other channels are used, IPA controlling is not properly.
>
> Signed-off-by: Keita Kobayashi <keita.kobayashi.ym@xxxxxxxxxxx>
> [gaku.inami.xw: fix the trip temperature for cooling-device]
> [gaku.inami.xw: fix the power coefficient]
> Signed-off-by: Gaku Inami <gaku.inami.xw@xxxxxxxxxxxxxx>
> Signed-off-by: Hien Dang <hien.dang.eb@xxxxxxxxxxxxxxx>
> Signed-off-by: An Huynh <an.huynh.uj@xxxxxxxxxxxxxxx>
> [takeshi.kihara.df: fix W=1 dtc unit_address_vs_reg warnings]
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@xxxxxxxxxxx>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@xxxxxxxxx>
> ---
>
> This patch is based on the devel branch of Simon Horman's renesas tree.
>
> arch/arm64/boot/dts/renesas/r8a7795.dtsi | 64 ++++++++++++++------------------
> 1 file changed, 27 insertions(+), 37 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> index af9605d..dd52b50 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> @@ -155,6 +155,9 @@
> power-domains = <&sysc R8A7795_PD_CA57_CPU0>;
> next-level-cache = <&L2_CA57>;
> enable-method = "psci";
> + dynamic-power-coefficient = <854>;
> + cooling-min-level = <0>;
> + cooling-max-level = <2>;
I can't find any documentation or code which makes use of the
cooling-min-level and cooling-max-level properties on v5.0-rc1. What is
the intended usage for these properties?
> clocks = <&cpg CPG_CORE R8A7795_CLK_Z>;
> operating-points-v2 = <&cluster0_opp>;
> capacity-dmips-mhz = <1024>;
> @@ -207,6 +210,10 @@
> power-domains = <&sysc R8A7795_PD_CA53_CPU0>;
> next-level-cache = <&L2_CA53>;
> enable-method = "psci";
> + #cooling-cells = <2>;
> + dynamic-power-coefficient = <277>;
> + cooling-min-level = <0>;
> + cooling-max-level = <2>;
> clocks = <&cpg CPG_CORE R8A7795_CLK_Z2>;
> operating-points-v2 = <&cluster1_opp>;
> capacity-dmips-mhz = <535>;
> @@ -3098,58 +3105,30 @@
> polling-delay-passive = <250>;
> polling-delay = <1000>;
> thermal-sensors = <&tsc 0>;
> + sustainable-power = <6313>;
>
> trips {
> - sensor1_passive: sensor1-passive {
> - temperature = <95000>;
> - hysteresis = <1000>;
> - type = "passive";
> - };
> sensor1_crit: sensor1-crit {
> temperature = <120000>;
> hysteresis = <1000>;
> type = "critical";
> };
> };
> -
> - cooling-maps {
> - map0 {
> - trip = <&sensor1_passive>;
> - cooling-device = <&a57_0 4 4>,
> - <&a57_1 4 4>,
> - <&a57_2 4 4>,
> - <&a57_3 4 4>;
> - };
> - };
> };
>
> sensor_thermal2: sensor-thermal2 {
> polling-delay-passive = <250>;
> polling-delay = <1000>;
> thermal-sensors = <&tsc 1>;
> + sustainable-power = <6313>;
>
> trips {
> - sensor2_passive: sensor2-passive {
> - temperature = <95000>;
> - hysteresis = <1000>;
> - type = "passive";
> - };
> sensor2_crit: sensor2-crit {
> temperature = <120000>;
> hysteresis = <1000>;
> type = "critical";
> };
> };
> -
> - cooling-maps {
> - map0 {
> - trip = <&sensor2_passive>;
> - cooling-device = <&a57_0 4 4>,
> - <&a57_1 4 4>,
> - <&a57_2 4 4>,
> - <&a57_3 4 4>;
> - };
> - };
> };
>
> sensor_thermal3: sensor-thermal3 {
> @@ -3158,11 +3137,18 @@
> thermal-sensors = <&tsc 2>;
>
> trips {
> - sensor3_passive: sensor3-passive {
> - temperature = <95000>;
> + threshold: trip-point0 {
> + temperature = <90000>;
> + hysteresis = <1000>;
> + type = "passive";
> + };
What is the usage of trip-point0? The label threshold is never
referenced anywhere, or am I missing something?
> +
> + target: trip-point1 {
> + temperature = <100000>;
> hysteresis = <1000>;
> type = "passive";
> };
> +
> sensor3_crit: sensor3-crit {
> temperature = <120000>;
> hysteresis = <1000>;
> @@ -3172,11 +3158,15 @@
>
> cooling-maps {
> map0 {
> - trip = <&sensor3_passive>;
> - cooling-device = <&a57_0 4 4>,
> - <&a57_1 4 4>,
> - <&a57_2 4 4>,
> - <&a57_3 4 4>;
> + trip = <&target>;
> + cooling-device = <&a57_0 0 2>;
We have 5 (0-4) cooling states for the A57s on this SoC. Out of
curiosity why allow states 0-2 here and not force it do more cooling or
keep the to max cooling (4) as before this change as this is set to a
trip point with a rather large temperature? Not saying this is wrong
only curious :-)
> + contribution = <1024>;
> + };
> +
> + map1 {
> + trip = <&target>;
> + cooling-device = <&a53_0 0 2>;
> + contribution = <1024>;
> };
> };
> };
> --
> 1.9.1
>
--
Regards,
Niklas Söderlund
