Re: [Buildroot] [PATCH] board/cubietech/cubieboard1: add Cubieboard 1

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: "Gero Schwäricke" <gero@schwaericke.email>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH] board/cubietech/cubieboard1: add Cubieboard 1
Date: Fri, 17 Feb 2023 16:15:21 +0100	[thread overview]
Message-ID: <20230217161521.01f4f2a7@windsurf> (raw)
In-Reply-To: <20221105194712.541286-1-gero@schwaericke.email>

Hello Gero,

On Sat,  5 Nov 2022 19:47:12 +0000
Gero Schwäricke <gero@schwaericke.email> wrote:

> Board support for the Cubieboard 1 from Cubietech. Based the config for
> Cubieboard 2 and the Olimex A10-OLinuXino-LIME which is also an
> Allwinner A10 board (sunxi kernel).
> 
> Signed-off-by: Gero Schwäricke <gero@schwaericke.email>

First of all, thanks a lot for your contribution, and sorry for the
very slow feedback. See below some feedback.

First of all, the commit title should be:

	configs/cubieboard1: new defconfig

> ---
>  board/cubietech/cubieboard1/boot.cmd     |  7 +++++
>  board/cubietech/cubieboard1/genimage.cfg | 36 ++++++++++++++++++++++
>  configs/cubieboard1_defconfig            | 39 ++++++++++++++++++++++++
>  3 files changed, 82 insertions(+)
>  create mode 100644 board/cubietech/cubieboard1/boot.cmd
>  create mode 100644 board/cubietech/cubieboard1/genimage.cfg
>  create mode 100644 configs/cubieboard1_defconfig

Could you add an entry in the DEVELOPERS file, both for the
board/cubietech/cubieboard1/ folder and for the
configs/cubieboard1_defconfig file?

Another thing that is missing is a
board/cubietech/cubieboard1/readme.txt that explains how to use this
defconfig on this particular board.

> diff --git a/board/cubietech/cubieboard1/boot.cmd b/board/cubietech/cubieboard1/boot.cmd
> new file mode 100644
> index 0000000000..eefbdc8169
> --- /dev/null
> +++ b/board/cubietech/cubieboard1/boot.cmd
> @@ -0,0 +1,7 @@
> +# based on board/cubieboard2/boot.cmd
> +
> +setenv fdt_high ffffffff
> +setenv bootargs console=ttyS0,115200 earlyprintk root=/dev/mmcblk0p2 rootwait
> +fatload mmc 0 $kernel_addr_r zImage
> +fatload mmc 0 $fdt_addr_r sun4i-a10-cubieboard.dtb
> +bootz $kernel_addr_r - $fdt_addr_r

There's no support for extlinux.conf in the default U-Boot
configuration for this board?

Otherwise, it all looks good to me. Could you address the minor issues
listed here, and send a 2nd version of your patch?

Thanks a lot!

Thomas Petazzoni
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot