Re: [PATCH v3 4/4] regulator: core: Balance coupled regulators voltages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






On 12/12/2017 12:54 PM, Mark Brown wrote:
On Thu, Dec 07, 2017 at 10:46:15AM +0100, Maciej Purski wrote:

@@ -2447,10 +2482,9 @@ static int _regulator_is_enabled(struct regulator_dev *rdev)
  	return rdev->desc->ops->is_enabled(rdev);
  }
-static int _regulator_list_voltage(struct regulator *regulator,
-				    unsigned selector, int lock)
+static int _regulator_list_voltage(struct regulator_dev *rdev,
+				   unsigned selector, int lock)
  {

Please split this refactoring of _list_voltage() into a separate patch
for ease of review.  It can go in separately which will make this change
smaller and easier to review.

@@ -2928,6 +2961,35 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
  	if (ret < 0)
  		goto out2;
+ /*
+	 * If the regulator is not coupled just set voltage normally, else
+	 * return after changing consumer demands without changing voltage.
+	 * This will be handled outside the function
+	 * by regulator_balance_coupled()
+	 */
+	if (!rdev->coupling_desc) {
+		ret = regulator_set_voltage_rdev(regulator->rdev,
+						 min_uV, max_uV);
+		if (ret < 0)
+			goto out2;
+	}

As I think I said on the previous version I'm not enthusiastic about
having two separate code paths for setting the voltage, it makes it much
more likely that things will break especially given how rare coupled
regulators are.  It would be cleaner to make uncoupled regulators just
be a special case of coupled regulators, that way more of the code is
shared.  To that end I'd adjust the code so that we always have a
coupling descriptor and then handle the case where there's only one
regulator described in there.


Do you have any suggestion, how should I implement that path? The thing which
makes it more complicated is locking, because set_voltage_unlocked is done under one regulator's mutex and its suppliers, while balance procedure locks every coupled regulator without its suppliers. The suppliers for a single regulator are locked when setting a single regulator's voltage takes place.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux