Re: [PATCH v3] arm64: dts: qcom: sdm845-xiaomi-beryllium: Add DSI and panel bits

[Amit Pundir]

Hi Angelo,

On Thu, 11 Feb 2021 at 00:25, AngeloGioacchino Del Regno
<angelogioacchino.delregno@xxxxxxxxxxxxxx> wrote:
>
> Il 10/02/21 09:18, Amit Pundir ha scritto:
> > From: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
> >
> > Enabling the Display panel for beryllium requires DSI
> > labibb regulators and panel dts nodes to be added.
> > It is also required to keep some of the regulators as
> > always-on.
> >
> > Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
> > Signed-off-by: Amit Pundir <amit.pundir@xxxxxxxxxx>
> > ---
>
> Hello!
> Your patch looks good, however, I have a few concerns...
>
> > +&ibb {
> > +     regulator-min-microvolt = <4600000>;
> > +     regulator-max-microvolt = <6000000>;
> > +};
> > +
>
> I think you want to also configure overvoltage and overcurrent
> protection values for both LAB and IBB, as these regulators may be a bit
> dangerous if used without.

It turned out I can't boot if I set regulator-max-microamp lab/ibb
values from downstream DT. Verified it a few times and same result.
Here is the decompiled downstream DTB for reference:
https://pastebin.ubuntu.com/p/4nPxzq6zf7/, just in case.

I'm not sure if over current protection value is tied to max-microamp,
but I added it anyway along with pull-down and soft-start properties
in next version of this patch.

Regards,
Amit Pundir

> Besides that, even if it wouldn't be that dangerous, since the
> protection features are present, it would be nice to configure them
> properly as in the rare event that something bad happens, you would be
> able to save the hardware (or at least have a chance to!).
>
> > +&lab {
> > +     regulator-min-microvolt = <4600000>;
> > +     regulator-max-microvolt = <6000000>;
> > +};
> > +
>
> Same here.
>
> Yours,
> -- Angelo