| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130724113508.GI26801@laptop>
Date: Wed, 24 Jul 2013 12:35:08 +0100
From: Lee Jones <lee.jones@...aro.org>
To: Grygorii Strashko <grygorii.strashko@...com>
Cc: Samuel Ortiz <sameo@...ux.intel.com>,
Kevin Hilman <khilman@...aro.org>,
Graeme Gregory <gg@...mlogic.co.uk>,
linux-omap@...r.kernel.org, Ruslan Bilovol <ruslan.bilovol@...com>,
linux-kernel@...r.kernel.org
Subject: Re: [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@...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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists