From: tglx@linutronix.de (Thomas Gleixner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios
Date: Fri, 2 Sep 2016 21:10:43 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.20.1609022057490.5647@nanos> (raw)
In-Reply-To: <1472830398-13275-7-git-send-email-alexandre.torgue@st.com>
On Fri, 2 Sep 2016, Alexandre TORGUE wrote:
> +static int stm32_gpio_domain_translate(struct irq_domain *d,
> + struct irq_fwspec *fwspec,
> + unsigned long *hwirq,
> + unsigned int *type)
> +{
> + if ((fwspec->param_count != 2) ||
> + (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
> + return -EINVAL;
Just a nitpick. This is unnecessarily hard to parse because you indented
the line break like a conditional statement
> + if ((fwspec->param_count != 2) ||
> + (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
> + return -EINVAL;
Makes it immediately obvious that the second line belongs to the if.
> +static void stm32_gpio_domain_activate(struct irq_domain *d,
> + struct irq_data *irq_data)
> +{
> + struct stm32_gpio_bank *bank = d->host_data;
> + struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
> +
> + if (gpiochip_lock_as_irq(&bank->gpio_chip, irq_data->hwirq)) {
> + dev_err(pctl->dev,
> + "Unable to configure STM32 %s%ld as IRQ\n",
> + bank->gpio_chip.label, irq_data->hwirq);
> + return;
Hmm, that's nasty. When an interrupt is mapped then we don't expect the
activate function to fail. You really should lock that interrupt when it's
mapped.
> + }
> + regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->range.id);
> +}
> +static int stm32_gpio_domain_alloc(struct irq_domain *domain,
> + unsigned int virq,
> + unsigned int nr_irqs, void *data)
> +{
> + struct irq_fwspec *fwspec = data;
> + struct irq_fwspec parent_fwspec;
> + struct stm32_pinctrl *pctl = domain->host_data;
> + irq_hw_number_t hwirq;
> + unsigned int i;
> +
> + hwirq = fwspec->param[0];
> + for (i = 0; i < nr_irqs; i++)
> + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> + &stm32_gpio_irq_chip, pctl);
> +
> + parent_fwspec.fwnode = domain->parent->fwnode;
> + parent_fwspec.param_count = 2;
> + parent_fwspec.param[0] = fwspec->param[0];
> + parent_fwspec.param[1] = fwspec->param[1];
> +
> + return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> + &parent_fwspec);
So doing it here would be probably the right thing to do:
ret = gpiochip_lock_as_irq();
if (ret)
return ret;
ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
&parent_fwspec);
if (ret)
gpiochip_unlock_as_irq();
return ret;
So of course you need your own free() function which undoes that lock
thingy.
Thanks,
tglx
next prev parent reply other threads:[~2016-09-02 19:10 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-02 15:33 [PATCH v3 0/9] Add STM32 EXTI interrupt controller support Alexandre TORGUE
2016-09-02 15:33 ` [PATCH v3 1/9] Documentation: dt-bindings: Document STM32 EXTI controller bindings Alexandre TORGUE
2016-09-02 15:33 ` [PATCH v3 2/9] drivers: irqchip: Add STM32 external interrupts support Alexandre TORGUE
2016-09-02 18:57 ` Thomas Gleixner
2016-09-05 7:47 ` Alexandre Torgue
2016-09-02 15:33 ` [PATCH v3 3/9] ARM: STM32: Select external interrupts controller Alexandre TORGUE
2016-09-02 15:33 ` [PATCH v3 4/9] ARM: dts: Add EXTI controller node to stm32f429 Alexandre TORGUE
2016-09-02 15:33 ` [PATCH v3 5/9] Documentation: dt-bindings: Add IRQ related properties of STM32 pinctrl Alexandre TORGUE
2016-09-02 15:33 ` [PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios Alexandre TORGUE
2016-09-02 19:10 ` Thomas Gleixner [this message]
2016-09-05 7:53 ` Alexandre Torgue
2016-09-02 15:33 ` [PATCH v3 7/9] ARM: dts: Add GPIO irq support to STM2F429 Alexandre TORGUE
2016-09-02 15:33 ` [PATCH v3 8/9] ARM: dts: Declare push button as GPIO key on stm32f429 boards Alexandre TORGUE
2016-09-02 15:33 ` [PATCH v3 9/9] ARM: config: Enable GPIO Key driver in stm32_defconfig Alexandre TORGUE
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=alpine.DEB.2.20.1609022057490.5647@nanos \
--to=tglx@linutronix.de \
--cc=linux-arm-kernel@lists.infradead.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).