linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] regulator: unexport regulator_lock/unlock()
@ 2020-08-10  4:33 Michał Mirosław
  2020-08-10  4:33 ` [PATCH 2/3] regulator: remove locking around regulator_notifier_call_chain() Michał Mirosław
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Michał Mirosław @ 2020-08-10  4:33 UTC (permalink / raw)
  To: Dmitry Osipenko, Liam Girdwood, Mark Brown, Support Opensource
  Cc: linux-kernel, patches

This removes regulator_lock/unlock() calls around
regulator_notifier_call_chain() as they are redundant - drivers
already have to guarantee regulator_dev's existence during the call.

This should make reasoing about the lock easier, as this was the only
use outside regulator core code.

The only client that needed recursive locking from the notifier chain
was drivers/usb/host/ohci-da8xx.c, which responds to over-current
notification by calling regulator_disable().

Michał Mirosław (3):
  regulator: don't require mutex for regulator_notifier_call_chain()
  regulator: remove locking around regulator_notifier_call_chain()
  regulator: unexport regulator_lock/unlock()

 drivers/regulator/core.c               | 11 +++--------
 drivers/regulator/da9055-regulator.c   |  2 --
 drivers/regulator/da9062-regulator.c   |  2 --
 drivers/regulator/da9063-regulator.c   |  2 --
 drivers/regulator/da9210-regulator.c   |  4 ----
 drivers/regulator/da9211-regulator.c   |  4 ----
 drivers/regulator/lp8755.c             |  6 ------
 drivers/regulator/ltc3589.c            | 10 ++--------
 drivers/regulator/ltc3676.c            | 10 ++--------
 drivers/regulator/pv88060-regulator.c  | 10 ++--------
 drivers/regulator/pv88080-regulator.c  | 10 ++--------
 drivers/regulator/pv88090-regulator.c  | 10 ++--------
 drivers/regulator/slg51000-regulator.c |  4 ----
 drivers/regulator/stpmic1_regulator.c  |  4 ----
 drivers/regulator/wm831x-dcdc.c        |  4 ----
 drivers/regulator/wm831x-isink.c       |  2 --
 drivers/regulator/wm831x-ldo.c         |  2 --
 drivers/regulator/wm8350-regulator.c   |  2 --
 include/linux/regulator/driver.h       |  3 ---
 19 files changed, 13 insertions(+), 89 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] regulator: don't require mutex for regulator_notifier_call_chain()
  2020-08-10  4:33 [PATCH 0/3] regulator: unexport regulator_lock/unlock() Michał Mirosław
  2020-08-10  4:33 ` [PATCH 2/3] regulator: remove locking around regulator_notifier_call_chain() Michał Mirosław
@ 2020-08-10  4:33 ` Michał Mirosław
  2020-09-06 11:55   ` Dmitry Osipenko
  2020-08-10  4:33 ` [PATCH 3/3] regulator: unexport regulator_lock/unlock() Michał Mirosław
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Michał Mirosław @ 2020-08-10  4:33 UTC (permalink / raw)
  To: Dmitry Osipenko, Liam Girdwood, Mark Brown, Support Opensource
  Cc: linux-kernel, patches

Since 3801b86aa482 ("regulator: Refactor supply implementation
to work as regular consumers") we no longer cascade notifications
and so notifier head's built-in rwsem is enough to protect the
notifier chain. Remove the requirement to fix one case where
rdev->mutex might be forced to be taken recursively.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/regulator/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b0662927487c..f4035167e7ba 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4658,14 +4658,11 @@ EXPORT_SYMBOL_GPL(regulator_bulk_free);
  * @data: callback-specific data.
  *
  * Called by regulator drivers to notify clients a regulator event has
- * occurred. We also notify regulator clients downstream.
- * Note lock must be held by caller.
+ * occurred.
  */
 int regulator_notifier_call_chain(struct regulator_dev *rdev,
 				  unsigned long event, void *data)
 {
-	lockdep_assert_held_once(&rdev->mutex.base);
-
 	_notifier_call_chain(rdev, event, data);
 	return NOTIFY_DONE;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] regulator: remove locking around regulator_notifier_call_chain()
  2020-08-10  4:33 [PATCH 0/3] regulator: unexport regulator_lock/unlock() Michał Mirosław
@ 2020-08-10  4:33 ` Michał Mirosław
  2020-08-10 14:17   ` Adam Thomson
  2020-09-06 11:58   ` Dmitry Osipenko
  2020-08-10  4:33 ` [PATCH 1/3] regulator: don't require mutex for regulator_notifier_call_chain() Michał Mirosław
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Michał Mirosław @ 2020-08-10  4:33 UTC (permalink / raw)
  To: Support Opensource, Liam Girdwood, Mark Brown, Dmitry Osipenko
  Cc: linux-kernel, patches

regulator_notifier_call_chain() doesn't need rdev lock and rdev's
existence is assumed in the code anyway. Remove the locks from drivers.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/regulator/da9055-regulator.c   |  2 --
 drivers/regulator/da9062-regulator.c   |  2 --
 drivers/regulator/da9063-regulator.c   |  2 --
 drivers/regulator/da9210-regulator.c   |  4 ----
 drivers/regulator/da9211-regulator.c   |  4 ----
 drivers/regulator/lp8755.c             |  6 ------
 drivers/regulator/ltc3589.c            | 10 ++--------
 drivers/regulator/ltc3676.c            | 10 ++--------
 drivers/regulator/pv88060-regulator.c  | 10 ++--------
 drivers/regulator/pv88080-regulator.c  | 10 ++--------
 drivers/regulator/pv88090-regulator.c  | 10 ++--------
 drivers/regulator/slg51000-regulator.c |  4 ----
 drivers/regulator/stpmic1_regulator.c  |  4 ----
 drivers/regulator/wm831x-dcdc.c        |  4 ----
 drivers/regulator/wm831x-isink.c       |  2 --
 drivers/regulator/wm831x-ldo.c         |  2 --
 drivers/regulator/wm8350-regulator.c   |  2 --
 17 files changed, 10 insertions(+), 78 deletions(-)

diff --git a/drivers/regulator/da9055-regulator.c b/drivers/regulator/da9055-regulator.c
index c025ccb1a30a..73ff5fc7d8d7 100644
--- a/drivers/regulator/da9055-regulator.c
+++ b/drivers/regulator/da9055-regulator.c
@@ -485,10 +485,8 @@ static irqreturn_t da9055_ldo5_6_oc_irq(int irq, void *data)
 {
 	struct da9055_regulator *regulator = data;
 
-	regulator_lock(regulator->rdev);
 	regulator_notifier_call_chain(regulator->rdev,
 				      REGULATOR_EVENT_OVER_CURRENT, NULL);
-	regulator_unlock(regulator->rdev);
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/regulator/da9062-regulator.c b/drivers/regulator/da9062-regulator.c
index d8112f56e94e..1a6324001027 100644
--- a/drivers/regulator/da9062-regulator.c
+++ b/drivers/regulator/da9062-regulator.c
@@ -907,10 +907,8 @@ static irqreturn_t da9062_ldo_lim_event(int irq, void *data)
 			continue;
 
 		if (BIT(regl->info->oc_event.lsb) & bits) {
-			regulator_lock(regl->rdev);
 			regulator_notifier_call_chain(regl->rdev,
 					REGULATOR_EVENT_OVER_CURRENT, NULL);
-			regulator_unlock(regl->rdev);
 			handled = IRQ_HANDLED;
 		}
 	}
diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063-regulator.c
index fe65b5acaf28..cf7d5341750e 100644
--- a/drivers/regulator/da9063-regulator.c
+++ b/drivers/regulator/da9063-regulator.c
@@ -574,10 +574,8 @@ static irqreturn_t da9063_ldo_lim_event(int irq, void *data)
 			continue;
 
 		if (BIT(regl->info->oc_event.lsb) & bits) {
-			regulator_lock(regl->rdev);
 			regulator_notifier_call_chain(regl->rdev,
 					REGULATOR_EVENT_OVER_CURRENT, NULL);
-			regulator_unlock(regl->rdev);
 		}
 	}
 
diff --git a/drivers/regulator/da9210-regulator.c b/drivers/regulator/da9210-regulator.c
index 0cdeb6186529..f5b0ad02964c 100644
--- a/drivers/regulator/da9210-regulator.c
+++ b/drivers/regulator/da9210-regulator.c
@@ -77,8 +77,6 @@ static irqreturn_t da9210_irq_handler(int irq, void *data)
 	if (error < 0)
 		goto error_i2c;
 
-	regulator_lock(chip->rdev);
-
 	if (val & DA9210_E_OVCURR) {
 		regulator_notifier_call_chain(chip->rdev,
 					      REGULATOR_EVENT_OVER_CURRENT,
@@ -103,8 +101,6 @@ static irqreturn_t da9210_irq_handler(int irq, void *data)
 		handled |= DA9210_E_VMAX;
 	}
 
-	regulator_unlock(chip->rdev);
-
 	if (handled) {
 		/* Clear handled events */
 		error = regmap_write(chip->regmap, DA9210_REG_EVENT_B, handled);
diff --git a/drivers/regulator/da9211-regulator.c b/drivers/regulator/da9211-regulator.c
index 297b3aa7c753..c8ca28aa360d 100644
--- a/drivers/regulator/da9211-regulator.c
+++ b/drivers/regulator/da9211-regulator.c
@@ -332,10 +332,8 @@ static irqreturn_t da9211_irq_handler(int irq, void *data)
 		goto error_i2c;
 
 	if (reg_val & DA9211_E_OV_CURR_A) {
-	        regulator_lock(chip->rdev[0]);
 		regulator_notifier_call_chain(chip->rdev[0],
 			REGULATOR_EVENT_OVER_CURRENT, NULL);
-	        regulator_unlock(chip->rdev[0]);
 
 		err = regmap_write(chip->regmap, DA9211_REG_EVENT_B,
 			DA9211_E_OV_CURR_A);
@@ -346,10 +344,8 @@ static irqreturn_t da9211_irq_handler(int irq, void *data)
 	}
 
 	if (reg_val & DA9211_E_OV_CURR_B) {
-	        regulator_lock(chip->rdev[1]);
 		regulator_notifier_call_chain(chip->rdev[1],
 			REGULATOR_EVENT_OVER_CURRENT, NULL);
-	        regulator_unlock(chip->rdev[1]);
 
 		err = regmap_write(chip->regmap, DA9211_REG_EVENT_B,
 			DA9211_E_OV_CURR_B);
diff --git a/drivers/regulator/lp8755.c b/drivers/regulator/lp8755.c
index 4291df077c39..0f93273fc115 100644
--- a/drivers/regulator/lp8755.c
+++ b/drivers/regulator/lp8755.c
@@ -369,11 +369,9 @@ static irqreturn_t lp8755_irq_handler(int irq, void *data)
 		if ((flag0 & (0x4 << icnt))
 		    && (pchip->irqmask & (0x04 << icnt))
 		    && (pchip->rdev[icnt] != NULL)) {
-			regulator_lock(pchip->rdev[icnt]);
 			regulator_notifier_call_chain(pchip->rdev[icnt],
 						      LP8755_EVENT_PWR_FAULT,
 						      NULL);
-			regulator_unlock(pchip->rdev[icnt]);
 		}
 
 	/* read flag1 register */
@@ -389,22 +387,18 @@ static irqreturn_t lp8755_irq_handler(int irq, void *data)
 	if ((flag1 & 0x01) && (pchip->irqmask & 0x01))
 		for (icnt = 0; icnt < LP8755_BUCK_MAX; icnt++)
 			if (pchip->rdev[icnt] != NULL) {
-				regulator_lock(pchip->rdev[icnt]);
 				regulator_notifier_call_chain(pchip->rdev[icnt],
 							      LP8755_EVENT_OCP,
 							      NULL);
-				regulator_unlock(pchip->rdev[icnt]);
 			}
 
 	/* send OVP event to all regulator devices */
 	if ((flag1 & 0x02) && (pchip->irqmask & 0x02))
 		for (icnt = 0; icnt < LP8755_BUCK_MAX; icnt++)
 			if (pchip->rdev[icnt] != NULL) {
-				regulator_lock(pchip->rdev[icnt]);
 				regulator_notifier_call_chain(pchip->rdev[icnt],
 							      LP8755_EVENT_OVP,
 							      NULL);
-				regulator_unlock(pchip->rdev[icnt]);
 			}
 	return IRQ_HANDLED;
 
diff --git a/drivers/regulator/ltc3589.c b/drivers/regulator/ltc3589.c
index 9a037fdc5fc5..26dce68e053b 100644
--- a/drivers/regulator/ltc3589.c
+++ b/drivers/regulator/ltc3589.c
@@ -357,22 +357,16 @@ static irqreturn_t ltc3589_isr(int irq, void *dev_id)
 
 	if (irqstat & LTC3589_IRQSTAT_THERMAL_WARN) {
 		event = REGULATOR_EVENT_OVER_TEMP;
-		for (i = 0; i < LTC3589_NUM_REGULATORS; i++) {
-		        regulator_lock(ltc3589->regulators[i]);
+		for (i = 0; i < LTC3589_NUM_REGULATORS; i++)
 			regulator_notifier_call_chain(ltc3589->regulators[i],
 						      event, NULL);
-		        regulator_unlock(ltc3589->regulators[i]);
-		}
 	}
 
 	if (irqstat & LTC3589_IRQSTAT_UNDERVOLT_WARN) {
 		event = REGULATOR_EVENT_UNDER_VOLTAGE;
-		for (i = 0; i < LTC3589_NUM_REGULATORS; i++) {
-		        regulator_lock(ltc3589->regulators[i]);
+		for (i = 0; i < LTC3589_NUM_REGULATORS; i++)
 			regulator_notifier_call_chain(ltc3589->regulators[i],
 						      event, NULL);
-		        regulator_unlock(ltc3589->regulators[i]);
-		}
 	}
 
 	/* Clear warning condition */
diff --git a/drivers/regulator/ltc3676.c b/drivers/regulator/ltc3676.c
index 093b3e4a6303..3e66515423df 100644
--- a/drivers/regulator/ltc3676.c
+++ b/drivers/regulator/ltc3676.c
@@ -276,23 +276,17 @@ static irqreturn_t ltc3676_isr(int irq, void *dev_id)
 	if (irqstat & LTC3676_IRQSTAT_THERMAL_WARN) {
 		dev_warn(dev, "Over-temperature Warning\n");
 		event = REGULATOR_EVENT_OVER_TEMP;
-		for (i = 0; i < LTC3676_NUM_REGULATORS; i++) {
-			regulator_lock(ltc3676->regulators[i]);
+		for (i = 0; i < LTC3676_NUM_REGULATORS; i++)
 			regulator_notifier_call_chain(ltc3676->regulators[i],
 						      event, NULL);
-			regulator_unlock(ltc3676->regulators[i]);
-		}
 	}
 
 	if (irqstat & LTC3676_IRQSTAT_UNDERVOLT_WARN) {
 		dev_info(dev, "Undervoltage Warning\n");
 		event = REGULATOR_EVENT_UNDER_VOLTAGE;
-		for (i = 0; i < LTC3676_NUM_REGULATORS; i++) {
-			regulator_lock(ltc3676->regulators[i]);
+		for (i = 0; i < LTC3676_NUM_REGULATORS; i++)
 			regulator_notifier_call_chain(ltc3676->regulators[i],
 						      event, NULL);
-			regulator_unlock(ltc3676->regulators[i]);
-		}
 	}
 
 	/* Clear warning condition */
diff --git a/drivers/regulator/pv88060-regulator.c b/drivers/regulator/pv88060-regulator.c
index 787ced918372..48238846f45c 100644
--- a/drivers/regulator/pv88060-regulator.c
+++ b/drivers/regulator/pv88060-regulator.c
@@ -233,13 +233,10 @@ static irqreturn_t pv88060_irq_handler(int irq, void *data)
 
 	if (reg_val & PV88060_E_VDD_FLT) {
 		for (i = 0; i < PV88060_MAX_REGULATORS; i++) {
-			if (chip->rdev[i] != NULL) {
-				regulator_lock(chip->rdev[i]);
+			if (chip->rdev[i] != NULL)
 				regulator_notifier_call_chain(chip->rdev[i],
 					REGULATOR_EVENT_UNDER_VOLTAGE,
 					NULL);
-				regulator_unlock(chip->rdev[i]);
-			}
 		}
 
 		err = regmap_write(chip->regmap, PV88060_REG_EVENT_A,
@@ -252,13 +249,10 @@ static irqreturn_t pv88060_irq_handler(int irq, void *data)
 
 	if (reg_val & PV88060_E_OVER_TEMP) {
 		for (i = 0; i < PV88060_MAX_REGULATORS; i++) {
-			if (chip->rdev[i] != NULL) {
-				regulator_lock(chip->rdev[i]);
+			if (chip->rdev[i] != NULL)
 				regulator_notifier_call_chain(chip->rdev[i],
 					REGULATOR_EVENT_OVER_TEMP,
 					NULL);
-				regulator_unlock(chip->rdev[i]);
-			}
 		}
 
 		err = regmap_write(chip->regmap, PV88060_REG_EVENT_A,
diff --git a/drivers/regulator/pv88080-regulator.c b/drivers/regulator/pv88080-regulator.c
index a444f68af1a8..2a74cc05acfe 100644
--- a/drivers/regulator/pv88080-regulator.c
+++ b/drivers/regulator/pv88080-regulator.c
@@ -334,13 +334,10 @@ static irqreturn_t pv88080_irq_handler(int irq, void *data)
 
 	if (reg_val & PV88080_E_VDD_FLT) {
 		for (i = 0; i < PV88080_MAX_REGULATORS; i++) {
-			if (chip->rdev[i] != NULL) {
-			        regulator_lock(chip->rdev[i]);
+			if (chip->rdev[i] != NULL)
 				regulator_notifier_call_chain(chip->rdev[i],
 					REGULATOR_EVENT_UNDER_VOLTAGE,
 					NULL);
-			        regulator_unlock(chip->rdev[i]);
-			}
 		}
 
 		err = regmap_write(chip->regmap, PV88080_REG_EVENT_A,
@@ -353,13 +350,10 @@ static irqreturn_t pv88080_irq_handler(int irq, void *data)
 
 	if (reg_val & PV88080_E_OVER_TEMP) {
 		for (i = 0; i < PV88080_MAX_REGULATORS; i++) {
-			if (chip->rdev[i] != NULL) {
-			        regulator_lock(chip->rdev[i]);
+			if (chip->rdev[i] != NULL)
 				regulator_notifier_call_chain(chip->rdev[i],
 					REGULATOR_EVENT_OVER_TEMP,
 					NULL);
-			        regulator_unlock(chip->rdev[i]);
-			}
 		}
 
 		err = regmap_write(chip->regmap, PV88080_REG_EVENT_A,
diff --git a/drivers/regulator/pv88090-regulator.c b/drivers/regulator/pv88090-regulator.c
index 784729ec2182..a80176bdf8ec 100644
--- a/drivers/regulator/pv88090-regulator.c
+++ b/drivers/regulator/pv88090-regulator.c
@@ -226,13 +226,10 @@ static irqreturn_t pv88090_irq_handler(int irq, void *data)
 
 	if (reg_val & PV88090_E_VDD_FLT) {
 		for (i = 0; i < PV88090_MAX_REGULATORS; i++) {
-			if (chip->rdev[i] != NULL) {
-			        regulator_lock(chip->rdev[i]);
+			if (chip->rdev[i] != NULL)
 				regulator_notifier_call_chain(chip->rdev[i],
 					REGULATOR_EVENT_UNDER_VOLTAGE,
 					NULL);
-			        regulator_unlock(chip->rdev[i]);
-			}
 		}
 
 		err = regmap_write(chip->regmap, PV88090_REG_EVENT_A,
@@ -245,13 +242,10 @@ static irqreturn_t pv88090_irq_handler(int irq, void *data)
 
 	if (reg_val & PV88090_E_OVER_TEMP) {
 		for (i = 0; i < PV88090_MAX_REGULATORS; i++) {
-			if (chip->rdev[i] != NULL) {
-			        regulator_lock(chip->rdev[i]);
+			if (chip->rdev[i] != NULL)
 				regulator_notifier_call_chain(chip->rdev[i],
 					REGULATOR_EVENT_OVER_TEMP,
 					NULL);
-			        regulator_unlock(chip->rdev[i]);
-			}
 		}
 
 		err = regmap_write(chip->regmap, PV88090_REG_EVENT_A,
diff --git a/drivers/regulator/slg51000-regulator.c b/drivers/regulator/slg51000-regulator.c
index 44e4cecbf6de..0f8041981436 100644
--- a/drivers/regulator/slg51000-regulator.c
+++ b/drivers/regulator/slg51000-regulator.c
@@ -386,10 +386,8 @@ static irqreturn_t slg51000_irq_handler(int irq, void *data)
 	for (i = 0; i < SLG51000_MAX_REGULATORS; i++) {
 		if (!(evt[i][R2] & SLG51000_IRQ_ILIM_FLAG_MASK) &&
 		    (evt[i][R0] & SLG51000_EVT_ILIM_FLAG_MASK)) {
-			regulator_lock(chip->rdev[i]);
 			regulator_notifier_call_chain(chip->rdev[i],
 					    REGULATOR_EVENT_OVER_CURRENT, NULL);
-			regulator_unlock(chip->rdev[i]);
 
 			if (evt[i][R1] & SLG51000_STA_ILIM_FLAG_MASK)
 				dev_warn(chip->dev,
@@ -403,10 +401,8 @@ static irqreturn_t slg51000_irq_handler(int irq, void *data)
 		for (i = 0; i < SLG51000_MAX_REGULATORS; i++) {
 			if (!(evt[i][R1] & SLG51000_STA_ILIM_FLAG_MASK) &&
 			    (evt[i][R1] & SLG51000_STA_VOUT_OK_FLAG_MASK)) {
-				regulator_lock(chip->rdev[i]);
 				regulator_notifier_call_chain(chip->rdev[i],
 					       REGULATOR_EVENT_OVER_TEMP, NULL);
-				regulator_unlock(chip->rdev[i]);
 			}
 		}
 		handled = IRQ_HANDLED;
diff --git a/drivers/regulator/stpmic1_regulator.c b/drivers/regulator/stpmic1_regulator.c
index 73e0ab2baeaa..cf10fdb72e32 100644
--- a/drivers/regulator/stpmic1_regulator.c
+++ b/drivers/regulator/stpmic1_regulator.c
@@ -505,15 +505,11 @@ static irqreturn_t stpmic1_curlim_irq_handler(int irq, void *data)
 {
 	struct regulator_dev *rdev = (struct regulator_dev *)data;
 
-	regulator_lock(rdev);
-
 	/* Send an overcurrent notification */
 	regulator_notifier_call_chain(rdev,
 				      REGULATOR_EVENT_OVER_CURRENT,
 				      NULL);
 
-	regulator_unlock(rdev);
-
 	return IRQ_HANDLED;
 }
 
diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index ad2203d11a88..e43ed4d93f71 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -178,11 +178,9 @@ static irqreturn_t wm831x_dcdc_uv_irq(int irq, void *data)
 {
 	struct wm831x_dcdc *dcdc = data;
 
-	regulator_lock(dcdc->regulator);
 	regulator_notifier_call_chain(dcdc->regulator,
 				      REGULATOR_EVENT_UNDER_VOLTAGE,
 				      NULL);
-	regulator_unlock(dcdc->regulator);
 
 	return IRQ_HANDLED;
 }
@@ -191,11 +189,9 @@ static irqreturn_t wm831x_dcdc_oc_irq(int irq, void *data)
 {
 	struct wm831x_dcdc *dcdc = data;
 
-	regulator_lock(dcdc->regulator);
 	regulator_notifier_call_chain(dcdc->regulator,
 				      REGULATOR_EVENT_OVER_CURRENT,
 				      NULL);
-	regulator_unlock(dcdc->regulator);
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/regulator/wm831x-isink.c b/drivers/regulator/wm831x-isink.c
index ff3d2bf50410..eade3ae3e333 100644
--- a/drivers/regulator/wm831x-isink.c
+++ b/drivers/regulator/wm831x-isink.c
@@ -99,11 +99,9 @@ static irqreturn_t wm831x_isink_irq(int irq, void *data)
 {
 	struct wm831x_isink *isink = data;
 
-	regulator_lock(isink->regulator);
 	regulator_notifier_call_chain(isink->regulator,
 				      REGULATOR_EVENT_OVER_CURRENT,
 				      NULL);
-	regulator_unlock(isink->regulator);
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/regulator/wm831x-ldo.c b/drivers/regulator/wm831x-ldo.c
index 7b6cf4810cb7..e091b189ecc0 100644
--- a/drivers/regulator/wm831x-ldo.c
+++ b/drivers/regulator/wm831x-ldo.c
@@ -46,11 +46,9 @@ static irqreturn_t wm831x_ldo_uv_irq(int irq, void *data)
 {
 	struct wm831x_ldo *ldo = data;
 
-	regulator_lock(ldo->regulator);
 	regulator_notifier_call_chain(ldo->regulator,
 				      REGULATOR_EVENT_UNDER_VOLTAGE,
 				      NULL);
-	regulator_unlock(ldo->regulator);
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/regulator/wm8350-regulator.c b/drivers/regulator/wm8350-regulator.c
index 2e7bfdf7c87b..6579bfdb0c26 100644
--- a/drivers/regulator/wm8350-regulator.c
+++ b/drivers/regulator/wm8350-regulator.c
@@ -1089,7 +1089,6 @@ static irqreturn_t pmic_uv_handler(int irq, void *data)
 {
 	struct regulator_dev *rdev = (struct regulator_dev *)data;
 
-	regulator_lock(rdev);
 	if (irq == WM8350_IRQ_CS1 || irq == WM8350_IRQ_CS2)
 		regulator_notifier_call_chain(rdev,
 					      REGULATOR_EVENT_REGULATION_OUT,
@@ -1098,7 +1097,6 @@ static irqreturn_t pmic_uv_handler(int irq, void *data)
 		regulator_notifier_call_chain(rdev,
 					      REGULATOR_EVENT_UNDER_VOLTAGE,
 					      NULL);
-	regulator_unlock(rdev);
 
 	return IRQ_HANDLED;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] regulator: unexport regulator_lock/unlock()
  2020-08-10  4:33 [PATCH 0/3] regulator: unexport regulator_lock/unlock() Michał Mirosław
  2020-08-10  4:33 ` [PATCH 2/3] regulator: remove locking around regulator_notifier_call_chain() Michał Mirosław
  2020-08-10  4:33 ` [PATCH 1/3] regulator: don't require mutex for regulator_notifier_call_chain() Michał Mirosław
@ 2020-08-10  4:33 ` Michał Mirosław
  2020-09-06 11:59   ` Dmitry Osipenko
  2020-09-06  0:07 ` [PATCH 0/3] " Michał Mirosław
       [not found] ` <159950194954.52863.7080307258573052895.b4-ty@kernel.org>
  4 siblings, 1 reply; 12+ messages in thread
From: Michał Mirosław @ 2020-08-10  4:33 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Dmitry Osipenko, Support Opensource
  Cc: linux-kernel, patches

regulator_lock/unlock() was used only to guard
regulator_notifier_call_chain(). As no users remain, make the functions
internal.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/regulator/core.c         | 6 ++----
 include/linux/regulator/driver.h | 3 ---
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f4035167e7ba..0a32c3da0e26 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -170,11 +170,10 @@ static inline int regulator_lock_nested(struct regulator_dev *rdev,
  * than the one, which initially locked the mutex, it will
  * wait on mutex.
  */
-void regulator_lock(struct regulator_dev *rdev)
+static void regulator_lock(struct regulator_dev *rdev)
 {
 	regulator_lock_nested(rdev, NULL);
 }
-EXPORT_SYMBOL_GPL(regulator_lock);
 
 /**
  * regulator_unlock - unlock a single regulator
@@ -183,7 +182,7 @@ EXPORT_SYMBOL_GPL(regulator_lock);
  * This function unlocks the mutex when the
  * reference counter reaches 0.
  */
-void regulator_unlock(struct regulator_dev *rdev)
+static void regulator_unlock(struct regulator_dev *rdev)
 {
 	if (WARN_ON_ONCE(rdev->ref_cnt <= 0))
 		return;
@@ -191,7 +190,6 @@ void regulator_unlock(struct regulator_dev *rdev)
 	if (--rdev->ref_cnt == 0)
 		ww_mutex_unlock(&rdev->mutex);
 }
-EXPORT_SYMBOL_GPL(regulator_unlock);
 
 static bool regulator_supply_is_couple(struct regulator_dev *rdev)
 {
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 2fe5b1bcbe2f..9c5a74cdebfe 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -532,9 +532,6 @@ int regulator_set_current_limit_regmap(struct regulator_dev *rdev,
 int regulator_get_current_limit_regmap(struct regulator_dev *rdev);
 void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
 
-void regulator_lock(struct regulator_dev *rdev);
-void regulator_unlock(struct regulator_dev *rdev);
-
 /*
  * Helper functions intended to be used by regulator drivers prior registering
  * their regulators.
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* RE: [PATCH 2/3] regulator: remove locking around regulator_notifier_call_chain()
  2020-08-10  4:33 ` [PATCH 2/3] regulator: remove locking around regulator_notifier_call_chain() Michał Mirosław
