diff mbox series

[PATCH/RFC,v4,1/4] regulator: core: add prepare and resume_early

Message ID 1593163942-5087-2-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series treewide: add regulator condition on _mmc_suspend() | expand

Commit Message

Yoshihiro Shimoda June 26, 2020, 9:32 a.m. UTC
The regulator-fixed driver is possible to be off by firmware
like PSCI while the system is suspended. If a consumer could get
such a condition from regulator_is_enabled(), it's useful by
consumers.

The regulator subsystem already has regulator-state-(standby|mem|disk)
sub-nodes and regulator-off-in-suspend property. However,
suitable regulator_ops APIs didn't exist.

So, add new regulator_ops APIs and prepare()/resume_early() in
the regulator_pm_ops to set/clear the condition by new APIs before
suspend() functions of consumers are called.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/regulator/core.c         | 42 ++++++++++++++++++++++++++++++++++++++++
 include/linux/regulator/driver.h |  6 ++++++
 2 files changed, 48 insertions(+)

Comments

Mark Brown June 26, 2020, 2:30 p.m. UTC | #1
On Fri, Jun 26, 2020 at 06:32:19PM +0900, Yoshihiro Shimoda wrote:

> The regulator-fixed driver is possible to be off by firmware
> like PSCI while the system is suspended. If a consumer could get
> such a condition from regulator_is_enabled(), it's useful by
> consumers.

> The regulator subsystem already has regulator-state-(standby|mem|disk)
> sub-nodes and regulator-off-in-suspend property. However,
> suitable regulator_ops APIs didn't exist.

> So, add new regulator_ops APIs and prepare()/resume_early() in
> the regulator_pm_ops to set/clear the condition by new APIs before
> suspend() functions of consumers are called.

I can't follow this explanation at all, I really can't understand what
these functions are supposed to do or how they are supposed to be used.
Nothing in the rest of this series is at all enlightening either.  It
seems there is some need for a consumer to query things about the
suspend state but there is no obvious connection from that to adding
these new operations for regulator drivers.
Yoshihiro Shimoda June 29, 2020, 2:12 a.m. UTC | #2
Hi Mark,

> From: Mark Brown, Sent: Friday, June 26, 2020 11:30 PM
> 
> On Fri, Jun 26, 2020 at 06:32:19PM +0900, Yoshihiro Shimoda wrote:
> 
> > The regulator-fixed driver is possible to be off by firmware
> > like PSCI while the system is suspended. If a consumer could get
> > such a condition from regulator_is_enabled(), it's useful by
> > consumers.
> 
> > The regulator subsystem already has regulator-state-(standby|mem|disk)
> > sub-nodes and regulator-off-in-suspend property. However,
> > suitable regulator_ops APIs didn't exist.
> 
> > So, add new regulator_ops APIs and prepare()/resume_early() in
> > the regulator_pm_ops to set/clear the condition by new APIs before
> > suspend() functions of consumers are called.
> 
> I can't follow this explanation at all, I really can't understand what
> these functions are supposed to do or how they are supposed to be used.
> Nothing in the rest of this series is at all enlightening either.  It
> seems there is some need for a consumer to query things about the
> suspend state but there is no obvious connection from that to adding
> these new operations for regulator drivers.

I'm very sorry for lack description... Perhaps I should have described
one of use cases like below.

regulator_prepare()
--> call regulator_ops.set_prepare_disable() if DISABLE_IN_SUSPEND
 --> A regulator can be disabled by the operation.
 --> We can guarantee an order which is called before a consumer if
     it uses dev_pm_ops.suspend().
..
A consumer driver's suspend().
--> call regulator_is_enabled()
 --> If the regulator was called set_prepare_disable(), this can returns false.

Best regards,
Yoshihiro Shimoda
diff mbox series

Patch

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 03154f5..93eb2a3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5286,6 +5286,46 @@  void regulator_unregister(struct regulator_dev *rdev)
 EXPORT_SYMBOL_GPL(regulator_unregister);
 
 #ifdef CONFIG_SUSPEND
+static int regulator_prepare(struct device *dev)
+{
+	struct regulator_dev *rdev = dev_to_rdev(dev);
+	suspend_state_t state = pm_suspend_target_state;
+	struct regulator_state *rstate;
+	int ret = 0;
+
+	rstate = regulator_get_suspend_state(rdev, state);
+	if (rstate == NULL)
+		return 0;
+
+	regulator_lock(rdev);
+	if (rstate->enabled == DISABLE_IN_SUSPEND &&
+	    rdev->desc->ops->set_prepare_disable)
+		ret = rdev->desc->ops->set_prepare_disable(rdev);
+	regulator_unlock(rdev);
+
+	return ret;
+}
+
+static int regulator_resume_early(struct device *dev)
+{
+	struct regulator_dev *rdev = dev_to_rdev(dev);
+	suspend_state_t state = pm_suspend_target_state;
+	struct regulator_state *rstate;
+	int ret = 0;
+
+	rstate = regulator_get_suspend_state(rdev, state);
+	if (rstate == NULL)
+		return 0;
+
+	regulator_lock(rdev);
+	if (rstate->enabled == DISABLE_IN_SUSPEND &&
+	    rdev->desc->ops->clear_resume_early_disable)
+		ret = rdev->desc->ops->clear_resume_early_disable(rdev);
+	regulator_unlock(rdev);
+
+	return ret;
+}
+
 /**
  * regulator_suspend - prepare regulators for system wide suspend
  * @dev: ``&struct device`` pointer that is passed to _regulator_suspend()
@@ -5336,6 +5376,8 @@  static int regulator_resume(struct device *dev)
 
 #ifdef CONFIG_PM
 static const struct dev_pm_ops __maybe_unused regulator_pm_ops = {
+	.prepare	= regulator_prepare,
+	.resume_early	= regulator_resume_early,
 	.suspend	= regulator_suspend,
 	.resume		= regulator_resume,
 };
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 7eb9fea..299a504 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -115,6 +115,10 @@  enum regulator_status {
  *                      suspended.
  * @set_suspend_disable: Mark the regulator as disabled when the system is
  *                       suspended.
+ * @set_prepare_disable: Mark the regulator as disabled when the system is
+ *                       suspending.
+ * @clear_resume_early_disable: Unmark the regulator as disabled when
+ *                              the system is resuming.
  * @set_suspend_mode: Set the operating mode for the regulator when the
  *                    system is suspended.
  *
@@ -195,6 +199,8 @@  struct regulator_ops {
 	/* enable/disable regulator in suspend state */
 	int (*set_suspend_enable) (struct regulator_dev *);
 	int (*set_suspend_disable) (struct regulator_dev *);
+	int (*set_prepare_disable) (struct regulator_dev *);
+	int (*clear_resume_early_disable) (struct regulator_dev *);
 
 	/* set regulator suspend operating mode (defined in consumer.h) */
 	int (*set_suspend_mode) (struct regulator_dev *, unsigned int mode);