[PATCH 0/2] irqchip: add TC9564 support

[PATCH 0/2] irqchip: add TC9564 support

[Alex Elder]

This series adds support for the "MSIGEN" (MSI generator) interrupt
controller present within the Toshiba TC9564 SoC.  This device
collects interrupts generated within the TC9564 and forwards them
to the host as MSIs.

The TC9564 attaches to a host system via PCIe.  It includes a PCIe
switch with three downstream ports.  An embedded PCIe endpoint is
attached to one of these ports, and it implements two PCI functions.
Each PCI function implements an MSIGEN unit.  Access to the registers
used by an MSIGEN is provided by a PCI endpoint bus, which exposes
BAR4 of the PCI function it is associated with.  

					-Alex

Alex Elder (1):
  dt-bindings: interrupt-controller: introduce
    toshiba,tc9564-msigen.yaml

Daniel Thompson (1):
  irqchip: introduce TC9564 SoC MSIGEN support

 .../toshiba,tc9564-msigen.yaml                |  70 ++++++
 MAINTAINERS                                   |   7 +
 drivers/irqchip/Kconfig                       |  10 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-tc9564-msigen.c           | 203 ++++++++++++++++++
 5 files changed, 291 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
 create mode 100644 drivers/irqchip/irq-tc9564-msigen.c


base-commit: becb95e45ef0dda9ffb35156d909d3da7a2789c3
-- 
2.53.0

[PATCH 1/2] dt-bindings: interrupt-controller: introduce toshiba,tc9564-msigen.yaml

[Alex Elder]

Define the binding for the MSIGEN interrupt controller present in the
Toshiba TC9564 SoC.

Co-developed-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 .../toshiba,tc9564-msigen.yaml                | 70 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml b/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
new file mode 100644
index 0000000000000..39dc63964bd52
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/toshiba,tc9564-msigen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba TC9564 MSIGEN Interrupt Controller
+
+maintainers:
+  - Daniel Thompson <daniel@riscstar.com>
+  - Alex Elder <elder@riscstar.com>
+
+description: |
+  The Toshiba TC9564 is an SoC accessed by a host system through the
+  upstream PCIe port on the PCIe switch it implements.  The switch
+  includes an embedded PCIe endpoint that provides access to various
+  SoC peripherals (including an interrupt controller) via its BARs.
+
+  The TC9564 MSIGEN (MSI Generator) provides an interrupt controller
+  with 25 hardware interrupt lines.  It aggregates internal interrupts
+  and generates a single PCIe MSI to the host.  Each PCIe function
+  implements an MSIGEN instance.
+
+  Each MSIGEN uses a gated clock and reset line.  These are provided
+  by the clock and reset controllers internal to the TC9564.
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    const: toshiba,tc9564-msigen
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 1
+
+  clocks:
+    maxItems: 1
+    description: Gate clock from the TC9564 clock controller
+
+  resets:
+    maxItems: 1
+    description: Reset signal from the TC9564 reset controller
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - '#interrupt-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/toshiba,tc9564.h>
+    #include <dt-bindings/reset/toshiba,tc9564.h>
+
+    interrupt-controller@f000 {
+        compatible = "toshiba,tc9564-msigen";
+        reg = <0xf000 0x100>;
+        interrupt-controller;
+        #interrupt-cells = <1>;
+        clocks = <&tc9564_clock0 CLOCK_MSIGEN>;
+        resets = <&tc9564_reset0 RESET_MSIGEN>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 252b6d7ee1638..9e73a912f0c0f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28173,6 +28173,12 @@ F:	Documentation/devicetree/bindings/media/i2c/toshiba,tc358743.yaml
 F:	drivers/media/i2c/tc358743*
 F:	include/media/i2c/tc358743.h
 
+TOSHIBA TC9564 MSIGEN DRIVER
+M:	Alex Elder <elder@kernel.org>
+M:	Daniel Thompson <danielt@kernel.org>
+S:	Maintained
+F:	Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
+
 TOSHIBA TC9564 PCI DRIVER
 M:	Alex Elder <elder@kernel.org>
 M:	Daniel Thompson <danielt@kernel.org>
-- 
2.53.0

[PATCH 2/2] irqchip: introduce TC9564 SoC MSIGEN support

[Alex Elder]

From: Daniel Thompson <daniel@riscstar.com>

Define a new irqchip platform driver that represents the MSIGEN
functionality present in the TC9564 SoC.

Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Co-developed-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 MAINTAINERS                         |   1 +
 drivers/irqchip/Kconfig             |  10 ++
 drivers/irqchip/Makefile            |   1 +
 drivers/irqchip/irq-tc9564-msigen.c | 203 ++++++++++++++++++++++++++++
 4 files changed, 215 insertions(+)
 create mode 100644 drivers/irqchip/irq-tc9564-msigen.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e73a912f0c0f..5b0ed7c1ab40a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28178,6 +28178,7 @@ M:	Alex Elder <elder@kernel.org>
 M:	Daniel Thompson <danielt@kernel.org>
 S:	Maintained
 F:	Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
+F:	drivers/irqchip/irq-tc9564-msigen.c
 
 TOSHIBA TC9564 PCI DRIVER
 M:	Alex Elder <elder@kernel.org>
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index a8f8c423b7957..205b5a167c805 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -842,4 +842,14 @@ config SUNPLUS_SP7021_INTC
 	  chained controller, routing all interrupt source in P-Chip to
 	  the primary controller on C-Chip.
 
+config TC9564_MSIGEN
+	tristate "Toshiba TC9564 MSI distributor"
+	depends on TC9564_PCI
+	depends on GENERIC_IRQ_CHIP
+	default TC9564_PCI
+	help
+	  This enables support for the "MSIGEN" MSI distributor embedded
+	  in the Toshiba TC9564.  This device receives wired interrupts
+	  within the TC9564 and raises them to the host using MSI.
+
 endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index ab33cccd84719..c82e00802c2b4 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -135,3 +135,4 @@ obj-$(CONFIG_APPLE_AIC)			+= irq-apple-aic.o
 obj-$(CONFIG_MCHP_EIC)			+= irq-mchp-eic.o
 obj-$(CONFIG_SOPHGO_SG2042_MSI)		+= irq-sg2042-msi.o
 obj-$(CONFIG_SUNPLUS_SP7021_INTC)	+= irq-sp7021-intc.o
+obj-$(CONFIG_TC9564_MSIGEN)		+= irq-tc9564-msigen.o
diff --git a/drivers/irqchip/irq-tc9564-msigen.c b/drivers/irqchip/irq-tc9564-msigen.c
new file mode 100644
index 0000000000000..a27edd5859e43
--- /dev/null
+++ b/drivers/irqchip/irq-tc9564-msigen.c
@@ -0,0 +1,203 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/clk.h>
+#include <linux/irqdomain.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+/**
+ * struct tc9564_msigen - Context for MSIGEN IRQ domain
+ * @ioaddr:	Pointer to mapped MSIGEN memory
+ * @irq:	IRQ number for MSIGEN
+ */
+struct tc9564_msigen {
+	void __iomem *ioaddr;
+	unsigned int irq;
+};
+
+#define HWIRQ_COUNT			25
+
+#define MSI_OUT_EN_OFFSET		0x0000
+#define MSI_MASK_CLR_OFFSET		0x000c
+#define MSI_MASK_VALUE			BIT(0)
+#define MSI_INT_STS_OFFSET		0x0010
+
+static void tc9564_msigen_irq_handler(struct irq_desc *desc)
+{
+	struct irq_domain *irq_domain = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct irq_chip_generic *gc;
+	unsigned long status;
+	unsigned long hwirq;
+
+	gc = irq_get_domain_generic_chip(irq_domain, 0);
+
+	chained_irq_enter(chip, desc);
+
+	status = irq_reg_readl(gc, MSI_INT_STS_OFFSET);
+	for_each_set_bit(hwirq, &status, HWIRQ_COUNT)
+		generic_handle_domain_irq(irq_domain, hwirq);
+
+	/*
+	 * Clear the MSI flag. Most interrupts within TC9564 are level-high
+	 * type. If any interrupts are still asserted then clearing this flag
+	 * will cause the (edge-triggered) MSI to be regenerated.
+	 */
+	irq_reg_writel(gc, MSI_MASK_VALUE, MSI_MASK_CLR_OFFSET);
+
+	chained_irq_exit(chip, desc);
+}
+
+static int tc9564_msigen_irq_chip_init(struct irq_chip_generic *gc)
+{
+	struct tc9564_msigen *msigen = gc->domain->host_data;
+
+	gc->reg_base = msigen->ioaddr;
+	gc->chip_types[0].regs.mask = MSI_OUT_EN_OFFSET;
+	gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
+	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit;
+
+	/* Disable all interrupts */
+	irq_reg_writel(gc, 0, MSI_OUT_EN_OFFSET);
+
+	return 0;
+}
+
+static void tc9564_msigen_irq_chip_exit(struct irq_chip_generic *gc)
+{
+	irq_reg_writel(gc, 0, MSI_OUT_EN_OFFSET);
+}
+
+static int tc9564_msigen_irq_domain_init(struct irq_domain *irq_domain)
+{
+	struct tc9564_msigen *msigen = irq_domain->host_data;
+
+	irq_set_chained_handler_and_data(msigen->irq, tc9564_msigen_irq_handler,
+					 irq_domain);
+	return 0;
+}
+
+static void tc9564_msigen_irq_domain_exit(struct irq_domain *irq_domain)
+{
+	struct tc9564_msigen *msigen = irq_domain->host_data;
+
+	irq_set_chained_handler_and_data(msigen->irq, NULL, NULL);
+}
+
+/* Find the PCI device we're associated with (not necessarily the parent) */
+static struct pci_dev *tc9564_msigen_find_pci_dev(struct device *dev)
+{
+	struct device *ancestor;
+
+	for (ancestor = dev->parent; ancestor; ancestor = ancestor->parent) {
+		if (dev_is_pci(ancestor)) {
+			/* We currently only support physical functions */
+			if (!dev_is_pf(ancestor))
+				return ERR_PTR(-EINVAL);
+
+			return to_pci_dev(ancestor);
+		}
+	}
+
+	return ERR_PTR(-ENXIO);
+}
+
+static void tc9564_msigen_free_irq_vectors(void *pci_dev)
+{
+	pci_free_irq_vectors(pci_dev);
+}
+
+static int tc9564_msigen_probe(struct platform_device *pdev)
+{
+	struct irq_domain_chip_generic_info dgc_info;
+	struct device *dev = &pdev->dev;
+	struct irq_domain *irq_domain;
+	struct tc9564_msigen *msigen;
+	struct irq_domain_info info;
+	struct reset_control *reset;
+	struct pci_dev *pci_dev;
+	struct clk *clk;
+	int ret;
+
+	pci_dev = tc9564_msigen_find_pci_dev(dev);
+	if (IS_ERR(pci_dev))
+		return dev_err_probe(dev, PTR_ERR(pci_dev),
+				     "failed to find PCI device\n");
+
+	msigen = devm_kzalloc(dev, sizeof(*msigen), GFP_KERNEL);
+	if (!msigen)
+		return -ENOMEM;
+
+	msigen->ioaddr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(msigen->ioaddr))
+		return dev_err_probe(dev, PTR_ERR(msigen->ioaddr),
+				     "failed to map registers\n");
+
+	clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk),
+				     "failed to get/enable clock\n");
+
+	reset = devm_reset_control_get_shared_deasserted(dev, NULL);
+	if (IS_ERR(reset))
+		return dev_err_probe(dev, PTR_ERR(reset),
+				     "failed to get/deassert reset\n");
+
+	ret = pci_alloc_irq_vectors(pci_dev, 1, 1, PCI_IRQ_MSI);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to alloc irq vector\n");
+
+	ret = devm_add_action_or_reset(dev, tc9564_msigen_free_irq_vectors,
+				       pci_dev);
+	if (ret)
+		return ret;
+
+	msigen->irq = pci_irq_vector(pci_dev, 0);
+
+	dgc_info.name = "tc9564-msigen";
+	dgc_info.handler = handle_level_irq;
+	dgc_info.irqs_per_chip = HWIRQ_COUNT;
+	dgc_info.num_ct = 1;
+	dgc_info.init = tc9564_msigen_irq_chip_init;
+	dgc_info.exit = tc9564_msigen_irq_chip_exit;
+
+	info.fwnode = of_fwnode_handle(dev->of_node);
+	info.domain_flags = IRQ_DOMAIN_FLAG_DESTROY_GC;
+	info.size = HWIRQ_COUNT;
+	info.hwirq_max = HWIRQ_COUNT;
+	info.ops = &irq_generic_chip_ops;
+	info.host_data = msigen;
+	info.dgc_info = &dgc_info;
+	info.init = tc9564_msigen_irq_domain_init;
+	info.exit = tc9564_msigen_irq_domain_exit;
+
+	irq_domain = devm_irq_domain_instantiate(dev, &info);
+	if (IS_ERR(irq_domain))
+		return dev_err_probe(dev, PTR_ERR(irq_domain),
+				     "failed to instantiate IRQ domain\n");
+
+	return 0;
+}
+
+static const struct of_device_id tc9564_msigen_of_match[] = {
+	{ .compatible = "toshiba,tc9564-msigen" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tc9564_msigen_of_match);
+
+static struct platform_driver tc9564_msigen_driver = {
+	.probe = tc9564_msigen_probe,
+	.driver = {
+		.name		= KBUILD_MODNAME,
+		.of_match_table	= tc9564_msigen_of_match,
+	},
+};
+module_platform_driver(tc9564_msigen_driver);
+
+MODULE_AUTHOR("Daniel Thompson <danielt@kernel.org>");
+MODULE_DESCRIPTION("TC9564 MSIGEN IRQ Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.53.0

Re: [PATCH 1/2] dt-bindings: interrupt-controller: introduce toshiba,tc9564-msigen.yaml

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: missing mandatory clock and reset properties in required list
- [Low] dt-bindings: non-existent header includes cause dt_binding_check failure
- [Low] dt-bindings: maintainer email addresses mismatch between MAINTAINERS and schema
--

commit 8cfa6ac36f149ea1d1b37b717117b76943010968
Author: Alex Elder <elder@riscstar.com>

dt-bindings: interrupt-controller: introduce toshiba,tc9564-msigen.yaml

Define the binding for the MSIGEN interrupt controller present in the
Toshiba TC9564 SoC.

> diff --git a/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml b/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
[ ... ]
> +title: Toshiba TC9564 MSIGEN Interrupt Controller
> +
> +maintainers:
> +  - Daniel Thompson <daniel@riscstar.com>
> +  - Alex Elder <elder@riscstar.com>

[Severity: Low]
This isn't a bug, but is this email domain mismatch intentional? The
email addresses listed here in the schema use @riscstar.com, but the
MAINTAINERS file entry added in this same patch uses @kernel.org.

This might cause automated tooling to route patch series to incorrect or
duplicate addresses.

[ ... ]
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - '#interrupt-cells'

[Severity: Medium]
Should clocks and resets be included in this required list? The binding
documentation specifies that each MSIGEN uses a gated clock and reset line.

If they are omitted here, device trees with incomplete hardware configurations
will silently pass schema validation, potentially leading to driver failures
when attempting to ungate clocks or deassert resets on uninitialized hardware.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/toshiba,tc9564.h>
> +    #include <dt-bindings/reset/toshiba,tc9564.h>

[Severity: Low]
Does this cause make dt_binding_check to fail? It looks like these header
files are not included in this patch series.

Including headers in the YAML example that do not exist in the source tree
will usually cause the C preprocessor to fail during schema validation
compilation.

[ ... ]
> diff --git a/MAINTAINERS b/MAINTAINERS
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
[ ... ]
>  F:	drivers/media/i2c/tc358743*
>  F:	include/media/i2c/tc358743.h
>  
> +TOSHIBA TC9564 MSIGEN DRIVER
> +M:	Alex Elder <elder@kernel.org>
> +M:	Daniel Thompson <danielt@kernel.org>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012190245.3988619-1-elder@riscstar.com?part=1

Re: [PATCH 2/2] irqchip: introduce TC9564 SoC MSIGEN support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] irqchip/tc9564: uninitialized stack variables info and dgc_info
- [High] irqchip/tc9564-msigen: Use-After-Free in handler during module removal
- [Low] kconfig: TC9564_MSIGEN depends on hidden symbol GENERIC_IRQ_CHIP
--

commit 3dfc2bc2208f270b240c50f9b867c34c54af58e0
Author: Daniel Thompson <daniel@riscstar.com>

irqchip: introduce TC9564 SoC MSIGEN support
    
Define a new irqchip platform driver that represents the MSIGEN
functionality present in the TC9564 SoC.

> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index a8f8c423b7957..205b5a167c805 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
[ ... ]
> +config TC9564_MSIGEN
> +	tristate "Toshiba TC9564 MSI distributor"
> +	depends on TC9564_PCI
> +	depends on GENERIC_IRQ_CHIP

[Severity: Low]
Is it intentional to use "depends on" instead of "select" for this hidden
symbol?

Since GENERIC_IRQ_CHIP is a hidden boolean symbol (without a prompt), using
"depends on" makes TC9564_MSIGEN impossible to enable on architectures or
platforms that do not already natively select GENERIC_IRQ_CHIP.

> +	default TC9564_PCI
> +	help
> +	  This enables support for the "MSIGEN" MSI distributor embedded
> +	  in the Toshiba TC9564.  This device receives wired interrupts
> +	  within the TC9564 and raises them to the host using MSI.
> +
>  endmenu
> diff --git a/drivers/irqchip/irq-tc9564-msigen.c b/drivers/irqchip/irq-tc9564-msigen.c
> new file mode 100644
> index 0000000000000..a27edd5859e43
> --- /dev/null
> +++ b/drivers/irqchip/irq-tc9564-msigen.c
[ ... ]
> +static void tc9564_msigen_irq_domain_exit(struct irq_domain *irq_domain)
> +{
> +	struct tc9564_msigen *msigen = irq_domain->host_data;
> +
> +	irq_set_chained_handler_and_data(msigen->irq, NULL, NULL);

[Severity: High]
Does this missing synchronization lead to a use-after-free?

When the chained handler is set to NULL during module removal, it doesn't wait
for currently executing handlers to finish. If tc9564_msigen_irq_handler() is
running concurrently on another CPU, it might access the irq_domain, generic
chip, and unmapped register memory after they are freed by the removal path.

Should this call synchronize_irq() to ensure the concurrent handler has
finished?

> +}
[ ... ]
> +static int tc9564_msigen_probe(struct platform_device *pdev)
> +{
> +	struct irq_domain_chip_generic_info dgc_info;
> +	struct device *dev = &pdev->dev;
> +	struct irq_domain *irq_domain;
> +	struct tc9564_msigen *msigen;
> +	struct irq_domain_info info;

[Severity: High]
Could these uninitialized stack variables cause a crash?

If info is not zero-initialized (e.g., using = {}), fields like info.parent
will contain stack garbage. When passed to devm_irq_domain_instantiate(),
could this result in dereferencing a wild pointer in __irq_domain_instantiate()
if CONFIG_IRQ_DOMAIN_HIERARCHY is enabled?

> +	struct reset_control *reset;
> +	struct pci_dev *pci_dev;
> +	struct clk *clk;
> +	int ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012190245.3988619-1-elder@riscstar.com?part=2

Re: [PATCH 1/2] dt-bindings: interrupt-controller: introduce toshiba,tc9564-msigen.yaml

[Alex Elder]

On 10/12/26 2:07 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] dt-bindings: missing mandatory clock and reset properties in required list
> - [Low] dt-bindings: non-existent header includes cause dt_binding_check failure
> - [Low] dt-bindings: maintainer email addresses mismatch between MAINTAINERS and schema
> --
> 
> commit 8cfa6ac36f149ea1d1b37b717117b76943010968
> Author: Alex Elder <elder@riscstar.com>
> 
> dt-bindings: interrupt-controller: introduce toshiba,tc9564-msigen.yaml
> 
> Define the binding for the MSIGEN interrupt controller present in the
> Toshiba TC9564 SoC.
> 
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml b/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
>> new file mode 100644
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
> [ ... ]
>> +title: Toshiba TC9564 MSIGEN Interrupt Controller
>> +
>> +maintainers:
>> +  - Daniel Thompson <daniel@riscstar.com>
>> +  - Alex Elder <elder@riscstar.com>
> 
> [Severity: Low]
> This isn't a bug, but is this email domain mismatch intentional? The
> email addresses listed here in the schema use @riscstar.com, but the
> MAINTAINERS file entry added in this same patch uses @kernel.org.
> 
> This might cause automated tooling to route patch series to incorrect or
> duplicate addresses.

Daniel and I are doing this work on behalf of RISCstar, but we
use our @kernel.org addresses in the MAINTAINERS file.

danielt@kernel.org is the same person as daniel@riscstar.com
elder@kernel.org is the same person as elder@riscstar.com

(We should both send updates to the .mailmap file...)
> 
> [ ... ]
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupt-controller
>> +  - '#interrupt-cells'
> 
> [Severity: Medium]
> Should clocks and resets be included in this required list? The binding
> documentation specifies that each MSIGEN uses a gated clock and reset line.

In the code, the calls to the clock and reset are not optional, so
yes, these should be required.  I will update this in version 2.

> If they are omitted here, device trees with incomplete hardware configurations
> will silently pass schema validation, potentially leading to driver failures
> when attempting to ungate clocks or deassert resets on uninitialized hardware.
> 
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/toshiba,tc9564.h>
>> +    #include <dt-bindings/reset/toshiba,tc9564.h>
> 
> [Severity: Low]
> Does this cause make dt_binding_check to fail? It looks like these header
> files are not included in this patch series.

I just checked, and yes, in fact this does cause a failure.

I did check this, but it was in my test environment that includes
a bunch of other drivers, etc.  These two headers come from two other
series that are currently out for review, and I neglected to mention
this.

I'll try find a better way to handle this for the next version.

					-Alex

> luding headers in the YAML example that do not exist in the source tree
> will usually cause the C preprocessor to fail during schema validation
> compilation.
> 
> [ ... ]
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
> [ ... ]
>>   F:	drivers/media/i2c/tc358743*
>>   F:	include/media/i2c/tc358743.h
>>   
>> +TOSHIBA TC9564 MSIGEN DRIVER
>> +M:	Alex Elder <elder@kernel.org>
>> +M:	Daniel Thompson <danielt@kernel.org>
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
>> +
> 

Re: [PATCH 2/2] irqchip: introduce TC9564 SoC MSIGEN support

[Alex Elder]

On 10/12/26 2:16 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] irqchip/tc9564: uninitialized stack variables info and dgc_info
> - [High] irqchip/tc9564-msigen: Use-After-Free in handler during module removal
> - [Low] kconfig: TC9564_MSIGEN depends on hidden symbol GENERIC_IRQ_CHIP

