Re: [PATCH v10 RESEND 3/6] arm64: dts: qcom: Add device tree for Nord Embedded variant

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Copy-paste errors in multiple QUP0 pin configurations (SE1 and SE5) result in duplicate or incorrect pin assignments, causing mapping conflicts.
- [Low] Inconsistent node names for `opp2` and `opp3` in the `rpmhpd_opp_table`.
--

--- Patch [3]: [PATCH v10 RESEND 3/6] arm64: dts: qcom: Add device tree for Nord Embedded variant ---
commit 643aac7df3bbbfb1eafbf771684900c0a9c93646
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

arm64: dts: qcom: Add device tree for Nord Embedded variant

Unlike the GearVM variant, Nord Embedded variant has platform resources
directly controlled by the operating system. Add a separate dtsi file
extending the existing top-level nord.dtsi with nodes representing these
peripherals as well as describing how they are wired up.

> diff --git a/arch/arm64/boot/dts/qcom/nord-embedded.dtsi b/arch/arm64/boot/dts/qcom/nord-embedded.dtsi
> new file mode 100644
> index 0000000000000..f27ccc6f56dbf
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/nord-embedded.dtsi

[ ... ]

> +				rpmhpd_opp_min_svs: opp-1 {
> +					opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> +				};
> +
> +				rpmhpd_opp_low_svs: opp2 {

[Severity: Low]
This isn't a bug, but should this node be named opp-2 instead of opp2 to
match the opp-<number> naming convention used by the other nodes in this
table?

> +					opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> +				};
> +
> +				rpmhpd_opp_svs: opp3 {

[Severity: Low]
Similarly, should this be named opp-3?

> +					opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
> +				};
> +
> +				rpmhpd_opp_svs_l1: opp-4 {

[ ... ]

> +	qup_i2c1_default: qup-i2c1-default-state {
> +		pins = "gpio111", "gpio112";
> +		function = "qup0_se1";

[Severity: Medium]
Should these pins be swapped for SE1? This reuses the exact layout of SE0
(gpio111, gpio112) without swapping the data/CS or TX/RX pins, which might
cause routing conflicts on the shared 4-pin block.

> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};

[ ... ]

> +	qup_spi1_default: qup-spi1-default-state {
> +		data-pins {
> +			pins = "gpio109", "gpio111", "gpio112";
> +			function = "qup0_se1";

[Severity: Medium]
Do these data-pins conflict with SE0? It looks like this configuration uses
the exact same pins (gpio109, gpio111, gpio112) as qup_spi0_default without
swapping assignments for the shared 4-pin block.

> +			drive-strength = <6>;
> +			bias-disable;
> +		};

[ ... ]

> +	qup_spi5_default: qup-spi5-default-state {
> +		data-pins {
> +			pins = "gpio109", "gpio121", "gpio122";
> +			function = "qup0_se5";

[Severity: Medium]
Is it intentional to include gpio109 here? It appears to incorrectly mix
the SE0 pin (gpio109) with the dedicated SE5 pins (gpio121, gpio122).

> +			drive-strength = <6>;
> +			bias-disable;
> +		};

[ ... ]

> +	qup_uart1_default: qup-uart1-default-state {
> +		pins = "gpio109", "gpio110";
> +		function = "qup0_se1";

[Severity: Medium]
Should these pins be swapped for SE1? This reuses the exact layout of SE0
(gpio109, gpio110) without swapping the TX/RX assignments.

> +		drive-strength = <2>;
> +		bias-disable;
> +	};

[ ... ]

> +	qup_uart5_default: qup-uart5-default-state {
> +		pins = "gpio109", "gpio110";
> +		function = "qup0_se5";

[Severity: Medium]
Does this configuration incorrectly claim the SE0 pins? It seems to use
gpio109 and gpio110 instead of SE5's own dedicated pins.

> +		drive-strength = <2>;
> +		bias-disable;
> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810114313.776563-1-shengchao.guo@oss.qualcomm.com?part=3