Re: [RFC PATCH 08/17] sunxi: introduce NCAT2 generation model

Andre Przywara
2023-05-16T21:10:15Z

On Mon, 15 May 2023 20:32:52 -0600
Sam Edwards <cfsworks@gmail.com> wrote:

Hi Sam,

> Hi Andre! Thank you for your efforts on this patchset; I've been
> test-driving it a bit myself this week.

many thanks for having a look, that's much appreciated!

> On 12/5/22 17:45, Andre Przywara wrote:
>
> > +#define SUNXI_RTC_BASE 0x07000000
> > +#define SUNXI_R_CPUCFG_BASE 0x07000400
> > +#define SUNXI_PRCM_BASE 0x07010000
> > +#define SUNXI_R_WDOG_BASE 0x07020400
> > +#define SUNXI_R_UART_BASE 0x07080000
> > +#define SUNXI_R_TWI_BASE 0x07081400
>
> How sure are we that this memory map is consistent across the whole
> NCAT2 family? The documentation for my target (T113-S3) puts the RTC
> base at 0x07090000, for example. I find no mention of there being a PRCM
> peripheral in this particular chip either.

This whole memory map is somewhat of a legacy. Apart from a few
addresses for the SPL needs we shouldn't have those defines at all.
Some symbols are needed because there are other macros using them,
although these then are eventually unused.
I have some patches to remove most of the symbols, and patch 14/17
demonstrates some idea how to pin this down to what's really needed.

For this particular case: this was copied from the H6 memory map, some
addresses are just plain wrong for the D1 family. I will try to remove
them as much as possible, leaving only the ones needed in.

> > diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> > index fef01bdd7da..fdd64db498f 100644
> > --- a/common/spl/Kconfig
> > +++ b/common/spl/Kconfig
> > @@ -265,7 +265,7 @@ config SPL_TEXT_BASE
> > default 0x402F0400 if AM33XX
> > default 0x40301350 if OMAP54XX
> > default 0x10060 if MACH_SUN50I || MACH_SUN50I_H5 || MACH_SUN9I
> > - default 0x20060 if SUN50I_GEN_H6
> > + default 0x20060 if SUN50I_GEN_H6 || SUNXI_GEN_NCAT2
> > default 0x00060 if ARCH_SUNXI
> > default 0xfffc0000 if ARCH_ZYNQMP
> > default 0x0
>
> Would it also be good to change the default for CONFIG_SPL_STACK? As-is
> it defaults to 0x8000, which would put it in the BROM region.
> Allwinner's boot0 starts its stack at 0x48000, which I've been using.

Yeah, well spotted, this was a bug in this early RTC post. I never ran
the SPL, because the DRAM code was missing still. I fixed the stack
pointer meanwhile in an updated (and working) version on Github:
https://github.com/apritzel/u-boot/commit/d5fa559abdf2#diff-19adb575625e29e7a996e0ffb1a266e24239d9b004f17154616220a3f17a24ed

> > diff --git a/include/sunxi_gpio.h b/include/sunxi_gpio.h
> > index 2f8b220f750..04d7aa3d632 100644
> > --- a/include/sunxi_gpio.h
> > +++ b/include/sunxi_gpio.h
> > @@ -16,6 +16,9 @@
> > #elif defined(CONFIG_SUN50I_GEN_H6)
> > #define SUNXI_PIO_BASE 0x0300b000
> > #define SUNXI_R_PIO_BASE 0x07022000
> > +#elif defined(CONFIG_SUNXI_GEN_NCAT2)
> > +#define SUNXI_PIO_BASE 0x02000000
> > +#define SUNXI_R_PIO_BASE 0
> > #else
> > #define SUNXI_PIO_BASE 0x01c20800
> > #define SUNXI_R_PIO_BASE 0x01f02c00
>
> Code elsewhere assumes that SUNXI_R_PIO_BASE is nonzero; on my local
> branch in particular I had to update
> arch/arm/mach-sunxi/board.c:gpio_init. Perhaps it would be better to
> leave SUNXI_R_PIO_BASE undefined in the chips where this gadget is missing?

I think leaving it undefined causes more problems, doesn't it? Looking
at #define BANK_TO_GPIO(bank) in
arch/arm/include/asm/arch-sunxi/gpio.h, specifically. I am saying that
because that's what I tried first ;-)

So where did you see problems? If you would (wrongly) reference
PortL somewhere in SPL GPIO code, it would use a wrong pointer, but at
least the code would still compile fine, wouldn't it?

Cheers,
Andre

P.S. Could you try the github post? Then compiled and booted fine for
me, and includes the DRAM code as well now: