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;
Date:   Mon, 17 Jul 2023 12:45:30 +0200
From:   Stephan Gerhold <stephan@gerhold.net>
To:     Bhupesh Sharma <bhupesh.sharma@linaro.org>
Cc:     linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-usb@vger.kernel.org, agross@kernel.org, andersson@kernel.org,
        konrad.dybcio@linaro.org, linux-kernel@vger.kernel.org,
        bhupesh.linux@gmail.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, krzysztof.kozlowski@linaro.org,
        quic_schowdhu@quicinc.com, gregkh@linuxfoundation.org
Subject: Re: [PATCH v8 3/4] arm64: dts: qcom: sm6115: Add EUD dt node and
 dwc3 connector
Message-ID: <ZLUbyocjNT2bGvVt@gerhold.net>
References: <20230717103236.1246771-1-bhupesh.sharma@linaro.org>
 <20230717103236.1246771-4-bhupesh.sharma@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230717103236.1246771-4-bhupesh.sharma@linaro.org>
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, Jul 17, 2023 at 04:02:35PM +0530, Bhupesh Sharma wrote:
> Add the Embedded USB Debugger(EUD) device tree node for
> SM6115 / SM4250 SoC.
> 
> The node contains EUD base register region, EUD mode manager
> register region and TCSR Base register region along with the
> interrupt entry.
> 
> [...]
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 50 ++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index 839c603512403..db45337c1082c 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> [...]
> @@ -789,6 +801,37 @@ gcc: clock-controller@1400000 {
>  			#power-domain-cells = <1>;
>  		};
>  
> +		eud: eud@1610000 {
> +			compatible = "qcom,sm6115-eud", "qcom,eud";
> +			reg = <0x0 0x01610000 0x0 0x2000>,
> +			      <0x0 0x01612000 0x0 0x1000>,
> +			      <0x0 0x003c0000 0x0 0x40000>;
> +			reg-names = "eud-base", "eud-mode-mgr", "tcsr-base";

TCSR is a separate hardware block unrelated to the EUD. IMHO it
shouldn't be listed as "reg" here.

Typically we describe it as syscon and then reference it from other
nodes. See e.g. sm8450.dtsi "tcsr: syscon@1fc0000" referenced in &scm
"qcom,dload-mode = <&tcsr 0x13000>". This is pretty much exactly the
same use case as you have. It also uses this to write something with
qcom_scm_io_writel() at the end.

Thanks,
Stephan