On 28/03/2024 15:57, Alexandru Marc Serdeliuc via B4 Relay wrote: > From: Alexandru Marc Serdeliuc <serdeliuk@xxxxxxxxx> > > Add support for Samsung Galaxy Z Fold5 (q5q) foldable phone > > Currently working features: > - Framebuffer > - UFS > - i2c > - Buttons > > Signed-off-by: Alexandru Marc Serdeliuc <serdeliuk@xxxxxxxxx> > --- > arch/arm64/boot/dts/qcom/Makefile | 1 + > arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts | 616 ++++++++++++++++++++++++ > 2 files changed, 617 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > index 7d40ec5e7d21..a7503fd35b6c 100644 > --- a/arch/arm64/boot/dts/qcom/Makefile > +++ b/arch/arm64/boot/dts/qcom/Makefile > @@ -241,6 +241,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sm8450-sony-xperia-nagara-pdx224.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8550-hdk.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8550-mtp.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8550-qrd.dtb > +dtb-$(CONFIG_ARCH_QCOM) += sm8550-samsung-q5q.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8650-mtp.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8650-qrd.dtb > dtb-$(CONFIG_ARCH_QCOM) += x1e80100-crd.dtb > diff --git a/arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts b/arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts > new file mode 100644 > index 000000000000..ac8392022a7f > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sm8550-samsung-q5q.dts > @@ -0,0 +1,616 @@ > +// SPDX-License-Identifier: BSD-3-Clause > +/* > + * Copyright (c) 2024, Alexandru Marc Serdeliuc <serdeliuk@xxxxxxxxx> > + * Copyright (c) 2024, David Wronek <david@xxxxxxxxxxxxxx> > + * Copyright (c) 2022, Linaro Limited > + */ > + > +/dts-v1/; > + > +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > +#include "sm8550.dtsi" > +#include "pm8550.dtsi" > +#include "pm8550vs.dtsi" > +#include "pmk8550.dtsi" > + > +/ { > + model = "Samsung Galaxy Z Fold5"; > + compatible = "samsung,q5q", "qcom,sm8550"; > + #address-cells = <0x02>; > + #size-cells = <0x02>; > + chassis-type = "handset"; > + > + aliases { > + serial0 = &uart7; > + }; > + > + chosen { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + bootargs = "console=tty0 clk_ignore_unused pd_ignore_unused"; Console should be chosed via stdout. The "unused" are not parts of hardware description, so drop entire bootargs. > + > + framebuffer: framebuffer@b8000000 { > + compatible = "simple-framebuffer"; > + reg = <0x0 0xb8000000 0x0 0x2b00000>; > + width = <2176>; > + height = <1812>; > + stride = <(2176 * 4)>; > + format = "a8r8g8b8"; > + }; > + }; > + > + gpio-keys { > + compatible = "gpio-keys"; > + pinctrl-0 = <&volume_up_n>; > + pinctrl-names = "default"; > + > + key-volume-up { > + label = "Volume Up"; > + linux,code = <KEY_VOLUMEUP>; > + gpios = <&pm8550_gpios 6 GPIO_ACTIVE_LOW>; > + debounce-interval = <15>; > + linux,can-disable; > + wakeup-source; > + }; > + }; > + > + vph_pwr: vph-pwr-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "vph_pwr"; > + regulator-min-microvolt = <3700000>; > + regulator-max-microvolt = <3700000>; > + regulator-always-on; > + regulator-boot-on; > + }; > + > + reserved-memory { > + chipinfo_region@81cf4000 { Underscores are not allowed, use hyphens. It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + reg = <0x0 0x81cf4000 0x0 0x1000>; > + no-map; > + }; > + > + kaslr_region@b01ff000 { > + reg = <0x0 0xb01ff000 0x0 0x1000>; > + no-map; > + }; > + > + uh_guest_region { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + reg = <0x0 0xb1000000 0x0 0x3000000>; > + no-map; > + }; > + > + uh_heap_region { > + reg = <0x0 0xb0200000 0x0 0x40000>; > + no-map; > + }; > + > +/* > + * The bootloader will only keep display hardware enabled > + * if this memory region is named exactly 'splash_region' > + */ Missing indentation > + splash_region { > + reg = <0x0 0xb8000000 0x0 0x2b00000>; > + no-map; > + }; > + }; // end reserved-memory Drop such comments. There is no such code in any mainline DTS. Please use mainline DTS as your starting point. You now duplicated a lot of issues which we solved already. That's not how you upstream your board. ... > + }; // end regulators-0 Really, drop the comment. ... > + > +&pcie0 { > + status = "okay"; Status is the last property. > + wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>; > + perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pcie0_default_state>; Reverse these two. > +}; > + > +&pcie0_phy { > + status = "okay"; > + vdda-phy-supply = <&vreg_l1e_0p88>; > + vdda-pll-supply = <&vreg_l3e_1p2>; > +}; > + > +&pcie1 { > + status = "okay"; > + wake-gpios = <&tlmm 99 GPIO_ACTIVE_HIGH>; > + perst-gpios = <&tlmm 97 GPIO_ACTIVE_LOW>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pcie1_default_state>; > +}; > + > +&pcie1_phy { > + vdda-phy-supply = <&vreg_l3c_0p91>; > + vdda-pll-supply = <&vreg_l3e_1p2>; > + vdda-qref-supply = <&vreg_l1e_0p88>; > + status = "okay"; > +}; > + > +&pm8550_gpios { > + volume_up_n: volume-up-n-state { > + pins = "gpio6"; > + function = "normal"; > + power-source = <1>; > + bias-pull-up; > + input-enable; > + }; > +}; > + > +&qupv3_id_0 { > + status = "okay"; > +}; > + > +&remoteproc_adsp { > + status = "okay"; > + firmware-name = "qcom/sm8550/adsp.mbn", > + "qcom/sm8550/adsp_dtb.mbn"; > +}; > + > +&remoteproc_cdsp { > + status = "okay"; > + firmware-name = "qcom/sm8550/cdsp.mbn", > + "qcom/sm8550/cdsp_dtb.mbn"; > +}; > + > +&remoteproc_mpss { > + status = "okay"; > + firmware-name = "qcom/sm8550/modem.mbn", > + "qcom/sm8550/modem_dtb.mbn"; > +}; > + > +&sleep_clk { > + clock-frequency = <32000>; > +}; > + > +&tlmm { > + gpio-reserved-ranges = <36 4>, <50 2>; > +}; > + > +&ufs_mem_hc { > + status = "okay"; > + reset-gpios = <&tlmm 210 GPIO_ACTIVE_LOW>; > + vcc-supply = <&vreg_l17b_2p5>; > + vcc-max-microamp = <1300000>; > + vccq-supply = <&vreg_l1g_1p2>; > + vccq-max-microamp = <1200000>; > + vdd-hba-supply = <&vreg_l3g_1p2>; > +}; > + > +&ufs_mem_phy { > + status = "okay"; > + vdda-phy-supply = <&vreg_l1d_0p88>; > + vdda-pll-supply = <&vreg_l3e_1p2>; > +}; > + > +&xo_board { > + clock-frequency = <76800000>; > +}; > + > +&dispcc { > + status = "disabled"; > +}; > + > +&pon_pwrkey { Does not look ordered by name. Keep alphanetical order of node overrides. Just like all other DTS does, which you should take as starting point. Best regards, Krzysztof