Re: [PATCH v2] ARM: dts: socfpga: Add basic support for Terrasic's de10-nano

[Krzysztof Kozlowski <krzk@kernel.org>]

On 30/01/2025 08:45, Uwe Kleine-König wrote:
> This dts is enough to make the board boot to Linux with the rootfs on
> a micro SD card.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> Hello,
> 
> changes since (implicit) v1 available at
> https://lore.kernel.org/linux-arm-kernel/20250128172917.4565-2-u.kleine-koenig@baylibre.com/:
> 
>  - Use rgmii-id for ethernet/mdio. (Andrew Lunn)
>  - Add a compatible entry for the machine (only had the SoC before),
>    fix compatible for accelerometer and various other small
>    improvements, most of them pointed out by the dt checker.
>    (Krzysztof Kozlowski)
> 
> There are still warnings when the dtb is built, but they all originate
> from the SoC dtsi.
> 
> Something I forgot to say in v1: The accelerometer fails to probe
> (readout of the device ID yields an error). This also doesn't work for
> me with the downstream kernel and dtb. I didn't debug that. Is that a
> reason to drop it?
> 
> Best regards
> Uwe
> 
>  arch/arm/boot/dts/intel/socfpga/Makefile      |  1 +
>  .../socfpga/socfpga_cyclone5_de10nano.dts     | 95 +++++++++++++++++++
>  2 files changed, 96 insertions(+)
>  create mode 100644 arch/arm/boot/dts/intel/socfpga/socfpga_cyclone5_de10nano.dts
> 
> diff --git a/arch/arm/boot/dts/intel/socfpga/Makefile b/arch/arm/boot/dts/intel/socfpga/Makefile
> index c467828aeb4b..7f69a0355ea5 100644
> --- a/arch/arm/boot/dts/intel/socfpga/Makefile
> +++ b/arch/arm/boot/dts/intel/socfpga/Makefile
> @@ -10,6 +10,7 @@ dtb-$(CONFIG_ARCH_INTEL_SOCFPGA) += \
>  	socfpga_cyclone5_mcvevk.dtb \
>  	socfpga_cyclone5_socdk.dtb \
>  	socfpga_cyclone5_de0_nano_soc.dtb \
> +	socfpga_cyclone5_de10nano.dtb \
>  	socfpga_cyclone5_sockit.dtb \
>  	socfpga_cyclone5_socrates.dtb \
>  	socfpga_cyclone5_sodia.dtb \
> diff --git a/arch/arm/boot/dts/intel/socfpga/socfpga_cyclone5_de10nano.dts b/arch/arm/boot/dts/intel/socfpga/socfpga_cyclone5_de10nano.dts
> new file mode 100644
> index 000000000000..ec25106caacf
> --- /dev/null
> +++ b/arch/arm/boot/dts/intel/socfpga/socfpga_cyclone5_de10nano.dts
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2017, Intel Corporation
> + *
> + * based on socfpga_cyclone5_de0_nano_soc.dts
> + */
> +/dts-v1/;
> +
> +#include "socfpga_cyclone5.dtsi"
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	model = "Terasic DE10-Nano";
> +	compatible = "terasic,de10-nano", "altr,socfpga-cyclone5", "altr,socfpga";

I already commented on this, you will not get different review.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.


Please run scripts/checkpatch.pl and fix reported warnings. After that,
run also `scripts/checkpatch.pl --strict` and (probably) fix more
warnings. Some warnings can be ignored, especially from --strict run,
but the code here looks like it needs a fix. Feel free to get in touch
if the warning is not clear.

Best regards,
Krzysztof