regulator: Refactor supply implementation to work as regular consumers

Currently the regulator supply implementation is somewhat complex and fragile as it doesn't look like standard consumers but is instead a parallel implementation. This causes issues with locking and reference counting. Move the implementation over to using standard consumers to address this. Rather than only notifying the supply on the first enable/disable we do so every time the regulator is enabled or disabled, simplifying locking as we don't need to hold a lock on the consumer we are about to enable. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>

regulator: Refactor supply implementation to work as regular consumers
Currently the regulator supply implementation is somewhat complex and fragile as it doesn't look like standard consumers but is instead a parallel implementation. This causes issues with locking and reference counting. Move the implementation over to using standard consumers to address this. Rather than only notifying the supply on the first enable/disable we do so every time the regulator is enabled or disabled, simplifying locking as we don't need to hold a lock on the consumer we are about to enable. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
Mark Brown · Liam Girdwood
1 parent e0eaedefda
Showing 2 changed files with 37 additions and 68 deletions Side-by-side Diff
drivers/regulator/core.c
include/linux/regulator/driver.h
@@ -82,8 +82,7 @@
 };
  
 static int _regulator_is_enabled(struct regulator_dev *rdev);
-static int _regulator_disable(struct regulator_dev *rdev,
-		struct regulator_dev **supply_rdev_ptr);
+static int _regulator_disable(struct regulator_dev *rdev);
 static int _regulator_get_voltage(struct regulator_dev *rdev);
 static int _regulator_get_current_limit(struct regulator_dev *rdev);
 static unsigned int _regulator_get_mode(struct regulator_dev *rdev);
@@ -91,6 +90,9 @@
 				  unsigned long event, void *data);
 static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 				     int min_uV, int max_uV);
+static struct regulator *create_regulator(struct regulator_dev *rdev,
+					  struct device *dev,
+					  const char *supply_name);
  
 static const char *rdev_get_name(struct regulator_dev *rdev)
 {
  
  
@@ -930,21 +932,20 @@
  * core if it's child is enabled.
  */
 static int set_supply(struct regulator_dev *rdev,
-	struct regulator_dev *supply_rdev)
+		      struct regulator_dev *supply_rdev)
 {
 	int err;
  
-	err = sysfs_create_link(&rdev->dev.kobj, &supply_rdev->dev.kobj,
-				"supply");
-	if (err) {
-		rdev_err(rdev, "could not add device link %s err %d\n",
-			 supply_rdev->dev.kobj.name, err);
-		       goto out;
+	rdev_info(rdev, "supplied by %s\n", rdev_get_name(supply_rdev));
+
+	rdev->supply = create_regulator(supply_rdev, &rdev->dev, "SUPPLY");
+	if (IS_ERR(rdev->supply)) {
+		err = PTR_ERR(rdev->supply);
+		rdev->supply = NULL;
+		return err;
 	}
-	rdev->supply = supply_rdev;
-	list_add(&rdev->slist, &supply_rdev->supply_list);
-out:
-	return err;
+
+	return 0;
 }
  
 /**
@@ -1303,19 +1304,6 @@
 {
 	int ret, delay;
  
-	if (rdev->use_count == 0) {
-		/* do we need to enable the supply regulator first */
-		if (rdev->supply) {
-			mutex_lock(&rdev->supply->mutex);
-			ret = _regulator_enable(rdev->supply);
-			mutex_unlock(&rdev->supply->mutex);
-			if (ret < 0) {
-				rdev_err(rdev, "failed to enable: %d\n", ret);
-				return ret;
-			}
-		}
-	}
-
 	/* check voltage and requested load before enabling */
 	if (rdev->constraints &&
 	    (rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_DRMS))
  
  
  
@@ -1390,19 +1378,27 @@
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret = 0;
  
+	if (rdev->supply) {
+		ret = regulator_enable(rdev->supply);
+		if (ret != 0)
+			return ret;
+	}
+
 	mutex_lock(&rdev->mutex);
 	ret = _regulator_enable(rdev);
 	mutex_unlock(&rdev->mutex);
+
+	if (ret != 0)
+		regulator_disable(rdev->supply);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_enable);
  
 /* locks held by regulator_disable() */
-static int _regulator_disable(struct regulator_dev *rdev,
-		struct regulator_dev **supply_rdev_ptr)
+static int _regulator_disable(struct regulator_dev *rdev)
 {
 	int ret = 0;
-	*supply_rdev_ptr = NULL;
  
 	if (WARN(rdev->use_count <= 0,
 		 "unbalanced disables for %s\n", rdev_get_name(rdev)))
@@ -1429,9 +1425,6 @@
 					     NULL);
 		}
  
-		/* decrease our supplies ref count and disable if required */
-		*supply_rdev_ptr = rdev->supply;
-
 		rdev->use_count = 0;
 	} else if (rdev->use_count > 1) {
  
@@ -1442,6 +1435,7 @@
  
 		rdev->use_count--;
 	}
+
 	return ret;
 }
  
  
  
  
  
@@ -1460,29 +1454,21 @@
 int regulator_disable(struct regulator *regulator)
 {
 	struct regulator_dev *rdev = regulator->rdev;
-	struct regulator_dev *supply_rdev = NULL;
 	int ret = 0;
  
 	mutex_lock(&rdev->mutex);
-	ret = _regulator_disable(rdev, &supply_rdev);
+	ret = _regulator_disable(rdev);
 	mutex_unlock(&rdev->mutex);
  
-	/* decrease our supplies ref count and disable if required */
-	while (supply_rdev != NULL) {
-		rdev = supply_rdev;
+	if (ret == 0 && rdev->supply)
+		regulator_disable(rdev->supply);
  
-		mutex_lock(&rdev->mutex);
-		_regulator_disable(rdev, &supply_rdev);
-		mutex_unlock(&rdev->mutex);
-	}
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_disable);
  
 /* locks held by regulator_force_disable() */
-static int _regulator_force_disable(struct regulator_dev *rdev,
-		struct regulator_dev **supply_rdev_ptr)
+static int _regulator_force_disable(struct regulator_dev *rdev)
 {
 	int ret = 0;
  
@@ -1499,10 +1485,6 @@
 			REGULATOR_EVENT_DISABLE, NULL);
 	}
  
-	/* decrease our supplies ref count and disable if required */
-	*supply_rdev_ptr = rdev->supply;
-
-	rdev->use_count = 0;
 	return ret;
 }
  
  
  
@@ -1518,16 +1500,16 @@
 int regulator_force_disable(struct regulator *regulator)
 {
 	struct regulator_dev *rdev = regulator->rdev;
-	struct regulator_dev *supply_rdev = NULL;
 	int ret;
  
 	mutex_lock(&rdev->mutex);
 	regulator->uA_load = 0;
-	ret = _regulator_force_disable(rdev, &supply_rdev);
+	ret = _regulator_force_disable(regulator->rdev);
 	mutex_unlock(&rdev->mutex);
  
-	if (supply_rdev)
-		regulator_disable(get_device_regulator(rdev_get_dev(supply_rdev)));
+	if (rdev->supply)
+		while (rdev->open_count--)
+			regulator_disable(rdev->supply);
  
 	return ret;
 }
@@ -2138,7 +2120,7 @@
 	/* get input voltage */
 	input_uV = 0;
 	if (rdev->supply)
-		input_uV = _regulator_get_voltage(rdev->supply);
+		input_uV = regulator_get_voltage(rdev->supply);
 	if (input_uV <= 0)
 		input_uV = rdev->constraints->input_uV;
 	if (input_uV <= 0) {
  
@@ -2208,17 +2190,8 @@
 static void _notifier_call_chain(struct regulator_dev *rdev,
 				  unsigned long event, void *data)
 {
-	struct regulator_dev *_rdev;
-
 	/* call rdev chain first */
 	blocking_notifier_call_chain(&rdev->notifier, event, NULL);
-
-	/* now notify regulator we supply */
-	list_for_each_entry(_rdev, &rdev->supply_list, slist) {
-		mutex_lock(&_rdev->mutex);
-		_notifier_call_chain(_rdev, event, data);
-		mutex_unlock(&_rdev->mutex);
-	}
 }
  
 /**
  
@@ -2610,9 +2583,7 @@
 	rdev->owner = regulator_desc->owner;
 	rdev->desc = regulator_desc;
 	INIT_LIST_HEAD(&rdev->consumer_list);
-	INIT_LIST_HEAD(&rdev->supply_list);
 	INIT_LIST_HEAD(&rdev->list);
-	INIT_LIST_HEAD(&rdev->slist);
 	BLOCKING_INIT_NOTIFIER_HEAD(&rdev->notifier);
  
 	/* preform any regulator specific init */
@@ -2724,7 +2695,7 @@
 	unset_regulator_supplies(rdev);
 	list_del(&rdev->list);
 	if (rdev->supply)
-		sysfs_remove_link(&rdev->dev.kobj, "supply");
+		regulator_put(rdev->supply);
 	device_unregister(&rdev->dev);
 	kfree(rdev->constraints);
 	mutex_unlock(&regulator_list_mutex);
@@ -188,18 +188,16 @@
  
 	/* lists we belong to */
 	struct list_head list; /* list of all regulators */
-	struct list_head slist; /* list of supplied regulators */
  
 	/* lists we own */
 	struct list_head consumer_list; /* consumers we supply */
-	struct list_head supply_list; /* regulators we supply */
  
 	struct blocking_notifier_head notifier;
 	struct mutex mutex; /* consumer lock */
 	struct module *owner;
 	struct device dev;
 	struct regulation_constraints *constraints;
-	struct regulator_dev *supply;	/* for tree */
+	struct regulator *supply;	/* for tree */
  
 	void *reg_data;		/* regulator_dev data */
...	...	@@ -82,8 +82,7 @@
82	82	};
83	83
84	84	static int _regulator_is_enabled(struct regulator_dev *rdev);
85		-static int _regulator_disable(struct regulator_dev *rdev,
86		- struct regulator_dev **supply_rdev_ptr);
	85	+static int _regulator_disable(struct regulator_dev *rdev);
87	86	static int _regulator_get_voltage(struct regulator_dev *rdev);
88	87	static int _regulator_get_current_limit(struct regulator_dev *rdev);
89	88	static unsigned int _regulator_get_mode(struct regulator_dev *rdev);
...	...	@@ -91,6 +90,9 @@
91	90	unsigned long event, void *data);
92	91	static int _regulator_do_set_voltage(struct regulator_dev *rdev,
93	92	int min_uV, int max_uV);
	93	+static struct regulator create_regulator(struct regulator_dev rdev,
	94	+ struct device *dev,
	95	+ const char *supply_name);
94	96
95	97	static const char rdev_get_name(struct regulator_dev rdev)
96	98	{
97	99
98	100
...	...	@@ -930,21 +932,20 @@
930	932	* core if it's child is enabled.
931	933	*/
932	934	static int set_supply(struct regulator_dev *rdev,
933		- struct regulator_dev *supply_rdev)
	935	+ struct regulator_dev *supply_rdev)
934	936	{
935	937	int err;
936	938
937		- err = sysfs_create_link(&rdev->dev.kobj, &supply_rdev->dev.kobj,
938		- "supply");
939		- if (err) {
940		- rdev_err(rdev, "could not add device link %s err %d\n",
941		- supply_rdev->dev.kobj.name, err);
942		- goto out;
	939	+ rdev_info(rdev, "supplied by %s\n", rdev_get_name(supply_rdev));
	940	+
	941	+ rdev->supply = create_regulator(supply_rdev, &rdev->dev, "SUPPLY");
	942	+ if (IS_ERR(rdev->supply)) {
	943	+ err = PTR_ERR(rdev->supply);
	944	+ rdev->supply = NULL;
	945	+ return err;
943	946	}
944		- rdev->supply = supply_rdev;
945		- list_add(&rdev->slist, &supply_rdev->supply_list);
946		-out:
947		- return err;
	947	+
	948	+ return 0;
948	949	}
949	950
950	951	/**
...	...	@@ -1303,19 +1304,6 @@
1303	1304	{
1304	1305	int ret, delay;
1305	1306
1306		- if (rdev->use_count == 0) {
1307		- /* do we need to enable the supply regulator first */
1308		- if (rdev->supply) {
1309		- mutex_lock(&rdev->supply->mutex);
1310		- ret = _regulator_enable(rdev->supply);
1311		- mutex_unlock(&rdev->supply->mutex);
1312		- if (ret < 0) {
1313		- rdev_err(rdev, "failed to enable: %d\n", ret);
1314		- return ret;
1315		- }
1316		- }
1317		- }
1318		-
1319	1307	/* check voltage and requested load before enabling */
1320	1308	if (rdev->constraints &&
1321	1309	(rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_DRMS))
1322	1310
1323	1311
1324	1312
...	...	@@ -1390,19 +1378,27 @@
1390	1378	struct regulator_dev *rdev = regulator->rdev;
1391	1379	int ret = 0;
1392	1380
	1381	+ if (rdev->supply) {
	1382	+ ret = regulator_enable(rdev->supply);
	1383	+ if (ret != 0)
	1384	+ return ret;
	1385	+ }
	1386	+
1393	1387	mutex_lock(&rdev->mutex);
1394	1388	ret = _regulator_enable(rdev);
1395	1389	mutex_unlock(&rdev->mutex);
	1390	+
	1391	+ if (ret != 0)
	1392	+ regulator_disable(rdev->supply);
	1393	+
1396	1394	return ret;
1397	1395	}
1398	1396	EXPORT_SYMBOL_GPL(regulator_enable);
1399	1397
1400	1398	/* locks held by regulator_disable() */
1401		-static int _regulator_disable(struct regulator_dev *rdev,
1402		- struct regulator_dev **supply_rdev_ptr)
	1399	+static int _regulator_disable(struct regulator_dev *rdev)
1403	1400	{
1404	1401	int ret = 0;
1405		- *supply_rdev_ptr = NULL;
1406	1402
1407	1403	if (WARN(rdev->use_count <= 0,
1408	1404	"unbalanced disables for %s\n", rdev_get_name(rdev)))
...	...	@@ -1429,9 +1425,6 @@
1429	1425	NULL);
1430	1426	}
1431	1427
1432		- /* decrease our supplies ref count and disable if required */
1433		- *supply_rdev_ptr = rdev->supply;
1434		-
1435	1428	rdev->use_count = 0;
1436	1429	} else if (rdev->use_count > 1) {
1437	1430
...	...	@@ -1442,6 +1435,7 @@
1442	1435
1443	1436	rdev->use_count--;
1444	1437	}
	1438	+
1445	1439	return ret;
1446	1440	}
1447	1441
1448	1442
1449	1443
1450	1444
1451	1445
...	...	@@ -1460,29 +1454,21 @@
1460	1454	int regulator_disable(struct regulator *regulator)
1461	1455	{
1462	1456	struct regulator_dev *rdev = regulator->rdev;
1463		- struct regulator_dev *supply_rdev = NULL;
1464	1457	int ret = 0;
1465	1458
1466	1459	mutex_lock(&rdev->mutex);
1467		- ret = _regulator_disable(rdev, &supply_rdev);
	1460	+ ret = _regulator_disable(rdev);
1468	1461	mutex_unlock(&rdev->mutex);
1469	1462
1470		- /* decrease our supplies ref count and disable if required */
1471		- while (supply_rdev != NULL) {
1472		- rdev = supply_rdev;
	1463	+ if (ret == 0 && rdev->supply)
	1464	+ regulator_disable(rdev->supply);
1473	1465
1474		- mutex_lock(&rdev->mutex);
1475		- _regulator_disable(rdev, &supply_rdev);
1476		- mutex_unlock(&rdev->mutex);
1477		- }
1478		-
1479	1466	return ret;
1480	1467	}
1481	1468	EXPORT_SYMBOL_GPL(regulator_disable);
1482	1469
1483	1470	/* locks held by regulator_force_disable() */
1484		-static int _regulator_force_disable(struct regulator_dev *rdev,
1485		- struct regulator_dev **supply_rdev_ptr)
	1471	+static int _regulator_force_disable(struct regulator_dev *rdev)
