Re: [PATCH v1 1/2] regulator: pca9450: add enable_val for all bucks

From: Marco Felsch
Date: Mon Aug 31 2020 - 06:53:21 EST


Hi Robin,

On 20-09-01 00:48, Robin Gong wrote:
> BuckX enable mode
> 00b = OFF
> 01b = ON by PMIC_ON_REQ = H
> 10b = ON by PMIC_ON_REQ = H && PMIC_STBY_REQ = L
> 11b = Always ON
>
> For such enable mode, enable_value should be clearly set in requlator desc,
> 00/11 is not enough, correct it now for different bucks. For example, buck2
> is designed for vddarm which could be off in 'PMIC_STBY_REQ = H' after kernel
> enter suspend, so should be set '10b' as ON, while others is '01b' as ON.
> All are the same as the default setting which means bucks no need to be
> enabled again during kernel boot even if they have been enabled already after
> pmic on.

I wouldn't hard-code the regulator behaviour because the behaviour comes
from the system design which in most cases are comming from our hw-guys.
Till now I saw a few intelligent designs don't following the pmic user
recommendations to save money. I would love to specify the regulator
behaviour/mode within the dt or acpi.

> Signed-off-by: Robin Gong <yibin.gong@xxxxxxx>
> ---
> drivers/regulator/pca9450-regulator.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
> index eb5822b..79f2a5a 100644
> --- a/drivers/regulator/pca9450-regulator.c
> +++ b/drivers/regulator/pca9450-regulator.c
> @@ -249,6 +249,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK1OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK1CTRL,
> .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -273,7 +274,8 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> .vsel_mask = BUCK2OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK2CTRL,
> - .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_mask = BUCK2_ENMODE_MASK,

Unrelated change?

> + .enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -299,6 +301,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK3OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK3CTRL,
> .enable_mask = BUCK3_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -324,6 +327,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK4OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK4CTRL,
> .enable_mask = BUCK4_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -342,6 +346,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK5OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK5CTRL,
> .enable_mask = BUCK5_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -360,6 +365,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK6OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK6CTRL,
> .enable_mask = BUCK6_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -475,6 +481,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_mask = BUCK1OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK1CTRL,
> .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -499,7 +506,8 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> .vsel_mask = BUCK2OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK2CTRL,
> - .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_mask = BUCK2_ENMODE_MASK,

Unrelated change?

Regards,
Marco
> + .enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -525,6 +533,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_mask = BUCK4OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK4CTRL,
> .enable_mask = BUCK4_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -543,6 +552,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_mask = BUCK5OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK5CTRL,
> .enable_mask = BUCK5_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -561,6 +571,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_mask = BUCK6OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK6CTRL,
> .enable_mask = BUCK6_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> --
> 2.7.4
>