Re: [RFC PATCH devicetree 00/10] Do something about ls-extirq interrupt-map breakage

[Marc Zyngier <maz@kernel.org>]

On Tue, 14 Dec 2021 01:37:50 +0000,
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> Currently the ls-extirq driver's use of the "interrupt-map" property is
> double-broken:
> - once by Rob Herring's commit 869f0ec048dc ("arm64: dts: freescale: Fix
>   'interrupt-map' parent address cells")
> - twice by Marc Zyngier's commit 041284181226 ("of/irq: Allow matching
>   of an interrupt-map local to an interrupt controller"), later revised,
>   not very elegantly, through commit de4adddcbcc2 ("of/irq: Add a quirk

Elegance is, I'm afraid to say, bloody overrated when dealing with
this sort of crap.


>   for controllers with their own definition of interrupt-map"). So this
>   part works but we're on an offender list.

Define 'part works'. Either it does, or it doesn't. There is no middle
ground here.

> 
> Mark suggests that the problem may lie with the ls-extirq driver, and
> its interpretation of the "interrupt-map" property, to be exact.

s/Mark/Marc/, unless you are talking about someone else (who?).

> 
> This set of changes attempts to make the problem smaller by using a
> vendor-specific name for the property, and reverts Rob's patch because
> similarity with "interrupt-map" isn't actually a desirable feature after
> all, it seems.
> 
> Vladimir Oltean (10):
>   irqchip/ls-extirq: rename "interrupt-map" OF property to
>     "fsl,extirq-map"
>   Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address
>     cells"
>   dt-bindings: ls-extirq: replace "interrupt-map" documentation with
>     "fsl,extirq-map"
>   arm64: dts: ls1043a: rename the "interrupt-map" of the extirq node to
>     "fsl,extirq-map"
>   arm64: dts: ls1046a: rename the "interrupt-map" of the extirq node to
>     "fsl,extirq-map"
>   arm64: dts: ls1088a: rename the "interrupt-map" of the extirq node to
>     "fsl,extirq-map"
>   arm64: dts: ls208xa: rename the "interrupt-map" of the extirq node to
>     "fsl,extirq-map"
>   arm64: dts: lx2160a: rename the "interrupt-map" of the extirq node to
>     "fsl,extirq-map"
>   ARM: dts: ls1021a: rename the "interrupt-map" of the extirq node to
>     "fsl,extirq-map"
>   dt-bindings: ls-extirq: add a YAML schema for the validator
> 
>  .../interrupt-controller/fsl,ls-extirq.txt    |  53 ---------
>  .../interrupt-controller/fsl,ls-extirq.yaml   | 110 ++++++++++++++++++
>  arch/arm/boot/dts/ls1021a.dtsi                |   3 +-
>  .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi |   3 +-
>  .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi |   3 +-
>  .../arm64/boot/dts/freescale/fsl-ls1088a.dtsi |  27 +++--
>  .../arm64/boot/dts/freescale/fsl-ls208xa.dtsi |  27 +++--
>  .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi |  27 +++--
>  drivers/irqchip/irq-ls-extirq.c               |  12 +-
>  9 files changed, 161 insertions(+), 104 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml

This is totally pointless. These machines have been in the wild for
years, and existing DTs will be there *forever*. The very notion of
'backporting' DT changes is totally ludicrous when it is some firmware
(ATF, u-boot, or something else *that isn't under your control*) that
provides the DT. It also breaks backward compatibility (old kernel
with new DT), which is just as important. Why do you think I went the
elegance-deprived route and added a quirk?

So no, I'm not taking the irqchip changes, as most of this churn
serves no purpose. The revert of 869f0ec048dc is the only thing that
makes some sense.

	M.

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