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

[PATCH v6 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
readback of 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 v6:
- Dropped the `get_status()` callback and all associated status-tracking
  (vreg->status, rpmh_vreg_update_status(), rpmh_regulator_vrm_get_status()),
  as Mark pointed out get_status() must reflect actual hardware state, not a
  cached/driver-computed value, and the ENABLE/MODE/BYPASS registers are
  control/vote registers written by this driver rather than free-running
  status registers. Renamed determine_initial_status() to
  determine_initial_mode() accordingly; mode/bypass/voltage readback via
  get_mode()/get_bypass()/get_voltage_sel() is unaffected.
- Link to v5: https://patch.msgid.link/20260720-b4-read-rpmh-v5-v5-0-cb93fb95f565@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 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 | 128 +++++++++++++++++++++++++++++++-
 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, 187 insertions(+), 8 deletions(-)
---
base-commit: 5c73cd9f0819c1c44e373e3dabb68318b1de1a12
change-id: 20260720-b4-read-rpmh-v5-bab1bd1cbe55

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

[PATCH v6 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 v6 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 v6 3/4] regulator: qcom-rpmh: readback voltage/bypass/mode 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 mode/bypass settings as well. This will
provide the regulator framework a sense of the initial settings done by
bootloader and thus preventing any redundant 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 | 120 ++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
index c47dc9f92279..77d402ee6bf6 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
@@ -248,9 +254,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;
 }
@@ -336,6 +367,22 @@ static int rpmh_regulator_vrm_set_mode(struct regulator_dev *rdev,
 	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);
@@ -540,6 +587,73 @@ static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
 	return 0;
 }
 
+static int rpmh_regulator_determine_initial_mode(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) {
+		dev_err(vreg->dev, "failed to read ENABLE status ret = %d\n", ret);
+
+		return ret;
+	}
+
+	sts = cmd.data & RPMH_REGULATOR_ENABLE_MASK;
+	if (!sts)
+		return 0;
+
+	if (vreg->hw_data->regulator_type == XOB)
+		return 0;
+
+	ret = rpmh_regulator_vrm_get_pmic_mode(vreg, &pmic_mode);
+	if (ret < 0) {
+		vreg->mode = REGULATOR_MODE_INVALID;
+		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, 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;
+			}
+		}
+
+		return 0;
+	}
+
+	if (vreg->hw_data->pmic_bypass_mode == pmic_mode) {
+		vreg->bypassed = true;
+
+		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;
+			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 +1952,12 @@ static int rpmh_regulator_probe(struct platform_device *pdev)
 						vreg_data);
 		if (ret < 0)
 			return ret;
+
+		ret = rpmh_regulator_determine_initial_mode(vreg);
+		if (ret < 0)
+			dev_err(dev, "failed to read initial mode for %s\n",
+					vreg->rdesc.name);
+
 	}
 
 	return 0;

-- 
2.43.0

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

[Konrad Dybcio]

On 8/1/26 10:00 AM, 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.
> 
> 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 mode/bypass settings as well. This will
> provide the regulator framework a sense of the initial settings done by
> bootloader and thus preventing any redundant 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>
> ---

SC8180X Primus hangs with this patch applied and so does SM8150 HDK.

Interestingly, SDM845 DB845c doesn't

I think we may need to opt some platforms out of this..

Konrad

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

[Kamal Wadhwa]

On Tue, Sep 01, 2026 at 11:49:42AM +0200, Konrad Dybcio wrote:
> On 8/1/26 10:00 AM, 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.
> > 
> > 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 mode/bypass settings as well. This will
> > provide the regulator framework a sense of the initial settings done by
> > bootloader and thus preventing any redundant 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>
> > ---
> 
> SC8180X Primus hangs with this patch applied and so does SM8150 HDK.

Ok, i did see some issues on SM8550 but those were mainly related to the
voltage range check leading to some regulator failing to probe, for that
this below change may help.
https://lore.kernel.org/all/20260720-b4-regulator-core-clamp-voltage-v1-1-8e5eec076a8e@oss.qualcomm.com/

But i suppose you may already have it?

do you see the problem with the rpmh_read() or in the voltage range check?

> 
> Interestingly, SDM845 DB845c doesn't
> 
> I think we may need to opt some platforms out of this..
> 
> Konrad

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

[Konrad Dybcio]

