[PATCH v5] Add support for SoM "VoCore2".

[Weijie Gao]

On Mon, 2020-02-17 at 14:22 +0100, Mauro Condarelli wrote:
> Hi Daniel,
> a gentle reminder...
> 
> I was unable to comply with a few of Your remarks (see below),
> What should I do?
> Submit v6 as is or do You have specific instructions?
> 
> Thanks a lot
> Mauro
> 
> On 2/12/20 11:01 PM, Mauro Condarelli wrote:
> > Thanks Daniel,
> >
> > On 2/12/20 5:58 PM, Daniel Schwierzeck wrote:
> >> On Wed, Feb 12, 2020 at 4:30 PM Mauro Condarelli <mc5686 at mclink.it> wrote:
> >>> Small patch series to add support for VoCore/VoCore2 board.
> >>>
> >>> VoCore is open hardware and runs OpenWrt/LEDE.
> >>> It has WIFI, USB, UART, 20+ GPIOs but is only one inch square.
> >>> It will help you to make a smart house, study embedded system
> >>> or even make the tiniest router in the world.
> >>>
> >>> ===8<---
> >>> diff --git a/board/vocore/vocore2/board.c b/board/vocore/vocore2/board.c
> >>> new file mode 100644
> >>> index 0000000000..27e42d1414
> >>> --- /dev/null
> >>> +++ b/board/vocore/vocore2/board.c
> >>> @@ -0,0 +1,6 @@
> >>> +// SPDX-License-Identifier: GPL-2.0+
> >>> +/*
> >>> + * Copyright (C) 2019 Mauro Condarelli <mc5686 at mclink.it>
> >>> + *
> >>> + * Nothing actually needed here
> >>> + */
> >> if that file is empty, don't add it at all
> > Ahem...
> >
> > I tried to remove it, but if I do I also have to remove entry
> > in board/vocore/vocore2/Makefile (leaving it empty) and this
> > leads (on a clean compile) to error:
> >   mipsel-linux-ld.bfd: cannot find board/vocore/vocore2/built-in.o: No
> > such file or directory
> > Completely removing both files bombs with:
> >   scripts/Makefile.build:57: board/vocore/vocore2/Makefile: No such file
> > or directory
> > It seems I should also remove board/vocore/vocore2/Kconfig,
> > but I'm unsure this is the right thing to do.
> >
> > I have no idea about how to solve this, sorry.
> > Can You point me in the right direction, please?
> >
> >
> >>> ===8<---
> >>> diff --git a/include/configs/vocore2.h b/include/configs/vocore2.h
> >>> new file mode 100644
> >>> index 0000000000..6b43aa766e
> >>> --- /dev/null
> >>> +++ b/include/configs/vocore2.h
> >>> @@ -0,0 +1,61 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0+ */
> >>> +/*
> >>> + * Copyright (C) 2019 Mauro Condarelli <mc5686 at mclink.it>
> >>> + */
> >>> +
> >>> +#ifndef __VOCORE2_CONFIG_H__
> >>> +#define __VOCORE2_CONFIG_H__
> >>> +
> >>> +/* CPU */
> >>> +#define CONFIG_SYS_MIPS_TIMER_FREQ     290000000
> >> is this still necessary? Weijie implemented a custom CPU clock
> >> handling which supports
> >> dynamic timer clocks.
> > This is referenced in arch/mips/cpu/time.c
> > I don't know if it's still relevant, but removing it doesn't seem
> > task for this patch.

You have to keep this to make sure arch/mips/cpu/time.c can be compiled.

> >
> >
> >>> +
> >>> +/* RAM */
> >>> +#define CONFIG_SYS_SDRAM_BASE          0x80000000
> >>> +
> >>> +#define CONFIG_SYS_LOAD_ADDR   CONFIG_SYS_SDRAM_BASE + 0x100000
> >>> +
> >>> +#define CONFIG_SYS_INIT_SP_OFFSET      0x400000
> >>> +
> >>> +/* SPL */
> >>> +#if defined(CONFIG_SPL) && !defined(CONFIG_SPL_BUILD)
> >>> +#define CONFIG_SKIP_LOWLEVEL_INIT
> >>> +#endif
> >> CONFIG_SPL_BUILD is only relevant in Makefiles and shouldn't be used
> >> in config header files
> > Removed, apparently without adverse consequences.
> Appearence was misguiding.
> Any change to this guard leads to code unable to boot from SPI NOR.
> I'll have to leave it is as-is.
> Someone more knowledgeable than me will have to understand why
> they are needed and how to remove them, if that's a requirement.
> After all the other boards with this SoC use the same code.

CONFIG_SPL_BUILD is defined only when building the SPL binary.
CONFIG_SPL is defined for both SPL and u-boot if SPL is enabled, and we
can only use CONFIG_SPL_BUILD to distinguish whether we are building SPL
or not.
CONFIG_SKIP_LOWLEVEL_INIT apparently means do not do lowlevel_init.
However lowlevel_init must be done once and only once for a SPI NOR
bootable version.

The following table describes whether lowlevel_init is needed:

             |  SPL  | u-boot
------------------------------
SPL enabled  |   Y   |   N
------------------------------
SPL disabled |       |   Y

As you can see we only want to define CONFIG_SKIP_LOWLEVEL_INIT for just
one case: SPL enabled and not building SPL, because lowlevel_init is
done in SPL and u-boot is a ram boot binary.

As a result this table produces the condition I used for other boards:
#if defined(CONFIG_SPL) && !defined(CONFIG_SPL_BUILD)

If we use only "defined(CONFIG_SPL)", CONFIG_SKIP_LOWLEVEL_INIT will be
defined for both parts, and this will finally lead to code unable to
boot from SPI NOR.

> 
> 
> >>> +
> >>> +#define CONFIG_SYS_UBOOT_START         CONFIG_SYS_TEXT_BASE
> >>> +#define CONFIG_SPL_BSS_START_ADDR      0x80010000
> >>> +#define CONFIG_SPL_BSS_MAX_SIZE                0x10000
> >>> +#define CONFIG_SPL_MAX_SIZE            0x10000
> >>> +
> >>> +/* Dummy value */
> >>> +#define CONFIG_SYS_UBOOT_BASE          0
> >> where is that needed?
> > This is referenced in common/spl/spl_nor.c
> > I don't know if it's still relevant, but removing it doesn't seem
> > task for this patch.

Same as CONFIG_SYS_MIPS_TIMER_FREQ.

> >
> >>> ==8<---
> >>>
> > I'll wait for input before submitting another iteration.
> >
> > Thanks a lot
> > Mauro
>