RE: [EXT] Re: [EXT] Re: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup additional vc created nodes
Hi,
Cool. Thanks for the trigger.
Works fine on my side.
You can add my
Tested-by: Denis Osterland-Heim
Regards, Denis
-Original Message-
From: Ahmad Fatoum
Sent: Tuesday, March 5, 2024 11:34 AM
To: Denis OSTERLAND-HEIM ; Roland Hieber
; Denis Osterland-Heim
Cc: barebox@lists.infradead.org; Alexander Dahl
Subject: [EXT] Re: [EXT] Re: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup
additional vc created nodes
[EXTERNAL EMAIL]
On 29.02.24 07:03, Denis OSTERLAND-HEIM wrote:
> Hi,
>
> Well, the kernel in root A may have a different device tree, than the
> one in root B.
> This is the kernel version update case.
> What would break, depends on the changes to the dt.
Hmm, ok. I just sent a patch that should support both my and your use case.
Cheers,
Ahmad
>
> Regards, Denis
>
> -Original Message-
> From: Ahmad Fatoum
> Sent: Monday, February 26, 2024 1:39 PM
> To: Denis OSTERLAND-HEIM ; Roland Hieber
> ; Denis Osterland-Heim
> Cc: barebox@lists.infradead.org; Alexander Dahl
> Subject: [EXT] Re: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup
> additional vc created nodes
>
> [EXTERNAL EMAIL]
>
>
> Hello Denis,
>
> On 21.02.24 08:46, Denis OSTERLAND-HEIM wrote:
>> Hi,
>>
>> It took me a while to think about it.
>> First I would like to explain my current boot process and then how I
>> think it would like with your suggestion.
>>
>>
>> Now:
>>
>> Disk layout:
>> - rpi boot fat: dt-2nd.img, barebox-dts, rpi-elf, config.txt
>> - root A
>> - root B
>>
>> Boot process:
>> - rpi boot loader
>> * load dt-2nd.img and dt
>> * read Hat Eeprom
>> * apply fixups to dt
>> - barebox with dt from r2
>> * bootchooser select A or B
>> * load kernel and dt from A or B
>> * copy stuff to kernel dt from barebox internal dt as fixups
>> - linux boot
>>
>>
>> Your approach:
>>
>> Disk layout:
>> - rpi boot fat: bb-rpi.img, some-dts, rpi-elf, config.txt
>> - root A
>> - root B
>>
>> Boot process:
>> - rpi boot loader
>> * load bb-rpi.img and dt
>> * read Hat Eeprom
>> * apply fixups to dt
>> - barebox with built-in dt
>> * bootchooser select A or B
>> * load kernel and dt from A or B
>> * copy stuff to kernel dt from /vc.dtb as fixups
>
> This I don't understand. Why not use vc.dtb as kernel DT? You can
> still have barebox apply boot arg fixups and so on, but use /vc.dtb as
base?
>
>> - linux boot
>>
>> I can remember that I tried to implement the fixups as script in the
>> environment first, but switched to C somewhen.
>> I can not recall the reason, sorry.
>> I guess it was related to recursive copy.
>> The C code uses of_merge_node or something like that, which is not
>> available as command, I think.
>
> This can be made available to shell too if there's use for it, but I
> think this is tangential.
>
>> If we can keep the patches an just do something like if /vc.dtb then
>> apply fixups, I would be fine with your approach.
>> But I guess this almost exactly matches the `if(!IS_ERR(root))
>> rpi_vc_fdt_parse(root);` approach.
>> If you just revert the patches, I guess I would have to find a way to
>> do it in script.
>
> What would break if you used vc.dtb as kernel DT?
>
> Thanks,
> Ahmad
>
>>
>> Regards, Denis
>>
>> -Original Message-
>> From: Ahmad Fatoum
>> Sent: Tuesday, February 20, 2024 4:33 PM
>> To: Denis OSTERLAND-HEIM ; Roland Hieber
>> ; Denis Osterland-Heim
>>
>> Cc: barebox@lists.infradead.org; Alexander Dahl
>> Subject: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup additional
>> vc created nodes
>>
>> [EXTERNAL EMAIL]
>>
>>
>> Hello Denis,
>>
>> On 20.02.24 09:16, Denis OSTERLAND-HEIM wrote:
>>> Hi,
>>>
>>> I think so, too.
>>> I think that my mistake was in 5ea6e19737e10973ce2cf785970e32562d9ee8f1.
>>
>> Yes.
>>
>>>> @@ -379,17 +381,17 @@ static void rpi_vc_fdt(void)
>>>>if (oftree->totalsize)
>>>>pr_err("there was an error copying fdt in pbl:
>>> %d\n",
>>>>be32_to_cpu(oftree->totalsize));
>>>> - return;
>>> This return previously avoided a call of rpi_vc_fdt_parse().
>>>
>>
>> [snip]
>>
>>>>rpi_env_init();
>>>> - rpi_vc_fdt();
>>>>
Re: [EXT] Re: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup additional vc created nodes
On 29.02.24 07:03, Denis OSTERLAND-HEIM wrote:
> Hi,
>
> Well, the kernel in root A may have a different device tree, than the one in
> root B.
> This is the kernel version update case.
> What would break, depends on the changes to the dt.
Hmm, ok. I just sent a patch that should support both my and your use case.
Cheers,
Ahmad
>
> Regards, Denis
>
> -Original Message-
> From: Ahmad Fatoum
> Sent: Monday, February 26, 2024 1:39 PM
> To: Denis OSTERLAND-HEIM ; Roland Hieber
> ; Denis Osterland-Heim
> Cc: barebox@lists.infradead.org; Alexander Dahl
> Subject: [EXT] Re: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup
> additional vc created nodes
>
> [EXTERNAL EMAIL]
>
>
> Hello Denis,
>
> On 21.02.24 08:46, Denis OSTERLAND-HEIM wrote:
>> Hi,
>>
>> It took me a while to think about it.
>> First I would like to explain my current boot process and then how I
>> think it would like with your suggestion.
>>
>>
>> Now:
>>
>> Disk layout:
>> - rpi boot fat: dt-2nd.img, barebox-dts, rpi-elf, config.txt
>> - root A
>> - root B
>>
>> Boot process:
>> - rpi boot loader
>> * load dt-2nd.img and dt
>> * read Hat Eeprom
>> * apply fixups to dt
>> - barebox with dt from r2
>> * bootchooser select A or B
>> * load kernel and dt from A or B
>> * copy stuff to kernel dt from barebox internal dt as fixups
>> - linux boot
>>
>>
>> Your approach:
>>
>> Disk layout:
>> - rpi boot fat: bb-rpi.img, some-dts, rpi-elf, config.txt
>> - root A
>> - root B
>>
>> Boot process:
>> - rpi boot loader
>> * load bb-rpi.img and dt
>> * read Hat Eeprom
>> * apply fixups to dt
>> - barebox with built-in dt
>> * bootchooser select A or B
>> * load kernel and dt from A or B
>> * copy stuff to kernel dt from /vc.dtb as fixups
>
> This I don't understand. Why not use vc.dtb as kernel DT? You can still have
> barebox apply boot arg fixups and so on, but use /vc.dtb as base?
>
>> - linux boot
>>
>> I can remember that I tried to implement the fixups as script in the
>> environment first, but switched to C somewhen.
>> I can not recall the reason, sorry.
>> I guess it was related to recursive copy.
>> The C code uses of_merge_node or something like that, which is not
>> available as command, I think.
>
> This can be made available to shell too if there's use for it, but I think
> this is tangential.
>
>> If we can keep the patches an just do something like if /vc.dtb then
>> apply fixups, I would be fine with your approach.
>> But I guess this almost exactly matches the `if(!IS_ERR(root))
>> rpi_vc_fdt_parse(root);` approach.
>> If you just revert the patches, I guess I would have to find a way to
>> do it in script.
>
> What would break if you used vc.dtb as kernel DT?
>
> Thanks,
> Ahmad
>
>>
>> Regards, Denis
>>
>> -Original Message-
>> From: Ahmad Fatoum
>> Sent: Tuesday, February 20, 2024 4:33 PM
>> To: Denis OSTERLAND-HEIM ; Roland Hieber
>> ; Denis Osterland-Heim
>> Cc: barebox@lists.infradead.org; Alexander Dahl
>> Subject: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup additional vc
>> created nodes
>>
>> [EXTERNAL EMAIL]
>>
>>
>> Hello Denis,
>>
>> On 20.02.24 09:16, Denis OSTERLAND-HEIM wrote:
>>> Hi,
>>>
>>> I think so, too.
>>> I think that my mistake was in 5ea6e19737e10973ce2cf785970e32562d9ee8f1.
>>
>> Yes.
>>
@@ -379,17 +381,17 @@ static void rpi_vc_fdt(void)
if (oftree->totalsize)
pr_err("there was an error copying fdt in pbl:
>>> %d\n",
be32_to_cpu(oftree->totalsize));
- return;
>>> This return previously avoided a call of rpi_vc_fdt_parse().
>>>
>>
>> [snip]
>>
rpi_env_init();
- rpi_vc_fdt();
+ root = rpi_vc_fdt();
+ rpi_vc_fdt_parse(IS_ERR(root) ? priv->dev->device_node : root);
>>> Now rpi_vc_fdt_parse() is called in both cases.
>>> So, it should be:
>>> if (!IS_ERR(root))
>>> rpi_vc_fdt_parse(root);
rpi_set_kernel_name();
>>> ...
>>>
>>> Or do I miss something?
>>
>> Now that I think of it, I think the commit should just be reverted.
>> I don't see the utility of using barebox-dt-2nd.img on the Raspberry Pi:
>>
>> - If the board is already supported, use barebox-raspberry-pi.img, which
>> has the DT built-in.
>>
>> - If the board is not supported , use barebox-raspberry-pi.img, which
>> will take the outside DT and save it where the board code expects it.
>>
>> What do you think?
>>
>> Thanks,
>> Ahmad
>>
>>>
>>> Regards, Denis
>>>
>>> -Original Message-
>>> From: Ahmad Fatoum
>>> Sent: Monday, February 19, 2024 10:43 PM
>>> To: Roland Hieber ; Denis Osterland-Heim
>>>
>>> Cc: barebox@lists.infradead.org; Denis OSTERLAND-HEIM
>>> ; Alexander Dahl
>>> Subject: [EXT] Re: [PATCH v2 2/2] raspi: fixup additional vc created
>>> nodes
>>>
>>> [EXTERNAL EMAIL]
>>>
>>>
>>> Hello Roland,
>>>
>>> On 19.02.24 20:14, Roland Hieber wrote:
Hi,
RE: [EXT] Re: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup additional vc created nodes
Hi,
Well, the kernel in root A may have a different device tree, than the one in
root B.
This is the kernel version update case.
What would break, depends on the changes to the dt.
Regards, Denis
-Original Message-
From: Ahmad Fatoum
Sent: Monday, February 26, 2024 1:39 PM
To: Denis OSTERLAND-HEIM ; Roland Hieber
; Denis Osterland-Heim
Cc: barebox@lists.infradead.org; Alexander Dahl
Subject: [EXT] Re: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup
additional vc created nodes
[EXTERNAL EMAIL]
Hello Denis,
On 21.02.24 08:46, Denis OSTERLAND-HEIM wrote:
> Hi,
>
> It took me a while to think about it.
> First I would like to explain my current boot process and then how I
> think it would like with your suggestion.
>
>
> Now:
>
> Disk layout:
> - rpi boot fat: dt-2nd.img, barebox-dts, rpi-elf, config.txt
> - root A
> - root B
>
> Boot process:
> - rpi boot loader
> * load dt-2nd.img and dt
> * read Hat Eeprom
> * apply fixups to dt
> - barebox with dt from r2
> * bootchooser select A or B
> * load kernel and dt from A or B
> * copy stuff to kernel dt from barebox internal dt as fixups
> - linux boot
>
>
> Your approach:
>
> Disk layout:
> - rpi boot fat: bb-rpi.img, some-dts, rpi-elf, config.txt
> - root A
> - root B
>
> Boot process:
> - rpi boot loader
> * load bb-rpi.img and dt
> * read Hat Eeprom
> * apply fixups to dt
> - barebox with built-in dt
> * bootchooser select A or B
> * load kernel and dt from A or B
> * copy stuff to kernel dt from /vc.dtb as fixups
This I don't understand. Why not use vc.dtb as kernel DT? You can still have
barebox apply boot arg fixups and so on, but use /vc.dtb as base?
> - linux boot
>
> I can remember that I tried to implement the fixups as script in the
> environment first, but switched to C somewhen.
> I can not recall the reason, sorry.
> I guess it was related to recursive copy.
> The C code uses of_merge_node or something like that, which is not
> available as command, I think.
This can be made available to shell too if there's use for it, but I think
this is tangential.
> If we can keep the patches an just do something like if /vc.dtb then
> apply fixups, I would be fine with your approach.
> But I guess this almost exactly matches the `if(!IS_ERR(root))
> rpi_vc_fdt_parse(root);` approach.
> If you just revert the patches, I guess I would have to find a way to
> do it in script.
What would break if you used vc.dtb as kernel DT?
Thanks,
Ahmad
>
> Regards, Denis
>
> -Original Message-
> From: Ahmad Fatoum
> Sent: Tuesday, February 20, 2024 4:33 PM
> To: Denis OSTERLAND-HEIM ; Roland Hieber
> ; Denis Osterland-Heim
> Cc: barebox@lists.infradead.org; Alexander Dahl
> Subject: [EXT] Re: [EXT] Re: [PATCH v2 2/2] raspi: fixup additional vc
> created nodes
>
> [EXTERNAL EMAIL]
>
>
> Hello Denis,
>
> On 20.02.24 09:16, Denis OSTERLAND-HEIM wrote:
>> Hi,
>>
>> I think so, too.
>> I think that my mistake was in 5ea6e19737e10973ce2cf785970e32562d9ee8f1.
>
> Yes.
>
>>> @@ -379,17 +381,17 @@ static void rpi_vc_fdt(void)
>>> if (oftree->totalsize)
>>> pr_err("there was an error copying fdt in pbl:
>> %d\n",
>>> be32_to_cpu(oftree->totalsize));
>>> - return;
>> This return previously avoided a call of rpi_vc_fdt_parse().
>>
>
> [snip]
>
>>> rpi_env_init();
>>> - rpi_vc_fdt();
>>> + root = rpi_vc_fdt();
>>> + rpi_vc_fdt_parse(IS_ERR(root) ? priv->dev->device_node : root);
>> Now rpi_vc_fdt_parse() is called in both cases.
>> So, it should be:
>> if (!IS_ERR(root))
>> rpi_vc_fdt_parse(root);
>>> rpi_set_kernel_name();
>>>
>> ...
>>
>> Or do I miss something?
>
> Now that I think of it, I think the commit should just be reverted.
> I don't see the utility of using barebox-dt-2nd.img on the Raspberry Pi:
>
> - If the board is already supported, use barebox-raspberry-pi.img, which
> has the DT built-in.
>
> - If the board is not supported , use barebox-raspberry-pi.img, which
> will take the outside DT and save it where the board code expects it.
>
> What do you think?
>
> Thanks,
> Ahmad
>
>>
>> Regards, Denis
>>
>> -Original Message-
>> From: Ahmad Fatoum
>> Sent: Monday, February 19, 2024 10:43 PM
>> To: Roland Hieber ; Denis Osterland-Heim
>>
>> Cc: barebox@lists.infradead.org; Denis OSTERLAND-HEIM
>> ; Alexander Dahl
>> Subject: [EXT] Re: [PATCH v2 2/2] raspi: fixup additional vc created
>> nodes
>>
>> [EXTERNAL EMAIL]
>>
>>
>> Hello Roland,
>>
>> On 19.02.24 20:14, Roland Hieber wrote:
>>> Hi,
>>>
>>> On Mon, Sep 25, 2023 at 01:10:05PM +0200, Denis Osterland-Heim wrote:
From: Denis OSTERLAND-HEIM
The video core creates some additional nodes.
This code takes over this values.
The /hat node is only there if an raspi hat with EEPROM is detected.
Signed-off-by: Denis
