* [RFC 1/2] irq_domain: Create common xlate functions that device drivers can use
@ 2012-01-25 0:18 Grant Likely
2012-01-25 1:50 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Grant Likely @ 2012-01-25 0:18 UTC (permalink / raw)
To: linux-arm-kernel
Rather than having each interrupt controller driver creating its own barely
unique .xlate function for irq_domain, create a library of translators which
any driver can use directly.
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
This builds on top of my irq_domain series and will be included with it when
I post v3. Rob, does this serve your needs for generic irq controller DT
support?
g.
include/linux/irqdomain.h | 15 ++++++++++
kernel/irq/irqdomain.c | 69 +++++++++++++++++++++++++++++++++++++++-----
2 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index e7379a3..5e497a0 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -110,6 +110,9 @@ struct irq_domain {
void *host_data;
irq_hw_number_t inval_irq;
+ /* Data for common irq xlate functions */
+ unsigned int xlate_type;
+
/* Optional device node pointer */
struct device_node *of_node;
};
@@ -163,6 +166,18 @@ extern unsigned int irq_linear_revmap(struct irq_domain *host,
irq_hw_number_t hwirq);
extern struct irq_domain_ops irq_domain_simple_ops;
+
+/* stock xlate functions */
+int irq_domain_xlate_onecell(struct irq_domain *d, struct device_node *ctrlr,
+ const u32 *intspec, unsigned int intsize,
+ irq_hw_number_t *out_hwirq, unsigned int *out_type);
+int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
+ const u32 *intspec, unsigned int intsize,
+ irq_hw_number_t *out_hwirq, unsigned int *out_type);
+int irq_domain_xlate_onetwocell(struct irq_domain *d, struct device_node *ctrlr,
+ const u32 *intspec, unsigned int intsize,
+ irq_hw_number_t *out_hwirq, unsigned int *out_type);
+
#if defined(CONFIG_OF_IRQ)
extern void irq_domain_generate_simple(const struct of_device_id *match,
u64 phys_base, unsigned int irq_start);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index def8e7b..df80205 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -53,6 +53,7 @@ static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
domain->ops = ops;
domain->host_data = host_data;
domain->of_node = of_node_get(of_node);
+ domain->xlate_type = IRQ_TYPE_NONE;
if (domain->ops->match == NULL)
domain->ops->match = default_irq_domain_match;
@@ -690,25 +691,77 @@ int irq_domain_simple_map(struct irq_domain *d, unsigned int irq,
return 0;
}
-int irq_domain_simple_xlate(struct irq_domain *d,
- struct device_node *controller,
- const u32 *intspec, unsigned int intsize,
- unsigned long *out_hwirq, unsigned int *out_type)
+/**
+ * irq_domain_xlate_onecell() - Generic xlate for direct one cell bindings
+ *
+ * Device Tree IRQ specifier translation function which works with one cell
+ * bindings where the cell value maps directly to the hwirq number.
+ */
+int irq_domain_xlate_onecell(struct irq_domain *d, struct device_node *ctrlr,
+ const u32 *intspec, unsigned int intsize,
+ unsigned long *out_hwirq, unsigned int *out_type)
+{
+ if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
+ ctrlr->full_name, intsize))
+ return -EINVAL;
+ *out_hwirq = intspec[0];
+ *out_type = d->xlate_type;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(irq_domain_xlate_onecell);
+
+/**
+ * irq_domain_xlate_twocell() - Generic xlate for direct two cell bindings
+ *
+ * Device Tree IRQ specifier translation function which works with two cell
+ * bindings where the cell values map directly to the hwirq number
+ * and linux irq flags.
+ */
+int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
+ const u32 *intspec, unsigned int intsize,
+ irq_hw_number_t *out_hwirq, unsigned int *out_type)
{
- if (d->of_node != controller)
+ if (WARN(intsize < 2, "Bad intspec for %s: intsize=%i < 2\n",
+ ctrlr->full_name, intsize))
+ return -EINVAL;
+ if (intsize < 2)
return -EINVAL;
- if (intsize < 1)
+ *out_hwirq = intspec[0];
+ *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
+
+/**
+ * irq_domain_xlate_onetwocell() - Generic xlate for one or two cell bindings
+ *
+ * Device Tree IRQ specifier translation function which works with either one
+ * or two cell bindings where the cell values map directly to the hwirq number
+ * and linux irq flags.
+ *
+ * Note: don't use this function unless your interrupt controller explicitly
+ * supports both one and two cell bindings. For the majority of controllers
+ * the _onecell() or _twocell() variants above should be used.
+ */
+int irq_domain_xlate_onetwocell(struct irq_domain *d,
+ struct device_node *ctrlr,
+ const u32 *intspec, unsigned int intsize,
+ unsigned long *out_hwirq, unsigned int *out_type)
+{
+ if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
+ ctrlr->full_name, intsize))
return -EINVAL;
*out_hwirq = intspec[0];
- *out_type = IRQ_TYPE_NONE;
+ *out_type = d->xlate_type;
if (intsize > 1)
*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
return 0;
}
+EXPORT_SYMBOL_GPL(irq_domain_xlate_onetwocell);
struct irq_domain_ops irq_domain_simple_ops = {
.map = irq_domain_simple_map,
- .xlate = irq_domain_simple_xlate,
+ .xlate = irq_domain_xlate_onetwocell,
};
EXPORT_SYMBOL_GPL(irq_domain_simple_ops);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC 1/2] irq_domain: Create common xlate functions that device drivers can use
2012-01-25 0:18 [RFC 1/2] irq_domain: Create common xlate functions that device drivers can use Grant Likely
@ 2012-01-25 1:50 ` Rob Herring
2012-01-25 5:35 ` Grant Likely
0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2012-01-25 1:50 UTC (permalink / raw)
To: linux-arm-kernel
On 01/24/2012 06:18 PM, Grant Likely wrote:
> Rather than having each interrupt controller driver creating its own barely
> unique .xlate function for irq_domain, create a library of translators which
> any driver can use directly.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> This builds on top of my irq_domain series and will be included with it when
> I post v3. Rob, does this serve your needs for generic irq controller DT
> support?
>
Yes, but comments below.
> g.
>
> include/linux/irqdomain.h | 15 ++++++++++
> kernel/irq/irqdomain.c | 69 +++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 76 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index e7379a3..5e497a0 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -110,6 +110,9 @@ struct irq_domain {
> void *host_data;
> irq_hw_number_t inval_irq;
>
> + /* Data for common irq xlate functions */
> + unsigned int xlate_type;
> +
How does this get set? Do we want interrupt controllers messing with the
domain struct directly long term?
> /* Optional device node pointer */
> struct device_node *of_node;
> };
> @@ -163,6 +166,18 @@ extern unsigned int irq_linear_revmap(struct irq_domain *host,
> irq_hw_number_t hwirq);
>
> extern struct irq_domain_ops irq_domain_simple_ops;
> +
> +/* stock xlate functions */
> +int irq_domain_xlate_onecell(struct irq_domain *d, struct device_node *ctrlr,
> + const u32 *intspec, unsigned int intsize,
> + irq_hw_number_t *out_hwirq, unsigned int *out_type);
> +int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
> + const u32 *intspec, unsigned int intsize,
> + irq_hw_number_t *out_hwirq, unsigned int *out_type);
> +int irq_domain_xlate_onetwocell(struct irq_domain *d, struct device_node *ctrlr,
> + const u32 *intspec, unsigned int intsize,
> + irq_hw_number_t *out_hwirq, unsigned int *out_type);
> +
> #if defined(CONFIG_OF_IRQ)
> extern void irq_domain_generate_simple(const struct of_device_id *match,
> u64 phys_base, unsigned int irq_start);
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index def8e7b..df80205 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -53,6 +53,7 @@ static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
> domain->ops = ops;
> domain->host_data = host_data;
> domain->of_node = of_node_get(of_node);
> + domain->xlate_type = IRQ_TYPE_NONE;
>
> if (domain->ops->match == NULL)
> domain->ops->match = default_irq_domain_match;
> @@ -690,25 +691,77 @@ int irq_domain_simple_map(struct irq_domain *d, unsigned int irq,
> return 0;
> }
>
> -int irq_domain_simple_xlate(struct irq_domain *d,
> - struct device_node *controller,
> - const u32 *intspec, unsigned int intsize,
> - unsigned long *out_hwirq, unsigned int *out_type)
> +/**
> + * irq_domain_xlate_onecell() - Generic xlate for direct one cell bindings
> + *
> + * Device Tree IRQ specifier translation function which works with one cell
> + * bindings where the cell value maps directly to the hwirq number.
> + */
> +int irq_domain_xlate_onecell(struct irq_domain *d, struct device_node *ctrlr,
> + const u32 *intspec, unsigned int intsize,
> + unsigned long *out_hwirq, unsigned int *out_type)
> +{
> + if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
> + ctrlr->full_name, intsize))
> + return -EINVAL;
> + *out_hwirq = intspec[0];
> + *out_type = d->xlate_type;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(irq_domain_xlate_onecell);
> +
> +/**
> + * irq_domain_xlate_twocell() - Generic xlate for direct two cell bindings
> + *
> + * Device Tree IRQ specifier translation function which works with two cell
> + * bindings where the cell values map directly to the hwirq number
> + * and linux irq flags.
> + */
> +int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
> + const u32 *intspec, unsigned int intsize,
> + irq_hw_number_t *out_hwirq, unsigned int *out_type)
> {
> - if (d->of_node != controller)
> + if (WARN(intsize < 2, "Bad intspec for %s: intsize=%i < 2\n",
> + ctrlr->full_name, intsize))
> + return -EINVAL;
> + if (intsize < 2)
You've already checked this...
> return -EINVAL;
> - if (intsize < 1)
> + *out_hwirq = intspec[0];
> + *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
> +
> +/**
> + * irq_domain_xlate_onetwocell() - Generic xlate for one or two cell bindings
> + *
> + * Device Tree IRQ specifier translation function which works with either one
> + * or two cell bindings where the cell values map directly to the hwirq number
> + * and linux irq flags.
> + *
> + * Note: don't use this function unless your interrupt controller explicitly
> + * supports both one and two cell bindings. For the majority of controllers
> + * the _onecell() or _twocell() variants above should be used.
This comment is confusing since you turn around and use it for simple
ops below.
> + */
> +int irq_domain_xlate_onetwocell(struct irq_domain *d,
> + struct device_node *ctrlr,
> + const u32 *intspec, unsigned int intsize,
> + unsigned long *out_hwirq, unsigned int *out_type)
> +{
> + if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
> + ctrlr->full_name, intsize))
> return -EINVAL;
Isn't this check redundant with the check in of_irq_map_raw:
if (ointsize != intsize)
return -EINVAL;
The only diff in these functions is really checking the size. So I
really don't think we need 3 versions if that can be factored out where
you know value of #interrupt-cells.
Rob
> *out_hwirq = intspec[0];
> - *out_type = IRQ_TYPE_NONE;
> + *out_type = d->xlate_type;
> if (intsize > 1)
> *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
> return 0;
> }
> +EXPORT_SYMBOL_GPL(irq_domain_xlate_onetwocell);
>
> struct irq_domain_ops irq_domain_simple_ops = {
> .map = irq_domain_simple_map,
> - .xlate = irq_domain_simple_xlate,
> + .xlate = irq_domain_xlate_onetwocell,
> };
> EXPORT_SYMBOL_GPL(irq_domain_simple_ops);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC 1/2] irq_domain: Create common xlate functions that device drivers can use
2012-01-25 1:50 ` Rob Herring
@ 2012-01-25 5:35 ` Grant Likely
2012-01-25 17:59 ` Grant Likely
0 siblings, 1 reply; 6+ messages in thread
From: Grant Likely @ 2012-01-25 5:35 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 24, 2012 at 6:50 PM, Rob Herring <robherring2@gmail.com> wrote:
>
>
> On 01/24/2012 06:18 PM, Grant Likely wrote:
>> Rather than having each interrupt controller driver creating its own barely
>> unique .xlate function for irq_domain, create a library of translators which
>> any driver can use directly.
>>
>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>> index e7379a3..5e497a0 100644
>> --- a/include/linux/irqdomain.h
>> +++ b/include/linux/irqdomain.h
>> @@ -110,6 +110,9 @@ struct irq_domain {
>> ? ? ? void *host_data;
>> ? ? ? irq_hw_number_t inval_irq;
>>
>> + ? ? /* Data for common irq xlate functions */
>> + ? ? unsigned int xlate_type;
>> +
>
> How does this get set? Do we want interrupt controllers messing with the
> domain struct directly long term?
It defaults to IRQ_TYPE_NONE in the alloc function and drivers can
override it. Alternately it could be made part of the
irq_domain_add() arguments, but I'm not thrilled with adding a whole
bunch of arguments to the function prototype.
>
>> ? ? ? /* Optional device node pointer */
>> ? ? ? struct device_node *of_node;
>> ?};
>> @@ -163,6 +166,18 @@ extern unsigned int irq_linear_revmap(struct irq_domain *host,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? irq_hw_number_t hwirq);
>>
>> ?extern struct irq_domain_ops irq_domain_simple_ops;
>> +
>> +/* stock xlate functions */
>> +int irq_domain_xlate_onecell(struct irq_domain *d, struct device_node *ctrlr,
>> + ? ? ? ? ? ? ? ? ? ? const u32 *intspec, unsigned int intsize,
>> + ? ? ? ? ? ? ? ? ? ? irq_hw_number_t *out_hwirq, unsigned int *out_type);
>> +int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
>> + ? ? ? ? ? ? ? ? ? ? const u32 *intspec, unsigned int intsize,
>> + ? ? ? ? ? ? ? ? ? ? irq_hw_number_t *out_hwirq, unsigned int *out_type);
>> +int irq_domain_xlate_onetwocell(struct irq_domain *d, struct device_node *ctrlr,
>> + ? ? ? ? ? ? ? ? ? ? const u32 *intspec, unsigned int intsize,
>> + ? ? ? ? ? ? ? ? ? ? irq_hw_number_t *out_hwirq, unsigned int *out_type);
>> +
>> ?#if defined(CONFIG_OF_IRQ)
>> ?extern void irq_domain_generate_simple(const struct of_device_id *match,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? u64 phys_base, unsigned int irq_start);
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index def8e7b..df80205 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -53,6 +53,7 @@ static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
>> ? ? ? domain->ops = ops;
>> ? ? ? domain->host_data = host_data;
>> ? ? ? domain->of_node = of_node_get(of_node);
>> + ? ? domain->xlate_type = IRQ_TYPE_NONE;
>>
>> ? ? ? if (domain->ops->match == NULL)
>> ? ? ? ? ? ? ? domain->ops->match = default_irq_domain_match;
>> @@ -690,25 +691,77 @@ int irq_domain_simple_map(struct irq_domain *d, unsigned int irq,
>> ? ? ? return 0;
>> ?}
>>
>> -int irq_domain_simple_xlate(struct irq_domain *d,
>> - ? ? ? ? ? ? ? ? ? ? ? ? struct device_node *controller,
>> - ? ? ? ? ? ? ? ? ? ? ? ? const u32 *intspec, unsigned int intsize,
>> - ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *out_hwirq, unsigned int *out_type)
>> +/**
>> + * irq_domain_xlate_onecell() - Generic xlate for direct one cell bindings
>> + *
>> + * Device Tree IRQ specifier translation function which works with one cell
>> + * bindings where the cell value maps directly to the hwirq number.
>> + */
>> +int irq_domain_xlate_onecell(struct irq_domain *d, struct device_node *ctrlr,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?const u32 *intspec, unsigned int intsize,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned long *out_hwirq, unsigned int *out_type)
>> +{
>> + ? ? if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
>> + ? ? ? ? ? ? ?ctrlr->full_name, intsize))
>> + ? ? ? ? ? ? return -EINVAL;
>> + ? ? *out_hwirq = intspec[0];
>> + ? ? *out_type = d->xlate_type;
>> + ? ? return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(irq_domain_xlate_onecell);
>> +
>> +/**
>> + * irq_domain_xlate_twocell() - Generic xlate for direct two cell bindings
>> + *
>> + * Device Tree IRQ specifier translation function which works with two cell
>> + * bindings where the cell values map directly to the hwirq number
>> + * and linux irq flags.
>> + */
>> +int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
>> + ? ? ? ? ? ? ? ? ? ? const u32 *intspec, unsigned int intsize,
>> + ? ? ? ? ? ? ? ? ? ? irq_hw_number_t *out_hwirq, unsigned int *out_type)
>> ?{
>> - ? ? if (d->of_node != controller)
>> + ? ? if (WARN(intsize < 2, "Bad intspec for %s: intsize=%i < 2\n",
>> + ? ? ? ? ? ? ?ctrlr->full_name, intsize))
>> + ? ? ? ? ? ? return -EINVAL;
>> + ? ? if (intsize < 2)
>
> You've already checked this...
cut-and-paste error. Fixed.
>
>> ? ? ? ? ? ? ? return -EINVAL;
>> - ? ? if (intsize < 1)
>> + ? ? *out_hwirq = intspec[0];
>> + ? ? *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
>> + ? ? return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
>> +
>> +/**
>> + * irq_domain_xlate_onetwocell() - Generic xlate for one or two cell bindings
>> + *
>> + * Device Tree IRQ specifier translation function which works with either one
>> + * or two cell bindings where the cell values map directly to the hwirq number
>> + * and linux irq flags.
>> + *
>> + * Note: don't use this function unless your interrupt controller explicitly
>> + * supports both one and two cell bindings. ?For the majority of controllers
>> + * the _onecell() or _twocell() variants above should be used.
>
> This comment is confusing since you turn around and use it for simple
> ops below.
I intend to faze out the generic simple ops so that most drivers will
explicitly use _onecell or _twocell.
>> + */
>> +int irq_domain_xlate_onetwocell(struct irq_domain *d,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct device_node *ctrlr,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? const u32 *intspec, unsigned int intsize,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *out_hwirq, unsigned int *out_type)
>> +{
>> + ? ? if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
>> + ? ? ? ? ? ? ?ctrlr->full_name, intsize))
>> ? ? ? ? ? ? ? return -EINVAL;
>
> Isn't this check redundant with the check in of_irq_map_raw:
>
> ? ? ? ?if (ointsize != intsize)
> ? ? ? ? ? ? ? ?return -EINVAL;
No. It's a different test. The core code does indeed ensure that
intsize matches the #interrupt-cells value defined in the device tree,
but it does nothing to check if the #interrupt-cells value is set to
something sane. This test ensures that the xlate function will not
dereference beyond the end of the intspec array.
Ideally the interrupt controller driver should already verify the
#interrupt-cells value at initialization, but I'm not going to rely on
it when this test is so cheap. The intent is to be really noisy about
bad data in the device tree.
> The only diff in these functions is really checking the size. So I
> really don't think we need 3 versions if that can be factored out where
> you know value of #interrupt-cells.
There are subtleties here and #interrupt-cells is not always the
correct thing to test. For example, some powerpc controllers has
#interrupt-cells = <2>, but the second cell must be ignored so the
_onecell version is the correct function. I originally investigated
parameterizing a single function, but I didn't like how it ended up
looking because it would require adding something like irq_cells_min
and irq_cells_max to the irq_domain structure. I think using
variations on the function ends up being cleaner. I may also add
something like a ..._twocell_lookup() version that uses a lookup table
for the type values to handle the other type of xlate common in the
powerpc tree.
Thanks for the quick review.
g.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC 1/2] irq_domain: Create common xlate functions that device drivers can use
2012-01-25 5:35 ` Grant Likely
@ 2012-01-25 17:59 ` Grant Likely
2012-01-26 14:50 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Grant Likely @ 2012-01-25 17:59 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 24, 2012 at 10:35 PM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Tue, Jan 24, 2012 at 6:50 PM, Rob Herring <robherring2@gmail.com> wrote:
>>
>>
>> On 01/24/2012 06:18 PM, Grant Likely wrote:
>>> Rather than having each interrupt controller driver creating its own barely
>>> unique .xlate function for irq_domain, create a library of translators which
>>> any driver can use directly.
>>>
>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>>> index e7379a3..5e497a0 100644
>>> --- a/include/linux/irqdomain.h
>>> +++ b/include/linux/irqdomain.h
>>> @@ -110,6 +110,9 @@ struct irq_domain {
>>> ? ? ? void *host_data;
>>> ? ? ? irq_hw_number_t inval_irq;
>>>
>>> + ? ? /* Data for common irq xlate functions */
>>> + ? ? unsigned int xlate_type;
>>> +
>>
>> How does this get set? Do we want interrupt controllers messing with the
>> domain struct directly long term?
>
> It defaults to IRQ_TYPE_NONE in the alloc function and drivers can
> override it. ?Alternately it could be made part of the
> irq_domain_add() arguments, but I'm not thrilled with adding a whole
> bunch of arguments to the function prototype.
Do you like this better (built on top of this patch)?
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index ef2b1fe..7856c04 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -701,12 +701,10 @@ int irq_domain_xlate_onecell(struct irq_domain
*d, struct device_node *ctrlr,
const u32 *intspec, unsigned int intsize,
unsigned long *out_hwirq, unsigned int *out_type)
{
- if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
- ctrlr->full_name, intsize))
+ if (WARN_ON(intsize < 1))
return -EINVAL;
- *out_hwirq = intspec[0];
- *out_type = d->xlate_type;
- return 0;
+ return irq_domain_xlate_onetwocell(d, ctrlr, intspec, 1,
+ out_hwirq, out_type);
}
EXPORT_SYMBOL_GPL(irq_domain_xlate_onecell);
@@ -721,12 +719,10 @@ int irq_domain_xlate_twocell(struct irq_domain
*d, struct device_node *ctrlr,
const u32 *intspec, unsigned int intsize,
irq_hw_number_t *out_hwirq, unsigned int *out_type)
{
- if (WARN(intsize < 2, "Bad intspec for %s: intsize=%i < 2\n",
- ctrlr->full_name, intsize))
+ if (WARN_ON(intsize < 2))
return -EINVAL;
- *out_hwirq = intspec[0];
- *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
- return 0;
+ return irq_domain_xlate_onetwocell(d, ctrlr, intspec, intsize,
+ out_hwirq, out_type);
}
EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
@@ -746,8 +742,7 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d,
const u32 *intspec, unsigned int intsize,
unsigned long *out_hwirq, unsigned int *out_type)
{
- if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
- ctrlr->full_name, intsize))
+ if (WARN_ON(intsize < 1))
return -EINVAL;
*out_hwirq = intspec[0];
*out_type = d->xlate_type;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC 1/2] irq_domain: Create common xlate functions that device drivers can use
2012-01-25 17:59 ` Grant Likely
@ 2012-01-26 14:50 ` Rob Herring
2012-01-26 18:15 ` Grant Likely
0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2012-01-26 14:50 UTC (permalink / raw)
To: linux-arm-kernel
On 01/25/2012 11:59 AM, Grant Likely wrote:
> On Tue, Jan 24, 2012 at 10:35 PM, Grant Likely
> <grant.likely@secretlab.ca> wrote:
>> On Tue, Jan 24, 2012 at 6:50 PM, Rob Herring <robherring2@gmail.com> wrote:
>>>
>>>
>>> On 01/24/2012 06:18 PM, Grant Likely wrote:
>>>> Rather than having each interrupt controller driver creating its own barely
>>>> unique .xlate function for irq_domain, create a library of translators which
>>>> any driver can use directly.
>>>>
>>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>>>> index e7379a3..5e497a0 100644
>>>> --- a/include/linux/irqdomain.h
>>>> +++ b/include/linux/irqdomain.h
>>>> @@ -110,6 +110,9 @@ struct irq_domain {
>>>> void *host_data;
>>>> irq_hw_number_t inval_irq;
>>>>
>>>> + /* Data for common irq xlate functions */
>>>> + unsigned int xlate_type;
>>>> +
>>>
>>> How does this get set? Do we want interrupt controllers messing with the
>>> domain struct directly long term?
>>
>> It defaults to IRQ_TYPE_NONE in the alloc function and drivers can
>> override it. Alternately it could be made part of the
>> irq_domain_add() arguments, but I'm not thrilled with adding a whole
>> bunch of arguments to the function prototype.
>
> Do you like this better (built on top of this patch)?
Yes, I think it's better.
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index ef2b1fe..7856c04 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -701,12 +701,10 @@ int irq_domain_xlate_onecell(struct irq_domain
> *d, struct device_node *ctrlr,
> const u32 *intspec, unsigned int intsize,
> unsigned long *out_hwirq, unsigned int *out_type)
> {
> - if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
> - ctrlr->full_name, intsize))
> + if (WARN_ON(intsize < 1))
> return -EINVAL;
> - *out_hwirq = intspec[0];
> - *out_type = d->xlate_type;
> - return 0;
> + return irq_domain_xlate_onetwocell(d, ctrlr, intspec, 1,
> + out_hwirq, out_type);
> }
> EXPORT_SYMBOL_GPL(irq_domain_xlate_onecell);
>
> @@ -721,12 +719,10 @@ int irq_domain_xlate_twocell(struct irq_domain
> *d, struct device_node *ctrlr,
> const u32 *intspec, unsigned int intsize,
> irq_hw_number_t *out_hwirq, unsigned int *out_type)
> {
> - if (WARN(intsize < 2, "Bad intspec for %s: intsize=%i < 2\n",
> - ctrlr->full_name, intsize))
> + if (WARN_ON(intsize < 2))
> return -EINVAL;
> - *out_hwirq = intspec[0];
> - *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
> - return 0;
> + return irq_domain_xlate_onetwocell(d, ctrlr, intspec, intsize,
> + out_hwirq, out_type);
> }
> EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
>
> @@ -746,8 +742,7 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d,
> const u32 *intspec, unsigned int intsize,
> unsigned long *out_hwirq, unsigned int *out_type)
> {
> - if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
> - ctrlr->full_name, intsize))
> + if (WARN_ON(intsize < 1))
> return -EINVAL;
> *out_hwirq = intspec[0];
> *out_type = d->xlate_type;
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC 1/2] irq_domain: Create common xlate functions that device drivers can use
2012-01-26 14:50 ` Rob Herring
@ 2012-01-26 18:15 ` Grant Likely
0 siblings, 0 replies; 6+ messages in thread
From: Grant Likely @ 2012-01-26 18:15 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 26, 2012 at 7:50 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 01/25/2012 11:59 AM, Grant Likely wrote:
>> On Tue, Jan 24, 2012 at 10:35 PM, Grant Likely
>> <grant.likely@secretlab.ca> wrote:
>>> On Tue, Jan 24, 2012 at 6:50 PM, Rob Herring <robherring2@gmail.com> wrote:
>>>>
>>>>
>>>> On 01/24/2012 06:18 PM, Grant Likely wrote:
>>>>> Rather than having each interrupt controller driver creating its own barely
>>>>> unique .xlate function for irq_domain, create a library of translators which
>>>>> any driver can use directly.
>>>>>
>>>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>>>>> index e7379a3..5e497a0 100644
>>>>> --- a/include/linux/irqdomain.h
>>>>> +++ b/include/linux/irqdomain.h
>>>>> @@ -110,6 +110,9 @@ struct irq_domain {
>>>>> ? ? ? void *host_data;
>>>>> ? ? ? irq_hw_number_t inval_irq;
>>>>>
>>>>> + ? ? /* Data for common irq xlate functions */
>>>>> + ? ? unsigned int xlate_type;
>>>>> +
>>>>
>>>> How does this get set? Do we want interrupt controllers messing with the
>>>> domain struct directly long term?
>>>
>>> It defaults to IRQ_TYPE_NONE in the alloc function and drivers can
>>> override it. ?Alternately it could be made part of the
>>> irq_domain_add() arguments, but I'm not thrilled with adding a whole
>>> bunch of arguments to the function prototype.
>>
>> Do you like this better (built on top of this patch)?
>
> Yes, I think it's better.
I've changed my mind on this after looking at the generated code.
These functions are so simple that it actually is larger (both in
source and object size) to call out to a common function. I've gone
back to each _xlate_*() function open coding the two assignments.
g.
>
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index ef2b1fe..7856c04 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -701,12 +701,10 @@ int irq_domain_xlate_onecell(struct irq_domain
>> *d, struct device_node *ctrlr,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ?const u32 *intspec, unsigned int intsize,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned long *out_hwirq, unsigned int *out_type)
>> ?{
>> - ? ? if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
>> - ? ? ? ? ? ? ?ctrlr->full_name, intsize))
>> + ? ? if (WARN_ON(intsize < 1))
>> ? ? ? ? ? ? ? return -EINVAL;
>> - ? ? *out_hwirq = intspec[0];
>> - ? ? *out_type = d->xlate_type;
>> - ? ? return 0;
>> + ? ? return irq_domain_xlate_onetwocell(d, ctrlr, intspec, 1,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?out_hwirq, out_type);
>> ?}
>> ?EXPORT_SYMBOL_GPL(irq_domain_xlate_onecell);
>>
>> @@ -721,12 +719,10 @@ int irq_domain_xlate_twocell(struct irq_domain
>> *d, struct device_node *ctrlr,
>> ? ? ? ? ? ? ? ? ? ? ? const u32 *intspec, unsigned int intsize,
>> ? ? ? ? ? ? ? ? ? ? ? irq_hw_number_t *out_hwirq, unsigned int *out_type)
>> ?{
>> - ? ? if (WARN(intsize < 2, "Bad intspec for %s: intsize=%i < 2\n",
>> - ? ? ? ? ? ? ?ctrlr->full_name, intsize))
>> + ? ? if (WARN_ON(intsize < 2))
>> ? ? ? ? ? ? ? return -EINVAL;
>> - ? ? *out_hwirq = intspec[0];
>> - ? ? *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
>> - ? ? return 0;
>> + ? ? return irq_domain_xlate_onetwocell(d, ctrlr, intspec, intsize,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?out_hwirq, out_type);
>> ?}
>> ?EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
>>
>> @@ -746,8 +742,7 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const u32 *intspec, unsigned int intsize,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *out_hwirq, unsigned int *out_type)
>> ?{
>> - ? ? if (WARN(intsize < 1, "Bad intspec for %s: intsize=%i < 1\n",
>> - ? ? ? ? ? ? ?ctrlr->full_name, intsize))
>> + ? ? if (WARN_ON(intsize < 1))
>> ? ? ? ? ? ? ? return -EINVAL;
>> ? ? ? *out_hwirq = intspec[0];
>> ? ? ? *out_type = d->xlate_type;
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-26 18:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-25 0:18 [RFC 1/2] irq_domain: Create common xlate functions that device drivers can use Grant Likely
2012-01-25 1:50 ` Rob Herring
2012-01-25 5:35 ` Grant Likely
2012-01-25 17:59 ` Grant Likely
2012-01-26 14:50 ` Rob Herring
2012-01-26 18:15 ` Grant Likely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).