Hi,

On Thu, Sep 23, 2021 at 09:57:31PM +0300, dsankou...@gmail.com wrote:
> From: Dzmitry Sankouski <dsankou...@gmail.com>
> 
> Hi-end qualcomm chip, introduced in late 2017.
> Mostly used in flagship phones and tablets of 2018.
> Features:
> - arm64 arch
> - total of 8 Kryo 385 Gold / Silver cores
> - Hexagon 685 DSP
> - Adreno 630 GPU
> 
> Tested only as second-stage bootloader.
> 
> Signed-off-by: Dzmitry Sankouski <dsankou...@gmail.com>
> Cc: Ramon Fried <rfried....@gmail.com>
> Cc: Tom Rini <tr...@konsulko.com>
> ---
>  arch/arm/dts/sdm845.dtsi                      | 118 ++++++++++++++++++
>  arch/arm/mach-snapdragon/Kconfig              |   4 +
>  arch/arm/mach-snapdragon/Makefile             |   4 +
>  .../include/mach/sysmap-sdm845.h              |  42 +++++++
>  arch/arm/mach-snapdragon/init_sdm845.c        |  82 ++++++++++++
>  arch/arm/mach-snapdragon/sysmap-sdm845.c      |  31 +++++
>  include/configs/sdm845.h                      |  33 +++++
>  7 files changed, 314 insertions(+)
>  create mode 100644 arch/arm/dts/sdm845.dtsi
>  create mode 100644 arch/arm/mach-snapdragon/include/mach/sysmap-sdm845.h
>  create mode 100644 arch/arm/mach-snapdragon/init_sdm845.c
>  create mode 100644 arch/arm/mach-snapdragon/sysmap-sdm845.c
>  create mode 100644 include/configs/sdm845.h
> 
> diff --git a/arch/arm/dts/sdm845.dtsi b/arch/arm/dts/sdm845.dtsi
> new file mode 100644
> index 0000000000..bf32c6995d
> --- /dev/null
> +++ b/arch/arm/dts/sdm845.dtsi
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Qualcomm SDM845 chip device tree source
> + *
> + * (C) Copyright 2021 Dzmitry Sankouski <dsankou...@gmail.com>
> + *
> + */
> +
> +/dts-v1/;
> +
> +#include "skeleton64.dtsi"
> +
> +/ {
> +     soc: soc {
> +             #address-cells = <1>;
> +             #size-cells = <1>;
> +             ranges = <0 0 0 0xffffffff>;
> +             compatible = "simple-bus";
> +
> +             gcc: clock-controller@100000 {
> +                     u-boot,dm-pre-reloc;
> +                     compatible = "qcom,gcc-sdm845";
> +                     reg = <0x00100000 0x1f0000>;
> +                     #clock-cells = <1>;
> +                     #reset-cells = <1>;
> +                     #power-domain-cells = <1>;
> +             };
> +
> +             gpio_north: gpio_north@3900000 {
> +                     u-boot,dm-pre-reloc;
> +                     #gpio-cells = <2>;
> +                     compatible = "qcom,sdm845-pinctrl";
> +                     reg = <0x3900000 0x400000>;
> +                     gpio-count = <150>;
> +                     gpio-controller;
> +                     gpio-ranges = <&gpio_north 0 0 150>;
> +                     gpio-bank-name = "soc_north.";
> +             };
> +
> +             tlmm_north: pinctrl_north@3900000 {
> +                     u-boot,dm-pre-reloc;
> +                     compatible = "qcom,tlmm-sdm845";
> +                     reg = <0x03900000 0x400000>;

Can you consistently pad "reg" with zeroes or not?
"gcc" is padded with zeroes to 8 digits, gpio_north is not,
then this is suddenly again. This makes it look like gpio_north
and tlmm_north have a different address when it's actually the same.

> +                     gpio-count = <150>;
> +                     gpio-controller;
> +                     #gpio-cells = <2>;
> +                     gpio-ranges = <&tlmm_north 0 0 150>;
> +
> +                     /* DEBUG UART */
> +                     qup_uart9: qup-uart9-default {
> +                             pinmux {
> +                                     pins = "GPIO_4", "GPIO_5";
> +                                     function = "qup9";
> +                             };
> +                     };
> +             };
> +
> +             debug_uart: serial@0xa84000 {

The 0x should not be in the unit name (after @).

> +                     compatible = "qcom,msm-geni-uart";
> +                     reg = <0xa84000 0x4000>;
> +                     reg-names = "se_phys";
> +                     clock-names = "se-clk";
> +                     clocks = <&gcc 0x58>;
> +                     pinctrl-names = "default";
> +                     pinctrl-0 = <&qup_uart9>;
> +                     qcom,wrapper-core = <0x8a>;
> +                     status = "disabled";
> +             };
> +
> +             spmi@c440000 {
> +                     compatible = "qcom,spmi-pmic-arb";
> +                     reg = <0xc440000 0x1100>,
> +                           <0xc600000 0x2000000>,
> +                           <0xe600000 0x100000>;
> +                     reg-names = "cnfg", "core", "obsrvr";
> +                     #address-cells = <0x1>;
> +                     #size-cells = <0x1>;
> +
> +                     qcom,revid@100 {
> +                             compatible = "qcom,qpnp-revid";
> +                             reg = <0x100 0x100>;
> +                             linux,phandle = <0x3ac>;
> +                             phandle = <0x3ac>;
> +                     };
> +

What is this used for? With the linux,phandle this looks a bit like
something copied from a decompiled downstream dtb.

> +                     pmic0: pm8998@0 {
> +                             compatible = "qcom,spmi-pmic";
> +                             reg = <0x0 0x1>;
> +                             #address-cells = <0x1>;
> +                             #size-cells = <0x1>;
> +
> +                             pm8998_pon: pm8998_pon@800 {
> +                                     compatible = "qcom,pm8998-pwrkey";
> +                                     reg = <0x800 0x100>;
> +                                     #gpio-cells = <2>;
> +                                     gpio-controller;
> +                                     gpio-bank-name="pm8998_key.";
> +                             };
> +
> +                             pm8998_gpios: pm8998_gpios@c000 {
> +                                     compatible = "qcom,pm8998-gpio";
> +                                     reg = <0xc000 0x1a00>;
> +                                     gpio-controller;
> +                                     gpio-count = <21>;
> +                                     #gpio-cells = <2>;
> +                                     gpio-bank-name="pm8998.";

Can you add spaces around '=' (gpio-bank-name = "pm8998.";), same for 
pm8998_pon.

Thanks,
Stephan

Reply via email to