1.Patch#1 fix an issue where the Linux IRQ number is not stored;
2.Patch#2 submit a code optimization to replace revmap_direct_max_irq
field with hwirq_max field;
v2:
1. Modify the commit log to make it easier for developers to understand in patch#1;
2. Modify a compile warning in Patch#2;
Xu Qiang (2):
irqdomain: Fix an issue where the Linux IRQ number is not stored
irqdomain: Replace revmap_direct_max_irq field with hwirq_max field
kernel/irq/irqdomain.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
--
2.17.1
In commit "4f86a06e2d6e irqdomain: Make normal and nomap irqdomains exclusive",
use revmap_size field instead of revmap_direct_max_irq. revmap_size field
originally indicates the maximum hwirq of linear Mapping. This results in
revmap_size having two different layers of meaning that can be confusing.
This patch optimization point is to solve this confusion point. During
direct mapping, the values of hwirq_max and revmap_direct_max_irq are the same
and have the same meanings. They both indicate the maximum hwirq supported by
direct Mapping. The optimization method is to delete revmap_direct_max_irq
field and use hwirq_max instead of revmap_direct_max_irq.
Signed-off-by: Xu Qiang <[email protected]>
---
kernel/irq/irqdomain.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 481abb885d61..8fe1da9614ee 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -147,7 +147,8 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
static atomic_t unknown_domains;
if (WARN_ON((size && direct_max) ||
- (!IS_ENABLED(CONFIG_IRQ_DOMAIN_NOMAP) && direct_max)))
+ (!IS_ENABLED(CONFIG_IRQ_DOMAIN_NOMAP) && direct_max) ||
+ (direct_max && (direct_max != hwirq_max))))
return NULL;
domain = kzalloc_node(struct_size(domain, revmap, size),
@@ -219,7 +220,6 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
domain->hwirq_max = hwirq_max;
if (direct_max) {
- size = direct_max;
domain->flags |= IRQ_DOMAIN_FLAG_NO_MAP;
}
@@ -650,9 +650,9 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
pr_debug("create_direct virq allocation failed\n");
return 0;
}
- if (virq >= domain->revmap_size) {
- pr_err("ERROR: no free irqs available below %i maximum\n",
- domain->revmap_size);
+ if (virq >= domain->hwirq_max) {
+ pr_err("ERROR: no free irqs available below %lu maximum\n",
+ domain->hwirq_max);
irq_free_desc(virq);
return 0;
}
@@ -906,7 +906,7 @@ struct irq_desc *__irq_resolve_mapping(struct irq_domain *domain,
return desc;
if (irq_domain_is_nomap(domain)) {
- if (hwirq < domain->revmap_size) {
+ if (hwirq < domain->hwirq_max) {
data = irq_domain_get_irq_data(domain, hwirq);
if (data && data->hwirq == hwirq)
desc = irq_data_to_desc(data);
--
2.17.1
When using a NOMAP domain, __irq_resolve_mapping() doesn't store
the Linux IRQ number at the address optionally provided by the caller.
While this isn't a huge deal (the returned value is guaranteed
to the hwirq that was passed as a parameter), let's honour the letter
of the API by writing the expected value.
Fixes: d22558dd0a6c (“irqdomain: Introduce irq_resolve_mapping()”)
Signed-off-by: Xu Qiang <[email protected]>
---
kernel/irq/irqdomain.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index d5ce96510549..481abb885d61 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -910,6 +910,8 @@ struct irq_desc *__irq_resolve_mapping(struct irq_domain *domain,
data = irq_domain_get_irq_data(domain, hwirq);
if (data && data->hwirq == hwirq)
desc = irq_data_to_desc(data);
+ if (irq && desc)
+ *irq = hwirq;
}
return desc;
--
2.17.1
On Tue, 19 Jul 2022 07:36:41 +0100,
Xu Qiang <[email protected]> wrote:
>
> In commit "4f86a06e2d6e irqdomain: Make normal and nomap irqdomains exclusive",
> use revmap_size field instead of revmap_direct_max_irq. revmap_size field
> originally indicates the maximum hwirq of linear Mapping. This results in
> revmap_size having two different layers of meaning that can be confusing.
>
> This patch optimization point is to solve this confusion point. During
> direct mapping, the values of hwirq_max and revmap_direct_max_irq are the same
> and have the same meanings. They both indicate the maximum hwirq supported by
> direct Mapping. The optimization method is to delete revmap_direct_max_irq
> field and use hwirq_max instead of revmap_direct_max_irq.
>
> Signed-off-by: Xu Qiang <[email protected]>
You keep referencing revmap_direct_max_irq, which is long gone.
How about instead:
<commit>
NOMAP irq domains use the revmap_size field to indicate the maximum
hwirq number the domain accepts. This is a bit confusing as
revmap_size is usually used to indicate the size of the revmap array,
which a NOMAP domain doesn't have.
Instead, use the hwirq_max field which has the correct semantics, and
keep revmap_size to 0 for a NOMAP domain.
</commit>
If you agree with this, please say so (no need to respin for this).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
$B:_(B 2022/7/19 18:24, Marc Zyngier $B<LF;(B:
> On Tue, 19 Jul 2022 07:36:41 +0100,
> Xu Qiang <[email protected]> wrote:
>> In commit "4f86a06e2d6e irqdomain: Make normal and nomap irqdomains exclusive",
>> use revmap_size field instead of revmap_direct_max_irq. revmap_size field
>> originally indicates the maximum hwirq of linear Mapping. This results in
>> revmap_size having two different layers of meaning that can be confusing.
>>
>> This patch optimization point is to solve this confusion point. During
>> direct mapping, the values of hwirq_max and revmap_direct_max_irq are the same
>> and have the same meanings. They both indicate the maximum hwirq supported by
>> direct Mapping. The optimization method is to delete revmap_direct_max_irq
>> field and use hwirq_max instead of revmap_direct_max_irq.
>>
>> Signed-off-by: Xu Qiang <[email protected]>
> You keep referencing revmap_direct_max_irq, which is long gone.
>
> How about instead:
>
> <commit>
> NOMAP irq domains use the revmap_size field to indicate the maximum
> hwirq number the domain accepts. This is a bit confusing as
> revmap_size is usually used to indicate the size of the revmap array,
> which a NOMAP domain doesn't have.
>
> Instead, use the hwirq_max field which has the correct semantics, and
> keep revmap_size to 0 for a NOMAP domain.
> </commit>
>
> If you agree with this, please say so (no need to respin for this).
Yes, I agree.
Thanks,
Xu.
>
> Thanks,
>
> M.
>
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: 6f194c99f466147148cc08452718b46664112548
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/6f194c99f466147148cc08452718b46664112548
Author: Xu Qiang <[email protected]>
AuthorDate: Tue, 19 Jul 2022 06:36:40
Committer: Marc Zyngier <[email protected]>
CommitterDate: Tue, 19 Jul 2022 14:51:13 +01:00
irqdomain: Report irq number for NOMAP domains
When using a NOMAP domain, __irq_resolve_mapping() doesn't store
the Linux IRQ number at the address optionally provided by the caller.
While this isn't a huge deal (the returned value is guaranteed
to the hwirq that was passed as a parameter), let's honour the letter
of the API by writing the expected value.
Fixes: d22558dd0a6c (“irqdomain: Introduce irq_resolve_mapping()”)
Signed-off-by: Xu Qiang <[email protected]>
[maz: commit message]
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/irq/irqdomain.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index d5ce965..481abb8 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -910,6 +910,8 @@ struct irq_desc *__irq_resolve_mapping(struct irq_domain *domain,
data = irq_domain_get_irq_data(domain, hwirq);
if (data && data->hwirq == hwirq)
desc = irq_data_to_desc(data);
+ if (irq && desc)
+ *irq = hwirq;
}
return desc;
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: ef50cd57a73a8bbfad403e5e2edb3309611f58ad
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/ef50cd57a73a8bbfad403e5e2edb3309611f58ad
Author: Xu Qiang <[email protected]>
AuthorDate: Tue, 19 Jul 2022 06:36:41
Committer: Marc Zyngier <[email protected]>
CommitterDate: Tue, 19 Jul 2022 14:51:56 +01:00
irqdomain: Use hwirq_max instead of revmap_size for NOMAP domains
NOMAP irq domains use the revmap_size field to indicate the maximum
hwirq number the domain accepts. This is a bit confusing as
revmap_size is usually used to indicate the size of the revmap array,
which a NOMAP domain doesn't have.
Instead, use the hwirq_max field which has the correct semantics, and
keep revmap_size to 0 for a NOMAP domain.
Signed-off-by: Xu Qiang <[email protected]>
[maz: commit message]
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/irq/irqdomain.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 481abb8..8fe1da9 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -147,7 +147,8 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
static atomic_t unknown_domains;
if (WARN_ON((size && direct_max) ||
- (!IS_ENABLED(CONFIG_IRQ_DOMAIN_NOMAP) && direct_max)))
+ (!IS_ENABLED(CONFIG_IRQ_DOMAIN_NOMAP) && direct_max) ||
+ (direct_max && (direct_max != hwirq_max))))
return NULL;
domain = kzalloc_node(struct_size(domain, revmap, size),
@@ -219,7 +220,6 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
domain->hwirq_max = hwirq_max;
if (direct_max) {
- size = direct_max;
domain->flags |= IRQ_DOMAIN_FLAG_NO_MAP;
}
@@ -650,9 +650,9 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
pr_debug("create_direct virq allocation failed\n");
return 0;
}
- if (virq >= domain->revmap_size) {
- pr_err("ERROR: no free irqs available below %i maximum\n",
- domain->revmap_size);
+ if (virq >= domain->hwirq_max) {
+ pr_err("ERROR: no free irqs available below %lu maximum\n",
+ domain->hwirq_max);
irq_free_desc(virq);
return 0;
}
@@ -906,7 +906,7 @@ struct irq_desc *__irq_resolve_mapping(struct irq_domain *domain,
return desc;
if (irq_domain_is_nomap(domain)) {
- if (hwirq < domain->revmap_size) {
+ if (hwirq < domain->hwirq_max) {
data = irq_domain_get_irq_data(domain, hwirq);
if (data && data->hwirq == hwirq)
desc = irq_data_to_desc(data);