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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 5 Feb 2021 06:55:45 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Ravi Kumar Bokka <rbokka@...eaurora.org>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>,
        dhavalp@...eaurora.org, mturney@...eaurora.org
Subject: Re: [PATCH] drivers: nvmem: Fix voltage settings for QTI qfprom-efuse

Hi,

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