[PATCH v5 0/4] regulator: qcom-rpmh: Support RPMH address reads and use it for rpmh-regulators

[PATCH v5 0/4] regulator: qcom-rpmh: Support RPMH address reads and use it for rpmh-regulators

[Kamal Wadhwa]

This patch series adds a new `rpmh_read()` API to allow reading RPMH
addresses. Using this API enhances the RPMH regulator driver by adding
new `get_status()` callback to reflect the regulator status and also
readback the voltage/bypass/mode settings as they have been applied by
APPS during the bootloader stage, so regulator framework can get them
via `get_mode`, `get_bypass` & `get_voltage_selector` callbacks during
regulator registration.

This is needed because currently regulator framework does a unnecessary
write with `min-microvolt` DT setting for all the RPMH regulators during
regulator registration, because the first time after boot the value is
seen as -ENOTRECOVERABLE, as there is no option to read these regulator
settings.

With this change this unnecessary write can be avoided and regulator
framework gets a sense of the initial state set during the bootloader
stage for all regulator settings.

NOTE - During discussion on the v2 series - PATCH 3/4, reviewer had
inquired about possible need for the use of the sync_state() to handle the
"multiple" client case - for maintaining the regulator settings till all
the clients are probed.

This case was not covered in my previous series and had originally planned
to do that series separately. But after the discussion decided to merge
the 2 series as it seemed this would be a better approach. But after
working on sync_state change. I realized a basic issue with using
sync_state() for regulators - that its per-driver and not per-regulator
resource. But we needed a sync_state callback for each regulator separately.

I had been experimenting with few ideas but seems its going to need more
time for me to close on the equivalent solution that has per-regulator
sync_state or something to that effect. So I thought to close on this
series and attend to that separately.

Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
---
Changes in v5:
- In `_rpmh_regulator_vrm_get_voltage()`, removed the min/max voltage
  range check that was suppressing out-of-range readback values, as
  suggested by Mark. The raw hardware-read voltage is now always
  reported to the regulator framework. A separate patch will be sent
  to handle out-of-range voltage constraint handling in core.c.
- Sorted the `#include <linux/bits.h>` alphabetically with the rest of
  the include block, as pointed out by Konrad.
- Removed the `rpmh_regulator_read_data()` wrapper (a oneliner around
  `rpmh_read()`) and inlined `rpmh_read()` at each call site instead,
  as suggested by Konrad.
- Dropped the unnecessary `sts = 0` initialization in
  `rpmh_regulator_determine_initial_status()`, since it is always
  initialized before use, as pointed out by Konrad.
- Added a blank line above the `return` statements in the `!pmic_mode`
  block of `rpmh_regulator_determine_initial_status()`, for consistency
  with the rest of the function, as pointed out by Konrad.
- Link to v4: https://lore.kernel.org/all/20260420-read-rpmh-v3-v4-0-70c152e6c958@oss.qualcomm.com/

Changes in v4:
- Modified the register mask macros to use  GENMASK()/BIT() instead of
  numbers as suggested by reviewer.
- Removed the unreachable code in the determine_initial_status() for XOB
  type regulators status update logic as suggested by reviewer.
- Link to v3: https://lore.kernel.org/all/20260407-read-rpmh-v3-v3-0-34079f92691c@oss.qualcomm.com/

Changes in v3:
- Removed "bypass_supported" as that is not needed for regulators
  that don't have set_bypass implemented, as pointed by Dmitry.
- Handled the corner case where the mode/bypass setting is read 0, but
  its unclear if the register has been set to 0 or its un-accessed.
- Dropped `convert_mode_to_status()` and use the `regulator_mode_to_status()`
  instead.
- Refactored some code to simplify the `status` update after every
  enable/mode/bypass setting change.
- Corrected subject line of all patches to have `regulator: qcom-rpmh:`
  for all the `qcom-rpmh-regulator.c` file changes, as pointed by Bjorn.
- Re-ordered the series to have the `rpmh.c` driver patches first and
  than `qcom-rpmh-regulator.c` driver patches as asked by Bjorn.
- In the BOB5 bypass fix patch (PATCH 1/4 in previous series), added
  the fixes commit#, as it was missed earlier.
- In the rpmh driver change(PATCH 2/4 in previous series), modified
  commit wording and removed linked as suggested by reviewer.
- Fixed kernel test robot issues and other formatting issues in
  PATCH 3/4 of last series.
- Corrected the checkpatch error fix PATCH 4/4 to keep to only
  one error in comment section which existed prior to this
  series.
- Modified the subject line a little and added proper tags
- Link to v2: https://lore.kernel.org/all/20251022-add-rpmh-read-support-v2-0-5c7a8e4df601@oss.qualcomm.com/

Changes in v2:
- Fixed the BOB bypass mode handling (existing issue in current driver).
  This was needed for `get_status()` implementation.
- Implemented `get_status()` callback.
- Callbacks for `is_enabled()` & `get_mode()` will now be used as-is
  ie. v1 changes reverted.
- Bootstrapped the read values for `mode` and `status` in probe, based on
  comments received from reviewer.
- Callback for `get_voltage_sel()` has been modified to handle cases
  where read voltage is out-of-range defined in the regulator DT settings,
  this is needed to ensure backward compatibilty. Regulator probes may
  fail otherwise for some older targets.
- This patch is rebased & tested on:
  https://lore.kernel.org/all/176070318151.57631.15443673679580823321.b4-ty@kernel.org/
  to avoid any merge issues.
- Fixed code style issues reported by checkpatch.pl script.
- Link to v1: https://lore.kernel.org/r/20250623-add-rpmh-read-support-v1-0-ae583d260195@oss.qualcomm.com

To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Vinod Koul <vkoul@kernel.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>

---
Kamal Wadhwa (3):
      regulator: qcom-rpmh: Fix PMIC5 BOB bypass mode handling
      regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup
      regulator: qcom-rpmh: Fix coding style issues

Maulik Shah (1):
      soc: qcom: rpmh: Add support to read back resource settings

 drivers/regulator/qcom-rpmh-regulator.c | 169 +++++++++++++++++++++++++++++++-
 drivers/soc/qcom/rpmh-rsc.c             |  13 ++-
 drivers/soc/qcom/rpmh.c                 |  47 ++++++++-
 include/soc/qcom/rpmh.h                 |   5 +
 include/soc/qcom/tcs.h                  |   2 +
 5 files changed, 228 insertions(+), 8 deletions(-)
---
base-commit: 5c73cd9f0819c1c44e373e3dabb68318b1de1a12
change-id: 20260720-b4-read-rpmh-v5-bab1bd1cbe55

Best regards,
--  
Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>

[PATCH v5 1/4] soc: qcom: rpmh: Add support to read back resource settings

[Kamal Wadhwa]

From: Maulik Shah <maulik.shah@oss.qualcomm.com>

All rpmh_*() APIs so far have supported placing votes for various resource
settings but the H/W also have option to read resource settings.

Add new rpmh_read() API to allow clients to read back resource setting
from H/W. This will be useful for clients like regulators, which currently
don't have a way to know the settings applied during bootloader stage.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
---
 drivers/soc/qcom/rpmh-rsc.c | 13 +++++++++++--
 drivers/soc/qcom/rpmh.c     | 47 +++++++++++++++++++++++++++++++++++++++++----
 include/soc/qcom/rpmh.h     |  5 +++++
 include/soc/qcom/tcs.h      |  2 ++
 4 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index c6f7d5c9c493..ec85c457ea45 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -443,6 +443,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
 	int i;
 	unsigned long irq_status;
 	const struct tcs_request *req;
+	u32 reg;
 
 	irq_status = readl_relaxed(drv->tcs_base + drv->regs[RSC_DRV_IRQ_STATUS]);
 
@@ -453,6 +454,11 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
 
 		trace_rpmh_tx_done(drv, i, req);
 
+		if (req->is_read) {
+			reg = drv->regs[RSC_DRV_CMD_RESP_DATA];
+			req->cmds[0].data = read_tcs_reg(drv, reg, i);
+		}
+
 		/* Clear AMC trigger & enable modes and
 		 * disable interrupt for this TCS
 		 */
