[PATCH] irq_domain: Standardise legacy/linear domain selection

[PATCH] irq_domain: Standardise legacy/linear domain selection

[Mark Brown]

A large proportion of interrupt controllers that support legacy mappings
do so because non-DT systems need to use fixed IRQ numbers when registering
devices via buses but can otherwise use a linear mapping. The interrupt
controller itself typically is not affected by the mapping used and best
practice is to use a linear mapping where possible so drivers frequently
select at runtime depending on if a legacy range has been allocated to
them.

Standardise this behaviour by providing irq_domain_register_simple() which
will allocate a linear mapping unless a positive first_irq is provided in
which case it will fall back to a legacy mapping. This helps make best
practice for irq_domain adoption clearer.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 Documentation/IRQ-domain.txt |    5 +++++
 include/linux/irqdomain.h    |    5 +++++
 kernel/irq/irqdomain.c       |   30 ++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/Documentation/IRQ-domain.txt b/Documentation/IRQ-domain.txt
index 27dcaab..1401cec 100644
--- a/Documentation/IRQ-domain.txt
+++ b/Documentation/IRQ-domain.txt
@@ -93,6 +93,7 @@ Linux IRQ number into the hardware.
 Most drivers cannot use this mapping.
 
 ==== Legacy ====
+irq_domain_add_simple()
 irq_domain_add_legacy()
 irq_domain_add_legacy_isa()
 
@@ -115,3 +116,7 @@ The legacy map should only be used if fixed IRQ mappings must be
 supported.  For example, ISA controllers would use the legacy map for
 mapping Linux IRQs 0-15 so that existing ISA drivers get the correct IRQ
 numbers.
+
+Most users of legacy mappings should use irq_domain_add_simple() which
+will use a legacy domain only if an IRQ range is supplied by the
+system and will otherwise use a linear domain mapping.
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index c65740d..fcf4142 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -112,6 +112,11 @@ struct irq_domain {
 };
 
 #ifdef CONFIG_IRQ_DOMAIN
+struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
+					 unsigned int size,
+					 unsigned int first_irq,
+					 const struct irq_domain_ops *ops,
+					 void *host_data);
 struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
 					 unsigned int size,
 					 unsigned int first_irq,
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index d0e50eb..86ae10f 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -77,6 +77,36 @@ static unsigned int irq_domain_legacy_revmap(struct irq_domain *domain,
 }
 
 /**
+ * irq_domain_add_simple() - Allocate and register a simple irq_domain.
+ * @of_node: pointer to interrupt controller's device tree node.
+ * @size: total number of irqs in mapping
+ * @first_irq: first number of irq block assigned to the domain
+ * @ops: map/unmap domain callbacks
+ * @host_data: Controller private data pointer
+ *
+ * Allocates a legacy irq_domain if irq_base is positive or a linear
+ * domain otherwise.
+ *
+ * This is intended to implement the expected behaviour for most
+ * interrupt controllers which is that a linear mapping should
+ * normally be used unless the system requires a legacy mapping in
+ * order to support supplying interrupt numbers during non-DT
+ * registration of devices.
+ */
+struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
+					 unsigned int size,
+					 unsigned int first_irq,
+					 const struct irq_domain_ops *ops,
+					 void *host_data)
+{
+	if (first_irq > 0)
+		return irq_domain_add_legacy(of_node, size, first_irq, 0,
+					     ops, host_data);
+	else
+		return irq_domain_add_linear(of_node, size, ops, host_data);
+}
+
+/**
  * irq_domain_add_legacy() - Allocate and register a legacy revmap irq_domain.
  * @of_node: pointer to interrupt controller's device tree node.
  * @size: total number of irqs in legacy mapping
-- 
1.7.10

Re: [PATCH] irq_domain: Standardise legacy/linear domain selection

[Grant Likely]

On Sat, 19 May 2012 13:02:00 +0100, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> A large proportion of interrupt controllers that support legacy mappings
> do so because non-DT systems need to use fixed IRQ numbers when registering
> devices via buses but can otherwise use a linear mapping. The interrupt
> controller itself typically is not affected by the mapping used and best
> practice is to use a linear mapping where possible so drivers frequently
> select at runtime depending on if a legacy range has been allocated to
> them.
> 
> Standardise this behaviour by providing irq_domain_register_simple() which
> will allocate a linear mapping unless a positive first_irq is provided in
> which case it will fall back to a legacy mapping. This helps make best
> practice for irq_domain adoption clearer.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  Documentation/IRQ-domain.txt |    5 +++++
>  include/linux/irqdomain.h    |    5 +++++
>  kernel/irq/irqdomain.c       |   30 ++++++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+)

I'm looking at killing the legacy mapping entirely which will push
changing all the users anyway.  I'm going to hold off on applying this
one for a day or so until I'm sure of what it will look like.

g.

> 
> diff --git a/Documentation/IRQ-domain.txt b/Documentation/IRQ-domain.txt
> index 27dcaab..1401cec 100644
> --- a/Documentation/IRQ-domain.txt
> +++ b/Documentation/IRQ-domain.txt
> @@ -93,6 +93,7 @@ Linux IRQ number into the hardware.
>  Most drivers cannot use this mapping.
>  
>  ==== Legacy ====
> +irq_domain_add_simple()
>  irq_domain_add_legacy()
>  irq_domain_add_legacy_isa()
>  
> @@ -115,3 +116,7 @@ The legacy map should only be used if fixed IRQ mappings must be
>  supported.  For example, ISA controllers would use the legacy map for
>  mapping Linux IRQs 0-15 so that existing ISA drivers get the correct IRQ
>  numbers.
> +
> +Most users of legacy mappings should use irq_domain_add_simple() which
> +will use a legacy domain only if an IRQ range is supplied by the
> +system and will otherwise use a linear domain mapping.
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index c65740d..fcf4142 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -112,6 +112,11 @@ struct irq_domain {
>  };
>  
>  #ifdef CONFIG_IRQ_DOMAIN
> +struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
> +					 unsigned int size,
> +					 unsigned int first_irq,
> +					 const struct irq_domain_ops *ops,
> +					 void *host_data);
>  struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
>  					 unsigned int size,
>  					 unsigned int first_irq,
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index d0e50eb..86ae10f 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -77,6 +77,36 @@ static unsigned int irq_domain_legacy_revmap(struct irq_domain *domain,
>  }
>  
>  /**
> + * irq_domain_add_simple() - Allocate and register a simple irq_domain.
> + * @of_node: pointer to interrupt controller's device tree node.
> + * @size: total number of irqs in mapping
> + * @first_irq: first number of irq block assigned to the domain
> + * @ops: map/unmap domain callbacks
> + * @host_data: Controller private data pointer
> + *
> + * Allocates a legacy irq_domain if irq_base is positive or a linear
> + * domain otherwise.
> + *
> + * This is intended to implement the expected behaviour for most
> + * interrupt controllers which is that a linear mapping should
> + * normally be used unless the system requires a legacy mapping in
> + * order to support supplying interrupt numbers during non-DT
> + * registration of devices.
> + */
> +struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
> +					 unsigned int size,
> +					 unsigned int first_irq,
> +					 const struct irq_domain_ops *ops,
> +					 void *host_data)
> +{
> +	if (first_irq > 0)
> +		return irq_domain_add_legacy(of_node, size, first_irq, 0,
> +					     ops, host_data);
> +	else
> +		return irq_domain_add_linear(of_node, size, ops, host_data);
> +}
> +
> +/**
>   * irq_domain_add_legacy() - Allocate and register a legacy revmap irq_domain.
>   * @of_node: pointer to interrupt controller's device tree node.
>   * @size: total number of irqs in legacy mapping
> -- 
> 1.7.10
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH] irq_domain: Standardise legacy/linear domain selection

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

On Sat, May 19, 2012 at 02:22:58PM -0600, Grant Likely wrote:

> I'm looking at killing the legacy mapping entirely which will push
> changing all the users anyway.  I'm going to hold off on applying this
> one for a day or so until I'm sure of what it will look like.

I take it you're going to replace the legacy mapping with an equivalent
thing, though, so the interface is preserved for boards?  In which case
it'd be more like replacing the linear mapping with a new mapping type
that can also be used by legacy users I guess?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] irq_domain: Standardise legacy/linear domain selection

[Grant Likely]

On Sat, May 19, 2012 at 2:54 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Sat, May 19, 2012 at 02:22:58PM -0600, Grant Likely wrote:
>
>> I'm looking at killing the legacy mapping entirely which will push
>> changing all the users anyway.  I'm going to hold off on applying this
>> one for a day or so until I'm sure of what it will look like.
>
> I take it you're going to replace the legacy mapping with an equivalent
> thing, though, so the interface is preserved for boards?  In which case
> it'd be more like replacing the linear mapping with a new mapping type
> that can also be used by legacy users I guess?

It will actually be like all legacy users switched to creating an
linear mapping, and then either associating already allocated
irq_descs with that domain or telling the domain to pre-map a specific
range of irq_descs (and manage allocations).  Those two use cases will
be separate APIs.  I don't think they should be combined (at least
from my current thinking) because it depends on whether or not the
irq_descs are already allocated.

g.



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH] irq_domain: Standardise legacy/linear domain selection

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

On Sat, May 19, 2012 at 02:59:51PM -0600, Grant Likely wrote:

> It will actually be like all legacy users switched to creating an
> linear mapping, and then either associating already allocated
> irq_descs with that domain or telling the domain to pre-map a specific
> range of irq_descs (and manage allocations).  Those two use cases will
> be separate APIs.  I don't think they should be combined (at least
> from my current thinking) because it depends on whether or not the
> irq_descs are already allocated.

Hrm.  Given that the target for this API is drivers that mostly don't
care and are only supporting the current linear mappings in order to
allow their GPIOs to be used with generic drivers that only want an IRQ
I'd expect we'll wind up with a similar helper somehow.  Copying a
number around for irq_start isn't much hassle for individual drivers but
having to worry about more than that seems like too much effort, they
shouldn't have to care about the management of irq_descs.  It'd be more
work and it'd make it more painful for platforms to change between the
preallocated and dynamic models.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] irq_domain: Standardise legacy/linear domain selection

[Mark Brown]

A large proportion of interrupt controllers that support legacy mappings
do so because non-DT systems need to use fixed IRQ numbers when registering
devices via buses but can otherwise use a linear mapping. The interrupt
controller itself typically is not affected by the mapping used and best
practice is to use a linear mapping where possible so drivers frequently
select at runtime depending on if a legacy range has been allocated to
them.

Standardise this behaviour by providing irq_domain_register_simple() which
will allocate a linear mapping unless a positive first_irq is provided in
which case it will fall back to a legacy mapping. This helps make best
practice for irq_domain adoption clearer.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 Documentation/IRQ-domain.txt |    5 +++++
 include/linux/irqdomain.h    |    5 +++++
 kernel/irq/irqdomain.c       |   30 ++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/Documentation/IRQ-domain.txt b/Documentation/IRQ-domain.txt
index 27dcaab..1401cec 100644
--- a/Documentation/IRQ-domain.txt
+++ b/Documentation/IRQ-domain.txt
@@ -93,6 +93,7 @@ Linux IRQ number into the hardware.
 Most drivers cannot use this mapping.
 
 ==== Legacy ====
+irq_domain_add_simple()
 irq_domain_add_legacy()
 irq_domain_add_legacy_isa()
 
@@ -115,3 +116,7 @@ The legacy map should only be used if fixed IRQ mappings must be
 supported.  For example, ISA controllers would use the legacy map for
 mapping Linux IRQs 0-15 so that existing ISA drivers get the correct IRQ
 numbers.
+
+Most users of legacy mappings should use irq_domain_add_simple() which
+will use a legacy domain only if an IRQ range is supplied by the
+system and will otherwise use a linear domain mapping.
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 5abb533e..17b60be 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -112,6 +112,11 @@ struct irq_domain {
 };
 
 #ifdef CONFIG_IRQ_DOMAIN
+struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
+					 unsigned int size,
+					 unsigned int first_irq,
+					 const struct irq_domain_ops *ops,
+					 void *host_data);
 struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
 					 unsigned int size,
 					 unsigned int first_irq,
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index d3968e9..0c51958 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -140,6 +140,36 @@ static unsigned int irq_domain_legacy_revmap(struct irq_domain *domain,
 }
 
 /**
+ * irq_domain_add_simple() - Allocate and register a simple irq_domain.
+ * @of_node: pointer to interrupt controller's device tree node.
+ * @size: total number of irqs in mapping
+ * @first_irq: first number of irq block assigned to the domain
+ * @ops: map/unmap domain callbacks
+ * @host_data: Controller private data pointer
+ *
+ * Allocates a legacy irq_domain if irq_base is positive or a linear
+ * domain otherwise.
+ *
+ * This is intended to implement the expected behaviour for most
+ * interrupt controllers which is that a linear mapping should
+ * normally be used unless the system requires a legacy mapping in
+ * order to support supplying interrupt numbers during non-DT
+ * registration of devices.
+ */
+struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
+					 unsigned int size,
+					 unsigned int first_irq,
+					 const struct irq_domain_ops *ops,
+					 void *host_data)
+{
+	if (first_irq > 0)
+		return irq_domain_add_legacy(of_node, size, first_irq, 0,
+					     ops, host_data);
+	else
+		return irq_domain_add_linear(of_node, size, ops, host_data);
+}
+
+/**
  * irq_domain_add_legacy() - Allocate and register a legacy revmap irq_domain.
  * @of_node: pointer to interrupt controller's device tree node.
  * @size: total number of irqs in legacy mapping
-- 
1.7.10

Re: [PATCH] irq_domain: Standardise legacy/linear domain selection

[Andy Shevchenko]

On Thu, Jul 5, 2012 at 2:19 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> A large proportion of interrupt controllers that support legacy mappings
> do so because non-DT systems need to use fixed IRQ numbers when registering
> devices via buses but can otherwise use a linear mapping. The interrupt
> controller itself typically is not affected by the mapping used and best
> practice is to use a linear mapping where possible so drivers frequently
> select at runtime depending on if a legacy range has been allocated to
> them.
>
> Standardise this behaviour by providing irq_domain_register_simple() which
> will allocate a linear mapping unless a positive first_irq is provided in
> which case it will fall back to a legacy mapping. This helps make best
> practice for irq_domain adoption clearer.

We have a hardware which doesn't support ACPI nor DT. We had got a nice
issue with irq domains after the gpio driver has been moved to the
linear scheme.
Our internal discussion is roughly about misdesigned irq domains
concept for that
kind of hardware. In generic case we could have more than one PIC, and more than
one hardware that requires real 1:1 mapping of the irq numbers, and
kernel doesn't
know about the last mapping until real driver module is loaded (for
example a PCI
controller of something).

The issue is in plain array of the numbers that are assigned to the devices.
Somehow looks better to have real namespaces, or even hide irq number in the API
struct device, request_irq(), but keep reference between driver and PIC via some
object.

So, given solution just hides an issue, but doesn't resolve it fully
from my p.o.v.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] irq_domain: Standardise legacy/linear domain selection

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

On Thu, Jul 05, 2012 at 02:43:42PM +0300, Andy Shevchenko wrote:

> The issue is in plain array of the numbers that are assigned to the devices.
> Somehow looks better to have real namespaces, or even hide irq number in the API
> struct device, request_irq(), but keep reference between driver and PIC via some
> object.

> So, given solution just hides an issue, but doesn't resolve it fully
> from my p.o.v.

This is unrelated to what you're talking about.  The devices concerned
are mostly MFDs which use their own interrupts.  Where other things need
to use the interrupt numbers then legacy mappings should still be used
and IRQ domains have no effect at all on the situation.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] irq_domain: Standardise legacy/linear domain selection

[Grant Likely]

On Thu,  5 Jul 2012 12:19:19 +0100, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> A large proportion of interrupt controllers that support legacy mappings
> do so because non-DT systems need to use fixed IRQ numbers when registering
> devices via buses but can otherwise use a linear mapping. The interrupt
> controller itself typically is not affected by the mapping used and best
> practice is to use a linear mapping where possible so drivers frequently
> select at runtime depending on if a legacy range has been allocated to
> them.
> 
> Standardise this behaviour by providing irq_domain_register_simple() which
> will allocate a linear mapping unless a positive first_irq is provided in
> which case it will fall back to a legacy mapping. This helps make best
> practice for irq_domain adoption clearer.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Applied, thanks.

g.

> ---
>  Documentation/IRQ-domain.txt |    5 +++++
>  include/linux/irqdomain.h    |    5 +++++
>  kernel/irq/irqdomain.c       |   30 ++++++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/Documentation/IRQ-domain.txt b/Documentation/IRQ-domain.txt
> index 27dcaab..1401cec 100644
> --- a/Documentation/IRQ-domain.txt
> +++ b/Documentation/IRQ-domain.txt
> @@ -93,6 +93,7 @@ Linux IRQ number into the hardware.
>  Most drivers cannot use this mapping.
>  
>  ==== Legacy ====
> +irq_domain_add_simple()
>  irq_domain_add_legacy()
>  irq_domain_add_legacy_isa()
>  
> @@ -115,3 +116,7 @@ The legacy map should only be used if fixed IRQ mappings must be
>  supported.  For example, ISA controllers would use the legacy map for
>  mapping Linux IRQs 0-15 so that existing ISA drivers get the correct IRQ
>  numbers.
> +
> +Most users of legacy mappings should use irq_domain_add_simple() which
> +will use a legacy domain only if an IRQ range is supplied by the
> +system and will otherwise use a linear domain mapping.
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 5abb533e..17b60be 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -112,6 +112,11 @@ struct irq_domain {
>  };
>  
>  #ifdef CONFIG_IRQ_DOMAIN
> +struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
> +					 unsigned int size,
> +					 unsigned int first_irq,
> +					 const struct irq_domain_ops *ops,
> +					 void *host_data);
>  struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
>  					 unsigned int size,
>  					 unsigned int first_irq,
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index d3968e9..0c51958 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -140,6 +140,36 @@ static unsigned int irq_domain_legacy_revmap(struct irq_domain *domain,
>  }
>  
>  /**
> + * irq_domain_add_simple() - Allocate and register a simple irq_domain.
> + * @of_node: pointer to interrupt controller's device tree node.
> + * @size: total number of irqs in mapping
> + * @first_irq: first number of irq block assigned to the domain
> + * @ops: map/unmap domain callbacks
> + * @host_data: Controller private data pointer
> + *
> + * Allocates a legacy irq_domain if irq_base is positive or a linear
> + * domain otherwise.
> + *
> + * This is intended to implement the expected behaviour for most
> + * interrupt controllers which is that a linear mapping should
> + * normally be used unless the system requires a legacy mapping in
> + * order to support supplying interrupt numbers during non-DT
> + * registration of devices.
> + */
> +struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
> +					 unsigned int size,
> +					 unsigned int first_irq,
> +					 const struct irq_domain_ops *ops,
> +					 void *host_data)
> +{
> +	if (first_irq > 0)
> +		return irq_domain_add_legacy(of_node, size, first_irq, 0,
> +					     ops, host_data);
> +	else
> +		return irq_domain_add_linear(of_node, size, ops, host_data);
> +}
> +
> +/**
>   * irq_domain_add_legacy() - Allocate and register a legacy revmap irq_domain.
>   * @of_node: pointer to interrupt controller's device tree node.
>   * @size: total number of irqs in legacy mapping
> -- 
> 1.7.10
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.