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

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

[Vladimir Oltean]

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

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

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

-- 
2.25.1

[RFC PATCH devicetree 01/10] irqchip/ls-extirq: rename "interrupt-map" OF property to "fsl,extirq-map"

[Vladimir Oltean]

This OF property was supposed to be named "fsl,extirq-map" since the
first patch submissions, but at Rob Herring's suggestion it was named
"interrupt-map":
https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/

At that time, the "interrupt-map" was ignored by the core for OF nodes
that also had an "interrupt-controller" property, but that changed with
commit 041284181226 ("of/irq: Allow matching of an interrupt-map local
to an interrupt controller"), which made the consumer drivers of the
ls-extirq break. To work around this breakage, the OF bindings for IRQs
have introduced a table of "OF IRQ interrupt-map abusers".
This can be seen in commit de4adddcbcc2 ("of/irq: Add a quirk for
controllers with their own definition of interrupt-map").

To stop being abusers, let's go back to the original form of these
bindings, before Rob's review. Compatibility will be kept with the
current abusive bindings for a few more kernel cycles, to give people
some time to update. But to also give them an incentive, print a warning
that the support for "interrupt-map" will be removed. This will be seen
when running a new kernel with an old device tree.

While the breakage was introduced relatively recently, the device tree
changes are intended to backport stable kernels for quicker conversion
to an acceptable set of bindings, and this driver will need to support
the updated stable bindings.

Fixes: 0dcd9f872769 ("irqchip: Add support for Layerscape external interrupt lines")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/irqchip/irq-ls-extirq.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-ls-extirq.c b/drivers/irqchip/irq-ls-extirq.c
index 853b3972dbe7..b6ecc5e3472f 100644
--- a/drivers/irqchip/irq-ls-extirq.c
+++ b/drivers/irqchip/irq-ls-extirq.c
@@ -101,9 +101,15 @@ ls_extirq_parse_map(struct ls_extirq_data *priv, struct device_node *node)
 	u32 mapsize;
 	int ret;
 
-	map = of_get_property(node, "interrupt-map", &mapsize);
-	if (!map)
-		return -ENOENT;
+	map = of_get_property(node, "fsl,extirq-map", &mapsize);
+	if (!map) {
+		map = of_get_property(node, "interrupt-map", &mapsize);
+		if (!map)
+			return -ENOENT;
+
+		pr_warn("\"interrupt-map\" is a reserved OF property, and support for it will be removed. Please use \"fsl,extirq-map\" instead.\n");
+	}
+
 	if (mapsize % sizeof(*map))
 		return -EINVAL;
 	mapsize /= sizeof(*map);
-- 
2.25.1

[RFC PATCH devicetree 02/10] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"

[Vladimir Oltean]

This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
updated the expected device tree binding format for the ls-extirq
driver, without also updating the parsing code to the new format.

While the fsl-extirq driver may have a non-standard format (one that
doesn't take into consideration the #address-cells of the interrupt
parent), it also has no business in making use of the "interrupt-map"
property in the first place. So the argument for the blamed patch was
kind of moot in the first place.

This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
previous bindings. A follow-up change will also rename "interrupt-map"
to "fsl,extirq-map" to prevent any further automatic conversions from
causing breakage. Keeping the revert as a separate change allows those
renaming changes to look the same on kernels that include the reverted
patch as on kernels that don't.

Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map' parent address cells")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 24 +++++++++----------
 .../arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 24 +++++++++----------
 .../arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 24 +++++++++----------
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index f891ef6a3754..605072317243 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -241,18 +241,18 @@ extirq: interrupt-controller@14 {
 				interrupt-controller;
 				reg = <0x14 4>;
 				interrupt-map =
-					<0 0 &gic 0 0 GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
-					<1 0 &gic 0 0 GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
-					<2 0 &gic 0 0 GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
-					<3 0 &gic 0 0 GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
-					<4 0 &gic 0 0 GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
-					<5 0 &gic 0 0 GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
-					<6 0 &gic 0 0 GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
-					<7 0 &gic 0 0 GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
-					<8 0 &gic 0 0 GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
-					<9 0 &gic 0 0 GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
-					<10 0 &gic 0 0 GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
-					<11 0 &gic 0 0 GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+					<0 0 &gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+					<1 0 &gic GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+					<2 0 &gic GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+					<3 0 &gic GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+					<4 0 &gic GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+					<5 0 &gic GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
+					<6 0 &gic GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					<7 0 &gic GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+					<8 0 &gic GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+					<9 0 &gic GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
+					<10 0 &gic GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+					<11 0 &gic GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-map-mask = <0xffffffff 0x0>;
 			};
 		};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index 3cb9c21d2775..1282b61da8a5 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -293,18 +293,18 @@ extirq: interrupt-controller@14 {
 				interrupt-controller;
 				reg = <0x14 4>;
 				interrupt-map =
-					<0 0 &gic 0 0 GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
-					<1 0 &gic 0 0 GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
-					<2 0 &gic 0 0 GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
-					<3 0 &gic 0 0 GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
-					<4 0 &gic 0 0 GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
-					<5 0 &gic 0 0 GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
-					<6 0 &gic 0 0 GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
-					<7 0 &gic 0 0 GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
-					<8 0 &gic 0 0 GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
-					<9 0 &gic 0 0 GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
-					<10 0 &gic 0 0 GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
-					<11 0 &gic 0 0 GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+					<0 0 &gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+					<1 0 &gic GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+					<2 0 &gic GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+					<3 0 &gic GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+					<4 0 &gic GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+					<5 0 &gic GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
+					<6 0 &gic GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					<7 0 &gic GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+					<8 0 &gic GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+					<9 0 &gic GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
+					<10 0 &gic GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+					<11 0 &gic GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-map-mask = <0xffffffff 0x0>;
 			};
 		};
diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
index dc8661ebd1f6..c4b1a59ba424 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
@@ -680,18 +680,18 @@ extirq: interrupt-controller@14 {
 				interrupt-controller;
 				reg = <0x14 4>;
 				interrupt-map =
-					<0 0 &gic 0 0 GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
-					<1 0 &gic 0 0 GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
-					<2 0 &gic 0 0 GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
-					<3 0 &gic 0 0 GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
-					<4 0 &gic 0 0 GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
-					<5 0 &gic 0 0 GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
-					<6 0 &gic 0 0 GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
-					<7 0 &gic 0 0 GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
-					<8 0 &gic 0 0 GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
-					<9 0 &gic 0 0 GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
-					<10 0 &gic 0 0 GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
-					<11 0 &gic 0 0 GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+					<0 0 &gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+					<1 0 &gic GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+					<2 0 &gic GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+					<3 0 &gic GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+					<4 0 &gic GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+					<5 0 &gic GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
+					<6 0 &gic GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					<7 0 &gic GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+					<8 0 &gic GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+					<9 0 &gic GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
+					<10 0 &gic GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+					<11 0 &gic GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-map-mask = <0xffffffff 0x0>;
 			};
 		};
-- 
2.25.1

[RFC PATCH devicetree 03/10] dt-bindings: ls-extirq: replace "interrupt-map" documentation with "fsl,extirq-map"

[Vladimir Oltean]

This change does 3 things at once:
- documents the recently added "fsl,extirq-map" property, which replaces
  "interrupt-map"
- clarifies the format of the property whose name is changing
- hides the "interrupt-map" property, that the driver still supports for
  backwards compatibility reasons, although not for long.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../bindings/interrupt-controller/fsl,ls-extirq.txt | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
index 4d47df1a5c91..cddf1aa032be 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
@@ -18,9 +18,13 @@ Required properties:
 - reg: Specifies the Interrupt Polarity Control Register (INTPCR) in
   the SCFG or the External Interrupt Control Register (IRQCR) in
   the ISC.
-- interrupt-map: Specifies the mapping from external interrupts to GIC
-  interrupts.
-- interrupt-map-mask: Must be <0xffffffff 0>.
+- fsl,extirq-map: An array of elements through which the mapping between
+  external interrupts and GIC interrupts is specified. The first member of each
+  array element is the index of the extirq line. The second member must be
+  zero. The third member must be a phandle to the interrupt parent (the GIC).
+  The remaining number of members in an array element depends on the
+  #interrupt-cells property of the interrupt parent, and are used to specify
+  the parent interrupt.
 
 Example:
 	scfg: scfg@1570000 {
@@ -37,14 +41,13 @@ Example:
 			#address-cells = <0>;
 			interrupt-controller;
 			reg = <0x1ac 4>;
-			interrupt-map =
+			fsl,extirq-map =
 				<0 0 &gic GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
 				<1 0 &gic GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
 				<2 0 &gic GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>,
 				<3 0 &gic GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
 				<4 0 &gic GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
 				<5 0 &gic GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-map-mask = <0xffffffff 0x0>;
 		};
 	};
 
-- 
2.25.1

[RFC PATCH devicetree 04/10] arm64: dts: ls1043a: rename the "interrupt-map" of the extirq node to "fsl,extirq-map"

[Vladimir Oltean]

