[PATCH 0/5] MT8516/MT8167 dtsi fixes

[PATCH 0/5] MT8516/MT8167 dtsi fixes

[Val Packett]

Hi everyone,

I've been working on mainline bringup on an MT8167 tablet I found at a
junkyard sale (lenovo,tb7304f) for postmarketOS :3

This first series consists of basic device tree fixes for the MT8516
dtsi that the MT8167 one inherits from.

The changes that follow add support for the MT6392 PMIC, and that's
mostly been implemented by Fabien Parent back in 2020 and not merged:

<https://patchwork.kernel.org/project/linux-arm-kernel/patch/20201027181157.862927-3-fparent@baylibre.com/>
<https://patchwork.kernel.org/project/linux-arm-kernel/patch/20201024200304.1427864-2-fparent@baylibre.com/>

but I have a couple changes on top of those patches (like adding the
missing mt6392_set_buck_vosel_reg). I'm wondering what the best way to
get this in would be, should I squash my changes and submit the "final"
patches with a Co-developed-by tag?

(Similar situation with DRM nodes, not merged due to "concerns about
the driver architecture" in 2021, now missing GCE/CMDQ mailbox props:
<https://lore.kernel.org/df4c57f9-115b-c4da-e656-e4bdec62c2d7@gmail.com/>)

By the way, is anyone familiar with PSCI cpuidle/hot-unplug issues on
Mediatek Android devices from around this time? Specifically on this
tablet, I can't make the cores come back from suspend. I have
investigated local-timer-stop and arm,no-tick-in-suspend, Fabien pointed
me to the mediatek timer and its required clocks, but nothing helped.
Trying the psci_checker, I realized that it's not just suspend: they
do not come back from hot-unplug either. Initial CPU_ON on boot is fine,
but then after a CPU_OFF they do not actually come back when CPU_ON
supposedly turns them on. Now I can't help but notice that the only DTS
in mainline for a device that came with Android, mt6795-sony-xperia-m5,
does not have any cpuidle nodes in its SoC's dtsi either..

Val Packett (5):
  arm64: dts: mediatek: mt8516: fix GICv2 range
  arm64: dts: mediatek: mt8516: fix wdt irq type
  arm64: dts: mediatek: mt8516: add i2c clock-div property
  arm64: dts: mediatek: mt8516: reserve 192 KiB for TF-A
  arm64: dts: mediatek: mt8516: add keypad node

 arch/arm64/boot/dts/mediatek/mt8516.dtsi      | 21 +++++++++++++++----
 .../boot/dts/mediatek/pumpkin-common.dtsi     |  2 --
 2 files changed, 17 insertions(+), 6 deletions(-)

-- 
2.47.1

[PATCH 1/5] arm64: dts: mediatek: mt8516: fix GICv2 range

[Val Packett]

On the MT8167 which is based on the MT8516 DTS, the following error
was appearing on boot, breaking interrupt operation:

GICv2 detected, but range too small and irqchip.gicv2_force_probe not set

Similar to what's been proposed for MT7622 which has the same issue,
fix by using the range reported by force_probe.

Link: https://lore.kernel.org/all/YmhNSLgp%2Fyg8Vr1F@makrotopia.org/
Fixes: 5236347bde42 ("arm64: dts: mediatek: add dtsi for MT8516")
Signed-off-by: Val Packett <val@packett.cool>
---
 arch/arm64/boot/dts/mediatek/mt8516.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
index d0b03dc4d3f4..444429341302 100644
--- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
@@ -268,7 +268,7 @@ gic: interrupt-controller@10310000 {
 			interrupt-parent = <&gic>;
 			interrupt-controller;
 			reg = <0 0x10310000 0 0x1000>,
-			      <0 0x10320000 0 0x1000>,
+			      <0 0x1032f000 0 0x2000>,
 			      <0 0x10340000 0 0x2000>,
 			      <0 0x10360000 0 0x2000>;
 			interrupts = <GIC_PPI 9
-- 
2.47.1

[PATCH 2/5] arm64: dts: mediatek: mt8516: fix wdt irq type

[Val Packett]

The GICv2 does not support EDGE_FALLING interrupts, so the watchdog
would refuse to attach due to a failing check coming from the GIC driver.

Fixes: 5236347bde42 ("arm64: dts: mediatek: add dtsi for MT8516")
Signed-off-by: Val Packett <val@packett.cool>
---
 arch/arm64/boot/dts/mediatek/mt8516.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
index 444429341302..098c32ebf678 100644
--- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
@@ -206,7 +206,7 @@ watchdog@10007000 {
 			compatible = "mediatek,mt8516-wdt",
 				     "mediatek,mt6589-wdt";
 			reg = <0 0x10007000 0 0x1000>;
-			interrupts = <GIC_SPI 198 IRQ_TYPE_EDGE_FALLING>;
+			interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
 			#reset-cells = <1>;
 		};
 
-- 
2.47.1

[PATCH 3/5] arm64: dts: mediatek: mt8516: add i2c clock-div property

[Val Packett]

Move the clock-div property from the pumpkin board dtsi to the SoC's
since it belongs to the SoC itself and is required on other devices.

Fixes: 5236347bde42 ("arm64: dts: mediatek: add dtsi for MT8516")
Signed-off-by: Val Packett <val@packett.cool>
---
 arch/arm64/boot/dts/mediatek/mt8516.dtsi         | 3 +++
 arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
index 098c32ebf678..dd17d8a88c19 100644
--- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
@@ -344,6 +344,7 @@ i2c0: i2c@11009000 {
 			reg = <0 0x11009000 0 0x90>,
 			      <0 0x11000180 0 0x80>;
 			interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_LOW>;
+			clock-div = <2>;
 			clocks = <&topckgen CLK_TOP_I2C0>,
 				 <&topckgen CLK_TOP_APDMA>;
 			clock-names = "main", "dma";
@@ -358,6 +359,7 @@ i2c1: i2c@1100a000 {
 			reg = <0 0x1100a000 0 0x90>,
 			      <0 0x11000200 0 0x80>;
 			interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
+			clock-div = <2>;
 			clocks = <&topckgen CLK_TOP_I2C1>,
 				 <&topckgen CLK_TOP_APDMA>;
 			clock-names = "main", "dma";
@@ -372,6 +374,7 @@ i2c2: i2c@1100b000 {
 			reg = <0 0x1100b000 0 0x90>,
 			      <0 0x11000280 0 0x80>;
 			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_LOW>;
+			clock-div = <2>;
 			clocks = <&topckgen CLK_TOP_I2C2>,
 				 <&topckgen CLK_TOP_APDMA>;
 			clock-names = "main", "dma";
diff --git a/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi b/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
index ec8dfb3d1c6d..a356db5fcc5f 100644
--- a/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
+++ b/arch/arm64/boot/dts/mediatek/pumpkin-common.dtsi
@@ -47,7 +47,6 @@ key-volume-down {
 };
 
 &i2c0 {
-	clock-div = <2>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c0_pins_a>;
 	status = "okay";
@@ -156,7 +155,6 @@ cam-pwdn-hog {
 };
 
 &i2c2 {
-	clock-div = <2>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c2_pins_a>;
 	status = "okay";
-- 
2.47.1

[PATCH 4/5] arm64: dts: mediatek: mt8516: reserve 192 KiB for TF-A

[Val Packett]

The Android DTB for the related MT8167 reserves 0x30000. This is likely
correct for MT8516 Android devices as well, and there's never any harm
in reserving 64KiB more.

Fixes: 5236347bde42 ("arm64: dts: mediatek: add dtsi for MT8516")
Signed-off-by: Val Packett <val@packett.cool>
---
 arch/arm64/boot/dts/mediatek/mt8516.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
index dd17d8a88c19..e30623ebac0e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
@@ -144,10 +144,10 @@ reserved-memory {
 		#size-cells = <2>;
 		ranges;
 
-		/* 128 KiB reserved for ARM Trusted Firmware (BL31) */
+		/* 192 KiB reserved for ARM Trusted Firmware (BL31) */
 		bl31_secmon_reserved: secmon@43000000 {
 			no-map;
-			reg = <0 0x43000000 0 0x20000>;
+			reg = <0 0x43000000 0 0x30000>;
 		};
 	};
 
-- 
2.47.1

[PATCH 5/5] arm64: dts: mediatek: mt8516: add keypad node

[Val Packett]

Add a keypad matrix node for the MT8516/MT8167 SoC.

Signed-off-by: Val Packett <val@packett.cool>
---
 arch/arm64/boot/dts/mediatek/mt8516.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
index e30623ebac0e..3beb9f74ec79 100644
--- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
@@ -220,6 +220,16 @@ timer: timer@10008000 {
 			clock-names = "clk13m", "bus";
 		};
 
+		keypad: keypad@10002000 {
+			compatible = "mediatek,mt6779-keypad";
+			reg = <0 0x10002000 0 0x1000>;
+			wakeup-source;
+			interrupts = <GIC_SPI 149 IRQ_TYPE_EDGE_FALLING>;
+			clocks = <&clk26m>;
+			clock-names = "kpd";
+			status = "disabled";
+		};
+
 		syscfg_pctl: syscfg-pctl@10005000 {
 			compatible = "syscon";
 			reg = <0 0x10005000 0 0x1000>;
-- 
2.47.1

Re: [PATCH 5/5] arm64: dts: mediatek: mt8516: add keypad node

[AngeloGioacchino Del Regno]

Il 04/12/24 20:05, Val Packett ha scritto:
> Add a keypad matrix node for the MT8516/MT8167 SoC.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>   arch/arm64/boot/dts/mediatek/mt8516.dtsi | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> index e30623ebac0e..3beb9f74ec79 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> @@ -220,6 +220,16 @@ timer: timer@10008000 {
>   			clock-names = "clk13m", "bus";
>   		};
>   
> +		keypad: keypad@10002000 {
> +			compatible = "mediatek,mt6779-keypad";

Noupe, you have to add your SoC to the compatible list... both here and in the
binding for mediatek.mt6779-keypad.yaml

compatible = "mediatek,mt8516-keypad", "mediatek,mt6779-keypad"

for the binding, it's just a simple addition to an enum, nothing else.

Cheers,
Angelo

> +			reg = <0 0x10002000 0 0x1000>;
> +			wakeup-source;
> +			interrupts = <GIC_SPI 149 IRQ_TYPE_EDGE_FALLING>;
> +			clocks = <&clk26m>;
> +			clock-names = "kpd";
> +			status = "disabled";
> +		};
> +
>   		syscfg_pctl: syscfg-pctl@10005000 {
>   			compatible = "syscon";
>   			reg = <0 0x10005000 0 0x1000>;

Re: [PATCH 4/5] arm64: dts: mediatek: mt8516: reserve 192 KiB for TF-A

[AngeloGioacchino Del Regno]

Il 04/12/24 20:05, Val Packett ha scritto:
> The Android DTB for the related MT8167 reserves 0x30000. This is likely
> correct for MT8516 Android devices as well, and there's never any harm
> in reserving 64KiB more.
> 
> Fixes: 5236347bde42 ("arm64: dts: mediatek: add dtsi for MT8516")
> Signed-off-by: Val Packett <val@packett.cool>

That has changed over time with new bootloaders, yes, so there's no harm
and it's actually a necessary change.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 3/5] arm64: dts: mediatek: mt8516: add i2c clock-div property

[AngeloGioacchino Del Regno]

Il 04/12/24 20:05, Val Packett ha scritto:
> Move the clock-div property from the pumpkin board dtsi to the SoC's
> since it belongs to the SoC itself and is required on other devices.
> 
> Fixes: 5236347bde42 ("arm64: dts: mediatek: add dtsi for MT8516")
> Signed-off-by: Val Packett <val@packett.cool>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 2/5] arm64: dts: mediatek: mt8516: fix wdt irq type

[AngeloGioacchino Del Regno]

Il 04/12/24 20:05, Val Packett ha scritto:
> The GICv2 does not support EDGE_FALLING interrupts, so the watchdog
> would refuse to attach due to a failing check coming from the GIC driver.
> 
> Fixes: 5236347bde42 ("arm64: dts: mediatek: add dtsi for MT8516")
> Signed-off-by: Val Packett <val@packett.cool>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   arch/arm64/boot/dts/mediatek/mt8516.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> index 444429341302..098c32ebf678 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> @@ -206,7 +206,7 @@ watchdog@10007000 {
>   			compatible = "mediatek,mt8516-wdt",
>   				     "mediatek,mt6589-wdt";
>   			reg = <0 0x10007000 0 0x1000>;
> -			interrupts = <GIC_SPI 198 IRQ_TYPE_EDGE_FALLING>;
> +			interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
>   			#reset-cells = <1>;
>   		};
>   

Re: [PATCH 1/5] arm64: dts: mediatek: mt8516: fix GICv2 range

[AngeloGioacchino Del Regno]

Il 04/12/24 20:05, Val Packett ha scritto:
> On the MT8167 which is based on the MT8516 DTS, the following error
> was appearing on boot, breaking interrupt operation:
> 
> GICv2 detected, but range too small and irqchip.gicv2_force_probe not set
> 
> Similar to what's been proposed for MT7622 which has the same issue,
> fix by using the range reported by force_probe.
> 
> Link: https://lore.kernel.org/all/YmhNSLgp%2Fyg8Vr1F@makrotopia.org/
> Fixes: 5236347bde42 ("arm64: dts: mediatek: add dtsi for MT8516")
> Signed-off-by: Val Packett <val@packett.cool>

Yeah, that's fine, I agree.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   arch/arm64/boot/dts/mediatek/mt8516.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> index d0b03dc4d3f4..444429341302 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> @@ -268,7 +268,7 @@ gic: interrupt-controller@10310000 {
>   			interrupt-parent = <&gic>;
>   			interrupt-controller;
>   			reg = <0 0x10310000 0 0x1000>,
> -			      <0 0x10320000 0 0x1000>,
> +			      <0 0x1032f000 0 0x2000>,
>   			      <0 0x10340000 0 0x2000>,
>   			      <0 0x10360000 0 0x2000>;
>   			interrupts = <GIC_PPI 9

Re: [PATCH 0/5] MT8516/MT8167 dtsi fixes

[AngeloGioacchino Del Regno]

Il 04/12/24 20:05, Val Packett ha scritto:
> Hi everyone,
> 
> I've been working on mainline bringup on an MT8167 tablet I found at a
> junkyard sale (lenovo,tb7304f) for postmarketOS :3
> 
> This first series consists of basic device tree fixes for the MT8516
> dtsi that the MT8167 one inherits from.
> 

...Yes, but I don't see any patch that is introducing your TB7304F device here.

I strongly suggest you to also send one that achieves basic boot with UART console
as a first step for upstreaming your board, and then go for incremental changes
everytime you get a new feature working.

> The changes that follow add support for the MT6392 PMIC, and that's
> mostly been implemented by Fabien Parent back in 2020 and not merged:
> 
> <https://patchwork.kernel.org/project/linux-arm-kernel/patch/20201027181157.862927-3-fparent@baylibre.com/>
> <https://patchwork.kernel.org/project/linux-arm-kernel/patch/20201024200304.1427864-2-fparent@baylibre.com/>
> 
> but I have a couple changes on top of those patches (like adding the
> missing mt6392_set_buck_vosel_reg). I'm wondering what the best way to
> get this in would be, should I squash my changes and submit the "final"
> patches with a Co-developed-by tag?
> 

Generally, if the patches are only simple additions, you could send the original
patches without any author variation (and fixing that MT6392_IRQ_numbers enum
in the original ones because lower case please!) and then your patches on top
with your additions.

Otherwise, if you're "changing a lot of stuff" it makes sense to grab authorship
and add the Co-Developed-by... but I don't think you're changing much, honestly.

Of course, you'll have to drop the TXT additions, as now they are YAML (where the
YAML additions go to a separated dt-bindings patch!).

In the end, it's your choice - that's more or less a gist of how to do it.

> (Similar situation with DRM nodes, not merged due to "concerns about
> the driver architecture" in 2021, now missing GCE/CMDQ mailbox props:
> <https://lore.kernel.org/df4c57f9-115b-c4da-e656-e4bdec62c2d7@gmail.com/>)
> 

The upstream driver just gained support for configuring the display paths
entirely in the devicetree as those are obviously device specific.

You can make use of that for upstreaming your tablet after adding the display
nodes (and bindings, if required) as if you go for the default configuration
that's probably not going to work because it's for the pumpkin boards which
will most probably have a different display pipeline compared to your board.

> By the way, is anyone familiar with PSCI cpuidle/hot-unplug issues on
> Mediatek Android devices from around this time? Specifically on this
> tablet, I can't make the cores come back from suspend. I have
> investigated local-timer-stop and arm,no-tick-in-suspend, Fabien pointed
> me to the mediatek timer and its required clocks, but nothing helped.
> Trying the psci_checker, I realized that it's not just suspend: they
> do not come back from hot-unplug either. Initial CPU_ON on boot is fine,
> but then after a CPU_OFF they do not actually come back when CPU_ON
> supposedly turns them on. Now I can't help but notice that the only DTS
> in mainline for a device that came with Android, mt6795-sony-xperia-m5,
> does not have any cpuidle nodes in its SoC's dtsi either..
> 

I did have some issues with an older bootloader on the Xperia M5 smartphone
and would even lock up at boot, because on the old firmwares the power
domains for the CPUs are not managed automatically by FW.

Before discovering that there was new FW (and this stuff is signed so you
cannot cross-flash...) I did engineer a solution, but never upstreamed it
because I did effectively lack time to clean it up and make it proper.

This solution is not upstreamable, as it makes the mtk-pm-domains driver
to be usable only as built-in and not as module anymore... so that would
probably need a separated driver or something like that.

Here's the (rather dirty, but working) code: 
https://gitlab.collabora.com/google/chromeos-kernel/-/commit/ae8f7048aadac03037391a64d9d79ca5af7b7a77

Cheers!
Angelo

> Val Packett (5):
>    arm64: dts: mediatek: mt8516: fix GICv2 range
>    arm64: dts: mediatek: mt8516: fix wdt irq type
>    arm64: dts: mediatek: mt8516: add i2c clock-div property
>    arm64: dts: mediatek: mt8516: reserve 192 KiB for TF-A
>    arm64: dts: mediatek: mt8516: add keypad node
> 
>   arch/arm64/boot/dts/mediatek/mt8516.dtsi      | 21 +++++++++++++++----
>   .../boot/dts/mediatek/pumpkin-common.dtsi     |  2 --
>   2 files changed, 17 insertions(+), 6 deletions(-)
> 

Re: [PATCH 0/5] MT8516/MT8167 dtsi fixes

[Val Packett]

On Thu, Dec 5 2024 at 01:27:01 PM +01:00:00, AngeloGioacchino Del Regno 
<angelogioacchino.delregno@collabora.com> wrote:
>> I strongly suggest you to also send one that achieves basic boot 
>> with UART console
> as a first step for upstreaming your board, and then go for 
> incremental changes
> everytime you get a new feature working.

Wanted to get the PMIC in first but sure, could try splitting out the 
initial version without the PMIC.

Oh! One dts/dtsi question: pretty much all MTK devices so far have all 
pinctrl configurations defined per-device. But on this SoC, pin 
assignments have their "canonical" function in the pin name e.g.: 
MT8167_PIN_58_SDA0__FUNC_SDA0_0 and on this device they are used as-is. 
Would it be fine to place these default pinctrl configs for SD/MMC, I2C 
etc. in the SoC dtsi?

>> Generally, if the patches are only simple additions, you could send 
>> the original
> patches without any author variation (and fixing that 
> MT6392_IRQ_numbers enum
> in the original ones because lower case please!) and then your 
> patches on top
> with your additions.

Right, I was mostly unsure if the email workflow supported just sending 
someone else's patches, but I guess that was silly - of course 
git-send-email should do the right thing!

> The upstream driver just gained support for configuring the display 
> paths
> entirely in the devicetree as those are obviously device specific.
> 
> You can make use of that for upstreaming your tablet after adding the 
> display
> nodes (and bindings, if required) as if you go for the default 
> configuration
> that's probably not going to work because it's for the pumpkin boards 
> which
> will most probably have a different display pipeline compared to your 
> board.

The pipeline seems to be the same.. The pumpkin board was brought up 
with DSI as well, the main pipeline I can find in the Android source is 
the same (+ PWM).

I am still struggling to get it to work though: DSI command mode 
configuration gets acknowledged fine, but in burst mode, the vblank 
never arrives. Tried fiddling with various things (CMDQ or not, mutex 
as vblank source since there was an Android commit doing that, etc.), 
nothing helped.

>> By the way, is anyone familiar with PSCI cpuidle/hot-unplug issues on
>> Mediatek Android devices from around this time? [..]
> 
> I did have some issues with an older bootloader on the Xperia M5 
> smartphone
> and would even lock up at boot, because on the old firmwares the power
> domains for the CPUs are not managed automatically by FW.

Interesting, thanks for the pointer!

In the Android kernel sources I could find though, there are no CPU 
domains in the mtk-scpsys-mt8167 driver, and the only references I 
could even find to the related register bits are from code that *reads* 
the status of the CPU power domains to make decisions about sleep 
states (only_one_cpu_online in mtk_idle). Trying to add those to the 
driver anyway, did not succeed so far.

~val

Re: [PATCH 0/5] MT8516/MT8167 dtsi fixes

[AngeloGioacchino Del Regno]

Il 07/12/24 04:11, Val Packett ha scritto:
> 
> On Thu, Dec 5 2024 at 01:27:01 PM +01:00:00, AngeloGioacchino Del Regno 
> <angelogioacchino.delregno@collabora.com> wrote:
>>> I strongly suggest you to also send one that achieves basic boot with UART console
>> as a first step for upstreaming your board, and then go for incremental changes
>> everytime you get a new feature working.
> 
> Wanted to get the PMIC in first but sure, could try splitting out the initial 
> version without the PMIC.
> 

We all want to get XYZ in first, but then, you'll see that sometimes having at
least something will help you justify other changes....

If there's nothing upstream, understanding your changes becomes increasingly
difficult (depending on the nature of those changes, of course) - so... just
take the advice: sending the initial DT will help you later in the process ;-)

> Oh! One dts/dtsi question: pretty much all MTK devices so far have all pinctrl 
> configurations defined per-device. But on this SoC, pin assignments have their 
> "canonical" function in the pin name e.g.: MT8167_PIN_58_SDA0__FUNC_SDA0_0 and on 
> this device they are used as-is. Would it be fine to place these default pinctrl 
> configs for SD/MMC, I2C etc. in the SoC dtsi?
> 

No, because even though this is the default on many boards, it's still something
board specific, as other boards may define different functions.

Please define that in your board DT.

>>> Generally, if the patches are only simple additions, you could send the original
>> patches without any author variation (and fixing that MT6392_IRQ_numbers enum
>> in the original ones because lower case please!) and then your patches on top
>> with your additions.
> 
> Right, I was mostly unsure if the email workflow supported just sending someone 
> else's patches, but I guess that was silly - of course git-send-email should do the 
> right thing!
> 
>> The upstream driver just gained support for configuring the display paths
>> entirely in the devicetree as those are obviously device specific.
>>
>> You can make use of that for upstreaming your tablet after adding the display
>> nodes (and bindings, if required) as if you go for the default configuration
>> that's probably not going to work because it's for the pumpkin boards which
>> will most probably have a different display pipeline compared to your board.
> 
> The pipeline seems to be the same.. The pumpkin board was brought up with DSI as 
> well, the main pipeline I can find in the Android source is the same (+ PWM).
> 
> I am still struggling to get it to work though: DSI command mode configuration gets 
> acknowledged fine, but in burst mode, the vblank never arrives. Tried fiddling with 
> various things (CMDQ or not, mutex as vblank source since there was an Android 
> commit doing that, etc.), nothing helped.
> 

DSI command mode is not supported upstream - only video mode... and that's why you
can't get your display to work. There's no WDMA driver...

Convert it to video mode and it's gonna work just perfect :-)

P.S.: Also check if your mmsys+mutex configuration is correct!

>>> By the way, is anyone familiar with PSCI cpuidle/hot-unplug issues on
>>> Mediatek Android devices from around this time? [..]
>>
>> I did have some issues with an older bootloader on the Xperia M5 smartphone
>> and would even lock up at boot, because on the old firmwares the power
>> domains for the CPUs are not managed automatically by FW.
> 
> Interesting, thanks for the pointer!
> 
> In the Android kernel sources I could find though, there are no CPU domains in the 
> mtk-scpsys-mt8167 driver, and the only references I could even find to the related 
> register bits are from code that *reads* the status of the CPU power domains to 
> make decisions about sleep states (only_one_cpu_online in mtk_idle). Trying to add 
> those to the driver anyway, did not succeed so far.
> 

Downstream, you will find references to "mtcmos" (or anyway cpu mtcmos) - that's
how power domains are called there (I think. At least, in older downstream kernels
that's how they're called - and that's the "real" hw name btw).

Cheers,
Angelo

> ~val
> 
> 
> 

Re: (subset) [PATCH 0/5] MT8516/MT8167 dtsi fixes

[AngeloGioacchino Del Regno]

On Wed, 04 Dec 2024 16:05:03 -0300, Val Packett wrote:
> I've been working on mainline bringup on an MT8167 tablet I found at a
> junkyard sale (lenovo,tb7304f) for postmarketOS :3
> 
> This first series consists of basic device tree fixes for the MT8516
> dtsi that the MT8167 one inherits from.
> 
> The changes that follow add support for the MT6392 PMIC, and that's
> mostly been implemented by Fabien Parent back in 2020 and not merged:
> 
> [...]

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

[1/5] arm64: dts: mediatek: mt8516: fix GICv2 range
      commit: e3ee31e4409f051c021a30122f3c470f093a7386
[2/5] arm64: dts: mediatek: mt8516: fix wdt irq type
      commit: 03a80442030e7147391738fb6cbe5fa0b3b91bb1
[3/5] arm64: dts: mediatek: mt8516: add i2c clock-div property
      commit: eb72341fd92b7af510d236e5a8554d855ed38d3c
[4/5] arm64: dts: mediatek: mt8516: reserve 192 KiB for TF-A
      commit: 2561c7d5d497b988deccc36fe5eac7fd50b937f8

Cheers,
Angelo