Re: [PATCH v2] arm64: dts: qcom: starqltechn: add initial device tree for starqltechn

From: Krzysztof Kozlowski
Date: Fri Jun 17 2022 - 22:00:11 EST


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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxx/

> +
> + 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@9D400000 {

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@9d400000 {
> + reg = <0x0 0x9d400000 0x0 0x02400000>;
> + no-map;
> + };
> +
> + ramoops@A1300000 {

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