In the discussions here:
https://lore.kernel.org/all/CAMuHMdXM34nNz1nfowNqDvdsdg+d69Bo3_ufs6fbcq65iYd5-A@mail.gmail.com/T/#m3449390080c5a0c8f59b8f8286a87e31e093b98b
Marc Zyngier points out that the ls-extirq driver should have never used
the "interrupt-map" keyword for mapping its channels to GIC interrupts.

This change modifies the device trees for the LS1043A to use a
driver-specific OF property for that purpose. The change is intended to
target stable kernels, to accelerate the conversion and ultimate removal
of the improperly used "interrupt-map" property.

Note that "interrupt-map-mask" isn't needed now (nor was it needed
before, it was just there to keep an apparent compatibility in form with
the reserved "interrupt-map" property). So delete it.

Fixes: 3f8c61a567eb ("arm64: dts: ls1043a: add DT node for external interrupt lines")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 01b01e320411..62958b784171 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -322,7 +322,7 @@ extirq: interrupt-controller@1ac {
 				#address-cells = <0>;
 				interrupt-controller;
 				reg = <0x1ac 4>;
-				interrupt-map =
+				fsl,extirq-map =
 					<0 0 &gic GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
 					<1 0 &gic GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
 					<2 0 &gic GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
@@ -335,7 +335,6 @@ extirq: interrupt-controller@1ac {
 					<9 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
 					<10 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
 					<11 0 &gic GIC_SPI 151 IRQ_TYPE_LEVEL_HIGH>;
-				interrupt-map-mask = <0xffffffff 0x0>;
 			};
 		};
 
-- 
2.25.1

[RFC PATCH devicetree 05/10] arm64: dts: ls1046a: rename the "interrupt-map" of the extirq node to "fsl,extirq-map"

[Vladimir Oltean]

In the discussions here:
https://lore.kernel.org/all/CAMuHMdXM34nNz1nfowNqDvdsdg+d69Bo3_ufs6fbcq65iYd5-A@mail.gmail.com/T/#m3449390080c5a0c8f59b8f8286a87e31e093b98b
Marc Zyngier points out that the ls-extirq driver should have never used
the "interrupt-map" keyword for mapping its channels to GIC interrupts.

This change modifies the device trees for the LS1046A to use a
driver-specific OF property for that purpose. The change is intended to
target stable kernels, to accelerate the conversion and ultimate removal
of the improperly used "interrupt-map" property.

Note that "interrupt-map-mask" isn't needed now (nor was it needed
before, it was just there to keep an apparent compatibility in form with
the reserved "interrupt-map" property). So delete it.

Fixes: 7968344126e5 ("arm64: dts: ls1046a: add DT node for external interrupt lines")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 687fea6d8afa..c735469010b7 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -328,7 +328,7 @@ extirq: interrupt-controller@1ac {
 				#address-cells = <0>;
 				interrupt-controller;
 				reg = <0x1ac 4>;
-				interrupt-map =
+				fsl,extirq-map =
 					<0 0 &gic GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
 					<1 0 &gic GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
 					<2 0 &gic GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
@@ -341,7 +341,6 @@ extirq: interrupt-controller@1ac {
 					<9 0 &gic GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>,
 					<10 0 &gic GIC_SPI 150 IRQ_TYPE_LEVEL_HIGH>,
 					<11 0 &gic GIC_SPI 151 IRQ_TYPE_LEVEL_HIGH>;
-				interrupt-map-mask = <0xffffffff 0x0>;
 			};
 		};
 
-- 
2.25.1

[RFC PATCH devicetree 06/10] arm64: dts: ls1088a: rename the "interrupt-map" of the extirq node to "fsl,extirq-map"

[Vladimir Oltean]

In the discussions here:
https://lore.kernel.org/all/CAMuHMdXM34nNz1nfowNqDvdsdg+d69Bo3_ufs6fbcq65iYd5-A@mail.gmail.com/T/#m3449390080c5a0c8f59b8f8286a87e31e093b98b
Marc Zyngier points out that the ls-extirq driver should have never used
the "interrupt-map" keyword for mapping its channels to GIC interrupts.

This change modifies the device trees for the LS1088A to use a
driver-specific OF property for that purpose. The change is intended to
target stable kernels, to accelerate the conversion and ultimate removal
of the improperly used "interrupt-map" property.

Note that "interrupt-map-mask" isn't needed now (nor was it needed
before, it was just there to keep an apparent compatibility in form with
the reserved "interrupt-map" property). So delete it.

Fixes: 0e88b5fd565d ("arm64: dts: ls1088a: add DT node for external interrupt lines")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index 605072317243..53ecc25a24a2 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -240,7 +240,7 @@ extirq: interrupt-controller@14 {
 				#address-cells = <0>;
 				interrupt-controller;
 				reg = <0x14 4>;
-				interrupt-map =
+				fsl,extirq-map =
 					<0 0 &gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
 					<1 0 &gic GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
 					<2 0 &gic GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
@@ -253,7 +253,6 @@ extirq: interrupt-controller@14 {
 					<9 0 &gic GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
 					<10 0 &gic GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
 					<11 0 &gic GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
-				interrupt-map-mask = <0xffffffff 0x0>;
 			};
 		};
 
-- 
2.25.1

[RFC PATCH devicetree 07/10] arm64: dts: ls208xa: rename the "interrupt-map" of the extirq node to "fsl,extirq-map"

[Vladimir Oltean]

In the discussions here:
https://lore.kernel.org/all/CAMuHMdXM34nNz1nfowNqDvdsdg+d69Bo3_ufs6fbcq65iYd5-A@mail.gmail.com/T/#m3449390080c5a0c8f59b8f8286a87e31e093b98b
Marc Zyngier points out that the ls-extirq driver should have never used
the "interrupt-map" keyword for mapping its channels to GIC interrupts.

This change modifies the device trees for the LS2 family of devices to
use a driver-specific OF property for that purpose. The change is
intended to target stable kernels, to accelerate the conversion and
ultimate removal of the improperly used "interrupt-map" property.

Note that "interrupt-map-mask" isn't needed now (nor was it needed
before, it was just there to keep an apparent compatibility in form with
the reserved "interrupt-map" property). So delete it.

Fixes: ebb0713736ac ("arm64: dts: ls208xa: add DT node for external interrupt lines")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index 1282b61da8a5..50d530d66750 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -292,7 +292,7 @@ extirq: interrupt-controller@14 {
 				#address-cells = <0>;
 				interrupt-controller;
 				reg = <0x14 4>;
-				interrupt-map =
+				fsl,extirq-map =
 					<0 0 &gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
 					<1 0 &gic GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
 					<2 0 &gic GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
@@ -305,7 +305,6 @@ extirq: interrupt-controller@14 {
 					<9 0 &gic GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
 					<10 0 &gic GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
 					<11 0 &gic GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
-				interrupt-map-mask = <0xffffffff 0x0>;
 			};
 		};
 
-- 
2.25.1

[RFC PATCH devicetree 08/10] arm64: dts: lx2160a: rename the "interrupt-map" of the extirq node to "fsl,extirq-map"

[Vladimir Oltean]

In the discussions here:
https://lore.kernel.org/all/CAMuHMdXM34nNz1nfowNqDvdsdg+d69Bo3_ufs6fbcq65iYd5-A@mail.gmail.com/T/#m3449390080c5a0c8f59b8f8286a87e31e093b98b
Marc Zyngier points out that the ls-extirq driver should have never used
the "interrupt-map" keyword for mapping its channels to GIC interrupts.

This change modifies the device trees for LX2160A and LX2162A to use a
driver-specific OF property for that purpose. The change is intended to
target stable kernels, to accelerate the conversion and ultimate removal
of the improperly used "interrupt-map" property.

Note that "interrupt-map-mask" isn't needed now (nor was it needed
before, it was just there to keep an apparent compatibility in form with
the reserved "interrupt-map" property). So delete it.

Fixes: 332b6a79b415 ("arm64: dts: lx2160a: add DT node for external interrupt lines")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
index c4b1a59ba424..c12cc402b566 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
@@ -679,7 +679,7 @@ extirq: interrupt-controller@14 {
 				#address-cells = <0>;
 				interrupt-controller;
 				reg = <0x14 4>;
-				interrupt-map =
+				fsl,extirq-map =
 					<0 0 &gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
 					<1 0 &gic GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
 					<2 0 &gic GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
@@ -692,7 +692,6 @@ extirq: interrupt-controller@14 {
 					<9 0 &gic GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
 					<10 0 &gic GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
 					<11 0 &gic GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
-				interrupt-map-mask = <0xffffffff 0x0>;
 			};
 		};
 
-- 
2.25.1

[RFC PATCH devicetree 09/10] ARM: dts: ls1021a: rename the "interrupt-map" of the extirq node to "fsl,extirq-map"

[Vladimir Oltean]

In the discussions here:
https://lore.kernel.org/all/CAMuHMdXM34nNz1nfowNqDvdsdg+d69Bo3_ufs6fbcq65iYd5-A@mail.gmail.com/T/#m3449390080c5a0c8f59b8f8286a87e31e093b98b
Marc Zyngier points out that the ls-extirq driver should have never used
the "interrupt-map" keyword for mapping its channels to GIC interrupts.

This change modifies the device trees for the LS1021A to use a
driver-specific OF property for that purpose. The change is intended to
target stable kernels, to accelerate the conversion and ultimate removal
of the improperly used "interrupt-map" property.

Note that "interrupt-map-mask" isn't needed now (nor was it needed
before, it was just there to keep an apparent compatibility in form with
the reserved "interrupt-map" property). So delete it.

Fixes: d27f9d634c9b ("ARM: dts: ls1021a: add node describing external interrupt lines")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 arch/arm/boot/dts/ls1021a.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 2e69d6eab4d1..1efdfeb9c110 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -185,14 +185,13 @@ extirq: interrupt-controller@1ac {
 				#address-cells = <0>;
 				interrupt-controller;
 				reg = <0x1ac 4>;
-				interrupt-map =
+				fsl,extirq-map =
 					<0 0 &gic GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
 					<1 0 &gic GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
 					<2 0 &gic GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>,
 					<3 0 &gic GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
 					<4 0 &gic GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
 					<5 0 &gic GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
-				interrupt-map-mask = <0xffffffff 0x0>;
 			};
 		};
 
-- 
2.25.1

[RFC PATCH devicetree 10/10] dt-bindings: ls-extirq: add a YAML schema for the validator

[Vladimir Oltean]

This is a conversion of the free-form description of the device tree
bindings to a YAML schema. The description of fsl,extirq-map is best
effort: it looks like the devicetree schema doesn't really like vendor
properties getting too complicated, and puts a bunch of descriptions on
what they can and can't describe. An array of uint32s is the best I
could come up with. It doesn't help, either, that the
schemas/interrupt-controller.yaml definition for interrupt-map, which
I was planning to use as an inspiration, is "true # FIXME", all things
which aren't valid in vendor properties.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../interrupt-controller/fsl,ls-extirq.txt    |  56 ---------
 .../interrupt-controller/fsl,ls-extirq.yaml   | 110 ++++++++++++++++++
 2 files changed, 110 insertions(+), 56 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

diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
deleted file mode 100644
index cddf1aa032be..000000000000
--- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt
+++ /dev/null
@@ -1,56 +0,0 @@
-* Freescale Layerscape external IRQs
-
-Some Layerscape SOCs (LS1021A, LS1043A, LS1046A
-LS1088A, LS208xA, LX216xA) support inverting
-the polarity of certain external interrupt lines.
-
-The device node must be a child of the node representing the
-Supplemental Configuration Unit (SCFG).
-
-Required properties:
-- compatible: should be "fsl,<soc-name>-extirq", e.g. "fsl,ls1021a-extirq".
-  "fsl,ls1043a-extirq": for LS1043A, LS1046A.
-  "fsl,ls1088a-extirq": for LS1088A, LS208xA, LX216xA.
-- #interrupt-cells: Must be 2. The first element is the index of the
-  external interrupt line. The second element is the trigger type.
-- #address-cells: Must be 0.
-- interrupt-controller: Identifies the node as an interrupt controller
-- reg: Specifies the Interrupt Polarity Control Register (INTPCR) in
-  the SCFG or the External Interrupt Control Register (IRQCR) in
-  the ISC.
-- fsl,extirq-map: An array of elements through which the mapping between
-  external interrupts and GIC interrupts is specified. The first member of each
-  array element is the index of the extirq line. The second member must be
-  zero. The third member must be a phandle to the interrupt parent (the GIC).
-  The remaining number of members in an array element depends on the
-  #interrupt-cells property of the interrupt parent, and are used to specify
-  the parent interrupt.
-
-Example:
-	scfg: scfg@1570000 {
-		compatible = "fsl,ls1021a-scfg", "syscon";
-		reg = <0x0 0x1570000 0x0 0x10000>;
-		big-endian;
-		#address-cells = <1>;
-		#size-cells = <1>;
-		ranges = <0x0 0x0 0x1570000 0x10000>;
-
-		extirq: interrupt-controller@1ac {
-			compatible = "fsl,ls1021a-extirq";
-			#interrupt-cells = <2>;
-			#address-cells = <0>;
-			interrupt-controller;
-			reg = <0x1ac 4>;
-			fsl,extirq-map =
-				<0 0 &gic GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
-				<1 0 &gic GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
-				<2 0 &gic GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>,
-				<3 0 &gic GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
-				<4 0 &gic GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
-				<5 0 &gic GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
-		};
-	};
-
-
-	interrupts-extended = <&gic GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>,
-			      <&extirq 1 IRQ_TYPE_LEVEL_LOW>;
diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml
new file mode 100644
index 000000000000..ead5f58949b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/fsl,ls-extirq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Layerscape external interrupt driver
+
+maintainers:
+  - Rasmus Villemoes <linux@rasmusvillemoes.dk>
+
+description: |
+  Some Layerscape SOCs (LS1021A, LS1043A, LS1046A LS1088A, LS208xA, LX216xA)
+  support inverting the polarity of certain external interrupt lines.
+  The device node must be a child of the node representing the
+  Supplemental Configuration Unit (SCFG).
+
+properties:
+  compatible:
+    oneOf:
+      - const: fsl,ls1021a-extirq
+      - const: fsl,ls1043a-extirq
+      - const: fsl,ls1088a-extirq
+      - items:
+          - const: fsl,ls1046a-extirq
+          - const: fsl,ls1043a-extirq
+      - items:
+          - const: fsl,lx2160a-extirq
+          - const: fsl,ls1088a-extirq
+      - items:
+          - const: fsl,ls2080a-extirq
+          - const: fsl,ls1088a-extirq
+
+  reg:
+    description: |
+      Specifies the offset to the Interrupt Polarity Control Register (INTPCR)
+      in the SCFG or the External Interrupt Control Register (IRQCR) in the
+      ISC.
+    maxItems: 1
+
+  "#interrupt-cells":
+    description: |
+      Specifies the number of cells needed to encode an interrupt source. Must
+      be equal to 2. The first element is the index of the external interrupt
+      line. The second element is the trigger type.
+    const: 2
+
+  "#address-cells":
+    const: 0
+
+  interrupt-controller: true
+
+  fsl,extirq-map:
+    description: |
+      An array of elements through which the mapping between external
+      interrupts and GIC interrupts is specified. This isn't really a phandle
+      array, it just contains some phandles. It should really be an array where
+      the items are extirq-map-spec elements, but it seems like the
+      vendor-props.yaml don't allow us to reference such things.
+    type: object
+    $ref: "/schemas/types.yaml#/definitions/phandle-array"
+
+required:
+  - compatible
+  - reg
+  - "#interrupt-cells"
+  - interrupt-controller
+  - fsl,extirq-map
+
+additionalProperties: false
+
+$defs:
+  extirq-map-spec:
+    description: |
+      The first member of each array element is the index of the extirq line.
+      The second member must be zero. The third member must be a phandle to the
+      interrupt parent. The remaining number of members in an array element
+      depends on the "#interrupt-cells" property of the interrupt parent, and
+      are used to specify the parent interrupt.
+    type: array
+    items:
+      $ref: "/schemas/types.yaml#/definitions/cell"
+    minItems: 4
+    maxItems: 4095
+
+examples:
+  - |
+    scfg: scfg@1570000 {
+        compatible = "fsl,ls1021a-scfg", "syscon";
+        reg = <0x0 0x1570000 0x0 0x10000>;
+        big-endian;
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges = <0x0 0x0 0x1570000 0x10000>;
+
+        extirq: interrupt-controller@1ac {
+            compatible = "fsl,ls1021a-extirq";
+            #interrupt-cells = <2>;
+            #address-cells = <0>;
+            interrupt-controller;
+            reg = <0x1ac 4>;
+            fsl,extirq-map =
+                <0 0 &gic GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
+                <1 0 &gic GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
+                <2 0 &gic GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>,
+                <3 0 &gic GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
+                <4 0 &gic GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
+                <5 0 &gic GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
+        };
+    };
-- 
2.25.1

Re: [RFC PATCH devicetree 01/10] irqchip/ls-extirq: rename "interrupt-map" OF property to "fsl,extirq-map"

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

On Tue Dec 14 2021, Vladimir Oltean wrote:
> This OF property was supposed to be named "fsl,extirq-map" since the
> first patch submissions, but at Rob Herring's suggestion it was named
> "interrupt-map":
> https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/

nit: The preferred form is https://lore.kernel.org/r/<message-id> 

[snip]

> diff --git a/drivers/irqchip/irq-ls-extirq.c b/drivers/irqchip/irq-ls-extirq.c
> index 853b3972dbe7..b6ecc5e3472f 100644
> --- a/drivers/irqchip/irq-ls-extirq.c
> +++ b/drivers/irqchip/irq-ls-extirq.c
> @@ -101,9 +101,15 @@ ls_extirq_parse_map(struct ls_extirq_data *priv, struct device_node *node)
>  	u32 mapsize;
>  	int ret;
>  
> -	map = of_get_property(node, "interrupt-map", &mapsize);
> -	if (!map)
> -		return -ENOENT;
> +	map = of_get_property(node, "fsl,extirq-map", &mapsize);
> +	if (!map) {
> +		map = of_get_property(node, "interrupt-map", &mapsize);
> +		if (!map)
> +			return -ENOENT;
> +
> +		pr_warn("\"interrupt-map\" is a reserved OF property, and support for it will be removed. Please use \"fsl,extirq-map\" instead.\n");
> +	}

