Re: [PATCH v3 5/6] ARM: socfpga: add support for Enclustra AA1

[Sascha Hauer <sha@pengutronix.de>]

On Mon, Jul 11, 2022 at 09:52:08AM +0200, Steffen Trumtrar wrote:
> From: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  arch/arm/boards/Makefile                      |   1 +
>  arch/arm/boards/enclustra-aa1/Makefile        |   4 +
>  arch/arm/boards/enclustra-aa1/board.c         |  47 ++++++++
>  arch/arm/boards/enclustra-aa1/lowlevel.c      | 113 ++++++++++++++++++
>  .../enclustra-aa1/pinmux-config-arria10.c     | 104 ++++++++++++++++
>  .../boards/enclustra-aa1/pll-config-arria10.c |  56 +++++++++
>  arch/arm/dts/Makefile                         |   1 +
>  arch/arm/dts/socfpga_arria10_mercury_aa1.dts  |  83 +++++++++++++
>  arch/arm/mach-socfpga/Kconfig                 |   4 +
>  images/Makefile.socfpga                       |  12 ++
>  10 files changed, 425 insertions(+)
>  create mode 100644 arch/arm/boards/enclustra-aa1/Makefile
>  create mode 100644 arch/arm/boards/enclustra-aa1/board.c
>  create mode 100644 arch/arm/boards/enclustra-aa1/lowlevel.c
>  create mode 100644 arch/arm/boards/enclustra-aa1/pinmux-config-arria10.c
>  create mode 100644 arch/arm/boards/enclustra-aa1/pll-config-arria10.c
>  create mode 100644 arch/arm/dts/socfpga_arria10_mercury_aa1.dts
> 
> diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
> index d303999614..9d700cfbda 100644
> --- a/arch/arm/boards/Makefile
> +++ b/arch/arm/boards/Makefile
> @@ -134,6 +134,7 @@ obj-$(CONFIG_MACH_SCB9328)			+= scb9328/
>  obj-$(CONFIG_MACH_SEEED_ODYSSEY)		+= seeed-odyssey/
>  obj-$(CONFIG_MACH_SOCFPGA_ALTERA_SOCDK)		+= altera-socdk/
>  obj-$(CONFIG_MACH_SOCFPGA_EBV_SOCRATES)		+= ebv-socrates/
> +obj-$(CONFIG_MACH_SOCFPGA_ENCLUSTRA_AA1)	+= enclustra-aa1/
>  obj-$(CONFIG_MACH_SOCFPGA_REFLEX_ACHILLES)	+= reflex-achilles/
>  obj-$(CONFIG_MACH_SOCFPGA_TERASIC_DE0_NANO_SOC)	+= terasic-de0-nano-soc/
>  obj-$(CONFIG_MACH_SOCFPGA_TERASIC_DE10_NANO)	+= terasic-de10-nano/
> diff --git a/arch/arm/boards/enclustra-aa1/Makefile b/arch/arm/boards/enclustra-aa1/Makefile
> new file mode 100644
> index 0000000000..5678718188
> --- /dev/null
> +++ b/arch/arm/boards/enclustra-aa1/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +lwl-y += lowlevel.o
> +obj-y += board.o
> diff --git a/arch/arm/boards/enclustra-aa1/board.c b/arch/arm/boards/enclustra-aa1/board.c
> new file mode 100644
> index 0000000000..5b8e5a5c9f
> --- /dev/null
> +++ b/arch/arm/boards/enclustra-aa1/board.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <common.h>
> +#include <init.h>
> +#include <io.h>
> +#include <bbu.h>
> +
> +static int aa1_init(void)
> +{
> +	int pbl_index = 0;
> +	uint32_t flag_barebox1 = 0;
> +	uint32_t flag_barebox2 = 0;
> +
> +	if (!of_machine_is_compatible("enclustra,mercury-aa1"))
> +		return 0;
> +
> +	pbl_index = readl(0xFFD06210);

This is ARRIA10_SYSMGR_ROM_INITSWLASTLD, right? Please use it.

> +
> +	pr_debug("Current barebox instance %d\n", pbl_index);
> +
> +	switch (pbl_index) {
> +	case 0:
> +		flag_barebox1 |= BBU_HANDLER_FLAG_DEFAULT;
> +		break;
> +	case 1:
> +		flag_barebox2 |= BBU_HANDLER_FLAG_DEFAULT;
> +		break;
> +	};
> +
> +	bbu_register_std_file_update("emmc-barebox1-xload", flag_barebox1,
> +					"/dev/mmc0.barebox1-xload",
> +					filetype_socfpga_xload);
> +
> +	bbu_register_std_file_update("emmc-barebox1", 0,
> +					"/dev/mmc0.barebox1",
> +					filetype_arm_barebox);
> +
> +	bbu_register_std_file_update("emmc-barebox2-xload", flag_barebox2,
> +					"/dev/mmc0.barebox2-xload",
> +					filetype_socfpga_xload);
> +
> +	bbu_register_std_file_update("emmc-barebox2", 0,
> +					"/dev/mmc0.barebox2",
> +					filetype_arm_barebox);

Should this be turned into something more intelligent like failsafe
update?

> +	return 0;
> +}
> +postcore_initcall(aa1_init);

You could turn this into a postcore_platform_driver.

> +#define BAREBOX_PART 0
> +#define BITSTREAM_PART 1
> +#define BAREBOX1_OFFSET    SZ_1M
> +#define BAREBOX2_OFFSET    BAREBOX1_OFFSET + SZ_512K
> +#define BAREBOX3_OFFSET    BAREBOX2_OFFSET + SZ_512K
> +#define BAREBOX4_OFFSET    BAREBOX3_OFFSET + SZ_512K
> +#define BITSTREAM1_OFFSET  0x0

>From looking into the device tree I would expect BITSTREAM1_OFFSET to be 0x700000.

> +#define BITSTREAM2_OFFSET  BITSTREAM1_OFFSET + SZ_32M

You should add braces around the macro definitions to make them safe
to use.

> +
> +extern char __dtb_z_socfpga_arria10_mercury_aa1_start[];
> +
> +#define ARRIA10_STACKTOP	ARRIA10_OCRAM_ADDR + SZ_256K
> +
> +ENTRY_FUNCTION_WITHSTACK(start_socfpga_aa1_xload, ARRIA10_STACKTOP, r0, r1, r2)
> +{
> +	int pbl_index = 0;
> +	int barebox = 0;
> +	int bitstream = 0;
> +
> +	arm_cpu_lowlevel_init();
> +
> +	relocate_to_current_adr();
> +
> +	setup_c();
> +
> +	arria10_init(&mainpll_cfg, &perpll_cfg, pinmux);
> +
> +	arria10_prepare_mmc(BAREBOX_PART, BITSTREAM_PART);
> +
> +	pbl_index = readl(ARRIA10_SYSMGR_ROM_INITSWLASTLD);
> +
> +	/* Allow booting from both PBL0 and PBL1 to allow atomic updates.
> +	 * Bitstreams redundant too and expected to reside in the second
> +	 * partition.
> +	 * There is a fixed relation between the PBL/barebox instance and its
> +	 * bitstream location (offset) that requires to update them together */
> +	switch (pbl_index) {
> +	case 0:
> +		barebox = BAREBOX1_OFFSET;
> +		bitstream = BITSTREAM1_OFFSET;
> +		break;
> +	case 1:
> +		barebox = BAREBOX2_OFFSET;
> +		bitstream = BITSTREAM1_OFFSET;
> +		break;
> +	case 2:
> +	case 3:
> +		/* Left blank for future extension */
> +		break;

You should either bail out or use a sane default for unhandled cases.

> +	}
> +
> +	arria10_load_fpga(bitstream, SZ_32M);
> +
> +	arria10_finish_io(&mainpll_cfg, &perpll_cfg, pinmux);
> +
> +	arria10_ddr_calibration_sequence();
> +
> +	arria10_start_image(barebox);
> +}
> +
> +ENTRY_FUNCTION(start_socfpga_aa1, r0, r1, r2)
> +{
> +	void *fdt;
> +
> +	fdt = __dtb_z_socfpga_arria10_mercury_aa1_start + get_runtime_offset();
> +
> +	barebox_arm_entry(0x0, SZ_2G, fdt);
> +}
> +
> +ENTRY_FUNCTION_WITHSTACK(start_socfpga_aa1_bringup, ARRIA10_STACKTOP, r0, r1, r2)
> +{
> +	void *fdt;
> +
> +	arm_cpu_lowlevel_init();
> +
> +	relocate_to_current_adr();
> +	setup_c();
> +
> +	arria10_init(&mainpll_cfg, &perpll_cfg, pinmux);
> +
> +	/* wait for fpga_usermode */
> +	a10_wait_for_usermode(0x1000000);
> +
> +	arria10_finish_io(&mainpll_cfg, &perpll_cfg, pinmux);
> +
> +	arria10_ddr_calibration_sequence();
> +
> +	fdt = __dtb_z_socfpga_arria10_mercury_aa1_start + get_runtime_offset();
> +
> +	barebox_arm_entry(0x0, SZ_2G, fdt);
> +}
> diff --git a/arch/arm/dts/socfpga_arria10_mercury_aa1.dts b/arch/arm/dts/socfpga_arria10_mercury_aa1.dts
> new file mode 100644
> index 0000000000..ef3afc9b98
> --- /dev/null
> +&mmc {
> +	bus-width = <8>;
> +	non-removable;
> +	disable-wp;
> +	no-sd;
> +
> +	partitions {
> +		compatible = "fixed-partitions";
> +		#size-cells = <1>;
> +		#address-cells = <1>;
> +
> +		barebox1_xload: partition@100000 {
> +			label = "barebox1-xload";
> +			reg = <0x100000 0x40000>;
> +		};
> +
> +		barebox2_xload: partition@140000 {
> +			label = "barebox2-xload";
> +			reg = <0x140000 0x40000>;
> +		};
> +
> +		barebox1: partition@200000 {
> +			label = "barebox1";
> +			reg = <0x200000 0x80000>;
> +		};
> +
> +		barebox2: partition@280000 {
> +			label = "barebox2";
> +			reg = <0x280000 0x80000>;
> +		};

It might be worth increasing the size to 1MiB. It's easy to make barebox
bigger than 512KiB.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |