On 03/01/2021 23:43, Samuel Holland wrote:

Hi Jernej,

thanks for that patch, that's a nice solution to avoid those long #ifdef
chains!

> On 1/3/21 3:26 AM, Jernej Skrabec wrote:
>> It turns out that there are at least 2 other SoCs which have basically
>> the same memory map, similar clocks and other features as H6. It's very
>> likely that we'll see more such SoCs in the future. In order to ease
>> porting to new SoCs and lower ifdef clutter, introduce common symbol for
>> them.
>>
>> Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
>> ---
>>  arch/arm/include/asm/arch-sunxi/boot0.h |  2 +-
>>  arch/arm/include/asm/arch-sunxi/clock.h |  2 +-
>>  arch/arm/include/asm/arch-sunxi/cpu.h   |  2 +-
>>  arch/arm/include/asm/arch-sunxi/timer.h |  2 +-
>>  arch/arm/mach-sunxi/Kconfig             | 21 +++++++++++++--------
>>  arch/arm/mach-sunxi/Makefile            |  2 +-
>>  arch/arm/mach-sunxi/board.c             |  4 ++--
>>  arch/arm/mach-sunxi/rmr_switch.S        |  2 +-
>>  common/spl/Kconfig                      |  4 ++--
>>  include/configs/sun50i.h                |  2 +-
>>  10 files changed, 24 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/boot0.h 
>> b/arch/arm/include/asm/arch-sunxi/boot0.h
>> index 46d0f0666c2b..e8e8e38f0556 100644
>> --- a/arch/arm/include/asm/arch-sunxi/boot0.h
>> +++ b/arch/arm/include/asm/arch-sunxi/boot0.h
>> @@ -39,7 +39,7 @@
>>      .word   0xf57ff06f      // isb     sy
>>      .word   0xe320f003      // wfi
>>      .word   0xeafffffd      // b       @wfi
>> -#ifndef CONFIG_MACH_SUN50I_H6
>> +#ifndef CONFIG_SUN50I_GEN_H6
>>      .word   0x017000a0      // writeable RVBAR mapping address
>>  #else
>>      .word   0x09010040      // writeable RVBAR mapping address
>> diff --git a/arch/arm/include/asm/arch-sunxi/clock.h 
>> b/arch/arm/include/asm/arch-sunxi/clock.h
>> index 5994130e6b54..cbbe5c7a1e68 100644
>> --- a/arch/arm/include/asm/arch-sunxi/clock.h
>> +++ b/arch/arm/include/asm/arch-sunxi/clock.h
>> @@ -16,7 +16,7 @@
>>  /* clock control module regs definition */
>>  #if defined(CONFIG_MACH_SUN8I_A83T)
>>  #include <asm/arch/clock_sun8i_a83t.h>
>> -#elif defined(CONFIG_MACH_SUN50I_H6)
>> +#elif defined(CONFIG_SUN50I_GEN_H6)
>>  #include <asm/arch/clock_sun50i_h6.h>
>>  #elif defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN8I) || \
>>        defined(CONFIG_MACH_SUN50I)
>> diff --git a/arch/arm/include/asm/arch-sunxi/cpu.h 
>> b/arch/arm/include/asm/arch-sunxi/cpu.h
>> index 8b57d24e2f0c..b08f2023748c 100644
>> --- a/arch/arm/include/asm/arch-sunxi/cpu.h
>> +++ b/arch/arm/include/asm/arch-sunxi/cpu.h
>> @@ -8,7 +8,7 @@
>>  
>>  #if defined(CONFIG_MACH_SUN9I)
>>  #include <asm/arch/cpu_sun9i.h>
>> -#elif defined(CONFIG_MACH_SUN50I_H6)
>> +#elif defined(CONFIG_SUN50I_GEN_H6)
>>  #include <asm/arch/cpu_sun50i_h6.h>
>>  #else
>>  #include <asm/arch/cpu_sun4i.h>
>> diff --git a/arch/arm/include/asm/arch-sunxi/timer.h 
>> b/arch/arm/include/asm/arch-sunxi/timer.h
>> index 6f138d04b806..bb5626d893bb 100644
>> --- a/arch/arm/include/asm/arch-sunxi/timer.h
>> +++ b/arch/arm/include/asm/arch-sunxi/timer.h
>> @@ -76,7 +76,7 @@ struct sunxi_timer_reg {
>>      struct sunxi_tgp tgp[4];
>>      u8 res5[8];
>>      u32 cpu_cfg;
>> -#elif defined(CONFIG_SUNXI_GEN_SUN6I) || defined(CONFIG_MACH_SUN50I_H6)
>> +#elif defined(CONFIG_SUNXI_GEN_SUN6I) || defined(CONFIG_SUN50I_GEN_H6)
>>      u8 res3[16];
>>      struct sunxi_wdog wdog[5];      /* We have 5 watchdogs */
>>  #endif
>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
>> index 49ef217f08c0..1cf79bad7cf6 100644
>> --- a/arch/arm/mach-sunxi/Kconfig
>> +++ b/arch/arm/mach-sunxi/Kconfig
>> @@ -82,7 +82,7 @@ config SUN8I_RSB
>>  config SUNXI_SRAM_ADDRESS
>>      hex
>>      default 0x10000 if MACH_SUN9I || MACH_SUN50I || MACH_SUN50I_H5
>> -    default 0x20000 if MACH_SUN50I_H6
>> +    default 0x20000 if SUN50I_GEN_H6
>>      default 0x0
>>      ---help---
>>      Older Allwinner SoCs have their mask boot ROM mapped just below 4GB,
>> @@ -108,6 +108,15 @@ config SUNXI_GEN_SUN6I
>>      separate ahb reset control registers, custom pmic bus, new style
>>      watchdog, etc.
>>  
>> +config SUN50I_GEN_H6
> 
> The new memory map is also used for 32-bit SoCs V5 (sun8iw12p1) and
> newer. So this is not sun50i-specific, and I'd suggest SUNXI_GEN_H6 (or
> even SUNXI_GEN_H6_V5). It is unfortunate there appears to be no name for
> this family.

