Re: [PATCH v6 10/11] arm64: dts: qcom: shikra: Enable WiFi/BT on SoMs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Critical under-voltage and over-voltage hazards due to incorrect regulator assignments for the WCN3988 PMU and WiFi nodes.
- [High] Missing `vddch1-supply` causes power sequencing driver probe failure on constrained systems.
- [Medium] Spurious definition of `vreg_pmu_ch1` (ldo4) for a 1x1 WiFi chip.
- [Low] Incorrect indentation of properties inside the pinctrl node.
--

--- Patch [10]: [PATCH v6 10/11] arm64: dts: qcom: shikra: Enable WiFi/BT on SoMs ---
commit a8e0b18d5ed43960802fc198b1e54e52d1ff11d0
Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>

arm64: dts: qcom: shikra: Enable WiFi/BT on SoMs
    
Shikra SoM cards include WCN3988 WiFi/Bluetooth chip, with supplies
provided by on-card PMICs. Enable both interfaces and provide the
required supply and calibration data.

> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> index dc3861489f64d..8e30d451ab410 100644
> --- a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
[ ... ]
> @@ -27,6 +31,42 @@ key-volume-up {
>  			linux,can-disable;
>  		};
>  	};
> +
> +	wcn3988-pmu {
> +		compatible = "qcom,wcn3988-pmu";
> +
> +		pinctrl-0 = <&sw_ctrl_default>;
> +		pinctrl-names = "default";
> +
> +		swctrl-gpios = <&tlmm 88 GPIO_ACTIVE_HIGH>;
> +
> +		vddio-supply = <&pm4125_l7>;

[Severity: Critical]
Does this correctly assign the voltage rail for the VDDIO supply?
pm4125_l7 appears to be a 664mV LDO, but VDDIO requires 1.8V. This assignment
could result in severe undervoltage and failure to power the PMU.

> +		vddxo-supply = <&pm4125_l13>;
> +		vddrf-supply = <&pm4125_l10>;
> +		vddch0-supply = <&pm4125_l22>;
> +

[Severity: High]
Does this node also need a vddch1-supply?

Although omitting the vddch1-supply correctly models the 1x1 WCN3988 hardware,
the qcom,wcn3988-pmu driver hardcodes vddch1 as a strictly required regulator.
Without it, devm_regulator_bulk_get() will fail with -ENODEV on systems with
full constraints enabled, which disables Wi-Fi/Bluetooth initialization entirely.

A dummy regulator supply might be needed here to bypass the driver bug until
it is fixed.

> +		regulators {
[ ... ]
> +			vreg_pmu_ch0: ldo3 {
> +				regulator-name = "vreg_pmu_ch0";
> +			};
> +
> +			vreg_pmu_ch1: ldo4 {
> +				regulator-name = "vreg_pmu_ch1";
> +			};

[Severity: Medium]
Is it correct to define vreg_pmu_ch1 for this chip?

The WCN3988 is a 1x1 WiFi module and physically lacks a second RF chain.
Exposing a non-existent hardware LDO to the regulator core misrepresents
the hardware capabilities of the PMU.

> +		};
> +	};
>  };
[ ... ]
> +&tlmm {
> +	sw_ctrl_default: sw-ctrl-default-state {
> +	pins = "gpio88";
> +	function = "gpio";
> +	bias-pull-down;
> +	};
> +};

[Severity: Low]
Are these properties properly indented? Standard device tree formatting requires
properties inside a node to be indented by an additional level relative to the
node name.

[ ... ]
> diff --git a/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi
> index 73945bf42112d..62e37409ac8f6 100644
> --- a/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi
> +++ b/arch/arm64/boot/dts/qcom/shikra-iqs-som.dtsi
[ ... ]
> +	wcn3988-pmu {
[ ... ]
> +		regulators {
[ ... ]
> +			vreg_pmu_ch0: ldo3 {
> +				regulator-name = "vreg_pmu_ch0";
> +			};
> +
> +			vreg_pmu_ch1: ldo4 {
> +				regulator-name = "vreg_pmu_ch1";
> +			};

[Severity: Medium]
Is it correct to define vreg_pmu_ch1 here as well? The WCN3988 is a 1x1 WiFi
module, so exposing this non-existent hardware LDO misrepresents the actual
hardware capabilities.

> +		};
> +	};
>  };
[ ... ]
> +&tlmm {
> +	sw_ctrl_default: sw-ctrl-default-state {
> +	pins = "gpio88";
> +	function = "gpio";
> +	bias-pull-down;
> +	};
> +};

[Severity: Low]
Could we indent these properties by an additional level to follow standard
device tree formatting guidelines?

[ ... ]
> +&wifi {
> +	vdd-0.8-cx-mx-supply = <&pm8150_s4>;

[Severity: Critical]
Does this correctly assign the supply for the 0.8V CX/MX digital core?

pm8150_s4 is a 1.08V - 2.04V SMPS. Assigning it here will severely over-volt
the 0.8V rail and risks permanently damaging the Wi-Fi/BT silicon.

> +	vdd-1.8-xo-supply = <&vreg_pmu_xo>;
> +	vdd-1.3-rfa-supply = <&vreg_pmu_rf>;
> +	vdd-3.3-ch0-supply = <&vreg_pmu_ch0>;
> +
> +	qcom,calibration-variant = "Shikra_EVK";
> +	firmware-name = "shikra";
> +
> +	status = "okay";
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-shikra-dt-m1-v6-0-bee265d3499b@oss.qualcomm.com?part=10