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; + } 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) + 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,
Because of making irq_create_acpi_mapping more generic, now we can use it for acpi too. Assumption for now is to use default irq domain.
Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org --- drivers/acpi/plat/arm-core.c | 7 ++++++- kernel/irq/irqdomain.c | 27 --------------------------- 2 files changed, 6 insertions(+), 28 deletions(-)
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 509b847..2e103e7 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -484,7 +484,12 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) else irq_type = IRQ_TYPE_NONE;
- irq = irq_create_acpi_mapping(gsi, irq_type); + /* + * Create mapping, use default domain. + * TODO: Based on MADT create list of GIC domains and find proper one + * here. + */ + irq = irq_create_mapping_type(NULL, gsi, irq_type); return irq; } EXPORT_SYMBOL_GPL(acpi_register_gsi); diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index bf74a0b..3255c5d 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -490,33 +490,6 @@ unsigned int irq_create_mapping_type(struct irq_domain *domain, } EXPORT_SYMBOL_GPL(irq_create_mapping_type);
-#ifdef CONFIG_ACPI -unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq, - unsigned int type) -{ - struct irq_domain *domain; - unsigned int virq; - - domain = irq_default_domain; - if (!domain) { - pr_warn("no irq domain found !\n"); - return 0; - } - - /* Create mapping */ - virq = irq_create_mapping(domain, hwirq); - if (!virq) - return virq; - - /* Set type if specified and different than the current one */ - if (type != IRQ_TYPE_NONE && - type != irq_get_trigger_type(virq)) - irq_set_irq_type(virq, type); - return virq; -} -EXPORT_SYMBOL_GPL(irq_create_acpi_mapping); -#endif - /** * irq_dispose_mapping() - Unmap an interrupt * @virq: linux irq number of the interrupt to unmap
Hi,
Since I still working on multi GICs domain patch set, I am sending that I have right now.
Tomasz
On 12/12/13 17: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;- } 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)
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,
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;- } 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)
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?
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;- } 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)
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.
Regards, Tomasz
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
On 12/13/13 11:06, Hanjun Guo wrote:
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.
You're right, I assumed here that setting domain = NULL, would force irq_create_mapping to set default domain automaticly. Now I see NULL reference case here. Setting domain to irq_default_domain here is not so simple as well since irq_default_domain is static variable. That would require to add next function like irq_get_default_domain(). I will fix this.
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
irq_create_acpi_mapping is not ACPI specific and assumes to use default domain. By passing NULL domain to irq_create_mapping we can obtain the same result.
Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org --- drivers/acpi/plat/arm-core.c | 15 ++++++++++++--- include/linux/acpi.h | 2 -- kernel/irq/irqdomain.c | 27 --------------------------- 3 files changed, 12 insertions(+), 32 deletions(-)
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 509b847..1c5cca7 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -453,7 +453,7 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq); */ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) { - unsigned int irq; + unsigned int virq; unsigned int irq_type;
/* @@ -484,8 +484,17 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) else irq_type = IRQ_TYPE_NONE;
- irq = irq_create_acpi_mapping(gsi, irq_type); - return irq; + /* Create mapping, use default domain */ + virq = irq_create_mapping(NULL, gsi); + if (!virq) + return virq; + + /* Set type if specified and different than the current one */ + if (irq_type != IRQ_TYPE_NONE && + irq_type != irq_get_trigger_type(virq)) + irq_set_irq_type(virq, irq_type); + + return virq; } EXPORT_SYMBOL_GPL(acpi_register_gsi);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index aeda856..a2dbe67 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -136,8 +136,6 @@ int acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu); int acpi_unmap_lsapic(int cpu); #endif /* CONFIG_ACPI_HOTPLUG_CPU */
-unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq, - unsigned int type); int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base); int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base); void acpi_irq_stats_init(void); diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index c661552..cf68bb3 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -501,33 +501,6 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data) } EXPORT_SYMBOL_GPL(irq_create_of_mapping);
-#ifdef CONFIG_ACPI -unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq, - unsigned int type) -{ - struct irq_domain *domain; - unsigned int virq; - - domain = irq_default_domain; - if (!domain) { - pr_warn("no irq domain found !\n"); - return 0; - } - - /* Create mapping */ - virq = irq_create_mapping(domain, hwirq); - if (!virq) - return virq; - - /* Set type if specified and different than the current one */ - if (type != IRQ_TYPE_NONE && - type != irq_get_trigger_type(virq)) - irq_set_irq_type(virq, type); - return virq; -} -EXPORT_SYMBOL_GPL(irq_create_acpi_mapping); -#endif - /** * irq_dispose_mapping() - Unmap an interrupt * @virq: linux irq number of the interrupt to unmap
-
Hanjun Guo -
Tomasz Nowicki