All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, will@kernel.org,
	catalin.marinas@arm.com, james.morse@arm.com, marcan@marcan.st,
	maz@kernel.org, tglx@linutronix.de
Subject: Re: [PATCHv3 2/6] arm64: don't use GENERIC_IRQ_MULTI_HANDLER
Date: Mon, 22 Mar 2021 10:29:38 +0000	[thread overview]
Message-ID: <20210322102938.GA75892@C02TD0UTHF1T.local> (raw)
In-Reply-To: <20210315192803.GB154861@infradead.org>

Hi Christoph,

On Mon, Mar 15, 2021 at 07:28:03PM +0000, Christoph Hellwig wrote:
> On Mon, Mar 15, 2021 at 11:56:25AM +0000, Mark Rutland wrote:
> > From: Marc Zyngier <maz@kernel.org>
> > 
> > In subsequent patches we want to allow irqchip drivers to register as
> > FIQ handlers, with a set_handle_fiq() function. To keep the IRQ/FIQ
> > paths similar, we want arm64 to provide both set_handle_irq() and
> > set_handle_fiq(), rather than using GENERIC_IRQ_MULTI_HANDLER for the
> > former.
> 
> Having looked through the series I do not understand this rationale
> at all.  You've only added the default_handle_irq logic, which seems
> perfectly suitable and desirable for the generic version. 

The default_handle_irq thing isn't the point of the series, that part is
all preparatory work. I agree that probably makes sense for the generic
code, and I'm happy to update core code with this.

The big thing here is that (unlike most architectures), with arm64 a CPU
has two interrupt pins, IRQ and FIQ, and we need separate root handlers
for these. That's what this series aims to do, and patches 1-5 are all
preparatory work with that appearing in patch 6.

Our initial stab at this did try to add that support to core code, but
that was more painful to deal with, since you either add abstractions to
make this look generic that make the code more complex for bot hthe
genreic code and arch code, or you place arch-specific assumptions in
the core code. See Marc's eariler stab at this, where in effect we had
to duplicate the logic in the core code so that we didn't adversely
affect existing entry assembly on other architectures due to the way the
function pointers were stored.

> Please don't fork off generic code for no good reason.

I appreciate that this runs counter to the general goal of making things
generic wherever possible, but I do think in this case we have good
reasons, and the duplication is better than adding single-user
abstractions in the generic code that complicate the generic code and
arch code.

Thanks,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, will@kernel.org,
	catalin.marinas@arm.com, james.morse@arm.com, marcan@marcan.st,
	maz@kernel.org, tglx@linutronix.de
Subject: Re: [PATCHv3 2/6] arm64: don't use GENERIC_IRQ_MULTI_HANDLER
Date: Mon, 22 Mar 2021 10:29:38 +0000	[thread overview]
Message-ID: <20210322102938.GA75892@C02TD0UTHF1T.local> (raw)
In-Reply-To: <20210315192803.GB154861@infradead.org>

Hi Christoph,

On Mon, Mar 15, 2021 at 07:28:03PM +0000, Christoph Hellwig wrote:
> On Mon, Mar 15, 2021 at 11:56:25AM +0000, Mark Rutland wrote:
> > From: Marc Zyngier <maz@kernel.org>
> > 
> > In subsequent patches we want to allow irqchip drivers to register as
> > FIQ handlers, with a set_handle_fiq() function. To keep the IRQ/FIQ
> > paths similar, we want arm64 to provide both set_handle_irq() and
> > set_handle_fiq(), rather than using GENERIC_IRQ_MULTI_HANDLER for the
> > former.
> 
> Having looked through the series I do not understand this rationale
> at all.  You've only added the default_handle_irq logic, which seems
> perfectly suitable and desirable for the generic version. 

The default_handle_irq thing isn't the point of the series, that part is
all preparatory work. I agree that probably makes sense for the generic
code, and I'm happy to update core code with this.

The big thing here is that (unlike most architectures), with arm64 a CPU
has two interrupt pins, IRQ and FIQ, and we need separate root handlers
for these. That's what this series aims to do, and patches 1-5 are all
preparatory work with that appearing in patch 6.

Our initial stab at this did try to add that support to core code, but
that was more painful to deal with, since you either add abstractions to
make this look generic that make the code more complex for bot hthe
genreic code and arch code, or you place arch-specific assumptions in
the core code. See Marc's eariler stab at this, where in effect we had
to duplicate the logic in the core code so that we didn't adversely
affect existing entry assembly on other architectures due to the way the
function pointers were stored.

> Please don't fork off generic code for no good reason.

I appreciate that this runs counter to the general goal of making things
generic wherever possible, but I do think in this case we have good
reasons, and the duplication is better than adding single-user
abstractions in the generic code that complicate the generic code and
arch code.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-03-22 10:31 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-15 11:56 [PATCHv3 0/6] arm64: Support FIQ controller registration Mark Rutland
2021-03-15 11:56 ` Mark Rutland
2021-03-15 11:56 ` [PATCHv3 1/6] genirq: Allow architectures to override set_handle_irq() fallback Mark Rutland
2021-03-15 11:56   ` Mark Rutland
2021-03-15 11:56 ` [PATCHv3 2/6] arm64: don't use GENERIC_IRQ_MULTI_HANDLER Mark Rutland
2021-03-15 11:56   ` Mark Rutland
2021-03-15 19:28   ` Christoph Hellwig
2021-03-15 19:28     ` Christoph Hellwig
2021-03-22 10:29     ` Mark Rutland [this message]
2021-03-22 10:29       ` Mark Rutland
2021-03-15 11:56 ` [PATCHv3 3/6] arm64: irq: rework root IRQ handler registration Mark Rutland
2021-03-15 11:56   ` Mark Rutland
2021-03-24 16:49   ` Will Deacon
2021-03-24 16:49     ` Will Deacon
2021-03-15 11:56 ` [PATCHv3 4/6] arm64: entry: factor irq triage logic into macros Mark Rutland
2021-03-15 11:56   ` Mark Rutland
2021-03-24 16:56   ` Will Deacon
2021-03-24 16:56     ` Will Deacon
2021-03-15 11:56 ` [PATCHv3 5/6] arm64: Always keep DAIF.[IF] in sync Mark Rutland
2021-03-15 11:56   ` Mark Rutland
2021-03-24 17:00   ` Will Deacon
2021-03-24 17:00     ` Will Deacon
2021-03-15 11:56 ` [PATCHv3 6/6] arm64: irq: allow FIQs to be handled Mark Rutland
2021-03-15 11:56   ` Mark Rutland
2021-03-24 17:03   ` Will Deacon
2021-03-24 17:03     ` Will Deacon
2021-03-24 20:38 ` [PATCHv3 0/6] arm64: Support FIQ controller registration Catalin Marinas
2021-03-24 20:38   ` Catalin Marinas

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=20210322102938.GA75892@C02TD0UTHF1T.local \
    --to=mark.rutland@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=hch@infradead.org \
    --cc=james.morse@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcan@marcan.st \
    --cc=maz@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    /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.