Re: [PATCH] arm64: dts: qcom: sc7280-idp: Add vcc-supply for qfprom

[Doug Anderson <dianders@chromium.org>]

Hi,

On Mon, Sep 13, 2021 at 12:57 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Rajendra Nayak (2021-09-13 03:58:55)
> > Add vcc-supply for the IDP boards that was missed when the
> > qfprom device tree properties were added for the sc7280 SoC.
> >
> > Fixes: c1b2189a19cf ("arm64: dts: qcom: sc7280: Add qfprom node")
> > Reported-by: satya priya <skakit@codeaurora.org>
> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> > ---
>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>
> >  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > index 371a2a9..99f9ee5 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > @@ -207,6 +207,10 @@
> >         };
> >  };
> >
> > +&qfprom {
>
> Maybe that node should also be marked status = "disabled" by default so
> that it can only be marked OK if the board has setup the regulator
> properly?

I don't think that's a good idea. You still want to be able to use the
nvmem in "read only" mode and it doesn't seem to need the regulator
for that. This is only for burning.

I believe that actually everything will work out fine-ish for boards
that leave this regulator off. They'll get a dummy regulator. When
they start trying to burn fuses and the call the
regulator_set_voltage() happens then that'll fail (you can't do that
on a dummy) and we should error out of writing. Reading will work
fine. It would probably be better if qfprom_probe() used
devm_regulator_get_optional() though and fell back to read-only mode.
I'm happy to review a patch that does that.

-Doug