lkml.org 
[lkml]   [2013]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/4] mfd: twl6030-irq: convert to use linear irq_domain
    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 <grygorii.strashko@ti.com>
    > ---
    > 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/

    \
     
     \ /
      Last update: 2013-07-24 14:21    [W:3.493 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site