Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d6b98629 authored by Jagadeesh Kona's avatar Jagadeesh Kona
Browse files

clk: qcom: gdsc-regulator: Remove regulator_lock() call from GDSC driver 



Remove regulator_lock() and regulator_is_enabled() calls in GDSC driver
as they are resulting in deadlock cases when lockdep is 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. But it does not need the
per-process reentrancy provided by regulator_lock() and regulator_unlock().
Therefore replace the calls with ww_mutex_lock() and ww_mutex_unlock().

Change-Id: Ic20f9a7478b47c9fde2e686b68d636342a4b1d48
Signed-off-by: default avatarJagadeesh Kona <jkona@codeaurora.org>
parent ca57039f

drivers/clk/qcom/gdsc-regulator.c

+27 −21
Original line number Diff line number Diff line
@@ -344,20 +344,27 @@ 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) {

		regulator_lock(rdev->supply->rdev);

		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);

			goto done;

		}

		parent_rdev = rdev->supply->rdev;



		/*

		 * 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))

			lock = true;



		if (!parent_enabled) {

		if (!parent_rdev->use_count) {

			dev_err(&rdev->dev, "%s cannot disable GDSC while parent is disabled\n",

				sc->rdesc.name);

			ret = -EIO;
@@ -435,8 +442,8 @@ static int gdsc_disable(struct regulator_dev *rdev) 
	sc->is_gdsc_enabled = false;



done:

	if (rdev->supply)

		regulator_unlock(rdev->supply->rdev);

	if (rdev->supply && lock)

		ww_mutex_unlock(&parent_rdev->mutex);



	return ret;

}
@@ -466,10 +473,12 @@ 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;



	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
@@ -479,15 +488,12 @@ static int gdsc_set_mode(struct regulator_dev *rdev, unsigned int mode) 
		 * state cannot change after checking due to a race with another

		 * consumer.

		 */

		regulator_lock(rdev->supply->rdev);

		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);

			goto done;

		} 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)) {

				sc->rdesc.name);

			ret = -EIO;

			goto done;

		}
@@ -554,7 +560,7 @@ static int gdsc_set_mode(struct regulator_dev *rdev, unsigned int mode) 


done:

	if (rdev->supply)

		regulator_unlock(rdev->supply->rdev);

		ww_mutex_unlock(&parent_rdev->mutex);



	return ret;

}