[PATCH 5/5] board: qualcomm: Add support for QRB4210-RB2

[Tom Rini]

On Fri, Mar 24, 2023 at 01:34:18PM +0530, Bhupesh Sharma wrote:

> Add support for Qualcomm QRB4210-RB2 evaluation board
> (based on Qualcomm QRB4210 SoC).
> 
> Features:
>    - Qualcomm Snapdragon QRB4210 (Robotics version of SM6115 SoC).
>    - 2GiB RAM (on-board) [max: 8 GiB].
>    - 16GiB eMMC, uSD slot.
> 
> U-boot is chain loaded by ABL in 64-bit mode as part of boot.img.
> 
> For detailed build and boot instructions, refer to
> doc/board/qualcomm/qrb4210-rb2.rst.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma at linaro.org>
[snip]
You don't say where the dtsi file is synced from, and it should be in
linux-next or similar, at least.

[snip]
> diff --git a/arch/arm/dts/qrb4210-rb2-uboot.dtsi b/arch/arm/dts/qrb4210-rb2-uboot.dtsi
> new file mode 100644
> index 0000000000..8588dc2bf6
> --- /dev/null
> +++ b/arch/arm/dts/qrb4210-rb2-uboot.dtsi

"-u-boot.dtsi" is automatically included, but this needs #included,
also:

> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * U-Boot addition to handle QRB4210-RB2 pre-relocation devices
> + *
> + * (C) Copyright 2023 Bhupesh Sharma <bhupesh.sharma at linaro.org>
> + */
> +
> +/ {
> +	soc {
> +		u-boot,dm-pre-reloc;

You didn't build-test on -next, where these are all bootph-* now. And,
they go upstream too as the binding exists there.

> diff --git a/board/qualcomm/qrb4210-rb2/MAINTAINERS b/board/qualcomm/qrb4210-rb2/MAINTAINERS
> new file mode 100644
> index 0000000000..2b569bb983
> --- /dev/null
> +++ b/board/qualcomm/qrb4210-rb2/MAINTAINERS
> @@ -0,0 +1,6 @@
> +Qualcomm Technologies, Inc. QRB4210-RB2 evaluation board
> +M:	Bhupesh Sharma <bhupesh.sharma at linaro.org>
> +S:	Maintained
> +F:	board/qualcomm/qrb4210-rb2/
> +F:	include/configs/qrb4210-rb2.h
> +F:	configs/qrb4210rb2_defconfig

Please list the board doc file as well here (which I'm quite glad to see
you wrote!).

> diff --git a/include/configs/qrb4210-rb2.h b/include/configs/qrb4210-rb2.h
> new file mode 100644
> index 0000000000..80b9c5e2dd
> --- /dev/null
> +++ b/include/configs/qrb4210-rb2.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Configuration file for QRB4210-RB2 board
> + *
> + * (C) Copyright 2023 Bhupesh Sharma <bhupesh.sharma at linaro.org>
> + */
> +
> +#ifndef __CONFIGS_QRB4210RB2_H
> +#define __CONFIGS_QRB4210RB2_H
> +
> +#include <linux/sizes.h>
> +#include <asm/arch/sysmap-qrb4210rb2.h>

We should really really not need include files here. You aren't using
either of them directly.

> +#define CFG_SYS_BAUDRATE_TABLE	{ 115200 }
> +
> +#define CFG_EXTRA_ENV_SETTINGS \
> +	"bootm_size=0x5000000\0"	\
> +	"bootm_low=0x80000000\0"	\
> +	"bootcmd=bootm $prevbl_initrd_start_addr\0"

Shouldn't this be using either distro_bootcmd or bootstd (see
doc/develop/bootstd.rst) instead?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230324/00211201/attachment.sig>