-----Original Message----- From: sound-open-firmware-bounces@alsa-project.org [mailto:sound-open- firmware-bounces@alsa-project.org] On Behalf Of Rander Wang Sent: Thursday, May 10, 2018 1:32 PM To: sound-open-firmware@alsa-project.org; marcin.maka@linux.intel.com Cc: Liam Girdwood liam.r.girdwood@linux.intel.com; Rander Wang rander.wang@linux.intel.com Subject: [Sound-open-firmware] [PATCH_V3 1/8] interrupt: refine irq structure and algorithm
The issue for original data structure and algorithm is that: (1)it only support 2 level interrupt architecture. (2)it doesn't support two HW interrupt triggered by one irq pin.
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 list instead of 32 child.
V2: check whether memory allocation failed V3: remove useless declaration in interrupt.h
test on CNL & APL & BYT, pass SOF: master c1f2682c210201 kernel: v4.14 d09db67c5a9d6d SOF-tools: master 13b56fa6047c566a
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com
src/include/sof/interrupt.h | 29 +++++----- src/lib/interrupt.c | 126 +++++++++++++++++++++++++------------------- 2 files changed, 83 insertions(+), 72 deletions(-)
diff --git a/src/include/sof/interrupt.h b/src/include/sof/interrupt.h index 2dc8954..bbda635 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, @@ -86,11 +88,4 @@ static inline void interrupt_global_enable(uint32_t flags) arch_interrupt_global_enable(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);
#endif diff --git a/src/lib/interrupt.c b/src/lib/interrupt.c index e8e2899..a1f22eb 100644 --- a/src/lib/interrupt.c +++ b/src/lib/interrupt.c @@ -38,127 +38,143 @@ #include <stdint.h> #include <stdlib.h>
-int irq_register_child(struct irq_parent *parent, int irq,
- void (*handler)(void *arg), void *arg)
+static int irq_register_child(struct irq_desc *parent, int irq,
void (*handler)(void *arg), void *arg); static void+irq_unregister_child(struct irq_desc *parent, int irq); static uint32_t +irq_enable_child(struct irq_desc *parent, int irq); static uint32_t +irq_disable_child(struct irq_desc *parent, int irq);
+static 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 */
- /* init child */
- child = rzalloc(RZONE_SYS, SOF_MEM_CAPS_RAM,
sizeof(struct irq_desc));- if (!child) {
ret = -ENOMEM;
- /* 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->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) +static 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);
Is it possible here the parent also has parent? If so, we need a recursive calling here.
finish: spin_unlock(&parent->lock); }
-uint32_t irq_enable_child(struct irq_parent *parent, int irq) +static 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;
Is child here has its child possible?
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) +static 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;
Same question here.
Thanks, ~Keyon
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);- }
-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 +186,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 +198,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 +210,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);
-- 2.14.1
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware