Re: [PATCH v4 3/3] arm: dts: altera: add Draeger M48 CPU Module

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Unsafe register access due to unconditional probing of FPGA-based IPs without LWHPS2FPGA bridge enablement.
- [Medium] Hardware description mismatch between commit message and device tree.
- [Low] Use of deprecated 'snps,nr-gpios' property instead of standard 'ngpios'.
- [Low] The 'name' property is explicitly deprecated in device tree nodes.
--

commit 45faa524ae6c9f608be9c1559da6ec21aff7dbea
Author: Petko Manolov <petko.manolov@konsulko.com>

arm: dts: altera: add Draeger M48 CPU Module

This commit adds a new device tree for the Draeger M48 CPU module.

>     Draeger M48 is a CPU module built around the NXP i.MX6q.

[Severity: Medium]
Is this description accurate? The device tree added in this patch clearly
describes an Altera Cyclone V SoC, which contradicts the commit message
stating it is built around the NXP i.MX6q.

> diff --git a/arch/arm/boot/dts/intel/socfpga/socfpga_cyclone5_draeger_m48.dts b/arch/arm/boot/dts/intel/socfpga/socfpga_cyclone5_draeger_m48.dts
> new file mode 100644
> index 0000000000000..838708acb31bd
> --- /dev/null
> +++ b/arch/arm/boot/dts/intel/socfpga/socfpga_cyclone5_draeger_m48.dts
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  (C) Copyright 2020 Draeger and Licensors, info@draeger.com
> + */
> +#include "socfpga_cyclone5.dtsi"
> +
> +/ {
> +	model = "Draeger M48 Cyclone V SoC";
> +	compatible = "draeger,M48", "altr,socfpga-cyclone5", "altr,socfpga";
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	memory@0 {
> +		name = "memory";

[Severity: Low]
Is the "name" property needed here? According to the Device Tree
Specification, nodes must be identified by their unit-name and properties
like device_type or compatible. The name property is deprecated and
generates compiler warnings.

> +		device_type = "memory";
> +		reg = <0x0 0x40000000>;
> +	};

[ ... ]

> +	soc {
> +		gpio3: gpio@ff202000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "snps,dw-apb-gpio";
> +			reg = <0xff202000 0x20>;

[Severity: High]
Can this unconditional probing cause a bus error during early boot?

The dw-apb-gpio driver will probe this node because it defaults to status =
"okay". This hardcoded address (0xff202000) falls in the Cyclone V
Lightweight HPS-to-FPGA (LWHPS2FPGA) memory window. If the FPGA is
unprogrammed, or the bridge is disabled in the included socfpga.dtsi,
accessing this region triggers a precise external data abort, crashing the
kernel.

Should these IPs be explicitly nested under an fpga-region, or should the
device tree guarantee bridge enablement?

> +
> +			portd: gpio-controller@0 {
> +				compatible = "snps,dw-apb-gpio-port";
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				snps,nr-gpios = <2>;

[Severity: Low]
Does this violate the device tree bindings for snps,dw-apb-gpio? The
snps,nr-gpios property is deprecated in favor of the standard ngpios
property.

> +				reg = <0>;
> +			};
> +		};
> +
> +		gpio4: gpio@ff202020 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "snps,dw-apb-gpio";
> +			reg = <0xff202020 0x20>;

[Severity: High]
Similarly, can this access cause a bus error if the LWHPS2FPGA bridge is
inactive or placed in reset?

> +
> +			porte: gpio-controller@0 {
> +				compatible = "snps,dw-apb-gpio-port";
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				snps,nr-gpios = <2>;

[Severity: Low]
Could this use the standard ngpios property instead of the deprecated
snps,nr-gpios?

> +				reg = <0>;
> +			};
> +		};
> +	};
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911150755.2803736-1-petko.manolov@konsulko.com?part=3