| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3c4e63af-1b03-460f-8dc1-24d80b39f054@linaro.org>
Date: Thu, 28 Mar 2024 17:19:49 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: serdeliuk@...oo.com, Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: dts: qcom: Add support for Samsung Galaxy Z Fold5
On 28/03/2024 15:57, Alexandru Marc Serdeliuc via B4 Relay wrote:
> From: Alexandru Marc Serdeliuc <serdeliuk@...oo.com>
>
> Add support for Samsung Galaxy Z Fold5 (q5q) foldable phone
>
> Currently working features:
> - Framebuffer
> - UFS
> - i2c
> - Buttons
>
> Signed-off-by: Alexandru Marc Serdeliuc <serdeliuk@...oo.com>
> ---
> 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@...oo.com>
> + * Copyright (c) 2024, David Wronek <david@...nlining.org>
> + * 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@...00000 {
> + 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@...f4000 {
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@...ff000 {
> + 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
Powered by blists - more mailing lists