From: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> To: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>, Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>, Jiang Liu <jiang.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>, Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Kevin Hilman <khilman-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>, Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>, Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [RFC PATCH V2 1/8] irqdomain: Ensure type settings match for an existing mapping Date: Fri, 18 Dec 2015 10:10:15 +0000 [thread overview] Message-ID: <5673DB87.6040106@nvidia.com> (raw) In-Reply-To: <CACRpkdYPvMfqou7t9K_5=Ojx3U_sc8B2Zkxgeu=1JXxCUU_E2Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> On 17/12/15 13:16, Linus Walleij wrote: > On Thu, Dec 17, 2015 at 11:48 AM, Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote: > >> When mapping an IRQ, if a mapping already exists, then we simply return >> the virual IRQ number. However, we do not check that the type settings for > > ^virtual > > Just that it isn't virtual, it's a Linux IRQ number, we actually use > hwirq for the non-virtual IRQ number/offse in this function. > > But I know I may be fighting weathermills here. Ok, will re-word this. >> unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec) >> { >> + struct device_node *of_node; >> struct irq_domain *domain; >> irq_hw_number_t hwirq; >> + unsigned int cur_type = IRQ_TYPE_NONE; >> unsigned int type = IRQ_TYPE_NONE; >> int virq; >> >> @@ -587,23 +589,49 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec) >> if (irq_domain_translate(domain, fwspec, &hwirq, &type)) >> return 0; >> >> - if (irq_domain_is_hierarchy(domain)) { >> - /* >> - * If we've already configured this interrupt, >> - * don't do it again, or hell will break loose. >> - */ >> - virq = irq_find_mapping(domain, hwirq); >> - if (virq) >> - return virq; >> + of_node = irq_domain_get_of_node(domain); > > Marc's patches went to great lengths to do this fwspec-neutral, > i.e. it doesn't matter if it's done by DT or ACPI (or whatever). > > This just drives a truck through all of that by making > the whole function OF-specific again. Yes, was not sure if this would be popular. I was on the fence, but I saw the following ... if (!domain) { pr_warn("no irq domain found for %s !\n", of_node_full_name(to_of_node(fwspec->fwnode))); return 0; } ... and thought we are not completely agnostic. However, if you prefer I park my mini else where, I can definitely drop this, no big deal ;-) >> >> - virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, fwspec); >> - if (virq <= 0) >> - return 0; >> + /* >> + * If we've already configured this interrupt, >> + * don't do it again, or hell will break loose. >> + */ >> + virq = irq_find_mapping(domain, hwirq); >> + if (!virq) { >> + if (irq_domain_is_hierarchy(domain)) { >> + virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, >> + fwspec); >> + if (virq <= 0) >> + return 0; >> + } else { >> + virq = irq_domain_alloc_descs(-1, 1, hwirq, >> + of_node_to_nid(of_node)); > > What is this all of a sudden? Not even mentioned in the > commit. Plus I bet ACPI need something else than OF nid > passed here. Do you mean the else part of all of the above? So in the current code, the else part calls irq_create_mapping() and this function internally, calls irq_find_mapping(). Given that I am now calling irq_find_mapping() before the if, I don't really need to call irq_create_mapping() again, I just need to call the functions in irq_create_mapping() that allocate and setup the IRQ number. Sorry, I did not really explain this. However, if it is simpler, I can call irq_create_mapping() instead and may be this makes the change easier to read. Cheers Jon
WARNING: multiple messages have this Message-ID (diff)
From: Jon Hunter <jonathanh@nvidia.com> To: Linus Walleij <linus.walleij@linaro.org> Cc: Thomas Gleixner <tglx@linutronix.de>, Jason Cooper <jason@lakedaemon.net>, Marc Zyngier <marc.zyngier@arm.com>, Jiang Liu <jiang.liu@linux.intel.com>, Stephen Warren <swarren@wwwdotorg.org>, Thierry Reding <thierry.reding@gmail.com>, Kevin Hilman <khilman@kernel.org>, Geert Uytterhoeven <geert@linux-m68k.org>, Grygorii Strashko <grygorii.strashko@ti.com>, Lars-Peter Clausen <lars@metafoo.de>, Soren Brinkmann <soren.brinkmann@xilinx.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org> Subject: Re: [RFC PATCH V2 1/8] irqdomain: Ensure type settings match for an existing mapping Date: Fri, 18 Dec 2015 10:10:15 +0000 [thread overview] Message-ID: <5673DB87.6040106@nvidia.com> (raw) In-Reply-To: <CACRpkdYPvMfqou7t9K_5=Ojx3U_sc8B2Zkxgeu=1JXxCUU_E2Q@mail.gmail.com> On 17/12/15 13:16, Linus Walleij wrote: > On Thu, Dec 17, 2015 at 11:48 AM, Jon Hunter <jonathanh@nvidia.com> wrote: > >> When mapping an IRQ, if a mapping already exists, then we simply return >> the virual IRQ number. However, we do not check that the type settings for > > ^virtual > > Just that it isn't virtual, it's a Linux IRQ number, we actually use > hwirq for the non-virtual IRQ number/offse in this function. > > But I know I may be fighting weathermills here. Ok, will re-word this. >> unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec) >> { >> + struct device_node *of_node; >> struct irq_domain *domain; >> irq_hw_number_t hwirq; >> + unsigned int cur_type = IRQ_TYPE_NONE; >> unsigned int type = IRQ_TYPE_NONE; >> int virq; >> >> @@ -587,23 +589,49 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec) >> if (irq_domain_translate(domain, fwspec, &hwirq, &type)) >> return 0; >> >> - if (irq_domain_is_hierarchy(domain)) { >> - /* >> - * If we've already configured this interrupt, >> - * don't do it again, or hell will break loose. >> - */ >> - virq = irq_find_mapping(domain, hwirq); >> - if (virq) >> - return virq; >> + of_node = irq_domain_get_of_node(domain); > > Marc's patches went to great lengths to do this fwspec-neutral, > i.e. it doesn't matter if it's done by DT or ACPI (or whatever). > > This just drives a truck through all of that by making > the whole function OF-specific again. Yes, was not sure if this would be popular. I was on the fence, but I saw the following ... if (!domain) { pr_warn("no irq domain found for %s !\n", of_node_full_name(to_of_node(fwspec->fwnode))); return 0; } ... and thought we are not completely agnostic. However, if you prefer I park my mini else where, I can definitely drop this, no big deal ;-) >> >> - virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, fwspec); >> - if (virq <= 0) >> - return 0; >> + /* >> + * If we've already configured this interrupt, >> + * don't do it again, or hell will break loose. >> + */ >> + virq = irq_find_mapping(domain, hwirq); >> + if (!virq) { >> + if (irq_domain_is_hierarchy(domain)) { >> + virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, >> + fwspec); >> + if (virq <= 0) >> + return 0; >> + } else { >> + virq = irq_domain_alloc_descs(-1, 1, hwirq, >> + of_node_to_nid(of_node)); > > What is this all of a sudden? Not even mentioned in the > commit. Plus I bet ACPI need something else than OF nid > passed here. Do you mean the else part of all of the above? So in the current code, the else part calls irq_create_mapping() and this function internally, calls irq_find_mapping(). Given that I am now calling irq_find_mapping() before the if, I don't really need to call irq_create_mapping() again, I just need to call the functions in irq_create_mapping() that allocate and setup the IRQ number. Sorry, I did not really explain this. However, if it is simpler, I can call irq_create_mapping() instead and may be this makes the change easier to read. Cheers Jon
next prev parent reply other threads:[~2015-12-18 10:10 UTC|newest] Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-12-17 10:48 [RFC PATCH V2 0/8] Add support for Tegra210 AGIC Jon Hunter 2015-12-17 10:48 ` Jon Hunter 2015-12-17 10:48 ` [RFC PATCH V2 1/8] irqdomain: Ensure type settings match for an existing mapping Jon Hunter 2015-12-17 10:48 ` Jon Hunter 2015-12-17 13:16 ` Linus Walleij [not found] ` <CACRpkdYPvMfqou7t9K_5=Ojx3U_sc8B2Zkxgeu=1JXxCUU_E2Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-18 10:10 ` Jon Hunter [this message] 2015-12-18 10:10 ` Jon Hunter 2015-12-22 9:58 ` Linus Walleij [not found] ` <CACRpkdbjRiW8gcZcifHLELjBukpsCKyTQ+NpP51+v3kYLDcPHA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-22 10:00 ` Linus Walleij 2015-12-22 10:00 ` Linus Walleij [not found] ` <CACRpkdasLeVyE7MXyJ=LQHSYxUDE77VttX_TdqMV6afgk_NqrQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-22 11:27 ` Jon Hunter 2015-12-22 11:27 ` Jon Hunter 2015-12-22 11:31 ` Grygorii Strashko 2015-12-22 11:31 ` Grygorii Strashko [not found] ` <1450349309-8107-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2015-12-17 10:48 ` [RFC PATCH V2 2/8] irqdomain: Don't set type when mapping an IRQ Jon Hunter 2015-12-17 10:48 ` Jon Hunter [not found] ` <1450349309-8107-3-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2015-12-17 12:18 ` Linus Walleij 2015-12-17 12:18 ` Linus Walleij 2015-12-22 11:18 ` Grygorii Strashko 2015-12-22 11:18 ` Grygorii Strashko [not found] ` <56793191.30502-l0cyMroinI0@public.gmane.org> 2015-12-22 11:56 ` Jon Hunter 2015-12-22 11:56 ` Jon Hunter [not found] ` <56793A5B.4040302-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-03-18 9:16 ` Geert Uytterhoeven 2016-03-18 9:16 ` Geert Uytterhoeven 2015-12-17 10:48 ` [RFC PATCH V2 3/8] genirq: Add runtime power management support for IRQ chips Jon Hunter 2015-12-17 10:48 ` Jon Hunter 2015-12-17 13:19 ` Linus Walleij 2015-12-18 10:20 ` Jon Hunter 2016-01-12 18:40 ` Grygorii Strashko 2016-01-12 18:40 ` Grygorii Strashko 2016-01-12 21:43 ` Sören Brinkmann 2016-01-12 21:43 ` Sören Brinkmann [not found] ` <1450349309-8107-4-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-01-18 14:47 ` Ulf Hansson 2016-01-18 14:47 ` Ulf Hansson 2016-01-19 10:43 ` Jon Hunter 2016-01-20 15:30 ` Thomas Gleixner 2016-01-21 8:38 ` Jon Hunter 2016-01-21 8:38 ` Jon Hunter 2016-01-21 12:40 ` Ulf Hansson 2016-01-21 12:40 ` Ulf Hansson 2016-01-21 12:40 ` Ulf Hansson 2016-01-21 19:51 ` Thomas Gleixner 2016-01-22 11:08 ` Ulf Hansson 2016-01-22 11:08 ` Ulf Hansson 2016-01-26 17:17 ` Thomas Gleixner 2016-02-05 14:37 ` Linus Walleij 2016-02-05 14:37 ` Linus Walleij [not found] ` <CACRpkdbE-Ny585yK+DBkNENpWyk8rSEvdRdvLgMTCBp13grp4w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-03-18 13:57 ` Grygorii Strashko 2016-03-18 13:57 ` Grygorii Strashko 2015-12-17 10:48 ` [RFC PATCH V2 4/8] irqchip/gic: Don't initialise chip if mapping IO space fails Jon Hunter 2015-12-17 10:48 ` Jon Hunter [not found] ` <1450349309-8107-5-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2015-12-17 13:21 ` Linus Walleij 2015-12-17 13:21 ` Linus Walleij 2015-12-17 10:48 ` [RFC PATCH V2 5/8] irqchip/gic: Return an error if GIC initialisation fails Jon Hunter 2015-12-17 10:48 ` Jon Hunter [not found] ` <1450349309-8107-6-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2015-12-17 13:26 ` Linus Walleij 2015-12-17 13:26 ` Linus Walleij [not found] ` <CACRpkdaw+DM5ddi27UpJEg-+3A3ffS7k_Qnm4i-w2rLhpxp+8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-18 10:24 ` Jon Hunter 2015-12-18 10:24 ` Jon Hunter 2015-12-17 10:48 ` [RFC PATCH V2 6/8] irqchip/gic: Assign irqchip dynamically Jon Hunter 2015-12-17 10:48 ` Jon Hunter [not found] ` <1450349309-8107-7-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2015-12-17 11:00 ` Marc Zyngier 2015-12-17 11:00 ` Marc Zyngier [not found] ` <567295B5.8050900-5wv7dgnIgG8@public.gmane.org> 2015-12-18 10:26 ` Jon Hunter 2015-12-18 10:26 ` Jon Hunter 2015-12-17 10:48 ` [RFC PATCH V2 7/8] irqchip/gic: Prepare for adding platform driver Jon Hunter 2015-12-17 10:48 ` Jon Hunter 2015-12-17 10:48 ` [RFC PATCH V2 8/8] irqchip/gic: Add support for tegra AGIC interrupt controller Jon Hunter 2015-12-17 10:48 ` Jon Hunter 2015-12-17 10:58 ` Jon Hunter 2015-12-17 10:58 ` Jon Hunter [not found] ` <1450349309-8107-9-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2015-12-17 13:32 ` Linus Walleij 2015-12-17 13:32 ` Linus Walleij 2015-12-18 10:44 ` Jon Hunter [not found] ` <5673E38B.7060702-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2015-12-22 10:03 ` Linus Walleij 2015-12-22 10:03 ` Linus Walleij
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5673DB87.6040106@nvidia.com \ --to=jonathanh-ddmlm1+adcrqt0dzr+alfa@public.gmane.org \ --cc=geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org \ --cc=grygorii.strashko-l0cyMroinI0@public.gmane.org \ --cc=jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org \ --cc=jiang.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \ --cc=khilman-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \ --cc=lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org \ --cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \ --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=marc.zyngier-5wv7dgnIgG8@public.gmane.org \ --cc=soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org \ --cc=swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org \ --cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \ --cc=thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.