linux-kernel - Re: [PATCH 2/2] nvmem: qfprom: Add support for fuse blowing on sc7280

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]

Message-ID: <CAD=FV=VT6xUmUK7Kss8cpF1wjw9tLx67wJMtiV6yTDaVYXXrqA@mail.gmail.com>
Date:   Tue, 30 Mar 2021 14:19:15 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Rajendra Nayak <rnayak@...eaurora.org>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        Ravi Kumar Bokka <rbokka@...eaurora.org>
Subject: Re: [PATCH 2/2] nvmem: qfprom: Add support for fuse blowing on sc7280

Hi,

On Wed, Mar 24, 2021 at 10:45 PM Rajendra Nayak <rnayak@...eaurora.org> wrote:
>
> @@ -111,6 +113,15 @@ static const struct qfprom_soc_compatible_data sc7180_qfprom = {
>         .nkeepout = ARRAY_SIZE(sc7180_qfprom_keepout)
>  };
>
> +static const struct nvmem_keepout sc7280_qfprom_keepout[] = {
> +       {.start = 0x128, .end = 0x148},
> +       {.start = 0x238, .end = 0x248}
> +};
> +
> +static const struct qfprom_soc_compatible_data sc7280_qfprom = {
> +       .keepout = sc7280_qfprom_keepout,
> +       .nkeepout = ARRAY_SIZE(sc7280_qfprom_keepout)
> +};
>  /**

nit: blank line between structure and comment?


> @@ -187,9 +199,9 @@ static int qfprom_enable_fuse_blowing(const struct qfprom_priv *priv,
>          * a rail shared do don't specify a max--regulator constraints
>          * will handle.
>          */
> -       ret = regulator_set_voltage(priv->vcc, 1800000, INT_MAX);
> +       ret = regulator_set_voltage(priv->vcc, qfprom_blow_uV, INT_MAX);
>         if (ret) {
> -               dev_err(priv->dev, "Failed to set 1.8 voltage\n");
> +               dev_err(priv->dev, "Failed to set %duV\n", qfprom_blow_uV);

nit: the comment above this block (not in the unified diff)
specifically calls out 1.8V. It'd be nice if you updated the comment
since it's no longer fixed at 1.8V.


> @@ -379,6 +399,8 @@ static int qfprom_probe(struct platform_device *pdev)
>
>                 if (major_version == 7 && minor_version == 8)
>                         priv->soc_data = &qfprom_7_8_data;
> +               if (major_version == 7 && minor_version == 15)
> +                       priv->soc_data = &qfprom_7_15_data;

nit: "else if" instead of "if"?


I guess I'm a little late since I think this already got applied, but
all the above are nits. Maybe you could send a follow-up patch to
address them?

-Doug