archive mirror
 help / color / mirror / Atom feed
From: Frank Rowand <>
To: Viresh Kumar <>,
	Pantelis Antoniou <>,
	Rob Herring <>
	Vincent Guittot <>,
	Bill Mills <>,, Masahiro Yamada <>
Subject: Re: [PATCH V4 0/3] scripts: dtc: Build fdtoverlay
Date: Tue, 19 Jan 2021 11:12:11 -0600	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 1/12/21 2:28 AM, Viresh Kumar wrote:
> We will start building overlays for platforms soon in the kernel and
> would need fdtoverlay tool going forward. Lets start fetching and
> building it.
> While at it, also remove fdtdump.c file, which isn't used by the kernel.
> V4:
> - Don't fetch and build fdtdump.c
> - Remove fdtdump.c
> Viresh Kumar (3):
>   scripts: dtc: Add fdtoverlay.c to DTC_SOURCE
>   scripts: dtc: Build fdtoverlay tool
>   scripts: dtc: Remove the unused fdtdump.c file
>  scripts/dtc/Makefile             |   6 +-
>  scripts/dtc/fdtdump.c            | 163 -------------------------------
>  scripts/dtc/ |   6 +-
>  3 files changed, 8 insertions(+), 167 deletions(-)
>  delete mode 100644 scripts/dtc/fdtdump.c

My first inclination was to accept fdtoverlay, as is, from the upstream

But my experiences debugging use of fdtoverlay against the existing
unittest overlay files has me very wary of accepting fdtoverlay in
it's current form.

As an exmple, adding an overlay that fails to reply results in the
following build messages:

   linux--5.11-rc> make zImage
   make[1]: Entering directory '/local/frowand_nobackup/src/git_linus/build/dragon_linus_5.11-rc'
     GEN     Makefile
     CALL    /local/frowand_nobackup/src/git_linus/linux--5.11-rc/scripts/
     CALL    /local/frowand_nobackup/src/git_linus/linux--5.11-rc/scripts/atomic/
     CHK     include/generated/compile.h
     FDTOVERLAY drivers/of/unittest-data/static_test.dtb

   Failed to apply 'drivers/of/unittest-data/overlay.dtb': FDT_ERR_NOTFOUND
   make[4]: *** [/local/frowand_nobackup/src/git_linus/linux--5.11-rc/drivers/of/unittest-data/Makefile:96: drivers/of/unittest-data/static_test.dtb] Error 1
   make[3]: *** [/local/frowand_nobackup/src/git_linus/linux--5.11-rc/scripts/ drivers/of/unittest-data] Error 2
   make[2]: *** [/local/frowand_nobackup/src/git_linus/linux--5.11-rc/scripts/ drivers/of] Error 2
   make[1]: *** [/local/frowand_nobackup/src/git_linus/linux--5.11-rc/Makefile:1805: drivers] Error 2
   make[1]: Leaving directory '/local/frowand_nobackup/src/git_linus/build/dragon_linus_5.11-rc'
   make: *** [Makefile:185: __sub-make] Error 2

The specific error message (copied from above) is:

   Failed to apply 'drivers/of/unittest-data/overlay.dtb': FDT_ERR_NOTFOUND

which is cryptic and does not even point to the location in the overlay that
is problematic.  If you look at the source of fdtoverlay / libfdt, you will
find that FDT_ERR_NOTFOUND may be generated in one of many places.

I do _not_ want to do a full review of fdtoverlay, but I think that it is
reasonable to request enhancing fdtoverlay in the parent project to generate
usable error messages before enabling fdtoverlay in the Linux kernel tree.

fdtoverlay in it's current form adds a potential maintenance burden to me
(as the overlay maintainer).  I now have the experience of how difficult it
was to debug the use of fdtoverlay in the context of the proposed patch to
use it with the devicetree unittest overlay .dtb files.


  parent reply	other threads:[~2021-01-19 18:28 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-12  8:28 [PATCH V4 0/3] scripts: dtc: Build fdtoverlay Viresh Kumar
2021-01-12  8:29 ` [PATCH V4 1/3] scripts: dtc: Add fdtoverlay.c to DTC_SOURCE Viresh Kumar
2021-01-19 16:32   ` Frank Rowand
2021-01-12  8:29 ` [PATCH V4 2/3] scripts: dtc: Build fdtoverlay tool Viresh Kumar
2021-01-19 16:37   ` Frank Rowand
2021-01-20 15:47     ` Rob Herring
2021-01-12  8:29 ` [PATCH V4 3/3] scripts: dtc: Remove the unused fdtdump.c file Viresh Kumar
2021-01-19 17:12 ` Frank Rowand [this message]
2021-01-20  5:17   ` [PATCH V4 0/3] scripts: dtc: Build fdtoverlay Viresh Kumar
2021-01-22  6:34     ` David Gibson
2021-01-26  3:42       ` Frank Rowand
2021-02-01  4:07         ` David Gibson
2021-02-03  9:51           ` Viresh Kumar
2021-02-04 14:25           ` Rob Herring
2021-02-22  6:17             ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).