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

Hans de Goede hdegoede at redhat.com
Mon May 4 11:04:06 CEST 2015


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

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


>> 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.

>
> If this (and the following instances) is an intended bug fix then it
> should be mentioned in the commit message (likewise the SUN6I||SUN8I
> ones below which change things for SUN9I when switched)
>
>> diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c
>> index bff7afe..5073329 100644
>> --- a/drivers/video/sunxi_display.c
>> +++ b/drivers/video/sunxi_display.c
>> @@ -84,7 +84,7 @@ static int sunxi_hdmi_hpd_detect(int hpd_delay)
>>   			CCM_HDMI_CTRL_PLL3);
>>
>>   	/* Set ahb gating to pass */
>> -#ifdef CONFIG_MACH_SUN6I
>> +#ifdef CONFIG_SUNXI_GEN_SUN6I
>
> Another possible behaviour change for SUN8I and SUN9I.

This is in the hdmi code path, sun8i does not have hdmi, and sun9i
has a different hdmi ip which we do not support yet. If a sun8i
variant with sun4i / sun6i hdmi ip ever shows up then this code will
do the right thing, the CONFIG_SUNXI_GEN_SUN6I is about there being
a separate ahb reset register and that is exactly what this ifdef
catches.

>
>>   	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_HDMI);
>>   #endif
>>   	setbits_le32(&ccm->ahb_gate1, 1 << AHB_GATE_OFFSET_HDMI);
>> @@ -113,7 +113,7 @@ static void sunxi_hdmi_shutdown(void)
>>   	clrbits_le32(&hdmi->ctrl, SUNXI_HDMI_CTRL_ENABLE);
>>   	clrbits_le32(&ccm->hdmi_clk_cfg, CCM_HDMI_CTRL_GATE);
>>   	clrbits_le32(&ccm->ahb_gate1, 1 << AHB_GATE_OFFSET_HDMI);
>> -#ifdef CONFIG_MACH_SUN6I
>> +#ifdef CONFIG_SUNXI_GEN_SUN6I
>
> And again.

Same here.

>
>>   	clrbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_HDMI);
>>   #endif
>>   	clock_set_pll3(0);
>> @@ -404,7 +404,7 @@ static void sunxi_composer_init(void)
>>
>>   	sunxi_frontend_init();
>>
>> -#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I
>> +#ifdef CONFIG_SUNXI_GEN_SUN6I
>
> SUN9I changing?

sun9i has a whole new display pipeline, so no CONFIG_VIDEO for sun9i.

>
>>   	/* Reset off */
>>   	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_DE_BE0);
>>   #endif
>> @@ -549,7 +549,7 @@ static void sunxi_lcdc_init(void)
>>   		(struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE;
>>
>>   	/* Reset off */
>> -#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I
>> +#ifdef CONFIG_SUNXI_GEN_SUN6I
>
> SUN9I?

Same.

>
>>   	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_LCD0);
>>   #else
>>   	setbits_le32(&ccm->lcd0_ch0_clk_cfg, CCM_LCD_CH0_CTRL_RST);
>> @@ -942,7 +942,7 @@ static void sunxi_vga_enable(void)
>>
>>   static void sunxi_drc_init(void)
>>   {
>> -#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I
>> +#ifdef CONFIG_SUNXI_GEN_SUN6I
>
> SUN9I again.

Same.

Regards,

Hans


More information about the U-Boot mailing list