Re: [PATCH v4 5/6] arm64: dts: sdm845: Add serial console support

From: Stephen Boyd <swboyd@chromium.org>
To: Karthikeyan Ramasubramanian <kramasub@codeaurora.org>,
	andy.gross@linaro.org, corbet@lwn.net, david.brown@linaro.org,
	gregkh@linuxfoundation.org, mark.rutland@arm.com,
	robh+dt@kernel.org, wsa@the-dreams.de
Cc: Rajendra Nayak <rnayak@codeaurora.org>,
	linux-doc@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-i2c@vger.kernel.org,
	linux-serial@vger.kernel.org, jslaby@suse.com,
	evgreen@chromium.org, acourbot@chromium.org
Subject: Re: [PATCH v4 5/6] arm64: dts: sdm845: Add serial console support
Date: Tue, 20 Mar 2018 12:39:05 -0700	[thread overview]
Message-ID: <152157474508.187949.10348604942277810227@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <1521071931-9294-6-git-send-email-kramasub@codeaurora.org>

Quoting Karthikeyan Ramasubramanian (2018-03-14 16:58:50)
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 979ab49..ea3efc5 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -12,4 +12,43 @@
>  / {
>         model = "Qualcomm Technologies, Inc. SDM845 MTP";
>         compatible = "qcom,sdm845-mtp";
> +
> +       aliases {
> +               serial0 = &uart2;
> +       };
> +
> +       chosen {
> +               stdout-path = "serial0";

Also add :115200n8 ?

> +       };
> +};
> +
> +&soc {

I think the method is to put these inside soc node without using the
phandle reference. So indent everything once more.

> +       geniqup@ac0000 {
> +               serial@a84000 {
> +                       status = "okay";
> +               };
> +       };
> +
> +       pinctrl@3400000 {
> +               qup-uart2-default {
> +                       pinconf_tx {
> +                               pins = "gpio4";
> +                               drive-strength = <2>;
> +                               bias-disable;
> +                       };
> +
> +                       pinconf_rx {
> +                               pins = "gpio5";
> +                               drive-strength = <2>;
> +                               bias-pull-up;
> +                       };
> +               };
> +
> +               qup-uart2-sleep {
> +                       pinconf {
> +                               pins = "gpio4", "gpio5";
> +                               bias-pull-down;
> +                       };
> +               };
> +       };
>  };
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 32f8561..59334d9 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -6,6 +6,7 @@
>   */
>  
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/qcom,gcc-sdm845.h>
>  
>  / {
>         interrupt-parent = <&intc>;
> @@ -194,6 +195,20 @@
>                         #gpio-cells = <2>;
>                         interrupt-controller;
>                         #interrupt-cells = <2>;
> +
> +                       qup_uart2_default: qup-uart2-default {
> +                               pinmux {
> +                                       function = "qup9";
> +                                       pins = "gpio4", "gpio5";
> +                               };
> +                       };
> +
> +                       qup_uart2_sleep: qup-uart2-sleep {
> +                               pinmux {
> +                                       function = "gpio";
> +                                       pins = "gpio4", "gpio5";
> +                               };
> +                       };

Are these supposed to go to the board file?

>                 };
>  
>                 timer@17c90000 {
> @@ -272,5 +287,28 @@
>                         #interrupt-cells = <4>;
>                         cell-index = <0>;
>                 };
> +
> +               geniqup@ac0000 {
> +                       compatible = "qcom,geni-se-qup";
> +                       reg = <0xac0000 0x6000>;
> +                       clock-names = "m-ahb", "s-ahb";
> +                       clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
> +                                <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges;

Add a status = "disabled" here too.

> +
> +                       uart2: serial@a84000 {
> +                               compatible = "qcom,geni-debug-uart";
> +                               reg = <0xa84000 0x4000>;
> +                               clock-names = "se";
> +                               clocks = <&gcc GCC_QUPV3_WRAP1_S1_CLK>;
> +                               pinctrl-names = "default", "sleep";
> +                               pinctrl-0 = <&qup_uart2_default>;
> +                               pinctrl-1 = <&qup_uart2_sleep>;
> +                               interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
> +                               status = "disabled";
> +                       };
> +               };
>         };
>  };