[U-Boot] [PATCH 0/3] Add generic FDT memory bank decoding and gd initialization

[Igor Grinberg]

On 12/12/16 10:27, Michal Simek wrote:
> On 12.12.2016 09:24, Igor Grinberg wrote:
>> On 12/12/16 10:02, Michal Simek wrote:
>>> On 12.12.2016 08:13, Nathan Rossi wrote:
>>>> On 12 December 2016 at 03:11, Igor Grinberg <grinberg at compulab.co.il> wrote:
>>>>> dropping the list for this one as the question seems to me irrelevant to your patch set.
>>>>>
>>>>> On 12/11/16 18:47, Nathan Rossi wrote:
>>>>>> On 12 December 2016 at 01:08, Igor Grinberg <grinberg at compulab.co.il> wrote:
>>>>>>> Hi Nathan,
>>>>>>>
>>>>>>> On 12/11/16 15:58, Nathan Rossi wrote:
>>>>>>>> This series adds two functions for handling the memory bank decoding and
>>>>>>>> initialization of global data for use by boards in their dram_init and
>>>>>>>> dram_init_banksize functions.
>>>>>>>
>>>>>>> I might have missed some discussions on this meter,
>>>>>>> can you please provide the use cases for this?
>>>>>>> IMO, the bootloader's job is to initialize the DRAM, detect its size, and pass
>>>>>>> the detected DRAM configuration on to an OS.
>>>>>>
>>>>>> Hi Igor,
>>>>>>
>>>>>> I do not think there have been any discussions on this (at least none
>>>>>> that I am aware of).
>>>>>>
>>>>>> Some boards (like Zynq and ZynqMP ones) are using
>>>>>> CONFIG_SYS_SDRAM_SIZE to define the amount of memory that is available
>>>>>> (since detection is not possible).
>>>>>
>>>>> May I ask why is detection not possible on these boards (may be SoCs)?
>>>>
>>>> That is correct, Zynq and ZynqMP are ARM SoCs. Which in the majority
>>>> of cases are used in boards which have fixed memory device setups
>>>> (without any SPD or equivalent).
>>>
>>> For example zcu102 zynqmp development board is using modules with SPD on
>>> it but if you look at generic SPD support then you will find out that
>>> FSL(drivers/ddr/fsl) is one of the major platform which is using it for
>>> ddr size detection.
>>> Anyway as Nathan wrote above the majority of boards with zynq/zynqmp
>>> devices need to be configured at build time or at run time by DTB.
>>>
>>> There is also topic.nl boards which contain ddr configuration the same
>>> for different ddr sizes and Mike sent this patch to get it work
>>> http://lists.denx.de/pipermail/u-boot/2016-November/273385.html
>>
>> That's exactly my point. I think Mike's patch does a way better job
>> detecting at run time than any compiled in or a DT based pseudo detection...
>>
>>>
>>> Anyway in general there are some ways how to configure DDR size
>>> - build time setup (CONFIG_SYS_SDRAM*)
>>> - run time setup
>>> 	- ddr detection
>>> 		- via SPD (like FSL)
>>> 		- via algorithm (like topic.nl)
>>> 	- configuration
>>> 		- read DTB
>>>
>>> Nathan's path is trying to address last run time DTB configuration
>>> method to be shared across platforms because similar stuff Uniphier is
>>> using too. And it doesn't make sense to copy that functions everywhere
>>> that's why this should go core parts.
>>
>> Yep. That's exactly what I thought. I see Nathan's patch set as an
>> improvement of the current situation anyway.
>>
>> Also I think Mike's patch does a much better job on this.
>>
> 
> Just keep in your mind just in case that you know that your
> configuration supports it. If you don't have DDR connected to hard block
> and you use ddr to PL you don't even know where to look for DDR.
> And with arm64 it is pretty huge space.

I see this as exactly the type of information that should be provided by
the board code or a dtb.

> It means we really have to provide ways how to do it and these patches
> are just generic way how to do it via reading DTB instead of code
> duplication (+ bug) which we have now.

No doubt, Nathan's patch improves the situation.

-- 
Regards,
Igor.