All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Niklas Cassel <niklas.cassel@wdc.com>,
	Damien Le Moal <damien.lemoal@wdc.com>,
	Mark Corbin <mark@dibsco.co.uk>,
	Jonathan Ben Avraham <yba@tkos.co.il>,
	buildroot@buildroot.org,
	Giulio Benetti <giulio.benetti@benettiengineering.com>,
	Romain Naour <romain.naour@gmail.com>,
	Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Subject: Re: [Buildroot] [PATCH v2 4/4] arch: rework MMU option handling and move to "Target architecture" menu
Date: Wed, 27 Jul 2022 11:22:07 +0200	[thread overview]
Message-ID: <20220727112207.309f4ec4@windsurf> (raw)
In-Reply-To: <20220727083020.GD1085273@scaer>

Hello Yann,

Thanks for the review!

On Wed, 27 Jul 2022 10:30:20 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> I am not sure why you are removing a generic option in favour of two
> arch-specific options that behave exactly the same.

The motivation is explained in the commit log. Quoting it

================================================================
However, if we simply move it in arch/Config.in, it means that we
would have the following order of options:

 Target architecture
 Target architecture variant
 ABI
 MMU
 Binary format

But really, the MMU option should be right below the Target
architecture variant, and the available ABIs derived from that.
================================================================

> Why not just keep that:
> 
>     config BR2_USE_MMU
>         bool "MMU support" if BR2_ARCH_HAS_MMU_OPTIONAL
>         default y if BR2_ARCH_HAS_MMU_OPTIONAL
> 
> This still achieves what you want:
>   - arch/variants that have a mandatory MMU can directly select
>     BR2_USE_MMU as this patch does, and the prompt does not appear
>     in that case;
>   - arch/variants that have no MMU don't select BR2_USE_MMU, and
>     there is no prompt either
>   - arch/variants that have an optrional MMU select
>     BR2_ARCH_HAS_MMU_OPTIONAL, BR2_USE_MMU is default y, and there
>     is a prompt.
> 
> And then we drop one Kconfig knob (MMU_MANDATORY), and do not need to
> introduce two others (one for riscv and one for xtensa).
> 
> I.e. the case for arch-specific options is not clear to me...

See above :-)

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2022-07-27  9:22 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-26 16:39 [Buildroot] [PATCH v2 1/4] arch/Config.in.riscv: lp64f ABI is only supported if MMU is enabled Thomas Petazzoni via buildroot
2022-07-26 16:39 ` [Buildroot] [PATCH v2 2/4] package/Makefile.in: add detection for the lack of C library Thomas Petazzoni via buildroot
2022-07-27  7:58   ` Yann E. MORIN
2022-07-27  8:24     ` Thomas Petazzoni via buildroot
2022-08-29 20:15   ` Peter Korsgaard
2022-07-26 16:39 ` [Buildroot] [PATCH v2 3/4] arch/Config.in: move the binary format selection further down Thomas Petazzoni via buildroot
2022-07-27  7:58   ` Yann E. MORIN
2022-07-26 16:39 ` [Buildroot] [PATCH v2 4/4] arch: rework MMU option handling and move to "Target architecture" menu Thomas Petazzoni via buildroot
2022-07-27  7:03   ` Damien Le Moal via buildroot
2022-07-27  8:30   ` Yann E. MORIN
2022-07-27  9:22     ` Thomas Petazzoni via buildroot [this message]
2022-07-27  9:35       ` Yann E. MORIN
2022-07-27  9:43   ` Yann E. MORIN
2022-07-27  9:55     ` Thomas Petazzoni via buildroot
2022-07-27  7:57 ` [Buildroot] [PATCH v2 1/4] arch/Config.in.riscv: lp64f ABI is only supported if MMU is enabled Yann E. MORIN
2022-08-29 20:13 ` Peter Korsgaard

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=20220727112207.309f4ec4@windsurf \
    --to=buildroot@buildroot.org \
    --cc=damien.lemoal@wdc.com \
    --cc=giulio.benetti@benettiengineering.com \
    --cc=mark@dibsco.co.uk \
    --cc=niklas.cassel@wdc.com \
    --cc=romain.naour@gmail.com \
    --cc=thomas.de_schampheleire@nokia.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=yann.morin.1998@free.fr \
    --cc=yba@tkos.co.il \
    /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.