I think all three of these are legitimate issues, and I describe
below how they'll be fixed in the next version of the series.

> --
> 
> commit 3dfc2bc2208f270b240c50f9b867c34c54af58e0
> Author: Daniel Thompson <daniel@riscstar.com>
> 
> irqchip: introduce TC9564 SoC MSIGEN support
>      
> Define a new irqchip platform driver that represents the MSIGEN
> functionality present in the TC9564 SoC.
> 
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index a8f8c423b7957..205b5a167c805 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
> [ ... ]
>> +config TC9564_MSIGEN
>> +	tristate "Toshiba TC9564 MSI distributor"
>> +	depends on TC9564_PCI
>> +	depends on GENERIC_IRQ_CHIP
> 
> [Severity: Low]
> Is it intentional to use "depends on" instead of "select" for this hidden
> symbol?
> 
> Since GENERIC_IRQ_CHIP is a hidden boolean symbol (without a prompt), using
> "depends on" makes TC9564_MSIGEN impossible to enable on architectures or
> platforms that do not already natively select GENERIC_IRQ_CHIP.

When I used "select" for GENERIC_IRQ_CHIP, there is a recursive
Kconfig dependency.

error: recursive dependency detected!
         symbol GENERIC_IRQ_CHIP is selected by TC9564_MSIGEN
         symbol TC9564_MSIGEN depends on TC9564_PCI
         symbol TC9564_PCI depends on PCI_DYNAMIC_OF_NODES
         symbol PCI_DYNAMIC_OF_NODES depends on OF_IRQ
         symbol OF_IRQ depends on IRQ_DOMAIN
         symbol IRQ_DOMAIN is selected by GENERIC_IRQ_CHIP

