* [PATCH] powerpc: pseries: use irq_of_parse_and_map helper @ 2018-01-04 22:45 Rob Herring 2018-01-23 6:53 ` Michael Ellerman 0 siblings, 1 reply; 4+ messages in thread From: Rob Herring @ 2018-01-04 22:45 UTC (permalink / raw) To: Michael Ellerman Cc: linux-kernel, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev Instead of calling both of_irq_parse_one and irq_create_of_mapping, call of_irq_parse_and_map instead which does the same thing. This gets us closer to making the former 2 functions static. Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Rob Herring <robh@kernel.org> --- arch/powerpc/platforms/pseries/event_sources.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/platforms/pseries/event_sources.c b/arch/powerpc/platforms/pseries/event_sources.c index 6eeb0d4bab61..b0d8c146fe7b 100644 --- a/arch/powerpc/platforms/pseries/event_sources.c +++ b/arch/powerpc/platforms/pseries/event_sources.c @@ -16,7 +16,8 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#include <asm/prom.h> +#include <linux/interrupt.h> +#include <linux/of_irq.h> #include "pseries.h" @@ -25,15 +26,11 @@ void request_event_sources_irqs(struct device_node *np, const char *name) { int i, index, count = 0; - struct of_phandle_args oirq; unsigned int virqs[16]; /* First try to do a proper OF tree parsing */ - for (index = 0; of_irq_parse_one(np, index, &oirq) == 0; - index++) { - if (count > 15) - break; - virqs[count] = irq_create_of_mapping(&oirq); + for (index = 0; count < 16; index++) { + virqs[count] = irq_of_parse_and_map(np, index); if (!virqs[count]) { pr_err("event-sources: Unable to allocate " "interrupt number for %pOF\n", -- 2.14.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc: pseries: use irq_of_parse_and_map helper 2018-01-04 22:45 [PATCH] powerpc: pseries: use irq_of_parse_and_map helper Rob Herring @ 2018-01-23 6:53 ` Michael Ellerman 2018-01-23 14:11 ` Rob Herring 0 siblings, 1 reply; 4+ messages in thread From: Michael Ellerman @ 2018-01-23 6:53 UTC (permalink / raw) To: Rob Herring Cc: linux-kernel, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev Rob Herring <robh@kernel.org> writes: > Instead of calling both of_irq_parse_one and irq_create_of_mapping, call > of_irq_parse_and_map instead which does the same thing. This gets us closer > to making the former 2 functions static. > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: linuxppc-dev@lists.ozlabs.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- > arch/powerpc/platforms/pseries/event_sources.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) Sorry NAK, this doesn't boot. > diff --git a/arch/powerpc/platforms/pseries/event_sources.c b/arch/powerpc/platforms/pseries/event_sources.c > index 6eeb0d4bab61..b0d8c146fe7b 100644 > --- a/arch/powerpc/platforms/pseries/event_sources.c > +++ b/arch/powerpc/platforms/pseries/event_sources.c > @@ -16,7 +16,8 @@ > * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > */ > > -#include <asm/prom.h> > +#include <linux/interrupt.h> > +#include <linux/of_irq.h> > > #include "pseries.h" > > @@ -25,15 +26,11 @@ void request_event_sources_irqs(struct device_node *np, > const char *name) > { > int i, index, count = 0; > - struct of_phandle_args oirq; > unsigned int virqs[16]; > > /* First try to do a proper OF tree parsing */ > - for (index = 0; of_irq_parse_one(np, index, &oirq) == 0; > - index++) { > - if (count > 15) > - break; > - virqs[count] = irq_create_of_mapping(&oirq); > + for (index = 0; count < 16; index++) { > + virqs[count] = irq_of_parse_and_map(np, index); > if (!virqs[count]) { > pr_err("event-sources: Unable to allocate " > "interrupt number for %pOF\n", np); WARN_ON(1); } else { count++; } } Which is an infinite loop if we have less than 16 irqs, and spews the warning continuously. Are you trying to remove the low-level routines or is this just a cleanup? The patch below works, it loses the error handling if the interrupts property is corrupt/empty, but that's probably overly paranoid anyway. cheers diff --git a/arch/powerpc/platforms/pseries/event_sources.c b/arch/powerpc/platforms/pseries/event_sources.c index 6eeb0d4bab61..25c38077c894 100644 --- a/arch/powerpc/platforms/pseries/event_sources.c +++ b/arch/powerpc/platforms/pseries/event_sources.c @@ -16,7 +16,8 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#include <asm/prom.h> +#include <linux/interrupt.h> +#include <linux/of_irq.h> #include "pseries.h" @@ -24,34 +25,16 @@ void request_event_sources_irqs(struct device_node *np, irq_handler_t handler, const char *name) { - int i, index, count = 0; - struct of_phandle_args oirq; - unsigned int virqs[16]; + unsigned int virq; + int i, rc; - /* First try to do a proper OF tree parsing */ - for (index = 0; of_irq_parse_one(np, index, &oirq) == 0; - index++) { - if (count > 15) + for (i = 0; i < 16; i++) { + virq = irq_of_parse_and_map(np, i); + if (!virq) break; - virqs[count] = irq_create_of_mapping(&oirq); - if (!virqs[count]) { - pr_err("event-sources: Unable to allocate " - "interrupt number for %pOF\n", - np); - WARN_ON(1); - } else { - count++; - } - } - /* Now request them */ - for (i = 0; i < count; i++) { - if (request_irq(virqs[i], handler, 0, name, NULL)) { - pr_err("event-sources: Unable to request interrupt " - "%d for %pOF\n", virqs[i], np); - WARN_ON(1); - return; - } + rc = request_irq(virq, handler, 0, name, NULL); + WARN(rc, "event-sources: Unable to request interrupt %d for %pOF\n", + virq, np); } } - ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc: pseries: use irq_of_parse_and_map helper 2018-01-23 6:53 ` Michael Ellerman @ 2018-01-23 14:11 ` Rob Herring 2018-01-25 6:08 ` Michael Ellerman 0 siblings, 1 reply; 4+ messages in thread From: Rob Herring @ 2018-01-23 14:11 UTC (permalink / raw) To: Michael Ellerman Cc: linux-kernel@vger.kernel.org, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev On Tue, Jan 23, 2018 at 12:53 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: > Rob Herring <robh@kernel.org> writes: > >> Instead of calling both of_irq_parse_one and irq_create_of_mapping, call >> of_irq_parse_and_map instead which does the same thing. This gets us closer >> to making the former 2 functions static. >> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Cc: Paul Mackerras <paulus@samba.org> >> Cc: Michael Ellerman <mpe@ellerman.id.au> >> Cc: linuxppc-dev@lists.ozlabs.org >> Signed-off-by: Rob Herring <robh@kernel.org> >> --- >> arch/powerpc/platforms/pseries/event_sources.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) > > Sorry NAK, this doesn't boot. > >> diff --git a/arch/powerpc/platforms/pseries/event_sources.c b/arch/powerpc/platforms/pseries/event_sources.c >> index 6eeb0d4bab61..b0d8c146fe7b 100644 >> --- a/arch/powerpc/platforms/pseries/event_sources.c >> +++ b/arch/powerpc/platforms/pseries/event_sources.c >> @@ -16,7 +16,8 @@ >> * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA >> */ >> >> -#include <asm/prom.h> >> +#include <linux/interrupt.h> >> +#include <linux/of_irq.h> >> >> #include "pseries.h" >> >> @@ -25,15 +26,11 @@ void request_event_sources_irqs(struct device_node *np, >> const char *name) >> { >> int i, index, count = 0; >> - struct of_phandle_args oirq; >> unsigned int virqs[16]; >> >> /* First try to do a proper OF tree parsing */ >> - for (index = 0; of_irq_parse_one(np, index, &oirq) == 0; >> - index++) { >> - if (count > 15) >> - break; >> - virqs[count] = irq_create_of_mapping(&oirq); >> + for (index = 0; count < 16; index++) { >> + virqs[count] = irq_of_parse_and_map(np, index); >> if (!virqs[count]) { >> pr_err("event-sources: Unable to allocate " >> "interrupt number for %pOF\n", > > np); > WARN_ON(1); > } else { > count++; > } > } > > > Which is an infinite loop if we have less than 16 irqs, and spews the > warning continuously. > > Are you trying to remove the low-level routines or is this just a > cleanup? The former, but I'm not sure that will happen. There's a handful of others left, but they aren't simply a call to of_irq_parse_one and then irq_create_of_mapping. > The patch below works, it loses the error handling if the interrupts > property is corrupt/empty, but that's probably overly paranoid anyway. Not quite. Previously, it was silent if parsing failed. Only the mapping would give an error which would mean the interrupt parent had some error. Actually, we could use of_irq_get here to preserve the error handling. It will return error codes from parsing, 0 on mapping failure, or the Linux irq number. It adds an irq_find_host call for deferred probe, but that should be harmless. I'll respin it. Rob ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc: pseries: use irq_of_parse_and_map helper 2018-01-23 14:11 ` Rob Herring @ 2018-01-25 6:08 ` Michael Ellerman 0 siblings, 0 replies; 4+ messages in thread From: Michael Ellerman @ 2018-01-25 6:08 UTC (permalink / raw) To: Rob Herring Cc: linux-kernel@vger.kernel.org, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev Rob Herring <robh@kernel.org> writes: > On Tue, Jan 23, 2018 at 12:53 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: >> Rob Herring <robh@kernel.org> writes: >> >>> Instead of calling both of_irq_parse_one and irq_create_of_mapping, call >>> of_irq_parse_and_map instead which does the same thing. This gets us closer >>> to making the former 2 functions static. ... >> Are you trying to remove the low-level routines or is this just a >> cleanup? > > The former, but I'm not sure that will happen. There's a handful of > others left, but they aren't simply a call to of_irq_parse_one and > then irq_create_of_mapping. > >> The patch below works, it loses the error handling if the interrupts >> property is corrupt/empty, but that's probably overly paranoid anyway. > > Not quite. Previously, it was silent if parsing failed. Only the > mapping would give an error which would mean the interrupt parent had > some error. > > Actually, we could use of_irq_get here to preserve the error handling. > It will return error codes from parsing, 0 on mapping failure, or the > Linux irq number. It adds an irq_find_host call for deferred probe, > but that should be harmless. I'll respin it. OK thanks. cheers ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-25 6:08 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-04 22:45 [PATCH] powerpc: pseries: use irq_of_parse_and_map helper Rob Herring 2018-01-23 6:53 ` Michael Ellerman 2018-01-23 14:11 ` Rob Herring 2018-01-25 6:08 ` Michael Ellerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).