| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AM6PR10MB2263CCDAED3D22683B96D91580F60@AM6PR10MB2263.EURPRD10.PROD.OUTLOOK.COM>
Date: Tue, 17 Mar 2020 16:48:00 +0000
From: Adam Thomson <Adam.Thomson.Opensource@...semi.com>
To: Martin Fuzzey <martin.fuzzey@...wbird.group>,
Support Opensource <Support.Opensource@...semi.com>
CC: Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] regulator: da9063: fix suspend
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@...wbird.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
Powered by blists - more mailing lists