[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200831105304.6qcpjfmranay6q4y@pengutronix.de>
Date: Mon, 31 Aug 2020 12:53:04 +0200
From: Marco Felsch <m.felsch@...gutronix.de>
To: Robin Gong <yibin.gong@....com>
Cc: robh+dt@...nel.org, shawnguo@...nel.org, s.hauer@...gutronix.de,
festevam@...il.com, lgirdwood@...il.com, broonie@...nel.org,
Anson.Huang@....com, linux-arm-kernel@...ts.infradead.org,
devicetree@...r.kernel.org, linux-imx@....com,
kernel@...gutronix.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/2] regulator: pca9450: add enable_val for all bucks
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@....com>
> ---
> 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
>
Powered by blists - more mailing lists