[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

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

[Mark Brown]

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

On Thu, Jun 18, 2026 at 10:03:06AM +0800, joy.zou@oss.nxp.com wrote:

> -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),

It's a bit weird to have all these single register ranges, usually when
we have large blocks of single registers we specify them with a big
switch statement and let the compiler figure out optimising how to look
them up.  It also looks like this isn't joined up with the interrupt
handler code, regu_irqs[] lists _INT registers not _STS registers so
presumably both need to be volatile (I'm guessing _STS is live state and
_INT latched or similar).

This also omits PF0900_REG_STATUS[12]_{INT,STS}, while we don't seem
to handle these as interrupts (though we check STATUS1 during probe)
they look like they should be volatile too?

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

Re: [PATCH 0/2] regulator code improvements

[Mark Brown]

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

On Thu, Jun 18, 2026 at 10:03:04AM +0800, joy.zou@oss.nxp.com wrote:
> The main improvements include:
> 1. Correct the default toff_deb for PCA9451A and PCA9452.
> 2. Modify volatile register range definition.

Just a general style note: when you've got unrelated changes like this
which aren't really related to each other it's better to send as
separate patches, it makes it easier to track things and avoids CCing
people on things they're not interested in.  A series makes sense in
cases where there's some relationship between the changes, either
similar changes in several different places or where the series builds
to implement a feature.

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

Re: (subset) [PATCH 0/2] regulator code improvements

[Mark Brown]

On Thu, 18 Jun 2026 10:03:04 +0800, joy.zou@oss.nxp.com wrote:
> regulator code improvements
> 
> The main improvements include:
> 1. Correct the default toff_deb for PCA9451A and PCA9452.
> 2. Modify volatile register range definition.
> 
> 
> [...]

Applied to

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

Thanks!

[1/2] regulator: pca9450: Correct default t_off_deb for PCA9451A/PCA9452
      https://git.kernel.org/broonie/regulator/c/25c476711cc6

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