linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()"
       [not found] <930778a1-5e8b-6df6-3276-42dcdadaf682@systemli.org>
@ 2022-12-02  5:26 ` Viresh Kumar
  2022-12-02 12:17   ` Rafael J. Wysocki
  2022-12-02 12:47   ` Nick
  0 siblings, 2 replies; 7+ messages in thread
From: Viresh Kumar @ 2022-12-02  5:26 UTC (permalink / raw)
  To: Rafael J. Wysocki, Viresh Kumar, Matthias Brugger
  Cc: linux-pm, Vincent Guittot, regressions, daniel, thomas.huehn,
	v5 . 19+,
	Nick, linux-kernel, linux-arm-kernel, linux-mediatek

This reverts commit 6a17b3876bc8303612d7ad59ecf7cbc0db418bcd.

This commit caused regression on Banana Pi R64 (MT7622), revert until
the problem is identified and fixed properly.

Link: https://lore.kernel.org/all/930778a1-5e8b-6df6-3276-42dcdadaf682@systemli.org/
Cc: v5.19+ <stable@vger.kernel.org> # v5.19+
Reported-by: Nick <vincent@systemli.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/mediatek-cpufreq.c | 147 +++++++++++++++++++----------
 1 file changed, 96 insertions(+), 51 deletions(-)

diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index 7f2680bc9a0f..4466d0c91a6a 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -8,7 +8,6 @@
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
-#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
@@ -16,6 +15,8 @@
 #include <linux/pm_opp.h>
 #include <linux/regulator/consumer.h>
 
+#define VOLT_TOL		(10000)
+
 struct mtk_cpufreq_platform_data {
 	int min_volt_shift;
 	int max_volt_shift;
@@ -55,7 +56,6 @@ struct mtk_cpu_dvfs_info {
 	unsigned int opp_cpu;
 	unsigned long current_freq;
 	const struct mtk_cpufreq_platform_data *soc_data;
-	int vtrack_max;
 	bool ccifreq_bound;
 };
 
@@ -82,7 +82,6 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 	struct regulator *proc_reg = info->proc_reg;
 	struct regulator *sram_reg = info->sram_reg;
 	int pre_vproc, pre_vsram, new_vsram, vsram, vproc, ret;
-	int retry = info->vtrack_max;
 
 	pre_vproc = regulator_get_voltage(proc_reg);
 	if (pre_vproc < 0) {
@@ -90,44 +89,91 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 			"invalid Vproc value: %d\n", pre_vproc);
 		return pre_vproc;
 	}
+	/* Vsram should not exceed the maximum allowed voltage of SoC. */
+	new_vsram = min(new_vproc + soc_data->min_volt_shift,
+			soc_data->sram_max_volt);
+
+	if (pre_vproc < new_vproc) {
+		/*
+		 * When scaling up voltages, Vsram and Vproc scale up step
+		 * by step. At each step, set Vsram to (Vproc + 200mV) first,
+		 * then set Vproc to (Vsram - 100mV).
+		 * Keep doing it until Vsram and Vproc hit target voltages.
+		 */
+		do {
+			pre_vsram = regulator_get_voltage(sram_reg);
+			if (pre_vsram < 0) {
+				dev_err(info->cpu_dev,
+					"invalid Vsram value: %d\n", pre_vsram);
+				return pre_vsram;
+			}
+			pre_vproc = regulator_get_voltage(proc_reg);
+			if (pre_vproc < 0) {
+				dev_err(info->cpu_dev,
+					"invalid Vproc value: %d\n", pre_vproc);
+				return pre_vproc;
+			}
 
-	pre_vsram = regulator_get_voltage(sram_reg);
-	if (pre_vsram < 0) {
-		dev_err(info->cpu_dev, "invalid Vsram value: %d\n", pre_vsram);
-		return pre_vsram;
-	}
+			vsram = min(new_vsram,
+				    pre_vproc + soc_data->min_volt_shift);
 
-	new_vsram = clamp(new_vproc + soc_data->min_volt_shift,
-			  soc_data->sram_min_volt, soc_data->sram_max_volt);
+			if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
+				vsram = soc_data->sram_max_volt;
 
-	do {
-		if (pre_vproc <= new_vproc) {
-			vsram = clamp(pre_vproc + soc_data->max_volt_shift,
-				      soc_data->sram_min_volt, new_vsram);
-			ret = regulator_set_voltage(sram_reg, vsram,
-						    soc_data->sram_max_volt);
+				/*
+				 * If the target Vsram hits the maximum voltage,
+				 * try to set the exact voltage value first.
+				 */
+				ret = regulator_set_voltage(sram_reg, vsram,
+							    vsram);
+				if (ret)
+					ret = regulator_set_voltage(sram_reg,
+							vsram - VOLT_TOL,
+							vsram);
 
-			if (ret)
-				return ret;
-
-			if (vsram == soc_data->sram_max_volt ||
-			    new_vsram == soc_data->sram_min_volt)
 				vproc = new_vproc;
-			else
+			} else {
+				ret = regulator_set_voltage(sram_reg, vsram,
+							    vsram + VOLT_TOL);
+
 				vproc = vsram - soc_data->min_volt_shift;
+			}
+			if (ret)
+				return ret;
 
 			ret = regulator_set_voltage(proc_reg, vproc,
-						    soc_data->proc_max_volt);
+						    vproc + VOLT_TOL);
 			if (ret) {
 				regulator_set_voltage(sram_reg, pre_vsram,
-						      soc_data->sram_max_volt);
+						      pre_vsram);
 				return ret;
 			}
-		} else if (pre_vproc > new_vproc) {
+		} while (vproc < new_vproc || vsram < new_vsram);
+	} else if (pre_vproc > new_vproc) {
+		/*
+		 * When scaling down voltages, Vsram and Vproc scale down step
+		 * by step. At each step, set Vproc to (Vsram - 200mV) first,
+		 * then set Vproc to (Vproc + 100mV).
+		 * Keep doing it until Vsram and Vproc hit target voltages.
+		 */
+		do {
+			pre_vproc = regulator_get_voltage(proc_reg);
+			if (pre_vproc < 0) {
+				dev_err(info->cpu_dev,
+					"invalid Vproc value: %d\n", pre_vproc);
+				return pre_vproc;
+			}
+			pre_vsram = regulator_get_voltage(sram_reg);
+			if (pre_vsram < 0) {
+				dev_err(info->cpu_dev,
+					"invalid Vsram value: %d\n", pre_vsram);
+				return pre_vsram;
+			}
+
 			vproc = max(new_vproc,
 				    pre_vsram - soc_data->max_volt_shift);
 			ret = regulator_set_voltage(proc_reg, vproc,
-						    soc_data->proc_max_volt);
+						    vproc + VOLT_TOL);
 			if (ret)
 				return ret;
 
@@ -137,24 +183,32 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
 				vsram = max(new_vsram,
 					    vproc + soc_data->min_volt_shift);
 
-			ret = regulator_set_voltage(sram_reg, vsram,
-						    soc_data->sram_max_volt);
+			if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
+				vsram = soc_data->sram_max_volt;
+
+				/*
+				 * If the target Vsram hits the maximum voltage,
+				 * try to set the exact voltage value first.
+				 */
+				ret = regulator_set_voltage(sram_reg, vsram,
+							    vsram);
+				if (ret)
+					ret = regulator_set_voltage(sram_reg,
+							vsram - VOLT_TOL,
+							vsram);
+			} else {
+				ret = regulator_set_voltage(sram_reg, vsram,
+							    vsram + VOLT_TOL);
+			}
+
 			if (ret) {
 				regulator_set_voltage(proc_reg, pre_vproc,
-						      soc_data->proc_max_volt);
+						      pre_vproc);
 				return ret;
 			}
-		}
-
-		pre_vproc = vproc;
-		pre_vsram = vsram;
-
-		if (--retry < 0) {
-			dev_err(info->cpu_dev,
-				"over loop count, failed to set voltage\n");
-			return -EINVAL;
-		}
-	} while (vproc != new_vproc || vsram != new_vsram);
+		} while (vproc > new_vproc + VOLT_TOL ||
+			 vsram > new_vsram + VOLT_TOL);
+	}
 
 	return 0;
 }
@@ -250,8 +304,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
 	 * If the new voltage or the intermediate voltage is higher than the
 	 * current voltage, scale up voltage first.
 	 */
-	target_vproc = max(inter_vproc, vproc);
-	if (pre_vproc <= target_vproc) {
+	target_vproc = (inter_vproc > vproc) ? inter_vproc : vproc;
+	if (pre_vproc < target_vproc) {
 		ret = mtk_cpufreq_set_voltage(info, target_vproc);
 		if (ret) {
 			dev_err(cpu_dev,
@@ -513,15 +567,6 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
 	 */
 	info->need_voltage_tracking = (info->sram_reg != NULL);
 
-	/*
-	 * We assume min voltage is 0 and tracking target voltage using
-	 * min_volt_shift for each iteration.
-	 * The vtrack_max is 3 times of expeted iteration count.
-	 */
-	info->vtrack_max = 3 * DIV_ROUND_UP(max(info->soc_data->sram_max_volt,
-						info->soc_data->proc_max_volt),
-					    info->soc_data->min_volt_shift);
-
 	return 0;
 
 out_disable_inter_clock:
-- 
2.31.1.272.g89b43f80a514


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()"
  2022-12-02  5:26 ` [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()" Viresh Kumar
@ 2022-12-02 12:17   ` Rafael J. Wysocki
  2022-12-02 19:46     ` Rafael J. Wysocki
  2022-12-02 12:47   ` Nick
  1 sibling, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2022-12-02 12:17 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael J. Wysocki, Matthias Brugger, linux-pm, Vincent Guittot,
	regressions, daniel, thomas.huehn, v5 . 19+,
	Nick, linux-kernel, linux-arm-kernel, linux-mediatek

On Fri, Dec 2, 2022 at 6:26 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> This reverts commit 6a17b3876bc8303612d7ad59ecf7cbc0db418bcd.
>
> This commit caused regression on Banana Pi R64 (MT7622), revert until
> the problem is identified and fixed properly.
>
> Link: https://lore.kernel.org/all/930778a1-5e8b-6df6-3276-42dcdadaf682@systemli.org/
> Cc: v5.19+ <stable@vger.kernel.org> # v5.19+
> Reported-by: Nick <vincent@systemli.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Do you want me to push this revert for -rc8?

> ---
>  drivers/cpufreq/mediatek-cpufreq.c | 147 +++++++++++++++++++----------
>  1 file changed, 96 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> index 7f2680bc9a0f..4466d0c91a6a 100644
> --- a/drivers/cpufreq/mediatek-cpufreq.c
> +++ b/drivers/cpufreq/mediatek-cpufreq.c
> @@ -8,7 +8,6 @@
>  #include <linux/cpu.h>
>  #include <linux/cpufreq.h>
>  #include <linux/cpumask.h>
> -#include <linux/minmax.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> @@ -16,6 +15,8 @@
>  #include <linux/pm_opp.h>
>  #include <linux/regulator/consumer.h>
>
> +#define VOLT_TOL               (10000)
> +
>  struct mtk_cpufreq_platform_data {
>         int min_volt_shift;
>         int max_volt_shift;
> @@ -55,7 +56,6 @@ struct mtk_cpu_dvfs_info {
>         unsigned int opp_cpu;
>         unsigned long current_freq;
>         const struct mtk_cpufreq_platform_data *soc_data;
> -       int vtrack_max;
>         bool ccifreq_bound;
>  };
>
> @@ -82,7 +82,6 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
>         struct regulator *proc_reg = info->proc_reg;
>         struct regulator *sram_reg = info->sram_reg;
>         int pre_vproc, pre_vsram, new_vsram, vsram, vproc, ret;
> -       int retry = info->vtrack_max;
>
>         pre_vproc = regulator_get_voltage(proc_reg);
>         if (pre_vproc < 0) {
> @@ -90,44 +89,91 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
>                         "invalid Vproc value: %d\n", pre_vproc);
>                 return pre_vproc;
>         }
> +       /* Vsram should not exceed the maximum allowed voltage of SoC. */
> +       new_vsram = min(new_vproc + soc_data->min_volt_shift,
> +                       soc_data->sram_max_volt);
> +
> +       if (pre_vproc < new_vproc) {
> +               /*
> +                * When scaling up voltages, Vsram and Vproc scale up step
> +                * by step. At each step, set Vsram to (Vproc + 200mV) first,
> +                * then set Vproc to (Vsram - 100mV).
> +                * Keep doing it until Vsram and Vproc hit target voltages.
> +                */
> +               do {
> +                       pre_vsram = regulator_get_voltage(sram_reg);
> +                       if (pre_vsram < 0) {
> +                               dev_err(info->cpu_dev,
> +                                       "invalid Vsram value: %d\n", pre_vsram);
> +                               return pre_vsram;
> +                       }
> +                       pre_vproc = regulator_get_voltage(proc_reg);
> +                       if (pre_vproc < 0) {
> +                               dev_err(info->cpu_dev,
> +                                       "invalid Vproc value: %d\n", pre_vproc);
> +                               return pre_vproc;
> +                       }
>
> -       pre_vsram = regulator_get_voltage(sram_reg);
> -       if (pre_vsram < 0) {
> -               dev_err(info->cpu_dev, "invalid Vsram value: %d\n", pre_vsram);
> -               return pre_vsram;
> -       }
> +                       vsram = min(new_vsram,
> +                                   pre_vproc + soc_data->min_volt_shift);
>
> -       new_vsram = clamp(new_vproc + soc_data->min_volt_shift,
> -                         soc_data->sram_min_volt, soc_data->sram_max_volt);
> +                       if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
> +                               vsram = soc_data->sram_max_volt;
>
> -       do {
> -               if (pre_vproc <= new_vproc) {
> -                       vsram = clamp(pre_vproc + soc_data->max_volt_shift,
> -                                     soc_data->sram_min_volt, new_vsram);
> -                       ret = regulator_set_voltage(sram_reg, vsram,
> -                                                   soc_data->sram_max_volt);
> +                               /*
> +                                * If the target Vsram hits the maximum voltage,
> +                                * try to set the exact voltage value first.
> +                                */
> +                               ret = regulator_set_voltage(sram_reg, vsram,
> +                                                           vsram);
> +                               if (ret)
> +                                       ret = regulator_set_voltage(sram_reg,
> +                                                       vsram - VOLT_TOL,
> +                                                       vsram);
>
> -                       if (ret)
> -                               return ret;
> -
> -                       if (vsram == soc_data->sram_max_volt ||
> -                           new_vsram == soc_data->sram_min_volt)
>                                 vproc = new_vproc;
> -                       else
> +                       } else {
> +                               ret = regulator_set_voltage(sram_reg, vsram,
> +                                                           vsram + VOLT_TOL);
> +
>                                 vproc = vsram - soc_data->min_volt_shift;
> +                       }
> +                       if (ret)
> +                               return ret;
>
>                         ret = regulator_set_voltage(proc_reg, vproc,
> -                                                   soc_data->proc_max_volt);
> +                                                   vproc + VOLT_TOL);
>                         if (ret) {
>                                 regulator_set_voltage(sram_reg, pre_vsram,
> -                                                     soc_data->sram_max_volt);
> +                                                     pre_vsram);
>                                 return ret;
>                         }
> -               } else if (pre_vproc > new_vproc) {
> +               } while (vproc < new_vproc || vsram < new_vsram);
> +       } else if (pre_vproc > new_vproc) {
> +               /*
> +                * When scaling down voltages, Vsram and Vproc scale down step
> +                * by step. At each step, set Vproc to (Vsram - 200mV) first,
> +                * then set Vproc to (Vproc + 100mV).
> +                * Keep doing it until Vsram and Vproc hit target voltages.
> +                */
> +               do {
> +                       pre_vproc = regulator_get_voltage(proc_reg);
> +                       if (pre_vproc < 0) {
> +                               dev_err(info->cpu_dev,
> +                                       "invalid Vproc value: %d\n", pre_vproc);
> +                               return pre_vproc;
> +                       }
> +                       pre_vsram = regulator_get_voltage(sram_reg);
> +                       if (pre_vsram < 0) {
> +                               dev_err(info->cpu_dev,
> +                                       "invalid Vsram value: %d\n", pre_vsram);
> +                               return pre_vsram;
> +                       }
> +
>                         vproc = max(new_vproc,
>                                     pre_vsram - soc_data->max_volt_shift);
>                         ret = regulator_set_voltage(proc_reg, vproc,
> -                                                   soc_data->proc_max_volt);
> +                                                   vproc + VOLT_TOL);
>                         if (ret)
>                                 return ret;
>
> @@ -137,24 +183,32 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
>                                 vsram = max(new_vsram,
>                                             vproc + soc_data->min_volt_shift);
>
> -                       ret = regulator_set_voltage(sram_reg, vsram,
> -                                                   soc_data->sram_max_volt);
> +                       if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
> +                               vsram = soc_data->sram_max_volt;
> +
> +                               /*
> +                                * If the target Vsram hits the maximum voltage,
> +                                * try to set the exact voltage value first.
> +                                */
> +                               ret = regulator_set_voltage(sram_reg, vsram,
> +                                                           vsram);
> +                               if (ret)
> +                                       ret = regulator_set_voltage(sram_reg,
> +                                                       vsram - VOLT_TOL,
> +                                                       vsram);
> +                       } else {
> +                               ret = regulator_set_voltage(sram_reg, vsram,
> +                                                           vsram + VOLT_TOL);
> +                       }
> +
>                         if (ret) {
>                                 regulator_set_voltage(proc_reg, pre_vproc,
> -                                                     soc_data->proc_max_volt);
> +                                                     pre_vproc);
>                                 return ret;
>                         }
> -               }
> -
> -               pre_vproc = vproc;
> -               pre_vsram = vsram;
> -
> -               if (--retry < 0) {
> -                       dev_err(info->cpu_dev,
> -                               "over loop count, failed to set voltage\n");
> -                       return -EINVAL;
> -               }
> -       } while (vproc != new_vproc || vsram != new_vsram);
> +               } while (vproc > new_vproc + VOLT_TOL ||
> +                        vsram > new_vsram + VOLT_TOL);
> +       }
>
>         return 0;
>  }
> @@ -250,8 +304,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
>          * If the new voltage or the intermediate voltage is higher than the
>          * current voltage, scale up voltage first.
>          */
> -       target_vproc = max(inter_vproc, vproc);
> -       if (pre_vproc <= target_vproc) {
> +       target_vproc = (inter_vproc > vproc) ? inter_vproc : vproc;
> +       if (pre_vproc < target_vproc) {
>                 ret = mtk_cpufreq_set_voltage(info, target_vproc);
>                 if (ret) {
>                         dev_err(cpu_dev,
> @@ -513,15 +567,6 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
>          */
>         info->need_voltage_tracking = (info->sram_reg != NULL);
>
> -       /*
> -        * We assume min voltage is 0 and tracking target voltage using
> -        * min_volt_shift for each iteration.
> -        * The vtrack_max is 3 times of expeted iteration count.
> -        */
> -       info->vtrack_max = 3 * DIV_ROUND_UP(max(info->soc_data->sram_max_volt,
> -                                               info->soc_data->proc_max_volt),
> -                                           info->soc_data->min_volt_shift);
> -
>         return 0;
>
>  out_disable_inter_clock:
> --
> 2.31.1.272.g89b43f80a514
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()"
  2022-12-02  5:26 ` [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()" Viresh Kumar
  2022-12-02 12:17   ` Rafael J. Wysocki
@ 2022-12-02 12:47   ` Nick
  1 sibling, 0 replies; 7+ messages in thread
From: Nick @ 2022-12-02 12:47 UTC (permalink / raw)
  To: Viresh Kumar, Rafael J. Wysocki, Matthias Brugger
  Cc: linux-pm, Vincent Guittot, regressions, daniel, thomas.huehn,
	v5 . 19+,
	linux-kernel, linux-arm-kernel, linux-mediatek

I tested this again on linux/master on the Banana Pi R64 (mt7622). Seems 
to work fine:
https://gist.githubusercontent.com/PolynomialDivision/5022dec1874a1c411ece6c9dabec59b5/raw/7ac62b38d10e41a56ff1db3142571117ee6f4c26/mt7622-cpufreq-revert.log

So if you want you can add:
Reported-by: Nick Hainke <vincent@systemli.org>
Tested-by: Nick Hainke <vincent@systemli.org>

Bests
Nick

On 12/2/22 06:26, Viresh Kumar wrote:
> This reverts commit 6a17b3876bc8303612d7ad59ecf7cbc0db418bcd.
>
> This commit caused regression on Banana Pi R64 (MT7622), revert until
> the problem is identified and fixed properly.
>
> Link: https://lore.kernel.org/all/930778a1-5e8b-6df6-3276-42dcdadaf682@systemli.org/
> Cc: v5.19+ <stable@vger.kernel.org> # v5.19+
> Reported-by: Nick <vincent@systemli.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   drivers/cpufreq/mediatek-cpufreq.c | 147 +++++++++++++++++++----------
>   1 file changed, 96 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> index 7f2680bc9a0f..4466d0c91a6a 100644
> --- a/drivers/cpufreq/mediatek-cpufreq.c
> +++ b/drivers/cpufreq/mediatek-cpufreq.c
> @@ -8,7 +8,6 @@
>   #include <linux/cpu.h>
>   #include <linux/cpufreq.h>
>   #include <linux/cpumask.h>
> -#include <linux/minmax.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
>   #include <linux/of_platform.h>
> @@ -16,6 +15,8 @@
>   #include <linux/pm_opp.h>
>   #include <linux/regulator/consumer.h>
>   
> +#define VOLT_TOL		(10000)
> +
>   struct mtk_cpufreq_platform_data {
>   	int min_volt_shift;
>   	int max_volt_shift;
> @@ -55,7 +56,6 @@ struct mtk_cpu_dvfs_info {
>   	unsigned int opp_cpu;
>   	unsigned long current_freq;
>   	const struct mtk_cpufreq_platform_data *soc_data;
> -	int vtrack_max;
>   	bool ccifreq_bound;
>   };
>   
> @@ -82,7 +82,6 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
>   	struct regulator *proc_reg = info->proc_reg;
>   	struct regulator *sram_reg = info->sram_reg;
>   	int pre_vproc, pre_vsram, new_vsram, vsram, vproc, ret;
> -	int retry = info->vtrack_max;
>   
>   	pre_vproc = regulator_get_voltage(proc_reg);
>   	if (pre_vproc < 0) {
> @@ -90,44 +89,91 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
>   			"invalid Vproc value: %d\n", pre_vproc);
>   		return pre_vproc;
>   	}
> +	/* Vsram should not exceed the maximum allowed voltage of SoC. */
> +	new_vsram = min(new_vproc + soc_data->min_volt_shift,
> +			soc_data->sram_max_volt);
> +
> +	if (pre_vproc < new_vproc) {
> +		/*
> +		 * When scaling up voltages, Vsram and Vproc scale up step
> +		 * by step. At each step, set Vsram to (Vproc + 200mV) first,
> +		 * then set Vproc to (Vsram - 100mV).
> +		 * Keep doing it until Vsram and Vproc hit target voltages.
> +		 */
> +		do {
> +			pre_vsram = regulator_get_voltage(sram_reg);
> +			if (pre_vsram < 0) {
> +				dev_err(info->cpu_dev,
> +					"invalid Vsram value: %d\n", pre_vsram);
> +				return pre_vsram;
> +			}
> +			pre_vproc = regulator_get_voltage(proc_reg);
> +			if (pre_vproc < 0) {
> +				dev_err(info->cpu_dev,
> +					"invalid Vproc value: %d\n", pre_vproc);
> +				return pre_vproc;
> +			}
>   
> -	pre_vsram = regulator_get_voltage(sram_reg);
> -	if (pre_vsram < 0) {
> -		dev_err(info->cpu_dev, "invalid Vsram value: %d\n", pre_vsram);
> -		return pre_vsram;
> -	}
> +			vsram = min(new_vsram,
> +				    pre_vproc + soc_data->min_volt_shift);
>   
> -	new_vsram = clamp(new_vproc + soc_data->min_volt_shift,
> -			  soc_data->sram_min_volt, soc_data->sram_max_volt);
> +			if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
> +				vsram = soc_data->sram_max_volt;
>   
> -	do {
> -		if (pre_vproc <= new_vproc) {
> -			vsram = clamp(pre_vproc + soc_data->max_volt_shift,
> -				      soc_data->sram_min_volt, new_vsram);
> -			ret = regulator_set_voltage(sram_reg, vsram,
> -						    soc_data->sram_max_volt);
> +				/*
> +				 * If the target Vsram hits the maximum voltage,
> +				 * try to set the exact voltage value first.
> +				 */
> +				ret = regulator_set_voltage(sram_reg, vsram,
> +							    vsram);
> +				if (ret)
> +					ret = regulator_set_voltage(sram_reg,
> +							vsram - VOLT_TOL,
> +							vsram);
>   
> -			if (ret)
> -				return ret;
> -
> -			if (vsram == soc_data->sram_max_volt ||
> -			    new_vsram == soc_data->sram_min_volt)
>   				vproc = new_vproc;
> -			else
> +			} else {
> +				ret = regulator_set_voltage(sram_reg, vsram,
> +							    vsram + VOLT_TOL);
> +
>   				vproc = vsram - soc_data->min_volt_shift;
> +			}
> +			if (ret)
> +				return ret;
>   
>   			ret = regulator_set_voltage(proc_reg, vproc,
> -						    soc_data->proc_max_volt);
> +						    vproc + VOLT_TOL);
>   			if (ret) {
>   				regulator_set_voltage(sram_reg, pre_vsram,
> -						      soc_data->sram_max_volt);
> +						      pre_vsram);
>   				return ret;
>   			}
> -		} else if (pre_vproc > new_vproc) {
> +		} while (vproc < new_vproc || vsram < new_vsram);
> +	} else if (pre_vproc > new_vproc) {
> +		/*
> +		 * When scaling down voltages, Vsram and Vproc scale down step
> +		 * by step. At each step, set Vproc to (Vsram - 200mV) first,
> +		 * then set Vproc to (Vproc + 100mV).
> +		 * Keep doing it until Vsram and Vproc hit target voltages.
> +		 */
> +		do {
> +			pre_vproc = regulator_get_voltage(proc_reg);
> +			if (pre_vproc < 0) {
> +				dev_err(info->cpu_dev,
> +					"invalid Vproc value: %d\n", pre_vproc);
> +				return pre_vproc;
> +			}
> +			pre_vsram = regulator_get_voltage(sram_reg);
> +			if (pre_vsram < 0) {
> +				dev_err(info->cpu_dev,
> +					"invalid Vsram value: %d\n", pre_vsram);
> +				return pre_vsram;
> +			}
> +
>   			vproc = max(new_vproc,
>   				    pre_vsram - soc_data->max_volt_shift);
>   			ret = regulator_set_voltage(proc_reg, vproc,
> -						    soc_data->proc_max_volt);
> +						    vproc + VOLT_TOL);
>   			if (ret)
>   				return ret;
>   
> @@ -137,24 +183,32 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
>   				vsram = max(new_vsram,
>   					    vproc + soc_data->min_volt_shift);
>   
> -			ret = regulator_set_voltage(sram_reg, vsram,
> -						    soc_data->sram_max_volt);
> +			if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
> +				vsram = soc_data->sram_max_volt;
> +
> +				/*
> +				 * If the target Vsram hits the maximum voltage,
> +				 * try to set the exact voltage value first.
> +				 */
> +				ret = regulator_set_voltage(sram_reg, vsram,
> +							    vsram);
> +				if (ret)
> +					ret = regulator_set_voltage(sram_reg,
> +							vsram - VOLT_TOL,
> +							vsram);
> +			} else {
> +				ret = regulator_set_voltage(sram_reg, vsram,
> +							    vsram + VOLT_TOL);
> +			}
> +
>   			if (ret) {
>   				regulator_set_voltage(proc_reg, pre_vproc,
> -						      soc_data->proc_max_volt);
> +						      pre_vproc);
>   				return ret;
>   			}
> -		}
> -
> -		pre_vproc = vproc;
> -		pre_vsram = vsram;
> -
> -		if (--retry < 0) {
> -			dev_err(info->cpu_dev,
> -				"over loop count, failed to set voltage\n");
> -			return -EINVAL;
> -		}
> -	} while (vproc != new_vproc || vsram != new_vsram);
> +		} while (vproc > new_vproc + VOLT_TOL ||
> +			 vsram > new_vsram + VOLT_TOL);
> +	}
>   
>   	return 0;
>   }
> @@ -250,8 +304,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
>   	 * If the new voltage or the intermediate voltage is higher than the
>   	 * current voltage, scale up voltage first.
>   	 */
> -	target_vproc = max(inter_vproc, vproc);
> -	if (pre_vproc <= target_vproc) {
> +	target_vproc = (inter_vproc > vproc) ? inter_vproc : vproc;
> +	if (pre_vproc < target_vproc) {
>   		ret = mtk_cpufreq_set_voltage(info, target_vproc);
>   		if (ret) {
>   			dev_err(cpu_dev,
> @@ -513,15 +567,6 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
>   	 */
>   	info->need_voltage_tracking = (info->sram_reg != NULL);
>   
> -	/*
> -	 * We assume min voltage is 0 and tracking target voltage using
> -	 * min_volt_shift for each iteration.
> -	 * The vtrack_max is 3 times of expeted iteration count.
> -	 */
> -	info->vtrack_max = 3 * DIV_ROUND_UP(max(info->soc_data->sram_max_volt,
> -						info->soc_data->proc_max_volt),
> -					    info->soc_data->min_volt_shift);
> -
>   	return 0;
>   
>   out_disable_inter_clock:

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()"
  2022-12-02 12:17   ` Rafael J. Wysocki
@ 2022-12-02 19:46     ` Rafael J. Wysocki
  2022-12-05  4:30       ` Viresh Kumar
  0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2022-12-02 19:46 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Matthias Brugger, linux-pm, Vincent Guittot, regressions, daniel,
	thomas.huehn, v5 . 19+,
	Nick, linux-kernel, linux-arm-kernel, linux-mediatek

On Fri, Dec 2, 2022 at 1:17 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Dec 2, 2022 at 6:26 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > This reverts commit 6a17b3876bc8303612d7ad59ecf7cbc0db418bcd.
> >
> > This commit caused regression on Banana Pi R64 (MT7622), revert until
> > the problem is identified and fixed properly.
> >
> > Link: https://lore.kernel.org/all/930778a1-5e8b-6df6-3276-42dcdadaf682@systemli.org/
> > Cc: v5.19+ <stable@vger.kernel.org> # v5.19+
> > Reported-by: Nick <vincent@systemli.org>
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> Do you want me to push this revert for -rc8?

After all, I've decided to queue it up for 6.2, thanks!

> > ---
> >  drivers/cpufreq/mediatek-cpufreq.c | 147 +++++++++++++++++++----------
> >  1 file changed, 96 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> > index 7f2680bc9a0f..4466d0c91a6a 100644
> > --- a/drivers/cpufreq/mediatek-cpufreq.c
> > +++ b/drivers/cpufreq/mediatek-cpufreq.c
> > @@ -8,7 +8,6 @@
> >  #include <linux/cpu.h>
> >  #include <linux/cpufreq.h>
> >  #include <linux/cpumask.h>
> > -#include <linux/minmax.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/of_platform.h>
> > @@ -16,6 +15,8 @@
> >  #include <linux/pm_opp.h>
> >  #include <linux/regulator/consumer.h>
> >
> > +#define VOLT_TOL               (10000)
> > +
> >  struct mtk_cpufreq_platform_data {
> >         int min_volt_shift;
> >         int max_volt_shift;
> > @@ -55,7 +56,6 @@ struct mtk_cpu_dvfs_info {
> >         unsigned int opp_cpu;
> >         unsigned long current_freq;
> >         const struct mtk_cpufreq_platform_data *soc_data;
> > -       int vtrack_max;
> >         bool ccifreq_bound;
> >  };
> >
> > @@ -82,7 +82,6 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
> >         struct regulator *proc_reg = info->proc_reg;
> >         struct regulator *sram_reg = info->sram_reg;
> >         int pre_vproc, pre_vsram, new_vsram, vsram, vproc, ret;
> > -       int retry = info->vtrack_max;
> >
> >         pre_vproc = regulator_get_voltage(proc_reg);
> >         if (pre_vproc < 0) {
> > @@ -90,44 +89,91 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
> >                         "invalid Vproc value: %d\n", pre_vproc);
> >                 return pre_vproc;
> >         }
> > +       /* Vsram should not exceed the maximum allowed voltage of SoC. */
> > +       new_vsram = min(new_vproc + soc_data->min_volt_shift,
> > +                       soc_data->sram_max_volt);
> > +
> > +       if (pre_vproc < new_vproc) {
> > +               /*
> > +                * When scaling up voltages, Vsram and Vproc scale up step
> > +                * by step. At each step, set Vsram to (Vproc + 200mV) first,
> > +                * then set Vproc to (Vsram - 100mV).
> > +                * Keep doing it until Vsram and Vproc hit target voltages.
> > +                */
> > +               do {
> > +                       pre_vsram = regulator_get_voltage(sram_reg);
> > +                       if (pre_vsram < 0) {
> > +                               dev_err(info->cpu_dev,
> > +                                       "invalid Vsram value: %d\n", pre_vsram);
> > +                               return pre_vsram;
> > +                       }
> > +                       pre_vproc = regulator_get_voltage(proc_reg);
> > +                       if (pre_vproc < 0) {
> > +                               dev_err(info->cpu_dev,
> > +                                       "invalid Vproc value: %d\n", pre_vproc);
> > +                               return pre_vproc;
> > +                       }
> >
> > -       pre_vsram = regulator_get_voltage(sram_reg);
> > -       if (pre_vsram < 0) {
> > -               dev_err(info->cpu_dev, "invalid Vsram value: %d\n", pre_vsram);
> > -               return pre_vsram;
> > -       }
> > +                       vsram = min(new_vsram,
> > +                                   pre_vproc + soc_data->min_volt_shift);
> >
> > -       new_vsram = clamp(new_vproc + soc_data->min_volt_shift,
> > -                         soc_data->sram_min_volt, soc_data->sram_max_volt);
> > +                       if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
> > +                               vsram = soc_data->sram_max_volt;
> >
> > -       do {
> > -               if (pre_vproc <= new_vproc) {
> > -                       vsram = clamp(pre_vproc + soc_data->max_volt_shift,
> > -                                     soc_data->sram_min_volt, new_vsram);
> > -                       ret = regulator_set_voltage(sram_reg, vsram,
> > -                                                   soc_data->sram_max_volt);
> > +                               /*
> > +                                * If the target Vsram hits the maximum voltage,
> > +                                * try to set the exact voltage value first.
> > +                                */
> > +                               ret = regulator_set_voltage(sram_reg, vsram,
> > +                                                           vsram);
> > +                               if (ret)
> > +                                       ret = regulator_set_voltage(sram_reg,
> > +                                                       vsram - VOLT_TOL,
> > +                                                       vsram);
> >
> > -                       if (ret)
> > -                               return ret;
> > -
> > -                       if (vsram == soc_data->sram_max_volt ||
> > -                           new_vsram == soc_data->sram_min_volt)
> >                                 vproc = new_vproc;
> > -                       else
> > +                       } else {
> > +                               ret = regulator_set_voltage(sram_reg, vsram,
> > +                                                           vsram + VOLT_TOL);
> > +
> >                                 vproc = vsram - soc_data->min_volt_shift;
> > +                       }
> > +                       if (ret)
> > +                               return ret;
> >
> >                         ret = regulator_set_voltage(proc_reg, vproc,
> > -                                                   soc_data->proc_max_volt);
> > +                                                   vproc + VOLT_TOL);
> >                         if (ret) {
> >                                 regulator_set_voltage(sram_reg, pre_vsram,
> > -                                                     soc_data->sram_max_volt);
> > +                                                     pre_vsram);
> >                                 return ret;
> >                         }
> > -               } else if (pre_vproc > new_vproc) {
> > +               } while (vproc < new_vproc || vsram < new_vsram);
> > +       } else if (pre_vproc > new_vproc) {
> > +               /*
> > +                * When scaling down voltages, Vsram and Vproc scale down step
> > +                * by step. At each step, set Vproc to (Vsram - 200mV) first,
> > +                * then set Vproc to (Vproc + 100mV).
> > +                * Keep doing it until Vsram and Vproc hit target voltages.
> > +                */
> > +               do {
> > +                       pre_vproc = regulator_get_voltage(proc_reg);
> > +                       if (pre_vproc < 0) {
> > +                               dev_err(info->cpu_dev,
> > +                                       "invalid Vproc value: %d\n", pre_vproc);
> > +                               return pre_vproc;
> > +                       }
> > +                       pre_vsram = regulator_get_voltage(sram_reg);
> > +                       if (pre_vsram < 0) {
> > +                               dev_err(info->cpu_dev,
> > +                                       "invalid Vsram value: %d\n", pre_vsram);
> > +                               return pre_vsram;
> > +                       }
> > +
> >                         vproc = max(new_vproc,
> >                                     pre_vsram - soc_data->max_volt_shift);
> >                         ret = regulator_set_voltage(proc_reg, vproc,
> > -                                                   soc_data->proc_max_volt);
> > +                                                   vproc + VOLT_TOL);
> >                         if (ret)
> >                                 return ret;
> >
> > @@ -137,24 +183,32 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
> >                                 vsram = max(new_vsram,
> >                                             vproc + soc_data->min_volt_shift);
> >
> > -                       ret = regulator_set_voltage(sram_reg, vsram,
> > -                                                   soc_data->sram_max_volt);
> > +                       if (vsram + VOLT_TOL >= soc_data->sram_max_volt) {
> > +                               vsram = soc_data->sram_max_volt;
> > +
> > +                               /*
> > +                                * If the target Vsram hits the maximum voltage,
> > +                                * try to set the exact voltage value first.
> > +                                */
> > +                               ret = regulator_set_voltage(sram_reg, vsram,
> > +                                                           vsram);
> > +                               if (ret)
> > +                                       ret = regulator_set_voltage(sram_reg,
> > +                                                       vsram - VOLT_TOL,
> > +                                                       vsram);
> > +                       } else {
> > +                               ret = regulator_set_voltage(sram_reg, vsram,
> > +                                                           vsram + VOLT_TOL);
> > +                       }
> > +
> >                         if (ret) {
> >                                 regulator_set_voltage(proc_reg, pre_vproc,
> > -                                                     soc_data->proc_max_volt);
> > +                                                     pre_vproc);
> >                                 return ret;
> >                         }
> > -               }
> > -
> > -               pre_vproc = vproc;
> > -               pre_vsram = vsram;
> > -
> > -               if (--retry < 0) {
> > -                       dev_err(info->cpu_dev,
> > -                               "over loop count, failed to set voltage\n");
> > -                       return -EINVAL;
> > -               }
> > -       } while (vproc != new_vproc || vsram != new_vsram);
> > +               } while (vproc > new_vproc + VOLT_TOL ||
> > +                        vsram > new_vsram + VOLT_TOL);
> > +       }
> >
> >         return 0;
> >  }
> > @@ -250,8 +304,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
> >          * If the new voltage or the intermediate voltage is higher than the
> >          * current voltage, scale up voltage first.
> >          */
> > -       target_vproc = max(inter_vproc, vproc);
> > -       if (pre_vproc <= target_vproc) {
> > +       target_vproc = (inter_vproc > vproc) ? inter_vproc : vproc;
> > +       if (pre_vproc < target_vproc) {
> >                 ret = mtk_cpufreq_set_voltage(info, target_vproc);
> >                 if (ret) {
> >                         dev_err(cpu_dev,
> > @@ -513,15 +567,6 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
> >          */
> >         info->need_voltage_tracking = (info->sram_reg != NULL);
> >
> > -       /*
> > -        * We assume min voltage is 0 and tracking target voltage using
> > -        * min_volt_shift for each iteration.
> > -        * The vtrack_max is 3 times of expeted iteration count.
> > -        */
> > -       info->vtrack_max = 3 * DIV_ROUND_UP(max(info->soc_data->sram_max_volt,
> > -                                               info->soc_data->proc_max_volt),
> > -                                           info->soc_data->min_volt_shift);
> > -
> >         return 0;
> >
> >  out_disable_inter_clock:
> > --

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()"
  2022-12-02 19:46     ` Rafael J. Wysocki
@ 2022-12-05  4:30       ` Viresh Kumar
  2022-12-05 12:08         ` Rafael J. Wysocki
  0 siblings, 1 reply; 7+ messages in thread
From: Viresh Kumar @ 2022-12-05  4:30 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Matthias Brugger, linux-pm, Vincent Guittot, regressions, daniel,
	thomas.huehn, v5 . 19+,
	Nick, linux-kernel, linux-arm-kernel, linux-mediatek

On 02-12-22, 20:46, Rafael J. Wysocki wrote:
> On Fri, Dec 2, 2022 at 1:17 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Fri, Dec 2, 2022 at 6:26 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > This reverts commit 6a17b3876bc8303612d7ad59ecf7cbc0db418bcd.
> > >
> > > This commit caused regression on Banana Pi R64 (MT7622), revert until
> > > the problem is identified and fixed properly.
> > >
> > > Link: https://lore.kernel.org/all/930778a1-5e8b-6df6-3276-42dcdadaf682@systemli.org/
> > > Cc: v5.19+ <stable@vger.kernel.org> # v5.19+
> > > Reported-by: Nick <vincent@systemli.org>
> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> >
> > Do you want me to push this revert for -rc8?

No, I was planning to make it part of my pull request.

> After all, I've decided to queue it up for 6.2, thanks!

Can you please drop that ? AngeloGioacchino already reported that
Reverting the proposed commit will make MT8183 unstable.

So the right thing to do now is apply the fix, which is on the list
and getting tested.

-- 
viresh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()"
  2022-12-05  4:30       ` Viresh Kumar
@ 2022-12-05 12:08         ` Rafael J. Wysocki
  2022-12-05 23:30           ` Viresh Kumar
  0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2022-12-05 12:08 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael J. Wysocki, Matthias Brugger, linux-pm, Vincent Guittot,
	regressions, daniel, thomas.huehn, v5 . 19+,
	Nick, linux-kernel, linux-arm-kernel, linux-mediatek

On Mon, Dec 5, 2022 at 5:30 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 02-12-22, 20:46, Rafael J. Wysocki wrote:
> > On Fri, Dec 2, 2022 at 1:17 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Fri, Dec 2, 2022 at 6:26 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > >
> > > > This reverts commit 6a17b3876bc8303612d7ad59ecf7cbc0db418bcd.
> > > >
> > > > This commit caused regression on Banana Pi R64 (MT7622), revert until
> > > > the problem is identified and fixed properly.
> > > >
> > > > Link: https://lore.kernel.org/all/930778a1-5e8b-6df6-3276-42dcdadaf682@systemli.org/
> > > > Cc: v5.19+ <stable@vger.kernel.org> # v5.19+
> > > > Reported-by: Nick <vincent@systemli.org>
> > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > >
> > > Do you want me to push this revert for -rc8?
>
> No, I was planning to make it part of my pull request.

Well, this was a bit unclear.

> > After all, I've decided to queue it up for 6.2, thanks!
>
> Can you please drop that ? AngeloGioacchino already reported that
> Reverting the proposed commit will make MT8183 unstable.

OK, dropped now.

> So the right thing to do now is apply the fix, which is on the list
> and getting tested.

Alright then, I'll assume that the proper fix will come in through
your pull request for 6.2 (but please send that one before the merge
window opens), thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()"
  2022-12-05 12:08         ` Rafael J. Wysocki
@ 2022-12-05 23:30           ` Viresh Kumar
  0 siblings, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2022-12-05 23:30 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Matthias Brugger, linux-pm, Vincent Guittot, regressions, daniel,
	thomas.huehn, v5 . 19+,
	Nick, linux-kernel, linux-arm-kernel, linux-mediatek

On 05-12-22, 13:08, Rafael J. Wysocki wrote:
> Well, this was a bit unclear.

Hmm, yeah. I assumed that the arm stuff will go via my tree and you
will be in sync with this. But yeah, a comment in the patch won't have
hurt.

> Alright then, I'll assume that the proper fix will come in through
> your pull request for 6.2 (but please send that one before the merge
> window opens), thanks!

I was ready with pull request since several days, was just waiting for
this to get sorted out. And I think this may get delayed a bit too, so
I better send the first pull request now and worry about this later.

-- 
viresh

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-12-05 23:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <930778a1-5e8b-6df6-3276-42dcdadaf682@systemli.org>
2022-12-02  5:26 ` [PATCH] Revert "cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()" Viresh Kumar
2022-12-02 12:17   ` Rafael J. Wysocki
2022-12-02 19:46     ` Rafael J. Wysocki
2022-12-05  4:30       ` Viresh Kumar
2022-12-05 12:08         ` Rafael J. Wysocki
2022-12-05 23:30           ` Viresh Kumar
2022-12-02 12:47   ` Nick

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).