| Message ID | 20200612104052.1.Idb6dab984884f50e47f061ac36fa89da760babd4@changeid |
|---|---|
| State | Accepted |
| Commit | 8b8c99bd3e1e9463e27ccd1bca18f4a7627901aa |
| Delegated to: | Patrick Delaunay |
| Headers | show |
| Series | power: regulator: stm32: vrefbuf: fix a possible overshoot when re-enabling | expand |
Hi Patrick On 6/12/20 10:40 AM, Patrick Delaunay wrote: > From: Fabrice Gasnier <fabrice.gasnier@st.com> > > There maybe an overshoot: > - when disabling, then re-enabling vrefbuf too quickly > - or upon platform reset as external capacitor maybe slow > discharging (VREFBUF is HiZ at reset by default). > VREFBUF is used by ADC/DAC on some boards. An overshoot on the reference > voltage make the conversions inaccurate for a short period of time. So: > - Don't put the VREFBUF in HiZ when disabling, to force an active > discharge. > - Enforce a 1ms OFF/ON delay, also upon reset > > Penalty is a 1ms delay is applied (even for a cold boot), when enabling > VREFBUF. > > Fixes: 93cf0ae7758d ("power: regulator: Add support for stm32-vrefbuf") > > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> > --- > > drivers/power/regulator/stm32-vrefbuf.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/power/regulator/stm32-vrefbuf.c b/drivers/power/regulator/stm32-vrefbuf.c > index 250773514f..92136961c2 100644 > --- a/drivers/power/regulator/stm32-vrefbuf.c > +++ b/drivers/power/regulator/stm32-vrefbuf.c > @@ -43,8 +43,20 @@ static int stm32_vrefbuf_set_enable(struct udevice *dev, bool enable) > u32 val; > int ret; > > - clrsetbits_le32(priv->base + STM32_VREFBUF_CSR, STM32_HIZ | STM32_ENVR, > - enable ? STM32_ENVR : STM32_HIZ); > + if (enable && !(readl(priv->base + STM32_VREFBUF_CSR) & STM32_ENVR)) { > + /* > + * There maybe an overshoot: > + * - when disabling, then re-enabling vrefbuf too quickly > + * - or upon platform reset as external capacitor maybe slow > + * discharging (VREFBUF is HiZ at reset by default). > + * So force active discharge (HiZ=0) for 1ms before enabling. > + */ > + clrbits_le32(priv->base + STM32_VREFBUF_CSR, STM32_HIZ); > + udelay(1000); > + } > + > + clrsetbits_le32(priv->base + STM32_VREFBUF_CSR, STM32_ENVR, > + enable ? STM32_ENVR : 0); > if (!enable) > return 0; > Reviewed-by: Patrice Chotard <patrice.chotard@st.com> Thanks
On 7/2/20 4:28 PM, Patrice CHOTARD wrote: > Hi Patrick > > On 6/12/20 10:40 AM, Patrick Delaunay wrote: >> From: Fabrice Gasnier <fabrice.gasnier@st.com> >> >> There maybe an overshoot: >> - when disabling, then re-enabling vrefbuf too quickly >> - or upon platform reset as external capacitor maybe slow >> discharging (VREFBUF is HiZ at reset by default). >> VREFBUF is used by ADC/DAC on some boards. An overshoot on the reference >> voltage make the conversions inaccurate for a short period of time. So: >> - Don't put the VREFBUF in HiZ when disabling, to force an active >> discharge. >> - Enforce a 1ms OFF/ON delay, also upon reset >> >> Penalty is a 1ms delay is applied (even for a cold boot), when enabling >> VREFBUF. >> >> Fixes: 93cf0ae7758d ("power: regulator: Add support for stm32-vrefbuf") >> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> >> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> >> --- >> >> drivers/power/regulator/stm32-vrefbuf.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/power/regulator/stm32-vrefbuf.c b/drivers/power/regulator/stm32-vrefbuf.c >> index 250773514f..92136961c2 100644 >> --- a/drivers/power/regulator/stm32-vrefbuf.c >> +++ b/drivers/power/regulator/stm32-vrefbuf.c >> @@ -43,8 +43,20 @@ static int stm32_vrefbuf_set_enable(struct udevice *dev, bool enable) >> u32 val; >> int ret; >> >> - clrsetbits_le32(priv->base + STM32_VREFBUF_CSR, STM32_HIZ | STM32_ENVR, >> - enable ? STM32_ENVR : STM32_HIZ); >> + if (enable && !(readl(priv->base + STM32_VREFBUF_CSR) & STM32_ENVR)) { >> + /* >> + * There maybe an overshoot: >> + * - when disabling, then re-enabling vrefbuf too quickly >> + * - or upon platform reset as external capacitor maybe slow >> + * discharging (VREFBUF is HiZ at reset by default). >> + * So force active discharge (HiZ=0) for 1ms before enabling. >> + */ >> + clrbits_le32(priv->base + STM32_VREFBUF_CSR, STM32_HIZ); >> + udelay(1000); >> + } >> + >> + clrsetbits_le32(priv->base + STM32_VREFBUF_CSR, STM32_ENVR, >> + enable ? STM32_ENVR : 0); >> if (!enable) >> return 0; >> > > Reviewed-by: Patrice Chotard <patrice.chotard@st.com> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> > > Thanks >
diff --git a/drivers/power/regulator/stm32-vrefbuf.c b/drivers/power/regulator/stm32-vrefbuf.c index 250773514f..92136961c2 100644 --- a/drivers/power/regulator/stm32-vrefbuf.c +++ b/drivers/power/regulator/stm32-vrefbuf.c @@ -43,8 +43,20 @@ static int stm32_vrefbuf_set_enable(struct udevice *dev, bool enable) u32 val; int ret; - clrsetbits_le32(priv->base + STM32_VREFBUF_CSR, STM32_HIZ | STM32_ENVR, - enable ? STM32_ENVR : STM32_HIZ); + if (enable && !(readl(priv->base + STM32_VREFBUF_CSR) & STM32_ENVR)) { + /* + * There maybe an overshoot: + * - when disabling, then re-enabling vrefbuf too quickly + * - or upon platform reset as external capacitor maybe slow + * discharging (VREFBUF is HiZ at reset by default). + * So force active discharge (HiZ=0) for 1ms before enabling. + */ + clrbits_le32(priv->base + STM32_VREFBUF_CSR, STM32_HIZ); + udelay(1000); + } + + clrsetbits_le32(priv->base + STM32_VREFBUF_CSR, STM32_ENVR, + enable ? STM32_ENVR : 0); if (!enable) return 0;