* [PATCH] pmic: pca9450: permit config on all bucks and LDOs
@ 2022-06-20 3:49 Heiko Thiery
2022-06-20 14:49 ` Fabio Estevam
[not found] ` <CGME20220726082032epcas1p2cc28cb27c7791d8a4495325ca0abb268@epcas1p2.samsung.com>
0 siblings, 2 replies; 4+ messages in thread
From: Heiko Thiery @ 2022-06-20 3:49 UTC (permalink / raw)
To: u-boot
Cc: Jaehoon Chung, Heiko Thiery, Marek Vasut, Fabio Estevam,
Michael Nazzareno Trimarchi
In order to have the possibility to configure the regulators at system
startup through DM support, all LDOs and bucks must be able to be
changeable. Currently there is a limitation to change the values when
the output is enabled. Since the driver is based on the ROHM BD71837 and a
comment that describes a limitation about switching while the output is
enabled can also be found there, the limitation probably comes from this type.
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
drivers/power/regulator/pca9450.c | 42 ++++++++++---------------------
1 file changed, 13 insertions(+), 29 deletions(-)
diff --git a/drivers/power/regulator/pca9450.c b/drivers/power/regulator/pca9450.c
index 23badaa332..fe1869397c 100644
--- a/drivers/power/regulator/pca9450.c
+++ b/drivers/power/regulator/pca9450.c
@@ -44,7 +44,6 @@ struct pca9450_vrange {
* @ranges: pointer to ranges of regulator voltages and matching register
* values
* @numranges: number of voltage ranges pointed by ranges
- * @dvs: whether the voltage can be changed when regulator is enabled
*/
struct pca9450_plat {
const char *name;
@@ -54,7 +53,6 @@ struct pca9450_plat {
u8 volt_mask;
struct pca9450_vrange *ranges;
unsigned int numranges;
- bool dvs;
};
#define PCA_RANGE(_min, _vstep, _sel_low, _sel_hi) \
@@ -63,11 +61,11 @@ struct pca9450_plat {
.min_sel = (_sel_low), .max_sel = (_sel_hi), \
}
-#define PCA_DATA(_name, enreg, enmask, vreg, vmask, _range, _dvs) \
+#define PCA_DATA(_name, enreg, enmask, vreg, vmask, _range) \
{ \
.name = (_name), .enable_reg = (enreg), .enablemask = (enmask), \
.volt_reg = (vreg), .volt_mask = (vmask), .ranges = (_range), \
- .numranges = ARRAY_SIZE(_range), .dvs = (_dvs), \
+ .numranges = ARRAY_SIZE(_range) \
}
static struct pca9450_vrange pca9450_buck123_vranges[] = {
@@ -107,39 +105,39 @@ static struct pca9450_plat pca9450_reg_data[] = {
/* Bucks 1-3 which support dynamic voltage scaling */
PCA_DATA("BUCK1", PCA9450_BUCK1CTRL, HW_STATE_CONTROL,
PCA9450_BUCK1OUT_DVS0, PCA9450_DVS_BUCK_RUN_MASK,
- pca9450_buck123_vranges, true),
+ pca9450_buck123_vranges),
PCA_DATA("BUCK2", PCA9450_BUCK2CTRL, HW_STATE_CONTROL,
PCA9450_BUCK2OUT_DVS0, PCA9450_DVS_BUCK_RUN_MASK,
- pca9450_buck123_vranges, true),
+ pca9450_buck123_vranges),
PCA_DATA("BUCK3", PCA9450_BUCK3CTRL, HW_STATE_CONTROL,
PCA9450_BUCK3OUT_DVS0, PCA9450_DVS_BUCK_RUN_MASK,
- pca9450_buck123_vranges, true),
+ pca9450_buck123_vranges),
/* Bucks 4-6 which do not support dynamic voltage scaling */
PCA_DATA("BUCK4", PCA9450_BUCK4CTRL, HW_STATE_CONTROL,
PCA9450_BUCK4OUT, PCA9450_DVS_BUCK_RUN_MASK,
- pca9450_buck456_vranges, false),
+ pca9450_buck456_vranges),
PCA_DATA("BUCK5", PCA9450_BUCK5CTRL, HW_STATE_CONTROL,
PCA9450_BUCK5OUT, PCA9450_DVS_BUCK_RUN_MASK,
- pca9450_buck456_vranges, false),
+ pca9450_buck456_vranges),
PCA_DATA("BUCK6", PCA9450_BUCK6CTRL, HW_STATE_CONTROL,
PCA9450_BUCK6OUT, PCA9450_DVS_BUCK_RUN_MASK,
- pca9450_buck456_vranges, false),
+ pca9450_buck456_vranges),
/* LDOs */
PCA_DATA("LDO1", PCA9450_LDO1CTRL, HW_STATE_CONTROL,
PCA9450_LDO1CTRL, PCA9450_LDO12_MASK,
- pca9450_ldo1_vranges, false),
+ pca9450_ldo1_vranges),
PCA_DATA("LDO2", PCA9450_LDO2CTRL, HW_STATE_CONTROL,
PCA9450_LDO2CTRL, PCA9450_LDO12_MASK,
- pca9450_ldo2_vranges, false),
+ pca9450_ldo2_vranges),
PCA_DATA("LDO3", PCA9450_LDO3CTRL, HW_STATE_CONTROL,
PCA9450_LDO3CTRL, PCA9450_LDO34_MASK,
- pca9450_ldo34_vranges, false),
+ pca9450_ldo34_vranges),
PCA_DATA("LDO4", PCA9450_LDO4CTRL, HW_STATE_CONTROL,
PCA9450_LDO4CTRL, PCA9450_LDO34_MASK,
- pca9450_ldo34_vranges, false),
+ pca9450_ldo34_vranges),
PCA_DATA("LDO5", PCA9450_LDO5CTRL_H, HW_STATE_CONTROL,
PCA9450_LDO5CTRL_H, PCA9450_LDO5_MASK,
- pca9450_ldo5_vranges, false),
+ pca9450_ldo5_vranges),
};
static int vrange_find_value(struct pca9450_vrange *r, unsigned int sel,
@@ -246,20 +244,6 @@ static int pca9450_set_value(struct udevice *dev, int uvolt)
unsigned int sel;
int i, found = 0;
- /*
- * An under/overshooting may occur if voltage is changed for other
- * regulators but buck 1,2,3 or 4 when regulator is enabled. Prevent
- * change to protect the HW
- */
- if (!plat->dvs)
- if (pca9450_get_enable(dev)) {
- /* If the value is already set, skip the warning. */
- if (pca9450_get_value(dev) == uvolt)
- return 0;
- pr_err("Only DVS bucks can be changed when enabled\n");
- return -EINVAL;
- }
-
for (i = 0; i < plat->numranges; i++) {
struct pca9450_vrange *r = &plat->ranges[i];
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] pmic: pca9450: permit config on all bucks and LDOs
2022-06-20 3:49 [PATCH] pmic: pca9450: permit config on all bucks and LDOs Heiko Thiery
@ 2022-06-20 14:49 ` Fabio Estevam
[not found] ` <CGME20220726082032epcas1p2cc28cb27c7791d8a4495325ca0abb268@epcas1p2.samsung.com>
1 sibling, 0 replies; 4+ messages in thread
From: Fabio Estevam @ 2022-06-20 14:49 UTC (permalink / raw)
To: Heiko Thiery
Cc: u-boot, Jaehoon Chung, Marek Vasut, Michael Nazzareno Trimarchi
On 20/06/2022 00:49, Heiko Thiery wrote:
> In order to have the possibility to configure the regulators at system
> startup through DM support, all LDOs and bucks must be able to be
> changeable. Currently there is a limitation to change the values when
> the output is enabled. Since the driver is based on the ROHM BD71837
> and a
> comment that describes a limitation about switching while the output is
> enabled can also be found there, the limitation probably comes from
> this type.
>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
Reviewed-by: Fabio Estevam <festevam@denx.de>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pmic: pca9450: permit config on all bucks and LDOs
[not found] ` <CGME20220726082032epcas1p2cc28cb27c7791d8a4495325ca0abb268@epcas1p2.samsung.com>
@ 2022-07-26 8:20 ` Jaehoon Chung
2022-08-22 10:19 ` Michael Nazzareno Trimarchi
0 siblings, 1 reply; 4+ messages in thread
From: Jaehoon Chung @ 2022-07-26 8:20 UTC (permalink / raw)
To: Heiko Thiery, u-boot
Cc: Marek Vasut, Fabio Estevam, Michael Nazzareno Trimarchi
On 6/20/22 12:49, Heiko Thiery wrote:
> In order to have the possibility to configure the regulators at system
> startup through DM support, all LDOs and bucks must be able to be
> changeable. Currently there is a limitation to change the values when
> the output is enabled. Since the driver is based on the ROHM BD71837 and a
> comment that describes a limitation about switching while the output is
> enabled can also be found there, the limitation probably comes from this type.
>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> Reviewed-by: Fabio Estevam <festevam@denx.de>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Best Regards,
Jaehoon Chung
> ---
> drivers/power/regulator/pca9450.c | 42 ++++++++++---------------------
> 1 file changed, 13 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/power/regulator/pca9450.c b/drivers/power/regulator/pca9450.c
> index 23badaa332..fe1869397c 100644
> --- a/drivers/power/regulator/pca9450.c
> +++ b/drivers/power/regulator/pca9450.c
> @@ -44,7 +44,6 @@ struct pca9450_vrange {
> * @ranges: pointer to ranges of regulator voltages and matching register
> * values
> * @numranges: number of voltage ranges pointed by ranges
> - * @dvs: whether the voltage can be changed when regulator is enabled
> */
> struct pca9450_plat {
> const char *name;
> @@ -54,7 +53,6 @@ struct pca9450_plat {
> u8 volt_mask;
> struct pca9450_vrange *ranges;
> unsigned int numranges;
> - bool dvs;
> };
>
> #define PCA_RANGE(_min, _vstep, _sel_low, _sel_hi) \
> @@ -63,11 +61,11 @@ struct pca9450_plat {
> .min_sel = (_sel_low), .max_sel = (_sel_hi), \
> }
>
> -#define PCA_DATA(_name, enreg, enmask, vreg, vmask, _range, _dvs) \
> +#define PCA_DATA(_name, enreg, enmask, vreg, vmask, _range) \
> { \
> .name = (_name), .enable_reg = (enreg), .enablemask = (enmask), \
> .volt_reg = (vreg), .volt_mask = (vmask), .ranges = (_range), \
> - .numranges = ARRAY_SIZE(_range), .dvs = (_dvs), \
> + .numranges = ARRAY_SIZE(_range) \
> }
>
> static struct pca9450_vrange pca9450_buck123_vranges[] = {
> @@ -107,39 +105,39 @@ static struct pca9450_plat pca9450_reg_data[] = {
> /* Bucks 1-3 which support dynamic voltage scaling */
> PCA_DATA("BUCK1", PCA9450_BUCK1CTRL, HW_STATE_CONTROL,
> PCA9450_BUCK1OUT_DVS0, PCA9450_DVS_BUCK_RUN_MASK,
> - pca9450_buck123_vranges, true),
> + pca9450_buck123_vranges),
> PCA_DATA("BUCK2", PCA9450_BUCK2CTRL, HW_STATE_CONTROL,
> PCA9450_BUCK2OUT_DVS0, PCA9450_DVS_BUCK_RUN_MASK,
> - pca9450_buck123_vranges, true),
> + pca9450_buck123_vranges),
> PCA_DATA("BUCK3", PCA9450_BUCK3CTRL, HW_STATE_CONTROL,
> PCA9450_BUCK3OUT_DVS0, PCA9450_DVS_BUCK_RUN_MASK,
> - pca9450_buck123_vranges, true),
> + pca9450_buck123_vranges),
> /* Bucks 4-6 which do not support dynamic voltage scaling */
> PCA_DATA("BUCK4", PCA9450_BUCK4CTRL, HW_STATE_CONTROL,
> PCA9450_BUCK4OUT, PCA9450_DVS_BUCK_RUN_MASK,
> - pca9450_buck456_vranges, false),
> + pca9450_buck456_vranges),
> PCA_DATA("BUCK5", PCA9450_BUCK5CTRL, HW_STATE_CONTROL,
> PCA9450_BUCK5OUT, PCA9450_DVS_BUCK_RUN_MASK,
> - pca9450_buck456_vranges, false),
> + pca9450_buck456_vranges),
> PCA_DATA("BUCK6", PCA9450_BUCK6CTRL, HW_STATE_CONTROL,
> PCA9450_BUCK6OUT, PCA9450_DVS_BUCK_RUN_MASK,
> - pca9450_buck456_vranges, false),
> + pca9450_buck456_vranges),
> /* LDOs */
> PCA_DATA("LDO1", PCA9450_LDO1CTRL, HW_STATE_CONTROL,
> PCA9450_LDO1CTRL, PCA9450_LDO12_MASK,
> - pca9450_ldo1_vranges, false),
> + pca9450_ldo1_vranges),
> PCA_DATA("LDO2", PCA9450_LDO2CTRL, HW_STATE_CONTROL,
> PCA9450_LDO2CTRL, PCA9450_LDO12_MASK,
> - pca9450_ldo2_vranges, false),
> + pca9450_ldo2_vranges),
> PCA_DATA("LDO3", PCA9450_LDO3CTRL, HW_STATE_CONTROL,
> PCA9450_LDO3CTRL, PCA9450_LDO34_MASK,
> - pca9450_ldo34_vranges, false),
> + pca9450_ldo34_vranges),
> PCA_DATA("LDO4", PCA9450_LDO4CTRL, HW_STATE_CONTROL,
> PCA9450_LDO4CTRL, PCA9450_LDO34_MASK,
> - pca9450_ldo34_vranges, false),
> + pca9450_ldo34_vranges),
> PCA_DATA("LDO5", PCA9450_LDO5CTRL_H, HW_STATE_CONTROL,
> PCA9450_LDO5CTRL_H, PCA9450_LDO5_MASK,
> - pca9450_ldo5_vranges, false),
> + pca9450_ldo5_vranges),
> };
>
> static int vrange_find_value(struct pca9450_vrange *r, unsigned int sel,
> @@ -246,20 +244,6 @@ static int pca9450_set_value(struct udevice *dev, int uvolt)
> unsigned int sel;
> int i, found = 0;
>
> - /*
> - * An under/overshooting may occur if voltage is changed for other
> - * regulators but buck 1,2,3 or 4 when regulator is enabled. Prevent
> - * change to protect the HW
> - */
> - if (!plat->dvs)
> - if (pca9450_get_enable(dev)) {
> - /* If the value is already set, skip the warning. */
> - if (pca9450_get_value(dev) == uvolt)
> - return 0;
> - pr_err("Only DVS bucks can be changed when enabled\n");
> - return -EINVAL;
> - }
> -
> for (i = 0; i < plat->numranges; i++) {
> struct pca9450_vrange *r = &plat->ranges[i];
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pmic: pca9450: permit config on all bucks and LDOs
2022-07-26 8:20 ` Jaehoon Chung
@ 2022-08-22 10:19 ` Michael Nazzareno Trimarchi
0 siblings, 0 replies; 4+ messages in thread
From: Michael Nazzareno Trimarchi @ 2022-08-22 10:19 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Heiko Thiery, u-boot, Marek Vasut, Fabio Estevam
Hi Heiko
On Tue, Jul 26, 2022 at 10:20 AM Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
> On 6/20/22 12:49, Heiko Thiery wrote:
> > In order to have the possibility to configure the regulators at system
> > startup through DM support, all LDOs and bucks must be able to be
> > changeable. Currently there is a limitation to change the values when
> > the output is enabled. Since the driver is based on the ROHM BD71837 and a
> > comment that describes a limitation about switching while the output is
> > enabled can also be found there, the limitation probably comes from this type.
> >
> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> > Reviewed-by: Fabio Estevam <festevam@denx.de>
>
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>
I think that this can be valid even for bd71837. Are you agree?
Michael
> Best Regards,
> Jaehoon Chung
>
> > ---
> > drivers/power/regulator/pca9450.c | 42 ++++++++++---------------------
> > 1 file changed, 13 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/power/regulator/pca9450.c b/drivers/power/regulator/pca9450.c
> > index 23badaa332..fe1869397c 100644
> > --- a/drivers/power/regulator/pca9450.c
> > +++ b/drivers/power/regulator/pca9450.c
> > @@ -44,7 +44,6 @@ struct pca9450_vrange {
> > * @ranges: pointer to ranges of regulator voltages and matching register
> > * values
> > * @numranges: number of voltage ranges pointed by ranges
> > - * @dvs: whether the voltage can be changed when regulator is enabled
> > */
> > struct pca9450_plat {
> > const char *name;
> > @@ -54,7 +53,6 @@ struct pca9450_plat {
> > u8 volt_mask;
> > struct pca9450_vrange *ranges;
> > unsigned int numranges;
> > - bool dvs;
> > };
> >
> > #define PCA_RANGE(_min, _vstep, _sel_low, _sel_hi) \
> > @@ -63,11 +61,11 @@ struct pca9450_plat {
> > .min_sel = (_sel_low), .max_sel = (_sel_hi), \
> > }
> >
> > -#define PCA_DATA(_name, enreg, enmask, vreg, vmask, _range, _dvs) \
> > +#define PCA_DATA(_name, enreg, enmask, vreg, vmask, _range) \
> > { \
> > .name = (_name), .enable_reg = (enreg), .enablemask = (enmask), \
> > .volt_reg = (vreg), .volt_mask = (vmask), .ranges = (_range), \
> > - .numranges = ARRAY_SIZE(_range), .dvs = (_dvs), \
> > + .numranges = ARRAY_SIZE(_range) \
> > }
> >
> > static struct pca9450_vrange pca9450_buck123_vranges[] = {
> > @@ -107,39 +105,39 @@ static struct pca9450_plat pca9450_reg_data[] = {
> > /* Bucks 1-3 which support dynamic voltage scaling */
> > PCA_DATA("BUCK1", PCA9450_BUCK1CTRL, HW_STATE_CONTROL,
> > PCA9450_BUCK1OUT_DVS0, PCA9450_DVS_BUCK_RUN_MASK,
> > - pca9450_buck123_vranges, true),
> > + pca9450_buck123_vranges),
> > PCA_DATA("BUCK2", PCA9450_BUCK2CTRL, HW_STATE_CONTROL,
> > PCA9450_BUCK2OUT_DVS0, PCA9450_DVS_BUCK_RUN_MASK,
> > - pca9450_buck123_vranges, true),
> > + pca9450_buck123_vranges),
> > PCA_DATA("BUCK3", PCA9450_BUCK3CTRL, HW_STATE_CONTROL,
> > PCA9450_BUCK3OUT_DVS0, PCA9450_DVS_BUCK_RUN_MASK,
> > - pca9450_buck123_vranges, true),
> > + pca9450_buck123_vranges),
> > /* Bucks 4-6 which do not support dynamic voltage scaling */
> > PCA_DATA("BUCK4", PCA9450_BUCK4CTRL, HW_STATE_CONTROL,
> > PCA9450_BUCK4OUT, PCA9450_DVS_BUCK_RUN_MASK,
> > - pca9450_buck456_vranges, false),
> > + pca9450_buck456_vranges),
> > PCA_DATA("BUCK5", PCA9450_BUCK5CTRL, HW_STATE_CONTROL,
> > PCA9450_BUCK5OUT, PCA9450_DVS_BUCK_RUN_MASK,
> > - pca9450_buck456_vranges, false),
> > + pca9450_buck456_vranges),
> > PCA_DATA("BUCK6", PCA9450_BUCK6CTRL, HW_STATE_CONTROL,
> > PCA9450_BUCK6OUT, PCA9450_DVS_BUCK_RUN_MASK,
> > - pca9450_buck456_vranges, false),
> > + pca9450_buck456_vranges),
> > /* LDOs */
> > PCA_DATA("LDO1", PCA9450_LDO1CTRL, HW_STATE_CONTROL,
> > PCA9450_LDO1CTRL, PCA9450_LDO12_MASK,
> > - pca9450_ldo1_vranges, false),
> > + pca9450_ldo1_vranges),
> > PCA_DATA("LDO2", PCA9450_LDO2CTRL, HW_STATE_CONTROL,
> > PCA9450_LDO2CTRL, PCA9450_LDO12_MASK,
> > - pca9450_ldo2_vranges, false),
> > + pca9450_ldo2_vranges),
> > PCA_DATA("LDO3", PCA9450_LDO3CTRL, HW_STATE_CONTROL,
> > PCA9450_LDO3CTRL, PCA9450_LDO34_MASK,
> > - pca9450_ldo34_vranges, false),
> > + pca9450_ldo34_vranges),
> > PCA_DATA("LDO4", PCA9450_LDO4CTRL, HW_STATE_CONTROL,
> > PCA9450_LDO4CTRL, PCA9450_LDO34_MASK,
> > - pca9450_ldo34_vranges, false),
> > + pca9450_ldo34_vranges),
> > PCA_DATA("LDO5", PCA9450_LDO5CTRL_H, HW_STATE_CONTROL,
> > PCA9450_LDO5CTRL_H, PCA9450_LDO5_MASK,
> > - pca9450_ldo5_vranges, false),
> > + pca9450_ldo5_vranges),
> > };
> >
> > static int vrange_find_value(struct pca9450_vrange *r, unsigned int sel,
> > @@ -246,20 +244,6 @@ static int pca9450_set_value(struct udevice *dev, int uvolt)
> > unsigned int sel;
> > int i, found = 0;
> >
> > - /*
> > - * An under/overshooting may occur if voltage is changed for other
> > - * regulators but buck 1,2,3 or 4 when regulator is enabled. Prevent
> > - * change to protect the HW
> > - */
> > - if (!plat->dvs)
> > - if (pca9450_get_enable(dev)) {
> > - /* If the value is already set, skip the warning. */
> > - if (pca9450_get_value(dev) == uvolt)
> > - return 0;
> > - pr_err("Only DVS bucks can be changed when enabled\n");
> > - return -EINVAL;
> > - }
> > -
> > for (i = 0; i < plat->numranges; i++) {
> > struct pca9450_vrange *r = &plat->ranges[i];
> >
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-22 10:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-20 3:49 [PATCH] pmic: pca9450: permit config on all bucks and LDOs Heiko Thiery
2022-06-20 14:49 ` Fabio Estevam
[not found] ` <CGME20220726082032epcas1p2cc28cb27c7791d8a4495325ca0abb268@epcas1p2.samsung.com>
2022-07-26 8:20 ` Jaehoon Chung
2022-08-22 10:19 ` Michael Nazzareno Trimarchi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.