Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751622Ab3GXLfR (ORCPT ); Wed, 24 Jul 2013 07:35:17 -0400 Received: from mail-ea0-f173.google.com ([209.85.215.173]:34063 "EHLO mail-ea0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751419Ab3GXLfP (ORCPT ); Wed, 24 Jul 2013 07:35:15 -0400 Date: Wed, 24 Jul 2013 12:35:08 +0100 From: Lee Jones To: Grygorii Strashko Cc: Samuel Ortiz , Kevin Hilman , Graeme Gregory , linux-omap@vger.kernel.org, Ruslan Bilovol , linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/4] mfd: twl6030-irq: convert to use linear irq_domain Message-ID: <20130724113508.GI26801@laptop> References: <1374595624-15054-1-git-send-email-grygorii.strashko@ti.com> <1374595624-15054-4-git-send-email-grygorii.strashko@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1374595624-15054-4-git-send-email-grygorii.strashko@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7139 Lines: 231 On Tue, 23 Jul 2013, Grygorii Strashko wrote: > Since the TWL6030 PMIC is used with OMAP4 SoCs only and OMAP4 legacy > boot is dropped there are no needs to allocate the range of IRQ > descriptors during system boot to support TWL6030 IRQs. > > Hence, convert it to use linear irq_domain and move IRQ configuration in > .map()/.unmap() callbacks of irq_domain. So, IRQ mapping and descriptors > allocation will be performed dynamically basing on DT configuration. > > The error message will be reported in case if unmapped IRQ is received by > TWL6030 (virq==0). > > Signed-off-by: Grygorii Strashko > --- > drivers/mfd/twl6030-irq.c | 114 ++++++++++++++++++++++++--------------------- > 1 file changed, 61 insertions(+), 53 deletions(-) > > diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c > index 790cc28..89f130b 100644 > --- a/drivers/mfd/twl6030-irq.c > +++ b/drivers/mfd/twl6030-irq.c > @@ -138,6 +138,7 @@ static struct notifier_block twl6030_irq_pm_notifier_block = { > static irqreturn_t twl6030_irq_thread(int irq, void *data) > { > int i, ret; > + struct irq_domain *irq_domain = (struct irq_domain *)data; > union { > u8 bytes[4]; > u32 int_sts; > @@ -161,9 +162,14 @@ static irqreturn_t twl6030_irq_thread(int irq, void *data) > > for (i = 0; sts.int_sts; sts.int_sts >>= 1, i++) > if (sts.int_sts & 0x1) { > - int module_irq = twl6030_irq_base + > - twl6030_interrupt_mapping[i]; > - handle_nested_irq(module_irq); > + int module_irq = > + irq_find_mapping(irq_domain, > + twl6030_interrupt_mapping[i]); > + if (module_irq) > + handle_nested_irq(module_irq); > + else > + pr_err("%s: Unmapped PIH ISR %u detected\n", > + __func__, i); Same here. Does the user really care which function failed? Please consider replacing with the device name. > pr_debug("%s: PIH ISR %u, virq%u\n", > __func__, i, module_irq); > } > @@ -186,19 +192,6 @@ static irqreturn_t twl6030_irq_thread(int irq, void *data) > > /*----------------------------------------------------------------------*/ > > -static inline void activate_irq(int irq) > -{ > -#ifdef CONFIG_ARM > - /* ARM requires an extra step to clear IRQ_NOREQUEST, which it > - * sets on behalf of every irq_chip. Also sets IRQ_NOPROBE. > - */ > - set_irq_flags(irq, IRQF_VALID); > -#else > - /* same effect on other architectures */ > - irq_set_noprobe(irq); > -#endif > -} > - > static int twl6030_irq_set_wake(struct irq_data *d, unsigned int on) > { > if (on) > @@ -308,28 +301,54 @@ int twl6030_mmc_card_detect(struct device *dev, int slot) > } > EXPORT_SYMBOL(twl6030_mmc_card_detect); > > +static struct irq_chip twl6030_irq_chip; > + > +static int twl6030_irq_map(struct irq_domain *d, unsigned int virq, > + irq_hw_number_t hwirq) > +{ > + irq_set_chip_data(virq, &twl6030_irq_chip); > + irq_set_chip_and_handler(virq, &twl6030_irq_chip, handle_simple_irq); > + irq_set_nested_thread(virq, true); > + > +#ifdef CONFIG_ARM > + /* > + * ARM requires an extra step to clear IRQ_NOREQUEST, which it > + * sets on behalf of every irq_chip. Also sets IRQ_NOPROBE. > + */ > + set_irq_flags(virq, IRQF_VALID); > +#else > + /* same effect on other architectures */ > + irq_set_noprobe(virq); > +#endif > + > + return 0; > +} > + > +static void twl6030_irq_unmap(struct irq_domain *d, unsigned int virq) > +{ > +#ifdef CONFIG_ARM > + set_irq_flags(virq, 0); > +#endif > + irq_set_chip_and_handler(virq, NULL, NULL); > + irq_set_chip_data(virq, NULL); > +} > + > +static struct irq_domain_ops twl6030_irq_domain_ops = { > + .map = twl6030_irq_map, > + .unmap = twl6030_irq_unmap, > + .xlate = irq_domain_xlate_onetwocell, > +}; > + > int twl6030_init_irq(struct device *dev, int irq_num) > { > struct device_node *node = dev->of_node; > - int nr_irqs, irq_base, irq_end; > - static struct irq_chip twl6030_irq_chip; > + int nr_irqs; > int status; > - int i; > u8 mask[3]; > + struct irq_domain *irq_domain; > > nr_irqs = TWL6030_NR_IRQS; > > - irq_base = irq_alloc_descs(-1, 0, nr_irqs, 0); > - if (IS_ERR_VALUE(irq_base)) { > - dev_err(dev, "Fail to allocate IRQ descs\n"); > - return irq_base; > - } > - > - irq_domain_add_legacy(node, nr_irqs, irq_base, 0, > - &irq_domain_simple_ops, NULL); > - > - irq_end = irq_base + nr_irqs; > - > mask[0] = 0xFF; > mask[1] = 0xFF; > mask[2] = 0xFF; > @@ -346,8 +365,6 @@ int twl6030_init_irq(struct device *dev, int irq_num) > return status; > } > > - twl6030_irq_base = irq_base; > - > /* > * install an irq handler for each of the modules; > * clone dummy irq_chip since PIH can't *do* anything > @@ -357,20 +374,18 @@ int twl6030_init_irq(struct device *dev, int irq_num) > twl6030_irq_chip.irq_set_type = NULL; > twl6030_irq_chip.irq_set_wake = twl6030_irq_set_wake; > > - for (i = irq_base; i < irq_end; i++) { > - irq_set_chip_and_handler(i, &twl6030_irq_chip, > - handle_simple_irq); > - irq_set_chip_data(i, (void *)irq_num); > - irq_set_nested_thread(i, true); > - activate_irq(i); > + irq_domain = irq_domain_add_linear(node, nr_irqs, > + &twl6030_irq_domain_ops, NULL); > + if (!irq_domain) { > + dev_err(dev, "Can't add irq_domain\n"); > + return -ENOMEM; > } > > - dev_info(dev, "PIH (irq %d) nested IRQs %d..%d\n", > - irq_num, irq_base, irq_end); > + dev_info(dev, "PIH (irq %d) nested IRQs\n", irq_num); > > /* install an irq handler to demultiplex the TWL6030 interrupt */ > status = request_threaded_irq(irq_num, NULL, twl6030_irq_thread, > - IRQF_ONESHOT, "TWL6030-PIH", NULL); > + IRQF_ONESHOT, "TWL6030-PIH", irq_domain); > if (status < 0) { > dev_err(dev, "could not claim irq %d: %d\n", irq_num, status); > goto fail_irq; > @@ -378,26 +393,19 @@ int twl6030_init_irq(struct device *dev, int irq_num) > > twl_irq = irq_num; > register_pm_notifier(&twl6030_irq_pm_notifier_block); > - return irq_base; > + return irq_num; I think you need to change twl-core to now expect the total number of IRQs rather than the base one now. > fail_irq: > - for (i = irq_base; i < irq_end; i++) > - irq_set_chip_and_handler(i, NULL, NULL); > - > + irq_domain_remove(irq_domain); Why do you kill the irqdomain here, but not in exit()? > return status; > } > > int twl6030_exit_irq(void) > { > - unregister_pm_notifier(&twl6030_irq_pm_notifier_block); > - > - if (!twl6030_irq_base) { > - pr_err("twl6030: can't yet clean up IRQs?\n"); > - return -ENOSYS; > + if (twl_irq) { > + unregister_pm_notifier(&twl6030_irq_pm_notifier_block); > + free_irq(twl_irq, NULL); > } Ah yes, that's better. > - > - free_irq(twl_irq, NULL); > - > return 0; > } > -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/