1486	1472	{
1487	1473	int ret = 0;
1488	1474
...	...	@@ -1499,10 +1485,6 @@
1499	1485	REGULATOR_EVENT_DISABLE, NULL);
1500	1486	}
1501	1487
1502		- /* decrease our supplies ref count and disable if required */
1503		- *supply_rdev_ptr = rdev->supply;
1504		-
1505		- rdev->use_count = 0;
1506	1488	return ret;
1507	1489	}
1508	1490
1509	1491
1510	1492
...	...	@@ -1518,16 +1500,16 @@
1518	1500	int regulator_force_disable(struct regulator *regulator)
1519	1501	{
1520	1502	struct regulator_dev *rdev = regulator->rdev;
1521		- struct regulator_dev *supply_rdev = NULL;
1522	1503	int ret;
1523	1504
1524	1505	mutex_lock(&rdev->mutex);
1525	1506	regulator->uA_load = 0;
1526		- ret = _regulator_force_disable(rdev, &supply_rdev);
	1507	+ ret = _regulator_force_disable(regulator->rdev);
1527	1508	mutex_unlock(&rdev->mutex);
1528	1509
1529		- if (supply_rdev)
1530		- regulator_disable(get_device_regulator(rdev_get_dev(supply_rdev)));
	1510	+ if (rdev->supply)
	1511	+ while (rdev->open_count--)
	1512	+ regulator_disable(rdev->supply);
1531	1513
1532	1514	return ret;
1533	1515	}
...	...	@@ -2138,7 +2120,7 @@
2138	2120	/* get input voltage */
2139	2121	input_uV = 0;
2140	2122	if (rdev->supply)
2141		- input_uV = _regulator_get_voltage(rdev->supply);
	2123	+ input_uV = regulator_get_voltage(rdev->supply);
2142	2124	if (input_uV <= 0)
2143	2125	input_uV = rdev->constraints->input_uV;
2144	2126	if (input_uV <= 0) {
2145	2127
...	...	@@ -2208,17 +2190,8 @@
2208	2190	static void _notifier_call_chain(struct regulator_dev *rdev,
2209	2191	unsigned long event, void *data)
2210	2192	{
2211		- struct regulator_dev *_rdev;
2212		-
2213	2193	/* call rdev chain first */
2214	2194	blocking_notifier_call_chain(&rdev->notifier, event, NULL);
2215		-
2216		- /* now notify regulator we supply */
2217		- list_for_each_entry(_rdev, &rdev->supply_list, slist) {
2218		- mutex_lock(&_rdev->mutex);
2219		- _notifier_call_chain(_rdev, event, data);
2220		- mutex_unlock(&_rdev->mutex);
2221		- }
2222	2195	}
2223	2196
2224	2197	/**
2225	2198
...	...	@@ -2610,9 +2583,7 @@
2610	2583	rdev->owner = regulator_desc->owner;
2611	2584	rdev->desc = regulator_desc;
2612	2585	INIT_LIST_HEAD(&rdev->consumer_list);
2613		- INIT_LIST_HEAD(&rdev->supply_list);
2614	2586	INIT_LIST_HEAD(&rdev->list);
2615		- INIT_LIST_HEAD(&rdev->slist);
2616	2587	BLOCKING_INIT_NOTIFIER_HEAD(&rdev->notifier);
2617	2588
2618	2589	/* preform any regulator specific init */
...	...	@@ -2724,7 +2695,7 @@
2724	2695	unset_regulator_supplies(rdev);
2725	2696	list_del(&rdev->list);
2726	2697	if (rdev->supply)
2727		- sysfs_remove_link(&rdev->dev.kobj, "supply");
	2698	+ regulator_put(rdev->supply);
2728	2699	device_unregister(&rdev->dev);
2729	2700	kfree(rdev->constraints);
2730	2701	mutex_unlock(&regulator_list_mutex);
...	...	@@ -188,18 +188,16 @@
188	188
189	189	/* lists we belong to */
190	190	struct list_head list; /* list of all regulators */
191		- struct list_head slist; /* list of supplied regulators */
192	191
193	192	/* lists we own */
194	193	struct list_head consumer_list; /* consumers we supply */
195		- struct list_head supply_list; /* regulators we supply */
196	194
197	195	struct blocking_notifier_head notifier;
198	196	struct mutex mutex; /* consumer lock */
199	197	struct module *owner;
200	198	struct device dev;
201	199	struct regulation_constraints *constraints;
202		- struct regulator_dev supply; / for tree */
	200	+ struct regulator supply; / for tree */
203	201
204	202	void reg_data; / regulator_dev data */
205	203