Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d960d442-4b0a-ef33-04da-8fe3a3c8fb90@somainline.org>
Date:   Thu, 21 Jul 2022 20:41:34 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH 1/2] arm64: dts: qcom: introduce sa8540p-ride dts
Content-Language: en-US
To:     Parikshit Pareek <quic_ppareek@quicinc.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc:     linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220721154057.15276-1-quic_ppareek@quicinc.com>
From:   Konrad Dybcio <konrad.dybcio@somainline.org>
In-Reply-To: <20220721154057.15276-1-quic_ppareek@quicinc.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


Hello!

On 21.07.2022 17:40, Parikshit Pareek wrote:
> Create new dts file specific for Qdrive board based on sa8540p chipset.
Is the SA8540P any different than SA8295P? My wild guess is that they're
binned versions of each other with different DVFS levels.. That could
use a separate SoC DTSI to hold these differences should that be true..


> Introduce common dtsi file sa8295p-adp.dtsi, to be included for adp and
> Qdrive board.
> 
> Signed-off-by: Parikshit Pareek <quic_ppareek@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/Makefile             |   1 +
>  arch/arm64/boot/dts/qcom/sa8295p-adp.dts      | 378 +----------------
>  arch/arm64/boot/dts/qcom/sa8295p-adp.dtsi     | 385 ++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sa8540p-adp-ride.dts |  15 +
>  4 files changed, 403 insertions(+), 376 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/qcom/sa8295p-adp.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/sa8540p-adp-ride.dts
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 9e2a13d75f9d..fa0abcf7660b 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -51,6 +51,7 @@ 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) += sa8540p-adp-ride.dtb
This needs to be a tab.


>  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
> index 9398f0349944..adb6637117bc 100644
> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> @@ -2,388 +2,14 @@
>  /*
>   * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>   * Copyright (c) 2022, Linaro Limited
> + * Copyright (c) 2022 Qualcomm Innovaion Center, Inc. All rights reserved.
Please add a comma after the year to keep it consistent.


>   */
>  
>  /dts-v1/;

[...]

> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dtsi b/arch/arm64/boot/dts/qcom/sa8295p-adp.dtsi
> new file mode 100644
> index 000000000000..75ef77b7cac1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dtsi
Are the boards based on each other? Or on a similar base platform?
Maybe sa8295p/sa8540p/sasomethingelse-automotive.dtsi could be a 
better name in the latter case?


[...]

> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sa8540p-adp-ride.dts
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2022, Linaro Limited
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "sa8295p-adp.dtsi"
> +
> +/ {
> +	model = "Qualcomm SA8540 ADP";
So "Qdrive board" == SA8540 ADP == SA8540 ADP Ride? Or is there
a base platform for all of them? Maybe the Qdrive is simply based
on the ADPs? Is there a clear distinction between ADP and ADP Ride?

Konrad

> +	compatible = "qcom,sa8540p-adp-ride", "qcom,sa8540p";
> +};