This patchset adds external interrupt controller driver for the StarFive
JH81000 SoC. It can be used to handle high-level input interrupt signals.
It also send the output interrupt signal to RISC-V PLIC.
changes since v1:
- Rebased on tag v6.8-rc1.
- Dropped store reset_contorl.
- Replaced "of_reset_control_get_by_index" with of_reset_control_get_exclusive
- Printed the error code via %pe
v1: https://lore.kernel.org/all/[email protected]/
Changhuang Liang (2):
dt-bindings: interrupt-controller: Add starfive,jh8100-intc
irqchip: Add StarFive external interrupt controller
.../starfive,jh8100-intc.yaml | 61 ++++++
MAINTAINERS | 6 +
drivers/irqchip/Kconfig | 11 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-starfive-jh8100-intc.c | 180 ++++++++++++++++++
5 files changed, 259 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml
create mode 100644 drivers/irqchip/irq-starfive-jh8100-intc.c
--
2.25.1
StarFive SoCs like the JH8100 use a interrupt controller. Add a binding
for it.
Signed-off-by: Changhuang Liang <[email protected]>
Reviewed-by: Ley Foon Tan <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../starfive,jh8100-intc.yaml | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml
diff --git a/Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml
new file mode 100644
index 000000000000..ada5788602d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/starfive,jh8100-intc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive External Interrupt Controller
+
+description:
+ StarFive SoC JH8100 contain a external interrupt controller. It can be used
+ to handle high-level input interrupt signals. It also send the output
+ interrupt signal to RISC-V PLIC.
+
+maintainers:
+ - Changhuang Liang <[email protected]>
+
+properties:
+ compatible:
+ const: starfive,jh8100-intc
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ description: APB clock for the interrupt controller
+ maxItems: 1
+
+ resets:
+ description: APB reset for the interrupt controller
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-controller: true
+
+ "#interrupt-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - resets
+ - interrupts
+ - interrupt-controller
+ - "#interrupt-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ interrupt-controller@12260000 {
+ compatible = "starfive,jh8100-intc";
+ reg = <0x12260000 0x10000>;
+ clocks = <&syscrg_ne 76>;
+ resets = <&syscrg_ne 13>;
+ interrupts = <45>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
--
2.25.1
Add StarFive external interrupt controller for JH8100 SoC.
Signed-off-by: Changhuang Liang <[email protected]>
Reviewed-by: Ley Foon Tan <[email protected]>
---
MAINTAINERS | 6 +
drivers/irqchip/Kconfig | 11 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-starfive-jh8100-intc.c | 180 +++++++++++++++++++++
4 files changed, 198 insertions(+)
create mode 100644 drivers/irqchip/irq-starfive-jh8100-intc.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 8d1052fa6a69..ef678f04c830 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20956,6 +20956,12 @@ F: Documentation/devicetree/bindings/phy/starfive,jh7110-usb-phy.yaml
F: drivers/phy/starfive/phy-jh7110-pcie.c
F: drivers/phy/starfive/phy-jh7110-usb.c
+STARFIVE JH8100 EXTERNAL INTERRUPT CONTROLLER DRIVER
+M: Changhuang Liang <[email protected]>
+S: Supported
+F: Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml
+F: drivers/irqchip/irq-starfive-jh8100-intc.c
+
STATIC BRANCH/CALL
M: Peter Zijlstra <[email protected]>
M: Josh Poimboeuf <[email protected]>
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index f7149d0f3d45..72c07a12f5e1 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -546,6 +546,17 @@ config SIFIVE_PLIC
select IRQ_DOMAIN_HIERARCHY
select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP
+config STARFIVE_JH8100_INTC
+ bool "StarFive JH8100 External Interrupt Controller"
+ depends on ARCH_STARFIVE || COMPILE_TEST
+ default ARCH_STARFIVE
+ select IRQ_DOMAIN_HIERARCHY
+ help
+ This enables support for the INTC chip found in StarFive JH8100
+ SoC.
+
+ If you don't know what to do here, say Y.
+
config EXYNOS_IRQ_COMBINER
bool "Samsung Exynos IRQ combiner support" if COMPILE_TEST
depends on (ARCH_EXYNOS && ARM) || COMPILE_TEST
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index ffd945fe71aa..ec4a18380998 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC) += irq-csky-mpintc.o
obj-$(CONFIG_CSKY_APB_INTC) += irq-csky-apb-intc.o
obj-$(CONFIG_RISCV_INTC) += irq-riscv-intc.o
obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o
+obj-$(CONFIG_STARFIVE_JH8100_INTC) += irq-starfive-jh8100-intc.o
obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o
obj-$(CONFIG_IMX_INTMUX) += irq-imx-intmux.o
obj-$(CONFIG_IMX_MU_MSI) += irq-imx-mu-msi.o
diff --git a/drivers/irqchip/irq-starfive-jh8100-intc.c b/drivers/irqchip/irq-starfive-jh8100-intc.c
new file mode 100644
index 000000000000..344f7d871518
--- /dev/null
+++ b/drivers/irqchip/irq-starfive-jh8100-intc.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * StarFive JH8100 External Interrupt Controller driver
+ *
+ * Copyright (C) 2023 StarFive Technology Co., Ltd.
+ *
+ * Author: Changhuang Liang <[email protected]>
+ */
+
+#define pr_fmt(fmt) "irq-starfive-jh8100: " fmt
+
+#include <linux/bitops.h>
+#include <linux/clk.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/reset.h>
+
+#define STARFIVE_INTC_SRC0_CLEAR 0x10
+#define STARFIVE_INTC_SRC0_MASK 0x14
+#define STARFIVE_INTC_SRC0_INT 0x1c
+
+#define STARFIVE_INTC_SRC_IRQ_NUM 32
+
+struct starfive_irq_chip {
+ void __iomem *base;
+ struct irq_domain *root_domain;
+ struct clk *clk;
+};
+
+static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32 reg, u32 mask, u32 data)
+{
+ u32 value;
+
+ value = ioread32(irqc->base + reg) & ~mask;
+ data &= mask;
+ data |= value;
+ iowrite32(data, irqc->base + reg);
+}
+
+static void starfive_intc_unmask(struct irq_data *d)
+{
+ struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+
+ starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), 0);
+}
+
+static void starfive_intc_mask(struct irq_data *d)
+{
+ struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+
+ starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), BIT(d->hwirq));
+}
+
+static struct irq_chip intc_dev = {
+ .name = "starfive jh8100 intc",
+ .irq_unmask = starfive_intc_unmask,
+ .irq_mask = starfive_intc_mask,
+};
+
+static int starfive_intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq)
+{
+ irq_domain_set_info(d, irq, hwirq, &intc_dev, d->host_data,
+ handle_level_irq, NULL, NULL);
+
+ return 0;
+}
+
+static const struct irq_domain_ops starfive_intc_domain_ops = {
+ .xlate = irq_domain_xlate_onecell,
+ .map = starfive_intc_map,
+};
+
+static void starfive_intc_irq_handler(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct starfive_irq_chip *irqc = irq_data_get_irq_handler_data(&desc->irq_data);
+ unsigned long value = 0;
+ int hwirq;
+
+ chained_irq_enter(chip, desc);
+
+ value = ioread32(irqc->base + STARFIVE_INTC_SRC0_INT);
+ while (value) {
+ hwirq = ffs(value) - 1;
+
+ generic_handle_domain_irq(irqc->root_domain, hwirq);
+
+ starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq), BIT(hwirq));
+ starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq), 0);
+
+ clear_bit(hwirq, &value);
+ }
+
+ chained_irq_exit(chip, desc);
+}
+
+static int __init starfive_intc_init(struct device_node *intc,
+ struct device_node *parent)
+{
+ struct starfive_irq_chip *irqc;
+ struct reset_control *rst;
+ int ret;
+ int parent_irq;
+
+ irqc = kzalloc(sizeof(*irqc), GFP_KERNEL);
+ if (!irqc)
+ return -ENOMEM;
+
+ irqc->base = of_iomap(intc, 0);
+ if (!irqc->base) {
+ pr_err("Unable to map IC registers\n");
+ ret = -ENXIO;
+ goto err_free;
+ }
+
+ rst = of_reset_control_get_exclusive(intc, NULL);
+ if (IS_ERR(rst)) {
+ pr_err("Unable to get reset control %pe\n", rst);
+ ret = PTR_ERR(rst);
+ goto err_unmap;
+ }
+
+ irqc->clk = of_clk_get(intc, 0);
+ if (IS_ERR(irqc->clk)) {
+ pr_err("Unable to get clock\n");
+ ret = PTR_ERR(irqc->clk);
+ goto err_rst;
+ }
+
+ ret = reset_control_deassert(rst);
+ if (ret)
+ goto err_clk;
+
+ ret = clk_prepare_enable(irqc->clk);
+ if (ret)
+ goto err_clk;
+
+ irqc->root_domain = irq_domain_add_linear(intc, STARFIVE_INTC_SRC_IRQ_NUM,
+ &starfive_intc_domain_ops, irqc);
+ if (!irqc->root_domain) {
+ pr_err("Unable to create IRQ domain\n");
+ ret = -EINVAL;
+ goto err_clk;
+ }
+
+ parent_irq = of_irq_get(intc, 0);
+ if (parent_irq < 0) {
+ pr_err("Failed to get main IRQ: %d\n", parent_irq);
+ ret = parent_irq;
+ goto err_clk;
+ }
+
+ irq_set_chained_handler_and_data(parent_irq, starfive_intc_irq_handler, irqc);
+
+ pr_info("Interrupt controller register, nr_irqs %d\n", STARFIVE_INTC_SRC_IRQ_NUM);
+
+ return 0;
+
+err_clk:
+ clk_put(irqc->clk);
+err_rst:
+ reset_control_put(rst);
+err_unmap:
+ iounmap(irqc->base);
+err_free:
+ kfree(irqc);
+ return ret;
+}
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(starfive_intc)
+IRQCHIP_MATCH("starfive,jh8100-intc", starfive_intc_init)
+IRQCHIP_PLATFORM_DRIVER_END(starfive_intc)
+
+MODULE_DESCRIPTION("StarFive JH8100 External Interrupt Controller");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Changhuang Liang <[email protected]>");
--
2.25.1
On Mon, Jan 29 2024 at 21:58, Changhuang Liang wrote:
> +
> +struct starfive_irq_chip {
> + void __iomem *base;
> + struct irq_domain *root_domain;
> + struct clk *clk;
> +};
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
Please.
> +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32 reg, u32 mask, u32 data)
> +{
> + u32 value;
> +
> + value = ioread32(irqc->base + reg) & ~mask;
> + data &= mask;
Why?
> + data |= value;
> + iowrite32(data, irqc->base + reg);
How is this serialized against concurrent invocations of this code on
different CPUs for different interrupts?
It's not and this requires a raw_spinlock for protection.
> +}
> +
> +static void starfive_intc_unmask(struct irq_data *d)
> +{
> + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d);
> +
> + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), 0);
> +}
> +
> +static void starfive_intc_mask(struct irq_data *d)
> +{
> + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d);
> +
> + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), BIT(d->hwirq));
> +}
> +
> +static struct irq_chip intc_dev = {
> + .name = "starfive jh8100 intc",
> + .irq_unmask = starfive_intc_unmask,
> + .irq_mask = starfive_intc_mask,
> +};
See documentation please.
> +static int starfive_intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq)
> +{
> + irq_domain_set_info(d, irq, hwirq, &intc_dev, d->host_data,
> + handle_level_irq, NULL, NULL);
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops starfive_intc_domain_ops = {
> + .xlate = irq_domain_xlate_onecell,
> + .map = starfive_intc_map,
> +};
Ditto.
> +static void starfive_intc_irq_handler(struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct starfive_irq_chip *irqc = irq_data_get_irq_handler_data(&desc->irq_data);
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
> + unsigned long value = 0;
Pointless initialization
> + int hwirq;
> +
> + chained_irq_enter(chip, desc);
> +
> + value = ioread32(irqc->base + STARFIVE_INTC_SRC0_INT);
> + while (value) {
> + hwirq = ffs(value) - 1;
> +
> + generic_handle_domain_irq(irqc->root_domain, hwirq);
> +
> + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq), BIT(hwirq));
> + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq), 0);
> +
> + clear_bit(hwirq, &value);
> + }
> +
> + chained_irq_exit(chip, desc);
> +}
> +
> +static int __init starfive_intc_init(struct device_node *intc,
> + struct device_node *parent)
> +{
> + struct starfive_irq_chip *irqc;
> + struct reset_control *rst;
> + int ret;
> + int parent_irq;
See Documentation
> + irqc = kzalloc(sizeof(*irqc), GFP_KERNEL);
> + if (!irqc)
> + return -ENOMEM;
> +
> + irqc->base = of_iomap(intc, 0);
> + if (!irqc->base) {
> + pr_err("Unable to map IC registers\n");
> + ret = -ENXIO;
> + goto err_free;
> + }
> +
> + rst = of_reset_control_get_exclusive(intc, NULL);
> + if (IS_ERR(rst)) {
> + pr_err("Unable to get reset control %pe\n", rst);
> + ret = PTR_ERR(rst);
> + goto err_unmap;
> + }
> +
> + irqc->clk = of_clk_get(intc, 0);
> + if (IS_ERR(irqc->clk)) {
> + pr_err("Unable to get clock\n");
> + ret = PTR_ERR(irqc->clk);
> + goto err_rst;
> + }
> +
> + ret = reset_control_deassert(rst);
> + if (ret)
> + goto err_clk;
> +
> + ret = clk_prepare_enable(irqc->clk);
> + if (ret)
> + goto err_clk;
> +
> + irqc->root_domain = irq_domain_add_linear(intc, STARFIVE_INTC_SRC_IRQ_NUM,
> + &starfive_intc_domain_ops, irqc);
> + if (!irqc->root_domain) {
> + pr_err("Unable to create IRQ domain\n");
> + ret = -EINVAL;
> + goto err_clk;
> + }
> +
> + parent_irq = of_irq_get(intc, 0);
> + if (parent_irq < 0) {
> + pr_err("Failed to get main IRQ: %d\n", parent_irq);
> + ret = parent_irq;
> + goto err_clk;
Leaks the interrupt domain, no?
Thanks,
tglx
Hi, Thomas
Thanks for your comment.
> On Mon, Jan 29 2024 at 21:58, Changhuang Liang wrote:
[...]
> > +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32
> > +reg, u32 mask, u32 data) {
> > + u32 value;
> > +
> > + value = ioread32(irqc->base + reg) & ~mask;
> > + data &= mask;
>
> Why?
>
If I want to update the reg GENMASK(7, 4) to value 5, the data I will pass in 5 << 4
> > + data |= value;
> > + iowrite32(data, irqc->base + reg);
>
> How is this serialized against concurrent invocations of this code on different
> CPUs for different interrupts?
>
> It's not and this requires a raw_spinlock for protection.
>
Will use raw_spinlock.
> > +}
> > +
> > +static void starfive_intc_unmask(struct irq_data *d) {
> > + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d);
> > +
> > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), 0);
> > +}
> > +
> > +static void starfive_intc_mask(struct irq_data *d) {
> > + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d);
> > +
> > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq),
> > +BIT(d->hwirq)); }
> > +
[...]
> > + irqc->root_domain = irq_domain_add_linear(intc,
> STARFIVE_INTC_SRC_IRQ_NUM,
> > + &starfive_intc_domain_ops, irqc);
> > + if (!irqc->root_domain) {
> > + pr_err("Unable to create IRQ domain\n");
> > + ret = -EINVAL;
> > + goto err_clk;
> > + }
> > +
> > + parent_irq = of_irq_get(intc, 0);
> > + if (parent_irq < 0) {
> > + pr_err("Failed to get main IRQ: %d\n", parent_irq);
> > + ret = parent_irq;
> > + goto err_clk;
>
> Leaks the interrupt domain, no?
>
> Thanks,
>
Will use irq_domain_remove() free domain.
regards
Changhuang
On Sun, Feb 18 2024 at 02:36, Changhuang Liang wrote:
>> On Mon, Jan 29 2024 at 21:58, Changhuang Liang wrote:
> [...]
>> > +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32
>> > +reg, u32 mask, u32 data) {
>> > + u32 value;
>> > +
>> > + value = ioread32(irqc->base + reg) & ~mask;
>> > + data &= mask;
>>
>> Why?
>>
>
> If I want to update the reg GENMASK(7, 4) to value 5, the data I
> will pass in 5 << 4
All call sites pass a single bit to set/clear, right? So this GENMASK
argument does not make sense at all.
Thanks,
tglx
Hi, Thomas
> -----?ʼ?ԭ??-----
> ??????: Thomas Gleixner <[email protected]>
> ????ʱ??: 2024??2??20?? 17:28
> ?ռ???: Changhuang Liang <[email protected]>; Rob
> Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>;
> Philipp Zabel <[email protected]>
> ????: Leyfoon Tan <[email protected]>; Jack Zhu
> <[email protected]>; [email protected];
> [email protected]
> ????: Re: ?ظ?: [PATCH v2 2/2] irqchip: Add StarFive external interrupt
> controller
>
> On Sun, Feb 18 2024 at 02:36, Changhuang Liang wrote:
> >> On Mon, Jan 29 2024 at 21:58, Changhuang Liang wrote:
> > [...]
> >> > +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32
> >> > +reg, u32 mask, u32 data) {
> >> > + u32 value;
> >> > +
> >> > + value = ioread32(irqc->base + reg) & ~mask;
> >> > + data &= mask;
> >>
> >> Why?
> >>
> >
> > If I want to update the reg GENMASK(7, 4) to value 5, the data I
> > will pass in 5 << 4
>
> All call sites pass a single bit to set/clear, right? So this GENMASK argument
> does not make sense at all.
>
Yes, I will update this function