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

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

Reply via email to