Hi Jesse, Andre, All, > Il giorno 29 gen 2022, alle ore 20:24, Jesse Taube <mr.bossman...@gmail.com> > ha scritto: > > > >> 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_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. > Ah the boot source is in R2 when save_boot_params is called. > I dont know how i would add it though.
save_boot_params is called at the very beginning: https://elixir.bootlin.com/u-boot/latest/source/arch/arm/cpu/armv7/start.S#L39 So maybe that is something left from BROM after running. Are the values present in R2 compatible to the ones used from sunxi? We could add a global variable in start.S to save the value and use it later in the code to know the media we’re booting from. Best regards —- Giulio Benetti Benetti Engineering sas >> 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, >> 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) { >>>>>