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

Tue Dec 13 13:08:06 CET 2016

On 12/13/16 13:07, Michal Simek wrote:
> On 13.12.2016 09:53, Igor Grinberg wrote:
>> On 12/12/16 13:56, Michal Simek wrote:
>>> On 12.12.2016 12:10, Igor Grinberg wrote:
>>>> On 12/12/16 11:03, Michal Simek wrote:
>>>>> On 12.12.2016 09:54, Igor Grinberg wrote:
>>>>>> 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.
>>>>>
>>>>> I tend to remove all board files for zynq/zynqmp targets. Will see how
>>>>> we can do it in future.
>>>>
>>>> This is not really related to current thread...
>>>
>>> not directly but there is a connection.
>>>
>>>> I'm not sure how this can be done... We're in a bootloader world here...
>>>> It should be board specific by definition...
>>>> There should be board specific code (not static data) somewhere, and
>>>> I think we had agreed a year or so ago... on this meter.
>>>> I think if you go this way, we will end up having board specific middleware
>>>> all around... and lots of tools for changing the static data (e.g. dtbs).
>>>
>>> Look at microblaze. I am not accepting any board to be added for it
>>> because every configuration is different.
>>
>> I'm not sure I get your point.
>>
>>> The same can be done for
>>> zynq/zynqmp boards when we move stuff to DM.
>>> Then for supporting new platform you don't need to create folder in
>>> board and file include/configs/ and all you need is dts file and
>>> defconfig. In case of SPL for us we need to also put somewhere ps7*/psu*
>>> init files.
>>
>> Not exactly...
>> This can be true if you force all vendors to copy the ref design...
>> I believe you can't do this.
>> There are many boards out there that copy the reference design and this
>> will work with them, but there are also boards who don't follow the ref design
>> and this will only push them back from upstream...
>>
>>> DT is design to describe hardware and current configuration which I
>>> believe we can.
>>
>> Yes. Only static data...
> 
> Static initial configuration - yes.
> 
>>
>>> There is also DT overlay stuff which has been merged recently.
>>
>> This does improve the situation, but you still need code for detection and
>> and loading of the overlays.
> 
> You need to manage this process. In most cases I don't think that there
> will be auto detection option in place. And it will be up to user to
> decide what to do.

If you let the board provide its logic for auto detection, there certainly
will be auto detection. I don't think that users should mess with
this stuff.
We have plenty of users who barely can follow our documentation...

> 
> 
>>> All this together is going to end up in situation that you will have
>>> generic hardcoded config for core stuff and the rest in DTS file.
>>
>> You will always have boards (e.g. SoMs) that have its different wiring
>> and requires dynamic code. Unless you force everyone to ref design, but
>> what's the point then... ARMs strength is in diversity and flexibility.
> 
> It is just about how to decide to sort this and handle it.
> Different combinations end up in sort of static configuration and
> definitely if you have an option to detect stuff at run time you can use
> it.
> 
>>> In connection to this thread. If you have all ddr start/size algorithms
>>> prepared then you can probably select them via defconfig because static
>>> part is board specific which means it should go to Kconfig/defconfig.
>>
>> That might work...
>> From my humble experience, I have never had a board with the same
>> algorithm used in between SoC generations and even different algorithms
>> on the same SoC generation but different boards.
> 
> 
> TBH maybe we are just don't understand each other properly. I will be
> the last person
> not to move to run-time case. We do fpgas and I don't know any other
> chips which are more capable in run time configurations then fpgas.
> We have partial reconfiguration in place if you want change HW at run time.
> And this patch is simply just trying to improve one particular problem
> which is loading memory configuration from DTB about DDR setting.

As already said, I think this patch is an improvement.
But I wouldn't like people to think that this is the direction that
everybody should take.

> If there are other ways how to detect DDR size that's different topic.
> If we can combine detection algorithms together that's also something
> what we can discuss.

Yep. That's great with me!

> 
> I am not marketing guy to say how often there are designs with only
> different DDR size like Mike's example but in fpga world you are not
> buying this chip to have only static part but you want to use fpga part
> and for that you need to use design tools. Because every design is
> unique you can generate device tree description directly from design
> tools which covers your target and this is what I believe people use.

Sort of...
Additionally, there are SoMs that are a kind of partial design as it is not
a final product.
I think that is exactly, why there should be a kind of limited freedom
for board to provides its stuff.

-- 
Regards,
Igor.