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.gasn...@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.gasn...@st.com>
>> Signed-off-by: Patrick Delaunay <patrick.delau...@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.chot...@st.com>
Reviewed-by: Jaehoon Chung <jh80.ch...@samsung.com>
>
> Thanks
>
