Support for the Aspeed Interrupt Controller found on Aspeed Silicon SoCs,
such as the AST2700, which is arm64 architecture.
To support ASPEED interrupt controller(INTC) maps the internal interrupt
sources of the AST27XX device to an parent interrupt controller.
---
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-aspeed-intc.c | 139 ++++++++++++++++++++++++++++++
2 files changed, 140 insertions(+)
create mode 100644 drivers/irqchip/irq-aspeed-intc.c
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e3679ec2b9f7..086911bf4db6 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_MVEBU_SEI) += irq-mvebu-sei.o
obj-$(CONFIG_LS_EXTIRQ) += irq-ls-extirq.o
obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o
obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o irq-aspeed-scu-ic.o
+obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-intc.o
obj-$(CONFIG_STM32MP_EXTI) += irq-stm32mp-exti.o
obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o
diff --git a/drivers/irqchip/irq-aspeed-intc.c b/drivers/irqchip/irq-aspeed-intc.c
new file mode 100644
index 000000000000..ef1c095ad09e
--- /dev/null
+++ b/drivers/irqchip/irq-aspeed-intc.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Aspeed Interrupt Controller.
+ *
+ * Copyright (C) 2023 ASPEED Technology Inc.
+ */
+
+#include <linux/bitops.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/io.h>
+#include <linux/spinlock.h>
+
+#define INTC_INT_ENABLE_REG 0x00
+#define INTC_INT_STATUS_REG 0x04
+#define IRQS_PER_WORD 32
+
+struct aspeed_intc_ic {
+ void __iomem *base;
+ raw_spinlock_t gic_lock;
+ raw_spinlock_t intc_lock;
+ struct irq_domain *irq_domain;
+};
+
+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;
+
+ chained_irq_enter(chip, desc);
+
+ 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);
+}
+
+static void aspeed_intc_irq_mask(struct irq_data *data)
+{
+ struct aspeed_intc_ic *intc_ic = irq_data_get_irq_chip_data(data);
+ unsigned int mask = readl(intc_ic->base + INTC_INT_ENABLE_REG) & ~BIT(data->hwirq);
+
+ guard(raw_spinlock)(&intc_ic->intc_lock);
+ writel(mask, intc_ic->base + INTC_INT_ENABLE_REG);
+}
+
+static void aspeed_intc_irq_unmask(struct irq_data *data)
+{
+ struct aspeed_intc_ic *intc_ic = irq_data_get_irq_chip_data(data);
+ unsigned int unmask = readl(intc_ic->base + INTC_INT_ENABLE_REG) | BIT(data->hwirq);
+
+ guard(raw_spinlock)(&intc_ic->intc_lock);
+ writel(unmask, intc_ic->base + INTC_INT_ENABLE_REG);
+}
+
+static struct irq_chip aspeed_intc_chip = {
+ .name = "ASPEED INTC",
+ .irq_mask = aspeed_intc_irq_mask,
+ .irq_unmask = aspeed_intc_irq_unmask,
+};
+
+static int aspeed_intc_ic_map_irq_domain(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &aspeed_intc_chip, handle_level_irq);
+ irq_set_chip_data(irq, domain->host_data);
+
+ return 0;
+}
+
+static const struct irq_domain_ops aspeed_intc_ic_irq_domain_ops = {
+ .map = aspeed_intc_ic_map_irq_domain,
+};
+
+static int __init aspeed_intc_ic_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ struct aspeed_intc_ic *intc_ic;
+ int ret = 0;
+ int irq, i;
+
+ intc_ic = kzalloc(sizeof(*intc_ic), GFP_KERNEL);
+ if (!intc_ic)
+ return -ENOMEM;
+
+ intc_ic->base = of_iomap(node, 0);
+ if (!intc_ic->base) {
+ pr_err("Failed to iomap intc_ic base\n");
+ ret = -ENOMEM;
+ goto err_free_ic;
+ }
+ writel(0xffffffff, intc_ic->base + INTC_INT_STATUS_REG);
+ writel(0x0, intc_ic->base + INTC_INT_ENABLE_REG);
+
+ intc_ic->irq_domain = irq_domain_add_linear(node, 32,
+ &aspeed_intc_ic_irq_domain_ops, intc_ic);
+ if (!intc_ic->irq_domain) {
+ ret = -ENOMEM;
+ goto err_iounmap;
+ }
+
+ raw_spin_lock_init(&intc_ic->gic_lock);
+ raw_spin_lock_init(&intc_ic->intc_lock);
+
+ /* Check all the irq numbers valid. If not, unmaps all the base and frees the data. */
+ for (i = 0; i < of_irq_count(node); i++) {
+ irq = irq_of_parse_and_map(node, i);
+ if (!irq) {
+ pr_err("Failed to get irq number\n");
+ ret = -EINVAL;
+ goto err_iounmap;
+ }
+ }
+
+ for (i = 0; i < of_irq_count(node); i++) {
+ irq = irq_of_parse_and_map(node, i);
+ irq_set_chained_handler_and_data(irq, aspeed_intc_ic_irq_handler, intc_ic);
+ }
+
+ return 0;
+
+err_iounmap:
+ iounmap(intc_ic->base);
+err_free_ic:
+ kfree(intc_ic);
+ return ret;
+}
+
+IRQCHIP_DECLARE(ast2700_intc_ic, "aspeed,ast2700-intc-ic", aspeed_intc_ic_of_init);
--
2.34.1On Wed, Oct 09, 2024 at 07:58:13PM +0800, Kevin Chen wrote:
> +static int __init aspeed_intc_ic_of_init(struct device_node *node,
> + struct device_node *parent)
> +{
> + struct aspeed_intc_ic *intc_ic;
> + int ret = 0;
> + int irq, i;
> +
> + intc_ic = kzalloc(sizeof(*intc_ic), GFP_KERNEL);
> + if (!intc_ic)
> + return -ENOMEM;
> +
> + intc_ic->base = of_iomap(node, 0);
> + if (!intc_ic->base) {
> + pr_err("Failed to iomap intc_ic base\n");
> + ret = -ENOMEM;
> + goto err_free_ic;
> + }
> + writel(0xffffffff, intc_ic->base + INTC_INT_STATUS_REG);
> + writel(0x0, intc_ic->base + INTC_INT_ENABLE_REG);
> +
> + intc_ic->irq_domain = irq_domain_add_linear(node, 32,
> + &aspeed_intc_ic_irq_domain_ops, intc_ic);
> + if (!intc_ic->irq_domain) {
> + ret = -ENOMEM;
> + goto err_iounmap;
> + }
> +
> + raw_spin_lock_init(&intc_ic->gic_lock);
> + raw_spin_lock_init(&intc_ic->intc_lock);
> +
> + /* Check all the irq numbers valid. If not, unmaps all the base and frees the data. */
> + for (i = 0; i < of_irq_count(node); i++) {
> + irq = irq_of_parse_and_map(node, i);
> + if (!irq) {
> + pr_err("Failed to get irq number\n");
> + ret = -EINVAL;
> + goto err_iounmap;
> + }
> + }
> +
> + for (i = 0; i < of_irq_count(node); i++) {
> + irq = irq_of_parse_and_map(node, i);
> + irq_set_chained_handler_and_data(irq, aspeed_intc_ic_irq_handler, intc_ic);
There is an extra tab on this line.
regards,
dan carpenter
> + }
> +
> + return 0;
…
> 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
On Wed, Oct 09 2024 at 14:32, Markus Elfring wrote:
>> +
>> + chained_irq_enter(chip, desc);
>
> Would you become interested to collaborate with another scoped guard
> for this programming interface?
Collaborate in which way? What are you collaborating on?
You are merely asking people to do work which you think is useful. You
can do that, but that does not make it useful.
Making a guard variant of chained_irq_enter/exit needs some thought and
a general plan for cleaning the whole chained irq usage up. It's on the
cleanup list already with quite some other items.
We are not adhoc adding a guard variant because guards are hip right
now. And no this does not need a scoped variant ever.
guards are not the panacea for everything.
> https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained_irq.h#L13
Please refrain from these silly links. People know to find the functions
on their own.
Kevin, please update the change log, add your SOB and move the local
variables (unsigned long bit, status;) into the scoped_guard() zone.
Leave chained_irq_enter/exit() alone and resubmit.
Thanks,
tglx
> Making a guard variant of chained_irq_enter/exit needs some thought and > a general plan for cleaning the whole chained irq usage up. It's on the > cleanup list already with quite some other items. I became also curious how API usage will evolve further here. > We are not adhoc adding a guard variant because guards are hip right now. Application interests are growing, aren't they? > And no this does not need a scoped variant ever. There are subsystems which seem to prefer such a programming interface occasionally. > guards are not the panacea for everything. Their usage might become more popular. Regards, Markus
© 2016 - 2024 Red Hat, Inc.