On 04/01/2021 18:28, Jernej Škrabec wrote: > Dne ponedeljek, 04. januar 2021 ob 11:35:41 CET je André Przywara napisal(a): >> 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? > > I would leave it as it is, it's common to name something, usually driver, but > in this case symbol, after first known device that implemented particular > feature.
Yes, but typically that's for drivers, and here this is specifying a group of SoCs, like CONFIG_SUNXI_GEN_SUN6I does (where we also use _SUNXI_). But this is really nitpicking, since there is no ABI, we can rename it easily later with a simple patch, should we agree on the right name for that baby. Cheers, Andre. > >> >>>> + 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. > > Ops, leftover after refactoring. > >> >>>> 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.> > > That's also an oversight after refactoring. > >>>> 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. > > Ok, > > Best regards, > Jernej > >> >> 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 > > > >