Received-SPF: pass (google.com: domain of linux-kernel+bounces-13086-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Fri, 29 Dec 2023 13:55:04 +0100
From: Johan Hovold <johan@kernel.org>
To: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Bjorn Andersson <andersson@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Johan Hovold <johan+linaro@kernel.org>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Konrad Dybcio <konrad.dybcio@somainline.org>
Subject: Re: [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: Add PCIe CLKREQ#
 sleep state
Message-ID: <ZY7BqLJZ0btpuL4b@hovoldconsulting.com>
References: <20231227-topic-8280_pcie_dts-v1-0-13d12b1698ff@linaro.org>
 <20231227-topic-8280_pcie_dts-v1-3-13d12b1698ff@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231227-topic-8280_pcie_dts-v1-3-13d12b1698ff@linaro.org>

On Wed, Dec 27, 2023 at 11:28:28PM +0100, Konrad Dybcio wrote:
> The CLKREQ pin should not be muxed to its active function when the RC
> is asleep. 

You forgot to explain *why* you think this is needed.

Note that this is only appears to be done for one upstream Qualcomm SoC
(msm8996) currently, and that, notably, there is no driver support for
actually changing the pin state.

> Add the missing pin sleep states to resolve that.

> Fixes: d907fe5acbf1 ("arm64: dts: qcom: sc8280xp-crd: enable WiFi controller")
> Fixes: 17e2ccaf65d1 ("arm64: dts: qcom: sc8280xp-crd: enable SDX55 modem")
> Fixes: 6a1ec5eca73c ("arm64: dts: qcom: sc8280xp-crd: enable NVMe SSD")

So not sure these Fixes tags are warranted either.

> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 78 ++++++++++++++++++++-----------
>  1 file changed, 51 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> index ffc4406422ae..58c0c2d10cb3 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> @@ -530,8 +530,9 @@ &pcie2a {
>  
>  	vddpe-3v3-supply = <&vreg_nvme>;
>  
> -	pinctrl-names = "default";
> -	pinctrl-0 = <&pcie2a_default>;
> +	pinctrl-0 = <&pcie2a_default>, <&pcie2a_clkreq_default>;
> +	pinctrl-1 = <&pcie2a_default>, <&pcie2a_clkreq_sleep>;
> +	pinctrl-names = "default", "sleep";

Johan