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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YrLfwnbmdcniKeOG@hovoldconsulting.com>
Date:   Wed, 22 Jun 2022 11:24:18 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     Andy Gross <agross@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Jassi Brar <jassisinghbrar@...il.com>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/6] arm64: dts: qcom: add SA8540P and ADP

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@...aro.org>
> ---

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@...nel.org>

Johan

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