| Message ID | 20211214013800.2703568-2-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Do something about ls-extirq interrupt-map breakage | expand |
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
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/
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);
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(-)