Yeah, the Allwinner generation naming being *core* dependent is really
annyoing and not helpful.
I am fine with a rename, but then it would be very close to
"CONFIG_SUNXI_GEN_SUN6I", which is quite confusing to any readers
(already stumbled upon it myself in patch 04/17).

Any opinions?

> 
>> +    bool
>> +    select FIT
>> +    select SPL_LOAD_FIT
>> +    select SUPPORT_SPL
>> +    ---help---
>> +    Select this for sunxi SoCs which have H6 like peripherals, clocks
>> +    and memory map.
>> +
>>  config SUNXI_DRAM_DW
>>      bool
>>      ---help---
>> @@ -302,10 +311,7 @@ config MACH_SUN50I_H5
>>  config MACH_SUN50I_H6
>>      bool "sun50i (Allwinner H6)"
>>      select ARM64
>> -    select SUPPORT_SPL
>> -    select FIT
>>      select PHY_SUN4I_USB
>> -    select SPL_LOAD_FIT
>>      select DRAM_SUN50I_H6
> 
> I would expect this to select SUN50I_GEN_H6.

Indeed, that breaks H6 builds.

>>  endchoice
>> @@ -540,7 +546,6 @@ config SYS_CLK_FREQ
>>      default 816000000 if MACH_SUN50I || MACH_SUN50I_H5
>>      default 1008000000 if MACH_SUN8I
>>      default 1008000000 if MACH_SUN9I
>> -    default 888000000 if MACH_SUN50I_H6
> 
> Was this removal intentional? You later add a different value for H616 only.
> 
>>  config SYS_CONFIG_NAME
>>      default "sun4i" if MACH_SUN4I
>> @@ -756,7 +761,7 @@ config VIDEO_SUNXI
>>      depends on !MACH_SUN8I_V3S
>>      depends on !MACH_SUN9I
>>      depends on !MACH_SUN50I
>> -    depends on !MACH_SUN50I_H6
>> +    depends on !SUN50I_GEN_H6
>>      select VIDEO
>>      imply VIDEO_DT_SIMPLEFB
>>      default y
>> @@ -989,11 +994,11 @@ config SPL_STACK_R_ADDR
>>      default 0x4fe00000 if MACH_SUN8I
>>      default 0x2fe00000 if MACH_SUN9I
>>      default 0x4fe00000 if MACH_SUN50I
>> -    default 0x4fe00000 if MACH_SUN50I_H6
>> +    default 0x4fe00000 if SUN50I_GEN_H6
>>  
>>  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 || SUN50I_GEN_H6

I know I suggested this, but it was a bit premature. The SPL SPI code
needs some adjustments first. I have a patch, will send it ASAP. But for
now you can just drop this change.

Thanks!
Andre


