From 5b2e722764e84b9e1393be352e0e9eb08f640918 Mon Sep 17 00:00:00 2001 From: Vivek Aknurwar Date: Wed, 18 Mar 2020 22:11:45 -0700 Subject: [PATCH] clk: qcom: gdsc-regulator: Remove regulator voltage level voting On Lahaina, both VDD_GFX and VDD_MMCX support LowSVS as the minimum non-OFF level. Therefore regulator_set_voltage() calls are redundant and can be safely removed. Commit f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") introduced a call to mutex_lock(®ulator_list_mutex) inside of regulator_lock_dependent(). This additional locking allows deadlock to occur for regulator devices whose regulator_ops callback functions internally call consumer regulator framework API functions (e.g. regulator_enable(), regulator_disable(), or regulator_set_voltage()). For short term remove all regulator_set_voltage() calls in the gdsc-regulator driver as a short term partial workaround for regulator framework deadlock issue. Change-Id: I36dff589fe51ea4c932c310b7290a45b21f77992 Signed-off-by: Vivek Aknurwar --- drivers/clk/qcom/gdsc-regulator.c | 79 +++++-------------------------- 1 file changed, 12 insertions(+), 67 deletions(-) diff --git a/drivers/clk/qcom/gdsc-regulator.c b/drivers/clk/qcom/gdsc-regulator.c index 1812619642bf..f7125105e95a 100644 --- a/drivers/clk/qcom/gdsc-regulator.c +++ b/drivers/clk/qcom/gdsc-regulator.c @@ -20,8 +20,6 @@ #include #include -#include - /* GDSCR */ #define PWR_ON_MASK BIT(31) #define CLK_DIS_WAIT_MASK (0xF << 12) @@ -137,19 +135,8 @@ static int gdsc_is_enabled(struct regulator_dev *rdev) if (regulator_is_enabled(sc->parent_regulator) <= 0) return false; - /* - * Place an explicit vote on the parent rail to cover cases when - * it might be disabled between this point and reading the GDSC - * registers. - */ - if (regulator_set_voltage(sc->parent_regulator, - RPMH_REGULATOR_LEVEL_LOW_SVS, INT_MAX)) + if (regulator_enable(sc->parent_regulator)) return false; - - if (regulator_enable(sc->parent_regulator)) { - regulator_set_voltage(sc->parent_regulator, 0, INT_MAX); - return false; - } } regmap_read(sc->regmap, REG_OFFSET, ®val); @@ -161,19 +148,16 @@ static int gdsc_is_enabled(struct regulator_dev *rdev) * determine if HLOS has voted for it. */ if (!(regval & SW_COLLAPSE_MASK)) { - if (sc->parent_regulator) { + if (sc->parent_regulator) regulator_disable(sc->parent_regulator); - regulator_set_voltage(sc->parent_regulator, 0, - INT_MAX); - } + return true; } } - if (sc->parent_regulator) { + if (sc->parent_regulator) regulator_disable(sc->parent_regulator); - regulator_set_voltage(sc->parent_regulator, 0, INT_MAX); - } + return false; } @@ -183,13 +167,6 @@ static int gdsc_enable(struct regulator_dev *rdev) uint32_t regval, hw_ctrl_regval = 0x0; int i, ret = 0; - if (sc->parent_regulator) { - ret = regulator_set_voltage(sc->parent_regulator, - RPMH_REGULATOR_LEVEL_LOW_SVS, INT_MAX); - if (ret) - return ret; - } - if (sc->root_en || sc->force_root_en) clk_prepare_enable(sc->clocks[sc->root_clk_idx]); @@ -197,8 +174,7 @@ static int gdsc_enable(struct regulator_dev *rdev) if (regval & HW_CONTROL_MASK) { dev_warn(&rdev->dev, "Invalid enable while %s is under HW control\n", sc->rdesc.name); - ret = -EBUSY; - goto end; + return -EBUSY; } if (sc->toggle_logic) { @@ -281,7 +257,7 @@ static int gdsc_enable(struct regulator_dev *rdev) dev_err(&rdev->dev, "%s final state (after additional %d us timeout): 0x%x, GDS_HW_CTRL: 0x%x\n", sc->rdesc.name, sc->gds_timeout, regval, hw_ctrl_regval); - goto end; + return ret; } } else { dev_err(&rdev->dev, "%s enable timed out: 0x%x\n", @@ -293,7 +269,7 @@ static int gdsc_enable(struct regulator_dev *rdev) dev_err(&rdev->dev, "%s final state: 0x%x (%d us after timeout)\n", sc->rdesc.name, regval, sc->gds_timeout); - goto end; + return ret; } } @@ -322,9 +298,6 @@ static int gdsc_enable(struct regulator_dev *rdev) clk_disable_unprepare(sc->clocks[sc->root_clk_idx]); sc->is_gdsc_enabled = true; -end: - if (sc->parent_regulator) - regulator_set_voltage(sc->parent_regulator, 0, INT_MAX); return ret; } @@ -335,13 +308,6 @@ static int gdsc_disable(struct regulator_dev *rdev) uint32_t regval; int i, ret = 0; - if (sc->parent_regulator) { - ret = regulator_set_voltage(sc->parent_regulator, - RPMH_REGULATOR_LEVEL_LOW_SVS, INT_MAX); - if (ret) - return ret; - } - if (sc->force_root_en) clk_prepare_enable(sc->clocks[sc->root_clk_idx]); @@ -390,9 +356,6 @@ static int gdsc_disable(struct regulator_dev *rdev) if ((sc->is_gdsc_enabled && sc->root_en) || sc->force_root_en) clk_disable_unprepare(sc->clocks[sc->root_clk_idx]); - if (sc->parent_regulator) - regulator_set_voltage(sc->parent_regulator, 0, INT_MAX); - sc->is_gdsc_enabled = false; return ret; @@ -405,24 +368,15 @@ static unsigned int gdsc_get_mode(struct regulator_dev *rdev) int ret; if (sc->parent_regulator) { - ret = regulator_set_voltage(sc->parent_regulator, - RPMH_REGULATOR_LEVEL_LOW_SVS, INT_MAX); + ret = regulator_enable(sc->parent_regulator); if (ret) return ret; - - ret = regulator_enable(sc->parent_regulator); - if (ret) { - regulator_set_voltage(sc->parent_regulator, 0, INT_MAX); - return ret; - } } regmap_read(sc->regmap, REG_OFFSET, ®val); - if (sc->parent_regulator) { + if (sc->parent_regulator) regulator_disable(sc->parent_regulator); - regulator_set_voltage(sc->parent_regulator, 0, INT_MAX); - } if (regval & HW_CONTROL_MASK) return REGULATOR_MODE_FAST; @@ -437,16 +391,9 @@ static int gdsc_set_mode(struct regulator_dev *rdev, unsigned int mode) int ret = 0; if (sc->parent_regulator) { - ret = regulator_set_voltage(sc->parent_regulator, - RPMH_REGULATOR_LEVEL_LOW_SVS, INT_MAX); + ret = regulator_enable(sc->parent_regulator); if (ret) return ret; - - ret = regulator_enable(sc->parent_regulator); - if (ret) { - regulator_set_voltage(sc->parent_regulator, 0, INT_MAX); - return ret; - } } regmap_read(sc->regmap, REG_OFFSET, ®val); @@ -486,10 +433,8 @@ static int gdsc_set_mode(struct regulator_dev *rdev, unsigned int mode) break; } - if (sc->parent_regulator) { + if (sc->parent_regulator) regulator_disable(sc->parent_regulator); - regulator_set_voltage(sc->parent_regulator, 0, INT_MAX); - } return ret; }