On 2017年09月19日 22:53, Liam Girdwood wrote:
On Tue, 2017-09-19 at 15:47 +0800, Keyon Jie wrote:
Make it possible to support layer 2 (e.g. external level 2/3/4, which may include up to 16/32 sources) interrupt, which may be used for timer/IPC/DMAC/SSP...
Signed-off-by: Keyon Jie yang.jie@linux.intel.com
src/drivers/dw-dma.c | 6 +- src/include/reef/interrupt.h | 86 ++++++++++-------- src/lib/Makefile.am | 3 +- src/lib/interrupt.c | 210 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 264 insertions(+), 41 deletions(-) create mode 100644 src/lib/interrupt.c
diff --git a/src/drivers/dw-dma.c b/src/drivers/dw-dma.c index e851e54..1020f66 100644 --- a/src/drivers/dw-dma.c +++ b/src/drivers/dw-dma.c @@ -330,7 +330,8 @@ static int dw_dma_start(struct dma *dma, int channel) dw_write(dma, DW_CLEAR_ERR, 0x1 << channel);
/* clear platform interrupt */
- platform_interrupt_clear(dma_irq(dma), 1 << channel);
- if (plf_irq_ops && plf_irq_ops->int_clear)
plf_irq_ops->int_clear(dma_irq(dma), 1 << channel);This should still use platform_irq_clear(). platform_irq_clear will then do any ops if necessary.
platform_irq_clear() is actually put into plf_irq_ops->int_clear, for platforms which don't need clear, the int_clear may be set NULL.
#if DW_USE_HW_LLI /* TODO: Revisit: are we using LLP mode or single transfer ? */ @@ -829,7 +830,8 @@ static void dw_dma_irq_handler(void *data)
/* clear platform and DSP interrupt */ pmask = status_block | status_tfr | status_err;
- platform_interrupt_clear(dma_irq(dma), pmask);
if (plf_irq_ops && plf_irq_ops->int_clear)
plf_irq_ops->int_clear(dma_irq(dma), pmask);for (i = 0; i < DW_MAX_CHAN; i++) {
diff --git a/src/include/reef/interrupt.h b/src/include/reef/interrupt.h index 3c78e77..8a9b0b6 100644 --- a/src/include/reef/interrupt.h +++ b/src/include/reef/interrupt.h @@ -35,53 +35,63 @@ #include <arch/interrupt.h> #include <reef/trace.h> #include <reef/debug.h> +#include <reef/list.h>
#define trace_irq(__e) trace_event(TRACE_CLASS_IRQ | __e)
-static inline int interrupt_register(int irq,
- void(*handler)(void *arg), void *arg)
-{
- return arch_interrupt_register(irq, handler, arg);
-} +typedef void (irq2_handler)(void *arg); +typedef void (irq_num_handler)(void *arg);
-static inline void interrupt_unregister(int irq) -{
- arch_interrupt_unregister(irq);
-} +/* source item under L2 interrupt aggregator */ +struct l2src_irq {
- uint32_t bit_id;
- uint32_t enabled;
- irq2_handler *handler;
- void *handler_arg;
- /* list in the registered irq of the same number */
- struct list_item list;
+};
-static inline uint32_t interrupt_enable(uint32_t irq) -{
- return arch_interrupt_enable_mask(1 << irq);
-} +/* root interrupt item which has Level 2 sources */
The naming "root" implies it's the level 1 IRQ ?
yep.
+struct l2root_irq {
- int num;
- irq_num_handler *handler;
- uint32_t src_count;
- uint32_t enabled_count;
- /* list in the registered l2 irq of the same number */
- struct list_item src_list;
+};
-static inline uint32_t interrupt_disable(uint32_t irq) -{
- return arch_interrupt_disable_mask(1 <<irq);
-} +/* platform specific irq ops */ +struct plf_irq_ops {
- int (*int_register)(uint32_t irq, void(*handler)(void *a src/lib/Makefile.am | 3 +-rg), void *arg);
- void (*int_unregister)(uint32_t irq);
- uint32_t (*int_enable)(uint32_t irq);
- uint32_t (*int_disable)(uint32_t irq);
-static inline void interrupt_set(int irq) -{
- arch_interrupt_set(irq);
-}
- uint32_t (*get_enabled)(void);
- void (*int_clear)(uint32_t irq, uint32_t mask);
- void (*int_mask)(uint32_t irq);
- void (*int_unmask)(uint32_t irq);
+};
-static inline void interrupt_clear(int irq) -{
- arch_interrupt_clear(irq);
-} +extern const struct plf_irq_ops *plf_irq_ops;
-static inline uint32_t interrupt_global_disable(void) -{
- return arch_interrupt_global_disable();
-} +/* uniformed interrupt API called outside interrupt */ +int interrupt_register(int irq, void(*handler)(void *arg), void *arg); +void interrupt_unregister(int irq); +uint32_t interrupt_enable(uint32_t irq); +uint32_t interrupt_disable(uint32_t irq); +void interrupt_set(int irq); +void interrupt_clear(int irq); +uint32_t interrupt_global_disable(void); +void interrupt_global_enable(uint32_t flags);
-static inline void interrupt_global_enable(uint32_t flags) -{
- arch_interrupt_global_enable(flags);
-}
-uint32_t platform_interrupt_get_enabled(); -void platform_interrupt_clear(uint32_t irq, uint32_t mask); -void platform_interrupt_mask(uint32_t irq, uint32_t mask); -void platform_interrupt_unmask(uint32_t irq, uint32_t mask); +/* called by platform interrupt ops*/ +int irq_l2_register(int irq, struct l2root_irq *l2_int,
- void (*handler)(void *arg), void *arg);
+void irq_l2_unregister(int irq, struct l2root_irq *l2_int); +uint32_t irq_l2_enable(int irq, struct l2root_irq *l2_int); +uint32_t irq_l2_disable(int irq, struct l2root_irq *l2_int);
#endif diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am index acdbd85..9923467 100644 --- a/src/lib/Makefile.am +++ b/src/lib/Makefile.am @@ -6,7 +6,8 @@ libcore_a_SOURCES = \ work.c \ notifier.c \ trace.c \
- schedule.c
schedule.c \
interrupt.c
libcore_a_CFLAGS = \ $(ARCH_CFLAGS) \
diff --git a/src/lib/interrupt.c b/src/lib/interrupt.c new file mode 100644 index 0000000..a7f8c9f --- /dev/null +++ b/src/lib/interrupt.c @@ -0,0 +1,210 @@ +/*
- Copyright (c) 2017, Intel Corporation
- All rights reserved.
- Redistribution and use in source and binary forms, with or without
- modification, are permitted provided that the following conditions are met:
- Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
- Redistributions in binary form must reproduce the above copyright
notice, this list of conditions and the following disclaimer in the
documentation and/or other materials provided with the distribution.
- Neither the name of the Intel Corporation nor the
names of its contributors may be used to endorse or promote products
derived from this software without specific prior written permission.
- THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- POSSIBILITY OF SUCH DAMAGE.
- Author: Keyon Jie yang.jie@linux.intel.com
- */
+#include <reef/interrupt.h> +#include <reef/interrupt-map.h> +#include <reef/alloc.h> +#include <arch/interrupt.h> +#include <platform/interrupt.h> +#include <stdint.h> +#include <stdlib.h>
+int irq_l2_register(int irq, struct l2root_irq *l2_int,
- void (*handler)(void *arg), void *arg)
Please comment who calls this and why.
OK, will add in next version.
+{
- int ret = 0;
- struct l2src_irq *l2_src = NULL;
- if (l2_int == NULL)
return -1;-EINVAL
OK, will fix.
- /* Todo: may need to check if this bit id has already been registered */
- l2_src = rzalloc(RZONE_SYS, RFLAGS_NONE, sizeof(struct l2src_irq));
- list_init(&l2_src->list);
- l2_src->bit_id = REEF_IRQ_BIT(irq);
- l2_src->enabled = 0;
- l2_src->handler = handler;
- l2_src->handler_arg = arg;
- if (l2_int->src_count == 0) {
list_init(&l2_int->src_list);ret = arch_interrupt_register(l2_int->num,l2_int->handler, l2_int);error handling needed when ret is < 0
We will return this ret and let caller handle it. At the same time, arch_interrupt_register() won't fail atm.
- }
- /* add to registered list */
- list_item_append(&l2_src->list, &l2_int->src_list);
locking ?
OK, let me add this.
- l2_int->src_count += 1;
++
- return ret;
+}
comment describing new api call
same with register one, will describe there.
+void irq_l2_unregister(int irq, struct l2root_irq *l2_int) +{
- struct l2src_irq *l2_src = NULL;
- struct list_item *slist, *tmp;
- /* looking for the bit id l2 src */
locking ?
OK.
- list_for_item_safe(slist, tmp, &l2_int->src_list) {
l2_src = container_of(slist, struct l2src_irq, list);if (l2_src->bit_id != REEF_IRQ_BIT(irq))continue;/* delete from lists */list_item_del(&l2_src->list);l2_int->src_count -= 1;--
/* free the l2 src which unregistered */if (l2_src->handler)l2_src->handler = NULL;rfree(l2_src);- }
comment
OK.
- if (l2_int->src_count == 0)
arch_interrupt_unregister(l2_int->num);+}
+uint32_t irq_l2_enable(int irq, struct l2root_irq *l2_int)
irq_unmask() ? return void
It will calculate the src masked count and unmask the root if needed.
+{
- struct l2src_irq *l2_src = NULL;
- struct list_item *slist;
- /* looking for the bit id l2 src */
locking.
OK.
- list_for_item(slist, &l2_int->src_list) {
l2_src = container_of(slist, struct l2src_irq, list);if (l2_src->bit_id != REEF_IRQ_BIT(irq))continue;if (!l2_src->enabled) {/* enable the root interrupt */best to name l1, l2 etc
OK, let me change them to l1_int and l2_src.
if (l2_int->enabled_count == 0)arch_interrupt_enable_mask(1 <<REEF_IRQ_NUMBER(irq));/* enable the src interrupt */if (plf_irq_ops && plf_irq_ops->int_unmask)plf_irq_ops->int_unmask(irq);l2_src->enabled = 1;l2_int->enabled_count += 1;++
}- }
- return 0;
+}
+uint32_t irq_l2_disable(int irq, struct l2root_irq *l2_int) +{
mask() and return void
Returning uint32_t is aligned with arch_interrupt_global_disable().
- struct l2src_irq *l2_src = NULL;
- struct list_item *slist;
- /* looking for the bit id l2 src */
- list_for_item(slist, &l2_int->src_list) {
l2_src = container_of(slist, struct l2src_irq, list);if (l2_src->bit_id != REEF_IRQ_BIT(irq))continue;if (l2_src->enabled) {/* disable the src interrupt */if (plf_irq_ops && plf_irq_ops->int_mask)plf_irq_ops->int_mask(irq);l2_src->enabled = 0;l2_int->enabled_count -= 1;/* disable the root interrupt */if (l2_int->enabled_count == 0)arch_interrupt_disable_mask(1 <<REEF_IRQ_NUMBER(irq));}- }
- return 0;
+}
+int interrupt_register(int irq,
- void (*handler)(void *arg), void *arg)
+{
- if (plf_irq_ops && plf_irq_ops->int_register)
return plf_irq_ops->int_register(irq, handler, arg);- else
return arch_interrupt_register(irq, handler, arg);this if() will be known at build time. same for the rest
even for byt we can use this ops or not. it should be known at build time, but using macros (e.g. CONFIG_BAYTRAIL) looks not good and checking it at run time may be more accurate, and, these functions won't be called frequently.
Thanks, ~Keyon
+}
+void interrupt_unregister(int irq) +{
- if (plf_irq_ops && plf_irq_ops->int_unregister)
plf_irq_ops->int_unregister(irq);- else
arch_interrupt_unregister(irq);+}
+uint32_t interrupt_enable(uint32_t irq) +{
- if (plf_irq_ops && plf_irq_ops->int_enable)
return plf_irq_ops->int_enable(irq);- else
return arch_interrupt_enable_mask(1 << irq);+}
+uint32_t interrupt_disable(uint32_t irq) +{
- if (plf_irq_ops && plf_irq_ops->int_disable)
return plf_irq_ops->int_disable(irq);- else
return arch_interrupt_disable_mask(1 << irq);+}
+/* set/clear */ +void interrupt_set(int irq) +{
- /* don't support set/clear l2 src irq */
- return arch_interrupt_set(REEF_IRQ_NUMBER(irq));
+}
+void interrupt_clear(int irq) +{
- /* don't support set/clear l2 src irq */
- arch_interrupt_clear(REEF_IRQ_NUMBER(irq));
+}
+uint32_t interrupt_global_disable(void) +{
- return arch_interrupt_global_disable();
+}
+void interrupt_global_enable(uint32_t flags) +{
- arch_interrupt_global_enable(flags);
+}
+const struct plf_irq_ops *plf_irq_ops;