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.
> 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
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
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?!
> > + 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?
> 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.
> > + ranges = <0x00000000 0xff200000 0x00200000>;
> > + };
> > + };
> > +};
> > +
> > [...]
> > +&i2c0 {
> > + clock-frequency = <100000>;
> > + status = "okay";
> > +
> > + accelerometer@53 {
> > + compatible = "adi,adxl34x";
>
> There is no such compatible and nothing in changelog refers to missing
> bindings. Always provide link in the changelog to your bindings which
> were not yet accepted.
Oops, indeed. Will make this "adi,adxl345" for the next revision.
Best regards and thanks for your feedback,
Uwe
Attachment:
signature.asc
Description: PGP signature
