mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Steffen Trumtrar <s.trumtrar@pengutronix.de>
To: Sascha Hauer <sha@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH v3 5/6] ARM: socfpga: add support for Enclustra AA1
Date: Tue, 12 Jul 2022 07:48:04 +0200	[thread overview]
Message-ID: <87sfn6n8ty.fsf@pengutronix.de> (raw)
In-Reply-To: <20220711085415.GM5208@pengutronix.de>


Hi!

Sascha Hauer <sha@pengutronix.de> writes:

(...)
>> +++ b/arch/arm/boards/enclustra-aa1/board.c
(...)
>> +	pbl_index = readl(0xFFD06210);
>
> This is ARRIA10_SYSMGR_ROM_INITSWLASTLD, right? Please use it.
>

Sure.

>> +
>> +	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?
>

You mean like you did for rockchip? Might be a good idea.

>> +	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.
>

Ack.

>> +#define BITSTREAM2_OFFSET  BITSTREAM1_OFFSET + SZ_32M
>
> You should add braces around the macro definitions to make them safe
> to use.
>

Ack.

>> +
>> +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.
>

Ack.

>> 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.
>

Sure.


Thanks,
Steffen

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



  reply	other threads:[~2022-07-12  6:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-11  7:52 [PATCH v3 1/6] scripts: socfgpa_xml_to_config: document pincfg Steffen Trumtrar
2022-07-11  7:52 ` [PATCH v3 2/6] ARM: socfpga: achilles: remove cargo-cult Steffen Trumtrar
2022-07-11  7:52 ` [PATCH v3 3/6] ARM: socfpga: achilles: use dtbz instead of dtb Steffen Trumtrar
2022-07-11  7:52 ` [PATCH v3 4/6] ARM: socfpga: achilles: use ENTRY_FUNCTION_WITHSTACK Steffen Trumtrar
2022-07-11  7:52 ` [PATCH v3 5/6] ARM: socfpga: add support for Enclustra AA1 Steffen Trumtrar
2022-07-11  8:54   ` Sascha Hauer
2022-07-12  5:48     ` Steffen Trumtrar [this message]
2022-07-11  7:52 ` [PATCH v3 6/6] ARM: socfpga: defconfig: add aa1 Steffen Trumtrar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87sfn6n8ty.fsf@pengutronix.de \
    --to=s.trumtrar@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=sha@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox