All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anup Patel <anup@brainfault.org>
To: Marc Zyngier <maz@kernel.org>
Cc: Anup Patel <anup.patel@wdc.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Atish Patra <atish.patra@wdc.com>,
	Alistair Francis <Alistair.Francis@wdc.com>,
	linux-riscv <linux-riscv@lists.infradead.org>,
	"linux-kernel@vger.kernel.org List"
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 3/6] irqchip: RISC-V per-HART local interrupt controller driver
Date: Fri, 29 May 2020 17:15:03 +0530	[thread overview]
Message-ID: <CAAhSdy3iDbMSPeTSv7vM=1DSY8i1a1ugoB2Vxt3PY_if8JWJKA@mail.gmail.com> (raw)
In-Reply-To: <40251a7764fc23ed19426df0adf0fc4d@kernel.org>

On Fri, May 29, 2020 at 4:40 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On 2020-05-29 11:45, Anup Patel wrote:
> > On Fri, May 29, 2020 at 3:39 PM Marc Zyngier <maz@kernel.org> wrote:
> >>
> >> On 2020-05-21 14:32, Anup Patel wrote:
>
> [...]
>
> >> > +/* Get the OF device node used by INTC irq domain */
> >> > +struct device_node *riscv_of_intc_domain_node(void)
> >> > +{
> >> > +     return intc_domain_node;
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(riscv_of_intc_domain_node);
> >>
> >> Why do you need this? Why can't the timer node refer to its
> >> interrupt-parent? The irqchip shouldn't be in the business of
> >> working around DT issues.
> >>
> >> At worse, use the default irqdomain if you must, but please
> >> avoid this kind of construct.
> >
> > Even, I don't like exporting riscv_of_intc_domain_node().
> >
> > Thanks for your suggestion, I will certainly use the default irqdomain.
>
> This should be a last resort solution. The irqdomain should
> naturally come from the parent interrupt controller, accessible
> from the device (the timer in this case) node.
>
> Use it to for backward compatibility if you *really* must,
> but this is generally a very bad idea as it allows all kind
> of bizarre fallbacks and hides bugs.

Okay, I will explore other approaches and keep default irqdomain
as a last resort solution.

Regards,
Anup

WARNING: multiple messages have this Message-ID (diff)
From: Anup Patel <anup@brainfault.org>
To: Marc Zyngier <maz@kernel.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>,
	Jason Cooper <jason@lakedaemon.net>,
	Anup Patel <anup.patel@wdc.com>,
	"linux-kernel@vger.kernel.org List"
	<linux-kernel@vger.kernel.org>, Atish Patra <atish.patra@wdc.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Alistair Francis <Alistair.Francis@wdc.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-riscv <linux-riscv@lists.infradead.org>
Subject: Re: [PATCH v5 3/6] irqchip: RISC-V per-HART local interrupt controller driver
Date: Fri, 29 May 2020 17:15:03 +0530	[thread overview]
Message-ID: <CAAhSdy3iDbMSPeTSv7vM=1DSY8i1a1ugoB2Vxt3PY_if8JWJKA@mail.gmail.com> (raw)
In-Reply-To: <40251a7764fc23ed19426df0adf0fc4d@kernel.org>

On Fri, May 29, 2020 at 4:40 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On 2020-05-29 11:45, Anup Patel wrote:
> > On Fri, May 29, 2020 at 3:39 PM Marc Zyngier <maz@kernel.org> wrote:
> >>
> >> On 2020-05-21 14:32, Anup Patel wrote:
>
> [...]
>
> >> > +/* Get the OF device node used by INTC irq domain */
> >> > +struct device_node *riscv_of_intc_domain_node(void)
> >> > +{
> >> > +     return intc_domain_node;
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(riscv_of_intc_domain_node);
> >>
> >> Why do you need this? Why can't the timer node refer to its
> >> interrupt-parent? The irqchip shouldn't be in the business of
> >> working around DT issues.
> >>
> >> At worse, use the default irqdomain if you must, but please
> >> avoid this kind of construct.
> >
> > Even, I don't like exporting riscv_of_intc_domain_node().
> >
> > Thanks for your suggestion, I will certainly use the default irqdomain.
>
> This should be a last resort solution. The irqdomain should
> naturally come from the parent interrupt controller, accessible
> from the device (the timer in this case) node.
>
> Use it to for backward compatibility if you *really* must,
> but this is generally a very bad idea as it allows all kind
> of bizarre fallbacks and hides bugs.

Okay, I will explore other approaches and keep default irqdomain
as a last resort solution.

Regards,
Anup


  reply	other threads:[~2020-05-29 11:45 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-21 13:32 [PATCH v5 0/6] New RISC-V Local Interrupt Controller Driver Anup Patel
2020-05-21 13:32 ` Anup Patel
2020-05-21 13:32 ` [PATCH v5 1/6] RISC-V: self-contained IPI handling routine Anup Patel
2020-05-21 13:32   ` Anup Patel
2020-05-28 23:08   ` Atish Patra
2020-05-28 23:08     ` Atish Patra
2020-05-29  1:37   ` Palmer Dabbelt
2020-05-29  1:37     ` Palmer Dabbelt
2020-05-21 13:32 ` [PATCH v5 2/6] RISC-V: Rename and move plic_find_hart_id() to arch directory Anup Patel
2020-05-21 13:32   ` Anup Patel
2020-05-28 23:09   ` Atish Patra
2020-05-28 23:09     ` Atish Patra
2020-05-29  1:37   ` Palmer Dabbelt
2020-05-29  1:37     ` Palmer Dabbelt
2020-05-21 13:32 ` [PATCH v5 3/6] irqchip: RISC-V per-HART local interrupt controller driver Anup Patel
2020-05-21 13:32   ` Anup Patel
2020-05-29  2:40   ` Palmer Dabbelt
2020-05-29  2:40     ` Palmer Dabbelt
2020-05-29  4:15     ` Anup Patel
2020-05-29  4:15       ` Anup Patel
2020-05-29 10:09   ` Marc Zyngier
2020-05-29 10:09     ` Marc Zyngier
2020-05-29 10:45     ` Anup Patel
2020-05-29 10:45       ` Anup Patel
2020-05-29 11:10       ` Marc Zyngier
2020-05-29 11:10         ` Marc Zyngier
2020-05-29 11:45         ` Anup Patel [this message]
2020-05-29 11:45           ` Anup Patel
2020-05-21 13:32 ` [PATCH v5 4/6] clocksource/drivers/timer-riscv: Use per-CPU timer interrupt Anup Patel
2020-05-21 13:32   ` Anup Patel
2020-05-22 13:08   ` Daniel Lezcano
2020-05-22 13:08     ` Daniel Lezcano
2020-05-23  5:15     ` Anup Patel
2020-05-23  5:15       ` Anup Patel
2020-05-29  5:04   ` Atish Patra
2020-05-29  5:04     ` Atish Patra
2020-05-21 13:33 ` [PATCH v5 5/6] RISC-V: Remove do_IRQ() function Anup Patel
2020-05-21 13:33   ` Anup Patel
2020-05-29  0:32   ` Atish Patra
2020-05-29  0:32     ` Atish Patra
2020-05-21 13:33 ` [PATCH v5 6/6] RISC-V: Force select RISCV_INTC for CONFIG_RISCV Anup Patel
2020-05-21 13:33   ` Anup Patel
2020-05-29  0:33   ` Atish Patra
2020-05-29  0:33     ` Atish Patra
2020-05-27 18:47 ` [PATCH v5 0/6] New RISC-V Local Interrupt Controller Driver Palmer Dabbelt
2020-05-27 18:47   ` Palmer Dabbelt
2020-05-29  3:57   ` Anup Patel
2020-05-29  3:57     ` Anup Patel
2020-05-29  4:13     ` Palmer Dabbelt
2020-05-29  4:13       ` Palmer Dabbelt
2020-05-29  4:24       ` Anup Patel
2020-05-29  4:24         ` Anup Patel
2020-05-29 10:13   ` Marc Zyngier
2020-05-29 10:13     ` Marc Zyngier

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='CAAhSdy3iDbMSPeTSv7vM=1DSY8i1a1ugoB2Vxt3PY_if8JWJKA@mail.gmail.com' \
    --to=anup@brainfault.org \
    --cc=Alistair.Francis@wdc.com \
    --cc=anup.patel@wdc.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=atish.patra@wdc.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=jason@lakedaemon.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --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
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.