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

[Jaehoon Chung]

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 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>

Reviewed-by: Jaehoon Chung <jh80.chung at samsung.com>

> 
> Thanks
>