Re: [PATCH v2 5/6] arm64: dts: qcom: add SA8540P and ADP

From: Johan Hovold
Date: Wed Jun 22 2022 - 05:24:32 EST

On Tue, Jun 21, 2022 at 09:12:23PM -0700, Bjorn Andersson wrote:
> Introduce the Qualcomm SA8540P automotive platform and the SA8295P ADP
> development board.
>
> The SA8540P and SC8280XP are fairly similar, so the SA8540P is built
> ontop of the SC8280XP dtsi to reduce duplication. As more advanced
> features are integrated this might be re-evaluated.
>
> This initial contribution supports SMP, CPUFreq, cluster idle, UFS, RPMh
> regulators, debug UART, PMICs, remoteprocs (NSPs crashes shortly after
> booting) and USB.
>
> The SA8295P ADP contains four PM8450 PMICs, which according to their
> revid are compatible with PM8150. They are defined within the ADP for
> now, to avoid creating additional .dtsi files for PM8150 with just
> addresses changed - and to allow using the labels from the schematics.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> ---

No change log?

> arch/arm64/boot/dts/qcom/Makefile | 1 +
> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 427 +++++++++++++++++++++++
> arch/arm64/boot/dts/qcom/sa8540p.dtsi | 133 +++++++
> 3 files changed, 561 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> create mode 100644 arch/arm64/boot/dts/qcom/sa8540p.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index ceeae094a59f..2f416b84b71c 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -52,6 +52,7 @@ dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qrb5165-rb5.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sa8155p-adp.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += sa8295p-adp.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sc7180-idp.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r1.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r1-lte.dtb
> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> new file mode 100644
> index 000000000000..8dbcd95966b0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts

> +&remoteproc_adsp {
> + status = "okay";

Please move the status properties last also for ADP.

> + firmware-name = "qcom/sa8540p/adsp.mbn";
> +};

> +/* PINCTRL */
> +&pm8450c_gpios {
> + usb2_en_state: usb2-en-state {
> + pins = "gpio9";
> + function = "normal";
> + output-high;
> + power-source = <0>;
> + };
> +};
> +
> +&pm8450e_gpios {
> + usb3_en_state: usb3-en-state {
> + pins = "gpio5";
> + function = "normal";
> + output-high;
> + power-source = <0>;
> + };
> +};
> +
> +&pm8450g_gpios {
> + usb4_en_state: usb4-en-state {
> + pins = "gpio5";
> + function = "normal";
> + output-high;
> + power-source = <0>;
> + };
> +
> + usb5_en_state: usb5-en-state {
> + pins = "gpio9";
> + function = "normal";
> + output-high;
> + power-source = <0>;
> + };
> +};

Perhaps hold off with adding these always-on VBUS "regulators" until
adding support for the multiport controller?

> diff --git a/arch/arm64/boot/dts/qcom/sa8540p.dtsi b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> new file mode 100644
> index 000000000000..8ea2886fbab2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2022, Linaro Limited
> + */
> +
> +#include "sc8280xp.dtsi"
> +
> +/delete-node/ &cpu0_opp_table;
> +/delete-node/ &cpu4_opp_table;
> +
> +/ {
> + cpu0_opp_table: cpu0-opp-table {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-403200000 {
> + opp-hz = /bits/ 64 <403200000>;
> + };
> + opp-499200000 {
> + opp-hz = /bits/ 64 <499200000>;
> + };
> + opp-595200000 {
> + opp-hz = /bits/ 64 <595200000>;
> + };
> + opp-710400000 {
> + opp-hz = /bits/ 64 <710400000>;
> + };
> + opp-806400000 {
> + opp-hz = /bits/ 64 <806400000>;
> + };
> + opp-902400000 {
> + opp-hz = /bits/ 64 <902400000>;
> + };
> + opp-1017600000 {
> + opp-hz = /bits/ 64 <1017600000>;
> + };
> + opp-1113600000 {
> + opp-hz = /bits/ 64 <1113600000>;
> + };
> + opp-1209600000 {
> + opp-hz = /bits/ 64 <1209600000>;
> + };
> + opp-1324800000 {
> + opp-hz = /bits/ 64 <1324800000>;
> + };
> + opp-1440000000 {
> + opp-hz = /bits/ 64 <1440000000>;
> + };
> + opp-1555200000 {
> + opp-hz = /bits/ 64 <1555200000>;
> + };
> + opp-1670400000 {
> + opp-hz = /bits/ 64 <1670400000>;
> + };
> + opp-1785600000 {
> + opp-hz = /bits/ 64 <1785600000>;
> + };
> + opp-1881600000 {
> + opp-hz = /bits/ 64 <1881600000>;
> + };
> + opp-2016000000 {
> + opp-hz = /bits/ 64 <2016000000>;
> + };
> + opp-2131200000 {
> + opp-hz = /bits/ 64 <2131200000>;
> + };
> + opp-2246400000 {
> + opp-hz = /bits/ 64 <2246400000>;
> + };
> + };
> +
> + cpu4_opp_table: cpu4-opp-table {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-825600000 {
> + opp-hz = /bits/ 64 <825600000>;
> + };
> + opp-940800000 {
> + opp-hz = /bits/ 64 <940800000>;
> + };
> + opp-1056000000 {
> + opp-hz = /bits/ 64 <1056000000>;
> + };
> + opp-1171200000 {
> + opp-hz = /bits/ 64 <1171200000>;
> + };
> + opp-1286400000 {
> + opp-hz = /bits/ 64 <1286400000>;
> + };
> + opp-1401600000 {
> + opp-hz = /bits/ 64 <1401600000>;
> + };
> + opp-1516800000 {
> + opp-hz = /bits/ 64 <1516800000>;
> + };
> + opp-1632000000 {
> + opp-hz = /bits/ 64 <1632000000>;
> + };
> + opp-1747200000 {
> + opp-hz = /bits/ 64 <1747200000>;
> + };
> + opp-1862400000 {
> + opp-hz = /bits/ 64 <1862400000>;
> + };
> + opp-1977600000 {
> + opp-hz = /bits/ 64 <1977600000>;
> + };
> + opp-2073600000 {
> + opp-hz = /bits/ 64 <2073600000>;
> + };
> + opp-2169600000 {
> + opp-hz = /bits/ 64 <2169600000>;
> + };
> + opp-2284800000 {
> + opp-hz = /bits/ 64 <2284800000>;
> + };
> + opp-2380800000 {
> + opp-hz = /bits/ 64 <2380800000>;
> + };
> + opp-2496000000 {
> + opp-hz = /bits/ 64 <2496000000>;
> + };
> + opp-2592000000 {
> + opp-hz = /bits/ 64 <2592000000>;
> + };
> + };
> +};
> +
> +&rpmhpd {
> + compatible = "qcom,sa8540p-rpmhpd";
> +};

Should we add sa8540p compatibles (with sc8280xp as fallback) for the
other peripherals from the start in case we find out that there are
indeed differences down the line?

Looks good otherwise:

Reviewed-by: Johan Hovold <johan+linaro@xxxxxxxxxx>

Johan