@ 2020-08-10 14:17   ` Adam Thomson
  2020-09-06 11:58   ` Dmitry Osipenko
  1 sibling, 0 replies; 12+ messages in thread
From: Adam Thomson @ 2020-08-10 14:17 UTC (permalink / raw)
  To: Michał Mirosław, Support Opensource, Liam Girdwood,
	Mark Brown, Dmitry Osipenko
  Cc: linux-kernel, patches

On 10 August 2020 05:34, Michał Mirosław wrote:

> regulator_notifier_call_chain() doesn't need rdev lock and rdev's
> existence is assumed in the code anyway. Remove the locks from drivers.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

For da9*-regulator.c, pv88*-regulator.c and slg51000-regulator.c:

Acked-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> ---
>  drivers/regulator/da9055-regulator.c   |  2 --
>  drivers/regulator/da9062-regulator.c   |  2 --
>  drivers/regulator/da9063-regulator.c   |  2 --
>  drivers/regulator/da9210-regulator.c   |  4 ----
>  drivers/regulator/da9211-regulator.c   |  4 ----
>  drivers/regulator/lp8755.c             |  6 ------
>  drivers/regulator/ltc3589.c            | 10 ++--------
>  drivers/regulator/ltc3676.c            | 10 ++--------
>  drivers/regulator/pv88060-regulator.c  | 10 ++--------
>  drivers/regulator/pv88080-regulator.c  | 10 ++--------
>  drivers/regulator/pv88090-regulator.c  | 10 ++--------
>  drivers/regulator/slg51000-regulator.c |  4 ----
>  drivers/regulator/stpmic1_regulator.c  |  4 ----
>  drivers/regulator/wm831x-dcdc.c        |  4 ----
>  drivers/regulator/wm831x-isink.c       |  2 --
>  drivers/regulator/wm831x-ldo.c         |  2 --
>  drivers/regulator/wm8350-regulator.c   |  2 --
>  17 files changed, 10 insertions(+), 78 deletions(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] regulator: unexport regulator_lock/unlock()
  2020-08-10  4:33 [PATCH 0/3] regulator: unexport regulator_lock/unlock() Michał Mirosław
                   ` (2 preceding siblings ...)
  2020-08-10  4:33 ` [PATCH 3/3] regulator: unexport regulator_lock/unlock() Michał Mirosław
@ 2020-09-06  0:07 ` Michał Mirosław
       [not found] ` <159950194954.52863.7080307258573052895.b4-ty@kernel.org>
  4 siblings, 0 replies; 12+ messages in thread
From: Michał Mirosław @ 2020-09-06  0:07 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Dmitry Osipenko; +Cc: linux-kernel, patches

On Mon, Aug 10, 2020 at 06:33:30AM +0200, Michał Mirosław wrote:
> This removes regulator_lock/unlock() calls around
> regulator_notifier_call_chain() as they are redundant - drivers
> already have to guarantee regulator_dev's existence during the call.
> 
> This should make reasoing about the lock easier, as this was the only
> use outside regulator core code.
> 
> The only client that needed recursive locking from the notifier chain
> was drivers/usb/host/ohci-da8xx.c, which responds to over-current
> notification by calling regulator_disable().

I can't see the series in regulator/for-next and got no comments.
Should I resend?

As a side note: this is a step towards fixing regulator-coupling-related
locking issues.

Best Regards,
Michał Mirosław

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] regulator: don't require mutex for regulator_notifier_call_chain()
  2020-08-10  4:33 ` [PATCH 1/3] regulator: don't require mutex for regulator_notifier_call_chain() Michał Mirosław
@ 2020-09-06 11:55   ` Dmitry Osipenko
  0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Osipenko @ 2020-09-06 11:55 UTC (permalink / raw)
  To: Michał Mirosław, Liam Girdwood, Mark Brown, Support Opensource
  Cc: linux-kernel, patches

