From: Yu-Chien Peter Lin <peterlin@andestech.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: <acme@kernel.org>, <adrian.hunter@intel.com>,
<ajones@ventanamicro.com>, <alexander.shishkin@linux.intel.com>,
<andre.przywara@arm.com>, <anup@brainfault.org>,
<aou@eecs.berkeley.edu>, <atishp@atishpatra.org>,
<conor+dt@kernel.org>, <conor.dooley@microchip.com>,
<conor@kernel.org>, <devicetree@vger.kernel.org>,
<evan@rivosinc.com>, <geert+renesas@glider.be>,
<guoren@kernel.org>, <heiko@sntech.de>, <irogers@google.com>,
<jernej.skrabec@gmail.com>, <jolsa@kernel.org>,
<jszhang@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>,
<linux-perf-users@vger.kernel.org>,
<linux-renesas-soc@vger.kernel.org>,
<linux-riscv@lists.infradead.org>, <linux-sunxi@lists.linux.dev>,
<locus84@andestech.com>, <magnus.damm@gmail.com>,
<mark.rutland@arm.com>, <mingo@redhat.com>, <n.shubin@yadro.com>,
<namhyung@kernel.org>, <palmer@dabbelt.com>,
<paul.walmsley@sifive.com>, <peterz@infradead.org>,
<prabhakar.mahadev-lad.rj@bp.renesas.com>,
<rdunlap@infradead.org>, <robh+dt@kernel.org>,
<samuel@sholland.org>, <sunilvl@ventanamicro.com>,
<tim609@andestech.com>, <uwu@icenowy.me>, <wens@csie.org>,
<will@kernel.org>, <inochiama@outlook.com>,
<unicorn_wang@outlook.com>, <wefu@redhat.com>,
Randolph <randolph@andestech.com>,
Atish Patra <atishp@rivosinc.com>
Subject: Re: [PATCH v8 02/10] irqchip/riscv-intc: Allow large non-standard interrupt number
Date: Thu, 22 Feb 2024 11:25:35 +0800 [thread overview]
Message-ID: <Zda-r16ysaKzPdLV@APC323> (raw)
In-Reply-To: <877cj8issa.ffs@tglx>
Hi Thomas,
On Tue, Feb 13, 2024 at 11:04:53AM +0100, Thomas Gleixner wrote:
> On Mon, Jan 29 2024 at 17:25, Yu Chien Peter Lin wrote:
> > static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
> > {
> > unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;
> >
> > - if (unlikely(cause >= BITS_PER_LONG))
> > - panic("unexpected interrupt cause");
> > -
> > - generic_handle_domain_irq(intc_domain, cause);
> > + if (generic_handle_domain_irq(intc_domain, cause))
> > + pr_warn_ratelimited("Failed to handle interrupt (cause: %ld)\n",
> > + cause);
>
> Either let the cause stick out or you need brackets. See:
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#bracket-rules
>
> > }
> >
> > /*
> > @@ -93,6 +95,14 @@ static int riscv_intc_domain_alloc(struct irq_domain *domain,
> > if (ret)
> > return ret;
> >
> > + /*
> > + * Only allow hwirq for which we have corresponding standard or
> > + * custom interrupt enable register.
> > + */
> > + if ((riscv_intc_nr_irqs <= hwirq && hwirq < riscv_intc_custom_base) ||
> > + (riscv_intc_custom_base + riscv_intc_custom_nr_irqs) <= hwirq)
> > + return -EINVAL;
>
> Duh. This mix of ordering required to read this 3 times. What's wrong
> with writing this consistently:
>
> if ((hwirq >= riscv_intc_nr_irqs && hwirq < riscv_intc_custom_base) ||
> (hwirq >= iscv_intc_custom_base + riscv_intc_custom_nr_irqs)
> return -EINVAL;
>
> Hmm?
>
> > - pr_info("%d local interrupts mapped\n", BITS_PER_LONG);
> > + pr_info("%d local interrupts mapped\n", riscv_intc_nr_irqs);
> > + if (riscv_intc_custom_nr_irqs)
> > + pr_info("%d custom local interrupts mapped\n",
> > + riscv_intc_custom_nr_irqs);
>
> See bracket rules.
>
> > return 0;
> > }
> > @@ -166,6 +178,10 @@ static int __init riscv_intc_init(struct device_node *node,
> > return 0;
> > }
> >
> > + riscv_intc_nr_irqs = BITS_PER_LONG;
> > + riscv_intc_custom_base = riscv_intc_nr_irqs;
>
> Why don't you initialize the static variables with constants right away?
>
> > + riscv_intc_custom_nr_irqs = 0;
>
> It's already 0, no?
>
> > return riscv_intc_init_common(of_node_to_fwnode(node));
> > }
>
> Thanks,
>
> tglx
Thanks for pointing these out, I'll fix them in PATCH v9.
Regards,
Peter Lin
next prev parent reply other threads:[~2024-02-22 3:26 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-29 9:25 [PATCH v8 00/10] Support Andes PMU extension Yu Chien Peter Lin
2024-01-29 9:25 ` [PATCH v8 01/10] riscv: errata: Rename defines for Andes Yu Chien Peter Lin
2024-01-29 9:25 ` [PATCH v8 02/10] irqchip/riscv-intc: Allow large non-standard interrupt number Yu Chien Peter Lin
2024-02-13 10:04 ` Thomas Gleixner
2024-02-22 3:25 ` Yu-Chien Peter Lin [this message]
2024-01-29 9:25 ` [PATCH v8 03/10] irqchip/riscv-intc: Introduce Andes hart-level interrupt controller Yu Chien Peter Lin
2024-01-29 9:25 ` [PATCH v8 04/10] dt-bindings: riscv: Add Andes interrupt controller compatible string Yu Chien Peter Lin
2024-01-29 9:25 ` [PATCH v8 05/10] riscv: dts: renesas: r9a07g043f: Update compatible string to use Andes INTC Yu Chien Peter Lin
2024-01-29 9:25 ` [PATCH v8 06/10] perf: RISC-V: Eliminate redundant interrupt enable/disable operations Yu Chien Peter Lin
2024-01-29 9:25 ` [PATCH v8 07/10] perf: RISC-V: Introduce Andes PMU to support perf event sampling Yu Chien Peter Lin
2024-01-29 9:25 ` [PATCH v8 08/10] dt-bindings: riscv: Add Andes PMU extension description Yu Chien Peter Lin
2024-01-29 9:25 ` [PATCH v8 09/10] riscv: dts: renesas: Add Andes PMU extension for r9a07g043f Yu Chien Peter Lin
2024-01-29 9:25 ` [PATCH v8 10/10] riscv: andes: Support specifying symbolic firmware and hardware raw events Yu Chien Peter Lin
2024-02-21 20:58 ` [PATCH v8 00/10] Support Andes PMU extension Palmer Dabbelt
2024-02-22 3:23 ` Yu-Chien Peter Lin
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=Zda-r16ysaKzPdLV@APC323 \
--to=peterlin@andestech.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ajones@ventanamicro.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=andre.przywara@arm.com \
--cc=anup@brainfault.org \
--cc=aou@eecs.berkeley.edu \
--cc=atishp@atishpatra.org \
--cc=atishp@rivosinc.com \
--cc=conor+dt@kernel.org \
--cc=conor.dooley@microchip.com \
--cc=conor@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=evan@rivosinc.com \
--cc=geert+renesas@glider.be \
--cc=guoren@kernel.org \
--cc=heiko@sntech.de \
--cc=inochiama@outlook.com \
--cc=irogers@google.com \
--cc=jernej.skrabec@gmail.com \
--cc=jolsa@kernel.org \
--cc=jszhang@kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=locus84@andestech.com \
--cc=magnus.damm@gmail.com \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=n.shubin@yadro.com \
--cc=namhyung@kernel.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=peterz@infradead.org \
--cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
--cc=randolph@andestech.com \
--cc=rdunlap@infradead.org \
--cc=robh+dt@kernel.org \
--cc=samuel@sholland.org \
--cc=sunilvl@ventanamicro.com \
--cc=tglx@linutronix.de \
--cc=tim609@andestech.com \
--cc=unicorn_wang@outlook.com \
--cc=uwu@icenowy.me \
--cc=wefu@redhat.com \
--cc=wens@csie.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).