diff mbox series

irqchip/sifive-plic: default to enabled

Message ID 20221117185942.3896559-1-conor@kernel.org (mailing list archive)
State Superseded
Headers show
Series irqchip/sifive-plic: default to enabled | expand

Checks

Context Check Description
conchuod/patch_count success Link
conchuod/cover_letter success Single patches do not need cover letters
conchuod/tree_selection success Guessed tree name to be for-next
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/module_param success Was 0 now: 0
conchuod/build_rv32_defconfig success Build OK
conchuod/build_warn_rv64 success Errors and warnings before: 0 this patch: 0
conchuod/dtb_warn_rv64 success Errors and warnings before: 0 this patch: 0
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch success total: 0 errors, 0 warnings, 0 checks, 20 lines checked
conchuod/source_inline success Was 0 now: 0
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Conor Dooley Nov. 17, 2022, 6:59 p.m. UTC
From: Conor Dooley <conor.dooley@microchip.com>

The SiFive PLIC driver is used by all current implementations, including
those that do not have a SiFive PLIC. Default the driver to enabled,
with the intention of later removing the current "every SOC selects
this" situation in Kconfig.socs at the moment.

The speculative "potential others" in the description no longer makes
any sense, as the driver is always used. Update the Kconfig symbol's
description to reflect the driver's ubiquitous state.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
Hey Marc,

I recall some discussion when this driver was extended to other PLICs a
few months ago:
https://lore.kernel.org/linux-riscv/20511a05f39408c8ffbcc98923c4abd2@kernel.org/

Perhaps I got the wrong impression, but it seemed to me that you intend
for future implementations to reuse this driver where possible?

I'd like to think, and surely will be proven wrong, that ~all future
plic implementations should be similar enough to fit that bill.
It's kinda on this basis that I figure switching this thing to default y
should be okay. It's already only buildable on RISC-V & every
implementation uses it, so no difference there.

Thanks,
Conor.
---
 drivers/irqchip/Kconfig | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Marc Zyngier Nov. 17, 2022, 7:36 p.m. UTC | #1
On Thu, 17 Nov 2022 18:59:43 +0000,
Conor Dooley <conor@kernel.org> wrote:
> 
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The SiFive PLIC driver is used by all current implementations, including
> those that do not have a SiFive PLIC. Default the driver to enabled,
> with the intention of later removing the current "every SOC selects
> this" situation in Kconfig.socs at the moment.
> 
> The speculative "potential others" in the description no longer makes
> any sense, as the driver is always used. Update the Kconfig symbol's
> description to reflect the driver's ubiquitous state.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Hey Marc,
> 
> I recall some discussion when this driver was extended to other PLICs a
> few months ago:
> https://lore.kernel.org/linux-riscv/20511a05f39408c8ffbcc98923c4abd2@kernel.org/
> 
> Perhaps I got the wrong impression, but it seemed to me that you intend
> for future implementations to reuse this driver where possible?

Well, within reasons. People seem to have some very liberal
interpretations of the architecture spec...

> 
> I'd like to think, and surely will be proven wrong, that ~all future
> plic implementations should be similar enough to fit that bill.
> It's kinda on this basis that I figure switching this thing to default y
> should be okay. It's already only buildable on RISC-V & every
> implementation uses it, so no difference there.

If you expect this to be present at all times, why isn't this selected
by the architecture Kconfig instead? I always find it pretty odd to
have something that is 'default y' and yet constrained by a 'depend
MYARCH'. A 'select PLIC' would make a lot more sense.

And then you can stop making this user selectable.

Thanks,

	M.
Conor Dooley Nov. 17, 2022, 7:57 p.m. UTC | #2
On Thu, Nov 17, 2022 at 07:36:57PM +0000, Marc Zyngier wrote:
> On Thu, 17 Nov 2022 18:59:43 +0000,
> Conor Dooley <conor@kernel.org> wrote:
> > 
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > The SiFive PLIC driver is used by all current implementations, including
> > those that do not have a SiFive PLIC. Default the driver to enabled,
> > with the intention of later removing the current "every SOC selects
> > this" situation in Kconfig.socs at the moment.
> > 
> > The speculative "potential others" in the description no longer makes
> > any sense, as the driver is always used. Update the Kconfig symbol's
> > description to reflect the driver's ubiquitous state.
> > 
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > Hey Marc,
> > 
> > I recall some discussion when this driver was extended to other PLICs a
> > few months ago:
> > https://lore.kernel.org/linux-riscv/20511a05f39408c8ffbcc98923c4abd2@kernel.org/
> > 
> > Perhaps I got the wrong impression, but it seemed to me that you intend
> > for future implementations to reuse this driver where possible?
> 
> Well, within reasons. People seem to have some very liberal
> interpretations of the architecture spec...

Yeah, I know.. something something "RISC-V is meant to be extensible"
something something. Even if that means doing some "standard" thing
your own way apparently.

