[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGXv+5H_=He4+vBcaoVbtO05c6hPWV46OTRfR+S8XVuvk_EaYg@mail.gmail.com>
Date: Tue, 10 Dec 2024 16:40:08 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: Matthias Brugger <matthias.bgg@...il.com>, devicetree@...r.kernel.org,
linux-mediatek@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH 1/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop
pp3300_panel voltage settings
On Tue, Dec 10, 2024 at 4:27 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com> wrote:
>
> Il 04/11/24 14:47, Chen-Yu Tsai ha scritto:
> > On Mon, Nov 4, 2024 at 9:19 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@...labora.com> wrote:
> >>
> >> Il 04/11/24 14:00, Chen-Yu Tsai ha scritto:
> >>> On Wed, Oct 30, 2024 at 3:02 PM Chen-Yu Tsai <wenst@...omium.org> wrote:
> >>>>
> >>>> The pp3300_panel fixed regulator is just a load switch. It does not have
> >>>> any regulating capabilities. Thus having voltage constraints on it is
> >>>> wrong.
> >>>>
> >>>> Remove the voltage constraints.
> >>>>
> >>>> Fixes: cabc71b08eb5 ("arm64: dts: mt8183: Add kukui-jacuzzi-damu board")
> >>>> Signed-off-by: Chen-Yu Tsai <wenst@...omium.org>
> >>>
> >>> I see that the other three patches were merged and included in the pull
> >>> request, but not this one. Were there any concerns?
> >>>
> >>
> >> Sorry I forgot to actually provide an explanation for that - yes, I do have some
> >> comment about this one.
> >>
> >> Despite this being a load switch, it's still switching power from regulator A to
> >> target device X, so this is technically still providing 3.3V to device X.
> >>
> >> Think about how a "regular" full-fledged regulator works: you can (sometimes) set
> >> a voltage, and then you can ENABLE the VOUT for said regulator (/rail): this kind
> >> of "load switch" does exactly the same as the ENABLE switch for a full-fledged
> >> regulator.
> >
> > But it does not provide regulation. One cannot "set" the voltage on a load
> > switch; one can only set it on its upstream supply, if that supply provides
> > regulation.
> >
> > IIRC Mark said some years ago that if a regulator doesn't regulate the
> > voltage, then the voltage constraints should not be given. The constraints
> > are then derived from its upstream supply.
> >
> > That's the guideline I've followed for all the regulator related changes
> > I've done over the years. Does that work for you?
> >
>
> Ok, let's go with that then.
>
> There's only one problem - patches 2 to 4 don't apply for whatever reason, is there
> any dependency?
I believe they were already applied in the previous cycle? At least
they were dropped from my branch automatically after a recent rebase.
ChenYu
> Cheers,
> Angelo
>
> >> So, this is switching on and off a power rail that is derived from a source rail,
> >> practically creating... well, a "new" rail, with...
> >>
> >> VIN=somewhere-3.3v,
> >> VOUT=somewhere-still-3.3v
> >>
> >> Any objections/doubts/etc? :-)
> >
> > I agree with most of it, except the part that I laid out above about the
> > load switch not providing regulation.
> >
> >> P.S.: I'm writing fast, sorry if anything appears unclear, feel free to shoot more
> >> questions in case :-)
> >
> > No, it's pretty clear, and I believe one of the common interpretations
> > I see. Thank you for the quick response.
> >
> >
> > Thanks
> > ChenYu
> >
> >> Cheers,
> >> Angelo
> >>
> >>>
> >>> ChenYu
> >>>
> >>>> ---
> >>>> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi | 2 --
> >>>> 1 file changed, 2 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> >>>> index 783c333107bc..7bbafe926558 100644
> >>>> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> >>>> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
> >>>> @@ -35,8 +35,6 @@ pp1800_mipibrdg: pp1800-mipibrdg {
> >>>> pp3300_panel: pp3300-panel {
> >>>> compatible = "regulator-fixed";
> >>>> regulator-name = "pp3300_panel";
> >>>> - regulator-min-microvolt = <3300000>;
> >>>> - regulator-max-microvolt = <3300000>;
> >>>> pinctrl-names = "default";
> >>>> pinctrl-0 = <&pp3300_panel_pins>;
> >>>>
> >>>> --
> >>>> 2.47.0.163.g1226f6d8fa-goog
> >>>>
> >>
>
>
Powered by blists - more mailing lists