If I instead simply omit "depends on GENERIC_IRQ_CHIP" this
circular dependency goes away.  I can rely on TC9564_PCI
to (eventually) cause GENERIC_IRQ_CHIP to get selected
instead, so that's what I'll do in the next version.

>> +	default TC9564_PCI
>> +	help
>> +	  This enables support for the "MSIGEN" MSI distributor embedded
>> +	  in the Toshiba TC9564.  This device receives wired interrupts
>> +	  within the TC9564 and raises them to the host using MSI.
>> +
>>   endmenu
>> diff --git a/drivers/irqchip/irq-tc9564-msigen.c b/drivers/irqchip/irq-tc9564-msigen.c
>> new file mode 100644
>> index 0000000000000..a27edd5859e43
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-tc9564-msigen.c
> [ ... ]
>> +static void tc9564_msigen_irq_domain_exit(struct irq_domain *irq_domain)
>> +{
>> +	struct tc9564_msigen *msigen = irq_domain->host_data;
>> +
>> +	irq_set_chained_handler_and_data(msigen->irq, NULL, NULL);
> 
> [Severity: High]
> Does this missing synchronization lead to a use-after-free?
> 
> When the chained handler is set to NULL during module removal, it doesn't wait
> for currently executing handlers to finish. If tc9564_msigen_irq_handler() is
> running concurrently on another CPU, it might access the irq_domain, generic
> chip, and unmapped register memory after they are freed by the removal path.
> 
> Should this call synchronize_irq() to ensure the concurrent handler has
> finished?

Sounds like it should.  I will investigate further, but presumably
this suggestion will be included in the next version.

> 
>> +}
> [ ... ]
>> +static int tc9564_msigen_probe(struct platform_device *pdev)
>> +{
>> +	struct irq_domain_chip_generic_info dgc_info;
>> +	struct device *dev = &pdev->dev;
>> +	struct irq_domain *irq_domain;
>> +	struct tc9564_msigen *msigen;
>> +	struct irq_domain_info info;
> 
> [Severity: High]
> Could these uninitialized stack variables cause a crash?

I missed this.  Yes, the info structure is (further) initialized
in the code below, so it should be initially zeroed.  This will
be fixed in the next version.
	
					-Alex

> If info is not zero-initialized (e.g., using = {}), fields like info.parent
> will contain stack garbage. When passed to devm_irq_domain_instantiate(),
> could this result in dereferencing a wild pointer in __irq_domain_instantiate()
> if CONFIG_IRQ_DOMAIN_HIERARCHY is enabled?
> 
>> +	struct reset_control *reset;
>> +	struct pci_dev *pci_dev;
>> +	struct clk *clk;
>> +	int ret;
> 

Re: [PATCH 1/2] dt-bindings: interrupt-controller: introduce toshiba,tc9564-msigen.yaml

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4302 bytes --]

On Mon, Oct 12, 2026 at 02:02:43PM -0500, Alex Elder wrote:
> Define the binding for the MSIGEN interrupt controller present in the
> Toshiba TC9564 SoC.
> 
> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
>  .../toshiba,tc9564-msigen.yaml                | 70 +++++++++++++++++++
>  MAINTAINERS                                   |  6 ++
>  2 files changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml b/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
> new file mode 100644
> index 0000000000000..39dc63964bd52
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/toshiba,tc9564-msigen.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Toshiba TC9564 MSIGEN Interrupt Controller
> +
> +maintainers:
> +  - Daniel Thompson <daniel@riscstar.com>
> +  - Alex Elder <elder@riscstar.com>
> +
> +description: |
> +  The Toshiba TC9564 is an SoC accessed by a host system through the
> +  upstream PCIe port on the PCIe switch it implements.  The switch
> +  includes an embedded PCIe endpoint that provides access to various
> +  SoC peripherals (including an interrupt controller) via its BARs.
> +
> +  The TC9564 MSIGEN (MSI Generator) provides an interrupt controller
> +  with 25 hardware interrupt lines.  It aggregates internal interrupts
> +  and generates a single PCIe MSI to the host.  Each PCIe function
> +  implements an MSIGEN instance.
> +
> +  Each MSIGEN uses a gated clock and reset line.  These are provided
> +  by the clock and reset controllers internal to the TC9564.
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: toshiba,tc9564-msigen
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description: Gate clock from the TC9564 clock controller

