Add CP9314 regulator driver #68177

rriveramcrus · 2024-01-26T19:55:12Z

Adds support for the CP9314 switched-capacitor regulator. Functional modes supported:

Both 3:1 and 2:1 converter op modes
Synchronous (shared clock) mode
Enable via GPIO or I2C

Only B0 silicon is supported.

aasinclair

Is there a datasheet for this part available?

drivers/regulator/regulator_cp9314.c

drivers/regulator/regulator_cp9314.h

gmarull · 2024-01-30T11:59:45Z

drivers/regulator/regulator_cp9314.c

+static struct cp9314_reg_patch b0_reg_patch[18] = {
+	{CP9314_REG_CRUS_CTRL, GENMASK(7, 0), CP9314_CRUS_KEY_UNLOCK},
+	{CP9314_REG_LION_COMP_CTRL_3, CP9314_VIN_OV_CFG, 0x1B},
+	{CP9314_REG_LION_COMP_CTRL_1, CP9314_VOUT_OV_CFG_0, 0x30},
+	{CP9314_REG_LION_COMP_CTRL_2, CP9314_VOUT_OV_CFG_1, 0xC},
+	{CP9314_REG_VIN2OUT_OVP, CP9314_VIN2OUT_OVP, 0x2},
+	{CP9314_REG_VIN2OUT_UVP, CP9314_VIN2OUT_UVP, 0x1},
+	{CP9314_REG_VOUT_UVP, CP9314_VOUT_UVP_DIS, 0},
+	{CP9314_REG_VOUT_UVP, CP9314_VOUT_UVP, 0},
+	{CP9314_REG_LION_COMP_CTRL_1, CP9314_VIN_SWITCH_OK_DIS_0, 0},
+	{CP9314_REG_LION_COMP_CTRL_4, CP9314_VIN_SWITCH_OK_DIS_1, 0},
+	{CP9314_REG_LION_COMP_CTRL_1, CP9314_VIN_SWITCH_OK_CFG, 0},
+	{CP9314_REG_LION_CFG_3, CP9314_LB_MIN_FREQ_SEL_0, 0x80},
+	{CP9314_REG_LB_CTRL, CP9314_LB_MIN_FREQ_SEL_1, 0x4},
+	{CP9314_REG_TRIM_8, CP9314_MODE_CTRL_UPDATE_BW_0, 0x2},
+	{CP9314_REG_LION_CFG_3, CP9314_MODE_CTRL_UPDATE_BW_1, 0x2},
+	{CP9314_REG_IIN_OCP, CP9314_IIN_OCP_DIS, CP9314_IIN_OCP_DIS},
+	{CP9314_REG_IIN_PEAK_OCP, CP9314_IIN_PEAK_OCP_DIS, CP9314_IIN_PEAK_OCP_DIS},
+	{CP9314_REG_CRUS_CTRL, GENMASK(7, 0), CP9314_CRUS_KEY_LOCK},
+};
+
+static struct cp9314_reg_patch otp_1_patch[3] = {
+	{CP9314_REG_OPTION_REG_1, CP9314_LB1_DELAY_CFG, 0},
+	{CP9314_REG_BST_CP_PD_CFG, CP9314_LB1_BLANK_CFG, CP9314_LB1_BLANK_CFG},
+	{CP9314_REG_TSBAT_CTRL, CP9314_LB1_STOP_PHASE_SEL, CP9314_LB1_STOP_PHASE_SEL},
+};


these probably deserve some docstring explaining what they are

ack, let me know if the added comments are sufficient.

It could do with an overall comment for the block giving a rough indication of what it is for, so that users can see what is being fixed.
e.g.
/** Patch for b0 silicon to correct trim errata and feedback scaling errors */

gmarull · 2024-01-30T12:00:24Z

drivers/regulator/regulator_cp9314.c

+	{CP9314_REG_TSBAT_CTRL, CP9314_LB1_STOP_PHASE_SEL, CP9314_LB1_STOP_PHASE_SEL},
+};
+
+LOG_MODULE_REGISTER(CP9314);


by convention, move just after includes. This also misses a log level.

