All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2021-12-14 13:58 ` Vladimir Oltean
  0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Oltean @ 2021-12-14 13:58 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Rob Herring, Shawn Guo, Li Yang, Biwen Li,
	Zhiqiang Hou, Kurt Kanzenbach, Rasmus Villemoes

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

The context is that the ls-extirq driver uses the standard
"interrupt-map" OF property in a non-standard way, as suggested by
Rob Herring during review:
https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/

This has turned out to be problematic, as Marc Zyngier discovered
through commit 041284181226 ("of/irq: Allow matching of an interrupt-map
local to an interrupt controller"), later fixed through commit
de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
definition of interrupt-map"). Marc's position, expressed on multiple
opportunities, is that:

(a) [ making private use of the reserved "interrupt-map" name in a
    driver ] "is wrong, by the very letter of what an interrupt-map
    means. If the interrupt map points to an interrupt controller,
    that's the target for the interrupt."
https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/

(b) [ updating the driver's bindings to accept a non-reserved name for
    this property, as an alternative, is ] "is totally pointless. These
    machines have been in the wild for years, and existing DTs will be
    there *forever*."
https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/

Considering the above, the Linux kernel has quirks in place to deal with
the ls-extirq's non-standard use of the "interrupt-map". These quirks
may be needed in other operating systems that consume this device tree,
yet this is seen as the only viable solution.

Therefore, the premise of the patch being reverted here is invalid.
It doesn't matter whether the driver, in its non-standard use of the
property, complies to the standard format or not, since this property
isn't expected to be used for interrupt translation by the core.

This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
previous bindings, which allows these systems to continue to use
external interrupt lines with the correct polarity.

Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map' parent address cells")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: remove the other 9 patches that rename "interrupt-map" to
        "fsl,extirq-map", at Marc's suggestion.

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2021-12-14 13:58 ` Vladimir Oltean
  0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Oltean @ 2021-12-14 13:58 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Rob Herring, Shawn Guo, Li Yang, Biwen Li,
	Zhiqiang Hou, Kurt Kanzenbach, Rasmus Villemoes

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

The context is that the ls-extirq driver uses the standard
"interrupt-map" OF property in a non-standard way, as suggested by
Rob Herring during review:
https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/

