Linux-EFI Archive on lore.kernel.org
 help / color / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Narendra K <Narendra.K@dell.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	linux-efi <linux-efi@vger.kernel.org>,
	Mario Limonciello <Mario.Limonciello@dell.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	James Morse <james.morse@arm.com>, Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH] Ask user input only when CONFIG_X86 or CONFIG_COMPILE_TEST is set to y
Date: Fri, 11 Oct 2019 12:01:25 +0200
Message-ID: <CAMuHMdUMkyyCZACyJ7dvd4SaicpN77g5pFd0aGEzQW_q7m3Q0g@mail.gmail.com> (raw)
In-Reply-To: <20191011094322.GA3065@localhost.localdomain>

Hi Narendra,

On Fri, Oct 11, 2019 at 11:43 AM <Narendra.K@dell.com> wrote:
> On Thu, Oct 10, 2019 at 08:50:45PM +0200, Geert Uytterhoeven wrote:
> [...]
> > > > >  drivers/firmware/efi/Kconfig | 5 ++++-
> > > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> > > > > index 178ee8106828..6e4c46e8a954 100644
> > > > > --- a/drivers/firmware/efi/Kconfig
> > > > > +++ b/drivers/firmware/efi/Kconfig
> > > > > @@ -181,7 +181,10 @@ config RESET_ATTACK_MITIGATION
> > > > >           reboots.
> > > > >
> > > > >  config EFI_RCI2_TABLE
> > > > > -       bool "EFI Runtime Configuration Interface Table Version 2 Support"
> > > > > +       bool
> > > > > +       prompt "EFI RCI Table Version 2 Support" if X86 || COMPILE_TEST
> >
> > Why the split of bool and prompt?
> > Why not simply add a single line "depends on X86 || COMPILE_TEST"?
>
> It is because of the findings shared in [1]. Please let me know your
> thoughts on the findings.

So you want to prevent the user from seeing a prompt for an option he may
or may not need to enable, when running "make oldconfig"?

One common approach is to let the Kconfig symbol for the platform (not for
all of X86!) select EFI_RCI2_TABLE.
That way it will be enabled automatically when needed.

Another approach is to not force the option on, but guide the user towards
enabling it, by adding "default y if <platform_symbol>".

> > > > You can drop the || COMPILE_TEST as well.
> > >
> > > I will drop this part of the change in the next version of the patch.
> >
> > Why drop that part? Isn't it good to have more compile test coverage?
>
> It is per the suggestion in the previous review comment.
>
> Ard, please share your thought here. I could add the || COMPILE_TEST.

Without the "|| COMPILE_TEST", you cannot enable compile-testing of
the driver on non-x86 platforms with EFI.

> [1]  Re: [PATCH 4/5] efi: Export Runtime Configuration Interface table to sysfs
> https://lore.kernel.org/linux-efi/20190812150452.27983-1-ard.biesheuvel@linaro.org/T/#mebff9ba48499808f59b33b2daef2d94e006296d8

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

  reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 19:44 Narendra.K
2019-10-09 14:11 ` Ard Biesheuvel
2019-10-10 17:47   ` Narendra.K
2019-10-10 18:50     ` Geert Uytterhoeven
2019-10-11  9:43       ` Narendra.K
2019-10-11 10:01         ` Geert Uytterhoeven [this message]
2019-10-11 12:55           ` Narendra.K
2019-10-12 17:04             ` Geert Uytterhoeven
2019-10-13 19:08               ` Narendra.K

Reply instructions:

You may reply publically 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=CAMuHMdUMkyyCZACyJ7dvd4SaicpN77g5pFd0aGEzQW_q7m3Q0g@mail.gmail.com \
    --to=geert@linux-m68k.org \
    --cc=Mario.Limonciello@dell.com \
    --cc=Narendra.K@dell.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=james.morse@arm.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    /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

Linux-EFI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-efi/0 linux-efi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-efi linux-efi/ https://lore.kernel.org/linux-efi \
		linux-efi@vger.kernel.org
	public-inbox-index linux-efi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-efi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git