gmarull · 2024-01-30T12:01:08Z

drivers/regulator/regulator_cp9314.c

+		return ret;
+	}
+
+	data->op_mode = mode;


data->op_mode doesn't seem to be used anywhere else.

ack. A leftover from the earlier silicon revision.

gmarull · 2024-01-30T12:02:54Z

drivers/regulator/regulator_cp9314.c

+	int i, ret = 0;
+
+	for (i = 0; i < ARRAY_SIZE(b0_reg_patch); i++) {


i scope can be reduced, for (size_t i = 0U; ...

gmarull · 2024-01-30T12:03:18Z

drivers/regulator/regulator_cp9314.c

+		ret |= i2c_reg_update_byte_dt(&config->i2c, b0_reg_patch[i].reg_addr,
+					      b0_reg_patch[i].mask, b0_reg_patch[i].value);


you should fail fast, also, errno can't be ored together.

gmarull · 2024-01-30T12:03:32Z

drivers/regulator/regulator_cp9314.c

+		return ret;
+	}
+
+	k_msleep(200);


magic sleep, please define

gmarull · 2024-01-30T12:03:52Z

drivers/regulator/regulator_cp9314.c

+	uint8_t value;
+	int i, ret;
+
+	/**


not a doxygen docstring, use /*

gmarull · 2024-01-30T12:05:47Z

drivers/regulator/regulator_cp9314.c

+		return -ENOTSUP;
+	}
+
+	data->rev_id = value;


data->rev_id is not used

gmarull · 2024-01-30T12:06:44Z

include/zephyr/dt-bindings/regulator/cp9314.h

+/**
+ * @name CP9314 Regulator modes
+ * @{
+ */
+/** 2:1 mode */
+#define CP9314_MODE_2TO1 1
+/** 3:1 mode */
+#define CP9314_MODE_3TO1 2
+/** @} */


can datasheet or details about these modes be provided? are these regulation modes?

ack #68177 (comment)

rriveramcrus · 2024-01-30T16:30:59Z

@aasinclair and @gmarull

Is there a datasheet for this part available?

Unfortunately not one that can be shared. This is kind of a more quirky power IC, so I can expand on a bit here and update some of the block comments in the source as well.

The power converter:

This IC is meant for portable systems with 2 or 3 battery cells in series (ex. Laptop). The ICs purpose is to take the 2 or 3 cell battery voltage and buck down to a single cell battery voltage, this is the 2:1/3:1 operational modes.

This sort of power converter (switched capacitor) is able to convert the power without resistive elements (greatly reducing heat aka power wasted) but is fixed to these sort of n:1 operational modes. So it's not really aiming for a regulated target voltage but rather only performing the immediate large power conversion.

The functionality:

The IC conversion operation can be switched on and off via GPIO or I2C command from the host. The operational mode (2:1 or 3:1) can be controlled via I2C from the host. Additionally, the IC can exist standalone or paired in parallel via a shared clock line provided by the initiator instance of the IC. In this mode, the ICs each have a unique I2C address and must be configured by the host based on their ADDR_LVL.

Let me know if this is enough detail to clarify what's going on in the source.

Thanks
Ricardo

drivers/regulator/Kconfig.cp9314

drivers/regulator/regulator_cp9314.c

aasinclair · 2024-02-07T08:05:43Z

This IC is meant for portable systems with 2 or 3 battery cells in series (ex. Laptop). The ICs purpose is to take the 2 or 3 cell battery voltage and buck down to a single cell battery voltage, this is the 2:1/3:1 operational modes.

This sort of power converter (switched capacitor) is able to convert the power without resistive elements (greatly reducing heat aka power wasted) but is fixed to these sort of n:1 operational modes. So it's not really aiming for a regulated target voltage but rather only performing the immediate large power conversion.

Is this something that the user is expected to change at runtime, or is it likely to be fixed by the design?
Is there another default mode, or is it 2:1/3:1?

Thanks

rriveramcrus · 2024-02-07T16:37:20Z

@aasinclair ,

Is this something that the user is expected to change at runtime, or is it likely to be fixed by the design?

It is fixed to the design. I will take an action to remove the runtime control of the op mode.

Is there another default mode, or is it 2:1/3:1?

The default is set with a resistor. We do have one customer using the same BOM for two different SKUs (therefore different op modes), so the driver needs to be able to initialize and overwrite the op mode.

rriveramcrus · 2024-02-08T19:50:04Z

@aasinclair ,

It is fixed to the design. I will take an action to remove the runtime control of the op mode.

Ah, my bad. If I get rid of .set_mode member of the regulator_driver_api, then regulator_common_init() would not be able to set config->initial_mode. I could add an initialization flag to regulator_cp9314_data and conditionally prevent any post-init regulator_set_mode calls from succeeding.

Let me know if you would rather the initialization flag approach or just leaving it alone.

aasinclair · 2024-02-12T09:04:02Z

@aasinclair ,

It is fixed to the design. I will take an action to remove the runtime control of the op mode.

Ah, my bad. If I get rid of .set_mode member of the regulator_driver_api, then regulator_common_init() would not be able to set config->initial_mode. I could add an initialization flag to regulator_cp9314_data and conditionally prevent any post-init regulator_set_mode calls from succeeding.

Let me know if you would rather the initialization flag approach or just leaving it alone.

I think this would be better implemented as a custom property, rather than as a mode.
E.g something like this:

zephyr/dts/bindings/regulator/nxp,vref.yaml

Lines 23 to 27 in 592a7cd

    
           nxp,ground-select: 
        
             type: string 
        
             enum: 
        
               - "VREFL3V" # 0 
        
               - "VSSA" # 1

rriveramcrus · 2024-02-15T18:38:02Z

@aasinclair

I think this would be better implemented as a custom property, rather than as a mode.

Implemented it as cirrus,initial-converter-mode

gmarull · 2024-02-16T16:11:27Z

dts/bindings/regulator/cirrus,cp9314.yaml

+
+include:
+  - name: i2c-device.yaml
+  - name: regulator.yaml


shouldn't this filter allowed props?

ack. The only regulator we can use right now is regulator-boot-on. Some protection properties may be added from regulator.yaml down the line.

gmarull · 2024-02-16T16:11:52Z

drivers/regulator/regulator_cp9314.c

+static struct cp9314_reg_patch b0_reg_patch[18] = {
+	{CP9314_REG_CRUS_CTRL, GENMASK(7, 0), CP9314_CRUS_KEY_UNLOCK},
+	{CP9314_REG_LION_COMP_CTRL_3, CP9314_VIN_OV_CFG, 0x1B},
+	{CP9314_REG_LION_COMP_CTRL_1, CP9314_VOUT_OV_CFG_0, 0x30},
+	{CP9314_REG_LION_COMP_CTRL_2, CP9314_VOUT_OV_CFG_1, 0xC},
+	{CP9314_REG_VIN2OUT_OVP, CP9314_VIN2OUT_OVP, 0x2},
+	{CP9314_REG_VIN2OUT_UVP, CP9314_VIN2OUT_UVP, 0x1},
+	{CP9314_REG_VOUT_UVP, CP9314_VOUT_UVP_DIS, 0},
+	{CP9314_REG_VOUT_UVP, CP9314_VOUT_UVP, 0},
+	{CP9314_REG_LION_COMP_CTRL_1, CP9314_VIN_SWITCH_OK_DIS_0, 0},
+	{CP9314_REG_LION_COMP_CTRL_4, CP9314_VIN_SWITCH_OK_DIS_1, 0},
+	{CP9314_REG_LION_COMP_CTRL_1, CP9314_VIN_SWITCH_OK_CFG, 0},
+	{CP9314_REG_LION_CFG_3, CP9314_LB_MIN_FREQ_SEL_0, 0x80},
+	{CP9314_REG_LB_CTRL, CP9314_LB_MIN_FREQ_SEL_1, 0x4},
+	{CP9314_REG_TRIM_8, CP9314_MODE_CTRL_UPDATE_BW_0, 0x2},
+	{CP9314_REG_LION_CFG_3, CP9314_MODE_CTRL_UPDATE_BW_1, 0x2},
+	{CP9314_REG_IIN_OCP, CP9314_IIN_OCP_DIS, CP9314_IIN_OCP_DIS},
+	{CP9314_REG_IIN_PEAK_OCP, CP9314_IIN_PEAK_OCP_DIS, CP9314_IIN_PEAK_OCP_DIS},
+	{CP9314_REG_CRUS_CTRL, GENMASK(7, 0), CP9314_CRUS_KEY_LOCK},
+};
+
+static struct cp9314_reg_patch otp_1_patch[3] = {
+	{CP9314_REG_OPTION_REG_1, CP9314_LB1_DELAY_CFG, 0},
+	{CP9314_REG_BST_CP_PD_CFG, CP9314_LB1_BLANK_CFG, CP9314_LB1_BLANK_CFG},
+	{CP9314_REG_TSBAT_CTRL, CP9314_LB1_STOP_PHASE_SEL, CP9314_LB1_STOP_PHASE_SEL},
+};


drivers/regulator/regulator_cp9314.c

gmarull · 2024-02-16T16:13:08Z

include/zephyr/dt-bindings/regulator/cp9314.h

+ */
+/** 2:1 mode */
+#define CP9314_MODE_2TO1 1
+/** 3:1 mode */
+#define CP9314_MODE_3TO1 2
+/** @} */


are these used now? If for the new vendor property, please, add an example in the bindings file showing its usage.

also, an enum on such property (if used there) to avoid user mistakes would be useful as well

If you use enums & ENUM_IDX macros, you can remove this file.

dts/bindings/regulator/cirrus,cp9314.yaml

drivers/regulator/regulator_cp9314.c

tests/drivers/build_all/regulator/i2c.dtsi

aasinclair · 2024-02-19T09:22:41Z

include/zephyr/dt-bindings/regulator/cp9314.h

+ */
+/** 2:1 mode */
+#define CP9314_MODE_2TO1 1
+/** 3:1 mode */
+#define CP9314_MODE_3TO1 2
+/** @} */


If you use enums & ENUM_IDX macros, you can remove this file.

dts/bindings/regulator/cirrus,cp9314.yaml

drivers/regulator/regulator_cp9314.c

Adds devicetree bindings for the Cirrus CP9314 Switched Capacitor 3:1/2:1 DC/DC converter. Signed-off-by: Ricardo Rivera-Matos <rriveram@opensource.cirrus.com>

aasinclair

Typo nit, but otherwise looks great.

drivers/regulator/regulator_cp9314.c

Adds support for the CP9314 switched capacitor converter. The CP9314 is a multi-level DC/DC converter capable of operating in 2:1 and 3:1 modes. Signed-off-by: Ricardo Rivera-Matos <rriveram@opensource.cirrus.com>

Adds the CP9314 device to the regulator build_all test suite. Signed-off-by: Ricardo Rivera-Matos <rriveram@opensource.cirrus.com>

rriveramcrus force-pushed the cp9314-latest branch from 9fc7075 to e26dfa6 Compare January 26, 2024 20:26

rriveramcrus marked this pull request as ready for review January 29, 2024 16:28

rriveramcrus requested a review from gmarull as a code owner January 29, 2024 16:28

zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Regulators labels Jan 29, 2024

zephyrbot requested review from aasinclair, danieldegrasse and galak January 29, 2024 16:28

zephyrbot assigned aasinclair Jan 29, 2024

aasinclair requested changes Jan 30, 2024

View reviewed changes

drivers/regulator/regulator_cp9314.c Outdated Show resolved Hide resolved

drivers/regulator/regulator_cp9314.c Outdated Show resolved Hide resolved

drivers/regulator/regulator_cp9314.h Outdated Show resolved Hide resolved

gmarull requested changes Jan 30, 2024

View reviewed changes

rriveramcrus force-pushed the cp9314-latest branch 3 times, most recently from e0684e5 to 9042103 Compare January 30, 2024 20:02

rriveramcrus requested review from aasinclair and gmarull January 31, 2024 16:28

aasinclair requested changes Feb 1, 2024

View reviewed changes

drivers/regulator/Kconfig.cp9314 Show resolved Hide resolved

drivers/regulator/regulator_cp9314.c Show resolved Hide resolved

rriveramcrus force-pushed the cp9314-latest branch from 9042103 to 768daf2 Compare February 1, 2024 18:51

rriveramcrus force-pushed the cp9314-latest branch 2 times, most recently from c1f6dcd to 6621d76 Compare February 15, 2024 17:45

rriveramcrus requested a review from aasinclair February 15, 2024 18:38

gmarull reviewed Feb 16, 2024

View reviewed changes

aasinclair requested changes Feb 19, 2024

View reviewed changes

rriveramcrus force-pushed the cp9314-latest branch from 6621d76 to 0d3f8a8 Compare February 19, 2024 17:17

rriveramcrus requested review from gmarull and aasinclair February 19, 2024 17:50

aasinclair reviewed Feb 20, 2024

View reviewed changes

dts/bindings/regulator/cirrus,cp9314.yaml Outdated Show resolved Hide resolved

aasinclair reviewed Feb 20, 2024

View reviewed changes

dts/bindings/regulator/cirrus,cp9314.yaml Outdated Show resolved Hide resolved

drivers/regulator/regulator_cp9314.c Outdated Show resolved Hide resolved

dts: regulator: Adds bindings for CP9314 Buck

c792733

Adds devicetree bindings for the Cirrus CP9314 Switched Capacitor 3:1/2:1 DC/DC converter. Signed-off-by: Ricardo Rivera-Matos <rriveram@opensource.cirrus.com>

rriveramcrus force-pushed the cp9314-latest branch from 0d3f8a8 to 11b313a Compare February 20, 2024 16:15

rriveramcrus requested a review from aasinclair February 20, 2024 17:04

aasinclair previously approved these changes Feb 21, 2024

View reviewed changes

drivers/regulator/regulator_cp9314.c Outdated Show resolved Hide resolved

rriveramcrus added 2 commits February 21, 2024 10:32

drivers: regulator: cp9314: adds initial support

72e3769

Adds support for the CP9314 switched capacitor converter. The CP9314 is a multi-level DC/DC converter capable of operating in 2:1 and 3:1 modes. Signed-off-by: Ricardo Rivera-Matos <rriveram@opensource.cirrus.com>

build_all: regulator: Adds CP9314 entry

14e7375

Adds the CP9314 device to the regulator build_all test suite. Signed-off-by: Ricardo Rivera-Matos <rriveram@opensource.cirrus.com>

rriveramcrus dismissed aasinclair’s stale review via 14e7375 February 21, 2024 16:33

rriveramcrus force-pushed the cp9314-latest branch from 11b313a to 14e7375 Compare February 21, 2024 16:33

aasinclair approved these changes Feb 22, 2024

View reviewed changes

gmarull approved these changes Feb 26, 2024

View reviewed changes

aescolar merged commit aff2564 into zephyrproject-rtos:main Feb 27, 2024
18 checks passed

rriveramcrus mentioned this pull request Mar 27, 2024

regulator: cp9314: Adds support for PGOOD pin #70805

Merged

		int i, ret = 0;

		for (i = 0; i < ARRAY_SIZE(b0_reg_patch); i++) {

		ret \|= i2c_reg_update_byte_dt(&config->i2c, b0_reg_patch[i].reg_addr,
		b0_reg_patch[i].mask, b0_reg_patch[i].value);

Add CP9314 regulator driver #68177

Add CP9314 regulator driver #68177

Conversation

rriveramcrus commented Jan 26, 2024 • edited

aasinclair left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmarull Jan 30, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rriveramcrus commented Jan 30, 2024

aasinclair commented Feb 7, 2024

rriveramcrus commented Feb 7, 2024

rriveramcrus commented Feb 8, 2024 • edited

aasinclair commented Feb 12, 2024

rriveramcrus commented Feb 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aasinclair left a comment

Choose a reason for hiding this comment

rriveramcrus commented Jan 26, 2024 •

edited

gmarull Jan 30, 2024 •

edited

rriveramcrus commented Feb 8, 2024 •

edited