I'm surprised that the clock is not mandatory.. Realistically the reset
is also mandatory, there's probably one instance of this on the device,
where both clock and reset are connected. I'd add both to the required
list.

Otherwise this looks
fine to me. I know you mentioned this device in the other series that
we were discussing but I don't see how it is related and is perfectly
fine to implement as a standalone device as you have done here.

Thanks,
Conor.

> +
> +  resets:
> +    maxItems: 1
> +    description: Reset signal from the TC9564 reset controller
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - '#interrupt-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/toshiba,tc9564.h>
> +    #include <dt-bindings/reset/toshiba,tc9564.h>
> +
> +    interrupt-controller@f000 {
> +        compatible = "toshiba,tc9564-msigen";
> +        reg = <0xf000 0x100>;
> +        interrupt-controller;
> +        #interrupt-cells = <1>;
> +        clocks = <&tc9564_clock0 CLOCK_MSIGEN>;
> +        resets = <&tc9564_reset0 RESET_MSIGEN>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 252b6d7ee1638..9e73a912f0c0f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -28173,6 +28173,12 @@ F:	Documentation/devicetree/bindings/media/i2c/toshiba,tc358743.yaml
>  F:	drivers/media/i2c/tc358743*
>  F:	include/media/i2c/tc358743.h
>  
> +TOSHIBA TC9564 MSIGEN DRIVER
> +M:	Alex Elder <elder@kernel.org>
> +M:	Daniel Thompson <danielt@kernel.org>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/interrupt-controller/toshiba,tc9564-msigen.yaml
> +
>  TOSHIBA TC9564 PCI DRIVER
>  M:	Alex Elder <elder@kernel.org>
>  M:	Daniel Thompson <danielt@kernel.org>
> -- 
> 2.53.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]