> > I'd like to think, and surely will be proven wrong, that ~all future
> > plic implementations should be similar enough to fit that bill.
> > It's kinda on this basis that I figure switching this thing to default y
> > should be okay. It's already only buildable on RISC-V & every
> > implementation uses it, so no difference there.
> 
> If you expect this to be present at all times, why isn't this selected
> by the architecture Kconfig instead?

Everyone at the moment needs it, but that's not always going to be true.
The AIA APLIC that's currently out for review is the "next generation"
interrupt controller. When we will actually see one in the wild is
another question.

> I always find it pretty odd to
> have something that is 'default y' and yet constrained by a 'depend
> MYARCH'. A 'select PLIC' would make a lot more sense.
> 
> And then you can stop making this user selectable.

I was considering moving the select to arch level, but settled for this
as while I'd like to stop the individual SOCs doing `select PLIC`, I can
see why someone building for a (future) system with the new AIA stuff
may not care to build it.

Or maybe the overhead of this one driver is nothing to care about?

Thanks,
Conor.
Marc Zyngier Nov. 17, 2022, 8:26 p.m. UTC | #3
On Thu, 17 Nov 2022 19:57:17 +0000,
Conor Dooley <conor@kernel.org> wrote:
> 
> On Thu, Nov 17, 2022 at 07:36:57PM +0000, Marc Zyngier wrote:
> > On Thu, 17 Nov 2022 18:59:43 +0000,
> > Conor Dooley <conor@kernel.org> wrote:
> > > 
> > > From: Conor Dooley <conor.dooley@microchip.com>
> > > 
> > > The SiFive PLIC driver is used by all current implementations, including
> > > those that do not have a SiFive PLIC. Default the driver to enabled,
> > > with the intention of later removing the current "every SOC selects
> > > this" situation in Kconfig.socs at the moment.
> > > 
> > > The speculative "potential others" in the description no longer makes
> > > any sense, as the driver is always used. Update the Kconfig symbol's
> > > description to reflect the driver's ubiquitous state.
> > > 
> > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > ---
> > > Hey Marc,
> > > 
> > > I recall some discussion when this driver was extended to other PLICs a
> > > few months ago:
> > > https://lore.kernel.org/linux-riscv/20511a05f39408c8ffbcc98923c4abd2@kernel.org/
> > > 
> > > Perhaps I got the wrong impression, but it seemed to me that you intend
> > > for future implementations to reuse this driver where possible?
> > 
> > Well, within reasons. People seem to have some very liberal
> > interpretations of the architecture spec...
> 
> Yeah, I know.. something something "RISC-V is meant to be extensible"
> something something. Even if that means doing some "standard" thing
> your own way apparently.

Funny how someone's "extensible" is someone else's "terminally
broken". I guess HW folks will eventually learn that, possibly the
hard way.

> 
> > > I'd like to think, and surely will be proven wrong, that ~all future
> > > plic implementations should be similar enough to fit that bill.
> > > It's kinda on this basis that I figure switching this thing to default y
> > > should be okay. It's already only buildable on RISC-V & every
> > > implementation uses it, so no difference there.
> > 
> > If you expect this to be present at all times, why isn't this selected
> > by the architecture Kconfig instead?
> 
> Everyone at the moment needs it, but that's not always going to be true.
> The AIA APLIC that's currently out for review is the "next generation"
> interrupt controller. When we will actually see one in the wild is
> another question.
>
> > I always find it pretty odd to
> > have something that is 'default y' and yet constrained by a 'depend
> > MYARCH'. A 'select PLIC' would make a lot more sense.
> > 
> > And then you can stop making this user selectable.
> 
> I was considering moving the select to arch level, but settled for this
> as while I'd like to stop the individual SOCs doing `select PLIC`, I can
> see why someone building for a (future) system with the new AIA stuff
> may not care to build it.
> 
> Or maybe the overhead of this one driver is nothing to care about?

In the grand scheme of things, it really doesn't matter. Hardly anyone
is configuring their own kernel. People use distro kernels, which will
have *everything* enabled.

As an example, on the arm64 side we have long decided that some things
were not worth the trouble, such as selectable root interrupt
controllers and non-SMP support. Life is too short to care about
those.

	M.
diff mbox series

Patch

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 7ef9f5e696d3..6f99919ba66c 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -553,14 +553,15 @@  config RISCV_INTC
 config SIFIVE_PLIC
 	bool "SiFive Platform-Level Interrupt Controller"
 	depends on RISCV
+	default y
 	select IRQ_DOMAIN_HIERARCHY
 	select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
 	help
-	   This enables support for the PLIC chip found in SiFive (and
-	   potentially other) RISC-V systems.  The PLIC controls devices
-	   interrupts and connects them to each core's local interrupt
-	   controller.  Aside from timer and software interrupts, all other
-	   interrupt sources are subordinate to the PLIC.
+	   This enables support for the PLIC chip found in SiFive & other
+	   RISC-V systems.  The PLIC controls devices interrupts and connects
+	   them to each core's local interrupt controller.  Aside from timer
+	   and software interrupts, all other interrupt sources are
+	   subordinate to the PLIC.
 
 	   If you don't know what to do here, say Y.