diff mbox

[PATCHv5,01/11] irqdomain: add irq_alloc_mapping() function

Message ID 1373889167-27878-2-git-send-email-thomas.petazzoni@free-electrons.com
State Not Applicable
Headers show

Commit Message

Thomas Petazzoni July 15, 2013, 11:52 a.m. UTC
This commit extends the irqdomain subsystem with an
irq_alloc_mapping() function which allows to let the irqdomain code
find an available hwirq number in the range [ 0 ; domain size ] for
the given domain, and create a virq mapping for it.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 include/linux/irqdomain.h |  2 ++
 kernel/irq/irqdomain.c    | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

Comments

Thomas Petazzoni July 16, 2013, 8:31 a.m. UTC | #1
Grant,

Would it be possible to get your opinion on the below patch? We have
already discussed it in the past, and it was implemented according to
your suggestions, so I guess it should be fine, but I'd like to have
your formal Acked-by if possible, or additional comments. It is needed
for the MSI support on Marvell PCIe, but now the Tegra people are also
interested by it, and we hope to get this merged in 3.12.

Thanks!

Thomas

On Mon, 15 Jul 2013 13:52:37 +0200, Thomas Petazzoni wrote:
> This commit extends the irqdomain subsystem with an
> irq_alloc_mapping() function which allows to let the irqdomain code
> find an available hwirq number in the range [ 0 ; domain size ] for
> the given domain, and create a virq mapping for it.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  include/linux/irqdomain.h |  2 ++
>  kernel/irq/irqdomain.c    | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index c983ed1..1ffa336 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -195,6 +195,8 @@ static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
>  extern unsigned int irq_find_mapping(struct irq_domain *host,
>  				     irq_hw_number_t hwirq);
>  extern unsigned int irq_create_direct_mapping(struct irq_domain *host);
> +extern unsigned int irq_alloc_mapping(struct irq_domain *host,
> +				      irq_hw_number_t *hwirq);
>  extern int irq_create_strict_mappings(struct irq_domain *domain,
>  				      unsigned int irq_base,
>  				      irq_hw_number_t hwirq_base, int count);
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 706724e..b9ddb94 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -375,6 +375,38 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
>  EXPORT_SYMBOL_GPL(irq_create_direct_mapping);
>  
>  /**
> + * irq_alloc_mapping() - Allocate an irq for mapping
> + * @domain: domain to allocate the irq for or NULL for default domain
> + * @hwirq:  reference to the returned hwirq
> + *
> + * This routine are used for irq controllers which can choose the
> + * hardware interrupt number from a range [ 0 ; domain size ], such as
> + * is often the case with PCI MSI controllers. The function will
> + * returned the allocated hwirq number in the hwirq pointer, and the
> + * corresponding virq number as the return value.
> + */
> +unsigned int irq_alloc_mapping(struct irq_domain *domain,
> +			       irq_hw_number_t *out_hwirq)
> +{
> +	irq_hw_number_t hwirq;
> +
> +	pr_debug("irq_alloc_mapping(0x%p)\n", domain);
> +
> +	for (hwirq = 0; hwirq < domain->hwirq_max; hwirq++)
> +		if (!irq_find_mapping(domain, hwirq))
> +			break;
> +
> +	if (hwirq == domain->hwirq_max) {
> +		pr_debug("-> no available hwirq found\n");
> +		return 0;
> +	}
> +
> +	*out_hwirq = hwirq;
> +	return irq_create_mapping(domain, hwirq);
> +}
> +EXPORT_SYMBOL_GPL(irq_alloc_mapping);
> +
> +/**
>   * irq_create_mapping() - Map a hardware interrupt into linux irq space
>   * @domain: domain owning this hardware interrupt or NULL for default domain
>   * @hwirq: hardware irq number in that domain space
Grant Likely July 28, 2013, 4:11 a.m. UTC | #2
On Mon, 15 Jul 2013 13:52:37 +0200, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> This commit extends the irqdomain subsystem with an
> irq_alloc_mapping() function which allows to let the irqdomain code
> find an available hwirq number in the range [ 0 ; domain size ] for
> the given domain, and create a virq mapping for it.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  include/linux/irqdomain.h |  2 ++
>  kernel/irq/irqdomain.c    | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index c983ed1..1ffa336 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -195,6 +195,8 @@ static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
>  extern unsigned int irq_find_mapping(struct irq_domain *host,
>  				     irq_hw_number_t hwirq);
>  extern unsigned int irq_create_direct_mapping(struct irq_domain *host);
> +extern unsigned int irq_alloc_mapping(struct irq_domain *host,
> +				      irq_hw_number_t *hwirq);
>  extern int irq_create_strict_mappings(struct irq_domain *domain,
>  				      unsigned int irq_base,
>  				      irq_hw_number_t hwirq_base, int count);
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 706724e..b9ddb94 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -375,6 +375,38 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
>  EXPORT_SYMBOL_GPL(irq_create_direct_mapping);
>  
>  /**
> + * irq_alloc_mapping() - Allocate an irq for mapping
> + * @domain: domain to allocate the irq for or NULL for default domain
> + * @hwirq:  reference to the returned hwirq
> + *
> + * This routine are used for irq controllers which can choose the
> + * hardware interrupt number from a range [ 0 ; domain size ], such as
> + * is often the case with PCI MSI controllers. The function will
> + * returned the allocated hwirq number in the hwirq pointer, and the
> + * corresponding virq number as the return value.
> + */
> +unsigned int irq_alloc_mapping(struct irq_domain *domain,
> +			       irq_hw_number_t *out_hwirq)
> +{
> +	irq_hw_number_t hwirq;
> +
> +	pr_debug("irq_alloc_mapping(0x%p)\n", domain);
> +
> +	for (hwirq = 0; hwirq < domain->hwirq_max; hwirq++)
> +		if (!irq_find_mapping(domain, hwirq))
> +			break;

Okay, you can add my acked-by, but only if you change the above hunk to:

	for (hwirq = 0; hwirq < domain->revmap_size; hwirq++)
		if (domain->linear_revmap[hwirq] == 0)
			break;

In some cases hwirq_max will be set to ~0, which means calling
irq_find_mapping about 4 billion times. Not safe. The above change
prevents that problem, but it also means the function won't work for the
tree map. A followup patch can be crafted to add support for the tree
mapping.

> +
> +	if (hwirq == domain->hwirq_max) {
> +		pr_debug("-> no available hwirq found\n");
> +		return 0;
> +	}
> +
> +	*out_hwirq = hwirq;
> +	return irq_create_mapping(domain, hwirq);

out_hwirq should *not* be modified if irq_create_mapping fails. You'll
need to rework the above to:

	rc = irq_create_mapping(domain, hwirq);
	if (rc)
		*out_hwirq = hwirq;
	return rc;

When the above two issues are fixed:

Acked-by: Grant Likely <grant.likely@linaro.org>

> +}
> +EXPORT_SYMBOL_GPL(irq_alloc_mapping);
> +
> +/**
>   * irq_create_mapping() - Map a hardware interrupt into linux irq space
>   * @domain: domain owning this hardware interrupt or NULL for default domain
>   * @hwirq: hardware irq number in that domain space
> -- 
> 1.8.1.2
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index c983ed1..1ffa336 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -195,6 +195,8 @@  static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
 extern unsigned int irq_find_mapping(struct irq_domain *host,
 				     irq_hw_number_t hwirq);
 extern unsigned int irq_create_direct_mapping(struct irq_domain *host);
+extern unsigned int irq_alloc_mapping(struct irq_domain *host,
+				      irq_hw_number_t *hwirq);
 extern int irq_create_strict_mappings(struct irq_domain *domain,
 				      unsigned int irq_base,
 				      irq_hw_number_t hwirq_base, int count);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 706724e..b9ddb94 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -375,6 +375,38 @@  unsigned int irq_create_direct_mapping(struct irq_domain *domain)
 EXPORT_SYMBOL_GPL(irq_create_direct_mapping);
 
 /**
+ * irq_alloc_mapping() - Allocate an irq for mapping
+ * @domain: domain to allocate the irq for or NULL for default domain
+ * @hwirq:  reference to the returned hwirq
+ *
+ * This routine are used for irq controllers which can choose the
+ * hardware interrupt number from a range [ 0 ; domain size ], such as
+ * is often the case with PCI MSI controllers. The function will
+ * returned the allocated hwirq number in the hwirq pointer, and the
+ * corresponding virq number as the return value.
+ */
+unsigned int irq_alloc_mapping(struct irq_domain *domain,
+			       irq_hw_number_t *out_hwirq)
+{
+	irq_hw_number_t hwirq;
+
+	pr_debug("irq_alloc_mapping(0x%p)\n", domain);
+
+	for (hwirq = 0; hwirq < domain->hwirq_max; hwirq++)
+		if (!irq_find_mapping(domain, hwirq))
+			break;
+
+	if (hwirq == domain->hwirq_max) {
+		pr_debug("-> no available hwirq found\n");
+		return 0;
+	}
+
+	*out_hwirq = hwirq;
+	return irq_create_mapping(domain, hwirq);
+}
+EXPORT_SYMBOL_GPL(irq_alloc_mapping);
+
+/**
  * irq_create_mapping() - Map a hardware interrupt into linux irq space
  * @domain: domain owning this hardware interrupt or NULL for default domain
  * @hwirq: hardware irq number in that domain space