[PATCH v5 0/4] Per-parent domains for realtek-rtl IRQ driver

Sander Vanheule posted 4 patches 4 years, 4 months ago
.../realtek,rtl-intc.yaml                     |  82 +++++--
drivers/irqchip/irq-realtek-rtl.c             | 231 ++++++++++++------
2 files changed, 221 insertions(+), 92 deletions(-)
[PATCH v5 0/4] Per-parent domains for realtek-rtl IRQ driver
Posted by Sander Vanheule 4 years, 4 months ago
The original implementation for this interrupt controller/router used
an interrupt-map parser to determine which parent interrupts were
present. However, this controller is not transparent, so a list of
parent interrupts seems more appropriate, while also getting rid of the
assumed routing to parent interrupts.

Additionally, N real cascaded interrupts are implemented, instead of
handling all input interrupts with one cascaded interrupt. Otherwise it
is possible that the priority of the parent interrupts is not respected.

Changes since v4:
Link: https://lore.kernel.org/all/cover.1644165421.git.sander@svanheule.net/

- Add Rob's Reviewed-by
- Use irq_domain_add_linear instead of irq_domain_add_simple
- Drop 'inline' specifiers from static functions
- Drop WARN in intc_select() to only warn once for old bindings

Changes since v3:
Link: https://lore.kernel.org/all/cover.1641739718.git.sander@svanheule.net/

- Patches with fixes were merged, so these are no longer included.
- Update the devicetree changes to more clearly indicate the controller
  is not transparent.

Changes since v2 (RFC):
Link: https://lore.kernel.org/all/cover.1640548009.git.sander@svanheule.net/

- Define new, two-part compatibles for devicetree bindings. The existing format
  is kept for the old one-part compatible, but deprecated. New compatibles will
  require a different way of specifying parent interrupts and interrupt routing.
- Add change to handle all pending SoC interrupts in one go.

Changes since v1 (RFC):
Link: https://lore.kernel.org/all/cover.1640261161.git.sander@svanheule.net/

- Split some of the changes to limit the patch scope to one issue.
- Dropped some small (spurious or unneeded) changes
- Instead of dropping/replacing interrupt-map, the last patches now provide an
  implementation that amends the current situtation.

Sander Vanheule (4):
  irqchip/realtek-rtl: use irq_domain_add_linear
  dt-bindings: interrupt-controller: realtek,rtl-intc: require parents
  irqchip/realtek-rtl: use parent interrupts
  irqchip/realtek-rtl: use per-parent domains

 .../realtek,rtl-intc.yaml                     |  82 +++++--
 drivers/irqchip/irq-realtek-rtl.c             | 231 ++++++++++++------
 2 files changed, 221 insertions(+), 92 deletions(-)

-- 
2.35.1

Re: [PATCH v5 0/4] Per-parent domains for realtek-rtl IRQ driver
Posted by Marc Zyngier 4 years, 4 months ago
On Mon, 14 Feb 2022 18:56:57 +0000,
Sander Vanheule <sander@svanheule.net> wrote:
> 
> The original implementation for this interrupt controller/router used
> an interrupt-map parser to determine which parent interrupts were
> present. However, this controller is not transparent, so a list of
> parent interrupts seems more appropriate, while also getting rid of the
> assumed routing to parent interrupts.
> 
> Additionally, N real cascaded interrupts are implemented, instead of
> handling all input interrupts with one cascaded interrupt. Otherwise it
> is possible that the priority of the parent interrupts is not respected.

My original question[1] still stands. An old kernel breaks with a new
DT. I am not convinced that this is an acceptable outcome.

	M.

[1] https://lore.kernel.org/all/874k585efy.wl-maz@kernel.org

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH v5 0/4] Per-parent domains for realtek-rtl IRQ driver
Posted by Sander Vanheule 4 years, 4 months ago
Hi Marc,

On Tue, 2022-02-15 at 12:09 +0000, Marc Zyngier wrote:
> On Mon, 14 Feb 2022 18:56:57 +0000,
> Sander Vanheule <sander@svanheule.net> wrote:
> > 
> > The original implementation for this interrupt controller/router used
> > an interrupt-map parser to determine which parent interrupts were
> > present. However, this controller is not transparent, so a list of
> > parent interrupts seems more appropriate, while also getting rid of the
> > assumed routing to parent interrupts.
> > 
> > Additionally, N real cascaded interrupts are implemented, instead of
> > handling all input interrupts with one cascaded interrupt. Otherwise it
> > is possible that the priority of the parent interrupts is not respected.
> 
> My original question[1] still stands. An old kernel breaks with a new
> DT. I am not convinced that this is an acceptable outcome.
> 
>         M.
> 
> [1] https://lore.kernel.org/all/874k585efy.wl-maz@kernel.org


My apologies for the delay in replying, although I suppose the lack of response
from others perhaps indicates that there is little interest maintaining old
kernel/new DT compatibility for this hardware. John has previously argued in
favour of breaking compatibility [2].

Chances of someone running a vanilla kernel build on this hardware are close to
zero at this moment. The most important part, the internal ethernet switch, is
only supported with out-of-tree patches. If patches can be included on an old
(LTS) kernel to provide networking support, then patches can be included to be
compatible with a new DT specification for the interrupts as well. OpenWrt does
exactly this: use an old (5.10) kernel with new upstream features backported.

The binding could be adjusted to allow (but deprecate) interrupt-map for the new
two-part compatibles. This would require a new DT to both specify two-cell
interrupt specifiers, and an equivalent interrupt-map definition to ensure
perfect two-way compatibility. This duplicated info would need to be maintained
for years however, as LTS kernels stay around for a long time. In my opinion,
breaking compatibility with old kernels would allow us to move forward with a
cleaner driver and binding.

Best,
Sander

[2]
https://lore.kernel.org/all/9c169aad-3c7b-2ffb-90a2-1ca791a3f411@phrozen.org/