[U-Boot] [PATCH 1/8] xtensa: add support for the xtensa processor architecture [1/2]

[Max Filippov]

Hi Simon,

On Tue, Jul 12, 2016 at 03:56:01PM -0600, Simon Glass wrote:
> On 8 July 2016 at 09:41, Max Filippov <jcmvbkbc at gmail.com> wrote:
> > From: Chris Zankel <chris at zankel.net>
> >
> > The Xtensa processor architecture is a configurable, extensible,
> > and synthesizable 32-bit RISC processor core provided by Cadence.
> >
> > This is the first part of the basic architecture port with changes to
> > common files. The 'arch/xtensa' directory, and boards and additional
> > drivers will be in separate commits.
> >
> > Signed-off-by: Chris Zankel <chris at zankel.net>
> > Signed-off-by: Max Filippov <jcmvbkbc at gmail.com>
> > ---
> >  MAINTAINERS                 |   5 +++
> >  MAKEALL                     |   6 +++
> >  Makefile                    |  10 ++++-
> >  cmd/bdinfo.c                |  22 ++++++++++
> >  common/board_f.c            |   7 ++--
> >  common/image.c              |   1 +
> >  doc/README.xtensa           | 100 ++++++++++++++++++++++++++++++++++++++++++++
> >  examples/standalone/stubs.c |  47 +++++++++++++++++++++
> >  include/image.h             |   1 +
> >  include/linux/stat.h        |   4 +-
> >  10 files changed, 197 insertions(+), 6 deletions(-)
> >  create mode 100644 doc/README.xtensa
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>

Thanks for the review.

[...]

> > diff --git a/MAKEALL b/MAKEALL
> > index 7e42f10..e669aa9 100755
> > --- a/MAKEALL
> > +++ b/MAKEALL
> 
> This is deprecated and will be removed soon, so you don't need to
> update it. See buildman.

Ok.

[...]

> > diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
> > index 1c4bed9..076b950 100644
> > --- a/cmd/bdinfo.c
> > +++ b/cmd/bdinfo.c
> > @@ -563,6 +563,28 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >         return 0;
> >  }
> >
> > +#elif defined(CONFIG_XTENSA)
> > +
> > +int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > +{
> > +       bd_t *bd = gd->bd;
> > +
> > +       print_num("boot_params",        (ulong)bd->bi_boot_params);
> > +       print_num("memstart",           (ulong)bd->bi_memstart);
> > +       print_lnum("memsize",           (u64)bd->bi_memsize);
> > +       print_num("flashstart",         (ulong)bd->bi_flashstart);
> > +       print_num("flashsize",          (ulong)bd->bi_flashsize);
> > +       print_num("flashoffset",        (ulong)bd->bi_flashoffset);
> > +
> > +#if defined(CONFIG_CMD_NET)
> > +       print_eth(0);
> > +       printf("ip_addr     = %s\n", getenv("ipaddr"));
> > +#endif
> > +       printf("baudrate    = %d bps\n", gd->baudrate);
> 
> It would be great if you could put this in a common function so that
> other archs can call it. There is a lot of duplication in this file.

Ok, let me look at that.

[...]

> > diff --git a/doc/README.xtensa b/doc/README.xtensa
> > new file mode 100644
> > index 0000000..a461714
> > --- /dev/null
> > +++ b/doc/README.xtensa

[...]

> > +The header files for one particular processor configuration are inside
> > +a variant specific directory located in the arch/xtensa/include/asm
> 
> variant-specific

Ok.

[...]

> > +Windowed ABI presents unique difficulties with the approach based on
> > +keeping global data pointer in dedicated register. Because the register
> > +window rotates during a call, there is no register that is constantly
> > +available for the gd pointer. Therefore, a global variable is used in
> > +the same manner as i386.
> 
> Or you could make gd a simple global variable. It you keep it in the
> data section that should work.

gd is a simple global variable on xtensa, I haven't actually checked
what 'in the same manner as i386' means now. Will fix wording.

[...]

> > +U-Boot for Xtensa provides a special memory exception handler to prevent
> > +hanging. The memory exception handler simply skips over the offending
> > +instruction. It is called for all memory exception causes that might
> > +be triggered by bad user input. While this is simplistic, it's better
> > +than hanging in most cases, while keeping U-Boot small and simple.
> 
> Wouldn't it be better to print an exception message? See for example
> ARM's do_prefetch_abort().

I see that on ARM such errors result in panic and reset. Sure I can do
that if that's the preferred way of dealing with such errors.

-- 
Thanks.
-- Max