linux-kernel - Re: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f65dd139-1021-47d6-93a1-1477d6b4ca1d@web.de>
Date: Wed, 9 Oct 2024 14:32:10 +0200
From: Markus Elfring <Markus.Elfring@....de>
To: Kevin Chen <kevin_chen@...eedtech.com>, linux-aspeed@...ts.ozlabs.org,
 linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
 Andrew Jeffery <andrew@...econstruct.com.au>,
 Conor Dooley <conor+dt@...nel.org>, Joel Stanley <joel@....id.au>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Rob Herring <robh@...nel.org>,
 Thomas Gleixner <tglx@...utronix.de>
Cc: LKML <linux-kernel@...r.kernel.org>, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC

…
> To support ASPEED interrupt controller(INTC) maps the internal interrupt
> sources of the AST27XX device to an parent interrupt controller.
> ---

* I miss your tag “Signed-off-by”.
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12-rc2#n396

* How do you think about to choose an additional imperative wording
  for an improved change description?
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12-rc2#n94

…
> +++ b/drivers/irqchip/irq-aspeed-intc.c
> @@ -0,0 +1,139 @@
…
> +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc)
+{
> +	struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	unsigned long bit, status;

I suggest to reduce the scopes for three local variables.

> +
> +	chained_irq_enter(chip, desc);

Would you become interested to collaborate with another scoped guard
for this programming interface?
https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained_irq.h#L13

> +
> +	scoped_guard(raw_spinlock, &intc_ic->gic_lock) {
> +		status = readl(intc_ic->base + INTC_INT_STATUS_REG);
> +		for_each_set_bit(bit, &status, IRQS_PER_WORD) {
> +			generic_handle_domain_irq(intc_ic->irq_domain, bit);
> +			writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG);
> +		}
> +	}
> +
> +	chained_irq_exit(chip, desc);
> +}

Regards,
Markus