Re: [patch 07/16] genirq: Introduce helper irq_domain_set_info() to reduce duplicated code

[Marc Zyngier]

On 12/11/14 13:43, Thomas Gleixner wrote:
> From: Jiang Liu <jiang.liu@xxxxxxxxxxxxxxx>
> 
> Signed-off-by: Jiang Liu <jiang.liu@xxxxxxxxxxxxxxx>
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: Grant Likely <grant.likely@xxxxxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Cc: Yingjoe Chen <yingjoe.chen@xxxxxxxxxxxx>
> Cc: Yijing Wang <wangyijing@xxxxxxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> ---
>  include/linux/irqdomain.h |    5 +++++
>  kernel/irq/irqdomain.c    |   10 ++++++++++
>  2 files changed, 15 insertions(+)
> 
> Index: tip/include/linux/irqdomain.h
> ===================================================================
> --- tip.orig/include/linux/irqdomain.h
> +++ tip/include/linux/irqdomain.h
> @@ -33,6 +33,7 @@
>  #define _LINUX_IRQDOMAIN_H
>  
>  #include <linux/types.h>
> +#include <linux/irqhandler.h>
>  #include <linux/radix-tree.h>
>  
>  struct device_node;
> @@ -263,6 +264,10 @@ extern int irq_domain_set_hwirq_and_chip
>  					 irq_hw_number_t hwirq,
>  					 struct irq_chip *chip,
>  					 void *chip_data);
> +extern void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,
> +				irq_hw_number_t hwirq, struct irq_chip *chip,
> +				void *chip_data, irq_flow_handler_t handler,
> +				void *handler_data, const char *handler_name);
>  extern void irq_domain_reset_irq_data(struct irq_data *irq_data);
>  extern void irq_domain_free_irqs_common(struct irq_domain *domain,
>  					int virq, int nr_irqs);
> Index: tip/kernel/irq/irqdomain.c
> ===================================================================
> --- tip.orig/kernel/irq/irqdomain.c
> +++ tip/kernel/irq/irqdomain.c
> @@ -882,6 +882,16 @@ int irq_domain_set_hwirq_and_chip(struct
>  	return 0;
>  }
>  
> +void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,
> +			 irq_hw_number_t hwirq, struct irq_chip *chip,
> +			 void *chip_data, irq_flow_handler_t handler,
> +			 void *handler_data, const char *handler_name)
> +{
> +	irq_domain_set_hwirq_and_chip(domain, virq, hwirq, chip, chip_data);
> +	__irq_set_handler(virq, handler, 0, handler_name);
> +	irq_set_handler_data(virq, handler_data);
> +}
> +

We still have the issue that, depending on where in the stack this is
 called, this will succeed or fail: If this is called from the inner
irqchip, __irq_set_handler() will fail, as it will look at the outer
domain as the (desc->irq_data.chip == &no_irq_chip) test fails (we
haven't set the top level yet).

I have this very imperfect workaround in my tree:

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index d028b34..91e6515 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -731,7 +731,16 @@ __irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained,
 	if (!handle) {
 		handle = handle_bad_irq;
 	} else {
-		if (WARN_ON(desc->irq_data.chip == &no_irq_chip))
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+		struct irq_data *irq_data = &desc->irq_data;
+		while (irq_data) {
+			if (irq_data->chip != &no_irq_chip)
+				break;
+			irq_data = irq_data->parent_data;
+		}
+#endif
+
+		if (WARN_ON(!irq_data || irq_data->chip == &no_irq_chip))
 			goto out;
 	}
 
Which translate into: If there is at least one irqchip in the domain,
it will probably sort itself out. Not ideal. Any real solution to
this problem?

GICv2 faces this exact problem, as some of its interrupts are used
directly, and some others are used through the MSI domain. In the
GIC driver, it is almost impossible to find out...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/