* [PATCH v2,2/9] arm: dts: ls1021a: fix typo of MSI compatible string
2017-01-05 8:10 [PATCH v2,1/9] irqchip/ls-scfg-msi: fix typo of MSI compatible strings Minghuan Lian
@ 2017-01-05 8:10 ` Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,3/9] arm64: dts: ls1043a: " Minghuan Lian
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Minghuan Lian @ 2017-01-05 8:10 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, devicetree
Cc: Marc Zyngier, Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu,
Stuart Yoder, Yang-Leo Li, Scott Wood, Minghuan Lian
"1" should be replaced by "l". This is a typo.
The patch is to fix it.
Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
v2-v1:
- None
arch/arm/boot/dts/ls1021a.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 282d854..6651938 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -122,14 +122,14 @@
};
msi1: msi-controller@1570e00 {
- compatible = "fsl,1s1021a-msi";
+ compatible = "fsl,ls1021a-msi";
reg = <0x0 0x1570e00 0x0 0x8>;
msi-controller;
interrupts = <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH>;
};
msi2: msi-controller@1570e08 {
- compatible = "fsl,1s1021a-msi";
+ compatible = "fsl,ls1021a-msi";
reg = <0x0 0x1570e08 0x0 0x8>;
msi-controller;
interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2,3/9] arm64: dts: ls1043a: fix typo of MSI compatible string
2017-01-05 8:10 [PATCH v2,1/9] irqchip/ls-scfg-msi: fix typo of MSI compatible strings Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,2/9] arm: dts: ls1021a: fix typo of MSI compatible string Minghuan Lian
@ 2017-01-05 8:10 ` Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,4/9] arm: dts: ls1021a: share all MSIs Minghuan Lian
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Minghuan Lian @ 2017-01-05 8:10 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, devicetree
Cc: Marc Zyngier, Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu,
Stuart Yoder, Yang-Leo Li, Scott Wood, Minghuan Lian
"1" should be replaced by "l". This is a typo.
The patch is to fix it.
Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
v2-v1:
- None
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index ec13a6e..692fc35 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -589,21 +589,21 @@
};
msi1: msi-controller1@1571000 {
- compatible = "fsl,1s1043a-msi";
+ compatible = "fsl,ls1043a-msi";
reg = <0x0 0x1571000 0x0 0x8>;
msi-controller;
interrupts = <0 116 0x4>;
};
msi2: msi-controller2@1572000 {
- compatible = "fsl,1s1043a-msi";
+ compatible = "fsl,ls1043a-msi";
reg = <0x0 0x1572000 0x0 0x8>;
msi-controller;
interrupts = <0 126 0x4>;
};
msi3: msi-controller3@1573000 {
- compatible = "fsl,1s1043a-msi";
+ compatible = "fsl,ls1043a-msi";
reg = <0x0 0x1573000 0x0 0x8>;
msi-controller;
interrupts = <0 160 0x4>;
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2,4/9] arm: dts: ls1021a: share all MSIs
2017-01-05 8:10 [PATCH v2,1/9] irqchip/ls-scfg-msi: fix typo of MSI compatible strings Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,2/9] arm: dts: ls1021a: fix typo of MSI compatible string Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,3/9] arm64: dts: ls1043a: " Minghuan Lian
@ 2017-01-05 8:10 ` Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,5/9] arm64: dts: ls1043a: " Minghuan Lian
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Minghuan Lian @ 2017-01-05 8:10 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, devicetree
Cc: Marc Zyngier, Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu,
Stuart Yoder, Yang-Leo Li, Scott Wood, Minghuan Lian
In order to maximize the use of MSI, a PCIe controller will share
all MSI controllers. The patch changes msi-parent to refer to all
MSI controller dts nodes.
Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
v2-v1:
- None
arch/arm/boot/dts/ls1021a.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 6651938..1c82024 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -723,7 +723,7 @@
bus-range = <0x0 0xff>;
ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000 /* downstream I/O */
0x82000000 0x0 0x40000000 0x40 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
- msi-parent = <&msi1>;
+ msi-parent = <&msi1>, <&msi2>;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0000 0 0 1 &gic GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>,
@@ -746,7 +746,7 @@
bus-range = <0x0 0xff>;
ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000 /* downstream I/O */
0x82000000 0x0 0x40000000 0x48 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
- msi-parent = <&msi2>;
+ msi-parent = <&msi1>, <&msi2>;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0000 0 0 1 &gic GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>,
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2,5/9] arm64: dts: ls1043a: share all MSIs
2017-01-05 8:10 [PATCH v2,1/9] irqchip/ls-scfg-msi: fix typo of MSI compatible strings Minghuan Lian
` (2 preceding siblings ...)
2017-01-05 8:10 ` [PATCH v2,4/9] arm: dts: ls1021a: share all MSIs Minghuan Lian
@ 2017-01-05 8:10 ` Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,6/9] arm64: dts: ls1046a: add MSI dts node Minghuan Lian
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Minghuan Lian @ 2017-01-05 8:10 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, devicetree
Cc: Marc Zyngier, Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu,
Stuart Yoder, Yang-Leo Li, Scott Wood, Minghuan Lian
In order to maximize the use of MSI, a PCIe controller will share
all MSI controllers. The patch changes "msi-parent" to refer to all
MSI controller dts nodes.
Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
v2-v1:
- None
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 692fc35..3947220 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -625,7 +625,7 @@
bus-range = <0x0 0xff>;
ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000 /* downstream I/O */
0x82000000 0x0 0x40000000 0x40 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
- msi-parent = <&msi1>;
+ msi-parent = <&msi1>, <&msi2>, <&msi3>;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0000 0 0 1 &gic 0 110 0x4>,
@@ -650,7 +650,7 @@
bus-range = <0x0 0xff>;
ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000 /* downstream I/O */
0x82000000 0x0 0x40000000 0x48 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
- msi-parent = <&msi2>;
+ msi-parent = <&msi1>, <&msi2>, <&msi3>;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0000 0 0 1 &gic 0 120 0x4>,
@@ -675,7 +675,7 @@
bus-range = <0x0 0xff>;
ranges = <0x81000000 0x0 0x00000000 0x50 0x00010000 0x0 0x00010000 /* downstream I/O */
0x82000000 0x0 0x40000000 0x50 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
- msi-parent = <&msi3>;
+ msi-parent = <&msi1>, <&msi2>, <&msi3>;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0000 0 0 1 &gic 0 154 0x4>,
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2,6/9] arm64: dts: ls1046a: add MSI dts node
2017-01-05 8:10 [PATCH v2,1/9] irqchip/ls-scfg-msi: fix typo of MSI compatible strings Minghuan Lian
` (3 preceding siblings ...)
2017-01-05 8:10 ` [PATCH v2,5/9] arm64: dts: ls1043a: " Minghuan Lian
@ 2017-01-05 8:10 ` Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,7/9] irqchip/ls-scfg-msi: add LS1046a MSI support Minghuan Lian
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Minghuan Lian @ 2017-01-05 8:10 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, devicetree
Cc: Marc Zyngier, Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu,
Stuart Yoder, Yang-Leo Li, Scott Wood, Minghuan Lian
LS1046a includes 3 MSI controllers.
Each controller supports 128 interrupts.
Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
Acked-by: Rob Herring <robh@kernel.org>
---
v2-v1:
- change whitespace number
.../interrupt-controller/fsl,ls-scfg-msi.txt | 1 +
arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 31 ++++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
index 2755cd1..dde4552 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
@@ -6,6 +6,7 @@ Required properties:
Layerscape PCIe MSI controller block such as:
"fsl,ls1021a-msi"
"fsl,ls1043a-msi"
+ "fsl,ls1046a-msi"
- msi-controller: indicates that this is a PCIe MSI controller node
- reg: physical base address of the controller and length of memory mapped.
- interrupts: an interrupt to the parent interrupt controller.
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 38806ca..49dbafc 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -511,5 +511,36 @@
interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clockgen 4 1>;
};
+
+ msi1: msi-controller@1580000 {
+ compatible = "fsl,ls1046a-msi";
+ msi-controller;
+ reg = <0x0 0x1580000 0x0 0x10000>;
+ interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ msi2: msi-controller@1590000 {
+ compatible = "fsl,ls1046a-msi";
+ msi-controller;
+ reg = <0x0 0x1590000 0x0 0x10000>;
+ interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ msi3: msi-controller@15a0000 {
+ compatible = "fsl,ls1046a-msi";
+ msi-controller;
+ reg = <0x0 0x15a0000 0x0 0x10000>;
+ interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
};
};
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2,7/9] irqchip/ls-scfg-msi: add LS1046a MSI support
2017-01-05 8:10 [PATCH v2,1/9] irqchip/ls-scfg-msi: fix typo of MSI compatible strings Minghuan Lian
` (4 preceding siblings ...)
2017-01-05 8:10 ` [PATCH v2,6/9] arm64: dts: ls1046a: add MSI dts node Minghuan Lian
@ 2017-01-05 8:10 ` Minghuan Lian
2017-01-05 15:19 ` Marc Zyngier
2017-01-05 8:10 ` [PATCH v2,8/9] irqchip/ls-scfg-msi: add LS1043a v1.1 " Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,9/9] irqchip/ls-scfg-msi: add MSI affinity support Minghuan Lian
7 siblings, 1 reply; 13+ messages in thread
From: Minghuan Lian @ 2017-01-05 8:10 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, devicetree
Cc: Marc Zyngier, Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu,
Stuart Yoder, Yang-Leo Li, Scott Wood, Minghuan Lian
LS1046a includes 4 MSIRs, each MSIR is assigned a dedicate GIC
SPI interrupt and provides 32 MSI interrupts. Compared to previous
MSI, LS1046a's IBS(interrupt bit select) shift is changed to 2 and
total MSI interrupt number is changed to 128.
The patch adds structure 'ls_scfg_msir' to describe MSIR setting and
'ibs_shift' to store the different value between the SoCs.
Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
v2-v1:
- MSI dts node change has been merged into the patch 6/9
drivers/irqchip/irq-ls-scfg-msi.c | 161 +++++++++++++++++++++++++++++---------
1 file changed, 126 insertions(+), 35 deletions(-)
diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
index cef67cc..67547bd 100644
--- a/drivers/irqchip/irq-ls-scfg-msi.c
+++ b/drivers/irqchip/irq-ls-scfg-msi.c
@@ -17,13 +17,24 @@
#include <linux/irq.h>
#include <linux/irqchip/chained_irq.h>
#include <linux/irqdomain.h>
+#include <linux/of_irq.h>
#include <linux/of_pci.h>
#include <linux/of_platform.h>
#include <linux/spinlock.h>
-#define MSI_MAX_IRQS 32
-#define MSI_IBS_SHIFT 3
-#define MSIR 4
+#define MSI_IRQS_PER_MSIR 32
+#define MSI_MSIR_OFFSET 4
+
+struct ls_scfg_msi_cfg {
+ u32 ibs_shift; /* Shift of interrupt bit select */
+};
+
+struct ls_scfg_msir {
+ struct ls_scfg_msi *msi_data;
+ unsigned int index;
+ unsigned int gic_irq;
+ void __iomem *reg;
+};
struct ls_scfg_msi {
spinlock_t lock;
@@ -32,8 +43,11 @@ struct ls_scfg_msi {
struct irq_domain *msi_domain;
void __iomem *regs;
phys_addr_t msiir_addr;
- int irq;
- DECLARE_BITMAP(used, MSI_MAX_IRQS);
+ struct ls_scfg_msi_cfg *cfg;
+ u32 msir_num;
+ struct ls_scfg_msir *msir;
+ u32 irqs_num;
+ unsigned long *used;
};
static struct irq_chip ls_scfg_msi_irq_chip = {
@@ -55,7 +69,7 @@ static void ls_scfg_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
msg->address_hi = upper_32_bits(msi_data->msiir_addr);
msg->address_lo = lower_32_bits(msi_data->msiir_addr);
- msg->data = data->hwirq << MSI_IBS_SHIFT;
+ msg->data = data->hwirq;
}
static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
@@ -81,8 +95,8 @@ static int ls_scfg_msi_domain_irq_alloc(struct irq_domain *domain,
WARN_ON(nr_irqs != 1);
spin_lock(&msi_data->lock);
- pos = find_first_zero_bit(msi_data->used, MSI_MAX_IRQS);
- if (pos < MSI_MAX_IRQS)
+ pos = find_first_zero_bit(msi_data->used, msi_data->irqs_num);
+ if (pos < msi_data->irqs_num)
__set_bit(pos, msi_data->used);
else
err = -ENOSPC;
@@ -106,7 +120,7 @@ static void ls_scfg_msi_domain_irq_free(struct irq_domain *domain,
int pos;
pos = d->hwirq;
- if (pos < 0 || pos >= MSI_MAX_IRQS) {
+ if (pos < 0 || pos >= msi_data->irqs_num) {
pr_err("failed to teardown msi. Invalid hwirq %d\n", pos);
return;
}
@@ -123,15 +137,17 @@ static void ls_scfg_msi_domain_irq_free(struct irq_domain *domain,
static void ls_scfg_msi_irq_handler(struct irq_desc *desc)
{
- struct ls_scfg_msi *msi_data = irq_desc_get_handler_data(desc);
+ struct ls_scfg_msir *msir = irq_desc_get_handler_data(desc);
+ struct ls_scfg_msi *msi_data = msir->msi_data;
unsigned long val;
- int pos, virq;
+ int pos, virq, hwirq;
chained_irq_enter(irq_desc_get_chip(desc), desc);
- val = ioread32be(msi_data->regs + MSIR);
- for_each_set_bit(pos, &val, MSI_MAX_IRQS) {
- virq = irq_find_mapping(msi_data->parent, (31 - pos));
+ val = ioread32be(msir->reg);
+ for_each_set_bit(pos, &val, MSI_IRQS_PER_MSIR) {
+ hwirq = ((31 - pos) << msi_data->cfg->ibs_shift) | msir->index;
+ virq = irq_find_mapping(msi_data->parent, hwirq);
if (virq)
generic_handle_irq(virq);
}
@@ -143,7 +159,7 @@ static int ls_scfg_msi_domains_init(struct ls_scfg_msi *msi_data)
{
/* Initialize MSI domain parent */
msi_data->parent = irq_domain_add_linear(NULL,
- MSI_MAX_IRQS,
+ msi_data->irqs_num,
&ls_scfg_msi_domain_ops,
msi_data);
if (!msi_data->parent) {
@@ -164,16 +180,83 @@ static int ls_scfg_msi_domains_init(struct ls_scfg_msi *msi_data)
return 0;
}
+static int ls_scfg_msi_setup_hwirq(struct ls_scfg_msi *msi_data, int index)
+{
+ struct ls_scfg_msir *msir;
+ int virq, i, hwirq;
+
+ virq = platform_get_irq(msi_data->pdev, index);
+ if (virq <= 0)
+ return -ENODEV;
+
+ msir = &msi_data->msir[index];
+ msir->index = index;
+ msir->msi_data = msi_data;
+ msir->gic_irq = virq;
+ msir->reg = msi_data->regs + MSI_MSIR_OFFSET + 4 * index;
+
+ irq_set_chained_handler_and_data(msir->gic_irq,
+ ls_scfg_msi_irq_handler,
+ msir);
+
+ /* Release the hwirqs corresponding to this MSIR */
+ for (i = 0; i < MSI_IRQS_PER_MSIR; i++) {
+ hwirq = i << msi_data->cfg->ibs_shift | msir->index;
+ bitmap_clear(msi_data->used, hwirq, 1);
+ }
+
+ return 0;
+}
+
+static int ls_scfg_msi_teardown_hwirq(struct ls_scfg_msir *msir)
+{
+ struct ls_scfg_msi *msi_data = msir->msi_data;
+ int i, hwirq;
+
+ if (msir->gic_irq > 0)
+ irq_set_chained_handler_and_data(msir->gic_irq, NULL, NULL);
+
+ for (i = 0; i < MSI_IRQS_PER_MSIR; i++) {
+ hwirq = i << msi_data->cfg->ibs_shift | msir->index;
+ bitmap_set(msi_data->used, hwirq, 1);
+ }
+
+ return 0;
+}
+
+static struct ls_scfg_msi_cfg ls1021_msi_cfg = {
+ .ibs_shift = 3,
+};
+
+static struct ls_scfg_msi_cfg ls1046_msi_cfg = {
+ .ibs_shift = 2,
+};
+
+static const struct of_device_id ls_scfg_msi_id[] = {
+ { .compatible = "fsl,ls1021a-msi", .data = &ls1021_msi_cfg },
+ { .compatible = "fsl,ls1043a-msi", .data = &ls1021_msi_cfg },
+ { .compatible = "fsl,ls1046a-msi", .data = &ls1046_msi_cfg },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ls_scfg_msi_id);
+
static int ls_scfg_msi_probe(struct platform_device *pdev)
{
+ const struct of_device_id *match;
struct ls_scfg_msi *msi_data;
struct resource *res;
- int ret;
+ int i, ret;
+
+ match = of_match_device(ls_scfg_msi_id, &pdev->dev);
+ if (!match)
+ return -ENODEV;
msi_data = devm_kzalloc(&pdev->dev, sizeof(*msi_data), GFP_KERNEL);
if (!msi_data)
return -ENOMEM;
+ msi_data->cfg = (struct ls_scfg_msi_cfg *) match->data;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
msi_data->regs = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(msi_data->regs)) {
@@ -182,23 +265,37 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
}
msi_data->msiir_addr = res->start;
- msi_data->irq = platform_get_irq(pdev, 0);
- if (msi_data->irq <= 0) {
- dev_err(&pdev->dev, "failed to get MSI irq\n");
- return -ENODEV;
- }
-
msi_data->pdev = pdev;
spin_lock_init(&msi_data->lock);
+ msi_data->irqs_num = MSI_IRQS_PER_MSIR *
+ (1 << msi_data->cfg->ibs_shift);
+ msi_data->used = devm_kcalloc(&pdev->dev,
+ BITS_TO_LONGS(msi_data->irqs_num),
+ sizeof(*msi_data->used),
+ GFP_KERNEL);
+ if (!msi_data->used)
+ return -ENOMEM;
+ /*
+ * Reserve all the hwirqs
+ * The available hwirqs will be released in ls1_msi_setup_hwirq()
+ */
+ bitmap_set(msi_data->used, 0, msi_data->irqs_num);
+
+ msi_data->msir_num = of_irq_count(pdev->dev.of_node);
+ msi_data->msir = devm_kcalloc(&pdev->dev, msi_data->msir_num,
+ sizeof(*msi_data->msir),
+ GFP_KERNEL);
+ if (!msi_data->msir)
+ return -ENOMEM;
+
+ for (i = 0; i < msi_data->msir_num; i++)
+ ls_scfg_msi_setup_hwirq(msi_data, i);
+
ret = ls_scfg_msi_domains_init(msi_data);
if (ret)
return ret;
- irq_set_chained_handler_and_data(msi_data->irq,
- ls_scfg_msi_irq_handler,
- msi_data);
-
platform_set_drvdata(pdev, msi_data);
return 0;
@@ -207,8 +304,10 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
static int ls_scfg_msi_remove(struct platform_device *pdev)
{
struct ls_scfg_msi *msi_data = platform_get_drvdata(pdev);
+ int i;
- irq_set_chained_handler_and_data(msi_data->irq, NULL, NULL);
+ for (i = 0; i < msi_data->msir_num; i++)
+ ls_scfg_msi_teardown_hwirq(&msi_data->msir[i]);
irq_domain_remove(msi_data->msi_domain);
irq_domain_remove(msi_data->parent);
@@ -218,14 +317,6 @@ static int ls_scfg_msi_remove(struct platform_device *pdev)
return 0;
}
-static const struct of_device_id ls_scfg_msi_id[] = {
- { .compatible = "fsl,1s1021a-msi", }, /* a typo */
- { .compatible = "fsl,1s1043a-msi", }, /* a typo */
- { .compatible = "fsl,ls1021a-msi", },
- { .compatible = "fsl,ls1043a-msi", },
- {},
-};
-
static struct platform_driver ls_scfg_msi_driver = {
.driver = {
.name = "ls-scfg-msi",
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2,7/9] irqchip/ls-scfg-msi: add LS1046a MSI support
2017-01-05 8:10 ` [PATCH v2,7/9] irqchip/ls-scfg-msi: add LS1046a MSI support Minghuan Lian
@ 2017-01-05 15:19 ` Marc Zyngier
2017-01-06 1:45 ` M.H. Lian
0 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2017-01-05 15:19 UTC (permalink / raw)
To: Minghuan Lian, linux-arm-kernel, linux-kernel, devicetree
Cc: Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu, Stuart Yoder,
Yang-Leo Li, Scott Wood
On 05/01/17 08:10, Minghuan Lian wrote:
> LS1046a includes 4 MSIRs, each MSIR is assigned a dedicate GIC
> SPI interrupt and provides 32 MSI interrupts. Compared to previous
> MSI, LS1046a's IBS(interrupt bit select) shift is changed to 2 and
> total MSI interrupt number is changed to 128.
>
> The patch adds structure 'ls_scfg_msir' to describe MSIR setting and
> 'ibs_shift' to store the different value between the SoCs.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
> v2-v1:
> - MSI dts node change has been merged into the patch 6/9
>
> drivers/irqchip/irq-ls-scfg-msi.c | 161 +++++++++++++++++++++++++++++---------
> 1 file changed, 126 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
> index cef67cc..67547bd 100644
> --- a/drivers/irqchip/irq-ls-scfg-msi.c
> +++ b/drivers/irqchip/irq-ls-scfg-msi.c
> @@ -17,13 +17,24 @@
> #include <linux/irq.h>
> #include <linux/irqchip/chained_irq.h>
> #include <linux/irqdomain.h>
> +#include <linux/of_irq.h>
> #include <linux/of_pci.h>
> #include <linux/of_platform.h>
> #include <linux/spinlock.h>
>
> -#define MSI_MAX_IRQS 32
> -#define MSI_IBS_SHIFT 3
> -#define MSIR 4
> +#define MSI_IRQS_PER_MSIR 32
> +#define MSI_MSIR_OFFSET 4
> +
> +struct ls_scfg_msi_cfg {
> + u32 ibs_shift; /* Shift of interrupt bit select */
> +};
> +
> +struct ls_scfg_msir {
> + struct ls_scfg_msi *msi_data;
> + unsigned int index;
> + unsigned int gic_irq;
> + void __iomem *reg;
> +};
>
> struct ls_scfg_msi {
> spinlock_t lock;
> @@ -32,8 +43,11 @@ struct ls_scfg_msi {
> struct irq_domain *msi_domain;
> void __iomem *regs;
> phys_addr_t msiir_addr;
> - int irq;
> - DECLARE_BITMAP(used, MSI_MAX_IRQS);
> + struct ls_scfg_msi_cfg *cfg;
> + u32 msir_num;
> + struct ls_scfg_msir *msir;
> + u32 irqs_num;
> + unsigned long *used;
> };
>
> static struct irq_chip ls_scfg_msi_irq_chip = {
> @@ -55,7 +69,7 @@ static void ls_scfg_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
>
> msg->address_hi = upper_32_bits(msi_data->msiir_addr);
> msg->address_lo = lower_32_bits(msi_data->msiir_addr);
> - msg->data = data->hwirq << MSI_IBS_SHIFT;
> + msg->data = data->hwirq;
> }
>
> static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
> @@ -81,8 +95,8 @@ static int ls_scfg_msi_domain_irq_alloc(struct irq_domain *domain,
> WARN_ON(nr_irqs != 1);
>
> spin_lock(&msi_data->lock);
> - pos = find_first_zero_bit(msi_data->used, MSI_MAX_IRQS);
> - if (pos < MSI_MAX_IRQS)
> + pos = find_first_zero_bit(msi_data->used, msi_data->irqs_num);
> + if (pos < msi_data->irqs_num)
> __set_bit(pos, msi_data->used);
> else
> err = -ENOSPC;
> @@ -106,7 +120,7 @@ static void ls_scfg_msi_domain_irq_free(struct irq_domain *domain,
> int pos;
>
> pos = d->hwirq;
> - if (pos < 0 || pos >= MSI_MAX_IRQS) {
> + if (pos < 0 || pos >= msi_data->irqs_num) {
> pr_err("failed to teardown msi. Invalid hwirq %d\n", pos);
> return;
> }
> @@ -123,15 +137,17 @@ static void ls_scfg_msi_domain_irq_free(struct irq_domain *domain,
>
> static void ls_scfg_msi_irq_handler(struct irq_desc *desc)
> {
> - struct ls_scfg_msi *msi_data = irq_desc_get_handler_data(desc);
> + struct ls_scfg_msir *msir = irq_desc_get_handler_data(desc);
> + struct ls_scfg_msi *msi_data = msir->msi_data;
> unsigned long val;
> - int pos, virq;
> + int pos, virq, hwirq;
>
> chained_irq_enter(irq_desc_get_chip(desc), desc);
>
> - val = ioread32be(msi_data->regs + MSIR);
> - for_each_set_bit(pos, &val, MSI_MAX_IRQS) {
> - virq = irq_find_mapping(msi_data->parent, (31 - pos));
> + val = ioread32be(msir->reg);
> + for_each_set_bit(pos, &val, MSI_IRQS_PER_MSIR) {
> + hwirq = ((31 - pos) << msi_data->cfg->ibs_shift) | msir->index;
> + virq = irq_find_mapping(msi_data->parent, hwirq);
> if (virq)
> generic_handle_irq(virq);
> }
> @@ -143,7 +159,7 @@ static int ls_scfg_msi_domains_init(struct ls_scfg_msi *msi_data)
> {
> /* Initialize MSI domain parent */
> msi_data->parent = irq_domain_add_linear(NULL,
> - MSI_MAX_IRQS,
> + msi_data->irqs_num,
> &ls_scfg_msi_domain_ops,
> msi_data);
> if (!msi_data->parent) {
> @@ -164,16 +180,83 @@ static int ls_scfg_msi_domains_init(struct ls_scfg_msi *msi_data)
> return 0;
> }
>
> +static int ls_scfg_msi_setup_hwirq(struct ls_scfg_msi *msi_data, int index)
> +{
> + struct ls_scfg_msir *msir;
> + int virq, i, hwirq;
> +
> + virq = platform_get_irq(msi_data->pdev, index);
> + if (virq <= 0)
> + return -ENODEV;
> +
> + msir = &msi_data->msir[index];
> + msir->index = index;
> + msir->msi_data = msi_data;
> + msir->gic_irq = virq;
> + msir->reg = msi_data->regs + MSI_MSIR_OFFSET + 4 * index;
> +
> + irq_set_chained_handler_and_data(msir->gic_irq,
> + ls_scfg_msi_irq_handler,
> + msir);
> +
> + /* Release the hwirqs corresponding to this MSIR */
> + for (i = 0; i < MSI_IRQS_PER_MSIR; i++) {
> + hwirq = i << msi_data->cfg->ibs_shift | msir->index;
> + bitmap_clear(msi_data->used, hwirq, 1);
> + }
> +
> + return 0;
> +}
> +
> +static int ls_scfg_msi_teardown_hwirq(struct ls_scfg_msir *msir)
> +{
> + struct ls_scfg_msi *msi_data = msir->msi_data;
> + int i, hwirq;
> +
> + if (msir->gic_irq > 0)
> + irq_set_chained_handler_and_data(msir->gic_irq, NULL, NULL);
> +
> + for (i = 0; i < MSI_IRQS_PER_MSIR; i++) {
> + hwirq = i << msi_data->cfg->ibs_shift | msir->index;
> + bitmap_set(msi_data->used, hwirq, 1);
> + }
> +
> + return 0;
> +}
> +
> +static struct ls_scfg_msi_cfg ls1021_msi_cfg = {
> + .ibs_shift = 3,
> +};
> +
> +static struct ls_scfg_msi_cfg ls1046_msi_cfg = {
> + .ibs_shift = 2,
> +};
> +
> +static const struct of_device_id ls_scfg_msi_id[] = {
> + { .compatible = "fsl,ls1021a-msi", .data = &ls1021_msi_cfg },
> + { .compatible = "fsl,ls1043a-msi", .data = &ls1021_msi_cfg },
> + { .compatible = "fsl,ls1046a-msi", .data = &ls1046_msi_cfg },
So after 3 patches adding compatibility between the fsl,1s10 and
fsl,ls10 names, you discard them? How does it work again with the old DTs?
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ls_scfg_msi_id);
> +
> static int ls_scfg_msi_probe(struct platform_device *pdev)
> {
> + const struct of_device_id *match;
> struct ls_scfg_msi *msi_data;
> struct resource *res;
> - int ret;
> + int i, ret;
> +
> + match = of_match_device(ls_scfg_msi_id, &pdev->dev);
> + if (!match)
> + return -ENODEV;
>
> msi_data = devm_kzalloc(&pdev->dev, sizeof(*msi_data), GFP_KERNEL);
> if (!msi_data)
> return -ENOMEM;
>
> + msi_data->cfg = (struct ls_scfg_msi_cfg *) match->data;
> +
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> msi_data->regs = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(msi_data->regs)) {
> @@ -182,23 +265,37 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
> }
> msi_data->msiir_addr = res->start;
>
> - msi_data->irq = platform_get_irq(pdev, 0);
> - if (msi_data->irq <= 0) {
> - dev_err(&pdev->dev, "failed to get MSI irq\n");
> - return -ENODEV;
> - }
> -
> msi_data->pdev = pdev;
> spin_lock_init(&msi_data->lock);
>
> + msi_data->irqs_num = MSI_IRQS_PER_MSIR *
> + (1 << msi_data->cfg->ibs_shift);
> + msi_data->used = devm_kcalloc(&pdev->dev,
> + BITS_TO_LONGS(msi_data->irqs_num),
> + sizeof(*msi_data->used),
> + GFP_KERNEL);
> + if (!msi_data->used)
> + return -ENOMEM;
> + /*
> + * Reserve all the hwirqs
> + * The available hwirqs will be released in ls1_msi_setup_hwirq()
> + */
> + bitmap_set(msi_data->used, 0, msi_data->irqs_num);
> +
> + msi_data->msir_num = of_irq_count(pdev->dev.of_node);
> + msi_data->msir = devm_kcalloc(&pdev->dev, msi_data->msir_num,
> + sizeof(*msi_data->msir),
> + GFP_KERNEL);
> + if (!msi_data->msir)
> + return -ENOMEM;
> +
> + for (i = 0; i < msi_data->msir_num; i++)
> + ls_scfg_msi_setup_hwirq(msi_data, i);
> +
> ret = ls_scfg_msi_domains_init(msi_data);
> if (ret)
> return ret;
>
> - irq_set_chained_handler_and_data(msi_data->irq,
> - ls_scfg_msi_irq_handler,
> - msi_data);
> -
> platform_set_drvdata(pdev, msi_data);
>
> return 0;
> @@ -207,8 +304,10 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
> static int ls_scfg_msi_remove(struct platform_device *pdev)
> {
> struct ls_scfg_msi *msi_data = platform_get_drvdata(pdev);
> + int i;
>
> - irq_set_chained_handler_and_data(msi_data->irq, NULL, NULL);
> + for (i = 0; i < msi_data->msir_num; i++)
> + ls_scfg_msi_teardown_hwirq(&msi_data->msir[i]);
>
> irq_domain_remove(msi_data->msi_domain);
> irq_domain_remove(msi_data->parent);
> @@ -218,14 +317,6 @@ static int ls_scfg_msi_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id ls_scfg_msi_id[] = {
> - { .compatible = "fsl,1s1021a-msi", }, /* a typo */
> - { .compatible = "fsl,1s1043a-msi", }, /* a typo */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
these entries definitely need to be restored.
Also, please make sure you have a cover letter at the beginning of the
series. I flag series to review by flagging the cover letter, and I
suspect many of us are doing something similar. Not doing so is likely
to leave your series unreviewed...
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2,7/9] irqchip/ls-scfg-msi: add LS1046a MSI support
2017-01-05 15:19 ` Marc Zyngier
@ 2017-01-06 1:45 ` M.H. Lian
0 siblings, 0 replies; 13+ messages in thread
From: M.H. Lian @ 2017-01-06 1:45 UTC (permalink / raw)
To: Marc Zyngier, linux-arm-kernel, linux-kernel, devicetree
Cc: Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu, Stuart Yoder,
Leo Li, Scott Wood
Hi Marc,
Thanks for your review.
Please see my comments inline.
Thanks,
Minghuan
> -----Original Message-----
> From: Marc Zyngier [mailto:marc.zyngier@arm.com]
> Sent: Thursday, January 05, 2017 11:19 PM
> To: M.H. Lian <minghuan.lian@nxp.com>; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>; Jason Cooper
> <jason@lakedaemon.net>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Stuart Yoder <stuart.yoder@nxp.com>; Leo Li
> <leoyang.li@nxp.com>; Scott Wood <scott.wood@nxp.com>
> Subject: Re: [PATCH v2,7/9] irqchip/ls-scfg-msi: add LS1046a MSI support
>
> On 05/01/17 08:10, Minghuan Lian wrote:
> > LS1046a includes 4 MSIRs, each MSIR is assigned a dedicate GIC SPI
> > interrupt and provides 32 MSI interrupts. Compared to previous MSI,
> > LS1046a's IBS(interrupt bit select) shift is changed to 2 and total
> > MSI interrupt number is changed to 128.
> >
> > The patch adds structure 'ls_scfg_msir' to describe MSIR setting and
> > 'ibs_shift' to store the different value between the SoCs.
> >
> > Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> > ---
> > v2-v1:
> > - MSI dts node change has been merged into the patch 6/9
> >
> > drivers/irqchip/irq-ls-scfg-msi.c | 161
> > +++++++++++++++++++++++++++++---------
> > 1 file changed, 126 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-ls-scfg-msi.c
> > b/drivers/irqchip/irq-ls-scfg-msi.c
> > index cef67cc..67547bd 100644
> > --- a/drivers/irqchip/irq-ls-scfg-msi.c
> > +++ b/drivers/irqchip/irq-ls-scfg-msi.c
> > @@ -17,13 +17,24 @@
> > #include <linux/irq.h>
> > #include <linux/irqchip/chained_irq.h> #include <linux/irqdomain.h>
> > +#include <linux/of_irq.h>
> > #include <linux/of_pci.h>
> > #include <linux/of_platform.h>
> > #include <linux/spinlock.h>
> >
> > -#define MSI_MAX_IRQS 32
> > -#define MSI_IBS_SHIFT 3
> > -#define MSIR 4
> > +#define MSI_IRQS_PER_MSIR 32
> > +#define MSI_MSIR_OFFSET 4
> > +
> > +struct ls_scfg_msi_cfg {
> > + u32 ibs_shift; /* Shift of interrupt bit select */ };
> > +
> > +struct ls_scfg_msir {
> > + struct ls_scfg_msi *msi_data;
> > + unsigned int index;
> > + unsigned int gic_irq;
> > + void __iomem *reg;
> > +};
> >
> > struct ls_scfg_msi {
> > spinlock_t lock;
> > @@ -32,8 +43,11 @@ struct ls_scfg_msi {
> > struct irq_domain *msi_domain;
> > void __iomem *regs;
> > phys_addr_t msiir_addr;
> > - int irq;
> > - DECLARE_BITMAP(used, MSI_MAX_IRQS);
> > + struct ls_scfg_msi_cfg *cfg;
> > + u32 msir_num;
> > + struct ls_scfg_msir *msir;
> > + u32 irqs_num;
> > + unsigned long *used;
> > };
> >
> > static struct irq_chip ls_scfg_msi_irq_chip = { @@ -55,7 +69,7 @@
> > static void ls_scfg_msi_compose_msg(struct irq_data *data, struct
> > msi_msg *msg)
> >
> > msg->address_hi = upper_32_bits(msi_data->msiir_addr);
> > msg->address_lo = lower_32_bits(msi_data->msiir_addr);
> > - msg->data = data->hwirq << MSI_IBS_SHIFT;
> > + msg->data = data->hwirq;
> > }
> >
> > static int ls_scfg_msi_set_affinity(struct irq_data *irq_data, @@
> > -81,8 +95,8 @@ static int ls_scfg_msi_domain_irq_alloc(struct irq_domain
> *domain,
> > WARN_ON(nr_irqs != 1);
> >
> > spin_lock(&msi_data->lock);
> > - pos = find_first_zero_bit(msi_data->used, MSI_MAX_IRQS);
> > - if (pos < MSI_MAX_IRQS)
> > + pos = find_first_zero_bit(msi_data->used, msi_data->irqs_num);
> > + if (pos < msi_data->irqs_num)
> > __set_bit(pos, msi_data->used);
> > else
> > err = -ENOSPC;
> > @@ -106,7 +120,7 @@ static void ls_scfg_msi_domain_irq_free(struct
> irq_domain *domain,
> > int pos;
> >
> > pos = d->hwirq;
> > - if (pos < 0 || pos >= MSI_MAX_IRQS) {
> > + if (pos < 0 || pos >= msi_data->irqs_num) {
> > pr_err("failed to teardown msi. Invalid hwirq %d\n", pos);
> > return;
> > }
> > @@ -123,15 +137,17 @@ static void ls_scfg_msi_domain_irq_free(struct
> > irq_domain *domain,
> >
> > static void ls_scfg_msi_irq_handler(struct irq_desc *desc) {
> > - struct ls_scfg_msi *msi_data = irq_desc_get_handler_data(desc);
> > + struct ls_scfg_msir *msir = irq_desc_get_handler_data(desc);
> > + struct ls_scfg_msi *msi_data = msir->msi_data;
> > unsigned long val;
> > - int pos, virq;
> > + int pos, virq, hwirq;
> >
> > chained_irq_enter(irq_desc_get_chip(desc), desc);
> >
> > - val = ioread32be(msi_data->regs + MSIR);
> > - for_each_set_bit(pos, &val, MSI_MAX_IRQS) {
> > - virq = irq_find_mapping(msi_data->parent, (31 - pos));
> > + val = ioread32be(msir->reg);
> > + for_each_set_bit(pos, &val, MSI_IRQS_PER_MSIR) {
> > + hwirq = ((31 - pos) << msi_data->cfg->ibs_shift) | msir-
> >index;
> > + virq = irq_find_mapping(msi_data->parent, hwirq);
> > if (virq)
> > generic_handle_irq(virq);
> > }
> > @@ -143,7 +159,7 @@ static int ls_scfg_msi_domains_init(struct
> > ls_scfg_msi *msi_data) {
> > /* Initialize MSI domain parent */
> > msi_data->parent = irq_domain_add_linear(NULL,
> > - MSI_MAX_IRQS,
> > + msi_data->irqs_num,
> > &ls_scfg_msi_domain_ops,
> > msi_data);
> > if (!msi_data->parent) {
> > @@ -164,16 +180,83 @@ static int ls_scfg_msi_domains_init(struct
> ls_scfg_msi *msi_data)
> > return 0;
> > }
> >
> > +static int ls_scfg_msi_setup_hwirq(struct ls_scfg_msi *msi_data, int
> > +index) {
> > + struct ls_scfg_msir *msir;
> > + int virq, i, hwirq;
> > +
> > + virq = platform_get_irq(msi_data->pdev, index);
> > + if (virq <= 0)
> > + return -ENODEV;
> > +
> > + msir = &msi_data->msir[index];
> > + msir->index = index;
> > + msir->msi_data = msi_data;
> > + msir->gic_irq = virq;
> > + msir->reg = msi_data->regs + MSI_MSIR_OFFSET + 4 * index;
> > +
> > + irq_set_chained_handler_and_data(msir->gic_irq,
> > + ls_scfg_msi_irq_handler,
> > + msir);
> > +
> > + /* Release the hwirqs corresponding to this MSIR */
> > + for (i = 0; i < MSI_IRQS_PER_MSIR; i++) {
> > + hwirq = i << msi_data->cfg->ibs_shift | msir->index;
> > + bitmap_clear(msi_data->used, hwirq, 1);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int ls_scfg_msi_teardown_hwirq(struct ls_scfg_msir *msir) {
> > + struct ls_scfg_msi *msi_data = msir->msi_data;
> > + int i, hwirq;
> > +
> > + if (msir->gic_irq > 0)
> > + irq_set_chained_handler_and_data(msir->gic_irq, NULL,
> NULL);
> > +
> > + for (i = 0; i < MSI_IRQS_PER_MSIR; i++) {
> > + hwirq = i << msi_data->cfg->ibs_shift | msir->index;
> > + bitmap_set(msi_data->used, hwirq, 1);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct ls_scfg_msi_cfg ls1021_msi_cfg = {
> > + .ibs_shift = 3,
> > +};
> > +
> > +static struct ls_scfg_msi_cfg ls1046_msi_cfg = {
> > + .ibs_shift = 2,
> > +};
> > +
> > +static const struct of_device_id ls_scfg_msi_id[] = {
> > + { .compatible = "fsl,ls1021a-msi", .data = &ls1021_msi_cfg },
> > + { .compatible = "fsl,ls1043a-msi", .data = &ls1021_msi_cfg },
> > + { .compatible = "fsl,ls1046a-msi", .data = &ls1046_msi_cfg },
>
> So after 3 patches adding compatibility between the fsl,1s10 and
> fsl,ls10 names, you discard them? How does it work again with the old DTs?
>
[Minghuan Lian] ok, I will store the old compatible.
This typo makes me very uncomfortable :(
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, ls_scfg_msi_id);
> > +
> > static int ls_scfg_msi_probe(struct platform_device *pdev) {
> > + const struct of_device_id *match;
> > struct ls_scfg_msi *msi_data;
> > struct resource *res;
> > - int ret;
> > + int i, ret;
> > +
> > + match = of_match_device(ls_scfg_msi_id, &pdev->dev);
> > + if (!match)
> > + return -ENODEV;
> >
> > msi_data = devm_kzalloc(&pdev->dev, sizeof(*msi_data),
> GFP_KERNEL);
> > if (!msi_data)
> > return -ENOMEM;
> >
> > + msi_data->cfg = (struct ls_scfg_msi_cfg *) match->data;
> > +
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > msi_data->regs = devm_ioremap_resource(&pdev->dev, res);
> > if (IS_ERR(msi_data->regs)) {
> > @@ -182,23 +265,37 @@ static int ls_scfg_msi_probe(struct
> platform_device *pdev)
> > }
> > msi_data->msiir_addr = res->start;
> >
> > - msi_data->irq = platform_get_irq(pdev, 0);
> > - if (msi_data->irq <= 0) {
> > - dev_err(&pdev->dev, "failed to get MSI irq\n");
> > - return -ENODEV;
> > - }
> > -
> > msi_data->pdev = pdev;
> > spin_lock_init(&msi_data->lock);
> >
> > + msi_data->irqs_num = MSI_IRQS_PER_MSIR *
> > + (1 << msi_data->cfg->ibs_shift);
> > + msi_data->used = devm_kcalloc(&pdev->dev,
> > + BITS_TO_LONGS(msi_data->irqs_num),
> > + sizeof(*msi_data->used),
> > + GFP_KERNEL);
> > + if (!msi_data->used)
> > + return -ENOMEM;
> > + /*
> > + * Reserve all the hwirqs
> > + * The available hwirqs will be released in ls1_msi_setup_hwirq()
> > + */
> > + bitmap_set(msi_data->used, 0, msi_data->irqs_num);
> > +
> > + msi_data->msir_num = of_irq_count(pdev->dev.of_node);
> > + msi_data->msir = devm_kcalloc(&pdev->dev, msi_data->msir_num,
> > + sizeof(*msi_data->msir),
> > + GFP_KERNEL);
> > + if (!msi_data->msir)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < msi_data->msir_num; i++)
> > + ls_scfg_msi_setup_hwirq(msi_data, i);
> > +
> > ret = ls_scfg_msi_domains_init(msi_data);
> > if (ret)
> > return ret;
> >
> > - irq_set_chained_handler_and_data(msi_data->irq,
> > - ls_scfg_msi_irq_handler,
> > - msi_data);
> > -
> > platform_set_drvdata(pdev, msi_data);
> >
> > return 0;
> > @@ -207,8 +304,10 @@ static int ls_scfg_msi_probe(struct
> > platform_device *pdev) static int ls_scfg_msi_remove(struct
> > platform_device *pdev) {
> > struct ls_scfg_msi *msi_data = platform_get_drvdata(pdev);
> > + int i;
> >
> > - irq_set_chained_handler_and_data(msi_data->irq, NULL, NULL);
> > + for (i = 0; i < msi_data->msir_num; i++)
> > + ls_scfg_msi_teardown_hwirq(&msi_data->msir[i]);
> >
> > irq_domain_remove(msi_data->msi_domain);
> > irq_domain_remove(msi_data->parent);
> > @@ -218,14 +317,6 @@ static int ls_scfg_msi_remove(struct
> platform_device *pdev)
> > return 0;
> > }
> >
> > -static const struct of_device_id ls_scfg_msi_id[] = {
> > - { .compatible = "fsl,1s1021a-msi", }, /* a typo */
> > - { .compatible = "fsl,1s1043a-msi", }, /* a typo */
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> ^
> these entries definitely need to be restored.
>
> Also, please make sure you have a cover letter at the beginning of the series.
> I flag series to review by flagging the cover letter, and I suspect many of us
> are doing something similar. Not doing so is likely to leave your series
> unreviewed...
[Minghuan Lian] I see. I will add cover letter for this series. And I will
do this for all series of patches.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2,8/9] irqchip/ls-scfg-msi: add LS1043a v1.1 MSI support
2017-01-05 8:10 [PATCH v2,1/9] irqchip/ls-scfg-msi: fix typo of MSI compatible strings Minghuan Lian
` (5 preceding siblings ...)
2017-01-05 8:10 ` [PATCH v2,7/9] irqchip/ls-scfg-msi: add LS1046a MSI support Minghuan Lian
@ 2017-01-05 8:10 ` Minghuan Lian
2017-01-05 8:10 ` [PATCH v2,9/9] irqchip/ls-scfg-msi: add MSI affinity support Minghuan Lian
7 siblings, 0 replies; 13+ messages in thread
From: Minghuan Lian @ 2017-01-05 8:10 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, devicetree
Cc: Marc Zyngier, Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu,
Stuart Yoder, Yang-Leo Li, Scott Wood, Minghuan Lian
A MSI controller of LS1043a v1.0 only includes one MSIR and
is assigned one GIC interrupt. In order to support affinity,
LS1043a v1.1 MSI is assigned 4 MSIRs and 4 GIC interrupts.
But the MSIR has the different offset and only supports 8 MSIs.
The bits between variable bit_start and bit_end in structure
ls_scfg_msir are used to show 8 MSI interrupts. msir_irqs and
msir_base are added to describe the difference of MSI between
LS1043a v1.1 and other SoCs.
Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
Acked-by: Rob Herring <robh@kernel.org>
---
v2-v1:
- None
.../interrupt-controller/fsl,ls-scfg-msi.txt | 1 +
drivers/irqchip/irq-ls-scfg-msi.c | 45 +++++++++++++++++++---
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
index dde4552..49ccabb 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
@@ -7,6 +7,7 @@ Required properties:
"fsl,ls1021a-msi"
"fsl,ls1043a-msi"
"fsl,ls1046a-msi"
+ "fsl,ls1043a-v1.1-msi"
- msi-controller: indicates that this is a PCIe MSI controller node
- reg: physical base address of the controller and length of memory mapped.
- interrupts: an interrupt to the parent interrupt controller.
diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
index 67547bd..dc19569 100644
--- a/drivers/irqchip/irq-ls-scfg-msi.c
+++ b/drivers/irqchip/irq-ls-scfg-msi.c
@@ -25,14 +25,21 @@
#define MSI_IRQS_PER_MSIR 32
#define MSI_MSIR_OFFSET 4
+#define MSI_LS1043V1_1_IRQS_PER_MSIR 8
+#define MSI_LS1043V1_1_MSIR_OFFSET 0x10
+
struct ls_scfg_msi_cfg {
u32 ibs_shift; /* Shift of interrupt bit select */
+ u32 msir_irqs; /* The irq number per MSIR */
+ u32 msir_base; /* The base address of MSIR */
};
struct ls_scfg_msir {
struct ls_scfg_msi *msi_data;
unsigned int index;
unsigned int gic_irq;
+ unsigned int bit_start;
+ unsigned int bit_end;
void __iomem *reg;
};
@@ -140,13 +147,18 @@ static void ls_scfg_msi_irq_handler(struct irq_desc *desc)
struct ls_scfg_msir *msir = irq_desc_get_handler_data(desc);
struct ls_scfg_msi *msi_data = msir->msi_data;
unsigned long val;
- int pos, virq, hwirq;
+ int pos, size, virq, hwirq;
chained_irq_enter(irq_desc_get_chip(desc), desc);
val = ioread32be(msir->reg);
- for_each_set_bit(pos, &val, MSI_IRQS_PER_MSIR) {
- hwirq = ((31 - pos) << msi_data->cfg->ibs_shift) | msir->index;
+
+ pos = msir->bit_start;
+ size = msir->bit_end + 1;
+
+ for_each_set_bit_from(pos, &val, size) {
+ hwirq = ((msir->bit_end - pos) << msi_data->cfg->ibs_shift) |
+ msir->index;
virq = irq_find_mapping(msi_data->parent, hwirq);
if (virq)
generic_handle_irq(virq);
@@ -193,14 +205,24 @@ static int ls_scfg_msi_setup_hwirq(struct ls_scfg_msi *msi_data, int index)
msir->index = index;
msir->msi_data = msi_data;
msir->gic_irq = virq;
- msir->reg = msi_data->regs + MSI_MSIR_OFFSET + 4 * index;
+ msir->reg = msi_data->regs + msi_data->cfg->msir_base + 4 * index;
+
+ if (msi_data->cfg->msir_irqs == MSI_LS1043V1_1_IRQS_PER_MSIR) {
+ msir->bit_start = 32 - ((msir->index + 1) *
+ MSI_LS1043V1_1_IRQS_PER_MSIR);
+ msir->bit_end = msir->bit_start +
+ MSI_LS1043V1_1_IRQS_PER_MSIR - 1;
+ } else {
+ msir->bit_start = 0;
+ msir->bit_end = msi_data->cfg->msir_irqs - 1;
+ }
irq_set_chained_handler_and_data(msir->gic_irq,
ls_scfg_msi_irq_handler,
msir);
/* Release the hwirqs corresponding to this MSIR */
- for (i = 0; i < MSI_IRQS_PER_MSIR; i++) {
+ for (i = 0; i < msi_data->cfg->msir_irqs; i++) {
hwirq = i << msi_data->cfg->ibs_shift | msir->index;
bitmap_clear(msi_data->used, hwirq, 1);
}
@@ -216,7 +238,7 @@ static int ls_scfg_msi_teardown_hwirq(struct ls_scfg_msir *msir)
if (msir->gic_irq > 0)
irq_set_chained_handler_and_data(msir->gic_irq, NULL, NULL);
- for (i = 0; i < MSI_IRQS_PER_MSIR; i++) {
+ for (i = 0; i < msi_data->cfg->msir_irqs; i++) {
hwirq = i << msi_data->cfg->ibs_shift | msir->index;
bitmap_set(msi_data->used, hwirq, 1);
}
@@ -226,15 +248,26 @@ static int ls_scfg_msi_teardown_hwirq(struct ls_scfg_msir *msir)
static struct ls_scfg_msi_cfg ls1021_msi_cfg = {
.ibs_shift = 3,
+ .msir_irqs = MSI_IRQS_PER_MSIR,
+ .msir_base = MSI_MSIR_OFFSET,
};
static struct ls_scfg_msi_cfg ls1046_msi_cfg = {
.ibs_shift = 2,
+ .msir_irqs = MSI_IRQS_PER_MSIR,
+ .msir_base = MSI_MSIR_OFFSET,
+};
+
+static struct ls_scfg_msi_cfg ls1043_v1_1_msi_cfg = {
+ .ibs_shift = 2,
+ .msir_irqs = MSI_LS1043V1_1_IRQS_PER_MSIR,
+ .msir_base = MSI_LS1043V1_1_MSIR_OFFSET,
};
static const struct of_device_id ls_scfg_msi_id[] = {
{ .compatible = "fsl,ls1021a-msi", .data = &ls1021_msi_cfg },
{ .compatible = "fsl,ls1043a-msi", .data = &ls1021_msi_cfg },
+ { .compatible = "fsl,ls1043a-v1.1-msi", .data = &ls1043_v1_1_msi_cfg },
{ .compatible = "fsl,ls1046a-msi", .data = &ls1046_msi_cfg },
{},
};
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2,9/9] irqchip/ls-scfg-msi: add MSI affinity support
2017-01-05 8:10 [PATCH v2,1/9] irqchip/ls-scfg-msi: fix typo of MSI compatible strings Minghuan Lian
` (6 preceding siblings ...)
2017-01-05 8:10 ` [PATCH v2,8/9] irqchip/ls-scfg-msi: add LS1043a v1.1 " Minghuan Lian
@ 2017-01-05 8:10 ` Minghuan Lian
2017-01-05 15:33 ` Marc Zyngier
7 siblings, 1 reply; 13+ messages in thread
From: Minghuan Lian @ 2017-01-05 8:10 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, devicetree
Cc: Marc Zyngier, Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu,
Stuart Yoder, Yang-Leo Li, Scott Wood, Minghuan Lian
For LS1046a and LS1043a v1.1, the MSI controller has 4 MSIRs and 4
CPUs. A GIC SPI interrupt of MSIR can be associated with a CPU.
When changing MSI interrupt affinity, this MSI will be moved to the
corresponding MSIR and MSI message data will be changed according to
MSIR. when requesting a MSI, the bits of all 4 MSIR will be reserved.
The parameter 'msi_affinity_flag' is provide to change this mode.
"lsmsi=no-affinity" will disable affinity, all MSI can only be
associated with CPU 0.
Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
v2-v1:
- None
drivers/irqchip/irq-ls-scfg-msi.c | 75 ++++++++++++++++++++++++++++++++++++---
1 file changed, 70 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
index dc19569..753fe39 100644
--- a/drivers/irqchip/irq-ls-scfg-msi.c
+++ b/drivers/irqchip/irq-ls-scfg-msi.c
@@ -40,6 +40,7 @@ struct ls_scfg_msir {
unsigned int gic_irq;
unsigned int bit_start;
unsigned int bit_end;
+ unsigned int srs; /* Shared interrupt register select */
void __iomem *reg;
};
@@ -70,6 +71,19 @@ struct ls_scfg_msi {
.chip = &ls_scfg_msi_irq_chip,
};
+static int msi_affinity_flag = 1;
+
+static int __init early_parse_ls_scfg_msi(char *p)
+{
+ if (p && strncmp(p, "no-affinity", 11) == 0)
+ msi_affinity_flag = 0;
+ else
+ msi_affinity_flag = 1;
+
+ return 0;
+}
+early_param("lsmsi", early_parse_ls_scfg_msi);
+
static void ls_scfg_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
{
struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(data);
@@ -77,12 +91,43 @@ static void ls_scfg_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
msg->address_hi = upper_32_bits(msi_data->msiir_addr);
msg->address_lo = lower_32_bits(msi_data->msiir_addr);
msg->data = data->hwirq;
+
+ if (msi_affinity_flag) {
+ u32 msir_index;
+
+ msir_index = cpumask_first(data->common->affinity);
+ if (msir_index >= msi_data->msir_num)
+ msir_index = 0;
+
+ msg->data |= msir_index;
+ }
}
static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
const struct cpumask *mask, bool force)
{
- return -EINVAL;
+ struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(irq_data);
+ u32 cpu;
+
+ if (!msi_affinity_flag)
+ return -EINVAL;
+
+ if (!force)
+ cpu = cpumask_any_and(mask, cpu_online_mask);
+ else
+ cpu = cpumask_first(mask);
+
+ if (cpu >= msi_data->msir_num)
+ return -EINVAL;
+
+ if (msi_data->msir[cpu].gic_irq <= 0) {
+ pr_warn("cannot bind the irq to cpu%d\n", cpu);
+ return -EINVAL;
+ }
+
+ cpumask_copy(irq_data->common->affinity, mask);
+
+ return IRQ_SET_MASK_OK;
}
static struct irq_chip ls_scfg_msi_parent_chip = {
@@ -158,7 +203,7 @@ static void ls_scfg_msi_irq_handler(struct irq_desc *desc)
for_each_set_bit_from(pos, &val, size) {
hwirq = ((msir->bit_end - pos) << msi_data->cfg->ibs_shift) |
- msir->index;
+ msir->srs;
virq = irq_find_mapping(msi_data->parent, hwirq);
if (virq)
generic_handle_irq(virq);
@@ -221,10 +266,19 @@ static int ls_scfg_msi_setup_hwirq(struct ls_scfg_msi *msi_data, int index)
ls_scfg_msi_irq_handler,
msir);
+ if (msi_affinity_flag) {
+ /* Associate MSIR interrupt to the cpu */
+ irq_set_affinity(msir->gic_irq, get_cpu_mask(index));
+ msir->srs = 0; /* This value is determined by the CPU */
+ } else
+ msir->srs = index;
+
/* Release the hwirqs corresponding to this MSIR */
- for (i = 0; i < msi_data->cfg->msir_irqs; i++) {
- hwirq = i << msi_data->cfg->ibs_shift | msir->index;
- bitmap_clear(msi_data->used, hwirq, 1);
+ if (!msi_affinity_flag || msir->index == 0) {
+ for (i = 0; i < msi_data->cfg->msir_irqs; i++) {
+ hwirq = i << msi_data->cfg->ibs_shift | msir->index;
+ bitmap_clear(msi_data->used, hwirq, 1);
+ }
}
return 0;
@@ -316,6 +370,17 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
bitmap_set(msi_data->used, 0, msi_data->irqs_num);
msi_data->msir_num = of_irq_count(pdev->dev.of_node);
+
+ if (msi_affinity_flag) {
+ u32 cpu_num;
+
+ cpu_num = num_possible_cpus();
+ if (msi_data->msir_num >= cpu_num)
+ msi_data->msir_num = cpu_num;
+ else
+ msi_affinity_flag = 0;
+ }
+
msi_data->msir = devm_kcalloc(&pdev->dev, msi_data->msir_num,
sizeof(*msi_data->msir),
GFP_KERNEL);
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2,9/9] irqchip/ls-scfg-msi: add MSI affinity support
2017-01-05 8:10 ` [PATCH v2,9/9] irqchip/ls-scfg-msi: add MSI affinity support Minghuan Lian
@ 2017-01-05 15:33 ` Marc Zyngier
2017-01-06 8:23 ` M.H. Lian
0 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2017-01-05 15:33 UTC (permalink / raw)
To: Minghuan Lian, linux-arm-kernel, linux-kernel, devicetree
Cc: Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu, Stuart Yoder,
Yang-Leo Li, Scott Wood
On 05/01/17 08:10, Minghuan Lian wrote:
> For LS1046a and LS1043a v1.1, the MSI controller has 4 MSIRs and 4
> CPUs. A GIC SPI interrupt of MSIR can be associated with a CPU.
> When changing MSI interrupt affinity, this MSI will be moved to the
> corresponding MSIR and MSI message data will be changed according to
> MSIR. when requesting a MSI, the bits of all 4 MSIR will be reserved.
> The parameter 'msi_affinity_flag' is provide to change this mode.
> "lsmsi=no-affinity" will disable affinity, all MSI can only be
> associated with CPU 0.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
> v2-v1:
> - None
>
> drivers/irqchip/irq-ls-scfg-msi.c | 75 ++++++++++++++++++++++++++++++++++++---
> 1 file changed, 70 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
> index dc19569..753fe39 100644
> --- a/drivers/irqchip/irq-ls-scfg-msi.c
> +++ b/drivers/irqchip/irq-ls-scfg-msi.c
> @@ -40,6 +40,7 @@ struct ls_scfg_msir {
> unsigned int gic_irq;
> unsigned int bit_start;
> unsigned int bit_end;
> + unsigned int srs; /* Shared interrupt register select */
> void __iomem *reg;
> };
>
> @@ -70,6 +71,19 @@ struct ls_scfg_msi {
> .chip = &ls_scfg_msi_irq_chip,
> };
>
> +static int msi_affinity_flag = 1;
> +
> +static int __init early_parse_ls_scfg_msi(char *p)
> +{
> + if (p && strncmp(p, "no-affinity", 11) == 0)
> + msi_affinity_flag = 0;
> + else
> + msi_affinity_flag = 1;
> +
> + return 0;
> +}
> +early_param("lsmsi", early_parse_ls_scfg_msi);
What is the point of this option? If feels like an unnecessary complexity.
> +
> static void ls_scfg_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
> {
> struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(data);
> @@ -77,12 +91,43 @@ static void ls_scfg_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
> msg->address_hi = upper_32_bits(msi_data->msiir_addr);
> msg->address_lo = lower_32_bits(msi_data->msiir_addr);
> msg->data = data->hwirq;
> +
> + if (msi_affinity_flag) {
> + u32 msir_index;
> +
> + msir_index = cpumask_first(data->common->affinity);
> + if (msir_index >= msi_data->msir_num)
> + msir_index = 0;
How can this happen?
> +
> + msg->data |= msir_index;
How do you guarantee that the low bits were clear? Please document the
way you encode your MSI payload.
> + }
> }
>
> static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
> const struct cpumask *mask, bool force)
> {
> - return -EINVAL;
> + struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(irq_data);
> + u32 cpu;
> +
> + if (!msi_affinity_flag)
> + return -EINVAL;
> +
> + if (!force)
> + cpu = cpumask_any_and(mask, cpu_online_mask);
> + else
> + cpu = cpumask_first(mask);
> +
> + if (cpu >= msi_data->msir_num)
> + return -EINVAL;
> +
> + if (msi_data->msir[cpu].gic_irq <= 0) {
> + pr_warn("cannot bind the irq to cpu%d\n", cpu);
Please don't. Returning an error is enough. If you really want to have
something, turn it into a proper debug message.
> + return -EINVAL;
> + }
> +
> + cpumask_copy(irq_data->common->affinity, mask);
> +
> + return IRQ_SET_MASK_OK;
> }
>
> static struct irq_chip ls_scfg_msi_parent_chip = {
> @@ -158,7 +203,7 @@ static void ls_scfg_msi_irq_handler(struct irq_desc *desc)
>
> for_each_set_bit_from(pos, &val, size) {
> hwirq = ((msir->bit_end - pos) << msi_data->cfg->ibs_shift) |
> - msir->index;
> + msir->srs;
> virq = irq_find_mapping(msi_data->parent, hwirq);
> if (virq)
> generic_handle_irq(virq);
> @@ -221,10 +266,19 @@ static int ls_scfg_msi_setup_hwirq(struct ls_scfg_msi *msi_data, int index)
> ls_scfg_msi_irq_handler,
> msir);
>
> + if (msi_affinity_flag) {
> + /* Associate MSIR interrupt to the cpu */
> + irq_set_affinity(msir->gic_irq, get_cpu_mask(index));
> + msir->srs = 0; /* This value is determined by the CPU */
> + } else
> + msir->srs = index;
> +
> /* Release the hwirqs corresponding to this MSIR */
> - for (i = 0; i < msi_data->cfg->msir_irqs; i++) {
> - hwirq = i << msi_data->cfg->ibs_shift | msir->index;
> - bitmap_clear(msi_data->used, hwirq, 1);
> + if (!msi_affinity_flag || msir->index == 0) {
> + for (i = 0; i < msi_data->cfg->msir_irqs; i++) {
> + hwirq = i << msi_data->cfg->ibs_shift | msir->index;
> + bitmap_clear(msi_data->used, hwirq, 1);
> + }
> }
>
> return 0;
> @@ -316,6 +370,17 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
> bitmap_set(msi_data->used, 0, msi_data->irqs_num);
>
> msi_data->msir_num = of_irq_count(pdev->dev.of_node);
> +
> + if (msi_affinity_flag) {
> + u32 cpu_num;
> +
> + cpu_num = num_possible_cpus();
> + if (msi_data->msir_num >= cpu_num)
> + msi_data->msir_num = cpu_num;
> + else
> + msi_affinity_flag = 0;
> + }
> +
> msi_data->msir = devm_kcalloc(&pdev->dev, msi_data->msir_num,
> sizeof(*msi_data->msir),
> GFP_KERNEL);
>
This is a very confusing patch. Please get rid of this useless option
and document how you encode the routing in the hwirq.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2,9/9] irqchip/ls-scfg-msi: add MSI affinity support
2017-01-05 15:33 ` Marc Zyngier
@ 2017-01-06 8:23 ` M.H. Lian
0 siblings, 0 replies; 13+ messages in thread
From: M.H. Lian @ 2017-01-06 8:23 UTC (permalink / raw)
To: Marc Zyngier, linux-arm-kernel, linux-kernel, devicetree
Cc: Rob Herring, Jason Cooper, Roy Zang, Mingkai Hu, Stuart Yoder,
Leo Li, Scott Wood
Hi Marc,
Thanks for your review.
Please see my comments inline.
Thanks,
Minghuan
> -----Original Message-----
> From: Marc Zyngier [mailto:marc.zyngier@arm.com]
> Sent: Thursday, January 05, 2017 11:33 PM
> To: M.H. Lian <minghuan.lian@nxp.com>; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>; Jason Cooper
> <jason@lakedaemon.net>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu
> <mingkai.hu@nxp.com>; Stuart Yoder <stuart.yoder@nxp.com>; Leo Li
> <leoyang.li@nxp.com>; Scott Wood <scott.wood@nxp.com>
> Subject: Re: [PATCH v2,9/9] irqchip/ls-scfg-msi: add MSI affinity support
>
> On 05/01/17 08:10, Minghuan Lian wrote:
> > For LS1046a and LS1043a v1.1, the MSI controller has 4 MSIRs and 4
> > CPUs. A GIC SPI interrupt of MSIR can be associated with a CPU.
> > When changing MSI interrupt affinity, this MSI will be moved to the
> > corresponding MSIR and MSI message data will be changed according to
> > MSIR. when requesting a MSI, the bits of all 4 MSIR will be reserved.
> > The parameter 'msi_affinity_flag' is provide to change this mode.
> > "lsmsi=no-affinity" will disable affinity, all MSI can only be
> > associated with CPU 0.
> >
> > Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> > ---
> > v2-v1:
> > - None
> >
> > drivers/irqchip/irq-ls-scfg-msi.c | 75
> > ++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 70 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-ls-scfg-msi.c
> > b/drivers/irqchip/irq-ls-scfg-msi.c
> > index dc19569..753fe39 100644
> > --- a/drivers/irqchip/irq-ls-scfg-msi.c
> > +++ b/drivers/irqchip/irq-ls-scfg-msi.c
> > @@ -40,6 +40,7 @@ struct ls_scfg_msir {
> > unsigned int gic_irq;
> > unsigned int bit_start;
> > unsigned int bit_end;
> > + unsigned int srs; /* Shared interrupt register select */
> > void __iomem *reg;
> > };
> >
> > @@ -70,6 +71,19 @@ struct ls_scfg_msi {
> > .chip = &ls_scfg_msi_irq_chip,
> > };
> >
> > +static int msi_affinity_flag = 1;
> > +
> > +static int __init early_parse_ls_scfg_msi(char *p) {
> > + if (p && strncmp(p, "no-affinity", 11) == 0)
> > + msi_affinity_flag = 0;
> > + else
> > + msi_affinity_flag = 1;
> > +
> > + return 0;
> > +}
> > +early_param("lsmsi", early_parse_ls_scfg_msi);
>
> What is the point of this option? If feels like an unnecessary complexity.
[Minghuan Lian] For LS1043a rev1.1, there are only 8 MSI interrupts for each MSI controller when enable affinity.
(32 MSI interrupts are evenly distributed to 4 cores). 3 MSI controllers can only provide 32 MSI interrupts.
When disable affinity, the MSI interrupts number of 3 controllers can up to 32*3= 96.
32 MSI interrupts may be not enough.
For example: an Ethernet card with 4 ports, each port needs 4 RX, 4TX and 1 error interrupts, totally need 4*(4+4+1)
36 MSI interrupts.
With the parameter "lsmsi=no-affinity" this Ethernet card can work well, although the performance is poor.
>
> > +
> > static void ls_scfg_msi_compose_msg(struct irq_data *data, struct
> > msi_msg *msg) {
> > struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(data);
> @@
> > -77,12 +91,43 @@ static void ls_scfg_msi_compose_msg(struct irq_data
> *data, struct msi_msg *msg)
> > msg->address_hi = upper_32_bits(msi_data->msiir_addr);
> > msg->address_lo = lower_32_bits(msi_data->msiir_addr);
> > msg->data = data->hwirq;
> > +
> > + if (msi_affinity_flag) {
> > + u32 msir_index;
> > +
> > + msir_index = cpumask_first(data->common->affinity);
> > + if (msir_index >= msi_data->msir_num)
> > + msir_index = 0;
>
> How can this happen?
[Minghuan Lian] This will never happen. I will remove this "if" sentence.
>
> > +
> > + msg->data |= msir_index;
>
> How do you guarantee that the low bits were clear? Please document the
> way you encode your MSI payload.
[Minghuan Lian] the available message data is a bytes.
7 6 5 4 3 2 1 0
| - | ibs | srs |
SRS bit 0-1 is used to select MSIR register/CPU. A MSIR is associated with a CPU.
IBS bit2-6 of ls1046, bit2-4 for ls1043 is used to select bit of the MSIR.
When enable affinity, only bits of MSIR0(srs = 0 cpu0) is be freed for requesting MSI.
all bits of the MSIR1-3(cpu1-3) are reserved to be sure this MSI can be successfully associated with cpu 1-3.
So, When requesting a MSI interrupt, srs always is 0.
The hwirq always equals bits number of the MSIR0(SRS = 0).
First, MSI message data payload equals hwirq, and then plus the real SRS according to smp_affinity.
This MSI interrupt will be routed to the corresponding the MSIR/CPU.
>
> > + }
> > }
> >
> > static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
> > const struct cpumask *mask, bool force) {
> > - return -EINVAL;
> > + struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(irq_data);
> > + u32 cpu;
> > +
> > + if (!msi_affinity_flag)
> > + return -EINVAL;
> > +
> > + if (!force)
> > + cpu = cpumask_any_and(mask, cpu_online_mask);
> > + else
> > + cpu = cpumask_first(mask);
> > +
> > + if (cpu >= msi_data->msir_num)
> > + return -EINVAL;
> > +
> > + if (msi_data->msir[cpu].gic_irq <= 0) {
> > + pr_warn("cannot bind the irq to cpu%d\n", cpu);
>
> Please don't. Returning an error is enough. If you really want to have
> something, turn it into a proper debug message.
[Minghuan Lian] ok, I will change it.
>
> > + return -EINVAL;
> > + }
> > +
> > + cpumask_copy(irq_data->common->affinity, mask);
> > +
> > + return IRQ_SET_MASK_OK;
> > }
> >
> > static struct irq_chip ls_scfg_msi_parent_chip = { @@ -158,7 +203,7
> > @@ static void ls_scfg_msi_irq_handler(struct irq_desc *desc)
> >
> > for_each_set_bit_from(pos, &val, size) {
> > hwirq = ((msir->bit_end - pos) << msi_data->cfg->ibs_shift) |
> > - msir->index;
> > + msir->srs;
> > virq = irq_find_mapping(msi_data->parent, hwirq);
> > if (virq)
> > generic_handle_irq(virq);
> > @@ -221,10 +266,19 @@ static int ls_scfg_msi_setup_hwirq(struct
> ls_scfg_msi *msi_data, int index)
> > ls_scfg_msi_irq_handler,
> > msir);
> >
> > + if (msi_affinity_flag) {
> > + /* Associate MSIR interrupt to the cpu */
> > + irq_set_affinity(msir->gic_irq, get_cpu_mask(index));
> > + msir->srs = 0; /* This value is determined by the CPU */
> > + } else
> > + msir->srs = index;
> > +
> > /* Release the hwirqs corresponding to this MSIR */
> > - for (i = 0; i < msi_data->cfg->msir_irqs; i++) {
> > - hwirq = i << msi_data->cfg->ibs_shift | msir->index;
> > - bitmap_clear(msi_data->used, hwirq, 1);
> > + if (!msi_affinity_flag || msir->index == 0) {
> > + for (i = 0; i < msi_data->cfg->msir_irqs; i++) {
> > + hwirq = i << msi_data->cfg->ibs_shift | msir->index;
> > + bitmap_clear(msi_data->used, hwirq, 1);
> > + }
> > }
> >
> > return 0;
> > @@ -316,6 +370,17 @@ static int ls_scfg_msi_probe(struct
> platform_device *pdev)
> > bitmap_set(msi_data->used, 0, msi_data->irqs_num);
> >
> > msi_data->msir_num = of_irq_count(pdev->dev.of_node);
> > +
> > + if (msi_affinity_flag) {
> > + u32 cpu_num;
> > +
> > + cpu_num = num_possible_cpus();
> > + if (msi_data->msir_num >= cpu_num)
> > + msi_data->msir_num = cpu_num;
> > + else
> > + msi_affinity_flag = 0;
> > + }
> > +
> > msi_data->msir = devm_kcalloc(&pdev->dev, msi_data->msir_num,
> > sizeof(*msi_data->msir),
> > GFP_KERNEL);
> >
>
> This is a very confusing patch. Please get rid of this useless option and
> document how you encode the routing in the hwirq.
[Minghuan Lian] Both LS1021a and LS1043av1.0 have only a MSIR, a gic interrupt.
MSI controllers cannot support affinity.
Then LS1046a/LS1043av1.1 extends MSIR number to 4 same to cpu number. So each MSIR with a GIC interrupt can be associated with a cpu.
To keep simple, the first patch for ls1046 and ls1043av1.1 keep the same way with ls1021 and ls1043av1.0 that does not support affinity and
all interrupts of MSIR0-3 are different and can be used for requesting MSI interrupts.
This patch is to enable affinity, that means, for ls1046a and ls1043av1.1, the same bit of MSIR0-3 will be looked as one interrupt using the same hwirq. And MSIRN only is used when the MSI interrupt is associated with the corresponding cpu.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 13+ messages in thread