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

From: Sam Edwards <cfsworks@gmail.com>
To: Andre Przywara <andre.przywara@arm.com>,
	Samuel Holland <samuel@sholland.org>,
	Jagan Teki <jagan@amarulasolutions.com>
Cc: u-boot@lists.denx.de, Icenowy Zheng <uwu@icenowy.me>,
	Jernej Skrabec <jernej.skrabec@gmail.com>
Subject: Re: [RFC PATCH 08/17] sunxi: introduce NCAT2 generation model
Date: Mon, 15 May 2023 20:32:52 -0600	[thread overview]
Message-ID: <e9e460b5-6b27-7ae4-265a-fad974aecdea@gmail.com> (raw)
In-Reply-To: <20221206004549.29015-9-andre.przywara@arm.com>

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

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.

> 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.

> 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?

Much gratitude,
Sam