mirror of
https://git.openwrt.org/openwrt/openwrt.git
synced 2026-05-04 17:36:12 +02:00
mediatek: don't let devfreq power-off the CPU
Fix a long standing bug in the mediatek-cci-devfreq driver which leads to the driver switching off the CPU power regulator in case of another resource not being ready in time -- a classic probe-order race condition. As a work-around it would of course just as well be possible to set the CPU regulator as 'regulator-always-on' (and not just 'regulator-boot-on'), but practically all MT7988 devices have copy&pasted the PMIC device tree hunk which sets only 'regulator-boot-on'). Hence, in order not having to fix all device trees, a proper fix in the driver is preferred. Fixes: #683 Signed-off-by: Daniel Golle <daniel@makrotopia.org>
This commit is contained in:
parent
ca8d931205
commit
a45ce4c788
@ -0,0 +1,241 @@
|
||||
From b61fcc3c7f95734e14741d8787fc9eb69b8437d4 Mon Sep 17 00:00:00 2001
|
||||
From: Daniel Golle <daniel@makrotopia.org>
|
||||
Date: Sat, 11 Apr 2026 22:47:15 +0100
|
||||
Subject: [PATCH 1/2] PM / devfreq: mtk-cci: use devres for resource management
|
||||
in probe
|
||||
|
||||
Convert mtk_ccifreq_probe() to use devm-managed resource cleanup
|
||||
instead of manual goto-based error handling.
|
||||
|
||||
This pattern (devm_add_action_or_reset with a regulator disable
|
||||
callback) is well-established in the kernel, used by drivers such as
|
||||
ads7846, max6639 and others.
|
||||
|
||||
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
|
||||
---
|
||||
drivers/devfreq/mtk-cci-devfreq.c | 150 +++++++++++++++---------------
|
||||
1 file changed, 73 insertions(+), 77 deletions(-)
|
||||
|
||||
--- a/drivers/devfreq/mtk-cci-devfreq.c
|
||||
+++ b/drivers/devfreq/mtk-cci-devfreq.c
|
||||
@@ -246,6 +246,21 @@ static struct devfreq_dev_profile mtk_cc
|
||||
.target = mtk_ccifreq_target,
|
||||
};
|
||||
|
||||
+static void mtk_ccifreq_regulator_disable(void *data)
|
||||
+{
|
||||
+ regulator_disable(data);
|
||||
+}
|
||||
+
|
||||
+static void mtk_ccifreq_clk_disable_unprepare(void *data)
|
||||
+{
|
||||
+ clk_disable_unprepare(data);
|
||||
+}
|
||||
+
|
||||
+static void mtk_ccifreq_opp_of_remove_table(void *data)
|
||||
+{
|
||||
+ dev_pm_opp_of_remove_table(data);
|
||||
+}
|
||||
+
|
||||
static int mtk_ccifreq_probe(struct platform_device *pdev)
|
||||
{
|
||||
struct device *dev = &pdev->dev;
|
||||
@@ -266,44 +281,47 @@ static int mtk_ccifreq_probe(struct plat
|
||||
platform_set_drvdata(pdev, drv);
|
||||
|
||||
drv->cci_clk = devm_clk_get(dev, "cci");
|
||||
- if (IS_ERR(drv->cci_clk)) {
|
||||
- ret = PTR_ERR(drv->cci_clk);
|
||||
- return dev_err_probe(dev, ret, "failed to get cci clk\n");
|
||||
- }
|
||||
+ if (IS_ERR(drv->cci_clk))
|
||||
+ return dev_err_probe(dev, PTR_ERR(drv->cci_clk),
|
||||
+ "failed to get cci clk\n");
|
||||
|
||||
drv->inter_clk = devm_clk_get(dev, "intermediate");
|
||||
- if (IS_ERR(drv->inter_clk)) {
|
||||
- ret = PTR_ERR(drv->inter_clk);
|
||||
- return dev_err_probe(dev, ret,
|
||||
+ if (IS_ERR(drv->inter_clk))
|
||||
+ return dev_err_probe(dev, PTR_ERR(drv->inter_clk),
|
||||
"failed to get intermediate clk\n");
|
||||
- }
|
||||
|
||||
drv->proc_reg = devm_regulator_get_optional(dev, "proc");
|
||||
- if (IS_ERR(drv->proc_reg)) {
|
||||
- ret = PTR_ERR(drv->proc_reg);
|
||||
- return dev_err_probe(dev, ret,
|
||||
+ if (IS_ERR(drv->proc_reg))
|
||||
+ return dev_err_probe(dev, PTR_ERR(drv->proc_reg),
|
||||
"failed to get proc regulator\n");
|
||||
- }
|
||||
|
||||
ret = regulator_enable(drv->proc_reg);
|
||||
- if (ret) {
|
||||
- dev_err(dev, "failed to enable proc regulator\n");
|
||||
+ if (ret)
|
||||
+ return dev_err_probe(dev, ret,
|
||||
+ "failed to enable proc regulator\n");
|
||||
+
|
||||
+ ret = devm_add_action_or_reset(dev, mtk_ccifreq_regulator_disable,
|
||||
+ drv->proc_reg);
|
||||
+ if (ret)
|
||||
return ret;
|
||||
- }
|
||||
|
||||
drv->sram_reg = devm_regulator_get_optional(dev, "sram");
|
||||
if (IS_ERR(drv->sram_reg)) {
|
||||
- ret = PTR_ERR(drv->sram_reg);
|
||||
- if (ret == -EPROBE_DEFER)
|
||||
- goto out_free_resources;
|
||||
+ if (PTR_ERR(drv->sram_reg) == -EPROBE_DEFER)
|
||||
+ return -EPROBE_DEFER;
|
||||
|
||||
drv->sram_reg = NULL;
|
||||
} else {
|
||||
ret = regulator_enable(drv->sram_reg);
|
||||
- if (ret) {
|
||||
- dev_err(dev, "failed to enable sram regulator\n");
|
||||
- goto out_free_resources;
|
||||
- }
|
||||
+ if (ret)
|
||||
+ return dev_err_probe(dev, ret,
|
||||
+ "failed to enable sram regulator\n");
|
||||
+
|
||||
+ ret = devm_add_action_or_reset(dev,
|
||||
+ mtk_ccifreq_regulator_disable,
|
||||
+ drv->sram_reg);
|
||||
+ if (ret)
|
||||
+ return ret;
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -317,31 +335,36 @@ static int mtk_ccifreq_probe(struct plat
|
||||
|
||||
ret = clk_prepare_enable(drv->cci_clk);
|
||||
if (ret)
|
||||
- goto out_free_resources;
|
||||
+ return ret;
|
||||
+
|
||||
+ ret = devm_add_action_or_reset(dev, mtk_ccifreq_clk_disable_unprepare,
|
||||
+ drv->cci_clk);
|
||||
+ if (ret)
|
||||
+ return ret;
|
||||
|
||||
ret = dev_pm_opp_of_add_table(dev);
|
||||
- if (ret) {
|
||||
- dev_err(dev, "failed to add opp table: %d\n", ret);
|
||||
- goto out_disable_cci_clk;
|
||||
- }
|
||||
+ if (ret)
|
||||
+ return dev_err_probe(dev, ret, "failed to add opp table\n");
|
||||
+
|
||||
+ ret = devm_add_action_or_reset(dev, mtk_ccifreq_opp_of_remove_table,
|
||||
+ dev);
|
||||
+ if (ret)
|
||||
+ return ret;
|
||||
|
||||
rate = clk_get_rate(drv->inter_clk);
|
||||
opp = dev_pm_opp_find_freq_ceil(dev, &rate);
|
||||
- if (IS_ERR(opp)) {
|
||||
- ret = PTR_ERR(opp);
|
||||
- dev_err(dev, "failed to get intermediate opp: %d\n", ret);
|
||||
- goto out_remove_opp_table;
|
||||
- }
|
||||
+ if (IS_ERR(opp))
|
||||
+ return dev_err_probe(dev, PTR_ERR(opp),
|
||||
+ "failed to get intermediate opp\n");
|
||||
+
|
||||
drv->inter_voltage = dev_pm_opp_get_voltage(opp);
|
||||
dev_pm_opp_put(opp);
|
||||
|
||||
rate = U32_MAX;
|
||||
opp = dev_pm_opp_find_freq_floor(drv->dev, &rate);
|
||||
- if (IS_ERR(opp)) {
|
||||
- dev_err(dev, "failed to get opp\n");
|
||||
- ret = PTR_ERR(opp);
|
||||
- goto out_remove_opp_table;
|
||||
- }
|
||||
+ if (IS_ERR(opp))
|
||||
+ return dev_err_probe(dev, PTR_ERR(opp),
|
||||
+ "failed to get opp\n");
|
||||
|
||||
opp_volt = dev_pm_opp_get_voltage(opp);
|
||||
dev_pm_opp_put(opp);
|
||||
@@ -349,63 +372,36 @@ static int mtk_ccifreq_probe(struct plat
|
||||
if (ret) {
|
||||
dev_err(dev, "failed to scale to highest voltage %lu in proc_reg\n",
|
||||
opp_volt);
|
||||
- goto out_remove_opp_table;
|
||||
+ return ret;
|
||||
}
|
||||
|
||||
passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL);
|
||||
- if (!passive_data) {
|
||||
- ret = -ENOMEM;
|
||||
- goto out_remove_opp_table;
|
||||
- }
|
||||
+ if (!passive_data)
|
||||
+ return -ENOMEM;
|
||||
|
||||
passive_data->parent_type = CPUFREQ_PARENT_DEV;
|
||||
drv->devfreq = devm_devfreq_add_device(dev, &mtk_ccifreq_profile,
|
||||
DEVFREQ_GOV_PASSIVE,
|
||||
passive_data);
|
||||
- if (IS_ERR(drv->devfreq)) {
|
||||
- ret = -EPROBE_DEFER;
|
||||
- dev_err(dev, "failed to add devfreq device: %ld\n",
|
||||
- PTR_ERR(drv->devfreq));
|
||||
- goto out_remove_opp_table;
|
||||
- }
|
||||
+ if (IS_ERR(drv->devfreq))
|
||||
+ return dev_err_probe(dev, -EPROBE_DEFER,
|
||||
+ "failed to add devfreq device: %ld\n",
|
||||
+ PTR_ERR(drv->devfreq));
|
||||
|
||||
drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier;
|
||||
ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb);
|
||||
- if (ret) {
|
||||
- dev_err(dev, "failed to register opp notifier: %d\n", ret);
|
||||
- goto out_remove_opp_table;
|
||||
- }
|
||||
- return 0;
|
||||
-
|
||||
-out_remove_opp_table:
|
||||
- dev_pm_opp_of_remove_table(dev);
|
||||
-
|
||||
-out_disable_cci_clk:
|
||||
- clk_disable_unprepare(drv->cci_clk);
|
||||
-
|
||||
-out_free_resources:
|
||||
- if (regulator_is_enabled(drv->proc_reg))
|
||||
- regulator_disable(drv->proc_reg);
|
||||
- if (!IS_ERR_OR_NULL(drv->sram_reg) &&
|
||||
- regulator_is_enabled(drv->sram_reg))
|
||||
- regulator_disable(drv->sram_reg);
|
||||
+ if (ret)
|
||||
+ return dev_err_probe(dev, ret,
|
||||
+ "failed to register opp notifier\n");
|
||||
|
||||
- return ret;
|
||||
+ return 0;
|
||||
}
|
||||
|
||||
static void mtk_ccifreq_remove(struct platform_device *pdev)
|
||||
{
|
||||
- struct device *dev = &pdev->dev;
|
||||
- struct mtk_ccifreq_drv *drv;
|
||||
-
|
||||
- drv = platform_get_drvdata(pdev);
|
||||
+ struct mtk_ccifreq_drv *drv = platform_get_drvdata(pdev);
|
||||
|
||||
- dev_pm_opp_unregister_notifier(dev, &drv->opp_nb);
|
||||
- dev_pm_opp_of_remove_table(dev);
|
||||
- clk_disable_unprepare(drv->cci_clk);
|
||||
- regulator_disable(drv->proc_reg);
|
||||
- if (drv->sram_reg)
|
||||
- regulator_disable(drv->sram_reg);
|
||||
+ dev_pm_opp_unregister_notifier(&pdev->dev, &drv->opp_nb);
|
||||
}
|
||||
|
||||
static const struct mtk_ccifreq_platform_data mt8183_platform_data = {
|
||||
@ -0,0 +1,53 @@
|
||||
From b19968e432fe2ebe3af4bc9190923edb70b6aeee Mon Sep 17 00:00:00 2001
|
||||
From: Daniel Golle <daniel@makrotopia.org>
|
||||
Date: Mon, 13 Apr 2026 15:22:05 +0100
|
||||
Subject: [PATCH 2/2] PM / devfreq: mtk-cci: check cpufreq availability early
|
||||
|
||||
Check spufreq availablility early at probe so -EPROBE_DEFER is emitted
|
||||
before touching any regulators.
|
||||
|
||||
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
|
||||
---
|
||||
drivers/devfreq/mtk-cci-devfreq.c | 18 +++++++++++++++++-
|
||||
1 file changed, 17 insertions(+), 1 deletion(-)
|
||||
|
||||
--- a/drivers/devfreq/mtk-cci-devfreq.c
|
||||
+++ b/drivers/devfreq/mtk-cci-devfreq.c
|
||||
@@ -4,6 +4,7 @@
|
||||
*/
|
||||
|
||||
#include <linux/clk.h>
|
||||
+#include <linux/cpufreq.h>
|
||||
#include <linux/devfreq.h>
|
||||
#include <linux/minmax.h>
|
||||
#include <linux/module.h>
|
||||
@@ -263,13 +264,28 @@ static void mtk_ccifreq_opp_of_remove_ta
|
||||
|
||||
static int mtk_ccifreq_probe(struct platform_device *pdev)
|
||||
{
|
||||
+ struct devfreq_passive_data *passive_data;
|
||||
struct device *dev = &pdev->dev;
|
||||
+ struct cpufreq_policy *policy;
|
||||
struct mtk_ccifreq_drv *drv;
|
||||
- struct devfreq_passive_data *passive_data;
|
||||
struct dev_pm_opp *opp;
|
||||
unsigned long rate, opp_volt;
|
||||
int ret;
|
||||
|
||||
+ /*
|
||||
+ * Check if cpufreq is available before touching any regulators.
|
||||
+ * The passive devfreq governor needs cpufreq as its parent and
|
||||
+ * will return -EPROBE_DEFER if it is not yet registered. If we
|
||||
+ * enable the proc regulator (CPU core power) before this check,
|
||||
+ * the subsequent probe failure causes devres to disable that
|
||||
+ * regulator, potentially cutting CPU core voltage and hanging
|
||||
+ * the system.
|
||||
+ */
|
||||
+ policy = cpufreq_cpu_get(0);
|
||||
+ if (!policy)
|
||||
+ return -EPROBE_DEFER;
|
||||
+ cpufreq_cpu_put(policy);
|
||||
+
|
||||
drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
|
||||
if (!drv)
|
||||
return -ENOMEM;
|
||||
Loading…
x
Reference in New Issue
Block a user