[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9bf584a1-bab9-33f3-02b6-6b10a7cfb043@linaro.org>
Date: Fri, 17 Jun 2022 19:00:01 -0700
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Dzmitry Sankouski <dsankouski@...il.com>
Cc: Andy Gross <agross@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Kees Cook <keescook@...omium.org>,
Anton Vorontsov <anton@...msg.org>,
Colin Cross <ccross@...roid.com>,
Tony Luck <tony.luck@...el.com>, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] arm64: dts: qcom: starqltechn: add initial device tree
for starqltechn
On 17/06/2022 04:01, Dzmitry Sankouski wrote:
> New device support - Samsung S9 (SM-G9600) phone
> What works:
> - simple framebuffer
> - storage (both main and sdcard)
> - ramoops
>
> Signed-off-by: Dzmitry Sankouski <dsankouski@...il.com>
> ---
> Changes for v2:
> - rebase on Linus's master branch
>
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> .../dts/qcom/sdm845-samsung-starqltechn.dts | 45 ++
> .../dts/qcom/sdm845-samsung-starqltechn.dtsi | 448 ++++++++++++++++++
> 3 files changed, 494 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 2f8aec2cc6db..85140db507b6 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -103,6 +103,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-mtp.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-oneplus-enchilada.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-oneplus-fajita.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += sdm845-samsung-starqltechn.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-sony-xperia-tama-akari.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-sony-xperia-tama-akatsuki.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-sony-xperia-tama-apollo.dtb
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts b/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> new file mode 100644
> index 000000000000..53bb2ebf0061
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SDM845 Samsung S9 (SM-G9600) (starqltechn) specific device tree
> + *
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + */
> +
> +#include "sdm845-samsung-starqltechn.dtsi"
> +
> +/ {
> + model = "Samsung S9 SM-G9600";
> + compatible = "samsung,starqltechn", "qcom,sdm845";
Missing documentation. I guess you should rebase on my series adding
compatibles for sdm845.
Bjorn, can we get my series merged? It's really more and more DTs
needing it.
https://lore.kernel.org/all/20220521164550.91115-7-krzysztof.kozlowski@linaro.org/
> +
> + chosen {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + bootargs = "console=tty1 loglevel=15 clk_ignore_unused PMOS_NO_OUTPUT_REDIRECT";
This should not be in DTS. Especially debugging stuff... console should
be selected by stdout-path.
> + framebuffer: framebuffer@...00000 {
lowercase addresses everywhere
> + compatible = "simple-framebuffer";
> + reg = <0 0x9D400000 0 (2960 * 1440 * 4)>;//2400000
> + width = <1440>;
> + height = <2960>;
> + stride = <(1440 * 4)>;
> + format = "a8r8g8b8";
> + };
> + };
> +
> +
Just one blank line.
> + reserved-memory {
> + framebuffer_region@...00000 {
> + reg = <0x0 0x9d400000 0x0 0x02400000>;
> + no-map;
> + };
> +
> + ramoops@...00000 {
lowercase hex. In entire file.
> + compatible = "ramoops";
> + reg = <0x0 0xa1300000 0x0 0x100000>;
> + record-size = <0x40000>;
> + console-size = <0x40000>;
> + ftrace-size = <0x40000>;
> + pmsg-size = <0x40000>;
> + };
> + };
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dtsi b/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dtsi
> new file mode 100644
> index 000000000000..1d640d88c61a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dtsi
Why do you have DTSI for one DTS?
> @@ -0,0 +1,448 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SDM845 Samsung S9 (SM-G9600) (starqltechn / star2qltechn) common device tree source
> + *
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +#include "sdm845.dtsi"
> +
> +/ {
> + chassis-type = "handset";
> +
> + vph_pwr: vph-pwr-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vph_pwr";
> + regulator-min-microvolt = <3700000>;
> + regulator-max-microvolt = <3700000>;
> + };
> +
> + /*
> + * Apparently RPMh does not provide support for PM8998 S4 because it
> + * is always-on; model it as a fixed regulator.
> + */
> + vreg_s4a_1p8: pm8998-smps4 {
Consistent naming. Please apply all comments from your other patchset...
> + compatible = "regulator-fixed";
> + regulator-name = "vreg_s4a_1p8";
> +
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> +
> + regulator-always-on;
> + regulator-boot-on;
> +
> + vin-supply = <&vph_pwr>;
> + };
> +};
> +
> +
One blank line.
> +&apps_rsc {
> + pm8998-rpmh-regulators {
> + compatible = "qcom,pm8998-rpmh-regulators";
> + qcom,pmic-id = "a";
> +
Best regards,
Krzysztof
Powered by blists - more mailing lists