Re: [PATCH] drivers: nvmem: Fix voltage settings for QTI qfprom-efuse

[Rajendra Nayak <rnayak@codeaurora.org>]

On 2/5/2021 8:25 PM, Doug Anderson wrote:
> Hi,
> 
> On Fri, Feb 5, 2021 at 3:29 AM Ravi Kumar Bokka <rbokka@codeaurora.org> wrote:
>>
>> QFPROM controller hardware requires 1.8V min for fuse blowing.
>> So, this change sets the voltage to 1.8V, required to blow the fuse
>> for qfprom-efuse controller.
>>
>> To disable fuse blowing, we set the voltage to 0V since this may
>> be a shared rail and may be able to run at a lower rate when we're
>> not blowing fuses.
>>
>> Fixes: 93b4e49f8c86 ("nvmem: qfprom: Add fuse blowing support")
>> Reported-by: Douglas Anderson <dianders@chromium.org>
>> Suggested-by: Douglas Anderson <dianders@chromium.org>
>> Signed-off-by: Ravi Kumar Bokka <rbokka@codeaurora.org>
>> ---
>>   drivers/nvmem/qfprom.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
>> index 6cace24..100d69d 100644
>> --- a/drivers/nvmem/qfprom.c
>> +++ b/drivers/nvmem/qfprom.c
>> @@ -127,6 +127,16 @@ static void qfprom_disable_fuse_blowing(const struct qfprom_priv *priv,
>>   {
>>          int ret;
>>
>> +       /*
>> +        * This may be a shared rail and may be able to run at a lower rate
>> +        * when we're not blowing fuses.  At the moment, the regulator framework
>> +        * applies voltage constraints even on disabled rails, so remove our
>> +        * constraints and allow the rail to be adjusted by other users.
> 
> Some year maybe I'll try to fix the regulator framework to not count
> voltage constraints for disbled rails, or perhaps have it be optional.
> ;-)  In theory it should be much easier after the patches we already
> landed not to count current requests for disabled rails...
> 
> 
>> +        */
>> +       ret = regulator_set_voltage(priv->vcc, 0, INT_MAX);
>> +       if (ret)
>> +               dev_warn(priv->dev, "Failed to set 0 voltage (ignoring)\n");
>> +
>>          ret = regulator_disable(priv->vcc);
>>          if (ret)
>>                  dev_warn(priv->dev, "Failed to disable regulator (ignoring)\n");
>> @@ -172,6 +182,17 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
>>                  goto err_clk_prepared;
>>          }
>>
>> +       /*
>> +        * Hardware requires 1.8V min for fuse blowing; this may be
>> +        * a rail shared do don't specify a max--regulator constraints
>> +        * will handle.
>> +        */
>> +       ret = regulator_set_voltage(priv->vcc, 1800000, INT_MAX);
>> +       if (ret) {
>> +               dev_err(priv->dev, "Failed to set 1.8 voltage\n");
>> +               goto err_clk_rate_set;
>> +       }
>> +
> 
> Looks right to me.  Assuming that this works.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Srini, any plans to queue this up for merge?  

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation