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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29/01/2025 13:19, Uwe Kleine-König wrote:
> Hello Krzysztof,
> 
> On Wed, Jan 29, 2025 at 10:27:22AM +0100, Krzysztof Kozlowski wrote:
>> On 28/01/2025 18:29, 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@xxxxxxxxxxxx>
>>> ---
>>> 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..d1f23a57a94d
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/intel/socfpga/socfpga_cyclone5_de10nano.dts
>>> @@ -0,0 +1,90 @@
>>> +// 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 = "altr,socfpga-cyclone5", "altr,socfpga";
>>
>> Incorrect compatible. It's not cyclone5 board. cyclone5 is the SoC.
> 
> OK, adding "terasic,de10-nano".
> 
>> There is no altr,socfpga
> 
> What does that mean?
> `git grep \"altr,socfpga\" linus/master:arch/arm/boot` gives me 15
> matches from other boards with socfpga SoCs. If you give me a bit more
> verbose description for the problem you're pointing out here, I can work
> on that.

My bad, some poor git grep result. There is such compatible, so it is fine.

> 
>> This wasn't ever tested with bindings.
> 
> I tried
> 
> 	dt-validate -m -u Documentation/devicetree/bindings -p ~/work/kbuild/arm/Documentation/devicetree/bindings/processed-schema.json ~/work/kbuild/arm/arch/arm/boot/dts/intel/socfpga/socfpga_cyclone5_de10nano.dtb

That's unusual way of running the check, but of course might work.

> 
> which emitted a bunch of warnings and after processing some of them I
> gave up because they were all triggered by the soc.dtsi file. I started

These are fine, but you introduce issues purely here.

> another try comparing the output for "my" socfpga_cyclone5_de10nano.dts
> with one for a file that only has the includes and the machine
> compatible. (Which generates 124 lines and 123 lines respectively.)
> 
> I'll work on the diff between the two for the next revision. Or would
> you recommend a different approach?
> 
>>> +	chosen {
>>> +		stdout-path = "serial0:115200n8";
>>> +	};
>>> +
>>> +	memory@0 {
>>> +		/* 1 GiB */
>>> +		device_type = "memory";
>>> +		reg = <0x0 0x40000000>;
>>> +	};
>>> +
>>> +	soc {
>>> +		fpga_axi: axi_h2f_lw_bridge@ff200000 {
>>
>> Follow DTS coding style. You just sent us something from downstream.
> 
> Indeed this is from downstream. Looking at the matching dt-validate
> output I guess just "axi@ff200000" would be appropriate?!

bus@

> 
>>> +			compatible = "simple-bus";
>>> +			reg = <0xff200000 0x00200000>;
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>
>> ranges would be after reg, but they are pointless here, no?
> 
> I thought it's "compatible", "reg" at the start, "status" at the end and
> the rest in-between in alphabetic order. What is the right ordering?

DTS coding style could be clearer here. It exactly says what is the
first, what is the second and what is the third.

> 
>> Where is the child?
> 
> I intend to add children using dt-overlays. I have a prototype here, but
> that's still to embarrassing to show.

The entire bus is in such case a bit confusing. If you have nothing
connected here in the base board, does it really exist in FPGA bitstream?



Best regards,
Krzysztof




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux