[PATCH] power: regulator: stm32: vrefbuf: fix a possible overshoot when re-enabling

[Patrice CHOTARD]

Hi Patrick

On 6/12/20 10:40 AM, Patrick Delaunay wrote:
> From: Fabrice Gasnier <fabrice.gasnier at 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 at st.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at 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 at st.com>

Thanks