[PATCH 09/11] sunxi: Add support for SUNIV architecture
Andre Przywara
andre.przywara at arm.com
Sat Jan 29 12:51:18 CET 2022
On Fri, 28 Jan 2022 22:21:28 -0500
Jesse Taube <mr.bossman075 at gmail.com> wrote:
> On 1/26/22 09:38, Jesse Taube wrote:
> >
> >
> > On 1/26/22 09:13, Andre Przywara wrote:
> >> On Tue, 4 Jan 2022 19:35:06 -0500
> >> Jesse Taube <mr.bossman075 at gmail.com> wrote:
> >>
> >> Hi Jesse,
> >>
> >> I was checking some bits and pieces here, so sorry for the delay. I saw
> >> your v2, and will review that ASAP, so that we get one step closer. Please
> >> don't send a v3 before that.
> >>
> >> If you have some time, can you also meanwhile check if this series is
> >> bisectable, meaning that every patch compiles? I have the feeling there is
> >> something off, but didn't check it. Pick an H3 and an A64 board, and
> >> compile it after each patch. I can do this as well, if you don't find the
> >> time for this.
> > I didnt check for bisectability but i did order the patches to avoid it.
> >>
> >> In general I am tempted to merge this still in this cycle, since we don't
> >> have other big changes, but we would need to settle this by early next
> >> week then.
> >>
> >> See below for more work ;-) (Sorry!)
> > Its okay.
> >>
> >>> From: Icenowy Zheng <icenowy at aosc.io>
> >>>
> >>> Add support for the suniv architecture, which is newer ARM9 SoCs by
> >>> Allwinner. The design of it seems to be a mixture of sun3i, sun4i and
> >>> sun6i.
> >>>
> >>> Signed-off-by: Icenowy Zheng <icenowy at aosc.io>
> >>> Signed-off-by: Jesse Taube <Mr.Bossman075 at gmail.com>
> >>> ---
> >>> arch/arm/mach-sunxi/Kconfig | 16 +++++++++--
> >>> arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++--
> >>> arch/arm/mach-sunxi/clock.c | 3 +-
> >>> arch/arm/mach-sunxi/clock_sun6i.c | 46 ++++++++++++++++++++++++++++++-
> >>> arch/arm/mach-sunxi/cpu_info.c | 2 ++
> >>> 5 files changed, 91 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> >>> index 2c18cf02d1..9bb7717731 100644
> >>> --- a/arch/arm/mach-sunxi/Kconfig
> >>> +++ b/arch/arm/mach-sunxi/Kconfig
> >>> @@ -1,7 +1,8 @@
> >>> if ARCH_SUNXI
> >>>
> >>> config SPL_LDSCRIPT
> >>> - default "arch/arm/cpu/armv7/sunxi/u-boot-spl.lds" if !ARM64
> >>> + default "arch/arm/cpu/arm926ejs/sunxi/u-boot-spl.lds" if MACH_SUNIV
> >>> + default "arch/arm/cpu/armv7/sunxi/u-boot-spl.lds" if !ARM64 && !MACH_SUNIV
> >>>
> >>> config IDENT_STRING
> >>> default " Allwinner Technology"
> >>> @@ -183,6 +184,12 @@ choice
> >>> prompt "Sunxi SoC Variant"
> >>> optional
> >>>
> >>> +config MACH_SUNIV
> >>> + bool "suniv (Allwinner F1C100s/F1C200s/F1C600/R6)"
> >>> + select CPU_ARM926EJS
> >>> + select SUNXI_GEN_SUN6I
> >>> + select SUPPORT_SPL
> >>> +
> >>> config MACH_SUN4I
> >>> bool "sun4i (Allwinner A10)"
> >>> select CPU_V7A
> >>> @@ -587,6 +594,7 @@ config DRAM_ODT_CORRECTION
> >>> endif
> >>>
> >>> config SYS_CLK_FREQ
> >>> + default 408000000 if MACH_SUNIV
> >>> default 1008000000 if MACH_SUN4I
> >>> default 1008000000 if MACH_SUN5I
> >>> default 1008000000 if MACH_SUN6I
> >>> @@ -598,6 +606,7 @@ config SYS_CLK_FREQ
> >>> default 1008000000 if MACH_SUN50I_H616
> >>>
> >>> config SYS_CONFIG_NAME
> >>> + default "suniv" if MACH_SUNIV
> >>> default "sun4i" if MACH_SUN4I
> >>> default "sun5i" if MACH_SUN5I
> >>> default "sun6i" if MACH_SUN6I
> >>> @@ -805,7 +814,7 @@ config VIDEO_SUNXI
> >>>
> >>> config VIDEO_HDMI
> >>> bool "HDMI output support"
> >>> - depends on VIDEO_SUNXI && !MACH_SUN8I
> >>> + depends on VIDEO_SUNXI && !MACH_SUN8I && !MACH_SUNIV
> >>> default y
> >>> ---help---
> >>> Say Y here to add support for outputting video over HDMI.
> >>> @@ -1005,6 +1014,7 @@ config GMAC_TX_DELAY
> >>> Set the GMAC Transmit Clock Delay Chain value.
> >>>
> >>> config SPL_STACK_R_ADDR
> >>> + default 0x81e00000 if MACH_SUNIV
> >>> default 0x4fe00000 if MACH_SUN4I
> >>> default 0x4fe00000 if MACH_SUN5I
> >>> default 0x4fe00000 if MACH_SUN6I
> >>> @@ -1016,7 +1026,7 @@ config SPL_STACK_R_ADDR
> >>>
> >>> config SPL_SPI_SUNXI
> >>> bool "Support for SPI Flash on Allwinner SoCs in SPL"
> >>> - depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5 || MACH_SUN50I || MACH_SUN8I_R40 || MACH_SUN50I_H6
> >>> + depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5 || MACH_SUN50I || MACH_SUN8I_R40 || MACH_SUN50I_H6 || MACH_SUNIV
> >>
> >> I think this is premature without the corresponding patch to
> >> spl_spi_sunxi.c.
> > Ill look into this.
> >>
> >>> help
> >>> Enable support for SPI Flash. This option allows SPL to read from
> >>> sunxi SPI Flash. It uses the same method as the boot ROM, so does
> >>> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> >>> index 3ef179742c..2fee86b49b 100644
> >>> --- a/arch/arm/mach-sunxi/board.c
> >>> +++ b/arch/arm/mach-sunxi/board.c
> >>> @@ -86,7 +86,8 @@ static int gpio_init(void)
> >>> sunxi_gpio_set_cfgpin(SUNXI_GPB(22), SUNXI_GPIO_INPUT);
> >>> sunxi_gpio_set_cfgpin(SUNXI_GPB(23), SUNXI_GPIO_INPUT);
> >>> #endif
> >>> -#if defined(CONFIG_MACH_SUN8I) && !defined(CONFIG_MACH_SUN8I_R40)
> >>> +#if (defined(CONFIG_MACH_SUN8I) && !defined(CONFIG_MACH_SUN8I_R40)) || \
> >>> + defined(CONFIG_MACH_SUNIV)
> >>> sunxi_gpio_set_cfgpin(SUNXI_GPF(2), SUN8I_GPF_UART0);
> >>> sunxi_gpio_set_cfgpin(SUNXI_GPF(4), SUN8I_GPF_UART0);
> >>> #else
> >>> @@ -94,6 +95,10 @@ static int gpio_init(void)
> >>> sunxi_gpio_set_cfgpin(SUNXI_GPF(4), SUNXI_GPF_UART0);
> >>> #endif
> >>> sunxi_gpio_set_pull(SUNXI_GPF(4), 1);
> >>> +#elif CONFIG_CONS_INDEX == 1 && defined(CONFIG_MACH_SUNIV)
> >>> + sunxi_gpio_set_cfgpin(SUNXI_GPE(0), SUNIV_GPE_UART0);
> >>> + sunxi_gpio_set_cfgpin(SUNXI_GPE(1), SUNIV_GPE_UART0);
> >>> + sunxi_gpio_set_pull(SUNXI_GPE(1), SUNXI_GPIO_PULL_UP);
> >>> #elif CONFIG_CONS_INDEX == 1 && (defined(CONFIG_MACH_SUN4I) || \
> >>> defined(CONFIG_MACH_SUN7I) || \
> >>> defined(CONFIG_MACH_SUN8I_R40))
> >>> @@ -219,7 +224,8 @@ void s_init(void)
> >>> /* No H3 BSP, boot0 seems to not modify SUNXI_SRAMC_BASE + 0x44 */
> >>> #endif
> >>>
> >>> -#if !defined(CONFIG_ARM_CORTEX_CPU_IS_UP) && !defined(CONFIG_ARM64)
> >>> +#if !defined(CONFIG_ARM_CORTEX_CPU_IS_UP) && !defined(CONFIG_ARM64) && \
> >>> + !defined(CONFIG_MACH_SUNIV)
> >>
> >> That looks correct for now, but should become obsolete with my
> >> lowlevel_init cleanup series.
> > Yes in V2 i fix this line, but it breaks compiling without your patch.
> > Idk if you want a bad compile or a bad merge.
> >>
> >>> /* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
> >>> asm volatile(
> >>> "mrc p15, 0, r0, c1, c0, 1\n"
> >>> @@ -328,10 +334,31 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc,
> >>> return sector;
> >>> }
> >>>
> >>> +#ifndef CONFIG_MACH_SUNIV
> >>
> >> Can you please flip this around to avoid the negative logic?
> > Ah yes i will.
> >>
> >>> u32 spl_boot_device(void)
> >>> {
> >>> return sunxi_get_boot_device();
> >>> }
> >>> +#else
> >>> +/*
> >>> + * suniv BROM do not pass the boot media type to SPL, so we try with the
> >>> + * boot sequence in BROM: mmc0->spinor->fail.
> >>> + */
> >>> +void board_boot_order(u32 *spl_boot_list)
> >>> +{
> >>> + /*
> >>> + * See the comments above in sunxi_get_boot_device() for information
> >>> + * about FEL boot.
> >>> + */
> >>> + if (!is_boot0_magic(SPL_ADDR + 4)) {
> >>> + spl_boot_list[0] = BOOT_DEVICE_BOARD;
> >>> + return;
> >>> + }
> >>> +
> >>> + spl_boot_list[0] = BOOT_DEVICE_MMC1;
> >>
> >> So does that mean that it tries MMC first, even when booted via SPI? So if
> >> there is a *non*-bootable microSD card in, it will read something from
> >> sector 80, and will execute that if this is a FIT or legacy image?
> > yes
> Uh sorry to bother you again but I cant seem to find a way to find where
> the bootrom got the spl. I could check other periphirals like pinmux. I
> could also just have it configured at build. Are both these options
> okay? I will try to find a way to find the boot device at runtime first.
Don't bother for this version, it's fine as it is now, we can refine
this later. It's only a problem if there is a non-valid SPL, but a
valid U-Boot proper legacy image on the SD card.
I don't want to have a build time option, we try to keep a single image
for all boot sources.
So eventually I'd prefer the pinmux/clock check, since that's cheaper.
The alternative would be to read the SPL (again), check for a valid
header and verify the checksum. You can look at this for inspiration:
https://patchwork.ozlabs.org/project/uboot/patch/20210712100651.6912-3-andre.przywara@arm.com/
> Also in my next patch i added spi boot for the spl. I also have a patch
> for DT spi driver, but I didn't add it to v3.
Please stay with the number and scope of the patches as we have it at
the moment. You can send the SPL SPI and DT SPI as follow-up patches,
and we can take them later.
For now I want to get the basic support in, even if that means FEL
only. Otherwise it becomes an issue of herding cats, with constant
rebase burden.
Cheers,
Andre
> thank you,
> Jesse Taube
> >>
> >> I wonder if we actually have some indication of SPI booting, for instance
> >> the pinmux or clock settings?
> >> Otherwise we would really need to mimic the BROM, and read and verify the
> >> eGON header again, to be reliable.
> > HMM this should be fixed thx for pointing this out.
> >>
> >> I might be talked into ignoring this issue for now, if there will be a fix
> >> patch later on.
> >>
> >>> + spl_boot_list[1] = BOOT_DEVICE_SPI;
> >>> +}
> >>> +#endif
> >>>
> >>> void board_init_f(ulong dummy)
> >>> {
> >>> diff --git a/arch/arm/mach-sunxi/clock.c b/arch/arm/mach-sunxi/clock.c
> >>> index de7e875298..da3a0eb058 100644
> >>> --- a/arch/arm/mach-sunxi/clock.c
> >>> +++ b/arch/arm/mach-sunxi/clock.c
> >>> @@ -35,7 +35,8 @@ int clock_init(void)
> >>> }
> >>>
> >>> /* These functions are shared between various SoCs so put them here. */
> >>> -#if defined CONFIG_SUNXI_GEN_SUN6I && !defined CONFIG_MACH_SUN9I
> >>> +#if defined CONFIG_SUNXI_GEN_SUN6I && !defined CONFIG_MACH_SUN9I && \
> >>> + !defined CONFIG_MACH_SUNIV
> >>> int clock_twi_onoff(int port, int state)
> >>> {
> >>> struct sunxi_ccm_reg *const ccm =
> >>> diff --git a/arch/arm/mach-sunxi/clock_sun6i.c b/arch/arm/mach-sunxi/clock_sun6i.c
> >>> index 8e84062bd7..b0b3ea4d30 100644
> >>> --- a/arch/arm/mach-sunxi/clock_sun6i.c
> >>> +++ b/arch/arm/mach-sunxi/clock_sun6i.c
> >>> @@ -23,7 +23,8 @@ void clock_init_safe(void)
> >>> struct sunxi_ccm_reg * const ccm =
> >>> (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> >>>
> >>> -#if !defined(CONFIG_MACH_SUNXI_H3_H5) && !defined(CONFIG_MACH_SUN50I)
> >>> +#if !defined(CONFIG_MACH_SUNXI_H3_H5) && !defined(CONFIG_MACH_SUN50I) && \
> >>> + !defined(CONFIG_MACH_SUNIV)
> >>> struct sunxi_prcm_reg * const prcm =
> >>> (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
> >>>
> >>> @@ -49,9 +50,11 @@ void clock_init_safe(void)
> >>>
> >>> writel(AHB1_ABP1_DIV_DEFAULT, &ccm->ahb1_apb1_div);
> >>>
> >>> +#ifndef CONFIG_MACH_SUNIV
> >>> writel(MBUS_CLK_DEFAULT, &ccm->mbus0_clk_cfg);
> >>> if (IS_ENABLED(CONFIG_MACH_SUN6I))
> >>> writel(MBUS_CLK_DEFAULT, &ccm->mbus1_clk_cfg);
> >>> +#endif
> >>>
> >>> #if defined(CONFIG_MACH_SUN8I_R40) && defined(CONFIG_SUNXI_AHCI)
> >>> setbits_le32(&ccm->sata_pll_cfg, CCM_SATA_PLL_DEFAULT);
> >>> @@ -87,6 +90,7 @@ void clock_init_uart(void)
> >>> struct sunxi_ccm_reg *const ccm =
> >>> (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> >>>
> >>> +#ifndef CONFIG_MACH_SUNIV
> >>
> >> Please again negate and swap the branches.
> >>
> >>> /* uart clock source is apb2 */
> >>> writel(APB2_CLK_SRC_OSC24M|
> >>> APB2_CLK_RATE_N_1|
> >>> @@ -102,6 +106,24 @@ void clock_init_uart(void)
> >>> setbits_le32(&ccm->apb2_reset_cfg,
> >>> 1 << (APB2_RESET_UART_SHIFT +
> >>> CONFIG_CONS_INDEX - 1));
> >>> +#else
> >>> + /* suniv doesn't have apb2, so uart clock source is apb1 */
> >>> + writel(PLL6_CFG_DEFAULT, &ccm->pll6_cfg);
> >>> + while (!(readl(&ccm->pll6_cfg) & CCM_PLL6_CTRL_LOCK))
> >>> + ;
> >>> +
> >>> + writel(AHB1_ABP1_DIV_DEFAULT, &ccm->ahb1_apb1_div);
> >>
> >> This is done is clock_init_safe() already, which is called before
> >> clock_init_uart(). I feel we should drop it here, not only because
> >> touching a PLL again might cause problems.
> > Okay.
> >>
> >>> +
> >>> + /* open the clock for uart */
> >>> + setbits_le32(&ccm->apb1_gate,
> >>> + CLK_GATE_OPEN << (APB1_GATE_UART_SHIFT +
> >>> + CONFIG_CONS_INDEX - 1));
> >>> +
> >>> + /* deassert uart reset */
> >>> + setbits_le32(&ccm->apb1_reset_cfg,
> >>> + 1 << (APB1_RESET_UART_SHIFT +
> >>> + CONFIG_CONS_INDEX - 1));
> >>> +#endif
> >>> #else
> >>> /* enable R_PIO and R_UART clocks, and de-assert resets */
> >>> prcm_apb0_enable(PRCM_APB0_GATE_PIO | PRCM_APB0_GATE_UART);
> >>> @@ -125,10 +147,15 @@ void clock_set_pll1(unsigned int clk)
> >>> }
> >>>
> >>> /* Switch to 24MHz clock while changing PLL1 */
> >>> +#ifndef CONFIG_MACH_SUNIV
> >>
> >> Same negation here. And can you please turn those preprocessor guards into
> >> if (IS_ENABLED(CONFIG_MACH_SUNIV)) ...
> >> where applicable? That's always preferred, if all symbols used inside both
> >> branches are defined in either case, as in here, for instance.
> >> As the compiler will see that it's a compile-time constant, it will remove
> >> the non-applicable branch. So the effect on the code is mostly the same,
> >> but both branches are "compile-tested" and it's more readable, as the
> >> nesting is done properly.
> >>
> >>> writel(AXI_DIV_3 << AXI_DIV_SHIFT |
> >>> ATB_DIV_2 << ATB_DIV_SHIFT |
> >>> CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
> >>> &ccm->cpu_axi_cfg);
> >>> +#else
> >>> + writel(CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
> >>> + &ccm->cpu_axi_cfg);
> >>> +#endif
> >>>
> >>> /*
> >>> * sun6i: PLL1 rate = ((24000000 * n * k) >> 0) / m (p is ignored)
> >>> @@ -137,13 +164,26 @@ void clock_set_pll1(unsigned int clk)
> >>> writel(CCM_PLL1_CTRL_EN | CCM_PLL1_CTRL_P(p) |
> >>> CCM_PLL1_CTRL_N(clk / (24000000 * k / m)) |
> >>> CCM_PLL1_CTRL_K(k) | CCM_PLL1_CTRL_M(m), &ccm->pll1_cfg);
> >>> +#ifndef CONFIG_MACH_SUNIV
> >>
> >> Guess ...
> > Sorry i didnt think it would be too bit a problem.
> >>
> >>> sdelay(200);
> >>> +#else
> >>> + /* ARM926EJ-S code does not have sdelay */
> >>
> >> I wonder if we should just copy a definition of it to
> >> arch/arm/cpu/arm926ejs. The ARMv7 assembly should just work.
> > I will try. if it doesnt work i will add.
> >>> + volatile int i = 200;
> >>> +
> >>> + while (i > 0)
> >>> + i--;
> >>> +#endif
> >>>
> >>> /* Switch CPU to PLL1 */
> >>> +#ifndef CONFIG_MACH_SUNIV
> >>
> >> Negate and if(IS_ENABLED(...)), please.
> >>
> >>> writel(AXI_DIV_3 << AXI_DIV_SHIFT |
> >>> ATB_DIV_2 << ATB_DIV_SHIFT |
> >>> CPU_CLK_SRC_PLL1 << CPU_CLK_SRC_SHIFT,
> >>> &ccm->cpu_axi_cfg);
> >>> +#else
> >>> + writel(CPU_CLK_SRC_PLL1 << CPU_CLK_SRC_SHIFT,
> >>> + &ccm->cpu_axi_cfg);
> >>> +#endif
> >>> }
> >>> #endif
> >>>
> >>> @@ -317,7 +357,11 @@ unsigned int clock_get_pll6(void)
> >>> uint32_t rval = readl(&ccm->pll6_cfg);
> >>> int n = ((rval & CCM_PLL6_CTRL_N_MASK) >> CCM_PLL6_CTRL_N_SHIFT) + 1;
> >>> int k = ((rval & CCM_PLL6_CTRL_K_MASK) >> CCM_PLL6_CTRL_K_SHIFT) + 1;
> >>> +#ifndef CONFIG_MACH_SUNIV
> >>
> >> Negate and if(IS_ENABLED(...)), please.
> >>
> >>> return 24000000 * n * k / 2;
> >>> +#else
> >>> + return 24000000 * n * k;
> >>> +#endif
> >>> }
> >>
> >> This whole file is admittedly quite messy. We should be able to move the
> >> video clocks out here and into our DM clock driver, but this is
> >> non-trivial, I believe, so I won't ask you doing this ;-)
> > Ah yes I was changing all the added ifdefs to ifs then I noticed that
> > there where alreay ifdefs everywhere so i didnt change it. I will
> > continue with it.
> >>
> >> Cheers,
> >> Andre
> >>
> >>>
> >>> unsigned int clock_get_mipi_pll(void)
> >>> diff --git a/arch/arm/mach-sunxi/cpu_info.c b/arch/arm/mach-sunxi/cpu_info.c
> >>> index ba33ef2430..7eef178859 100644
> >>> --- a/arch/arm/mach-sunxi/cpu_info.c
> >>> +++ b/arch/arm/mach-sunxi/cpu_info.c
> >>> @@ -57,6 +57,8 @@ int print_cpuinfo(void)
> >>> {
> >>> #ifdef CONFIG_MACH_SUN4I
> >>> puts("CPU: Allwinner A10 (SUN4I)\n");
> >>> +#elif defined CONFIG_MACH_SUNIV
> >>> + puts("CPU: Allwinner F Series (SUNIV)\n");
> >>> #elif defined CONFIG_MACH_SUN5I
> >>> u32 val = readl(SUNXI_SID_BASE + 0x08);
> >>> switch ((val >> 12) & 0xf) {
> >>
More information about the U-Boot
mailing list