From: Ma Jun <[email protected]>
This patch set adds the driver of mbigen and binding document for Hisilicon
Mbigen chips.
Compared with previous version, this version changed much.
Because during the time between V3 and V4 of my patch, there are two
related patches were committed by Mr.Marc Zyngier and Mr. Mark Rutland.
First, Mr. Marc Zyngier changed MSI frame and added supporting for
platform MSI.
https://lkml.org/lkml/2015/7/28/552
Second, Mr.Mark Rutland changed Generic PCI MSI + IOMMU topology bindings
https://lkml.org/lkml/2015/7/23/558
After V5 patch posted, Mr.Marc Zyngier posted a new patch set
"Adding core support for wire-MSI bridges"
https://lkml.org/lkml/2015/10/15/545
So, mbigen v6 patch is based on this new patch even though this patch is
still under review.
Changes in v8:
--- Fixed some tiny bugs.
Changes in v7:
--- Fixed the build test error when applied patch v6 3/4
Changes in v6:
--- Re-based mbigen driver on kernel 4.3.0-rc5 and Marc's new patch
--- Change the mbigen chip node definition(dts).
--- Change the interrupt cells definition(dts).
Changes in v5:
--- Split mbigen driver patch into 2 smaller patches.
--- Change mbigen chip and mbigen device initialzing sequence.
--- Initializing mbigen device instead of mbigen chip as interrupt controller
--- Remove mbigen node from driver to make this driver more easily read.
--- Change the mbigen chip node definition(dts).
--- Change the interrupt cells definition(dts).
Changes in v4:
--- Re-based mbigen driver on kernel 4.2.0-rc2 and Marc's patch
--- Changed the binding document based on Mark's patch.
Ma Jun (4):
dt-binding:Documents of the mbigen bindings
irqchip: add platform device driver for mbigen device
irqchip:create irq domain for each mbigen device
irqchip:implement the mbigen irq chip operation functions
Documentation/devicetree/bindings/arm/mbigen.txt | 63 ++++
drivers/irqchip/Kconfig | 8 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-mbigen.c | 328 ++++++++++++++++++++++
4 files changed, 400 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/mbigen.txt
create mode 100644 drivers/irqchip/irq-mbigen.c
From: Ma Jun <[email protected]>
Add the mbigen msi interrupt controller bindings document.
This patch based on Mark Rutland's patch
https://lkml.org/lkml/2015/7/23/558
Signed-off-by: Ma Jun <[email protected]>
---
Documentation/devicetree/bindings/arm/mbigen.txt | 63 ++++++++++++++++++++++
1 files changed, 63 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/mbigen.txt
diff --git a/Documentation/devicetree/bindings/arm/mbigen.txt b/Documentation/devicetree/bindings/arm/mbigen.txt
new file mode 100644
index 0000000..eb9a7fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mbigen.txt
@@ -0,0 +1,63 @@
+Hisilicon mbigen device tree bindings.
+=======================================
+
+Mbigen means: message based interrupt generator.
+
+MBI is kind of msi interrupt only used on Non-PCI devices.
+
+To reduce the wired interrupt number connected to GIC,
+Hisilicon designed mbigen to collect and generate interrupt.
+
+
+Non-pci devices can connect to mbigen and generate the
+interrupt by writing ITS register.
+
+The mbigen chip and devices connect to mbigen have the following properties:
+
+Mbigen main node required properties:
+-------------------------------------------
+- compatible: Should be "hisilicon,mbigen-v2"
+- reg: Specifies the base physical address and size of the Mbigen
+ registers.
+- interrupt controller: Identifies the node as an interrupt controller
+- msi-parent: This property has two cells.
+ The 1st cell specifies the ITS this device connected.
+ The 2nd cell specifies the device id.
+- nr-msis:Specifies the total number of interrupt this device has.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The value is 2 now.
+
+ The 1st cell is global hardware pin number of the interrupt.
+ This value depends on the Soc design.
+ The 2nd cell is the interrupt trigger type.
+
+Examples:
+
+ mbigen_device_gmac:intc {
+ compatible = "hisilicon,mbigen-v2";
+ reg = <0x0 0xc0080000 0x0 0x10000>;
+ interrupt-controller;
+ msi-parent = <&its_dsa 0x40b1c>;
+ num-msis = <9>;
+ #interrupt-cells = <2>;
+ };
+
+Devices connect to mbigen required properties:
+----------------------------------------------------
+-interrupt-parent: Specifies the mbigen device node which device connected.
+-interrupts:specifies the interrupt source.
+ The 1st cell is global hardware pin number of the interrupt.
+ This value depends on the Soc design.
+ The 2nd cell is the interrupt trigger type
+
+Examples:
+ gmac0: ethernet@c2080000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0 0xc2080000 0 0x20000>,
+ <0 0xc0000000 0 0x1000>;
+ interrupt-parent = <&mbigen_device_gmac>;
+ interrupts = <656 1>,
+ <657 1>;
+ };
+
--
1.7.1
From: Ma Jun <[email protected]>
Mbigen means Message Based Interrupt Generator(MBIGEN).
Its a kind of interrupt controller that collects
the interrupts from external devices and generate msi interrupt.
Mbigen is applied to reduce the number of wire connected interrupts.
As the peripherals increasing, the interrupts lines needed is
increasing much, especially on the Arm64 server SOC.
Therefore, the interrupt pin in GIC is not enough to cover so
many peripherals.
Mbigen is designed to fix this problem.
Mbigen chip locates in ITS or outside of ITS.
Mbigen chip hardware structure shows as below:
mbigen chip
|---------------------|-------------------|
mgn_node0 mgn_node1 mgn_node2
| |-------| |-------|------|
dev1 dev1 dev2 dev1 dev3 dev4
Each mbigen chip contains several mbigen nodes.
External devices can connect to mbigen node through wire connecting way.
Because a mbigen node only can support 128 interrupt maximum, depends
on the interrupt lines number of devices, a device can connects to one
more mbigen nodes.
Also, several different devices can connect to a same mbigen node.
When devices triggered interrupt,mbigen chip detects and collects
the interrupts and generates the MBI interrupts by writing the ITS
Translator register.
To simplify mbigen driver,I used a new conception--mbigen device.
Each mbigen device is initialized as a platform device.
Mbigen device presents the parts(register, pin definition etc.) in
mbigen chip corresponding to a peripheral device.
So from software view, the structure likes below
mbigen chip
|---------------------|-----------------|
mbigen device1 mbigen device2 mbigen device3
| | |
dev1 dev2 dev3
Signed-off-by: Ma Jun <[email protected]>
---
drivers/irqchip/Kconfig | 8 ++++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-mbigen.c | 73 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 0 deletions(-)
create mode 100644 drivers/irqchip/irq-mbigen.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 4d7294e..b205e15 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -27,6 +27,14 @@ config ARM_GIC_V3_ITS
bool
select PCI_MSI_IRQ_DOMAIN
+config HISILICON_IRQ_MBIGEN
+ bool "Support mbigen interrupt controller"
+ default n
+ depends on ARM_GIC_V3 && ARM_GIC_V3_ITS && GENERIC_MSI_IRQ_DOMAIN
+ help
+ Enable the mbigen interrupt controller used on
+ Hisilicon platform.
+
config ARM_NVIC
bool
select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 177f78f..cd76b11 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_ARM_GIC) += irq-gic.o irq-gic-common.o
obj-$(CONFIG_ARM_GIC_V2M) += irq-gic-v2m.o
obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o irq-gic-common.o
obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-gic-v3-its-platform-msi.o
+obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
obj-$(CONFIG_ARM_VIC) += irq-vic.o
obj-$(CONFIG_ATMEL_AIC_IRQ) += irq-atmel-aic-common.o irq-atmel-aic.o
diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
new file mode 100644
index 0000000..25e4000
--- /dev/null
+++ b/drivers/irqchip/irq-mbigen.c
@@ -0,0 +1,73 @@
+/*
+ * Copyright (C) 2015 Hisilicon Limited, All Rights Reserved.
+ * Author: Jun Ma <[email protected]>
+ * Author: Yun Wu <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/**
+ * struct mbigen_device - holds the information of mbigen device.
+ *
+ * @pdev: pointer to the platform device structure of mbigen chip.
+ * @base: mapped address of this mbigen chip.
+ */
+struct mbigen_device {
+ struct platform_device *pdev;
+ void __iomem *base;
+};
+
+static int mbigen_device_probe(struct platform_device *pdev)
+{
+ struct mbigen_device *mgn_chip;
+
+ mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
+ if (!mgn_chip)
+ return -ENOMEM;
+
+ mgn_chip->pdev = pdev;
+ mgn_chip->base = of_iomap(pdev->dev.of_node, 0);
+
+ platform_set_drvdata(pdev, mgn_chip);
+
+ return 0;
+}
+
+static const struct of_device_id mbigen_of_match[] = {
+ { .compatible = "hisilicon,mbigen-v2" },
+ { /* END */ }
+};
+MODULE_DEVICE_TABLE(of, mbigen_of_match);
+
+static struct platform_driver mbigen_platform_driver = {
+ .driver = {
+ .name = "Hisilicon MBIGEN-V2",
+ .owner = THIS_MODULE,
+ .of_match_table = mbigen_of_match,
+ },
+ .probe = mbigen_device_probe,
+};
+
+module_platform_driver(mbigen_platform_driver);
+
+MODULE_AUTHOR("Jun Ma <[email protected]>");
+MODULE_AUTHOR("Yun Wu <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Hisilicon MBI Generator driver");
--
1.7.1
From: Ma Jun <[email protected]>
For peripheral devices which connect to mbigen,mbigen is a interrupt
controller. So, we create irq domain for each mbigen device and add
mbigen irq domain into irq hierarchy structure.
Signed-off-by: Ma Jun <[email protected]>
---
drivers/irqchip/irq-mbigen.c | 163 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 163 insertions(+), 0 deletions(-)
diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 25e4000..71291cb 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -16,27 +16,176 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <linux/interrupt.h>
+#include <linux/irqchip.h>
#include <linux/module.h>
+#include <linux/msi.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+/* Interrupt numbers per mbigen node supported */
+#define IRQS_PER_MBIGEN_NODE 128
+
+/* 16 irqs (Pin0-pin15) are reserved for each mbigen chip */
+#define RESERVED_IRQ_PER_MBIGEN_CHIP 16
+
+/**
+ * In mbigen vector register
+ * bit[21:12]: event id value
+ * bit[11:0]: device id
+ */
+#define IRQ_EVENT_ID_SHIFT 12
+#define IRQ_EVENT_ID_MASK 0x3ff
+
+/* register range of each mbigen node */
+#define MBIGEN_NODE_OFFSET 0x1000
+
+/* offset of vector register in mbigen node */
+#define REG_MBIGEN_VEC_OFFSET 0x200
+
+
/**
* struct mbigen_device - holds the information of mbigen device.
*
* @pdev: pointer to the platform device structure of mbigen chip.
* @base: mapped address of this mbigen chip.
+ * @domain: pointer to the irq domain
*/
struct mbigen_device {
struct platform_device *pdev;
void __iomem *base;
+ struct irq_domain *domain;
+};
+
+/**
+ * struct mbigen_irq_data - private data of each irq
+ *
+ * @base: mapped address of mbigen chip
+ * @reg_vec: addr offset of interrupt vector register.
+ */
+struct mbigen_irq_data {
+ void __iomem *base;
+ unsigned int reg_vec;
+};
+
+static inline int get_mbigen_vec_reg(u32 nid, u32 offset)
+{
+ return (offset * 4) + nid * MBIGEN_NODE_OFFSET
+ + REG_MBIGEN_VEC_OFFSET;
+}
+
+static struct irq_chip mbigen_irq_chip = {
+ .name = "mbigen-v2",
+};
+
+static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
+{
+ struct mbigen_irq_data *mgn_irq_data = irq_get_chip_data(desc->irq);
+ u32 val;
+
+ val = readl_relaxed(mgn_irq_data->reg_vec + mgn_irq_data->base);
+
+ val &= ~(IRQ_EVENT_ID_MASK << IRQ_EVENT_ID_SHIFT);
+ val |= (msg->data << IRQ_EVENT_ID_SHIFT);
+
+ writel_relaxed(val, mgn_irq_data->reg_vec + mgn_irq_data->base);
+}
+
+static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq)
+{
+ struct mbigen_irq_data *datap;
+ unsigned int nid, pin_offset;
+
+ datap = kzalloc(sizeof(*datap), GFP_KERNEL);
+ if (!datap)
+ return NULL;
+
+ /* get the mbigen node number */
+ nid = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP) / IRQS_PER_MBIGEN_NODE + 1;
+
+ pin_offset = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP)
+ % IRQS_PER_MBIGEN_NODE;
+
+ datap->reg_vec = get_mbigen_vec_reg(nid, pin_offset);
+ return datap;
+}
+
+static int mbigen_domain_translate(struct irq_domain *d,
+ struct irq_fwspec *fwspec,
+ unsigned long *hwirq,
+ unsigned int *type)
+{
+ if (is_of_node(fwspec->fwnode)) {
+ if (fwspec->param_count != 2)
+ return -EINVAL;
+
+ *hwirq = fwspec->param[0];
+ *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
+
+ return 0;
+ }
+ return -EINVAL;
+}
+
+static int mbigen_irq_domain_alloc(struct irq_domain *domain,
+ unsigned int virq,
+ unsigned int nr_irqs,
+ void *args)
+{
+ struct irq_fwspec *fwspec = args;
+ irq_hw_number_t hwirq;
+ unsigned int type;
+ struct mbigen_device *mgn_chip;
+ struct mbigen_irq_data *mgn_irq_data;
+ int i, err;
+
+ err = mbigen_domain_translate(domain, fwspec, &hwirq, &type);
+ if (err)
+ return err;
+
+ err = platform_msi_domain_alloc(domain, virq, nr_irqs);
+ if (err)
+ return err;
+
+ /* set related information of this irq */
+ mgn_irq_data = set_mbigen_irq_data(hwirq);
+ if (!mgn_irq_data)
+ return err;
+
+ mgn_chip = platform_msi_get_host_data(domain);
+ mgn_irq_data->base = mgn_chip->base;
+
+ for (i = 0; i < nr_irqs; i++)
+ irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+ &mbigen_irq_chip, mgn_irq_data);
+
+ return 0;
+}
+
+static void mbigen_domain_free(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs)
+{
+ struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+ struct mbigen_irq_data *mgn_irq_data = irq_data_get_irq_chip_data(d);
+
+ kfree(mgn_irq_data);
+ irq_domain_free_irqs_common(domain, virq, nr_irqs);
+}
+
+static struct irq_domain_ops mbigen_domain_ops = {
+ .translate = mbigen_domain_translate,
+ .alloc = mbigen_irq_domain_alloc,
+ .free = mbigen_domain_free,
};
static int mbigen_device_probe(struct platform_device *pdev)
{
struct mbigen_device *mgn_chip;
+ struct irq_domain *domain;
+ u32 num_msis;
mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
if (!mgn_chip)
@@ -45,6 +194,20 @@ static int mbigen_device_probe(struct platform_device *pdev)
mgn_chip->pdev = pdev;
mgn_chip->base = of_iomap(pdev->dev.of_node, 0);
+ /* If there is no "num-msi" property, assume 64... */
+ if (of_property_read_u32(pdev->dev.of_node, "num-msis", &num_msis) < 0)
+ num_msis = 64;
+
+ domain = platform_msi_create_device_domain(&pdev->dev, num_msis,
+ mbigen_write_msg,
+ &mbigen_domain_ops,
+ mgn_chip);
+
+ if (!domain)
+ return -ENOMEM;
+
+ mgn_chip->domain = domain;
+
platform_set_drvdata(pdev, mgn_chip);
return 0;
--
1.7.1
From: Ma Jun <[email protected]>
Add the interrupt controller chip operation functions of mbigen chip.
Signed-off-by: Ma Jun <[email protected]>
---
drivers/irqchip/irq-mbigen.c | 102 +++++++++++++++++++++++++++++++++++++++--
1 files changed, 97 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 71291cb..155c210 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -46,6 +46,19 @@
/* offset of vector register in mbigen node */
#define REG_MBIGEN_VEC_OFFSET 0x200
+/**
+ * offset of clear register in mbigen node
+ * This register is used to clear the status
+ * of interrupt
+ */
+#define REG_MBIGEN_CLEAR_OFFSET 0xa00
+
+/**
+ * offset of interrupt type register
+ * This register is used to configure interrupt
+ * trigger type
+ */
+#define REG_MBIGEN_TYPE_OFFSET 0x0
/**
* struct mbigen_device - holds the information of mbigen device.
@@ -64,11 +77,19 @@ struct mbigen_device {
* struct mbigen_irq_data - private data of each irq
*
* @base: mapped address of mbigen chip
+ * @pin_offset: local pin offset of interrupt.
* @reg_vec: addr offset of interrupt vector register.
+ * @reg_type: addr offset of interrupt trigger type register.
+ * @reg_clear: addr offset of interrupt clear register.
+ * @type: interrupt trigger type.
*/
struct mbigen_irq_data {
void __iomem *base;
+ unsigned int pin_offset;
unsigned int reg_vec;
+ unsigned int reg_type;
+ unsigned int reg_clear;
+ unsigned int type;
};
static inline int get_mbigen_vec_reg(u32 nid, u32 offset)
@@ -77,8 +98,68 @@ static inline int get_mbigen_vec_reg(u32 nid, u32 offset)
+ REG_MBIGEN_VEC_OFFSET;
}
+static int get_mbigen_type_reg(u32 nid, u32 offset)
+{
+ int ofst;
+
+ ofst = offset / 32 * 4;
+ return ofst + nid * MBIGEN_NODE_OFFSET
+ + REG_MBIGEN_TYPE_OFFSET;
+}
+
+static int get_mbigen_clear_reg(u32 nid, u32 offset)
+{
+ int ofst;
+
+ ofst = offset / 32 * 4;
+ return ofst + nid * MBIGEN_NODE_OFFSET
+ + REG_MBIGEN_CLEAR_OFFSET;
+}
+
+static void mbigen_eoi_irq(struct irq_data *data)
+{
+ struct mbigen_irq_data *mgn_irq_data = irq_data_get_irq_chip_data(data);
+ u32 mask;
+
+ /* only level triggered interrupt need to clear status */
+ if (mgn_irq_data->type == IRQ_TYPE_LEVEL_HIGH) {
+ mask = 1 << (mgn_irq_data->pin_offset % 32);
+ writel_relaxed(mask, mgn_irq_data->reg_clear + mgn_irq_data->base);
+ }
+
+ irq_chip_eoi_parent(data);
+}
+
+static int mbigen_set_type(struct irq_data *d, unsigned int type)
+{
+ struct mbigen_irq_data *mgn_irq_data = irq_data_get_irq_chip_data(d);
+ u32 mask;
+ int val;
+
+ if (type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_EDGE_RISING)
+ return -EINVAL;
+
+ mask = 1 << (mgn_irq_data->pin_offset % 32);
+
+ val = readl_relaxed(mgn_irq_data->reg_type + mgn_irq_data->base);
+
+ if (type == IRQ_TYPE_LEVEL_HIGH)
+ val |= mask;
+ else if (type == IRQ_TYPE_EDGE_RISING)
+ val &= ~mask;
+
+ writel_relaxed(val, mgn_irq_data->reg_type + mgn_irq_data->base);
+
+ return 0;
+}
+
static struct irq_chip mbigen_irq_chip = {
.name = "mbigen-v2",
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_eoi = mbigen_eoi_irq,
+ .irq_set_type = mbigen_set_type,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
};
static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
@@ -94,10 +175,11 @@ static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
writel_relaxed(val, mgn_irq_data->reg_vec + mgn_irq_data->base);
}
-static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq)
+static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq,
+ unsigned int type)
{
struct mbigen_irq_data *datap;
- unsigned int nid, pin_offset;
+ unsigned int nid;
datap = kzalloc(sizeof(*datap), GFP_KERNEL);
if (!datap)
@@ -106,10 +188,20 @@ static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq)
/* get the mbigen node number */
nid = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP) / IRQS_PER_MBIGEN_NODE + 1;
- pin_offset = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP)
+ datap->pin_offset = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP)
% IRQS_PER_MBIGEN_NODE;
- datap->reg_vec = get_mbigen_vec_reg(nid, pin_offset);
+ datap->reg_vec = get_mbigen_vec_reg(nid, datap->pin_offset);
+ datap->reg_type = get_mbigen_type_reg(nid, datap->pin_offset);
+
+ /* no clear register for edge triggered interrupt */
+ if (type == IRQ_TYPE_EDGE_RISING)
+ datap->reg_clear = 0;
+ else
+ datap->reg_clear = get_mbigen_clear_reg(nid,
+ datap->pin_offset);
+
+ datap->type = type;
return datap;
}
@@ -151,7 +243,7 @@ static int mbigen_irq_domain_alloc(struct irq_domain *domain,
return err;
/* set related information of this irq */
- mgn_irq_data = set_mbigen_irq_data(hwirq);
+ mgn_irq_data = set_mbigen_irq_data(hwirq, type);
if (!mgn_irq_data)
return err;
--
1.7.1
On 06/11/15 08:28, MaJun wrote:
> From: Ma Jun <[email protected]>
>
> Add the mbigen msi interrupt controller bindings document.
>
> This patch based on Mark Rutland's patch
> https://lkml.org/lkml/2015/7/23/558
>
> Signed-off-by: Ma Jun <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/mbigen.txt | 63 ++++++++++++++++++++++
> 1 files changed, 63 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/arm/mbigen.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/mbigen.txt b/Documentation/devicetree/bindings/arm/mbigen.txt
> new file mode 100644
> index 0000000..eb9a7fd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/mbigen.txt
> @@ -0,0 +1,63 @@
> +Hisilicon mbigen device tree bindings.
> +=======================================
> +
> +Mbigen means: message based interrupt generator.
> +
> +MBI is kind of msi interrupt only used on Non-PCI devices.
> +
> +To reduce the wired interrupt number connected to GIC,
> +Hisilicon designed mbigen to collect and generate interrupt.
> +
> +
> +Non-pci devices can connect to mbigen and generate the
> +interrupt by writing ITS register.
> +
> +The mbigen chip and devices connect to mbigen have the following properties:
> +
> +Mbigen main node required properties:
> +-------------------------------------------
> +- compatible: Should be "hisilicon,mbigen-v2"
> +- reg: Specifies the base physical address and size of the Mbigen
> + registers.
> +- interrupt controller: Identifies the node as an interrupt controller
> +- msi-parent: This property has two cells.
> + The 1st cell specifies the ITS this device connected.
> + The 2nd cell specifies the device id.
> +- nr-msis:Specifies the total number of interrupt this device has.
So here you have the nr-msis property...
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> + interrupt source. The value is 2 now.
> +
Just say "Must be 2".
> + The 1st cell is global hardware pin number of the interrupt.
> + This value depends on the Soc design.
> + The 2nd cell is the interrupt trigger type.
> +
> +Examples:
> +
> + mbigen_device_gmac:intc {
> + compatible = "hisilicon,mbigen-v2";
> + reg = <0x0 0xc0080000 0x0 0x10000>;
> + interrupt-controller;
> + msi-parent = <&its_dsa 0x40b1c>;
> + num-msis = <9>;
... and here this is num-msis.
Which one is it? The driver seems to use num-msis as well, but I have no
idea which one is the right one.
> + #interrupt-cells = <2>;
> + };
> +
> +Devices connect to mbigen required properties:
> +----------------------------------------------------
> +-interrupt-parent: Specifies the mbigen device node which device connected.
> +-interrupts:specifies the interrupt source.
> + The 1st cell is global hardware pin number of the interrupt.
> + This value depends on the Soc design.
> + The 2nd cell is the interrupt trigger type
> +
> +Examples:
> + gmac0: ethernet@c2080000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0 0xc2080000 0 0x20000>,
> + <0 0xc0000000 0 0x1000>;
> + interrupt-parent = <&mbigen_device_gmac>;
> + interrupts = <656 1>,
> + <657 1>;
> + };
> +
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On 06/11/15 08:28, MaJun wrote:
> From: Ma Jun <[email protected]>
>
> For peripheral devices which connect to mbigen,mbigen is a interrupt
> controller. So, we create irq domain for each mbigen device and add
> mbigen irq domain into irq hierarchy structure.
>
> Signed-off-by: Ma Jun <[email protected]>
> ---
> drivers/irqchip/irq-mbigen.c | 163 ++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 163 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index 25e4000..71291cb 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -16,27 +16,176 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <linux/interrupt.h>
> +#include <linux/irqchip.h>
> #include <linux/module.h>
> +#include <linux/msi.h>
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
>
> +/* Interrupt numbers per mbigen node supported */
> +#define IRQS_PER_MBIGEN_NODE 128
> +
> +/* 16 irqs (Pin0-pin15) are reserved for each mbigen chip */
> +#define RESERVED_IRQ_PER_MBIGEN_CHIP 16
> +
> +/**
> + * In mbigen vector register
> + * bit[21:12]: event id value
> + * bit[11:0]: device id
> + */
> +#define IRQ_EVENT_ID_SHIFT 12
> +#define IRQ_EVENT_ID_MASK 0x3ff
> +
> +/* register range of each mbigen node */
> +#define MBIGEN_NODE_OFFSET 0x1000
> +
> +/* offset of vector register in mbigen node */
> +#define REG_MBIGEN_VEC_OFFSET 0x200
> +
> +
> /**
> * struct mbigen_device - holds the information of mbigen device.
> *
> * @pdev: pointer to the platform device structure of mbigen chip.
> * @base: mapped address of this mbigen chip.
> + * @domain: pointer to the irq domain
> */
> struct mbigen_device {
> struct platform_device *pdev;
> void __iomem *base;
> + struct irq_domain *domain;
> +};
> +
> +/**
> + * struct mbigen_irq_data - private data of each irq
> + *
> + * @base: mapped address of mbigen chip
> + * @reg_vec: addr offset of interrupt vector register.
> + */
> +struct mbigen_irq_data {
> + void __iomem *base;
> + unsigned int reg_vec;
> +};
> +
> +static inline int get_mbigen_vec_reg(u32 nid, u32 offset)
> +{
> + return (offset * 4) + nid * MBIGEN_NODE_OFFSET
> + + REG_MBIGEN_VEC_OFFSET;
> +}
> +
> +static struct irq_chip mbigen_irq_chip = {
> + .name = "mbigen-v2",
> +};
> +
> +static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
> +{
> + struct mbigen_irq_data *mgn_irq_data = irq_get_chip_data(desc->irq);
> + u32 val;
> +
> + val = readl_relaxed(mgn_irq_data->reg_vec + mgn_irq_data->base);
> +
> + val &= ~(IRQ_EVENT_ID_MASK << IRQ_EVENT_ID_SHIFT);
> + val |= (msg->data << IRQ_EVENT_ID_SHIFT);
> +
> + writel_relaxed(val, mgn_irq_data->reg_vec + mgn_irq_data->base);
> +}
> +
> +static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq)
> +{
> + struct mbigen_irq_data *datap;
> + unsigned int nid, pin_offset;
> +
> + datap = kzalloc(sizeof(*datap), GFP_KERNEL);
> + if (!datap)
> + return NULL;
> +
> + /* get the mbigen node number */
> + nid = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP) / IRQS_PER_MBIGEN_NODE + 1;
> +
> + pin_offset = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP)
> + % IRQS_PER_MBIGEN_NODE;
> +
> + datap->reg_vec = get_mbigen_vec_reg(nid, pin_offset);
> + return datap;
> +}
I really don't like the fact that you are allocating per a interrupt
structure to cache something that is essentially a set of constants. You
can derive everything from hwirq and work out the scaling at runtime
(the compiler should be pretty smart at optimizing those, ending up
being much cheaper than fetching random data from memory).
All you have to do is cache the base address of your mbichip in the
chip_data field.
For example, your reg_vec usage:
static inline unsigned int get_mbigen_vec_offset(irq_hw_number_t hwirq)
{
unsigned int nid, pin;
hwirq -= RESERVED_IRQ_PER_MBIGEN_CHIP;
nid = hwirq / IRQS_PER_MBIGEN_NODE + 1;
pin = hwirq % IRQS_PER_MBIGEN_NODE;
return pin * 4 + nid * MBIGEN_NODE_OFFSET +
REG_MBIGEN_VEC_OFFSET;
}
static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
{
struct irq_data *d = irq_get_irq_data(desc->irq);
void __iomem *base = d->chip_data;
u32 val;
base += get_mgigen_vec_offset(d->hwirq);
val = readl_relaxed(base);
val &= ~(IRQ_EVENT_ID_MASK << IRQ_EVENT_ID_SHIFT);
val |= (msg->data << IRQ_EVENT_ID_SHIFT);
writel_relaxed(val, base);
}
No allocation, and GCC should be able to turn get_mbigen_vec_offset into
a nice bunch of shifts and masks. Same goes for the additional stuff you
have in patch #4.
> +
> +static int mbigen_domain_translate(struct irq_domain *d,
> + struct irq_fwspec *fwspec,
> + unsigned long *hwirq,
> + unsigned int *type)
> +{
> + if (is_of_node(fwspec->fwnode)) {
> + if (fwspec->param_count != 2)
> + return -EINVAL;
> +
> + *hwirq = fwspec->param[0];
> + *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
> +
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +static int mbigen_irq_domain_alloc(struct irq_domain *domain,
> + unsigned int virq,
> + unsigned int nr_irqs,
> + void *args)
> +{
> + struct irq_fwspec *fwspec = args;
> + irq_hw_number_t hwirq;
> + unsigned int type;
> + struct mbigen_device *mgn_chip;
> + struct mbigen_irq_data *mgn_irq_data;
> + int i, err;
> +
> + err = mbigen_domain_translate(domain, fwspec, &hwirq, &type);
> + if (err)
> + return err;
> +
> + err = platform_msi_domain_alloc(domain, virq, nr_irqs);
> + if (err)
> + return err;
> +
> + /* set related information of this irq */
> + mgn_irq_data = set_mbigen_irq_data(hwirq);
> + if (!mgn_irq_data)
> + return err;
You can now get rid of this.
> +
> + mgn_chip = platform_msi_get_host_data(domain);
> + mgn_irq_data->base = mgn_chip->base;
> +
> + for (i = 0; i < nr_irqs; i++)
> + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> + &mbigen_irq_chip, mgn_irq_data);
and just store the base.
> +
> + return 0;
> +}
> +
> +static void mbigen_domain_free(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs)
> +{
> + struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> + struct mbigen_irq_data *mgn_irq_data = irq_data_get_irq_chip_data(d);
> +
> + kfree(mgn_irq_data);
> + irq_domain_free_irqs_common(domain, virq, nr_irqs);
> +}
> +
> +static struct irq_domain_ops mbigen_domain_ops = {
> + .translate = mbigen_domain_translate,
> + .alloc = mbigen_irq_domain_alloc,
> + .free = mbigen_domain_free,
> };
>
> static int mbigen_device_probe(struct platform_device *pdev)
> {
> struct mbigen_device *mgn_chip;
> + struct irq_domain *domain;
> + u32 num_msis;
>
> mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
> if (!mgn_chip)
> @@ -45,6 +194,20 @@ static int mbigen_device_probe(struct platform_device *pdev)
> mgn_chip->pdev = pdev;
> mgn_chip->base = of_iomap(pdev->dev.of_node, 0);
>
> + /* If there is no "num-msi" property, assume 64... */
> + if (of_property_read_u32(pdev->dev.of_node, "num-msis", &num_msis) < 0)
> + num_msis = 64;
> +
See my comment about patch #2.
> + domain = platform_msi_create_device_domain(&pdev->dev, num_msis,
> + mbigen_write_msg,
> + &mbigen_domain_ops,
> + mgn_chip);
> +
> + if (!domain)
> + return -ENOMEM;
> +
> + mgn_chip->domain = domain;
> +
> platform_set_drvdata(pdev, mgn_chip);
>
> return 0;
>
I'll look at the rest tomorrow.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On 06/11/15 08:28, MaJun wrote:
> From: Ma Jun <[email protected]>
>
> Mbigen means Message Based Interrupt Generator(MBIGEN).
>
> Its a kind of interrupt controller that collects
> the interrupts from external devices and generate msi interrupt.
> Mbigen is applied to reduce the number of wire connected interrupts.
>
> As the peripherals increasing, the interrupts lines needed is
> increasing much, especially on the Arm64 server SOC.
>
> Therefore, the interrupt pin in GIC is not enough to cover so
> many peripherals.
>
> Mbigen is designed to fix this problem.
>
> Mbigen chip locates in ITS or outside of ITS.
>
> Mbigen chip hardware structure shows as below:
>
> mbigen chip
> |---------------------|-------------------|
> mgn_node0 mgn_node1 mgn_node2
> | |-------| |-------|------|
> dev1 dev1 dev2 dev1 dev3 dev4
>
> Each mbigen chip contains several mbigen nodes.
>
> External devices can connect to mbigen node through wire connecting way.
>
> Because a mbigen node only can support 128 interrupt maximum, depends
> on the interrupt lines number of devices, a device can connects to one
> more mbigen nodes.
>
> Also, several different devices can connect to a same mbigen node.
>
> When devices triggered interrupt,mbigen chip detects and collects
> the interrupts and generates the MBI interrupts by writing the ITS
> Translator register.
>
> To simplify mbigen driver,I used a new conception--mbigen device.
> Each mbigen device is initialized as a platform device.
>
> Mbigen device presents the parts(register, pin definition etc.) in
> mbigen chip corresponding to a peripheral device.
>
> So from software view, the structure likes below
>
> mbigen chip
> |---------------------|-----------------|
> mbigen device1 mbigen device2 mbigen device3
> | | |
> dev1 dev2 dev3
>
> Signed-off-by: Ma Jun <[email protected]>
> ---
> drivers/irqchip/Kconfig | 8 ++++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-mbigen.c | 73 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 82 insertions(+), 0 deletions(-)
> create mode 100644 drivers/irqchip/irq-mbigen.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 4d7294e..b205e15 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -27,6 +27,14 @@ config ARM_GIC_V3_ITS
> bool
> select PCI_MSI_IRQ_DOMAIN
>
> +config HISILICON_IRQ_MBIGEN
> + bool "Support mbigen interrupt controller"
> + default n
> + depends on ARM_GIC_V3 && ARM_GIC_V3_ITS && GENERIC_MSI_IRQ_DOMAIN
> + help
> + Enable the mbigen interrupt controller used on
> + Hisilicon platform.
> +
> config ARM_NVIC
> bool
> select IRQ_DOMAIN
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 177f78f..cd76b11 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_ARM_GIC) += irq-gic.o irq-gic-common.o
> obj-$(CONFIG_ARM_GIC_V2M) += irq-gic-v2m.o
> obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o irq-gic-common.o
> obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-gic-v3-its-platform-msi.o
> +obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
> obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
> obj-$(CONFIG_ARM_VIC) += irq-vic.o
> obj-$(CONFIG_ATMEL_AIC_IRQ) += irq-atmel-aic-common.o irq-atmel-aic.o
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> new file mode 100644
> index 0000000..25e4000
> --- /dev/null
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -0,0 +1,73 @@
> +/*
> + * Copyright (C) 2015 Hisilicon Limited, All Rights Reserved.
> + * Author: Jun Ma <[email protected]>
> + * Author: Yun Wu <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/**
> + * struct mbigen_device - holds the information of mbigen device.
> + *
> + * @pdev: pointer to the platform device structure of mbigen chip.
> + * @base: mapped address of this mbigen chip.
> + */
> +struct mbigen_device {
> + struct platform_device *pdev;
> + void __iomem *base;
> +};
> +
> +static int mbigen_device_probe(struct platform_device *pdev)
> +{
> + struct mbigen_device *mgn_chip;
> +
> + mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
> + if (!mgn_chip)
> + return -ENOMEM;
> +
> + mgn_chip->pdev = pdev;
> + mgn_chip->base = of_iomap(pdev->dev.of_node, 0);
> +
If you're going to use the devm_* stuff for your allocations, you might
as well use devm_ioremap_resource as a matter of consistency. And
checking the return value is not superfluous.
> + platform_set_drvdata(pdev, mgn_chip);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mbigen_of_match[] = {
> + { .compatible = "hisilicon,mbigen-v2" },
> + { /* END */ }
> +};
> +MODULE_DEVICE_TABLE(of, mbigen_of_match);
> +
> +static struct platform_driver mbigen_platform_driver = {
> + .driver = {
> + .name = "Hisilicon MBIGEN-V2",
> + .owner = THIS_MODULE,
> + .of_match_table = mbigen_of_match,
> + },
> + .probe = mbigen_device_probe,
> +};
> +
> +module_platform_driver(mbigen_platform_driver);
> +
> +MODULE_AUTHOR("Jun Ma <[email protected]>");
> +MODULE_AUTHOR("Yun Wu <[email protected]>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Hisilicon MBI Generator driver");
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Fri, 6 Nov 2015 16:28:42 +0800
MaJun <[email protected]> wrote:
> From: Ma Jun <[email protected]>
>
> Add the interrupt controller chip operation functions of mbigen chip.
>
> Signed-off-by: Ma Jun <[email protected]>
> ---
> drivers/irqchip/irq-mbigen.c | 102 +++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 97 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index 71291cb..155c210 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -46,6 +46,19 @@
> /* offset of vector register in mbigen node */
> #define REG_MBIGEN_VEC_OFFSET 0x200
>
> +/**
> + * offset of clear register in mbigen node
> + * This register is used to clear the status
> + * of interrupt
> + */
> +#define REG_MBIGEN_CLEAR_OFFSET 0xa00
> +
> +/**
> + * offset of interrupt type register
> + * This register is used to configure interrupt
> + * trigger type
> + */
> +#define REG_MBIGEN_TYPE_OFFSET 0x0
>
> /**
> * struct mbigen_device - holds the information of mbigen device.
> @@ -64,11 +77,19 @@ struct mbigen_device {
> * struct mbigen_irq_data - private data of each irq
> *
> * @base: mapped address of mbigen chip
> + * @pin_offset: local pin offset of interrupt.
> * @reg_vec: addr offset of interrupt vector register.
> + * @reg_type: addr offset of interrupt trigger type register.
> + * @reg_clear: addr offset of interrupt clear register.
> + * @type: interrupt trigger type.
> */
> struct mbigen_irq_data {
> void __iomem *base;
> + unsigned int pin_offset;
> unsigned int reg_vec;
> + unsigned int reg_type;
> + unsigned int reg_clear;
> + unsigned int type;
> };
I have the same comments here as for patch #3. You're storing
information that are just a pure function of hwirq, and essentially
free to compute at runtime. Please fix it in a similar way.
>
> static inline int get_mbigen_vec_reg(u32 nid, u32 offset)
> @@ -77,8 +98,68 @@ static inline int get_mbigen_vec_reg(u32 nid, u32 offset)
> + REG_MBIGEN_VEC_OFFSET;
> }
>
> +static int get_mbigen_type_reg(u32 nid, u32 offset)
> +{
> + int ofst;
> +
> + ofst = offset / 32 * 4;
> + return ofst + nid * MBIGEN_NODE_OFFSET
> + + REG_MBIGEN_TYPE_OFFSET;
> +}
> +
> +static int get_mbigen_clear_reg(u32 nid, u32 offset)
> +{
> + int ofst;
> +
> + ofst = offset / 32 * 4;
> + return ofst + nid * MBIGEN_NODE_OFFSET
> + + REG_MBIGEN_CLEAR_OFFSET;
> +}
> +
> +static void mbigen_eoi_irq(struct irq_data *data)
> +{
> + struct mbigen_irq_data *mgn_irq_data = irq_data_get_irq_chip_data(data);
> + u32 mask;
> +
> + /* only level triggered interrupt need to clear status */
> + if (mgn_irq_data->type == IRQ_TYPE_LEVEL_HIGH) {
> + mask = 1 << (mgn_irq_data->pin_offset % 32);
> + writel_relaxed(mask, mgn_irq_data->reg_clear + mgn_irq_data->base);
> + }
Does this have the effect of regenerating an edge if the level is still
active?
> +
> + irq_chip_eoi_parent(data);
> +}
> +
> +static int mbigen_set_type(struct irq_data *d, unsigned int type)
> +{
> + struct mbigen_irq_data *mgn_irq_data = irq_data_get_irq_chip_data(d);
> + u32 mask;
> + int val;
> +
> + if (type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_EDGE_RISING)
> + return -EINVAL;
If these are the only two interrupt triggers you support, then you
should update the documentation (DT binding) to reflect this, as all it
says is "The 2nd cell is the interrupt trigger type" which is pretty
vague.
> +
> + mask = 1 << (mgn_irq_data->pin_offset % 32);
> +
> + val = readl_relaxed(mgn_irq_data->reg_type + mgn_irq_data->base);
> +
> + if (type == IRQ_TYPE_LEVEL_HIGH)
> + val |= mask;
> + else if (type == IRQ_TYPE_EDGE_RISING)
> + val &= ~mask;
Given that you've excluded anything but LEVEL_HIGH and EDGE_RISING
already, the second if() is superfluous.
> +
> + writel_relaxed(val, mgn_irq_data->reg_type + mgn_irq_data->base);
> +
> + return 0;
> +}
> +
> static struct irq_chip mbigen_irq_chip = {
> .name = "mbigen-v2",
> + .irq_mask = irq_chip_mask_parent,
> + .irq_unmask = irq_chip_unmask_parent,
> + .irq_eoi = mbigen_eoi_irq,
> + .irq_set_type = mbigen_set_type,
> + .irq_set_affinity = irq_chip_set_affinity_parent,
> };
>
> static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
> @@ -94,10 +175,11 @@ static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
> writel_relaxed(val, mgn_irq_data->reg_vec + mgn_irq_data->base);
> }
>
> -static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq)
> +static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq,
> + unsigned int type)
> {
> struct mbigen_irq_data *datap;
> - unsigned int nid, pin_offset;
> + unsigned int nid;
>
> datap = kzalloc(sizeof(*datap), GFP_KERNEL);
> if (!datap)
> @@ -106,10 +188,20 @@ static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq)
> /* get the mbigen node number */
> nid = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP) / IRQS_PER_MBIGEN_NODE + 1;
>
> - pin_offset = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP)
> + datap->pin_offset = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP)
> % IRQS_PER_MBIGEN_NODE;
>
> - datap->reg_vec = get_mbigen_vec_reg(nid, pin_offset);
> + datap->reg_vec = get_mbigen_vec_reg(nid, datap->pin_offset);
> + datap->reg_type = get_mbigen_type_reg(nid, datap->pin_offset);
> +
> + /* no clear register for edge triggered interrupt */
> + if (type == IRQ_TYPE_EDGE_RISING)
> + datap->reg_clear = 0;
> + else
> + datap->reg_clear = get_mbigen_clear_reg(nid,
> + datap->pin_offset);
> +
> + datap->type = type;
> return datap;
> }
That function can entirely go.
>
> @@ -151,7 +243,7 @@ static int mbigen_irq_domain_alloc(struct irq_domain *domain,
> return err;
>
> /* set related information of this irq */
> - mgn_irq_data = set_mbigen_irq_data(hwirq);
> + mgn_irq_data = set_mbigen_irq_data(hwirq, type);
> if (!mgn_irq_data)
> return err;
>
Thanks,
M.
--
Jazz is not dead. It just smells funny.
Hi Marc:
在 2015/11/19 17:41, Marc Zyngier 写道:
> On Fri, 6 Nov 2015 16:28:42 +0800
> MaJun <[email protected]> wrote:
>
>> From: Ma Jun <[email protected]>
>>
[...]
>> struct mbigen_irq_data {
>> void __iomem *base;
>> + unsigned int pin_offset;
>> unsigned int reg_vec;
>> + unsigned int reg_type;
>> + unsigned int reg_clear;
>> + unsigned int type;
>> };
>
> I have the same comments here as for patch #3. You're storing
> information that are just a pure function of hwirq, and essentially
> free to compute at runtime. Please fix it in a similar way.
>
ok, I'll fix this.
>>
>> static inline int get_mbigen_vec_reg(u32 nid, u32 offset)
>> @@ -77,8 +98,68 @@ static inline int get_mbigen_vec_reg(u32 nid, u32 offset)
>> + REG_MBIGEN_VEC_OFFSET;
>> }
>>
>> +static int get_mbigen_type_reg(u32 nid, u32 offset)
>> +{
>> + int ofst;
>> +
>> + ofst = offset / 32 * 4;
>> + return ofst + nid * MBIGEN_NODE_OFFSET
>> + + REG_MBIGEN_TYPE_OFFSET;
>> +}
>> +
>> +static int get_mbigen_clear_reg(u32 nid, u32 offset)
>> +{
>> + int ofst;
>> +
>> + ofst = offset / 32 * 4;
>> + return ofst + nid * MBIGEN_NODE_OFFSET
>> + + REG_MBIGEN_CLEAR_OFFSET;
>> +}
>> +
>> +static void mbigen_eoi_irq(struct irq_data *data)
>> +{
>> + struct mbigen_irq_data *mgn_irq_data = irq_data_get_irq_chip_data(data);
>> + u32 mask;
>> +
>> + /* only level triggered interrupt need to clear status */
>> + if (mgn_irq_data->type == IRQ_TYPE_LEVEL_HIGH) {
>> + mask = 1 << (mgn_irq_data->pin_offset % 32);
>> + writel_relaxed(mask, mgn_irq_data->reg_clear + mgn_irq_data->base);
>> + }
>
> Does this have the effect of regenerating an edge if the level is still
> active?
>
yes, it does.
>> +
>> + irq_chip_eoi_parent(data);
>> +}
>> +
>> +static int mbigen_set_type(struct irq_data *d, unsigned int type)
>> +{
>> + struct mbigen_irq_data *mgn_irq_data = irq_data_get_irq_chip_data(d);
>> + u32 mask;
>> + int val;
>> +
>> + if (type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_EDGE_RISING)
>> + return -EINVAL;
>
> If these are the only two interrupt triggers you support, then you
> should update the documentation (DT binding) to reflect this, as all it
> says is "The 2nd cell is the interrupt trigger type" which is pretty
> vague.
>
I'll specify this point in DT binding.
Thanks
Ma Jun
>> +
>> + mask = 1 << (mgn_irq_data->pin_offset % 32);
>> +
>> + val = readl_relaxed(mgn_irq_data->reg_type + mgn_irq_data->base);
>> +
>> + if (type == IRQ_TYPE_LEVEL_HIGH)
>> + val |= mask;
>> + else if (type == IRQ_TYPE_EDGE_RISING)
>> + val &= ~mask;
>
> Given that you've excluded anything but LEVEL_HIGH and EDGE_RISING
> already, the second if() is superfluous.
>
>> +
>> + writel_relaxed(val, mgn_irq_data->reg_type + mgn_irq_data->base);
>> +
>> + return 0;
>> +}
>> +
>> static struct irq_chip mbigen_irq_chip = {
>> .name = "mbigen-v2",
>> + .irq_mask = irq_chip_mask_parent,
>> + .irq_unmask = irq_chip_unmask_parent,
>> + .irq_eoi = mbigen_eoi_irq,
>> + .irq_set_type = mbigen_set_type,
>> + .irq_set_affinity = irq_chip_set_affinity_parent,
>> };
>>
>> static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
>> @@ -94,10 +175,11 @@ static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
>> writel_relaxed(val, mgn_irq_data->reg_vec + mgn_irq_data->base);
>> }
>>
>> -static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq)
>> +static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq,
>> + unsigned int type)
>> {
>> struct mbigen_irq_data *datap;
>> - unsigned int nid, pin_offset;
>> + unsigned int nid;
>>
>> datap = kzalloc(sizeof(*datap), GFP_KERNEL);
>> if (!datap)
>> @@ -106,10 +188,20 @@ static struct mbigen_irq_data *set_mbigen_irq_data(int hwirq)
>> /* get the mbigen node number */
>> nid = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP) / IRQS_PER_MBIGEN_NODE + 1;
>>
>> - pin_offset = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP)
>> + datap->pin_offset = (hwirq - RESERVED_IRQ_PER_MBIGEN_CHIP)
>> % IRQS_PER_MBIGEN_NODE;
>>
>> - datap->reg_vec = get_mbigen_vec_reg(nid, pin_offset);
>> + datap->reg_vec = get_mbigen_vec_reg(nid, datap->pin_offset);
>> + datap->reg_type = get_mbigen_type_reg(nid, datap->pin_offset);
>> +
>> + /* no clear register for edge triggered interrupt */
>> + if (type == IRQ_TYPE_EDGE_RISING)
>> + datap->reg_clear = 0;
>> + else
>> + datap->reg_clear = get_mbigen_clear_reg(nid,
>> + datap->pin_offset);
>> +
>> + datap->type = type;
>> return datap;
>> }
>
> That function can entirely go.
>
>>
>> @@ -151,7 +243,7 @@ static int mbigen_irq_domain_alloc(struct irq_domain *domain,
>> return err;
>>
>> /* set related information of this irq */
>> - mgn_irq_data = set_mbigen_irq_data(hwirq);
>> + mgn_irq_data = set_mbigen_irq_data(hwirq, type);
>> if (!mgn_irq_data)
>> return err;
>>
>
> Thanks,
>
> M.
>
在 2015/11/19 1:50, Marc Zyngier 写道:
> On 06/11/15 08:28, MaJun wrote:
>> From: Ma Jun <[email protected]>
>>
>> Add the mbigen msi interrupt controller bindings document.
>>
>> This patch based on Mark Rutland's patch
>> https://lkml.org/lkml/2015/7/23/558
>>
>> Signed-off-by: Ma Jun <[email protected]>
>> ---
>> Documentation/devicetree/bindings/arm/mbigen.txt | 63 ++++++++++++++++++++++
>> 1 files changed, 63 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/arm/mbigen.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/mbigen.txt b/Documentation/devicetree/bindings/arm/mbigen.txt
>> new file mode 100644
>> index 0000000..eb9a7fd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/mbigen.txt
>> @@ -0,0 +1,63 @@
>> +Hisilicon mbigen device tree bindings.
>> +=======================================
>> +
>> +Mbigen means: message based interrupt generator.
>> +
>> +MBI is kind of msi interrupt only used on Non-PCI devices.
>> +
>> +To reduce the wired interrupt number connected to GIC,
>> +Hisilicon designed mbigen to collect and generate interrupt.
>> +
>> +
>> +Non-pci devices can connect to mbigen and generate the
>> +interrupt by writing ITS register.
>> +
>> +The mbigen chip and devices connect to mbigen have the following properties:
>> +
>> +Mbigen main node required properties:
>> +-------------------------------------------
>> +- compatible: Should be "hisilicon,mbigen-v2"
>> +- reg: Specifies the base physical address and size of the Mbigen
>> + registers.
>> +- interrupt controller: Identifies the node as an interrupt controller
>> +- msi-parent: This property has two cells.
>> + The 1st cell specifies the ITS this device connected.
>> + The 2nd cell specifies the device id.
>> +- nr-msis:Specifies the total number of interrupt this device has.
>
> So here you have the nr-msis property...
I'll change this to num-msis.
Thanks!
Ma Jun
>
>> +- #interrupt-cells : Specifies the number of cells needed to encode an
>> + interrupt source. The value is 2 now.
>> +
>
> Just say "Must be 2".
>
>> + The 1st cell is global hardware pin number of the interrupt.
>> + This value depends on the Soc design.
>> + The 2nd cell is the interrupt trigger type.
>> +
>> +Examples:
>> +
>> + mbigen_device_gmac:intc {
>> + compatible = "hisilicon,mbigen-v2";
>> + reg = <0x0 0xc0080000 0x0 0x10000>;
>> + interrupt-controller;
>> + msi-parent = <&its_dsa 0x40b1c>;
>> + num-msis = <9>;
>
> ... and here this is num-msis.
>
> Which one is it? The driver seems to use num-msis as well, but I have no
> idea which one is the right one.
>
>> + #interrupt-cells = <2>;
>> + };
>> +
>> +Devices connect to mbigen required properties:
>> +----------------------------------------------------
>> +-interrupt-parent: Specifies the mbigen device node which device connected.
>> +-interrupts:specifies the interrupt source.
>> + The 1st cell is global hardware pin number of the interrupt.
>> + This value depends on the Soc design.
>> + The 2nd cell is the interrupt trigger type
>> +
>> +Examples:
>> + gmac0: ethernet@c2080000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0 0xc2080000 0 0x20000>,
>> + <0 0xc0000000 0 0x1000>;
>> + interrupt-parent = <&mbigen_device_gmac>;
>> + interrupts = <656 1>,
>> + <657 1>;
>> + };
>> +
>>
>
> Thanks,
>
> M.
>