This has turned out to be problematic, as Marc Zyngier discovered
through commit 041284181226 ("of/irq: Allow matching of an interrupt-map
local to an interrupt controller"), later fixed through commit
de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
definition of interrupt-map"). Marc's position, expressed on multiple
opportunities, is that:

(a) [ making private use of the reserved "interrupt-map" name in a
    driver ] "is wrong, by the very letter of what an interrupt-map
    means. If the interrupt map points to an interrupt controller,
    that's the target for the interrupt."
https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/

(b) [ updating the driver's bindings to accept a non-reserved name for
    this property, as an alternative, is ] "is totally pointless. These
    machines have been in the wild for years, and existing DTs will be
    there *forever*."
https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/

Considering the above, the Linux kernel has quirks in place to deal with
the ls-extirq's non-standard use of the "interrupt-map". These quirks
may be needed in other operating systems that consume this device tree,
yet this is seen as the only viable solution.

Therefore, the premise of the patch being reverted here is invalid.
It doesn't matter whether the driver, in its non-standard use of the
property, complies to the standard format or not, since this property
isn't expected to be used for interrupt translation by the core.

This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
previous bindings, which allows these systems to continue to use
external interrupt lines with the correct polarity.

Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map' parent address cells")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: remove the other 9 patches that rename "interrupt-map" to
        "fsl,extirq-map", at Marc's suggestion.

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
  2021-12-14 13:58 ` Vladimir Oltean
@ 2021-12-31 18:13   ` Vladimir Oltean
  -1 siblings, 0 replies; 18+ messages in thread
From: Vladimir Oltean @ 2021-12-31 18:13 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Rob Herring, Shawn Guo, Leo Li, Biwen Li, Z.Q. Hou,
	Kurt Kanzenbach, Rasmus Villemoes

Hello,

On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
> This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> updated the expected device tree binding format for the ls-extirq
> driver, without also updating the parsing code (ls_extirq_parse_map)
> to the new format.
> 
> The context is that the ls-extirq driver uses the standard
> "interrupt-map" OF property in a non-standard way, as suggested by
> Rob Herring during review:
> https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
> 
> This has turned out to be problematic, as Marc Zyngier discovered
> through commit 041284181226 ("of/irq: Allow matching of an interrupt-map
> local to an interrupt controller"), later fixed through commit
> de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> definition of interrupt-map"). Marc's position, expressed on multiple
> opportunities, is that:
> 
> (a) [ making private use of the reserved "interrupt-map" name in a
>     driver ] "is wrong, by the very letter of what an interrupt-map
>     means. If the interrupt map points to an interrupt controller,
>     that's the target for the interrupt."
> https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
> 
> (b) [ updating the driver's bindings to accept a non-reserved name for
>     this property, as an alternative, is ] "is totally pointless. These
>     machines have been in the wild for years, and existing DTs will be
>     there *forever*."
> https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
> 
> Considering the above, the Linux kernel has quirks in place to deal with
> the ls-extirq's non-standard use of the "interrupt-map". These quirks
> may be needed in other operating systems that consume this device tree,
> yet this is seen as the only viable solution.
> 
> Therefore, the premise of the patch being reverted here is invalid.
> It doesn't matter whether the driver, in its non-standard use of the
> property, complies to the standard format or not, since this property
> isn't expected to be used for interrupt translation by the core.
> 
> This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
> previous bindings, which allows these systems to continue to use
> external interrupt lines with the correct polarity.
> 
> Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map' parent address cells")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> v1->v2: remove the other 9 patches that rename "interrupt-map" to
>         "fsl,extirq-map", at Marc's suggestion.

Could this patch be considered for merging in v5.16? The problem is
going to be quite a bit more severe and tricky to fix otherwise. Thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2021-12-31 18:13   ` Vladimir Oltean
  0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Oltean @ 2021-12-31 18:13 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Rob Herring, Shawn Guo, Leo Li, Biwen Li, Z.Q. Hou,
	Kurt Kanzenbach, Rasmus Villemoes

Hello,

On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
> This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> updated the expected device tree binding format for the ls-extirq
> driver, without also updating the parsing code (ls_extirq_parse_map)
> to the new format.
> 
> The context is that the ls-extirq driver uses the standard
> "interrupt-map" OF property in a non-standard way, as suggested by
> Rob Herring during review:
> https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
> 
> This has turned out to be problematic, as Marc Zyngier discovered
> through commit 041284181226 ("of/irq: Allow matching of an interrupt-map
> local to an interrupt controller"), later fixed through commit
> de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> definition of interrupt-map"). Marc's position, expressed on multiple
> opportunities, is that:
> 
> (a) [ making private use of the reserved "interrupt-map" name in a
>     driver ] "is wrong, by the very letter of what an interrupt-map
>     means. If the interrupt map points to an interrupt controller,
>     that's the target for the interrupt."
> https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
> 
> (b) [ updating the driver's bindings to accept a non-reserved name for
>     this property, as an alternative, is ] "is totally pointless. These
>     machines have been in the wild for years, and existing DTs will be
>     there *forever*."
> https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
> 
> Considering the above, the Linux kernel has quirks in place to deal with
> the ls-extirq's non-standard use of the "interrupt-map". These quirks
> may be needed in other operating systems that consume this device tree,
> yet this is seen as the only viable solution.
> 
> Therefore, the premise of the patch being reverted here is invalid.
> It doesn't matter whether the driver, in its non-standard use of the
> property, complies to the standard format or not, since this property
> isn't expected to be used for interrupt translation by the core.
> 
> This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
> previous bindings, which allows these systems to continue to use
> external interrupt lines with the correct polarity.
> 
> Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map' parent address cells")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> v1->v2: remove the other 9 patches that rename "interrupt-map" to
>         "fsl,extirq-map", at Marc's suggestion.

Could this patch be considered for merging in v5.16? The problem is
going to be quite a bit more severe and tricky to fix otherwise. Thanks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
  2021-12-31 18:13   ` Vladimir Oltean
@ 2022-01-02 13:08     ` Marc Zyngier
  -1 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2022-01-02 13:08 UTC (permalink / raw)
  To: Vladimir Oltean, Rob Herring, Shawn Guo
  Cc: devicetree, linux-kernel, linux-arm-kernel, Leo Li, Biwen Li,
	Z.Q. Hou, Kurt Kanzenbach, Rasmus Villemoes

On 2021-12-31 18:13, Vladimir Oltean wrote:
> Hello,
> 
> On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
>> This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
>> updated the expected device tree binding format for the ls-extirq
>> driver, without also updating the parsing code (ls_extirq_parse_map)
>> to the new format.
>> 
>> The context is that the ls-extirq driver uses the standard
>> "interrupt-map" OF property in a non-standard way, as suggested by
>> Rob Herring during review:
>> https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
>> 
>> This has turned out to be problematic, as Marc Zyngier discovered
>> through commit 041284181226 ("of/irq: Allow matching of an 
>> interrupt-map
>> local to an interrupt controller"), later fixed through commit
>> de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
>> definition of interrupt-map"). Marc's position, expressed on multiple
>> opportunities, is that:
>> 
>> (a) [ making private use of the reserved "interrupt-map" name in a
>>     driver ] "is wrong, by the very letter of what an interrupt-map
>>     means. If the interrupt map points to an interrupt controller,
>>     that's the target for the interrupt."
>> https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
>> 
>> (b) [ updating the driver's bindings to accept a non-reserved name for
>>     this property, as an alternative, is ] "is totally pointless. 
>> These
>>     machines have been in the wild for years, and existing DTs will be
>>     there *forever*."
>> https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
>> 
>> Considering the above, the Linux kernel has quirks in place to deal 
>> with
>> the ls-extirq's non-standard use of the "interrupt-map". These quirks
>> may be needed in other operating systems that consume this device 
>> tree,
>> yet this is seen as the only viable solution.
>> 
>> Therefore, the premise of the patch being reverted here is invalid.
>> It doesn't matter whether the driver, in its non-standard use of the
>> property, complies to the standard format or not, since this property
>> isn't expected to be used for interrupt translation by the core.
>> 
>> This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
>> previous bindings, which allows these systems to continue to use
>> external interrupt lines with the correct polarity.
>> 
>> Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map' 
>> parent address cells")
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>> ---
>> v1->v2: remove the other 9 patches that rename "interrupt-map" to
>>         "fsl,extirq-map", at Marc's suggestion.
> 
> Could this patch be considered for merging in v5.16? The problem is
> going to be quite a bit more severe and tricky to fix otherwise. 
> Thanks.

FWIW:

Acked-by: Marc Zyngier <maz@kernel.org>

Rob, Shawn, can you please queue this as an urgent fix for 5.16?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2022-01-02 13:08     ` Marc Zyngier
  0 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2022-01-02 13:08 UTC (permalink / raw)
  To: Vladimir Oltean, Rob Herring, Shawn Guo
  Cc: devicetree, linux-kernel, linux-arm-kernel, Leo Li, Biwen Li,
	Z.Q. Hou, Kurt Kanzenbach, Rasmus Villemoes

On 2021-12-31 18:13, Vladimir Oltean wrote:
> Hello,
> 
> On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
>> This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
>> updated the expected device tree binding format for the ls-extirq
>> driver, without also updating the parsing code (ls_extirq_parse_map)
>> to the new format.
>> 
>> The context is that the ls-extirq driver uses the standard
>> "interrupt-map" OF property in a non-standard way, as suggested by
>> Rob Herring during review:
>> https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
>> 
>> This has turned out to be problematic, as Marc Zyngier discovered
>> through commit 041284181226 ("of/irq: Allow matching of an 
>> interrupt-map
>> local to an interrupt controller"), later fixed through commit
>> de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
>> definition of interrupt-map"). Marc's position, expressed on multiple
>> opportunities, is that:
>> 
>> (a) [ making private use of the reserved "interrupt-map" name in a
>>     driver ] "is wrong, by the very letter of what an interrupt-map
>>     means. If the interrupt map points to an interrupt controller,
>>     that's the target for the interrupt."
>> https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
>> 
>> (b) [ updating the driver's bindings to accept a non-reserved name for
>>     this property, as an alternative, is ] "is totally pointless. 
>> These
>>     machines have been in the wild for years, and existing DTs will be
>>     there *forever*."
>> https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
>> 
>> Considering the above, the Linux kernel has quirks in place to deal 
>> with
>> the ls-extirq's non-standard use of the "interrupt-map". These quirks
>> may be needed in other operating systems that consume this device 
>> tree,
>> yet this is seen as the only viable solution.
>> 
>> Therefore, the premise of the patch being reverted here is invalid.
>> It doesn't matter whether the driver, in its non-standard use of the
>> property, complies to the standard format or not, since this property
>> isn't expected to be used for interrupt translation by the core.
>> 
>> This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
>> previous bindings, which allows these systems to continue to use
>> external interrupt lines with the correct polarity.
>> 
>> Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map' 
>> parent address cells")
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>> ---
>> v1->v2: remove the other 9 patches that rename "interrupt-map" to
>>         "fsl,extirq-map", at Marc's suggestion.
> 
> Could this patch be considered for merging in v5.16? The problem is
> going to be quite a bit more severe and tricky to fix otherwise. 
> Thanks.

FWIW:

Acked-by: Marc Zyngier <maz@kernel.org>

Rob, Shawn, can you please queue this as an urgent fix for 5.16?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
  2022-01-02 13:08     ` Marc Zyngier
@ 2022-01-03 11:30       ` Shawn Guo
  -1 siblings, 0 replies; 18+ messages in thread
From: Shawn Guo @ 2022-01-03 11:30 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Vladimir Oltean, Rob Herring, devicetree, linux-kernel,
	linux-arm-kernel, Leo Li, Biwen Li, Z.Q. Hou, Kurt Kanzenbach,
	Rasmus Villemoes

On Sun, Jan 02, 2022 at 01:08:28PM +0000, Marc Zyngier wrote:
> On 2021-12-31 18:13, Vladimir Oltean wrote:
> > Hello,
> > 
> > On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
> > > This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> > > updated the expected device tree binding format for the ls-extirq
> > > driver, without also updating the parsing code (ls_extirq_parse_map)
> > > to the new format.
> > > 
> > > The context is that the ls-extirq driver uses the standard
> > > "interrupt-map" OF property in a non-standard way, as suggested by
> > > Rob Herring during review:
> > > https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
> > > 
> > > This has turned out to be problematic, as Marc Zyngier discovered
> > > through commit 041284181226 ("of/irq: Allow matching of an
> > > interrupt-map
> > > local to an interrupt controller"), later fixed through commit
> > > de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> > > definition of interrupt-map"). Marc's position, expressed on multiple
> > > opportunities, is that:
> > > 
> > > (a) [ making private use of the reserved "interrupt-map" name in a
> > >     driver ] "is wrong, by the very letter of what an interrupt-map
> > >     means. If the interrupt map points to an interrupt controller,
> > >     that's the target for the interrupt."
> > > https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
> > > 
> > > (b) [ updating the driver's bindings to accept a non-reserved name for
> > >     this property, as an alternative, is ] "is totally pointless.
> > > These
> > >     machines have been in the wild for years, and existing DTs will be
> > >     there *forever*."
> > > https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
> > > 
> > > Considering the above, the Linux kernel has quirks in place to deal
> > > with
> > > the ls-extirq's non-standard use of the "interrupt-map". These quirks
> > > may be needed in other operating systems that consume this device
> > > tree,
> > > yet this is seen as the only viable solution.
> > > 
> > > Therefore, the premise of the patch being reverted here is invalid.
> > > It doesn't matter whether the driver, in its non-standard use of the
> > > property, complies to the standard format or not, since this property
> > > isn't expected to be used for interrupt translation by the core.
> > > 
> > > This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
> > > previous bindings, which allows these systems to continue to use
> > > external interrupt lines with the correct polarity.
> > > 
> > > Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map'
> > > parent address cells")
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > ---
> > > v1->v2: remove the other 9 patches that rename "interrupt-map" to
> > >         "fsl,extirq-map", at Marc's suggestion.
> > 
> > Could this patch be considered for merging in v5.16? The problem is
> > going to be quite a bit more severe and tricky to fix otherwise. Thanks.
> 
> FWIW:
> 
> Acked-by: Marc Zyngier <maz@kernel.org>
> 
> Rob, Shawn, can you please queue this as an urgent fix for 5.16?

I would rather leave this to Rob, as I haven't heard anything from him
on this reverting (on his commit).

Shawn

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2022-01-03 11:30       ` Shawn Guo
  0 siblings, 0 replies; 18+ messages in thread
From: Shawn Guo @ 2022-01-03 11:30 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Vladimir Oltean, Rob Herring, devicetree, linux-kernel,
	linux-arm-kernel, Leo Li, Biwen Li, Z.Q. Hou, Kurt Kanzenbach,
	Rasmus Villemoes

On Sun, Jan 02, 2022 at 01:08:28PM +0000, Marc Zyngier wrote:
> On 2021-12-31 18:13, Vladimir Oltean wrote:
> > Hello,
> > 
> > On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
> > > This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> > > updated the expected device tree binding format for the ls-extirq
> > > driver, without also updating the parsing code (ls_extirq_parse_map)
> > > to the new format.
> > > 
> > > The context is that the ls-extirq driver uses the standard
> > > "interrupt-map" OF property in a non-standard way, as suggested by
> > > Rob Herring during review:
> > > https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
> > > 
> > > This has turned out to be problematic, as Marc Zyngier discovered
> > > through commit 041284181226 ("of/irq: Allow matching of an
> > > interrupt-map
> > > local to an interrupt controller"), later fixed through commit
> > > de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> > > definition of interrupt-map"). Marc's position, expressed on multiple
> > > opportunities, is that:
> > > 
> > > (a) [ making private use of the reserved "interrupt-map" name in a
> > >     driver ] "is wrong, by the very letter of what an interrupt-map
> > >     means. If the interrupt map points to an interrupt controller,
> > >     that's the target for the interrupt."
> > > https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
> > > 
> > > (b) [ updating the driver's bindings to accept a non-reserved name for
> > >     this property, as an alternative, is ] "is totally pointless.
> > > These
> > >     machines have been in the wild for years, and existing DTs will be
> > >     there *forever*."
> > > https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
> > > 
> > > Considering the above, the Linux kernel has quirks in place to deal
> > > with
> > > the ls-extirq's non-standard use of the "interrupt-map". These quirks
> > > may be needed in other operating systems that consume this device
> > > tree,
> > > yet this is seen as the only viable solution.
> > > 
> > > Therefore, the premise of the patch being reverted here is invalid.
> > > It doesn't matter whether the driver, in its non-standard use of the
> > > property, complies to the standard format or not, since this property
> > > isn't expected to be used for interrupt translation by the core.
> > > 
> > > This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
> > > previous bindings, which allows these systems to continue to use
> > > external interrupt lines with the correct polarity.
> > > 
> > > Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map'
> > > parent address cells")
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > ---
> > > v1->v2: remove the other 9 patches that rename "interrupt-map" to
> > >         "fsl,extirq-map", at Marc's suggestion.
> > 
> > Could this patch be considered for merging in v5.16? The problem is
> > going to be quite a bit more severe and tricky to fix otherwise. Thanks.
> 
> FWIW:
> 
> Acked-by: Marc Zyngier <maz@kernel.org>
> 
> Rob, Shawn, can you please queue this as an urgent fix for 5.16?

I would rather leave this to Rob, as I haven't heard anything from him
on this reverting (on his commit).

Shawn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
  2022-01-03 11:30       ` Shawn Guo
@ 2022-02-09 11:54         ` Ioana Ciornei
  -1 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2022-02-09 11:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Marc Zyngier, Vladimir Oltean, Shawn Guo, Rob Herring,
	devicetree, linux-kernel, linux-arm-kernel, Leo Li, Biwen Li,
	Z.Q. Hou, Kurt Kanzenbach, Rasmus Villemoes

On Mon, Jan 03, 2022 at 07:30:44PM +0800, Shawn Guo wrote:
> On Sun, Jan 02, 2022 at 01:08:28PM +0000, Marc Zyngier wrote:
> > On 2021-12-31 18:13, Vladimir Oltean wrote:
> > > Hello,
> > > 
> > > On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
> > > > This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> > > > updated the expected device tree binding format for the ls-extirq
> > > > driver, without also updating the parsing code (ls_extirq_parse_map)
> > > > to the new format.
> > > > 
> > > > The context is that the ls-extirq driver uses the standard
> > > > "interrupt-map" OF property in a non-standard way, as suggested by
> > > > Rob Herring during review:
> > > > https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
> > > > 
> > > > This has turned out to be problematic, as Marc Zyngier discovered
> > > > through commit 041284181226 ("of/irq: Allow matching of an
> > > > interrupt-map
> > > > local to an interrupt controller"), later fixed through commit
> > > > de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> > > > definition of interrupt-map"). Marc's position, expressed on multiple
> > > > opportunities, is that:
> > > > 
> > > > (a) [ making private use of the reserved "interrupt-map" name in a
> > > >     driver ] "is wrong, by the very letter of what an interrupt-map
> > > >     means. If the interrupt map points to an interrupt controller,
> > > >     that's the target for the interrupt."
> > > > https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
> > > > 
> > > > (b) [ updating the driver's bindings to accept a non-reserved name for
> > > >     this property, as an alternative, is ] "is totally pointless.
> > > > These
> > > >     machines have been in the wild for years, and existing DTs will be
> > > >     there *forever*."
> > > > https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
> > > > 
> > > > Considering the above, the Linux kernel has quirks in place to deal
> > > > with
> > > > the ls-extirq's non-standard use of the "interrupt-map". These quirks
> > > > may be needed in other operating systems that consume this device
> > > > tree,
> > > > yet this is seen as the only viable solution.
> > > > 
> > > > Therefore, the premise of the patch being reverted here is invalid.
> > > > It doesn't matter whether the driver, in its non-standard use of the
> > > > property, complies to the standard format or not, since this property
> > > > isn't expected to be used for interrupt translation by the core.
> > > > 
> > > > This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
> > > > previous bindings, which allows these systems to continue to use
> > > > external interrupt lines with the correct polarity.
> > > > 
> > > > Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map'
> > > > parent address cells")
> > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > ---
> > > > v1->v2: remove the other 9 patches that rename "interrupt-map" to
> > > >         "fsl,extirq-map", at Marc's suggestion.
> > > 
> > > Could this patch be considered for merging in v5.16? The problem is
> > > going to be quite a bit more severe and tricky to fix otherwise. Thanks.
> > 
> > FWIW:
> > 
> > Acked-by: Marc Zyngier <maz@kernel.org>
> > 
> > Rob, Shawn, can you please queue this as an urgent fix for 5.16?
> 
> I would rather leave this to Rob, as I haven't heard anything from him
> on this reverting (on his commit).
> 

Could this patch be queued up as a fix for v5.16 and v5.17?

Ioana


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2022-02-09 11:54         ` Ioana Ciornei
  0 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2022-02-09 11:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Marc Zyngier, Vladimir Oltean, Shawn Guo, Rob Herring,
	devicetree, linux-kernel, linux-arm-kernel, Leo Li, Biwen Li,
	Z.Q. Hou, Kurt Kanzenbach, Rasmus Villemoes

On Mon, Jan 03, 2022 at 07:30:44PM +0800, Shawn Guo wrote:
> On Sun, Jan 02, 2022 at 01:08:28PM +0000, Marc Zyngier wrote:
> > On 2021-12-31 18:13, Vladimir Oltean wrote:
> > > Hello,
> > > 
> > > On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
> > > > This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> > > > updated the expected device tree binding format for the ls-extirq
> > > > driver, without also updating the parsing code (ls_extirq_parse_map)
> > > > to the new format.
> > > > 
> > > > The context is that the ls-extirq driver uses the standard
> > > > "interrupt-map" OF property in a non-standard way, as suggested by
> > > > Rob Herring during review:
> > > > https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
> > > > 
> > > > This has turned out to be problematic, as Marc Zyngier discovered
> > > > through commit 041284181226 ("of/irq: Allow matching of an
> > > > interrupt-map
> > > > local to an interrupt controller"), later fixed through commit
> > > > de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> > > > definition of interrupt-map"). Marc's position, expressed on multiple
> > > > opportunities, is that:
> > > > 
> > > > (a) [ making private use of the reserved "interrupt-map" name in a
> > > >     driver ] "is wrong, by the very letter of what an interrupt-map
> > > >     means. If the interrupt map points to an interrupt controller,
> > > >     that's the target for the interrupt."
> > > > https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
> > > > 
> > > > (b) [ updating the driver's bindings to accept a non-reserved name for
> > > >     this property, as an alternative, is ] "is totally pointless.
> > > > These
> > > >     machines have been in the wild for years, and existing DTs will be
> > > >     there *forever*."
> > > > https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
> > > > 
> > > > Considering the above, the Linux kernel has quirks in place to deal
> > > > with
> > > > the ls-extirq's non-standard use of the "interrupt-map". These quirks
> > > > may be needed in other operating systems that consume this device
> > > > tree,
> > > > yet this is seen as the only viable solution.
> > > > 
> > > > Therefore, the premise of the patch being reverted here is invalid.
> > > > It doesn't matter whether the driver, in its non-standard use of the
> > > > property, complies to the standard format or not, since this property
> > > > isn't expected to be used for interrupt translation by the core.
> > > > 
> > > > This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
> > > > previous bindings, which allows these systems to continue to use
> > > > external interrupt lines with the correct polarity.
> > > > 
> > > > Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map'
> > > > parent address cells")
> > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > ---
> > > > v1->v2: remove the other 9 patches that rename "interrupt-map" to
> > > >         "fsl,extirq-map", at Marc's suggestion.
> > > 
> > > Could this patch be considered for merging in v5.16? The problem is
> > > going to be quite a bit more severe and tricky to fix otherwise. Thanks.
> > 
> > FWIW:
> > 
> > Acked-by: Marc Zyngier <maz@kernel.org>
> > 
> > Rob, Shawn, can you please queue this as an urgent fix for 5.16?
> 
> I would rather leave this to Rob, as I haven't heard anything from him
> on this reverting (on his commit).
> 

Could this patch be queued up as a fix for v5.16 and v5.17?

Ioana


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
  2022-02-09 11:54         ` Ioana Ciornei
@ 2022-03-10 20:09           ` Vladimir Oltean
  -1 siblings, 0 replies; 18+ messages in thread
From: Vladimir Oltean @ 2022-03-10 20:09 UTC (permalink / raw)
  To: Rob Herring
  Cc: Ioana Ciornei, Marc Zyngier, Shawn Guo, devicetree, linux-kernel,
	linux-arm-kernel, Leo Li, Biwen Li, Z.Q. Hou, Kurt Kanzenbach,
	Rasmus Villemoes

Hello Rob,

On Wed, Feb 09, 2022 at 11:54:35AM +0000, Ioana Ciornei wrote:
> On Mon, Jan 03, 2022 at 07:30:44PM +0800, Shawn Guo wrote:
> > On Sun, Jan 02, 2022 at 01:08:28PM +0000, Marc Zyngier wrote:
> > > On 2021-12-31 18:13, Vladimir Oltean wrote:
> > > > Hello,
> > > > 
> > > > On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
> > > > > This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> > > > > updated the expected device tree binding format for the ls-extirq
> > > > > driver, without also updating the parsing code (ls_extirq_parse_map)
> > > > > to the new format.
> > > > > 
> > > > > The context is that the ls-extirq driver uses the standard
> > > > > "interrupt-map" OF property in a non-standard way, as suggested by
> > > > > Rob Herring during review:
> > > > > https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
> > > > > 
> > > > > This has turned out to be problematic, as Marc Zyngier discovered
> > > > > through commit 041284181226 ("of/irq: Allow matching of an
> > > > > interrupt-map
> > > > > local to an interrupt controller"), later fixed through commit
> > > > > de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> > > > > definition of interrupt-map"). Marc's position, expressed on multiple
> > > > > opportunities, is that:
> > > > > 
> > > > > (a) [ making private use of the reserved "interrupt-map" name in a
> > > > >     driver ] "is wrong, by the very letter of what an interrupt-map
> > > > >     means. If the interrupt map points to an interrupt controller,
> > > > >     that's the target for the interrupt."
> > > > > https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
> > > > > 
> > > > > (b) [ updating the driver's bindings to accept a non-reserved name for
> > > > >     this property, as an alternative, is ] "is totally pointless.
> > > > > These
> > > > >     machines have been in the wild for years, and existing DTs will be
> > > > >     there *forever*."
> > > > > https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
> > > > > 
> > > > > Considering the above, the Linux kernel has quirks in place to deal
> > > > > with
> > > > > the ls-extirq's non-standard use of the "interrupt-map". These quirks
> > > > > may be needed in other operating systems that consume this device
> > > > > tree,
> > > > > yet this is seen as the only viable solution.
> > > > > 
> > > > > Therefore, the premise of the patch being reverted here is invalid.
> > > > > It doesn't matter whether the driver, in its non-standard use of the
> > > > > property, complies to the standard format or not, since this property
> > > > > isn't expected to be used for interrupt translation by the core.
> > > > > 
> > > > > This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
> > > > > previous bindings, which allows these systems to continue to use
> > > > > external interrupt lines with the correct polarity.
> > > > > 
> > > > > Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map'
> > > > > parent address cells")
> > > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > > ---
> > > > > v1->v2: remove the other 9 patches that rename "interrupt-map" to
> > > > >         "fsl,extirq-map", at Marc's suggestion.
> > > > 
> > > > Could this patch be considered for merging in v5.16? The problem is
> > > > going to be quite a bit more severe and tricky to fix otherwise. Thanks.
> > > 
> > > FWIW:
> > > 
> > > Acked-by: Marc Zyngier <maz@kernel.org>
> > > 
> > > Rob, Shawn, can you please queue this as an urgent fix for 5.16?
> > 
> > I would rather leave this to Rob, as I haven't heard anything from him
> > on this reverting (on his commit).
> > 
> 
> Could this patch be queued up as a fix for v5.16 and v5.17?
> 
> Ioana

This is a reminder that interrupts through the fsl-extirq driver are
still broken. Not to mention that we aren't even in the same release
cycle as the patch that introduced the breakage anymore, so broken
device trees (or "syntactically correct", depending on how you wish to
see it) have already started circulating. Do you have a better
suggestion to fix this?

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2022-03-10 20:09           ` Vladimir Oltean
  0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Oltean @ 2022-03-10 20:09 UTC (permalink / raw)
  To: Rob Herring
  Cc: Ioana Ciornei, Marc Zyngier, Shawn Guo, devicetree, linux-kernel,
	linux-arm-kernel, Leo Li, Biwen Li, Z.Q. Hou, Kurt Kanzenbach,
	Rasmus Villemoes

Hello Rob,

On Wed, Feb 09, 2022 at 11:54:35AM +0000, Ioana Ciornei wrote:
> On Mon, Jan 03, 2022 at 07:30:44PM +0800, Shawn Guo wrote:
> > On Sun, Jan 02, 2022 at 01:08:28PM +0000, Marc Zyngier wrote:
> > > On 2021-12-31 18:13, Vladimir Oltean wrote:
> > > > Hello,
> > > > 
> > > > On Tue, Dec 14, 2021 at 03:58:52PM +0200, Vladimir Oltean wrote:
> > > > > This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> > > > > updated the expected device tree binding format for the ls-extirq
> > > > > driver, without also updating the parsing code (ls_extirq_parse_map)
> > > > > to the new format.
> > > > > 
> > > > > The context is that the ls-extirq driver uses the standard
> > > > > "interrupt-map" OF property in a non-standard way, as suggested by
> > > > > Rob Herring during review:
> > > > > https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
> > > > > 
> > > > > This has turned out to be problematic, as Marc Zyngier discovered
> > > > > through commit 041284181226 ("of/irq: Allow matching of an
> > > > > interrupt-map
> > > > > local to an interrupt controller"), later fixed through commit
> > > > > de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> > > > > definition of interrupt-map"). Marc's position, expressed on multiple
> > > > > opportunities, is that:
> > > > > 
> > > > > (a) [ making private use of the reserved "interrupt-map" name in a
> > > > >     driver ] "is wrong, by the very letter of what an interrupt-map
> > > > >     means. If the interrupt map points to an interrupt controller,
> > > > >     that's the target for the interrupt."
> > > > > https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
> > > > > 
> > > > > (b) [ updating the driver's bindings to accept a non-reserved name for
> > > > >     this property, as an alternative, is ] "is totally pointless.
> > > > > These
> > > > >     machines have been in the wild for years, and existing DTs will be
> > > > >     there *forever*."
> > > > > https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
> > > > > 
> > > > > Considering the above, the Linux kernel has quirks in place to deal
> > > > > with
> > > > > the ls-extirq's non-standard use of the "interrupt-map". These quirks
> > > > > may be needed in other operating systems that consume this device
> > > > > tree,
> > > > > yet this is seen as the only viable solution.
> > > > > 
> > > > > Therefore, the premise of the patch being reverted here is invalid.
> > > > > It doesn't matter whether the driver, in its non-standard use of the
> > > > > property, complies to the standard format or not, since this property
> > > > > isn't expected to be used for interrupt translation by the core.
> > > > > 
> > > > > This change restores LS1088A, LS2088A/LS2085A and LX2160A to their
> > > > > previous bindings, which allows these systems to continue to use
> > > > > external interrupt lines with the correct polarity.
> > > > > 
> > > > > Fixes: 869f0ec048dc ("arm64: dts: freescale: Fix 'interrupt-map'
> > > > > parent address cells")
> > > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > > ---
> > > > > v1->v2: remove the other 9 patches that rename "interrupt-map" to
> > > > >         "fsl,extirq-map", at Marc's suggestion.
> > > > 
> > > > Could this patch be considered for merging in v5.16? The problem is
> > > > going to be quite a bit more severe and tricky to fix otherwise. Thanks.
> > > 
> > > FWIW:
> > > 
> > > Acked-by: Marc Zyngier <maz@kernel.org>
> > > 
> > > Rob, Shawn, can you please queue this as an urgent fix for 5.16?
> > 
> > I would rather leave this to Rob, as I haven't heard anything from him
> > on this reverting (on his commit).
> > 
> 
> Could this patch be queued up as a fix for v5.16 and v5.17?
> 
> Ioana

This is a reminder that interrupts through the fsl-extirq driver are
still broken. Not to mention that we aren't even in the same release
cycle as the patch that introduced the breakage anymore, so broken
device trees (or "syntactically correct", depending on how you wish to
see it) have already started circulating. Do you have a better
suggestion to fix this?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
  2021-12-14 13:58 ` Vladimir Oltean
@ 2022-03-14 15:15   ` Rob Herring
  -1 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2022-03-14 15:15 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: devicetree, linux-kernel, linux-arm-kernel, Marc Zyngier,
	Shawn Guo, Li Yang, Biwen Li, Zhiqiang Hou, Kurt Kanzenbach,
	Rasmus Villemoes, Arnd Bergmann

On Tue, Dec 14, 2021 at 6:59 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> updated the expected device tree binding format for the ls-extirq
> driver, without also updating the parsing code (ls_extirq_parse_map)
> to the new format.

Sorry, I missed this until mentioned on IRC.

>
> The context is that the ls-extirq driver uses the standard
> "interrupt-map" OF property in a non-standard way, as suggested by
> Rob Herring during review:
> https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
>
> This has turned out to be problematic, as Marc Zyngier discovered
> through commit 041284181226 ("of/irq: Allow matching of an interrupt-map
> local to an interrupt controller"), later fixed through commit
> de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> definition of interrupt-map"). Marc's position, expressed on multiple
> opportunities, is that:
>
> (a) [ making private use of the reserved "interrupt-map" name in a
>     driver ] "is wrong, by the very letter of what an interrupt-map
>     means. If the interrupt map points to an interrupt controller,
>     that's the target for the interrupt."
> https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
>
> (b) [ updating the driver's bindings to accept a non-reserved name for
>     this property, as an alternative, is ] "is totally pointless. These
>     machines have been in the wild for years, and existing DTs will be
>     there *forever*."
> https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
>
> Considering the above, the Linux kernel has quirks in place to deal with
> the ls-extirq's non-standard use of the "interrupt-map". These quirks
> may be needed in other operating systems that consume this device tree,
> yet this is seen as the only viable solution.
>
> Therefore, the premise of the patch being reverted here is invalid.
> It doesn't matter whether the driver, in its non-standard use of the
> property, complies to the standard format or not, since this property
> isn't expected to be used for interrupt translation by the core.

I disagree. The non-standard part is that 'interrupt-map' translation
is not transparent. 'interrupt-map' that can't be parsed in the
standard way is just wrong, and I imagine was never the intention
here. We simply cannot have platforms defining their own format for
standard properties.

Reverting this will cause dtc warnings now (IIRC) and just kicks the
can down the road. Reverting is fine for now (I gave Arnd the okay on
IRC), but I think the parsing will need to be updated to honor
#address-cells and detect an old DT (probably by looking at the total
size of 'interrupt-map') and mark that change for stable. That would
only leave a new dt with an old kernel without stable updates broken.
Seems unlikely a device is getting firmware updates, but not OS
updates.

Rob

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2022-03-14 15:15   ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2022-03-14 15:15 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: devicetree, linux-kernel, linux-arm-kernel, Marc Zyngier,
	Shawn Guo, Li Yang, Biwen Li, Zhiqiang Hou, Kurt Kanzenbach,
	Rasmus Villemoes, Arnd Bergmann

On Tue, Dec 14, 2021 at 6:59 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> This reverts commit 869f0ec048dc8fd88c0b2003373bd985795179fb. That
> updated the expected device tree binding format for the ls-extirq
> driver, without also updating the parsing code (ls_extirq_parse_map)
> to the new format.

Sorry, I missed this until mentioned on IRC.

>
> The context is that the ls-extirq driver uses the standard
> "interrupt-map" OF property in a non-standard way, as suggested by
> Rob Herring during review:
> https://lore.kernel.org/lkml/20190927161118.GA19333@bogus/
>
> This has turned out to be problematic, as Marc Zyngier discovered
> through commit 041284181226 ("of/irq: Allow matching of an interrupt-map
> local to an interrupt controller"), later fixed through commit
> de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> definition of interrupt-map"). Marc's position, expressed on multiple
> opportunities, is that:
>
> (a) [ making private use of the reserved "interrupt-map" name in a
>     driver ] "is wrong, by the very letter of what an interrupt-map
>     means. If the interrupt map points to an interrupt controller,
>     that's the target for the interrupt."
> https://lore.kernel.org/lkml/87k0g8jlmg.wl-maz@kernel.org/
>
> (b) [ updating the driver's bindings to accept a non-reserved name for
>     this property, as an alternative, is ] "is totally pointless. These
>     machines have been in the wild for years, and existing DTs will be
>     there *forever*."
> https://lore.kernel.org/lkml/87ilvrk1r0.wl-maz@kernel.org/
>
> Considering the above, the Linux kernel has quirks in place to deal with
> the ls-extirq's non-standard use of the "interrupt-map". These quirks
> may be needed in other operating systems that consume this device tree,
> yet this is seen as the only viable solution.
>
> Therefore, the premise of the patch being reverted here is invalid.
> It doesn't matter whether the driver, in its non-standard use of the
> property, complies to the standard format or not, since this property
> isn't expected to be used for interrupt translation by the core.

I disagree. The non-standard part is that 'interrupt-map' translation
is not transparent. 'interrupt-map' that can't be parsed in the
standard way is just wrong, and I imagine was never the intention
here. We simply cannot have platforms defining their own format for
standard properties.

Reverting this will cause dtc warnings now (IIRC) and just kicks the
can down the road. Reverting is fine for now (I gave Arnd the okay on
IRC), but I think the parsing will need to be updated to honor
#address-cells and detect an old DT (probably by looking at the total
size of 'interrupt-map') and mark that change for stable. That would
only leave a new dt with an old kernel without stable updates broken.
Seems unlikely a device is getting firmware updates, but not OS
updates.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
  2022-03-14 15:15   ` Rob Herring
@ 2022-03-14 15:34     ` Marc Zyngier
  -1 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2022-03-14 15:34 UTC (permalink / raw)
  To: Rob Herring
  Cc: Vladimir Oltean, devicetree, linux-kernel, linux-arm-kernel,
	Shawn Guo, Li Yang, Biwen Li, Zhiqiang Hou, Kurt Kanzenbach,
	Rasmus Villemoes, Arnd Bergmann

On Mon, 14 Mar 2022 15:15:15 +0000,
Rob Herring <robh+dt@kernel.org> wrote:
> 
> On Tue, Dec 14, 2021 at 6:59 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > Therefore, the premise of the patch being reverted here is invalid.
> > It doesn't matter whether the driver, in its non-standard use of the
> > property, complies to the standard format or not, since this property
> > isn't expected to be used for interrupt translation by the core.
> 
> I disagree. The non-standard part is that 'interrupt-map' translation
> is not transparent. 'interrupt-map' that can't be parsed in the
> standard way is just wrong, and I imagine was never the intention
> here. We simply cannot have platforms defining their own format for
> standard properties.

That ship sailed a long while ago. We have a list of offenders, and we
can make sure we don't get additional ones.

> Reverting this will cause dtc warnings now (IIRC) and just kicks the
> can down the road. Reverting is fine for now (I gave Arnd the okay on
> IRC), but I think the parsing will need to be updated to honor
> #address-cells and detect an old DT (probably by looking at the total
> size of 'interrupt-map') and mark that change for stable. That would
> only leave a new dt with an old kernel without stable updates broken.
> Seems unlikely a device is getting firmware updates, but not OS
> updates.

Being able to rollback firmware and OS independently is important. The
tooling can be taught about the broken instances, which should be
enough.  Adding to the parsing only makes things harder to maintain,
for no real gain.

	M.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2022-03-14 15:34     ` Marc Zyngier
  0 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2022-03-14 15:34 UTC (permalink / raw)
  To: Rob Herring
  Cc: Vladimir Oltean, devicetree, linux-kernel, linux-arm-kernel,
	Shawn Guo, Li Yang, Biwen Li, Zhiqiang Hou, Kurt Kanzenbach,
	Rasmus Villemoes, Arnd Bergmann

On Mon, 14 Mar 2022 15:15:15 +0000,
Rob Herring <robh+dt@kernel.org> wrote:
> 
> On Tue, Dec 14, 2021 at 6:59 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > Therefore, the premise of the patch being reverted here is invalid.
> > It doesn't matter whether the driver, in its non-standard use of the
> > property, complies to the standard format or not, since this property
> > isn't expected to be used for interrupt translation by the core.
> 
> I disagree. The non-standard part is that 'interrupt-map' translation
> is not transparent. 'interrupt-map' that can't be parsed in the
> standard way is just wrong, and I imagine was never the intention
> here. We simply cannot have platforms defining their own format for
> standard properties.

That ship sailed a long while ago. We have a list of offenders, and we
can make sure we don't get additional ones.

> Reverting this will cause dtc warnings now (IIRC) and just kicks the
> can down the road. Reverting is fine for now (I gave Arnd the okay on
> IRC), but I think the parsing will need to be updated to honor
> #address-cells and detect an old DT (probably by looking at the total
> size of 'interrupt-map') and mark that change for stable. That would
> only leave a new dt with an old kernel without stable updates broken.
> Seems unlikely a device is getting firmware updates, but not OS
> updates.

Being able to rollback firmware and OS independently is important. The
tooling can be taught about the broken instances, which should be
enough.  Adding to the parsing only makes things harder to maintain,
for no real gain.

	M.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
  2022-03-14 15:34     ` Marc Zyngier
@ 2022-03-14 16:08       ` Rob Herring
  -1 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2022-03-14 16:08 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Vladimir Oltean, devicetree, linux-kernel, linux-arm-kernel,
	Shawn Guo, Li Yang, Biwen Li, Zhiqiang Hou, Kurt Kanzenbach,
	Rasmus Villemoes, Arnd Bergmann

On Mon, Mar 14, 2022 at 9:34 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 14 Mar 2022 15:15:15 +0000,
> Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Tue, Dec 14, 2021 at 6:59 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > Therefore, the premise of the patch being reverted here is invalid.
> > > It doesn't matter whether the driver, in its non-standard use of the
> > > property, complies to the standard format or not, since this property
> > > isn't expected to be used for interrupt translation by the core.
> >
> > I disagree. The non-standard part is that 'interrupt-map' translation
> > is not transparent. 'interrupt-map' that can't be parsed in the
> > standard way is just wrong, and I imagine was never the intention
> > here. We simply cannot have platforms defining their own format for
> > standard properties.
>
> That ship sailed a long while ago. We have a list of offenders, and we
> can make sure we don't get additional ones.
>
> > Reverting this will cause dtc warnings now (IIRC) and just kicks the
> > can down the road. Reverting is fine for now (I gave Arnd the okay on
> > IRC), but I think the parsing will need to be updated to honor
> > #address-cells and detect an old DT (probably by looking at the total
> > size of 'interrupt-map') and mark that change for stable. That would
> > only leave a new dt with an old kernel without stable updates broken.
> > Seems unlikely a device is getting firmware updates, but not OS
> > updates.
>
> Being able to rollback firmware and OS independently is important. The
> tooling can be taught about the broken instances, which should be
> enough.  Adding to the parsing only makes things harder to maintain,
> for no real gain.

It's up to individual platforms to care about that. If they don't,
then compatibility has been broken multiple times most certainly
because in general there is 0 testing for it. Why make our lives more
complicated in those cases?

Rob

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells"
@ 2022-03-14 16:08       ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2022-03-14 16:08 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Vladimir Oltean, devicetree, linux-kernel, linux-arm-kernel,
	Shawn Guo, Li Yang, Biwen Li, Zhiqiang Hou, Kurt Kanzenbach,
	Rasmus Villemoes, Arnd Bergmann

On Mon, Mar 14, 2022 at 9:34 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 14 Mar 2022 15:15:15 +0000,
> Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Tue, Dec 14, 2021 at 6:59 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > Therefore, the premise of the patch being reverted here is invalid.
> > > It doesn't matter whether the driver, in its non-standard use of the
> > > property, complies to the standard format or not, since this property
> > > isn't expected to be used for interrupt translation by the core.
> >
> > I disagree. The non-standard part is that 'interrupt-map' translation
> > is not transparent. 'interrupt-map' that can't be parsed in the
> > standard way is just wrong, and I imagine was never the intention
> > here. We simply cannot have platforms defining their own format for
> > standard properties.
>
> That ship sailed a long while ago. We have a list of offenders, and we
> can make sure we don't get additional ones.
>
> > Reverting this will cause dtc warnings now (IIRC) and just kicks the
> > can down the road. Reverting is fine for now (I gave Arnd the okay on
> > IRC), but I think the parsing will need to be updated to honor
> > #address-cells and detect an old DT (probably by looking at the total
> > size of 'interrupt-map') and mark that change for stable. That would
> > only leave a new dt with an old kernel without stable updates broken.
> > Seems unlikely a device is getting firmware updates, but not OS
> > updates.
>
> Being able to rollback firmware and OS independently is important. The
> tooling can be taught about the broken instances, which should be
> enough.  Adding to the parsing only makes things harder to maintain,
> for no real gain.

It's up to individual platforms to care about that. If they don't,
then compatibility has been broken multiple times most certainly
because in general there is 0 testing for it. Why make our lives more
complicated in those cases?

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-03-14 16:10 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-14 13:58 [PATCH v2 devicetree] Revert "arm64: dts: freescale: Fix 'interrupt-map' parent address cells" Vladimir Oltean
2021-12-14 13:58 ` Vladimir Oltean
2021-12-31 18:13 ` Vladimir Oltean
2021-12-31 18:13   ` Vladimir Oltean
2022-01-02 13:08   ` Marc Zyngier
2022-01-02 13:08     ` Marc Zyngier
2022-01-03 11:30     ` Shawn Guo
2022-01-03 11:30       ` Shawn Guo
2022-02-09 11:54       ` Ioana Ciornei
2022-02-09 11:54         ` Ioana Ciornei
2022-03-10 20:09         ` Vladimir Oltean
2022-03-10 20:09           ` Vladimir Oltean
2022-03-14 15:15 ` Rob Herring
2022-03-14 15:15   ` Rob Herring
2022-03-14 15:34   ` Marc Zyngier
2022-03-14 15:34     ` Marc Zyngier
2022-03-14 16:08     ` Rob Herring
2022-03-14 16:08       ` Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.