[PATCH 0/4] arm64: dts: mediatek: mt8183 cleanups

[PATCH 0/4] arm64: dts: mediatek: mt8183 cleanups

[Chen-Yu Tsai]

Hi,

Here are some cleanups for the MT8183 Kukui and Jacuzzi device trees.
These just fix up some names, add missing supplies, and drop redundant
regulators.

Please have a look.


Thanks
ChenYu

Chen-Yu Tsai (4):
  arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop pp3300_panel voltage
    settings
  arm64: dts: mediatek: mt8183-kukui-jacuzzi: Fix DP bridge supply names
  arm64: dts: mediatek: mt8183-kukui-jacuzzi: Add supplies for fixed
    regulators
  arm64: dts: mediatek: mt8183-kukui: Drop bogus fixed regulators

 .../dts/mediatek/mt8183-kukui-jacuzzi.dtsi    | 32 +++++++++++--------
 .../dts/mediatek/mt8183-kukui-katsu-sku32.dts |  2 +-
 .../dts/mediatek/mt8183-kukui-katsu-sku38.dts |  2 +-
 .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 25 ++-------------
 4 files changed, 22 insertions(+), 39 deletions(-)

-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH 1/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop pp3300_panel voltage settings

[Chen-Yu Tsai]

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-goog

