Message ID | 20221121042026.419383-1-uwu@icenowy.me (mailing list archive) |
---|---|
State | Rejected |
Headers | show |
Series | irqchip/sifive-plic: drop quirk for two-cell variant | expand |
Context | Check | Description |
---|---|---|
conchuod/patch_count | success | Link |
conchuod/cover_letter | success | Single patches do not need cover letters |
conchuod/tree_selection | success | Guessed tree name to be for-next |
conchuod/fixes_present | success | Fixes tag not required for -next series |
conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
conchuod/module_param | success | Was 0 now: 0 |
conchuod/build_rv32_defconfig | success | Build OK |
conchuod/build_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
conchuod/header_inline | success | No static functions without inline keyword in header files |
conchuod/checkpatch | warning | WARNING: 'thead' may be misspelled - perhaps 'thread'? |
conchuod/source_inline | success | Was 0 now: 0 |
conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
conchuod/verify_fixes | success | No Fixes tag |
conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Mon, 21 Nov 2022 04:20:26 +0000, Icenowy Zheng <uwu@icenowy.me> wrote: > > As the special handling of edge-triggered interrupts are defined in the > PLIC spec, we can assume it's not a quirk, but a feature of the PLIC > spec; thus making it a quirk and use quirk-based codepath is not so > necessary. It *is* necessary. > > Move to a #interrupt-cells-based practice which will allow both device > trees without interrupt flags and with interrupt flags work for all > compatible strings. No. You're tying together two unrelated concepts: - Edges get dropped in some implementations (and only some). You can argue that the architecture allows it, but I see it is an implementation bug. - The need for expressing additional information in the interrupt specifier is not necessarily related to the above. Other interrupt controllers use extra cells to encode the interrupt affinity, for example. I want these two things to be kept separate. Otherwise, once we get some fancy ACPI support for RISCV (no, please...), we'll have to redo the whole thing... > In addition, this addresses a stable version DT binding violation -- > Linux v5.19 comes with "thead,c900-plic" with #interrupt-cells defined to > be 1 instead of 2, this commit will allow DTs that complies to Linux > v5.19 binding work (although no such DT is devliered to the public now). *That* is what should get fixed. Thanks, M.
在 2022-11-22星期二的 17:28 +0000,Marc Zyngier写道: > On Mon, 21 Nov 2022 04:20:26 +0000, > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > As the special handling of edge-triggered interrupts are defined in > > the > > PLIC spec, we can assume it's not a quirk, but a feature of the > > PLIC > > spec; thus making it a quirk and use quirk-based codepath is not so > > necessary. > > It *is* necessary. > > > > > Move to a #interrupt-cells-based practice which will allow both > > device > > trees without interrupt flags and with interrupt flags work for all > > compatible strings. > > No. You're tying together two unrelated concepts: > > - Edges get dropped in some implementations (and only some). You can > argue that the architecture allows it, but I see it is an > implementation bug. As the specification allows it, it's not an implementation bug -- and for those which do not show this problem, it's possible that it's just all using the same trigger type (e.g. Rocket). > > - The need for expressing additional information in the interrupt > specifier is not necessarily related to the above. Other interrupt > controllers use extra cells to encode the interrupt affinity, for > example. I think in these situations, if the interrupt controller does not contain any special handling for edge interrupts, we can just describe them as level ones in SW. > > I want these two things to be kept separate. Otherwise, once we get > some fancy ACPI support for RISCV (no, please...), we'll have to redo > the whole thing... > > > In addition, this addresses a stable version DT binding violation - > > - > > Linux v5.19 comes with "thead,c900-plic" with #interrupt-cells > > defined to > > be 1 instead of 2, this commit will allow DTs that complies to > > Linux > > v5.19 binding work (although no such DT is devliered to the public > > now). > > *That* is what should get fixed. Supporting all stable versions' DT binding is our promise, I think. > > Thanks, > > M. >
On Wed, 23 Nov 2022 12:38:56 +0000, Icenowy Zheng <uwu@icenowy.me> wrote: > > 在 2022-11-22星期二的 17:28 +0000,Marc Zyngier写道: > > On Mon, 21 Nov 2022 04:20:26 +0000, > > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > > As the special handling of edge-triggered interrupts are defined in > > > the > > > PLIC spec, we can assume it's not a quirk, but a feature of the > > > PLIC > > > spec; thus making it a quirk and use quirk-based codepath is not so > > > necessary. > > > > It *is* necessary. > > > > > > > > Move to a #interrupt-cells-based practice which will allow both > > > device > > > trees without interrupt flags and with interrupt flags work for all > > > compatible strings. > > > > No. You're tying together two unrelated concepts: > > > > - Edges get dropped in some implementations (and only some). You can > > argue that the architecture allows it, but I see it is an > > implementation bug. > > As the specification allows it, it's not an implementation bug -- and > for those which do not show this problem, it's possible that it's just > all using the same trigger type (e.g. Rocket). What are you against? The fact that this is flagged as a quirk? Honestly, I don't care about that. If we can fold all implementations into the same scheme, that's fine by me. > > > > > - The need for expressing additional information in the interrupt > > specifier is not necessarily related to the above. Other interrupt > > controllers use extra cells to encode the interrupt affinity, for > > example. > > I think in these situations, if the interrupt controller does not > contain any special handling for edge interrupts, we can just describe > them as level ones in SW. No, that's utterly wrong. We don't describe an edge as level. Ever. > > > > > I want these two things to be kept separate. Otherwise, once we get > > some fancy ACPI support for RISCV (no, please...), we'll have to redo > > the whole thing... > > > > > In addition, this addresses a stable version DT binding violation - > > > - > > > Linux v5.19 comes with "thead,c900-plic" with #interrupt-cells > > > defined to > > > be 1 instead of 2, this commit will allow DTs that complies to > > > Linux > > > v5.19 binding work (although no such DT is devliered to the public > > > now). > > > > *That* is what should get fixed. > > Supporting all stable versions' DT binding is our promise, I think. Absolutely. And I'm asking you to fix it. And only that. Thanks, M.
在 2022-11-23星期三的 13:13 +0000,Marc Zyngier写道: > On Wed, 23 Nov 2022 12:38:56 +0000, > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > 在 2022-11-22星期二的 17:28 +0000,Marc Zyngier写道: > > > On Mon, 21 Nov 2022 04:20:26 +0000, > > > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > > > > As the special handling of edge-triggered interrupts are > > > > defined in > > > > the > > > > PLIC spec, we can assume it's not a quirk, but a feature of the > > > > PLIC > > > > spec; thus making it a quirk and use quirk-based codepath is > > > > not so > > > > necessary. > > > > > > It *is* necessary. > > > > > > > > > > > Move to a #interrupt-cells-based practice which will allow both > > > > device > > > > trees without interrupt flags and with interrupt flags work for > > > > all > > > > compatible strings. > > > > > > No. You're tying together two unrelated concepts: > > > > > > - Edges get dropped in some implementations (and only some). You > > > can > > > argue that the architecture allows it, but I see it is an > > > implementation bug. > > > > As the specification allows it, it's not an implementation bug -- > > and > > for those which do not show this problem, it's possible that it's > > just > > all using the same trigger type (e.g. Rocket). > > What are you against? The fact that this is flagged as a quirk? > Honestly, I don't care about that. If we can fold all implementations > into the same scheme, that's fine by me. Then what should I do? > > > > > > > > > - The need for expressing additional information in the interrupt > > > specifier is not necessarily related to the above. Other > > > interrupt > > > controllers use extra cells to encode the interrupt affinity, > > > for > > > example. > > > > I think in these situations, if the interrupt controller does not > > contain any special handling for edge interrupts, we can just > > describe > > them as level ones in SW. > > No, that's utterly wrong. We don't describe an edge as level. Ever. > > > > > > > > > I want these two things to be kept separate. Otherwise, once we > > > get > > > some fancy ACPI support for RISCV (no, please...), we'll have to > > > redo > > > the whole thing... > > > > > > > In addition, this addresses a stable version DT binding > > > > violation - > > > > - > > > > Linux v5.19 comes with "thead,c900-plic" with #interrupt-cells > > > > defined to > > > > be 1 instead of 2, this commit will allow DTs that complies to > > > > Linux > > > > v5.19 binding work (although no such DT is devliered to the > > > > public > > > > now). > > > > > > *That* is what should get fixed. > > > > Supporting all stable versions' DT binding is our promise, I think. > > Absolutely. And I'm asking you to fix it. And only that. Then what should I do? Mask this as another quirk that is only applicable to c900-plic? Sounds more crazy... > > Thanks, > > M. >
On Wed, 23 Nov 2022 13:16:01 +0000, Icenowy Zheng <uwu@icenowy.me> wrote: > > 在 2022-11-23星期三的 13:13 +0000,Marc Zyngier写道: > > On Wed, 23 Nov 2022 12:38:56 +0000, > > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > > 在 2022-11-22星期二的 17:28 +0000,Marc Zyngier写道: > > > > On Mon, 21 Nov 2022 04:20:26 +0000, > > > > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > > > > > > As the special handling of edge-triggered interrupts are > > > > > defined in > > > > > the > > > > > PLIC spec, we can assume it's not a quirk, but a feature of the > > > > > PLIC > > > > > spec; thus making it a quirk and use quirk-based codepath is > > > > > not so > > > > > necessary. > > > > > > > > It *is* necessary. > > > > > > > > > > > > > > Move to a #interrupt-cells-based practice which will allow both > > > > > device > > > > > trees without interrupt flags and with interrupt flags work for > > > > > all > > > > > compatible strings. > > > > > > > > No. You're tying together two unrelated concepts: > > > > > > > > - Edges get dropped in some implementations (and only some). You > > > > can > > > > argue that the architecture allows it, but I see it is an > > > > implementation bug. > > > > > > As the specification allows it, it's not an implementation bug -- > > > and > > > for those which do not show this problem, it's possible that it's > > > just > > > all using the same trigger type (e.g. Rocket). > > > > What are you against? The fact that this is flagged as a quirk? > > Honestly, I don't care about that. If we can fold all implementations > > into the same scheme, that's fine by me. > > Then what should I do? Make all edge-triggered interrupts use the edge flow. > > > > > > > > > > > > > > - The need for expressing additional information in the interrupt > > > > specifier is not necessarily related to the above. Other > > > > interrupt > > > > controllers use extra cells to encode the interrupt affinity, > > > > for > > > > example. > > > > > > I think in these situations, if the interrupt controller does not > > > contain any special handling for edge interrupts, we can just > > > describe > > > them as level ones in SW. > > > > No, that's utterly wrong. We don't describe an edge as level. Ever. > > > > > > > > > > > > > I want these two things to be kept separate. Otherwise, once we > > > > get > > > > some fancy ACPI support for RISCV (no, please...), we'll have to > > > > redo > > > > the whole thing... > > > > > > > > > In addition, this addresses a stable version DT binding > > > > > violation - > > > > > - > > > > > Linux v5.19 comes with "thead,c900-plic" with #interrupt-cells > > > > > defined to > > > > > be 1 instead of 2, this commit will allow DTs that complies to > > > > > Linux > > > > > v5.19 binding work (although no such DT is devliered to the > > > > > public > > > > > now). > > > > > > > > *That* is what should get fixed. > > > > > > Supporting all stable versions' DT binding is our promise, I think. > > > > Absolutely. And I'm asking you to fix it. And only that. > > Then what should I do? Mask this as another quirk that is only > applicable to c900-plic? No. Make interrupts with a single cell use the level flow. > Sounds more crazy... There is obviously no accounting for taste. M.
在 2022-11-23星期三的 13:31 +0000,Marc Zyngier写道: > On Wed, 23 Nov 2022 13:16:01 +0000, > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > 在 2022-11-23星期三的 13:13 +0000,Marc Zyngier写道: > > > On Wed, 23 Nov 2022 12:38:56 +0000, > > > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > > > > 在 2022-11-22星期二的 17:28 +0000,Marc Zyngier写道: > > > > > On Mon, 21 Nov 2022 04:20:26 +0000, > > > > > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > > > > > > > > As the special handling of edge-triggered interrupts are > > > > > > defined in > > > > > > the > > > > > > PLIC spec, we can assume it's not a quirk, but a feature of > > > > > > the > > > > > > PLIC > > > > > > spec; thus making it a quirk and use quirk-based codepath > > > > > > is > > > > > > not so > > > > > > necessary. > > > > > > > > > > It *is* necessary. > > > > > > > > > > > > > > > > > Move to a #interrupt-cells-based practice which will allow > > > > > > both > > > > > > device > > > > > > trees without interrupt flags and with interrupt flags work > > > > > > for > > > > > > all > > > > > > compatible strings. > > > > > > > > > > No. You're tying together two unrelated concepts: > > > > > > > > > > - Edges get dropped in some implementations (and only some). > > > > > You > > > > > can > > > > > argue that the architecture allows it, but I see it is an > > > > > implementation bug. > > > > > > > > As the specification allows it, it's not an implementation bug > > > > -- > > > > and > > > > for those which do not show this problem, it's possible that > > > > it's > > > > just > > > > all using the same trigger type (e.g. Rocket). > > > > > > What are you against? The fact that this is flagged as a quirk? > > > Honestly, I don't care about that. If we can fold all > > > implementations > > > into the same scheme, that's fine by me. > > > > Then what should I do? > > Make all edge-triggered interrupts use the edge flow. > > > > > > > > > > > > > > > > > > > > - The need for expressing additional information in the > > > > > interrupt > > > > > specifier is not necessarily related to the above. Other > > > > > interrupt > > > > > controllers use extra cells to encode the interrupt > > > > > affinity, > > > > > for > > > > > example. > > > > > > > > I think in these situations, if the interrupt controller does > > > > not > > > > contain any special handling for edge interrupts, we can just > > > > describe > > > > them as level ones in SW. > > > > > > No, that's utterly wrong. We don't describe an edge as level. > > > Ever. > > > > > > > > > > > > > > > > > I want these two things to be kept separate. Otherwise, once > > > > > we > > > > > get > > > > > some fancy ACPI support for RISCV (no, please...), we'll have > > > > > to > > > > > redo > > > > > the whole thing... > > > > > > > > > > > In addition, this addresses a stable version DT binding > > > > > > violation - > > > > > > - > > > > > > Linux v5.19 comes with "thead,c900-plic" with #interrupt- > > > > > > cells > > > > > > defined to > > > > > > be 1 instead of 2, this commit will allow DTs that complies > > > > > > to > > > > > > Linux > > > > > > v5.19 binding work (although no such DT is devliered to the > > > > > > public > > > > > > now). > > > > > > > > > > *That* is what should get fixed. > > > > > > > > Supporting all stable versions' DT binding is our promise, I > > > > think. > > > > > > Absolutely. And I'm asking you to fix it. And only that. > > > > Then what should I do? Mask this as another quirk that is only > > applicable to c900-plic? > > No. Make interrupts with a single cell use the level flow. This sounds exactly like what we do in this patch now. Or, should we keep the quirk, and require both a flag cell containing IRQ_TYPE_EDGE_RISING and an interrupt controller that matches the quirk to use the special codepath for edge interrupts? > > > Sounds more crazy... > > There is obviously no accounting for taste. > > M. >
On Wed, 23 Nov 2022 13:35:58 +0000, Icenowy Zheng <uwu@icenowy.me> wrote: > > 在 2022-11-23星期三的 13:31 +0000,Marc Zyngier写道: > > On Wed, 23 Nov 2022 13:16:01 +0000, > > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > > 在 2022-11-23星期三的 13:13 +0000,Marc Zyngier写道: > > > > On Wed, 23 Nov 2022 12:38:56 +0000, > > > > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > > > > > > 在 2022-11-22星期二的 17:28 +0000,Marc Zyngier写道: > > > > > > On Mon, 21 Nov 2022 04:20:26 +0000, > > > > > > Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > > > > > > > > > > As the special handling of edge-triggered interrupts are > > > > > > > defined in > > > > > > > the > > > > > > > PLIC spec, we can assume it's not a quirk, but a feature of > > > > > > > the > > > > > > > PLIC > > > > > > > spec; thus making it a quirk and use quirk-based codepath > > > > > > > is > > > > > > > not so > > > > > > > necessary. > > > > > > > > > > > > It *is* necessary. > > > > > > > > > > > > > > > > > > > > Move to a #interrupt-cells-based practice which will allow > > > > > > > both > > > > > > > device > > > > > > > trees without interrupt flags and with interrupt flags work > > > > > > > for > > > > > > > all > > > > > > > compatible strings. > > > > > > > > > > > > No. You're tying together two unrelated concepts: > > > > > > > > > > > > - Edges get dropped in some implementations (and only some). > > > > > > You > > > > > > can > > > > > > argue that the architecture allows it, but I see it is an > > > > > > implementation bug. > > > > > > > > > > As the specification allows it, it's not an implementation bug > > > > > -- > > > > > and > > > > > for those which do not show this problem, it's possible that > > > > > it's > > > > > just > > > > > all using the same trigger type (e.g. Rocket). > > > > > > > > What are you against? The fact that this is flagged as a quirk? > > > > Honestly, I don't care about that. If we can fold all > > > > implementations > > > > into the same scheme, that's fine by me. > > > > > > Then what should I do? > > > > Make all edge-triggered interrupts use the edge flow. > > > > > > > > > > > > > > > > > > > > > > > > > > - The need for expressing additional information in the > > > > > > interrupt > > > > > > specifier is not necessarily related to the above. Other > > > > > > interrupt > > > > > > controllers use extra cells to encode the interrupt > > > > > > affinity, > > > > > > for > > > > > > example. > > > > > > > > > > I think in these situations, if the interrupt controller does > > > > > not > > > > > contain any special handling for edge interrupts, we can just > > > > > describe > > > > > them as level ones in SW. > > > > > > > > No, that's utterly wrong. We don't describe an edge as level. > > > > Ever. > > > > > > > > > > > > > > > > > > > > > I want these two things to be kept separate. Otherwise, once > > > > > > we > > > > > > get > > > > > > some fancy ACPI support for RISCV (no, please...), we'll have > > > > > > to > > > > > > redo > > > > > > the whole thing... > > > > > > > > > > > > > In addition, this addresses a stable version DT binding > > > > > > > violation - > > > > > > > - > > > > > > > Linux v5.19 comes with "thead,c900-plic" with #interrupt- > > > > > > > cells > > > > > > > defined to > > > > > > > be 1 instead of 2, this commit will allow DTs that complies > > > > > > > to > > > > > > > Linux > > > > > > > v5.19 binding work (although no such DT is devliered to the > > > > > > > public > > > > > > > now). > > > > > > > > > > > > *That* is what should get fixed. > > > > > > > > > > Supporting all stable versions' DT binding is our promise, I > > > > > think. > > > > > > > > Absolutely. And I'm asking you to fix it. And only that. > > > > > > Then what should I do? Mask this as another quirk that is only > > > applicable to c900-plic? > > > > No. Make interrupts with a single cell use the level flow. > > This sounds exactly like what we do in this patch now. No. Really not. If anything, you add more pointless crap. > Or, should we keep the quirk, and require both a flag cell containing > IRQ_TYPE_EDGE_RISING and an interrupt controller that matches the quirk > to use the special codepath for edge interrupts? This is becoming tedious. M. diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 2f4784860df5..6774ae19ad0b 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -60,13 +60,10 @@ #define PLIC_DISABLE_THRESHOLD 0x7 #define PLIC_ENABLE_THRESHOLD 0 -#define PLIC_QUIRK_EDGE_INTERRUPT 0 - struct plic_priv { struct cpumask lmask; struct irq_domain *irqdomain; void __iomem *regs; - unsigned long plic_quirks; }; struct plic_handler { @@ -208,9 +205,6 @@ static int plic_irq_set_type(struct irq_data *d, unsigned int type) { struct plic_priv *priv = irq_data_get_irq_chip_data(d); - if (!test_bit(PLIC_QUIRK_EDGE_INTERRUPT, &priv->plic_quirks)) - return IRQ_SET_MASK_OK_NOCOPY; - switch (type) { case IRQ_TYPE_EDGE_RISING: irq_set_chip_handler_name_locked(d, &plic_edge_chip, @@ -244,9 +238,7 @@ static int plic_irq_domain_translate(struct irq_domain *d, unsigned long *hwirq, unsigned int *type) { - struct plic_priv *priv = d->host_data; - - if (test_bit(PLIC_QUIRK_EDGE_INTERRUPT, &priv->plic_quirks)) + if (irq_fwspec->param_count >= 2) return irq_domain_translate_twocell(d, fwspec, hwirq, type); return irq_domain_translate_onecell(d, fwspec, hwirq, type); @@ -335,9 +327,8 @@ static int plic_starting_cpu(unsigned int cpu) return 0; } -static int __init __plic_init(struct device_node *node, - struct device_node *parent, - unsigned long plic_quirks) +static int __init plic_init(struct device_node *node, + struct device_node *parent) { int error = 0, nr_contexts, nr_handlers = 0, i; u32 nr_irqs; @@ -348,8 +339,6 @@ static int __init __plic_init(struct device_node *node, if (!priv) return -ENOMEM; - priv->plic_quirks = plic_quirks; - priv->regs = of_iomap(node, 0); if (WARN_ON(!priv->regs)) { error = -EIO; @@ -471,20 +460,7 @@ static int __init __plic_init(struct device_node *node, return error; } -static int __init plic_init(struct device_node *node, - struct device_node *parent) -{ - return __plic_init(node, parent, 0); -} - IRQCHIP_DECLARE(sifive_plic, "sifive,plic-1.0.0", plic_init); IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */ - -static int __init plic_edge_init(struct device_node *node, - struct device_node *parent) -{ - return __plic_init(node, parent, BIT(PLIC_QUIRK_EDGE_INTERRUPT)); -} - -IRQCHIP_DECLARE(andestech_nceplic100, "andestech,nceplic100", plic_edge_init); -IRQCHIP_DECLARE(thead_c900_plic, "thead,c900-plic", plic_edge_init); +IRQCHIP_DECLARE(andestech_nceplic100, "andestech,nceplic100", plic_init); +IRQCHIP_DECLARE(thead_c900_plic, "thead,c900-plic", plic_init);
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 2f4784860df5..219e4f1b62f0 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -67,6 +67,7 @@ struct plic_priv { struct irq_domain *irqdomain; void __iomem *regs; unsigned long plic_quirks; + u32 interrupt_cells; }; struct plic_handler { @@ -208,7 +209,7 @@ static int plic_irq_set_type(struct irq_data *d, unsigned int type) { struct plic_priv *priv = irq_data_get_irq_chip_data(d); - if (!test_bit(PLIC_QUIRK_EDGE_INTERRUPT, &priv->plic_quirks)) + if (priv->interrupt_cells < 2) return IRQ_SET_MASK_OK_NOCOPY; switch (type) { @@ -246,7 +247,7 @@ static int plic_irq_domain_translate(struct irq_domain *d, { struct plic_priv *priv = d->host_data; - if (test_bit(PLIC_QUIRK_EDGE_INTERRUPT, &priv->plic_quirks)) + if (priv->interrupt_cells >= 2) return irq_domain_translate_twocell(d, fwspec, hwirq, type); return irq_domain_translate_onecell(d, fwspec, hwirq, type); @@ -357,6 +358,10 @@ static int __init __plic_init(struct device_node *node, } error = -EINVAL; + of_property_read_u32(node, "#interrupt-cells", &priv->interrupt_cells); + if (WARN_ON(!priv->interrupt_cells)) + goto out_iounmap; + of_property_read_u32(node, "riscv,ndev", &nr_irqs); if (WARN_ON(!nr_irqs)) goto out_iounmap; @@ -479,12 +484,5 @@ static int __init plic_init(struct device_node *node, IRQCHIP_DECLARE(sifive_plic, "sifive,plic-1.0.0", plic_init); IRQCHIP_DECLARE(riscv_plic0, "riscv,plic0", plic_init); /* for legacy systems */ - -static int __init plic_edge_init(struct device_node *node, - struct device_node *parent) -{ - return __plic_init(node, parent, BIT(PLIC_QUIRK_EDGE_INTERRUPT)); -} - -IRQCHIP_DECLARE(andestech_nceplic100, "andestech,nceplic100", plic_edge_init); -IRQCHIP_DECLARE(thead_c900_plic, "thead,c900-plic", plic_edge_init); +IRQCHIP_DECLARE(andestech_nceplic100, "andestech,nceplic100", plic_init); +IRQCHIP_DECLARE(thead_c900_plic, "thead,c900-plic", plic_init);
As the special handling of edge-triggered interrupts are defined in the PLIC spec, we can assume it's not a quirk, but a feature of the PLIC spec; thus making it a quirk and use quirk-based codepath is not so necessary. Move to a #interrupt-cells-based practice which will allow both device trees without interrupt flags and with interrupt flags work for all compatible strings. In addition, this addresses a stable version DT binding violation -- Linux v5.19 comes with "thead,c900-plic" with #interrupt-cells defined to be 1 instead of 2, this commit will allow DTs that complies to Linux v5.19 binding work (although no such DT is devliered to the public now). Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- drivers/irqchip/irq-sifive-plic.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)