From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Giulio Benetti <giulio.benetti@benettiengineering.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>, buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 000/102] Standardize/beautify genimage .cfg files for all boards
Date: Mon, 8 Nov 2021 16:07:31 +0100 [thread overview]
Message-ID: <20211108150731.GD680793@scaer> (raw)
In-Reply-To: <20211107015835.3055951-1-giulio.benetti@benettiengineering.com>
Guilio, All,
(I dropped the Cc list, but Peter, Thomas, and Arnout.)
On 2021-11-07 02:58 +0100, Giulio Benetti spake thusly:
> This patchset aims to set a standard for all genimage .cfg files in
> Buildroot since at the moment various formats are used.
I know this as been discussed on another thread with Peter, about using
K/M instead of bare numbers, and it was my understanding that if a
change were to be made, it would be restricted to just that.
However, I question the relevance of this *huge* patchset just for a
makeover.
Yes, usually, I like consistency, but here we have a two-edge issue:
- the series is huge, and causes a lot of churn making pending patches
more difficult to apply,
- the genimage format is not well defined, and there is no linter, so
the existing files *will* diverge again, and we won't notice that
new files won't be compliant.
We know the files will diverge again, because that already hapenned with
our python scripts, even though we did have a check running flake8. So,
for genimage files without a linter? No way they can't and won't...
> First of all I've changed every double space into tab, and in general tab
> becomes the separator in place of space that more than sometimes is used
> mixed with tabs.
I usually l̶o̶̶a̶t̶h̶ d̶e̶s̶p̶i̶s̶e̶ h̶a̶t̶e̶ do not like TABs... The width of a TAB
varies depending on one's environment and coding practices; the usual
8-space width makes for very long, ugly lines; TAB break carefull
indentation.
> There are some space, newline left, no newlines after nodes while
> newlines after other.
> All the offsets and sizes are specified in a mixed way of bytes number,
> K, M, G, hexadecimal.
> So basically these file has never had a standard. Even if a real standard
> is not really defined I ask you to accept this long patchset to improve
> consistency and to ease the readibility of offsets and sizes.
Arguably, one should easily recognise powers-of-two, especially in our
field! ;-]
And especially since 'K' (uppercase) is for Kelvin [0], while kilo is
'k' (lowercase) [1].
Besides, strictly speaking, 'k' is 1000 [1], while 'ki' is 1024 [2], so
seeing 10k can be more confusing than 10240 (but that's not your fault
if genimage accepts 'k' as a power of 1024 ;-) ).
[0] https://en.wikipedia.org/wiki/SI_base_unit
[1] https://en.wikipedia.org/wiki/International_System_of_Units#Prefixes
[2] https://en.wikipedia.org/wiki/Binary_prefix
> I've created a patch per file instead of "topic" like "missing space",
> "convert from space to tab" etc. because it was a big mix of everything
> in more than one file.
And then this gave a 107-patch series. And the commits all have the same
typo: 'beatify' instead of 'beautify'. Although I can forgive a typo
(who would I be not to?), and this one was funny one, fixing all those
manually when applying will be a tedious task, though, so you would have
to do that on your side and respin.
Also, maybe it would be worth categorising by type of board, like all
qemu configs at once, all rpi at once, and so on, with a final commit
with all the remaining un-categorised boards, maybe...
But before you do so, please wait a bit for others to speak up, if this
series is really meaningful.
> I ask, can I add a section in Documentation where to explain how to write
> a "standard" genimage .cfg file? That way there is a guideline.
Yes, please add a section about best practices (and note that existing
files may not yet be up-to-date with those new best practices). Make
that the first patch in the series.
Regards,
Yann E. MORIN.
> Best regards
>
> Giulio Benetti (102):
> board/aarch64-efi/genimage.cfg: beautify file
> board/acmesystems/acqua-a5/genimage.cfg: beautify file
> board/acmesystems/aria-g25/genimage.cfg: beautify file
> board/acmesystems/arietta-g25/genimage.cfg: beautify file
> board/altera/socrates_cyclone5/genimage.cfg: beautify file
> board/amarula/a64-relic/genimage.cfg: beautify file
> board/atmel/at91sam9x5ek_mmc/genimage.cfg: beautify file
> board/atmel/sama5d27_som1_ek_mmc/genimage.cfg: beautify file
> board/atmel/sama5d27_wlsom1_ek_mmc/genimage.cfg: beautify file
> board/atmel/sama5d2_xplained_mmc/genimage.cfg: beautify file
> board/atmel/sama5d3_xplained_mmc/genimage.cfg: beautify file
> board/atmel/sama5d4_xplained_mmc/genimage.cfg: beautify file
> board/bananapi/bananapi-m1/genimage.cfg: beautify file
> board/bananapi/bananapi-m2-ultra/genimage.cfg: beautify file
> board/bananapi/bananapi-m2-zero/genimage.cfg: beautify file
> board/bananapi/bananapi-m64/genimage.cfg: beautify file
> board/beagleboardx15/genimage.cfg: beautify file
> board/beaglebone/genimage.cfg: beautify file
> board/beagleboneai/genimage.cfg: beautify file
> board/beaglev/genimage.cfg: beautify file
> board/beelink/gs1/genimage.cfg: beautify file
> board/ci20/genimage.cfg: beautify file
> board/cubietech/cubieboard2/genimage.cfg: beautify file
> board/embest/riotboard/genimage.cfg: beautify file
> board/engicam/geam6ul/genimage.cfg: beautify file
> board/engicam/icorem6/genimage.cfg: beautify file
> board/engicam/icorem6_rqs/genimage.cfg: beautify file
> board/engicam/isiot/genimage.cfg: beautify file
> board/friendlyarm/nanopc-t4/genimage.cfg: beautify file
> board/friendlyarm/nanopi-a64/genimage.cfg: beautify file
> board/friendlyarm/nanopi-m1-plus/genimage.cfg: beautify file
> board/friendlyarm/nanopi-m1/genimage.cfg: beautify file
> board/friendlyarm/nanopi-m4/genimage.cfg: beautify file
> board/friendlyarm/nanopi-neo-plus2/genimage.cfg: beautify file
> board/friendlyarm/nanopi-neo/genimage.cfg: beautify file
> board/friendlyarm/nanopi-neo2/genimage.cfg: beautify file
> board/friendlyarm/nanopi-r1/genimage.cfg: beautify file
> board/grinn/chiliboard/genimage.cfg: beautify file
> board/grinn/liteboard/genimage.cfg: beautify file
> board/hardkernel/odroidc2/genimage.cfg: beautify file
> board/hardkernel/odroidxu4/genimage.cfg: beautify file
> board/intel/galileo/genimage.cfg: beautify file
> board/lego/ev3/genimage.cfg: beautify file
> board/lemaker/bananapro/genimage.cfg: beautify file
> board/licheepi/genimage.cfg: beautify file
> board/linksprite/pcduino/genimage.cfg: beautify file
> board/microchip/sam9x60ek_mmc/genimage.cfg: beautify file
> board/microchip/sama5d2_icp/genimage.cfg: beautify file
> board/minnowboard/genimage.cfg: beautify file
> board/nexbox/a95x/genimage.cfg: beautify file
> board/olimex/a13_olinuxino/genimage.cfg: beautify file
> board/olimex/a20_olinuxino/genimage.cfg: beautify file
> board/olimex/a33_olinuxino/genimage.cfg: beautify file
> board/olimex/a64_olinuxino/genimage.cfg: beautify file
> board/orangepi/orangepi-lite/genimage.cfg: beautify file
> board/orangepi/orangepi-lite2/genimage.cfg: beautify file
> board/orangepi/orangepi-one-plus/genimage.cfg: beautify file
> board/orangepi/orangepi-one/genimage.cfg: beautify file
> board/orangepi/orangepi-pc-plus/genimage.cfg: beautify file
> board/orangepi/orangepi-pc/genimage.cfg: beautify file
> board/orangepi/orangepi-pc2/genimage.cfg: beautify file
> board/orangepi/orangepi-plus/genimage.cfg: beautify file
> board/orangepi/orangepi-prime/genimage.cfg: beautify file
> board/orangepi/orangepi-r1/genimage.cfg: beautify file
> board/orangepi/orangepi-rk3399/genimage.cfg: beautify file
> board/orangepi/orangepi-win/genimage.cfg: beautify file
> board/orangepi/orangepi-zero-plus/genimage.cfg: beautify file
> board/orangepi/orangepi-zero-plus2/genimage.cfg: beautify file
> board/orangepi/orangepi-zero/genimage.cfg: beautify file
> board/pandaboard/genimage.cfg: beautify file
> board/pc/genimage-bios.cfg: beautify file
> board/pc/genimage-efi.cfg: beautify file
> board/pine64/pine64/genimage.cfg: beautify file
> board/pine64/rock64/genimage.cfg: beautify file
> board/pine64/rockpro64/genimage.cfg: beautify file
> board/pine64/sopine/genimage.cfg: beautify file
> board/qemu/aarch64-sbsa/genimage.cfg: beautify file
> board/radxa/rockpi-4/genimage.cfg: beautify file
> board/radxa/rockpi-n10/genimage.cfg: beautify file
> board/radxa/rockpi-n8/genimage.cfg: beautify file
> board/raspberrypi/genimage-raspberrypi.cfg: beautify file
> board/raspberrypi/genimage-raspberrypi0.cfg: beautify file
> board/raspberrypi/genimage-raspberrypi0w.cfg: beautify file
> board/raspberrypi/genimage-raspberrypi2.cfg: beautify file
> board/raspberrypi/genimage-raspberrypi3-64.cfg: beautify file
> board/raspberrypi/genimage-raspberrypi3.cfg: beautify file
> board/raspberrypi/genimage-raspberrypi4-64.cfg: beautify file
> board/raspberrypi/genimage-raspberrypi4.cfg: beautify file
> board/raspberrypi/genimage-raspberrypicm4io-64.cfg: beautify file
> board/raspberrypi/genimage-raspberrypicm4io.cfg: beautify file
> board/roseapplepi/genimage.cfg: beautify file
> board/sinovoip/m1-plus/genimage.cfg: beautify file
> board/sinovoip/m2-plus/genimage.cfg: beautify file
> board/solidrun/macchiatobin/genimage.cfg: beautify file
> board/stmicroelectronics/stm32f469-disco/genimage.cfg: beautify file
> board/synopsys/hsdk/genimage.cfg: beautify file
> board/technologic/ts7680/genimage.cfg: beautify file
> board/terasic/de10nano_cyclone5/genimage.cfg: beautify file
> board/toradex/apalis-imx6/genimage.cfg: beautify file
> board/uevm5432/genimage.cfg: beautify file
> board/zynq/genimage.cfg: beautify file
> board/zynqmp/genimage.cfg: beautify file
>
> board/aarch64-efi/genimage-efi.cfg | 45 +++++++-------
> board/acmesystems/acqua-a5/genimage.cfg | 1 +
> board/acmesystems/aria-g25/genimage.cfg | 1 +
> board/acmesystems/arietta-g25/genimage.cfg | 7 ++-
> board/altera/socrates_cyclone5/genimage.cfg | 5 +-
> board/amarula/a64-relic/genimage.cfg | 1 +
> board/atmel/at91sam9x5ek_mmc/genimage.cfg | 1 +
> board/atmel/sama5d27_som1_ek_mmc/genimage.cfg | 1 +
> .../atmel/sama5d27_wlsom1_ek_mmc/genimage.cfg | 1 +
> board/atmel/sama5d2_xplained_mmc/genimage.cfg | 1 +
> board/atmel/sama5d3_xplained_mmc/genimage.cfg | 1 +
> board/atmel/sama5d4_xplained_mmc/genimage.cfg | 1 +
> board/bananapi/bananapi-m1/genimage.cfg | 5 +-
> board/bananapi/bananapi-m2-ultra/genimage.cfg | 5 +-
> board/bananapi/bananapi-m2-zero/genimage.cfg | 5 +-
> board/bananapi/bananapi-m64/genimage.cfg | 5 +-
> board/beagleboardx15/genimage.cfg | 3 +-
> board/beaglebone/genimage.cfg | 3 +-
> board/beagleboneai/genimage.cfg | 1 +
> board/beaglev/genimage.cfg | 18 +++---
> board/beelink/gs1/genimage.cfg | 5 +-
> board/ci20/genimage.cfg | 44 +++++++-------
> board/cubietech/cubieboard2/genimage.cfg | 5 +-
> board/embest/riotboard/genimage.cfg | 26 ++++----
> board/engicam/geam6ul/genimage.cfg | 3 +-
> board/engicam/icorem6/genimage.cfg | 3 +-
> board/engicam/icorem6_rqs/genimage.cfg | 3 +-
> board/engicam/isiot/genimage.cfg | 3 +-
> board/friendlyarm/nanopc-t4/genimage.cfg | 60 +++++++++----------
> board/friendlyarm/nanopi-a64/genimage.cfg | 5 +-
> board/friendlyarm/nanopi-m1-plus/genimage.cfg | 5 +-
> board/friendlyarm/nanopi-m1/genimage.cfg | 5 +-
> board/friendlyarm/nanopi-m4/genimage.cfg | 2 +-
> .../friendlyarm/nanopi-neo-plus2/genimage.cfg | 5 +-
> board/friendlyarm/nanopi-neo/genimage.cfg | 5 +-
> board/friendlyarm/nanopi-neo2/genimage.cfg | 5 +-
> board/friendlyarm/nanopi-r1/genimage.cfg | 5 +-
> board/grinn/chiliboard/genimage.cfg | 43 ++++++-------
> board/grinn/liteboard/genimage.cfg | 59 +++++++++---------
> board/hardkernel/odroidc2/genimage.cfg | 3 +-
> board/hardkernel/odroidxu4/genimage.cfg | 17 +++---
> board/intel/galileo/genimage.cfg | 3 +-
> board/lego/ev3/genimage.cfg | 18 ++++--
> board/lemaker/bananapro/genimage.cfg | 5 +-
> board/licheepi/genimage.cfg | 5 +-
> board/linksprite/pcduino/genimage.cfg | 5 +-
> board/microchip/sam9x60ek_mmc/genimage.cfg | 1 +
> board/microchip/sama5d2_icp/genimage.cfg | 1 +
> board/minnowboard/genimage.cfg | 1 +
> board/nexbox/a95x/genimage.cfg | 1 +
> board/olimex/a13_olinuxino/genimage.cfg | 32 +++++-----
> board/olimex/a20_olinuxino/genimage.cfg | 4 +-
> board/olimex/a33_olinuxino/genimage.cfg | 4 +-
> board/olimex/a64-olinuxino/genimage.cfg | 5 +-
> board/orangepi/orangepi-lite/genimage.cfg | 5 +-
> board/orangepi/orangepi-lite2/genimage.cfg | 5 +-
> board/orangepi/orangepi-one-plus/genimage.cfg | 5 +-
> board/orangepi/orangepi-one/genimage.cfg | 5 +-
> board/orangepi/orangepi-pc-plus/genimage.cfg | 5 +-
> board/orangepi/orangepi-pc/genimage.cfg | 5 +-
> board/orangepi/orangepi-pc2/genimage.cfg | 5 +-
> board/orangepi/orangepi-plus/genimage.cfg | 5 +-
> board/orangepi/orangepi-prime/genimage.cfg | 5 +-
> board/orangepi/orangepi-r1/genimage.cfg | 5 +-
> board/orangepi/orangepi-rk3399/genimage.cfg | 2 +-
> board/orangepi/orangepi-win/genimage.cfg | 5 +-
> .../orangepi/orangepi-zero-plus/genimage.cfg | 5 +-
> .../orangepi/orangepi-zero-plus2/genimage.cfg | 5 +-
> board/orangepi/orangepi-zero/genimage.cfg | 4 +-
> board/pandaboard/genimage.cfg | 1 +
> board/pc/genimage-bios.cfg | 36 ++++++-----
> board/pc/genimage-efi.cfg | 49 +++++++--------
> board/pine64/pine64/genimage.cfg | 5 +-
> board/pine64/rock64/genimage.cfg | 36 +++++------
> board/pine64/rockpro64/genimage.cfg | 2 +-
> board/pine64/sopine/genimage.cfg | 5 +-
> board/qemu/aarch64-sbsa/genimage.cfg | 42 ++++++-------
> board/radxa/rockpi-4/genimage.cfg | 2 +-
> board/radxa/rockpi-n10/genimage.cfg | 2 +-
> board/radxa/rockpi-n8/genimage.cfg | 2 +-
> board/raspberrypi/genimage-raspberrypi.cfg | 51 ++++++++--------
> board/raspberrypi/genimage-raspberrypi0.cfg | 47 ++++++++-------
> board/raspberrypi/genimage-raspberrypi0w.cfg | 49 +++++++--------
> board/raspberrypi/genimage-raspberrypi2.cfg | 47 ++++++++-------
> .../raspberrypi/genimage-raspberrypi3-64.cfg | 53 ++++++++--------
> board/raspberrypi/genimage-raspberrypi3.cfg | 53 ++++++++--------
> .../raspberrypi/genimage-raspberrypi4-64.cfg | 47 ++++++++-------
> board/raspberrypi/genimage-raspberrypi4.cfg | 47 ++++++++-------
> .../genimage-raspberrypicm4io-64.cfg | 47 ++++++++-------
> .../raspberrypi/genimage-raspberrypicm4io.cfg | 47 ++++++++-------
> board/roseapplepi/genimage.cfg | 12 ++--
> board/sinovoip/m1-plus/genimage.cfg | 5 +-
> board/sinovoip/m2-plus/genimage.cfg | 5 +-
> board/solidrun/macchiatobin/genimage.cfg | 4 +-
> .../stm32f469-disco/genimage.cfg | 4 +-
> board/synopsys/hsdk/genimage.cfg | 42 ++++++-------
> board/technologic/ts7680/genimage.cfg | 2 +-
> board/terasic/de10nano_cyclone5/genimage.cfg | 1 +
> board/toradex/apalis-imx6/genimage.cfg | 14 ++---
> board/uevm5432/genimage.cfg | 1 +
> board/zynq/genimage.cfg | 2 +
> board/zynqmp/genimage.cfg | 1 +
> 102 files changed, 716 insertions(+), 625 deletions(-)
>
> --
> 2.25.1
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
next prev parent reply other threads:[~2021-11-08 15:07 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-07 1:58 [Buildroot] [PATCH 000/102] Standardize/beautify genimage .cfg files for all boards Giulio Benetti
2021-11-07 1:58 ` [Buildroot] [PATCH 001/102] board/aarch64-efi/genimage.cfg: beautify file Giulio Benetti
2021-11-07 1:58 ` [Buildroot] [PATCH 002/102] board/acmesystems/acqua-a5/genimage.cfg: " Giulio Benetti
2021-11-08 15:07 ` Yann E. MORIN [this message]
2021-11-08 15:24 ` [Buildroot] [PATCH 000/102] Standardize/beautify genimage .cfg files for all boards Thomas Petazzoni
2021-11-08 16:17 ` Giulio Benetti
2021-11-08 22:08 ` Arnout Vandecappelle
2021-11-08 16:01 ` Giulio Benetti
2021-11-08 18:38 ` Arnout Vandecappelle
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=20211108150731.GD680793@scaer \
--to=yann.morin.1998@free.fr \
--cc=buildroot@buildroot.org \
--cc=giulio.benetti@benettiengineering.com \
--cc=thomas.petazzoni@bootlin.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox