[PATCH v2 1/4] regulator: rpmh-regulator: Fix PMIC5 BOB bypass mode handling

Kamal Wadhwa posted 4 patches 11 hours ago
[PATCH v2 1/4] regulator: rpmh-regulator: Fix PMIC5 BOB bypass mode handling
Posted by Kamal Wadhwa 11 hours ago
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 parameters:
 - `bypass_supported` to check if bypass is supported.
 - `pmic_bypass_mode` to store bypass mode value.

Use these 2 parameters to send correct PMIC bypass mode that
corresponds to PMIC4/5 BOB regulators from the helper function.

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

diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
index 0a561f1d94523bf479f48e8c2062f79cf64f5b5f..947fb5241233c92eaeda974b1b64d227d5946a59 100644
--- a/drivers/regulator/qcom-rpmh-regulator.c
+++ b/drivers/regulator/qcom-rpmh-regulator.c
@@ -111,6 +111,9 @@ 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. This is only
+ *				used if bypass_supported == true.
+ * @bypass_supported:		Indicates if bypass mode is supported
  * @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 +128,8 @@ struct rpmh_vreg_hw_data {
 	int					n_linear_ranges;
 	int					n_voltages;
 	int					hpm_min_load_uA;
+	int					pmic_bypass_mode;
+	bool					bypass_supported;
 	const int				*pmic_mode_map;
 	unsigned int			      (*of_map_mode)(unsigned int mode);
 };
@@ -310,10 +315,13 @@ static int rpmh_regulator_vrm_set_mode_bypass(struct rpmh_vreg *vreg,
 	if (pmic_mode < 0)
 		return pmic_mode;
 
-	if (bypassed)
-		cmd.data = PMIC4_BOB_MODE_PASS;
-	else
+	if (bypassed) {
+		if(!vreg->hw_data->bypass_supported)
+			return -EINVAL;
+		cmd.data = vreg->hw_data->pmic_bypass_mode;
+	} else {
 		cmd.data = pmic_mode;
+	}
 
 	return rpmh_regulator_send_request(vreg, &cmd, true);
 }
@@ -767,6 +775,8 @@ static const struct rpmh_vreg_hw_data pmic4_bob = {
 	},
 	.n_linear_ranges = 1,
 	.n_voltages = 84,
+	.bypass_supported = true,
+	.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 +985,8 @@ static const struct rpmh_vreg_hw_data pmic5_bob = {
 	},
 	.n_linear_ranges = 1,
 	.n_voltages = 32,
+	.bypass_supported = true,
+	.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.25.1
Re: [PATCH v2 1/4] regulator: rpmh-regulator: Fix PMIC5 BOB bypass mode handling
Posted by Dmitry Baryshkov 10 hours ago
On Wed, Oct 22, 2025 at 02:38:53AM +0530, Kamal Wadhwa wrote:
> 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 parameters:
>  - `bypass_supported` to check if bypass is supported.
>  - `pmic_bypass_mode` to store bypass mode value.
> 
> Use these 2 parameters to send correct PMIC bypass mode that
> corresponds to PMIC4/5 BOB regulators from the helper function.
> 
> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>

Also missing the Fixes tag.

> ---
>  drivers/regulator/qcom-rpmh-regulator.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 

-- 
With best wishes
Dmitry
Re: [PATCH v2 1/4] regulator: rpmh-regulator: Fix PMIC5 BOB bypass mode handling
Posted by Dmitry Baryshkov 10 hours ago
On Wed, Oct 22, 2025 at 02:38:53AM +0530, Kamal Wadhwa wrote:
> 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.

The universe will end, the Sun will explode and Ragnarök will be upon
us. Please describe the issue, why sending bypass value is bad.

> 
> To fix this, introduce new hw_data parameters:
>  - `bypass_supported` to check if bypass is supported.
>  - `pmic_bypass_mode` to store bypass mode value.
> 
> Use these 2 parameters to send correct PMIC bypass mode that
> corresponds to PMIC4/5 BOB regulators from the helper function.
> 
> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
> ---
>  drivers/regulator/qcom-rpmh-regulator.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
> index 0a561f1d94523bf479f48e8c2062f79cf64f5b5f..947fb5241233c92eaeda974b1b64d227d5946a59 100644
> --- a/drivers/regulator/qcom-rpmh-regulator.c
> +++ b/drivers/regulator/qcom-rpmh-regulator.c
> @@ -111,6 +111,9 @@ 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. This is only
> + *				used if bypass_supported == true.
> + * @bypass_supported:		Indicates if bypass mode is supported
>   * @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 +128,8 @@ struct rpmh_vreg_hw_data {
>  	int					n_linear_ranges;
>  	int					n_voltages;
>  	int					hpm_min_load_uA;
> +	int					pmic_bypass_mode;
> +	bool					bypass_supported;
>  	const int				*pmic_mode_map;
>  	unsigned int			      (*of_map_mode)(unsigned int mode);
>  };
> @@ -310,10 +315,13 @@ static int rpmh_regulator_vrm_set_mode_bypass(struct rpmh_vreg *vreg,
>  	if (pmic_mode < 0)
>  		return pmic_mode;
>  
> -	if (bypassed)
> -		cmd.data = PMIC4_BOB_MODE_PASS;
> -	else
> +	if (bypassed) {
> +		if(!vreg->hw_data->bypass_supported)
> +			return -EINVAL;

This is redundant, the regulators which don't support bypass should not
be using rpmh_regulator_vrm_bypass_ops.

> +		cmd.data = vreg->hw_data->pmic_bypass_mode;
> +	} else {
>  		cmd.data = pmic_mode;
> +	}

Can we extend pmic_mode_map to include the value for bypass?

>  
>  	return rpmh_regulator_send_request(vreg, &cmd, true);
>  }
> @@ -767,6 +775,8 @@ static const struct rpmh_vreg_hw_data pmic4_bob = {
>  	},
>  	.n_linear_ranges = 1,
>  	.n_voltages = 84,
> +	.bypass_supported = true,
> +	.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 +985,8 @@ static const struct rpmh_vreg_hw_data pmic5_bob = {
>  	},
>  	.n_linear_ranges = 1,
>  	.n_voltages = 32,
> +	.bypass_supported = true,
> +	.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.25.1
> 

-- 
With best wishes
Dmitry