Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-22 Thread Suravee Suthikulanit 

On 12/17/2015 10:57 AM, Bjorn Helgaas wrote:

On Wed, Dec 16, 2015 at 06:23:49PM -0600, Suravee Suthikulanit wrote:

Hi Bjorn,

Thanks for your review. Please see my comments below.

On 12/16/2015 4:12 PM, Bjorn Helgaas wrote:

On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:

This patch replaces the struct device_node with struct fwnode_handle
since this structure is common between DT and ACPI.

It also refactors gicv2m_init_one() to prepare for ACPI support.
The only functional change is removing the node name from pr_info.

Reviewed-by: Marc Zyngier 
Signed-off-by: Suravee Suthikulpanit 



@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
*node,
}

list_add_tail(>entry, _nodes);
-   pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
-   (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
-   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));

+   pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
+   (unsigned long)res->start, (unsigned long)res->end,
+   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));


You didn't change this, but I don't think this message has enough
context.  It's pretty cryptic all by itself.  It'd be nice if it could
at least include a device name, e.g., if you could use dev_info().


Here is the example of the information printed:
[0.00] GICv2m: range[0xe118:0xe1181000], SPI[64:320]

Basically, the v2m is just an extension of the GIC. Here, we are
printing the memory range that it is covering, which can be used to
identify different V2m frame and the associate interrupt range
(SPI). The node name is not really providing any values. So, we are
removing it.


I noticed the pr_fmt definition later; that adds some useful context I
didn't know about.  I guess there's no struct device for the GIC?  I
don't see one in struct device_node.  Seems like this piece of
hardware that apparently responds to a memory range *could* have a
struct device,but I'm a little fuzzy on how we handle ACPI and OF
device descriptions in that regard.


For DT, v2m is advertised as a sub-node inside GIC. So, both of them has 
the struct device_node references. IIUC, GIC node is match as irqchip, 
and not as a traditional platform bus device.


Similarly, for ACPI, v2m is advertised as a sub-table inside MADT, and 
we are using the fwnode_handle to reference to.



I hadn't noticed the memory range part; maybe you could use %pR there?


I guess we could have :) I can send a separate patch to clean this up.


Just to double-check, there's no off-by-one error in the SPI range, is
there?  The pattern I usually expect is "start, start + nr_items - 1".


In that case, this should have been [64:319]. I'll send a small patch to 
clean this up.



I'm just kibbitzing here; this isn't PCI code, and you don't need my
ack, so just consider these as random observations.

Bjorn



Thanks for sharing your observation. It's always been good ones :)

Suravee
--
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/

Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-22 Thread Suravee Suthikulanit 

On 12/17/2015 10:57 AM, Bjorn Helgaas wrote:

On Wed, Dec 16, 2015 at 06:23:49PM -0600, Suravee Suthikulanit wrote:

Hi Bjorn,

Thanks for your review. Please see my comments below.

On 12/16/2015 4:12 PM, Bjorn Helgaas wrote:

On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:

This patch replaces the struct device_node with struct fwnode_handle
since this structure is common between DT and ACPI.

It also refactors gicv2m_init_one() to prepare for ACPI support.
The only functional change is removing the node name from pr_info.

Reviewed-by: Marc Zyngier 
Signed-off-by: Suravee Suthikulpanit 



@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
*node,
}

list_add_tail(>entry, _nodes);
-   pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
-   (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
-   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));

+   pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
+   (unsigned long)res->start, (unsigned long)res->end,
+   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));


You didn't change this, but I don't think this message has enough
context.  It's pretty cryptic all by itself.  It'd be nice if it could
at least include a device name, e.g., if you could use dev_info().


Here is the example of the information printed:
[0.00] GICv2m: range[0xe118:0xe1181000], SPI[64:320]

Basically, the v2m is just an extension of the GIC. Here, we are
printing the memory range that it is covering, which can be used to
identify different V2m frame and the associate interrupt range
(SPI). The node name is not really providing any values. So, we are
removing it.


