Re: [PATCH 7/7] arm64: dts: mediatek: Add support for MT6795 Sony Xperia M5 smartphone

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 27/10/2022 11:28, AngeloGioacchino Del Regno wrote:
Il 29/07/22 14:00, Konrad Dybcio ha scritto:


On 29.07.2022 12:44, AngeloGioacchino Del Regno wrote:
Add a basic support for the Sony Xperia M5 (codename "Holly")
smartphone, powered by a MediaTek Helio X10 SoC.

This achieves a console boot.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>

Hello Konrad,
First of all, I'm sorry for the very late reply.

---
  arch/arm64/boot/dts/mediatek/Makefile         |  1 +
  .../dts/mediatek/mt6795-sony-xperia-m5.dts    | 90 +++++++++++++++++++
  2 files changed, 91 insertions(+)
  create mode 100644 arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index af362a085a02..72fd683c9264 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -3,6 +3,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6779-evb.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-sony-xperia-m5.dtb
-holly.dtb?


I prefer using the commercial name to identify the device.
"Holly" is the smartphone project codename and that is mentioned almost nowhere: the aim here is to enhance readability as to make it immediately understandable
that this devicetree is for the Xperia M5 device.

Ok, sounds good.



  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts
new file mode 100644
index 000000000000..94d011c4126c
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt6795-sony-xperia-m5.dts
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022, Collabora Ltd
+ * Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
+ */
+
+/dts-v1/;
+#include "mt6795.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
Looks unused.


Right, I'll remove that in v2.

+
+/ {
+    model = "Sony Xperia M5";
+    compatible = "sony,xperia-m5", "mediatek,mt6795";
sony,holly?


I'm sorry, but I can't understand the sense of adding that compatible string to the mix. To the kernel, it doesn't mean anything - and we already have another
string advertising the specific machine, which is "sony,xperia-m5".

I was suggesting replacing xperia-m5 with holly, but since we agreed on keeping

m5 in the dtb name, I suppose it's fine for this one to stay too.



Of course, there is no Xperia M5 with a different SoC and, even if there was a xperia-m5 with a different SoC, we anyway have both a machine compatible and a
SoC compatible in here, so that would still not pose any issue.

+    chassis-type = "handset";
+
+    aliases {
+        mmc0 = &mmc0;
+        mmc1 = &mmc1;
+        serial0 = &uart0;
+        serial1 = &uart1;
+    };
+
+    memory@40000000 {
+        device_type = "memory";
+        reg = <0 0x40000000 0 0x1E800000>;
Lowercase hex in size. Also, doesn't the bootloader fill it in?


Updating the device to the latest software version will give you a bootloader that fills that in, but the first-ever software release contains one that will
not do that in particular conditions (fastboot boot).

Ugh. If only vendors tested their software before shipping it to users..

I think it's worth to adding a comment mentioning that, though.



+    };
+
+    reserved_memory: reserved-memory {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        ranges;
+
+        /* 128 KiB reserved for ARM Trusted Firmware (BL31) */
Is that true for all devices with this SoC, or..? If so, it may be worth
moving this into mt6795.dtsi.

+        bl31_secmon_reserved: secmon@43000000 {
memory@, everywhere. Use labels to name the nodes.


I'm afraid that's not possible, as the bootloader is reading the devicetree
and requires these nodes to follow this naming.

Wow that's bad.. probably deserves a comment to prevent 'cleanups' breaking this.


Konrad


+            no-map;
reg goes first.

Will fix in v2.

Best regards,
Angelo





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux