[U-Boot,v3,5/5] Exynos: Split 5250 and 5420 memory bank configuration

From: Michael Pratt <mpratt@chromium.org>

Since snow has a different memory configuration than peach, split the
configuration between the 5250 and 5420. Exynos 5420 supports runtime
memory configuration detection, and can make the determination between 4
and 7 banks at runtime.

Include the bank size with the number of banks for context to make the
number of banks meaningful.

Signed-off-by: Michael Pratt <mpratt@chromium.org>
Signed-off-by: Akshay Saraswat <akshay.s@samsung.com>
Acked-by: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
---
Changes since v2:
	- Added "Acked-by" & "Tested-by".
Changes since v1:
	- New patch.

 include/configs/exynos5-dt.h    | 2 --
 include/configs/exynos5250-dt.h | 5 +++++
 include/configs/exynos5420.h    | 4 ++++
 3 files changed, 9 insertions(+), 2 deletions(-)

Dear Akshay Saraswat,

On 04/06/14 01:17, Akshay Saraswat wrote:
> From: Michael Pratt <mpratt@chromium.org>
> 
> Since snow has a different memory configuration than peach, split the
> configuration between the 5250 and 5420. Exynos 5420 supports runtime
> memory configuration detection, and can make the determination between 4
> and 7 banks at runtime.

I think this patch should be included to your peach-pit patchset.
And I think, the number of banks and the size of bank seems to board specific feature.
Can you guarantee if it uses same SoC then have same memory banks?

> 
> Include the bank size with the number of banks for context to make the
> number of banks meaningful.
> 
> Signed-off-by: Michael Pratt <mpratt@chromium.org>
> Signed-off-by: Akshay Saraswat <akshay.s@samsung.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> Tested-by: Simon Glass <sjg@chromium.org>
> ---
> Changes since v2:
> 	- Added "Acked-by" & "Tested-by".
> Changes since v1:
> 	- New patch.
> 
>  include/configs/exynos5-dt.h    | 2 --
>  include/configs/exynos5250-dt.h | 5 +++++
>  include/configs/exynos5420.h    | 4 ++++
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/include/configs/exynos5-dt.h b/include/configs/exynos5-dt.h
> index d3ef44c..fd607ee 100644
> --- a/include/configs/exynos5-dt.h
> +++ b/include/configs/exynos5-dt.h
> @@ -161,8 +161,6 @@
>  
>  #define CONFIG_RD_LVL
>  
> -#define CONFIG_NR_DRAM_BANKS	8
> -#define SDRAM_BANK_SIZE		(256UL << 20UL)	/* 256 MB */
>  #define PHYS_SDRAM_1		CONFIG_SYS_SDRAM_BASE
>  #define PHYS_SDRAM_1_SIZE	SDRAM_BANK_SIZE
>  #define PHYS_SDRAM_2		(CONFIG_SYS_SDRAM_BASE + SDRAM_BANK_SIZE)
> diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
> index 10b8942..27aa455 100644
> --- a/include/configs/exynos5250-dt.h
> +++ b/include/configs/exynos5250-dt.h
> @@ -65,4 +65,9 @@
>  #define LCD_YRES			1600
>  #define LCD_BPP			LCD_COLOR16
>  #endif
> +
> +/* DRAM Memory Banks */
> +#define CONFIG_NR_DRAM_BANKS	8
> +#define SDRAM_BANK_SIZE		(256UL << 20UL)	/* 256 MB */
> +
>  #endif  /* __CONFIG_5250_H */
> diff --git a/include/configs/exynos5420.h b/include/configs/exynos5420.h
> index 2ffe5ee..d2a9556 100644
> --- a/include/configs/exynos5420.h
> +++ b/include/configs/exynos5420.h
> @@ -45,4 +45,8 @@
>   */
>  #define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_IRAM_TOP - 0x800)
>  
> +/* DRAM Memory Banks */
> +#define CONFIG_NR_DRAM_BANKS	7
> +#define SDRAM_BANK_SIZE		(512UL << 20UL)	/* 512 MB */
> +
>  #endif	/* __CONFIG_EXYNOS5420_H */
> 

Thanks,
Minkyu Kang.

Hi Minkyu,

On 16 June 2014 23:28, Minkyu Kang <mk7.kang@samsung.com> wrote:
> Dear Akshay Saraswat,
>
> On 04/06/14 01:17, Akshay Saraswat wrote:
>> From: Michael Pratt <mpratt@chromium.org>
>>
>> Since snow has a different memory configuration than peach, split the
>> configuration between the 5250 and 5420. Exynos 5420 supports runtime
>> memory configuration detection, and can make the determination between 4
>> and 7 banks at runtime.
>
> I think this patch should be included to your peach-pit patchset.
> And I think, the number of banks and the size of bank seems to board specific feature.
> Can you guarantee if it uses same SoC then have same memory banks?

I think this is better than what we have there at present. There is a
patch from Chromium that puts this in the device tree, but it is
probably best dealt with when more patches have landed.

Regards,
Simon

On 18/06/14 11:11, Simon Glass wrote:
> Hi Minkyu,
> 
> On 16 June 2014 23:28, Minkyu Kang <mk7.kang@samsung.com> wrote:
>> Dear Akshay Saraswat,
>>
>> On 04/06/14 01:17, Akshay Saraswat wrote:
>>> From: Michael Pratt <mpratt@chromium.org>
>>>
>>> Since snow has a different memory configuration than peach, split the
>>> configuration between the 5250 and 5420. Exynos 5420 supports runtime
>>> memory configuration detection, and can make the determination between 4
>>> and 7 banks at runtime.
>>
>> I think this patch should be included to your peach-pit patchset.
>> And I think, the number of banks and the size of bank seems to board specific feature.
>> Can you guarantee if it uses same SoC then have same memory banks?
> 
> I think this is better than what we have there at present. There is a
> patch from Chromium that puts this in the device tree, but it is
> probably best dealt with when more patches have landed.
> 

I didn't deny this patch.
My comment is about present state.
If you can not guarantee that have same memory banks
then please split this configuration to each board's configs.

Thanks,
Minkyu Kang.

Hi Minkyu,

On 17 June 2014 23:30, Minkyu Kang <mk7.kang@samsung.com> wrote:
> On 18/06/14 11:11, Simon Glass wrote:
>> Hi Minkyu,
>>
>> On 16 June 2014 23:28, Minkyu Kang <mk7.kang@samsung.com> wrote:
>>> Dear Akshay Saraswat,
>>>
>>> On 04/06/14 01:17, Akshay Saraswat wrote:
>>>> From: Michael Pratt <mpratt@chromium.org>
>>>>
>>>> Since snow has a different memory configuration than peach, split the
>>>> configuration between the 5250 and 5420. Exynos 5420 supports runtime
>>>> memory configuration detection, and can make the determination between 4
>>>> and 7 banks at runtime.
>>>
>>> I think this patch should be included to your peach-pit patchset.
>>> And I think, the number of banks and the size of bank seems to board specific feature.
>>> Can you guarantee if it uses same SoC then have same memory banks?
>>
>> I think this is better than what we have there at present. There is a
>> patch from Chromium that puts this in the device tree, but it is
>> probably best dealt with when more patches have landed.
>>
>
> I didn't deny this patch.
> My comment is about present state.
> If you can not guarantee that have same memory banks
> then please split this configuration to each board's configs.

For Pit which can support 2GB or 4GB, U-Boot detects the correct size
at run-time. So the setting in the config file is the *maximum* memory
supported by that SOC, which is indeed fixed by the SOC and has
nothing to do with the board.

But i maybe misunderstand what you are getting at?

Regards,
Simon

Dear Simon Glass,

On 18/06/14 15:47, Simon Glass wrote:
> Hi Minkyu,
> 
> On 17 June 2014 23:30, Minkyu Kang <mk7.kang@samsung.com> wrote:
>> On 18/06/14 11:11, Simon Glass wrote:
>>> Hi Minkyu,
>>>
>>> On 16 June 2014 23:28, Minkyu Kang <mk7.kang@samsung.com> wrote:
>>>> Dear Akshay Saraswat,
>>>>
>>>> On 04/06/14 01:17, Akshay Saraswat wrote:
>>>>> From: Michael Pratt <mpratt@chromium.org>
>>>>>
>>>>> Since snow has a different memory configuration than peach, split the
>>>>> configuration between the 5250 and 5420. Exynos 5420 supports runtime
>>>>> memory configuration detection, and can make the determination between 4
>>>>> and 7 banks at runtime.
>>>>
>>>> I think this patch should be included to your peach-pit patchset.
>>>> And I think, the number of banks and the size of bank seems to board specific feature.
>>>> Can you guarantee if it uses same SoC then have same memory banks?
>>>
>>> I think this is better than what we have there at present. There is a
>>> patch from Chromium that puts this in the device tree, but it is
>>> probably best dealt with when more patches have landed.
>>>
>>
>> I didn't deny this patch.
>> My comment is about present state.
>> If you can not guarantee that have same memory banks
>> then please split this configuration to each board's configs.
> 
> For Pit which can support 2GB or 4GB, U-Boot detects the correct size
> at run-time. So the setting in the config file is the *maximum* memory
> supported by that SOC, which is indeed fixed by the SOC and has
> nothing to do with the board.

I see.
Then looks good to me.

Thanks,
Minkyu Kang.