[U-Boot] [PATCH v2 03/10] sunxi: Introduce a hidden SUNXI_GEN_SUNxI Kconfig bool

[Hans de Goede]

Hi,

On 04-05-15 11:31, Ian Campbell wrote:
> On Mon, 2015-05-04 at 11:04 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 02-05-15 15:31, Ian Campbell wrote:
>>> On Sun, 2015-04-26 at 17:21 +0200, Hans de Goede wrote:
>>>> sun6i and newer (derived) SoCs such as the sun8i-a23, sun8i-a33 and sun9i
>>>> have a various things in common, like having separate ahb reset control
>>>> registers, the SID living inside the pmic, custom pmic busses, new style
>>>> watchdog, etc.
>>>>
>>>> This commit introduces a new hidden SUNXI_GEN_SUN6I Kconfig bool which can be
>>>> used to check for these features avoiding the need for an ever growing list
>>>> of "#if defined CONFIG_MACH_SUN?I" conditionals as we add support for more
>>>> "new style" sunxi SoCs.
>>>>
>>>> For completeness this also adds a SUNXI_GEN_SUN4I bool for A10/A13/A20.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>>> ---
>>>> Changes in v2:
>>>> -Use SUNXI_GEN_SUN?I instead of ARCH_SUN6I
>>>> ---
>>>>    arch/arm/cpu/armv7/sunxi/board.c            | 18 +++++++++---------
>>>>    arch/arm/cpu/armv7/sunxi/usbc.c             |  4 ++--
>>>>    arch/arm/include/asm/arch-sunxi/cpu_sun4i.h | 12 ++++++------
>>>>    arch/arm/include/asm/arch-sunxi/mmc.h       |  3 +--
>>>>    arch/arm/include/asm/arch-sunxi/timer.h     |  8 ++++----
>>>>    board/sunxi/Kconfig                         | 21 +++++++++++++++++++++
>>>>    board/sunxi/gmac.c                          |  6 +++---
>>>>    drivers/mmc/sunxi_mmc.c                     |  3 +--
>>>>    drivers/video/sunxi_display.c               | 10 +++++-----
>>>>    9 files changed, 52 insertions(+), 33 deletions(-)
>>>>
>>>> diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
>>>> index f403742..afa55d6 100644
>>>> --- a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
>>>> +++ b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
>>>> @@ -37,7 +37,7 @@
>>>>    #define SUNXI_MMC1_BASE			0x01c10000
>>>>    #define SUNXI_MMC2_BASE			0x01c11000
>>>>    #define SUNXI_MMC3_BASE			0x01c12000
>>>> -#if !defined CONFIG_MACH_SUN6I && !defined CONFIG_MACH_SUN8I
>>>> +#ifndef CONFIG_SUNXI_GEN_SUN6I
>>>
>>> #ifdef CONFIG_SUNXI_GEN_SUN4I?
>>>
>>>
>>>> diff --git a/arch/arm/include/asm/arch-sunxi/mmc.h b/arch/arm/include/asm/arch-sunxi/mmc.h
>>>> index 74833b5..cb52e64 100644
>>>> --- a/arch/arm/include/asm/arch-sunxi/mmc.h
>>>> +++ b/arch/arm/include/asm/arch-sunxi/mmc.h
>>>> @@ -43,8 +43,7 @@ struct sunxi_mmc {
>>>>    	u32 chda;		/* 0x90 */
>>>>    	u32 cbda;		/* 0x94 */
>>>>    	u32 res1[26];
>>>> -#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN8I) || \
>>>> -    defined(CONFIG_MACH_SUN9I)
>>>
>>> Did you miss the select GEN_SUN6I from the SUN9I Kconfig entry?
>>>
>>> Oh, there isn't one, I suppose this was some vestige of another patch
>>> and in that case it's fine as it is (unless this patch is supposed to be
>>> on top of one which adds Kconfig SUN9I).
>>>
>>> Anyway, which ever way it is I trust you know what to do (even if that's
>>> nothing).
>>
>> Right, so there is a MACH_SUN9I is my sunxi-wip tree, but that is not ready
>> yet to go upstream. I still need to address your comments about dealing with
>> the different DRAM base address between sun9i and the rest in sunxi-common.h.
>> But I've been busy with other stuff / been distracted by shinier things.
>>
>> So the reason why there is no SUN9I related code in this patch is because the
>> SUN9I code is intended to get merged at some point in the future after this
>> patch-set (and it sits after this one in the history of my sunxi-wip branch).
>
> Great thanks.
>
>>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>>> index 88e3358..b716a8b 100644
>>>> --- a/board/sunxi/Kconfig
>>>> +++ b/board/sunxi/Kconfig
>>>> @@ -1,21 +1,40 @@
>>>>    if ARCH_SUNXI
>>>>
>>>> +# Note only one of these may be selected at a time! But hidden choices are
>>>> +# not supported by Kconfig
>>>
>>> Probably not worth it, but is "depends on !THE_OTHER_THING" a usable
>>> construct?
>>
>> Good idea, unfortunately it seems Kconfig does not like it:
>>
>> board/sunxi/Kconfig:12:error: recursive dependency detected!
>> board/sunxi/Kconfig:12: symbol SUNXI_GEN_SUN6I depends on SUNXI_GEN_SUN4I
>> board/sunxi/Kconfig:5:  symbol SUNXI_GEN_SUN4I depends on SUNXI_GEN_SUN6I
>
> Shame.
>
>>>> diff --git a/board/sunxi/gmac.c b/board/sunxi/gmac.c
>>>> index d90eed4..4e222d8 100644
>>>> --- a/board/sunxi/gmac.c
>>>> +++ b/board/sunxi/gmac.c
>>>> @@ -13,11 +13,11 @@ int sunxi_gmac_initialize(bd_t *bis)
>>>>    		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
>>>>
>>>>    	/* Set up clock gating */
>>>> -#ifndef CONFIG_MACH_SUN6I
>>>> -	setbits_le32(&ccm->ahb_gate1, 0x1 << AHB_GATE_OFFSET_GMAC);
>>>> -#else
>>>> +#ifdef CONFIG_SUNXI_GEN_SUN6I
>>>
>>> Is this one not changing the behaviour for SUN8I (and SUN9I perhaps).
>>> Previously they would have gotten the else case.
>>
>> sun8i chips (at least the a23 and a33) do not have an ethernet chip, and
>> the Makefile has:
>>
>> obj-$(CONFIG_SUNXI_GMAC)        += gmac.o
>>
>> So this is never build for them. If it was build then the sun6i code
>> would be the correct thing to do since sun8i does have separate ahb reset
>> registers like sun6i.
>>
>> For sun9i it is unclear / unknown atm what sort of ethernet mac ip it
>> has, but if it is the classic dw gmac, then it too would need the
>> sun6i case of the code.
>
> OK, slightly confusing to have such intertwined things, but I see the
> logic now.
>
> Would be useful to make explicit reference to this Ethernet case and the
> HDMI/video ones in the commit message, but in any case:
>
> Acked-by: Ian Campbell <ijc at hellion.org.uk>

Thanks, I'll add a mention of the behavior changes for hdmi and gmac
for sun8i/sun9i and how that does not matter as they are not used,
to the commit message.

Regards,

Hans