On 9/7/26 2:59 PM, Kamal Wadhwa wrote:
> On Tue, Sep 01, 2026 at 11:49:42AM +0200, Konrad Dybcio wrote:
>> On 8/1/26 10:00 AM, 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.
>>>
>>> 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 mode/bypass settings as well. This will
>>> provide the regulator framework a sense of the initial settings done by
>>> bootloader and thus preventing any redundant 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>
>>> ---
>>
>> SC8180X Primus hangs with this patch applied and so does SM8150 HDK.
> 
> Ok, i did see some issues on SM8550 but those were mainly related to the
> voltage range check leading to some regulator failing to probe, for that
> this below change may help.
> https://lore.kernel.org/all/20260720-b4-regulator-core-clamp-voltage-v1-1-8e5eec076a8e@oss.qualcomm.com/
> 
> But i suppose you may already have it?

Yes


> do you see the problem with the rpmh_read() or in the voltage range check?

Applying

20260812-rpmh-timeout-debug-v1-v3-0-68c0a40dce23@oss.qualcomm.com

I get:

[   12.735771] qcom_rpmh RSC:apps_rsc
[   12.739294] qcom_rpmh Request: tcs-in-use:YES state=2 wait_for_compl=1
[   12.749622] qcom_rpmh TCS=0 [ctrlr-sts:BUSY amc-mode:0x1010000 irq-sts:WAITING]
[   12.763769] qcom_rpmh        CMD=0 [addr=0x43100(VRM/smpc8) data=0x7 resp-required sts=triggered+sent-to-aoss]
[   12.781024] qcom_rpmh Request: tcs-in-use:YES state=2 wait_for_compl=1
[   12.797493] qcom_rpmh TCS=1 [ctrlr-sts:BUSY amc-mode:0x1010000 irq-sts:WAITING]
[   12.811733] qcom_rpmh        CMD=0 [addr=0x40000(VRM/smpa5) data=0x0 resp-required sts=triggered+sent-to-aoss]
[   12.829006] qcom_rpmh HW IRQ 37 is NOT PENDING at GIC
[   12.843942] qcom_rpmh Completion is not done
[   12.853536] qcom_rpmh ERROR: Accelerator(s) at AOSS did not respond

Neither S8C nor S5A are "special", neither of them feeds an
ARC resource (at a glance, anyway)

so it's the read part,

RPMH_REGULATOR_REG_VRM_VOLTAGE and RPMH_REGULATOR_REG_VRM_MODE stall

RPMH_REGULATOR_REG_ENABLE interestingly doesn't

Konrad

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

[Dmitry Baryshkov]

On Tue, Sep 01, 2026 at 11:49:42AM +0200, Konrad Dybcio wrote:
> On 8/1/26 10:00 AM, 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.
> > 
> > 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 mode/bypass settings as well. This will
> > provide the regulator framework a sense of the initial settings done by
> > bootloader and thus preventing any redundant 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>
> > ---
> 
> SC8180X Primus hangs with this patch applied and so does SM8150 HDK.

[    4.015947] cpu cpu0: EM: created perf domain
[   14.216480] qcom-rpmh-regulator 18200000.rsc:regulators-1: failed to read VOLTAGE ret = -110
[   14.216731] ------------[ cut here ]------------
[   14.225298] ldo1: Setting 1800000-1800000uV
[   14.230090] WARNING: drivers/soc/qcom/rpmh.c:441 at rpmh_write_batch+0x190/0x2b0, CPU#3: sugov:0/113
[   14.230135] Modules linked in:
[   14.247173] CPU: 3 UID: 0 PID: 113 Comm: sugov:0 Not tainted 7.3.0-rc3-next-20260918-g7afa080bdfc7 #5098 PREEMPT 
[   14.257742] Hardware name: Qualcomm Technologies, Inc. SM8150 HDK (DT)
[   14.264472] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   14.271648] pc : rpmh_write_batch+0x190/0x2b0
[   14.276164] lr : rpmh_write_batch+0x188/0x2b0
[   14.280679] sp : ffff80008180b740
[...]
[   14.357986] Call trace:
[   14.360529]  rpmh_write_batch+0x190/0x2b0 (P)
[   14.365051]  qcom_icc_bcm_voter_commit+0x33c/0x500
[   14.370016]  qcom_icc_set+0x20/0x34
[   14.373642]  apply_constraints+0x78/0xbc
[   14.377710]  icc_set_bw+0x22c/0x480
[   14.381337]  _set_opp_bw+0x50/0xe0
[   14.384879]  _set_opp+0xd0/0x5fc
[   14.388244]  dev_pm_opp_set_opp+0x70/0xfc
[   14.392401]  qcom_cpufreq_hw_target_index+0x10c/0x120
[   14.397629]  __cpufreq_driver_target+0x1f0/0x388
[   14.402411]  sugov_work+0x58/0x80
[   14.405865]  kthread_worker_fn+0xd8/0x4f0
[   14.410025]  kthread+0x134/0x140
[   14.413388]  ret_from_fork+0x10/0x20
[   14.417106] irq event stamp: 40
[   14.420365] hardirqs last  enabled at (39): [<ffffbf371a8ff250>] _raw_spin_unlock_irq+0x30/0x64
[   14.429336] hardirqs last disabled at (40): [<ffffbf371a8e8a80>] el1_brk64+0x20/0x60
[   14.437324] softirqs last  enabled at (0): [<ffffbf37196e8008>] copy_process+0x8a8/0x1e64
[   14.445758] softirqs last disabled at (0): [<0000000000000000>] 0x0
[   14.452243] ---[ end trace 0000000000000000 ]---
[   14.457163] Error sending AMC RPMH requests (-110)


> 
> Interestingly, SDM845 DB845c doesn't

Neither does SDM845 HDK.

> 
> I think we may need to opt some platforms out of this..

Would you send a patch or should I?

-- 
With best wishes
Dmitry

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

[Kamal Wadhwa]

On Sun, Sep 20, 2026 at 06:45:13PM +0300, Dmitry Baryshkov wrote:
> On Tue, Sep 01, 2026 at 11:49:42AM +0200, Konrad Dybcio wrote:
> > On 8/1/26 10:00 AM, 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.
> > > 
> > > 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 mode/bypass settings as well. This will
> > > provide the regulator framework a sense of the initial settings done by
> > > bootloader and thus preventing any redundant 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>
> > > ---
> > 
> > SC8180X Primus hangs with this patch applied and so does SM8150 HDK.
> 
> [    4.015947] cpu cpu0: EM: created perf domain
> [   14.216480] qcom-rpmh-regulator 18200000.rsc:regulators-1: failed to read VOLTAGE ret = -110
> [   14.216731] ------------[ cut here ]------------
> [   14.225298] ldo1: Setting 1800000-1800000uV
> [   14.230090] WARNING: drivers/soc/qcom/rpmh.c:441 at rpmh_write_batch+0x190/0x2b0, CPU#3: sugov:0/113
> [   14.230135] Modules linked in:
> [   14.247173] CPU: 3 UID: 0 PID: 113 Comm: sugov:0 Not tainted 7.3.0-rc3-next-20260918-g7afa080bdfc7 #5098 PREEMPT 
> [   14.257742] Hardware name: Qualcomm Technologies, Inc. SM8150 HDK (DT)
> [   14.264472] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [   14.271648] pc : rpmh_write_batch+0x190/0x2b0
> [   14.276164] lr : rpmh_write_batch+0x188/0x2b0
> [   14.280679] sp : ffff80008180b740
> [...]
> [   14.357986] Call trace:
> [   14.360529]  rpmh_write_batch+0x190/0x2b0 (P)
> [   14.365051]  qcom_icc_bcm_voter_commit+0x33c/0x500
> [   14.370016]  qcom_icc_set+0x20/0x34
> [   14.373642]  apply_constraints+0x78/0xbc
> [   14.377710]  icc_set_bw+0x22c/0x480
> [   14.381337]  _set_opp_bw+0x50/0xe0
> [   14.384879]  _set_opp+0xd0/0x5fc
> [   14.388244]  dev_pm_opp_set_opp+0x70/0xfc
> [   14.392401]  qcom_cpufreq_hw_target_index+0x10c/0x120
> [   14.397629]  __cpufreq_driver_target+0x1f0/0x388
> [   14.402411]  sugov_work+0x58/0x80
> [   14.405865]  kthread_worker_fn+0xd8/0x4f0
> [   14.410025]  kthread+0x134/0x140
> [   14.413388]  ret_from_fork+0x10/0x20
> [   14.417106] irq event stamp: 40
> [   14.420365] hardirqs last  enabled at (39): [<ffffbf371a8ff250>] _raw_spin_unlock_irq+0x30/0x64
> [   14.429336] hardirqs last disabled at (40): [<ffffbf371a8e8a80>] el1_brk64+0x20/0x60
> [   14.437324] softirqs last  enabled at (0): [<ffffbf37196e8008>] copy_process+0x8a8/0x1e64
> [   14.445758] softirqs last disabled at (0): [<0000000000000000>] 0x0
> [   14.452243] ---[ end trace 0000000000000000 ]---
> [   14.457163] Error sending AMC RPMH requests (-110)
> 
> 
> > 
> > Interestingly, SDM845 DB845c doesn't
> 
> Neither does SDM845 HDK.
> 
> > 
> > I think we may need to opt some platforms out of this..
> 
> Would you send a patch or should I?

It seems that Maulik has already sent a patch to skip reads for some platforms.
https://lore.kernel.org/all/20260911-rsc_read-v2-2-98675c248278@oss.qualcomm.com/

> 
> -- 
> With best wishes
> Dmitry

Regards,
Kamal

[PATCH v6 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 77d402ee6bf6..250b2c6bb795 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 v6 0/4] regulator: qcom-rpmh: Support RPMH address reads and use it for rpmh-regulators

[Mark Brown]

On Sat, 01 Aug 2026 13:30:26 +0530, Kamal Wadhwa wrote:
> regulator: qcom-rpmh: Support RPMH address reads and use it for rpmh-regulators
> 
> This patch series adds a new `rpmh_read()` API to allow reading RPMH
> addresses. Using this API enhances the RPMH regulator driver by adding
> readback of 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.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-7.3

Thanks!

[1/4] soc: qcom: rpmh: Add support to read back resource settings
      https://git.kernel.org/broonie/regulator/c/edbafe65eef2
[2/4] regulator: qcom-rpmh: Fix PMIC5 BOB bypass mode handling
      https://git.kernel.org/broonie/regulator/c/abd14bebb87e
[3/4] regulator: qcom-rpmh: readback voltage/bypass/mode set during bootup
      https://git.kernel.org/broonie/regulator/c/09d99ff7fc3c
[4/4] regulator: qcom-rpmh: Fix coding style issues
      https://git.kernel.org/broonie/regulator/c/216e8873bc80

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

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

[Kanishka Chakraborty]

Hi,

with this series in v7.3, SM7150 (Snapdragon 732G, Xiaomi Redmi Note 10
Pro) no longer boots. SM7150 support is not upstream yet; I am running
v7.3 with the sm7150-mainline patches on top, but the problem looks
independent of that and may affect other SoCs with older RSCs.

Symptom: the boot stops around the time the RPMh regulators register.
The display (left on by the bootloader) goes grey at that point and the
initramfs never runs, while the kernel apparently keeps running (the
watchdog keeps being serviced). The v7.1-based sm7150-mainline tree
boots fine on the same device with the same DT.

What I checked:

- Reverting 09d99ff7fc3c ("regulator: qcom-rpmh: readback
  voltage/bypass/mode set during bootup") and its follow-up 216e8873bc80
  ("regulator: qcom-rpmh: Fix coding style issues") makes v7.3 boot.

- A log-only variant that still issues every rpmh_read() but throws the
  results away (vreg->mode, bypassed and voltage_selector restored to the
  pre-readback defaults) hangs exactly the same way. So issuing the reads
  is enough to break it, independent of what is done with the values.
  I couldn't capture the per-read log lines: the display dies at that
  point and the boot never gets far enough for USB or a log.

- The apps RSC here reports RSC_DRV_ID = 0x00010100, i.e. version 1.1.
  rpmh-rsc only describes the v2.7 and v3.0 register layouts and uses the
  v2.7 one for everything below 3.0, so on this RSC the read path writes
  CMD_MSGID without the write bit and later reads RESP_DATA at the v2.7
  offset. This SoC has two active-only TCSes.

I don't know whether read requests are supported on RSC v1.x at all, or
only need a different register layout there. Would it make sense to only
issue reads on RSCs known to support them, and let rpmh_read() return
-EOPNOTSUPP otherwise, so the regulator driver falls back to the old
behaviour?

I'm happy to test patches or collect more data on this device.

Thanks,
Kanishka