Looks reasonable. For instance, DSA does the same thing wrt "ports"
vs. "ethernet-ports".

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

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

[Marc Zyngier]

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.

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

[Vladimir Oltean]

Hi Marc (with a c),

On Tue, Dec 14, 2021 at 08:51:47AM +0000, Marc Zyngier wrote:
> 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.

"Part" is the subject, "works" is the predicate. It is a part of a
larger set of changes that now works after some breakage.

> > 
> > 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?).

Twas a typo, my hand must have slipped. This will not happen again.

> > 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?

I wish the firmware for these SoCs was smart enough to be compatible
with the bindings that are in the kernel and provide a blob that the
kernel could actually use. Some work has been started there and this is
work in progress. True, I don't know what other OF-based firmware some
other customers may use, but I trust it isn't a lot more advanced than
what U-Boot currently has :)

Also, the machines may have been in the wild for years, but the
ls-extirq driver was added in November 2019. So not with the
introduction of the SoC device trees themselves. That isn't so long ago.

As for compatibility between old kernel and new DT: I guess you'll hear
various opinions on this one.
https://www.spinics.net/lists/linux-mips/msg07778.html

| > Are we okay with the new device tree blobs breaking the old kernel?
|
| From my point of view, newer device trees are not required to work on
| older kernel, this would impose an unreasonable limitation and the use
| case is very limited.

Sadly we're at a point where we cannot have software any longer that
works with all device trees in circulation, after Rob's change, because
the ls-extirq driver won't know what is the expected correct length of
an "interrupt-map"/"fsl,extirq-map"/whatever-you-want-to-call-it specifier.

And even with the revert, the argument you've brought to the table still
holds: any kernel running on a device tree with Rob's change will stay
broken no matter what we do. I'd like to take a more constructive
approach and see what we can do going forward (in the direction of the
arrow of time, in case that's not clear), at least.

> 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.

The devicetree changes were for Shawn to pick up anyway. But I posted
the entire series as RFC to gather comments. Other opinions still welcome.

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

[Marc Zyngier]

On Tue, 14 Dec 2021 09:58:54 +0000,
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> Hi Marc (with a c),
> 
> I wish the firmware for these SoCs was smart enough to be compatible
> with the bindings that are in the kernel and provide a blob that the
> kernel could actually use. Some work has been started there and this is
> work in progress. True, I don't know what other OF-based firmware some
> other customers may use, but I trust it isn't a lot more advanced than
> what U-Boot currently has :)
> 
> Also, the machines may have been in the wild for years, but the
> ls-extirq driver was added in November 2019. So not with the
> introduction of the SoC device trees themselves. That isn't so long ago.
> 
> As for compatibility between old kernel and new DT: I guess you'll hear
> various opinions on this one.
> https://www.spinics.net/lists/linux-mips/msg07778.html
> 
> | > Are we okay with the new device tree blobs breaking the old kernel?
> |
> | From my point of view, newer device trees are not required to work on
> | older kernel, this would impose an unreasonable limitation and the use
> | case is very limited.

My views are on the opposite side. DT is an ABI, full stop. If you
change something, you *must* guarantee forward *and* backward
compatibility. That's because:

- you don't control how updatable the firmware is

- people may need to revert to other versions of the kernel because
  the new one is broken

- there are plenty of DT users beyond Linux, and we are not creating
  bindings for Linux only.

You may disagree with this, but for the subsystems I maintain, this is
the rule I intent to stick to.

	M.

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

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

[Vladimir Oltean]

On Tue, Dec 14, 2021 at 10:20:36AM +0000, Marc Zyngier wrote:
> On Tue, 14 Dec 2021 09:58:54 +0000,
> Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > 
> > Hi Marc (with a c),
> > 
> > I wish the firmware for these SoCs was smart enough to be compatible
> > with the bindings that are in the kernel and provide a blob that the
> > kernel could actually use. Some work has been started there and this is
> > work in progress. True, I don't know what other OF-based firmware some
> > other customers may use, but I trust it isn't a lot more advanced than
> > what U-Boot currently has :)
> > 
> > Also, the machines may have been in the wild for years, but the
> > ls-extirq driver was added in November 2019. So not with the
> > introduction of the SoC device trees themselves. That isn't so long ago.
> > 
> > As for compatibility between old kernel and new DT: I guess you'll hear
> > various opinions on this one.
> > https://www.spinics.net/lists/linux-mips/msg07778.html
> > 
> > | > Are we okay with the new device tree blobs breaking the old kernel?
> > |
> > | From my point of view, newer device trees are not required to work on
> > | older kernel, this would impose an unreasonable limitation and the use
> > | case is very limited.
> 
> My views are on the opposite side. DT is an ABI, full stop. If you
> change something, you *must* guarantee forward *and* backward
> compatibility. That's because:
> 
> - you don't control how updatable the firmware is
> 
> - people may need to revert to other versions of the kernel because
>   the new one is broken
> 
> - there are plenty of DT users beyond Linux, and we are not creating
>   bindings for Linux only.
> 
> You may disagree with this, but for the subsystems I maintain, this is
> the rule I intent to stick to.

That's an honorable set of guiding principles, but how do you apply them
here? Reverting Rob's change won't fix the past, and updating the code
to account for one format will break the other. As for trying one
format, and if there's an error try the other, there may be situations
in which you accept invalid input as valid.

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

[Marc Zyngier]

On Tue, 14 Dec 2021 10:30:26 +0000,
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> On Tue, Dec 14, 2021 at 10:20:36AM +0000, Marc Zyngier wrote:
> > On Tue, 14 Dec 2021 09:58:54 +0000,
> > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > 
> > > Hi Marc (with a c),
> > > 
> > > I wish the firmware for these SoCs was smart enough to be compatible
> > > with the bindings that are in the kernel and provide a blob that the
> > > kernel could actually use. Some work has been started there and this is
> > > work in progress. True, I don't know what other OF-based firmware some
> > > other customers may use, but I trust it isn't a lot more advanced than
> > > what U-Boot currently has :)
> > > 
> > > Also, the machines may have been in the wild for years, but the
> > > ls-extirq driver was added in November 2019. So not with the
> > > introduction of the SoC device trees themselves. That isn't so long ago.
> > > 
> > > As for compatibility between old kernel and new DT: I guess you'll hear
> > > various opinions on this one.
> > > https://www.spinics.net/lists/linux-mips/msg07778.html
> > > 
> > > | > Are we okay with the new device tree blobs breaking the old kernel?
> > > |
> > > | From my point of view, newer device trees are not required to work on
> > > | older kernel, this would impose an unreasonable limitation and the use
> > > | case is very limited.
> > 
> > My views are on the opposite side. DT is an ABI, full stop. If you
> > change something, you *must* guarantee forward *and* backward
> > compatibility. That's because:
> > 
> > - you don't control how updatable the firmware is
> > 
> > - people may need to revert to other versions of the kernel because
> >   the new one is broken
> > 
> > - there are plenty of DT users beyond Linux, and we are not creating
> >   bindings for Linux only.
> > 
> > You may disagree with this, but for the subsystems I maintain, this is
> > the rule I intent to stick to.
> 
> That's an honorable set of guiding principles, but how do you apply them
> here? Reverting Rob's change won't fix the past, and updating the code
> to account for one format will break the other. As for trying one
> format, and if there's an error try the other, there may be situations
> in which you accept invalid input as valid.

maz@hot-poop:~/arm-platforms$ git describe --contains 869f0ec048dc --match=v\*
v5.16-rc1~125^2~19^2~16

This patch landed in -rc1, and isn't part of any release. Just revert
it, and no damage is done.

	M.

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

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

[Vladimir Oltean]

