Re: [PATCH 07/10] irqchip/gic-v3-its: Split probing from its node initialization
On 08.11.18 11:25:24, Julien Thierry wrote:
> On 07/11/18 22:03, Robert Richter wrote:
> >-static int its_init_domain(struct fwnode_handle *handle, struct its_node
> >*its)
> >+static int its_init_domain(struct its_node *its)
> > {
> > struct irq_domain *inner_domain;
> > struct msi_domain_info *info;
> >@@ -3384,7 +3385,8 @@ static int its_init_domain(struct fwnode_handle
> >*handle, struct its_node *its)
> > if (!info)
> > return -ENOMEM;
> >
> >- inner_domain = irq_domain_create_tree(handle, _domain_ops, its);
> >+ inner_domain = irq_domain_create_tree(its->fwnode_handle,
> >+ _domain_ops, its);
>
> Separate change?
>
> > if (!inner_domain) {
> > kfree(info);
> > return -ENOMEM;
> >@@ -3441,8 +3443,7 @@ static int its_init_vpe_domain(void)
> > return 0;
> > }
> >
> >-static int __init its_compute_its_list_map(struct resource *res,
> >-void __iomem *its_base)
> >+static int __init its_compute_its_list_map(struct its_node *its)
> > {
> > int its_number;
> > u32 ctlr;
> >@@ -3456,15 +3457,15 @@ static int __init its_compute_its_list_map(struct
> >resource *res,
> > its_number = find_first_zero_bit(_list_map, GICv4_ITS_LIST_MAX);
> > if (its_number >= GICv4_ITS_LIST_MAX) {
> > pr_err("ITS@%pa: No ITSList entry available!\n",
> >->start);
> >+>phys_base);
> > return -EINVAL;
> > }
> >
> >- ctlr = readl_relaxed(its_base + GITS_CTLR);
> >+ ctlr = readl_relaxed(its->base + GITS_CTLR);
> > ctlr &= ~GITS_CTLR_ITS_NUMBER;
> > ctlr |= its_number << GITS_CTLR_ITS_NUMBER_SHIFT;
> >- writel_relaxed(ctlr, its_base + GITS_CTLR);
> >- ctlr = readl_relaxed(its_base + GITS_CTLR);
> >+ writel_relaxed(ctlr, its->base + GITS_CTLR);
> >+ ctlr = readl_relaxed(its->base + GITS_CTLR);
>
> This (removal of its_base parameter) also feel like a separate change.
In a separate change the motivation of the change would not be
obvious. While the change of the variable itself is trivial from the
perspective of review and testing, I decided to keep it in the context
of the overall change of this patch.
>
> Also, I would define a local variable its_base to avoid dereferencing
> its every time in order to get the base address.
Hmm, there is not much difference in reading the code then, while the
use of a local variable just adds more code without benefit. The
compiler does not care as the value is probably stored in a register
anyway. There are also other struct members, should all of them being
mirrored in a local variable?
>
> > if ((ctlr & GITS_CTLR_ITS_NUMBER) != (its_number <<
> > GITS_CTLR_ITS_NUMBER_SHIFT)) {
> > its_number = ctlr & GITS_CTLR_ITS_NUMBER;
> > its_number >>= GITS_CTLR_ITS_NUMBER_SHIFT;
> >@@ -3472,83 +3473,110 @@ static int __init its_compute_its_list_map(struct
> >resource *res,
> >
> > if (test_and_set_bit(its_number, _list_map)) {
> > pr_err("ITS@%pa: Duplicate ITSList entry %d\n",
> >->start, its_number);
> >+>phys_base, its_number);
> > return -EINVAL;
> > }
> >
> > return its_number;
> > }
> >
> >+static void its_free(struct its_node *its)
> >+{
> >+ raw_spin_lock(_lock);
> >+ list_del(>entry);
> >+ raw_spin_unlock(_lock);
> >+
> >+ kfree(its);
> >+}
> >+
> >+static int __init its_init_one(struct its_node *its);
>
> You might as well define its_init_one here, no?
This is an intermediate definition that will be removed in a later
patch. Moving the whole code here would make the change less readable.
>
> >+
> > static int __init its_probe_one(struct resource *res,
> > struct fwnode_handle *handle, int numa_node)
> > {
> > struct its_node *its;
> >+ int err;
> >+
> >+ its = kzalloc(sizeof(*its), GFP_KERNEL);
> >+ if (!its)
> >+ return -ENOMEM;
> >+
> >+ raw_spin_lock_init(>lock);
> >+ INIT_LIST_HEAD(>entry);
> >+ INIT_LIST_HEAD(>its_device_list);
> >+ its->fwnode_handle = handle;
> >+ its->phys_base = res->start;
> >+ its->phys_size = resource_size(res);
> >+ its->numa_node = numa_node;
> >+
> >+ raw_spin_lock(_lock);
> >+ list_add_tail(>entry, _nodes);
> >+ raw_spin_unlock(_lock);
> >+
> >+ pr_info("ITS %pR\n", res);
> >+
> >+ err = its_init_one(its);
> >+ if (err)
> >+ its_free(its);
> >+
> >+ return err;
> >+}
> >+
> >+static int __init its_init_one(struct its_node *its)
> >+{
> > void __iomem *its_base;
> > u32 val, ctlr;
> > u64 baser, tmp, typer;
> > int err;
> >
> >- its_base = ioremap(res->start, resource_size(res));
> >+ its_base = ioremap(its->phys_base, its->phys_size);
> > if (!its_base) {
> >- pr_warn("ITS@%pa:
Re: [PATCH 07/10] irqchip/gic-v3-its: Split probing from its node initialization
On 08.11.18 11:25:24, Julien Thierry wrote:
> On 07/11/18 22:03, Robert Richter wrote:
> >-static int its_init_domain(struct fwnode_handle *handle, struct its_node
> >*its)
> >+static int its_init_domain(struct its_node *its)
> > {
> > struct irq_domain *inner_domain;
> > struct msi_domain_info *info;
> >@@ -3384,7 +3385,8 @@ static int its_init_domain(struct fwnode_handle
> >*handle, struct its_node *its)
> > if (!info)
> > return -ENOMEM;
> >
> >- inner_domain = irq_domain_create_tree(handle, _domain_ops, its);
> >+ inner_domain = irq_domain_create_tree(its->fwnode_handle,
> >+ _domain_ops, its);
>
> Separate change?
>
> > if (!inner_domain) {
> > kfree(info);
> > return -ENOMEM;
> >@@ -3441,8 +3443,7 @@ static int its_init_vpe_domain(void)
> > return 0;
> > }
> >
> >-static int __init its_compute_its_list_map(struct resource *res,
> >-void __iomem *its_base)
> >+static int __init its_compute_its_list_map(struct its_node *its)
> > {
> > int its_number;
> > u32 ctlr;
> >@@ -3456,15 +3457,15 @@ static int __init its_compute_its_list_map(struct
> >resource *res,
> > its_number = find_first_zero_bit(_list_map, GICv4_ITS_LIST_MAX);
> > if (its_number >= GICv4_ITS_LIST_MAX) {
> > pr_err("ITS@%pa: No ITSList entry available!\n",
> >->start);
> >+>phys_base);
> > return -EINVAL;
> > }
> >
> >- ctlr = readl_relaxed(its_base + GITS_CTLR);
> >+ ctlr = readl_relaxed(its->base + GITS_CTLR);
> > ctlr &= ~GITS_CTLR_ITS_NUMBER;
> > ctlr |= its_number << GITS_CTLR_ITS_NUMBER_SHIFT;
> >- writel_relaxed(ctlr, its_base + GITS_CTLR);
> >- ctlr = readl_relaxed(its_base + GITS_CTLR);
> >+ writel_relaxed(ctlr, its->base + GITS_CTLR);
> >+ ctlr = readl_relaxed(its->base + GITS_CTLR);
>
> This (removal of its_base parameter) also feel like a separate change.
In a separate change the motivation of the change would not be
obvious. While the change of the variable itself is trivial from the
perspective of review and testing, I decided to keep it in the context
of the overall change of this patch.
>
> Also, I would define a local variable its_base to avoid dereferencing
> its every time in order to get the base address.
Hmm, there is not much difference in reading the code then, while the
use of a local variable just adds more code without benefit. The
compiler does not care as the value is probably stored in a register
anyway. There are also other struct members, should all of them being
mirrored in a local variable?
>
> > if ((ctlr & GITS_CTLR_ITS_NUMBER) != (its_number <<
> > GITS_CTLR_ITS_NUMBER_SHIFT)) {
> > its_number = ctlr & GITS_CTLR_ITS_NUMBER;
> > its_number >>= GITS_CTLR_ITS_NUMBER_SHIFT;
> >@@ -3472,83 +3473,110 @@ static int __init its_compute_its_list_map(struct
> >resource *res,
> >
> > if (test_and_set_bit(its_number, _list_map)) {
> > pr_err("ITS@%pa: Duplicate ITSList entry %d\n",
> >->start, its_number);
> >+>phys_base, its_number);
> > return -EINVAL;
> > }
> >
> > return its_number;
> > }
> >
> >+static void its_free(struct its_node *its)
> >+{
> >+ raw_spin_lock(_lock);
> >+ list_del(>entry);
> >+ raw_spin_unlock(_lock);
> >+
> >+ kfree(its);
> >+}
> >+
> >+static int __init its_init_one(struct its_node *its);
>
> You might as well define its_init_one here, no?
This is an intermediate definition that will be removed in a later
patch. Moving the whole code here would make the change less readable.
>
> >+
> > static int __init its_probe_one(struct resource *res,
> > struct fwnode_handle *handle, int numa_node)
> > {
> > struct its_node *its;
> >+ int err;
> >+
> >+ its = kzalloc(sizeof(*its), GFP_KERNEL);
> >+ if (!its)
> >+ return -ENOMEM;
> >+
> >+ raw_spin_lock_init(>lock);
> >+ INIT_LIST_HEAD(>entry);
> >+ INIT_LIST_HEAD(>its_device_list);
> >+ its->fwnode_handle = handle;
> >+ its->phys_base = res->start;
> >+ its->phys_size = resource_size(res);
> >+ its->numa_node = numa_node;
> >+
> >+ raw_spin_lock(_lock);
> >+ list_add_tail(>entry, _nodes);
> >+ raw_spin_unlock(_lock);
> >+
> >+ pr_info("ITS %pR\n", res);
> >+
> >+ err = its_init_one(its);
> >+ if (err)
> >+ its_free(its);
> >+
> >+ return err;
> >+}
> >+
> >+static int __init its_init_one(struct its_node *its)
> >+{
> > void __iomem *its_base;
> > u32 val, ctlr;
> > u64 baser, tmp, typer;
> > int err;
> >
> >- its_base = ioremap(res->start, resource_size(res));
> >+ its_base = ioremap(its->phys_base, its->phys_size);
> > if (!its_base) {
> >- pr_warn("ITS@%pa:
Re: [PATCH 07/10] irqchip/gic-v3-its: Split probing from its node initialization
On 07/11/18 22:03, Robert Richter wrote:
To initialize the its nodes at a later point during boot, we need to
split probing from initialization. Collect all information required
for initialization in struct its_node. We can then use the its node
list for initialization.
Signed-off-by: Robert Richter
---
drivers/irqchip/irq-gic-v3-its.c | 135 +++--
drivers/irqchip/irq-gic-v3.c | 2 +-
include/linux/irqchip/arm-gic-v3.h | 4 +-
3 files changed, 87 insertions(+), 54 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4033f71f5181..c28f4158ff70 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -103,6 +103,7 @@ struct its_node {
struct list_headentry;
void __iomem*base;
phys_addr_t phys_base;
+ phys_addr_t phys_size;
struct its_cmd_block*cmd_base;
struct its_cmd_block*cmd_write;
struct its_basertables[GITS_BASER_NR_REGS];
@@ -3375,7 +3376,7 @@ static struct syscore_ops its_syscore_ops = {
.resume = its_restore_enable,
};
-static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
+static int its_init_domain(struct its_node *its)
{
struct irq_domain *inner_domain;
struct msi_domain_info *info;
@@ -3384,7 +3385,8 @@ static int its_init_domain(struct fwnode_handle *handle,
struct its_node *its)
if (!info)
return -ENOMEM;
- inner_domain = irq_domain_create_tree(handle, _domain_ops, its);
+ inner_domain = irq_domain_create_tree(its->fwnode_handle,
+ _domain_ops, its);
Separate change?
if (!inner_domain) {
kfree(info);
return -ENOMEM;
@@ -3441,8 +3443,7 @@ static int its_init_vpe_domain(void)
return 0;
}
-static int __init its_compute_its_list_map(struct resource *res,
- void __iomem *its_base)
+static int __init its_compute_its_list_map(struct its_node *its)
{
int its_number;
u32 ctlr;
@@ -3456,15 +3457,15 @@ static int __init its_compute_its_list_map(struct
resource *res,
its_number = find_first_zero_bit(_list_map, GICv4_ITS_LIST_MAX);
if (its_number >= GICv4_ITS_LIST_MAX) {
pr_err("ITS@%pa: No ITSList entry available!\n",
- >start);
+ >phys_base);
return -EINVAL;
}
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
ctlr &= ~GITS_CTLR_ITS_NUMBER;
ctlr |= its_number << GITS_CTLR_ITS_NUMBER_SHIFT;
- writel_relaxed(ctlr, its_base + GITS_CTLR);
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ writel_relaxed(ctlr, its->base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
This (removal of its_base parameter) also feel like a separate change.
Also, I would define a local variable its_base to avoid dereferencing
its every time in order to get the base address.
if ((ctlr & GITS_CTLR_ITS_NUMBER) != (its_number <<
GITS_CTLR_ITS_NUMBER_SHIFT)) {
its_number = ctlr & GITS_CTLR_ITS_NUMBER;
its_number >>= GITS_CTLR_ITS_NUMBER_SHIFT;
@@ -3472,83 +3473,110 @@ static int __init its_compute_its_list_map(struct
resource *res,
if (test_and_set_bit(its_number, _list_map)) {
pr_err("ITS@%pa: Duplicate ITSList entry %d\n",
- >start, its_number);
+ >phys_base, its_number);
return -EINVAL;
}
return its_number;
}
+static void its_free(struct its_node *its)
+{
+ raw_spin_lock(_lock);
+ list_del(>entry);
+ raw_spin_unlock(_lock);
+
+ kfree(its);
+}
+
+static int __init its_init_one(struct its_node *its);
You might as well define its_init_one here, no?
+
static int __init its_probe_one(struct resource *res,
struct fwnode_handle *handle, int numa_node)
{
struct its_node *its;
+ int err;
+
+ its = kzalloc(sizeof(*its), GFP_KERNEL);
+ if (!its)
+ return -ENOMEM;
+
+ raw_spin_lock_init(>lock);
+ INIT_LIST_HEAD(>entry);
+ INIT_LIST_HEAD(>its_device_list);
+ its->fwnode_handle = handle;
+ its->phys_base = res->start;
+ its->phys_size = resource_size(res);
+ its->numa_node = numa_node;
+
+ raw_spin_lock(_lock);
+ list_add_tail(>entry, _nodes);
+ raw_spin_unlock(_lock);
+
+ pr_info("ITS %pR\n", res);
+
+ err = its_init_one(its);
+ if (err)
+ its_free(its);
+
+ return err;
+}
+
+static int __init its_init_one(struct its_node *its)
+{
void __iomem *its_base;
u32 val, ctlr;
u64 baser, tmp, typer;
Re: [PATCH 07/10] irqchip/gic-v3-its: Split probing from its node initialization
On 07/11/18 22:03, Robert Richter wrote:
To initialize the its nodes at a later point during boot, we need to
split probing from initialization. Collect all information required
for initialization in struct its_node. We can then use the its node
list for initialization.
Signed-off-by: Robert Richter
---
drivers/irqchip/irq-gic-v3-its.c | 135 +++--
drivers/irqchip/irq-gic-v3.c | 2 +-
include/linux/irqchip/arm-gic-v3.h | 4 +-
3 files changed, 87 insertions(+), 54 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4033f71f5181..c28f4158ff70 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -103,6 +103,7 @@ struct its_node {
struct list_headentry;
void __iomem*base;
phys_addr_t phys_base;
+ phys_addr_t phys_size;
struct its_cmd_block*cmd_base;
struct its_cmd_block*cmd_write;
struct its_basertables[GITS_BASER_NR_REGS];
@@ -3375,7 +3376,7 @@ static struct syscore_ops its_syscore_ops = {
.resume = its_restore_enable,
};
-static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
+static int its_init_domain(struct its_node *its)
{
struct irq_domain *inner_domain;
struct msi_domain_info *info;
@@ -3384,7 +3385,8 @@ static int its_init_domain(struct fwnode_handle *handle,
struct its_node *its)
if (!info)
return -ENOMEM;
- inner_domain = irq_domain_create_tree(handle, _domain_ops, its);
+ inner_domain = irq_domain_create_tree(its->fwnode_handle,
+ _domain_ops, its);
Separate change?
if (!inner_domain) {
kfree(info);
return -ENOMEM;
@@ -3441,8 +3443,7 @@ static int its_init_vpe_domain(void)
return 0;
}
-static int __init its_compute_its_list_map(struct resource *res,
- void __iomem *its_base)
+static int __init its_compute_its_list_map(struct its_node *its)
{
int its_number;
u32 ctlr;
@@ -3456,15 +3457,15 @@ static int __init its_compute_its_list_map(struct
resource *res,
its_number = find_first_zero_bit(_list_map, GICv4_ITS_LIST_MAX);
if (its_number >= GICv4_ITS_LIST_MAX) {
pr_err("ITS@%pa: No ITSList entry available!\n",
- >start);
+ >phys_base);
return -EINVAL;
}
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
ctlr &= ~GITS_CTLR_ITS_NUMBER;
ctlr |= its_number << GITS_CTLR_ITS_NUMBER_SHIFT;
- writel_relaxed(ctlr, its_base + GITS_CTLR);
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ writel_relaxed(ctlr, its->base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
This (removal of its_base parameter) also feel like a separate change.
Also, I would define a local variable its_base to avoid dereferencing
its every time in order to get the base address.
if ((ctlr & GITS_CTLR_ITS_NUMBER) != (its_number <<
GITS_CTLR_ITS_NUMBER_SHIFT)) {
its_number = ctlr & GITS_CTLR_ITS_NUMBER;
its_number >>= GITS_CTLR_ITS_NUMBER_SHIFT;
@@ -3472,83 +3473,110 @@ static int __init its_compute_its_list_map(struct
resource *res,
if (test_and_set_bit(its_number, _list_map)) {
pr_err("ITS@%pa: Duplicate ITSList entry %d\n",
- >start, its_number);
+ >phys_base, its_number);
return -EINVAL;
}
return its_number;
}
+static void its_free(struct its_node *its)
+{
+ raw_spin_lock(_lock);
+ list_del(>entry);
+ raw_spin_unlock(_lock);
+
+ kfree(its);
+}
+
+static int __init its_init_one(struct its_node *its);
You might as well define its_init_one here, no?
+
static int __init its_probe_one(struct resource *res,
struct fwnode_handle *handle, int numa_node)
{
struct its_node *its;
+ int err;
+
+ its = kzalloc(sizeof(*its), GFP_KERNEL);
+ if (!its)
+ return -ENOMEM;
+
+ raw_spin_lock_init(>lock);
+ INIT_LIST_HEAD(>entry);
+ INIT_LIST_HEAD(>its_device_list);
+ its->fwnode_handle = handle;
+ its->phys_base = res->start;
+ its->phys_size = resource_size(res);
+ its->numa_node = numa_node;
+
+ raw_spin_lock(_lock);
+ list_add_tail(>entry, _nodes);
+ raw_spin_unlock(_lock);
+
+ pr_info("ITS %pR\n", res);
+
+ err = its_init_one(its);
+ if (err)
+ its_free(its);
+
+ return err;
+}
+
+static int __init its_init_one(struct its_node *its)
+{
void __iomem *its_base;
u32 val, ctlr;
u64 baser, tmp, typer;
[PATCH 07/10] irqchip/gic-v3-its: Split probing from its node initialization
To initialize the its nodes at a later point during boot, we need to
split probing from initialization. Collect all information required
for initialization in struct its_node. We can then use the its node
list for initialization.
Signed-off-by: Robert Richter
---
drivers/irqchip/irq-gic-v3-its.c | 135 +++--
drivers/irqchip/irq-gic-v3.c | 2 +-
include/linux/irqchip/arm-gic-v3.h | 4 +-
3 files changed, 87 insertions(+), 54 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4033f71f5181..c28f4158ff70 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -103,6 +103,7 @@ struct its_node {
struct list_headentry;
void __iomem*base;
phys_addr_t phys_base;
+ phys_addr_t phys_size;
struct its_cmd_block*cmd_base;
struct its_cmd_block*cmd_write;
struct its_basertables[GITS_BASER_NR_REGS];
@@ -3375,7 +3376,7 @@ static struct syscore_ops its_syscore_ops = {
.resume = its_restore_enable,
};
-static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
+static int its_init_domain(struct its_node *its)
{
struct irq_domain *inner_domain;
struct msi_domain_info *info;
@@ -3384,7 +3385,8 @@ static int its_init_domain(struct fwnode_handle *handle,
struct its_node *its)
if (!info)
return -ENOMEM;
- inner_domain = irq_domain_create_tree(handle, _domain_ops, its);
+ inner_domain = irq_domain_create_tree(its->fwnode_handle,
+ _domain_ops, its);
if (!inner_domain) {
kfree(info);
return -ENOMEM;
@@ -3441,8 +3443,7 @@ static int its_init_vpe_domain(void)
return 0;
}
-static int __init its_compute_its_list_map(struct resource *res,
- void __iomem *its_base)
+static int __init its_compute_its_list_map(struct its_node *its)
{
int its_number;
u32 ctlr;
@@ -3456,15 +3457,15 @@ static int __init its_compute_its_list_map(struct
resource *res,
its_number = find_first_zero_bit(_list_map, GICv4_ITS_LIST_MAX);
if (its_number >= GICv4_ITS_LIST_MAX) {
pr_err("ITS@%pa: No ITSList entry available!\n",
- >start);
+ >phys_base);
return -EINVAL;
}
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
ctlr &= ~GITS_CTLR_ITS_NUMBER;
ctlr |= its_number << GITS_CTLR_ITS_NUMBER_SHIFT;
- writel_relaxed(ctlr, its_base + GITS_CTLR);
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ writel_relaxed(ctlr, its->base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
if ((ctlr & GITS_CTLR_ITS_NUMBER) != (its_number <<
GITS_CTLR_ITS_NUMBER_SHIFT)) {
its_number = ctlr & GITS_CTLR_ITS_NUMBER;
its_number >>= GITS_CTLR_ITS_NUMBER_SHIFT;
@@ -3472,83 +3473,110 @@ static int __init its_compute_its_list_map(struct
resource *res,
if (test_and_set_bit(its_number, _list_map)) {
pr_err("ITS@%pa: Duplicate ITSList entry %d\n",
- >start, its_number);
+ >phys_base, its_number);
return -EINVAL;
}
return its_number;
}
+static void its_free(struct its_node *its)
+{
+ raw_spin_lock(_lock);
+ list_del(>entry);
+ raw_spin_unlock(_lock);
+
+ kfree(its);
+}
+
+static int __init its_init_one(struct its_node *its);
+
static int __init its_probe_one(struct resource *res,
struct fwnode_handle *handle, int numa_node)
{
struct its_node *its;
+ int err;
+
+ its = kzalloc(sizeof(*its), GFP_KERNEL);
+ if (!its)
+ return -ENOMEM;
+
+ raw_spin_lock_init(>lock);
+ INIT_LIST_HEAD(>entry);
+ INIT_LIST_HEAD(>its_device_list);
+ its->fwnode_handle = handle;
+ its->phys_base = res->start;
+ its->phys_size = resource_size(res);
+ its->numa_node = numa_node;
+
+ raw_spin_lock(_lock);
+ list_add_tail(>entry, _nodes);
+ raw_spin_unlock(_lock);
+
+ pr_info("ITS %pR\n", res);
+
+ err = its_init_one(its);
+ if (err)
+ its_free(its);
+
+ return err;
+}
+
+static int __init its_init_one(struct its_node *its)
+{
void __iomem *its_base;
u32 val, ctlr;
u64 baser, tmp, typer;
int err;
- its_base = ioremap(res->start, resource_size(res));
+ its_base = ioremap(its->phys_base, its->phys_size);
if (!its_base) {
- pr_warn("ITS@%pa: Unable to map ITS registers\n", >start);
- return -ENOMEM;
+ pr_warn("ITS@%pa: Unable to map
[PATCH 07/10] irqchip/gic-v3-its: Split probing from its node initialization
To initialize the its nodes at a later point during boot, we need to
split probing from initialization. Collect all information required
for initialization in struct its_node. We can then use the its node
list for initialization.
Signed-off-by: Robert Richter
---
drivers/irqchip/irq-gic-v3-its.c | 135 +++--
drivers/irqchip/irq-gic-v3.c | 2 +-
include/linux/irqchip/arm-gic-v3.h | 4 +-
3 files changed, 87 insertions(+), 54 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4033f71f5181..c28f4158ff70 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -103,6 +103,7 @@ struct its_node {
struct list_headentry;
void __iomem*base;
phys_addr_t phys_base;
+ phys_addr_t phys_size;
struct its_cmd_block*cmd_base;
struct its_cmd_block*cmd_write;
struct its_basertables[GITS_BASER_NR_REGS];
@@ -3375,7 +3376,7 @@ static struct syscore_ops its_syscore_ops = {
.resume = its_restore_enable,
};
-static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
+static int its_init_domain(struct its_node *its)
{
struct irq_domain *inner_domain;
struct msi_domain_info *info;
@@ -3384,7 +3385,8 @@ static int its_init_domain(struct fwnode_handle *handle,
struct its_node *its)
if (!info)
return -ENOMEM;
- inner_domain = irq_domain_create_tree(handle, _domain_ops, its);
+ inner_domain = irq_domain_create_tree(its->fwnode_handle,
+ _domain_ops, its);
if (!inner_domain) {
kfree(info);
return -ENOMEM;
@@ -3441,8 +3443,7 @@ static int its_init_vpe_domain(void)
return 0;
}
-static int __init its_compute_its_list_map(struct resource *res,
- void __iomem *its_base)
+static int __init its_compute_its_list_map(struct its_node *its)
{
int its_number;
u32 ctlr;
@@ -3456,15 +3457,15 @@ static int __init its_compute_its_list_map(struct
resource *res,
its_number = find_first_zero_bit(_list_map, GICv4_ITS_LIST_MAX);
if (its_number >= GICv4_ITS_LIST_MAX) {
pr_err("ITS@%pa: No ITSList entry available!\n",
- >start);
+ >phys_base);
return -EINVAL;
}
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
ctlr &= ~GITS_CTLR_ITS_NUMBER;
ctlr |= its_number << GITS_CTLR_ITS_NUMBER_SHIFT;
- writel_relaxed(ctlr, its_base + GITS_CTLR);
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ writel_relaxed(ctlr, its->base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
if ((ctlr & GITS_CTLR_ITS_NUMBER) != (its_number <<
GITS_CTLR_ITS_NUMBER_SHIFT)) {
its_number = ctlr & GITS_CTLR_ITS_NUMBER;
its_number >>= GITS_CTLR_ITS_NUMBER_SHIFT;
@@ -3472,83 +3473,110 @@ static int __init its_compute_its_list_map(struct
resource *res,
if (test_and_set_bit(its_number, _list_map)) {
pr_err("ITS@%pa: Duplicate ITSList entry %d\n",
- >start, its_number);
+ >phys_base, its_number);
return -EINVAL;
}
return its_number;
}
+static void its_free(struct its_node *its)
+{
+ raw_spin_lock(_lock);
+ list_del(>entry);
+ raw_spin_unlock(_lock);
+
+ kfree(its);
+}
+
+static int __init its_init_one(struct its_node *its);
+
static int __init its_probe_one(struct resource *res,
struct fwnode_handle *handle, int numa_node)
{
struct its_node *its;
+ int err;
+
+ its = kzalloc(sizeof(*its), GFP_KERNEL);
+ if (!its)
+ return -ENOMEM;
+
+ raw_spin_lock_init(>lock);
+ INIT_LIST_HEAD(>entry);
+ INIT_LIST_HEAD(>its_device_list);
+ its->fwnode_handle = handle;
+ its->phys_base = res->start;
+ its->phys_size = resource_size(res);
+ its->numa_node = numa_node;
+
+ raw_spin_lock(_lock);
+ list_add_tail(>entry, _nodes);
+ raw_spin_unlock(_lock);
+
+ pr_info("ITS %pR\n", res);
+
+ err = its_init_one(its);
+ if (err)
+ its_free(its);
+
+ return err;
+}
+
+static int __init its_init_one(struct its_node *its)
+{
void __iomem *its_base;
u32 val, ctlr;
u64 baser, tmp, typer;
int err;
- its_base = ioremap(res->start, resource_size(res));
+ its_base = ioremap(its->phys_base, its->phys_size);
if (!its_base) {
- pr_warn("ITS@%pa: Unable to map ITS registers\n", >start);
- return -ENOMEM;
+ pr_warn("ITS@%pa: Unable to map
