* [PATCH v2 1/6] irqchip: gicv3-its: zero itt before handling to hardware
2015-02-15 9:31 [PATCH v2 0/6] enhance configuring an ITS Yun Wu
@ 2015-02-15 9:31 ` Yun Wu
2015-02-15 9:31 ` [PATCH v2 2/6] irqchip: gicv3-its: use 64KB page as default granule Yun Wu
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Yun Wu @ 2015-02-15 9:31 UTC (permalink / raw)
To: marc.zyngier, tglx, jason; +Cc: linux-kernel, linux-arm-kernel, Yun Wu
Some kind of brain-dead implementations chooses to insert ITEes in
rapid sequence of disabled ITEes, and an un-zeroed ITT will confuse
ITS on judging whether an ITE is really enabled or not. Considering
the implementations are still supported by the GICv3 architecture,
in which ITT is not required to be zeroed before being handled to
hardware, we do the favor in ITS driver.
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
---
drivers/irqchip/irq-gic-v3-its.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 6850141..69eeea3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1076,7 +1076,7 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
nr_ites = max(2UL, roundup_pow_of_two(nvecs));
sz = nr_ites * its->ite_size;
sz = max(sz, ITS_ITT_ALIGN) + ITS_ITT_ALIGN - 1;
- itt = kmalloc(sz, GFP_KERNEL);
+ itt = kzalloc(sz, GFP_KERNEL);
lpi_map = its_lpi_alloc_chunks(nvecs, &lpi_base, &nr_lpis);
if (!dev || !itt || !lpi_map) {
--
1.8.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/6] irqchip: gicv3-its: use 64KB page as default granule
2015-02-15 9:31 [PATCH v2 0/6] enhance configuring an ITS Yun Wu
2015-02-15 9:31 ` [PATCH v2 1/6] irqchip: gicv3-its: zero itt before handling to hardware Yun Wu
@ 2015-02-15 9:31 ` Yun Wu
2015-02-17 9:46 ` Marc Zyngier
2015-02-15 9:32 ` [PATCH v2 3/6] irqchip: gicv3-its: limit order of DT size to MAX_ORDER Yun Wu
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Yun Wu @ 2015-02-15 9:31 UTC (permalink / raw)
To: marc.zyngier, tglx, jason; +Cc: linux-kernel, linux-arm-kernel, Yun Wu
The field of page size in register GITS_BASERn might be read-only
if an implementation only supports a single, fixed page size. But
currently the ITS driver will throw out an error when PAGE_SIZE
is less than the minimum size supported by an ITS. So addressing
this problem by using 64KB pages as default granule for all the
ITS base tables.
Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
---
drivers/irqchip/irq-gic-v3-its.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 69eeea3..f5bfa42 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -800,7 +800,7 @@ static int its_alloc_tables(struct its_node *its)
{
int err;
int i;
- int psz = PAGE_SIZE;
+ int psz = SZ_64K;
u64 shr = GITS_BASER_InnerShareable;
for (i = 0; i < GITS_BASER_NR_REGS; i++) {
--
1.8.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/6] irqchip: gicv3-its: use 64KB page as default granule
2015-02-15 9:31 ` [PATCH v2 2/6] irqchip: gicv3-its: use 64KB page as default granule Yun Wu
@ 2015-02-17 9:46 ` Marc Zyngier
0 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2015-02-17 9:46 UTC (permalink / raw)
To: Yun Wu; +Cc: tglx, jason, linux-kernel, linux-arm-kernel
On Sun, 15 Feb 2015 09:31:59 +0000
Yun Wu <wuyun.wu@huawei.com> wrote:
> The field of page size in register GITS_BASERn might be read-only
> if an implementation only supports a single, fixed page size. But
> currently the ITS driver will throw out an error when PAGE_SIZE
> is less than the minimum size supported by an ITS. So addressing
> this problem by using 64KB pages as default granule for all the
> ITS base tables.
>
> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c
> b/drivers/irqchip/irq-gic-v3-its.c index 69eeea3..f5bfa42 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -800,7 +800,7 @@ static int its_alloc_tables(struct its_node *its)
> {
> int err;
> int i;
> - int psz = PAGE_SIZE;
> + int psz = SZ_64K;
> u64 shr = GITS_BASER_InnerShareable;
>
> for (i = 0; i < GITS_BASER_NR_REGS; i++) {
> --
> 1.8.0
>
>
>
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 3/6] irqchip: gicv3-its: limit order of DT size to MAX_ORDER
2015-02-15 9:31 [PATCH v2 0/6] enhance configuring an ITS Yun Wu
2015-02-15 9:31 ` [PATCH v2 1/6] irqchip: gicv3-its: zero itt before handling to hardware Yun Wu
2015-02-15 9:31 ` [PATCH v2 2/6] irqchip: gicv3-its: use 64KB page as default granule Yun Wu
@ 2015-02-15 9:32 ` Yun Wu
2015-02-17 9:19 ` Marc Zyngier
2015-02-15 9:32 ` [PATCH v2 4/6] irqchip: gicv3-its: define macros for GITS_CTLR fields Yun Wu
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Yun Wu @ 2015-02-15 9:32 UTC (permalink / raw)
To: marc.zyngier, tglx, jason; +Cc: linux-kernel, linux-arm-kernel, Yun Wu
When required DT size is out of the kmalloc()'s capability, the whole
ITS will fail in probing. This actually is not the hardware's problem
and is mainly a limitation of the kernel memory allocator. This patch
will keep ITS going on to the next initializaion step with an explicit
warning.
Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
---
drivers/irqchip/irq-gic-v3-its.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index f5bfa42..de36606 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -828,6 +828,12 @@ static int its_alloc_tables(struct its_node *its)
u32 ids = GITS_TYPER_DEVBITS(typer);
order = get_order((1UL << ids) * entry_size);
+ if (order >= MAX_ORDER) {
+ pr_warn("%s: DT size too large, reduce to %u pages\n",
+ its->msi_chip.of_node->full_name,
+ 1 << order);
+ order = MAX_ORDER;
+ }
}
alloc_size = (1 << order) * PAGE_SIZE;
--
1.8.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/6] irqchip: gicv3-its: limit order of DT size to MAX_ORDER
2015-02-15 9:32 ` [PATCH v2 3/6] irqchip: gicv3-its: limit order of DT size to MAX_ORDER Yun Wu
@ 2015-02-17 9:19 ` Marc Zyngier
2015-02-17 10:00 ` Yun Wu (Abel)
0 siblings, 1 reply; 17+ messages in thread
From: Marc Zyngier @ 2015-02-17 9:19 UTC (permalink / raw)
To: Yun Wu; +Cc: tglx, jason, linux-kernel, linux-arm-kernel
On Sun, 15 Feb 2015 09:32:00 +0000
Yun Wu <wuyun.wu@huawei.com> wrote:
> When required DT size is out of the kmalloc()'s capability, the whole
Nit: Using the DT acronym is very confusing here, as it means "Device
Tree" to most people, including me. Please use "Device Table" instead.
Also, the reference to kmalloc is wrong, as we're really using the page
allocator.
> ITS will fail in probing. This actually is not the hardware's problem
> and is mainly a limitation of the kernel memory allocator. This patch
> will keep ITS going on to the next initializaion step with an explicit
> warning.
>
> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c
> b/drivers/irqchip/irq-gic-v3-its.c index f5bfa42..de36606 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -828,6 +828,12 @@ static int its_alloc_tables(struct its_node *its)
> u32 ids = GITS_TYPER_DEVBITS(typer);
>
> order = get_order((1UL << ids) * entry_size);
> + if (order >= MAX_ORDER) {
> + pr_warn("%s: DT size too large,
> reduce to %u pages\n",
> +
> its->msi_chip.of_node->full_name,
> + 1 << order);
> + order = MAX_ORDER;
> + }
> }
Something is wrong here. Either (order == MAX_ORDER) is acceptable, or
it is not. Also, your warning says that you're reducing the size to a
new value, but you're displaying the size you can't handle. That
message should be fixed.
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/6] irqchip: gicv3-its: limit order of DT size to MAX_ORDER
2015-02-17 9:19 ` Marc Zyngier
@ 2015-02-17 10:00 ` Yun Wu (Abel)
0 siblings, 0 replies; 17+ messages in thread
From: Yun Wu (Abel) @ 2015-02-17 10:00 UTC (permalink / raw)
To: Marc Zyngier; +Cc: tglx, jason, linux-kernel, linux-arm-kernel
On 2015/2/17 17:19, Marc Zyngier wrote:
> On Sun, 15 Feb 2015 09:32:00 +0000
> Yun Wu <wuyun.wu@huawei.com> wrote:
>
>> When required DT size is out of the kmalloc()'s capability, the whole
>
> Nit: Using the DT acronym is very confusing here, as it means "Device
> Tree" to most people, including me. Please use "Device Table" instead.
>
> Also, the reference to kmalloc is wrong, as we're really using the page
> allocator.
OK, I will get the description fixed in the next version.
>
>> ITS will fail in probing. This actually is not the hardware's problem
>> and is mainly a limitation of the kernel memory allocator. This patch
>> will keep ITS going on to the next initializaion step with an explicit
>> warning.
>>
>> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>> b/drivers/irqchip/irq-gic-v3-its.c index f5bfa42..de36606 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -828,6 +828,12 @@ static int its_alloc_tables(struct its_node *its)
>> u32 ids = GITS_TYPER_DEVBITS(typer);
>>
>> order = get_order((1UL << ids) * entry_size);
>> + if (order >= MAX_ORDER) {
>> + pr_warn("%s: DT size too large,
>> reduce to %u pages\n",
>> +
>> its->msi_chip.of_node->full_name,
>> + 1 << order);
>> + order = MAX_ORDER;
>> + }
>> }
>
> Something is wrong here. Either (order == MAX_ORDER) is acceptable, or
> it is not. Also, your warning says that you're reducing the size to a
> new value, but you're displaying the size you can't handle. That
> message should be fixed.
>
Yes, my fault, I will fix them.
Thanks,
Abel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 4/6] irqchip: gicv3-its: define macros for GITS_CTLR fields
2015-02-15 9:31 [PATCH v2 0/6] enhance configuring an ITS Yun Wu
` (2 preceding siblings ...)
2015-02-15 9:32 ` [PATCH v2 3/6] irqchip: gicv3-its: limit order of DT size to MAX_ORDER Yun Wu
@ 2015-02-15 9:32 ` Yun Wu
2015-02-15 9:32 ` [PATCH v2 5/6] irqchip: gicv3-its: add support for power down Yun Wu
2015-02-15 9:32 ` [PATCH v2 6/6] irqchip: gicv3: skip ITS init when no ITS available Yun Wu
5 siblings, 0 replies; 17+ messages in thread
From: Yun Wu @ 2015-02-15 9:32 UTC (permalink / raw)
To: marc.zyngier, tglx, jason; +Cc: linux-kernel, linux-arm-kernel, Yun Wu
Define macros for GITS_CTLR fields to avoid using magic numbers.
Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
---
drivers/irqchip/irq-gic-v3-its.c | 2 +-
include/linux/irqchip/arm-gic-v3.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index de36606..42c03b2 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1389,7 +1389,7 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
writeq_relaxed(baser, its->base + GITS_CBASER);
tmp = readq_relaxed(its->base + GITS_CBASER);
writeq_relaxed(0, its->base + GITS_CWRITER);
- writel_relaxed(1, its->base + GITS_CTLR);
+ writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);
if ((tmp ^ baser) & GITS_BASER_SHAREABILITY_MASK) {
pr_info("ITS: using cache flushing for cmd queue\n");
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 3459b43..c9d3002 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -134,6 +134,9 @@
#define GITS_TRANSLATER 0x10040
+#define GITS_CTLR_ENABLE (1U << 0)
+#define GITS_CTLR_QUIESCENT (1U << 31)
+
#define GITS_TYPER_DEVBITS_SHIFT 13
#define GITS_TYPER_DEVBITS(r) ((((r) >> GITS_TYPER_DEVBITS_SHIFT) & 0x1f) + 1)
#define GITS_TYPER_PTA (1UL << 19)
--
1.8.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 5/6] irqchip: gicv3-its: add support for power down
2015-02-15 9:31 [PATCH v2 0/6] enhance configuring an ITS Yun Wu
` (3 preceding siblings ...)
2015-02-15 9:32 ` [PATCH v2 4/6] irqchip: gicv3-its: define macros for GITS_CTLR fields Yun Wu
@ 2015-02-15 9:32 ` Yun Wu
2015-02-17 9:29 ` Marc Zyngier
2015-02-15 9:32 ` [PATCH v2 6/6] irqchip: gicv3: skip ITS init when no ITS available Yun Wu
5 siblings, 1 reply; 17+ messages in thread
From: Yun Wu @ 2015-02-15 9:32 UTC (permalink / raw)
To: marc.zyngier, tglx, jason; +Cc: linux-kernel, linux-arm-kernel, Yun Wu
It's unsafe to change the configurations of an activated ITS directly
since this will lead to unpredictable results. This patch guarantees
a safe quiescent status before initializing an ITS.
Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
---
drivers/irqchip/irq-gic-v3-its.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 42c03b2..29eb665 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1321,6 +1321,31 @@ static const struct irq_domain_ops its_domain_ops = {
.deactivate = its_irq_domain_deactivate,
};
+static int its_check_quiesced(void __iomem *base)
+{
+ u32 count = 1000000; /* 1s */
+ u32 val;
+
+ val = readl_relaxed(base + GITS_CTLR);
+ if (val & GITS_CTLR_QUIESCENT)
+ return 0;
+
+ /* Disable the generation of all interrupts to this ITS */
+ val &= ~GITS_CTLR_ENABLE;
+ writel_relaxed(val, base + GITS_CTLR);
+
+ /* Poll GITS_CTLR and wait until ITS becomes quiescent */
+ while (count--) {
+ val = readl_relaxed(base + GITS_CTLR);
+ if (val & GITS_CTLR_QUIESCENT)
+ return 0;
+ cpu_relax();
+ udelay(1);
+ }
+
+ return -EBUSY;
+}
+
static int its_probe(struct device_node *node, struct irq_domain *parent)
{
struct resource res;
@@ -1349,6 +1374,13 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
goto out_unmap;
}
+ err = its_check_quiesced(its_base);
+ if (err) {
+ pr_warn("%s: failed to quiesce, behaviour unpredictable\n",
+ node->full_name);
+ goto out_unmap;
+ }
+
pr_info("ITS: %s\n", node->full_name);
its = kzalloc(sizeof(*its), GFP_KERNEL);
--
1.8.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/6] irqchip: gicv3-its: add support for power down
2015-02-15 9:32 ` [PATCH v2 5/6] irqchip: gicv3-its: add support for power down Yun Wu
@ 2015-02-17 9:29 ` Marc Zyngier
2015-02-17 10:15 ` Yun Wu (Abel)
0 siblings, 1 reply; 17+ messages in thread
From: Marc Zyngier @ 2015-02-17 9:29 UTC (permalink / raw)
To: Yun Wu; +Cc: tglx, jason, linux-kernel, linux-arm-kernel
On Sun, 15 Feb 2015 09:32:02 +0000
Yun Wu <wuyun.wu@huawei.com> wrote:
> It's unsafe to change the configurations of an activated ITS directly
> since this will lead to unpredictable results. This patch guarantees
> a safe quiescent status before initializing an ITS.
Please change the title of this patch to reflect what it actually
does. Nothing here is about powering down anything.
> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 32
> ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c
> b/drivers/irqchip/irq-gic-v3-its.c index 42c03b2..29eb665 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1321,6 +1321,31 @@ static const struct irq_domain_ops
> its_domain_ops = { .deactivate =
> its_irq_domain_deactivate, };
>
> +static int its_check_quiesced(void __iomem *base)
> +{
> + u32 count = 1000000; /* 1s */
> + u32 val;
> +
> + val = readl_relaxed(base + GITS_CTLR);
> + if (val & GITS_CTLR_QUIESCENT)
> + return 0;
> +
> + /* Disable the generation of all interrupts to this ITS */
> + val &= ~GITS_CTLR_ENABLE;
> + writel_relaxed(val, base + GITS_CTLR);
> +
> + /* Poll GITS_CTLR and wait until ITS becomes quiescent */
> + while (count--) {
> + val = readl_relaxed(base + GITS_CTLR);
> + if (val & GITS_CTLR_QUIESCENT)
> + return 0;
> + cpu_relax();
> + udelay(1);
> + }
You're now introducing a third variant of a 1s timeout loop. Notice a
pattern?
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/6] irqchip: gicv3-its: add support for power down
2015-02-17 9:29 ` Marc Zyngier
@ 2015-02-17 10:15 ` Yun Wu (Abel)
2015-02-17 11:11 ` Marc Zyngier
0 siblings, 1 reply; 17+ messages in thread
From: Yun Wu (Abel) @ 2015-02-17 10:15 UTC (permalink / raw)
To: Marc Zyngier; +Cc: tglx, jason, linux-arm-kernel, linux-kernel
On 2015/2/17 17:29, Marc Zyngier wrote:
> On Sun, 15 Feb 2015 09:32:02 +0000
> Yun Wu <wuyun.wu@huawei.com> wrote:
>
>> It's unsafe to change the configurations of an activated ITS directly
>> since this will lead to unpredictable results. This patch guarantees
>> a safe quiescent status before initializing an ITS.
>
> Please change the title of this patch to reflect what it actually
> does. Nothing here is about powering down anything.
My miss, I will fix this in next version.
>
>> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 32
>> ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>> b/drivers/irqchip/irq-gic-v3-its.c index 42c03b2..29eb665 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -1321,6 +1321,31 @@ static const struct irq_domain_ops
>> its_domain_ops = { .deactivate =
>> its_irq_domain_deactivate, };
>>
>> +static int its_check_quiesced(void __iomem *base)
>> +{
>> + u32 count = 1000000; /* 1s */
>> + u32 val;
>> +
>> + val = readl_relaxed(base + GITS_CTLR);
>> + if (val & GITS_CTLR_QUIESCENT)
>> + return 0;
>> +
>> + /* Disable the generation of all interrupts to this ITS */
>> + val &= ~GITS_CTLR_ENABLE;
>> + writel_relaxed(val, base + GITS_CTLR);
>> +
>> + /* Poll GITS_CTLR and wait until ITS becomes quiescent */
>> + while (count--) {
>> + val = readl_relaxed(base + GITS_CTLR);
>> + if (val & GITS_CTLR_QUIESCENT)
>> + return 0;
>> + cpu_relax();
>> + udelay(1);
>> + }
>
> You're now introducing a third variant of a 1s timeout loop. Notice a
> pattern?
>
I am not sure I know exactly what you suggest. Do you mean I should code
like below to keep the coding style same as the other 2 loops?
while (1) {
val = readl_relaxed(base + GITS_CTLR);
if (val & GITS_CTLR_QUIESCENT)
return 0;
count--;
if (!count)
return -EBUSY;
cpu_relax();
udelay(1);
}
Thanks,
Abel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/6] irqchip: gicv3-its: add support for power down
2015-02-17 10:15 ` Yun Wu (Abel)
@ 2015-02-17 11:11 ` Marc Zyngier
2015-02-17 12:27 ` Yun Wu (Abel)
0 siblings, 1 reply; 17+ messages in thread
From: Marc Zyngier @ 2015-02-17 11:11 UTC (permalink / raw)
To: Yun Wu (Abel); +Cc: tglx, jason, linux-arm-kernel, linux-kernel
On Tue, 17 Feb 2015 10:15:15 +0000
"Yun Wu (Abel)" <wuyun.wu@huawei.com> wrote:
> On 2015/2/17 17:29, Marc Zyngier wrote:
>
> > On Sun, 15 Feb 2015 09:32:02 +0000
> > Yun Wu <wuyun.wu@huawei.com> wrote:
> >
> >> It's unsafe to change the configurations of an activated ITS
> >> directly since this will lead to unpredictable results. This patch
> >> guarantees a safe quiescent status before initializing an ITS.
> >
> > Please change the title of this patch to reflect what it actually
> > does. Nothing here is about powering down anything.
>
> My miss, I will fix this in next version.
>
> >
> >> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
> >> ---
> >> drivers/irqchip/irq-gic-v3-its.c | 32
> >> ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
> >>
> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c
> >> b/drivers/irqchip/irq-gic-v3-its.c index 42c03b2..29eb665 100644
> >> --- a/drivers/irqchip/irq-gic-v3-its.c
> >> +++ b/drivers/irqchip/irq-gic-v3-its.c
> >> @@ -1321,6 +1321,31 @@ static const struct irq_domain_ops
> >> its_domain_ops = { .deactivate =
> >> its_irq_domain_deactivate, };
> >>
> >> +static int its_check_quiesced(void __iomem *base)
> >> +{
> >> + u32 count = 1000000; /* 1s */
> >> + u32 val;
> >> +
> >> + val = readl_relaxed(base + GITS_CTLR);
> >> + if (val & GITS_CTLR_QUIESCENT)
> >> + return 0;
> >> +
> >> + /* Disable the generation of all interrupts to this ITS */
> >> + val &= ~GITS_CTLR_ENABLE;
> >> + writel_relaxed(val, base + GITS_CTLR);
> >> +
> >> + /* Poll GITS_CTLR and wait until ITS becomes quiescent */
> >> + while (count--) {
> >> + val = readl_relaxed(base + GITS_CTLR);
> >> + if (val & GITS_CTLR_QUIESCENT)
> >> + return 0;
> >> + cpu_relax();
> >> + udelay(1);
> >> + }
> >
> > You're now introducing a third variant of a 1s timeout loop. Notice
> > a pattern?
> >
>
> I am not sure I know exactly what you suggest. Do you mean I should
> code like below to keep the coding style same as the other 2 loops?
>
> while (1) {
> val = readl_relaxed(base + GITS_CTLR);
> if (val & GITS_CTLR_QUIESCENT)
> return 0;
>
> count--;
> if (!count)
> return -EBUSY;
>
> cpu_relax();
> udelay(1);
> }
That'd be a good start. But given that this is starting to be a common
construct, it could probably be rewritten as:
static int its_poll_timeout(struct its_node *its, void *data,
int (*fn)(struct its_node *its,
void *data))
{
while (1) {
if (!fn(its, data))
return 0;
...
}
}
and have the call sites to provide the right utility function. We also
have two similar timeout loops in the main GICv3 driver, so there
should be room for improvement.
Thoughts?
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/6] irqchip: gicv3-its: add support for power down
2015-02-17 11:11 ` Marc Zyngier
@ 2015-02-17 12:27 ` Yun Wu (Abel)
2015-03-04 3:10 ` Yun Wu (Abel)
0 siblings, 1 reply; 17+ messages in thread
From: Yun Wu (Abel) @ 2015-02-17 12:27 UTC (permalink / raw)
To: Marc Zyngier; +Cc: tglx, jason, linux-arm-kernel, linux-kernel
On 2015/2/17 19:11, Marc Zyngier wrote:
> On Tue, 17 Feb 2015 10:15:15 +0000
> "Yun Wu (Abel)" <wuyun.wu@huawei.com> wrote:
>
>> On 2015/2/17 17:29, Marc Zyngier wrote:
>>
>>> On Sun, 15 Feb 2015 09:32:02 +0000
>>> Yun Wu <wuyun.wu@huawei.com> wrote:
>>>
>>>> It's unsafe to change the configurations of an activated ITS
>>>> directly since this will lead to unpredictable results. This patch
>>>> guarantees a safe quiescent status before initializing an ITS.
>>>
>>> Please change the title of this patch to reflect what it actually
>>> does. Nothing here is about powering down anything.
>>
>> My miss, I will fix this in next version.
>>
>>>
>>>> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
>>>> ---
>>>> drivers/irqchip/irq-gic-v3-its.c | 32
>>>> ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
>>>>
>>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>>>> b/drivers/irqchip/irq-gic-v3-its.c index 42c03b2..29eb665 100644
>>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>>> @@ -1321,6 +1321,31 @@ static const struct irq_domain_ops
>>>> its_domain_ops = { .deactivate =
>>>> its_irq_domain_deactivate, };
>>>>
>>>> +static int its_check_quiesced(void __iomem *base)
>>>> +{
>>>> + u32 count = 1000000; /* 1s */
>>>> + u32 val;
>>>> +
>>>> + val = readl_relaxed(base + GITS_CTLR);
>>>> + if (val & GITS_CTLR_QUIESCENT)
>>>> + return 0;
>>>> +
>>>> + /* Disable the generation of all interrupts to this ITS */
>>>> + val &= ~GITS_CTLR_ENABLE;
>>>> + writel_relaxed(val, base + GITS_CTLR);
>>>> +
>>>> + /* Poll GITS_CTLR and wait until ITS becomes quiescent */
>>>> + while (count--) {
>>>> + val = readl_relaxed(base + GITS_CTLR);
>>>> + if (val & GITS_CTLR_QUIESCENT)
>>>> + return 0;
>>>> + cpu_relax();
>>>> + udelay(1);
>>>> + }
>>>
>>> You're now introducing a third variant of a 1s timeout loop. Notice
>>> a pattern?
>>>
>>
>> I am not sure I know exactly what you suggest. Do you mean I should
>> code like below to keep the coding style same as the other 2 loops?
>>
>> while (1) {
>> val = readl_relaxed(base + GITS_CTLR);
>> if (val & GITS_CTLR_QUIESCENT)
>> return 0;
>>
>> count--;
>> if (!count)
>> return -EBUSY;
>>
>> cpu_relax();
>> udelay(1);
>> }
>
> That'd be a good start. But given that this is starting to be a common
> construct, it could probably be rewritten as:
>
> static int its_poll_timeout(struct its_node *its, void *data,
> int (*fn)(struct its_node *its,
> void *data))
> {
> while (1) {
> if (!fn(its, data))
> return 0;
>
> ...
> }
> }
>
> and have the call sites to provide the right utility function. We also
> have two similar timeout loops in the main GICv3 driver, so there
> should be room for improvement.
>
> Thoughts?
>
It looks fine to me. I will make some improvement in the next version after
Chinese Spring Festival. :)
Thanks,
Abel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/6] irqchip: gicv3-its: add support for power down
2015-02-17 12:27 ` Yun Wu (Abel)
@ 2015-03-04 3:10 ` Yun Wu (Abel)
0 siblings, 0 replies; 17+ messages in thread
From: Yun Wu (Abel) @ 2015-03-04 3:10 UTC (permalink / raw)
To: Marc Zyngier; +Cc: tglx, jason, linux-arm-kernel, linux-kernel
On 2015/2/17 20:27, Yun Wu (Abel) wrote:
> On 2015/2/17 19:11, Marc Zyngier wrote:
>
>> On Tue, 17 Feb 2015 10:15:15 +0000
>> "Yun Wu (Abel)" <wuyun.wu@huawei.com> wrote:
>>
>>> On 2015/2/17 17:29, Marc Zyngier wrote:
>>>
>>>> On Sun, 15 Feb 2015 09:32:02 +0000
>>>> Yun Wu <wuyun.wu@huawei.com> wrote:
>>>>
>>>>> It's unsafe to change the configurations of an activated ITS
>>>>> directly since this will lead to unpredictable results. This patch
>>>>> guarantees a safe quiescent status before initializing an ITS.
>>>>
>>>> Please change the title of this patch to reflect what it actually
>>>> does. Nothing here is about powering down anything.
>>>
>>> My miss, I will fix this in next version.
>>>
>>>>
>>>>> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
>>>>> ---
>>>>> drivers/irqchip/irq-gic-v3-its.c | 32
>>>>> ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
>>>>>
>>>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>>>>> b/drivers/irqchip/irq-gic-v3-its.c index 42c03b2..29eb665 100644
>>>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>>>> @@ -1321,6 +1321,31 @@ static const struct irq_domain_ops
>>>>> its_domain_ops = { .deactivate =
>>>>> its_irq_domain_deactivate, };
>>>>>
>>>>> +static int its_check_quiesced(void __iomem *base)
>>>>> +{
>>>>> + u32 count = 1000000; /* 1s */
>>>>> + u32 val;
>>>>> +
>>>>> + val = readl_relaxed(base + GITS_CTLR);
>>>>> + if (val & GITS_CTLR_QUIESCENT)
>>>>> + return 0;
>>>>> +
>>>>> + /* Disable the generation of all interrupts to this ITS */
>>>>> + val &= ~GITS_CTLR_ENABLE;
>>>>> + writel_relaxed(val, base + GITS_CTLR);
>>>>> +
>>>>> + /* Poll GITS_CTLR and wait until ITS becomes quiescent */
>>>>> + while (count--) {
>>>>> + val = readl_relaxed(base + GITS_CTLR);
>>>>> + if (val & GITS_CTLR_QUIESCENT)
>>>>> + return 0;
>>>>> + cpu_relax();
>>>>> + udelay(1);
>>>>> + }
>>>>
>>>> You're now introducing a third variant of a 1s timeout loop. Notice
>>>> a pattern?
>>>>
>>>
>>> I am not sure I know exactly what you suggest. Do you mean I should
>>> code like below to keep the coding style same as the other 2 loops?
>>>
>>> while (1) {
>>> val = readl_relaxed(base + GITS_CTLR);
>>> if (val & GITS_CTLR_QUIESCENT)
>>> return 0;
>>>
>>> count--;
>>> if (!count)
>>> return -EBUSY;
>>>
>>> cpu_relax();
>>> udelay(1);
>>> }
>>
>> That'd be a good start. But given that this is starting to be a common
>> construct, it could probably be rewritten as:
>>
>> static int its_poll_timeout(struct its_node *its, void *data,
>> int (*fn)(struct its_node *its,
>> void *data))
>> {
>> while (1) {
>> if (!fn(its, data))
>> return 0;
>>
>> ...
>> }
>> }
>>
>> and have the call sites to provide the right utility function. We also
>> have two similar timeout loops in the main GICv3 driver, so there
>> should be room for improvement.
>>
>> Thoughts?
>>
Hi Marc,
Currently I didn't make any improvement on providing a unified utility
function to do the timeout loops, because I haven't found a proper way
yet. And to achieve this, at least three aspects I can imagine are
needed to be considered.
Refactoring the existing loop functions comes first. A prototype is
showed below.
static T1 func_prototype(T2 node, char *msg, void **args)
{
u32 count = 1000000;
do_something_here(node, args);
while (1) {
if (condition_satisfied(node, args))
return (T1)SUCCESS;
count--;
if (!count) {
print_err_msg(msg);
return (T1)FAIL;
}
cpu_relax();
udelay(1);
}
}
Obviously it will make things complicated to move do_something_here() and
print_err_msg() outside of func_prototype(), because func_prototype() could
be called sereval places. But the two functions are different from each
loop function, so...
static T1 func_prototype(T2 node, char *msg, void **args)
{
u32 count = 1000000;
do_something_here(node, args);
while (count--) {
if (condition_satisfied(node, args))
return (T1)SUCCESS;
cpu_relax();
udelay(1);
}
print_err_msg(msg);
return (T1)FAIL;
}
Now we can unify the loop part,
static bool condition_satisfied(T2 node, void **args);
static u32 poll_timeout(T2 node, void **args,
bool (*fn)(T2, void **))
{
u32 count = 1000000;
while (count--) {
if (fn(node, args))
break;
cpu_relax();
udelay(1);
}
return count;
}
static T1 func_prototype(T2 node, char *msg, void **args)
{
do_something_here(node, args);
if (poll_timeout(node, args, condition_satisfied)) {
return (T1)SUCCESS;
} else {
print_err_msg(msg);
return (T1)FAIL;
}
}
Look at what I have done, turn the original N loop functions to 2*N+1 functions.
It can hardly be called improvement.. :(
The 2nd and 3rd aspects are return value and list of parameters respectively.
Using (void *) may help a lot, I think.
Thoughts?
Thanks,
Abel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 6/6] irqchip: gicv3: skip ITS init when no ITS available
2015-02-15 9:31 [PATCH v2 0/6] enhance configuring an ITS Yun Wu
` (4 preceding siblings ...)
2015-02-15 9:32 ` [PATCH v2 5/6] irqchip: gicv3-its: add support for power down Yun Wu
@ 2015-02-15 9:32 ` Yun Wu
2015-02-16 10:05 ` Vladimir Murzin
5 siblings, 1 reply; 17+ messages in thread
From: Yun Wu @ 2015-02-15 9:32 UTC (permalink / raw)
To: marc.zyngier, tglx, jason; +Cc: linux-kernel, linux-arm-kernel, Yun Wu
There is one more condition that needs to be considered when judging
whether LPI feature is enabled or not, which is whether there is any
ITS available and correctly enabled.
This patch will fix this by caching ITS enabling status in the GIC
chip data structure.
Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
---
drivers/irqchip/irq-gic-v3.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 1a146cc..e17faca 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -47,6 +47,7 @@ struct gic_chip_data {
u64 redist_stride;
u32 nr_redist_regions;
unsigned int irq_nr;
+ int lpi_enabled;
};
static struct gic_chip_data gic_data __read_mostly;
@@ -390,11 +391,6 @@ static void gic_cpu_sys_reg_init(void)
gic_write_grpen1(1);
}
-static int gic_dist_supports_lpis(void)
-{
- return !!(readl_relaxed(gic_data.dist_base + GICD_TYPER) & GICD_TYPER_LPIS);
-}
-
static void gic_cpu_init(void)
{
void __iomem *rbase;
@@ -410,7 +406,7 @@ static void gic_cpu_init(void)
gic_cpu_config(rbase, gic_redist_wait_for_rwp);
/* Give LPIs a spin */
- if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
+ if (gic_data.lpi_enabled)
its_cpu_init();
/* initialise system registers */
@@ -629,7 +625,7 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
}
/* LPIs */
if (hw >= 8192 && hw < GIC_ID_NR) {
- if (!gic_dist_supports_lpis())
+ if (!gic_data.lpi_enabled)
return -EPERM;
irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
handle_fasteoi_irq, NULL, NULL);
@@ -785,8 +781,12 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
set_handle_irq(gic_handle_irq);
- if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
- its_init(node, &gic_data.rdists, gic_data.domain);
+ if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) &&
+ !!(readl_relaxed(dist_base + GICD_TYPER) & GICD_TYPER_LPIS) &&
+ !its_init(node, &gic_data.rdists, gic_data.domain))
+ gic_data.lpi_enabled = 1;
+ else
+ gic_data.lpi_enabled = 0;
gic_smp_init();
gic_dist_init();
--
1.8.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 6/6] irqchip: gicv3: skip ITS init when no ITS available
2015-02-15 9:32 ` [PATCH v2 6/6] irqchip: gicv3: skip ITS init when no ITS available Yun Wu
@ 2015-02-16 10:05 ` Vladimir Murzin
2015-02-16 14:57 ` Yun Wu (Abel)
0 siblings, 1 reply; 17+ messages in thread
From: Vladimir Murzin @ 2015-02-16 10:05 UTC (permalink / raw)
To: Yun Wu, Marc Zyngier, tglx, jason; +Cc: linux-kernel, linux-arm-kernel
Hi Yun,
On 15/02/15 09:32, Yun Wu wrote:
> There is one more condition that needs to be considered when judging
> whether LPI feature is enabled or not, which is whether there is any
> ITS available and correctly enabled.
>
> This patch will fix this by caching ITS enabling status in the GIC
> chip data structure.
I posted patch for that before [1] and it landed in Marc's tree
(irq/gic-fixes). It is not clear from the commit message what the "one
more condition" is, but I guess it is the same dts stuff. Do you see
issue without your patch applied?
[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/314752.html
Thanks
Vladimir
>
> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
> ---
> drivers/irqchip/irq-gic-v3.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 1a146cc..e17faca 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -47,6 +47,7 @@ struct gic_chip_data {
> u64 redist_stride;
> u32 nr_redist_regions;
> unsigned int irq_nr;
> + int lpi_enabled;
> };
>
> static struct gic_chip_data gic_data __read_mostly;
> @@ -390,11 +391,6 @@ static void gic_cpu_sys_reg_init(void)
> gic_write_grpen1(1);
> }
>
> -static int gic_dist_supports_lpis(void)
> -{
> - return !!(readl_relaxed(gic_data.dist_base + GICD_TYPER) & GICD_TYPER_LPIS);
> -}
> -
> static void gic_cpu_init(void)
> {
> void __iomem *rbase;
> @@ -410,7 +406,7 @@ static void gic_cpu_init(void)
> gic_cpu_config(rbase, gic_redist_wait_for_rwp);
>
> /* Give LPIs a spin */
> - if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
> + if (gic_data.lpi_enabled)
> its_cpu_init();
>
> /* initialise system registers */
> @@ -629,7 +625,7 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
> }
> /* LPIs */
> if (hw >= 8192 && hw < GIC_ID_NR) {
> - if (!gic_dist_supports_lpis())
> + if (!gic_data.lpi_enabled)
> return -EPERM;
> irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
> handle_fasteoi_irq, NULL, NULL);
> @@ -785,8 +781,12 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
>
> set_handle_irq(gic_handle_irq);
>
> - if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
> - its_init(node, &gic_data.rdists, gic_data.domain);
> + if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) &&
> + !!(readl_relaxed(dist_base + GICD_TYPER) & GICD_TYPER_LPIS) &&
> + !its_init(node, &gic_data.rdists, gic_data.domain))
> + gic_data.lpi_enabled = 1;
> + else
> + gic_data.lpi_enabled = 0;
>
> gic_smp_init();
> gic_dist_init();
> --
> 1.8.0
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 6/6] irqchip: gicv3: skip ITS init when no ITS available
2015-02-16 10:05 ` Vladimir Murzin
@ 2015-02-16 14:57 ` Yun Wu (Abel)
0 siblings, 0 replies; 17+ messages in thread
From: Yun Wu (Abel) @ 2015-02-16 14:57 UTC (permalink / raw)
To: Vladimir Murzin; +Cc: Marc Zyngier, tglx, jason, linux-kernel, linux-arm-kernel
Hi Murzin,
On 2015/2/16 18:05, Vladimir Murzin wrote:
> Hi Yun,
>
> On 15/02/15 09:32, Yun Wu wrote:
>> There is one more condition that needs to be considered when judging
>> whether LPI feature is enabled or not, which is whether there is any
>> ITS available and correctly enabled.
>>
>> This patch will fix this by caching ITS enabling status in the GIC
>> chip data structure.
>
> I posted patch for that before [1] and it landed in Marc's tree
> (irq/gic-fixes). It is not clear from the commit message what the "one
> more condition" is, but I guess it is the same dts stuff. Do you see
> issue without your patch applied?
Oh yes, your patch perfectly solved this problem, so this one is no longer
needed. And sorry for not noticing your patch. :)
Thanks,
Abel
>
> [1]
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/314752.html
>
> Thanks
> Vladimir
>
>>
>> Signed-off-by: Yun Wu <wuyun.wu@huawei.com>
>> ---
>> drivers/irqchip/irq-gic-v3.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
>> index 1a146cc..e17faca 100644
>> --- a/drivers/irqchip/irq-gic-v3.c
>> +++ b/drivers/irqchip/irq-gic-v3.c
>> @@ -47,6 +47,7 @@ struct gic_chip_data {
>> u64 redist_stride;
>> u32 nr_redist_regions;
>> unsigned int irq_nr;
>> + int lpi_enabled;
>> };
>>
>> static struct gic_chip_data gic_data __read_mostly;
>> @@ -390,11 +391,6 @@ static void gic_cpu_sys_reg_init(void)
>> gic_write_grpen1(1);
>> }
>>
>> -static int gic_dist_supports_lpis(void)
>> -{
>> - return !!(readl_relaxed(gic_data.dist_base + GICD_TYPER) & GICD_TYPER_LPIS);
>> -}
>> -
>> static void gic_cpu_init(void)
>> {
>> void __iomem *rbase;
>> @@ -410,7 +406,7 @@ static void gic_cpu_init(void)
>> gic_cpu_config(rbase, gic_redist_wait_for_rwp);
>>
>> /* Give LPIs a spin */
>> - if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
>> + if (gic_data.lpi_enabled)
>> its_cpu_init();
>>
>> /* initialise system registers */
>> @@ -629,7 +625,7 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>> }
>> /* LPIs */
>> if (hw >= 8192 && hw < GIC_ID_NR) {
>> - if (!gic_dist_supports_lpis())
>> + if (!gic_data.lpi_enabled)
>> return -EPERM;
>> irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
>> handle_fasteoi_irq, NULL, NULL);
>> @@ -785,8 +781,12 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
>>
>> set_handle_irq(gic_handle_irq);
>>
>> - if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
>> - its_init(node, &gic_data.rdists, gic_data.domain);
>> + if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) &&
>> + !!(readl_relaxed(dist_base + GICD_TYPER) & GICD_TYPER_LPIS) &&
>> + !its_init(node, &gic_data.rdists, gic_data.domain))
>> + gic_data.lpi_enabled = 1;
>> + else
>> + gic_data.lpi_enabled = 0;
>>
>> gic_smp_init();
>> gic_dist_init();
>> --
>> 1.8.0
>>
^ permalink raw reply [flat|nested] 17+ messages in thread