Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 26 Jul 2021 08:42:38 -0700
From:   Matthias Kaehlcke <mka@chromium.org>
To:     Rajesh Patil <rajpat@codeaurora.org>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, rnayak@codeaurora.org,
        saiprakash.ranjan@codeaurora.org, msavaliy@qti.qualcomm.com,
        skakit@codeaurora.org, Roja Rani Yarubandi <rojay@codeaurora.org>
Subject: Re: [PATCH V4 3/4] arm64: dts: sc7280: Update QUPv3 Debug UART DT
 node
Message-ID: <YP7X7kjH9wd818Xg@google.com>
References: <1627306847-25308-1-git-send-email-rajpat@codeaurora.org>
 <1627306847-25308-4-git-send-email-rajpat@codeaurora.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1627306847-25308-4-git-send-email-rajpat@codeaurora.org>
Precedence: bulk

On Mon, Jul 26, 2021 at 07:10:46PM +0530, Rajesh Patil wrote:
> From: Roja Rani Yarubandi <rojay@codeaurora.org>
> 
> Update QUPv3 Debug UART DT node with the interconnect names and
> functions for SC7280 SoC.
> 
> Split the Debug UART pin control functions.
> 
> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org>
> Signed-off-by: Rajesh Patil <rajpat@codeaurora.org>
> ---
> Changes in V4:
>  - As per Bjorn's comment, posting this debug-uart node update
>    as seperate patch
> 
>  arch/arm64/boot/dts/qcom/sc7280-idp.dts | 18 +++++++-----------
>  arch/arm64/boot/dts/qcom/sc7280.dtsi    | 28 ++++++++++++++++++++++++----
>  2 files changed, 31 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> index f63cf51..a50c9e5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> @@ -383,18 +383,14 @@
>  	bias-pull-up;
>  };
>  
> -&qup_uart5_default {
> -	tx {
> -		pins = "gpio46";
> -		drive-strength = <2>;
> -		bias-disable;
> -	};
> +&qup_uart5_tx {
> +	drive-strength = <2>;
> +	bias-disable;
> +};
>  
> -	rx {
> -		pins = "gpio47";
> -		drive-strength = <2>;
> -		bias-pull-up;
> -	};
> +&qup_uart5_rx {
> +	drive-strength = <2>;
> +	bias-pull-up;
>  };
>  
>  &sdc1_on {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 455e58f..951818f 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -853,8 +853,13 @@
>  				clock-names = "se";
>  				clocks = <&gcc GCC_QUPV3_WRAP0_S5_CLK>;
>  				pinctrl-names = "default";
> -				pinctrl-0 = <&qup_uart5_default>;
> +				pinctrl-0 = <&qup_uart5_cts>, <&qup_uart5_rts>, <&qup_uart5_tx>, <&qup_uart5_rx>;
>  				interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>;
> +				power-domains = <&rpmhpd SC7280_CX>;
> +				operating-points-v2 = <&qup_opp_table>;
> +				interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
> +						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>;
> +				interconnect-names = "qup-core", "qup-config";


Most of the above should be added by patch '[2/4] arm64: dts: sc7280: Add QUPv3
wrapper_0 nodes'.

I have to say I dislike that the SoC DT file dictates which UART to use for
the serial console. Technically it could be any of them, right? uart5 is
used because that's what the IDP does, and the rest of the world is expected
to follow. Why not configure uart5 as "qcom,geni-uart" by default and
overwrite the compatible string and pinctrl in the board file? You could even
add 'qup-uartN-all' (or similar) pinconfigs to sc7280.dtsi, which would make
the changes in the board file trivial.