* [PATCH 0/3] riscv: interrupt-controller: Add T-HEAD C900 ACLINT SSWI
@ 2024-10-04 8:05 Inochi Amaoto
2024-10-04 8:05 ` [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2044 " Inochi Amaoto
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Inochi Amaoto @ 2024-10-04 8:05 UTC (permalink / raw)
To: Chen Wang, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Peter Zijlstra, Inochi Amaoto, Guo Ren, Lad Prabhakar, Hal Feng,
Heikki Krogerus, Geert Uytterhoeven
Cc: Yixun Lan, Inochi Amaoto, linux-kernel, devicetree, linux-riscv
New version of T-HEAD C920 implement a fully featured ACLINT[1] device
(This core is used by Sophgo SG2044). This ACLINT device provides a
SSWI field to support fast S-mode IPI. This SSWI device is like the
MSWI device in CLINT/ACLINT, but for S-mode. The only thing is different
from the draft is that the T-HEAD version SSWI needs to write 0 on the
SSWI address to clear the IPI.
Add full support for T-HEAD C900 SSWI device.
[1] https://github.com/riscv/riscv-aclint
Inochi Amaoto (3):
dt-bindings: interrupt-controller: Add Sophgo SG2044 ACLINT SSWI
irqchip: add T-HEAD C900 ACLINT SSWI driver
riscv: defconfig: Enable T-HEAD C900 ACLINT SSWI drivers
.../thead,c900-aclint-sswi.yaml | 58 ++++++
arch/riscv/configs/defconfig | 1 +
drivers/irqchip/Kconfig | 10 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-thead-c900-aclint-sswi.c | 169 ++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
6 files changed, 240 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
create mode 100644 drivers/irqchip/irq-thead-c900-aclint-sswi.c
--
2.46.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2044 ACLINT SSWI
2024-10-04 8:05 [PATCH 0/3] riscv: interrupt-controller: Add T-HEAD C900 ACLINT SSWI Inochi Amaoto
@ 2024-10-04 8:05 ` Inochi Amaoto
2024-10-04 15:44 ` Conor Dooley
2024-10-04 8:05 ` [PATCH 2/3] irqchip: add T-HEAD C900 ACLINT SSWI driver Inochi Amaoto
2024-10-04 8:05 ` [PATCH 3/3] riscv: defconfig: Enable T-HEAD C900 ACLINT SSWI drivers Inochi Amaoto
2 siblings, 1 reply; 9+ messages in thread
From: Inochi Amaoto @ 2024-10-04 8:05 UTC (permalink / raw)
To: Chen Wang, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Peter Zijlstra, Inochi Amaoto, Guo Ren, Lad Prabhakar, Hal Feng,
Heikki Krogerus, Geert Uytterhoeven
Cc: Yixun Lan, Inochi Amaoto, linux-kernel, devicetree, linux-riscv
Sophgo SG2044 has a new version of T-HEAD C920, which implement
a fully featured ACLINT device. This ACLINT has an extra SSWI
field to support fast S-mode IPI.
Add necessary compatible string for the T-HEAD ACLINT sswi device.
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
.../thead,c900-aclint-sswi.yaml | 58 +++++++++++++++++++
1 file changed, 58 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
diff --git a/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml b/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
new file mode 100644
index 000000000000..0106fbf3ea1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/thead,c900-aclint-sswi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo sg2044 ACLINT Supervisor-level Software Interrupt Device
+
+maintainers:
+ - Inochi Amaoto <inochiama@outlook.com>
+
+description:
+ The SSWI device is a part of the riscv ACLINT device. It provides
+ supervisor-level IPI functionality for a set of HARTs on a RISC-V
+ platform. It provides a register to set an IPI (SETSSIP) for each
+ HART connected to the SSWI device.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - sophgo,sg2044-aclint-sswi
+ - const: thead,c900-aclint-sswi
+
+ reg:
+ maxItems: 1
+
+ "#interrupt-cells":
+ const: 0
+
+ interrupt-controller: true
+
+ interrupts-extended:
+ minItems: 1
+ maxItems: 4095
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - "#interrupt-cells"
+ - interrupt-controller
+ - interrupts-extended
+
+examples:
+ - |
+ interrupt-controller@94000000 {
+ compatible = "sophgo,sg2044-aclint-sswi", "thead,c900-aclint-sswi";
+ reg = <0x94000000 0x00004000>;
+ #interrupt-cells = <0>;
+ interrupt-controller;
+ interrupts-extended = <&cpu1intc 1>,
+ <&cpu2intc 1>,
+ <&cpu3intc 1>,
+ <&cpu4intc 1>;
+ };
+...
--
2.46.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] irqchip: add T-HEAD C900 ACLINT SSWI driver
2024-10-04 8:05 [PATCH 0/3] riscv: interrupt-controller: Add T-HEAD C900 ACLINT SSWI Inochi Amaoto
2024-10-04 8:05 ` [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2044 " Inochi Amaoto
@ 2024-10-04 8:05 ` Inochi Amaoto
2024-10-06 19:50 ` Thomas Gleixner
2024-10-04 8:05 ` [PATCH 3/3] riscv: defconfig: Enable T-HEAD C900 ACLINT SSWI drivers Inochi Amaoto
2 siblings, 1 reply; 9+ messages in thread
From: Inochi Amaoto @ 2024-10-04 8:05 UTC (permalink / raw)
To: Chen Wang, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Peter Zijlstra, Inochi Amaoto, Guo Ren, Lad Prabhakar, Hal Feng,
Heikki Krogerus, Geert Uytterhoeven
Cc: Yixun Lan, Inochi Amaoto, linux-kernel, devicetree, linux-riscv
Add a driver for the T-HEAD C900 ACLINT SSWI device, which is an
enhanced implementation of the RISC-V ACLINT SSWI specification.
This device allows the system to send ipi via fast device interface.
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
drivers/irqchip/Kconfig | 10 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-thead-c900-aclint-sswi.c | 169 +++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
4 files changed, 181 insertions(+)
create mode 100644 drivers/irqchip/irq-thead-c900-aclint-sswi.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 341cd9ca5a05..32671385cbb7 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -611,6 +611,16 @@ config STARFIVE_JH8100_INTC
If you don't know what to do here, say Y.
+config THEAD_C900_ACLINT_SSWI
+ bool "THEAD C9XX ACLINT S-mode IPI Interrupt Controller"
+ depends on RISCV
+ select IRQ_DOMAIN_HIERARCHY
+ help
+ This enables support for T-HEAD specific ACLINT SSWI device
+ support.
+
+ 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 e3679ec2b9f7..583418261253 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_RISCV_APLIC_MSI) += irq-riscv-aplic-msi.o
obj-$(CONFIG_RISCV_IMSIC) += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o
obj-$(CONFIG_STARFIVE_JH8100_INTC) += irq-starfive-jh8100-intc.o
+obj-$(CONFIG_THEAD_C900_ACLINT_SSWI) += irq-thead-c900-aclint-sswi.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-thead-c900-aclint-sswi.c b/drivers/irqchip/irq-thead-c900-aclint-sswi.c
new file mode 100644
index 000000000000..7bd06369b409
--- /dev/null
+++ b/drivers/irqchip/irq-thead-c900-aclint-sswi.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Inochi Amaoto <inochiama@gmail.com>
+ */
+
+#define pr_fmt(fmt) "thead-c900-aclint-sswi: " fmt
+#include <linux/acpi.h>
+#include <linux/cpu.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/pci.h>
+#include <linux/spinlock.h>
+#include <linux/smp.h>
+
+#define ACLINT_xSWI_REGISTER_SIZE 4
+
+struct aclint_sswi_cpu_config {
+ void __iomem *reg;
+ u32 offset;
+};
+
+static int sswi_ipi_virq __ro_after_init;
+static DEFINE_PER_CPU(struct aclint_sswi_cpu_config, sswi_cpus);
+
+static void thead_aclint_sswi_ipi_send(unsigned int cpu)
+{
+ struct aclint_sswi_cpu_config *config = per_cpu_ptr(&sswi_cpus, cpu);
+
+ writel_relaxed(0x1, config->reg + config->offset);
+}
+
+static void thead_aclint_sswi_ipi_clear(void)
+{
+ unsigned int cpu = smp_processor_id();
+ struct aclint_sswi_cpu_config *config = per_cpu_ptr(&sswi_cpus, cpu);
+
+ writel_relaxed(0x0, config->reg + config->offset);
+}
+
+static void thead_aclint_sswi_ipi_handle(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+
+ chained_irq_enter(chip, desc);
+
+ csr_clear(CSR_IP, IE_SIE);
+ thead_aclint_sswi_ipi_clear();
+
+ ipi_mux_process();
+
+ chained_irq_exit(chip, desc);
+}
+
+static int aclint_sswi_ipi_starting_cpu(unsigned int cpu)
+{
+ enable_percpu_irq(sswi_ipi_virq, irq_get_trigger_type(sswi_ipi_virq));
+ return 0;
+}
+
+static int aclint_sswi_parse_irq(struct fwnode_handle *fwnode,
+ void __iomem *reg)
+{
+ struct of_phandle_args parent;
+ unsigned long hartid;
+ u32 contexts, i;
+ int rc, cpu;
+ struct aclint_sswi_cpu_config *config;
+
+ contexts = of_irq_count(to_of_node(fwnode));
+ if (WARN_ON(!(contexts))) {
+ pr_err("%pfwP: no ACLINT SSWI context available\n", fwnode);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < contexts; i++) {
+ rc = of_irq_parse_one(to_of_node(fwnode), i, &parent);
+ if (rc)
+ return rc;
+
+ rc = riscv_of_parent_hartid(parent.np, &hartid);
+ if (rc)
+ return rc;
+
+ if (parent.args[0] != RV_IRQ_SOFT)
+ return -ENOTSUPP;
+
+ cpu = riscv_hartid_to_cpuid(hartid);
+ config = per_cpu_ptr(&sswi_cpus, cpu);
+
+ config->offset = i * ACLINT_xSWI_REGISTER_SIZE;
+ config->reg = reg;
+ }
+
+ pr_info("%pfwP: register %u CPU\n", fwnode, contexts);
+
+ return 0;
+}
+
+static int __init aclint_sswi_probe(struct fwnode_handle *fwnode)
+{
+ void __iomem *reg;
+ struct irq_domain *domain;
+ int virq, rc;
+
+ if (!is_of_node(fwnode))
+ return -EINVAL;
+
+ reg = of_iomap(to_of_node(fwnode), 0);
+ if (!reg)
+ return -ENOMEM;
+
+ /* Parse SSWI setting */
+ rc = aclint_sswi_parse_irq(fwnode, reg);
+ if (rc < 0)
+ return rc;
+
+ /* If mulitple SSWI devices are present, do not register irq again */
+ if (sswi_ipi_virq)
+ return 0;
+
+ /* Find and create irq domain */
+ domain = irq_find_matching_fwnode(riscv_get_intc_hwnode(), DOMAIN_BUS_ANY);
+ if (!domain) {
+ pr_err("%pfwP: Failed to find INTC domain\n", fwnode);
+ return -ENOENT;
+ }
+
+ sswi_ipi_virq = irq_create_mapping(domain, RV_IRQ_SOFT);
+ if (!sswi_ipi_virq) {
+ pr_err("unable to create ACLINT SSWI IRQ mapping\n");
+ return -ENOMEM;
+ }
+
+ /* Register SSWI irq and handler */
+ virq = ipi_mux_create(BITS_PER_BYTE, thead_aclint_sswi_ipi_send);
+ if (virq <= 0) {
+ pr_err("unable to create muxed IPIs\n");
+ irq_dispose_mapping(sswi_ipi_virq);
+ return virq < 0 ? virq : -ENOMEM;
+ }
+
+ irq_set_chained_handler(sswi_ipi_virq, thead_aclint_sswi_ipi_handle);
+
+ cpuhp_setup_state(CPUHP_AP_IRQ_THEAD_ACLINT_SSWI_STARTING,
+ "irqchip/thead-aclint-sswi:starting",
+ aclint_sswi_ipi_starting_cpu, NULL);
+
+ riscv_ipi_set_virq_range(virq, BITS_PER_BYTE);
+
+ /* Announce that SSWI is providing IPIs */
+ pr_info("providing IPIs using THEAD ACLINT SSWI\n");
+
+ return 0;
+}
+
+static int __init aclint_sswi_early_probe(struct device_node *node,
+ struct device_node *parent)
+{
+ return aclint_sswi_probe(&node->fwnode);
+}
+
+IRQCHIP_DECLARE(thead_aclint_sswi, "thead,c900-aclint-sswi", aclint_sswi_early_probe);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 2361ed4d2b15..799052249c7b 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -147,6 +147,7 @@ enum cpuhp_state {
CPUHP_AP_IRQ_EIOINTC_STARTING,
CPUHP_AP_IRQ_AVECINTC_STARTING,
CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
+ CPUHP_AP_IRQ_THEAD_ACLINT_SSWI_STARTING,
CPUHP_AP_IRQ_RISCV_IMSIC_STARTING,
CPUHP_AP_IRQ_RISCV_SBI_IPI_STARTING,
CPUHP_AP_ARM_MVEBU_COHERENCY,
--
2.46.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] riscv: defconfig: Enable T-HEAD C900 ACLINT SSWI drivers
2024-10-04 8:05 [PATCH 0/3] riscv: interrupt-controller: Add T-HEAD C900 ACLINT SSWI Inochi Amaoto
2024-10-04 8:05 ` [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2044 " Inochi Amaoto
2024-10-04 8:05 ` [PATCH 2/3] irqchip: add T-HEAD C900 ACLINT SSWI driver Inochi Amaoto
@ 2024-10-04 8:05 ` Inochi Amaoto
2 siblings, 0 replies; 9+ messages in thread
From: Inochi Amaoto @ 2024-10-04 8:05 UTC (permalink / raw)
To: Chen Wang, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Peter Zijlstra, Inochi Amaoto, Guo Ren, Lad Prabhakar, Hal Feng,
Heikki Krogerus, Geert Uytterhoeven
Cc: Yixun Lan, Inochi Amaoto, linux-kernel, devicetree, linux-riscv
Add support for T-HEAD C900 ACLINT SSWI irqchip.
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
arch/riscv/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 2341393cfac1..5b1d6325df85 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -256,6 +256,7 @@ CONFIG_RPMSG_CTRL=y
CONFIG_RPMSG_VIRTIO=y
CONFIG_PM_DEVFREQ=y
CONFIG_IIO=y
+CONFIG_THEAD_C900_ACLINT_SSWI=y
CONFIG_PHY_SUN4I_USB=m
CONFIG_PHY_STARFIVE_JH7110_DPHY_RX=m
CONFIG_PHY_STARFIVE_JH7110_PCIE=m
--
2.46.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2044 ACLINT SSWI
2024-10-04 8:05 ` [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2044 " Inochi Amaoto
@ 2024-10-04 15:44 ` Conor Dooley
2024-10-05 0:46 ` Inochi Amaoto
0 siblings, 1 reply; 9+ messages in thread
From: Conor Dooley @ 2024-10-04 15:44 UTC (permalink / raw)
To: Inochi Amaoto
Cc: Chen Wang, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Peter Zijlstra, Inochi Amaoto, Guo Ren, Lad Prabhakar, Hal Feng,
Heikki Krogerus, Geert Uytterhoeven, Yixun Lan, linux-kernel,
devicetree, linux-riscv
[-- Attachment #1: Type: text/plain, Size: 1768 bytes --]
On Fri, Oct 04, 2024 at 04:05:55PM +0800, Inochi Amaoto wrote:
> Sophgo SG2044 has a new version of T-HEAD C920, which implement
> a fully featured ACLINT device. This ACLINT has an extra SSWI
> field to support fast S-mode IPI.
>
> Add necessary compatible string for the T-HEAD ACLINT sswi device.
>
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
> .../thead,c900-aclint-sswi.yaml | 58 +++++++++++++++++++
> 1 file changed, 58 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml b/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
> new file mode 100644
> index 000000000000..0106fbf3ea1f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/thead,c900-aclint-sswi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sophgo sg2044 ACLINT Supervisor-level Software Interrupt Device
> +
> +maintainers:
> + - Inochi Amaoto <inochiama@outlook.com>
> +
> +description:
> + The SSWI device is a part of the riscv ACLINT device. It provides
> + supervisor-level IPI functionality for a set of HARTs on a RISC-V
> + platform. It provides a register to set an IPI (SETSSIP) for each
> + HART connected to the SSWI device.
If it is part of the aclint, why should it have a separate node, rather
than be part of the existing aclint node as a third reg property?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2044 ACLINT SSWI
2024-10-04 15:44 ` Conor Dooley
@ 2024-10-05 0:46 ` Inochi Amaoto
2024-10-10 16:16 ` Conor Dooley
0 siblings, 1 reply; 9+ messages in thread
From: Inochi Amaoto @ 2024-10-05 0:46 UTC (permalink / raw)
To: Conor Dooley, Inochi Amaoto
Cc: Chen Wang, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Peter Zijlstra, Inochi Amaoto, Guo Ren, Lad Prabhakar, Hal Feng,
Heikki Krogerus, Geert Uytterhoeven, Yixun Lan, linux-kernel,
devicetree, linux-riscv
On Fri, Oct 04, 2024 at 04:44:22PM +0100, Conor Dooley wrote:
> On Fri, Oct 04, 2024 at 04:05:55PM +0800, Inochi Amaoto wrote:
> > Sophgo SG2044 has a new version of T-HEAD C920, which implement
> > a fully featured ACLINT device. This ACLINT has an extra SSWI
> > field to support fast S-mode IPI.
> >
> > Add necessary compatible string for the T-HEAD ACLINT sswi device.
> >
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> > .../thead,c900-aclint-sswi.yaml | 58 +++++++++++++++++++
> > 1 file changed, 58 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml b/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
> > new file mode 100644
> > index 000000000000..0106fbf3ea1f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/interrupt-controller/thead,c900-aclint-sswi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sophgo sg2044 ACLINT Supervisor-level Software Interrupt Device
> > +
> > +maintainers:
> > + - Inochi Amaoto <inochiama@outlook.com>
> > +
> > +description:
> > + The SSWI device is a part of the riscv ACLINT device. It provides
> > + supervisor-level IPI functionality for a set of HARTs on a RISC-V
> > + platform. It provides a register to set an IPI (SETSSIP) for each
> > + HART connected to the SSWI device.
>
> If it is part of the aclint, why should it have a separate node, rather
> than be part of the existing aclint node as a third reg property?
For aclint, the current nodes that have documented are mswi and mtime.
Since the mtime is a M-mode time source, it is not suitable to add the
sswi reg into this device. For mswi, it is OK to add a sswi reg, but
this will cause problem while checking "interrupt-extend". Do we just
double the maxItem? Or just left it unchanged?
Another reason to add it as a separate node is that the draft says
sswi can be multiple. If we add this device by adding reg. It will be
hard if we have multiple sswi devices but one mswi device.
Regard,
Inochi
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] irqchip: add T-HEAD C900 ACLINT SSWI driver
2024-10-04 8:05 ` [PATCH 2/3] irqchip: add T-HEAD C900 ACLINT SSWI driver Inochi Amaoto
@ 2024-10-06 19:50 ` Thomas Gleixner
2024-10-07 0:38 ` Inochi Amaoto
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2024-10-06 19:50 UTC (permalink / raw)
To: Inochi Amaoto, Chen Wang, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Peter Zijlstra, Inochi Amaoto, Guo Ren, Lad Prabhakar, Hal Feng,
Heikki Krogerus, Geert Uytterhoeven
Cc: Yixun Lan, Inochi Amaoto, linux-kernel, devicetree, linux-riscv
On Fri, Oct 04 2024 at 16:05, Inochi Amaoto wrote:
> +#define pr_fmt(fmt) "thead-c900-aclint-sswi: " fmt
> +#include <linux/acpi.h>
What is this header used for?
> +static void thead_aclint_sswi_ipi_clear(void)
> +{
> + unsigned int cpu = smp_processor_id();
> + struct aclint_sswi_cpu_config *config = per_cpu_ptr(&sswi_cpus, cpu);
That's an unnecessary indirection.
*config = __this_cpu_ptr(&sswi_cpus);
is what you want here.
> + writel_relaxed(0x0, config->reg + config->offset);
> +}
...
> +static int aclint_sswi_parse_irq(struct fwnode_handle *fwnode,
> + void __iomem *reg)
Please avoid line breaks and use up to 100 characters per line.
> +{
> + struct of_phandle_args parent;
> + unsigned long hartid;
> + u32 contexts, i;
> + int rc, cpu;
> + struct aclint_sswi_cpu_config *config;
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
> +
> + contexts = of_irq_count(to_of_node(fwnode));
> + if (WARN_ON(!(contexts))) {
That WARN_ON() is pointless. The call chain is known and the pr_err() is
sufficient.
> + pr_err("%pfwP: no ACLINT SSWI context available\n", fwnode);
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < contexts; i++) {
> + rc = of_irq_parse_one(to_of_node(fwnode), i, &parent);
> + if (rc)
> + return rc;
> +
> + rc = riscv_of_parent_hartid(parent.np, &hartid);
> + if (rc)
> + return rc;
> +
> + if (parent.args[0] != RV_IRQ_SOFT)
> + return -ENOTSUPP;
> +
> + cpu = riscv_hartid_to_cpuid(hartid);
> + config = per_cpu_ptr(&sswi_cpus, cpu);
> +
> + config->offset = i * ACLINT_xSWI_REGISTER_SIZE;
> + config->reg = reg;
Why do you need config->reg and config->offset? All call sites access
the register via:
config->reg + config->offset
So you can spare the exercise of adding the offset in the hotpath by
adding it at setup time, no?
> + }
> +
> + pr_info("%pfwP: register %u CPU\n", fwnode, contexts);
...CPU%s\n", fwnode, contexts, str_plural(contexts));
> +
> + return 0;
> +}
> +
> +static int __init aclint_sswi_probe(struct fwnode_handle *fwnode)
> +{
> + void __iomem *reg;
> + struct irq_domain *domain;
> + int virq, rc;
See above.
> + if (!is_of_node(fwnode))
> + return -EINVAL;
> +
> + reg = of_iomap(to_of_node(fwnode), 0);
> + if (!reg)
> + return -ENOMEM;
> +
> + /* Parse SSWI setting */
> + rc = aclint_sswi_parse_irq(fwnode, reg);
> + if (rc < 0)
> + return rc;
> +
> + /* If mulitple SSWI devices are present, do not register irq again */
> + if (sswi_ipi_virq)
> + return 0;
> +
> + /* Find and create irq domain */
Which domain is created here?
> + domain = irq_find_matching_fwnode(riscv_get_intc_hwnode(), DOMAIN_BUS_ANY);
> + if (!domain) {
> + pr_err("%pfwP: Failed to find INTC domain\n", fwnode);
> + return -ENOENT;
> + }
> +
> + sswi_ipi_virq = irq_create_mapping(domain, RV_IRQ_SOFT);
> + if (!sswi_ipi_virq) {
> + pr_err("unable to create ACLINT SSWI IRQ mapping\n");
> + return -ENOMEM;
> + }
> +
> + /* Register SSWI irq and handler */
> + virq = ipi_mux_create(BITS_PER_BYTE, thead_aclint_sswi_ipi_send);
> + if (virq <= 0) {
> + pr_err("unable to create muxed IPIs\n");
> + irq_dispose_mapping(sswi_ipi_virq);
> + return virq < 0 ? virq : -ENOMEM;
> + }
> +
> + irq_set_chained_handler(sswi_ipi_virq, thead_aclint_sswi_ipi_handle);
> +
> + cpuhp_setup_state(CPUHP_AP_IRQ_THEAD_ACLINT_SSWI_STARTING,
> + "irqchip/thead-aclint-sswi:starting",
> + aclint_sswi_ipi_starting_cpu, NULL);
The startup callback enables the per CPU interrupt. When a CPU is
offlined then the per CPU interrupt stays enabled because the teardown
callback is NULL. I'm not convinced that this is a good idea.
> +
> + riscv_ipi_set_virq_range(virq, BITS_PER_BYTE);
> +
> + /* Announce that SSWI is providing IPIs */
> + pr_info("providing IPIs using THEAD ACLINT SSWI\n");
> +
> + return 0;
> +}
> +
> +static int __init aclint_sswi_early_probe(struct device_node *node,
> + struct device_node *parent)
> +{
> + return aclint_sswi_probe(&node->fwnode);
> +}
What's the point of this indirection?
> +
Pointless newline.
> +IRQCHIP_DECLARE(thead_aclint_sswi, "thead,c900-aclint-sswi", aclint_sswi_early_probe);
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] irqchip: add T-HEAD C900 ACLINT SSWI driver
2024-10-06 19:50 ` Thomas Gleixner
@ 2024-10-07 0:38 ` Inochi Amaoto
0 siblings, 0 replies; 9+ messages in thread
From: Inochi Amaoto @ 2024-10-07 0:38 UTC (permalink / raw)
To: Thomas Gleixner, Inochi Amaoto, Chen Wang, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Peter Zijlstra, Inochi Amaoto, Guo Ren, Lad Prabhakar,
Hal Feng, Heikki Krogerus, Geert Uytterhoeven
Cc: Yixun Lan, linux-kernel, devicetree, linux-riscv
On Sun, Oct 06, 2024 at 09:50:39PM +0200, Thomas Gleixner wrote:
> On Fri, Oct 04 2024 at 16:05, Inochi Amaoto wrote:
>
> > +#define pr_fmt(fmt) "thead-c900-aclint-sswi: " fmt
> > +#include <linux/acpi.h>
>
> What is this header used for?
>
This is copy-pasted error, I wiil remove it.
> > +static void thead_aclint_sswi_ipi_clear(void)
> > +{
> > + unsigned int cpu = smp_processor_id();
> > + struct aclint_sswi_cpu_config *config = per_cpu_ptr(&sswi_cpus, cpu);
>
> That's an unnecessary indirection.
>
> *config = __this_cpu_ptr(&sswi_cpus);
>
> is what you want here.
Thanks.
>
> > + writel_relaxed(0x0, config->reg + config->offset);
> > +}
>
> ...
>
> > +static int aclint_sswi_parse_irq(struct fwnode_handle *fwnode,
> > + void __iomem *reg)
>
> Please avoid line breaks and use up to 100 characters per line.
>
> > +{
> > + struct of_phandle_args parent;
> > + unsigned long hartid;
> > + u32 contexts, i;
> > + int rc, cpu;
> > + struct aclint_sswi_cpu_config *config;
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
>
> > +
> > + contexts = of_irq_count(to_of_node(fwnode));
> > + if (WARN_ON(!(contexts))) {
>
> That WARN_ON() is pointless. The call chain is known and the pr_err() is
> sufficient.
>
> > + pr_err("%pfwP: no ACLINT SSWI context available\n", fwnode);
> > + return -EINVAL;
> > + }
> > +
> > + for (i = 0; i < contexts; i++) {
> > + rc = of_irq_parse_one(to_of_node(fwnode), i, &parent);
> > + if (rc)
> > + return rc;
> > +
> > + rc = riscv_of_parent_hartid(parent.np, &hartid);
> > + if (rc)
> > + return rc;
> > +
> > + if (parent.args[0] != RV_IRQ_SOFT)
> > + return -ENOTSUPP;
> > +
> > + cpu = riscv_hartid_to_cpuid(hartid);
> > + config = per_cpu_ptr(&sswi_cpus, cpu);
> > +
> > + config->offset = i * ACLINT_xSWI_REGISTER_SIZE;
> > + config->reg = reg;
>
> Why do you need config->reg and config->offset? All call sites access
> the register via:
>
> config->reg + config->offset
>
> So you can spare the exercise of adding the offset in the hotpath by
> adding it at setup time, no?
Thanks, I only consider supporting multiple device, but forgot that it
can be computed earily.
>
>
> > + }
> > +
> > + pr_info("%pfwP: register %u CPU\n", fwnode, contexts);
>
> ...CPU%s\n", fwnode, contexts, str_plural(contexts));
>
> > +
> > + return 0;
> > +}
> > +
> > +static int __init aclint_sswi_probe(struct fwnode_handle *fwnode)
> > +{
> > + void __iomem *reg;
> > + struct irq_domain *domain;
> > + int virq, rc;
>
> See above.
>
> > + if (!is_of_node(fwnode))
> > + return -EINVAL;
> > +
> > + reg = of_iomap(to_of_node(fwnode), 0);
> > + if (!reg)
> > + return -ENOMEM;
> > +
> > + /* Parse SSWI setting */
> > + rc = aclint_sswi_parse_irq(fwnode, reg);
> > + if (rc < 0)
> > + return rc;
> > +
> > + /* If mulitple SSWI devices are present, do not register irq again */
> > + if (sswi_ipi_virq)
> > + return 0;
> > +
> > + /* Find and create irq domain */
>
> Which domain is created here?
>
It will create an IPI domain. I will update the comment.
> > + domain = irq_find_matching_fwnode(riscv_get_intc_hwnode(), DOMAIN_BUS_ANY);
> > + if (!domain) {
> > + pr_err("%pfwP: Failed to find INTC domain\n", fwnode);
> > + return -ENOENT;
> > + }
> > +
> > + sswi_ipi_virq = irq_create_mapping(domain, RV_IRQ_SOFT);
> > + if (!sswi_ipi_virq) {
> > + pr_err("unable to create ACLINT SSWI IRQ mapping\n");
> > + return -ENOMEM;
> > + }
> > +
> > + /* Register SSWI irq and handler */
> > + virq = ipi_mux_create(BITS_PER_BYTE, thead_aclint_sswi_ipi_send);
> > + if (virq <= 0) {
> > + pr_err("unable to create muxed IPIs\n");
> > + irq_dispose_mapping(sswi_ipi_virq);
> > + return virq < 0 ? virq : -ENOMEM;
> > + }
> > +
> > + irq_set_chained_handler(sswi_ipi_virq, thead_aclint_sswi_ipi_handle);
> > +
> > + cpuhp_setup_state(CPUHP_AP_IRQ_THEAD_ACLINT_SSWI_STARTING,
> > + "irqchip/thead-aclint-sswi:starting",
> > + aclint_sswi_ipi_starting_cpu, NULL);
>
> The startup callback enables the per CPU interrupt. When a CPU is
> offlined then the per CPU interrupt stays enabled because the teardown
> callback is NULL. I'm not convinced that this is a good idea.
>
Yes, I will add the cleanup handle to clear IPI and disable the IPI
irq for the CPU.
> > +
> > + riscv_ipi_set_virq_range(virq, BITS_PER_BYTE);
> > +
> > + /* Announce that SSWI is providing IPIs */
> > + pr_info("providing IPIs using THEAD ACLINT SSWI\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int __init aclint_sswi_early_probe(struct device_node *node,
> > + struct device_node *parent)
> > +{
> > + return aclint_sswi_probe(&node->fwnode);
> > +}
>
> What's the point of this indirection?
>
This is make room for the future ACPI probe.
> > +
>
> Pointless newline.
>
> > +IRQCHIP_DECLARE(thead_aclint_sswi, "thead,c900-aclint-sswi", aclint_sswi_early_probe);
>
> Thanks,
>
> tglx
Regards,
Inochi
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2044 ACLINT SSWI
2024-10-05 0:46 ` Inochi Amaoto
@ 2024-10-10 16:16 ` Conor Dooley
0 siblings, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2024-10-10 16:16 UTC (permalink / raw)
To: Inochi Amaoto
Cc: Chen Wang, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Peter Zijlstra, Inochi Amaoto, Guo Ren, Lad Prabhakar, Hal Feng,
Heikki Krogerus, Geert Uytterhoeven, Yixun Lan, linux-kernel,
devicetree, linux-riscv
[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]
On Sat, Oct 05, 2024 at 08:46:37AM +0800, Inochi Amaoto wrote:
> On Fri, Oct 04, 2024 at 04:44:22PM +0100, Conor Dooley wrote:
> > On Fri, Oct 04, 2024 at 04:05:55PM +0800, Inochi Amaoto wrote:
> > > Sophgo SG2044 has a new version of T-HEAD C920, which implement
> > > a fully featured ACLINT device. This ACLINT has an extra SSWI
> > > field to support fast S-mode IPI.
> > >
> > > Add necessary compatible string for the T-HEAD ACLINT sswi device.
> > >
> > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > ---
> > > .../thead,c900-aclint-sswi.yaml | 58 +++++++++++++++++++
> > > 1 file changed, 58 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml b/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
> > > new file mode 100644
> > > index 000000000000..0106fbf3ea1f
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/thead,c900-aclint-sswi.yaml
> > > @@ -0,0 +1,58 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/interrupt-controller/thead,c900-aclint-sswi.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Sophgo sg2044 ACLINT Supervisor-level Software Interrupt Device
> > > +
> > > +maintainers:
> > > + - Inochi Amaoto <inochiama@outlook.com>
> > > +
> > > +description:
> > > + The SSWI device is a part of the riscv ACLINT device. It provides
> > > + supervisor-level IPI functionality for a set of HARTs on a RISC-V
> > > + platform. It provides a register to set an IPI (SETSSIP) for each
> > > + HART connected to the SSWI device.
> >
> > If it is part of the aclint, why should it have a separate node, rather
> > than be part of the existing aclint node as a third reg property?
>
> For aclint, the current nodes that have documented are mswi and mtime.
> Since the mtime is a M-mode time source, it is not suitable to add the
> sswi reg into this device. For mswi, it is OK to add a sswi reg, but
> this will cause problem while checking "interrupt-extend". Do we just
> double the maxItem? Or just left it unchanged?
>
> Another reason to add it as a separate node is that the draft says
> sswi can be multiple. If we add this device by adding reg. It will be
> hard if we have multiple sswi devices but one mswi device.
Ah, I see we do indeed have 2 devices already for the aclint, one for
mswi and mtimer.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-10-10 16:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-04 8:05 [PATCH 0/3] riscv: interrupt-controller: Add T-HEAD C900 ACLINT SSWI Inochi Amaoto
2024-10-04 8:05 ` [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2044 " Inochi Amaoto
2024-10-04 15:44 ` Conor Dooley
2024-10-05 0:46 ` Inochi Amaoto
2024-10-10 16:16 ` Conor Dooley
2024-10-04 8:05 ` [PATCH 2/3] irqchip: add T-HEAD C900 ACLINT SSWI driver Inochi Amaoto
2024-10-06 19:50 ` Thomas Gleixner
2024-10-07 0:38 ` Inochi Amaoto
2024-10-04 8:05 ` [PATCH 3/3] riscv: defconfig: Enable T-HEAD C900 ACLINT SSWI drivers Inochi Amaoto
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 NNTP newsgroup(s).