[Sound-open-firmware] [PATCH 1/7] interrupt: refine irq structure and algorithm
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Fri May 4 15:36:14 CEST 2018
On 5/4/18 3:14 AM, Rander Wang wrote:
> The issues for original data structure and algorithm are:
> (1)it only support 2 level interrupt architecture.
> (2)it doesn't support different HW interrupt shares one IRQ value.
>
> Unify irq parent and child to irq_desc. Now each irq_desc maybe
> directly attach to xtensa core or another irq_desc.Each irq_desc
> may have 32 child lists instead of 32 child.
>
> Signed-off-by: Rander Wang <rander.wang at linux.intel.com>
> Signed-off-by: Liam Girdwood <liam.r.girdwood at linux.intel.com>
> ---
> src/include/sof/interrupt.h | 32 ++++++------
> src/lib/interrupt.c | 120 +++++++++++++++++++++++---------------------
> 2 files changed, 79 insertions(+), 73 deletions(-)
>
> diff --git a/src/include/sof/interrupt.h b/src/include/sof/interrupt.h
> index 2dc8954..f5c37c3 100644
> --- a/src/include/sof/interrupt.h
> +++ b/src/include/sof/interrupt.h
> @@ -37,27 +37,29 @@
> #include <sof/trace.h>
> #include <sof/debug.h>
> #include <sof/lock.h>
> +#include <sof/list.h>
>
> #define trace_irq(__e) trace_event(TRACE_CLASS_IRQ, __e)
> #define trace_irq_error(__e) trace_error(TRACE_CLASS_IRQ, __e)
>
> -/* child interrupt source */
> -struct irq_child {
> - uint32_t enabled;
> +struct irq_desc {
> + /* irq must be first for constructor */
> + int irq; /* logical IRQ number */
>
> + /* handler is optional for constructor */
> void (*handler)(void *arg);
> void *handler_arg;
> -};
>
> -/* parent source */
> -struct irq_parent {
> - int num;
> - void (*handler)(void *arg);
> - uint32_t enabled_count;
> + /* to identify interrupt with the same IRQ */
> + int id;
> spinlock_t lock;
> + uint32_t enabled_count;
> +
> + /* to link to other irq_desc */
> + struct list_item irq_list;
>
> uint32_t num_children;
> - struct irq_child *child[PLATFORM_IRQ_CHILDREN];
> + struct list_item child[PLATFORM_IRQ_CHILDREN];
> };
>
> int interrupt_register(uint32_t irq,
> @@ -87,10 +89,10 @@ static inline void interrupt_global_enable(uint32_t flags)
> }
>
> /* called by platform interrupt ops */
> -int irq_register_child(struct irq_parent *parent, int irq,
> - void (*handler)(void *arg), void *arg);
> -void irq_unregister_child(struct irq_parent *parent, int irq);
> -uint32_t irq_enable_child(struct irq_parent *parent, int irq);
> -uint32_t irq_disable_child(struct irq_parent *parent, int irq);
> +int irq_register_child(struct irq_desc *parent, int irq,
> + void (*handler)(void *arg), void *arg);
> +void irq_unregister_child(struct irq_desc *parent, int irq);
> +uint32_t irq_enable_child(struct irq_desc *parent, int irq);
> +uint32_t irq_disable_child(struct irq_desc *parent, int irq);
>
> #endif
> diff --git a/src/lib/interrupt.c b/src/lib/interrupt.c
> index e8e2899..e53d4c8 100644
> --- a/src/lib/interrupt.c
> +++ b/src/lib/interrupt.c
> @@ -38,127 +38,131 @@
> #include <stdint.h>
> #include <stdlib.h>
>
> -int irq_register_child(struct irq_parent *parent, int irq,
> - void (*handler)(void *arg), void *arg)
> +int irq_register_child(struct irq_desc *parent, int irq,
> + void (*handler)(void *arg), void *arg)
> {
> int ret = 0;
> + struct irq_desc *child;
>
> if (parent == NULL)
> return -EINVAL;
>
> spin_lock(&parent->lock);
>
> - /* does child already exist ? */
> - if (parent->child[SOF_IRQ_BIT(irq)]) {
> - /* already registered, return */
> - goto finish;
> - }
> -
> /* init child */
> - parent->child[SOF_IRQ_BIT(irq)] =
> - rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM,
> - sizeof(struct irq_child)); > - parent->child[SOF_IRQ_BIT(irq)]->enabled = 0;
> - parent->child[SOF_IRQ_BIT(irq)]->handler = handler;
> - parent->child[SOF_IRQ_BIT(irq)]->handler_arg = arg;
> + child = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM,
> + sizeof(struct irq_desc));
if (!child)
ret = -ENOMEM;
goto finish;
}
> + child->enabled_count = 0;
> + child->handler = handler;
> + child->handler_arg = arg;
> + child->id = SOF_IRQ_ID(irq);
> +
> + list_item_append(&child->irq_list, &parent->child[SOF_IRQ_BIT(irq)]);
>
> /* do we need to register parent ? */
> if (parent->num_children == 0) {
> - ret = arch_interrupt_register(parent->num,
> - parent->handler, parent);
> + ret = arch_interrupt_register(parent->irq,
> + parent->handler, parent);
> }
>
> /* increment number of children */
> - parent->num_children += 1;
> + parent->num_children++;
>
> -finish:
> spin_unlock(&parent->lock);
> return ret;
> -
> }
>
> -void irq_unregister_child(struct irq_parent *parent, int irq)
> +void irq_unregister_child(struct irq_desc *parent, int irq)
> {
> spin_lock(&parent->lock);
> + struct irq_desc *child;
> + struct list_item *clist;
>
> /* does child already exist ? */
> - if (!parent->child[SOF_IRQ_BIT(irq)])
> + if (list_is_empty(&parent->child[SOF_IRQ_BIT(irq)]))
> goto finish;
>
> - /* free child */
> - parent->num_children -= 1;
> - rfree(parent->child[SOF_IRQ_BIT(irq)]);
> - parent->child[SOF_IRQ_BIT(irq)] = NULL;
> + list_for_item(clist, &parent->child[SOF_IRQ_BIT(irq)]) {
> + child = container_of(clist, struct irq_desc, irq_list);
> +
> + if (SOF_IRQ_ID(irq) == child->id) {
> + list_item_del(&child->irq_list);
> + rfree(child);
> + parent->num_children--;
> + }
> + }
>
> /*
> * unregister the root interrupt if the this l2 is
> * the last registered one.
> */
> if (parent->num_children == 0)
> - arch_interrupt_unregister(parent->num);
> + arch_interrupt_unregister(parent->irq);
>
> finish:
> spin_unlock(&parent->lock);
> }
>
> -uint32_t irq_enable_child(struct irq_parent *parent, int irq)
> +uint32_t irq_enable_child(struct irq_desc *parent, int irq)
> {
> - struct irq_child *child;
> + struct irq_desc *child;
> + struct list_item *clist;
>
> spin_lock(&parent->lock);
>
> - child = parent->child[SOF_IRQ_BIT(irq)];
> -
> - /* already enabled ? */
> - if (child->enabled)
> - goto finish;
> -
> /* enable the parent interrupt */
> if (parent->enabled_count == 0)
> arch_interrupt_enable_mask(1 << SOF_IRQ_NUMBER(irq));
> - child->enabled = 1;
> - parent->enabled_count++;
>
> - /* enable the child interrupt */
> - platform_interrupt_unmask(irq, 0);
> + list_for_item(clist, &parent->child[SOF_IRQ_BIT(irq)]) {
> + child = container_of(clist, struct irq_desc, irq_list);
> +
> + if ((SOF_IRQ_ID(irq) == child->id) &&
> + !child->enabled_count) {
> + child->enabled_count = 1;
> + parent->enabled_count++;
> +
> + /* enable the child interrupt */
> + platform_interrupt_unmask(irq, 0);
> + }
> + }
>
> -finish:
> spin_unlock(&parent->lock);
> return 0;
>
> }
>
> -uint32_t irq_disable_child(struct irq_parent *parent, int irq)
> +uint32_t irq_disable_child(struct irq_desc *parent, int irq)
> {
> - struct irq_child *child;
> + struct irq_desc *child;
> + struct list_item *clist;
>
> spin_lock(&parent->lock);
>
> - child = parent->child[SOF_IRQ_BIT(irq)];
> -
> - /* already disabled ? */
> - if (!child->enabled)
> - goto finish;
> + list_for_item(clist, &parent->child[SOF_IRQ_BIT(irq)]) {
> + child = container_of(clist, struct irq_desc, irq_list);
>
> - /* disable the child interrupt */
> - platform_interrupt_mask(irq, 0);
> - child->enabled = 0;
> + if ((SOF_IRQ_ID(irq) == child->id) &&
> + child->enabled_count) {
> + child->enabled_count = 0;
> + parent->enabled_count--;
> + }
> + }
>
> - /* disable the parent interrupt */
> - parent->enabled_count--;
> - if (parent->enabled_count == 0)
> + if (parent->enabled_count == 0) {
> + /* disable the child interrupt */
> + platform_interrupt_mask(irq, 0);
> arch_interrupt_disable_mask(1 << SOF_IRQ_NUMBER(irq));
> + }
>
> -finish:
> spin_unlock(&parent->lock);
> return 0;
> -
> }
>
> int interrupt_register(uint32_t irq,
> void (*handler)(void *arg), void *arg)
> {
> - struct irq_parent *parent;
> + struct irq_desc *parent;
>
> /* no parent means we are registering DSP internal IRQ */
> parent = platform_irq_get_parent(irq);
> @@ -170,7 +174,7 @@ int interrupt_register(uint32_t irq,
>
> void interrupt_unregister(uint32_t irq)
> {
> - struct irq_parent *parent;
> + struct irq_desc *parent;
>
> /* no parent means we are unregistering DSP internal IRQ */
> parent = platform_irq_get_parent(irq);
> @@ -182,7 +186,7 @@ void interrupt_unregister(uint32_t irq)
>
> uint32_t interrupt_enable(uint32_t irq)
> {
> - struct irq_parent *parent;
> + struct irq_desc *parent;
>
> /* no parent means we are enabling DSP internal IRQ */
> parent = platform_irq_get_parent(irq);
> @@ -194,7 +198,7 @@ uint32_t interrupt_enable(uint32_t irq)
>
> uint32_t interrupt_disable(uint32_t irq)
> {
> - struct irq_parent *parent;
> + struct irq_desc *parent;
>
> /* no parent means we are disabling DSP internal IRQ */
> parent = platform_irq_get_parent(irq);
>
More information about the Sound-open-firmware
mailing list