I noticed the pr_fmt definition later; that adds some useful context I
didn't know about.  I guess there's no struct device for the GIC?  I
don't see one in struct device_node.  Seems like this piece of
hardware that apparently responds to a memory range *could* have a
struct device,but I'm a little fuzzy on how we handle ACPI and OF
device descriptions in that regard.


For DT, v2m is advertised as a sub-node inside GIC. So, both of them has 
the struct device_node references. IIUC, GIC node is match as irqchip, 
and not as a traditional platform bus device.


Similarly, for ACPI, v2m is advertised as a sub-table inside MADT, and 
we are using the fwnode_handle to reference to.



I hadn't noticed the memory range part; maybe you could use %pR there?


I guess we could have :) I can send a separate patch to clean this up.


Just to double-check, there's no off-by-one error in the SPI range, is
there?  The pattern I usually expect is "start, start + nr_items - 1".


In that case, this should have been [64:319]. I'll send a small patch to 
clean this up.



I'm just kibbitzing here; this isn't PCI code, and you don't need my
ack, so just consider these as random observations.

Bjorn



Thanks for sharing your observation. It's always been good ones :)

Suravee
--
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/

Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-17 Thread Bjorn Helgaas 
On Wed, Dec 16, 2015 at 06:23:49PM -0600, Suravee Suthikulanit wrote:
> Hi Bjorn,
> 
> Thanks for your review. Please see my comments below.
> 
> On 12/16/2015 4:12 PM, Bjorn Helgaas wrote:
> >On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:
> >>This patch replaces the struct device_node with struct fwnode_handle
> >>since this structure is common between DT and ACPI.
> >>
> >>It also refactors gicv2m_init_one() to prepare for ACPI support.
> >>The only functional change is removing the node name from pr_info.
> >>
> >>Reviewed-by: Marc Zyngier 
> >>Signed-off-by: Suravee Suthikulpanit 
> >
> >>@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
> >>*node,
> >>}
> >>
> >>list_add_tail(>entry, _nodes);
> >>-   pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
> >>-   (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
> >>-   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
> >>
> >>+   pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
> >>+   (unsigned long)res->start, (unsigned long)res->end,
> >>+   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
> >
> >You didn't change this, but I don't think this message has enough
> >context.  It's pretty cryptic all by itself.  It'd be nice if it could
> >at least include a device name, e.g., if you could use dev_info().
> 
> Here is the example of the information printed:
> [0.00] GICv2m: range[0xe118:0xe1181000], SPI[64:320]
> 
> Basically, the v2m is just an extension of the GIC. Here, we are
> printing the memory range that it is covering, which can be used to
> identify different V2m frame and the associate interrupt range
> (SPI). The node name is not really providing any values. So, we are
> removing it.

I noticed the pr_fmt definition later; that adds some useful context I
didn't know about.  I guess there's no struct device for the GIC?  I
don't see one in struct device_node.  Seems like this piece of
hardware that apparently responds to a memory range *could* have a
struct device, but I'm a little fuzzy on how we handle ACPI and OF
device descriptions in that regard.

I hadn't noticed the memory range part; maybe you could use %pR there?

Just to double-check, there's no off-by-one error in the SPI range, is
there?  The pattern I usually expect is "start, start + nr_items - 1".

I'm just kibbitzing here; this isn't PCI code, and you don't need my
ack, so just consider these as random observations.

Bjorn
--
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/

Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-17 Thread Bjorn Helgaas 
On Wed, Dec 16, 2015 at 06:23:49PM -0600, Suravee Suthikulanit wrote:
> Hi Bjorn,
> 
> Thanks for your review. Please see my comments below.
> 
> On 12/16/2015 4:12 PM, Bjorn Helgaas wrote:
> >On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:
> >>This patch replaces the struct device_node with struct fwnode_handle
> >>since this structure is common between DT and ACPI.
> >>
> >>It also refactors gicv2m_init_one() to prepare for ACPI support.
> >>The only functional change is removing the node name from pr_info.
> >>
> >>Reviewed-by: Marc Zyngier 
> >>Signed-off-by: Suravee Suthikulpanit 
> >
> >>@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
> >>*node,
> >>}
> >>
> >>list_add_tail(>entry, _nodes);
> >>-   pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
> >>-   (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
> >>-   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
> >>
> >>+   pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
> >>+   (unsigned long)res->start, (unsigned long)res->end,
> >>+   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
> >
> >You didn't change this, but I don't think this message has enough
> >context.  It's pretty cryptic all by itself.  It'd be nice if it could
> >at least include a device name, e.g., if you could use dev_info().
> 
> Here is the example of the information printed:
> [0.00] GICv2m: range[0xe118:0xe1181000], SPI[64:320]
> 
> Basically, the v2m is just an extension of the GIC. Here, we are
> printing the memory range that it is covering, which can be used to
> identify different V2m frame and the associate interrupt range
> (SPI). The node name is not really providing any values. So, we are
> removing it.

I noticed the pr_fmt definition later; that adds some useful context I
didn't know about.  I guess there's no struct device for the GIC?  I
don't see one in struct device_node.  Seems like this piece of
hardware that apparently responds to a memory range *could* have a
struct device, but I'm a little fuzzy on how we handle ACPI and OF
device descriptions in that regard.

I hadn't noticed the memory range part; maybe you could use %pR there?

Just to double-check, there's no off-by-one error in the SPI range, is
there?  The pattern I usually expect is "start, start + nr_items - 1".

I'm just kibbitzing here; this isn't PCI code, and you don't need my
ack, so just consider these as random observations.

Bjorn
--
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/

Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-16 Thread Suravee Suthikulanit 

Hi Bjorn,

Thanks for your review. Please see my comments below.

On 12/16/2015 4:12 PM, Bjorn Helgaas wrote:

On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:

This patch replaces the struct device_node with struct fwnode_handle
since this structure is common between DT and ACPI.

It also refactors gicv2m_init_one() to prepare for ACPI support.
The only functional change is removing the node name from pr_info.

Reviewed-by: Marc Zyngier 
Signed-off-by: Suravee Suthikulpanit 



@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
*node,
}

list_add_tail(>entry, _nodes);
-   pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
-   (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
-   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));

+   pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
+   (unsigned long)res->start, (unsigned long)res->end,
+   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));


You didn't change this, but I don't think this message has enough
context.  It's pretty cryptic all by itself.  It'd be nice if it could
at least include a device name, e.g., if you could use dev_info().


Here is the example of the information printed:
[0.00] GICv2m: range[0xe118:0xe1181000], SPI[64:320]

Basically, the v2m is just an extension of the GIC. Here, we are 
printing the memory range that it is covering, which can be used to 
identify different V2m frame and the associate interrupt range (SPI). 
The node name is not really providing any values. So, we are removing it.


Thanks,
Suravee


If that's possible, I guess a separate patch would be appropriate,
since there are other similar things in this file.
j
Bjorn




--
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/

Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-16 Thread Bjorn Helgaas 
On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:
> This patch replaces the struct device_node with struct fwnode_handle
> since this structure is common between DT and ACPI.
> 
> It also refactors gicv2m_init_one() to prepare for ACPI support.
> The only functional change is removing the node name from pr_info.
> 
> Reviewed-by: Marc Zyngier 
> Signed-off-by: Suravee Suthikulpanit 

> @@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
> *node,
>   }
>  
>   list_add_tail(>entry, _nodes);
> - pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
> - (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
> - v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
>  
> + pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
> + (unsigned long)res->start, (unsigned long)res->end,
> + v2m->spi_start, (v2m->spi_start + v2m->nr_spis));

You didn't change this, but I don't think this message has enough
context.  It's pretty cryptic all by itself.  It'd be nice if it could
at least include a device name, e.g., if you could use dev_info().

If that's possible, I guess a separate patch would be appropriate,
since there are other similar things in this file.

