| Message ID | 20111110162859.GA7088@oksana.dev.rtsoft.ru |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On 11/10/2011 11:28 AM, Anton Vorontsov wrote: > Drivers should not use NO_IRQ; moreover, some architectures don't > have it nowadays. '0' is the 'no irq' case. > > Signed-off-by: Anton Vorontsov<cbouatmailru@gmail.com> > Acked-by: Alan Cox<alan@linux.intel.com> > --- > > On Thu, Nov 10, 2011 at 03:38:16PM +0000, Alan Cox wrote: >> On Thu, 10 Nov 2011 19:26:06 +0400 >> Anton Vorontsov<cbouatmailru@gmail.com> wrote: >> >>> Drivers should not use NO_IRQ; moreover, some architectures don't >>> have it nowadays. '0' is the 'no irq' case. >>> >>> Signed-off-by: Anton Vorontsov<cbouatmailru@gmail.com> >> >> Acked-by: Alan Cox<alan@linux.intel.com> > > In case if we don't want a "band-aid fix" for 3.2, here is the patch > that just does the proper fix (w/ a risk to break minor architectures). > > drivers/ata/pata_of_platform.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c > index a72ab0d..2a472c5 100644 > --- a/drivers/ata/pata_of_platform.c > +++ b/drivers/ata/pata_of_platform.c > @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev) > } > > ret = of_irq_to_resource(dn, 0,&irq_res); > - if (ret == NO_IRQ) > + if (!ret) > irq_res.start = irq_res.end = 0; > else > irq_res.flags = 0; Unless someone screams, that is what I'll push upstream. Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 10, 2011 at 08:28:59PM +0400, Anton Vorontsov wrote: > Drivers should not use NO_IRQ; moreover, some architectures don't > have it nowadays. '0' is the 'no irq' case. > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> > Acked-by: Alan Cox <alan@linux.intel.com> > --- > > On Thu, Nov 10, 2011 at 03:38:16PM +0000, Alan Cox wrote: > > On Thu, 10 Nov 2011 19:26:06 +0400 > > Anton Vorontsov <cbouatmailru@gmail.com> wrote: > > > > > Drivers should not use NO_IRQ; moreover, some architectures don't > > > have it nowadays. '0' is the 'no irq' case. > > > > > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> > > > > Acked-by: Alan Cox <alan@linux.intel.com> > > In case if we don't want a "band-aid fix" for 3.2, here is the patch > that just does the proper fix (w/ a risk to break minor architectures). This is now broken on ARM where, for good or bad, NO_IRQ currently is used and is -1. How do we resolve it? If we are ready to eliminate NO_IRQ from drivers/of/irq.c (or indeed, all code that uses it) and just use 0 for that case, we should surely just do it... but I'm not confident I can judge on that. Half-removing NO_IRQ is going to be problematic, though... I really don't care whether the "no irq" value is 0 or -1, but it is abundantly clear that choosing different values to mean the same thing on opposite sides of an interface does not work. Cheers ---Dave > > drivers/ata/pata_of_platform.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c > index a72ab0d..2a472c5 100644 > --- a/drivers/ata/pata_of_platform.c > +++ b/drivers/ata/pata_of_platform.c > @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev) > } > > ret = of_irq_to_resource(dn, 0, &irq_res); > - if (ret == NO_IRQ) > + if (!ret) > irq_res.start = irq_res.end = 0; > else > irq_res.flags = 0; > -- > 1.7.5.3 > > _______________________________________________ > devicetree-discuss mailing list > devicetree-discuss@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[expanding CC -- apologies to anyone who gets this mail twice] On Thu, Nov 10, 2011 at 08:28:59PM +0400, Anton Vorontsov wrote: > Drivers should not use NO_IRQ; moreover, some architectures don't > have it nowadays. '0' is the 'no irq' case. > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> > Acked-by: Alan Cox <alan@linux.intel.com> > --- > > On Thu, Nov 10, 2011 at 03:38:16PM +0000, Alan Cox wrote: > > On Thu, 10 Nov 2011 19:26:06 +0400 > > Anton Vorontsov <cbouatmailru@gmail.com> wrote: > > > > > Drivers should not use NO_IRQ; moreover, some architectures don't > > > have it nowadays. '0' is the 'no irq' case. > > > > > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> > > > > Acked-by: Alan Cox <alan@linux.intel.com> > > In case if we don't want a "band-aid fix" for 3.2, here is the patch > that just does the proper fix (w/ a risk to break minor architectures). This is now broken on ARM where, for good or bad, NO_IRQ currently is used and is -1. How do we resolve it? If we are ready to eliminate NO_IRQ from drivers/of/irq.c (or indeed, all code that uses it) and just use 0 for that case, we should surely just do it... but I'm not confident I can judge on that. Half-removing NO_IRQ is going to be problematic, though... I really don't care whether the "no irq" value is 0 or -1, but it is abundantly clear that choosing different values to mean the same thing on opposite sides of an interface does not work. Cheers ---Dave > > drivers/ata/pata_of_platform.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c > index a72ab0d..2a472c5 100644 > --- a/drivers/ata/pata_of_platform.c > +++ b/drivers/ata/pata_of_platform.c > @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev) > } > > ret = of_irq_to_resource(dn, 0, &irq_res); > - if (ret == NO_IRQ) > + if (!ret) > irq_res.start = irq_res.end = 0; > else > irq_res.flags = 0; > -- > 1.7.5.3 > > _______________________________________________ > devicetree-discuss mailing list > devicetree-discuss@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 2, 2011 at 11:26 AM, Dave Martin <dave.martin@linaro.org> wrote: > > This is now broken on ARM where, for good or bad, NO_IRQ currently is > used and is -1. > > How do we resolve it? If we are ready to eliminate NO_IRQ from > drivers/of/irq.c (or indeed, all code that uses it) and just use 0 for > that case, we should surely just do it... but I'm not confident I can > judge on that. Just stop using NO_IRQ. First in drivers/of/irq.c, then in any drivers as you notice breakage. Don't *change* NO_IRQ to zero (that whole #define is broken - leave it around as a marker of brokenness), just start removing it from all the ARM drivers that use the OF infrastructure. Which is presumably not all that many yet. So whenever you find breakage, the fix now is to just remove NO_IRQ tests, and replace them with "!irq". Linus -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 02, 2011 at 07:19:17PM +0000, Dave Martin wrote: [...] > > > > Drivers should not use NO_IRQ; moreover, some architectures don't > > > > have it nowadays. '0' is the 'no irq' case. > > > > > > > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> > > > > > > Acked-by: Alan Cox <alan@linux.intel.com> > > > > In case if we don't want a "band-aid fix" for 3.2, here is the patch > > that just does the proper fix (w/ a risk to break minor architectures). > > This is now broken on ARM where, for good or bad, NO_IRQ currently is > used and is -1. > > How do we resolve it? One option is to test this patch on a board that is now broken: http://lkml.org/lkml/2011/11/10/290 So that someone provide Tested-by tag. With the tag we probably can push the patch for 3.2, and thus fix the issue once and forever. The other option is to revert the correct fix, and push the bogus one, i.e. this: http://lkml.org/lkml/2011/11/10/287 But that last option is less likely, as this was NAKed by Alan Cox. Thanks,
On Sat, Dec 03, 2011 at 02:34:02AM +0400, Anton Vorontsov wrote: > On Fri, Dec 02, 2011 at 07:19:17PM +0000, Dave Martin wrote: > [...] > > > > > Drivers should not use NO_IRQ; moreover, some architectures don't > > > > > have it nowadays. '0' is the 'no irq' case. > > > > > > > > > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> > > > > > > > > Acked-by: Alan Cox <alan@linux.intel.com> > > > > > > In case if we don't want a "band-aid fix" for 3.2, here is the patch > > > that just does the proper fix (w/ a risk to break minor architectures). > > > > This is now broken on ARM where, for good or bad, NO_IRQ currently is > > used and is -1. > > > > How do we resolve it? > > One option is to test this patch on a board that is now broken: > > http://lkml.org/lkml/2011/11/10/290 Oh, actually, reading my own patch: "ARM defines NO_IRQ to -1, but OF code relies on IRQ domains support, which returns correct ('0') value in 'no irq' case. So everything should be fine." I forgot that on ARM we use IRQ domains, so ARM should be OK. Do you really see any breakage, and if so, what board? Thanks,
On Fri, Dec 2, 2011 at 2:34 PM, Anton Vorontsov <anton.vorontsov@linaro.org> wrote: > > One option is to test this patch on a board that is now broken: > > http://lkml.org/lkml/2011/11/10/290 That seems broken. Spot the trouble: + ret = irq_create_of_mapping(oirq.controller, oirq.specifier, + oirq.size); +no_irq: +#ifdef NO_IRQ +#if NO_IRQ != 0 + if (ret == NO_IRQ) + pr_warn("Hit NO_IRQ case for your arch. Drivers might expect " + "NO_IRQ, but we return 0. If anything breaks, driver " + "have to be fixed.\n"); +#endif +#endif + return ret; It claims "we return 0", but then doesn't return zero.. Hmm? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 03, 2011 at 02:40:18AM +0400, Anton Vorontsov wrote: > On Sat, Dec 03, 2011 at 02:34:02AM +0400, Anton Vorontsov wrote: > > On Fri, Dec 02, 2011 at 07:19:17PM +0000, Dave Martin wrote: > > [...] > > > > > > Drivers should not use NO_IRQ; moreover, some architectures don't > > > > > > have it nowadays. '0' is the 'no irq' case. > > > > > > > > > > > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com> > > > > > > > > > > Acked-by: Alan Cox <alan@linux.intel.com> > > > > > > > > In case if we don't want a "band-aid fix" for 3.2, here is the patch > > > > that just does the proper fix (w/ a risk to break minor architectures). > > > > > > This is now broken on ARM where, for good or bad, NO_IRQ currently is > > > used and is -1. > > > > > > How do we resolve it? > > > > One option is to test this patch on a board that is now broken: > > > > http://lkml.org/lkml/2011/11/10/290 > > Oh, actually, reading my own patch: > > "ARM defines NO_IRQ to -1, but OF code relies on IRQ domains support, > which returns correct ('0') value in 'no irq' case. So everything > should be fine." Ahh. Forget it, the remark was for the of/irq.c fix itself. So, we need the http://lkml.org/lkml/2011/11/10/290 fix. Otherwise the driver is indeed broken for ARM. Would be great if somebody could test it. Thanks,
On Fri, 2011-12-02 at 11:28 -0800, Linus Torvalds wrote: > On Fri, Dec 2, 2011 at 11:26 AM, Dave Martin <dave.martin@linaro.org> wrote: > > > > This is now broken on ARM where, for good or bad, NO_IRQ currently is > > used and is -1. > > > > How do we resolve it? If we are ready to eliminate NO_IRQ from > > drivers/of/irq.c (or indeed, all code that uses it) and just use 0 for > > that case, we should surely just do it... but I'm not confident I can > > judge on that. > > Just stop using NO_IRQ. First in drivers/of/irq.c, then in any drivers > as you notice breakage. Agreed. In fact the whole hack in drivers/of/irq.c was to accomodate ARM which still uses -1, powerpc changed to 0 a long time ago. Now that we have a generic remapper between HW and "linux" IRQ numbers, there is no reason to stick to -1 even on ARM. > Don't *change* NO_IRQ to zero (that whole #define is broken - leave it > around as a marker of brokenness), just start removing it from all the > ARM drivers that use the OF infrastructure. Which is presumably not > all that many yet. > > So whenever you find breakage, the fix now is to just remove NO_IRQ > tests, and replace them with "!irq". Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> This is now broken on ARM where, for good or bad, NO_IRQ currently is > used and is -1. Good. ARM developers have been told to change this for several years. The nice approach hasn't worked, the patient approach hasn't worked so now finally ARM is going to be dragged kicking and screaming into doing the work everyone else did several years ago. I have so little sympathy over this that you'll need a quantum physicist to measure it. > Half-removing NO_IRQ is going to be problematic, though... > I really don't care whether the "no irq" value is 0 or -1, but it is > abundantly clear that choosing different values to mean the same thing > on opposite sides of an interface does not work. You've had years to fix it. If I were you I'd delete NO_IRQ from your tree, type make and get it done. It's not even a big job to clean it out. At that point various other drivers will also start working properly on ARM because they use 0 for polled mode. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 3, 2011 at 00:22, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: >> This is now broken on ARM where, for good or bad, NO_IRQ currently is >> used and is -1. > > Good. > > ARM developers have been told to change this for several years. The nice > approach hasn't worked, the patient approach hasn't worked so now finally > ARM is going to be dragged kicking and screaming into doing the work > everyone else did several years ago. > > I have so little sympathy over this that you'll need a quantum physicist > to measure it. > >> Half-removing NO_IRQ is going to be problematic, though... >> I really don't care whether the "no irq" value is 0 or -1, but it is >> abundantly clear that choosing different values to mean the same thing >> on opposite sides of an interface does not work. > > You've had years to fix it. If I were you I'd delete NO_IRQ from your > tree, type make and get it done. It's not even a big job to clean it out. > > At that point various other drivers will also start working properly on > ARM because they use 0 for polled mode. Not just ARM: arch/arm/include/asm/irq.h:#ifndef NO_IRQarch/arm/include/asm/irq.h:#define NO_IRQ ((unsigned int)(-1))arch/microblaze/include/asm/irq.h:#define NO_IRQ (-1)arch/mn10300/include/asm/irq.h:#define NO_IRQ INT_MAXarch/openrisc/include/asm/irq.h:#define NO_IRQ (-1)arch/parisc/include/asm/irq.h:#define NO_IRQ (-1)arch/powerpc/include/asm/irq.h:#define NO_IRQ (0)arch/powerpc/include/asm/machdep.h: /* Return an irq, or NO_IRQ to indicate arch/powerpc/include/asm/parport.h: if (virq == NO_IRQ)arch/powerpc/include/asm/qe_ic.h: if (cascade_irq != NO_IRQ)arch/powerpc/include/asm/qe_ic.h: if (cascade_irq != NO_IRQ)arch/powerpc/include/asm/qe_ic.h: if (cascade_irq != NO_IRQ)arch/powerpc/include/asm/qe_ic.h: if (cascade_irq != NO_IRQ)arch/powerpc/include/asm/qe_ic.h: if (cascade_irq == NO_IRQ)arch/powerpc/include/asm/qe_ic.h: if (cascade_irq != NO_IRQ)arch/sparc/include/asm/irq_32.h:#define NO_IRQ 0xffffffffarch/sparc/include/asm/irq_64.h:#define NO_IRQ 0xffffffff And it's not just definitions of NO_IRQ. These are easy to find. On some archs (notably ARM) zero still seems to be a valid IRQ number, e.g. IRQ_LOCOMO_KEY and IRQ_DMA0C0. Also, UML has TIMER_IRQ being zero. A quick grep found many more IRQ definitions being zero, but surprisingly the few I looked into were definitions without users (e.g. SE7343_FPGA_IRQ_MRSHPC0, ROUTE_VIA_IRQ0 aka IRQ_MB93493_VDC_ROUTE). Perhaps request_irq() should just reject zero to find all of them? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 03, 2011 at 10:12:53AM +1100, Benjamin Herrenschmidt wrote: > On Fri, 2011-12-02 at 11:28 -0800, Linus Torvalds wrote: > > On Fri, Dec 2, 2011 at 11:26 AM, Dave Martin <dave.martin@linaro.org> wrote: > > > > > > This is now broken on ARM where, for good or bad, NO_IRQ currently is > > > used and is -1. > > > > > > How do we resolve it? If we are ready to eliminate NO_IRQ from > > > drivers/of/irq.c (or indeed, all code that uses it) and just use 0 for > > > that case, we should surely just do it... but I'm not confident I can > > > judge on that. > > > > Just stop using NO_IRQ. First in drivers/of/irq.c, then in any drivers > > as you notice breakage. > > Agreed. In fact the whole hack in drivers/of/irq.c was to accomodate ARM > which still uses -1, powerpc changed to 0 a long time ago. > > Now that we have a generic remapper between HW and "linux" IRQ numbers, > there is no reason to stick to -1 even on ARM. > > > Don't *change* NO_IRQ to zero (that whole #define is broken - leave it > > around as a marker of brokenness), just start removing it from all the > > ARM drivers that use the OF infrastructure. Which is presumably not > > all that many yet. > > > > So whenever you find breakage, the fix now is to just remove NO_IRQ > > tests, and replace them with "!irq". > Russell, do you know whether it would make sense to set a timeline for removing NO_IRQ from ARM platforms and migrating to 0 for the no-interrupt case? I'm assuming that this mainly involves migrating existing hard-wired code that deals with interrupt numbers to use irq domains. I worry that if we just change the convention for the OF case, we'll end up with OF-ised platform drivers which have to deal with a different no- irq convention depending on whether they are probed as platform drivers or through the OF framework ... and these ported or semi-ported drivers will be intermixed with unported drivers, confusing maintainers. If board code starts initialising platform data for non-OF-ised platform drivers based on IRQ numbers fetched via the OF code, things will get even more confused... Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 5 Dec 2011, Dave Martin wrote: > On Sat, Dec 03, 2011 at 10:12:53AM +1100, Benjamin Herrenschmidt wrote: > > On Fri, 2011-12-02 at 11:28 -0800, Linus Torvalds wrote: > > > Don't *change* NO_IRQ to zero (that whole #define is broken - leave it > > > around as a marker of brokenness), just start removing it from all the > > > ARM drivers that use the OF infrastructure. Which is presumably not > > > all that many yet. > > > > > > So whenever you find breakage, the fix now is to just remove NO_IRQ > > > tests, and replace them with "!irq". > > > > Russell, do you know whether it would make sense to set a timeline for > removing NO_IRQ from ARM platforms and migrating to 0 for the no-interrupt > case? I'm assuming that this mainly involves migrating existing hard-wired > code that deals with interrupt numbers to use irq domains. How many drivers do use IRQ #0 to start with? We might discover that in practice there is only a very few cases where this is an issue if 0 would mean no IRQ. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Russell, do you know whether it would make sense to set a timeline for > removing NO_IRQ from ARM platforms and migrating to 0 for the no-interrupt > case? I'm assuming that this mainly involves migrating existing hard-wired > code that deals with interrupt numbers to use irq domains. The timelime was several years ago. Several years of complete inaction later the chickens have come home to roost. I refer you to Linus mail of 26 Sept 2006 to linux-kernel ('restore libata build on frv') Quoting Linus email: >> That's fine -- but don't use zero to mean none. We have NO_IRQ for >> that, and zero isn't an appropriate choice. > > Zero _is_ an appropriate choice, dammit! > > That NO_IRQ thing should be zero, and any architecture that thinks that >zero is a valid IRQ just needs to fix its own irq mapping so that the > "cookie" doesn't work. > > The thing is, it's zero. Get over it. It can't be "-1" or some other > random value like people have indicated, because that thing is often read > from places where "-1" simply isn't a possible value (eg it gets its > default value initialized from a "unsigned char" in MMIO space on x86). > > So instead of making everybody and their dog to silly things with some > NO_IRQ define that they haven't historically done, the rule is simple: "0" > means "no irq", so that you can test for it with obvious code like > > if (!dev->irq) > .. > > and then, if your actual _hardware_ things that the bit-pattern with all > bits clear is a valid irq that can be used for normal devices, then what > you do is you add a irq number translation layer (WHICH WE NEED AND HAVE > _ANYWAY_) and make sure that nobody sees that on a _software_ level. ---------- On 15th October 2008 Linus said the following to linux-next > Grr. Can we please just get rid of that IDIOTIC thing instead? > > NO_IRQ was a bad idea to begin with. Let's not add more. > > I assume that broken driver is some ARM-specific thing. I certainly don't > want to see NO_IRQ in any general drivers. So instead of having that > NO_IRQ insanity spread any more, I'd much rather see the driver either > fixed to not use it, or just marked ARM-only. > > The proper way to test for whether an interrupt is valid or not is to do > > if (dev->irq) { > ... > and no other. There is no spoon. That NO_IRQ was insane. And > architectures or drivers that still think otherwise should fix themselves. ------------ So there we are.. ARM spent years ignoring clear direction. If ARM breaks for a release now so be it. You've had *YEARS* to get off your collective backsides and sort it out. > I worry that if we just change the convention for the OF case, we'll end > up with OF-ised platform drivers which have to deal with a different no- > irq convention depending on whether they are probed as platform drivers > or through the OF framework ... and these ported or semi-ported drivers > will be intermixed with unported drivers, confusing maintainers All drivers should assume that if (!dev->irq) works. Zero is not an IRQ except in certain buried internal invisible cases in arch code (legacy PC timer being the obvious one). Come to think about it we had a prior discussion about NO_IRQ in 2005 even! The core kernel generic IRQ code knows about zero being special, many common driver layer components such as serial and network phylib do, so if anything it's going to fix bugs sorting the mess out on ARM. Jut fix it. Other platforms have done so without problem. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 05, 2011 at 12:40:16PM -0500, Nicolas Pitre wrote: > On Mon, 5 Dec 2011, Dave Martin wrote: > > On Sat, Dec 03, 2011 at 10:12:53AM +1100, Benjamin Herrenschmidt wrote: > > > On Fri, 2011-12-02 at 11:28 -0800, Linus Torvalds wrote: > > > > Don't *change* NO_IRQ to zero (that whole #define is broken - leave it > > > > around as a marker of brokenness), just start removing it from all the > > > > ARM drivers that use the OF infrastructure. Which is presumably not > > > > all that many yet. > > > > > > > > So whenever you find breakage, the fix now is to just remove NO_IRQ > > > > tests, and replace them with "!irq". > > > > > > > Russell, do you know whether it would make sense to set a timeline for > > removing NO_IRQ from ARM platforms and migrating to 0 for the no-interrupt > > case? I'm assuming that this mainly involves migrating existing hard-wired > > code that deals with interrupt numbers to use irq domains. > > How many drivers do use IRQ #0 to start with? We might discover that in > practice there is only a very few cases where this is an issue if 0 > would mean no IRQ. The total number of files referring to NO_IRQ is not that huge: arch/arm/ 188 matches in 39 files drivers/ 174 matches in 84 files Unfortunately, NO_IRQ is often not spelled "NO_IRQ". It looks like the assumption "irq < 0 === no irq" may be quite a lot more widespread than "NO_IRQ === no irq". Since there's no specific thing we can grep for (and simply due to volume) finding all such instances may be quite a bit harder. For example, git grep 'irq.*\(>=\|<[^=]\) *0' gives drivers/ 435 matches in 314 files arch/arm/ 18 matches in 15 files A few examples: drivers/input/mouse/pxa930_trkball.c: if (irq < 0) { drivers/input/keyboard/tegra-kbc.c: if (irq < 0) { drivers/crypto/omap-sham.c: if (dd->irq >= 0) ...etc., etc., although there are probably a fair number of false positives here. whereas git grep 'irq.*\(<\|>\|<=\|>=\|==\|!=\) \+-1' gives drivers/ 68 matches in 28 files arch/arm/ 18 matches in 15 files Examples: ...and that's just the code which is C and is also kind enough to put irq numbers in variables with names containing "irq". It also doesn't catch people initialising variables or struct/array members to -1, unadorned "-1" arguments to functions and so on... though those are likely to appear in mostly the same files matching the above expressions, it won't be an exact 1:1 correspondence. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 5, 2011 at 19:02, Dave Martin <dave.martin@linaro.org> wrote: > Unfortunately, NO_IRQ is often not spelled "NO_IRQ". It looks like the assumption > "irq < 0 === no irq" may be quite a lot more widespread than "NO_IRQ === no irq". Can we make irq unsigned, and hope the compiler catches all of them (comparison always false blah blah blah)? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 5 Dec 2011, Dave Martin wrote: > On Mon, Dec 05, 2011 at 12:40:16PM -0500, Nicolas Pitre wrote: > > On Mon, 5 Dec 2011, Dave Martin wrote: > > > On Sat, Dec 03, 2011 at 10:12:53AM +1100, Benjamin Herrenschmidt wrote: > > > > On Fri, 2011-12-02 at 11:28 -0800, Linus Torvalds wrote: > > > > > Don't *change* NO_IRQ to zero (that whole #define is broken - leave it > > > > > around as a marker of brokenness), just start removing it from all the > > > > > ARM drivers that use the OF infrastructure. Which is presumably not > > > > > all that many yet. > > > > > > > > > > So whenever you find breakage, the fix now is to just remove NO_IRQ > > > > > tests, and replace them with "!irq". > > > > > > > > > > Russell, do you know whether it would make sense to set a timeline for > > > removing NO_IRQ from ARM platforms and migrating to 0 for the no-interrupt > > > case? I'm assuming that this mainly involves migrating existing hard-wired > > > code that deals with interrupt numbers to use irq domains. > > > > How many drivers do use IRQ #0 to start with? We might discover that in > > practice there is only a very few cases where this is an issue if 0 > > would mean no IRQ. > > The total number of files referring to NO_IRQ is not that huge: > > arch/arm/ 188 matches in 39 files > drivers/ 174 matches in 84 files > > Unfortunately, NO_IRQ is often not spelled "NO_IRQ". It looks like the assumption > "irq < 0 === no irq" may be quite a lot more widespread than "NO_IRQ === no irq". > Since there's no specific thing we can grep for (and simply due to volume) > finding all such instances may be quite a bit harder. [...] ARgh. My point was about current actual usage of the IRQ numbered 0 which probably prompted the introduction of NO_IRQ in the first place. What I was saying is that the number of occurrences where IRQ #0 is currently used into drivers that would get confused if 0 would mean no IRQ is probably quite small. But as you illustrated, there is a large number of drivers that already assume no IRQ is < 0, even if they don't use any IRQ #0 themselves. That is a much bigger problem to fix. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> But as you illustrated, there is a large number of drivers that already > assume no IRQ is < 0, even if they don't use any IRQ #0 themselves. > That is a much bigger problem to fix. And a much larger number assuming the reverse is true which are hiding potential bugs on ARM. Looking at the serial stuff the best checks appear to be looking at "irq", "-1" and NO_IRQ. For migration stuff that's doing broken things like if (irq < 0) can be changed to if (irq <= 0) and that can be done before NO_IRQ itself is nailed on ARM and PA-RISC. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/05/2011 12:18 PM, Nicolas Pitre wrote: > On Mon, 5 Dec 2011, Dave Martin wrote: > >> On Mon, Dec 05, 2011 at 12:40:16PM -0500, Nicolas Pitre wrote: >>> On Mon, 5 Dec 2011, Dave Martin wrote: >>>> On Sat, Dec 03, 2011 at 10:12:53AM +1100, Benjamin Herrenschmidt wrote: >>>>> On Fri, 2011-12-02 at 11:28 -0800, Linus Torvalds wrote: >>>>>> Don't *change* NO_IRQ to zero (that whole #define is broken - leave it >>>>>> around as a marker of brokenness), just start removing it from all the >>>>>> ARM drivers that use the OF infrastructure. Which is presumably not >>>>>> all that many yet. >>>>>> >>>>>> So whenever you find breakage, the fix now is to just remove NO_IRQ >>>>>> tests, and replace them with "!irq". >>>>> >>>> >>>> Russell, do you know whether it would make sense to set a timeline for >>>> removing NO_IRQ from ARM platforms and migrating to 0 for the no-interrupt >>>> case? I'm assuming that this mainly involves migrating existing hard-wired >>>> code that deals with interrupt numbers to use irq domains. >>> >>> How many drivers do use IRQ #0 to start with? We might discover that in >>> practice there is only a very few cases where this is an issue if 0 >>> would mean no IRQ. >> >> The total number of files referring to NO_IRQ is not that huge: >> >> arch/arm/ 188 matches in 39 files >> drivers/ 174 matches in 84 files >> >> Unfortunately, NO_IRQ is often not spelled "NO_IRQ". It looks like the assumption >> "irq < 0 === no irq" may be quite a lot more widespread than "NO_IRQ === no irq". >> Since there's no specific thing we can grep for (and simply due to volume) >> finding all such instances may be quite a bit harder. > [...] > > ARgh. > > My point was about current actual usage of the IRQ numbered 0 which > probably prompted the introduction of NO_IRQ in the first place. What I > was saying is that the number of occurrences where IRQ #0 is currently > used into drivers that would get confused if 0 would mean no IRQ is > probably quite small. > > But as you illustrated, there is a large number of drivers that already > assume no IRQ is < 0, even if they don't use any IRQ #0 themselves. > That is a much bigger problem to fix. > At least for DT enabled platforms, we could force "no irq" to be 0 in the DT irq code. Searching the dts files, I found 2 occurrences of IRQ0. Prima2 has timer on IRQ0, and VersatileAB has watchdog on IRQ0. Prima2 should be fine currently as it doesn't use the of_irq_* functions to get the timer irq, but that is an issue as it skips any translation. VersatileAB should be okay with the VIC irqdomain support. Changing it would also affect microblaze and openrisc which have NO_IRQ set to -1. From what I can tell, they would both be fine at least in terms of not using IRQ0. Also, there's roughly 50 irq_chips that need irq_domain support under arch/arm. So that's not a simple solution either. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-12-05 at 18:45 +0000, Alan Cox wrote: > > But as you illustrated, there is a large number of drivers that already > > assume no IRQ is < 0, even if they don't use any IRQ #0 themselves. > > That is a much bigger problem to fix. > > And a much larger number assuming the reverse is true which are hiding > potential bugs on ARM. > > Looking at the serial stuff the best checks appear to be looking at > "irq", "-1" and NO_IRQ. > > For migration stuff that's doing broken things like > > if (irq < 0) > > can be changed to > > if (irq <= 0) > > and that can be done before NO_IRQ itself is nailed on ARM and PA-RISC. To be honest, we don't care very much. Parisc interrupts are cascading and mostly software assigned (except our EIEM which we keep internal). We use a base offset at 16 or 64 (depending on GSC presence or not) so IRQs 0-15 aren't legal on parisc either (we frob some of the hard coded ISA interrupts on the WAX eisa bus). We use NO_IRQ as an IRQ assignment error return and that's about it (and that error shouldn't ever really occur). James -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 05, 2011 at 01:18:30PM -0500, Nicolas Pitre wrote: > On Mon, 5 Dec 2011, Dave Martin wrote: > > > On Mon, Dec 05, 2011 at 12:40:16PM -0500, Nicolas Pitre wrote: > > > On Mon, 5 Dec 2011, Dave Martin wrote: > > > > On Sat, Dec 03, 2011 at 10:12:53AM +1100, Benjamin Herrenschmidt wrote: > > > > > On Fri, 2011-12-02 at 11:28 -0800, Linus Torvalds wrote: > > > > > > Don't *change* NO_IRQ to zero (that whole #define is broken - leave it > > > > > > around as a marker of brokenness), just start removing it from all the > > > > > > ARM drivers that use the OF infrastructure. Which is presumably not > > > > > > all that many yet. > > > > > > > > > > > > So whenever you find breakage, the fix now is to just remove NO_IRQ > > > > > > tests, and replace them with "!irq". > > > > > > > > > > > > > Russell, do you know whether it would make sense to set a timeline for > > > > removing NO_IRQ from ARM platforms and migrating to 0 for the no-interrupt > > > > case? I'm assuming that this mainly involves migrating existing hard-wired > > > > code that deals with interrupt numbers to use irq domains. > > > > > > How many drivers do use IRQ #0 to start with? We might discover that in > > > practice there is only a very few cases where this is an issue if 0 > > > would mean no IRQ. > > > > The total number of files referring to NO_IRQ is not that huge: > > > > arch/arm/ 188 matches in 39 files > > drivers/ 174 matches in 84 files > > > > Unfortunately, NO_IRQ is often not spelled "NO_IRQ". It looks like the assumption > > "irq < 0 === no irq" may be quite a lot more widespread than "NO_IRQ === no irq". > > Since there's no specific thing we can grep for (and simply due to volume) > > finding all such instances may be quite a bit harder. > [...] > > ARgh. > > My point was about current actual usage of the IRQ numbered 0 which > probably prompted the introduction of NO_IRQ in the first place. What I > was saying is that the number of occurrences where IRQ #0 is currently > used into drivers that would get confused if 0 would mean no IRQ is > probably quite small. Ah, I misunderstood -- that's a separate issue, but also an important one. I guess this applies to a fair number of older boards. One way of fixing this would be to migrate those boards to use irq domains -- but those boards may be sporadically maintained. > But as you illustrated, there is a large number of drivers that already > assume no IRQ is < 0, even if they don't use any IRQ #0 themselves. > That is a much bigger problem to fix. My concern is that as soon as we start to change this in significant volume, a _lot_ of stuff is going to break. Everywhere that an irq value is passed from one piece of code to another, there is a potential interface mismatch -- there seems to be no single place where we can apply a conversion and fix everything. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 5 Dec 2011, Dave Martin wrote: > On Mon, Dec 05, 2011 at 01:18:30PM -0500, Nicolas Pitre wrote: > > On Mon, 5 Dec 2011, Dave Martin wrote: > > > > > On Mon, Dec 05, 2011 at 12:40:16PM -0500, Nicolas Pitre wrote: > > > > On Mon, 5 Dec 2011, Dave Martin wrote: > > > > > On Sat, Dec 03, 2011 at 10:12:53AM +1100, Benjamin Herrenschmidt wrote: > > > > > > On Fri, 2011-12-02 at 11:28 -0800, Linus Torvalds wrote: > > > > > > > Don't *change* NO_IRQ to zero (that whole #define is broken - leave it > > > > > > > around as a marker of brokenness), just start removing it from all the > > > > > > > ARM drivers that use the OF infrastructure. Which is presumably not > > > > > > > all that many yet. > > > > > > > > > > > > > > So whenever you find breakage, the fix now is to just remove NO_IRQ > > > > > > > tests, and replace them with "!irq". > > > > > > > > > > > > > > > > Russell, do you know whether it would make sense to set a timeline for > > > > > removing NO_IRQ from ARM platforms and migrating to 0 for the no-interrupt > > > > > case? I'm assuming that this mainly involves migrating existing hard-wired > > > > > code that deals with interrupt numbers to use irq domains. > > > > > > > > How many drivers do use IRQ #0 to start with? We might discover that in > > > > practice there is only a very few cases where this is an issue if 0 > > > > would mean no IRQ. > > > > > > The total number of files referring to NO_IRQ is not that huge: > > > > > > arch/arm/ 188 matches in 39 files > > > drivers/ 174 matches in 84 files > > > > > > Unfortunately, NO_IRQ is often not spelled "NO_IRQ". It looks like the assumption > > > "irq < 0 === no irq" may be quite a lot more widespread than "NO_IRQ === no irq". > > > Since there's no specific thing we can grep for (and simply due to volume) > > > finding all such instances may be quite a bit harder. > > [...] > > > > ARgh. > > > > My point was about current actual usage of the IRQ numbered 0 which > > probably prompted the introduction of NO_IRQ in the first place. What I > > was saying is that the number of occurrences where IRQ #0 is currently > > used into drivers that would get confused if 0 would mean no IRQ is > > probably quite small. > > Ah, I misunderstood -- that's a separate issue, but also an important one. > I guess this applies to a fair number of older boards. One way of fixing > this would be to migrate those boards to use irq domains -- but those boards > may be sporadically maintained. > > > But as you illustrated, there is a large number of drivers that already > > assume no IRQ is < 0, even if they don't use any IRQ #0 themselves. > > That is a much bigger problem to fix. > > My concern is that as soon as we start to change this in significant > volume, a _lot_ of stuff is going to break. Everywhere that an irq value > is passed from one piece of code to another, there is a potential > interface mismatch -- there seems to be no single place where we can > apply a conversion and fix everything. No need to convert everything. First move is to make irq=0 meaning no IRQ. That means making things like: if (irq < 0) if (irq >= 0) into if (irq <= 0) if (irq > 0) And replace NO_IRQ with 0. That change shouldn't break anything, except those drivers which are 1) being passed an actual IRQ #0 and 2) testing for no IRQ. I suspect that those conditions aren't very common together. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 05, 2011 at 01:16:39PM -0600, Rob Herring wrote: [...] > At least for DT enabled platforms, we could force "no irq" to be 0 in > the DT irq code. Searching the dts files, I found 2 occurrences of IRQ0. Please note that there are HW IRQ numbers and "Virtual" IRQ numbers. dev->irq and thus the thing that we pass into request_irq() is a virtual IRQ thing, a "cookie". While in device tree you see real HW IRQ numbers. Legal VIRQ is always > 0, while HW IRQ could be >= 0. > Prima2 has timer on IRQ0, and VersatileAB has watchdog on IRQ0. Prima2 > should be fine currently as it doesn't use the of_irq_* functions to get > the timer irq, but that is an issue as it skips any translation. > VersatileAB should be okay with the VIC irqdomain support. It shouldn't be an issue to use of_irq_*() functions for these IRQs. of_irq_*() will remap HW IRQ 0 to some other VIRQ. If it does not do this currently, then it's a bug and should be fixed. Thanks,
On 12/05/2011 02:21 PM, Anton Vorontsov wrote: > On Mon, Dec 05, 2011 at 01:16:39PM -0600, Rob Herring wrote: > [...] >> At least for DT enabled platforms, we could force "no irq" to be 0 in >> the DT irq code. Searching the dts files, I found 2 occurrences of IRQ0. > > Please note that there are HW IRQ numbers and "Virtual" IRQ numbers. > dev->irq and thus the thing that we pass into request_irq() is a > virtual IRQ thing, a "cookie". > > While in device tree you see real HW IRQ numbers. > > Legal VIRQ is always > 0, while HW IRQ could be >= 0. > If this was all true, then there would be no discussion. This is what we are working towards, but irq_chips all over the arm tree do not support any translation or have base fixed at compile time. Only a few have been converted. And some ARM platforms may never get converted to DT. >> Prima2 has timer on IRQ0, and VersatileAB has watchdog on IRQ0. Prima2 >> should be fine currently as it doesn't use the of_irq_* functions to get >> the timer irq, but that is an issue as it skips any translation. >> VersatileAB should be okay with the VIC irqdomain support. > > It shouldn't be an issue to use of_irq_*() functions for these IRQs. > of_irq_*() will remap HW IRQ 0 to some other VIRQ. If it does not do > this currently, then it's a bug and should be fixed. I think that's what I'm saying. It's either a bug or incomplete DT conversion for the platform. Either way, those should get fixed first. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 05 Dec 2011 14:47:29 -0600 Rob Herring <robherring2@gmail.com> wrote: > On 12/05/2011 02:21 PM, Anton Vorontsov wrote: > > On Mon, Dec 05, 2011 at 01:16:39PM -0600, Rob Herring wrote: > > [...] > >> At least for DT enabled platforms, we could force "no irq" to be 0 in > >> the DT irq code. Searching the dts files, I found 2 occurrences of IRQ0. > > > > Please note that there are HW IRQ numbers and "Virtual" IRQ numbers. > > dev->irq and thus the thing that we pass into request_irq() is a > > virtual IRQ thing, a "cookie". > > > > While in device tree you see real HW IRQ numbers. > > > > Legal VIRQ is always > 0, while HW IRQ could be >= 0. > > > > If this was all true, then there would be no discussion. Or more to the point. If the ARM people concerned had listened in 2005, 2006 or 2008 there would be no discussion. > This is what we are working towards, but irq_chips all over the arm tree > do not support any translation or have base fixed at compile time. Only > a few have been converted. And some ARM platforms may never get > converted to DT. You've had six years. Let them break, it'll motivate any users to fix them. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18:45 Mon 05 Dec , Alan Cox wrote: > > But as you illustrated, there is a large number of drivers that already > > assume no IRQ is < 0, even if they don't use any IRQ #0 themselves. > > That is a much bigger problem to fix. > > And a much larger number assuming the reverse is true which are hiding > potential bugs on ARM. > > Looking at the serial stuff the best checks appear to be looking at > "irq", "-1" and NO_IRQ. > > For migration stuff that's doing broken things like > > if (irq < 0) > > can be changed to > > if (irq <= 0) > > and that can be done before NO_IRQ itself is nailed on ARM and PA-RISC. can we sinply introduce a macro irq_is_valid and make it chip dependant as gpio_is_valid and then move away from irq 0 valid so we do not need to brake anthing first and then easly convert them Best Regards, J. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 05, 2011 at 02:47:29PM -0600, Rob Herring wrote: > On 12/05/2011 02:21 PM, Anton Vorontsov wrote: > > On Mon, Dec 05, 2011 at 01:16:39PM -0600, Rob Herring wrote: > > [...] > >> At least for DT enabled platforms, we could force "no irq" to be 0 in > >> the DT irq code. Searching the dts files, I found 2 occurrences of IRQ0. > > > > Please note that there are HW IRQ numbers and "Virtual" IRQ numbers. > > dev->irq and thus the thing that we pass into request_irq() is a > > virtual IRQ thing, a "cookie". > > > > While in device tree you see real HW IRQ numbers. > > > > Legal VIRQ is always > 0, while HW IRQ could be >= 0. > > > > If this was all true, then there would be no discussion. > > This is what we are working towards, but irq_chips all over the arm tree > do not support any translation or have base fixed at compile time. Only > a few have been converted. And some ARM platforms may never get > converted to DT. > > >> Prima2 has timer on IRQ0, and VersatileAB has watchdog on IRQ0. Prima2 > >> should be fine currently as it doesn't use the of_irq_* functions to get > >> the timer irq, but that is an issue as it skips any translation. > >> VersatileAB should be okay with the VIC irqdomain support. > > > > It shouldn't be an issue to use of_irq_*() functions for these IRQs. > > of_irq_*() will remap HW IRQ 0 to some other VIRQ. If it does not do > > this currently, then it's a bug and should be fixed. > > I think that's what I'm saying. It's either a bug or incomplete DT > conversion for the platform. Either way, those should get fixed first. Do we expect there to be any platform drivers which are shared between legacy platforms and newer DT-ised platforms? Those drivers would be pain points since they would need to understand both conventions. So far as I can see, only boards which are not DT-ised, which do not use DT-ised drivers and which do not use drivers which use interrupts and are either used by DT-ised boards or by arches with a non-zero NO_IRQ could safely carry on using a non-zero NO_IRQ. Tracking down exactly which boards and drivers this applies to could be hard. We could have a CONFIG_NO_IRQ and make them depend on it, but we still have to find that list of boards and drivers in the first place. Otherwise, it feels like we might need a strategy for migrating pretty much everything if we don't want to end up in a mess. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 05, 2011 at 02:49:01PM -0500, Nicolas Pitre wrote: [...] > > > > Unfortunately, NO_IRQ is often not spelled "NO_IRQ". It looks like the assumption > > > > "irq < 0 === no irq" may be quite a lot more widespread than "NO_IRQ === no irq". > > > > Since there's no specific thing we can grep for (and simply due to volume) > > > > finding all such instances may be quite a bit harder. > > > [...] > > > > > > ARgh. > > > > > > My point was about current actual usage of the IRQ numbered 0 which > > > probably prompted the introduction of NO_IRQ in the first place. What I > > > was saying is that the number of occurrences where IRQ #0 is currently > > > used into drivers that would get confused if 0 would mean no IRQ is > > > probably quite small. > > > > Ah, I misunderstood -- that's a separate issue, but also an important one. > > I guess this applies to a fair number of older boards. One way of fixing > > this would be to migrate those boards to use irq domains -- but those boards > > may be sporadically maintained. > > > > > But as you illustrated, there is a large number of drivers that already > > > assume no IRQ is < 0, even if they don't use any IRQ #0 themselves. > > > That is a much bigger problem to fix. > > > > My concern is that as soon as we start to change this in significant > > volume, a _lot_ of stuff is going to break. Everywhere that an irq value > > is passed from one piece of code to another, there is a potential > > interface mismatch -- there seems to be no single place where we can > > apply a conversion and fix everything. > > No need to convert everything. > > First move is to make irq=0 meaning no IRQ. That means making things > like: > > if (irq < 0) > if (irq >= 0) > > into > > if (irq <= 0) > if (irq > 0) > > And replace NO_IRQ with 0. > > That change shouldn't break anything, except those drivers which are 1) > being passed an actual IRQ #0 and 2) testing for no IRQ. I suspect that > those conditions aren't very common together. To clarify, you're suggesting that the meanings of all other IRQ values would not change initially? (i.e., we remap HW irq 0, if there is one, to some other random number but have a 1:1 mapping for everything else). That could make sense as an approach. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Otherwise, it feels like we might need a strategy for migrating pretty > much everything if we don't want to end up in a mess. You really do anyway - lots of generic driver code knows !dev->irq is a valid test. That covers things like 8250 based UART hardware, network phy layer code and vast amounts more. The bugs will already be there because ARM isn't using 0, they just aren't getting seen or aren't getting hit. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 06, 2011 at 09:37:09AM +0000, Dave Martin wrote: > To clarify, you're suggesting that the meanings of all other IRQ values > would not change initially? (i.e., we remap HW irq 0, if there is one, > to some other random number but have a 1:1 mapping for everything else). Even better. Avoid the first 16 IRQ numbers altogether - so that ISA drivers which have these numbers hard-encoded in them will see failures if they're expecting standard ISA IRQ numbering. We already do that with the GIC, partly because of the hardware design. We do that on Footbridge based systems, because they may or may not have a real ISA IRQ controller. But.. let's make one thing clear: Alan Cox and Linus have been going on about how IRQ0 should not be used. Let's be crystal clear: even x86 uses IRQ0. It happens to be the PIC timer, and that gets claimed early on during the x86 boot. So please don't tell me that x86 avoids IRQ0. It doesn't. It just happens that x86 never shows IRQ0 to anything but the i8253 PIC driver. So lets see how x86 squeels if we make the i8253 PIC driver reject IRQ0. I bet that there'd be absolute fury at such a suggestion. When x86 sorts this out, there _might_ be some more motivation to take such comments seriously. Until then it's more like a joke. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 06, 2011 at 09:30:00AM +0000, Dave Martin wrote: > Do we expect there to be any platform drivers which are shared between > legacy platforms and newer DT-ised platforms? > > Those drivers would be pain points since they would need to understand > both conventions. > > So far as I can see, only boards which are not DT-ised, which do not use > DT-ised drivers and which do not use drivers which use interrupts and > are either used by DT-ised boards or by arches with a non-zero NO_IRQ > could safely carry on using a non-zero NO_IRQ. Tracking down exactly > which boards and drivers this applies to could be hard. We could have a > CONFIG_NO_IRQ and make them depend on it, but we still have to find that > list of boards and drivers in the first place. You're digging too deeply into this. Drivers which need to know whether an IRQ is valid need to know this if they wish to do something different for 'this device doesn't have an IRQ wired'. These are the drivers which have problems because of the -1 vs 0 thing. That is different from 'this is an invalid IRQ number', which is what you find out when you call request_irq(). So please, stop thinking 'we need to convert drivers to check for <= 0'. We don't. We just need to make sure that we're not passing a zero IRQ number to any driver. On platforms where IRQ0 is special like x86, request_irq() will fail with -EBUSY on drivers which don't care (or other kind of refusal to initialize), and will cause 'polling mode' with the 8250 driver. So, all that we need to do is to ensure that all the IRQ chip stuff is fixed up so that IRQ0 is only used for the same purpose as x86 - the PIC timer on systems with an ISA 8253 timer. Everything else should not pass IRQ0 outside core platform code. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Even better. Avoid the first 16 IRQ numbers altogether - so that ISA > drivers which have these numbers hard-encoded in them will see failures > if they're expecting standard ISA IRQ numbering. The ISA bus space is non-discoverable so that doesn't make any sense. > But.. let's make one thing clear: Alan Cox and Linus have been going on > about how IRQ0 should not be used. Let's be crystal clear: even x86 > uses IRQ0. It happens to be the PIC timer, and that gets claimed early > on during the x86 boot. So please don't tell me that x86 avoids IRQ0. > It doesn't. It just happens that x86 never shows IRQ0 to anything but > the i8253 PIC driver. x86 has an internal invisible IRQ 0 on some platforms. It's never exposed beyond the arch code. > So lets see how x86 squeels if we make the i8253 PIC driver reject IRQ0. > I bet that there'd be absolute fury at such a suggestion. Actually it would be about ten minutes work to remap it to some other number that isn't used. It never goes via request_irq or via any core or driver layer code however. In the ARM case the same is going to be true. If you have IRQ 0 plumbing that only goes internally in the arch/arm code - eg an ARM with IRQ 0 wired to something totally arch specific and non-driver then it's not going to break and like the internals of x86 doesn't matter. > When x86 sorts this out, there _might_ be some more motivation to take > such comments seriously. Until then it's more like a joke. Pity you feel that way, but if ARM wants to continue to break more and more as we clean up NO_IRQ for everything else that's your privilege, but don't expect any sympathy. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 06, 2011 at 11:05:54AM +0000, Alan Cox wrote: > > Even better. Avoid the first 16 IRQ numbers altogether - so that ISA > > drivers which have these numbers hard-encoded in them will see failures > > if they're expecting standard ISA IRQ numbering. > > The ISA bus space is non-discoverable so that doesn't make any sense. > > > But.. let's make one thing clear: Alan Cox and Linus have been going on > > about how IRQ0 should not be used. Let's be crystal clear: even x86 > > uses IRQ0. It happens to be the PIC timer, and that gets claimed early > > on during the x86 boot. So please don't tell me that x86 avoids IRQ0. > > It doesn't. It just happens that x86 never shows IRQ0 to anything but > > the i8253 PIC driver. > > x86 has an internal invisible IRQ 0 on some platforms. It's never exposed > beyond the arch code. > > > So lets see how x86 squeels if we make the i8253 PIC driver reject IRQ0. > > I bet that there'd be absolute fury at such a suggestion. > > Actually it would be about ten minutes work to remap it to some other > number that isn't used. It never goes via request_irq or via any core or > driver layer code however. > > In the ARM case the same is going to be true. If you have IRQ 0 plumbing > that only goes internally in the arch/arm code - eg an ARM with IRQ 0 > wired to something totally arch specific and non-driver then it's not > going to break and like the internals of x86 doesn't matter. > > > When x86 sorts this out, there _might_ be some more motivation to take > > such comments seriously. Until then it's more like a joke. > > Pity you feel that way, but if ARM wants to continue to break more and > more as we clean up NO_IRQ for everything else that's your privilege, but > don't expect any sympathy. For the platforms I care about, it probably won't break. For those which do break, it's a matter of fixing their include/mach/irqs.h and the code in their irqchips to convert the IRQ number to the correct bitmask for the register. However, I have suggested in the past that new platforms _should_ avoid not just IRQ0 but IRQ0-15 (for a completely different reason to that of 'IRQ0 means no IRQ'.) But such comments just get ignored, so I just don't see the point in doing anything about this. If people experience breakage, so be it. I too will have little sympathy but not for the same reason. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> can we sinply introduce a macro irq_is_valid
See the 2005, 2006 and 2008 discussion.
if (!dev->irq)
is the proper test.
The <= is just a temporary thing while ARM gets its publically visible
house in order so it can be done without breakage.
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 06, 2011 at 10:46:54AM +0000, Russell King - ARM Linux wrote: > On Tue, Dec 06, 2011 at 09:37:09AM +0000, Dave Martin wrote: > > To clarify, you're suggesting that the meanings of all other IRQ values > > would not change initially? (i.e., we remap HW irq 0, if there is one, > > to some other random number but have a 1:1 mapping for everything else). > > Even better. Avoid the first 16 IRQ numbers altogether - so that ISA > drivers which have these numbers hard-encoded in them will see failures > if they're expecting standard ISA IRQ numbering. > > We already do that with the GIC, partly because of the hardware design. > We do that on Footbridge based systems, because they may or may not have > a real ISA IRQ controller. > > But.. let's make one thing clear: Alan Cox and Linus have been going on > about how IRQ0 should not be used. Let's be crystal clear: even x86 > uses IRQ0. It happens to be the PIC timer, and that gets claimed early > on during the x86 boot. So please don't tell me that x86 avoids IRQ0. > It doesn't. It just happens that x86 never shows IRQ0 to anything but > the i8253 PIC driver. > > So lets see how x86 squeels if we make the i8253 PIC driver reject IRQ0. > I bet that there'd be absolute fury at such a suggestion. > > When x86 sorts this out, there _might_ be some more motivation to take > such comments seriously. Until then it's more like a joke. OK -- but the situation is breaking OF-based drivers on ARM platforms today. Based on what you've suggested, does the following policy sound reasonable for resolving that deadlock? 1) All OF code and drivers should be migrating to use 0 instead of NO_IRQ for the no-interrupt case. Code which receives irq numbers directly from the OF framework and refers to NO_IRQ, or expects 0 to be a valid needs to be fixed. 2) Where we hit a problem, board code needs to be adapted to remap HW IRQs 0-15 to different software values. (This could be done using irq domains, or not) I'm still not sure what the correct approach is for drivers which get irq numbers from OF indirectly -- this particularly applies to platform and AMBA devices. If we expect board code to start populating platform data based on information from the OF code, we need to fix the board not to use linux irq 0 to describe a real HW interrupt, if it matters (as in (2)). AMBA devices registered via of_platform_populate() already get their irq numbers from OF. So long as OF used to explicitly return NO_IRQ there was no problem -- but if OF is moving to return 0 instead, we have a potential problem for each AMBA driver which may be used by a board which can boot without DT... if we have any scenarios where that driver is given real irq 0. Maybe we can fix these breakages as they occur -- I don't really know the scale of the impact. What are your thoughts on this? Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 06, 2011 at 11:37:35AM +0000, Dave Martin wrote: > 1) All OF code and drivers should be migrating to use 0 instead of NO_IRQ > for the no-interrupt case. Code which receives irq numbers directly > from the OF framework and refers to NO_IRQ, or expects 0 to be a valid > needs to be fixed. > > 2) Where we hit a problem, board code needs to be adapted to remap HW IRQs > 0-15 to different software values. (This could be done using irq > domains, or not) No AMBA driver I'm aware of ever uses an IRQ number 0 or is passed such an IRQ number. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> However, I have suggested in the past that new platforms _should_ avoid > not just IRQ0 but IRQ0-15 (for a completely different reason to that of > 'IRQ0 means no IRQ'.) But such comments just get ignored, so I just > don't see the point in doing anything about this. If people experience > breakage, so be it. I too will have little sympathy but not for the same > reason. The one I can think of that is capable of taking EISA/ISA cards but has differently IRQ plumbing arrangements is PA-RISC, and they do exactly this. Beyond that it probably doesn't come up except in the weird world of PCI legacy compatibility for legacy IDE and VGA vertical interrupt routing. In those cases we fix up the PCI config space so the platform in turn can do proper IRQ plumbing. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 06, 2011 at 11:49:52AM +0000, Russell King - ARM Linux wrote: > On Tue, Dec 06, 2011 at 11:37:35AM +0000, Dave Martin wrote: > > 1) All OF code and drivers should be migrating to use 0 instead of NO_IRQ > > for the no-interrupt case. Code which receives irq numbers directly > > from the OF framework and refers to NO_IRQ, or expects 0 to be a valid > > needs to be fixed. > > > > 2) Where we hit a problem, board code needs to be adapted to remap HW IRQs > > 0-15 to different software values. (This could be done using irq > > domains, or not) > > No AMBA driver I'm aware of ever uses an IRQ number 0 or is passed such > an IRQ number. OK, hopefully we can safely ignore that case, then. But other than that, you're in agreement? Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 6 Dec 2011, Dave Martin wrote: > On Mon, Dec 05, 2011 at 02:49:01PM -0500, Nicolas Pitre wrote: > > > No need to convert everything. > > > > First move is to make irq=0 meaning no IRQ. That means making things > > like: > > > > if (irq < 0) > > if (irq >= 0) > > > > into > > > > if (irq <= 0) > > if (irq > 0) > > > > And replace NO_IRQ with 0. > > > > That change shouldn't break anything, except those drivers which are 1) > > being passed an actual IRQ #0 and 2) testing for no IRQ. I suspect that > > those conditions aren't very common together. > > To clarify, you're suggesting that the meanings of all other IRQ values > would not change initially? Initially, or even ever. > (i.e., we remap HW irq 0, if there is one, > to some other random number but have a 1:1 mapping for everything else). Exact. > That could make sense as an approach. You might notice that a true IRQ #0 passed to generic drivers is not really frequent. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 6, 2011 at 2:46 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > But.. let's make one thing clear: Alan Cox and Linus have been going on > about how IRQ0 should not be used. Let's be crystal clear: even x86 > uses IRQ0. Not for any device driver, though. It's used entirely internally, and it doesn't even use "request_irq()". It uses the magic internal "setup_irq()" and never *ever* exposes irq0 as anything that a driver can see. That's what matters. You can use irq0 in ARM land all you like, AS LONG AS IT'S SOME HIDDEN INTERNAL USE. No drivers. No *nothing* that ever uses that absolutely *idiotic* NO_IRQ crap. In fact, you may be *forced* to use what is "physically" irq0 - it's just that you should never expose it as such to drivers. And x86 doesn't. So Russell, if you think this has anything to do with NO_IRQ, and how x86 isn't doing things right, you're wrong. It's just like the internal exception thing, or the magical "cascade interrupt", or the "x87 exception mapped through the PIC". They are magic hidden interrupts that are set up in one place (well, one place *each*), and are never exposed anywhere else. The problem with NO_IRQ is that stupid "we expose our mind-numbingly stupid interfaces across the whole kernel". x86 never did that. ARM still does. x86 doesn't have to fix anything. ARM does. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/06/2011 05:49 AM, Russell King - ARM Linux wrote: > On Tue, Dec 06, 2011 at 11:37:35AM +0000, Dave Martin wrote: >> 1) All OF code and drivers should be migrating to use 0 instead of NO_IRQ >> for the no-interrupt case. Code which receives irq numbers directly >> from the OF framework and refers to NO_IRQ, or expects 0 to be a valid >> needs to be fixed. >> >> 2) Where we hit a problem, board code needs to be adapted to remap HW IRQs >> 0-15 to different software values. (This could be done using irq >> domains, or not) > > No AMBA driver I'm aware of ever uses an IRQ number 0 or is passed such > an IRQ number. The watchdog on VersatileAB is on Linux IRQ0. This is easily fixed with VIC irqdomain patches which are queued up. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 06, 2011 at 11:20:49AM -0800, Linus Torvalds wrote: > Not for any device driver, though. > > It's used entirely internally, and it doesn't even use > "request_irq()". It uses the magic internal "setup_irq()" and never > *ever* exposes irq0 as anything that a driver can see. > > That's what matters. You can use irq0 in ARM land all you like, AS > LONG AS IT'S SOME HIDDEN INTERNAL USE. No drivers. No *nothing* that > ever uses that absolutely *idiotic* NO_IRQ crap. > > In fact, you may be *forced* to use what is "physically" irq0 - it's > just that you should never expose it as such to drivers. And x86 > doesn't. > > So Russell, if you think this has anything to do with NO_IRQ, and how > x86 isn't doing things right, you're wrong. It's just like the > internal exception thing, or the magical "cascade interrupt", or the > "x87 exception mapped through the PIC". They are magic hidden > interrupts that are set up in one place (well, one place *each*), and > are never exposed anywhere else. > > The problem with NO_IRQ is that stupid "we expose our mind-numbingly > stupid interfaces across the whole kernel". > > x86 never did that. ARM still does. x86 doesn't have to fix anything. ARM does. Remember you said that I shouldn't take things personally? Well, this is one issue I really don't care about. I don't think any platform I _actually_ have will be impacted by any change in this area. Other platform maintainers may have their own issues but that's not _my_ problem. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Linus, On Tue, Dec 06, 2011 at 11:20:49AM -0800, Linus Torvalds wrote: > On Tue, Dec 6, 2011 at 2:46 AM, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > > > But.. let's make one thing clear: Alan Cox and Linus have been going on > > about how IRQ0 should not be used. Let's be crystal clear: even x86 > > uses IRQ0. > > Not for any device driver, though. > > It's used entirely internally, and it doesn't even use > "request_irq()". It uses the magic internal "setup_irq()" and never > *ever* exposes irq0 as anything that a driver can see. > > That's what matters. You can use irq0 in ARM land all you like, AS > LONG AS IT'S SOME HIDDEN INTERNAL USE. No drivers. No *nothing* that > ever uses that absolutely *idiotic* NO_IRQ crap. > > In fact, you may be *forced* to use what is "physically" irq0 - it's > just that you should never expose it as such to drivers. And x86 > doesn't. > > So Russell, if you think this has anything to do with NO_IRQ, and how > x86 isn't doing things right, you're wrong. It's just like the > internal exception thing, or the magical "cascade interrupt", or the > "x87 exception mapped through the PIC". They are magic hidden > interrupts that are set up in one place (well, one place *each*), and > are never exposed anywhere else. Well there is try_misrouted_irq in kernel/irq/spurious.c that assumes irq0 to be something that it never is on ARM (and maybe all other platforms apart from x86). So at least it's not internal to a single (x86 specific) place. I tried to patch that two years ago, but that only ended in people saying "don't use irq0". I don't know if try_misrouted_irq sees hardware irqs, but if it does it's a bug even on archs != X86 that use virtual irqs. (Note that this doesn't oppose to your statement that using NO_IRQ is crap.) > The problem with NO_IRQ is that stupid "we expose our mind-numbingly > stupid interfaces across the whole kernel". > > x86 never did that. ARM still does. x86 doesn't have to fix anything. ARM does. Best regards Uwe
diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c index a72ab0d..2a472c5 100644 --- a/drivers/ata/pata_of_platform.c +++ b/drivers/ata/pata_of_platform.c @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev) } ret = of_irq_to_resource(dn, 0, &irq_res); - if (ret == NO_IRQ) + if (!ret) irq_res.start = irq_res.end = 0; else irq_res.flags = 0;