Re: [PATCH 1/4] arm64: dts: qcom: sdm845: fix number of pins in 'gpio-ranges'

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Shawn Guo <shawn.guo@linaro.org>
Cc: devicetree@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, Evan Green <evgreen@chromium.org>
Subject: Re: [PATCH 1/4] arm64: dts: qcom: sdm845: fix number of pins in 'gpio-ranges'
Date: Fri, 5 Mar 2021 15:43:08 -0600	[thread overview]
Message-ID: <YEKl7GbxBhui4eoT@builder.lan> (raw)
In-Reply-To: <20210303033106.549-2-shawn.guo@linaro.org>

On Tue 02 Mar 21:31 CST 2021, Shawn Guo wrote:

> The last cell of 'gpio-ranges' should be number of GPIO pins, and in
> case of qcom platform it should match msm_pinctrl_soc_data.ngpio rather
> than msm_pinctrl_soc_data.ngpio - 1.
> 

This is a historical artifact, SDM845 has 150 GPIO pins. In addition to
this there's an output-only pin for UFS, which I exposed as an GPIO as
well - but it's only supposed to be used as a reset-gpio for the UFS
device.

Perhaps that still mandates that gpio-ranges should cover it?

> This fixes the problem that when the last GPIO pin in the range is
> configured with the following call sequence, it always fails with
> -EPROBE_DEFER.
> 
>     pinctrl_gpio_set_config()
>         pinctrl_get_device_gpio_range()
>             pinctrl_match_gpio_range()

When do we hit this sequence? I didn't think operations on the UFS
GP(I)O would ever take this code path?

Regards,
Bjorn

> 
> Fixes: bc2c806293c6 ("arm64: dts: qcom: sdm845: Add gpio-ranges to TLMM node")
> Cc: Evan Green <evgreen@chromium.org>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 454f794af547..6a2ed02d383d 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -2382,7 +2382,7 @@
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> -			gpio-ranges = <&tlmm 0 0 150>;
> +			gpio-ranges = <&tlmm 0 0 151>;
>  			wakeup-parent = <&pdc_intc>;
>  
>  			cci0_default: cci0-default {
> -- 
> 2.17.1
> 