| Message ID | 1433874401-27161-3-git-send-email-robh@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 09, 2015 at 01:26:28PM -0500, Rob Herring wrote: > clear that is really needed. There appears to be a great deal of blind > copy and paste of this code. Well, I don't think it's blind - IIRC it's just that this code all predates the genirq flags and ARM had the opposite default to x86 (I guess because of being conservative about all the random interrupt controllers you get and what might be hanging off them) so we needed to do this dance to get interrupts to work. Anyway, I've applied this - thanks.
On Tue, Jun 09, 2015 at 08:12:11PM +0100, Mark Brown wrote: > On Tue, Jun 09, 2015 at 01:26:28PM -0500, Rob Herring wrote: > > > clear that is really needed. There appears to be a great deal of blind > > copy and paste of this code. > > Well, I don't think it's blind - IIRC it's just that this code all > predates the genirq flags and ARM had the opposite default to x86 (I > guess because of being conservative about all the random interrupt > controllers you get and what might be hanging off them) so we needed to > do this dance to get interrupts to work. It's because there exist platforms where specific IRQs are permanently asserted, and unexpectedly claiming such an interrupt (eg, through IRQ probing) would lock the system - at least before we ended up with the detection in genirq (which has its roots in the ARM code.) The problem is we're dealing with history here, and flipping ARM from defaulting to "IRQs not valid" to "All IRQs valid" is a far from trivial task. I'd suggest people think carefully about applying these patches. They have only been around for a matter of hours, so to rush to apply them when they haven't been revewied as a whole to assess whether the approach is the right one is rather hasty.
On Tue, Jun 09, 2015 at 10:27:09PM +0100, Russell King - ARM Linux wrote: > It's because there exist platforms where specific IRQs are permanently > asserted, and unexpectedly claiming such an interrupt (eg, through IRQ > probing) would lock the system - at least before we ended up with the > detection in genirq (which has its roots in the ARM code.) Ah, yes - now I remember. I just had a vauge recollection of broken probing. > I'd suggest people think carefully about applying these patches. They > have only been around for a matter of hours, so to rush to apply them > when they haven't been revewied as a whole to assess whether the approach > is the right one is rather hasty. I'm fairly comfortable with the patch for regmap as that code is architecture neutral, should already have been consistent and the non-ARM path has definitely been tested (I've done it myself).
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index a6c3f75..a9cb98a 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -306,14 +306,7 @@ static int regmap_irq_map(struct irq_domain *h, unsigned int virq, irq_set_chip_data(virq, data); irq_set_chip(virq, &data->irq_chip); irq_set_nested_thread(virq, 1); - - /* ARM needs us to explicitly flag the IRQ as valid - * and will set them noprobe when we do so. */ -#ifdef CONFIG_ARM - set_irq_flags(virq, IRQF_VALID); -#else irq_set_noprobe(virq); -#endif return 0; }
set_irq_flags is ARM specific with custom flags which have genirq equivalents. Convert drivers to use the genirq interfaces directly, so we can kill off set_irq_flags. The translation of flags is as follows: IRQF_VALID -> !IRQ_NOREQUEST IRQF_PROBE -> !IRQ_NOPROBE IRQF_NOAUTOEN -> IRQ_NOAUTOEN For IRQs managed by an irqdomain, the irqdomain core code handles clearing and setting IRQ_NOREQUEST already, so there is no need to do this in .map() functions and we can simply remove the set_irq_flags calls. Some users also set IRQ_NOPROBE and this has been maintained although it is not clear that is really needed. There appears to be a great deal of blind copy and paste of this code. Signed-off-by: Rob Herring <robh@kernel.org> Cc: Mark Brown <broonie@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/base/regmap/regmap-irq.c | 7 ------- 1 file changed, 7 deletions(-)