Bjorn
--
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/

Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-16 Thread Bjorn Helgaas 
On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:
> This patch replaces the struct device_node with struct fwnode_handle
> since this structure is common between DT and ACPI.
> 
> It also refactors gicv2m_init_one() to prepare for ACPI support.
> The only functional change is removing the node name from pr_info.
> 
> Reviewed-by: Marc Zyngier 
> Signed-off-by: Suravee Suthikulpanit 

> @@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
> *node,
>   }
>  
>   list_add_tail(>entry, _nodes);
> - pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
> - (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
> - v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
>  
> + pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
> + (unsigned long)res->start, (unsigned long)res->end,
> + v2m->spi_start, (v2m->spi_start + v2m->nr_spis));

You didn't change this, but I don't think this message has enough
context.  It's pretty cryptic all by itself.  It'd be nice if it could
at least include a device name, e.g., if you could use dev_info().

If that's possible, I guess a separate patch would be appropriate,
since there are other similar things in this file.

Bjorn
--
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/

Re: [PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-16 Thread Suravee Suthikulanit 

Hi Bjorn,

Thanks for your review. Please see my comments below.

On 12/16/2015 4:12 PM, Bjorn Helgaas wrote:

On Thu, Dec 10, 2015 at 08:55:29AM -0800, Suravee Suthikulpanit wrote:

This patch replaces the struct device_node with struct fwnode_handle
since this structure is common between DT and ACPI.

It also refactors gicv2m_init_one() to prepare for ACPI support.
The only functional change is removing the node name from pr_info.

Reviewed-by: Marc Zyngier 
Signed-off-by: Suravee Suthikulpanit 



@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
*node,
}

list_add_tail(>entry, _nodes);
-   pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
-   (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
-   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));

+   pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
+   (unsigned long)res->start, (unsigned long)res->end,
+   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));


You didn't change this, but I don't think this message has enough
context.  It's pretty cryptic all by itself.  It'd be nice if it could
at least include a device name, e.g., if you could use dev_info().


Here is the example of the information printed:
[0.00] GICv2m: range[0xe118:0xe1181000], SPI[64:320]

Basically, the v2m is just an extension of the GIC. Here, we are 
printing the memory range that it is covering, which can be used to 
identify different V2m frame and the associate interrupt range (SPI). 
The node name is not really providing any values. So, we are removing it.


Thanks,
Suravee


If that's possible, I guess a separate patch would be appropriate,
since there are other similar things in this file.
j
Bjorn




--
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/

[PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-10 Thread Suravee Suthikulpanit 
This patch replaces the struct device_node with struct fwnode_handle
since this structure is common between DT and ACPI.

It also refactors gicv2m_init_one() to prepare for ACPI support.
The only functional change is removing the node name from pr_info.

Reviewed-by: Marc Zyngier 
Signed-off-by: Suravee Suthikulpanit 
---
 drivers/irqchip/irq-gic-v2m.c | 53 ++-
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 87f8d10..779c390 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -55,7 +55,7 @@ static DEFINE_SPINLOCK(v2m_lock);
 
 struct v2m_data {
struct list_head entry;
-   struct device_node *node;
+   struct fwnode_handle *fwnode;
struct resource res;/* GICv2m resource */
void __iomem *base; /* GICv2m virt address */
u32 spi_start;  /* The SPI number that MSIs start */
@@ -254,7 +254,7 @@ static void gicv2m_teardown(void)
list_del(>entry);
kfree(v2m->bm);
iounmap(v2m->base);
-   of_node_put(v2m->node);
+   of_node_put(to_of_node(v2m->fwnode));
kfree(v2m);
}
 }
@@ -268,7 +268,7 @@ static int gicv2m_allocate_domains(struct irq_domain 
*parent)
if (!v2m)
return 0;
 
