* [PATCH v3 1/9] PCI: keystone: Cleanup interrupt related macros
2019-02-13 13:26 [PATCH v3 0/9] PCI: DWC/Keystone: MSI configuration cleanup Kishon Vijay Abraham I
@ 2019-02-13 13:26 ` Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 2/9] PCI: keystone: Add separate functions for configuring MSI and legacy interrupt Kishon Vijay Abraham I
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-13 13:26 UTC (permalink / raw)
To: Murali Karicheri, Lorenzo Pieralisi
Cc: Jingoo Han, linux-pci, linux-kernel, Kishon Vijay Abraham I,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
No functional change. Change both MSI interrupt and legacy interrupt
related macros to take an additional argument in order to return the
correct register offset.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/pci/controller/dwc/pci-keystone.c | 26 +++++++++++------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 14f2b0b4ed5e..5286a480f76b 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -52,17 +52,17 @@
/* IRQ register defines */
#define IRQ_EOI 0x050
-#define IRQ_STATUS 0x184
-#define IRQ_ENABLE_SET 0x188
-#define IRQ_ENABLE_CLR 0x18c
#define MSI_IRQ 0x054
-#define MSI0_IRQ_STATUS 0x104
-#define MSI0_IRQ_ENABLE_SET 0x108
-#define MSI0_IRQ_ENABLE_CLR 0x10c
-#define IRQ_STATUS 0x184
+#define MSI_IRQ_STATUS(n) (0x104 + ((n) << 4))
+#define MSI_IRQ_ENABLE_SET(n) (0x108 + ((n) << 4))
+#define MSI_IRQ_ENABLE_CLR(n) (0x10c + ((n) << 4))
#define MSI_IRQ_OFFSET 4
+#define IRQ_STATUS(n) (0x184 + ((n) << 4))
+#define IRQ_ENABLE_SET(n) (0x188 + ((n) << 4))
+#define INTx_EN BIT(0)
+
#define ERR_IRQ_STATUS 0x1c4
#define ERR_IRQ_ENABLE_SET 0x1c8
#define ERR_AER BIT(5) /* ECRC error */
@@ -142,7 +142,7 @@ static void ks_pcie_handle_msi_irq(struct keystone_pcie *ks_pcie, int offset)
u32 pending, vector;
int src, virq;
- pending = ks_pcie_app_readl(ks_pcie, MSI0_IRQ_STATUS + (offset << 4));
+ pending = ks_pcie_app_readl(ks_pcie, MSI_IRQ_STATUS(offset));
/*
* MSI0 status bit 0-3 shows vectors 0, 8, 16, 24, MSI1 status bit
@@ -169,7 +169,7 @@ static void ks_pcie_msi_irq_ack(int irq, struct pcie_port *pp)
ks_pcie = to_keystone_pcie(pci);
update_reg_offset_bit_pos(irq, ®_offset, &bit_pos);
- ks_pcie_app_writel(ks_pcie, MSI0_IRQ_STATUS + (reg_offset << 4),
+ ks_pcie_app_writel(ks_pcie, MSI_IRQ_STATUS(reg_offset),
BIT(bit_pos));
ks_pcie_app_writel(ks_pcie, IRQ_EOI, reg_offset + MSI_IRQ_OFFSET);
}
@@ -181,7 +181,7 @@ static void ks_pcie_msi_set_irq(struct pcie_port *pp, int irq)
struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
update_reg_offset_bit_pos(irq, ®_offset, &bit_pos);
- ks_pcie_app_writel(ks_pcie, MSI0_IRQ_ENABLE_SET + (reg_offset << 4),
+ ks_pcie_app_writel(ks_pcie, MSI_IRQ_ENABLE_SET(reg_offset),
BIT(bit_pos));
}
@@ -192,7 +192,7 @@ static void ks_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
update_reg_offset_bit_pos(irq, ®_offset, &bit_pos);
- ks_pcie_app_writel(ks_pcie, MSI0_IRQ_ENABLE_CLR + (reg_offset << 4),
+ ks_pcie_app_writel(ks_pcie, MSI_IRQ_ENABLE_CLR(reg_offset),
BIT(bit_pos));
}
@@ -206,7 +206,7 @@ static void ks_pcie_enable_legacy_irqs(struct keystone_pcie *ks_pcie)
int i;
for (i = 0; i < PCI_NUM_INTX; i++)
- ks_pcie_app_writel(ks_pcie, IRQ_ENABLE_SET + (i << 4), 0x1);
+ ks_pcie_app_writel(ks_pcie, IRQ_ENABLE_SET(i), 0x1);
}
static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
@@ -217,7 +217,7 @@ static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
u32 pending;
int virq;
- pending = ks_pcie_app_readl(ks_pcie, IRQ_STATUS + (offset << 4));
+ pending = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(offset));
if (BIT(0) & pending) {
virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 2/9] PCI: keystone: Add separate functions for configuring MSI and legacy interrupt
2019-02-13 13:26 [PATCH v3 0/9] PCI: DWC/Keystone: MSI configuration cleanup Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 1/9] PCI: keystone: Cleanup interrupt related macros Kishon Vijay Abraham I
@ 2019-02-13 13:26 ` Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset Kishon Vijay Abraham I
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-13 13:26 UTC (permalink / raw)
To: Murali Karicheri, Lorenzo Pieralisi
Cc: Jingoo Han, linux-pci, linux-kernel, Kishon Vijay Abraham I,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
ks_pcie_get_irq_controller_info() was used to configure both MSI and
legacy interrupt. This will prevent MSI or legacy interrupt specific
intializations. Add separate functions to configure MSI and legacy
interrupts.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/pci/controller/dwc/pci-keystone.c | 185 +++++++++++-----------
1 file changed, 93 insertions(+), 92 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 5286a480f76b..e8b1d8eca78e 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -87,11 +87,9 @@ struct keystone_pcie {
struct dw_pcie *pci;
/* PCI Device ID */
u32 device_id;
- int num_legacy_host_irqs;
int legacy_host_irqs[PCI_NUM_INTX];
struct device_node *legacy_intc_np;
- int num_msi_host_irqs;
int msi_host_irqs[MAX_MSI_HOST_IRQS];
int num_lanes;
u32 num_viewport;
@@ -201,14 +199,6 @@ static int ks_pcie_msi_host_init(struct pcie_port *pp)
return dw_pcie_allocate_domains(pp);
}
-static void ks_pcie_enable_legacy_irqs(struct keystone_pcie *ks_pcie)
-{
- int i;
-
- for (i = 0; i < PCI_NUM_INTX; i++)
- ks_pcie_app_writel(ks_pcie, IRQ_ENABLE_SET(i), 0x1);
-}
-
static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
int offset)
{
@@ -495,17 +485,6 @@ static int __init ks_pcie_dw_host_init(struct keystone_pcie *ks_pcie)
ks_pcie->app = *res;
- /* Create legacy IRQ domain */
- ks_pcie->legacy_irq_domain =
- irq_domain_add_linear(ks_pcie->legacy_intc_np,
- PCI_NUM_INTX,
- &ks_pcie_legacy_irq_domain_ops,
- NULL);
- if (!ks_pcie->legacy_irq_domain) {
- dev_err(dev, "Failed to add irq domain for legacy irqs\n");
- return -EINVAL;
- }
-
return dw_pcie_host_init(pp);
}
@@ -622,85 +601,113 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}
-static int ks_pcie_get_irq_controller_info(struct keystone_pcie *ks_pcie,
- char *controller, int *num_irqs)
+static int ks_pcie_config_msi_irq(struct keystone_pcie *ks_pcie)
{
- int temp, max_host_irqs, legacy = 1, *host_irqs;
struct device *dev = ks_pcie->pci->dev;
- struct device_node *np_pcie = dev->of_node, **np_temp;
-
- if (!strcmp(controller, "msi-interrupt-controller"))
- legacy = 0;
-
- if (legacy) {
- np_temp = &ks_pcie->legacy_intc_np;
- max_host_irqs = PCI_NUM_INTX;
- host_irqs = &ks_pcie->legacy_host_irqs[0];
- } else {
- np_temp = &ks_pcie->msi_intc_np;
- max_host_irqs = MAX_MSI_HOST_IRQS;
- host_irqs = &ks_pcie->msi_host_irqs[0];
- }
+ struct device_node *np = ks_pcie->np;
+ struct device_node *intc_np;
+ int irq_count, irq, ret, i;
- /* interrupt controller is in a child node */
- *np_temp = of_get_child_by_name(np_pcie, controller);
- if (!(*np_temp)) {
- dev_err(dev, "Node for %s is absent\n", controller);
- return -EINVAL;
- }
+ if (!IS_ENABLED(CONFIG_PCI_MSI))
+ return 0;
- temp = of_irq_count(*np_temp);
- if (!temp) {
- dev_err(dev, "No IRQ entries in %s\n", controller);
- of_node_put(*np_temp);
+ intc_np = of_get_child_by_name(np, "msi-interrupt-controller");
+ if (!intc_np) {
+ dev_warn(dev, "msi-interrupt-controller node is absent\n");
return -EINVAL;
}
- if (temp > max_host_irqs)
- dev_warn(dev, "Too many %s interrupts defined %u\n",
- (legacy ? "legacy" : "MSI"), temp);
+ irq_count = of_irq_count(intc_np);
+ if (!irq_count) {
+ dev_err(dev, "No IRQ entries in msi-interrupt-controller\n");
+ ret = -EINVAL;
+ goto err;
+ }
- /*
- * support upto max_host_irqs. In dt from index 0 to 3 (legacy) or 0 to
- * 7 (MSI)
- */
- for (temp = 0; temp < max_host_irqs; temp++) {
- host_irqs[temp] = irq_of_parse_and_map(*np_temp, temp);
- if (!host_irqs[temp])
- break;
+ if (irq_count > MAX_MSI_HOST_IRQS) {
+ dev_warn(dev, "Too many MSI interrupt lines defined %u\n",
+ irq_count);
+ irq_count = MAX_MSI_HOST_IRQS;
}
- of_node_put(*np_temp);
+ for (i = 0; i < irq_count; i++) {
+ irq = irq_of_parse_and_map(intc_np, i);
+ if (!irq) {
+ ret = -EINVAL;
+ goto err;
+ }
+ ks_pcie->msi_host_irqs[i] = irq;
- if (temp) {
- *num_irqs = temp;
- return 0;
+ irq_set_chained_handler_and_data(irq, ks_pcie_msi_irq_handler,
+ ks_pcie);
}
- return -EINVAL;
+ of_node_put(intc_np);
+ return 0;
+
+err:
+ of_node_put(intc_np);
+ return ret;
}
-static void ks_pcie_setup_interrupts(struct keystone_pcie *ks_pcie)
+static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
{
- int i;
+ struct device *dev = ks_pcie->pci->dev;
+ struct irq_domain *legacy_irq_domain;
+ struct device_node *np = ks_pcie->np;
+ struct device_node *intc_np;
+ int irq_count, irq, ret, i;
+
+ intc_np = of_get_child_by_name(np, "legacy-interrupt-controller");
+ if (!intc_np) {
+ dev_warn(dev, "legacy-interrupt-controller node is absent\n");
+ return -EINVAL;
+ }
- /* Legacy IRQ */
- for (i = 0; i < ks_pcie->num_legacy_host_irqs; i++) {
- irq_set_chained_handler_and_data(ks_pcie->legacy_host_irqs[i],
+ irq_count = of_irq_count(intc_np);
+ if (!irq_count) {
+ dev_err(dev, "No IRQ entries in legacy-interrupt-controller\n");
+ ret = -EINVAL;
+ goto err;
+ }
+
+ for (i = 0; i < irq_count; i++) {
+ irq = irq_of_parse_and_map(intc_np, i);
+ if (!irq) {
+ ret = -EINVAL;
+ goto err;
+ }
+ ks_pcie->legacy_host_irqs[i] = irq;
+
+ irq_set_chained_handler_and_data(irq,
ks_pcie_legacy_irq_handler,
ks_pcie);
}
- ks_pcie_enable_legacy_irqs(ks_pcie);
-
- /* MSI IRQ */
- if (IS_ENABLED(CONFIG_PCI_MSI)) {
- for (i = 0; i < ks_pcie->num_msi_host_irqs; i++) {
- irq_set_chained_handler_and_data(ks_pcie->msi_host_irqs[i],
- ks_pcie_msi_irq_handler,
- ks_pcie);
- }
+
+ legacy_irq_domain =
+ irq_domain_add_linear(intc_np, PCI_NUM_INTX,
+ &ks_pcie_legacy_irq_domain_ops, NULL);
+ if (!legacy_irq_domain) {
+ dev_err(dev, "Failed to add irq domain for legacy irqs\n");
+ ret = -EINVAL;
+ goto err;
}
+ ks_pcie->legacy_irq_domain = legacy_irq_domain;
+
+ for (i = 0; i < PCI_NUM_INTX; i++)
+ ks_pcie_app_writel(ks_pcie, IRQ_ENABLE_SET(i), INTx_EN);
+
+ of_node_put(intc_np);
+
+ return 0;
+
+err:
+ of_node_put(intc_np);
+ return ret;
+}
+static void ks_pcie_setup_interrupts(struct keystone_pcie *ks_pcie)
+{
if (ks_pcie->error_irq > 0)
ks_pcie_enable_error_irq(ks_pcie);
}
@@ -754,6 +761,14 @@ static int __init ks_pcie_host_init(struct pcie_port *pp)
struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
int ret;
+ ret = ks_pcie_config_legacy_irq(ks_pcie);
+ if (ret)
+ return ret;
+
+ ret = ks_pcie_config_msi_irq(ks_pcie);
+ if (ret)
+ return ret;
+
dw_pcie_setup_rc(pp);
ks_pcie_establish_link(ks_pcie);
@@ -803,20 +818,6 @@ static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,
struct device *dev = &pdev->dev;
int ret;
- ret = ks_pcie_get_irq_controller_info(ks_pcie,
- "legacy-interrupt-controller",
- &ks_pcie->num_legacy_host_irqs);
- if (ret)
- return ret;
-
- if (IS_ENABLED(CONFIG_PCI_MSI)) {
- ret = ks_pcie_get_irq_controller_info(ks_pcie,
- "msi-interrupt-controller",
- &ks_pcie->num_msi_host_irqs);
- if (ret)
- return ret;
- }
-
/*
* Index 0 is the platform interrupt for error interrupt
* from RC. This is optional.
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset
2019-02-13 13:26 [PATCH v3 0/9] PCI: DWC/Keystone: MSI configuration cleanup Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 1/9] PCI: keystone: Cleanup interrupt related macros Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 2/9] PCI: keystone: Add separate functions for configuring MSI and legacy interrupt Kishon Vijay Abraham I
@ 2019-02-13 13:26 ` Kishon Vijay Abraham I
2019-02-13 16:57 ` Lorenzo Pieralisi
2019-02-13 13:26 ` [PATCH v3 4/9] PCI: keystone: Use hwirq to get the MSI " Kishon Vijay Abraham I
` (5 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-13 13:26 UTC (permalink / raw)
To: Murali Karicheri, Lorenzo Pieralisi
Cc: Jingoo Han, linux-pci, linux-kernel, Kishon Vijay Abraham I,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
ks_pcie_legacy_irq_handler() uses 'virq' to get the IRQ number offset.
This offset is used to get the correct IRQ_STATUS register
corresponding to the IRQ line that raised the interrupt.
There is no guarantee that 'virq' assigned for consecutive hardware
IRQ will be contiguous. And this might get us an incorrect IRQ number
offset.
Fix it here by using 'hwirq' to get the IRQ number offset.
Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/pci/controller/dwc/pci-keystone.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index e8b1d8eca78e..d35ac712a9f8 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -87,7 +87,7 @@ struct keystone_pcie {
struct dw_pcie *pci;
/* PCI Device ID */
u32 device_id;
- int legacy_host_irqs[PCI_NUM_INTX];
+ int legacy_host_irq;
struct device_node *legacy_intc_np;
int msi_host_irqs[MAX_MSI_HOST_IRQS];
@@ -582,11 +582,11 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
*/
static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
{
- unsigned int irq = irq_desc_get_irq(desc);
+ unsigned int irq = desc->irq_data.hwirq;
struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
struct dw_pcie *pci = ks_pcie->pci;
struct device *dev = pci->dev;
- u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
+ u32 irq_offset = irq - ks_pcie->legacy_host_irq;
struct irq_chip *chip = irq_desc_get_chip(desc);
dev_dbg(dev, ": Handling legacy irq %d\n", irq);
@@ -657,6 +657,7 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
struct device_node *np = ks_pcie->np;
struct device_node *intc_np;
int irq_count, irq, ret, i;
+ struct irq_data *irq_data;
intc_np = of_get_child_by_name(np, "legacy-interrupt-controller");
if (!intc_np) {
@@ -677,7 +678,15 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
ret = -EINVAL;
goto err;
}
- ks_pcie->legacy_host_irqs[i] = irq;
+
+ if (!ks_pcie->legacy_host_irq) {
+ irq_data = irq_get_irq_data(irq);
+ if (!irq_data) {
+ ret = -EINVAL;
+ goto err;
+ }
+ ks_pcie->legacy_host_irq = irq_data->hwirq;
+ }
irq_set_chained_handler_and_data(irq,
ks_pcie_legacy_irq_handler,
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset
2019-02-13 13:26 ` [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset Kishon Vijay Abraham I
@ 2019-02-13 16:57 ` Lorenzo Pieralisi
2019-02-14 4:57 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 15+ messages in thread
From: Lorenzo Pieralisi @ 2019-02-13 16:57 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Jingoo Han, linux-kernel, Murali Karicheri, linux-pci,
Bjorn Helgaas, Gustavo Pimentel, linux-arm-kernel
On Wed, Feb 13, 2019 at 06:56:23PM +0530, Kishon Vijay Abraham I wrote:
> ks_pcie_legacy_irq_handler() uses 'virq' to get the IRQ number offset.
> This offset is used to get the correct IRQ_STATUS register
> corresponding to the IRQ line that raised the interrupt.
> There is no guarantee that 'virq' assigned for consecutive hardware
> IRQ will be contiguous. And this might get us an incorrect IRQ number
> offset.
>
> Fix it here by using 'hwirq' to get the IRQ number offset.
>
> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/controller/dwc/pci-keystone.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index e8b1d8eca78e..d35ac712a9f8 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -87,7 +87,7 @@ struct keystone_pcie {
> struct dw_pcie *pci;
> /* PCI Device ID */
> u32 device_id;
> - int legacy_host_irqs[PCI_NUM_INTX];
> + int legacy_host_irq;
> struct device_node *legacy_intc_np;
>
> int msi_host_irqs[MAX_MSI_HOST_IRQS];
> @@ -582,11 +582,11 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
> */
> static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
> {
> - unsigned int irq = irq_desc_get_irq(desc);
> + unsigned int irq = desc->irq_data.hwirq;
> struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
> struct dw_pcie *pci = ks_pcie->pci;
> struct device *dev = pci->dev;
> - u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
> + u32 irq_offset = irq - ks_pcie->legacy_host_irq;
I think you should use the plain hwirq number (that if I understand
correctly range in [0,3]) and drop legacy_host_irq.
See below.
> struct irq_chip *chip = irq_desc_get_chip(desc);
>
> dev_dbg(dev, ": Handling legacy irq %d\n", irq);
> @@ -657,6 +657,7 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
> struct device_node *np = ks_pcie->np;
> struct device_node *intc_np;
> int irq_count, irq, ret, i;
> + struct irq_data *irq_data;
>
> intc_np = of_get_child_by_name(np, "legacy-interrupt-controller");
> if (!intc_np) {
> @@ -677,7 +678,15 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
> ret = -EINVAL;
> goto err;
> }
> - ks_pcie->legacy_host_irqs[i] = irq;
> +
> + if (!ks_pcie->legacy_host_irq) {
> + irq_data = irq_get_irq_data(irq);
> + if (!irq_data) {
> + ret = -EINVAL;
> + goto err;
> + }
> + ks_pcie->legacy_host_irq = irq_data->hwirq;
This depends on DT property ordering and may well be wrong. On top
of that as I said above I do not think it is even needed.
Thanks,
Lorenzo
> + }
>
> irq_set_chained_handler_and_data(irq,
> ks_pcie_legacy_irq_handler,
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset
2019-02-13 16:57 ` Lorenzo Pieralisi
@ 2019-02-14 4:57 ` Kishon Vijay Abraham I
2019-02-14 10:31 ` Lorenzo Pieralisi
0 siblings, 1 reply; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-14 4:57 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Jingoo Han, linux-kernel, Murali Karicheri, linux-pci,
Bjorn Helgaas, Gustavo Pimentel, linux-arm-kernel
Hi Lorenzo,
On 13/02/19 10:27 PM, Lorenzo Pieralisi wrote:
> On Wed, Feb 13, 2019 at 06:56:23PM +0530, Kishon Vijay Abraham I wrote:
>> ks_pcie_legacy_irq_handler() uses 'virq' to get the IRQ number offset.
>> This offset is used to get the correct IRQ_STATUS register
>> corresponding to the IRQ line that raised the interrupt.
>> There is no guarantee that 'virq' assigned for consecutive hardware
>> IRQ will be contiguous. And this might get us an incorrect IRQ number
>> offset.
>>
>> Fix it here by using 'hwirq' to get the IRQ number offset.
>>
>> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> drivers/pci/controller/dwc/pci-keystone.c | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
>> index e8b1d8eca78e..d35ac712a9f8 100644
>> --- a/drivers/pci/controller/dwc/pci-keystone.c
>> +++ b/drivers/pci/controller/dwc/pci-keystone.c
>> @@ -87,7 +87,7 @@ struct keystone_pcie {
>> struct dw_pcie *pci;
>> /* PCI Device ID */
>> u32 device_id;
>> - int legacy_host_irqs[PCI_NUM_INTX];
>> + int legacy_host_irq;
>> struct device_node *legacy_intc_np;
>>
>> int msi_host_irqs[MAX_MSI_HOST_IRQS];
>> @@ -582,11 +582,11 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
>> */
>> static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>> {
>> - unsigned int irq = irq_desc_get_irq(desc);
>> + unsigned int irq = desc->irq_data.hwirq;
>> struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
>> struct dw_pcie *pci = ks_pcie->pci;
>> struct device *dev = pci->dev;
>> - u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
>> + u32 irq_offset = irq - ks_pcie->legacy_host_irq;
>
> I think you should use the plain hwirq number (that if I understand
> correctly range in [0,3]) and drop legacy_host_irq.
The hwirq is [80, 83] for Keystone. We store legacy_host_irq (for Keystone it
is 80) to get the correct offset in the range [0, 3].
>
> See below.
>
>> struct irq_chip *chip = irq_desc_get_chip(desc);
>>
>> dev_dbg(dev, ": Handling legacy irq %d\n", irq);
>> @@ -657,6 +657,7 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
>> struct device_node *np = ks_pcie->np;
>> struct device_node *intc_np;
>> int irq_count, irq, ret, i;
>> + struct irq_data *irq_data;
>>
>> intc_np = of_get_child_by_name(np, "legacy-interrupt-controller");
>> if (!intc_np) {
>> @@ -677,7 +678,15 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
>> ret = -EINVAL;
>> goto err;
>> }
>> - ks_pcie->legacy_host_irqs[i] = irq;
>> +
>> + if (!ks_pcie->legacy_host_irq) {
>> + irq_data = irq_get_irq_data(irq);
>> + if (!irq_data) {
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> + ks_pcie->legacy_host_irq = irq_data->hwirq;
>
> This depends on DT property ordering and may well be wrong. On top
> of that as I said above I do not think it is even needed.
right, it depends on the ordering of the entries in the "interrupts" property.
Right now we have something like below
interrupts = <GIC_SPI 48 IRQ_TYPE_EDGE_RISING>,
<GIC_SPI 49 IRQ_TYPE_EDGE_RISING>,
<GIC_SPI 50 IRQ_TYPE_EDGE_RISING>,
<GIC_SPI 51 IRQ_TYPE_EDGE_RISING>;
And changing the order will make legacy_host_irq store the wrong hwirq. With
this patch we solve the problem of assuming virq to be contiguous for
contiguous hwirq however we do expect the hwirqs to be populated in the DT in
the correct order.
Ordering entries within a property is also followed in other properties like
"reg" where the drivers use platform_get_resource with index as argument. I'm
not sure if there's a way to get rid of the ordering requirement in DT for the
"interrupts" property.
Thanks
Kishon
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset
2019-02-14 4:57 ` Kishon Vijay Abraham I
@ 2019-02-14 10:31 ` Lorenzo Pieralisi
2019-02-14 12:46 ` Marc Zyngier
2019-02-21 9:18 ` Kishon Vijay Abraham I
0 siblings, 2 replies; 15+ messages in thread
From: Lorenzo Pieralisi @ 2019-02-14 10:31 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: marc.zyngier, Jingoo Han, linux-kernel, Murali Karicheri,
linux-pci, Bjorn Helgaas, Gustavo Pimentel, linux-arm-kernel
[CC'ed MarcZ]
On Thu, Feb 14, 2019 at 10:27:19AM +0530, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
>
> On 13/02/19 10:27 PM, Lorenzo Pieralisi wrote:
> > On Wed, Feb 13, 2019 at 06:56:23PM +0530, Kishon Vijay Abraham I wrote:
> >> ks_pcie_legacy_irq_handler() uses 'virq' to get the IRQ number offset.
> >> This offset is used to get the correct IRQ_STATUS register
> >> corresponding to the IRQ line that raised the interrupt.
> >> There is no guarantee that 'virq' assigned for consecutive hardware
> >> IRQ will be contiguous. And this might get us an incorrect IRQ number
> >> offset.
> >>
> >> Fix it here by using 'hwirq' to get the IRQ number offset.
> >>
> >> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> ---
> >> drivers/pci/controller/dwc/pci-keystone.c | 17 +++++++++++++----
> >> 1 file changed, 13 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> >> index e8b1d8eca78e..d35ac712a9f8 100644
> >> --- a/drivers/pci/controller/dwc/pci-keystone.c
> >> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> >> @@ -87,7 +87,7 @@ struct keystone_pcie {
> >> struct dw_pcie *pci;
> >> /* PCI Device ID */
> >> u32 device_id;
> >> - int legacy_host_irqs[PCI_NUM_INTX];
> >> + int legacy_host_irq;
> >> struct device_node *legacy_intc_np;
> >>
> >> int msi_host_irqs[MAX_MSI_HOST_IRQS];
> >> @@ -582,11 +582,11 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
> >> */
> >> static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
> >> {
> >> - unsigned int irq = irq_desc_get_irq(desc);
> >> + unsigned int irq = desc->irq_data.hwirq;
> >> struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
> >> struct dw_pcie *pci = ks_pcie->pci;
> >> struct device *dev = pci->dev;
> >> - u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
> >> + u32 irq_offset = irq - ks_pcie->legacy_host_irq;
> >
> > I think you should use the plain hwirq number (that if I understand
> > correctly range in [0,3]) and drop legacy_host_irq.
>
> The hwirq is [80, 83] for Keystone. We store legacy_host_irq (for Keystone it
> is 80) to get the correct offset in the range [0, 3].
IIUC the _parent_ hw_irq number should be what you need here, Marc (if
he has time) can correct me if I am wrong (or what I am suggesting is
a violation of IRQ domain bstraction usage).
> >
> > See below.
> >
> >> struct irq_chip *chip = irq_desc_get_chip(desc);
> >>
> >> dev_dbg(dev, ": Handling legacy irq %d\n", irq);
> >> @@ -657,6 +657,7 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
> >> struct device_node *np = ks_pcie->np;
> >> struct device_node *intc_np;
> >> int irq_count, irq, ret, i;
> >> + struct irq_data *irq_data;
> >>
> >> intc_np = of_get_child_by_name(np, "legacy-interrupt-controller");
> >> if (!intc_np) {
> >> @@ -677,7 +678,15 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
> >> ret = -EINVAL;
> >> goto err;
> >> }
> >> - ks_pcie->legacy_host_irqs[i] = irq;
> >> +
> >> + if (!ks_pcie->legacy_host_irq) {
> >> + irq_data = irq_get_irq_data(irq);
> >> + if (!irq_data) {
> >> + ret = -EINVAL;
> >> + goto err;
> >> + }
> >> + ks_pcie->legacy_host_irq = irq_data->hwirq;
> >
> > This depends on DT property ordering and may well be wrong. On top
> > of that as I said above I do not think it is even needed.
>
> right, it depends on the ordering of the entries in the "interrupts" property.
> Right now we have something like below
>
> interrupts = <GIC_SPI 48 IRQ_TYPE_EDGE_RISING>,
> <GIC_SPI 49 IRQ_TYPE_EDGE_RISING>,
> <GIC_SPI 50 IRQ_TYPE_EDGE_RISING>,
> <GIC_SPI 51 IRQ_TYPE_EDGE_RISING>;
>
> And changing the order will make legacy_host_irq store the wrong hwirq. With
> this patch we solve the problem of assuming virq to be contiguous for
> contiguous hwirq however we do expect the hwirqs to be populated in the DT in
> the correct order.
>
> Ordering entries within a property is also followed in other
> properties like "reg" where the drivers use platform_get_resource with
> index as argument. I'm not sure if there's a way to get rid of the
> ordering requirement in DT for the "interrupts" property.
I am not sure there is an ordering requirement in place but hopefully we
should not need it, see above.
The 1:1 relationship between GIC-IRQ<->PCI-host-bridge-controller-IRQ is
what the kernel IRQ domain abstraction is there to implement, so a
reverse look-up should be feasible without adding anything else, lest
we duplicate IRQ domain data structures.
Thanks,
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset
2019-02-14 10:31 ` Lorenzo Pieralisi
@ 2019-02-14 12:46 ` Marc Zyngier
2019-02-21 9:18 ` Kishon Vijay Abraham I
1 sibling, 0 replies; 15+ messages in thread
From: Marc Zyngier @ 2019-02-14 12:46 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Jingoo Han, linux-kernel, Kishon Vijay Abraham I,
Murali Karicheri, linux-pci, Bjorn Helgaas, Gustavo Pimentel,
linux-arm-kernel
Hi Lirenzo,
On Thu, 14 Feb 2019 10:31:38 +0000,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
>
> [CC'ed MarcZ]
>
> On Thu, Feb 14, 2019 at 10:27:19AM +0530, Kishon Vijay Abraham I wrote:
> > Hi Lorenzo,
> >
> > On 13/02/19 10:27 PM, Lorenzo Pieralisi wrote:
> > > On Wed, Feb 13, 2019 at 06:56:23PM +0530, Kishon Vijay Abraham I wrote:
> > >> ks_pcie_legacy_irq_handler() uses 'virq' to get the IRQ number offset.
> > >> This offset is used to get the correct IRQ_STATUS register
> > >> corresponding to the IRQ line that raised the interrupt.
> > >> There is no guarantee that 'virq' assigned for consecutive hardware
> > >> IRQ will be contiguous. And this might get us an incorrect IRQ number
> > >> offset.
> > >>
> > >> Fix it here by using 'hwirq' to get the IRQ number offset.
> > >>
> > >> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
> > >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> > >> ---
> > >> drivers/pci/controller/dwc/pci-keystone.c | 17 +++++++++++++----
> > >> 1 file changed, 13 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> > >> index e8b1d8eca78e..d35ac712a9f8 100644
> > >> --- a/drivers/pci/controller/dwc/pci-keystone.c
> > >> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> > >> @@ -87,7 +87,7 @@ struct keystone_pcie {
> > >> struct dw_pcie *pci;
> > >> /* PCI Device ID */
> > >> u32 device_id;
> > >> - int legacy_host_irqs[PCI_NUM_INTX];
> > >> + int legacy_host_irq;
> > >> struct device_node *legacy_intc_np;
> > >>
> > >> int msi_host_irqs[MAX_MSI_HOST_IRQS];
> > >> @@ -582,11 +582,11 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
> > >> */
> > >> static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
> > >> {
> > >> - unsigned int irq = irq_desc_get_irq(desc);
> > >> + unsigned int irq = desc->irq_data.hwirq;
> > >> struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
> > >> struct dw_pcie *pci = ks_pcie->pci;
> > >> struct device *dev = pci->dev;
> > >> - u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
> > >> + u32 irq_offset = irq - ks_pcie->legacy_host_irq;
> > >
> > > I think you should use the plain hwirq number (that if I understand
> > > correctly range in [0,3]) and drop legacy_host_irq.
> >
> > The hwirq is [80, 83] for Keystone. We store legacy_host_irq (for Keystone it
> > is 80) to get the correct offset in the range [0, 3].
>
> IIUC the _parent_ hw_irq number should be what you need here, Marc (if
> he has time) can correct me if I am wrong (or what I am suggesting is
> a violation of IRQ domain bstraction usage).
It isn't pretty, but it is something we already do in some cases (see
irq-gic-v3-mbi.c::mbi_compose_msi_msg as an infamous example).
Thanks,
M.
--
Jazz is not dead, it just smell funny.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset
2019-02-14 10:31 ` Lorenzo Pieralisi
2019-02-14 12:46 ` Marc Zyngier
@ 2019-02-21 9:18 ` Kishon Vijay Abraham I
1 sibling, 0 replies; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-21 9:18 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: marc.zyngier, Jingoo Han, linux-kernel, Murali Karicheri,
linux-pci, Bjorn Helgaas, Gustavo Pimentel, linux-arm-kernel
Hi Lorenzo,
On 14/02/19 4:01 PM, Lorenzo Pieralisi wrote:
> [CC'ed MarcZ]
>
> On Thu, Feb 14, 2019 at 10:27:19AM +0530, Kishon Vijay Abraham I wrote:
>> Hi Lorenzo,
>>
>> On 13/02/19 10:27 PM, Lorenzo Pieralisi wrote:
>>> On Wed, Feb 13, 2019 at 06:56:23PM +0530, Kishon Vijay Abraham I wrote:
>>>> ks_pcie_legacy_irq_handler() uses 'virq' to get the IRQ number offset.
>>>> This offset is used to get the correct IRQ_STATUS register
>>>> corresponding to the IRQ line that raised the interrupt.
>>>> There is no guarantee that 'virq' assigned for consecutive hardware
>>>> IRQ will be contiguous. And this might get us an incorrect IRQ number
>>>> offset.
>>>>
>>>> Fix it here by using 'hwirq' to get the IRQ number offset.
>>>>
>>>> Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>> drivers/pci/controller/dwc/pci-keystone.c | 17 +++++++++++++----
>>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
>>>> index e8b1d8eca78e..d35ac712a9f8 100644
>>>> --- a/drivers/pci/controller/dwc/pci-keystone.c
>>>> +++ b/drivers/pci/controller/dwc/pci-keystone.c
>>>> @@ -87,7 +87,7 @@ struct keystone_pcie {
>>>> struct dw_pcie *pci;
>>>> /* PCI Device ID */
>>>> u32 device_id;
>>>> - int legacy_host_irqs[PCI_NUM_INTX];
>>>> + int legacy_host_irq;
>>>> struct device_node *legacy_intc_np;
>>>>
>>>> int msi_host_irqs[MAX_MSI_HOST_IRQS];
>>>> @@ -582,11 +582,11 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
>>>> */
>>>> static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
>>>> {
>>>> - unsigned int irq = irq_desc_get_irq(desc);
>>>> + unsigned int irq = desc->irq_data.hwirq;
>>>> struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
>>>> struct dw_pcie *pci = ks_pcie->pci;
>>>> struct device *dev = pci->dev;
>>>> - u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
>>>> + u32 irq_offset = irq - ks_pcie->legacy_host_irq;
>>>
>>> I think you should use the plain hwirq number (that if I understand
>>> correctly range in [0,3]) and drop legacy_host_irq.
>>
>> The hwirq is [80, 83] for Keystone. We store legacy_host_irq (for Keystone it
>> is 80) to get the correct offset in the range [0, 3].
>
> IIUC the _parent_ hw_irq number should be what you need here, Marc (if
> he has time) can correct me if I am wrong (or what I am suggesting is
> a violation of IRQ domain bstraction usage).
>
>>>
>>> See below.
>>>
>>>> struct irq_chip *chip = irq_desc_get_chip(desc);
>>>>
>>>> dev_dbg(dev, ": Handling legacy irq %d\n", irq);
>>>> @@ -657,6 +657,7 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
>>>> struct device_node *np = ks_pcie->np;
>>>> struct device_node *intc_np;
>>>> int irq_count, irq, ret, i;
>>>> + struct irq_data *irq_data;
>>>>
>>>> intc_np = of_get_child_by_name(np, "legacy-interrupt-controller");
>>>> if (!intc_np) {
>>>> @@ -677,7 +678,15 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
>>>> ret = -EINVAL;
>>>> goto err;
>>>> }
>>>> - ks_pcie->legacy_host_irqs[i] = irq;
>>>> +
>>>> + if (!ks_pcie->legacy_host_irq) {
>>>> + irq_data = irq_get_irq_data(irq);
>>>> + if (!irq_data) {
>>>> + ret = -EINVAL;
>>>> + goto err;
>>>> + }
>>>> + ks_pcie->legacy_host_irq = irq_data->hwirq;
>>>
>>> This depends on DT property ordering and may well be wrong. On top
>>> of that as I said above I do not think it is even needed.
>>
>> right, it depends on the ordering of the entries in the "interrupts" property.
>> Right now we have something like below
>>
>> interrupts = <GIC_SPI 48 IRQ_TYPE_EDGE_RISING>,
>> <GIC_SPI 49 IRQ_TYPE_EDGE_RISING>,
>> <GIC_SPI 50 IRQ_TYPE_EDGE_RISING>,
>> <GIC_SPI 51 IRQ_TYPE_EDGE_RISING>;
>>
>> And changing the order will make legacy_host_irq store the wrong hwirq. With
>> this patch we solve the problem of assuming virq to be contiguous for
>> contiguous hwirq however we do expect the hwirqs to be populated in the DT in
>> the correct order.
>>
>> Ordering entries within a property is also followed in other
>> properties like "reg" where the drivers use platform_get_resource with
>> index as argument. I'm not sure if there's a way to get rid of the
>> ordering requirement in DT for the "interrupts" property.
>
> I am not sure there is an ordering requirement in place but hopefully we
> should not need it, see above.
>
> The 1:1 relationship between GIC-IRQ<->PCI-host-bridge-controller-IRQ is
> what the kernel IRQ domain abstraction is there to implement, so a
> reverse look-up should be feasible without adding anything else, lest
> we duplicate IRQ domain data structures.
I got legacy interrupt working using hierarchy domain. With that we don't need
to have chained irq handler here. I'll post v4 soon.
Thanks
Kishon
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 4/9] PCI: keystone: Use hwirq to get the MSI IRQ number offset
2019-02-13 13:26 [PATCH v3 0/9] PCI: DWC/Keystone: MSI configuration cleanup Kishon Vijay Abraham I
` (2 preceding siblings ...)
2019-02-13 13:26 ` [PATCH v3 3/9] PCI: keystone: Use hwirq to get the legacy IRQ number offset Kishon Vijay Abraham I
@ 2019-02-13 13:26 ` Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 5/9] PCI: keystone: Cleanup ks_pcie_msi_irq_handler and ks_pcie_legacy_irq_handler Kishon Vijay Abraham I
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-13 13:26 UTC (permalink / raw)
To: Murali Karicheri, Lorenzo Pieralisi
Cc: Jingoo Han, linux-pci, linux-kernel, Kishon Vijay Abraham I,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
ks_pcie_msi_irq_handler() uses 'virq' to get the IRQ number offset.
This offset is used to get the correct MSI_IRQ_STATUS register
corresponding to the IRQ line that raised the interrupt.
There is no guarantee that 'virq' assigned for consecutive hardware
IRQ will be contiguous. And this might get us an incorrect IRQ number
offset.
Fix it here by using 'hwirq' to get the IRQ number offset. Since we
don't store the 'virq' numbers of all the IRQ numbers, stop checking
if irq count is greater than MAX_MSI_HOST_IRQS and remove
MAX_MSI_HOST_IRQS.
Link: https://lkml.kernel.org/r/bb081d21-7c03-0357-4294-7e92d95d838c@arm.com
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/pci/controller/dwc/pci-keystone.c | 24 ++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index d35ac712a9f8..90d64507acd0 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -74,7 +74,6 @@
#define ERR_IRQ_ALL (ERR_AER | ERR_AXI | ERR_CORR | \
ERR_NONFATAL | ERR_FATAL | ERR_SYS)
-#define MAX_MSI_HOST_IRQS 8
/* PCIE controller device IDs */
#define PCIE_RC_K2HK 0xb008
#define PCIE_RC_K2E 0xb009
@@ -90,7 +89,7 @@ struct keystone_pcie {
int legacy_host_irq;
struct device_node *legacy_intc_np;
- int msi_host_irqs[MAX_MSI_HOST_IRQS];
+ int msi_host_irq;
int num_lanes;
u32 num_viewport;
struct phy **phy;
@@ -553,9 +552,9 @@ static int ks_pcie_establish_link(struct keystone_pcie *ks_pcie)
static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
{
- unsigned int irq = irq_desc_get_irq(desc);
+ unsigned int irq = desc->irq_data.hwirq;
struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
- u32 offset = irq - ks_pcie->msi_host_irqs[0];
+ u32 offset = irq - ks_pcie->msi_host_irq;
struct dw_pcie *pci = ks_pcie->pci;
struct device *dev = pci->dev;
struct irq_chip *chip = irq_desc_get_chip(desc);
@@ -606,6 +605,7 @@ static int ks_pcie_config_msi_irq(struct keystone_pcie *ks_pcie)
struct device *dev = ks_pcie->pci->dev;
struct device_node *np = ks_pcie->np;
struct device_node *intc_np;
+ struct irq_data *irq_data;
int irq_count, irq, ret, i;
if (!IS_ENABLED(CONFIG_PCI_MSI))
@@ -624,19 +624,21 @@ static int ks_pcie_config_msi_irq(struct keystone_pcie *ks_pcie)
goto err;
}
- if (irq_count > MAX_MSI_HOST_IRQS) {
- dev_warn(dev, "Too many MSI interrupt lines defined %u\n",
- irq_count);
- irq_count = MAX_MSI_HOST_IRQS;
- }
-
for (i = 0; i < irq_count; i++) {
irq = irq_of_parse_and_map(intc_np, i);
if (!irq) {
ret = -EINVAL;
goto err;
}
- ks_pcie->msi_host_irqs[i] = irq;
+
+ if (!ks_pcie->msi_host_irq) {
+ irq_data = irq_get_irq_data(irq);
+ if (!irq_data) {
+ ret = -EINVAL;
+ goto err;
+ }
+ ks_pcie->msi_host_irq = irq_data->hwirq;
+ }
irq_set_chained_handler_and_data(irq, ks_pcie_msi_irq_handler,
ks_pcie);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 5/9] PCI: keystone: Cleanup ks_pcie_msi_irq_handler and ks_pcie_legacy_irq_handler
2019-02-13 13:26 [PATCH v3 0/9] PCI: DWC/Keystone: MSI configuration cleanup Kishon Vijay Abraham I
` (3 preceding siblings ...)
2019-02-13 13:26 ` [PATCH v3 4/9] PCI: keystone: Use hwirq to get the MSI " Kishon Vijay Abraham I
@ 2019-02-13 13:26 ` Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 6/9] PCI: dwc: Add support to use non default msi_irq_chip Kishon Vijay Abraham I
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-13 13:26 UTC (permalink / raw)
To: Murali Karicheri, Lorenzo Pieralisi
Cc: Jingoo Han, linux-pci, linux-kernel, Kishon Vijay Abraham I,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
Both ks_pcie_msi_irq_handler() and ks_pcie_legacy_irq_handler() invokes
ks_pcie_handle_msi_irq() and ks_pcie_handle_legacy_irq() respectively
for handling the interrupts.
Having two functions for handling the interrupt was used when keystone
PCIe driver was implemented using two files. But with
commit b492aca35c982011500377797d2 ("PCI: keystone: Merge
pci-keystone-dw.c and pci-keystone.c"), which merged the keystone PCIe
driver to use a single file, two functions for handling the
interrupt handler is not required.
Handle both MSI interrupt and legacy interrupt in a single interrupt
handler here.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/pci/controller/dwc/pci-keystone.c | 100 +++++++++-------------
1 file changed, 42 insertions(+), 58 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 90d64507acd0..88bd029c2f03 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -105,13 +105,6 @@ struct keystone_pcie {
struct resource app;
};
-static inline void update_reg_offset_bit_pos(u32 offset, u32 *reg_offset,
- u32 *bit_pos)
-{
- *reg_offset = offset % 8;
- *bit_pos = offset >> 3;
-}
-
static phys_addr_t ks_pcie_get_msi_addr(struct pcie_port *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -131,31 +124,6 @@ static void ks_pcie_app_writel(struct keystone_pcie *ks_pcie, u32 offset,
writel(val, ks_pcie->va_app_base + offset);
}
-static void ks_pcie_handle_msi_irq(struct keystone_pcie *ks_pcie, int offset)
-{
- struct dw_pcie *pci = ks_pcie->pci;
- struct pcie_port *pp = &pci->pp;
- struct device *dev = pci->dev;
- u32 pending, vector;
- int src, virq;
-
- pending = ks_pcie_app_readl(ks_pcie, MSI_IRQ_STATUS(offset));
-
- /*
- * MSI0 status bit 0-3 shows vectors 0, 8, 16, 24, MSI1 status bit
- * shows 1, 9, 17, 25 and so forth
- */
- for (src = 0; src < 4; src++) {
- if (BIT(src) & pending) {
- vector = offset + (src << 3);
- virq = irq_linear_revmap(pp->irq_domain, vector);
- dev_dbg(dev, "irq: bit %d, vector %d, virq %d\n",
- src, vector, virq);
- generic_handle_irq(virq);
- }
- }
-}
-
static void ks_pcie_msi_irq_ack(int irq, struct pcie_port *pp)
{
u32 reg_offset, bit_pos;
@@ -164,7 +132,9 @@ static void ks_pcie_msi_irq_ack(int irq, struct pcie_port *pp)
pci = to_dw_pcie_from_pp(pp);
ks_pcie = to_keystone_pcie(pci);
- update_reg_offset_bit_pos(irq, ®_offset, &bit_pos);
+
+ reg_offset = irq % 8;
+ bit_pos = irq >> 3;
ks_pcie_app_writel(ks_pcie, MSI_IRQ_STATUS(reg_offset),
BIT(bit_pos));
@@ -177,7 +147,9 @@ static void ks_pcie_msi_set_irq(struct pcie_port *pp, int irq)
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
- update_reg_offset_bit_pos(irq, ®_offset, &bit_pos);
+ reg_offset = irq % 8;
+ bit_pos = irq >> 3;
+
ks_pcie_app_writel(ks_pcie, MSI_IRQ_ENABLE_SET(reg_offset),
BIT(bit_pos));
}
@@ -188,7 +160,9 @@ static void ks_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
- update_reg_offset_bit_pos(irq, ®_offset, &bit_pos);
+ reg_offset = irq % 8;
+ bit_pos = irq >> 3;
+
ks_pcie_app_writel(ks_pcie, MSI_IRQ_ENABLE_CLR(reg_offset),
BIT(bit_pos));
}
@@ -198,26 +172,6 @@ static int ks_pcie_msi_host_init(struct pcie_port *pp)
return dw_pcie_allocate_domains(pp);
}
-static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
- int offset)
-{
- struct dw_pcie *pci = ks_pcie->pci;
- struct device *dev = pci->dev;
- u32 pending;
- int virq;
-
- pending = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(offset));
-
- if (BIT(0) & pending) {
- virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
- dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
- generic_handle_irq(virq);
- }
-
- /* EOI the INTx interrupt */
- ks_pcie_app_writel(ks_pcie, IRQ_EOI, offset);
-}
-
static void ks_pcie_enable_error_irq(struct keystone_pcie *ks_pcie)
{
ks_pcie_app_writel(ks_pcie, ERR_IRQ_ENABLE_SET, ERR_IRQ_ALL);
@@ -556,8 +510,10 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
u32 offset = irq - ks_pcie->msi_host_irq;
struct dw_pcie *pci = ks_pcie->pci;
+ struct pcie_port *pp = &pci->pp;
struct device *dev = pci->dev;
struct irq_chip *chip = irq_desc_get_chip(desc);
+ u32 vector, virq, reg, pos;
dev_dbg(dev, "%s, irq %d\n", __func__, irq);
@@ -567,7 +523,23 @@ static void ks_pcie_msi_irq_handler(struct irq_desc *desc)
* ack operation.
*/
chained_irq_enter(chip, desc);
- ks_pcie_handle_msi_irq(ks_pcie, offset);
+
+ reg = ks_pcie_app_readl(ks_pcie, MSI_IRQ_STATUS(offset));
+ /*
+ * MSI0 status bit 0-3 shows vectors 0, 8, 16, 24, MSI1 status bit
+ * shows 1, 9, 17, 25 and so forth
+ */
+ for (pos = 0; pos < 4; pos++) {
+ if (!(reg & BIT(pos)))
+ continue;
+
+ vector = offset + (pos << 3);
+ virq = irq_linear_revmap(pp->irq_domain, vector);
+ dev_dbg(dev, "irq: bit %d, vector %d, virq %d\n", pos, vector,
+ virq);
+ generic_handle_irq(virq);
+ }
+
chained_irq_exit(chip, desc);
}
@@ -585,8 +557,9 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
struct dw_pcie *pci = ks_pcie->pci;
struct device *dev = pci->dev;
- u32 irq_offset = irq - ks_pcie->legacy_host_irq;
+ u32 offset = irq - ks_pcie->legacy_host_irq;
struct irq_chip *chip = irq_desc_get_chip(desc);
+ u32 pending, virq;
dev_dbg(dev, ": Handling legacy irq %d\n", irq);
@@ -596,7 +569,18 @@ static void ks_pcie_legacy_irq_handler(struct irq_desc *desc)
* ack operation.
*/
chained_irq_enter(chip, desc);
- ks_pcie_handle_legacy_irq(ks_pcie, irq_offset);
+
+ pending = ks_pcie_app_readl(ks_pcie, IRQ_STATUS(offset));
+
+ if (BIT(0) & pending) {
+ virq = irq_linear_revmap(ks_pcie->legacy_irq_domain, offset);
+ dev_dbg(dev, ": irq: irq_offset %d, virq %d\n", offset, virq);
+ generic_handle_irq(virq);
+ }
+
+ /* EOI the INTx interrupt */
+ ks_pcie_app_writel(ks_pcie, IRQ_EOI, offset);
+
chained_irq_exit(chip, desc);
}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 6/9] PCI: dwc: Add support to use non default msi_irq_chip
2019-02-13 13:26 [PATCH v3 0/9] PCI: DWC/Keystone: MSI configuration cleanup Kishon Vijay Abraham I
` (4 preceding siblings ...)
2019-02-13 13:26 ` [PATCH v3 5/9] PCI: keystone: Cleanup ks_pcie_msi_irq_handler and ks_pcie_legacy_irq_handler Kishon Vijay Abraham I
@ 2019-02-13 13:26 ` Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 7/9] PCI: keystone: Use Keystone specific msi_irq_chip Kishon Vijay Abraham I
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-13 13:26 UTC (permalink / raw)
To: Murali Karicheri, Lorenzo Pieralisi
Cc: Jingoo Han, linux-pci, linux-kernel, Kishon Vijay Abraham I,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
Platforms using DesignWare IP use dw_pci_msi_bottom_irq_chip for
configuring the MSI controller logic within the DesignWare IP. However
certain platforms like Keystone (K2G) which uses DesignWare IP have
their own MSI controller logic. For handling such platforms,
the irqchip ops use msi_irq_ack(), msi_set_irq(), msi_clear_irq()
callback functions.
Add support to use different msi_irq_chip with default as
dw_pci_msi_bottom_irq_chip. This is in preparation to get rid of
msi_irq_ack(), msi_set_irq(), msi_clear_irq() and other Keystone
specific dw_pcie_host_ops.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 5 ++++-
drivers/pci/controller/dwc/pcie-designware.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 721d60a5d9e4..042de09b0451 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -245,7 +245,7 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
for (i = 0; i < nr_irqs; i++)
irq_domain_set_info(domain, virq + i, bit + i,
- &dw_pci_msi_bottom_irq_chip,
+ pp->msi_irq_chip,
pp, handle_edge_irq,
NULL, NULL);
@@ -277,6 +277,9 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node);
+ if (!pp->msi_irq_chip)
+ pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
+
pp->irq_domain = irq_domain_create_linear(fwnode, pp->num_vectors,
&dw_pcie_msi_domain_ops, pp);
if (!pp->irq_domain) {
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 9943d8c68335..cb6eeb062f47 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -176,6 +176,7 @@ struct pcie_port {
struct irq_domain *irq_domain;
struct irq_domain *msi_domain;
dma_addr_t msi_data;
+ struct irq_chip *msi_irq_chip;
u32 num_vectors;
u32 irq_status[MAX_MSI_CTRLS];
raw_spinlock_t lock;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 7/9] PCI: keystone: Use Keystone specific msi_irq_chip
2019-02-13 13:26 [PATCH v3 0/9] PCI: DWC/Keystone: MSI configuration cleanup Kishon Vijay Abraham I
` (5 preceding siblings ...)
2019-02-13 13:26 ` [PATCH v3 6/9] PCI: dwc: Add support to use non default msi_irq_chip Kishon Vijay Abraham I
@ 2019-02-13 13:26 ` Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 8/9] PCI: dwc: Remove Keystone specific dw_pcie_host_ops Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 9/9] PCI: dwc: Do not write to MSI control registers if the platform doesn't use it Kishon Vijay Abraham I
8 siblings, 0 replies; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-13 13:26 UTC (permalink / raw)
To: Murali Karicheri, Lorenzo Pieralisi
Cc: Jingoo Han, linux-pci, linux-kernel, Kishon Vijay Abraham I,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
Use Keystone specific msi_irq_chip to configure the MSI controller
logic in the PCIe keystone wrapper instead of using the default
Designware msi_irq chip (dw_pci_msi_bottom_irq_chip) with
callback functions for configuring the Keystone MSI controller.
This will help to remove Keystone specific callback functions
added in dw_pcie_host_ops.
Move the default msi_irq_chip assignment to dw_pcie_host_init() since
platforms that doesn't use the default msi_irq_chip will assign
msi_irq_chip in msi_host_init() callback.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/pci/controller/dwc/pci-keystone.c | 96 ++++++++++++++-----
.../pci/controller/dwc/pcie-designware-host.c | 5 +-
2 files changed, 74 insertions(+), 27 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 88bd029c2f03..e3a9b4bde19e 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -105,14 +105,6 @@ struct keystone_pcie {
struct resource app;
};
-static phys_addr_t ks_pcie_get_msi_addr(struct pcie_port *pp)
-{
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
-
- return ks_pcie->app.start + MSI_IRQ;
-}
-
static u32 ks_pcie_app_readl(struct keystone_pcie *ks_pcie, u32 offset)
{
return readl(ks_pcie->va_app_base + offset);
@@ -124,11 +116,14 @@ static void ks_pcie_app_writel(struct keystone_pcie *ks_pcie, u32 offset,
writel(val, ks_pcie->va_app_base + offset);
}
-static void ks_pcie_msi_irq_ack(int irq, struct pcie_port *pp)
+static void ks_pcie_msi_irq_ack(struct irq_data *data)
{
- u32 reg_offset, bit_pos;
+ struct pcie_port *pp = irq_data_get_irq_chip_data(data);
struct keystone_pcie *ks_pcie;
+ u32 irq = data->hwirq;
struct dw_pcie *pci;
+ u32 reg_offset;
+ u32 bit_pos;
pci = to_dw_pcie_from_pp(pp);
ks_pcie = to_keystone_pcie(pci);
@@ -141,34 +136,91 @@ static void ks_pcie_msi_irq_ack(int irq, struct pcie_port *pp)
ks_pcie_app_writel(ks_pcie, IRQ_EOI, reg_offset + MSI_IRQ_OFFSET);
}
-static void ks_pcie_msi_set_irq(struct pcie_port *pp, int irq)
+static void ks_pcie_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
{
- u32 reg_offset, bit_pos;
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
+ struct pcie_port *pp = irq_data_get_irq_chip_data(data);
+ struct keystone_pcie *ks_pcie;
+ struct dw_pcie *pci;
+ u64 msi_target;
+
+ pci = to_dw_pcie_from_pp(pp);
+ ks_pcie = to_keystone_pcie(pci);
+
+ msi_target = ks_pcie->app.start + MSI_IRQ;
+ msg->address_lo = lower_32_bits(msi_target);
+ msg->address_hi = upper_32_bits(msi_target);
+ msg->data = data->hwirq;
+
+ dev_dbg(pci->dev, "msi#%d address_hi %#x address_lo %#x\n",
+ (int)data->hwirq, msg->address_hi, msg->address_lo);
+}
+
+static int ks_pcie_msi_set_affinity(struct irq_data *irq_data,
+ const struct cpumask *mask, bool force)
+{
+ return -EINVAL;
+}
+
+static void ks_pcie_msi_mask(struct irq_data *data)
+{
+ struct pcie_port *pp = irq_data_get_irq_chip_data(data);
+ struct keystone_pcie *ks_pcie;
+ u32 irq = data->hwirq;
+ struct dw_pcie *pci;
+ unsigned long flags;
+ u32 reg_offset;
+ u32 bit_pos;
+
+ raw_spin_lock_irqsave(&pp->lock, flags);
+
+ pci = to_dw_pcie_from_pp(pp);
+ ks_pcie = to_keystone_pcie(pci);
reg_offset = irq % 8;
bit_pos = irq >> 3;
- ks_pcie_app_writel(ks_pcie, MSI_IRQ_ENABLE_SET(reg_offset),
+ ks_pcie_app_writel(ks_pcie, MSI_IRQ_ENABLE_CLR(reg_offset),
BIT(bit_pos));
+
+ raw_spin_unlock_irqrestore(&pp->lock, flags);
}
-static void ks_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
+static void ks_pcie_msi_unmask(struct irq_data *data)
{
- u32 reg_offset, bit_pos;
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
+ struct pcie_port *pp = irq_data_get_irq_chip_data(data);
+ struct keystone_pcie *ks_pcie;
+ u32 irq = data->hwirq;
+ struct dw_pcie *pci;
+ unsigned long flags;
+ u32 reg_offset;
+ u32 bit_pos;
+
+ raw_spin_lock_irqsave(&pp->lock, flags);
+
+ pci = to_dw_pcie_from_pp(pp);
+ ks_pcie = to_keystone_pcie(pci);
reg_offset = irq % 8;
bit_pos = irq >> 3;
- ks_pcie_app_writel(ks_pcie, MSI_IRQ_ENABLE_CLR(reg_offset),
+ ks_pcie_app_writel(ks_pcie, MSI_IRQ_ENABLE_SET(reg_offset),
BIT(bit_pos));
+
+ raw_spin_unlock_irqrestore(&pp->lock, flags);
}
+static struct irq_chip ks_pcie_msi_irq_chip = {
+ .name = "KEYSTONE-PCI-MSI",
+ .irq_ack = ks_pcie_msi_irq_ack,
+ .irq_compose_msi_msg = ks_pcie_compose_msi_msg,
+ .irq_set_affinity = ks_pcie_msi_set_affinity,
+ .irq_mask = ks_pcie_msi_mask,
+ .irq_unmask = ks_pcie_msi_unmask,
+};
+
static int ks_pcie_msi_host_init(struct pcie_port *pp)
{
+ pp->msi_irq_chip = &ks_pcie_msi_irq_chip;
return dw_pcie_allocate_domains(pp);
}
@@ -790,11 +842,7 @@ static const struct dw_pcie_host_ops ks_pcie_host_ops = {
.rd_other_conf = ks_pcie_rd_other_conf,
.wr_other_conf = ks_pcie_wr_other_conf,
.host_init = ks_pcie_host_init,
- .msi_set_irq = ks_pcie_msi_set_irq,
- .msi_clear_irq = ks_pcie_msi_clear_irq,
- .get_msi_addr = ks_pcie_get_msi_addr,
.msi_host_init = ks_pcie_msi_host_init,
- .msi_irq_ack = ks_pcie_msi_irq_ack,
.scan_bus = ks_pcie_v3_65_scan_bus,
};
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 042de09b0451..72cfedce5af1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -277,9 +277,6 @@ int dw_pcie_allocate_domains(struct pcie_port *pp)
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node);
- if (!pp->msi_irq_chip)
- pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
-
pp->irq_domain = irq_domain_create_linear(fwnode, pp->num_vectors,
&dw_pcie_msi_domain_ops, pp);
if (!pp->irq_domain) {
@@ -462,6 +459,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
}
if (!pp->ops->msi_host_init) {
+ pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip;
+
ret = dw_pcie_allocate_domains(pp);
if (ret)
goto error;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 8/9] PCI: dwc: Remove Keystone specific dw_pcie_host_ops
2019-02-13 13:26 [PATCH v3 0/9] PCI: DWC/Keystone: MSI configuration cleanup Kishon Vijay Abraham I
` (6 preceding siblings ...)
2019-02-13 13:26 ` [PATCH v3 7/9] PCI: keystone: Use Keystone specific msi_irq_chip Kishon Vijay Abraham I
@ 2019-02-13 13:26 ` Kishon Vijay Abraham I
2019-02-13 13:26 ` [PATCH v3 9/9] PCI: dwc: Do not write to MSI control registers if the platform doesn't use it Kishon Vijay Abraham I
8 siblings, 0 replies; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-13 13:26 UTC (permalink / raw)
To: Murali Karicheri, Lorenzo Pieralisi
Cc: Jingoo Han, linux-pci, linux-kernel, Kishon Vijay Abraham I,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
Now that Keystone started using its own msi_irq_chip, remove
Keystone specific callback functions defined in dw_pcie_host_ops.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
.../pci/controller/dwc/pcie-designware-host.c | 50 ++++++-------------
drivers/pci/controller/dwc/pcie-designware.h | 5 --
2 files changed, 14 insertions(+), 41 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 72cfedce5af1..781735f06dea 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -126,18 +126,12 @@ static void dw_pci_setup_msi_msg(struct irq_data *data, struct msi_msg *msg)
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
u64 msi_target;
- if (pp->ops->get_msi_addr)
- msi_target = pp->ops->get_msi_addr(pp);
- else
- msi_target = (u64)pp->msi_data;
+ msi_target = (u64)pp->msi_data;
msg->address_lo = lower_32_bits(msi_target);
msg->address_hi = upper_32_bits(msi_target);
- if (pp->ops->get_msi_data)
- msg->data = pp->ops->get_msi_data(pp, data->hwirq);
- else
- msg->data = data->hwirq;
+ msg->data = data->hwirq;
dev_dbg(pci->dev, "msi#%d address_hi %#x address_lo %#x\n",
(int)data->hwirq, msg->address_hi, msg->address_lo);
@@ -157,17 +151,13 @@ static void dw_pci_bottom_mask(struct irq_data *data)
raw_spin_lock_irqsave(&pp->lock, flags);
- if (pp->ops->msi_clear_irq) {
- pp->ops->msi_clear_irq(pp, data->hwirq);
- } else {
- ctrl = data->hwirq / MAX_MSI_IRQS_PER_CTRL;
- res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
- bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL;
+ ctrl = data->hwirq / MAX_MSI_IRQS_PER_CTRL;
+ res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
+ bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL;
- pp->irq_status[ctrl] &= ~(1 << bit);
- dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
- ~pp->irq_status[ctrl]);
- }
+ pp->irq_status[ctrl] &= ~(1 << bit);
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
+ ~pp->irq_status[ctrl]);
raw_spin_unlock_irqrestore(&pp->lock, flags);
}
@@ -180,17 +170,13 @@ static void dw_pci_bottom_unmask(struct irq_data *data)
raw_spin_lock_irqsave(&pp->lock, flags);
- if (pp->ops->msi_set_irq) {
- pp->ops->msi_set_irq(pp, data->hwirq);
- } else {
- ctrl = data->hwirq / MAX_MSI_IRQS_PER_CTRL;
- res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
- bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL;
+ ctrl = data->hwirq / MAX_MSI_IRQS_PER_CTRL;
+ res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
+ bit = data->hwirq % MAX_MSI_IRQS_PER_CTRL;
- pp->irq_status[ctrl] |= 1 << bit;
- dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
- ~pp->irq_status[ctrl]);
- }
+ pp->irq_status[ctrl] |= 1 << bit;
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
+ ~pp->irq_status[ctrl]);
raw_spin_unlock_irqrestore(&pp->lock, flags);
}
@@ -199,20 +185,12 @@ static void dw_pci_bottom_ack(struct irq_data *d)
{
struct pcie_port *pp = irq_data_get_irq_chip_data(d);
unsigned int res, bit, ctrl;
- unsigned long flags;
ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
- raw_spin_lock_irqsave(&pp->lock, flags);
-
dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + res, 4, 1 << bit);
-
- if (pp->ops->msi_irq_ack)
- pp->ops->msi_irq_ack(d->hwirq, pp);
-
- raw_spin_unlock_irqrestore(&pp->lock, flags);
}
static struct irq_chip dw_pci_msi_bottom_irq_chip = {
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index cb6eeb062f47..ac75cde80a0c 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -142,14 +142,9 @@ struct dw_pcie_host_ops {
int (*wr_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
unsigned int devfn, int where, int size, u32 val);
int (*host_init)(struct pcie_port *pp);
- void (*msi_set_irq)(struct pcie_port *pp, int irq);
- void (*msi_clear_irq)(struct pcie_port *pp, int irq);
- phys_addr_t (*get_msi_addr)(struct pcie_port *pp);
- u32 (*get_msi_data)(struct pcie_port *pp, int pos);
void (*scan_bus)(struct pcie_port *pp);
void (*set_num_vectors)(struct pcie_port *pp);
int (*msi_host_init)(struct pcie_port *pp);
- void (*msi_irq_ack)(int irq, struct pcie_port *pp);
};
struct pcie_port {
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 9/9] PCI: dwc: Do not write to MSI control registers if the platform doesn't use it
2019-02-13 13:26 [PATCH v3 0/9] PCI: DWC/Keystone: MSI configuration cleanup Kishon Vijay Abraham I
` (7 preceding siblings ...)
2019-02-13 13:26 ` [PATCH v3 8/9] PCI: dwc: Remove Keystone specific dw_pcie_host_ops Kishon Vijay Abraham I
@ 2019-02-13 13:26 ` Kishon Vijay Abraham I
8 siblings, 0 replies; 15+ messages in thread
From: Kishon Vijay Abraham I @ 2019-02-13 13:26 UTC (permalink / raw)
To: Murali Karicheri, Lorenzo Pieralisi
Cc: Jingoo Han, linux-pci, linux-kernel, Kishon Vijay Abraham I,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
Platforms which populate msi_host_init, has it's own MSI controller
logic. Writing to MSI control registers on platforms which doesn't use
Designware's MSI controller logic might have side effects. To
be safe, do not write to MSI control registers if the platform uses
it's own MSI controller logic instead of Designware's MSI controller
logic.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
.../pci/controller/dwc/pcie-designware-host.c | 24 ++++++++++---------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 781735f06dea..2bc2fd582124 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -641,17 +641,19 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
dw_pcie_setup(pci);
- num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
-
- /* Initialize IRQ Status array */
- for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
- dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK +
- (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
- 4, ~0);
- dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE +
- (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
- 4, ~0);
- pp->irq_status[ctrl] = 0;
+ if (!pp->ops->msi_host_init) {
+ num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
+ /* Initialize IRQ Status array */
+ for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK +
+ (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
+ 4, ~0);
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE +
+ (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
+ 4, ~0);
+ pp->irq_status[ctrl] = 0;
+ }
}
/* Setup RC BARs */
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread