[Buildroot] [PATCH v3] board: Add support for terasic DE10 Nano

[Gwenhael Goavec-Merou]

Thomas, all,

On Sun, 25 Jul 2021 16:08:33 +0200
Thomas Petazzoni <thomas.petazzoni at bootlin.com> wrote:

> Hello,
> 
> On Wed, 21 Jul 2021 14:59:08 +0200
> Gwenhael Goavec-Merou <gwenj at trabucayre.com> wrote:
> 
> > From: Gwenhael Goavec-Merou <gwenhael.goavec-merou at trabucayre.com>
> > 
> > Terasic DE10 Nano is an SoCFPGA cycloneV based board.
> > References:
> > - https://www.terasic.com.tw/cgi-bin/page/archive.pl?Language=English&CategoryNo=205&No=1046
> > - https://rocketboards.org/foswiki/Documentation/DE10NanoDevelopmentBoard
> > 
> > Note: there is no dts in kernel (mainline or intel), but since DE0 Nano Soc is
> > similar, the corresponding dts is usually used
> > 
> > Signed-off-by: Gwenhael Goavec-Merou <gwenhael.goavec-merou at trabucayre.com>  
> 
> Thanks for the patch. It looks mostly good, expect for a few details,
> see below.
> 
> > diff --git a/board/terasic/de10nano_cyclone5/genimage.cfg b/board/terasic/de10nano_cyclone5/genimage.cfg
> > new file mode 100644
> > index 0000000000..3261c62741
> > --- /dev/null
> > +++ b/board/terasic/de10nano_cyclone5/genimage.cfg
> > @@ -0,0 +1,57 @@
> > +image boot.vfat {
> > +	vfat {
> > +		file zImage {
> > +            image = "zImage"
> > +        }  
> 
> Starting from here the indentation is not correct.
Oups... -> fixed in v4
> 
> > +
> > +        file socfpga_cyclone5_de0_nano_soc.dtb {
> > +            image = "socfpga_cyclone5_de0_nano_soc.dtb"
> > +        }
> > +
> > +        file barebox.bin {
> > +            image = "barebox-socfpga-de10_nano.img"
> > +        }
> > +
> > +        file barebox.env {
> > +            image = "barebox-env"
> > +        }
> > +	}
> > +	size = 8M  
> 
> Could you fix the indentation in this block ?
> 
> > +}
> > +
> > +image boot.img {
> > +	hdimage {
> > +		partition-table = "no"
> > +	}
> > +
> > +	partition spl {
> > +		in-partition-table = "no"
> > +		image = "barebox-socfpga-de10_nano-xload.img"
> > +		offset = 0
> > +		size = 64k
> > +	}
> > +
> > +	size = 1M
> > +}  
> 
> I don't understand why you have a boot.img, which apparently only
> contains the raw contacts of this xload.img file. Why not use this
> xload.img file directly...
> 
In fact unlike some others CPU, this is not a raw area but a real partition
with type 0xA2. This genimage is based on socrate board wich use the same way.
> > +
> > +image sdcard.img {
> > +	hdimage {
> > +	}
> > +
> > +	partition spl {
> > +		partition-type = 0xa2
> > +		image = "boot.img"  
> 
> .. here ?
> 
> Example from another genimage.cfg:
> 
>         partition spl {
>                 in-partition-table = "no"
>                 image = "sunxi-spl.bin"
>                 offset = 8192
>         }
> 
>         partition u-boot {
>                 in-partition-table = "no"
>                 image = "u-boot.itb"
>                 offset = 40K
>                 size = 1M # 1MB - 40K
>         }
> 
> > +	partition rootfs {
> > +		partition-type = 0x83
> > +		image = "rootfs.ext2"
> > +		size = 500M  
> 
> I'd suggest to not put a size here, and instead tweak the size of the
> ext2 filesystem in the defconfig. Indeed, having a larger partition but
> not a larger filesystem is kind of useless (or requires a resize of the
> filesystem when booting).
>
Done for v4
> 
> > diff --git a/configs/terasic_de10nano_cyclone5_defconfig b/configs/terasic_de10nano_cyclone5_defconfig
> > new file mode 100644
> > index 0000000000..4a356f31a8
> > --- /dev/null
> > +++ b/configs/terasic_de10nano_cyclone5_defconfig
> > @@ -0,0 +1,28 @@
> > +BR2_arm=y
> > +BR2_cortex_a9=y
> > +BR2_ARM_ENABLE_NEON=y
> > +BR2_ARM_ENABLE_VFP=y
> > +BR2_ARM_FPU_NEON=y
> > +BR2_KERNEL_HEADERS_5_11=y  
> 
> Could you use:
> 
> BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_5_11=y
> 
> instead ?
Done for v4
> 
> Could you adjust those small details and send a v4?
> 
> Thanks a lot!
> 
> Thomas

Thanks
Gwen