[PATCH 2/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Fix DP bridge supply names

[Chen-Yu Tsai]

Some of the regulator supplies for the MIPI-DPI-to-DP bridge and their
associated nodes are incorrectly named. In particular, the 1.0V supply
was modeled as a 1.2V supply.

Fix all the incorrect names, and also fix the voltage of the 1.0V
regulator.

Fixes: cabc71b08eb5 ("arm64: dts: mt8183: Add kukui-jacuzzi-damu board")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 .../dts/mediatek/mt8183-kukui-jacuzzi.dtsi    | 26 ++++++++++---------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
index 7bbafe926558..f540dcac74ea 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
@@ -8,11 +8,13 @@
 #include <arm/cros-ec-keyboard.dtsi>
 
 / {
-	pp1200_mipibrdg: pp1200-mipibrdg {
+	pp1000_mipibrdg: pp1000-mipibrdg {
 		compatible = "regulator-fixed";
-		regulator-name = "pp1200_mipibrdg";
+		regulator-name = "pp1000_mipibrdg";
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
 		pinctrl-names = "default";
-		pinctrl-0 = <&pp1200_mipibrdg_en>;
+		pinctrl-0 = <&pp1000_mipibrdg_en>;
 
 		enable-active-high;
 		regulator-boot-on;
@@ -24,7 +26,7 @@ pp1800_mipibrdg: pp1800-mipibrdg {
 		compatible = "regulator-fixed";
 		regulator-name = "pp1800_mipibrdg";
 		pinctrl-names = "default";
-		pinctrl-0 = <&pp1800_lcd_en>;
+		pinctrl-0 = <&pp1800_mipibrdg_en>;
 
 		enable-active-high;
 		regulator-boot-on;
@@ -44,11 +46,11 @@ pp3300_panel: pp3300-panel {
 		gpio = <&pio 35 GPIO_ACTIVE_HIGH>;
 	};
 
-	vddio_mipibrdg: vddio-mipibrdg {
+	pp3300_mipibrdg: pp3300-mipibrdg {
 		compatible = "regulator-fixed";
-		regulator-name = "vddio_mipibrdg";
+		regulator-name = "pp3300_mipibrdg";
 		pinctrl-names = "default";
-		pinctrl-0 = <&vddio_mipibrdg_en>;
+		pinctrl-0 = <&pp3300_mipibrdg_en>;
 
 		enable-active-high;
 		regulator-boot-on;
@@ -144,9 +146,9 @@ anx_bridge: anx7625@58 {
 		pinctrl-0 = <&anx7625_pins>;
 		enable-gpios = <&pio 45 GPIO_ACTIVE_HIGH>;
 		reset-gpios = <&pio 73 GPIO_ACTIVE_HIGH>;
-		vdd10-supply = <&pp1200_mipibrdg>;
+		vdd10-supply = <&pp1000_mipibrdg>;
 		vdd18-supply = <&pp1800_mipibrdg>;
-		vdd33-supply = <&vddio_mipibrdg>;
+		vdd33-supply = <&pp3300_mipibrdg>;
 
 		ports {
 			#address-cells = <1>;
@@ -389,14 +391,14 @@ &pio {
 		"",
 		"";
 
-	pp1200_mipibrdg_en: pp1200-mipibrdg-en {
+	pp1000_mipibrdg_en: pp1000-mipibrdg-en {
 		pins1 {
 			pinmux = <PINMUX_GPIO54__FUNC_GPIO54>;
 			output-low;
 		};
 	};
 
-	pp1800_lcd_en: pp1800-lcd-en {
+	pp1800_mipibrdg_en: pp1800-mipibrdg-en {
 		pins1 {
 			pinmux = <PINMUX_GPIO36__FUNC_GPIO36>;
 			output-low;
@@ -458,7 +460,7 @@ trackpad-int {
 		};
 	};
 
-	vddio_mipibrdg_en: vddio-mipibrdg-en {
+	pp3300_mipibrdg_en: pp3300-mipibrdg-en {
 		pins1 {
 			pinmux = <PINMUX_GPIO37__FUNC_GPIO37>;
 			output-low;
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH 3/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Add supplies for fixed regulators

[Chen-Yu Tsai]

When the fixed regulators for the LCD panel and DP bridge were added,
their supplies were not modeled in. These, except for the 1.0V supply,
are just load switches, and need and have a supply.

Add the supplies for each of the fixed regulators.

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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
index f540dcac74ea..80888bd4ad82 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi
@@ -20,6 +20,7 @@ pp1000_mipibrdg: pp1000-mipibrdg {
 		regulator-boot-on;
 
 		gpio = <&pio 54 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&pp1800_alw>;
 	};
 
 	pp1800_mipibrdg: pp1800-mipibrdg {
@@ -32,6 +33,7 @@ pp1800_mipibrdg: pp1800-mipibrdg {
 		regulator-boot-on;
 
 		gpio = <&pio 36 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&pp1800_alw>;
 	};
 
 	pp3300_panel: pp3300-panel {
@@ -44,6 +46,7 @@ pp3300_panel: pp3300-panel {
 		regulator-boot-on;
 
 		gpio = <&pio 35 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&pp3300_alw>;
 	};
 
 	pp3300_mipibrdg: pp3300-mipibrdg {
@@ -56,6 +59,7 @@ pp3300_mipibrdg: pp3300-mipibrdg {
 		regulator-boot-on;
 
 		gpio = <&pio 37 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&pp3300_alw>;
 	};
 
 	volume_buttons: volume-buttons {
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH 4/4] arm64: dts: mediatek: mt8183-kukui: Drop bogus fixed regulators

[Chen-Yu Tsai]

A couple of the fixed regulators in the Kukui dtsi do not actually
exist in the hardware design; they are simply power rails connected
together with 0 ohm bridges.

Drop them and assign their upstream supplies to their consumers instead.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 .../dts/mediatek/mt8183-kukui-katsu-sku32.dts |  2 +-
 .../dts/mediatek/mt8183-kukui-katsu-sku38.dts |  2 +-
 .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 25 ++-----------------
 3 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-katsu-sku32.dts b/arch/arm64/boot/dts/mediatek/mt8183-kukui-katsu-sku32.dts
index 05361008e8ac..2b5a8d1f900e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-katsu-sku32.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-katsu-sku32.dts
@@ -23,7 +23,7 @@ touchscreen1: touchscreen@5d {
 		interrupts-extended = <&pio 155 IRQ_TYPE_LEVEL_LOW>;
 
 		reset-gpios = <&pio 156 GPIO_ACTIVE_LOW>;
-		vdd-supply = <&lcd_pp3300>;
+		vdd-supply = <&pp3300_alw>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-katsu-sku38.dts b/arch/arm64/boot/dts/mediatek/mt8183-kukui-katsu-sku38.dts
index cf008ed82878..75fadf2c7059 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-katsu-sku38.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-katsu-sku38.dts
@@ -23,7 +23,7 @@ touchscreen1: touchscreen@5d {
 		interrupts-extended = <&pio 155 IRQ_TYPE_LEVEL_LOW>;
 
 		reset-gpios = <&pio 156 GPIO_ACTIVE_LOW>;
-		vdd-supply = <&lcd_pp3300>;
+		vdd-supply = <&pp3300_alw>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index e9e6253141c6..0cf50b1f59c8 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -52,27 +52,6 @@ it6505_pp18_reg: regulator0 {
 		vin-supply = <&pp1800_alw>;
 	};
 
-	lcd_pp3300: regulator1 {
-		compatible = "regulator-fixed";
-		regulator-name = "lcd_pp3300";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-		regulator-always-on;
-		regulator-boot-on;
-	};
-
-	mmc1_fixed_power: regulator3 {
-		compatible = "regulator-fixed";
-		regulator-name = "mmc1_power";
-		vin-supply = <&pp3300_alw>;
-	};
-
-	mmc1_fixed_io: regulator4 {
-		compatible = "regulator-fixed";
-		regulator-name = "mmc1_io";
-		vin-supply = <&pp1800_alw>;
-	};
-
 	pp1800_alw: regulator5 {
 		compatible = "regulator-fixed";
 		regulator-name = "pp1800_alw";
@@ -373,8 +352,8 @@ &mmc1 {
 	pinctrl-names = "default", "state_uhs";
 	pinctrl-0 = <&mmc1_pins_default>;
 	pinctrl-1 = <&mmc1_pins_uhs>;
-	vmmc-supply = <&mmc1_fixed_power>;
-	vqmmc-supply = <&mmc1_fixed_io>;
+	vmmc-supply = <&pp3300_alw>;
+	vqmmc-supply = <&pp1800_alw>;
 	mmc-pwrseq = <&wifi_pwrseq>;
 	bus-width = <4>;
 	max-frequency = <200000000>;
-- 
2.47.0.163.g1226f6d8fa-goog

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop pp3300_panel voltage settings

[Chen-Yu Tsai]

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
>

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop pp3300_panel voltage settings

[AngeloGioacchino Del Regno]

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
>>

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop pp3300_panel voltage settings

[Chen-Yu Tsai]

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
> >>
>

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop pp3300_panel voltage settings

[AngeloGioacchino Del Regno]

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@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?
> 

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?

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
>>>>
>>

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop pp3300_panel voltage settings

[Chen-Yu Tsai]

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

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop pp3300_panel voltage settings

[Chen-Yu Tsai]

On Tue, Dec 10, 2024 at 4:40 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Tue, Dec 10, 2024 at 4:27 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.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@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?
> > >
> >
> > 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.

Ping? This one still hasn't been merged.

ChenYu

> 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
> > >>>>
> > >>
> >
> >

Re: (subset) [PATCH 0/4] arm64: dts: mediatek: mt8183 cleanups

[AngeloGioacchino Del Regno]

On Wed, 30 Oct 2024 15:02:19 +0800, Chen-Yu Tsai wrote:
> Here are some cleanups for the MT8183 Kukui and Jacuzzi device trees.
> These just fix up some names, add missing supplies, and drop redundant
> regulators.
> 
> Please have a look.
> 
> 
> [...]

Applied to v6.13-next/dts64, thanks!

[1/4] arm64: dts: mediatek: mt8183-kukui-jacuzzi: Drop pp3300_panel voltage settings
      commit: 0b5b1c881a909f17c05ef4b1ccb421e077f6e466

Cheers,
Angelo