Re: [PATCH v3 6/8] dm: treewide: Complete migration to new driver model schema

[Simon Glass <sjg@chromium.org>]

+Peter Maydell

Hi,

On Mon, 6 Feb 2023 at 07:56, Michal Simek <michal.simek@amd.com> wrote:
>
>
>
> On 2/6/23 15:44, Tom Rini wrote:
> > On Mon, Feb 06, 2023 at 01:22:48PM +0100, Michal Simek wrote:
> >> Hi Simon,
> >>
> >> On 2/1/23 23:54, Simon Glass wrote:
> >>> Update various build and test components to use the new schema.
> >>>
> >>> Signed-off-by: Simon Glass <sjg@chromium.org>
> >>> ---
> >>>
> >>> (no changes since v1)
> >>>
> >>>    drivers/core/ofnode.c            | 10 +++++-----
> >>>    drivers/video/video-uclass.c     |  4 ++--
> >>>    dts/Kconfig                      |  2 +-
> >>>    include/dm/device.h              |  2 +-
> >>>    include/dm/ofnode.h              | 10 +++++-----
> >>>    scripts/Makefile.lib             | 12 ++++++------
> >>>    test/dm/test-fdt.c               |  2 +-
> >>>    test/py/tests/test_ofplatdata.py |  8 ++++----
> >>>    tools/binman/binman.rst          |  3 +--
> >>>    tools/dtoc/test_fdt.py           |  8 ++++----
> >>>    10 files changed, 30 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> >>> index 4d56b1a7675..5249a60639b 100644
> >>> --- a/drivers/core/ofnode.c
> >>> +++ b/drivers/core/ofnode.c
> >>> @@ -1265,22 +1265,22 @@ bool ofnode_pre_reloc(ofnode node)
> >>>    {
> >>>    #if defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)
> >>>     /* for SPL and TPL the remaining nodes after the fdtgrep 1st pass
> >>> -    * had property dm-pre-reloc or u-boot,dm-spl/tpl.
> >>> +    * had property bootph-all or bootph-pre-sram/bootph-pre-ram.
> >>>      * They are removed in final dtb (fdtgrep 2nd pass)
> >>>      */
> >>>     return true;
> >>>    #else
> >>> -   if (ofnode_read_bool(node, "u-boot,dm-pre-reloc"))
> >>> +   if (ofnode_read_bool(node, "bootph-all"))
> >>>             return true;
> >>> -   if (ofnode_read_bool(node, "u-boot,dm-pre-proper"))
> >>> +   if (ofnode_read_bool(node, "bootph-some-ram"))
> >>>             return true;
> >>>     /*
> >>>      * In regular builds individual spl and tpl handling both
> >>>      * count as handled pre-relocation for later second init.
> >>>      */
> >>> -   if (ofnode_read_bool(node, "u-boot,dm-spl") ||
> >>> -       ofnode_read_bool(node, "u-boot,dm-tpl"))
> >>> +   if (ofnode_read_bool(node, "bootph-pre-ram") ||
> >>> +       ofnode_read_bool(node, "bootph-pre-sram"))
> >>>             return true;
> >>
> >> Please correct me if I am wrong but this change will likely break all boards
> >> which didn't migrate to this at this stage. And because targeting early
> >> stages people will be without console.
> >> I think we should have transition period for 1-2 releases to give people
> >> enough time to migrate. It means print big warning that they have to migrate
> >> their DTS.
> >
> > What's the migration case here we're missing? Is it platforms that
> > maintain a dts externally, via tooling / etc, that populate those nodes?
>
> Yes and I expect there will be a lot of DTs around with some changes for
> specific products.
>
> Also for example QEMU is also generating DT based on it's configuration and
> provide it to U-Boot.
> https://gitlab.com/qemu-project/qemu/-/blob/master/hw/arm/xlnx-versal-virt.c#L91
> When this patch is applied CI loop should fail for Versal.

I am not sure how it helps us to drag this out. It is a breaking
change, but a drawn-out process is just going to create a lot of
confusion. People should be free to use the schema in Linux .dts files
from now on, but if it is not immediately supported in U-Boot then
they cannot. This is the most important point, after all.

For QEMU, where to start... I already sent a patch to fix this
problem[1]. I spent considerable effort and time developing it and
trying to explain the need for it and why it is wrong for QEMU to
assume it knows what needs to be sent to U-Boot. The QEMU maintainer
took no real interest in it, just suggesting a workaround or an
impractical solution in U-Boot. There was even support from
saomLinaro, to no avail. I hate to be proved right, but QEMU seems to
have taken a middle path...not accepting the patch but instead
accepting the now 'old' U-Boot tags. So I will reiterate that QEMU
should accept that patch as it will solve a lot of problems, providing
an 'escape route' for people to fix problems without recompiling QEMU.
Failing that, time will heal.

Regards,
SImon

[1] https://patchwork.kernel.org/project/qemu-devel/patch/20210926183410.256484-1-sjg@chromium.org/#24474809