Re: [PATCH u-boot] powerpc/mpc85xx: socrates: Re-enable building u-boot-socrates.bin

[Heiko Schocher <hs@denx.de>]

Hello Pali,

On 31.12.22 16:37, Pali Rohár wrote:
> On Saturday 31 December 2022 16:31:57 Heiko Schocher wrote:
>> Hello Pali,
>>
>> On 31.12.22 14:36, Heiko Schocher wrote:
>>> Hello Pali,
>>>
>>> On 31.12.22 13:58, Pali Rohár wrote:
>>>> On Saturday 31 December 2022 10:36:07 Heiko Schocher wrote:
>>>>> Hello Pali,
>>>>>
>>>>> On 28.12.22 19:18, Pali Rohár wrote:
>>>>>> U-Boot build system builds final U-Boot binary for socrates board in custom
>>>>>> file u-boot-socrates.bin (instead of standard u-boot.bin). Output target
>>>>>> file u-boot-socrates.bin is generated by binman as defined in board binman
>>>>>> config file arch/powerpc/dts/socrates-u-boot.dtsi.
>>>>>>
>>>>>> But binman was disabled in commit 5af42eafd7e1 ("Makefile: Reduce usage of
>>>>>> custom mpc85xx u-boot.bin target") for all mpc85xx boards which do not use
>>>>>> standard powerpc binman config file arch/powerpc/dts/u-boot.dtsi and boards
>>>>>> which do not require binman at all.
>>>>>>
>>>>>> The only such mpc85xx board is socrates. So since that commit, U-Boot does
>>>>>> not final binary for socrates board anymore.
>>>>>>
>>>>>> Fix this issue by re-enabling binman for socrates board. And build process
>>>>>> starts again producing u-boot-socrates.bin binary.
>>>>>>
>>>>>> Note that build process for this socrates board always produce u-boot.bin
>>>>>> binary which is broken and not usable for socrates board. Long term
>>>>>> solution should be to disable building broken binary u-boot.bin and then
>>>>>> renaming u-boot-socrates.bin to u-boot.bin, or switching to use common
>>>>>> powerpc binman config file arch/powerpc/dts/socrates-u-boot.dtsi (if it is
>>>>>> possible).
>>>>>>
>>>>>> Fixes: 5af42eafd7e1 ("Makefile: Reduce usage of custom mpc85xx u-boot.bin target")
>>>>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>>>>> ---
>>>>>> Heiko Schocher: Could you test if u-boot is still working on this board?
>>>>>>
>>>>>> Tom Rini: Cannot be this issue handled by CI? For example that CI check
>>>>>> build process produce required output binaries?
>>>>>> ---
>>>>>>  arch/powerpc/cpu/mpc85xx/Kconfig | 1 +
>>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> With this patch, u-boot-socrates.bin is build again, so yes...
>>>>>
>>>>> Tested-by: Heiko Schocher <hs@denx.de>
>>>>>
>>>>> ... but current u-boot does not boot anymore on this board ... I have to
>>>>> dig into, obvious difference I see in hexdump is:
>>>>>
>>>>> old (2022.01) u-boot:
>>>>> """
>>>>> 00001930  74 65 00 6f 66 66 73 65  74 00 73 74 64 6f 75 74  |te.offset.stdout|
>>>>> 00001940  2d 70 61 74 68 00 ff ff  ff ff ff ff ff ff ff ff  |-path...........|
>>>>> 00001950  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
>>>>> *
>>>>> 00020000  27 05 19 56 3c 60 e4 01  60 63 3f 10 38 63 fb f0  |'..V<`..`c?.8c..|
>>>>> 00020010  3c 80 e4 01 60 84 40 00  38 00 00 00 38 84 ff fc  |<...`.@.8...8...|
>>>>> 00020020  90 04 00 00 7c 04 18 40  40 82 ff f4 3c 80 e4 01  |....|..@@...<...|
>>>>>
>>>>> """
>>>>>
>>>>> New
>>>>> """
>>>>> 00001930  74 65 00 6f 66 66 73 65  74 00 73 74 64 6f 75 74  |te.offset.stdout|
>>>>> 00001940  2d 70 61 74 68 00 ff ff  ff ff ff ff ff ff ff ff  |-path...........|
>>>>> 00001950  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
>>>>> *
>>>>> 00020000  3c 60 e4 01 60 63 3f 10  38 63 fb f0 3c 80 e4 01  |<`..`c?.8c..<...|
>>>>> 00020010  60 84 40 00 38 00 00 00  38 84 ff fc 90 04 00 00  |`.@.8...8.......|
>>>>> 00020020  7c 04 18 40 40 82 ff f4  3c 80 e4 01 60 84 3f 20  ||..@@...<...`.? |
>>>>>
>>>>> """
>>>>>
>>>>> So "U-Boot magic" is misssing ...
>>>>
>>>> It was removed in commit 2dcf776ebcf7 ("powerpc: mpc85xx: Drop _start symbol").
>>>> Was it used for something?
>>>
>>> I think (hope) not!
>>>
>>>>> reset vector at end of image is for both the same:
>>>>>
>>>>> 000bfff0  ff ff ff ff ff ff ff ff  ff ff ff ff 4b ff f0 04  |............K...|
>>>>> 000c0000
>>>>
>>>> 4b ff f0 04 is ppc branch instruction pos-0xffc, so to offset 0xbf000
>>>> in dumped file (not available in the output). I think this is correct.
>>>
>>> Yes, this is correct.
>>>
>>>>
>>>>> I have to dig deeper into it, to find out what have changed in the meantime,
>>>>> (Think I start a "git bisect") just find some more time for it...
>>>>
>>>> I think that git bisect would be needed to investigate what is the
>>>> problematic commit.
>>>>
>>>
>>> Just bisecting it ... and commit:
>>> """
>>> commit 985503439762c3168aeb80f529bb9bbcd773dd2c
>>> Author: Simon Glass <sjg@chromium.org>
>>> Date:   Thu Dec 16 20:59:31 2021 -0700
>>>
>>>     fdt: Don't call board_fdt_blob_setup() without OF_BOARD
>>> """
>>>
>>> poped up ... so I enabled CONFIG_OF_BOARD for socrates build to get
>>> board specific function board_fdt_blob_setup() again called, and
>>> with this change board boots again, based on this commit!
>>>
>>> But ... building current head:
>>> 3089d12a02efd1dc5dce01e0ec0fda9142693b11
>>>
>>> with this change, again, no u-boot output ...so there is a next "git bisect"
>>> round necessary. I have to stop now, else I get an angry wife ... will continue
>>> next week... hopefully with a good result...
>>>
>>> Have a good slide to the new year!
>>
>> Next git bisect round shows up:
>> """
>> commit be7dbb60c5bfa38ea444fe7de1dca8bd35f83f5b (refs/bisect/bad)
>> Author: Tom Rini <trini@konsulko.com>
>> Date:   Sun Dec 12 22:12:30 2021 -0500
>>
>>     Convert CONFIG_SYS_IMMR to Kconfig
>> """
>>
>> and yes, config symbol:
>>
>> CONFIG_SYS_IMMR=0xff700000
>>
>> is crap for socrates... you fixed this already in mainline with
>> """
>> commit 39f42fe20a8239c6a878f7fac03e758b2117009e
>> Author: Pali Rohár <pali@kernel.org>
>> Date:   Mon May 2 18:29:25 2022 +0200
>>
>>     powerpc: mpc85xx: Set default SYS_IMMR value for P1/P2 CPUs
>>
>>     This reduce usage of per-board custom settings.
>> """
> 
> This commit fixed SYS_IMMR only for ARCH_P1* and ARCH_P2*. Socrates is
> ARCH_MPC8544, so I'm not sure if that my commit really fixed it for
> MPC8544. Please recheck current master that CONFIG_SYS_IMMR is set to
> the same value as in old u-boot version where board worked fine.
> 
> SYS_IMMR should be set to the CCSR address *after* CCSR relocation.
> And also check SYS_CCSRBAR_DEFAULT that is is CCSR address *befere* CCSR
> relocation (it should be reset address).

Yes, I already checked it, looked fine. Will do a new bisect soon.

bye,
Heiko
> 
>> but with current HEAD, board still does not boot, CONFIG_SYS_IMMR is correct...
>>
>> puh... so I have to git bisect in a third round with correcting IMMR
>> value between your fix and commit be7dbb60c5
>>
>> bye,
>> Heiko
>>
>>>
>>> bye,
>>> Heiko
>>>
>>>
>>>>> Nevertheless, I think, this patch can go in...
>>>>>
>>>>> bye,
>>>>> Heiko
>>>>>
>>>>> -- 
>>>>> DENX Software Engineering GmbH,      Managing Director: Erika Unter
>>>>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>>>>> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de
>>>
>>>
>>> bye,
>>> Heiko
>>>
>>
>> -- 
>> DENX Software Engineering GmbH,      Managing Director: Erika Unter
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de