[PATCH 09/11] sunxi: Add support for SUNIV architecture

Jesse Taube mr.bossman075 at gmail.com
Wed Jan 26 15:38:44 CET 2022



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