[PATCH v2 -next 0/2] Fix a bug and commit a code optimization

[PATCH v2 -next 0/2] Fix a bug and commit a code optimization

[Xu Qiang]

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

[PATCH v2 -next 1/2] irqdomain: Fix an issue where the Linux IRQ number is not stored

[Xu Qiang]

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 <xuqiang36@huawei.com>
---
 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

[PATCH v2 -next 2/2] irqdomain: Replace revmap_direct_max_irq field with hwirq_max field

[Xu Qiang]

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 <xuqiang36@huawei.com>
---
 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

Re: [PATCH v2 -next 2/2] irqdomain: Replace revmap_direct_max_irq field with hwirq_max field

[Marc Zyngier]

On Tue, 19 Jul 2022 07:36:41 +0100,
Xu Qiang <xuqiang36@huawei.com> 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 <xuqiang36@huawei.com>

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.

Re: [PATCH v2 -next 2/2] irqdomain: Replace revmap_direct_max_irq field with hwirq_max field

[xuqiang (M)]

在 2022/7/19 18:24, Marc Zyngier 写道:
> On Tue, 19 Jul 2022 07:36:41 +0100,
> Xu Qiang <xuqiang36@huawei.com> 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 <xuqiang36@huawei.com>
> 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.
>

[irqchip: irq/irqchip-next] irqdomain: Use hwirq_max instead of revmap_size for NOMAP domains

[irqchip-bot for Xu Qiang]

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 <xuqiang36@huawei.com>
AuthorDate:    Tue, 19 Jul 2022 06:36:41 
Committer:     Marc Zyngier <maz@kernel.org>
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 <xuqiang36@huawei.com>
[maz: commit message]
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20220719063641.56541-3-xuqiang36@huawei.com
---
 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);

[irqchip: irq/irqchip-next] irqdomain: Report irq number for NOMAP domains

[irqchip-bot for Xu Qiang]

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 <xuqiang36@huawei.com>
AuthorDate:    Tue, 19 Jul 2022 06:36:40 
Committer:     Marc Zyngier <maz@kernel.org>
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 <xuqiang36@huawei.com>
[maz: commit message]
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20220719063641.56541-2-xuqiang36@huawei.com
---
 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;