* [PATCH 0/2] PCI: Add legacy interrupt support in pci-j721e @ 2021-08-11 12:38 ` Kishon Vijay Abraham I 0 siblings, 0 replies; 8+ messages in thread From: Kishon Vijay Abraham I @ 2021-08-11 12:38 UTC (permalink / raw) To: Lorenzo Pieralisi, Marc Zyngier, Rob Herring, Bjorn Helgaas Cc: Lokesh Vutla, kishon, linux-pci, linux-kernel, linux-omap, linux-arm-kernel Patch series adds support for legacy interrupt in pci-j721e. There are two HW implementations of legacy interrupt controller, one specific to J721E and the other for J7200/AM64. In both these implementations, the legacy interrupt is connect to pulse interrupt of GIC and level to pulse is handled by configuring EOI register. EOI to convert level to pulse is broken in J721E due to an errata but is functional in J7200. Hence legacy interrupt support is not added for J721E v1 of the patch series can be found @ [1] Patch series is created on top of [2] Changes from v2: 1) Dropped legacy interrupt support for J721e 2) Enable legacy interrupts registers in .irq_enable callback Changes from v1: 1) Only the legacy interrupt specific part is sent as part of this series. Rest are split and sent as a separate series [2] 2) Created irq_chip for legacy interrupt and used it's ops for enabling, disabling the interrupts. [1] -> http://lore.kernel.org/r/20210325090936.9306-1-kishon@ti.com [2] -> https://lore.kernel.org/linux-omap/20210811123336.31357-1-kishon@ti.com/ Kishon Vijay Abraham I (2): dt-bindings: PCI: ti,j721e: Add bindings to specify legacy interrupts PCI: j721e: Add PCI legacy interrupt support for J7200 .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++ drivers/pci/controller/cadence/pci-j721e.c | 119 ++++++++++++++++++ 2 files changed, 134 insertions(+) -- 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/2] PCI: Add legacy interrupt support in pci-j721e @ 2021-08-11 12:38 ` Kishon Vijay Abraham I 0 siblings, 0 replies; 8+ messages in thread From: Kishon Vijay Abraham I @ 2021-08-11 12:38 UTC (permalink / raw) To: Lorenzo Pieralisi, Marc Zyngier, Rob Herring, Bjorn Helgaas Cc: Lokesh Vutla, kishon, linux-pci, linux-kernel, linux-omap, linux-arm-kernel Patch series adds support for legacy interrupt in pci-j721e. There are two HW implementations of legacy interrupt controller, one specific to J721E and the other for J7200/AM64. In both these implementations, the legacy interrupt is connect to pulse interrupt of GIC and level to pulse is handled by configuring EOI register. EOI to convert level to pulse is broken in J721E due to an errata but is functional in J7200. Hence legacy interrupt support is not added for J721E v1 of the patch series can be found @ [1] Patch series is created on top of [2] Changes from v2: 1) Dropped legacy interrupt support for J721e 2) Enable legacy interrupts registers in .irq_enable callback Changes from v1: 1) Only the legacy interrupt specific part is sent as part of this series. Rest are split and sent as a separate series [2] 2) Created irq_chip for legacy interrupt and used it's ops for enabling, disabling the interrupts. [1] -> http://lore.kernel.org/r/20210325090936.9306-1-kishon@ti.com [2] -> https://lore.kernel.org/linux-omap/20210811123336.31357-1-kishon@ti.com/ Kishon Vijay Abraham I (2): dt-bindings: PCI: ti,j721e: Add bindings to specify legacy interrupts PCI: j721e: Add PCI legacy interrupt support for J7200 .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++ drivers/pci/controller/cadence/pci-j721e.c | 119 ++++++++++++++++++ 2 files changed, 134 insertions(+) -- 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] 8+ messages in thread
* [PATCH 1/2] dt-bindings: PCI: ti,j721e: Add bindings to specify legacy interrupts 2021-08-11 12:38 ` Kishon Vijay Abraham I @ 2021-08-11 12:38 ` Kishon Vijay Abraham I -1 siblings, 0 replies; 8+ messages in thread From: Kishon Vijay Abraham I @ 2021-08-11 12:38 UTC (permalink / raw) To: Lorenzo Pieralisi, Marc Zyngier, Rob Herring, Bjorn Helgaas Cc: Lokesh Vutla, kishon, linux-pci, linux-kernel, linux-omap, linux-arm-kernel Add bindings to specify interrupt controller for legacy interrupts. Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml index cc900202df29..f461d7b4c0cc 100644 --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml @@ -74,6 +74,11 @@ properties: msi-map: true +patternProperties: + "interrupt-controller": + type: object + description: interrupt controller to handle legacy interrupts. + required: - compatible - reg @@ -97,6 +102,8 @@ unevaluatedProperties: false examples: - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/soc/ti,sci_pm_domain.h> #include <dt-bindings/gpio/gpio.h> @@ -131,5 +138,13 @@ examples: ranges = <0x01000000 0x0 0x10001000 0x00 0x10001000 0x0 0x0010000>, <0x02000000 0x0 0x10011000 0x00 0x10011000 0x0 0x7fef000>; dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>; + + + pcie0_intc: interrupt-controller { + interrupt-controller; + #interrupt-cells = <1>; + interrupt-parent = <&gic500>; + interrupts = <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>; + }; }; }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/2] dt-bindings: PCI: ti, j721e: Add bindings to specify legacy interrupts @ 2021-08-11 12:38 ` Kishon Vijay Abraham I 0 siblings, 0 replies; 8+ messages in thread From: Kishon Vijay Abraham I @ 2021-08-11 12:38 UTC (permalink / raw) To: Lorenzo Pieralisi, Marc Zyngier, Rob Herring, Bjorn Helgaas Cc: Lokesh Vutla, kishon, linux-pci, linux-kernel, linux-omap, linux-arm-kernel Add bindings to specify interrupt controller for legacy interrupts. Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- .../bindings/pci/ti,j721e-pci-host.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml index cc900202df29..f461d7b4c0cc 100644 --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml @@ -74,6 +74,11 @@ properties: msi-map: true +patternProperties: + "interrupt-controller": + type: object + description: interrupt controller to handle legacy interrupts. + required: - compatible - reg @@ -97,6 +102,8 @@ unevaluatedProperties: false examples: - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/soc/ti,sci_pm_domain.h> #include <dt-bindings/gpio/gpio.h> @@ -131,5 +138,13 @@ examples: ranges = <0x01000000 0x0 0x10001000 0x00 0x10001000 0x0 0x0010000>, <0x02000000 0x0 0x10011000 0x00 0x10011000 0x0 0x7fef000>; dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>; + + + pcie0_intc: interrupt-controller { + interrupt-controller; + #interrupt-cells = <1>; + interrupt-parent = <&gic500>; + interrupts = <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>; + }; }; }; -- 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] 8+ messages in thread
* [PATCH 2/2] PCI: j721e: Add PCI legacy interrupt support for J7200 2021-08-11 12:38 ` Kishon Vijay Abraham I @ 2021-08-11 12:38 ` Kishon Vijay Abraham I -1 siblings, 0 replies; 8+ messages in thread From: Kishon Vijay Abraham I @ 2021-08-11 12:38 UTC (permalink / raw) To: Lorenzo Pieralisi, Marc Zyngier, Rob Herring, Bjorn Helgaas Cc: Lokesh Vutla, kishon, linux-pci, linux-kernel, linux-omap, linux-arm-kernel Add PCI legacy interrupt support for J7200. J7200 has a single HW interrupt line for all the four legacy interrupts INTA/INTB/INTC/INTD. The HW interrupt line connected to GIC is a pulse interrupt whereas the legacy interrupts by definition is level interrupt. In order to provide level interrupt functionality to edge interrupt line, PCIe in J7200 has provided USER_EOI_REG register. When the SW writes to USER_EOI_REG register after handling the interrupt, the IP checks the state of legacy interrupt and re-triggers pulse interrupt invoking the handler again. Due to Errata ID #i2094 ([1]), EOI feature is not enabled in J721E and only a single pulse interrupt will be generated for every ASSERT_INTx/DEASSERT_INTx. Hence legacy interrupt is not enabled in J721E. [1] -> J721E DRA829/TDA4VM Processors Silicon Revision 1.1/1.0 SPRZ455A – DECEMBER 2020 – REVISED AUGUST 2021 (https://www.ti.com/lit/er/sprz455a/sprz455a.pdf) Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- drivers/pci/controller/cadence/pci-j721e.c | 119 +++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c index ffb176d288cd..4e786d6b89e0 100644 --- a/drivers/pci/controller/cadence/pci-j721e.c +++ b/drivers/pci/controller/cadence/pci-j721e.c @@ -29,12 +29,24 @@ #define LINK_DOWN BIT(1) #define J7200_LINK_DOWN BIT(10) +#define ENABLE_REG_SYS_1 0x104 +#define STATUS_REG_SYS_1 0x504 +#define SYS1_INTx_EN(num) (1 << (22 + (num))) + #define J721E_PCIE_USER_CMD_STATUS 0x4 #define LINK_TRAINING_ENABLE BIT(0) #define J721E_PCIE_USER_LINKSTATUS 0x14 #define LINK_STATUS GENMASK(1, 0) +#define USER_EOI_REG 0xC8 +enum eoi_reg { + EOI_DOWNSTREAM_INTERRUPT, + EOI_FLR_INTERRUPT, + EOI_LEGACY_INTERRUPT, + EOI_POWER_STATE_INTERRUPT, +}; + enum link_status { NO_RECEIVERS_DETECTED, LINK_TRAINING_IN_PROGRESS, @@ -59,6 +71,7 @@ struct j721e_pcie { void __iomem *user_cfg_base; void __iomem *intd_cfg_base; u32 linkdown_irq_regfield; + struct irq_domain *legacy_irq_domain; }; enum j721e_pcie_mode { @@ -121,6 +134,108 @@ static void j721e_pcie_config_link_irq(struct j721e_pcie *pcie) j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_2, reg); } +static void j721e_pcie_legacy_irq_handler(struct irq_desc *desc) +{ + struct j721e_pcie *pcie = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + int i, virq; + u32 reg; + + chained_irq_enter(chip, desc); + + reg = j721e_pcie_intd_readl(pcie, STATUS_REG_SYS_1); + for (i = 0; i < PCI_NUM_INTX; i++) { + if (!(reg & SYS1_INTx_EN(i))) + continue; + + virq = irq_find_mapping(pcie->legacy_irq_domain, i); + generic_handle_irq(virq); + } + + chained_irq_exit(chip, desc); +} + +static void j721e_pcie_irq_eoi(struct irq_data *data) +{ + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); + + j721e_pcie_user_writel(pcie, USER_EOI_REG, EOI_LEGACY_INTERRUPT); +} + +static void j721e_pcie_irq_enable(struct irq_data *data) +{ + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); + u32 reg; + + reg = j721e_pcie_intd_readl(pcie, ENABLE_REG_SYS_1); + reg |= SYS1_INTx_EN(irqd_to_hwirq(data)); + j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_1, reg); +} + +static void j721e_pcie_irq_disable(struct irq_data *data) +{ + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); + u32 reg; + + reg = j721e_pcie_intd_readl(pcie, ENABLE_REG_SYS_1); + reg &= ~SYS1_INTx_EN(irqd_to_hwirq(data)); + j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_1, reg); +} + +struct irq_chip j721e_pcie_irq_chip = { + .name = "J721E-PCIE-INTX", + .irq_eoi = j721e_pcie_irq_eoi, + .irq_enable = j721e_pcie_irq_enable, + .irq_disable = j721e_pcie_irq_disable, +}; + +static int j721e_pcie_intx_map(struct irq_domain *domain, unsigned int irq, irq_hw_number_t hwirq) +{ + struct j721e_pcie *pcie = domain->host_data; + + irq_set_chip_and_handler(irq, &j721e_pcie_irq_chip, handle_fasteoi_irq); + irq_set_chip_data(irq, pcie); + + return 0; +} + +static const struct irq_domain_ops j721e_pcie_intx_domain_ops = { + .map = j721e_pcie_intx_map, +}; + +static int j721e_pcie_config_legacy_irq(struct j721e_pcie *pcie) +{ + struct irq_domain *legacy_irq_domain; + struct device *dev = pcie->dev; + struct device_node *node = dev->of_node; + struct device_node *intc_node; + int irq; + + intc_node = of_get_child_by_name(node, "interrupt-controller"); + if (!intc_node) { + dev_dbg(dev, "interrupt-controller node is absent. Legacy INTR not supported\n"); + return 0; + } + + irq = irq_of_parse_and_map(intc_node, 0); + if (!irq) { + dev_err(dev, "Failed to parse and map legacy irq\n"); + return -EINVAL; + } + + irq_set_chained_handler_and_data(irq, j721e_pcie_legacy_irq_handler, pcie); + + legacy_irq_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX, + &j721e_pcie_intx_domain_ops, pcie); + if (!legacy_irq_domain) { + dev_err(dev, "Failed to add irq domain for legacy irqs\n"); + return -EINVAL; + } + pcie->legacy_irq_domain = legacy_irq_domain; + + return 0; +} + static int j721e_pcie_start_link(struct cdns_pcie *cdns_pcie) { struct j721e_pcie *pcie = dev_get_drvdata(cdns_pcie->dev); @@ -433,6 +548,10 @@ static int j721e_pcie_probe(struct platform_device *pdev) goto err_get_sync; } + ret = j721e_pcie_config_legacy_irq(pcie); + if (ret < 0) + goto err_get_sync; + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc)); if (!bridge) { ret = -ENOMEM; -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] PCI: j721e: Add PCI legacy interrupt support for J7200 @ 2021-08-11 12:38 ` Kishon Vijay Abraham I 0 siblings, 0 replies; 8+ messages in thread From: Kishon Vijay Abraham I @ 2021-08-11 12:38 UTC (permalink / raw) To: Lorenzo Pieralisi, Marc Zyngier, Rob Herring, Bjorn Helgaas Cc: Lokesh Vutla, kishon, linux-pci, linux-kernel, linux-omap, linux-arm-kernel Add PCI legacy interrupt support for J7200. J7200 has a single HW interrupt line for all the four legacy interrupts INTA/INTB/INTC/INTD. The HW interrupt line connected to GIC is a pulse interrupt whereas the legacy interrupts by definition is level interrupt. In order to provide level interrupt functionality to edge interrupt line, PCIe in J7200 has provided USER_EOI_REG register. When the SW writes to USER_EOI_REG register after handling the interrupt, the IP checks the state of legacy interrupt and re-triggers pulse interrupt invoking the handler again. Due to Errata ID #i2094 ([1]), EOI feature is not enabled in J721E and only a single pulse interrupt will be generated for every ASSERT_INTx/DEASSERT_INTx. Hence legacy interrupt is not enabled in J721E. [1] -> J721E DRA829/TDA4VM Processors Silicon Revision 1.1/1.0 SPRZ455A – DECEMBER 2020 – REVISED AUGUST 2021 (https://www.ti.com/lit/er/sprz455a/sprz455a.pdf) Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- drivers/pci/controller/cadence/pci-j721e.c | 119 +++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c index ffb176d288cd..4e786d6b89e0 100644 --- a/drivers/pci/controller/cadence/pci-j721e.c +++ b/drivers/pci/controller/cadence/pci-j721e.c @@ -29,12 +29,24 @@ #define LINK_DOWN BIT(1) #define J7200_LINK_DOWN BIT(10) +#define ENABLE_REG_SYS_1 0x104 +#define STATUS_REG_SYS_1 0x504 +#define SYS1_INTx_EN(num) (1 << (22 + (num))) + #define J721E_PCIE_USER_CMD_STATUS 0x4 #define LINK_TRAINING_ENABLE BIT(0) #define J721E_PCIE_USER_LINKSTATUS 0x14 #define LINK_STATUS GENMASK(1, 0) +#define USER_EOI_REG 0xC8 +enum eoi_reg { + EOI_DOWNSTREAM_INTERRUPT, + EOI_FLR_INTERRUPT, + EOI_LEGACY_INTERRUPT, + EOI_POWER_STATE_INTERRUPT, +}; + enum link_status { NO_RECEIVERS_DETECTED, LINK_TRAINING_IN_PROGRESS, @@ -59,6 +71,7 @@ struct j721e_pcie { void __iomem *user_cfg_base; void __iomem *intd_cfg_base; u32 linkdown_irq_regfield; + struct irq_domain *legacy_irq_domain; }; enum j721e_pcie_mode { @@ -121,6 +134,108 @@ static void j721e_pcie_config_link_irq(struct j721e_pcie *pcie) j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_2, reg); } +static void j721e_pcie_legacy_irq_handler(struct irq_desc *desc) +{ + struct j721e_pcie *pcie = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + int i, virq; + u32 reg; + + chained_irq_enter(chip, desc); + + reg = j721e_pcie_intd_readl(pcie, STATUS_REG_SYS_1); + for (i = 0; i < PCI_NUM_INTX; i++) { + if (!(reg & SYS1_INTx_EN(i))) + continue; + + virq = irq_find_mapping(pcie->legacy_irq_domain, i); + generic_handle_irq(virq); + } + + chained_irq_exit(chip, desc); +} + +static void j721e_pcie_irq_eoi(struct irq_data *data) +{ + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); + + j721e_pcie_user_writel(pcie, USER_EOI_REG, EOI_LEGACY_INTERRUPT); +} + +static void j721e_pcie_irq_enable(struct irq_data *data) +{ + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); + u32 reg; + + reg = j721e_pcie_intd_readl(pcie, ENABLE_REG_SYS_1); + reg |= SYS1_INTx_EN(irqd_to_hwirq(data)); + j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_1, reg); +} + +static void j721e_pcie_irq_disable(struct irq_data *data) +{ + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); + u32 reg; + + reg = j721e_pcie_intd_readl(pcie, ENABLE_REG_SYS_1); + reg &= ~SYS1_INTx_EN(irqd_to_hwirq(data)); + j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_1, reg); +} + +struct irq_chip j721e_pcie_irq_chip = { + .name = "J721E-PCIE-INTX", + .irq_eoi = j721e_pcie_irq_eoi, + .irq_enable = j721e_pcie_irq_enable, + .irq_disable = j721e_pcie_irq_disable, +}; + +static int j721e_pcie_intx_map(struct irq_domain *domain, unsigned int irq, irq_hw_number_t hwirq) +{ + struct j721e_pcie *pcie = domain->host_data; + + irq_set_chip_and_handler(irq, &j721e_pcie_irq_chip, handle_fasteoi_irq); + irq_set_chip_data(irq, pcie); + + return 0; +} + +static const struct irq_domain_ops j721e_pcie_intx_domain_ops = { + .map = j721e_pcie_intx_map, +}; + +static int j721e_pcie_config_legacy_irq(struct j721e_pcie *pcie) +{ + struct irq_domain *legacy_irq_domain; + struct device *dev = pcie->dev; + struct device_node *node = dev->of_node; + struct device_node *intc_node; + int irq; + + intc_node = of_get_child_by_name(node, "interrupt-controller"); + if (!intc_node) { + dev_dbg(dev, "interrupt-controller node is absent. Legacy INTR not supported\n"); + return 0; + } + + irq = irq_of_parse_and_map(intc_node, 0); + if (!irq) { + dev_err(dev, "Failed to parse and map legacy irq\n"); + return -EINVAL; + } + + irq_set_chained_handler_and_data(irq, j721e_pcie_legacy_irq_handler, pcie); + + legacy_irq_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX, + &j721e_pcie_intx_domain_ops, pcie); + if (!legacy_irq_domain) { + dev_err(dev, "Failed to add irq domain for legacy irqs\n"); + return -EINVAL; + } + pcie->legacy_irq_domain = legacy_irq_domain; + + return 0; +} + static int j721e_pcie_start_link(struct cdns_pcie *cdns_pcie) { struct j721e_pcie *pcie = dev_get_drvdata(cdns_pcie->dev); @@ -433,6 +548,10 @@ static int j721e_pcie_probe(struct platform_device *pdev) goto err_get_sync; } + ret = j721e_pcie_config_legacy_irq(pcie); + if (ret < 0) + goto err_get_sync; + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc)); if (!bridge) { ret = -ENOMEM; -- 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] 8+ messages in thread
* Re: [PATCH 2/2] PCI: j721e: Add PCI legacy interrupt support for J7200 2021-08-11 12:38 ` Kishon Vijay Abraham I @ 2021-08-11 14:58 ` Marc Zyngier -1 siblings, 0 replies; 8+ messages in thread From: Marc Zyngier @ 2021-08-11 14:58 UTC (permalink / raw) To: Kishon Vijay Abraham I Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Lokesh Vutla, linux-pci, linux-kernel, linux-omap, linux-arm-kernel On Wed, 11 Aug 2021 13:38:46 +0100, Kishon Vijay Abraham I <kishon@ti.com> wrote: > > Add PCI legacy interrupt support for J7200. J7200 has a single HW > interrupt line for all the four legacy interrupts INTA/INTB/INTC/INTD. > The HW interrupt line connected to GIC is a pulse interrupt whereas > the legacy interrupts by definition is level interrupt. In order to > provide level interrupt functionality to edge interrupt line, PCIe > in J7200 has provided USER_EOI_REG register. When the SW writes to > USER_EOI_REG register after handling the interrupt, the IP checks the > state of legacy interrupt and re-triggers pulse interrupt invoking > the handler again. > > Due to Errata ID #i2094 ([1]), EOI feature is not enabled in J721E > and only a single pulse interrupt will be generated for every > ASSERT_INTx/DEASSERT_INTx. Hence legacy interrupt is not enabled in > J721E. How do you prevent this from being enabled on an affected platform? Just by virtue of not having the interrupt-controller property in DT? In which case, it may be useful to clarify this in the DT binding and say that it is only valid on J7200. > > [1] -> J721E DRA829/TDA4VM Processors Silicon Revision 1.1/1.0 SPRZ455A – > DECEMBER 2020 – REVISED AUGUST 2021 > (https://www.ti.com/lit/er/sprz455a/sprz455a.pdf) > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/controller/cadence/pci-j721e.c | 119 +++++++++++++++++++++ > 1 file changed, 119 insertions(+) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index ffb176d288cd..4e786d6b89e0 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -29,12 +29,24 @@ > #define LINK_DOWN BIT(1) > #define J7200_LINK_DOWN BIT(10) > > +#define ENABLE_REG_SYS_1 0x104 > +#define STATUS_REG_SYS_1 0x504 > +#define SYS1_INTx_EN(num) (1 << (22 + (num))) > + > #define J721E_PCIE_USER_CMD_STATUS 0x4 > #define LINK_TRAINING_ENABLE BIT(0) > > #define J721E_PCIE_USER_LINKSTATUS 0x14 > #define LINK_STATUS GENMASK(1, 0) > > +#define USER_EOI_REG 0xC8 > +enum eoi_reg { > + EOI_DOWNSTREAM_INTERRUPT, > + EOI_FLR_INTERRUPT, > + EOI_LEGACY_INTERRUPT, > + EOI_POWER_STATE_INTERRUPT, > +}; > + > enum link_status { > NO_RECEIVERS_DETECTED, > LINK_TRAINING_IN_PROGRESS, > @@ -59,6 +71,7 @@ struct j721e_pcie { > void __iomem *user_cfg_base; > void __iomem *intd_cfg_base; > u32 linkdown_irq_regfield; > + struct irq_domain *legacy_irq_domain; > }; > > enum j721e_pcie_mode { > @@ -121,6 +134,108 @@ static void j721e_pcie_config_link_irq(struct j721e_pcie *pcie) > j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_2, reg); > } > > +static void j721e_pcie_legacy_irq_handler(struct irq_desc *desc) > +{ > + struct j721e_pcie *pcie = irq_desc_get_handler_data(desc); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + int i, virq; > + u32 reg; > + > + chained_irq_enter(chip, desc); > + > + reg = j721e_pcie_intd_readl(pcie, STATUS_REG_SYS_1); > + for (i = 0; i < PCI_NUM_INTX; i++) { > + if (!(reg & SYS1_INTx_EN(i))) > + continue; > + > + virq = irq_find_mapping(pcie->legacy_irq_domain, i); > + generic_handle_irq(virq); Please use generic_handle_domain_irq(). > + } > + > + chained_irq_exit(chip, desc); > +} > + > +static void j721e_pcie_irq_eoi(struct irq_data *data) > +{ > + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); > + > + j721e_pcie_user_writel(pcie, USER_EOI_REG, EOI_LEGACY_INTERRUPT); > +} > + > +static void j721e_pcie_irq_enable(struct irq_data *data) > +{ > + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); > + u32 reg; > + > + reg = j721e_pcie_intd_readl(pcie, ENABLE_REG_SYS_1); > + reg |= SYS1_INTx_EN(irqd_to_hwirq(data)); > + j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_1, reg); RMW of a register shared between interrupts without a lock. It will eventually end badly. > +} > + > +static void j721e_pcie_irq_disable(struct irq_data *data) > +{ > + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); > + u32 reg; > + > + reg = j721e_pcie_intd_readl(pcie, ENABLE_REG_SYS_1); > + reg &= ~SYS1_INTx_EN(irqd_to_hwirq(data)); > + j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_1, reg); Same thing. > +} > + > +struct irq_chip j721e_pcie_irq_chip = { > + .name = "J721E-PCIE-INTX", > + .irq_eoi = j721e_pcie_irq_eoi, > + .irq_enable = j721e_pcie_irq_enable, > + .irq_disable = j721e_pcie_irq_disable, > +}; > + > +static int j721e_pcie_intx_map(struct irq_domain *domain, unsigned int irq, irq_hw_number_t hwirq) > +{ > + struct j721e_pcie *pcie = domain->host_data; > + > + irq_set_chip_and_handler(irq, &j721e_pcie_irq_chip, handle_fasteoi_irq); > + irq_set_chip_data(irq, pcie); > + > + return 0; > +} > + > +static const struct irq_domain_ops j721e_pcie_intx_domain_ops = { > + .map = j721e_pcie_intx_map, > +}; > + > +static int j721e_pcie_config_legacy_irq(struct j721e_pcie *pcie) > +{ > + struct irq_domain *legacy_irq_domain; > + struct device *dev = pcie->dev; > + struct device_node *node = dev->of_node; > + struct device_node *intc_node; > + int irq; > + > + intc_node = of_get_child_by_name(node, "interrupt-controller"); > + if (!intc_node) { > + dev_dbg(dev, "interrupt-controller node is absent. Legacy INTR not supported\n"); > + return 0; > + } > + > + irq = irq_of_parse_and_map(intc_node, 0); > + if (!irq) { > + dev_err(dev, "Failed to parse and map legacy irq\n"); > + return -EINVAL; > + } > + > + irq_set_chained_handler_and_data(irq, j721e_pcie_legacy_irq_handler, pcie); At this stage, you now have enabled the mux interrupt, and it can fire at will (who knows what state the enabled bits are, given that you don't initialise them?). However, you still haven't allocated the domain. What could possibly go wrong? Please initialise all the interrupts to their disabled state, and only register the handler once all the data structures have been populated. > + > + legacy_irq_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX, > + &j721e_pcie_intx_domain_ops, pcie); > + if (!legacy_irq_domain) { > + dev_err(dev, "Failed to add irq domain for legacy irqs\n"); > + return -EINVAL; > + } > + pcie->legacy_irq_domain = legacy_irq_domain; > + > + return 0; > +} > + > static int j721e_pcie_start_link(struct cdns_pcie *cdns_pcie) > { > struct j721e_pcie *pcie = dev_get_drvdata(cdns_pcie->dev); > @@ -433,6 +548,10 @@ static int j721e_pcie_probe(struct platform_device *pdev) > goto err_get_sync; > } > > + ret = j721e_pcie_config_legacy_irq(pcie); > + if (ret < 0) > + goto err_get_sync; > + > bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc)); > if (!bridge) { > ret = -ENOMEM; Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] PCI: j721e: Add PCI legacy interrupt support for J7200 @ 2021-08-11 14:58 ` Marc Zyngier 0 siblings, 0 replies; 8+ messages in thread From: Marc Zyngier @ 2021-08-11 14:58 UTC (permalink / raw) To: Kishon Vijay Abraham I Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Lokesh Vutla, linux-pci, linux-kernel, linux-omap, linux-arm-kernel On Wed, 11 Aug 2021 13:38:46 +0100, Kishon Vijay Abraham I <kishon@ti.com> wrote: > > Add PCI legacy interrupt support for J7200. J7200 has a single HW > interrupt line for all the four legacy interrupts INTA/INTB/INTC/INTD. > The HW interrupt line connected to GIC is a pulse interrupt whereas > the legacy interrupts by definition is level interrupt. In order to > provide level interrupt functionality to edge interrupt line, PCIe > in J7200 has provided USER_EOI_REG register. When the SW writes to > USER_EOI_REG register after handling the interrupt, the IP checks the > state of legacy interrupt and re-triggers pulse interrupt invoking > the handler again. > > Due to Errata ID #i2094 ([1]), EOI feature is not enabled in J721E > and only a single pulse interrupt will be generated for every > ASSERT_INTx/DEASSERT_INTx. Hence legacy interrupt is not enabled in > J721E. How do you prevent this from being enabled on an affected platform? Just by virtue of not having the interrupt-controller property in DT? In which case, it may be useful to clarify this in the DT binding and say that it is only valid on J7200. > > [1] -> J721E DRA829/TDA4VM Processors Silicon Revision 1.1/1.0 SPRZ455A – > DECEMBER 2020 – REVISED AUGUST 2021 > (https://www.ti.com/lit/er/sprz455a/sprz455a.pdf) > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/controller/cadence/pci-j721e.c | 119 +++++++++++++++++++++ > 1 file changed, 119 insertions(+) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index ffb176d288cd..4e786d6b89e0 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -29,12 +29,24 @@ > #define LINK_DOWN BIT(1) > #define J7200_LINK_DOWN BIT(10) > > +#define ENABLE_REG_SYS_1 0x104 > +#define STATUS_REG_SYS_1 0x504 > +#define SYS1_INTx_EN(num) (1 << (22 + (num))) > + > #define J721E_PCIE_USER_CMD_STATUS 0x4 > #define LINK_TRAINING_ENABLE BIT(0) > > #define J721E_PCIE_USER_LINKSTATUS 0x14 > #define LINK_STATUS GENMASK(1, 0) > > +#define USER_EOI_REG 0xC8 > +enum eoi_reg { > + EOI_DOWNSTREAM_INTERRUPT, > + EOI_FLR_INTERRUPT, > + EOI_LEGACY_INTERRUPT, > + EOI_POWER_STATE_INTERRUPT, > +}; > + > enum link_status { > NO_RECEIVERS_DETECTED, > LINK_TRAINING_IN_PROGRESS, > @@ -59,6 +71,7 @@ struct j721e_pcie { > void __iomem *user_cfg_base; > void __iomem *intd_cfg_base; > u32 linkdown_irq_regfield; > + struct irq_domain *legacy_irq_domain; > }; > > enum j721e_pcie_mode { > @@ -121,6 +134,108 @@ static void j721e_pcie_config_link_irq(struct j721e_pcie *pcie) > j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_2, reg); > } > > +static void j721e_pcie_legacy_irq_handler(struct irq_desc *desc) > +{ > + struct j721e_pcie *pcie = irq_desc_get_handler_data(desc); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + int i, virq; > + u32 reg; > + > + chained_irq_enter(chip, desc); > + > + reg = j721e_pcie_intd_readl(pcie, STATUS_REG_SYS_1); > + for (i = 0; i < PCI_NUM_INTX; i++) { > + if (!(reg & SYS1_INTx_EN(i))) > + continue; > + > + virq = irq_find_mapping(pcie->legacy_irq_domain, i); > + generic_handle_irq(virq); Please use generic_handle_domain_irq(). > + } > + > + chained_irq_exit(chip, desc); > +} > + > +static void j721e_pcie_irq_eoi(struct irq_data *data) > +{ > + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); > + > + j721e_pcie_user_writel(pcie, USER_EOI_REG, EOI_LEGACY_INTERRUPT); > +} > + > +static void j721e_pcie_irq_enable(struct irq_data *data) > +{ > + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); > + u32 reg; > + > + reg = j721e_pcie_intd_readl(pcie, ENABLE_REG_SYS_1); > + reg |= SYS1_INTx_EN(irqd_to_hwirq(data)); > + j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_1, reg); RMW of a register shared between interrupts without a lock. It will eventually end badly. > +} > + > +static void j721e_pcie_irq_disable(struct irq_data *data) > +{ > + struct j721e_pcie *pcie = irq_data_get_irq_chip_data(data); > + u32 reg; > + > + reg = j721e_pcie_intd_readl(pcie, ENABLE_REG_SYS_1); > + reg &= ~SYS1_INTx_EN(irqd_to_hwirq(data)); > + j721e_pcie_intd_writel(pcie, ENABLE_REG_SYS_1, reg); Same thing. > +} > + > +struct irq_chip j721e_pcie_irq_chip = { > + .name = "J721E-PCIE-INTX", > + .irq_eoi = j721e_pcie_irq_eoi, > + .irq_enable = j721e_pcie_irq_enable, > + .irq_disable = j721e_pcie_irq_disable, > +}; > + > +static int j721e_pcie_intx_map(struct irq_domain *domain, unsigned int irq, irq_hw_number_t hwirq) > +{ > + struct j721e_pcie *pcie = domain->host_data; > + > + irq_set_chip_and_handler(irq, &j721e_pcie_irq_chip, handle_fasteoi_irq); > + irq_set_chip_data(irq, pcie); > + > + return 0; > +} > + > +static const struct irq_domain_ops j721e_pcie_intx_domain_ops = { > + .map = j721e_pcie_intx_map, > +}; > + > +static int j721e_pcie_config_legacy_irq(struct j721e_pcie *pcie) > +{ > + struct irq_domain *legacy_irq_domain; > + struct device *dev = pcie->dev; > + struct device_node *node = dev->of_node; > + struct device_node *intc_node; > + int irq; > + > + intc_node = of_get_child_by_name(node, "interrupt-controller"); > + if (!intc_node) { > + dev_dbg(dev, "interrupt-controller node is absent. Legacy INTR not supported\n"); > + return 0; > + } > + > + irq = irq_of_parse_and_map(intc_node, 0); > + if (!irq) { > + dev_err(dev, "Failed to parse and map legacy irq\n"); > + return -EINVAL; > + } > + > + irq_set_chained_handler_and_data(irq, j721e_pcie_legacy_irq_handler, pcie); At this stage, you now have enabled the mux interrupt, and it can fire at will (who knows what state the enabled bits are, given that you don't initialise them?). However, you still haven't allocated the domain. What could possibly go wrong? Please initialise all the interrupts to their disabled state, and only register the handler once all the data structures have been populated. > + > + legacy_irq_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX, > + &j721e_pcie_intx_domain_ops, pcie); > + if (!legacy_irq_domain) { > + dev_err(dev, "Failed to add irq domain for legacy irqs\n"); > + return -EINVAL; > + } > + pcie->legacy_irq_domain = legacy_irq_domain; > + > + return 0; > +} > + > static int j721e_pcie_start_link(struct cdns_pcie *cdns_pcie) > { > struct j721e_pcie *pcie = dev_get_drvdata(cdns_pcie->dev); > @@ -433,6 +548,10 @@ static int j721e_pcie_probe(struct platform_device *pdev) > goto err_get_sync; > } > > + ret = j721e_pcie_config_legacy_irq(pcie); > + if (ret < 0) > + goto err_get_sync; > + > bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc)); > if (!bridge) { > ret = -ENOMEM; Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ 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] 8+ messages in thread
end of thread, other threads:[~2021-08-11 15:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-11 12:38 [PATCH 0/2] PCI: Add legacy interrupt support in pci-j721e Kishon Vijay Abraham I 2021-08-11 12:38 ` Kishon Vijay Abraham I 2021-08-11 12:38 ` [PATCH 1/2] dt-bindings: PCI: ti,j721e: Add bindings to specify legacy interrupts Kishon Vijay Abraham I 2021-08-11 12:38 ` [PATCH 1/2] dt-bindings: PCI: ti, j721e: " Kishon Vijay Abraham I 2021-08-11 12:38 ` [PATCH 2/2] PCI: j721e: Add PCI legacy interrupt support for J7200 Kishon Vijay Abraham I 2021-08-11 12:38 ` Kishon Vijay Abraham I 2021-08-11 14:58 ` Marc Zyngier 2021-08-11 14:58 ` Marc Zyngier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.