All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Seiderer <ps.report@gmx.net>
To: buildroot@busybox.net
Subject: [Buildroot] [RFC/next v2 1/2] package/rpi-firmware: rework boot/config file handling
Date: Thu, 18 Mar 2021 23:25:48 +0100	[thread overview]
Message-ID: <20210318232548.2bb4a716@gmx.net> (raw)
In-Reply-To: <20210309212942.GP2737665@scaer>

Hello Yann,

On Tue, 9 Mar 2021 22:29:42 +0100, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Peter, All,
>
> On 2021-03-09 21:32 +0100, Peter Seiderer spake thusly:
> > On Mon, 8 Mar 2021 23:27:53 +0100, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > > On 2021-03-08 23:14 +0100, Peter Seiderer spake thusly:
> > > > On Mon, 8 Mar 2021 22:55:41 +0100, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > > > > On 2021-02-16 21:11 +0100, Peter Seiderer spake thusly:
> > > > > > Try to be less smart (focused on the one target/one use-case),
> > > > > > instead reduce the rpi-firmware package to a selectable list
> > > > > > of (verbatim) installed firmware files.
> > > [--SNIP--]
> > > > > So, let's keep the qt5 example aside, and see what we can do. What if
> > > > > we'd go with the following:
> > > > >
> > > > >   - keep a single, generic config.txt in rpi-firmware, and install that,
> > > > >
> > > > >   - if BR2_aarch64=y, add arm_64bit=1
> > > > >
> > > > >   - add an option to rpi-firmware:
> > > > >
> > > > >     config BR2_PKG_RPI_FW_DTOVERLAY_LIST
> > > > >         string "DT overlays to load"
> > > > >         depends on BR2_PACKAGE_RPI_FIRMWARE_INSTALL_DTB_OVERLAYS
> > > > >         help
> > > > >           Space-separated list of DT overlays to be loaded from config.txt.
> > > > >
> > > > >           For example, miniuart-bt on rpi0w to restore the serial console.
> > > > >
> > > > >     and if this option is not empty, construct the list of dtoverlay and
> > > > >     add it to config.txt.
> > > > >
> > > > >   - eventually, adapt the defconfigs with that list of dtoverlays
> [--SNIP--]
> > More flexible than the possibility for a custom config.txt (mind that there
> > are overlays with parameters and more possible options besides the overlays)?
> [--SNIP--]
> > The part with 'override' feels a little bit ugly and would count for me as
> > an argument against the hard-coded logic/overlays-only approach, still
> > believing that sooner or later most/every beyond-buildroot-defconfig-only
> > user will use a hand-crafted config.txt file...
>
> OK, so here's my new proposal:
>
>   - keep your BR2_PACKAGE_RPI_FIRMWARE_CONFIG_FILE, which points to a
>     default, basic one, not unlike the busybox default config:
>         default "package/rpi-firmware/config.txt"
>
>   - package/rpi-firmware/config.txt is just the common part of all the
>     config.txt you currently had in your patch
>
>   - rpi-firmware.mk will add arm_64bit=1 as needed, based on
>     BR2_aarch64=y, because that *really* is not an option.
>
>   - add BR2_PKG_RPI_FW_DTOVERLAY_LIST as I suggest above, which is not
>     empty, will be copied as is to dtoverlay
>
>   - change the defconfig files to just set:
>     BR2_PKG_RPI_FW_DTOVERLAY_LIST="miniuart-bt"
>
> That way, we get best of both worlds:
>
>   - we avoid duplication of the config.txt, and we can still customise
>     it a bit with just "easy stuff" that we need for our example
>     defconfigs,
>
>   - users can stil point to their custom, fine-tuned config.txt (in
>     which case they will probably not set BR2_PKG_RPI_FW_DTOVERLAY_LIST)

Finally found time for a new iteration of the patch set:

- fixed two bugs
- changed the firmware list handling to one-assignment style
- changed the firmware install to make-level foreach loop
- changed the dtb overlay install to make-level foreach loop

- kept the verbatim BR2_PACKAGE_RPI_FIRMWARE_CONFIG_FILE option,
  as your suggestion missed one more point where the config.txt
  files diverge (aside from dtoverlay=miniuart-bt and arm_64bit=1),
  the start_file=start.elf/fixup_file=fixup.dat or start_file=start4.elf/
  fixup_file=fixup4.dat as the firmware files are now copied verbatim...,
  and still confident that the principal of taking a verbatim copy of
  a given file is more flexible than implementing some more or less smart
  logic...

Regards,
Peter


>
> Thoughts?
>
> Regards,
> Yann E. MORIN.
>

  parent reply	other threads:[~2021-03-18 22:25 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-16 20:11 [Buildroot] [RFC/next v2 1/2] package/rpi-firmware: rework boot/config file handling Peter Seiderer
2021-02-16 20:11 ` [Buildroot] [RFC/next v2 2/2] package/rpi-firmware: add option for the debug set of start/fixup boot files Peter Seiderer
2021-02-17  9:21 ` [Buildroot] [RFC/next v2 1/2] package/rpi-firmware: rework boot/config file handling Stefan Agner
2021-03-08 21:55 ` Yann E. MORIN
2021-03-08 22:14   ` Peter Seiderer
2021-03-08 22:27     ` Yann E. MORIN
2021-03-09 20:32       ` Peter Seiderer
2021-03-09 21:29         ` Yann E. MORIN
2021-03-09 21:35           ` Yann E. MORIN
2021-03-18 22:25           ` Peter Seiderer [this message]
2021-03-20 22:09             ` Yann E. MORIN
2021-03-21 12:00               ` Peter Seiderer
2021-03-08 22:04 ` Yann E. MORIN
2021-03-09 20:25   ` Peter Seiderer
2021-03-09 20:33     ` Yann E. MORIN

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20210318232548.2bb4a716@gmx.net \
    --to=ps.report@gmx.net \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.