On 1/29/22 06:51, Andre Przywara wrote:
On Fri, 28 Jan 2022 22:21:28 -0500 Jesse Taube <mr.bossman...@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.bossman...@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 <icen...@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 <icen...@aosc.io> Signed-off-by: Jesse Taube <mr.bossman...@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_SUNXIconfig 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_SUNIVconfig IDENT_STRINGdefault " 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 endifconfig 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_H616config 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_SUNXIconfig VIDEO_HDMIbool "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_ADDRconfig SPL_SPI_SUNXIbool "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_SUNIVI 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_SUNIVCan 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?yesUh 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.
Ah the boot source is in R2 when save_boot_params is called. I dont know how i would add it though.
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.przyw...@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, Andrethank you, Jesse TaubeI 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; +} +#endifvoid 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_SUNIVwritel(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_SUNIVPlease 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_SUNIVSame 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_SUNIVGuess ...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_SUNIVNegate 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_SUNIVNegate 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, Andreunsigned 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) {
