On Tue, Dec 10, 2024 at 4:27 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> > >>> > >>> 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 > >>>> > >> > >