Re: [PATCH 1/4] arm64: dts: qcom: Use hyphen in node names

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Renaming the `splash_region` node to `splash` violates an explicit bootloader ABI, breaking continuous splash screen and display handover.
- [Medium] The commit modifies node names beyond simply replacing underscores with hyphens, introducing a mismatch between the commit message and the actual changes.
--

commit f5e94b2fb05705d86098917e535c3beb4b025cf6
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

arm64: dts: qcom: Use hyphen in node names

This commit renames several device tree nodes to use hyphens instead of
underscores to align with DTS coding style. The commit message notes this
should be safe since node names are not considered an ABI.

>     DTS coding style prefers hyphens instead of underscores in the node
>     names.  Change should be safe, because node names are not considered an
>     ABI.

[Severity: High]
Is it accurate that node names are never considered an ABI? In some cases,
firmware components like ABL rely on exact node names for hardware handover.

[Severity: Medium]
The commit message claims to only replace underscores with hyphens in node
names, but the patch also alters prefixes and adds new "-regulator" suffixes
that go beyond the stated scope.

[ ... ]

> diff --git a/arch/arm/boot/dts/qcom/qcom-sdx65-mtp.dts b/arch/arm/boot/dts/qcom/qcom-sdx65-mtp.dts
> index 07c10c84eefa1..2c0590617ac46 100644
> --- a/arch/arm/boot/dts/qcom/qcom-sdx65-mtp.dts
> +++ b/arch/arm/boot/dts/qcom/qcom-sdx65-mtp.dts
> @@ -55,7 +55,7 @@ vph_pwr: vph-pwr-regulator {
>  		regulator-max-microvolt = <3700000>;
>  	};
>  
> -	vreg_bob_3p3: pmx65_bob {
> +	vreg_bob_3p3: pmx65-bob-regulator {
                      ^^^^^^^^^^^^^^^^^^^
[Severity: Medium]
This node was renamed by appending a completely new "-regulator" suffix not
mentioned in the commit message. Is this intentional?

>  		compatible = "regulator-fixed";
>  		regulator-name = "vreg_bob_3p3";
>  		regulator-min-microvolt = <3300000>;

[ ... ]

> diff --git a/arch/arm64/boot/dts/qcom/lemans-evk-camera.dtso b/arch/arm64/boot/dts/qcom/lemans-evk-camera.dtso
> index 4600d5441cce4..a4c6d39cf7778 100644
> --- a/arch/arm64/boot/dts/qcom/lemans-evk-camera.dtso
> +++ b/arch/arm64/boot/dts/qcom/lemans-evk-camera.dtso
> @@ -14,7 +14,7 @@
>  #include <dt-bindings/gpio/gpio.h>
>  
>  &{/} {
> -	vreg_cam1_1p8: vreg_cam1_1p8 {
> +	vreg_cam1_1p8: cam1-1p8-regulator {
                       ^^^^^^^^^^^^^^^^^^
[Severity: Medium]
The node prefix and suffix were arbitrarily altered here, going beyond a
simple underscore replacement. Could the commit message be updated to
reflect these additional naming changes?

>  		compatible = "regulator-fixed";
>  		regulator-name = "vreg_cam1_1p8";
>  		startup-delay-us = <10000>;

[ ... ]

> diff --git a/arch/arm64/boot/dts/qcom/milos-nothing-asteroids.dts b/arch/arm64/boot/dts/qcom/milos-nothing-asteroids.dts
> index 7393978926e44..0796d9dbe1582 100644
> --- a/arch/arm64/boot/dts/qcom/milos-nothing-asteroids.dts
> +++ b/arch/arm64/boot/dts/qcom/milos-nothing-asteroids.dts
> @@ -136,7 +136,7 @@ ramoops@81f20000 {
>  		 * ABL is powering down display and controller if this node is
>  		 * not named exactly "splash_region".
>  		 */
> -		splash_region@e3940000 {
> +		splash@e3940000 {
                ^^^^^^^^^^^^^^^
[Severity: High]
Does renaming "splash_region" to "splash" violate the explicit bootloader
ABI documented in the comment directly above?

The ABL bootloader parses the device tree to find the memory region node
named exactly "splash_region". Changing this name could cause the lookup
to fail and inadvertently power down the display hardware during the
bootloader-to-kernel transition.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-dts-qcom-style-checker-v1-0-16ce82a2bcfd@oss.qualcomm.com?part=1