mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Marco Felsch <m.felsch@pengutronix.de>
To: Jules Maselbas <jmaselbas@zdiv.net>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH v2 02/13] scripts: Add Allwinner eGON image support
Date: Tue, 20 Jun 2023 06:52:45 +0200	[thread overview]
Message-ID: <20230620045245.u5mwv3v335roq35l@pengutronix.de> (raw)
In-Reply-To: <6F176D27-AD10-4F80-AD5E-3B3C56C33327@zdiv.net>

[-- Attachment #1: Type: text/plain, Size: 5116 bytes --]

On 23-06-17, Jules Maselbas wrote:
> Hi Marco,
> 
> On June 17, 2023 12:00:57 AM GMT+02:00, Marco Felsch <m.felsch@pengutronix.de> wrote:
> > Hi Jules,
> > 
> > since I work on the D1 support I also had to port the eGON image support
> > to barebox ^^ Please see my below comments.
> > 
> > On 23-05-25, Jules Maselbas wrote:

...

> > > +#define EGON_HDR_BRANCH (0xea000000 | (sizeof(struct egon_header) / 4 - 2))
> > > +#define sunxi_egon_header(section) {					\
> > > +		__section(section) static const struct egon_header hdr= \
> > > +			{ .branch = EGON_HDR_BRANCH, .magic = "eGON" };	\
> > > +		__keep_symbolref(hdr);					\
> > > +	}
> > 
> > Using an additional sections seems a bit odd here. We can just write the
> > header within the image tool.
>
> That's what I wanted to do in the first place but I struggled a lot to
> get barebox relocation working.
> Having the eGON header embedded in the .text (since the header is
> loaded by bootrom) is the only solution i found to get the relocation
> working.

Hm.. at least on RISC-V I had no problems with
relocate_to_current_adr(). Also checking the ARM relocation code does
not show why the header should be a problem if
relocate_to_current_adr() is used.

> I am all for a better way but I really whish to have a first version
> applied.
> 
> > 
> > > +#endif
> > > diff --git a/scripts/Kconfig b/scripts/Kconfig
> > > index dcd5f32d1d..7517f5b79f 100644
> > > --- a/scripts/Kconfig
> > > +++ b/scripts/Kconfig
> > > @@ -56,6 +56,13 @@ config RK_IMAGE
> > >  	help
> > >  	  This enables building the image creation tool for Rockchip SoCs
> > >  
> > > +config EGON_IMAGE
> > > +       bool "Allwinner eGON image tool" if COMPILE_HOST_TOOLS
> > > +       depends on ARCH_SUNXI || COMPILE_HOST_TOOLS
> > > +       default y if ARCH_SUNXI
> > > +       help
> > > +         This enables building the image creation tool for Allwinner sunxi SoCs
> > > +
> > >  config OMAP_IMAGE
> > >  	bool "TI OMAP image tools" if COMPILE_HOST_TOOLS
> > >  	depends on ARCH_OMAP || COMPILE_HOST_TOOLS
> > > diff --git a/scripts/Makefile b/scripts/Makefile
> > > index 72ad9ad7a6..13e80db7af 100644
> > > --- a/scripts/Makefile
> > > +++ b/scripts/Makefile
> > > @@ -28,6 +28,7 @@ hostprogs-always-$(CONFIG_LAYERSCAPE_PBLIMAGE)		+= pblimage
> > >  hostprogs-always-$(CONFIG_STM32_IMAGE)			+= stm32image
> > >  hostprogs-always-$(CONFIG_RISCV)			+= prelink-riscv
> > >  hostprogs-always-$(CONFIG_RK_IMAGE)			+= rkimage
> > > +hostprogs-always-$(CONFIG_EGON_IMAGE)			+= egon_mkimage
> > >  HOSTCFLAGS_rkimage = `pkg-config --cflags openssl`
> > >  HOSTLDLIBS_rkimage = `pkg-config --libs openssl`
> > >  KBUILD_HOSTCFLAGS += -I$(srctree)/scripts/include/
> > > diff --git a/scripts/egon_mkimage.c b/scripts/egon_mkimage.c
> > > new file mode 100644
> > > index 0000000000..5983bdb28a
> > > --- /dev/null
> > > +++ b/scripts/egon_mkimage.c
> > > @@ -0,0 +1,122 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +
> > > +#include <stdio.h>
> > > +#include <errno.h>
> > > +#include <stdlib.h>
> > > +#include <stdint.h>
> > > +#include <string.h>
> > > +#include <linux/kernel.h>
> > > +
> > > +#include "../include/mach/sunxi/egon.h"
> > > +
> > > +#include "compiler.h"
> > > +#include "common.h"
> > > +#include "common.c"
> > > +
> > > +#define STAMP_VALUE 0x5f0a6c39
> > > +
> > > +static void mkimage(char *infile, char *outfile)
> > > +{
> > > +	struct egon_header *hdr;
> > > +	uint32_t *p32;
> > > +	uint32_t sum;
> > > +	int i;
> > > +	size_t hdr_size = sizeof(*hdr);
> > > +	size_t bin_size;
> > > +	size_t img_size;
> > > +	void *bin;
> > > +	int fd, ret;
> > > +
> > > +	bin = read_file(infile, &bin_size);
> > > +	if (!bin) {
> > > +		perror("read_file");
> > > +		exit(1);
> > > +	}
> > > +
> > > +	/* test if the binary has reserved space for the header */
> > > +	hdr = bin;
> > > +	if (hdr->branch == EGON_HDR_BRANCH && memcmp(hdr->magic, "eGON", 4) == 0) {
> > > +		/* strip/skip existing header */
> > > +		bin += hdr_size;
> > > +		bin_size -= hdr_size;
> > > +	}
> > 
> > Hm.. the 'normal' way is to write the header via the image tool, like it
> > is done for the i.MX. The infile don't need to have reserved space in
> > front, instead this tool should prepend the header.
>
> Yes, thisis only to accomodate for having the header in the .text (see
> my reply above)

Thanks for the explanation.

> > I attached you my two patches adding the eGON image support. Since I
> > work on the D1 it is RSIC-V related but the eGON image creation should
> > not differ that much, maybe the offset must be adapted which can be done
> > via the command line. We could skip this special section handling if my
> > patches do work for you as well :)
>
> Sounds nice, I don't when I will have time to test this.

