On 2013年12月13日 16:02, Tomasz Nowicki wrote:
On 12/13/13 08:48, Hanjun Guo wrote:
On 2013-12-13 0:10, Tomasz Nowicki wrote:
Move OF specific logic to OF irq.c file and make it more generic so acpi can use it too.
Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org
drivers/of/irq.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/linux/irqdomain.h | 3 +++ kernel/irq/irqdomain.c | 31 ++++++++++--------------------- 3 files changed, 55 insertions(+), 21 deletions(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 786b0b4..2bac976 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -26,6 +26,48 @@ #include <linux/string.h> #include <linux/slab.h>
+/**
- irq_create_of_mapping - Look up for domain and map an interrupt
given irq
- @irq_data: pointer to arguments structure
- This function looks for associated domain, optionally
translates, and maps
- int linux virq space
- */
+unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data) +{
- struct irq_domain *domain;
- irq_hw_number_t hwirq;
- unsigned int type = IRQ_TYPE_NONE;
- if (!irq_data->np) {
/** Nothing wrong with that, try default domain.*/domain = NULL;
You didn't try default domain here and didn't return, then will
- } else {
domain = irq_find_host(irq_data->np);if (!domain) {pr_warn("no irq domain found for %s !\n",of_node_full_name(irq_data->np));return 0;}- }
- /* If domain has no translation, then we assume interrupt line */
- if (domain->ops->xlate == NULL)
NULL reference here.
hwirq = irq_data->args[0];- else {
if (domain->ops->xlate(domain, irq_data->np, irq_data->args,irq_data->args_count, &hwirq, &type))return 0;- }
- /* Create mapping */
- return irq_create_mapping_type(domain, hwirq, type);
+} +EXPORT_SYMBOL_GPL(irq_create_of_mapping);
- /**
- irq_of_parse_and_map - Parse and map an interrupt into linux
virq space
- @dev: Device node of the device whose interrupt is to be mapped
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h index c983ed1..c14f59b 100644 --- a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -175,6 +175,9 @@ extern void irq_domain_associate_many(struct irq_domain *domain,
extern unsigned int irq_create_mapping(struct irq_domain *host, irq_hw_number_t hwirq); +extern unsigned int irq_create_mapping_type(struct irq_domain *domain,
irq_hw_number_t hwirq,unsigned int type);extern void irq_dispose_mapping(unsigned int virq);
/**
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index c661552..bf74a0b 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -465,29 +465,18 @@ int irq_create_strict_mappings(struct irq_domain *domain, unsigned int irq_base, } EXPORT_SYMBOL_GPL(irq_create_strict_mappings);
-unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data) +/**
- irq_create_mapping_type() - Create mapping and set given irq type
- @domain: domain owning this hardware interrupt
- @hwirq: hardware interrupt
- @type: interrupt type
- */
+unsigned int irq_create_mapping_type(struct irq_domain *domain,
irq_hw_number_t hwirq,unsigned int type)
struct irq_domain *domain;
irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE; unsigned int virq;
domain = irq_data->np ? irq_find_host(irq_data->np) :
irq_default_domain;
- if (!domain) {
pr_warn("no irq domain found for %s !\n",of_node_full_name(irq_data->np));return 0;- }
- /* If domain has no translation, then we assume interrupt line */
- if (domain->ops->xlate == NULL)
hwirq = irq_data->args[0];- else {
if (domain->ops->xlate(domain, irq_data->np, irq_data->args,irq_data->args_count, &hwirq, &type))return 0;- }
/* Create mapping */ virq = irq_create_mapping(domain, hwirq); if (!virq)@@ -499,7 +488,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data) irq_set_irq_type(virq, type); return virq; } -EXPORT_SYMBOL_GPL(irq_create_of_mapping); +EXPORT_SYMBOL_GPL(irq_create_mapping_type);
#ifdef CONFIG_ACPI unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq,
It seems that folks in the linux community had different comment for this, I paste them here:
On Wed, Dec 4, 2013 at 4:38 PM, Hanjun Guo hanjun.guo@linaro.org wrote:
On 2013年12月04日 01:25, Rob Herring wrote:
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
From: Amit Daniel Kachhap amit.daniel@samsung.com
This patch introduces a new API for acpi based irq mapping.
[hanjun: Rework this patch to delete the reference to gic_irq_domain_xlate() which can simplify the code a lot.]
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
(...)
+EXPORT_SYMBOL_GPL(irq_create_acpi_mapping);
There is nothing ACPI specific about this function. This is simply irq_create_of_mapping w/o translating of_phandle_args to a hwirq and type. So I expect the code to be re-factored here to mirror that.
Sorry for my bad english, do you mean create a OF free function and call that from the OF function ?
Sounds like a good idea, like if you move the OF function into kernel/irq/irqdomain.c and get rid of the OF specific naming then use that same function from ACPI too. It's all about using the irq_default_domain in a standard way after all right?
This is how I read it, Rob suggest to move OF specific things of irq_create_of_mapping to drivers/of/irq.c (he said to kernel/irq/irqdomain.c but this function is already there so I belive he misteake dircetionry). Then create OF free generic function in kernel/irq/irqdomain.c and call it by acpi and of. Correct me if I am wrong.
Yes, you are right, I agree with your solution.
Thanks Hanjun