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
