On 2014/11/15 1:35, Marc Zyngier wrote:
> On 14/11/14 15:41, Jiang Liu wrote:
>> On 2014/11/14 23:31, Marc Zyngier wrote:
>>> On 12/11/14 13:43, Thomas Gleixner wrote:
>>>> From: Jiang Liu <jiang....@linux.intel.com>
>>>>
>>>> Signed-off-by: Jiang Liu <jiang....@linux.intel.com>
>>>> Cc: Bjorn Helgaas <bhelg...@google.com>
>>>> Cc: Grant Likely <grant.lik...@linaro.org>
>>>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>>>> Cc: Yingjoe Chen <yingjoe.c...@mediatek.com>
>>>> Cc: Yijing Wang <wangyij...@huawei.com>
>>>> Signed-off-by: Thomas Gleixner <t...@linutronix.de>
>>>> ---
>>>> 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...
>> Hi Marc,
>> I prefer the above solution to relax the warning conditions.
>> Changing the calling order in irq_domain_ops->alloc() looks a little
>> strange, and other interrupt drivers may still run into the same issue.
>
> OK. Where do we from from there? Do you want a proper patch, or will you
> fold this into the existing code?
A patch will be great:)
>
> Thanks,
>
> M.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
