[PATCH v2 0/4] mkimage: sunxi_egon: add riscv support

[PATCH v2 0/4] mkimage: sunxi_egon: add riscv support

[Icenowy Zheng]

This patchset tries to make mkimage -T sunxi_egon to be able to generate
an eGON.BT0 image for Allwinner RISC-V SoCs (e.g. D1).

In addition, to keep the compatibility, it will still consider the
architecture to be ARM when no architecture is specified.

Icenowy Zheng (4):
  mkimage: add a flag to describe whether -A is specified
  mkimage: sunxi_egon: refactor for multi-architecture support
  mkimage: sunxi_egon: add support for riscv
  sunxi: specify architecture when generating SPL boot image

 scripts/Makefile.spl |  2 +-
 tools/imagetool.h    |  1 +
 tools/mkimage.c      |  1 +
 tools/sunxi_egon.c   | 86 ++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 82 insertions(+), 8 deletions(-)

-- 
2.30.2

[PATCH v2 1/4] mkimage: add a flag to describe whether -A is specified

[Icenowy Zheng]

The sunxi_egon type used to take no -A argument (because we assume sunxi
targets are all ARM). However, as Allwinner D1 appears as the first
RISC-V sunxi target, we need to support -A; in addition, as external
projects rely on U-Boot mkimage to generate sunxi eGON.BT0 header, we
need to keep compatibility with command line without -A.

As the default value of arch in mkimage is not proper (IH_ARCH_PPC
instead of IH_ARCH_INVALID), to keep more compatibility, add an Aflag
field to image parameters to describe whether an architecture is
explicitly specified.

Reviewed-by: Tom Rini <trini@konsulko.com>
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v2:
- Added Tom Rini's Review tag.

 tools/imagetool.h | 1 +
 tools/mkimage.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tools/imagetool.h b/tools/imagetool.h
index e229a34ffc..5dc28312c2 100644
--- a/tools/imagetool.h
+++ b/tools/imagetool.h
@@ -51,6 +51,7 @@ struct image_tool_params {
 	int pflag;
 	int vflag;
 	int xflag;
+	int Aflag;
 	int skipcpy;
 	int os;
 	int arch;
diff --git a/tools/mkimage.c b/tools/mkimage.c
index cc7b242faf..54d8e3835a 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -168,6 +168,7 @@ static void process_args(int argc, char **argv)
 				show_valid_options(IH_ARCH);
 				usage("Invalid architecture");
 			}
