| Message ID | 1430061671-25112-1-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Hans de Goede |
| Headers | show |
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@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). > 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? > 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. 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. > 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. > 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? > /* 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? > 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. Ian.
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@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
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@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@hellion.org.uk>
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@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@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
diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index 6471c6b..cc6c68b 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -173,7 +173,15 @@ void board_init_f(ulong dummy) void reset_cpu(ulong addr) { -#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUN7I) +#ifdef CONFIG_SUNXI_GEN_SUN6I + static const struct sunxi_wdog *wdog = + ((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->wdog; + + /* Set the watchdog for its shortest interval (.5s) and wait */ + writel(WDT_CFG_RESET, &wdog->cfg); + writel(WDT_MODE_EN, &wdog->mode); + writel(WDT_CTRL_KEY | WDT_CTRL_RESTART, &wdog->ctl); +#else static const struct sunxi_wdog *wdog = &((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->wdog; @@ -185,14 +193,6 @@ void reset_cpu(ulong addr) /* sun5i sometimes gets stuck without this */ writel(WDT_MODE_RESET_EN | WDT_MODE_EN, &wdog->mode); } -#else /* CONFIG_MACH_SUN6I || CONFIG_MACH_SUN8I || .. */ - static const struct sunxi_wdog *wdog = - ((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->wdog; - - /* Set the watchdog for its shortest interval (.5s) and wait */ - writel(WDT_CFG_RESET, &wdog->cfg); - writel(WDT_MODE_EN, &wdog->mode); - writel(WDT_CTRL_KEY | WDT_CTRL_RESTART, &wdog->ctl); #endif } diff --git a/arch/arm/cpu/armv7/sunxi/usbc.c b/arch/arm/cpu/armv7/sunxi/usbc.c index 80e4fc9..7d55e41 100644 --- a/arch/arm/cpu/armv7/sunxi/usbc.c +++ b/arch/arm/cpu/armv7/sunxi/usbc.c @@ -218,7 +218,7 @@ void sunxi_usbc_enable(int index) setbits_le32(&ccm->usb_clk_cfg, sunxi_usbc->usb_rst_mask); setbits_le32(&ccm->ahb_gate0, sunxi_usbc->ahb_clk_mask); -#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I +#ifdef CONFIG_SUNXI_GEN_SUN6I setbits_le32(&ccm->ahb_reset0_cfg, sunxi_usbc->ahb_clk_mask); #endif @@ -238,7 +238,7 @@ void sunxi_usbc_disable(int index) if (sunxi_usbc->id != 0) sunxi_usb_passby(sunxi_usbc, !SUNXI_USB_PASSBY_EN); -#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I +#ifdef CONFIG_SUNXI_GEN_SUN6I clrbits_le32(&ccm->ahb_reset0_cfg, sunxi_usbc->ahb_clk_mask); #endif clrbits_le32(&ccm->ahb_gate0, sunxi_usbc->ahb_clk_mask); 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 #define SUNXI_USB0_BASE 0x01c13000 #define SUNXI_USB1_BASE 0x01c14000 #endif @@ -45,15 +45,15 @@ #define SUNXI_HDMI_BASE 0x01c16000 #define SUNXI_SPI2_BASE 0x01c17000 #define SUNXI_SATA_BASE 0x01c18000 -#if !defined CONFIG_MACH_SUN6I && !defined CONFIG_MACH_SUN8I +#ifdef CONFIG_SUNXI_GEN_SUN6I +#define SUNXI_USB0_BASE 0x01c19000 +#define SUNXI_USB1_BASE 0x01c1a000 +#define SUNXI_USB2_BASE 0x01c1b000 +#else #define SUNXI_PATA_BASE 0x01c19000 #define SUNXI_ACE_BASE 0x01c1a000 #define SUNXI_TVE1_BASE 0x01c1b000 #define SUNXI_USB2_BASE 0x01c1c000 -#else -#define SUNXI_USB0_BASE 0x01c19000 -#define SUNXI_USB1_BASE 0x01c1a000 -#define SUNXI_USB2_BASE 0x01c1b000 #endif #define SUNXI_CSI1_BASE 0x01c1d000 #define SUNXI_TZASC_BASE 0x01c1e000 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) +#ifdef CONFIG_SUNXI_GEN_SUN6I u32 res2[64]; #endif u32 fifo; /* 0x100 / 0x200 FIFO access address */ diff --git a/arch/arm/include/asm/arch-sunxi/timer.h b/arch/arm/include/asm/arch-sunxi/timer.h index 9a5e488..aa0c14b 100644 --- a/arch/arm/include/asm/arch-sunxi/timer.h +++ b/arch/arm/include/asm/arch-sunxi/timer.h @@ -67,7 +67,10 @@ struct sunxi_timer_reg { struct sunxi_timer timer[6]; /* We have 6 timers */ u8 res2[16]; struct sunxi_avs avs; -#if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUN7I) +#ifdef CONFIG_SUNXI_GEN_SUN6I + u8 res3[16]; + struct sunxi_wdog wdog[5]; /* We have 5 watchdogs */ +#else struct sunxi_wdog wdog; /* 0x90 */ /* XXX the following is not accurate for sun5i/sun7i */ struct sunxi_64cnt cnt64; /* 0xa0 */ @@ -77,9 +80,6 @@ struct sunxi_timer_reg { struct sunxi_tgp tgp[4]; u8 res5[8]; u32 cpu_cfg; -#else /* CONFIG_MACH_SUN6I || CONFIG_MACH_SUN8I || ... */ - u8 res3[16]; - struct sunxi_wdog wdog[5]; /* We have 5 watchdogs */ #endif }; 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 +config SUNXI_GEN_SUN4I + bool + ---help--- + Select this for sunxi SoCs which have resets and clocks set up + as the original A10 (mach-sun4i). + +config SUNXI_GEN_SUN6I + bool + ---help--- + Select this for sunxi SoCs which have sun6i like periphery, like + separate ahb reset control registers, custom pmic bus, new style + watchdog, etc. + + choice prompt "Sunxi SoC Variant" config MACH_SUN4I bool "sun4i (Allwinner A10)" select CPU_V7 + select SUNXI_GEN_SUN4I select SUPPORT_SPL config MACH_SUN5I bool "sun5i (Allwinner A13)" select CPU_V7 + select SUNXI_GEN_SUN4I select SUPPORT_SPL config MACH_SUN6I bool "sun6i (Allwinner A31)" select CPU_V7 + select SUNXI_GEN_SUN6I select SUPPORT_SPL config MACH_SUN7I @@ -23,12 +42,14 @@ config MACH_SUN7I select CPU_V7 select CPU_V7_HAS_NONSEC select CPU_V7_HAS_VIRT + select SUNXI_GEN_SUN4I select SUPPORT_SPL select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT config MACH_SUN8I bool "sun8i (Allwinner A23)" select CPU_V7 + select SUNXI_GEN_SUN6I select SUPPORT_SPL endchoice 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 setbits_le32(&ccm->ahb_reset0_cfg, 0x1 << AHB_RESET_OFFSET_GMAC); setbits_le32(&ccm->ahb_gate0, 0x1 << AHB_GATE_OFFSET_GMAC); +#else + setbits_le32(&ccm->ahb_gate1, 0x1 << AHB_GATE_OFFSET_GMAC); #endif /* Set MII clock */ diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c index 2233545..1c234e2 100644 --- a/drivers/mmc/sunxi_mmc.c +++ b/drivers/mmc/sunxi_mmc.c @@ -151,8 +151,7 @@ static int mmc_clk_io_on(int sdc_no) /* config ahb clock */ setbits_le32(&ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_MMC(sdc_no)); -#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN8I) || \ - defined(CONFIG_MACH_SUN9I) +#ifdef CONFIG_SUNXI_GEN_SUN6I /* unassert reset */ setbits_le32(&ccm->ahb_reset0_cfg, 1 << AHB_RESET_OFFSET_MMC(sdc_no)); #endif 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 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 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 /* 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 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 struct sunxi_ccm_reg * const ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
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@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(-)