Re: [PATCH V3 06/10] irqchip, GICv3, ITS: Refator ITS dt init code to prepare for ACPI.

[Hanjun Guo]

On 2016/2/10 18:47, Marc Zyngier wrote:

On 19/01/16 13:11, Tomasz Nowicki wrote:

Similarly to GICv3 core, we need to extract common code before adding
ACPI support. No functional changes.

Signed-off-by: Hanjun Guo <hanjun....@linaro.org>
Signed-off-by: Tomasz Nowicki <t...@semihalf.com>
---
  drivers/irqchip/irq-gic-v3-its.c   | 82 +++++++++++++++++++++++---------------
  drivers/irqchip/irq-gic-v3.c       |  6 +--
  include/linux/irqchip/arm-gic-v3.h |  2 +-
  3 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 2bbed18..fecb7a6 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -813,7 +813,7 @@ static void its_free_tables(struct its_node *its)
        }
  }

-static int its_alloc_tables(const char *node_name, struct its_node *its)
+static int its_alloc_tables(struct its_node *its)
  {
        int err;
        int i;
@@ -868,8 +868,8 @@ static int its_alloc_tables(const char *node_name, struct 
its_node *its)
                                    order);
                        if (order >= MAX_ORDER) {
                                order = MAX_ORDER - 1;
-                               pr_warn("%s: Device Table too large, reduce its page 
order to %u\n",
-                                       node_name, order);
+                               pr_warn("ITS@0x%lx: Device Table too large, reduce 
its page order to %u\n",
+                                       its->phys_base, order);
                        }
                }

@@ -878,8 +878,8 @@ static int its_alloc_tables(const char *node_name, struct 
its_node *its)
                if (alloc_pages > GITS_BASER_PAGES_MAX) {
                        alloc_pages = GITS_BASER_PAGES_MAX;
                        order = get_order(GITS_BASER_PAGES_MAX * psz);
-                       pr_warn("%s: Device Table too large, reduce its page order 
to %u (%u pages)\n",
-                               node_name, order, alloc_pages);
+                       pr_warn("ITS@0x%lx: Device Table too large, reduce its page 
order to %u (%u pages)\n",
+                               its->phys_base, order, alloc_pages);
                }

                base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
@@ -948,8 +948,8 @@ retry_baser:
                }

                if (val != tmp) {
-                       pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n",
-                              node_name, i,
+                       pr_err("ITS@0x%lx: GITS_BASER%d doesn't stick: %lx 
%lx\n",
+                              its->phys_base, i,
                               (unsigned long) val, (unsigned long) tmp);
                        err = -ENXIO;
                        goto out_free;
@@ -1424,10 +1424,11 @@ static void its_enable_quirks(struct its_node *its)
        gic_enable_quirks(iidr, its_quirks, its);
  }

-static int __init its_probe(struct device_node *node,
-                           struct irq_domain *parent)
+static int __init its_probe_one(phys_addr_t phys_base, unsigned long size,
+                               struct irq_domain *parent,
+                               bool is_msi_controller,

I really question the fact that you are keeping this msi_controller
thing. Let's face it: if this is not an MSI controller, then the whole
thing is absolutely pointless.

So I'd rather you simplify the whole in a separate patch, and just don't
bother initializing the ITS if it cannot be used for MSIs.

Agree, that will simplify the code a lot.

Thanks
Hanjun