Re: [PATCH 4/4] microblaze: remove unnecessary system.dts

[Michal Simek]

On 5/13/25 06:50, Masahiro Yamada wrote:
> On Mon, Feb 3, 2025 at 8:17 PM Michal Simek <monstr@xxxxxxxxx> wrote:
> > On 2/1/25 04:42, Masahiro Yamada wrote:
> > > On Sat, Feb 1, 2025 at 7:25 AM Rob Herring <robh@xxxxxxxxxx> wrote:
> > > > On Tue, Jan 14, 2025 at 12:15 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
> > > > > The default image linux.bin does not contain any DTB, but a separate
> > > > > system.dtb is compiled.
> > > > >
> > > > > Michal Simek clearly explained "system.dtb is really old dtb more for
> > > > > demonstration purpose and nothing else and likely it is not working on
> > > > > any existing board." [1]
> > > > >
> > > > > The system.dts is not necessary even for demonstration purposes. There
> > > > > is no need to compile out-of-tree *.dts under arch/microblaze/boot/dts/
> > > > > unless it is embedded into the kernel. Users can directly use dtc.
> > > > >
> > > > > [1]: https://lore.kernel.org/all/d2bdfbfd-3721-407f-991e-566d48392add@xxxxxxx/
> > > > >
> > > > > Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > > > > ---
> > > > >
> > > > >    arch/microblaze/boot/dts/Makefile   |   3 +-
> > > > >    arch/microblaze/boot/dts/system.dts | 353 ----------------------------
> > > > >    2 files changed, 1 insertion(+), 355 deletions(-)
> > > > >    delete mode 100644 arch/microblaze/boot/dts/system.dts
> > > > >
> > > > > diff --git a/arch/microblaze/boot/dts/Makefile b/arch/microblaze/boot/dts/Makefile
> > > > > index 932dc7550a1b..fa0a6c0854ca 100644
> > > > > --- a/arch/microblaze/boot/dts/Makefile
> > > > > +++ b/arch/microblaze/boot/dts/Makefile
> > > > > @@ -1,8 +1,6 @@
> > > > >    # SPDX-License-Identifier: GPL-2.0
> > > > >    #
> > > > >
> > > > > -dtb-y := system.dtb
> > > > > -
> > > > >    ifneq ($(DTB),)
> > > > >    obj-y += linked_dtb.o
> > > > >
> > > > > @@ -11,6 +9,7 @@ $(obj)/linked_dtb.o: $(obj)/system.dtb
> > > > >
> > > > >    # Generate system.dtb from $(DTB).dtb
> > > > >    ifneq ($(DTB),system)
> > > >
> > > > Can't this be dropped as setting DTB=system.dtb should work if there's
> > > > not an in-tree system.dts anymore.
> > >
> > > I believe this ifneq is necessary, just in case
> > > a user adds system.dtb to arch/microblaze/boot/dts/.
> > >
> > > 'system.dtb' is a special name because
> > > arch/microblaze/boot/dts/linked_dtb.S wraps it.
> > >
> > > So, $(DTB) is copied to system.dtb, and then
> > > it is wrapped by linked_dtb.S.
> > >
> > > If $(DTB) is already 'system',
> > > we cannot copy system.dtb to itself.
> > >
> > >
> > > See the definition of cmd_copy in scripts/Makefile.lib
> > >
> > > cmd_copy = cat $< > $@
> > >
> > >
> > > "cat system.dtb > system.dtb"
> > > would create an empty system.dtb
> >
> > I have played with this and pretty much this patch is blocking
> > simpleImage.system build target.
> >
> > I have no issue with patches 1-3 and I would keep system.dts as empty and keep
> > it in the tree because users (including me) just rewrite system.dts with proper
> > DTS and call make simpleImage.system.
>
> Why is "system" so special?
>
> You hard-code this line:
>      dtb-y := system.dtb
>
>
> "make simpleImage.system" compiles system.dts to system.dtb

yes.



> However,
>
> "make simpleImage.foo" does not compile foo.dts to foo.dtb
> since "dtb-y := foo.dtb" is missing.

Correct but foo.dts is not in the source code too.
Downstream repos can add multiple dtses to source code based on configurations 
which are supporting and then they add

dtb-y += foo.dtb

there.

> This works only if you drop-in a pre-compiled foo.dtb

as above. It is up to users to decide how they want to do it.

If they add dts to source they have to also add a rule. If they want to just use 
DTB then can just copy it there to get it work.

Another option is also just overwrite system.dts by custom dts file.


> "make simpleImage.<name>" works only when <name> is "system".

With upstream repo yes.
And the reason is that system.dts is just example.
In Microblaze systems you can have unlimited amount of configurations that's why 
make no sense to start to push DTSes to the kernel because it is only supporting 
one particular configuration on one board.

Thanks,
Michal