Re: [PATCH v4 3/3] arm64: dts: qcom: sc8280xp-x13s: model the PMU of the on-board wcn6855

[Johan Hovold <johan@kernel.org>]

On Mon, Sep 30, 2024 at 12:30:39PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Add a node for the PMU of the WCN6855 and rework the inputs of the wifi
> and bluetooth nodes to consume the PMU's outputs.
> 
> With this we can drop the regulator-always-on properties from vreg_s11b
> and vreg_s12b as they will now be enabled by the power sequencing
> driver.
> 
> Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Without this patch I'm seeing an indefinite probe deferral with
6.12-rc1:

	platform 1c00000.pcie:pcie@0:wifi@0: deferred probe pending: pci-pwrctl-pwrseq: Failed to get the power sequencer

Can you please look into that and make sure that the existing DT
continues to work without such warnings.

> ---
>  .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    | 100 +++++++++++++++---
>  1 file changed, 86 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> index 6a28cab97189..7230d5420199 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> @@ -400,6 +400,67 @@ usb1_sbu_mux: endpoint {
>  			};
>  		};
>  	};
> +
> +	wcn6855-pmu {
> +		compatible = "qcom,wcn6855-pmu";
> +
> +		pinctrl-0 = <&bt_default>, <&wlan_en>;
> +		pinctrl-names = "default";
> +
> +		wlan-enable-gpios = <&tlmm 134 GPIO_ACTIVE_HIGH>;
> +		bt-enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>;

> @@ -1258,20 +1327,16 @@ &uart2 {
>  	bluetooth {
>  		compatible = "qcom,wcn6855-bt";
>  
> -		vddio-supply = <&vreg_s10b>;
> -		vddbtcxmx-supply = <&vreg_s12b>;
> -		vddrfacmn-supply = <&vreg_s12b>;
> -		vddrfa0p8-supply = <&vreg_s12b>;
> -		vddrfa1p2-supply = <&vreg_s11b>;
> -		vddrfa1p7-supply = <&vreg_s1c>;
> +		vddrfacmn-supply = <&vreg_pmu_rfa_cmn_0p8>;
> +		vddaon-supply = <&vreg_pmu_aon_0p8>;
> +		vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
> +		vddwlmx-supply = <&vreg_pmu_wlmx_0p8>;
> +		vddbtcmx-supply = <&vreg_pmu_btcmx_0p8>;
> +		vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
> +		vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
> +		vddrfa1p8-supply = <&vreg_pmu_rfa_1p7>;
>  
>  		max-speed = <3200000>;
> -
> -		enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>;
> -		swctrl-gpios = <&tlmm 132 GPIO_ACTIVE_HIGH>;

What about swctrl? You're just removing this pin from DT now without any
comment on why you think that is the right thing to do.

Should this one also be an input to the PMU block?

> -
> -		pinctrl-0 = <&bt_default>;
> -		pinctrl-names = "default";
>  	};
>  };
>  
> @@ -1761,4 +1826,11 @@ reset-pins {
>  			bias-disable;
>  		};
>  	};
> +
> +	wlan_en: wlan-en-state {
> +		pins = "gpio134";
> +		function = "gpio";
> +		drive-strength = <8>;

Yet another drive strength? Also from fw config?

> +		bias-pull-down;
> +	};
>  };

Johan