Re: [PATCH v4 02/10] board: boston-riscv: Add initial support for P8700 Boston board

[Yao Zi <ziyao@disroot.org>]

On Tue, Aug 19, 2025 at 10:32:00AM +0000, Uros Stajic wrote:
> From: Chao-ying Fu <cfu@mips.com>
> 
> Implement initial board-level support for the P8700 Boston SoC.
> 
> Signed-off-by: Chao-ying Fu <cfu@mips.com>
> Signed-off-by: Uros Stajic <uros.stajic@htecgroup.com>
> ---
>  arch/riscv/Kconfig                      |  10 +
>  arch/riscv/cpu/p8700/Kconfig            |   2 +-
>  arch/riscv/dts/Makefile                 |   1 +
>  arch/riscv/dts/boston-p8700.dts         | 263 ++++++++++++++++++++++++
>  board/mips/boston-riscv/Kconfig         |  43 ++++
>  board/mips/boston-riscv/MAINTAINERS     |   9 +
>  board/mips/boston-riscv/Makefile        |   8 +
>  board/mips/boston-riscv/boston-lcd.h    |  20 ++
>  board/mips/boston-riscv/boston-regs.h   |  38 ++++
>  board/mips/boston-riscv/boston-riscv.c  |   9 +
>  board/mips/boston-riscv/checkboard.c    |  43 ++++
>  board/mips/boston-riscv/config.mk       |  15 ++
>  board/mips/boston-riscv/lowlevel_init.S |  18 ++
>  board/mips/boston-riscv/reset.c         |  15 ++
>  configs/boston-p8700_defconfig          |  94 +++++++++
>  drivers/clk/Kconfig                     |   2 +-
>  include/asm-generic/global_data.h       |   5 +
>  include/configs/boston-riscv.h          |  11 +
>  18 files changed, 604 insertions(+), 2 deletions(-)
>  create mode 100644 arch/riscv/dts/boston-p8700.dts
>  create mode 100644 board/mips/boston-riscv/Kconfig
>  create mode 100644 board/mips/boston-riscv/MAINTAINERS
>  create mode 100644 board/mips/boston-riscv/Makefile
>  create mode 100644 board/mips/boston-riscv/boston-lcd.h
>  create mode 100644 board/mips/boston-riscv/boston-regs.h
>  create mode 100644 board/mips/boston-riscv/boston-riscv.c
>  create mode 100644 board/mips/boston-riscv/checkboard.c
>  create mode 100644 board/mips/boston-riscv/config.mk
>  create mode 100644 board/mips/boston-riscv/lowlevel_init.S
>  create mode 100644 board/mips/boston-riscv/reset.c
>  create mode 100644 configs/boston-p8700_defconfig
>  create mode 100644 include/configs/boston-riscv.h

...

> diff --git a/arch/riscv/cpu/p8700/Kconfig b/arch/riscv/cpu/p8700/Kconfig
> index 7023575a6be..0913a6ce8f2 100644
> --- a/arch/riscv/cpu/p8700/Kconfig
> +++ b/arch/riscv/cpu/p8700/Kconfig
> @@ -7,7 +7,7 @@ config P8700_RISCV
>  	select ARCH_EARLY_INIT_R
>  	imply CPU
>  	imply CPU_RISCV
> -	imply SIFIVE_CLINT if (RISCV_MMODE || SPL_RISCV_MMODE)
> +	imply RISCV_ACLINT if (RISCV_MMODE || SPL_RISCV_MMODE)
>  	imply CMD_CPU
>  	imply SPL_CPU_SUPPORT
>  	imply SPL_OPENSBI

As mentioned before, this should probably be squashed into PATCH 1.

...

> diff --git a/arch/riscv/dts/boston-p8700.dts b/arch/riscv/dts/boston-p8700.dts
> new file mode 100644
> index 00000000000..5a5c8826318
> --- /dev/null
> +++ b/arch/riscv/dts/boston-p8700.dts
> @@ -0,0 +1,263 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2021, Chao-ying Fu <cfu@mips.com>
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/clock/boston-clock.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/mips-gic.h>
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	model = "p8700";
> +	compatible = "img,boston";
> +
> +	chosen {
> +		stdout-path = &uart0;

Per device-tree specification, stdout-path should be a string instead of
a phandle.

	A string that specifies the full path to the node representing
	the device to be used for boot console output. If the character
	“:” is present in the value it terminates the path. The value
	may be an alias. If the stdin-path property is not spec-ified,
	stdout-path should be assumed to define the input device

> +		bootargs = "root=/dev/sda rw earlycon console=ttyS0,115200n8r";

And I don't think it's the appropriate way to pass cmdline arguments.

...

> diff --git a/board/mips/boston-riscv/boston-riscv.c b/board/mips/boston-riscv/boston-riscv.c
> new file mode 100644
> index 00000000000..e5cd6c42cf7
> --- /dev/null
> +++ b/board/mips/boston-riscv/boston-riscv.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2016 Imagination Technologies
> + */
> +
> +int board_init(void)
> +{
> +	return 0;
> +}

You could omit the empty board_init() by set CONFIG_BOARD_INIT to n.

...

> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index 506ee51cdb0..7b25de7bb68 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -702,6 +702,11 @@ enum gd_flags {
>  	 * drivers shall not be called.
>  	 */
>  	GD_FLG_HAVE_CONSOLE = 0x8000000,
> +	/**
> +	 * @GD_FLG_COHERENT_DMA: DMA is cache-coherent.
> +	 *
> +	 */
> +	GD_FLG_COHERENT_DMA = 0x10000000,
>  };

I don't think it's a flag generic enough to be put in the
platform-independent header. It may sound even more appropriate to add a
private API to indicate whether IO DMA is configured as coherent.

>  #endif /* __ASSEMBLY__ */
> diff --git a/include/configs/boston-riscv.h b/include/configs/boston-riscv.h
> new file mode 100644
> index 00000000000..3b3e2567214
> --- /dev/null
> +++ b/include/configs/boston-riscv.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2021, Chao-ying Fu <cfu@mips.com>
> + */
> +
> +#ifndef __CONFIG_BOSTON_RISCV_H
> +#define __CONFIG_BOSTON_RISCV_H
> +
> +#include <linux/sizes.h>

The header defines nothing for now, so I think the include is
unnecessary, at least in this patch.

> +#endif /* __CONFIG_BOSTON_RISCV_H */
> -- 
> 2.34.1

Best regards,
Yao Zi