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(-)
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-googOn 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
> >>
>
© 2016 - 2024 Red Hat, Inc.