[PATCH 0/2] regulator code improvements

[PATCH 0/2] regulator code improvements

[joy.zou]

The main improvements include:
1. Correct the default toff_deb for PCA9451A and PCA9452.
2. Modify volatile register range definition.

Signed-off-by: Joy Zou <joy.zou@oss.nxp.com>
---
Joy Zou (2):
      regulator: pca9450: Correct default t_off_deb for PCA9451A/PCA9452
      regulator: pf0900: Modify volatile register range definition

 drivers/regulator/pca9450-regulator.c |  7 ++++++-
 drivers/regulator/pf0900-regulator.c  | 19 +++++++++++++++----
 2 files changed, 21 insertions(+), 5 deletions(-)
---
base-commit: f7af91adc230aa99e23330ecf85bc9badd9780ad
change-id: 20260617-b4-regulator-opt-4eb51441e701

Best regards,
--  
Joy Zou <joy.zou@oss.nxp.com>

[PATCH 1/2] regulator: pca9450: Correct default t_off_deb for PCA9451A/PCA9452

[joy.zou]

From: Joy Zou <joy.zou@nxp.com>

The PMIC PCA9451A and PCA9452 have a default power-off debounce time of
2ms according to their datasheet, while PCA9450A and PCA9450BC use 120us.

Add default_t_off_deb field to struct pca9450 to support per-variant
default configuration when the device tree property is not specified.

Datasheet reference links:
- PCA9451A Rev.2.1: https://www.nxp.com/docs/en/data-sheet/PCA9451A.pdf
- PCA9452 Rev.1.0: https://www.nxp.com/docs/en/data-sheet/PCA9452.pdf

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
Changes in v2:
1. remove original switch case changes, and add default_t_off_deb field to
struct pca9450 to support per-variant default configuration.
2. modify commit message.
---
 drivers/regulator/pca9450-regulator.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
