On 5/10/2018 9:44 PM, Jie, Yang wrote:
-----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.
as we have discussed, parent connects to xtensa core, child connects to parent in a link list. only the first
level of irqs is parent which is used to set interrupt in xtensa core. other irqs are all children of corresponding parent,
and they are linked to the parent. you can refer to Linux kernel, all the irqs are linked in a list attached to a root irq,
no recursive level.
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