Messages in this thread | | | From | Adam Thomson <> | Subject | RE: [PATCH] regulator: da9063: fix suspend | Date | Tue, 17 Mar 2020 16:48:00 +0000 |
| |
On 17 March 2020 16:14, Martin Fuzzey wrote:
> The .set_suspend_enable() and .set_suspend_disable() methods are not > supposed to immediately change the regulator state but just indicated > if the regulator should be enabled or disabled when standby mode is > entered (by a hardware signal). > > However currently they set control the SEL bits in the DVC registers, > which causes the voltage to change to immediately between the "A" > (normal) and "B" (standby) values as programmed and does nothing for > the enable state... > > This means that "regulator-on-in-suspend" does not work (the regulator > is switched off when the PMIC enters standby mode on the hardware > signal) and, potentially, depending on the A and B voltage > configurations the voltage could be incorrectly changed *before* > actually entering suspend. > > The right bit to use for the functionality is the "CONF" bit in the > "CONT" register. > The detailed register description says "Sequencer target state" > for this bit which is not very clear but the functional description > is clearer. > > From 5.1.5 System Enable: > > De-asserting SYS_EN (changing from active to passive state) > clears control SYSTEM_EN which triggers a power down sequence > into hibernate/standby mode > ... > With the exception of supplies that have the xxxx_CONF control > bit asserted, all regulators in power domains POWER1, POWER, and > SYSTEM are sequentially disabled in reverse order. > Regulators with the <x>_CONF bit set remain on but change the > active voltage controlregisters from V<x>_A to V<x>_B > (if V<x>_B is notalready selected). >
Thanks for this. Actually it's very much the same as the changes made for DA9061/2 regulator driver, as per:
regulator: da9062: fix suspend_enable/disable preparation (commit: a72865f057820ea9f57597915da4b651d65eb92f)
Think we should also update the get_mode() functions for da9063 in a similar manner, given the updates being made here.
> Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group> > --- > drivers/regulator/da9063-regulator.c | 20 ++------------------ > 1 file changed, 2 insertions(+), 18 deletions(-) > > diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063- > regulator.c > index 2aceb3b..46b7301 100644 > --- a/drivers/regulator/da9063-regulator.c > +++ b/drivers/regulator/da9063-regulator.c > @@ -100,6 +100,7 @@ struct da9063_regulator_info { > .desc.vsel_mask = DA9063_V##regl_name##_MASK, \ > .desc.linear_min_sel = DA9063_V##regl_name##_BIAS, \ > .sleep = BFIELD(DA9063_REG_V##regl_name##_A, DA9063_LDO_SL), \ > + .suspend = BFIELD(DA9063_REG_##regl_name##_CONT, > DA9063_LDO_CONF), \ > .suspend_sleep = BFIELD(DA9063_REG_V##regl_name##_B, > DA9063_LDO_SL), \ > .suspend_vsel_reg = DA9063_REG_V##regl_name##_B > > @@ -124,6 +125,7 @@ struct da9063_regulator_info { > .desc.vsel_mask = DA9063_VBUCK_MASK, \ > .desc.linear_min_sel = DA9063_VBUCK_BIAS, \ > .sleep = BFIELD(DA9063_REG_V##regl_name##_A, DA9063_BUCK_SL), \ > + .suspend = BFIELD(DA9063_REG_##regl_name##_CONT, > DA9063_BUCK_CONF), \ > .suspend_sleep = BFIELD(DA9063_REG_V##regl_name##_B, > DA9063_BUCK_SL), \ > .suspend_vsel_reg = DA9063_REG_V##regl_name##_B, \ > .mode = BFIELD(DA9063_REG_##regl_name##_CFG, > DA9063_BUCK_MODE_MASK) > @@ -465,42 +467,36 @@ static int da9063_ldo_set_suspend_mode(struct > regulator_dev *rdev, unsigned mode > da9063_buck_a_limits, > DA9063_REG_BUCK_ILIM_C, > DA9063_BCORE1_ILIM_MASK), > DA9063_BUCK_COMMON_FIELDS(BCORE1), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VBCORE1_SEL), > }, > { > DA9063_BUCK(DA9063, BCORE2, 300, 10, 1570, > da9063_buck_a_limits, > DA9063_REG_BUCK_ILIM_C, > DA9063_BCORE2_ILIM_MASK), > DA9063_BUCK_COMMON_FIELDS(BCORE2), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VBCORE2_SEL), > }, > { > DA9063_BUCK(DA9063, BPRO, 530, 10, 1800, > da9063_buck_a_limits, > DA9063_REG_BUCK_ILIM_B, > DA9063_BPRO_ILIM_MASK), > DA9063_BUCK_COMMON_FIELDS(BPRO), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VBPRO_SEL), > }, > { > DA9063_BUCK(DA9063, BMEM, 800, 20, 3340, > da9063_buck_b_limits, > DA9063_REG_BUCK_ILIM_A, > DA9063_BMEM_ILIM_MASK), > DA9063_BUCK_COMMON_FIELDS(BMEM), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VBMEM_SEL), > }, > { > DA9063_BUCK(DA9063, BIO, 800, 20, 3340, > da9063_buck_b_limits, > DA9063_REG_BUCK_ILIM_A, > DA9063_BIO_ILIM_MASK), > DA9063_BUCK_COMMON_FIELDS(BIO), > - .suspend = BFIELD(DA9063_REG_DVC_2, DA9063_VBIO_SEL), > }, > { > DA9063_BUCK(DA9063, BPERI, 800, 20, 3340, > da9063_buck_b_limits, > DA9063_REG_BUCK_ILIM_B, > DA9063_BPERI_ILIM_MASK), > DA9063_BUCK_COMMON_FIELDS(BPERI), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VBPERI_SEL), > }, > { > DA9063_BUCK(DA9063, BCORES_MERGED, 300, 10, 1570, > @@ -508,7 +504,6 @@ static int da9063_ldo_set_suspend_mode(struct > regulator_dev *rdev, unsigned mode > DA9063_REG_BUCK_ILIM_C, > DA9063_BCORE1_ILIM_MASK), > /* BCORES_MERGED uses the same register fields as BCORE1 */ > DA9063_BUCK_COMMON_FIELDS(BCORE1), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VBCORE1_SEL), > }, > { > DA9063_BUCK(DA9063, BMEM_BIO_MERGED, 800, 20, 3340, > @@ -516,21 +511,17 @@ static int da9063_ldo_set_suspend_mode(struct > regulator_dev *rdev, unsigned mode > DA9063_REG_BUCK_ILIM_A, > DA9063_BMEM_ILIM_MASK), > /* BMEM_BIO_MERGED uses the same register fields as BMEM > */ > DA9063_BUCK_COMMON_FIELDS(BMEM), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VBMEM_SEL), > }, > { > DA9063_LDO(DA9063, LDO3, 900, 20, 3440), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VLDO3_SEL), > .oc_event = BFIELD(DA9063_REG_STATUS_D, > DA9063_LDO3_LIM), > }, > { > DA9063_LDO(DA9063, LDO7, 900, 50, 3600), > - .suspend = BFIELD(DA9063_REG_LDO7_CONT, > DA9063_VLDO7_SEL), > .oc_event = BFIELD(DA9063_REG_STATUS_D, > DA9063_LDO7_LIM), > }, > { > DA9063_LDO(DA9063, LDO8, 900, 50, 3600), > - .suspend = BFIELD(DA9063_REG_LDO8_CONT, > DA9063_VLDO8_SEL), > .oc_event = BFIELD(DA9063_REG_STATUS_D, > DA9063_LDO8_LIM), > }, > { > @@ -539,36 +530,29 @@ static int da9063_ldo_set_suspend_mode(struct > regulator_dev *rdev, unsigned mode > }, > { > DA9063_LDO(DA9063, LDO11, 900, 50, 3600), > - .suspend = BFIELD(DA9063_REG_LDO11_CONT, > DA9063_VLDO11_SEL), > .oc_event = BFIELD(DA9063_REG_STATUS_D, > DA9063_LDO11_LIM), > }, > > /* The following LDOs are present only on DA9063, not on DA9063L */ > { > DA9063_LDO(DA9063, LDO1, 600, 20, 1860), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VLDO1_SEL), > }, > { > DA9063_LDO(DA9063, LDO2, 600, 20, 1860), > - .suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VLDO2_SEL), > }, > { > DA9063_LDO(DA9063, LDO4, 900, 20, 3440), > - .suspend = BFIELD(DA9063_REG_DVC_2, DA9063_VLDO4_SEL), > .oc_event = BFIELD(DA9063_REG_STATUS_D, > DA9063_LDO4_LIM), > }, > { > DA9063_LDO(DA9063, LDO5, 900, 50, 3600), > - .suspend = BFIELD(DA9063_REG_LDO5_CONT, > DA9063_VLDO5_SEL), > }, > { > DA9063_LDO(DA9063, LDO6, 900, 50, 3600), > - .suspend = BFIELD(DA9063_REG_LDO6_CONT, > DA9063_VLDO6_SEL), > }, > > { > DA9063_LDO(DA9063, LDO10, 900, 50, 3600), > - .suspend = BFIELD(DA9063_REG_LDO10_CONT, > DA9063_VLDO10_SEL), > }, > }; > > -- > 1.9.1
| |