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
>
>
>
>
--
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/a953c610-0ba5-a390-be1c-b2532a98d884%40arm.com.
