[U-Boot] [PATCH v2 0/5] sun6i: A31s / CSQ_CS908 board support
Hi,
Here is v2 of my sun6i: A31s / CSQ_CS908 board support series.
Changes since v1: -"sun6i: Make dram clk and zq value Kconfig options" -Mention changing of default zq value in commit message -Drop "if EXPERT" usage, as that breaks setting things through defconfig files -"sun6i: Drop some "unknown magic" from dram init" -Mention that the info on what the dropped unknown magic did comes from Allwinner -"sun6i: Add new CSQ_CS908 board" -Add setting of LDO for phy, so that network works -Correct usb vbus settings
Regards,
Hans
It turns out that there is a too large spread between boards to handle this with a default value, turn this into Kconfig options, and set the values the factory images are using for the Colombus and Mele_M9 boards.
Note this changes the ZQ default when not overriden through defconfig from 120 to 123, as that is what most boards seem to actually use.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 12 +++++------- board/sunxi/Kconfig | 17 +++++++++++++++++ configs/Colombus_defconfig | 2 ++ configs/Mele_M9_defconfig | 2 ++ 4 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c index 10a6241..30439dc 100644 --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c @@ -17,9 +17,7 @@ #include <asm/arch/dram.h> #include <asm/arch/prcm.h>
-/* DRAM clk & zq defaults, maybe turn these into Kconfig options ? */ -#define DRAM_CLK_DEFAULT 312000000 -#define DRAM_ZQ_DEFAULT 0x78 +#define DRAM_CLK (CONFIG_DRAM_CLK * 1000000)
struct dram_sun6i_para { u8 bus_width; @@ -48,7 +46,7 @@ static void mctl_sys_init(void) (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; const int dram_clk_div = 2;
- clock_set_pll5(DRAM_CLK_DEFAULT * dram_clk_div); + clock_set_pll5(DRAM_CLK * dram_clk_div);
clrsetbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_DIV0_MASK, CCM_DRAMCLK_CFG_DIV0(dram_clk_div) | CCM_DRAMCLK_CFG_RST | @@ -173,7 +171,7 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
await_completion(&mctl_phy->pgsr, 0x03, 0x03);
- writel(DRAM_ZQ_DEFAULT, &mctl_phy->zq0cr1); + writel(CONFIG_DRAM_ZQ, &mctl_phy->zq0cr1);
setbits_le32(&mctl_phy->pir, MCTL_PIR_CLEAR_STATUS); writel(MCTL_PIR_STEP1, &mctl_phy->pir); @@ -219,9 +217,9 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para) await_completion(&mctl_ctl->sstat, 0x07, 0x01);
/* Set number of clks per micro-second */ - writel(DRAM_CLK_DEFAULT / 1000000, &mctl_ctl->togcnt1u); + writel(DRAM_CLK / 1000000, &mctl_ctl->togcnt1u); /* Set number of clks per 100 nano-seconds */ - writel(DRAM_CLK_DEFAULT / 10000000, &mctl_ctl->togcnt100n); + writel(DRAM_CLK / 10000000, &mctl_ctl->togcnt100n); /* Set memory timing registers */ writel(MCTL_TREFI, &mctl_ctl->trefi); writel(MCTL_TMRD, &mctl_ctl->tmrd); diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 246cd9a..6162227 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -32,6 +32,23 @@ config MACH_SUN8I
endchoice
+if MACH_SUN6I + +config DRAM_CLK + int "sun6i dram clock speed" + default 312 + ---help--- + Set the dram clock speed, valid range 240 - 480, must be a multiple + of 24. + +config DRAM_ZQ + int "sun6i dram zq value" + default 123 + ---help--- + Set the dram zq value. + +endif + config SYS_CONFIG_NAME string default "sun4i" if MACH_SUN4I diff --git a/configs/Colombus_defconfig b/configs/Colombus_defconfig index de78a01..9b4968f 100644 --- a/configs/Colombus_defconfig +++ b/configs/Colombus_defconfig @@ -5,3 +5,5 @@ CONFIG_USB_KEYBOARD=n +S:CONFIG_ARCH_SUNXI=y +S:CONFIG_MACH_SUN6I=y +S:CONFIG_TARGET_COLOMBUS=y ++S:CONFIG_DRAM_CLK=288 ++S:CONFIG_DRAM_ZQ=379 diff --git a/configs/Mele_M9_defconfig b/configs/Mele_M9_defconfig index 40eabce..740b931 100644 --- a/configs/Mele_M9_defconfig +++ b/configs/Mele_M9_defconfig @@ -5,6 +5,8 @@ CONFIG_FDTFILE="sun6i-a31-m9.dtb" +S:CONFIG_ARCH_SUNXI=y +S:CONFIG_MACH_SUN6I=y +S:CONFIG_TARGET_MELE_M9=y ++S:CONFIG_DRAM_CLK=312 ++S:CONFIG_DRAM_ZQ=120 # Ethernet phy power +S:CONFIG_AXP221_DLDO1_VOLT=3300 # USB hub power
On Sun, 2014-11-23 at 14:43 +0100, Hans de Goede wrote:
It turns out that there is a too large spread between boards to handle this with a default value, turn this into Kconfig options, and set the values the factory images are using for the Colombus and Mele_M9 boards.
Note this changes the ZQ default when not overriden through defconfig from 120 to 123, as that is what most boards seem to actually use.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Ian Campbell ijc@hellion.org.uk
With one minor query:
+config DRAM_CLK
- int "sun6i dram clock speed"
- default 312
- ---help---
- Set the dram clock speed, valid range 240 - 480, must be a multiple
- of 24.
[...] diff --git a/configs/Mele_M9_defconfig b/configs/Mele_M9_defconfig index 40eabce..740b931 100644 --- a/configs/Mele_M9_defconfig +++ b/configs/Mele_M9_defconfig @@ -5,6 +5,8 @@ CONFIG_FDTFILE="sun6i-a31-m9.dtb" +S:CONFIG_ARCH_SUNXI=y +S:CONFIG_MACH_SUN6I=y +S:CONFIG_TARGET_MELE_M9=y ++S:CONFIG_DRAM_CLK=312
You are overriding this to be the default, is that deliberate/necessary?
I suspect it's just to keep both or neither of CLK and ZQ explicitly give, which is a fine reason.
Ian.
Hi,
On 11/25/2014 09:55 AM, Ian Campbell wrote:
On Sun, 2014-11-23 at 14:43 +0100, Hans de Goede wrote:
It turns out that there is a too large spread between boards to handle this with a default value, turn this into Kconfig options, and set the values the factory images are using for the Colombus and Mele_M9 boards.
Note this changes the ZQ default when not overriden through defconfig from 120 to 123, as that is what most boards seem to actually use.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Ian Campbell ijc@hellion.org.uk
With one minor query:
+config DRAM_CLK
- int "sun6i dram clock speed"
- default 312
- ---help---
- Set the dram clock speed, valid range 240 - 480, must be a multiple
- of 24.
[...] diff --git a/configs/Mele_M9_defconfig b/configs/Mele_M9_defconfig index 40eabce..740b931 100644 --- a/configs/Mele_M9_defconfig +++ b/configs/Mele_M9_defconfig @@ -5,6 +5,8 @@ CONFIG_FDTFILE="sun6i-a31-m9.dtb" +S:CONFIG_ARCH_SUNXI=y +S:CONFIG_MACH_SUN6I=y +S:CONFIG_TARGET_MELE_M9=y ++S:CONFIG_DRAM_CLK=312
You are overriding this to be the default, is that deliberate/necessary?
I suspect it's just to keep both or neither of CLK and ZQ explicitly give, which is a fine reason.
Yes, this is just here to make it easier to see what CLK and ZQ is used without needing to look in the Kconfig, it is also here to make it a better template for other sun6i defconfig-s
Regards,
Hans
On Sun, 23 Nov 2014 14:43:11 +0100 Hans de Goede hdegoede@redhat.com wrote:
It turns out that there is a too large spread between boards to handle this with a default value, turn this into Kconfig options, and set the values the factory images are using for the Colombus and Mele_M9 boards.
Note this changes the ZQ default when not overriden through defconfig from 120 to 123, as that is what most boards seem to actually use.
Signed-off-by: Hans de Goede hdegoede@redhat.com
0x7b (or 123 in the decimal) form is a default reset value in the hardware register (at least on A10/A13/A20 and also on TI Keystone2 hardware). So it indeed makes sense to have it as the default value for u-boot. And also very likely shows that whoever provided the DRAM configurations with this value, did not in fact bother to really configure ZQ to something meaningful :-)
In general, if we read the JEDEC booklets, then ZQ calibration is the feature, which allows to improve reliability and increase DRAM clock speeds. The DRAM clock speeds, which are well beyond what the Allwinner A31 boards seem to be using at the moment.
I hope that 0x78 value for ZQ on Mele M9 is really there for a reason, and not something like actually 0x7B that got misread from the printed documentation or from screen :-) As the DRAM clock speeds on A31 hardware seem to be really very low, I would suspect that almost any ZQ value would be probably good enough in practice.
For A10/A13/A20 hardware we actually have easy tools to measure DRAM reliability. These tools can help to pick good ZQ settings. Something similar may be potentially implemented for A31 too.
Add a sunxi_get_ss_bonding_id() function, and use it to differentiate between the A31s and the A31.
Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Ian Campbell ijc@hellion.org.uk --- arch/arm/cpu/armv7/sunxi/cpu_info.c | 38 ++++++++++++++++++++++++++- arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 4 ++- arch/arm/include/asm/arch-sunxi/cpu.h | 5 ++++ 3 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/cpu_info.c b/arch/arm/cpu/armv7/sunxi/cpu_info.c index 41b9add..5146dc4 100644 --- a/arch/arm/cpu/armv7/sunxi/cpu_info.c +++ b/arch/arm/cpu/armv7/sunxi/cpu_info.c @@ -9,6 +9,32 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/cpu.h> +#include <asm/arch/clock.h> + +#ifdef CONFIG_MACH_SUN6I +int sunxi_get_ss_bonding_id(void) +{ + struct sunxi_ccm_reg * const ccm = + (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; + static int bonding_id = -1; + + if (bonding_id != -1) + return bonding_id; + + /* Enable Security System */ + setbits_le32(&ccm->ahb_reset0_cfg, 1 << AHB_RESET_OFFSET_SS); + setbits_le32(&ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_SS); + + bonding_id = readl(SUNXI_SS_BASE); + bonding_id = (bonding_id >> 16) & 0x7; + + /* Disable Security System again */ + clrbits_le32(&ccm->ahb_gate0, 1 << AHB_GATE_OFFSET_SS); + clrbits_le32(&ccm->ahb_reset0_cfg, 1 << AHB_RESET_OFFSET_SS); + + return bonding_id; +} +#endif
#ifdef CONFIG_DISPLAY_CPUINFO int print_cpuinfo(void) @@ -24,7 +50,17 @@ int print_cpuinfo(void) default: puts("CPU: Allwinner A1X (SUN5I)\n"); } #elif defined CONFIG_MACH_SUN6I - puts("CPU: Allwinner A31 (SUN6I)\n"); + switch (sunxi_get_ss_bonding_id()) { + case SUNXI_SS_BOND_ID_A31: + puts("CPU: Allwinner A31 (SUN6I)\n"); + break; + case SUNXI_SS_BOND_ID_A31S: + puts("CPU: Allwinner A31s (SUN6I)\n"); + break; + default: + printf("CPU: Allwinner A31? (SUN6I, id: %d)\n", + sunxi_get_ss_bonding_id()); + } #elif defined CONFIG_MACH_SUN7I puts("CPU: Allwinner A20 (SUN7I)\n"); #elif defined CONFIG_MACH_SUN8I diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h index 50a4b69..7e810bb 100644 --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h @@ -209,6 +209,7 @@ struct sunxi_ccm_reg { #define AHB_GATE_OFFSET_MMC1 9 #define AHB_GATE_OFFSET_MMC0 8 #define AHB_GATE_OFFSET_MMC(n) (AHB_GATE_OFFSET_MMC0 + (n)) +#define AHB_GATE_OFFSET_SS 5
/* ahb_gate1 offsets */ #define AHB_GATE_OFFSET_DRC0 25 @@ -270,8 +271,9 @@ struct sunxi_ccm_reg { #define AHB_RESET_OFFSET_MMC1 9 #define AHB_RESET_OFFSET_MMC0 8 #define AHB_RESET_OFFSET_MMC(n) (AHB_RESET_OFFSET_MMC0 + (n)) +#define AHB_RESET_OFFSET_SS 5
-/* ahb_reset0 offsets */ +/* ahb_reset1 offsets */ #define AHB_RESET_OFFSET_DRC0 25 #define AHB_RESET_OFFSET_DE_BE0 12 #define AHB_RESET_OFFSET_HDMI 11 diff --git a/arch/arm/include/asm/arch-sunxi/cpu.h b/arch/arm/include/asm/arch-sunxi/cpu.h index bdee89e..90e06c0 100644 --- a/arch/arm/include/asm/arch-sunxi/cpu.h +++ b/arch/arm/include/asm/arch-sunxi/cpu.h @@ -135,9 +135,14 @@
#define SUNXI_CPU_CFG (SUNXI_TIMER_BASE + 0x13c)
+/* SS bonding ids used for cpu identification */ +#define SUNXI_SS_BOND_ID_A31 4 +#define SUNXI_SS_BOND_ID_A31S 5 + #ifndef __ASSEMBLY__ void sunxi_board_init(void); void sunxi_reset(void); +int sunxi_get_ss_bonding_id(void); #endif /* __ASSEMBLY__ */
#endif /* _CPU_H */
The A31s only has one dram channel, so do not bother with trying to initialize a second channel.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- arch/arm/cpu/armv7/sunxi/Makefile | 2 +- arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile index 3b6ae47..1337b60 100644 --- a/arch/arm/cpu/armv7/sunxi/Makefile +++ b/arch/arm/cpu/armv7/sunxi/Makefile @@ -10,6 +10,7 @@ obj-y += timer.o obj-y += board.o obj-y += clock.o +obj-y += cpu_info.o obj-y += pinmux.o obj-$(CONFIG_MACH_SUN6I) += prcm.o obj-$(CONFIG_MACH_SUN8I) += prcm.o @@ -21,7 +22,6 @@ obj-$(CONFIG_MACH_SUN7I) += clock_sun4i.o obj-$(CONFIG_MACH_SUN8I) += clock_sun6i.o
ifndef CONFIG_SPL_BUILD -obj-y += cpu_info.o ifdef CONFIG_ARMV7_PSCI obj-y += psci.o endif diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c index 30439dc..2ac0b58 100644 --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c @@ -372,10 +372,15 @@ unsigned long sunxi_dram_init(void) .rows = 16, };
+ /* A31s only has one channel */ + if (sunxi_get_ss_bonding_id() == SUNXI_SS_BOND_ID_A31S) + para.chan = 1; + mctl_sys_init();
mctl_dll_init(0, ¶); - mctl_dll_init(1, ¶); + if (para.chan == 2) + mctl_dll_init(1, ¶);
setbits_le32(&mctl_com->ccr, MCTL_CCR_MASTER_CLK_EN | @@ -383,7 +388,9 @@ unsigned long sunxi_dram_init(void) MCTL_CCR_CH1_CLK_EN);
mctl_channel_init(0, ¶); - mctl_channel_init(1, ¶); + if (para.chan == 2) + mctl_channel_init(1, ¶); + mctl_com_init(¶); mctl_port_cfg();
On Sun, 2014-11-23 at 14:43 +0100, Hans de Goede wrote:
The A31s only has one dram channel, so do not bother with trying to initialize a second channel.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Ian Campbell ijc@hellion.org.uk
On Sun, 23 Nov 2014 14:43:13 +0100 Hans de Goede hdegoede@redhat.com wrote:
The A31s only has one dram channel, so do not bother with trying to initialize a second channel.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Ian Campbell has already noticed in the earlier review. In the case *if* the automatic detection works correctly, then why bother adding this *extra* code? Based on your description, it looks like a superfluous band-aid and increases the number of "moving parts". Which generally makes the code less maintainable.
There was a talk about calling it a "performance optimization" earlier, but the v2 commit message does not mention this. Neither does it present any benchmark numbers.
Basically, it boils down to whether we can trust the automatic detection code to do a proper job or not. Normally, if the automatic detection does not work correctly in all cases, then it needs to be fixed in the long run. But I can clearly understand if you are not willing to take any risks and just want to deploy something that somehow works to the end users sooner.
Either way, I have nothing against these patches. Just would prefer a little bit better transparency and more clear commit messages. Keep up the good job.
Hi,
On 12-12-14 21:25, Siarhei Siamashka wrote:
On Sun, 23 Nov 2014 14:43:13 +0100 Hans de Goede hdegoede@redhat.com wrote:
The A31s only has one dram channel, so do not bother with trying to initialize a second channel.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Ian Campbell has already noticed in the earlier review. In the case *if* the automatic detection works correctly,
It does work correctly in my testing.
then why bother adding this *extra* code?
I'm just mimicking what the boot0 code does here, and the boot0 code does detect the SoC type and not bother with the second channel on A31s, and that seems sensible to do, nothing good will come out of trying to use the second channel on A31s.
Based on your description, it looks like a superfluous band-aid and increases the number of "moving parts". Which generally makes the code less maintainable.
There was a talk about calling it a "performance optimization" earlier, but the v2 commit message does not mention this. Neither does it present any benchmark numbers.
Basically, it boils down to whether we can trust the automatic detection code to do a proper job or not. Normally, if the automatic detection does not work correctly in all cases, then it needs to be fixed in the long run. But I can clearly understand if you are not willing to take any risks and just want to deploy something that somehow works to the end users sooner.
In my testing the auto-detect code worked fine for automatically disabling the 2nd channel on on A31s, but as said it seems sensible to just not bother with the 2nd channel at all on A31s.
Regards,
Hans
On Sat, 13 Dec 2014 12:00:44 +0100 Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 12-12-14 21:25, Siarhei Siamashka wrote:
On Sun, 23 Nov 2014 14:43:13 +0100 Hans de Goede hdegoede@redhat.com wrote:
The A31s only has one dram channel, so do not bother with trying to initialize a second channel.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Ian Campbell has already noticed in the earlier review. In the case *if* the automatic detection works correctly,
It does work correctly in my testing.
Thanks. Yes, I have seen your confirmation earlier: http://lists.denx.de/pipermail/u-boot/2014-November/196198.html
"My assumption was that it would not work, as the A31s has only one channel, or so the datasheets claim. But it turned out it does work"
then why bother adding this *extra* code?
I'm just mimicking what the boot0 code does here, and the boot0 code does detect the SoC type and not bother with the second channel on A31s, and that seems sensible to do, nothing good will come out of trying to use the second channel on A31s.
That's a good point. If boot0 is doing this, then Allwinner developers probably had some valid reason. Too bad that we don't know this reason.
You are currently transplanting the Allwinner code into u-boot. And in the short run it is indeed safer to keep it as is. Especially considering that you are apparently not doing any extensive testing of the individual features and hardware registers, but instead heavily relying on assumptions.
Based on your description, it looks like a superfluous band-aid and increases the number of "moving parts". Which generally makes the code less maintainable.
There was a talk about calling it a "performance optimization" earlier, but the v2 commit message does not mention this. Neither does it present any benchmark numbers.
Basically, it boils down to whether we can trust the automatic detection code to do a proper job or not. Normally, if the automatic detection does not work correctly in all cases, then it needs to be fixed in the long run. But I can clearly understand if you are not willing to take any risks and just want to deploy something that somehow works to the end users sooner.
In my testing the auto-detect code worked fine for automatically disabling the 2nd channel on on A31s, but as said it seems sensible to just not bother with the 2nd channel at all on A31s.
In what way is it sensible to have three possible code paths (A31s, A31 with two channels, A31 with a single channel) instead of two before this change (A31/A31s with a single channel, A31 with two channels)? This just results in a worse testing coverage situation.
Allwinner tells us that this bit of code is the rtc ram being used to detect coming out of "super-standby" mode, and if that is the case, going out of self-refresh mode.
Since we do not support "super-standby" mode, this can be dropped.
Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Ian Campbell ijc@hellion.org.uk --- arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c index 2ac0b58..5e163cb 100644 --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c @@ -140,9 +140,6 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
writel((MCTL_TITMSRST << 18) | (MCTL_TDLLLOCK << 6) | MCTL_TDLLSRST, &mctl_phy->ptr0); - /* Unknown magic performed by boot0 */ - if ((readl(SUNXI_RTC_BASE + 0x20c) & 3) == 2) - setbits_le32(&mctl_phy->ptr0, 1 << 18);
writel((MCTL_TDINIT1 << 19) | MCTL_TDINIT0, &mctl_phy->ptr1); writel((MCTL_TDINIT3 << 17) | MCTL_TDINIT2, &mctl_phy->ptr2);
On Sun, 23 Nov 2014 14:43:14 +0100 Hans de Goede hdegoede@redhat.com wrote:
Sorry for a late reply. I was not on CC for these patches and don't properly keep track of the u-boot mailing list activity lately.
Allwinner tells us that this bit of code is the rtc ram being used to detect coming out of "super-standby" mode, and if that is the case, going out of self-refresh mode.
If I understand this paragraph correctly, you seem to have a privileged communication channel with Allwinner. And I wonder if you are keeping track of this information somehow and willing to share it with the others?
For sun4i/sun5i/sun7i DRAM controller, we actively used a wiki page: http://linux-sunxi.org/A10_DRAM_Controller_Register_Guide Basically, whenever something new is found about some hardware register, just do a quick edit to make sure that this information is not forgotten or lost. This does not really take much time. Trying to remember something later and searching it in the scattered old e-mails is usually a bigger waste of time.
The most interesting question is of course whether Allwinner has any plans to provide real DRAM controller documentation. If they do it, then we don't have to waste time on finding and documenting all the relevant information.
Since we do not support "super-standby" mode, this can be dropped.
This patch seems to have a similar purpose to what we did for sun4i/sun5i/sun7i dram controller earlier: http://git.denx.de/?p=u-boot.git;a=commitdiff;h=f2577967738f923571b7156ad46e...
A somewhat tricky part about this stuff is that while we don't support the "super-standby" mode, we still have to somehow deal with it whenever we actually encounter it in the wild. It is easy to reproduce if you have an Allwinner tablet with Android firmware (just disable WLAN, let it sleep for a while, insert an SD card with u-boot and GNU/Linux, try to press the power button to wake the device from sleep).
For example, on sun5i/sun7i hardware, doing "writel(0x16510000, &dram->ppwrsctl)" part is important if we want to really discard the RAM data and boot normally. A failure to do so just transitions the device into and unbootable state. Moreover, the device may look as "bricked" to the end user until the RTC battery runs out of juice or some special action is taken. I think that this scenario is exactly what was described in the NOTICE section: http://cubieboard.org/2014/01/13/upgrade-new-android-for-cubietruckv1-01/
I'm not sure if all the same applies to A31 hardware (does having a dedicated OpenRISC power management chip change anything?), but just chopping off the code and hoping for the best might be not the best action.
Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Ian Campbell ijc@hellion.org.uk
arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c index 2ac0b58..5e163cb 100644 --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c @@ -140,9 +140,6 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
writel((MCTL_TITMSRST << 18) | (MCTL_TDLLLOCK << 6) | MCTL_TDLLSRST, &mctl_phy->ptr0);
- /* Unknown magic performed by boot0 */
- if ((readl(SUNXI_RTC_BASE + 0x20c) & 3) == 2)
setbits_le32(&mctl_phy->ptr0, 1 << 18);
Regarding the "unknown magic". In fact we already have quite a lot of information about various obscure parts of the Allwinner DRAM code: http://lists.denx.de/pipermail/u-boot/2014-September/189199.html
For example, for this PTR0 PHY register, we have the following information from the RK3288 header files:
/* PTR0 */ #define tITMSRST(n) ((n)<<18) #define tDLLLOCK(n) ((n)<<6) #define tDLLSRST(n) ((n)<<0)
Having the bit fields as named identifiers instead of Allwinner magic numbers makes the code more readable.
And we also have the "4.41 PHY Timing Register 0 (PTR0)" section in http://www.ti.com/lit/ug/spruhn7a/spruhn7a.pdf for some general explanations.
The RK3288 is almost a perfect match, because all the hardware register addresses and the register names are nearly identical. The TI Keystone2 is a lot more distant relative, so the information in its documentation is less trustworthy for us.
The actual functionality of these bits still needs to be confirmed in an experimental way (instead of just making theoretical assumptions and hoping for the best). But again, wasting time on doing this only makes sense if Allwinner in fact has no plans to release proper documentation for the DRAM controllers used in their SoCs.
Hi,
On 12-12-14 21:24, Siarhei Siamashka wrote:
On Sun, 23 Nov 2014 14:43:14 +0100 Hans de Goede hdegoede@redhat.com wrote:
Sorry for a late reply. I was not on CC for these patches and don't properly keep track of the u-boot mailing list activity lately.
Allwinner tells us that this bit of code is the rtc ram being used to detect coming out of "super-standby" mode, and if that is the case, going out of self-refresh mode.
If I understand this paragraph correctly, you seem to have a privileged communication channel with Allwinner. And I wonder if you are keeping track of this information somehow and willing to share it with the others?
I do not really have a private communication channel, as Simos Xenitellis who has been working on getting beter collaboration between Allwinner and the linux-sunxi community has mentioned in the "[linux-sunxi] Introductions and Allwinner documentation update" thread, kevin@allwinnertech.com and shuge@allwinnertech.com are available to ask questions we may have and I've been doing that.
Note that I had multiple questions about the sun6i DRAM controller, and the info in this commit is all I got unfortunately they are not able to share anything wrt the DRAM controller.
For sun4i/sun5i/sun7i DRAM controller, we actively used a wiki page: http://linux-sunxi.org/A10_DRAM_Controller_Register_Guide Basically, whenever something new is found about some hardware register, just do a quick edit to make sure that this information is not forgotten or lost. This does not really take much time. Trying to remember something later and searching it in the scattered old e-mails is usually a bigger waste of time.
The most interesting question is of course whether Allwinner has any plans to provide real DRAM controller documentation. If they do it, then we don't have to waste time on finding and documenting all the relevant information.
AFAIK there are no plans to provide any documentation, or any info at all, as said I had multiple questions and the info in this commit is all the info I got.
The A23 dram support I'm about to post is also all my own work with no help from Allwinner.
Since we do not support "super-standby" mode, this can be dropped.
This patch seems to have a similar purpose to what we did for sun4i/sun5i/sun7i dram controller earlier: http://git.denx.de/?p=u-boot.git;a=commitdiff;h=f2577967738f923571b7156ad46e...
A somewhat tricky part about this stuff is that while we don't support the "super-standby" mode, we still have to somehow deal with it whenever we actually encounter it in the wild. It is easy to reproduce if you have an Allwinner tablet with Android firmware (just disable WLAN, let it sleep for a while, insert an SD card with u-boot and GNU/Linux, try to press the power button to wake the device from sleep).
For example, on sun5i/sun7i hardware, doing "writel(0x16510000, &dram->ppwrsctl)" part is important if we want to really discard the RAM data and boot normally. A failure to do so just transitions the device into and unbootable state. Moreover, the device may look as "bricked" to the end user until the RTC battery runs out of juice or some special action is taken. I think that this scenario is exactly what was described in the NOTICE section: http://cubieboard.org/2014/01/13/upgrade-new-android-for-cubietruckv1-01/
I'm not sure if all the same applies to A31 hardware (does having a dedicated OpenRISC power management chip change anything?), but just chopping off the code and hoping for the best might be not the best action.
A valid point, unfortunately I only have A31 based top set boxes and those do not do suspend / resume AFAIK...
But definitely something to keep in mind.
Regards,
Hans
Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Ian Campbell ijc@hellion.org.uk
arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c index 2ac0b58..5e163cb 100644 --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c @@ -140,9 +140,6 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
writel((MCTL_TITMSRST << 18) | (MCTL_TDLLLOCK << 6) | MCTL_TDLLSRST, &mctl_phy->ptr0);
- /* Unknown magic performed by boot0 */
- if ((readl(SUNXI_RTC_BASE + 0x20c) & 3) == 2)
setbits_le32(&mctl_phy->ptr0, 1 << 18);Regarding the "unknown magic". In fact we already have quite a lot of information about various obscure parts of the Allwinner DRAM code: http://lists.denx.de/pipermail/u-boot/2014-September/189199.html
For example, for this PTR0 PHY register, we have the following information from the RK3288 header files:
/* PTR0 */ #define tITMSRST(n) ((n)<<18) #define tDLLLOCK(n) ((n)<<6) #define tDLLSRST(n) ((n)<<0)
Having the bit fields as named identifiers instead of Allwinner magic numbers makes the code more readable.
And we also have the "4.41 PHY Timing Register 0 (PTR0)" section in http://www.ti.com/lit/ug/spruhn7a/spruhn7a.pdf for some general explanations.
The RK3288 is almost a perfect match, because all the hardware register addresses and the register names are nearly identical. The TI Keystone2 is a lot more distant relative, so the information in its documentation is less trustworthy for us.
The actual functionality of these bits still needs to be confirmed in an experimental way (instead of just making theoretical assumptions and hoping for the best). But again, wasting time on doing this only makes sense if Allwinner in fact has no plans to release proper documentation for the DRAM controllers used in their SoCs.
The CSQ CS908 is an A31s based top-set box, with 1G RAM, 8G NAND, rtl8188etv usb wifi, 2 USB A receptacles (1 connected through the OTG controller), ethernet, 3.5 mm jack with a/v out and hdmi out:
http://www.geekbuying.com/item/CS908-Allwinner-A31S-Quad-Core-1-2GHz-Android...
Note it has no sdcard slot and therefore can only be fel booted.
Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Ian Campbell ijc@hellion.org.uk --- configs/CSQ_CS908_defconfig | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 configs/CSQ_CS908_defconfig
diff --git a/configs/CSQ_CS908_defconfig b/configs/CSQ_CS908_defconfig new file mode 100644 index 0000000..18fe1f3 --- /dev/null +++ b/configs/CSQ_CS908_defconfig @@ -0,0 +1,19 @@ +CONFIG_SPL=y +CONFIG_SYS_EXTRA_OPTIONS="USB_EHCI" +CONFIG_FDTFILE="sun6i-a31s-cs908.dtb" ++S:CONFIG_ARM=y ++S:CONFIG_ARCH_SUNXI=y ++S:CONFIG_MACH_SUN6I=y ++S:CONFIG_TARGET_CSQ_CS908=y ++S:CONFIG_DRAM_CLK=432 ++S:CONFIG_DRAM_ZQ=123 +# Ethernet phy power ++S:CONFIG_AXP221_DLDO1_VOLT=3300 +# Wifi power ++S:CONFIG_AXP221_ALDO1_VOLT=3300 +# HDMI power ? ++S:CONFIG_AXP221_ALDO2_VOLT=1800 ++S:CONFIG_AXP221_ALDO3_VOLT=3000 +# No Vbus gpio for either usb ++S:CONFIG_USB1_VBUS_PIN="" ++S:CONFIG_USB2_VBUS_PIN=""
On Sun, 2014-11-23 at 14:43 +0100, Hans de Goede wrote:
The CSQ CS908 is an A31s based top-set box, with 1G RAM, 8G NAND, rtl8188etv usb wifi, 2 USB A receptacles (1 connected through the OTG controller), ethernet, 3.5 mm jack with a/v out and hdmi out:
http://www.geekbuying.com/item/CS908-Allwinner-A31S-Quad-Core-1-2GHz-Android...
Note it has no sdcard slot and therefore can only be fel booted.
Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Ian Campbell ijc@hellion.org.uk
configs/CSQ_CS908_defconfig | 19 +++++++++++++++++++
Apparently I'm terrible at remembering this during review: this is missing a MAINTAINERS entry. (MAKEALL warns about this when regenerating boards.cfg).
Since this patch is only in u-boot-sunxi#next so far do you just want to fold in an update to add it to your stanza (or add a new appropriate stanza) and force push? Ack to that plan if you do.
FYI I've just force pushed a similar change adding Chen-Yu's entry to the Hummingbird A31.
Ian.
Hi,
On 14-12-14 15:57, Ian Campbell wrote:
On Sun, 2014-11-23 at 14:43 +0100, Hans de Goede wrote:
The CSQ CS908 is an A31s based top-set box, with 1G RAM, 8G NAND, rtl8188etv usb wifi, 2 USB A receptacles (1 connected through the OTG controller), ethernet, 3.5 mm jack with a/v out and hdmi out:
http://www.geekbuying.com/item/CS908-Allwinner-A31S-Quad-Core-1-2GHz-Android...
Note it has no sdcard slot and therefore can only be fel booted.
Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Ian Campbell ijc@hellion.org.uk
configs/CSQ_CS908_defconfig | 19 +++++++++++++++++++
Apparently I'm terrible at remembering this during review: this is missing a MAINTAINERS entry. (MAKEALL warns about this when regenerating boards.cfg).
Yeah, I'm bad at remembering to update MAINTAINERS too...
Since this patch is only in u-boot-sunxi#next so far do you just want to fold in an update to add it to your stanza (or add a new appropriate stanza) and force push? Ack to that plan if you do.
Done (fixed, forced-push).
I've also fixed the same problem for the new Ippo_q8h_v1.2_defconfig in my sun8i spl / draminit set in my personal tree.
Regards,
Hans
On Thu, 2014-12-18 at 11:39 +0100, Hans de Goede wrote:
Hi,
On 14-12-14 15:57, Ian Campbell wrote:
On Sun, 2014-11-23 at 14:43 +0100, Hans de Goede wrote:
The CSQ CS908 is an A31s based top-set box, with 1G RAM, 8G NAND, rtl8188etv usb wifi, 2 USB A receptacles (1 connected through the OTG controller), ethernet, 3.5 mm jack with a/v out and hdmi out:
http://www.geekbuying.com/item/CS908-Allwinner-A31S-Quad-Core-1-2GHz-Android...
Note it has no sdcard slot and therefore can only be fel booted.
Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Ian Campbell ijc@hellion.org.uk
configs/CSQ_CS908_defconfig | 19 +++++++++++++++++++
Apparently I'm terrible at remembering this during review: this is missing a MAINTAINERS entry. (MAKEALL warns about this when regenerating boards.cfg).
Yeah, I'm bad at remembering to update MAINTAINERS too...
Since this patch is only in u-boot-sunxi#next so far do you just want to fold in an update to add it to your stanza (or add a new appropriate stanza) and force push? Ack to that plan if you do.
Done (fixed, forced-push).
Thanks.
I've also fixed the same problem for the new Ippo_q8h_v1.2_defconfig in my sun8i spl / draminit set in my personal tree.
Great, that's one less to forget then!
Ian.
participants (3)
-
Hans de Goede -
Ian Campbell -
Siarhei Siamashka