@@ -493,13 +499,15 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
 			       const struct tcs_request *msg)
 {
 	u32 msgid;
-	u32 cmd_msgid = CMD_MSGID_LEN | CMD_MSGID_WRITE;
+	u32 cmd_msgid = CMD_MSGID_LEN;
 	u32 cmd_enable = 0;
 	struct tcs_cmd *cmd;
 	int i, j;
 
 	/* Convert all commands to RR when the request has wait_for_compl set */
 	cmd_msgid |= msg->wait_for_compl ? CMD_MSGID_RESP_REQ : 0;
+	if (!msg->is_read)
+		cmd_msgid |= CMD_MSGID_WRITE;
 
 	for (i = 0, j = cmd_id; i < msg->num_cmds; i++, j++) {
 		cmd = &msg->cmds[i];
@@ -513,7 +521,8 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
 
 		write_tcs_cmd(drv, drv->regs[RSC_DRV_CMD_MSGID], tcs_id, j, msgid);
 		write_tcs_cmd(drv, drv->regs[RSC_DRV_CMD_ADDR], tcs_id, j, cmd->addr);
-		write_tcs_cmd(drv, drv->regs[RSC_DRV_CMD_DATA], tcs_id, j, cmd->data);
+		if (!msg->is_read)
+			write_tcs_cmd(drv, drv->regs[RSC_DRV_CMD_DATA], tcs_id, j, cmd->data);
 		trace_rpmh_send_msg(drv, tcs_id, msg->state, j, msgid, cmd);
 	}
 
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index ca37da3dc2b1..f881c4c757ec 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -175,6 +175,9 @@ static int __rpmh_write(const struct device *dev, enum rpmh_state state,
 	struct cache_req *req;
 	int i;
 
+	if (rpm_msg->msg.is_read)
+		goto send_data;
+
 	/* Cache the request in our store and link the payload */
 	for (i = 0; i < rpm_msg->msg.num_cmds; i++) {
 		req = cache_rpm_request(ctrlr, state, &rpm_msg->msg.cmds[i]);
@@ -182,6 +185,7 @@ static int __rpmh_write(const struct device *dev, enum rpmh_state state,
 			return PTR_ERR(req);
 	}
 
+send_data:
 	if (state == RPMH_ACTIVE_ONLY_STATE) {
 		ret = rpmh_rsc_send_data(ctrlr_to_drv(ctrlr), &rpm_msg->msg);
 	} else {
@@ -194,7 +198,7 @@ static int __rpmh_write(const struct device *dev, enum rpmh_state state,
 }
 
 static int __fill_rpmh_msg(struct rpmh_request *req, enum rpmh_state state,
-		const struct tcs_cmd *cmd, u32 n)
+		const struct tcs_cmd *cmd, u32 n, bool is_read)
 {
 	if (!cmd || !n || n > MAX_RPMH_PAYLOAD)
 		return -EINVAL;
@@ -204,10 +208,45 @@ static int __fill_rpmh_msg(struct rpmh_request *req, enum rpmh_state state,
 	req->msg.state = state;
 	req->msg.cmds = req->cmd;
 	req->msg.num_cmds = n;
+	req->msg.is_read = is_read;
 
 	return 0;
 }
 
+/**
+ * rpmh_read: Read a resource value
+ *
+ * @dev: The device making the request
+ * @cmd: The payload having address of resource to read
+ *
+ * Reads the value for the resource address given in tcs_cmd->addr
+ * and returns the tcs_cmd->data filled with same.
+ *
+ * Context: May sleep. Do not call from atomic contexts.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
+{
+	DECLARE_COMPLETION_ONSTACK(compl);
+	DEFINE_RPMH_MSG_ONSTACK(dev, RPMH_ACTIVE_ONLY_STATE, &compl, rpm_msg);
+	int ret;
+
+	ret = __fill_rpmh_msg(&rpm_msg, RPMH_ACTIVE_ONLY_STATE, cmd, 1, true);
+	if (ret)
+		return ret;
+
+	ret = __rpmh_write(dev, RPMH_ACTIVE_ONLY_STATE, &rpm_msg);
+	if (ret)
+		return ret;
+
+	ret = wait_for_completion_timeout(&compl, RPMH_TIMEOUT_MS);
+	cmd[0].data = rpm_msg.cmd[0].data;
+
+	return (ret > 0) ? 0 : -ETIMEDOUT;
+}
+EXPORT_SYMBOL_GPL(rpmh_read);
+
 /**
  * rpmh_write_async: Write a set of RPMH commands
  *
@@ -230,7 +269,7 @@ int rpmh_write_async(const struct device *dev, enum rpmh_state state,
 		return -ENOMEM;
 	rpm_msg->needs_free = true;
 
-	ret = __fill_rpmh_msg(rpm_msg, state, cmd, n);
+	ret = __fill_rpmh_msg(rpm_msg, state, cmd, n, false);
 	if (ret) {
 		kfree(rpm_msg);
 		return ret;
@@ -257,7 +296,7 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
 	DEFINE_RPMH_MSG_ONSTACK(dev, state, &compl, rpm_msg);
 	int ret;
 
-	ret = __fill_rpmh_msg(&rpm_msg, state, cmd, n);
+	ret = __fill_rpmh_msg(&rpm_msg, state, cmd, n, false);
 	if (ret)
 		return ret;
 
@@ -352,7 +391,7 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,
 	rpm_msgs = req->rpm_msgs;
 
 	for (i = 0; i < count; i++) {
-		__fill_rpmh_msg(rpm_msgs + i, state, cmd, n[i]);
+		__fill_rpmh_msg(rpm_msgs + i, state, cmd, n[i], false);
 		cmd += n[i];
 	}
 
diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
index bdbee1a97d36..14ecbf242b6b 100644
--- a/include/soc/qcom/rpmh.h
+++ b/include/soc/qcom/rpmh.h
@@ -11,6 +11,8 @@
 
 
 #if IS_ENABLED(CONFIG_QCOM_RPMH)
+int rpmh_read(const struct device *dev, struct tcs_cmd *cmd);
+
 int rpmh_write(const struct device *dev, enum rpmh_state state,
 	       const struct tcs_cmd *cmd, u32 n);
 
@@ -24,6 +26,9 @@ void rpmh_invalidate(const struct device *dev);
 
 #else
 
+static inline int rpmh_read(const struct device *dev, struct tcs_cmd *cmd)
+{ return -ENODEV; }
+
 static inline int rpmh_write(const struct device *dev, enum rpmh_state state,
 			     const struct tcs_cmd *cmd, u32 n)
 { return -ENODEV; }
diff --git a/include/soc/qcom/tcs.h b/include/soc/qcom/tcs.h
index cff67ce25488..45b8513be2f9 100644
--- a/include/soc/qcom/tcs.h
+++ b/include/soc/qcom/tcs.h
@@ -51,6 +51,7 @@ struct tcs_cmd {
  * struct tcs_request: A set of tcs_cmds sent together in a TCS
  *
  * @state:          state for the request.
+ * @is_read:        set for read only requests
  * @wait_for_compl: wait until we get a response from the h/w accelerator
  *                  (same as setting cmd->wait for all commands in the request)
  * @num_cmds:       the number of @cmds in this request
@@ -58,6 +59,7 @@ struct tcs_cmd {
  */
 struct tcs_request {
 	enum rpmh_state state;
+	bool is_read;
 	u32 wait_for_compl;
 	u32 num_cmds;
 	struct tcs_cmd *cmds;

-- 
2.43.0

[PATCH v5 2/4] regulator: qcom-rpmh: Fix PMIC5 BOB bypass mode handling

[Kamal Wadhwa]

Currently, when `rpmh_regulator_set_mode_bypass()` helper function
is called to set bypass mode, it sends PMIC4's BOB bypass mode
value for even if its a PMIC5 BOB.

To fix this, introduce new hw_data parameter`pmic_bypass_mode`
to store bypass mode value. Use it to send correct PMIC bypass
mode value that corresponds to PMIC4/5 BOB regulators from the
helper function.

Fixes: 610f29e5cc0e8d58 ("regulator: qcom-rpmh: Update PMIC modes for PMIC5")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
---
 drivers/regulator/qcom-rpmh-regulator.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
index 756a4201225e..c47dc9f92279 100644
--- a/drivers/regulator/qcom-rpmh-regulator.c
+++ b/drivers/regulator/qcom-rpmh-regulator.c
@@ -111,6 +111,7 @@ static const struct resource_name_formats vreg_rsc_name_lookup[NUM_REGULATOR_TYP
  * @hpm_min_load_uA:		Minimum load current in microamps that requires
  *				high power mode (HPM) operation.  This is used
  *				for LDO hardware type regulators only.
+ * @pmic_bypass_mode:		The PMIC bypass mode value.
  * @pmic_mode_map:		Array indexed by regulator framework mode
  *				containing PMIC hardware modes.  Must be large
  *				enough to index all framework modes supported
@@ -125,6 +126,7 @@ struct rpmh_vreg_hw_data {
 	int					n_linear_ranges;
 	int					n_voltages;
 	int					hpm_min_load_uA;
+	int					pmic_bypass_mode;
 	const int				*pmic_mode_map;
 	unsigned int			      (*of_map_mode)(unsigned int mode);
 };
@@ -311,7 +313,7 @@ static int rpmh_regulator_vrm_set_mode_bypass(struct rpmh_vreg *vreg,
 		return pmic_mode;
 
 	if (bypassed)
-		cmd.data = PMIC4_BOB_MODE_PASS;
+		cmd.data = vreg->hw_data->pmic_bypass_mode;
 	else
 		cmd.data = pmic_mode;
 
@@ -767,6 +769,7 @@ static const struct rpmh_vreg_hw_data pmic4_bob = {
 	},
 	.n_linear_ranges = 1,
 	.n_voltages = 84,
+	.pmic_bypass_mode = PMIC4_BOB_MODE_PASS,
 	.pmic_mode_map = pmic_mode_map_pmic4_bob,
 	.of_map_mode = rpmh_regulator_pmic4_bob_of_map_mode,
 };
@@ -975,6 +978,7 @@ static const struct rpmh_vreg_hw_data pmic5_bob = {
 	},
 	.n_linear_ranges = 1,
 	.n_voltages = 32,
+	.pmic_bypass_mode = PMIC5_BOB_MODE_PASS,
 	.pmic_mode_map = pmic_mode_map_pmic5_bob,
 	.of_map_mode = rpmh_regulator_pmic4_bob_of_map_mode,
 };

-- 
2.43.0

[PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Kamal Wadhwa]

Currently, during regulator registration, regulator framework sends an
unnecessary `min-microvolts` request for the rpmh-regulator device. This
happens because in current design, we do not have a way to readback the
voltage settings that was set during the bootloader stage.

Fix this by using the rpmh_read() API to read the regulator voltage
settings done during boot and make it available to regulator framework
from the very first read after the bootup.

Also use this API to read the status/mode/bypass settings as well. This
will provide the regulator framework a sense of the initial settings
done by bootloader and thus preventing any redundents writes for any
setting post bootup incase the same setting was already applied during
bootup.

Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
---
 drivers/regulator/qcom-rpmh-regulator.c | 161 ++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
index c47dc9f92279..56a51f8893b6 100644
--- a/drivers/regulator/qcom-rpmh-regulator.c
+++ b/drivers/regulator/qcom-rpmh-regulator.c
@@ -4,6 +4,7 @@
 
 #define pr_fmt(fmt) "%s: " fmt, __func__
 
+#include <linux/bits.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -61,8 +62,13 @@ static const struct resource_name_formats vreg_rsc_name_lookup[NUM_REGULATOR_TYP
 };
 
 #define RPMH_REGULATOR_REG_VRM_VOLTAGE		0x0
+#define RPMH_REGULATOR_VOLTAGE_MASK		GENMASK(12, 0)
+
 #define RPMH_REGULATOR_REG_ENABLE		0x4
+#define RPMH_REGULATOR_ENABLE_MASK		BIT(0)
+
 #define RPMH_REGULATOR_REG_VRM_MODE		0x8
+#define RPMH_REGULATOR_MODE_MASK		GENMASK(2, 0)
 
 #define PMIC4_LDO_MODE_RETENTION		4
 #define PMIC4_LDO_MODE_LPM			5
@@ -154,6 +160,7 @@ struct rpmh_vreg_hw_data {
  * @voltage_selector:		Selector used for get_voltage_sel() and
  *				set_voltage_sel() callbacks
  * @mode:			RPMh VRM regulator current framework mode
+ * @status:			RPMh VRM regulator current framework status
  */
 struct rpmh_vreg {
 	struct device			*dev;
@@ -166,6 +173,7 @@ struct rpmh_vreg {
 	bool				bypassed;
 	int				voltage_selector;
 	unsigned int			mode;
+	unsigned int			status;
 };
 
 /**
@@ -210,6 +218,16 @@ static int rpmh_regulator_send_request(struct rpmh_vreg *vreg,
 	return ret;
 }
 
+static void rpmh_vreg_update_status(struct rpmh_vreg *vreg)
+{
+	if (!vreg->enabled)
+		vreg->status = REGULATOR_STATUS_OFF;
+	else if (vreg->bypassed)
+		vreg->status = REGULATOR_STATUS_BYPASS;
+	else
+		vreg->status = regulator_mode_to_status(vreg->mode);
+}
+
 static int _rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
 				unsigned int selector, bool wait_for_ack)
 {
@@ -248,9 +266,34 @@ static int rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
 					selector > vreg->voltage_selector);
 }
 
+static int _rpmh_regulator_vrm_get_voltage(struct regulator_dev *rdev, int *uV)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_VRM_VOLTAGE,
+	};
+	int ret;
+
+	ret = rpmh_read(vreg->dev, &cmd);
+	if (!ret)
+		*uV = (cmd.data & RPMH_REGULATOR_VOLTAGE_MASK) * 1000;
+	else
+		dev_err(vreg->dev, "failed to read VOLTAGE ret = %d\n", ret);
+
+	return ret;
+}
+
 static int rpmh_regulator_vrm_get_voltage_sel(struct regulator_dev *rdev)
 {
 	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int ret, uV = 0;
+
+	if (vreg->voltage_selector < 0) {
+		ret = _rpmh_regulator_vrm_get_voltage(rdev, &uV);
+		if (!ret && uV != 0)
+			vreg->voltage_selector = regulator_map_voltage_linear_range(rdev,
+							uV, INT_MAX);
+	}
 
 	return vreg->voltage_selector;
 }
@@ -284,6 +327,8 @@ static int rpmh_regulator_set_enable_state(struct regulator_dev *rdev,
 	if (!ret)
 		vreg->enabled = enable;
 
+	rpmh_vreg_update_status(vreg);
+
 	return ret;
 }
 
@@ -333,9 +378,27 @@ static int rpmh_regulator_vrm_set_mode(struct regulator_dev *rdev,
 	if (!ret)
 		vreg->mode = mode;
 
+	rpmh_vreg_update_status(vreg);
+
 	return ret;
 }
 
+static int rpmh_regulator_vrm_get_pmic_mode(struct rpmh_vreg *vreg, int *pmic_mode)
+{
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_VRM_MODE,
+	};
+	int ret;
+
+	ret = rpmh_read(vreg->dev, &cmd);
+	if (!ret)
+		*pmic_mode = cmd.data & RPMH_REGULATOR_MODE_MASK;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
 static unsigned int rpmh_regulator_vrm_get_mode(struct regulator_dev *rdev)
 {
 	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
@@ -343,6 +406,13 @@ static unsigned int rpmh_regulator_vrm_get_mode(struct regulator_dev *rdev)
 	return vreg->mode;
 }
 
+static int rpmh_regulator_vrm_get_status(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->status;
+}
+
 /**
  * rpmh_regulator_vrm_get_optimum_mode() - get the mode based on the  load
  * @rdev:		Regulator device pointer for the rpmh-regulator
@@ -379,6 +449,8 @@ static int rpmh_regulator_vrm_set_bypass(struct regulator_dev *rdev,
 	if (!ret)
 		vreg->bypassed = enable;
 
+	rpmh_vreg_update_status(vreg);
+
 	return ret;
 }
 
@@ -401,6 +473,7 @@ static const struct regulator_ops rpmh_regulator_vrm_ops = {
 	.list_voltage		= regulator_list_voltage_linear_range,
 	.set_mode		= rpmh_regulator_vrm_set_mode,
 	.get_mode		= rpmh_regulator_vrm_get_mode,
+	.get_status		= rpmh_regulator_vrm_get_status,
 };
 
 static const struct regulator_ops rpmh_regulator_vrm_drms_ops = {
@@ -412,6 +485,7 @@ static const struct regulator_ops rpmh_regulator_vrm_drms_ops = {
 	.list_voltage		= regulator_list_voltage_linear_range,
 	.set_mode		= rpmh_regulator_vrm_set_mode,
 	.get_mode		= rpmh_regulator_vrm_get_mode,
+	.get_status		= rpmh_regulator_vrm_get_status,
 	.get_optimum_mode	= rpmh_regulator_vrm_get_optimum_mode,
 };
 
@@ -424,6 +498,7 @@ static const struct regulator_ops rpmh_regulator_vrm_bypass_ops = {
 	.list_voltage		= regulator_list_voltage_linear_range,
 	.set_mode		= rpmh_regulator_vrm_set_mode,
 	.get_mode		= rpmh_regulator_vrm_get_mode,
+	.get_status		= rpmh_regulator_vrm_get_status,
 	.set_bypass		= rpmh_regulator_vrm_set_bypass,
 	.get_bypass		= rpmh_regulator_vrm_get_bypass,
 };
@@ -432,6 +507,7 @@ static const struct regulator_ops rpmh_regulator_xob_ops = {
 	.enable			= rpmh_regulator_enable,
 	.disable		= rpmh_regulator_disable,
 	.is_enabled		= rpmh_regulator_is_enabled,
+	.get_status		= rpmh_regulator_vrm_get_status,
 };
 
 /**
@@ -540,6 +616,85 @@ static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
 	return 0;
 }
 
+static int rpmh_regulator_determine_initial_status(struct rpmh_vreg *vreg)
+{
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
+	};
+	int ret, pmic_mode, mode;
+	int sts;
+
+	ret = rpmh_read(vreg->dev, &cmd);
+	if (ret) {
+		vreg->status = REGULATOR_STATUS_UNDEFINED;
+		dev_err(vreg->dev, "failed to read ENABLE status ret = %d\n", ret);
+
+		return ret;
+	}
+
+	sts = cmd.data & RPMH_REGULATOR_ENABLE_MASK;
+	if (!sts) {
+		vreg->status = REGULATOR_STATUS_OFF;
+
+		return 0;
+	}
+
+	if (vreg->hw_data->regulator_type == XOB) {
+		vreg->status = REGULATOR_STATUS_ON;
+
+		return 0;
+	}
+
+	ret = rpmh_regulator_vrm_get_pmic_mode(vreg, &pmic_mode);
+	if (ret < 0) {
+		vreg->mode = REGULATOR_MODE_INVALID;
+		vreg->status = REGULATOR_STATUS_UNDEFINED;
+		dev_err(vreg->dev, "failed to read pmic_mode ret = %d\n", ret);
+
+		return ret;
+	}
+
+	/*
+	 * NOTE: Since BOB4 BYPASS_MODE value = 0 we cannot confirm if that BOB
+	 * regulator has been sent into bypass mode by bootloader or if bootloader
+	 * just has not requested for any mode voting. Due this limitation, we
+	 * must check if the read pmic_mode value is non-zero before comparing it
+	 * to bypass mode value. This also is needed to avoid setting BYPASS status
+	 * for LDOs which dont support bypass mode, and have the pmic_bypass_mode
+	 * uninitialized value as zero in the vreg hw data. For such cases assume
+	 * lowest mode/status, if pmic_mode is zero, to allow for mode voting.
+	 */
+	if (!pmic_mode) {
+		for (mode = REGULATOR_MODE_STANDBY; mode > REGULATOR_MODE_INVALID; mode >>= 1) {
+			if (vreg->hw_data->pmic_mode_map[mode] >= 0) {
+				vreg->mode = mode;
+				break;
+			}
+		}
+
+		vreg->status = regulator_mode_to_status(vreg->mode);
+
+		return 0;
+	}
+
+	if (vreg->hw_data->pmic_bypass_mode == pmic_mode) {
+		vreg->bypassed = true;
+		vreg->status = REGULATOR_STATUS_BYPASS;
+
+		return 0;
+	}
+
+	for (mode = REGULATOR_MODE_STANDBY; mode > REGULATOR_MODE_INVALID; mode >>= 1) {
+		if (pmic_mode == vreg->hw_data->pmic_mode_map[mode]) {
+			vreg->mode = mode;
+			vreg->status = regulator_mode_to_status(vreg->mode);
+			break;
+		}
+	}
+
+	return 0;
+}
+
 static const int pmic_mode_map_pmic4_ldo[REGULATOR_MODE_STANDBY + 1] = {
 	[REGULATOR_MODE_INVALID] = -EINVAL,
 	[REGULATOR_MODE_STANDBY] = PMIC4_LDO_MODE_RETENTION,
@@ -1838,6 +1993,12 @@ static int rpmh_regulator_probe(struct platform_device *pdev)
 						vreg_data);
 		if (ret < 0)
 			return ret;
+
+		ret = rpmh_regulator_determine_initial_status(vreg);
+		if (ret < 0)
+			dev_err(dev, "failed to read initial status for %s\n",
+					vreg->rdesc.name);
+
 	}
 
 	return 0;

-- 
2.43.0

[PATCH v5 4/4] regulator: qcom-rpmh: Fix coding style issues

[Kamal Wadhwa]

Fix the code style/format issues reported by checkpatch.pl
script.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
---
 drivers/regulator/qcom-rpmh-regulator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
index 56a51f8893b6..1845c865c735 100644
--- a/drivers/regulator/qcom-rpmh-regulator.c
+++ b/drivers/regulator/qcom-rpmh-regulator.c
@@ -110,7 +110,7 @@ static const struct resource_name_formats vreg_rsc_name_lookup[NUM_REGULATOR_TYP
  *				regulator
  * @ops:			Pointer to regulator ops callback structure
  * @voltage_ranges:		The possible ranges of voltages supported by this
- * 				PMIC regulator type
+ *				PMIC regulator type
  * @n_linear_ranges:		Number of entries in voltage_ranges
  * @n_voltages:			The number of unique voltage set points defined
  *				by voltage_ranges

-- 
2.43.0

Re: [PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Mark Brown]

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

On Mon, Jul 20, 2026 at 09:06:40PM +0530, Kamal Wadhwa wrote:

> Currently, during regulator registration, regulator framework sends an
> unnecessary `min-microvolts` request for the rpmh-regulator device. This
> happens because in current design, we do not have a way to readback the
> voltage settings that was set during the bootloader stage.

> +static int rpmh_regulator_vrm_get_status(struct regulator_dev *rdev)
> +{
> +	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
> +
> +	return vreg->status;
> +}

A get_status() operation should be reading the actual hardware status
right now, not driver state - this should be reading whatever the value
is right now.  If the hardware doesn't support this then just don't
provide the operation.

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

Re: [PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Kamal Wadhwa]

On Wed, Jul 22, 2026 at 05:46:37PM +0100, Mark Brown wrote:
> On Mon, Jul 20, 2026 at 09:06:40PM +0530, Kamal Wadhwa wrote:
> 
> > Currently, during regulator registration, regulator framework sends an
> > unnecessary `min-microvolts` request for the rpmh-regulator device. This
> > happens because in current design, we do not have a way to readback the
> > voltage settings that was set during the bootloader stage.
> 
> > +static int rpmh_regulator_vrm_get_status(struct regulator_dev *rdev)
> > +{
> > +	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
> > +
> > +	return vreg->status;
> > +}
> 
> A get_status() operation should be reading the actual hardware status
> right now, not driver state - this should be reading whatever the value
> is right now.  If the hardware doesn't support this then just don't
> provide the operation.

Actually we are reading from HW what we voted in terms of ENABLE/BYPASS/MODE
and we are combing these in the `rpmh_regulator_determine_initial_status()`
to come up with the `vreg->status`at the time of probe.

but later for every disable/enable/mode write we keep updating the
`vreg->status` as an optimization.

So, shall I read the HW MODE/BYPSS/ENABLE value in every get_status() call?
 OR
just remove the get_status() call and all the logic to calculate it based
MODE/ENABLE/BYPASS hw values in probe as well?

Regards,
Kamal

Re: [PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Mark Brown]

On Fri, Jul 24, 2026 at 04:13:02PM +0530, Kamal Wadhwa wrote:
> On Wed, Jul 22, 2026 at 05:46:37PM +0100, Mark Brown wrote:

> > A get_status() operation should be reading the actual hardware status
> > right now, not driver state - this should be reading whatever the value
> > is right now.  If the hardware doesn't support this then just don't
> > provide the operation.

> Actually we are reading from HW what we voted in terms of ENABLE/BYPASS/MODE
> and we are combing these in the `rpmh_regulator_determine_initial_status()`
> to come up with the `vreg->status`at the time of probe.

No, that's not the point.  A get_status() operation needs to return the
actual hardware status *now*, not a cached value from some random time
in the past and not something that was written by the driver.

> So, shall I read the HW MODE/BYPSS/ENABLE value in every get_status() call?

If they reflect values that might be dynamically updated by the
hardware, yes.  If they are just control registers that the hardware
will not update autonomously they are not suitable for a get_status()
operation.

Re: [PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Kamal Wadhwa]

On Fri, Jul 24, 2026 at 01:48:30PM +0100, Mark Brown wrote:
> On Fri, Jul 24, 2026 at 04:13:02PM +0530, Kamal Wadhwa wrote:
> > On Wed, Jul 22, 2026 at 05:46:37PM +0100, Mark Brown wrote:
> 
> > > A get_status() operation should be reading the actual hardware status
> > > right now, not driver state - this should be reading whatever the value
> > > is right now.  If the hardware doesn't support this then just don't
> > > provide the operation.
> 
> > Actually we are reading from HW what we voted in terms of ENABLE/BYPASS/MODE
> > and we are combing these in the `rpmh_regulator_determine_initial_status()`
> > to come up with the `vreg->status`at the time of probe.
> 
> No, that's not the point.  A get_status() operation needs to return the
> actual hardware status *now*, not a cached value from some random time
> in the past and not something that was written by the driver.

I can drop it. But I seek your guidance on how should i implement logic
to HOLD the enable/voltage/mode for the regulators which are turned ON by
bootloader.

we need this for shared regulators, to avoid a race between clients
probes to badly impact a client which needed the rail to hold its
mode/voltage/enable state before its probe is called.

I was planning to use get_status() (in a subsequent patch series) to check if
the rail was left ON by bootloader at the regulator registration stage, and
use that criteria to decide if we need to hold the voltage/mode/enable state
or not.

Besides this, since the sync_state() cannot be used.. i was thinking if holding
the voltage/mode/enable till regulator_late_cleanup(). is that ok?

Would really appriciate your help, if you can guide me in some direction..
Thanks in advance!

> 
> > So, shall I read the HW MODE/BYPSS/ENABLE value in every get_status() call?
> 
> If they reflect values that might be dynamically updated by the
> hardware, yes.  If they are just control registers that the hardware
> will not update autonomously they are not suitable for a get_status()
> operation.

It not will autonomously update, but it may be updated by the APPS bootloader.
I guess, that still would make it unsuitable to be used in get_status() ?

Regards,
Kamal

Re: [PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Mark Brown]

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

On Thu, Jul 30, 2026 at 01:11:36AM +0530, Kamal Wadhwa wrote:
> On Fri, Jul 24, 2026 at 01:48:30PM +0100, Mark Brown wrote:

> > No, that's not the point.  A get_status() operation needs to return the
> > actual hardware status *now*, not a cached value from some random time
> > in the past and not something that was written by the driver.

> I can drop it. But I seek your guidance on how should i implement logic
> to HOLD the enable/voltage/mode for the regulators which are turned ON by
> bootloader.

You could cache the values currently written to the control interface?

> I was planning to use get_status() (in a subsequent patch series) to check if
> the rail was left ON by bootloader at the regulator registration stage, and
> use that criteria to decide if we need to hold the voltage/mode/enable state
> or not.

That's logic that should be implemented in the core rather than driver
specific, this is an issue that affects everything - we just mostly get
away with it since we don't start touching things until drivers do so.
It's a real problem, just tough to solve.

> Besides this, since the sync_state() cannot be used.. i was thinking if holding
> the voltage/mode/enable till regulator_late_cleanup(). is that ok?

Consider what happens if something really needs a voltage raising to
enumerate - if you ignore it then the hardware might be in for a bad
time.

> > > So, shall I read the HW MODE/BYPSS/ENABLE value in every get_status() call?

> > If they reflect values that might be dynamically updated by the
> > hardware, yes.  If they are just control registers that the hardware
> > will not update autonomously they are not suitable for a get_status()
> > operation.

> It not will autonomously update, but it may be updated by the APPS bootloader.
> I guess, that still would make it unsuitable to be used in get_status() ?

Will the bootloader be running at the same time as the kernel, I really
mean "by something else" rather than "autonomously" - usually that's the
device itself (eg, reporting that a regulator was shut down due to over
current) but it could be something else I guess.

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

Re: [PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Kamal Wadhwa]

On Wed, Jul 29, 2026 at 09:04:47PM +0100, Mark Brown wrote:
> On Thu, Jul 30, 2026 at 01:11:36AM +0530, Kamal Wadhwa wrote:
> > On Fri, Jul 24, 2026 at 01:48:30PM +0100, Mark Brown wrote:
> 
> > > No, that's not the point.  A get_status() operation needs to return the
> > > actual hardware status *now*, not a cached value from some random time
> > > in the past and not something that was written by the driver.
> 
> > I can drop it. But I seek your guidance on how should i implement logic
> > to HOLD the enable/voltage/mode for the regulators which are turned ON by
> > bootloader.
> 
> You could cache the values currently written to the control interface?

We can cache it, but i was planning to add change in core,
which will require get_status() or something similar i think.

> 
> > I was planning to use get_status() (in a subsequent patch series) to check if
> > the rail was left ON by bootloader at the regulator registration stage, and
> > use that criteria to decide if we need to hold the voltage/mode/enable state
> > or not.
> 
> That's logic that should be implemented in the core rather than driver
> specific, this is an issue that affects everything - we just mostly get
> away with it since we don't start touching things until drivers do so.
> It's a real problem, just tough to solve.

yes yes, it needs to be fixed in core.

> 
> > Besides this, since the sync_state() cannot be used.. i was thinking if holding
> > the voltage/mode/enable till regulator_late_cleanup(). is that ok?
> 
> Consider what happens if something really needs a voltage raising to
> enumerate - if you ignore it then the hardware might be in for a bad
> time.

May be we can allow the up-ing of voltage/mode to go through? but cache the
lowering requests for voltage/mode and apply them in regulator_late_cleanup()
so we are sure that there is more then sufficient power during this sensitive
phase. will that work?

> 
> > > > So, shall I read the HW MODE/BYPSS/ENABLE value in every get_status() call?
> 
> > > If they reflect values that might be dynamically updated by the
> > > hardware, yes.  If they are just control registers that the hardware
> > > will not update autonomously they are not suitable for a get_status()
> > > operation.
> 
> > It not will autonomously update, but it may be updated by the APPS bootloader.
> > I guess, that still would make it unsuitable to be used in get_status() ?
> 
> Will the bootloader be running at the same time as the kernel, I really
> mean "by something else" rather than "autonomously" - usually that's the
> device itself (eg, reporting that a regulator was shut down due to over
> current) but it could be something else I guess.

Ok, so is there something similar to get_status() that i can use to pass on the
info to core (from the driver) that regulator state was ON at bootloader stage?

Regards,
Kamal

Re: [PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Mark Brown]

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

On Thu, Jul 30, 2026 at 03:28:39AM +0530, Kamal Wadhwa wrote:
> On Wed, Jul 29, 2026 at 09:04:47PM +0100, Mark Brown wrote:
> > On Thu, Jul 30, 2026 at 01:11:36AM +0530, Kamal Wadhwa wrote:

> > > I can drop it. But I seek your guidance on how should i implement logic
> > > to HOLD the enable/voltage/mode for the regulators which are turned ON by
> > > bootloader.

> > You could cache the values currently written to the control interface?

> We can cache it, but i was planning to add change in core,
> which will require get_status() or something similar i think.

I would have thought we could use the currently configured state here.

> > > Besides this, since the sync_state() cannot be used.. i was thinking if holding
> > > the voltage/mode/enable till regulator_late_cleanup(). is that ok?

> > Consider what happens if something really needs a voltage raising to
> > enumerate - if you ignore it then the hardware might be in for a bad
> > time.

> May be we can allow the up-ing of voltage/mode to go through? but cache the
> lowering requests for voltage/mode and apply them in regulator_late_cleanup()
> so we are sure that there is more then sufficient power during this sensitive
> phase. will that work?

It's certainly much less likely to be an issue.  There's some similar
issues around enables for exclusive regulatorss though, and I wouldn't
discount lower voltages being an issue (off the top of my head I've got
a feeling MMC needs to lower voltages to up clock rates but I could be
misremembering the use case).  Whole thing is a massive can of worms,
that's why it's not been addressed :/

> > Will the bootloader be running at the same time as the kernel, I really
> > mean "by something else" rather than "autonomously" - usually that's the
> > device itself (eg, reporting that a regulator was shut down due to over
> > current) but it could be something else I guess.

> Ok, so is there something similar to get_status() that i can use to pass on the
> info to core (from the driver) that regulator state was ON at bootloader stage?

is_enabled() would be the usual thing.

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

Re: [PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Kamal Wadhwa]

On Thu, Jul 30, 2026 at 12:11:26AM +0100, Mark Brown wrote:
> On Thu, Jul 30, 2026 at 03:28:39AM +0530, Kamal Wadhwa wrote:
> > On Wed, Jul 29, 2026 at 09:04:47PM +0100, Mark Brown wrote:
> > > On Thu, Jul 30, 2026 at 01:11:36AM +0530, Kamal Wadhwa wrote:
> 
> > > > I can drop it. But I seek your guidance on how should i implement logic
> > > > to HOLD the enable/voltage/mode for the regulators which are turned ON by
> > > > bootloader.
> 
> > > You could cache the values currently written to the control interface?
> 
> > We can cache it, but i was planning to add change in core,
> > which will require get_status() or something similar i think.
> 
> I would have thought we could use the currently configured state here.
> 
> > > > Besides this, since the sync_state() cannot be used.. i was thinking if holding
> > > > the voltage/mode/enable till regulator_late_cleanup(). is that ok?
> 
> > > Consider what happens if something really needs a voltage raising to
> > > enumerate - if you ignore it then the hardware might be in for a bad
> > > time.
> 
> > May be we can allow the up-ing of voltage/mode to go through? but cache the
> > lowering requests for voltage/mode and apply them in regulator_late_cleanup()
> > so we are sure that there is more then sufficient power during this sensitive
> > phase. will that work?
> 
> It's certainly much less likely to be an issue.  There's some similar
> issues around enables for exclusive regulatorss though, and I wouldn't
> discount lower voltages being an issue (off the top of my head I've got
> a feeling MMC needs to lower voltages to up clock rates but I could be
> misremembering the use case).  Whole thing is a massive can of worms,
> that's why it's not been addressed :/

Do you think allowing this behaviour via a new regulator DT prop like
`regulator-hold-on-boot` to allow per-regulator control.. rather then applying to
all the regulators which are ON at boot.. may help address some of those concerns?

IMHO, from the design point of view it has to be either allowing all the requests
OR allow only the up-ing of the voltage/mode till all clients are up, may be needed.

> 
> > > Will the bootloader be running at the same time as the kernel, I really
> > > mean "by something else" rather than "autonomously" - usually that's the
> > > device itself (eg, reporting that a regulator was shut down due to over
> > > current) but it could be something else I guess.
> 
> > Ok, so is there something similar to get_status() that i can use to pass on the
> > info to core (from the driver) that regulator state was ON at bootloader stage?
> 
> is_enabled() would be the usual thing.

thanks, i'll use that that then.

Re: [PATCH v5 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode/status set during bootup

[Mark Brown]

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

On Thu, Jul 30, 2026 at 01:40:05PM +0530, Kamal Wadhwa wrote:
> On Thu, Jul 30, 2026 at 12:11:26AM +0100, Mark Brown wrote:

> > It's certainly much less likely to be an issue.  There's some similar
> > issues around enables for exclusive regulatorss though, and I wouldn't
> > discount lower voltages being an issue (off the top of my head I've got
> > a feeling MMC needs to lower voltages to up clock rates but I could be
> > misremembering the use case).  Whole thing is a massive can of worms,
> > that's why it's not been addressed :/

> Do you think allowing this behaviour via a new regulator DT prop like
> `regulator-hold-on-boot` to allow per-regulator control.. rather then applying to
> all the regulators which are ON at boot.. may help address some of those concerns?

Possibly, though it's all feeling very implementation and not really DT.

> IMHO, from the design point of view it has to be either allowing all the requests
> OR allow only the up-ing of the voltage/mode till all clients are up, may be needed.

Like I say I'm not sure anything survives special cases well.

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