Hi,
On 12/12/16 12:29, Maxime Ripard wrote:
> On Tue, Dec 06, 2016 at 02:15:17PM +0000, Andre Przywara wrote:
>> Hi,
>>
>> On 06/12/16 11:20, Maxime Ripard wrote:
>>> On Mon, Dec 05, 2016 at 01:52:22AM +0000, Andre Przywara wrote:
>>>> From: Jens Kuske <jensku...@gmail.com>
>>>>
>>>> The A64 DRAM controller is very similar to the H3 one,
>>>> so the code can be reused with some small changes.
>>>> [Andre: fixed up typo, merged in fixes from Jens]
>>>>
>>>> Signed-off-by: Jens Kuske <jensku...@gmail.com>
>>>> Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
>>>> ---
>>>> arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 1 +
>>>> arch/arm/include/asm/arch-sunxi/dram.h | 2 +-
>>>> arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h | 10 +-
>>>> arch/arm/mach-sunxi/Makefile | 1 +
>>>> arch/arm/mach-sunxi/clock_sun6i.c | 2 +-
>>>> arch/arm/mach-sunxi/dram_sun8i_h3.c | 139
>>>> +++++++++++++++++++-----
>>>> 6 files changed, 123 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>>>> b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>>>> index be9fcfd..3f87672 100644
>>>> --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>>>> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>>>> @@ -322,6 +322,7 @@ struct sunxi_ccm_reg {
>>>> #define CCM_DRAMCLK_CFG_DIV0_MASK (0xf << 8)
>>>> #define CCM_DRAMCLK_CFG_SRC_PLL5 (0x0 << 20)
>>>> #define CCM_DRAMCLK_CFG_SRC_PLL6x2 (0x1 << 20)
>>>> +#define CCM_DRAMCLK_CFG_SRC_PLL11 (0x1 << 20) /* A64 only */
>>>> #define CCM_DRAMCLK_CFG_SRC_MASK (0x3 << 20)
>>>> #define CCM_DRAMCLK_CFG_UPD (0x1 << 16)
>>>> #define CCM_DRAMCLK_CFG_RST (0x1 << 31)
>>>> diff --git a/arch/arm/include/asm/arch-sunxi/dram.h
>>>> b/arch/arm/include/asm/arch-sunxi/dram.h
>>>> index e0be744..53e6d47 100644
>>>> --- a/arch/arm/include/asm/arch-sunxi/dram.h
>>>> +++ b/arch/arm/include/asm/arch-sunxi/dram.h
>>>> @@ -24,7 +24,7 @@
>>>> #include <asm/arch/dram_sun8i_a33.h>
>>>> #elif defined(CONFIG_MACH_SUN8I_A83T)
>>>> #include <asm/arch/dram_sun8i_a83t.h>
>>>> -#elif defined(CONFIG_MACH_SUN8I_H3)
>>>> +#elif defined(CONFIG_MACH_SUN8I_H3) || defined(CONFIG_MACH_SUN50I)
>>>> #include <asm/arch/dram_sun8i_h3.h>
>>>> #elif defined(CONFIG_MACH_SUN9I)
>>>> #include <asm/arch/dram_sun9i.h>
>>>> diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
>>>> b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
>>>> index 867fd12..b0e5d93 100644
>>>> --- a/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
>>>> +++ b/arch/arm/include/asm/arch-sunxi/dram_sun8i_h3.h
>>>> @@ -15,7 +15,8 @@
>>>>
>>>> struct sunxi_mctl_com_reg {
>>>> u32 cr; /* 0x00 control register */
>>>> - u8 res0[0xc]; /* 0x04 */
>>>> + u8 res0[0x8]; /* 0x04 */
>>>> + u32 tmr; /* 0x0c (A64 only) */
>>>
>>> #ifdef?
>>
>> What would that change aside from making it hard to read?
>> This is a structure definition, so it doesn't generate any code on its
>> own. And we only access that field from the A64, so it keeps its
>> reserved nature for the H3.
>
> Yes, but at least we can catch improper writes to the register on !A64
> platforms at compile time. We already use that construct in the u-boot
> code, and you're using it every where in the code already, so it
> wouldn't really make it harder to read than it already is.
Still not convinced, but I will give it a try.
>>
>>>> u32 mcr[16][2]; /* 0x10 */
>>>> u32 bwcr; /* 0x90 bandwidth control register */
>>>> u32 maer; /* 0x94 master enable register */
>>>> @@ -32,7 +33,9 @@ struct sunxi_mctl_com_reg {
>>>> u32 swoffr; /* 0xc4 */
>>>> u8 res2[0x8]; /* 0xc8 */
>>>> u32 cccr; /* 0xd0 */
>>>> - u8 res3[0x72c]; /* 0xd4 */
>>>> + u8 res3[0x54]; /* 0xd4 */
>>>> + u32 mdfs_bwlr[3]; /* 0x128 (A64 only) */
>>>> + u8 res4[0x6cc]; /* 0x134 */
>>>
>>> Ditto.
>>>
>>>> u32 protect; /* 0x800 */
>>>> };
>>>>
>>>> @@ -81,7 +84,8 @@ struct sunxi_mctl_ctl_reg {
>>>> u32 rfshtmg; /* 0x90 refresh timing */
>>>> u32 rfshctl1; /* 0x94 */
>>>> u32 pwrtmg; /* 0x98 */
>>>> - u8 res3[0x20]; /* 0x9c */
>>>> + u8 res3[0x1c]; /* 0x9c */
>>>> + u32 vtfcr; /* 0xb8 (A64 only) */
>>>
>>> Ditto
>>>
>>>> u32 dqsgmr; /* 0xbc */
>>>> u32 dtcr; /* 0xc0 */
>>>> u32 dtar[4]; /* 0xc4 */
>>>> diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
>>>> index e73114e..7daba11 100644
>>>> --- a/arch/arm/mach-sunxi/Makefile
>>>> +++ b/arch/arm/mach-sunxi/Makefile
>>>> @@ -50,4 +50,5 @@ obj-$(CONFIG_MACH_SUN8I_A33) += dram_sun8i_a33.o
>>>> obj-$(CONFIG_MACH_SUN8I_A83T) += dram_sun8i_a83t.o
>>>> obj-$(CONFIG_MACH_SUN8I_H3) += dram_sun8i_h3.o
>>>> obj-$(CONFIG_MACH_SUN9I) += dram_sun9i.o
>>>> +obj-$(CONFIG_MACH_SUN50I) += dram_sun8i_h3.o
>>>> endif
>>>> diff --git a/arch/arm/mach-sunxi/clock_sun6i.c
>>>> b/arch/arm/mach-sunxi/clock_sun6i.c
>>>> index 80cfc0b..99f515d 100644
>>>> --- a/arch/arm/mach-sunxi/clock_sun6i.c
>>>> +++ b/arch/arm/mach-sunxi/clock_sun6i.c
>>>> @@ -217,7 +217,7 @@ done:
>>>> }
>>>> #endif
>>>>
>>>> -#ifdef CONFIG_MACH_SUN8I_A33
>>>> +#if defined(CONFIG_MACH_SUN8I_A33) || defined(CONFIG_MACH_SUN50I)
>>>> void clock_set_pll11(unsigned int clk, bool sigma_delta_enable)
>>>> {
>>>> struct sunxi_ccm_reg * const ccm =
>>>> diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c
>>>> b/arch/arm/mach-sunxi/dram_sun8i_h3.c
>>>> index 1647d76..2dc2071 100644
>>>> --- a/arch/arm/mach-sunxi/dram_sun8i_h3.c
>>>> +++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c
>>>> @@ -32,30 +32,6 @@ static inline int ns_to_t(int nanoseconds)
>>>> return DIV_ROUND_UP(ctrl_freq * nanoseconds, 1000);
>>>> }
>>>>
>>>> -static u32 bin_to_mgray(int val)
>>>> -{
>>>> - static const u8 lookup_table[32] = {
>>>> - 0x00, 0x01, 0x02, 0x03, 0x06, 0x07, 0x04, 0x05,
>>>> - 0x0c, 0x0d, 0x0e, 0x0f, 0x0a, 0x0b, 0x08, 0x09,
>>>> - 0x18, 0x19, 0x1a, 0x1b, 0x1e, 0x1f, 0x1c, 0x1d,
>>>> - 0x14, 0x15, 0x16, 0x17, 0x12, 0x13, 0x10, 0x11,
>>>> - };
>>>> -
>>>> - return lookup_table[clamp(val, 0, 31)];
>>>> -}
>>>> -
>>>> -static int mgray_to_bin(u32 val)
>>>> -{
>>>> - static const u8 lookup_table[32] = {
>>>> - 0x00, 0x01, 0x02, 0x03, 0x06, 0x07, 0x04, 0x05,
>>>> - 0x0e, 0x0f, 0x0c, 0x0d, 0x08, 0x09, 0x0a, 0x0b,
>>>> - 0x1e, 0x1f, 0x1c, 0x1d, 0x18, 0x19, 0x1a, 0x1b,
>>>> - 0x10, 0x11, 0x12, 0x13, 0x16, 0x17, 0x14, 0x15,
>>>> - };
>>>> -
>>>> - return lookup_table[val & 0x1f];
>>>> -}
>>>> -
>>>> static void mctl_phy_init(u32 val)
>>>> {
>>>> struct sunxi_mctl_ctl_reg * const mctl_ctl =
>>>> @@ -91,8 +67,9 @@ static void mctl_set_master_priority(void)
>>>> struct sunxi_mctl_com_reg * const mctl_com =
>>>> (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
>>>>
>>>> +#if defined(CONFIG_MACH_SUN8I_H3)
>>>> /* enable bandwidth limit windows and set windows size 1us */
>>>> - writel(0x00010190, &mctl_com->bwcr);
>>>> + writel((1 << 16) | (400 << 0), &mctl_com->bwcr);
>>>>
>>>> /* set cpu high priority */
>>>> writel(0x00000001, &mctl_com->mapr);
>>>> @@ -121,6 +98,38 @@ static void mctl_set_master_priority(void)
>>>> writel(0x04001800, &mctl_com->mcr[10][1]);
>>>> writel(0x04000009, &mctl_com->mcr[11][0]);
>>>> writel(0x00400120, &mctl_com->mcr[11][1]);
>>>> +#elif defined(CONFIG_MACH_SUN50I)
>>>> + /* enable bandwidth limit windows and set windows size 1us */
>>>> + writel(399, &mctl_com->tmr);
>>>> + writel((1 << 16), &mctl_com->bwcr);
>>>> +
>>>> + writel(0x00a0000d, &mctl_com->mcr[0][0]);
>>>> + writel(0x00500064, &mctl_com->mcr[0][1]);
>>>> + writel(0x06000009, &mctl_com->mcr[1][0]);
>>>> + writel(0x01000578, &mctl_com->mcr[1][1]);
>>>> + writel(0x0200000d, &mctl_com->mcr[2][0]);
>>>> + writel(0x00600100, &mctl_com->mcr[2][1]);
>>>> + writel(0x01000009, &mctl_com->mcr[3][0]);
>>>> + writel(0x00500064, &mctl_com->mcr[3][1]);
>>>> + writel(0x07000009, &mctl_com->mcr[4][0]);
>>>> + writel(0x01000640, &mctl_com->mcr[4][1]);
>>>> + writel(0x01000009, &mctl_com->mcr[5][0]);
>>>> + writel(0x00000080, &mctl_com->mcr[5][1]);
>>>> + writel(0x01000009, &mctl_com->mcr[6][0]);
>>>> + writel(0x00400080, &mctl_com->mcr[6][1]);
>>>> + writel(0x0100000d, &mctl_com->mcr[7][0]);
>>>> + writel(0x00400080, &mctl_com->mcr[7][1]);
>>>> + writel(0x0100000d, &mctl_com->mcr[8][0]);
>>>> + writel(0x00400080, &mctl_com->mcr[8][1]);
>>>> + writel(0x04000009, &mctl_com->mcr[9][0]);
>>>> + writel(0x00400100, &mctl_com->mcr[9][1]);
>>>> + writel(0x20000209, &mctl_com->mcr[10][0]);
>>>> + writel(0x08001800, &mctl_com->mcr[10][1]);
>>>> + writel(0x05000009, &mctl_com->mcr[11][0]);
>>>> + writel(0x00400090, &mctl_com->mcr[11][1]);
>>>> +
>>>> + writel(0x81000004, &mctl_com->mdfs_bwlr[2]);
>>>
>>> Where is this pulled from? having some defines would be great..
>>
>> AFAIK this is from register dumps after boot0 has done its job. I am
>> afraid this is as far as we get in the moment. IIRC even the disassembly
>> of boot0 shows that these registers are just initialised with those
>> magic values and are not computed somehow.
>> So the original source code _might_ have names to it, but I guess we
>> will never know.
>
> Can you please make that a comment?
Even better there is a patch from Philipp which actually dissects this
into meaningful fields.
So I looked into the H3 BSP and found the respective code there as well,
so I have now a patch which turns the existing H3 mctl_com->mcr setup
code into something like:
MBUS_CONF(CPU, true, HIGHEST, 0, 160, 100, 80);
....
(with explanations for the numerical parameters as well).
Then this A64 patch will just copy that scheme, and will hopefully make
everyone happy.
Cheers,
Andre.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