On Tue, Dec 14, 2021 at 10:39:35AM +0000, Marc Zyngier wrote:
> On Tue, 14 Dec 2021 10:30:26 +0000,
> Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > 
> > On Tue, Dec 14, 2021 at 10:20:36AM +0000, Marc Zyngier wrote:
> > > On Tue, 14 Dec 2021 09:58:54 +0000,
> > > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > > 
> > > > Hi Marc (with a c),
> > > > 
> > > > I wish the firmware for these SoCs was smart enough to be compatible
> > > > with the bindings that are in the kernel and provide a blob that the
> > > > kernel could actually use. Some work has been started there and this is
> > > > work in progress. True, I don't know what other OF-based firmware some
> > > > other customers may use, but I trust it isn't a lot more advanced than
> > > > what U-Boot currently has :)
> > > > 
> > > > Also, the machines may have been in the wild for years, but the
> > > > ls-extirq driver was added in November 2019. So not with the
> > > > introduction of the SoC device trees themselves. That isn't so long ago.
> > > > 
> > > > As for compatibility between old kernel and new DT: I guess you'll hear
> > > > various opinions on this one.
> > > > https://www.spinics.net/lists/linux-mips/msg07778.html
> > > > 
> > > > | > Are we okay with the new device tree blobs breaking the old kernel?
> > > > |
> > > > | From my point of view, newer device trees are not required to work on
> > > > | older kernel, this would impose an unreasonable limitation and the use
> > > > | case is very limited.
> > > 
> > > My views are on the opposite side. DT is an ABI, full stop. If you
> > > change something, you *must* guarantee forward *and* backward
> > > compatibility. That's because:
> > > 
> > > - you don't control how updatable the firmware is
> > > 
> > > - people may need to revert to other versions of the kernel because
> > >   the new one is broken
> > > 
> > > - there are plenty of DT users beyond Linux, and we are not creating
> > >   bindings for Linux only.
> > > 
> > > You may disagree with this, but for the subsystems I maintain, this is
> > > the rule I intent to stick to.
> > 
> > That's an honorable set of guiding principles, but how do you apply them
> > here? Reverting Rob's change won't fix the past, and updating the code
> > to account for one format will break the other. As for trying one
> > format, and if there's an error try the other, there may be situations
> > in which you accept invalid input as valid.
> 
> maz@hot-poop:~/arm-platforms$ git describe --contains 869f0ec048dc --match=v\*
> v5.16-rc1~125^2~19^2~16
> 
> This patch landed in -rc1, and isn't part of any release. Just revert
> it, and no damage is done.

The revert is one of the patches posted here. It will fix the problem
short-term but it may not be enough long-term. I think Rob is working on
some sort of validation for "interrupt-map" and this is how the apparently
non-conformant property was brought to his attention. It will trigger
validation warnings that I'm afraid will be tempting for many to "fix".
Thus the rest of the patches. Maybe it's just me, but between having to
play a whack-a-mole game and snapping compatibility of old kernels with
new DT blobs, I think more time is lost with the latter.

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

[Marc Zyngier]

On Tue, 14 Dec 2021 10:53:16 +0000,
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> On Tue, Dec 14, 2021 at 10:39:35AM +0000, Marc Zyngier wrote:
> > On Tue, 14 Dec 2021 10:30:26 +0000,
> > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > 
> > > On Tue, Dec 14, 2021 at 10:20:36AM +0000, Marc Zyngier wrote:
> > > > On Tue, 14 Dec 2021 09:58:54 +0000,
> > > > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > > > 
> > > > > Hi Marc (with a c),
> > > > > 
> > > > > I wish the firmware for these SoCs was smart enough to be compatible
> > > > > with the bindings that are in the kernel and provide a blob that the
> > > > > kernel could actually use. Some work has been started there and this is
> > > > > work in progress. True, I don't know what other OF-based firmware some
> > > > > other customers may use, but I trust it isn't a lot more advanced than
> > > > > what U-Boot currently has :)
> > > > > 
> > > > > Also, the machines may have been in the wild for years, but the
> > > > > ls-extirq driver was added in November 2019. So not with the
> > > > > introduction of the SoC device trees themselves. That isn't so long ago.
> > > > > 
> > > > > As for compatibility between old kernel and new DT: I guess you'll hear
> > > > > various opinions on this one.
> > > > > https://www.spinics.net/lists/linux-mips/msg07778.html
> > > > > 
> > > > > | > Are we okay with the new device tree blobs breaking the old kernel?
> > > > > |
> > > > > | From my point of view, newer device trees are not required to work on
> > > > > | older kernel, this would impose an unreasonable limitation and the use
> > > > > | case is very limited.
> > > > 
> > > > My views are on the opposite side. DT is an ABI, full stop. If you
> > > > change something, you *must* guarantee forward *and* backward
> > > > compatibility. That's because:
> > > > 
> > > > - you don't control how updatable the firmware is
> > > > 
> > > > - people may need to revert to other versions of the kernel because
> > > >   the new one is broken
> > > > 
> > > > - there are plenty of DT users beyond Linux, and we are not creating
> > > >   bindings for Linux only.
> > > > 
> > > > You may disagree with this, but for the subsystems I maintain, this is
> > > > the rule I intent to stick to.
> > > 
> > > That's an honorable set of guiding principles, but how do you apply them
> > > here? Reverting Rob's change won't fix the past, and updating the code
> > > to account for one format will break the other. As for trying one
> > > format, and if there's an error try the other, there may be situations
> > > in which you accept invalid input as valid.
> > 
> > maz@hot-poop:~/arm-platforms$ git describe --contains 869f0ec048dc --match=v\*
> > v5.16-rc1~125^2~19^2~16
> > 
> > This patch landed in -rc1, and isn't part of any release. Just revert
> > it, and no damage is done.
> 
> The revert is one of the patches posted here. It will fix the problem
> short-term but it may not be enough long-term. I think Rob is working on
> some sort of validation for "interrupt-map" and this is how the apparently
> non-conformant property was brought to his attention. It will trigger
> validation warnings that I'm afraid will be tempting for many to "fix".

Then build an annotation mechanism for the warning not to fire for
quirked systems.

> Thus the rest of the patches. Maybe it's just me, but between having to
> play a whack-a-mole game and snapping compatibility of old kernels with
> new DT blobs, I think more time is lost with the latter.

I said what I had to say on the subject, and when it comes to wasted
time, that's more than enough.

	M.

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

Re: [RFC PATCH devicetree 01/10] irqchip/ls-extirq: rename "interrupt-map" OF property to "fsl,extirq-map"

[Rob Herring]

On Mon, Dec 13, 2021 at 7:38 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> This OF property was supposed to be named "fsl,extirq-map" since the
> first patch submissions, but at Rob Herring's suggestion it was named
> "interrupt-map":
> https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/

I'm still not okay with a custom property. The fact that multiple
platforms need the same thing is an indication this should be common.
Other cases are being fixed by using 'interrupts', but that's going to
take some cleanups[1].

> At that time, the "interrupt-map" was ignored by the core for OF nodes
> that also had an "interrupt-controller" property, but that changed with
> commit 041284181226 ("of/irq: Allow matching of an interrupt-map local
> to an interrupt controller"), which made the consumer drivers of the
> ls-extirq break. To work around this breakage, the OF bindings for IRQs
> have introduced a table of "OF IRQ interrupt-map abusers".
> This can be seen in commit de4adddcbcc2 ("of/irq: Add a quirk for
> controllers with their own definition of interrupt-map").
>
> To stop being abusers, let's go back to the original form of these
> bindings, before Rob's review. Compatibility will be kept with the
> current abusive bindings for a few more kernel cycles, to give people
> some time to update. But to also give them an incentive, print a warning
> that the support for "interrupt-map" will be removed. This will be seen
> when running a new kernel with an old device tree.
>
> While the breakage was introduced relatively recently, the device tree
> changes are intended to backport stable kernels for quicker conversion
> to an acceptable set of bindings, and this driver will need to support
> the updated stable bindings.

Using 'interrupts' is not going to work backporting to stable, but I
don't think trying to switch this is worth it.

Rob

[1] https://lore.kernel.org/all/CAL_Jsq+jyqbhA1jpgZ+yTwWGvCMRu9VmgoDq8MDM9SMqJ-XSBw@mail.gmail.com/

Re: [RFC PATCH devicetree 10/10] dt-bindings: ls-extirq: add a YAML schema for the validator

[Rob Herring]

On Tue, 14 Dec 2021 03:38:00 +0200, Vladimir Oltean wrote:
> This is a conversion of the free-form description of the device tree
> bindings to a YAML schema. The description of fsl,extirq-map is best
> effort: it looks like the devicetree schema doesn't really like vendor
> properties getting too complicated, and puts a bunch of descriptions on
> what they can and can't describe. An array of uint32s is the best I
> could come up with. It doesn't help, either, that the
> schemas/interrupt-controller.yaml definition for interrupt-map, which
> I was planning to use as an inspiration, is "true # FIXME", all things
> which aren't valid in vendor properties.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  .../interrupt-controller/fsl,ls-extirq.txt    |  56 ---------
>  .../interrupt-controller/fsl,ls-extirq.yaml   | 110 ++++++++++++++++++
>  2 files changed, 110 insertions(+), 56 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
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.example.dts:34.31-32 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1413: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1567537

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

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

[Vladimir Oltean]

Hello Marc,

On Tue, Dec 14, 2021 at 10:20:36AM +0000, Marc Zyngier wrote:
> On Tue, 14 Dec 2021 09:58:54 +0000,
> Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > 
> > Hi Marc (with a c),
> > 
> > I wish the firmware for these SoCs was smart enough to be compatible
> > with the bindings that are in the kernel and provide a blob that the
> > kernel could actually use. Some work has been started there and this is
> > work in progress. True, I don't know what other OF-based firmware some
> > other customers may use, but I trust it isn't a lot more advanced than
> > what U-Boot currently has :)
> > 
> > Also, the machines may have been in the wild for years, but the
> > ls-extirq driver was added in November 2019. So not with the
> > introduction of the SoC device trees themselves. That isn't so long ago.
> > 
> > As for compatibility between old kernel and new DT: I guess you'll hear
> > various opinions on this one.
> > https://www.spinics.net/lists/linux-mips/msg07778.html
> > 
> > | > Are we okay with the new device tree blobs breaking the old kernel?
> > |
> > | From my point of view, newer device trees are not required to work on
> > | older kernel, this would impose an unreasonable limitation and the use
> > | case is very limited.
> 
> My views are on the opposite side. DT is an ABI, full stop. If you
> change something, you *must* guarantee forward *and* backward
> compatibility. That's because:
> 
> - you don't control how updatable the firmware is
> 
> - people may need to revert to other versions of the kernel because
>   the new one is broken
> 
> - there are plenty of DT users beyond Linux, and we are not creating
>   bindings for Linux only.
> 
> You may disagree with this, but for the subsystems I maintain, this is
> the rule I intent to stick to.
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

I was just debugging an interesting issue with an old kernel not working
with a new DT blob, and after figuring out what the problem was (is),
I remembered this message and I'm curious what you have to say about it.

I have this DT layout:

	ethernet-phy@1 {
		reg = <0x1>;
		interrupts-extended = <&extirq 2 IRQ_TYPE_LEVEL_LOW>;
	};

	extirq: interrupt-controller@1ac {
		compatible = "fsl,ls1021a-extirq";
		<bla bla>
	};

I booted the new DT blob (which has "interrupts-extended") on a kernel
where the ls-extirq driver did not exist. This had the result of
of_mdiobus_phy_device_register() -> of_irq_get() returning -EPROBE_DEFER
forever and ever. So the PHY driver in turn never probed, and Ethernet
was broken. So I had to delete the interrupts OF property to let the PHY
at least work in poll mode.

What went wrong here in your opinion?

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

[Marc Zyngier]

On Thu, 24 Mar 2022 17:10:42 +0000,
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> Hello Marc,
> 
> On Tue, Dec 14, 2021 at 10:20:36AM +0000, Marc Zyngier wrote:
> > On Tue, 14 Dec 2021 09:58:54 +0000,
> > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > 
> > > Hi Marc (with a c),
> > > 
> > > I wish the firmware for these SoCs was smart enough to be compatible
> > > with the bindings that are in the kernel and provide a blob that the
> > > kernel could actually use. Some work has been started there and this is
> > > work in progress. True, I don't know what other OF-based firmware some
> > > other customers may use, but I trust it isn't a lot more advanced than
> > > what U-Boot currently has :)
> > > 
> > > Also, the machines may have been in the wild for years, but the
> > > ls-extirq driver was added in November 2019. So not with the
> > > introduction of the SoC device trees themselves. That isn't so long ago.
> > > 
> > > As for compatibility between old kernel and new DT: I guess you'll hear
> > > various opinions on this one.
> > > https://www.spinics.net/lists/linux-mips/msg07778.html
> > > 
> > > | > Are we okay with the new device tree blobs breaking the old kernel?
> > > |
> > > | From my point of view, newer device trees are not required to work on
> > > | older kernel, this would impose an unreasonable limitation and the use
> > > | case is very limited.
> > 
> > My views are on the opposite side. DT is an ABI, full stop. If you
> > change something, you *must* guarantee forward *and* backward
> > compatibility. That's because:
> > 
> > - you don't control how updatable the firmware is
> > 
> > - people may need to revert to other versions of the kernel because
> >   the new one is broken
> > 
> > - there are plenty of DT users beyond Linux, and we are not creating
> >   bindings for Linux only.
> > 
> > You may disagree with this, but for the subsystems I maintain, this is
> > the rule I intent to stick to.
> > 
> > 	M.
> > 
> > -- 
> > Without deviation from the norm, progress is not possible.
> 
> I was just debugging an interesting issue with an old kernel not working
> with a new DT blob, and after figuring out what the problem was (is),
> I remembered this message and I'm curious what you have to say about it.
> 
> I have this DT layout:
> 
> 	ethernet-phy@1 {
> 		reg = <0x1>;
> 		interrupts-extended = <&extirq 2 IRQ_TYPE_LEVEL_LOW>;
> 	};
> 
> 	extirq: interrupt-controller@1ac {
> 		compatible = "fsl,ls1021a-extirq";
> 		<bla bla>
> 	};
> 
> I booted the new DT blob (which has "interrupts-extended") on a kernel
> where the ls-extirq driver did not exist. This had the result of
> of_mdiobus_phy_device_register() -> of_irq_get() returning -EPROBE_DEFER
> forever and ever. So the PHY driver in turn never probed, and Ethernet
> was broken. So I had to delete the interrupts OF property to let the PHY
> at least work in poll mode.
> 
> What went wrong here in your opinion?

I'm not sure what you expect me to say here. You have a device that
references an interrupt. The DT seems sound (I don't get why you think
"interrupt-extended" is a problem here, but hey...).

If your kernel doesn't have a driver for the interrupt controller
referenced here, what do you expect, other than things not working?

	M.

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

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

[Vladimir Oltean]

On Thu, Mar 24, 2022 at 05:21:50PM +0000, Marc Zyngier wrote:
> On Thu, 24 Mar 2022 17:10:42 +0000,
> Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > 
> > Hello Marc,
> > 
> > On Tue, Dec 14, 2021 at 10:20:36AM +0000, Marc Zyngier wrote:
> > > On Tue, 14 Dec 2021 09:58:54 +0000,
> > > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > > 
> > > > Hi Marc (with a c),
> > > > 
> > > > I wish the firmware for these SoCs was smart enough to be compatible
> > > > with the bindings that are in the kernel and provide a blob that the
> > > > kernel could actually use. Some work has been started there and this is
> > > > work in progress. True, I don't know what other OF-based firmware some
> > > > other customers may use, but I trust it isn't a lot more advanced than
> > > > what U-Boot currently has :)
> > > > 
> > > > Also, the machines may have been in the wild for years, but the
> > > > ls-extirq driver was added in November 2019. So not with the
> > > > introduction of the SoC device trees themselves. That isn't so long ago.
> > > > 
> > > > As for compatibility between old kernel and new DT: I guess you'll hear
> > > > various opinions on this one.
> > > > https://www.spinics.net/lists/linux-mips/msg07778.html
> > > > 
> > > > | > Are we okay with the new device tree blobs breaking the old kernel?
> > > > |
> > > > | From my point of view, newer device trees are not required to work on
> > > > | older kernel, this would impose an unreasonable limitation and the use
> > > > | case is very limited.
> > > 
> > > My views are on the opposite side. DT is an ABI, full stop. If you
> > > change something, you *must* guarantee forward *and* backward
> > > compatibility. That's because:
> > > 
> > > - you don't control how updatable the firmware is
> > > 
> > > - people may need to revert to other versions of the kernel because
> > >   the new one is broken
> > > 
> > > - there are plenty of DT users beyond Linux, and we are not creating
> > >   bindings for Linux only.
> > > 
> > > You may disagree with this, but for the subsystems I maintain, this is
> > > the rule I intent to stick to.
> > > 
> > > 	M.
> > > 
> > > -- 
> > > Without deviation from the norm, progress is not possible.
> > 
> > I was just debugging an interesting issue with an old kernel not working
> > with a new DT blob, and after figuring out what the problem was (is),
> > I remembered this message and I'm curious what you have to say about it.
> > 
> > I have this DT layout:
> > 
> > 	ethernet-phy@1 {
> > 		reg = <0x1>;
> > 		interrupts-extended = <&extirq 2 IRQ_TYPE_LEVEL_LOW>;
> > 	};
> > 
> > 	extirq: interrupt-controller@1ac {
> > 		compatible = "fsl,ls1021a-extirq";
> > 		<bla bla>
> > 	};
> > 
> > I booted the new DT blob (which has "interrupts-extended") on a kernel
> > where the ls-extirq driver did not exist. This had the result of
> > of_mdiobus_phy_device_register() -> of_irq_get() returning -EPROBE_DEFER
> > forever and ever. So the PHY driver in turn never probed, and Ethernet
> > was broken. So I had to delete the interrupts OF property to let the PHY
> > at least work in poll mode.
> > 
> > What went wrong here in your opinion?
> 
> I'm not sure what you expect me to say here. You have a device that
> references an interrupt. The DT seems sound (I don't get why you think
> "interrupt-extended" is a problem here, but hey...).
> 
> If your kernel doesn't have a driver for the interrupt controller
> referenced here, what do you expect, other than things not working?
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

I was just raising this as what I thought would be a simple and
non-controversial counter example to your remark "If you change something,
you *must* guarantee forward *and* backward compatibility."

Practically speaking, what has happened is that the board DT appeared in
kernel N, the ls-extirq driver in kernel N+1, and the DT was updated to
enable PHY interrupts in kernel N+2. That DT update practically broke
kernel N from running correctly on DTs taken from kernel N+2 onwards.
This is the observable behavior, we can find as many justifications for
it as we wish.

(as to what I expect, Ethernet PHYs work without an interrupt too, but
 of_mdiobus_phy_device_register() treats -EPROBE_DEFER from of_irq_get()
 as special, because it assumes the IRQ domain will eventually come up.
 The IRQ is optional, as evidenced by the fact that kernel N worked)

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

[Marc Zyngier]

On Thu, 24 Mar 2022 17:34:06 +0000,
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> On Thu, Mar 24, 2022 at 05:21:50PM +0000, Marc Zyngier wrote:
> > On Thu, 24 Mar 2022 17:10:42 +0000,
> > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > 
> > > Hello Marc,
> > > 
> > > On Tue, Dec 14, 2021 at 10:20:36AM +0000, Marc Zyngier wrote:
> > > > On Tue, 14 Dec 2021 09:58:54 +0000,
> > > > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > > > 
> > > > > Hi Marc (with a c),
> > > > > 
> > > > > I wish the firmware for these SoCs was smart enough to be compatible
> > > > > with the bindings that are in the kernel and provide a blob that the
> > > > > kernel could actually use. Some work has been started there and this is
> > > > > work in progress. True, I don't know what other OF-based firmware some
> > > > > other customers may use, but I trust it isn't a lot more advanced than
> > > > > what U-Boot currently has :)
> > > > > 
> > > > > Also, the machines may have been in the wild for years, but the
> > > > > ls-extirq driver was added in November 2019. So not with the
> > > > > introduction of the SoC device trees themselves. That isn't so long ago.
> > > > > 
> > > > > As for compatibility between old kernel and new DT: I guess you'll hear
> > > > > various opinions on this one.
> > > > > https://www.spinics.net/lists/linux-mips/msg07778.html
> > > > > 
> > > > > | > Are we okay with the new device tree blobs breaking the old kernel?
> > > > > |
> > > > > | From my point of view, newer device trees are not required to work on
> > > > > | older kernel, this would impose an unreasonable limitation and the use
> > > > > | case is very limited.
> > > > 
> > > > My views are on the opposite side. DT is an ABI, full stop. If you
> > > > change something, you *must* guarantee forward *and* backward
> > > > compatibility. That's because:
> > > > 
> > > > - you don't control how updatable the firmware is
> > > > 
> > > > - people may need to revert to other versions of the kernel because
> > > >   the new one is broken
> > > > 
> > > > - there are plenty of DT users beyond Linux, and we are not creating
> > > >   bindings for Linux only.
> > > > 
> > > > You may disagree with this, but for the subsystems I maintain, this is
> > > > the rule I intent to stick to.
> > > > 
> > > > 	M.
> > > > 
> > > > -- 
> > > > Without deviation from the norm, progress is not possible.
> > > 
> > > I was just debugging an interesting issue with an old kernel not working
> > > with a new DT blob, and after figuring out what the problem was (is),
> > > I remembered this message and I'm curious what you have to say about it.
> > > 
> > > I have this DT layout:
> > > 
> > > 	ethernet-phy@1 {
> > > 		reg = <0x1>;
> > > 		interrupts-extended = <&extirq 2 IRQ_TYPE_LEVEL_LOW>;
> > > 	};
> > > 
> > > 	extirq: interrupt-controller@1ac {
> > > 		compatible = "fsl,ls1021a-extirq";
> > > 		<bla bla>
> > > 	};
> > > 
> > > I booted the new DT blob (which has "interrupts-extended") on a kernel
> > > where the ls-extirq driver did not exist. This had the result of
> > > of_mdiobus_phy_device_register() -> of_irq_get() returning -EPROBE_DEFER
> > > forever and ever. So the PHY driver in turn never probed, and Ethernet
> > > was broken. So I had to delete the interrupts OF property to let the PHY
> > > at least work in poll mode.
> > > 
> > > What went wrong here in your opinion?
> > 
> > I'm not sure what you expect me to say here. You have a device that
> > references an interrupt. The DT seems sound (I don't get why you think
> > "interrupt-extended" is a problem here, but hey...).
> > 
> > If your kernel doesn't have a driver for the interrupt controller
> > referenced here, what do you expect, other than things not working?
> > 
> > 	M.
> > 
> > -- 
> > Without deviation from the norm, progress is not possible.
> 
> I was just raising this as what I thought would be a simple and
> non-controversial counter example to your remark "If you change something,
> you *must* guarantee forward *and* backward compatibility."

If you change something *in the binding*, which was implicit in the
context, and makes no sense out of context.

> 
> Practically speaking, what has happened is that the board DT appeared in
> kernel N, the ls-extirq driver in kernel N+1, and the DT was updated to
> enable PHY interrupts in kernel N+2. That DT update practically broke
> kernel N from running correctly on DTs taken from kernel N+2 onwards.
> This is the observable behavior, we can find as many justifications for
> it as we wish.

Well, you can also argue that the DT was broken at N and N+1 for not
describing the HW correctly and completely. No binding has changed
here. Your DT was incomplete, and someone fixed it for you.

We can argue this things forever and a half. I've laid down the ground
rules for the stuff I maintain. If you're not happy with this, you can
fix it by either removing the NXP hardware from the tree, or taking
over from me as the irqchip maintainer. I'd be perfectly happy with
any (and even more, with both) of these outcomes.

	M.

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

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

[Vladimir Oltean]

On Thu, Mar 24, 2022 at 06:06:51PM +0000, Marc Zyngier wrote:
> > I was just raising this as what I thought would be a simple and
> > non-controversial counter example to your remark "If you change something,
> > you *must* guarantee forward *and* backward compatibility."
> 
> If you change something *in the binding*, which was implicit in the
> context, and makes no sense out of context.
> 
> > Practically speaking, what has happened is that the board DT appeared in
> > kernel N, the ls-extirq driver in kernel N+1, and the DT was updated to
> > enable PHY interrupts in kernel N+2. That DT update practically broke
> > kernel N from running correctly on DTs taken from kernel N+2 onwards.
> > This is the observable behavior, we can find as many justifications for
> > it as we wish.
> 
> Well, you can also argue that the DT was broken at N and N+1 for not
> describing the HW correctly and completely. No binding has changed
> here. Your DT was incomplete, and someone fixed it for you.
> 
> We can argue this things forever and a half. I've laid down the ground
> rules for the stuff I maintain. If you're not happy with this, you can
> fix it by either removing the NXP hardware from the tree, or taking
> over from me as the irqchip maintainer. I'd be perfectly happy with
> any (and even more, with both) of these outcomes.

Ok, my intention wasn't to inflame you even though the way in which I
presented the problem might have suggested otherwise.

With my developer hat I still don't agree with you even with the
additional clarification you've made that you were referring only to
bindings and not to any and all DT changes. The reason being that the DT
blob is a whole, and it doesn't matter if there's a regression because
of a binding change or something else, you still need to be prepared to
update it, sometimes in lockstep with the kernel, like it or not.

But as a user, I just wanted to get an opinion from you what can we do
to deal better with this situation: optional interrupt provided by
device with missing driver, which of_irq_get() doesn't seem to understand.
There are more angles to this than just "new DT with old kernel". It can
also be new kernel, but ls-extirq driver disabled, and I still see that
as a kernel <-> DT compatibility concern.

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

[Marc Zyngier]

On Thu, 24 Mar 2022 19:09:05 +0000,
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> On Thu, Mar 24, 2022 at 06:06:51PM +0000, Marc Zyngier wrote:
> > > I was just raising this as what I thought would be a simple and
> > > non-controversial counter example to your remark "If you change something,
> > > you *must* guarantee forward *and* backward compatibility."
> > 
> > If you change something *in the binding*, which was implicit in the
> > context, and makes no sense out of context.
> > 
> > > Practically speaking, what has happened is that the board DT appeared in
> > > kernel N, the ls-extirq driver in kernel N+1, and the DT was updated to
> > > enable PHY interrupts in kernel N+2. That DT update practically broke
> > > kernel N from running correctly on DTs taken from kernel N+2 onwards.
> > > This is the observable behavior, we can find as many justifications for
> > > it as we wish.
> > 
> > Well, you can also argue that the DT was broken at N and N+1 for not
> > describing the HW correctly and completely. No binding has changed
> > here. Your DT was incomplete, and someone fixed it for you.
> > 
> > We can argue this things forever and a half. I've laid down the ground
> > rules for the stuff I maintain. If you're not happy with this, you can
> > fix it by either removing the NXP hardware from the tree, or taking
> > over from me as the irqchip maintainer. I'd be perfectly happy with
> > any (and even more, with both) of these outcomes.
> 
> Ok, my intention wasn't to inflame you even though the way in which I
> presented the problem might have suggested otherwise.
> 
> With my developer hat I still don't agree with you even with the
> additional clarification you've made that you were referring only to
> bindings and not to any and all DT changes. The reason being that the DT
> blob is a whole, and it doesn't matter if there's a regression because
> of a binding change or something else, you still need to be prepared to
> update it, sometimes in lockstep with the kernel, like it or not.

<rant>
No. This doesn't happen on systems that ship the DT as part of their
firmware, and this doesn't happen with ACPI either. This only happens
on platform that are maintained like the NXP, Marvell, and other
similar platforms that are being used as a job security tool by doing
piecemeal enablement.

Properly maintained systems have had the same DT for years. Features
have been added over time, yet without breaking compatibility in
either direction. Yes, it requires some effort and planning. And even
quirks at times. Yet they don't break.

Amusingly, some of these better supported platforms do not have their
DT in the kernel tree. Synquacer, for example. Keeping the DTs in the
kernel tree has been one of the worse decision we have ever made. It
has simply moved the board files of old to a different place, under
the guise of separating description and code.  In practice, it
abstracted nothing at all, only made it more complicated because
people are treating DT as an integral part of the kernel code base,
which it really shouldn't be.
</rant>

> But as a user, I just wanted to get an opinion from you what can we do
> to deal better with this situation: optional interrupt provided by
> device with missing driver, which of_irq_get() doesn't seem to understand.
> There are more angles to this than just "new DT with old kernel". It can
> also be new kernel, but ls-extirq driver disabled, and I still see that
> as a kernel <-> DT compatibility concern.

If you're missing a driver, that's a user error. Or rather, a platform
maintainer error for not establishing the correct dependencies. This
has nothing to do with the DT. As for optional interrupts, that has
nothing to do with the DT either, but with the kernel code that
requests it. If you think the kernel should do better, you can always
post a patch.

And I'm done on that subject.

	M.

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

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

[Robin Murphy]

On 2022-03-24 19:09, Vladimir Oltean wrote:
> On Thu, Mar 24, 2022 at 06:06:51PM +0000, Marc Zyngier wrote:
>>> I was just raising this as what I thought would be a simple and
>>> non-controversial counter example to your remark "If you change something,
>>> you *must* guarantee forward *and* backward compatibility."
>>
>> If you change something *in the binding*, which was implicit in the
>> context, and makes no sense out of context.
>>
>>> Practically speaking, what has happened is that the board DT appeared in
>>> kernel N, the ls-extirq driver in kernel N+1, and the DT was updated to
>>> enable PHY interrupts in kernel N+2. That DT update practically broke
>>> kernel N from running correctly on DTs taken from kernel N+2 onwards.
>>> This is the observable behavior, we can find as many justifications for
>>> it as we wish.
>>
>> Well, you can also argue that the DT was broken at N and N+1 for not
>> describing the HW correctly and completely. No binding has changed
>> here. Your DT was incomplete, and someone fixed it for you.
>>
>> We can argue this things forever and a half. I've laid down the ground
>> rules for the stuff I maintain. If you're not happy with this, you can
>> fix it by either removing the NXP hardware from the tree, or taking
>> over from me as the irqchip maintainer. I'd be perfectly happy with
>> any (and even more, with both) of these outcomes.
> 
> Ok, my intention wasn't to inflame you even though the way in which I
> presented the problem might have suggested otherwise.
> 
> With my developer hat I still don't agree with you even with the
> additional clarification you've made that you were referring only to
> bindings and not to any and all DT changes. The reason being that the DT
> blob is a whole, and it doesn't matter if there's a regression because
> of a binding change or something else, you still need to be prepared to
> update it, sometimes in lockstep with the kernel, like it or not.
> 
> But as a user, I just wanted to get an opinion from you what can we do
> to deal better with this situation: optional interrupt provided by
> device with missing driver, which of_irq_get() doesn't seem to understand.

FWIW, of_irq_get() absolutely understands how to handle a missing IRQ 
provider driver; it returns -EPROBE_DEFER. If a caller considers the IRQ 
optional, then it's up to that caller to decide how long to keep waiting 
for the provider to appear until giving up and carrying on without it. 
If your phy driver is making the dumb decision to wait for ever for 
something which isn't critical, then you're free to fix it, or perhaps 
even propose for of_irq_get() to opt in to the 
driver_deferred_probe_check_state() mechanism if you believe it's a 
sufficiently general case.

If a new DT with an additional new property (either on an existing 
machine, or on a completely new machine which has the property from the 
start) exposes a bug in a driver, that's unfortunate, but it is entirely 
irrelevant to the ABI implications of changing the interpretation of an 
existing property.

Robin.

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

[Vladimir Oltean]

On Fri, Mar 25, 2022 at 10:34:13AM +0000, Robin Murphy wrote:
> On 2022-03-24 19:09, Vladimir Oltean wrote:
> > On Thu, Mar 24, 2022 at 06:06:51PM +0000, Marc Zyngier wrote:
> > > > I was just raising this as what I thought would be a simple and
> > > > non-controversial counter example to your remark "If you change something,
> > > > you *must* guarantee forward *and* backward compatibility."
> > > 
> > > If you change something *in the binding*, which was implicit in the
> > > context, and makes no sense out of context.
> > > 
> > > > Practically speaking, what has happened is that the board DT appeared in
> > > > kernel N, the ls-extirq driver in kernel N+1, and the DT was updated to
> > > > enable PHY interrupts in kernel N+2. That DT update practically broke
> > > > kernel N from running correctly on DTs taken from kernel N+2 onwards.
> > > > This is the observable behavior, we can find as many justifications for
> > > > it as we wish.
> > > 
> > > Well, you can also argue that the DT was broken at N and N+1 for not
> > > describing the HW correctly and completely. No binding has changed
> > > here. Your DT was incomplete, and someone fixed it for you.
> > > 
> > > We can argue this things forever and a half. I've laid down the ground
> > > rules for the stuff I maintain. If you're not happy with this, you can
> > > fix it by either removing the NXP hardware from the tree, or taking
> > > over from me as the irqchip maintainer. I'd be perfectly happy with
> > > any (and even more, with both) of these outcomes.
> > 
> > Ok, my intention wasn't to inflame you even though the way in which I
> > presented the problem might have suggested otherwise.
> > 
> > With my developer hat I still don't agree with you even with the
> > additional clarification you've made that you were referring only to
> > bindings and not to any and all DT changes. The reason being that the DT
> > blob is a whole, and it doesn't matter if there's a regression because
> > of a binding change or something else, you still need to be prepared to
> > update it, sometimes in lockstep with the kernel, like it or not.
> > 
> > But as a user, I just wanted to get an opinion from you what can we do
> > to deal better with this situation: optional interrupt provided by
> > device with missing driver, which of_irq_get() doesn't seem to understand.
> 
> FWIW, of_irq_get() absolutely understands how to handle a missing IRQ
> provider driver; it returns -EPROBE_DEFER. If a caller considers the IRQ
> optional, then it's up to that caller to decide how long to keep waiting for
> the provider to appear until giving up and carrying on without it. If your
> phy driver is making the dumb decision to wait for ever for something which
> isn't critical, then you're free to fix it, or perhaps even propose for
> of_irq_get() to opt in to the driver_deferred_probe_check_state() mechanism
> if you believe it's a sufficiently general case.

Thanks, I really needed that suggestion, at the moment I made this
change which seems to do what I want when I force-disable the ls-extirq
driver (which in turn simulates an unwritten driver from an old kernel):

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 1becb1a731f6..1c1584fca632 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -43,6 +43,11 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 	int rc;
 
 	rc = fwnode_irq_get(child, 0);
+	/* Don't wait forever if the IRQ provider doesn't become available,
+	 * just fall back to poll mode
+	 */
+	if (rc == -EPROBE_DEFER)
+		rc = driver_deferred_probe_check_state(&phy->mdio.dev);
 	if (rc == -EPROBE_DEFER)
 		return rc;
 

The trickier part seems to be to adjust this change for older kernels
where the MDIO bus code calls of_irq_get() directly, and not get
regressions in. That, plus I need to understand what changes in behavior
when the irqchip driver is built as module and the MDIO bus driver is
built-in, and the "driver_deferred_probe_timeout" kernel boot parameter
has the default value of 0.

> If a new DT with an additional new property (either on an existing machine,
> or on a completely new machine which has the property from the start)
> exposes a bug in a driver, that's unfortunate, but it is entirely irrelevant
> to the ABI implications of changing the interpretation of an existing
> property.
> 
> Robin.

Agree, but I'd like to at least be shot down for a point I _am_ trying
to make, not for one I am not.

When I resumed this discussion I wasn't really focused on the patch set
that proposed to change some DT bindings. That was a bad idea, I abandoned
it, issue was solved (more or less) through other means, end of story.

Instead I focused on one of the arguments that Marc brought, that being
able to roll back kernel independently of firmware is important.
As a realist, I can't help but remark that this is effectively a non-goal.
There is always a risk that old kernels are caught off guard by DT
changes which they aren't prepared to handle, and even if I was aware of
the fact that I'm making a breaking change for old kernels when I'm
adding the 'interrupts-extended' property to the PHY (which I wasn't),
I'd still do it 10 out of 10 times.
I guess I'll always treat the 'old kernel works with new DT blob' case
as a pleasant surprise rather than the inviolable norm that Marc is
trying to make it sound like. Mentality difference between NXP/Marvell
vs Socionext aside, I really don't see how you can systematically avoid
these issues, so it's just a losing game to try to claim that every
firmware blob should work with every kernel, for the simple reason that
you can't change the past.

As to whether this has any implication on the point you and Marc are
trying to make, that the firmware ABI contract shouldn't change,
maybe not, probably not, especially if there are alternatives. But bring
a more solid argument to the table. Changing DT bindings is not off the
table _because_ old kernels will stop working with new DT blobs.