No worries, I will start picking your patches as well for the D1
lowlevel support.

> Does the eGON header starts will a risc-v jump instruction ? Or is it
> still an arm32 insn ?

Yep, there is a special handling for each architecture. That been said,
I noticed that I had a fixup patch ontop of my image generation tool,
which I attached.

[-- Attachment #2: 0001-fixup-sunxi-add-image-support.patch --]
[-- Type: text/x-diff, Size: 3159 bytes --]

>From 1e9e0c9515e644abeda066c5387d1338d927cbda Mon Sep 17 00:00:00 2001
From: Marco Felsch <marco.felsch@gmail.com>
Date: Wed, 26 Apr 2023 21:51:26 +0200
Subject: [PATCH] fixup! sunxi: add image support

---
 scripts/sunxi_egon.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/scripts/sunxi_egon.c b/scripts/sunxi_egon.c
index 6d74a83ad8..b339955295 100644
--- a/scripts/sunxi_egon.c
+++ b/scripts/sunxi_egon.c
@@ -29,11 +29,12 @@ static int write_image(int infd, int outfd, enum sunxi_arch arch,
 		       unsigned long pblsize, unsigned long ofs)
 {
 	struct boot_file_head *header;
+	unsigned int header_sz;
 	uint32_t checksum = 0;
-	void *buf, *origbuf;
 	unsigned long size;
 	uint32_t *buf32;
        	uint32_t value;
+	void *buf;
 	int ret;
 	int i;
 
@@ -43,8 +44,10 @@ static int write_image(int infd, int outfd, enum sunxi_arch arch,
 	if (!buf)
 		return 1;
 
+	memset(buf, 0, size);
+
 	header = buf;
-	buf32 = buf;
+	header_sz = sizeof(*header);
 
 	/*
 	 * Different architectures need different first instruction to
@@ -53,7 +56,7 @@ static int write_image(int infd, int outfd, enum sunxi_arch arch,
 	switch (arch) {
 	case SUNXI_ARCH_ARM:
 		/* Generate an ARM branch instruction to jump over the header. */
-		value = 0xea000000 | (sizeof(*header) / 4 - 2);
+		value = 0xea000000 | (header_sz / 4 - 2);
 		header->b_instruction = cpu_to_le32(value);
 		break;
 	case SUNXI_ARCH_RISCV:
@@ -68,10 +71,10 @@ static int write_image(int infd, int outfd, enum sunxi_arch arch,
 		 * is not allowed).
 		 */
 		value = 0x0000006f |
-			((sizeof(*header) & 0x00100000) << 11) |
-			((sizeof(*header) & 0x000007fe) << 20) |
-			((sizeof(*header) & 0x00000800) << 9) |
-			((sizeof(*header) & 0x000ff000) << 0);
+			((header_sz & 0x00100000) << 11) |
+			((header_sz & 0x000007fe) << 20) |
+			((header_sz & 0x00000800) << 9) |
+			((header_sz & 0x000ff000) << 0);
 		header->b_instruction = cpu_to_le32(value);
 		break;
 	default:
@@ -85,33 +88,32 @@ static int write_image(int infd, int outfd, enum sunxi_arch arch,
 	memcpy(header->spl_signature, SPL_SIGNATURE, 3);
 	header->spl_signature[3] = SPL_ENV_HEADER_VERSION;
 
-	/* Calculate the checksum. Yes, it's that simple. */
-	for (i = 0; i < size / 4; i++)
-		checksum += le32_to_cpu(buf32[i]);
-	header->check_sum = cpu_to_le32(checksum);
-
-	origbuf = buf;
-	buf += sizeof(*header);
-	ret = read(infd, buf, pblsize);
+	ret = read(infd, buf + header_sz, pblsize);
 	if (ret > pblsize) {
 		printf("Error: While read: 0x%d > 0x%ld bytes!\n",
 		       ret, pblsize);
-		free(origbuf);
+		free(buf);
 		return 1;
 	}
 
+	/* Calculate the checksum. Yes, it's that simple. */
+	buf32 = buf;
+	for (i = 0; i < size / 4; i++)
+		checksum += le32_to_cpu(buf32[i]);
+	header->check_sum = cpu_to_le32(checksum);
+
 	if (ofs)
 		lseek(outfd, ofs, SEEK_SET);
 
-	ret = write(outfd, origbuf, size);
+	ret = write(outfd, buf, size);
 	if (ret != size) {
 		printf("Error: While write: 0x%d != 0x%ld bytes!\n",
 		       ret, size);
-		free(origbuf);
+		free(buf);
 		return 1;
 	}
 
-	free(origbuf);
+	free(buf);
 
 	return 0;
 }
-- 
2.39.2


  reply	other threads:[~2023-06-20  4:54 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-24 23:43 [PATCH v2 00/13] Add support for Allwinner (sunxi) A64 SoC Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 01/13] Documentation: sunxi: Add some documentation Jules Maselbas
2023-05-29  9:24   ` Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 02/13] scripts: Add Allwinner eGON image support Jules Maselbas
2023-06-16 22:00   ` Marco Felsch
2023-06-17  7:25     ` Jules Maselbas
2023-06-20  4:52       ` Marco Felsch [this message]
2023-06-21  8:26       ` Sascha Hauer
2023-05-24 23:43 ` [PATCH v2 03/13] ARM: sunxi: introduce mach-sunxi Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 04/13] ARM: lds: Add SoC specific sections to go before .text_head_prologue Jules Maselbas
2023-06-01  6:34   ` Ahmad Fatoum
2023-06-01 21:20     ` Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 05/13] ARM: sunxi: Add lowlevel switch to aarch64 Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 06/13] ARM: sunxi: Add debug_ll Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 07/13] clk: Add clock driver for sun50i-a64 Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 08/13] pinctrl: Add sun50i-a64 pinctrl driver Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 09/13] mci: Add sunxi-mmc driver Jules Maselbas
2023-05-30  8:14   ` Sascha Hauer
2023-06-01  6:15     ` Jules Maselbas
2023-06-01  8:35       ` Sascha Hauer
2023-05-24 23:43 ` [PATCH v2 10/13] ARM: sunxi: Add sun50i SDRAM init Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 11/13] ARM: boards: sunxi: Add initial support for the pinephone Jules Maselbas
2023-05-30  8:42   ` Sascha Hauer
2023-06-01  5:50     ` Jules Maselbas
2023-06-01  6:00       ` Ahmad Fatoum
2023-06-01  6:19         ` Jules Maselbas
2023-06-01  6:36           ` Ahmad Fatoum
2023-06-01  7:09             ` Ahmad Fatoum
2023-05-24 23:43 ` [PATCH v2 12/13] ARM: boards: sunxi: Add pine64 board Jules Maselbas
2023-05-24 23:43 ` [PATCH v2 13/13] ARM: sunxi: xload: Add helpers for chain-loading from SD-card Jules Maselbas

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=20230620045245.u5mwv3v335roq35l@pengutronix.de \
    --to=m.felsch@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=jmaselbas@zdiv.net \
    /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