index 45d7dc44c2cd..c41db70fa052 100644
--- a/drivers/regulator/pca9450-regulator.c
+++ b/drivers/regulator/pca9450-regulator.c
@@ -44,6 +44,7 @@ struct pca9450 {
 	unsigned int rcnt;
 	int irq;
 	bool sd_vsel_fixed_low;
+	int default_t_off_deb;
 };
 
 static const struct regmap_range pca9450_status_range = {
@@ -1209,7 +1210,7 @@ static int pca9450_of_init(struct pca9450 *pca9450)
 
 	ret = of_property_read_u32(i2c->dev.of_node, "nxp,pmic-on-req-off-debounce-us", &val);
 	if (ret == -EINVAL)
-		t_off_deb = T_OFF_DEB_120US;
+		t_off_deb = pca9450->default_t_off_deb;
 	else if (ret)
 		return ret;
 	else {
@@ -1304,21 +1305,25 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
 	case PCA9450_TYPE_PCA9450A:
 		regulator_desc = pca9450a_regulators;
 		pca9450->rcnt = ARRAY_SIZE(pca9450a_regulators);
+		pca9450->default_t_off_deb = T_OFF_DEB_120US;
 		type_name = "pca9450a";
 		break;
 	case PCA9450_TYPE_PCA9450BC:
 		regulator_desc = pca9450bc_regulators;
 		pca9450->rcnt = ARRAY_SIZE(pca9450bc_regulators);
+		pca9450->default_t_off_deb = T_OFF_DEB_120US;
 		type_name = "pca9450bc";
 		break;
 	case PCA9450_TYPE_PCA9451A:
 		regulator_desc = pca9451a_regulators;
 		pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
+		pca9450->default_t_off_deb = T_OFF_DEB_2MS;
 		type_name = "pca9451a";
 		break;
 	case PCA9450_TYPE_PCA9452:
 		regulator_desc = pca9451a_regulators;
 		pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
+		pca9450->default_t_off_deb = T_OFF_DEB_2MS;
 		type_name = "pca9452";
 		break;
 	default:

-- 
2.34.1

Re: [PATCH 1/2] regulator: pca9450: Correct default t_off_deb for PCA9451A/PCA9452

[Frank Li]

On Thu, Jun 18, 2026 at 10:03:05AM +0800, joy.zou@oss.nxp.com wrote:
> From: Joy Zou <joy.zou@nxp.com>
>
> The PMIC PCA9451A and PCA9452 have a default power-off debounce time of
> 2ms according to their datasheet, while PCA9450A and PCA9450BC use 120us.
>
> Add default_t_off_deb field to struct pca9450 to support per-variant
> default configuration when the device tree property is not specified.
>
> Datasheet reference links:
> - PCA9451A Rev.2.1: https://www.nxp.com/docs/en/data-sheet/PCA9451A.pdf
> - PCA9452 Rev.1.0: https://www.nxp.com/docs/en/data-sheet/PCA9452.pdf
>
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---

If you have time, suggest change to use drvdata replace switch case.

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v2:
> 1. remove original switch case changes, and add default_t_off_deb field to
> struct pca9450 to support per-variant default configuration.
> 2. modify commit message.
> ---
>  drivers/regulator/pca9450-regulator.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
> index 45d7dc44c2cd..c41db70fa052 100644
> --- a/drivers/regulator/pca9450-regulator.c
> +++ b/drivers/regulator/pca9450-regulator.c
> @@ -44,6 +44,7 @@ struct pca9450 {
>  	unsigned int rcnt;
>  	int irq;
>  	bool sd_vsel_fixed_low;
> +	int default_t_off_deb;
>  };
>
>  static const struct regmap_range pca9450_status_range = {
> @@ -1209,7 +1210,7 @@ static int pca9450_of_init(struct pca9450 *pca9450)
>
>  	ret = of_property_read_u32(i2c->dev.of_node, "nxp,pmic-on-req-off-debounce-us", &val);
>  	if (ret == -EINVAL)
> -		t_off_deb = T_OFF_DEB_120US;
> +		t_off_deb = pca9450->default_t_off_deb;
>  	else if (ret)
>  		return ret;
>  	else {
> @@ -1304,21 +1305,25 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
>  	case PCA9450_TYPE_PCA9450A:
>  		regulator_desc = pca9450a_regulators;
>  		pca9450->rcnt = ARRAY_SIZE(pca9450a_regulators);
> +		pca9450->default_t_off_deb = T_OFF_DEB_120US;
>  		type_name = "pca9450a";
>  		break;
>  	case PCA9450_TYPE_PCA9450BC:
>  		regulator_desc = pca9450bc_regulators;
>  		pca9450->rcnt = ARRAY_SIZE(pca9450bc_regulators);
> +		pca9450->default_t_off_deb = T_OFF_DEB_120US;
>  		type_name = "pca9450bc";
>  		break;
>  	case PCA9450_TYPE_PCA9451A:
>  		regulator_desc = pca9451a_regulators;
>  		pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
> +		pca9450->default_t_off_deb = T_OFF_DEB_2MS;
>  		type_name = "pca9451a";
>  		break;
>  	case PCA9450_TYPE_PCA9452:
>  		regulator_desc = pca9451a_regulators;
>  		pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
> +		pca9450->default_t_off_deb = T_OFF_DEB_2MS;
>  		type_name = "pca9452";
>  		break;
>  	default:
>
> --
> 2.34.1
>
>

[PATCH 2/2] regulator: pf0900: Modify volatile register range definition

[joy.zou]

From: Joy Zou <joy.zou@nxp.com>

The pf0900_range was incorrectly defined as a single continuous range
from PF0900_REG_DEV_ID to PF0900_REG_SYS_DIAG, which includes many
non-volatile registers. This could lead to unnecessary I2C/SPI reads
and potential performance issues.

Ensures only volatile registers are read from the device
on each access, while other registers can be cached by regmap.

Fixes: 162e23657e53 ("regulator: pf0900: Add PMIC PF0900 support")

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
 drivers/regulator/pf0900-regulator.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/pf0900-regulator.c b/drivers/regulator/pf0900-regulator.c
index 5c44d2dbcab4..bca76c58ed3b 100644
--- a/drivers/regulator/pf0900-regulator.c
+++ b/drivers/regulator/pf0900-regulator.c
@@ -284,13 +284,24 @@ struct pf0900_regulator_irq {
 	unsigned int  event;
 };
 
-static const struct regmap_range pf0900_range = {
-	.range_min = PF0900_REG_DEV_ID,
-	.range_max = PF0900_REG_SYS_DIAG,
+static const struct regmap_range pf0900_range[] = {
+	regmap_reg_range(PF0900_REG_SYSTEM_INT, PF0900_REG_SYSTEM_INT),
+	regmap_reg_range(PF0900_REG_STATUS1_SNS, PF0900_REG_STATUS1_SNS),
+	regmap_reg_range(PF0900_REG_STATUS2_SNS, PF0900_REG_STATUS2_SNS),
+	regmap_reg_range(PF0900_REG_SW_ILIM_SNS, PF0900_REG_SW_ILIM_SNS),
+	regmap_reg_range(PF0900_REG_LDO_ILIM_SNS, PF0900_REG_LDO_ILIM_SNS),
+	regmap_reg_range(PF0900_REG_SW_UV_SNS, PF0900_REG_SW_UV_SNS),
+	regmap_reg_range(PF0900_REG_SW_OV_SNS, PF0900_REG_SW_OV_SNS),
+	regmap_reg_range(PF0900_REG_LDO_UV_SNS, PF0900_REG_LDO_UV_SNS),
+	regmap_reg_range(PF0900_REG_LDO_OV_SNS, PF0900_REG_LDO_OV_SNS),
+	regmap_reg_range(PF0900_REG_IO_SNS, PF0900_REG_ABIST_IO),
+	regmap_reg_range(PF0900_REG_SECURE_WR1, PF0900_REG_SECURE_WR1),
+	regmap_reg_range(PF0900_REG_WD_CNT1, PF0900_REG_WD_CNT2),
+	regmap_reg_range(PF0900_REG_SYS_DIAG, PF0900_REG_SYS_DIAG),
 };
 
 static const struct regmap_access_table pf0900_volatile_regs = {
-	.yes_ranges = &pf0900_range,
+	.yes_ranges = pf0900_range,
 	.n_yes_ranges = 1,
 };
 

-- 
2.34.1

Re: [PATCH 2/2] regulator: pf0900: Modify volatile register range definition

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `.n_yes_ranges` field in `pf0900_volatile_regs` is hardcoded to 1, causing most volatile registers to be ignored.
- [High] Interrupt status registers (_INT) are omitted from the volatile ranges, breaking IRQ handling.
--

commit 8da34e61f8f6ebeaacd20e2bb532c2786ad28d72
Author: Joy Zou <joy.zou@nxp.com>

regulator: pf0900: Modify volatile register range definition
    
The pf0900_range was incorrectly defined as a single continuous range
from PF0900_REG_DEV_ID to PF0900_REG_SYS_DIAG. This commit ensures only
volatile registers are read from the device on each access, while other
registers can be cached by regmap.

> diff --git a/drivers/regulator/pf0900-regulator.c b/drivers/regulator/pf0900-regulator.c
> index 5c44d2dbcab4d..bca76c58ed3bf 100644
> --- a/drivers/regulator/pf0900-regulator.c
> +++ b/drivers/regulator/pf0900-regulator.c
> @@ -284,13 +284,24 @@ struct pf0900_regulator_irq {
>  	unsigned int  event;
>  };
>  
> -static const struct regmap_range pf0900_range = {
> -	.range_min = PF0900_REG_DEV_ID,
> -	.range_max = PF0900_REG_SYS_DIAG,
> +static const struct regmap_range pf0900_range[] = {
> +	regmap_reg_range(PF0900_REG_SYSTEM_INT, PF0900_REG_SYSTEM_INT),
> +	regmap_reg_range(PF0900_REG_STATUS1_SNS, PF0900_REG_STATUS1_SNS),

[Severity: High]
Does this new configuration omit the interrupt status registers needed by
pf0900_irq_handler()?

Looking at pf0900_irq_handler(), it relies on regmap_read() to query the
hardware interrupt status:

pf0900_irq_handler() {
	...
	ret = regmap_read(pf0900->regmap, regu_irqs[i].reg, &val);
	...
}

Since these _INT registers are no longer listed in the volatile ranges,
will regmap reads return cached values instead of the actual hardware
state, causing the driver to silently ignore active interrupts?

[ ... ]

> +	regmap_reg_range(PF0900_REG_SECURE_WR1, PF0900_REG_SECURE_WR1),
> +	regmap_reg_range(PF0900_REG_WD_CNT1, PF0900_REG_WD_CNT2),
> +	regmap_reg_range(PF0900_REG_SYS_DIAG, PF0900_REG_SYS_DIAG),
>  };
>  
>  static const struct regmap_access_table pf0900_volatile_regs = {
> -	.yes_ranges = &pf0900_range,
> +	.yes_ranges = pf0900_range,
>  	.n_yes_ranges = 1,

[Severity: High]
Should this be updated to ARRAY_SIZE(pf0900_range)?

With n_yes_ranges hardcoded to 1, won't the regmap subsystem ignore all
but the very first entry (PF0900_REG_SYSTEM_INT) in the array? This
could cause reads to the remaining hardware status registers to return
stale cached values.

>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-b4-regulator-opt-v1-0-c43b1f62aaf6@oss.nxp.com?part=2

Re: [PATCH 2/2] regulator: pf0900: Modify volatile register range definition

[Francesco Dolcini]

Hello Joy,
thanks for the patch

On Thu, Jun 18, 2026 at 10:03:06AM +0800, joy.zou@oss.nxp.com wrote:
> From: Joy Zou <joy.zou@nxp.com>
> 
> The pf0900_range was incorrectly defined as a single continuous range
> from PF0900_REG_DEV_ID to PF0900_REG_SYS_DIAG, which includes many
> non-volatile registers. This could lead to unnecessary I2C/SPI reads
> and potential performance issues.
> 
> Ensures only volatile registers are read from the device
> on each access, while other registers can be cached by regmap.
> 
> Fixes: 162e23657e53 ("regulator: pf0900: Add PMIC PF0900 support")
> 
> Signed-off-by: Joy Zou <joy.zou@nxp.com>

No empty lines in-between tags. Remove the newline after Fixes:

Francesco