+			params.Aflag = 1;
 			break;
 		case 'b':
 			if (add_content(IH_TYPE_FLATDT, optarg)) {
-- 
2.30.2

Re: [PATCH v2 1/4] mkimage: add a flag to describe whether -A is specified

[Simon Glass]

Hi Icenowy,

On Sat, 19 Jun 2021 at 03:19, Icenowy Zheng <icenowy@aosc.io> wrote:
>
> The sunxi_egon type used to take no -A argument (because we assume sunxi
> targets are all ARM). However, as Allwinner D1 appears as the first
> RISC-V sunxi target, we need to support -A; in addition, as external
> projects rely on U-Boot mkimage to generate sunxi eGON.BT0 header, we
> need to keep compatibility with command line without -A.
>
> As the default value of arch in mkimage is not proper (IH_ARCH_PPC
> instead of IH_ARCH_INVALID), to keep more compatibility, add an Aflag
> field to image parameters to describe whether an architecture is
> explicitly specified.
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> Changes in v2:
> - Added Tom Rini's Review tag.

This is not actually a change so should not appear in the log.

Also note 'patman status' can be used to automatically collect these.

>
>  tools/imagetool.h | 1 +
>  tools/mkimage.c   | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/tools/imagetool.h b/tools/imagetool.h
> index e229a34ffc..5dc28312c2 100644
> --- a/tools/imagetool.h
> +++ b/tools/imagetool.h
> @@ -51,6 +51,7 @@ struct image_tool_params {
>         int pflag;
>         int vflag;
>         int xflag;
> +       int Aflag;

aflag

>         int skipcpy;
>         int os;
>         int arch;
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index cc7b242faf..54d8e3835a 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -168,6 +168,7 @@ static void process_args(int argc, char **argv)
>                                 show_valid_options(IH_ARCH);
>                                 usage("Invalid architecture");
>                         }
> +                       params.Aflag = 1;
>                         break;
>                 case 'b':
>                         if (add_content(IH_TYPE_FLATDT, optarg)) {
> --
> 2.30.2

Regards,
Simon

Re: [PATCH v2 1/4] mkimage: add a flag to describe whether -A is specified

[Icenowy Zheng]

在 2021-06-26星期六的 12:31 -0600，Simon Glass写道：
> Hi Icenowy,
> 
> On Sat, 19 Jun 2021 at 03:19, Icenowy Zheng <icenowy@aosc.io> wrote:
> > 
> > The sunxi_egon type used to take no -A argument (because we assume
> > sunxi
> > targets are all ARM). However, as Allwinner D1 appears as the first
> > RISC-V sunxi target, we need to support -A; in addition, as
> > external
> > projects rely on U-Boot mkimage to generate sunxi eGON.BT0 header,
> > we
> > need to keep compatibility with command line without -A.
> > 
> > As the default value of arch in mkimage is not proper (IH_ARCH_PPC
> > instead of IH_ARCH_INVALID), to keep more compatibility, add an
> > Aflag
> > field to image parameters to describe whether an architecture is
> > explicitly specified.
> > 
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > ---
> > Changes in v2:
> > - Added Tom Rini's Review tag.
> 
> This is not actually a change so should not appear in the log.
> 
> Also note 'patman status' can be used to automatically collect these.
> 
> > 
> >  tools/imagetool.h | 1 +
> >  tools/mkimage.c   | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/tools/imagetool.h b/tools/imagetool.h
> > index e229a34ffc..5dc28312c2 100644
> > --- a/tools/imagetool.h
> > +++ b/tools/imagetool.h
> > @@ -51,6 +51,7 @@ struct image_tool_params {
> >         int pflag;
> >         int vflag;
> >         int xflag;
> > +       int Aflag;
> 
> aflag

I think a problem is that -a is a different flag than -A.

-a is addr, -A is architecture.

> 
> >         int skipcpy;
> >         int os;
> >         int arch;
> > diff --git a/tools/mkimage.c b/tools/mkimage.c
> > index cc7b242faf..54d8e3835a 100644
> > --- a/tools/mkimage.c
> > +++ b/tools/mkimage.c
> > @@ -168,6 +168,7 @@ static void process_args(int argc, char **argv)
> >                                 show_valid_options(IH_ARCH);
> >                                 usage("Invalid architecture");
> >                         }
> > +                       params.Aflag = 1;
> >                         break;
> >                 case 'b':
> >                         if (add_content(IH_TYPE_FLATDT, optarg)) {
> > --
> > 2.30.2
> 
> Regards,
> Simon

Re: [PATCH v2 1/4] mkimage: add a flag to describe whether -A is specified

[Simon Glass]

Hi Icenowy,

On Sat, 26 Jun 2021 at 17:57, Icenowy Zheng <icenowy@aosc.io> wrote:
>
> 在 2021-06-26星期六的 12:31 -0600，Simon Glass写道：
> > Hi Icenowy,
> >
> > On Sat, 19 Jun 2021 at 03:19, Icenowy Zheng <icenowy@aosc.io> wrote:
> > >
> > > The sunxi_egon type used to take no -A argument (because we assume
> > > sunxi
> > > targets are all ARM). However, as Allwinner D1 appears as the first
> > > RISC-V sunxi target, we need to support -A; in addition, as
> > > external
> > > projects rely on U-Boot mkimage to generate sunxi eGON.BT0 header,
> > > we
> > > need to keep compatibility with command line without -A.
> > >
> > > As the default value of arch in mkimage is not proper (IH_ARCH_PPC
> > > instead of IH_ARCH_INVALID), to keep more compatibility, add an
> > > Aflag
> > > field to image parameters to describe whether an architecture is
> > > explicitly specified.
> > >
> > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > ---
> > > Changes in v2:
> > > - Added Tom Rini's Review tag.
> >
> > This is not actually a change so should not appear in the log.
> >
> > Also note 'patman status' can be used to automatically collect these.
> >
> > >
> > >  tools/imagetool.h | 1 +
> > >  tools/mkimage.c   | 1 +
> > >  2 files changed, 2 insertions(+)
> > >
> > > diff --git a/tools/imagetool.h b/tools/imagetool.h
> > > index e229a34ffc..5dc28312c2 100644
> > > --- a/tools/imagetool.h
> > > +++ b/tools/imagetool.h
> > > @@ -51,6 +51,7 @@ struct image_tool_params {
> > >         int pflag;
> > >         int vflag;
> > >         int xflag;
> > > +       int Aflag;
> >
> > aflag
>
> I think a problem is that -a is a different flag than -A.
>
> -a is addr, -A is architecture.

Yes OK, but the code style doesn't allow capital letters, as I understand it.

How about addr_flag ?


>
> >
> > >         int skipcpy;
> > >         int os;
> > >         int arch;
> > > diff --git a/tools/mkimage.c b/tools/mkimage.c
> > > index cc7b242faf..54d8e3835a 100644
> > > --- a/tools/mkimage.c
> > > +++ b/tools/mkimage.c
> > > @@ -168,6 +168,7 @@ static void process_args(int argc, char **argv)
> > >                                 show_valid_options(IH_ARCH);
> > >                                 usage("Invalid architecture");
> > >                         }
> > > +                       params.Aflag = 1;
> > >                         break;
> > >                 case 'b':
> > >                         if (add_content(IH_TYPE_FLATDT, optarg)) {
> > > --
> > > 2.30.2

Regards,
Simon

Re: [PATCH v2 1/4] mkimage: add a flag to describe whether -A is specified

[Icenowy Zheng]

在 2021-06-27星期日的 13:32 -0600，Simon Glass写道：
> Hi Icenowy,
> 
> On Sat, 26 Jun 2021 at 17:57, Icenowy Zheng <icenowy@aosc.io> wrote:
> > 
> > 在 2021-06-26星期六的 12:31 -0600，Simon Glass写道：
> > > Hi Icenowy,
> > > 
> > > On Sat, 19 Jun 2021 at 03:19, Icenowy Zheng <icenowy@aosc.io>
> > > wrote:
> > > > 
> > > > The sunxi_egon type used to take no -A argument (because we
> > > > assume
> > > > sunxi
> > > > targets are all ARM). However, as Allwinner D1 appears as the
> > > > first
> > > > RISC-V sunxi target, we need to support -A; in addition, as
> > > > external
> > > > projects rely on U-Boot mkimage to generate sunxi eGON.BT0
> > > > header,
> > > > we
> > > > need to keep compatibility with command line without -A.
> > > > 
> > > > As the default value of arch in mkimage is not proper
> > > > (IH_ARCH_PPC
> > > > instead of IH_ARCH_INVALID), to keep more compatibility, add an
> > > > Aflag
> > > > field to image parameters to describe whether an architecture
> > > > is
> > > > explicitly specified.
> > > > 
> > > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > ---
> > > > Changes in v2:
> > > > - Added Tom Rini's Review tag.
> > > 
> > > This is not actually a change so should not appear in the log.
> > > 
> > > Also note 'patman status' can be used to automatically collect
> > > these.
> > > 
> > > > 
> > > >  tools/imagetool.h | 1 +
> > > >  tools/mkimage.c   | 1 +
> > > >  2 files changed, 2 insertions(+)
> > > > 
> > > > diff --git a/tools/imagetool.h b/tools/imagetool.h
> > > > index e229a34ffc..5dc28312c2 100644
> > > > --- a/tools/imagetool.h
> > > > +++ b/tools/imagetool.h
> > > > @@ -51,6 +51,7 @@ struct image_tool_params {
> > > >         int pflag;
> > > >         int vflag;
> > > >         int xflag;
> > > > +       int Aflag;
> > > 
> > > aflag
> > 
> > I think a problem is that -a is a different flag than -A.
> > 
> > -a is addr, -A is architecture.
> 
> Yes OK, but the code style doesn't allow capital letters, as I
> understand it.
> 
> How about addr_flag ?

maybe we just call -a aflag, and -A arch_flag?

> 
> 
> > 
> > > 
> > > >         int skipcpy;
> > > >         int os;
> > > >         int arch;
> > > > diff --git a/tools/mkimage.c b/tools/mkimage.c
> > > > index cc7b242faf..54d8e3835a 100644
> > > > --- a/tools/mkimage.c
> > > > +++ b/tools/mkimage.c
> > > > @@ -168,6 +168,7 @@ static void process_args(int argc, char
> > > > **argv)
> > > >                                 show_valid_options(IH_ARCH);
> > > >                                 usage("Invalid architecture");
> > > >                         }
> > > > +                       params.Aflag = 1;
> > > >                         break;
> > > >                 case 'b':
> > > >                         if (add_content(IH_TYPE_FLATDT,
> > > > optarg)) {
> > > > --
> > > > 2.30.2
> 
> Regards,
> Simon

Re: [PATCH v2 1/4] mkimage: add a flag to describe whether -A is specified

[Tom Rini]

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

On Mon, Jun 28, 2021 at 06:17:27AM +0800, Icenowy Zheng wrote:
> 在 2021-06-27星期日的 13:32 -0600，Simon Glass写道：
> > Hi Icenowy,
> > 
> > On Sat, 26 Jun 2021 at 17:57, Icenowy Zheng <icenowy@aosc.io> wrote:
> > > 
> > > 在 2021-06-26星期六的 12:31 -0600，Simon Glass写道：
> > > > Hi Icenowy,
> > > > 
> > > > On Sat, 19 Jun 2021 at 03:19, Icenowy Zheng <icenowy@aosc.io>
> > > > wrote:
> > > > > 
> > > > > The sunxi_egon type used to take no -A argument (because we
> > > > > assume
> > > > > sunxi
> > > > > targets are all ARM). However, as Allwinner D1 appears as the
> > > > > first
> > > > > RISC-V sunxi target, we need to support -A; in addition, as
> > > > > external
> > > > > projects rely on U-Boot mkimage to generate sunxi eGON.BT0
> > > > > header,
> > > > > we
> > > > > need to keep compatibility with command line without -A.
> > > > > 
> > > > > As the default value of arch in mkimage is not proper
> > > > > (IH_ARCH_PPC
> > > > > instead of IH_ARCH_INVALID), to keep more compatibility, add an
> > > > > Aflag
> > > > > field to image parameters to describe whether an architecture
> > > > > is
> > > > > explicitly specified.
> > > > > 
> > > > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > > ---
> > > > > Changes in v2:
> > > > > - Added Tom Rini's Review tag.
> > > > 
> > > > This is not actually a change so should not appear in the log.
> > > > 
> > > > Also note 'patman status' can be used to automatically collect
> > > > these.
> > > > 
> > > > > 
> > > > >  tools/imagetool.h | 1 +
> > > > >  tools/mkimage.c   | 1 +
> > > > >  2 files changed, 2 insertions(+)
> > > > > 
> > > > > diff --git a/tools/imagetool.h b/tools/imagetool.h
> > > > > index e229a34ffc..5dc28312c2 100644
> > > > > --- a/tools/imagetool.h
> > > > > +++ b/tools/imagetool.h
> > > > > @@ -51,6 +51,7 @@ struct image_tool_params {
> > > > >         int pflag;
> > > > >         int vflag;
> > > > >         int xflag;
> > > > > +       int Aflag;
> > > > 
> > > > aflag
> > > 
> > > I think a problem is that -a is a different flag than -A.
> > > 
> > > -a is addr, -A is architecture.
> > 
> > Yes OK, but the code style doesn't allow capital letters, as I
> > understand it.
> > 
> > How about addr_flag ?
> 
> maybe we just call -a aflag, and -A arch_flag?

Frankly I think Aflag is the most readable option here.  They're all
literally ${FLAG_USE}flag variables, yes?  CamelCase isn't great, but
there's times where the most readable thing isn't always what we use
everywhere else.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 1/4] mkimage: add a flag to describe whether -A is specified

[Simon Glass]

Hi Tom,

On Sun, 27 Jun 2021 at 17:43, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jun 28, 2021 at 06:17:27AM +0800, Icenowy Zheng wrote:
> > 在 2021-06-27星期日的 13:32 -0600，Simon Glass写道：
> > > Hi Icenowy,
> > >
> > > On Sat, 26 Jun 2021 at 17:57, Icenowy Zheng <icenowy@aosc.io> wrote:
> > > >
> > > > 在 2021-06-26星期六的 12:31 -0600，Simon Glass写道：
> > > > > Hi Icenowy,
> > > > >
> > > > > On Sat, 19 Jun 2021 at 03:19, Icenowy Zheng <icenowy@aosc.io>
> > > > > wrote:
> > > > > >
> > > > > > The sunxi_egon type used to take no -A argument (because we
> > > > > > assume
> > > > > > sunxi
> > > > > > targets are all ARM). However, as Allwinner D1 appears as the
> > > > > > first
> > > > > > RISC-V sunxi target, we need to support -A; in addition, as
> > > > > > external
> > > > > > projects rely on U-Boot mkimage to generate sunxi eGON.BT0
> > > > > > header,
> > > > > > we
> > > > > > need to keep compatibility with command line without -A.
> > > > > >
> > > > > > As the default value of arch in mkimage is not proper
> > > > > > (IH_ARCH_PPC
> > > > > > instead of IH_ARCH_INVALID), to keep more compatibility, add an
> > > > > > Aflag
> > > > > > field to image parameters to describe whether an architecture
> > > > > > is
> > > > > > explicitly specified.
> > > > > >
> > > > > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > > > ---
> > > > > > Changes in v2:
> > > > > > - Added Tom Rini's Review tag.
> > > > >
> > > > > This is not actually a change so should not appear in the log.
> > > > >
> > > > > Also note 'patman status' can be used to automatically collect
> > > > > these.
> > > > >
> > > > > >
> > > > > >  tools/imagetool.h | 1 +
> > > > > >  tools/mkimage.c   | 1 +
> > > > > >  2 files changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/tools/imagetool.h b/tools/imagetool.h
> > > > > > index e229a34ffc..5dc28312c2 100644
> > > > > > --- a/tools/imagetool.h
> > > > > > +++ b/tools/imagetool.h
> > > > > > @@ -51,6 +51,7 @@ struct image_tool_params {
> > > > > >         int pflag;
> > > > > >         int vflag;
> > > > > >         int xflag;
> > > > > > +       int Aflag;
> > > > >
> > > > > aflag
> > > >
> > > > I think a problem is that -a is a different flag than -A.
> > > >
> > > > -a is addr, -A is architecture.
> > >
> > > Yes OK, but the code style doesn't allow capital letters, as I
> > > understand it.
> > >
> > > How about addr_flag ?
> >
> > maybe we just call -a aflag, and -A arch_flag?
>
> Frankly I think Aflag is the most readable option here.  They're all
> literally ${FLAG_USE}flag variables, yes?  CamelCase isn't great, but
> there's times where the most readable thing isn't always what we use
> everywhere else.

I mean, sort of, but 'a' and 'A' are not that descriptive so you could
make the case that they should be named for the full flag
meaning/purpose.

Anyway, I see it has your review tag, so I'll leave it to you.

Regards,
Simon

Re: [PATCH v2 1/4] mkimage: add a flag to describe whether -A is specified

[Simon Glass]

Hi Icenowy,

On Sun, 27 Jun 2021 at 16:17, Icenowy Zheng <icenowy@aosc.io> wrote:
>
> 在 2021-06-27星期日的 13:32 -0600，Simon Glass写道：
> > Hi Icenowy,
> >
> > On Sat, 26 Jun 2021 at 17:57, Icenowy Zheng <icenowy@aosc.io> wrote:
> > >
> > > 在 2021-06-26星期六的 12:31 -0600，Simon Glass写道：
> > > > Hi Icenowy,
> > > >
> > > > On Sat, 19 Jun 2021 at 03:19, Icenowy Zheng <icenowy@aosc.io>
> > > > wrote:
> > > > >
> > > > > The sunxi_egon type used to take no -A argument (because we
> > > > > assume
> > > > > sunxi
> > > > > targets are all ARM). However, as Allwinner D1 appears as the
> > > > > first
> > > > > RISC-V sunxi target, we need to support -A; in addition, as
> > > > > external
> > > > > projects rely on U-Boot mkimage to generate sunxi eGON.BT0
> > > > > header,
> > > > > we
> > > > > need to keep compatibility with command line without -A.
> > > > >
> > > > > As the default value of arch in mkimage is not proper
> > > > > (IH_ARCH_PPC
> > > > > instead of IH_ARCH_INVALID), to keep more compatibility, add an
> > > > > Aflag
> > > > > field to image parameters to describe whether an architecture
> > > > > is
> > > > > explicitly specified.
> > > > >
> > > > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > > ---
> > > > > Changes in v2:
> > > > > - Added Tom Rini's Review tag.
> > > >
> > > > This is not actually a change so should not appear in the log.
> > > >
> > > > Also note 'patman status' can be used to automatically collect
> > > > these.
> > > >
> > > > >
> > > > >  tools/imagetool.h | 1 +
> > > > >  tools/mkimage.c   | 1 +
> > > > >  2 files changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/tools/imagetool.h b/tools/imagetool.h
> > > > > index e229a34ffc..5dc28312c2 100644
> > > > > --- a/tools/imagetool.h
> > > > > +++ b/tools/imagetool.h
> > > > > @@ -51,6 +51,7 @@ struct image_tool_params {
> > > > >         int pflag;
> > > > >         int vflag;
> > > > >         int xflag;
> > > > > +       int Aflag;
> > > >
> > > > aflag
> > >
> > > I think a problem is that -a is a different flag than -A.
> > >
> > > -a is addr, -A is architecture.
> >
> > Yes OK, but the code style doesn't allow capital letters, as I
> > understand it.
> >
> > How about addr_flag ?
>
> maybe we just call -a aflag, and -A arch_flag?

Sure if you prefer.

Regards,
Simon

[PATCH v2 2/4] mkimage: sunxi_egon: refactor for multi-architecture support

[Icenowy Zheng]

Refactor some functions in mkimage sunxi_egon type, in order to prepare
for adding support for more CPU architectures (e.g. RISC-V). In
addition, compatibility for operation w/o specified architecture is
kept, in this case the architecture is assumed as ARM.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v2:
- Merged fixes in the next patch in v1 (patch rebase issue).

 tools/sunxi_egon.c | 63 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 56 insertions(+), 7 deletions(-)

diff --git a/tools/sunxi_egon.c b/tools/sunxi_egon.c
index a5299eb6a1..062c9bc151 100644
--- a/tools/sunxi_egon.c
+++ b/tools/sunxi_egon.c
@@ -16,7 +16,25 @@
 
 static int egon_check_params(struct image_tool_params *params)
 {
-	/* We just need a binary image file. */
+	int arch;
+
+	/* Assume ARM when no architecture specified for compatibility */
+	if (params->Aflag)
+		arch = params->arch;
+	else
+		arch = IH_ARCH_ARM;
+
+	/*
+	 * Check whether the architecture is supported.
+	 */
+	switch (arch) {
+	case IH_ARCH_ARM:
+		break;
+	default:
+		return EXIT_FAILURE;
+	}
+
+	/* We need a binary image file. */
 	return !params->dflag;
 }
 
@@ -25,10 +43,26 @@ static int egon_verify_header(unsigned char *ptr, int image_size,
 {
 	const struct boot_file_head *header = (void *)ptr;
 	uint32_t length;
+	int arch;
 
-	/* First 4 bytes must be an ARM branch instruction. */
-	if ((le32_to_cpu(header->b_instruction) & 0xff000000) != 0xea000000)
-		return EXIT_FAILURE;
+	/* Assume ARM when no architecture specified for compatibility */
+	if (params->Aflag)
+		arch = params->arch;
+	else
+		arch = IH_ARCH_ARM;
+
+	/*
+	 * First 4 bytes must be a branch instruction of the corresponding
+	 * architecture.
+	 */
+	switch (arch) {
+	case IH_ARCH_ARM:
+		if ((le32_to_cpu(header->b_instruction) & 0xff000000) != 0xea000000)
+			return EXIT_FAILURE;
+		break;
+	default:
+		return EXIT_FAILURE; /* Unknown architecture */
+	}
 
 	if (memcmp(header->magic, BOOT0_MAGIC, sizeof(header->magic)))
 		return EXIT_FAILURE;
@@ -76,10 +110,25 @@ static void egon_set_header(void *buf, struct stat *sbuf, int infd,
 	uint32_t *buf32 = buf;
 	uint32_t checksum = 0, value;
 	int i;
+	int arch;
 
-	/* Generate an ARM branch instruction to jump over the header. */
-	value = 0xea000000 | (sizeof(struct boot_file_head) / 4 - 2);
-	header->b_instruction = cpu_to_le32(value);
+	/* Assume ARM when no architecture specified for compatibility */
+	if (params->Aflag)
+		arch = params->arch;
+	else
+		arch = IH_ARCH_ARM;
+
+	/*
+	 * Different architectures need different first instruction to
+	 * branch to the body.
+	 */
+	switch (arch) {
+	case IH_ARCH_ARM:
+		/* Generate an ARM branch instruction to jump over the header. */
+		value = 0xea000000 | (sizeof(struct boot_file_head) / 4 - 2);
+		header->b_instruction = cpu_to_le32(value);
+		break;
+	}
 
 	memcpy(header->magic, BOOT0_MAGIC, sizeof(header->magic));
 	header->check_sum = cpu_to_le32(BROM_STAMP_VALUE);
-- 
2.30.2

Re: [PATCH v2 2/4] mkimage: sunxi_egon: refactor for multi-architecture support

[Andre Przywara]

On Sat, 19 Jun 2021 17:20:04 +0800
Icenowy Zheng <icenowy@aosc.io> wrote:

> Refactor some functions in mkimage sunxi_egon type, in order to prepare
> for adding support for more CPU architectures (e.g. RISC-V). In
> addition, compatibility for operation w/o specified architecture is
> kept, in this case the architecture is assumed as ARM.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>

Looks alright:
Reviewed-by: Andre Przywara <andre.przywara@arm.com>

One thing you might want to improve (just when you respin anyway):

> ---
> Changes in v2:
> - Merged fixes in the next patch in v1 (patch rebase issue).
> 
>  tools/sunxi_egon.c | 63 ++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 56 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/sunxi_egon.c b/tools/sunxi_egon.c
> index a5299eb6a1..062c9bc151 100644
> --- a/tools/sunxi_egon.c
> +++ b/tools/sunxi_egon.c
> @@ -16,7 +16,25 @@
>  
>  static int egon_check_params(struct image_tool_params *params)
>  {
> -	/* We just need a binary image file. */
> +	int arch;
> +
> +	/* Assume ARM when no architecture specified for compatibility */
> +	if (params->Aflag)
> +		arch = params->arch;
> +	else
> +		arch = IH_ARCH_ARM;

This code snippet will be used three times in this file, so you might
want to factor this out.

Cheers,
Andre


> +
> +	/*
> +	 * Check whether the architecture is supported.
> +	 */
> +	switch (arch) {
> +	case IH_ARCH_ARM:
> +		break;
> +	default:
> +		return EXIT_FAILURE;
> +	}
> +
> +	/* We need a binary image file. */
>  	return !params->dflag;
>  }
>  
> @@ -25,10 +43,26 @@ static int egon_verify_header(unsigned char *ptr, int image_size,
>  {
>  	const struct boot_file_head *header = (void *)ptr;
>  	uint32_t length;
> +	int arch;
>  
> -	/* First 4 bytes must be an ARM branch instruction. */
> -	if ((le32_to_cpu(header->b_instruction) & 0xff000000) != 0xea000000)
> -		return EXIT_FAILURE;
> +	/* Assume ARM when no architecture specified for compatibility */
> +	if (params->Aflag)
> +		arch = params->arch;
> +	else
> +		arch = IH_ARCH_ARM;
> +
> +	/*
> +	 * First 4 bytes must be a branch instruction of the corresponding
> +	 * architecture.
> +	 */
> +	switch (arch) {
> +	case IH_ARCH_ARM:
> +		if ((le32_to_cpu(header->b_instruction) & 0xff000000) != 0xea000000)
> +			return EXIT_FAILURE;
> +		break;
> +	default:
> +		return EXIT_FAILURE; /* Unknown architecture */
> +	}
>  
>  	if (memcmp(header->magic, BOOT0_MAGIC, sizeof(header->magic)))
>  		return EXIT_FAILURE;
> @@ -76,10 +110,25 @@ static void egon_set_header(void *buf, struct stat *sbuf, int infd,
>  	uint32_t *buf32 = buf;
>  	uint32_t checksum = 0, value;
>  	int i;
> +	int arch;
>  
> -	/* Generate an ARM branch instruction to jump over the header. */
> -	value = 0xea000000 | (sizeof(struct boot_file_head) / 4 - 2);
> -	header->b_instruction = cpu_to_le32(value);
> +	/* Assume ARM when no architecture specified for compatibility */
> +	if (params->Aflag)
> +		arch = params->arch;
> +	else
> +		arch = IH_ARCH_ARM;
> +
> +	/*
> +	 * Different architectures need different first instruction to
> +	 * branch to the body.
> +	 */
> +	switch (arch) {
> +	case IH_ARCH_ARM:
> +		/* Generate an ARM branch instruction to jump over the header. */
> +		value = 0xea000000 | (sizeof(struct boot_file_head) / 4 - 2);
> +		header->b_instruction = cpu_to_le32(value);
> +		break;
> +	}
>  
>  	memcpy(header->magic, BOOT0_MAGIC, sizeof(header->magic));
>  	header->check_sum = cpu_to_le32(BROM_STAMP_VALUE);

[PATCH v2 3/4] mkimage: sunxi_egon: add support for riscv

[Icenowy Zheng]

There's now a sun20i family in sunxi, which uses RISC-V CPU.

Add support for making eGON.BT0 image for RISC-V.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v2:
- Removed changes that should belong to the previous patch in v1.

 tools/sunxi_egon.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tools/sunxi_egon.c b/tools/sunxi_egon.c
index 062c9bc151..836e99a6e6 100644
--- a/tools/sunxi_egon.c
+++ b/tools/sunxi_egon.c
@@ -29,6 +29,7 @@ static int egon_check_params(struct image_tool_params *params)
 	 */
 	switch (arch) {
 	case IH_ARCH_ARM:
+	case IH_ARCH_RISCV:
 		break;
 	default:
 		return EXIT_FAILURE;
@@ -60,6 +61,10 @@ static int egon_verify_header(unsigned char *ptr, int image_size,
 		if ((le32_to_cpu(header->b_instruction) & 0xff000000) != 0xea000000)
 			return EXIT_FAILURE;
 		break;
+	case IH_ARCH_RISCV:
+		if ((le32_to_cpu(header->b_instruction) & 0x00000fff) != 0x0000006f)
+			return EXIT_FAILURE;
+		break;
 	default:
 		return EXIT_FAILURE; /* Unknown architecture */
 	}
@@ -128,6 +133,24 @@ static void egon_set_header(void *buf, struct stat *sbuf, int infd,
 		value = 0xea000000 | (sizeof(struct boot_file_head) / 4 - 2);
 		header->b_instruction = cpu_to_le32(value);
 		break;
+	case IH_ARCH_RISCV:
+		/*
+		 * Generate a RISC-V JAL instruction with rd=x0
+		 * (pseudo instruction J, jump without side effects).
+		 *
+		 * The following weird bit operation maps imm[20]
+		 * to inst[31], imm[10:1] to inst[30:21],
+		 * imm[11] to inst[20], imm[19:12] to inst[19:12],
+		 * and imm[0] is dropped (because 1-byte RISC-V instruction
+		 * is not allowed).
+		 */
+		value = 0x0000006f |
+			((sizeof(struct boot_file_head) & 0x00100000) << 11) |
+			((sizeof(struct boot_file_head) & 0x000007fe) << 20) |
+			((sizeof(struct boot_file_head) & 0x00000800) << 9) |
+			((sizeof(struct boot_file_head) & 0x000ff000) << 0);
+		header->b_instruction = cpu_to_le32(value);
+		break;
 	}
 
 	memcpy(header->magic, BOOT0_MAGIC, sizeof(header->magic));
-- 
2.30.2

Re: [PATCH v2 3/4] mkimage: sunxi_egon: add support for riscv

[Andre Przywara]

On Sat, 19 Jun 2021 17:20:05 +0800
Icenowy Zheng <icenowy@aosc.io> wrote:

> There's now a sun20i family in sunxi, which uses RISC-V CPU.
> 
> Add support for making eGON.BT0 image for RISC-V.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>

Compared against the RISC-V manual.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
> Changes in v2:
> - Removed changes that should belong to the previous patch in v1.
> 
>  tools/sunxi_egon.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/tools/sunxi_egon.c b/tools/sunxi_egon.c
> index 062c9bc151..836e99a6e6 100644
> --- a/tools/sunxi_egon.c
> +++ b/tools/sunxi_egon.c
> @@ -29,6 +29,7 @@ static int egon_check_params(struct image_tool_params *params)
>  	 */
>  	switch (arch) {
>  	case IH_ARCH_ARM:
> +	case IH_ARCH_RISCV:
>  		break;
>  	default:
>  		return EXIT_FAILURE;
> @@ -60,6 +61,10 @@ static int egon_verify_header(unsigned char *ptr, int image_size,
>  		if ((le32_to_cpu(header->b_instruction) & 0xff000000) != 0xea000000)
>  			return EXIT_FAILURE;
>  		break;
> +	case IH_ARCH_RISCV:
> +		if ((le32_to_cpu(header->b_instruction) & 0x00000fff) != 0x0000006f)
> +			return EXIT_FAILURE;
> +		break;
>  	default:
>  		return EXIT_FAILURE; /* Unknown architecture */
>  	}
> @@ -128,6 +133,24 @@ static void egon_set_header(void *buf, struct stat *sbuf, int infd,
>  		value = 0xea000000 | (sizeof(struct boot_file_head) / 4 - 2);
>  		header->b_instruction = cpu_to_le32(value);
>  		break;
> +	case IH_ARCH_RISCV:
> +		/*
> +		 * Generate a RISC-V JAL instruction with rd=x0
> +		 * (pseudo instruction J, jump without side effects).
> +		 *
> +		 * The following weird bit operation maps imm[20]
> +		 * to inst[31], imm[10:1] to inst[30:21],
> +		 * imm[11] to inst[20], imm[19:12] to inst[19:12],
> +		 * and imm[0] is dropped (because 1-byte RISC-V instruction
> +		 * is not allowed).
> +		 */
> +		value = 0x0000006f |
> +			((sizeof(struct boot_file_head) & 0x00100000) << 11) |
> +			((sizeof(struct boot_file_head) & 0x000007fe) << 20) |
> +			((sizeof(struct boot_file_head) & 0x00000800) << 9) |
> +			((sizeof(struct boot_file_head) & 0x000ff000) << 0);
> +		header->b_instruction = cpu_to_le32(value);
> +		break;
>  	}
>  
>  	memcpy(header->magic, BOOT0_MAGIC, sizeof(header->magic));

[PATCH v2 4/4] sunxi: specify architecture when generating SPL boot image

[Icenowy Zheng]

As mkimage -T sunxi_egon now gains support for -A parameter, specify the
architecture when generating SPL boot image for sunxi.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
No changes in v2.

 scripts/Makefile.spl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 5be1a9ba1b..c13a41a243 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -414,7 +414,7 @@ endif
 $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
 	$(call if_changed,mkimage)
 
-MKIMAGEFLAGS_sunxi-spl.bin = -T sunxi_egon \
+MKIMAGEFLAGS_sunxi-spl.bin = -A $(ARCH) -T sunxi_egon \
 	-n $(CONFIG_DEFAULT_DEVICE_TREE)
 
 OBJCOPYFLAGS_u-boot-spl-dtb.hex := -I binary -O ihex --change-address=$(CONFIG_SPL_TEXT_BASE)
-- 
2.30.2

Re: [PATCH v2 4/4] sunxi: specify architecture when generating SPL boot image

[Andre Przywara]

On Sat, 19 Jun 2021 17:21:11 +0800
Icenowy Zheng <icenowy@aosc.io> wrote:

> As mkimage -T sunxi_egon now gains support for -A parameter, specify the
> architecture when generating SPL boot image for sunxi.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks!
Andre

> ---
> No changes in v2.
> 
>  scripts/Makefile.spl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 5be1a9ba1b..c13a41a243 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -414,7 +414,7 @@ endif
>  $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
>  	$(call if_changed,mkimage)
>  
> -MKIMAGEFLAGS_sunxi-spl.bin = -T sunxi_egon \
> +MKIMAGEFLAGS_sunxi-spl.bin = -A $(ARCH) -T sunxi_egon \
>  	-n $(CONFIG_DEFAULT_DEVICE_TREE)
>  
>  OBJCOPYFLAGS_u-boot-spl-dtb.hex := -I binary -O ihex --change-address=$(CONFIG_SPL_TEXT_BASE)

Re: [PATCH v2 0/4] mkimage: sunxi_egon: add riscv support

[Samuel Holland]

On 6/19/21 4:18 AM, Icenowy Zheng wrote:
> This patchset tries to make mkimage -T sunxi_egon to be able to generate
> an eGON.BT0 image for Allwinner RISC-V SoCs (e.g. D1).
> 
> In addition, to keep the compatibility, it will still consider the
> architecture to be ARM when no architecture is specified.
> 
> Icenowy Zheng (4):
>   mkimage: add a flag to describe whether -A is specified
>   mkimage: sunxi_egon: refactor for multi-architecture support
>   mkimage: sunxi_egon: add support for riscv
>   sunxi: specify architecture when generating SPL boot image
> 
>  scripts/Makefile.spl |  2 +-
>  tools/imagetool.h    |  1 +
>  tools/mkimage.c      |  1 +
>  tools/sunxi_egon.c   | 86 ++++++++++++++++++++++++++++++++++++++++----
>  4 files changed, 82 insertions(+), 8 deletions(-)

This works great on D1. For the whole series:

Tested-by: Samuel Holland <samuel@sholland.org>

Re: [PATCH v2 0/4] mkimage: sunxi_egon: add riscv support

[Andre Przywara]

On Sat, 19 Jun 2021 17:18:34 +0800
Icenowy Zheng <icenowy@aosc.io> wrote:

> This patchset tries to make mkimage -T sunxi_egon to be able to generate
> an eGON.BT0 image for Allwinner RISC-V SoCs (e.g. D1).
> 
> In addition, to keep the compatibility, it will still consider the
> architecture to be ARM when no architecture is specified.

All merged into sunxi/master (which already landed in mainline).

Thanks!
Andre

> 
> Icenowy Zheng (4):
>   mkimage: add a flag to describe whether -A is specified
>   mkimage: sunxi_egon: refactor for multi-architecture support
>   mkimage: sunxi_egon: add support for riscv
>   sunxi: specify architecture when generating SPL boot image
> 
>  scripts/Makefile.spl |  2 +-
>  tools/imagetool.h    |  1 +
>  tools/mkimage.c      |  1 +
>  tools/sunxi_egon.c   | 86 ++++++++++++++++++++++++++++++++++++++++----
>  4 files changed, 82 insertions(+), 8 deletions(-)
>