[PATCH 1/2] genirq: Update stale comment for __irq_domain_add

[PATCH 1/2] genirq: Update stale comment for __irq_domain_add

[Punit Agrawal]

Commit 1bf4ddc46c5d ("irqdomain: Introduce irq_domain_create_{linear,
tree}") introduced the use of fwnode_handle to identify the interrupt
controller when calling __irq_domain_add but missed updating the kernel
doc parameters for the function.

Update this comment. While we are touching this code, also consolidate
the declaration and assignment of of_node.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 kernel/irq/irqdomain.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 8798b6c..1fe2fea 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -80,7 +80,7 @@ EXPORT_SYMBOL_GPL(irq_domain_free_fwnode);
 
 /**
  * __irq_domain_add() - Allocate a new irq_domain data structure
- * @of_node: optional device-tree node of the interrupt controller
+ * @fwnode: firmware node for the interrupt controller
  * @size: Size of linear map; 0 for radix mapping only
  * @hwirq_max: Maximum number of interrupts supported by controller
  * @direct_max: Maximum value of direct maps; Use ~0 for no limit; 0 for no
@@ -97,9 +97,7 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
 				    void *host_data)
 {
 	struct irq_domain *domain;
-	struct device_node *of_node;
-
-	of_node = to_of_node(fwnode);
+	struct device_node *of_node = to_of_node(fwnode);
 
 	domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
 			      GFP_KERNEL, of_node_to_nid(of_node));
-- 
2.8.0.rc3

[PATCH 2/2] genirq: Use firmware identifier while adding domain

[Punit Agrawal]

Use the firmware provided identifier for the domain name.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 kernel/irq/irqdomain.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 1fe2fea..3af09e1 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -114,6 +114,10 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
 	domain->hwirq_max = hwirq_max;
 	domain->revmap_size = size;
 	domain->revmap_direct_max_irq = direct_max;
+	if (is_fwnode_irqchip(fwnode))
+		domain->name = container_of(fwnode, struct irqchip_fwid, fwnode)->name;
+	else
+		domain->name = of_node_full_name(of_node);
 	irq_domain_check_hierarchy(domain);
 
 	mutex_lock(&irq_domain_mutex);
-- 
2.8.0.rc3

Re: [PATCH 2/2] genirq: Use firmware identifier while adding domain

[Marc Zyngier]

On 31/05/16 13:56, Punit Agrawal wrote:
> Use the firmware provided identifier for the domain name.
> 
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
>  kernel/irq/irqdomain.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 1fe2fea..3af09e1 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -114,6 +114,10 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
>  	domain->hwirq_max = hwirq_max;
>  	domain->revmap_size = size;
>  	domain->revmap_direct_max_irq = direct_max;
> +	if (is_fwnode_irqchip(fwnode))
> +		domain->name = container_of(fwnode, struct irqchip_fwid, fwnode)->name;

So this thing worries me to no end. Look at the way irqchip_fwid is
constructed:

	name = kasprintf(GFP_KERNEL, "irqchip@%p", data);

You'd end-up disclosing a kernel address, which is not a very good idea.

> +	else
> +		domain->name = of_node_full_name(of_node);

And what if the node gets pruned (as it can happen on OpenFirmware
implementations)?

>  	irq_domain_check_hierarchy(domain);
>  
>  	mutex_lock(&irq_domain_mutex);
> 

Thanks,

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

Re: [PATCH 1/2] genirq: Update stale comment for __irq_domain_add

[Marc Zyngier]

[this has been languishing in my Inbox for months - thanks tglx for the
heads up]

On 31/05/16 13:56, Punit Agrawal wrote:
> Commit 1bf4ddc46c5d ("irqdomain: Introduce irq_domain_create_{linear,
> tree}") introduced the use of fwnode_handle to identify the interrupt
> controller when calling __irq_domain_add but missed updating the kernel
> doc parameters for the function.
> 
> Update this comment. While we are touching this code, also consolidate
> the declaration and assignment of of_node.
> 
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

[tip:irq/core] genirq: Update stale comment for __irq_domain_add

[tip-bot for Punit Agrawal]

Commit-ID:  545d5d657b720e9c4dc773265bb7e9d88e34b269
Gitweb:     http://git.kernel.org/tip/545d5d657b720e9c4dc773265bb7e9d88e34b269
Author:     Punit Agrawal <punit.agrawal@arm.com>
AuthorDate: Tue, 31 May 2016 13:56:48 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 5 Sep 2016 17:11:10 +0200

genirq: Update stale comment for __irq_domain_add

Commit 1bf4ddc46c5d ("irqdomain: Introduce irq_domain_create_{linear,
tree}") introduced the use of fwnode_handle to identify the interrupt
controller when calling __irq_domain_add but missed updating the kernel
doc parameters for the function.

Update this comment. While we are touching this code, also consolidate
the declaration and assignment of of_node.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Marc Zygnier <marc.zyngier@arm.com>
Link: http://lkml.kernel.org/r/1464699409-23113-1-git-send-email-punit.agrawal@arm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 kernel/irq/irqdomain.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index f10cffe..8c0a0ae 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -80,7 +80,7 @@ EXPORT_SYMBOL_GPL(irq_domain_free_fwnode);
 
 /**
  * __irq_domain_add() - Allocate a new irq_domain data structure
- * @of_node: optional device-tree node of the interrupt controller
+ * @fwnode: firmware node for the interrupt controller
  * @size: Size of linear map; 0 for radix mapping only
  * @hwirq_max: Maximum number of interrupts supported by controller
  * @direct_max: Maximum value of direct maps; Use ~0 for no limit; 0 for no
@@ -96,10 +96,8 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
 				    const struct irq_domain_ops *ops,
 				    void *host_data)
 {
+	struct device_node *of_node = to_of_node(fwnode);
 	struct irq_domain *domain;
-	struct device_node *of_node;
-
-	of_node = to_of_node(fwnode);
 
 	domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
 			      GFP_KERNEL, of_node_to_nid(of_node));