-   inner_domain = irq_domain_create_tree(of_node_to_fwnode(v2m->node),
+   inner_domain = irq_domain_create_tree(v2m->fwnode,
  _domain_ops, v2m);
if (!inner_domain) {
pr_err("Failed to create GICv2m domain\n");
@@ -277,10 +277,10 @@ static int gicv2m_allocate_domains(struct irq_domain 
*parent)
 
inner_domain->bus_token = DOMAIN_BUS_NEXUS;
inner_domain->parent = parent;
-   pci_domain = pci_msi_create_irq_domain(of_node_to_fwnode(v2m->node),
+   pci_domain = pci_msi_create_irq_domain(v2m->fwnode,
   _msi_domain_info,
   inner_domain);
-   plat_domain = 
platform_msi_create_irq_domain(of_node_to_fwnode(v2m->node),
+   plat_domain = platform_msi_create_irq_domain(v2m->fwnode,
 _pmsi_domain_info,
 inner_domain);
if (!pci_domain || !plat_domain) {
@@ -296,8 +296,9 @@ static int gicv2m_allocate_domains(struct irq_domain 
*parent)
return 0;
 }
 
-static int __init gicv2m_init_one(struct device_node *node,
- struct irq_domain *parent)
+static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
+ u32 spi_start, u32 nr_spis,
+ struct resource *res)
 {
int ret;
struct v2m_data *v2m;
@@ -309,13 +310,9 @@ static int __init gicv2m_init_one(struct device_node *node,
}
 
INIT_LIST_HEAD(>entry);
-   v2m->node = node;
+   v2m->fwnode = fwnode;
 
-   ret = of_address_to_resource(node, 0, >res);
-   if (ret) {
-   pr_err("Failed to allocate v2m resource.\n");
-   goto err_free_v2m;
-   }
+   memcpy(>res, res, sizeof(struct resource));
 
v2m->base = ioremap(v2m->res.start, resource_size(>res));
if (!v2m->base) {
@@ -324,10 +321,9 @@ static int __init gicv2m_init_one(struct device_node *node,
goto err_free_v2m;
}
 
-   if (!of_property_read_u32(node, "arm,msi-base-spi", >spi_start) &&
-   !of_property_read_u32(node, "arm,msi-num-spis", >nr_spis)) {
-   pr_info("Overriding V2M MSI_TYPER (base:%u, num:%u)\n",
-   v2m->spi_start, v2m->nr_spis);
+   if (spi_start && nr_spis) {
+   v2m->spi_start = spi_start;
+   v2m->nr_spis = nr_spis;
} else {
u32 typer = readl_relaxed(v2m->base + V2M_MSI_TYPER);
 
@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
*node,
}
 
list_add_tail(>entry, _nodes);
-   pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
-   (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
-   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
 
+   pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
+   (unsigned long)res->start, (unsigned long)res->end,
+   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
return 0;
 
 err_iounmap:
@@ -384,10 +380,25 @@ int __init gicv2m_of_init(struct device_node *node, 
struct irq_domain *parent)
 
for (child = of_find_matching_node(node, gicv2m_device_id); child;
 child = of_find_matching_node(child, gicv2m_device_id)) {
+   u32 spi_start = 0, nr_spis = 0;
+   struct resource res;
+
if

[PATCH v7 3/4] gicv2m: Refactor to prepare for ACPI support

2015-12-10 Thread Suravee Suthikulpanit 
This patch replaces the struct device_node with struct fwnode_handle
since this structure is common between DT and ACPI.

It also refactors gicv2m_init_one() to prepare for ACPI support.
The only functional change is removing the node name from pr_info.

Reviewed-by: Marc Zyngier 
Signed-off-by: Suravee Suthikulpanit 
---
 drivers/irqchip/irq-gic-v2m.c | 53 ++-
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 87f8d10..779c390 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -55,7 +55,7 @@ static DEFINE_SPINLOCK(v2m_lock);
 
 struct v2m_data {
struct list_head entry;
-   struct device_node *node;
+   struct fwnode_handle *fwnode;
struct resource res;/* GICv2m resource */
void __iomem *base; /* GICv2m virt address */
u32 spi_start;  /* The SPI number that MSIs start */
@@ -254,7 +254,7 @@ static void gicv2m_teardown(void)
list_del(>entry);
kfree(v2m->bm);
iounmap(v2m->base);
-   of_node_put(v2m->node);
+   of_node_put(to_of_node(v2m->fwnode));
kfree(v2m);
}
 }
@@ -268,7 +268,7 @@ static int gicv2m_allocate_domains(struct irq_domain 
*parent)
if (!v2m)
return 0;
 
-   inner_domain = irq_domain_create_tree(of_node_to_fwnode(v2m->node),
+   inner_domain = irq_domain_create_tree(v2m->fwnode,
  _domain_ops, v2m);
if (!inner_domain) {
pr_err("Failed to create GICv2m domain\n");
@@ -277,10 +277,10 @@ static int gicv2m_allocate_domains(struct irq_domain 
*parent)
 
inner_domain->bus_token = DOMAIN_BUS_NEXUS;
inner_domain->parent = parent;
-   pci_domain = pci_msi_create_irq_domain(of_node_to_fwnode(v2m->node),
+   pci_domain = pci_msi_create_irq_domain(v2m->fwnode,
   _msi_domain_info,
   inner_domain);
-   plat_domain = 
platform_msi_create_irq_domain(of_node_to_fwnode(v2m->node),
+   plat_domain = platform_msi_create_irq_domain(v2m->fwnode,
 _pmsi_domain_info,
 inner_domain);
if (!pci_domain || !plat_domain) {
@@ -296,8 +296,9 @@ static int gicv2m_allocate_domains(struct irq_domain 
*parent)
return 0;
 }
 
-static int __init gicv2m_init_one(struct device_node *node,
- struct irq_domain *parent)
+static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
+ u32 spi_start, u32 nr_spis,
+ struct resource *res)
 {
int ret;
struct v2m_data *v2m;
@@ -309,13 +310,9 @@ static int __init gicv2m_init_one(struct device_node *node,
}
 
INIT_LIST_HEAD(>entry);
-   v2m->node = node;
+   v2m->fwnode = fwnode;
 
-   ret = of_address_to_resource(node, 0, >res);
-   if (ret) {
-   pr_err("Failed to allocate v2m resource.\n");
-   goto err_free_v2m;
-   }
+   memcpy(>res, res, sizeof(struct resource));
 
v2m->base = ioremap(v2m->res.start, resource_size(>res));
if (!v2m->base) {
@@ -324,10 +321,9 @@ static int __init gicv2m_init_one(struct device_node *node,
goto err_free_v2m;
}
 
-   if (!of_property_read_u32(node, "arm,msi-base-spi", >spi_start) &&
-   !of_property_read_u32(node, "arm,msi-num-spis", >nr_spis)) {
-   pr_info("Overriding V2M MSI_TYPER (base:%u, num:%u)\n",
-   v2m->spi_start, v2m->nr_spis);
+   if (spi_start && nr_spis) {
+   v2m->spi_start = spi_start;
+   v2m->nr_spis = nr_spis;
} else {
u32 typer = readl_relaxed(v2m->base + V2M_MSI_TYPER);
 
@@ -359,10 +355,10 @@ static int __init gicv2m_init_one(struct device_node 
*node,
}
 
list_add_tail(>entry, _nodes);
-   pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
-   (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
-   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
 
+   pr_info("range[%#lx:%#lx], SPI[%d:%d]\n",
+   (unsigned long)res->start, (unsigned long)res->end,
+   v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
return 0;
 
 err_iounmap:
@@ -384,10 +380,25 @@ int __init gicv2m_of_init(struct device_node *node, 
struct irq_domain *parent)
 
for (child = of_find_matching_node(node, gicv2m_device_id); child;
 child = of_find_matching_node(child, gicv2m_device_id)) {
+   u32 spi_start = 0, nr_spis = 0;
+   struct