android/android_kernel_samsung_sm8650
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

clk: qcom: gdsc-regulator: Remove regulator_is_enabled() calls

Browse Source 
Remove regulator_is_enabled() calls in GDSC driver as they are resulting
in deadlock cases when lock debug options are enabled. But it is critical
to check if parent is enabled or not before performing any GDSC operation,
as GDSC register access requires parent to be enabled.

Use the parent's use_count to check if parent is enabled or not. For
gdsc_disable(), in most cases, parent's mutex is locked by regulator
framework before calling the gdsc_disable() callback, hence it is safe
to read use_count. But in code paths where parent lock is not acquired
by regulator framework, acquire parent's mutex before reading use_count
to avoid parent getting disabled while in the middle of GDSC operations.

For gdsc_set_mode(), parent locking is needed to avoid parent getting
disabled in the middle of set mode operation. Therefore acquire the
parent lock before checking parent is enabled or not and release the
lock at the end of set mode call using ww_mutex_lock and ww_mutex_unlock
API's.

Change-Id: Ic20f9a7478b47c9fde2e686b68d636342a4b1d48
Signed-off-by: Jagadeesh Kona <jkona@codeaurora.org>
Signed-off-by: Vivek Aknurwar <quic_viveka@quicinc.com>
Signed-off-by: Mike Tipton <quic_mdtipton@quicinc.com>
This commit is contained in:
Jagadeesh Kona 2021-02-26 21:14:14 +05:30 committed by Mike Tipton
parent 0609fc4d66
commit ab64664a9f
1 changed files with 45 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
drivers/clk/qcom/gdsc-regulator.c
View File

@ -440,22 +440,31 @@ static int gdsc_enable(struct regulator_dev *rdev)
static int gdsc_disable(struct regulator_dev *rdev)
{
struct gdsc *sc = rdev_get_drvdata(rdev);
struct regulator_dev *parent_rdev;
uint32_t regval;
int i, ret = 0, parent_enabled;
int i, ret = 0;
bool lock = false;
if (rdev->supply) {
parent_enabled = regulator_is_enabled(rdev->supply);
if (parent_enabled < 0) {
ret = parent_enabled;
dev_err(&rdev->dev, "%s unable to check parent enable state, ret=%d\n",
sc->rdesc.name, ret);
return ret;
}
parent_rdev = rdev->supply->rdev;
if (!parent_enabled) {
/*
* At this point, it can be assumed that parent supply's mutex is always
* locked by regulator framework before calling this callback but there
* are code paths where it isn't locked (e.g regulator_late_cleanup()).
*
* If parent supply is not locked, lock the parent supply mutex before
* checking it's enable count, so it won't get disabled while in the
* middle of GDSC operations
*/
if (ww_mutex_trylock(&parent_rdev->mutex, NULL))
lock = true;
if (!parent_rdev->use_count) {
dev_err(&rdev->dev, "%s cannot disable GDSC while parent is disabled\n",
sc->rdesc.name);
return -EIO;
ret = -EIO;
goto done;
}
}
@ -499,7 +508,7 @@ static int gdsc_disable(struct regulator_dev *rdev)
} else {
ret = check_gdsc_status(sc, &rdev->dev, DISABLED);
if (ret)
return ret;
goto done;
}
if (sc->domain_addr) {
@ -512,7 +521,7 @@ static int gdsc_disable(struct regulator_dev *rdev)
ret = icc_set_bw(sc->paths[i], 0, 0);
if (ret) {
dev_err(&rdev->dev, "Failed to unvote BW for %d: %d\n", i, ret);
return ret;
goto done;
}
}
} else {
@ -532,6 +541,10 @@ static int gdsc_disable(struct regulator_dev *rdev)
sc->is_gdsc_enabled = false;
done:
if (rdev->supply && lock)
ww_mutex_unlock(&parent_rdev->mutex);
return ret;
}
@ -566,6 +579,7 @@ static unsigned int gdsc_get_mode(struct regulator_dev *rdev)
static int gdsc_set_mode(struct regulator_dev *rdev, unsigned int mode)
{
struct gdsc *sc = rdev_get_drvdata(rdev);
struct regulator_dev *parent_rdev;
uint32_t regval;
int ret = 0;
@ -586,27 +600,27 @@ static int gdsc_set_mode(struct regulator_dev *rdev, unsigned int mode)
}
if (rdev->supply) {
parent_rdev = rdev->supply->rdev;
/*
* Ensure that the GDSC parent supply is enabled before
* continuing. This is needed to avoid an unclocked access
* of the GDSC control register for GDSCs whose register access
* is gated by the parent supply enable state in hardware.
*/
ret = regulator_is_enabled(rdev->supply);
if (ret < 0) {
dev_err(&rdev->dev, "%s unable to check parent enable state, ret=%d\n",
sc->rdesc.name, ret);
return ret;
} else if (WARN(!ret,
ww_mutex_lock(&parent_rdev->mutex, NULL);
if (!parent_rdev->use_count) {
dev_err(&rdev->dev,
"%s cannot change GDSC HW/SW control mode while parent is disabled\n",
sc->rdesc.name)) {
return -EIO;
sc->rdesc.name);
ret = -EIO;
goto done;
}
}
ret = regmap_read(sc->regmap, REG_OFFSET, &regval);
if (ret < 0)
return ret;
goto done;
switch (mode) {
case REGULATOR_MODE_FAST:
@ -614,7 +628,7 @@ static int gdsc_set_mode(struct regulator_dev *rdev, unsigned int mode)
regval |= HW_CONTROL_MASK;
ret = regmap_write(sc->regmap, REG_OFFSET, regval);
if (ret < 0)
return ret;
goto done;
/*
* There may be a race with internal HW trigger signal,
* that will result in GDSC going through a power down and
@ -633,7 +647,7 @@ static int gdsc_set_mode(struct regulator_dev *rdev, unsigned int mode)
regval &= ~HW_CONTROL_MASK;
ret = regmap_write(sc->regmap, REG_OFFSET, regval);
if (ret < 0)
return ret;
goto done;
/*
* There may be a race with internal HW trigger signal,
* that will result in GDSC going through a power down and
@ -651,15 +665,20 @@ static int gdsc_set_mode(struct regulator_dev *rdev, unsigned int mode)
if (sc->is_gdsc_enabled) {
ret = check_gdsc_status(sc, &rdev->dev, ENABLED);
if (ret)
return ret;
goto done;
}
sc->is_gdsc_hw_ctrl_mode = false;
break;
default:
return -EINVAL;
ret = -EINVAL;
break;
}
return 0;
done:
if (rdev->supply)
ww_mutex_unlock(&parent_rdev->mutex);
return ret;
}
static const struct regulator_ops gdsc_ops = {