On Fri, Jan 06, 2023 at 04:14:08PM -0500, Tom Rini wrote:
On Fri, Jan 06, 2023 at 09:22:56PM +0100, Pali Rohár wrote:
On Friday 06 January 2023 12:25:24 Tom Rini wrote:
On Fri, Jan 06, 2023 at 05:45:43PM +0100, Pali Rohár wrote:
On Friday 06 January 2023 10:51:43 Tom Rini wrote:
This reverts commit 777aaaa706bcfe08c284aed06886db7d482af3f8.
The changes to this generic function, which is intended to help with 32bit platforms with large amounts of memory has unintended side effects (which in turn lead to boot failures) on other platforms which were previously functional.
As mentioned previously, unfortunately this revert breaks 32-bit u-boot on 36-bit mpc85xx boards with 32-bit e500v2 cores and 4GB DDR module.
Which platforms currently have broken u-boot without this revert? The only one which was reported is stm32mp but for it there different workaround patch waiting in the queue.
Are you able to test on one of these PowerPC platforms currently? As the stm32 problem shows, not everything is getting tested frequently enough, so how many other cases are lurking out there. And, I think overall issue is that the overflow check-and-change you introduce here should just be in the CONFIG_MAX_MEM_MAPPED==true case. As that's the case you're dealing with, yes?
I was planning to do big retest again after all powerpc patches are reviewed and merged...
Yes, but can you test one of them now, to see if my idea works?
Anyway, if the issue here is with ram_size and its reduction was needed for mpc85xx (at the time of introduction of that patch), what about putting mpc85xx ifdef around ram_size reduction? For arm boards it would have same behavior as revert of that commit and for mpc85xx it would be no change.
I agree that this code needs to be revisited, together with ram_top issue and also code which fills DDR banks. Because really mapped memory for u-boot and real size of DDR are two different things here.
The issue here is that we see two now (given Fabio's reminder about another thread I had forgotten) of unintended consequences, on 32bit platforms trying to normally have 2GB of memory, which does not require special treatment.
What I'm leaning towards right now even, is that since it's hard to test the non-36bit platforms that do set CONFIG_MAX_MEM_MAPPED, to see if their behavior also changed here, the 36bit platforms should just be overriding get_effective_memsize.