| Message ID | 20241030070224.1006331-2-wenst@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: mediatek: mt8183 cleanups | expand |
On Wed, Oct 30, 2024 at 3:02 PM Chen-Yu Tsai <wenst@chromium.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@chromium.org> I see that the other three patches were merged and included in the pull request, but not this one. Were there any concerns? 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 >
Il 04/11/24 14:00, Chen-Yu Tsai ha scritto: > On Wed, Oct 30, 2024 at 3:02 PM Chen-Yu Tsai <wenst@chromium.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@chromium.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. 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? :-) P.S.: I'm writing fast, sorry if anything appears unclear, feel free to shoot more questions in case :-) 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 >>
On Mon, Nov 4, 2024 at 9:19 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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@chromium.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@chromium.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? > 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 > >> >
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>;
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@chromium.org> --- arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi | 2 -- 1 file changed, 2 deletions(-)