Linux-EFI Archive on lore.kernel.org
 help / color / Atom feed
From: <Narendra.K@dell.com>
To: <geert@linux-m68k.org>
Cc: <ard.biesheuvel@linaro.org>, <linux-efi@vger.kernel.org>,
	<Mario.Limonciello@dell.com>, <tglx@linutronix.de>,
	<linux-kernel@vger.kernel.org>, <james.morse@arm.com>,
	<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:55:12 +0000
Message-ID: <20191011125446.GA2170@localhost.localdomain> (raw)
In-Reply-To: <CAMuHMdUMkyyCZACyJ7dvd4SaicpN77g5pFd0aGEzQW_q7m3Q0g@mail.gmail.com>

On Fri, Oct 11, 2019 at 12:01:25PM +0200, Geert Uytterhoeven wrote:
> > > > > > -       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"?

Geert,

> The code in question is entirely architecture agnostic, and defaults
> to 'n', so I am not convinced this is needed. (It came up in the
> review as well)

>> "make oldconfig" still asks me the question on e.g. arm64, where it is
>> irrelevant, until arm64 variants of the hardware show up.

>> So IMHO it should have "depends on X86 || COMPILE_TEST".

From the discussion in [1] and [2](pasted a part of it above), my understanding
of the issue you reported is that 'make oldconfig' asks the user a question for arm64
though the EFI_RCI2_TABLE is not relevant for arm64. From the tests,
it seemed like adding "depends on X86 || COMPILE_TEST" does not fix the
issue, splitting bool into bool + prompt fixes it.

Please let me know if I am missing any detail in the issue you reported.  

With the way EFI_RCI2_TABLE is currently defined, my understanding is
that 'make oldconfig' does not set the EFI_RCI2_TABLE to 'y' by default
on arm64, but it asks the user the question. User has to say 'y' if he
wants it to be set to 'y', else by default 'n' is set. This behavior is
as expected. 

> 
> 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.

We did not intend to enable EFI_RCI2_TABLE option by default even on all
X86 systems from the begining. As a result, we chose to set it to 'n' by
default and added the guidance in 'help' section to say 'y' for Dell EMC
PowerEdge systems. 

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

As mentioned above, we want to keep the default to n.

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

Ok. We could keep the check. Could we make it independent of platforms
by adding 'defbool y if COMPILE_TEST' ? 

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

[2] Re: [PATCH 4/5] efi: Export Runtime Configuration Interface table to sysfs
https://lore.kernel.org/linux-efi/20190812150452.27983-5-ard.biesheuvel@linaro.org/T/#mf45a9bc1861c71f110c800a53c60f0be65c68ec7
-- 
With regards,
Narendra K

  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
2019-10-11 12:55           ` Narendra.K [this message]
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=20191011125446.GA2170@localhost.localdomain \
    --to=narendra.k@dell.com \
    --cc=Mario.Limonciello@dell.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=geert@linux-m68k.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 linux-efi@archiver.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