Re: [PATCH] irqchip/sifive-plic: drop quirk for two-cell variant

[Marc Zyngier]

On Wed, 23 Nov 2022 13:16:01 +0000,
Icenowy Zheng <uwu@xxxxxxxxxx> wrote:
> 
> 在 2022-11-23星期三的 13:13 +0000，Marc Zyngier写道：
> > On Wed, 23 Nov 2022 12:38:56 +0000,
> > Icenowy Zheng <uwu@xxxxxxxxxx> wrote:
> > > 
> > > 在 2022-11-22星期二的 17:28 +0000，Marc Zyngier写道：
> > > > On Mon, 21 Nov 2022 04:20:26 +0000,
> > > > Icenowy Zheng <uwu@xxxxxxxxxx> 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.

-- 
Without deviation from the norm, progress is not possible.