>>      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/Makefile b/arch/arm/mach-sunxi/Makefile
>> index d129f334798b..b8aca43d6630 100644
>> --- a/arch/arm/mach-sunxi/Makefile
>> +++ b/arch/arm/mach-sunxi/Makefile
>> @@ -26,7 +26,7 @@ else
>>  obj-$(CONFIG_MACH_SUN8I)    += clock_sun6i.o
>>  endif
>>  obj-$(CONFIG_MACH_SUN9I)    += clock_sun9i.o gtbus_sun9i.o
>> -obj-$(CONFIG_MACH_SUN50I_H6)        += clock_sun50i_h6.o
>> +obj-$(CONFIG_SUN50I_GEN_H6) += clock_sun50i_h6.o
>>  
>>  ifdef CONFIG_SPL_BUILD
>>  obj-$(CONFIG_DRAM_SUN4I)    += dram_sun4i.o
>> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
>> index f40fccd8f8b9..7a8b303f233c 100644
>> --- a/arch/arm/mach-sunxi/board.c
>> +++ b/arch/arm/mach-sunxi/board.c
>> @@ -144,7 +144,7 @@ static int gpio_init(void)
>>  #error Unsupported console port number. Please fix pin mux settings in 
>> board.c
>>  #endif
>>  
>> -#ifdef CONFIG_MACH_SUN50I_H6
>> +#ifdef CONFIG_SUN50I_GEN_H6
>>      /* Update PIO power bias configuration by copy hardware detected value 
>> */
>>      val = readl(SUNXI_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_VAL);
>>      writel(val, SUNXI_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_SEL);
>> @@ -329,7 +329,7 @@ void reset_cpu(ulong addr)
>>              /* sun5i sometimes gets stuck without this */
>>              writel(WDT_MODE_RESET_EN | WDT_MODE_EN, &wdog->mode);
>>      }
>> -#elif defined(CONFIG_SUNXI_GEN_SUN6I) || defined(CONFIG_MACH_SUN50I_H6)
>> +#elif defined(CONFIG_SUNXI_GEN_SUN6I) || defined(CONFIG_SUN50I_GEN_H6)
>>  #if defined(CONFIG_MACH_SUN50I_H6)
>>      /* WDOG is broken for some H6 rev. use the R_WDOG instead */
>>      static const struct sunxi_wdog *wdog =
>> diff --git a/arch/arm/mach-sunxi/rmr_switch.S 
>> b/arch/arm/mach-sunxi/rmr_switch.S
>> index fafd306f95b1..33e55d496865 100644
>> --- a/arch/arm/mach-sunxi/rmr_switch.S
>> +++ b/arch/arm/mach-sunxi/rmr_switch.S
>> @@ -30,7 +30,7 @@
>>  
>>  .text
>>  
>> -#ifndef CONFIG_MACH_SUN50I_H6
>> +#ifndef CONFIG_SUN50I_GEN_H6
>>      ldr     r1, =0x017000a0         @ MMIO mapped RVBAR[0] register
>>  #else
>>      ldr     r1, =0x09010040         @ MMIO mapped RVBAR[0] register
>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
>> index d8086bd9e874..bed715774d81 100644
>> --- a/common/spl/Kconfig
>> +++ b/common/spl/Kconfig
>> @@ -150,7 +150,7 @@ config SPL_TEXT_BASE
>>      hex "SPL Text Base"
>>      default ISW_ENTRY_ADDR if AM43XX || AM33XX || OMAP54XX || ARCH_KEYSTONE
>>      default 0x10060 if MACH_SUN50I || MACH_SUN50I_H5 || MACH_SUN9I
>> -    default 0x20060 if MACH_SUN50I_H6
>> +    default 0x20060 if SUN50I_GEN_H6
>>      default 0x00060 if ARCH_SUNXI
>>      default 0xfffc0000 if ARCH_ZYNQMP
>>      default 0x0
>> @@ -459,7 +459,7 @@ config SPL_SHA512_SUPPORT
>>  config SPL_FIT_IMAGE_TINY
>>      bool "Remove functionality from SPL FIT loading to reduce size"
>>      depends on SPL_FIT
>> -    default y if MACH_SUN50I || MACH_SUN50I_H5 || MACH_SUN50I_H6
>> +    default y if MACH_SUN50I || MACH_SUN50I_H5 || SUN50I_GEN_H6
> 
> This can probably be simplified to ARCH_SUNXI.
> 
>>      default y if ARCH_IMX8M
>>      help
>>        Enable this to reduce the size of the FIT image loading code
>> diff --git a/include/configs/sun50i.h b/include/configs/sun50i.h
>> index e050a5299f3b..bc2e3a3d0088 100644
>> --- a/include/configs/sun50i.h
>> +++ b/include/configs/sun50i.h
>> @@ -10,7 +10,7 @@
>>   * A64 specific configuration
>>   */
>>  
>> -#ifndef CONFIG_MACH_SUN50I_H6
>> +#ifndef CONFIG_SUN50I_GEN_H6
>>  #define GICD_BASE           0x1c81000
>>  #define GICC_BASE           0x1c82000
>>  #else
>>
> 
> Cheers,
> Samuel
> 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/98419fe6-0889-8940-091e-cf7a6a7bf5ad%40arm.com.

Reply via email to