lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAK7LNAQqR7rtuCSy895q-WeJR-uhVB_0UH1DrGPNLoz1_cRhtg@mail.gmail.com>
Date: Tue, 13 May 2025 13:50:22 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Michal Simek <monstr@...str.eu>
Cc: Rob Herring <robh@...nel.org>, "Simek, Michal" <michal.simek@....com>, linux-kernel@...r.kernel.org, 
	Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH 4/4] microblaze: remove unnecessary system.dts

On Mon, Feb 3, 2025 at 8:17 PM Michal Simek <monstr@...str.eu> wrote:
>
>
>
> On 2/1/25 04:42, Masahiro Yamada wrote:
> > On Sat, Feb 1, 2025 at 7:25 AM Rob Herring <robh@...nel.org> wrote:
> >>
> >> On Tue, Jan 14, 2025 at 12:15 PM Masahiro Yamada <masahiroy@...nel.org> 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@amd.com/
> >>>
> >>> Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>
> >>> ---
> >>>
> >>>   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


However,

"make simpleImage.foo" does not compile foo.dts to foo.dtb
since "dtb-y := foo.dtb" is missing.
This works only if you drop-in a pre-compiled foo.dtb


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

Is this what you mean?







-- 
Best Regards
Masahiro Yamada

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