10.08.2020 07:33, Michał Mirosław пишет:
> Since 3801b86aa482 ("regulator: Refactor supply implementation
> to work as regular consumers") we no longer cascade notifications
> and so notifier head's built-in rwsem is enough to protect the
> notifier chain. Remove the requirement to fix one case where
> rdev->mutex might be forced to be taken recursively.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  drivers/regulator/core.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index b0662927487c..f4035167e7ba 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -4658,14 +4658,11 @@ EXPORT_SYMBOL_GPL(regulator_bulk_free);
>   * @data: callback-specific data.
>   *
>   * Called by regulator drivers to notify clients a regulator event has
> - * occurred. We also notify regulator clients downstream.
> - * Note lock must be held by caller.
> + * occurred.
>   */
>  int regulator_notifier_call_chain(struct regulator_dev *rdev,
>  				  unsigned long event, void *data)
>  {
> -	lockdep_assert_held_once(&rdev->mutex.base);
> -
>  	_notifier_call_chain(rdev, event, data);
>  	return NOTIFY_DONE;
>  
> 

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] regulator: remove locking around regulator_notifier_call_chain()
  2020-08-10  4:33 ` [PATCH 2/3] regulator: remove locking around regulator_notifier_call_chain() Michał Mirosław
  2020-08-10 14:17   ` Adam Thomson
@ 2020-09-06 11:58   ` Dmitry Osipenko
  1 sibling, 0 replies; 12+ messages in thread
From: Dmitry Osipenko @ 2020-09-06 11:58 UTC (permalink / raw)
  To: Michał Mirosław, Support Opensource, Liam Girdwood, Mark Brown
  Cc: linux-kernel, patches

10.08.2020 07:33, Michał Mirosław пишет:
> regulator_notifier_call_chain() doesn't need rdev lock and rdev's
> existence is assumed in the code anyway. Remove the locks from drivers.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  drivers/regulator/da9055-regulator.c   |  2 --
>  drivers/regulator/da9062-regulator.c   |  2 --
>  drivers/regulator/da9063-regulator.c   |  2 --
>  drivers/regulator/da9210-regulator.c   |  4 ----
>  drivers/regulator/da9211-regulator.c   |  4 ----
>  drivers/regulator/lp8755.c             |  6 ------
>  drivers/regulator/ltc3589.c            | 10 ++--------
>  drivers/regulator/ltc3676.c            | 10 ++--------
>  drivers/regulator/pv88060-regulator.c  | 10 ++--------
>  drivers/regulator/pv88080-regulator.c  | 10 ++--------
>  drivers/regulator/pv88090-regulator.c  | 10 ++--------
>  drivers/regulator/slg51000-regulator.c |  4 ----
>  drivers/regulator/stpmic1_regulator.c  |  4 ----
>  drivers/regulator/wm831x-dcdc.c        |  4 ----
>  drivers/regulator/wm831x-isink.c       |  2 --
>  drivers/regulator/wm831x-ldo.c         |  2 --
>  drivers/regulator/wm8350-regulator.c   |  2 --
>  17 files changed, 10 insertions(+), 78 deletions(-)

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] regulator: unexport regulator_lock/unlock()
  2020-08-10  4:33 ` [PATCH 3/3] regulator: unexport regulator_lock/unlock() Michał Mirosław
@ 2020-09-06 11:59   ` Dmitry Osipenko
  0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Osipenko @ 2020-09-06 11:59 UTC (permalink / raw)
  To: Michał Mirosław, Liam Girdwood, Mark Brown, Support Opensource
  Cc: linux-kernel, patches

10.08.2020 07:33, Michał Mirosław пишет:
> regulator_lock/unlock() was used only to guard
> regulator_notifier_call_chain(). As no users remain, make the functions
> internal.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  drivers/regulator/core.c         | 6 ++----
>  include/linux/regulator/driver.h | 3 ---
>  2 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] regulator: unexport regulator_lock/unlock()
       [not found] ` <159950194954.52863.7080307258573052895.b4-ty@kernel.org>
@ 2020-09-15 18:55   ` Michał Mirosław
  2020-09-16 10:36     ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Michał Mirosław @ 2020-09-15 18:55 UTC (permalink / raw)
  To: Mark Brown
  Cc: Dmitry Osipenko, Liam Girdwood, Support Opensource, linux-kernel,
	patches

On Mon, Sep 07, 2020 at 07:05:49PM +0100, Mark Brown wrote:
> On Mon, 10 Aug 2020 06:33:30 +0200, Michał Mirosław wrote:
> > This removes regulator_lock/unlock() calls around
> > regulator_notifier_call_chain() as they are redundant - drivers
> > already have to guarantee regulator_dev's existence during the call.
> > 
> > This should make reasoing about the lock easier, as this was the only
> > use outside regulator core code.
> > 
> > [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
> 
> Thanks!
> 
> [1/3] regulator: don't require mutex for regulator_notifier_call_chain()
>       commit: 3bca239d6184df61a619d78764e0481242d844b4
> [2/3] regulator: remove locking around regulator_notifier_call_chain()
>       commit: e9c142b0d2c08178a9e146d47d8fe397373bda3e
> [3/3] regulator: unexport regulator_lock/unlock()
>       (no commit info)
[...]

It looks like the third one didn't get in? (Can't see it in the
for-next branch).

Best Regards
Michał Mirosław

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] regulator: unexport regulator_lock/unlock()
  2020-09-15 18:55   ` Michał Mirosław
@ 2020-09-16 10:36     ` Mark Brown
  2020-09-19 21:28       ` [PATCH v2] " Michał Mirosław
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2020-09-16 10:36 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Dmitry Osipenko, Liam Girdwood, Support Opensource, linux-kernel,
	patches

[-- Attachment #1: Type: text/plain, Size: 360 bytes --]

On Tue, Sep 15, 2020 at 08:55:06PM +0200, Michał Mirosław wrote:
> On Mon, Sep 07, 2020 at 07:05:49PM +0100, Mark Brown wrote:

> > [3/3] regulator: unexport regulator_lock/unlock()
> >       (no commit info)

> It looks like the third one didn't get in? (Can't see it in the
> for-next branch).

It probably didn't apply, please check and resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] regulator: unexport regulator_lock/unlock()
  2020-09-16 10:36     ` Mark Brown
@ 2020-09-19 21:28       ` Michał Mirosław
  0 siblings, 0 replies; 12+ messages in thread
From: Michał Mirosław @ 2020-09-19 21:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown; +Cc: linux-kernel

regulator_lock/unlock() was used only to guard
regulator_notifier_call_chain(). As no users remain, make the functions
internal.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/regulator/core.c         | 6 ++----
 include/linux/regulator/driver.h | 3 ---
 2 files changed, 2 insertions(+), 7 deletions(-)
---
 v2: rebased on current regulator/for-linus

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7ff507ec875a..8da37e0d1100 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -190,11 +190,10 @@ static inline int regulator_lock_nested(struct regulator_dev *rdev,
  * than the one, which initially locked the mutex, it will
  * wait on mutex.
  */
-void regulator_lock(struct regulator_dev *rdev)
+static void regulator_lock(struct regulator_dev *rdev)
 {
 	regulator_lock_nested(rdev, NULL);
 }
-EXPORT_SYMBOL_GPL(regulator_lock);
 
 /**
  * regulator_unlock - unlock a single regulator
@@ -203,7 +202,7 @@ EXPORT_SYMBOL_GPL(regulator_lock);
  * This function unlocks the mutex when the
  * reference counter reaches 0.
  */
-void regulator_unlock(struct regulator_dev *rdev)
+static void regulator_unlock(struct regulator_dev *rdev)
 {
 	mutex_lock(&regulator_nesting_mutex);
 
@@ -216,7 +215,6 @@ void regulator_unlock(struct regulator_dev *rdev)
 
 	mutex_unlock(&regulator_nesting_mutex);
 }
-EXPORT_SYMBOL_GPL(regulator_unlock);
 
 static bool regulator_supply_is_couple(struct regulator_dev *rdev)
 {
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 8539f34ae42b..11cade73726c 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -533,9 +533,6 @@ int regulator_set_current_limit_regmap(struct regulator_dev *rdev,
 int regulator_get_current_limit_regmap(struct regulator_dev *rdev);
 void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
 
-void regulator_lock(struct regulator_dev *rdev);
-void regulator_unlock(struct regulator_dev *rdev);
-
 /*
  * Helper functions intended to be used by regulator drivers prior registering
  * their regulators.
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-09-19 21:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-10  4:33 [PATCH 0/3] regulator: unexport regulator_lock/unlock() Michał Mirosław
2020-08-10  4:33 ` [PATCH 2/3] regulator: remove locking around regulator_notifier_call_chain() Michał Mirosław
2020-08-10 14:17   ` Adam Thomson
2020-09-06 11:58   ` Dmitry Osipenko
2020-08-10  4:33 ` [PATCH 1/3] regulator: don't require mutex for regulator_notifier_call_chain() Michał Mirosław
2020-09-06 11:55   ` Dmitry Osipenko
2020-08-10  4:33 ` [PATCH 3/3] regulator: unexport regulator_lock/unlock() Michał Mirosław
2020-09-06 11:59   ` Dmitry Osipenko
2020-09-06  0:07 ` [PATCH 0/3] " Michał Mirosław
     [not found] ` <159950194954.52863.7080307258573052895.b4-ty@kernel.org>
2020-09-15 18:55   ` Michał Mirosław
2020-09-16 10:36     ` Mark Brown
2020-09-19 21:28       ` [PATCH v2] " Michał Mirosław

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).