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

[Doug Anderson]

Hi,

On Fri, Feb 5, 2021 at 3:29 AM Ravi Kumar Bokka <rbokka@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> Suggested-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> Signed-off-by: Ravi Kumar Bokka <rbokka@xxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxx>